[PATCH] build: Disabled MEDIA_TUNER_TDA18250 for Kernels older than 4.3
From: Jasmin Jessich The previouse patch did enable the TDA18250 driver for Kernel 4.2. Corrected this now. Signed-off-by: Jasmin Jessich --- v4l/versions.txt | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/v4l/versions.txt b/v4l/versions.txt index 6885a05..ed1e056 100644 --- a/v4l/versions.txt +++ b/v4l/versions.txt @@ -32,11 +32,13 @@ VIDEO_OV13858 # needs gpiochip_get_data VIDEO_SOLO6X10 +[4.3.0] +# needs regmap_write_bits +MEDIA_TUNER_TDA18250 + [4.2.0] # needs led_trigger_remove V4L2_FLASH_LED_CLASS -# needs regmap_write_bits -MEDIA_TUNER_TDA18250 [4.0.0] # needs of_property_read_u64_array -- 2.7.4
Re: [PATCH v4 15/18] scripts: kernel-doc: handle nested struct function arguments
Hi Mauro,
I love your patch! Perhaps something to improve:
[auto build test WARNING on lwn/docs-next]
[also build test WARNING on v4.15-rc4 next-20171221]
[cannot apply to linus/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/kernel-doc-add-supported-to-document-nested-structs/20171221-055609
base: git://git.lwn.net/linux-2.6 docs-next
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1072.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1072.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1072.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1072.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1072.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1072.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1072.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1072.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1072.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <--
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Thu Dec 21 05:00:16 CET 2017 media-tree git hash:ae49432810c5cca2143afc1445edad6582c9f270 media_build git hash: fd6010ac1bcdf54a3a2b2752088def1b21b5e457 v4l-utils git hash: 6049ea8bd64f9d78ef87ef0c2b3dc9b5de1ca4a1 gcc version:i686-linux-gcc (GCC) 7.1.0 sparse version: v0.5.0-3911-g6f737e1f smatch version: v0.5.0-3911-g6f737e1f host hardware: x86_64 host os:4.13.0-164 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: WARNINGS linux-2.6.37.6-i686: WARNINGS linux-2.6.38.8-i686: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-3.0.60-i686: WARNINGS linux-3.1.10-i686: WARNINGS linux-3.2.37-i686: WARNINGS linux-3.3.8-i686: WARNINGS linux-3.4.27-i686: WARNINGS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-3.10.1-i686: ERRORS linux-3.11.1-i686: ERRORS linux-3.12.67-i686: ERRORS linux-3.13.11-i686: WARNINGS linux-3.14.9-i686: WARNINGS linux-3.15.2-i686: WARNINGS linux-3.16.7-i686: WARNINGS linux-3.17.8-i686: WARNINGS linux-3.18.7-i686: WARNINGS linux-3.19-i686: WARNINGS linux-4.0.9-i686: WARNINGS linux-4.1.33-i686: WARNINGS linux-4.2.8-i686: ERRORS linux-4.3.6-i686: WARNINGS linux-4.4.22-i686: OK linux-4.5.7-i686: WARNINGS linux-4.6.7-i686: OK linux-4.7.5-i686: OK linux-4.8-i686: OK linux-4.9.26-i686: OK linux-4.10.14-i686: OK linux-4.11-i686: OK linux-4.12.1-i686: OK linux-4.13-i686: OK linux-4.14-i686: OK linux-2.6.36.4-x86_64: WARNINGS linux-2.6.37.6-x86_64: WARNINGS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.60-x86_64: WARNINGS linux-3.1.10-x86_64: WARNINGS linux-3.2.37-x86_64: WARNINGS linux-3.3.8-x86_64: WARNINGS linux-3.4.27-x86_64: WARNINGS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12.67-x86_64: ERRORS linux-3.13.11-x86_64: WARNINGS linux-3.14.9-x86_64: WARNINGS linux-3.15.2-x86_64: WARNINGS linux-3.16.7-x86_64: WARNINGS linux-3.17.8-x86_64: WARNINGS linux-3.18.7-x86_64: WARNINGS linux-3.19-x86_64: WARNINGS linux-4.0.9-x86_64: WARNINGS linux-4.1.33-x86_64: WARNINGS linux-4.2.8-x86_64: ERRORS linux-4.3.6-x86_64: WARNINGS linux-4.4.22-x86_64: WARNINGS linux-4.5.7-x86_64: WARNINGS linux-4.6.7-x86_64: WARNINGS linux-4.7.5-x86_64: WARNINGS linux-4.8-x86_64: WARNINGS linux-4.9.26-x86_64: WARNINGS linux-4.10.14-x86_64: WARNINGS linux-4.11-x86_64: WARNINGS linux-4.12.1-x86_64: WARNINGS linux-4.13-x86_64: OK linux-4.14-x86_64: OK apps: OK spec-git: OK smatch: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v4 14/18] scripts: kernel-doc: print the declaration name on warnings
Hi Mauro,
I love your patch! Perhaps something to improve:
[auto build test WARNING on lwn/docs-next]
[also build test WARNING on v4.15-rc4 next-20171220]
[cannot apply to linus/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/kernel-doc-add-supported-to-document-nested-structs/20171221-055609
base: git://git.lwn.net/linux-2.6 docs-next
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1058.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1058.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1058.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1058.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1058.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1058.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1058.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1058.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1058.
Unescaped left brace in regex is deprecated here (and will be fatal in Perl
5.30), passed through in regex; marked by <-- HERE in
m/(struct|union)([^{};]+){ <-- HERE ([^{}]*)}([^{}\;]*)\;/ at
scripts/kernel-doc line 1036.
Re: [PATCH v2 2/3] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)
Hi,
On Tue, 19 Dec 2017 13:53:28 +0200
Sakari Ailus wrote:
> Hi Yong,
>
> On Thu, Jul 27, 2017 at 01:01:36PM +0800, Yong Deng wrote:
> > Add binding documentation for Allwinner V3s CSI.
> >
> > Signed-off-by: Yong Deng
>
> DT bindings should precede the driver.
OK.
>
> > ---
> > .../devicetree/bindings/media/sun6i-csi.txt| 49
> > ++
> > 1 file changed, 49 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt
> > b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> > new file mode 100644
> > index 000..f8d83f6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> > @@ -0,0 +1,49 @@
> > +Allwinner V3s Camera Sensor Interface
> > +--
> > +
> > +Required properties:
> > + - compatible: value must be "allwinner,sun8i-v3s-csi"
>
> What are sun6i and sun8i? Is this device first present in sun6i SoCs,
> whereas you have only defined bindings for sun8i?
Yes, some sun6i SoCs has the almost same CSI module.
There is only V3s on my hand. So, I only tested it on V3s. But
some people work on the others.
>
> > + - reg: base address and size of the memory-mapped region.
> > + - interrupts: interrupt associated to this IP
> > + - clocks: phandles to the clocks feeding the CSI
> > +* ahb: the CSI interface clock
> > +* mod: the CSI module clock
> > +* ram: the CSI DRAM clock
> > + - clock-names: the clock names mentioned above
> > + - resets: phandles to the reset line driving the CSI
> > +
> > +- ports: A ports node with endpoint definitions as defined in
> > + Documentation/devicetree/bindings/media/video-interfaces.txt.
>
> Please document mandatory and optional endpoint properties relevant for the
> hardware.
I have added below commit in my v3:
Currently, the driver only support the parallel interface. So, a single port
node with one endpoint and parallel bus is supported.
>
> > +
> > +Example:
> > +
> > + csi1: csi@01cb4000 {
> > + compatible = "allwinner,sun8i-v3s-csi";
> > + reg = <0x01cb4000 0x1000>;
> > + interrupts = ;
> > + clocks = <&ccu CLK_BUS_CSI>,
> > +<&ccu CLK_CSI1_SCLK>,
> > +<&ccu CLK_DRAM_CSI>;
> > + clock-names = "ahb", "mod", "ram";
> > + resets = <&ccu RST_BUS_CSI>;
> > +
> > + port {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + /* Parallel bus endpoint */
> > + csi1_ep: endpoint {
> > + remote-endpoint = <&adv7611_ep>;
> > + bus-width = <16>;
> > + data-shift = <0>;
> > +
> > + /* If hsync-active/vsync-active are missing,
> > + embedded BT.656 sync is used */
> > + hsync-active = <0>; /* Active low */
> > + vsync-active = <0>; /* Active low */
> > + data-active = <1>; /* Active high */
> > + pclk-sample = <1>; /* Rising */
> > + };
> > + };
> > + };
> > +
>
> --
> Kind regards,
>
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi
Thanks,
Yong
Re: [PATCH v2 3/3] media: MAINTAINERS: add entries for Allwinner V3s CSI
On Tue, 19 Dec 2017 13:48:03 +0200 Sakari Ailus wrote: > On Thu, Jul 27, 2017 at 01:01:37PM +0800, Yong Deng wrote: > > Signed-off-by: Yong Deng > > --- > > MAINTAINERS | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 9826a91..b91fa27 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -3686,6 +3686,14 @@ M: Jaya Kumar > > S: Maintained > > F: sound/pci/cs5535audio/ > > > > +CSI DRIVERS FOR ALLWINNER V3s > > +M: Yong Deng > > +L: linux-media@vger.kernel.org > > +T: git git://linuxtv.org/media_tree.git > > +S: Maintained > > +F: drivers/media/platform/sun6i-csi/ > > +F: Documentation/devicetree/bindings/media/sun6i-csi.txt > > + > > CW1200 WLAN driver > > M: Solomon Peachy > > S: Maintained > > Please squash to the driver patch. Thanks. OK. > > -- > Sakari Ailus > e-mail: sakari.ai...@iki.fi Thanks, Yong
Re: [linux-sunxi] [PATCH v3 1/3] media: V3s: Add support for Allwinner CSI.
Hi,
On Tue, 19 Dec 2017 18:35:49 +0800
Chen-Yu Tsai wrote:
> On Mon, Nov 13, 2017 at 3:30 PM, Yong Deng wrote:
> > Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
> > and CSI1 is used for parallel interface. This is not documented in
> > datasheet but by testing and guess.
> >
> > This patch implement a v4l2 framework driver for it.
> >
> > Currently, the driver only support the parallel interface. MIPI-CSI2,
> > ISP's support are not included in this patch.
> >
> > Signed-off-by: Yong Deng
> > ---
> > drivers/media/platform/Kconfig | 1 +
> > drivers/media/platform/Makefile| 2 +
> > drivers/media/platform/sunxi/sun6i-csi/Kconfig | 9 +
> > drivers/media/platform/sunxi/sun6i-csi/Makefile| 3 +
> > drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 918
> > +
> > drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 146
> > .../media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h | 203 +
> > .../media/platform/sunxi/sun6i-csi/sun6i_video.c | 722
> > .../media/platform/sunxi/sun6i-csi/sun6i_video.h | 61 ++
> > 9 files changed, 2065 insertions(+)
> > create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Kconfig
> > create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Makefile
> > create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> > create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h
> > create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> > create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h
> >
>
> [...]
>
> > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> > new file mode 100644
> > index 000..0cebcbd
> > --- /dev/null
> > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
>
> [...]
>
> > +/*
> > -
> > + * Media Operations
> > + */
> > +static int sun6i_video_formats_init(struct sun6i_video *video)
> > +{
> > + struct v4l2_subdev_mbus_code_enum mbus_code = { 0 };
> > + struct sun6i_csi *csi = video->csi;
> > + struct v4l2_format format;
> > + struct v4l2_subdev *subdev;
> > + u32 pad;
> > + const u32 *pixformats;
> > + int pixformat_count = 0;
> > + u32 subdev_codes[32]; /* subdev format codes, 32 should be enough */
> > + int codes_count = 0;
> > + int num_fmts = 0;
> > + int i, j;
> > +
> > + subdev = sun6i_video_remote_subdev(video, &pad);
> > + if (subdev == NULL)
> > + return -ENXIO;
> > +
> > + /* Get supported pixformats of CSI */
> > + pixformat_count = sun6i_csi_get_supported_pixformats(csi,
> > &pixformats);
> > + if (pixformat_count <= 0)
> > + return -ENXIO;
> > +
> > + /* Get subdev formats codes */
> > + mbus_code.pad = pad;
> > + mbus_code.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > + while (!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL,
> > +&mbus_code)) {
> > + if (codes_count >= ARRAY_SIZE(subdev_codes)) {
> > + dev_warn(video->csi->dev,
> > +"subdev_codes array is full!\n");
> > + break;
> > + }
> > + subdev_codes[codes_count] = mbus_code.code;
> > + codes_count++;
> > + mbus_code.index++;
> > + }
> > +
> > + if (!codes_count)
> > + return -ENXIO;
> > +
> > + /* Get supported formats count */
> > + for (i = 0; i < codes_count; i++) {
> > + for (j = 0; j < pixformat_count; j++) {
> > + if (!sun6i_csi_is_format_support(csi, pixformats[j],
> > + mbus_code.code)) {
>
> Bug here. You are testing against mbus_code.code, which would be whatever
> was leftover from the previous section. Instead you should be testing
> against subdev_codes[i], the list you just built.
>
> Without it, it won't get past this part of the code if the last enumerated
> media bus format isn't supported.
>
> > + continue;
> > + }
> > + num_fmts++;
> > + }
> > + }
> > +
> > + if (!num_fmts)
> > + return -ENXIO;
> > +
> > + video->num_formats = num_fmts;
> > + video->formats = devm_kcalloc(video->csi->dev, num_fmts,
> > + sizeof(struct sun6i_csi_format), GFP_KERNEL);
> > + if (!video->formats)
> > + return -ENOMEM;
> > +
> > + /* Get supported formats */
> > + num_fmts = 0;
> > + for (i = 0; i < codes_count; i++) {
> > +
Re: [PATCH v3 1/6] media: rc: update sunxi-ir driver to get base clock frequency from devicetree
Hi Philipp, On Tue, Dec 19, 2017 at 09:07:42AM +0100, Philipp Rossak wrote: > This patch updates the sunxi-ir driver to set the base clock frequency from > devicetree. > > This is necessary since there are different ir receivers on the > market, that operate with different frequencies. So this value could be > set if the attached ir receiver needs a different base clock frequency, > than the default 8 MHz. > > Signed-off-by: Philipp Rossak feel free to add Reviewed-by: Andi Shyti Andi
Re: [-next PATCH 3/4] treewide: Use DEVICE_ATTR_RO
On Tue, 2017-12-19 at 10:15 -0800, Joe Perches wrote:
> Convert DEVICE_ATTR uses to DEVICE_ATTR_RO where possible.
>
> Done with perl script:
>
> $ git grep -w --name-only DEVICE_ATTR | \
> xargs perl -i -e 'local $/; while (<>) {
> s/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?(?:\s*S_IRUGO\s*|\s*0444\s*)\)?
> \s*,\s*\1_show\s*,\s*NULL\s*\)/DEVICE_ATTR_RO(\1)/g; print;}'
>
> Signed-off-by: Joe Perches
> ---
> arch/arm/mach-pxa/sharpsl_pm.c | 4 ++--
> arch/sh/drivers/push-switch.c| 2 +-
> arch/tile/kernel/sysfs.c | 10 +-
> drivers/acpi/device_sysfs.c | 6 +++---
> drivers/char/ipmi/ipmi_msghandler.c | 17
> -
> drivers/gpu/drm/i915/i915_sysfs.c| 6 +++---
> drivers/nvme/host/core.c | 10 +-
> drivers/s390/cio/css.c | 8
> drivers/s390/cio/device.c| 8
> drivers/s390/crypto/ap_card.c| 2 +-
> drivers/scsi/hpsa.c | 10 +-
> drivers/scsi/lpfc/lpfc_attr.c| 18
> --
> drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c | 8
> drivers/thermal/thermal_sysfs.c | 6 +++---
For the thermal part,
ACK-by: Zhang Rui
thanks,
rui
> sound/soc/soc-core.c | 2 +-
> sound/soc/soc-dapm.c | 2 +-
> 16 files changed, 58 insertions(+), 61 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/sharpsl_pm.c b/arch/arm/mach-
> pxa/sharpsl_pm.c
> index 398ba9ba2632..ef9fd9b759cb 100644
> --- a/arch/arm/mach-pxa/sharpsl_pm.c
> +++ b/arch/arm/mach-pxa/sharpsl_pm.c
> @@ -802,8 +802,8 @@ static ssize_t battery_voltage_show(struct device
> *dev, struct device_attribute
> return sprintf(buf, "%d\n",
> sharpsl_pm.battstat.mainbat_voltage);
> }
>
> -static DEVICE_ATTR(battery_percentage, 0444,
> battery_percentage_show, NULL);
> -static DEVICE_ATTR(battery_voltage, 0444, battery_voltage_show,
> NULL);
> +static DEVICE_ATTR_RO(battery_percentage);
> +static DEVICE_ATTR_RO(battery_voltage);
>
> extern void (*apm_get_power_status)(struct apm_power_info *);
>
> diff --git a/arch/sh/drivers/push-switch.c b/arch/sh/drivers/push-
> switch.c
> index a17181160233..762bc5619910 100644
> --- a/arch/sh/drivers/push-switch.c
> +++ b/arch/sh/drivers/push-switch.c
> @@ -24,7 +24,7 @@ static ssize_t switch_show(struct device *dev,
> struct push_switch_platform_info *psw_info = dev-
> >platform_data;
> return sprintf(buf, "%s\n", psw_info->name);
> }
> -static DEVICE_ATTR(switch, S_IRUGO, switch_show, NULL);
> +static DEVICE_ATTR_RO(switch);
>
> static void switch_timer(struct timer_list *t)
> {
> diff --git a/arch/tile/kernel/sysfs.c b/arch/tile/kernel/sysfs.c
> index af5024f0fb5a..b09456a3d77a 100644
> --- a/arch/tile/kernel/sysfs.c
> +++ b/arch/tile/kernel/sysfs.c
> @@ -38,7 +38,7 @@ static ssize_t chip_width_show(struct device *dev,
> {
> return sprintf(page, "%u\n", smp_width);
> }
> -static DEVICE_ATTR(chip_width, 0444, chip_width_show, NULL);
> +static DEVICE_ATTR_RO(chip_width);
>
> static ssize_t chip_height_show(struct device *dev,
> struct device_attribute *attr,
> @@ -46,7 +46,7 @@ static ssize_t chip_height_show(struct device *dev,
> {
> return sprintf(page, "%u\n", smp_height);
> }
> -static DEVICE_ATTR(chip_height, 0444, chip_height_show, NULL);
> +static DEVICE_ATTR_RO(chip_height);
>
> static ssize_t chip_serial_show(struct device *dev,
> struct device_attribute *attr,
> @@ -54,7 +54,7 @@ static ssize_t chip_serial_show(struct device *dev,
> {
> return get_hv_confstr(page, HV_CONFSTR_CHIP_SERIAL_NUM);
> }
> -static DEVICE_ATTR(chip_serial, 0444, chip_serial_show, NULL);
> +static DEVICE_ATTR_RO(chip_serial);
>
> static ssize_t chip_revision_show(struct device *dev,
> struct device_attribute *attr,
> @@ -62,7 +62,7 @@ static ssize_t chip_revision_show(struct device
> *dev,
> {
> return get_hv_confstr(page, HV_CONFSTR_CHIP_REV);
> }
> -static DEVICE_ATTR(chip_revision, 0444, chip_revision_show, NULL);
> +static DEVICE_ATTR_RO(chip_revision);
>
>
> static ssize_t type_show(struct device *dev,
> @@ -71,7 +71,7 @@ static ssize_t type_show(struct device *dev,
> {
> return sprintf(page, "tilera\n");
> }
> -static DEVICE_ATTR(type, 0444, type_show, NULL);
> +static DEVICE_ATTR_RO(type);
>
> #define HV_CONF_ATTR(name, conf)
> \
> static ssize_t name ## _show(struct device *dev,
> \
> diff --git a/drivers/acpi/device_sysfs.c
> b/drivers/acpi/device_sysfs.c
> index a041689e5701..545e91420cde 100644
> --- a/drivers/acpi/device_sysfs.c
> +++ b/
Re: [PATCH v9 18/28] rcar-vin: break out format alignment and checking
Hi Laurent,
Thanks for your comments.
On 2017-12-08 12:01:08 +0200, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> On Friday, 8 December 2017 03:08:32 EET Niklas Söderlund wrote:
> > Part of the format alignment and checking can be shared with the Gen3
> > format handling. Break that part out to its own function. While doing
> > this clean up the checking and add more checks.
>
> I'd split that in two patches, they are unrelated and should be reviewed
> separately.
Good point, I will do so. In fact I will split this in to three patches.
- Move things to a separate new function
- Deal with the update bytesperline and sizeimage calculation
- Add the new check on pixelformat
>
> > Signed-off-by: Niklas Söderlund
> > Reviewed-by: Hans Verkuil
> > ---
> > drivers/media/platform/rcar-vin/rcar-v4l2.c | 98 +
> > 1 file changed, 51 insertions(+), 47 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> > 56c5183f55922e1d..0ffbf0c16fb7b00e 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > @@ -86,6 +86,56 @@ static u32 rvin_format_sizeimage(struct v4l2_pix_format
> > *pix) return pix->bytesperline * pix->height;
> > }
> >
> > +static int rvin_format_align(struct rvin_dev *vin, struct v4l2_pix_format
> > *pix)
> > +{
> > + u32 walign;
> > +
> > + /* If requested format is not supported fallback to the default */
> > + if (!rvin_format_from_pixel(pix->pixelformat)) {
> > + vin_dbg(vin, "Format 0x%x not found, using default 0x%x\n",
> > + pix->pixelformat, RVIN_DEFAULT_FORMAT);
> > + pix->pixelformat = RVIN_DEFAULT_FORMAT;
> > + }
> > +
> > + switch (pix->field) {
> > + case V4L2_FIELD_TOP:
> > + case V4L2_FIELD_BOTTOM:
> > + case V4L2_FIELD_NONE:
> > + case V4L2_FIELD_INTERLACED_TB:
> > + case V4L2_FIELD_INTERLACED_BT:
> > + case V4L2_FIELD_INTERLACED:
> > + break;
> > + default:
> > + pix->field = V4L2_FIELD_NONE;
> > + break;
> > + }
> > +
> > + /* Check that colorspace is reasonable, if not keep current */
> > + if (!pix->colorspace || pix->colorspace >= 0xff)
>
> Where does 0xff come from ? It seems a bit random to me.
It comes from v4l2-compliance source code, is that not how we are
suppose to pass a compliance test? :-) Exact location is
testColorspace() in utils/v4l2-compliance/v4l2-test-formats.cpp.
The only other way I can think of is to list all formats from enum
v4l2_colorspace in a swtich statement, but then if a new colorspace is
added this will fall behind. Other drivers just check for
!pix->colorspace but that is not enough to pass the test.
On Gen2 this is not an issue as as you describe bellow the colorspace is
always set to that of the source. But on Gen3 it's up to the user to set
to colorspace if the default one is not right. Then this check is needed
to not freak out v4l2-compliance.
I will keep this for now, and if I can think of something better I will
switch to that. But I need to think more on this, in the mean time break
it out to a separate patch.
>
> > + pix->colorspace = vin->format.colorspace;
>
> I don't think that's a good idea. You should pick a default if the colorspace
> can't be supported. Beside, what's the point in accepting colorspaces if
> they're not handled by the driver ? Why don't you just set a fixed value
> based
> on the colorspace reported by the source ?
Yes here I agree, it should set it to a default colorspace.
>
> > + /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */
> > + walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
> > +
> > + /* Limit to VIN capabilities */
> > + v4l_bound_align_image(&pix->width, 2, vin->info->max_width, walign,
> > + &pix->height, 4, vin->info->max_height, 2, 0);
> > +
> > + pix->bytesperline = rvin_format_bytesperline(pix);
> > + pix->sizeimage = rvin_format_sizeimage(pix);
>
> You're now hardcoding those values instead of only enforcing a minimum. Why
> is
> that ?
Hum I don't understand this comment I think. I checked other drivers and
this looks like how most of them are calculating this (vimc, xilinx,
etc). This change is mostly to get rid of things that never should have
made it out of soc_camera. It worked for Gen2 as bytesperline and
sizeimage where updated when querying the sensor. While on Gen3 this
will break as there is no sensor to update this and it is therefore
impossible to reduce these values.
>
> > +
> > + if (vin->info->chip == RCAR_M1 &&
> > + pix->pixelformat == V4L2_PIX_FMT_XBGR32) {
> > + vin_err(vin, "pixel format XBGR32 not supported on M1\n");
> > + return -EINVAL;
> > + }
>
> You should move this with the other format check at the beginning of the
> function. a
Re: [PATCH v2 2/2] media: coda: Add i.MX51 (CodaHx4) support
Hi Philipp,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.15-rc4 next-20171220]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Philipp-Zabel/media-dt-bindings-coda-Add-compatible-for-CodaHx4-on-i-MX51/20171221-050217
base: git://linuxtv.org/media_tree.git master
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/media/platform/coda/coda-bit.c: In function 'coda_setup_iram':
>> drivers/media/platform/coda/coda-bit.c:648:28: warning: 'me_bits' may be
>> used uninitialized in this function [-Wmaybe-uninitialized]
iram_info->axi_sram_use |= me_bits;
^~
vim +/me_bits +648 drivers/media/platform/coda/coda-bit.c
588
589 static void coda_setup_iram(struct coda_ctx *ctx)
590 {
591 struct coda_iram_info *iram_info = &ctx->iram_info;
592 struct coda_dev *dev = ctx->dev;
593 int w64, w128;
594 int mb_width;
595 int dbk_bits;
596 int bit_bits;
597 int ip_bits;
598 int me_bits;
599
600 memset(iram_info, 0, sizeof(*iram_info));
601 iram_info->next_paddr = dev->iram.paddr;
602 iram_info->remaining = dev->iram.size;
603
604 if (!dev->iram.vaddr)
605 return;
606
607 switch (dev->devtype->product) {
608 case CODA_HX4:
609 dbk_bits = CODA7_USE_HOST_DBK_ENABLE;
610 bit_bits = CODA7_USE_HOST_BIT_ENABLE;
611 ip_bits = CODA7_USE_HOST_IP_ENABLE;
612 me_bits = CODA7_USE_HOST_ME_ENABLE;
613 break;
614 case CODA_7541:
615 dbk_bits = CODA7_USE_HOST_DBK_ENABLE |
CODA7_USE_DBK_ENABLE;
616 bit_bits = CODA7_USE_HOST_BIT_ENABLE |
CODA7_USE_BIT_ENABLE;
617 ip_bits = CODA7_USE_HOST_IP_ENABLE |
CODA7_USE_IP_ENABLE;
618 me_bits = CODA7_USE_HOST_ME_ENABLE |
CODA7_USE_ME_ENABLE;
619 break;
620 case CODA_960:
621 dbk_bits = CODA9_USE_HOST_DBK_ENABLE |
CODA9_USE_DBK_ENABLE;
622 bit_bits = CODA9_USE_HOST_BIT_ENABLE |
CODA7_USE_BIT_ENABLE;
623 ip_bits = CODA9_USE_HOST_IP_ENABLE |
CODA7_USE_IP_ENABLE;
624 break;
625 default: /* CODA_DX6 */
626 return;
627 }
628
629 if (ctx->inst_type == CODA_INST_ENCODER) {
630 struct coda_q_data *q_data_src;
631
632 q_data_src = get_q_data(ctx,
V4L2_BUF_TYPE_VIDEO_OUTPUT);
633 mb_width = DIV_ROUND_UP(q_data_src->width, 16);
634 w128 = mb_width * 128;
635 w64 = mb_width * 64;
636
637 /* Prioritize in case IRAM is too small for everything
*/
638 if (dev->devtype->product == CODA_HX4 ||
639 dev->devtype->product == CODA_7541) {
640 iram_info->search_ram_size = round_up(mb_width
* 16 *
64136 +
2048, 1024);
642 iram_info->search_ram_paddr =
coda_iram_alloc(iram_info,
643
iram_info->search_ram_size);
644 if (!iram_info->search_ram_paddr) {
645 pr_err("IRAM is smaller than the search
ram size\n");
646 goto out;
647 }
> 648 iram_info->axi_sram_use |= me_bits;
649 }
650
651 /* Only H.264BP and H.263P3 are considered */
652 iram_info->buf_dbk_y_use = coda_iram_alloc(iram_info,
w64);
653 iram_info->buf_dbk_c_use = coda_iram_alloc(iram_info,
w64);
654 if (!iram_info->buf_dbk_c_use)
655 goto out;
656 iram_info->axi_sram_use |= dbk_bits;
657
658 iram_info->buf_bit_use = coda_iram_alloc(iram_info,
w128);
659 if (!iram_info->buf_bit_use)
660
Re: [PATCH v9 16/28] rcar-vin: add function to manipulate Gen3 chsel value
Hi Laurent,
Thanks for your comments.
On 2017-12-08 11:52:01 +0200, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> On Friday, 8 December 2017 03:08:30 EET Niklas Söderlund wrote:
> > On Gen3 the CSI-2 routing is controlled by the VnCSI_IFMD register. One
> > feature of this register is that it's only present in the VIN0 and VIN4
> > instances. The register in VIN0 controls the routing for VIN0-3 and the
> > register in VIN4 controls routing for VIN4-7.
> >
> > To be able to control routing from a media device this function is need
> > to control runtime PM for the subgroup master (VIN0 and VIN4). The
> > subgroup master must be switched on before the register is manipulated,
> > once the operation is complete it's safe to switch the master off and
> > the new routing will still be in effect.
> >
> > Signed-off-by: Niklas Söderlund
> > Reviewed-by: Hans Verkuil
> > ---
> > drivers/media/platform/rcar-vin/rcar-dma.c | 25 +
> > drivers/media/platform/rcar-vin/rcar-vin.h | 2 ++
> > 2 files changed, 27 insertions(+)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> > b/drivers/media/platform/rcar-vin/rcar-dma.c index
> > ace95d5b543a17e3..d2788d8bb9565aaa 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -16,6 +16,7 @@
> >
> > #include
> > #include
> > +#include
> >
> > #include
> >
> > @@ -1228,3 +1229,27 @@ int rvin_dma_register(struct rvin_dev *vin, int irq)
> >
> > return ret;
> > }
> > +
> > +/*
> > + * Gen3 CHSEL manipulation
> > + */
> > +
> > +void rvin_set_chsel(struct rvin_dev *vin, u8 chsel)
>
> How about naming the function a bit more explicitly,
> rvin_set_channel_routing() for instance ?
I agree, it's a much better name.
>
> > +{
> > + u32 ifmd, vnmc;
> > +
> > + pm_runtime_get_sync(vin->dev);
>
> Shouldn't you check the return value of this function ?
Sakari asked the same thing in v4 :-)
In short no its not needed please see Geert's response [1]. If I recall
correctly this was also discussed in more detail in another thread for
some other driver whit a bit longer answer saying that it
pm_runtime_get_sync() fails you have big problems but I can't find that
thread now :-(
1. https://www.spinics.net/lists/linux-media/msg115241.html
>
> > +
> > + /* Make register writes take effect immediately */
> > + vnmc = rvin_read(vin, VNMC_REG) & ~VNMC_VUP;
> > + rvin_write(vin, vnmc, VNMC_REG);
>
> Shouldn't you restore the original value of VNMC at the end of the function ?
> What if this races with device access local to the VIN0 or VIN4 instance ?
Media link changes are not allowed when any VIN in the group are
streaming so this should not be an issue. But I agree it's good form to
restore the value anyhow so I will update this for the next version.
>
> > + ifmd = VNCSI_IFMD_DES2 | VNCSI_IFMD_DES1 | VNCSI_IFMD_DES0 |
> > + VNCSI_IFMD_CSI_CHSEL(chsel);
> > +
> > + rvin_write(vin, ifmd, VNCSI_IFMD_REG);
> > +
> > + vin_dbg(vin, "Set IFMD 0x%x\n", ifmd);
> > +
> > + pm_runtime_put(vin->dev);
> > +}
> > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> > b/drivers/media/platform/rcar-vin/rcar-vin.h index
> > a440effe4b86af31..7819c760c2c13422 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> > @@ -163,4 +163,6 @@ void rvin_v4l2_unregister(struct rvin_dev *vin);
> >
> > const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat);
> >
> > +void rvin_set_chsel(struct rvin_dev *vin, u8 chsel);
> > +
> > #endif
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Regards,
Niklas Söderlund
Re: [PATCH v9 15/28] rcar-vin: enable Gen3 hardware configuration
Hi Laurent,
Thanks for your comments.
On 2017-12-08 11:47:13 +0200, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> On Friday, 8 December 2017 03:08:29 EET Niklas Söderlund wrote:
> > Add the register needed to work with Gen3 hardware. This patch adds
> > the logic for how to work with the Gen3 hardware. More work is required
> > to enable the subdevice structure needed to configure capturing.
> >
> > Signed-off-by: Niklas Söderlund
> > Reviewed-by: Hans Verkuil
> > ---
> > drivers/media/platform/rcar-vin/rcar-dma.c | 94 ++-
> > drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
> > 2 files changed, 64 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> > b/drivers/media/platform/rcar-vin/rcar-dma.c index
> > d7660f485a2df9e4..ace95d5b543a17e3 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -33,21 +33,23 @@
> > #define VNELPRC_REG0x10/* Video n End Line Pre-Clip Register */
> > #define VNSPPRC_REG0x14/* Video n Start Pixel Pre-Clip
> > Register */
> > #define VNEPPRC_REG0x18/* Video n End Pixel Pre-Clip Register
> > */
> > -#define VNSLPOC_REG0x1C/* Video n Start Line Post-Clip
> > Register */
> > -#define VNELPOC_REG0x20/* Video n End Line Post-Clip Register
> > */
> > -#define VNSPPOC_REG0x24/* Video n Start Pixel Post-Clip
> > Register */
> > -#define VNEPPOC_REG0x28/* Video n End Pixel Post-Clip Register
> > */
> > #define VNIS_REG 0x2C/* Video n Image Stride Register */
> > #define VNMB_REG(m)(0x30 + ((m) << 2)) /* Video n Memory Base m
> > Register
> > */ #define VNIE_REG 0x40/* Video n Interrupt Enable Register */
> > #define VNINTS_REG 0x44/* Video n Interrupt Status Register */
> > #define VNSI_REG 0x48/* Video n Scanline Interrupt Register */
> > #define VNMTC_REG 0x4C/* Video n Memory Transfer Control Register */
> > -#define VNYS_REG 0x50/* Video n Y Scale Register */
> > -#define VNXS_REG 0x54/* Video n X Scale Register */
> > #define VNDMR_REG 0x58/* Video n Data Mode Register */
> > #define VNDMR2_REG 0x5C/* Video n Data Mode Register 2 */
> > #define VNUVAOF_REG0x60/* Video n UV Address Offset Register */
> > +
> > +/* Register offsets specific for Gen2 */
> > +#define VNSLPOC_REG0x1C/* Video n Start Line Post-Clip
> > Register */
> > +#define VNELPOC_REG0x20/* Video n End Line Post-Clip Register
> > */
> > +#define VNSPPOC_REG0x24/* Video n Start Pixel Post-Clip
> > Register */
> > +#define VNEPPOC_REG0x28/* Video n End Pixel Post-Clip Register
> > */
> > +#define VNYS_REG 0x50/* Video n Y Scale Register */
> > +#define VNXS_REG 0x54/* Video n X Scale Register */
> > #define VNC1A_REG 0x80/* Video n Coefficient Set C1A Register */
> > #define VNC1B_REG 0x84/* Video n Coefficient Set C1B Register */
> > #define VNC1C_REG 0x88/* Video n Coefficient Set C1C Register */
> > @@ -73,9 +75,13 @@
> > #define VNC8B_REG 0xF4/* Video n Coefficient Set C8B Register */
> > #define VNC8C_REG 0xF8/* Video n Coefficient Set C8C Register */
> >
> > +/* Register offsets specific for Gen3 */
> > +#define VNCSI_IFMD_REG 0x20 /* Video n CSI2 Interface Mode
> > Register */
> >
> > /* Register bit fields for R-Car VIN */
> > /* Video n Main Control Register bits */
> > +#define VNMC_DPINE (1 << 27) /* Gen3 specific */
> > +#define VNMC_SCLE (1 << 26) /* Gen3 specific */
> > #define VNMC_FOC (1 << 21)
> > #define VNMC_YCAL (1 << 19)
> > #define VNMC_INF_YUV8_BT656(0 << 16)
> > @@ -119,6 +125,13 @@
> > #define VNDMR2_FTEV(1 << 17)
> > #define VNDMR2_VLV(n) ((n & 0xf) << 12)
> >
> > +/* Video n CSI2 Interface Mode Register (Gen3) */
> > +#define VNCSI_IFMD_DES2(1 << 27)
> > +#define VNCSI_IFMD_DES1(1 << 26)
> > +#define VNCSI_IFMD_DES0(1 << 25)
> > +#define VNCSI_IFMD_CSI_CHSEL(n) ((n & 0xf) << 0)
>
> *Always* enclose macro arguments in parentheses otherwise they are subject to
> side effects.
>
> #define VNCSI_IFMD_CSI_CHSEL(n) (((n) & 0xf) << 0)
Thanks for spotting this.
>
> > +#define VNCSI_IFMD_CSI_CHSEL_MASK 0xf
> > +
> > struct rvin_buffer {
> > struct vb2_v4l2_buffer vb;
> > struct list_head list;
> > @@ -514,28 +527,10 @@ static void rvin_set_coeff(struct rvin_dev *vin,
> > unsigned short xs) rvin_write(vin, p_set->coeff_set[23], VNC8C_REG);
> > }
> >
> > -static void rvin_crop_scale_comp(struct rvin_dev *vin)
> > +static void rvin_crop_scale_comp_gen2(struct rvin_dev *vin)
> > {
> > u32 xs, ys;
> >
> > - /* Set Start/End Pixel/Line Pre-Clip */
> > - rvin_write(vin, vin->crop.left, VNSPPRC_REG);
> > - rvin_write(vin, vin->
Re: [PATCH] v4l: rcar-csi2: Don't bail out from probe on no ep
Hi Laurent,
On 2017-12-20 20:06:13 +0200, Laurent Pinchart wrote:
> Hi Niklas,
>
> On Friday, 15 December 2017 16:23:10 EET Niklas Söderlund wrote:
> > On 2017-12-15 15:06:44 +0100, Hans Verkuil wrote:
> > > Niklas,
> > >
> > > Did you look at this? If I should take it, can you Ack it? If you are
> > > going to squash or add it to our of your own patch series, then let me
> > > know and I can remove it from my todo queue.
> >
> > I did look at it and talked to Jacobo about it. I think I have a better
> > solution to his problem which I hope to try out in the near future. If
> > my workaround turns out to not solve his problem I will squash this in
> > when I resend the rcar-csi2 patch-set so in any case I think you can
> > drop it from your todo queue.
> >
> > The reason I'm a bit reluctant to ack this straight away is that I think
> > it's kind of good that rcar-csi2 fails to probe if it finds no endpoints
> > to work with, there is little use for the driver instance then. The
> > problem Jacobo is trying to fix is related to how the rcar-vin Gen3
> > group parses DT and I made a small mistake there which was discovered by
> > Jacobo. And since the original fault is in the rcar-vin driver I think
> > the issue should be fixed in that driver.
>
> How do you plan to handle the case where only one CSI-2 receiver has a
> connected input ?
I'm sorry, I don't understand this question I think. If there only is
one CSI-2 receiver with connected endpoints, should that not be the only
hardware the driver handles? What would be the reason to enable a CSI-2
device in DT and not connect endpoints to it?
>
> > > On 05/12/17 21:41, Jacopo Mondi wrote:
> > >> When rcar-csi interface is not connected to any endpoint, it fails and
> > >> bails out from probe before registering its own video subdevice.
> > >> This prevents rcar-vin registered notifier from completing and no
> > >> subdevice is ever registered, also for other properly connected csi
> > >> interfaces.
> > >>
> > >> Fix this not returning an error when no endpoint is connected to a csi
> > >> interface and let the driver complete its probe function and register
> > >> its own video subdevice.
> > >>
> > >> ---
> > >> Niklas,
> > >>
> > >>please squash this patch in your next rcar-csi2 series (if you like
> > >>it ;)
> > >>
> > >> As we have discussed this is particularly useful for gmsl setup, where
> > >> adv748x is connected to CSI20 and max9286 to CSI40/CSI41. If we disable
> > >> adv748x from DTS we need CSI20 probe to complete anyhow otherwise no
> > >> subdevice gets registered for the two deserializers.
> > >>
> > >> Please note we cannot disable CSI20 entirely otherwise VIN's graph
> > >> parsing breaks.
> > >>
> > >> Thanks
> > >>
> > >>j
> > >>
> > >> ---
> > >>
> > >> drivers/media/platform/rcar-vin/rcar-csi2.c | 4 ++--
> > >> 1 file changed, 2 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > >> b/drivers/media/platform/rcar-vin/rcar-csi2.c index 2793efb..90c4062
> > >> 100644
> > >> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > >> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > >> @@ -928,8 +928,8 @@ static int rcar_csi2_parse_dt(struct rcar_csi2
> > >> *priv)
> > >>
> > >> ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
> > >> if (!ep) {
> > >> -dev_err(priv->dev, "Not connected to subdevice\n");
> > >> -return -EINVAL;
> > >> +dev_dbg(priv->dev, "Not connected to subdevice\n");
> > >> +return 0;
> > >> }
> > >>
> > >> ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep),
> > >> &v4l2_ep);
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Regards,
Niklas Söderlund
Re: [PATCH v5 4/4] ARM: dts: tegra20: Add video decoder node
On Tue, Dec 12, 2017 at 03:26:10AM +0300, Dmitry Osipenko wrote: > Add Video Decoder Engine device node. > > Signed-off-by: Dmitry Osipenko > --- > arch/arm/boot/dts/tegra20.dtsi | 27 +++ > 1 file changed, 27 insertions(+) Applied, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH v5 3/4] ARM: dts: tegra20: Add device tree node to describe IRAM
On Tue, Dec 12, 2017 at 03:26:09AM +0300, Dmitry Osipenko wrote: > From: Vladimir Zapolskiy > > All Tegra20 SoCs contain 256KiB IRAM, which is used to store > resume code and by a video decoder engine. > > Signed-off-by: Vladimir Zapolskiy > Signed-off-by: Dmitry Osipenko > --- > arch/arm/boot/dts/tegra20.dtsi | 8 > 1 file changed, 8 insertions(+) Applied, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH v3 2/6] media: dt: bindings: Update binding documentation for sunxi IR controller
On Tue, Dec 19, 2017 at 09:07:43AM +0100, Philipp Rossak wrote: > This patch updates documentation for Device-Tree bindings for sunxi IR > controller and adds the new optional property for the base clock > frequency. > > Signed-off-by: Philipp Rossak > --- > Documentation/devicetree/bindings/media/sunxi-ir.txt | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Rob Herring
Re: [PATCH v4 11/16] dt-bindings: Document the Rockchip MIPI RX D-PHY bindings
On Mon, Dec 18, 2017 at 08:14:40PM +0800, Jacob Chen wrote: > From: Jacob Chen > > Add DT bindings documentation for Rockchip MIPI D-PHY RX > > Signed-off-by: Jacob Chen > --- > .../bindings/media/rockchip-mipi-dphy.txt | 88 > ++ > 1 file changed, 88 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt Reviewed-by: Rob Herring
Re: [PATCH v4 10/16] dt-bindings: Document the Rockchip ISP1 bindings
On Mon, Dec 18, 2017 at 08:14:39PM +0800, Jacob Chen wrote: > From: Jacob Chen > > Add DT bindings documentation for Rockchip ISP1 > > Signed-off-by: Jacob Chen > --- > .../devicetree/bindings/media/rockchip-isp1.txt| 69 > ++ > 1 file changed, 69 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/rockchip-isp1.txt Reviewed-by: Rob Herring
Re: [PATCH] v4l: rcar-csi2: Don't bail out from probe on no ep
Hi Niklas,
On Friday, 15 December 2017 16:23:10 EET Niklas Söderlund wrote:
> On 2017-12-15 15:06:44 +0100, Hans Verkuil wrote:
> > Niklas,
> >
> > Did you look at this? If I should take it, can you Ack it? If you are
> > going to squash or add it to our of your own patch series, then let me
> > know and I can remove it from my todo queue.
>
> I did look at it and talked to Jacobo about it. I think I have a better
> solution to his problem which I hope to try out in the near future. If
> my workaround turns out to not solve his problem I will squash this in
> when I resend the rcar-csi2 patch-set so in any case I think you can
> drop it from your todo queue.
>
> The reason I'm a bit reluctant to ack this straight away is that I think
> it's kind of good that rcar-csi2 fails to probe if it finds no endpoints
> to work with, there is little use for the driver instance then. The
> problem Jacobo is trying to fix is related to how the rcar-vin Gen3
> group parses DT and I made a small mistake there which was discovered by
> Jacobo. And since the original fault is in the rcar-vin driver I think
> the issue should be fixed in that driver.
How do you plan to handle the case where only one CSI-2 receiver has a
connected input ?
> > On 05/12/17 21:41, Jacopo Mondi wrote:
> >> When rcar-csi interface is not connected to any endpoint, it fails and
> >> bails out from probe before registering its own video subdevice.
> >> This prevents rcar-vin registered notifier from completing and no
> >> subdevice is ever registered, also for other properly connected csi
> >> interfaces.
> >>
> >> Fix this not returning an error when no endpoint is connected to a csi
> >> interface and let the driver complete its probe function and register
> >> its own video subdevice.
> >>
> >> ---
> >> Niklas,
> >>
> >>please squash this patch in your next rcar-csi2 series (if you like
> >>it ;)
> >>
> >> As we have discussed this is particularly useful for gmsl setup, where
> >> adv748x is connected to CSI20 and max9286 to CSI40/CSI41. If we disable
> >> adv748x from DTS we need CSI20 probe to complete anyhow otherwise no
> >> subdevice gets registered for the two deserializers.
> >>
> >> Please note we cannot disable CSI20 entirely otherwise VIN's graph
> >> parsing breaks.
> >>
> >> Thanks
> >>
> >>j
> >>
> >> ---
> >>
> >> drivers/media/platform/rcar-vin/rcar-csi2.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> >> b/drivers/media/platform/rcar-vin/rcar-csi2.c index 2793efb..90c4062
> >> 100644
> >> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> >> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> >> @@ -928,8 +928,8 @@ static int rcar_csi2_parse_dt(struct rcar_csi2
> >> *priv)
> >>
> >>ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
> >>if (!ep) {
> >> - dev_err(priv->dev, "Not connected to subdevice\n");
> >> - return -EINVAL;
> >> + dev_dbg(priv->dev, "Not connected to subdevice\n");
> >> + return 0;
> >>}
> >>
> >>ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep);
--
Regards,
Laurent Pinchart
Re: [PATCH] devicetree: Add video bus switch
Hi Pavel,
On Saturday, 4 February 2017 23:56:10 EET Pavel Machek wrote:
> Hi!
>
> +Required properties
> +===
> +
> +compatible : must contain "video-bus-switch"
> >>>
> >>> How generic is this? Should we have e.g. nokia,video-bus-switch? And
> >>> if so, change the file name accordingly.
> >>
> >> Generic for "single GPIO controls the switch", AFAICT. But that should
> >> be common enough...
> >
> > Um, yes. Then... how about: video-bus-switch-gpio? No Nokia prefix.
>
> Ok, done. I also fixed the english a bit.
>
> +reg : The interface:
> + 0 - port for image signal processor
> + 1 - port for first camera sensor
> + 2 - port for second camera sensor
> >>>
> >>> I'd say this must be pretty much specific to the one in N900. You
> >>> could have more ports. Or you could say that ports beyond 0 are
> >>> camera sensors. I guess this is good enough for now though, it can be
> >>> changed later on with the source if a need arises.
> >>
> >> Well, I'd say that selecting between two sensors is going to be the
> >> common case. If someone needs more than two, it will no longer be
> >> simple GPIO, so we'll have some fixing to do.
> >
> > It could be two GPIOs --- that's how the GPIO I2C mux works.
> >
> > But I'd be surprised if someone ever uses something like that
> > again. ;-)
>
> I'd say.. lets handle that when we see hardware like that.
>
> >>> Btw. was it still considered a problem that the endpoint properties
> >>> for the sensors can be different? With the g_routing() pad op which is
> >>> to be added, the ISP driver (should actually go to a framework
> >>> somewhere) could parse the graph and find the proper endpoint there.
> >>
> >> I don't know about g_routing. I added g_endpoint_config method that
> >> passes the configuration, and that seems to work for me.
> >>
> >> I don't see g_routing in next-20170201 . Is there place to look?
> >
> > I think there was a patch by Laurent to LMML quite some time ago. I
> > suppose that set will be repicked soonish.
> >
> > I don't really object using g_endpoint_config() as a temporary solution;
> > I'd like to have Laurent's opinion on that though. Another option is to
> > wait, but we've already waited a looong time (as in total).
>
> Laurent, do you have some input here? We have simple "2 cameras
> connected to one signal processor" situation here. We need some way of
> passing endpoint configuration from the sensors through the switch. I
> did this:
Could you give me a bit more information about the platform you're targeting:
how the switch is connected, what kind of switch it is, and what endpoint
configuration data you need ?
> >> @@ -415,6 +416,8 @@ struct v4l2_subdev_video_ops {
> >> const struct v4l2_mbus_config *cfg);
> >> int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
> >>unsigned int *size);
> >> + int (*g_endpoint_config)(struct v4l2_subdev *sd,
> >> + struct v4l2_of_endpoint *cfg);
>
> Google of g_routing tells me:
>
> 9) Highly reconfigurable hardware - Julien Beraud
>
> - 44 sub-devices connected with an interconnect.
> - As long as formats match, any sub-device could be connected to any
> - other sub-device through a link.
> - The result is 44 * 44 links at worst.
> - A switch sub-device proposed as the solution to model the
> - interconnect. The sub-devices are connected to the switch
> - sub-devices through the hardware links that connect to the
> - interconnect.
> - The switch would be controlled through new IOCTLs S_ROUTING and
> - G_ROUTING.
> - Patches available:
> http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
>
> but the patches are from 2005. So I guess I'll need some guidance here...
You made me feel very old for a moment. The patches are from 2015 :-)
> > I'll reply to the other patch containing the code.
--
Regards,
Laurent Pinchart
Re: [PATCH v9 13/28] rcar-vin: fix handling of single field frames (top, bottom and alternate fields)
Hi Laurent,
Thanks for your comments.
On 2017-12-08 21:30:20 +0200, Laurent Pinchart wrote:
> Hi Niklas,
>
> On Friday, 8 December 2017 16:06:58 EET Niklas Söderlund wrote:
> > On 2017-12-08 11:35:18 +0200, Laurent Pinchart wrote:
> > > On Friday, 8 December 2017 03:08:27 EET Niklas Söderlund wrote:
> > >> There was never proper support in the VIN driver to deliver ALTERNATING
> > >> field format to user-space, remove this field option. For sources using
> > >> this field format instead use the VIN hardware feature of combining the
> > >> fields to an interlaced format. This mode of operation was previously
> > >> the default behavior and ALTERNATING was only delivered to user-space if
> > >> explicitly requested. Allowing this to be explicitly requested was a
> > >> mistake and was never properly tested and never worked due to the
> > >> constraints put on the field format when it comes to sequence numbers
> > >> and timestamps etc.
> > >
> > > I'm puzzled, why can't we support V4L2_FIELD_ALTERNATE if we can support
> > > V4L2_FIELD_TOP and V4L2_FIELD_BOTTOM ? I don't dispute the fact that the
> > > currently implemented logic might be wrong (although I haven't
> > > double-checked that), but what prevents us from implementing it correctly
> > > ?
> >
> > Maybe my commit message is fuzzy. We can support V4L2_FIELD_ALTERNATE as
> > a source to the VIN but we can't (yet) support delivering it to
> > user-space in a good way. So if we have a video source which outputs
> > V4L2_FIELD_ALTERNATE we are fine as we can use the hardware to interlace
> > that or only capture the TOP or BOTTOM fields.
> >
> > But the driver logic to capture frames (the whole dance with single and
> > continues capture modes) to be able to deal with situations where
> > buffers are not queued fast enough currently prevents us from delivering
> > V4L2_FIELD_ALTERNATE to user-space. The problem is we can only capture
> > (correctly) ALTERNATE if we run in continues mode, if the driver is feed
> > buffers to slow and switches to single capture mode we can't live up to
> > the specification of the field order from the documentation:
> >
> > "If fields are successive, without any dropped fields between them
> > (fields can drop individually), can be determined from the struct
> > v4l2_buffer sequence field."
> >
> > So even if in single capture mode we switch between TOP and BOTTOM for
> > each capture the sequence number would always be sequential but the
> > fields would in temporal time potentially be far apparat (depending on
> > how fast user-space queues buffers + the time it takes to shutdown and
> > startup the VIN capture).
> >
> > So instead of badly supporting this field order now I feel it's better
> > to not support it and once we tackle the issue of trying to remove
> > single capture mode (if at all possible) add support for it. But this is
> > a task for a different patch-set as this one is quiet large already and
> > it's focus is to add Gen3 support.
>
> OK, so we could support capturing alternating fields, but in that case we
> wouldn't be able to provide accurate sequence numbers. I'm fine with dropping
> support for ALTERNATE, but I would capture that information in the commit
> message, and probably as well in a comment in the code.
Agree, I tried to capture this in the commit message. But as You did not
understand it I need to make a better job. I will expand on this in the
commit message and add a comment in the code. Thanks.
>
> > >> The height should not be cut in half for the format for TOP or BOTTOM
> > >> fields settings. This was a mistake and it was made visible by the
> > >> scaling refactoring. Correct behavior is that the user should request a
> > >> frame size that fits the half height frame reflected in the field
> > >> setting. If not the VIN will do its best to scale the top or bottom to
> > >> the requested format and cropping and scaling do not work as expected.
> > >>
> > >> Signed-off-by: Niklas Söderlund
> > >> Reviewed-by: Hans Verkuil
> > >> ---
> > >>
> > >> drivers/media/platform/rcar-vin/rcar-dma.c | 15 +
> > >> drivers/media/platform/rcar-vin/rcar-v4l2.c | 48 +--
> > >> 2 files changed, 19 insertions(+), 44 deletions(-)
>
> [snip]
>
> > >> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > >> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> > >> 9cf9ff48ac1e2f4f..37fe1f6c646b0ea3 100644
> > >> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > >> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > >> @@ -102,6 +102,24 @@ static int rvin_get_sd_format(struct rvin_dev *vin,
> > >> struct v4l2_pix_format *pix)
> > >> if (ret)
> > >> return ret;
> > >>
> > >> +switch (fmt.format.field) {
> > >> +case V4L2_FIELD_TOP:
> > >> +case V4L2_FIELD_BOTTOM:
> > >> +case V4L2_FIELD_NONE:
> > >> +case V4L2_FIELD_INTERLACED_TB:
> > >> +case V4L2_FIELD_INTERLACED_BT:
[PATCH 1/4] meida: mt9m111: create subdevice device node
Set the V4L2_SUBDEV_FL_HAS_DEVNODE flag for the subdevice so that the subdevice device node is created. Cc: Sakari Ailus Cc: Mauro Carvalho Chehab Signed-off-by: Akinobu Mita --- drivers/media/i2c/mt9m111.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c index b1665d9..4fa10df 100644 --- a/drivers/media/i2c/mt9m111.c +++ b/drivers/media/i2c/mt9m111.c @@ -951,6 +951,8 @@ static int mt9m111_probe(struct i2c_client *client, mt9m111->ctx = &context_b; v4l2_i2c_subdev_init(&mt9m111->subdev, client, &mt9m111_subdev_ops); + mt9m111->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + v4l2_ctrl_handler_init(&mt9m111->hdl, 5); v4l2_ctrl_new_std(&mt9m111->hdl, &mt9m111_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); -- 2.7.4
[PATCH 3/4] meida: mt9m111: document missing required clocks property
The mt9m111 driver requires clocks property for the master clock to the
sensor, but there is no description for that. This adds it.
Cc: Rob Herring
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Signed-off-by: Akinobu Mita
---
Documentation/devicetree/bindings/media/i2c/mt9m111.txt | 4
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
b/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
index ed5a334..ffb57d1 100644
--- a/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
+++ b/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
@@ -6,6 +6,8 @@ interface.
Required Properties:
- compatible: value should be "micron,mt9m111"
+- clocks: reference to the master clock.
+- clock-names: should be "mclk".
For further reading on port node refer to
Documentation/devicetree/bindings/media/video-interfaces.txt.
@@ -16,6 +18,8 @@ Example:
mt9m111@5d {
compatible = "micron,mt9m111";
reg = <0x5d>;
+ clocks = <&mclk>;
+ clock-names = "mclk";
remote = <&pxa_camera>;
port {
--
2.7.4
[PATCH 2/4] meida: mt9m111: add media controller support
Create a source pad and set the media controller type to the sensor.
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Signed-off-by: Akinobu Mita
---
drivers/media/i2c/mt9m111.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index 4fa10df..e1d5032 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -215,6 +215,9 @@ struct mt9m111 {
int power_count;
const struct mt9m111_datafmt *fmt;
int lastpage; /* PageMap cache value */
+#ifdef CONFIG_MEDIA_CONTROLLER
+ struct media_pad pad;
+#endif
};
/* Find a data format by a pixel code */
@@ -971,6 +974,14 @@ static int mt9m111_probe(struct i2c_client *client,
goto out_clkput;
}
+#ifdef CONFIG_MEDIA_CONTROLLER
+ mt9m111->pad.flags = MEDIA_PAD_FL_SOURCE;
+ mt9m111->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+ ret = media_entity_pads_init(&mt9m111->subdev.entity, 1, &mt9m111->pad);
+ if (ret < 0)
+ goto out_hdlfree;
+#endif
+
/* Second stage probe - when a capture adapter is there */
mt9m111->rect.left = MT9M111_MIN_DARK_COLS;
mt9m111->rect.top = MT9M111_MIN_DARK_ROWS;
@@ -982,16 +993,20 @@ static int mt9m111_probe(struct i2c_client *client,
ret = mt9m111_video_probe(client);
if (ret < 0)
- goto out_hdlfree;
+ goto out_entityclean;
mt9m111->subdev.dev = &client->dev;
ret = v4l2_async_register_subdev(&mt9m111->subdev);
if (ret < 0)
- goto out_hdlfree;
+ goto out_entityclean;
return 0;
+out_entityclean:
+#ifdef CONFIG_MEDIA_CONTROLLER
+ media_entity_cleanup(&mt9m111->subdev.entity);
out_hdlfree:
+#endif
v4l2_ctrl_handler_free(&mt9m111->hdl);
out_clkput:
v4l2_clk_put(mt9m111->clk);
@@ -1004,6 +1019,9 @@ static int mt9m111_remove(struct i2c_client *client)
struct mt9m111 *mt9m111 = to_mt9m111(client);
v4l2_async_unregister_subdev(&mt9m111->subdev);
+#ifdef CONFIG_MEDIA_CONTROLLER
+ media_entity_cleanup(&mt9m111->subdev.entity);
+#endif
v4l2_clk_put(mt9m111->clk);
v4l2_ctrl_handler_free(&mt9m111->hdl);
--
2.7.4
[PATCH 4/4] meida: mt9m111: add V4L2_CID_TEST_PATTERN control
The mt9m111 has the test pattern generator features. This makes use of
it through V4L2_CID_TEST_PATTERN control.
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Signed-off-by: Akinobu Mita
---
drivers/media/i2c/mt9m111.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index e1d5032..d74f254 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -92,6 +92,7 @@
*/
#define MT9M111_OPER_MODE_CTRL 0x106
#define MT9M111_OUTPUT_FORMAT_CTRL 0x108
+#define MT9M111_TPG_CTRL 0x148
#define MT9M111_REDUCER_XZOOM_B0x1a0
#define MT9M111_REDUCER_XSIZE_B0x1a1
#define MT9M111_REDUCER_YZOOM_B0x1a3
@@ -124,6 +125,7 @@
#define MT9M111_OUTFMT_AVG_CHROMA (1 << 2)
#define MT9M111_OUTFMT_SWAP_YCbCr_C_Y_RGB_EVEN (1 << 1)
#define MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr_RGB_R_B(1 << 0)
+#define MT9M111_TPG_SEL_MASK GENMASK(2, 0)
/*
* Camera control register addresses (0x200..0x2ff not implemented)
@@ -706,6 +708,25 @@ static int mt9m111_set_autowhitebalance(struct mt9m111
*mt9m111, int on)
return reg_clear(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOWHITEBAL_EN);
}
+static const char * const mt9m111_test_pattern_menu[] = {
+ "Disabled",
+ "Vertical monochrome gradient",
+ "Flat color type 1",
+ "Flat color type 2",
+ "Flat color type 3",
+ "Flat color type 4",
+ "Flat color type 5",
+ "Color bar",
+};
+
+static int mt9m111_set_test_pattern(struct mt9m111 *mt9m111, int val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&mt9m111->subdev);
+
+ return mt9m111_reg_mask(client, MT9M111_TPG_CTRL, val,
+ MT9M111_TPG_SEL_MASK);
+}
+
static int mt9m111_s_ctrl(struct v4l2_ctrl *ctrl)
{
struct mt9m111 *mt9m111 = container_of(ctrl->handler,
@@ -724,6 +745,8 @@ static int mt9m111_s_ctrl(struct v4l2_ctrl *ctrl)
return mt9m111_set_autoexposure(mt9m111, ctrl->val);
case V4L2_CID_AUTO_WHITE_BALANCE:
return mt9m111_set_autowhitebalance(mt9m111, ctrl->val);
+ case V4L2_CID_TEST_PATTERN:
+ return mt9m111_set_test_pattern(mt9m111, ctrl->val);
}
return -EINVAL;
@@ -968,6 +991,10 @@ static int mt9m111_probe(struct i2c_client *client,
v4l2_ctrl_new_std_menu(&mt9m111->hdl,
&mt9m111_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
V4L2_EXPOSURE_AUTO);
+ v4l2_ctrl_new_std_menu_items(&mt9m111->hdl,
+ &mt9m111_ctrl_ops, V4L2_CID_TEST_PATTERN,
+ ARRAY_SIZE(mt9m111_test_pattern_menu) - 1, 0, 0,
+ mt9m111_test_pattern_menu);
mt9m111->subdev.ctrl_handler = &mt9m111->hdl;
if (mt9m111->hdl.error) {
ret = mt9m111->hdl.error;
--
2.7.4
[PATCH 0/4] media: mt9m111: media controller support and misc changes
This series adds media controller support and other miscellaneous changes to the mt9m111 driver. The MT9M111 camera modules are easily available for the hobbyists. Akinobu Mita (4): meida: mt9m111: create subdevice device node meida: mt9m111: add media controller support meida: mt9m111: document missing required clocks property meida: mt9m111: add V4L2_CID_TEST_PATTERN control .../devicetree/bindings/media/i2c/mt9m111.txt | 4 ++ drivers/media/i2c/mt9m111.c| 51 +- 2 files changed, 53 insertions(+), 2 deletions(-) Cc: Rob Herring Cc: Sakari Ailus Cc: Mauro Carvalho Chehab -- 2.7.4
[PATCH] [media] dvb-frontend/mxl5xx: add support for physical layer scrambling
From: Daniel Scheller
The MaxLinear MxL5xx has support for physical layer scrambling, which was
recently added to the DVB core via the new scrambling_sequence_index
property. Add required bits to the mxl5xx driver.
Picked up from dddvb master, commit 5c032058b9ba ("add support for PLS")
by Ralph Metzler , adapted to the different naming
of the pls property (pls vs. scrambling_sequence_index).
Cc: Ralph Metzler
Signed-off-by: Daniel Scheller
---
NB, I'm also prepping up another set of patches that enable the stv0910
driver to handle the recently added PLS support.
drivers/media/dvb-frontends/mxl5xx.c | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/mxl5xx.c
b/drivers/media/dvb-frontends/mxl5xx.c
index 1ebc3830579f..05f27f51fd03 100644
--- a/drivers/media/dvb-frontends/mxl5xx.c
+++ b/drivers/media/dvb-frontends/mxl5xx.c
@@ -380,6 +380,38 @@ static int get_algo(struct dvb_frontend *fe)
return DVBFE_ALGO_HW;
}
+static u32 gold2root(u32 gold)
+{
+ u32 x, g, tmp = gold;
+
+ if (tmp >= 0x3)
+ tmp = 0;
+ for (g = 0, x = 1; g < tmp; g++)
+ x = (((x ^ (x >> 7)) & 1) << 17) | (x >> 1);
+ return x;
+}
+
+static int cfg_scrambler(struct mxl *state, u32 gold)
+{
+ u32 root;
+ u8 buf[26] = {
+ MXL_HYDRA_PLID_CMD_WRITE, 24,
+ 0, MXL_HYDRA_DEMOD_SCRAMBLE_CODE_CMD, 0, 0,
+ state->demod, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 1, 0, 0, 0,
+ };
+
+ root = gold2root(gold);
+
+ buf[25] = (root >> 24) & 0xff;
+ buf[24] = (root >> 16) & 0xff;
+ buf[23] = (root >> 8) & 0xff;
+ buf[22] = root & 0xff;
+
+ return send_command(state, sizeof(buf), buf);
+}
+
static int cfg_demod_abort_tune(struct mxl *state)
{
struct MXL_HYDRA_DEMOD_ABORT_TUNE_T abort_tune_cmd;
@@ -437,7 +469,7 @@ static int set_parameters(struct dvb_frontend *fe)
demod_chan_cfg.roll_off = MXL_HYDRA_ROLLOFF_AUTO;
demod_chan_cfg.modulation_scheme = MXL_HYDRA_MOD_AUTO;
demod_chan_cfg.pilots = MXL_HYDRA_PILOTS_AUTO;
- /* cfg_scrambler(state); */
+ cfg_scrambler(state, p->scrambling_sequence_index);
break;
default:
return -EINVAL;
--
2.13.6
Re: [PATCH v9 11/28] rcar-vin: do not allow changing scaling and composing while streaming
Hi Laurent, On 2017-12-08 21:20:48 +0200, Laurent Pinchart wrote: > Hi Niklas, > > On Friday, 8 December 2017 16:14:23 EET Niklas Söderlund wrote: > > On 2017-12-08 11:04:26 +0200, Laurent Pinchart wrote: > > > On Friday, 8 December 2017 03:08:25 EET Niklas Söderlund wrote: > > >> It is possible on Gen2 to change the registers controlling composing and > > >> scaling while the stream is running. It is however not a good idea to do > > >> so and could result in trouble. There are also no good reasons to allow > > >> this, remove immediate reflection in hardware registers from > > >> vidioc_s_selection and only configure scaling and composing when the > > >> stream starts. > > > > > > There is a good reason: digital zoom. > > > > OK, so you would recommend me to drop this patch to keep the current > > behavior? > > Yes, unless you don't care about breaking use cases for Gen2, but in that > case > I'd recommend dropping Gen2 support altogether :-) Well I don't want to do that so I will drop this patch for the next version. Thanks for clarifying the use-case for this. > > > >> Signed-off-by: Niklas Söderlund > > >> Reviewed-by: Hans Verkuil > > >> --- > > >> > > >> drivers/media/platform/rcar-vin/rcar-dma.c | 2 +- > > >> drivers/media/platform/rcar-vin/rcar-v4l2.c | 3 --- > > >> drivers/media/platform/rcar-vin/rcar-vin.h | 3 --- > > >> 3 files changed, 1 insertion(+), 7 deletions(-) > > [snip] > > -- > Regards, > > Laurent Pinchart > -- Regards, Niklas Söderlund
Re: [PATCH v9 09/28] rcar-vin: all Gen2 boards can scale simplify logic
Hi Laurent,
Thanks for your comment.
On 2017-12-08 10:33:32 +0200, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> On Friday, 8 December 2017 03:08:23 EET Niklas Söderlund wrote:
> > The logic to preserve the requested format width and height are too
> > complex and come from a premature optimization for Gen3. All Gen2 SoC
> > can scale and the Gen3 implementation will not use these functions at
> > all so simply preserve the width and height when interacting with the
> > subdevice much like the field is preserved simplifies the logic quite a
> > bit.
> >
> > Signed-off-by: Niklas Söderlund
> > Reviewed-by: Hans Verkuil
> > ---
> > drivers/media/platform/rcar-vin/rcar-dma.c | 8
> > drivers/media/platform/rcar-vin/rcar-v4l2.c | 22 ++
> > drivers/media/platform/rcar-vin/rcar-vin.h | 2 --
> > 3 files changed, 10 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> > b/drivers/media/platform/rcar-vin/rcar-dma.c index
> > a7cda3922cb74baa..fd14be20a6604d7a 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -585,14 +585,6 @@ void rvin_crop_scale_comp(struct rvin_dev *vin)
> > 0, 0);
> > }
> >
> > -void rvin_scale_try(struct rvin_dev *vin, struct v4l2_pix_format *pix,
> > - u32 width, u32 height)
> > -{
> > - /* All VIN channels on Gen2 have scalers */
> > - pix->width = width;
> > - pix->height = height;
> > -}
> > -
> > /*
> > ---
> > -- * Hardware setup
> > */
> > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> > 19de99133f048960..1c5e7f6d5b963740 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > @@ -166,6 +166,7 @@ static int __rvin_try_format_source(struct rvin_dev
> > *vin, .which = which,
> > };
> > enum v4l2_field field;
> > + u32 width, height;
> > int ret;
> >
> > sd = vin_to_source(vin);
> > @@ -178,7 +179,10 @@ static int __rvin_try_format_source(struct rvin_dev
> > *vin,
> >
> > format.pad = vin->digital->source_pad;
> >
> > + /* Allow the video device to override field and to scale */
> > field = pix->field;
> > + width = pix->width;
> > + height = pix->height;
> >
> > ret = v4l2_subdev_call(sd, pad, set_fmt, pad_cfg, &format);
> > if (ret < 0 && ret != -ENOIOCTLCMD)
> > @@ -191,6 +195,9 @@ static int __rvin_try_format_source(struct rvin_dev
> > *vin, source->width = pix->width;
> > source->height = pix->height;
> >
>
> I would move the pix->field = field line not shown above to here.
Agree, thanks I will do so.
>
> Apart from that,
>
> Reviewed-by: Laurent Pinchart
Thanks.
>
> > + pix->width = width;
> > + pix->height = height;
> > +
> > vin_dbg(vin, "Source resolution: %ux%u\n", source->width,
> > source->height);
> >
> > @@ -204,13 +211,9 @@ static int __rvin_try_format(struct rvin_dev *vin,
> > struct v4l2_pix_format *pix,
> > struct rvin_source_fmt *source)
> > {
> > - u32 rwidth, rheight, walign;
> > + u32 walign;
> > int ret;
> >
> > - /* Requested */
> > - rwidth = pix->width;
> > - rheight = pix->height;
> > -
> > /* Keep current field if no specific one is asked for */
> > if (pix->field == V4L2_FIELD_ANY)
> > pix->field = vin->format.field;
> > @@ -248,10 +251,6 @@ static int __rvin_try_format(struct rvin_dev *vin,
> > break;
> > }
> >
> > - /* If source can't match format try if VIN can scale */
> > - if (source->width != rwidth || source->height != rheight)
> > - rvin_scale_try(vin, pix, rwidth, rheight);
> > -
> > /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */
> > walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
> >
> > @@ -270,9 +269,8 @@ static int __rvin_try_format(struct rvin_dev *vin,
> > return -EINVAL;
> > }
> >
> > - vin_dbg(vin, "Requested %ux%u Got %ux%u bpl: %d size: %d\n",
> > - rwidth, rheight, pix->width, pix->height,
> > - pix->bytesperline, pix->sizeimage);
> > + vin_dbg(vin, "Format %ux%u bpl: %d size: %d\n",
> > + pix->width, pix->height, pix->bytesperline, pix->sizeimage);
> >
> > return 0;
> > }
> > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> > b/drivers/media/platform/rcar-vin/rcar-vin.h index
> > 646f897f5c05ec4e..36d0f0cc4ce01a6e 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> > @@ -176,8 +176,6 @@ void rvin_v4l2_unregister(struct rvin_dev *vin);
> > const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat);
> >
> > /* Cropping, composing and scaling */
> >
Re: [PATCH v9 07/28] rcar-vin: change name of video device
Hi Laurent and Sakari, Thanks for your comments. On 2017-12-14 17:50:24 +0200, Laurent Pinchart wrote: > Hi Sakari, > > On Thursday, 14 December 2017 16:25:00 EET Sakari Ailus wrote: > > On Fri, Dec 08, 2017 at 10:17:36AM +0200, Laurent Pinchart wrote: > > > On Friday, 8 December 2017 03:08:21 EET Niklas Söderlund wrote: > > > > The rcar-vin driver needs to be part of a media controller to support > > > > Gen3. Give each VIN instance a unique name so it can be referenced from > > > > userspace. > > > > > > > > Signed-off-by: Niklas Söderlund > > > > Reviewed-by: Kieran Bingham > > > > Reviewed-by: Hans Verkuil > > > > --- > > > > > > > > drivers/media/platform/rcar-vin/rcar-v4l2.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c > > > > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index > > > > 59ec6d3d119590aa..19de99133f048960 100644 > > > > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > > > > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > > > > @@ -876,7 +876,8 @@ int rvin_v4l2_register(struct rvin_dev *vin) > > > > > > > > vdev->fops = &rvin_fops; > > > > vdev->v4l2_dev = &vin->v4l2_dev; > > > > vdev->queue = &vin->queue; > > > > > > > > - strlcpy(vdev->name, KBUILD_MODNAME, sizeof(vdev->name)); > > > > + snprintf(vdev->name, sizeof(vdev->name), "%s %s", > > > > KBUILD_MODNAME, > > > > +dev_name(vin->dev)); > > > > > > Do we need the module name here ? How about calling them "%s output", > > > dev_name(vin->dev) to emphasize the fact that this is a video node and not > > > a VIN subdev ? This is what the omap3isp and vsp1 drivers do. > > > > > > We're suffering a bit from the fact that V4L2 has never standardized a > > > naming scheme for the devices. It wouldn't be fair to ask you to fix that > > > as a prerequisite to get the VIN driver merged, but we clearly have to > > > work on that at some point. > > > > Agreed, this needs to be stable and I think aligning to what omap3isp or > > vsp1 do would be a good fix here. > > Even omap3isp and vsp1 are not fully aligned, so I think we need to design a > naming policy and document it. I agree that align this is a good idea. And for this reason I chosen to update this patch as such: "%s output", dev_name(vin->dev) I hope this is a step in the correct direction. If not please let me know as soon as possible so I can minimize the trouble for the other developers doing stuff on-top of this series and there test scripts :-) > > -- > Regards, > > Laurent Pinchart > -- Regards, Niklas Söderlund
Re: [PATCH v4 00/12] Intel IPU3 ImgU patchset
Hi, Em Fri, 17 Nov 2017 02:58:56 + "Mani, Rajmohan" escreveu: > Here is an update on the IPU3 documentation that we are currently working on. > > Image processing in IPU3 relies on the following. > > 1) HW configuration to enable ISP and > 2) setting customer specific 3A Tuning / Algorithm Parameters to achieve > desired image quality. > > We intend to provide documentation on ImgU driver programming interface to > help users of this driver to configure and enable ISP HW to meet their needs. > This documentation will include details on complete V4L2 Kernel driver > interface and IO-Control parameters, except for the ISP internal algorithm > and its parameters (which is Intel proprietary IP). Sakari asked me to take a look on this thread, specifically on this email. I took a look on the other e-mails from this thread that are discussing about this IP block. I understand that Intel wants to keep their internal 3A algorithm protected, just like other vendors protect their own algos. It was never a requirement to open whatever algorithm are used inside a hardware (or firmware). The only requirement is that firmwares should be licensed with redistribution permission, ideally merged at linux-firmware git tree. Yet, what I don't understand is why Intel also wants to also protect the interface for such 3A hardware/firmware algorithm. The parameters that are passed from an userspace application to Intel ISP logic doesn't contain the algorithm itself. What's the issue of documenting the meaning of each parameter? Thanks, Mauro
Re: [PATCH 2/8] media: v4l2-ioctl.h: convert debug into an enum of bits
Em Wed, 20 Dec 2017 12:47:23 +0200 Laurent Pinchart escreveu: > Hi Mauro, > > On Tuesday, 19 December 2017 17:34:46 EET Mauro Carvalho Chehab wrote: > > Em Tue, 19 Dec 2017 16:05:46 +0200 Laurent Pinchart escreveu: > > > On Tuesday, 19 December 2017 16:02:02 EET Laurent Pinchart wrote: > > >> On Tuesday, 19 December 2017 13:39:27 EET Sakari Ailus wrote: > > >>> On Mon, Dec 18, 2017 at 05:53:56PM -0200, Mauro Carvalho Chehab wrote: > > The V4L2_DEV_DEBUG_IOCTL macros actually define a bitmask, > > but without using Kernel's modern standards. Also, > > documentation looks akward. > > > > So, convert them into an enum with valid bits, adding > > the correspoinding kernel-doc documentation for it. > > >>> > > >>> The pattern of using bits for flags is a well established one and I > > >>> wouldn't deviate from that by requiring the use of the BIT() macro. > > >>> There are no benefits that I can see from here but the approach brings > > >>> additional risks: misuse of the flags and mimicing the same risky > > >>> pattern. > > >>> > > >>> I'd also like to echo Laurent's concern that code is being changed in > > >>> odd ways and not for itself, but due to deficiencies in documentation > > >>> tools. > > >>> > > >>> I believe the tooling has to be improved to address this properly. > > >>> That only needs to done once, compared to changing all flag > > >>> definitions to enums. > > >> > > >> That's my main concern too. We really must not sacrifice code > > >> readability or writing ease in order to work around limitations of the > > >> documentation system. For this reason I'm strongly opposed to patches 2 > > >> and 5 in this series. > > > > > > And I forgot to mention patch 8/8. Let's drop those three and improve the > > > documentation system instead. > > > > Are you volunteering yourself to write the kernel-doc patches? :-) > > I thought you were the expert in this field, given the number of > documentation > patches that you have merged in the kernel ? :-) c/c linux-doc, as this kind of discussion should be happening there. Let me recap your proposal here, for the others from linux-doc to understand this reply: Em Mon, 18 Dec 2017 17:32:11 +0200 Laurent Pinchart escreveu: > Hi Mauro, > > On Monday, 18 December 2017 17:13:26 EET Mauro Carvalho Chehab wrote: > > Em Fri, 13 Oct 2017 15:38:11 +0300 Laurent Pinchart escreveu: > > > On Thursday, 28 September 2017 00:46:51 EEST Mauro Carvalho Chehab wrote: > > > > > >> Currently, there's no way to document #define foo > > >> with kernel-doc. So, convert it to an enum, and document. > > > > > > The documentation seems fine to me (except for one comment below). > > > However, converting macros to an enum just to work around a defect of the > > > documentation system doesn't seem like a good idea to me. I'd rather find > > > a way to document macros. > > > > I agree that this limitation should be fixed. > > > > Yet, in this specific case where we have an "array" of defines, all > > associated to the same field (even being a bitmask), and assuming that > > we would add a way for kernel-doc to parse this kind of defines > > (not sure how easy/doable would be), then, in order to respect the > > way kernel-doc markup is, the documentation for those macros would be: > > > > > > /** > > * define: Just log the ioctl name + error code > > */ > > #define V4L2_DEV_DEBUG_IOCTL0x01 > > /** > > * define: Log the ioctl name arguments + error code > > */ > > #define V4L2_DEV_DEBUG_IOCTL_ARG0x02 > > /** > > * define: Log the file operations open, release, mmap and get_unmapped_area > > */ > > #define V4L2_DEV_DEBUG_FOP 0x04 > > /** > > * define: Log the read and write file operations and the VIDIOC_(D)QBUF > > ioctls */ > > #define V4L2_DEV_DEBUG_STREAMING0x08 > > > > IMHO, this is a way easier to read/understand by humans, and a way more > > coincise: > > > > /** > > * enum v4l2_debug_flags - Device debug flags to be used with the video > > * device debug attribute > > * > > * @V4L2_DEV_DEBUG_IOCTL: Just log the ioctl name + error code. > > * @V4L2_DEV_DEBUG_IOCTL_ARG: Log the ioctl name arguments + error > > code. > > * @V4L2_DEV_DEBUG_FOP: Log the file operations and open, > > release, > > * mmap and get_unmapped_area syscalls. > > * @V4L2_DEV_DEBUG_STREAMING: Log the read and write syscalls and > > * :c:ref:`VIDIOC_[Q|DQ]BUFF ` ioctls. > > */ > > > > It also underlines the aspect that those names are grouped altogether. > > > > So, IMHO, the main reason to place them inside an enum and document > > as such is that it looks a way better for humans to read. > > As we're talking about extending kerneldoc to document macros, we're free to > decide on a format that would make it easy and clear. Based on your above > example, we could write it > > /** > * define v4l2_de
Re: [PATCH v2 3/8] media: v4l2-async: simplify v4l2_async_subdev structure
Hi Mauro, Thanks for the patch. On Tue, Dec 19, 2017 at 11:18 AM, Mauro Carvalho Chehab wrote: > The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one > struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME > match criteria requires just a device name. > > So, it doesn't make sense to enclose those into structs, > as the criteria can go directly into the union. > > That makes easier to document it, as we don't need to document > weird senseless structs. > > At drivers, this makes even clearer about the match criteria. > > Acked-by: Sylwester Nawrocki > Acked-by: Benoit Parrot > Acked-by: Alexandre Belloni > Acked-by: Sakari Ailus > Acked-by: Philipp Zabel > Signed-off-by: Mauro Carvalho Chehab > --- > drivers/media/platform/am437x/am437x-vpfe.c| 6 +++--- For above: Acked-by: Lad, Prabhakar Cheers, --Prabhakar Lad
[PATCH] Intelsat34-55.5W: update definitions from Vivo streaming
There were some frequency changes on this Satellite. Update them. Signed-off-by: Mauro Carvalho Chehab --- dvb-s/Intelsat34-55.5W | 58 ++ 1 file changed, 7 insertions(+), 51 deletions(-) diff --git a/dvb-s/Intelsat34-55.5W b/dvb-s/Intelsat34-55.5W index 3b96fc894c99..28d57fbbf73e 100644 --- a/dvb-s/Intelsat34-55.5W +++ b/dvb-s/Intelsat34-55.5W @@ -2,28 +2,6 @@ # TV channels -[CHANNEL] - DELIVERY_SYSTEM = DVBS2 - FREQUENCY = 11825000 - POLARIZATION = HORIZONTAL - SYMBOL_RATE = 4500 - INNER_FEC = AUTO - ROLLOFF = AUTO - MODULATION = QAM/AUTO - STREAM_ID = 0 - INVERSION = AUTO - -[CHANNEL] - DELIVERY_SYSTEM = DVBS2 - FREQUENCY = 11825000 - POLARIZATION = VERTICAL - SYMBOL_RATE = 4500 - INNER_FEC = AUTO - ROLLOFF = AUTO - MODULATION = QAM/AUTO - STREAM_ID = 0 - INVERSION = AUTO - [CHANNEL] DELIVERY_SYSTEM = DVBS2 FREQUENCY = 1189 @@ -31,18 +9,7 @@ SYMBOL_RATE = 3000 INNER_FEC = AUTO ROLLOFF = AUTO - MODULATION = QAM/AUTO - STREAM_ID = 0 - INVERSION = AUTO - -[CHANNEL] - DELIVERY_SYSTEM = DVBS2 - FREQUENCY = 11905000 - POLARIZATION = VERTICAL - SYMBOL_RATE = 4500 - INNER_FEC = AUTO - ROLLOFF = AUTO - MODULATION = QAM/AUTO + MODULATION = PSK/8 STREAM_ID = 0 INVERSION = AUTO @@ -53,7 +20,7 @@ SYMBOL_RATE = 3000 INNER_FEC = AUTO ROLLOFF = AUTO - MODULATION = QAM/AUTO + MODULATION = PSK/8 STREAM_ID = 0 INVERSION = AUTO @@ -64,7 +31,7 @@ SYMBOL_RATE = 3000 INNER_FEC = AUTO ROLLOFF = AUTO - MODULATION = QAM/AUTO + MODULATION = PSK/8 STREAM_ID = 0 INVERSION = AUTO @@ -75,7 +42,7 @@ SYMBOL_RATE = 3000 INNER_FEC = AUTO ROLLOFF = AUTO - MODULATION = QAM/AUTO + MODULATION = PSK/8 STREAM_ID = 0 INVERSION = AUTO @@ -86,7 +53,7 @@ SYMBOL_RATE = 3000 INNER_FEC = AUTO ROLLOFF = AUTO - MODULATION = QAM/AUTO + MODULATION = PSK/8 STREAM_ID = 0 INVERSION = AUTO @@ -97,7 +64,7 @@ SYMBOL_RATE = 3000 INNER_FEC = AUTO ROLLOFF = AUTO - MODULATION = QAM/AUTO + MODULATION = PSK/8 STREAM_ID = 0 INVERSION = AUTO @@ -108,18 +75,7 @@ SYMBOL_RATE = 3000 INNER_FEC = AUTO ROLLOFF = AUTO - MODULATION = QAM/AUTO - STREAM_ID = 0 - INVERSION = AUTO - -[CHANNEL] - DELIVERY_SYSTEM = DVBS2 - FREQUENCY = 1217 - POLARIZATION = HORIZONTAL - SYMBOL_RATE = 3000 - INNER_FEC = AUTO - ROLLOFF = AUTO - MODULATION = QAM/AUTO + MODULATION = PSK/8 STREAM_ID = 0 INVERSION = AUTO -- 2.14.3
[PATCH REBASED] media: v4l-utils: dvbv5: Streaming I/O for DVB
From: Junghak Sung
Add a new scenario to use streaming I/O for TS recording.
Signed-off-by: Junghak Sung
Signed-off-by: Geunyoung Kim
Acked-by: Seung-Woo Kim
Acked-by: Inki Dae
Signed-off-by: Mauro Carvalho Chehab
---
This is the old patch adding dvbv5-zap support for the new DVB
mmap API. It is just rebased on the top of the current version
of v4l-utils.
For now, build-test only.
It should be noticed that, instead of adding mmap support into
the library, it just adds support for it directly at the
dvbv5-zap API, as it is meant just for testing the new API.
Once it works, the new mmap API logic should be moved to the
libdvbv5 core.
include/linux/dvb/dmx.h | 64 +
utils/dvb/dvbv5-zap.c | 185 +++-
2 files changed, 247 insertions(+), 2 deletions(-)
diff --git a/include/linux/dvb/dmx.h b/include/linux/dvb/dmx.h
index 7d27bf5c1d20..c8dd6d3234ab 100644
--- a/include/linux/dvb/dmx.h
+++ b/include/linux/dvb/dmx.h
@@ -209,6 +209,64 @@ struct dmx_stc {
__u64 stc;
};
+/**
+ * struct dmx_buffer - dmx buffer info
+ *
+ * @index: id number of the buffer
+ * @bytesused: number of bytes occupied by data in the buffer (payload);
+ * @offset:for buffers with memory == DMX_MEMORY_MMAP;
+ * offset from the start of the device memory for this plane,
+ * (or a "cookie" that should be passed to mmap() as offset)
+ * @length:size in bytes of the buffer
+ *
+ * Contains data exchanged by application and driver using one of the streaming
+ * I/O methods.
+ */
+struct dmx_buffer {
+ __u32 index;
+ __u32 bytesused;
+ __u32 offset;
+ __u32 length;
+ __u32 reserved[4];
+};
+
+/**
+ * struct dmx_requestbuffers - request dmx buffer information
+ *
+ * @count: number of requested buffers,
+ * @size: size in bytes of the requested buffer
+ *
+ * Contains data used for requesting a dmx buffer.
+ * All reserved fields must be set to zero.
+ */
+struct dmx_requestbuffers {
+ __u32 count;
+ __u32 size;
+ __u32 reserved[2];
+};
+
+/**
+ * struct dmx_exportbuffer - export of dmx buffer as DMABUF file descriptor
+ *
+ * @index: id number of the buffer
+ * @flags: flags for newly created file, currently only O_CLOEXEC is
+ * supported, refer to manual of open syscall for more details
+ * @fd:file descriptor associated with DMABUF (set by driver)
+ *
+ * Contains data used for exporting a dmx buffer as DMABUF file descriptor.
+ * The buffer is identified by a 'cookie' returned by DMX_QUERYBUF
+ * (identical to the cookie used to mmap() the buffer to userspace). All
+ * reserved fields must be set to zero. The field reserved0 is expected to
+ * become a structure 'type' allowing an alternative layout of the structure
+ * content. Therefore this field should not be used for any other extensions.
+ */
+struct dmx_exportbuffer {
+ __u32 index;
+ __u32 flags;
+ __s32 fd;
+ __u32 reserved[5];
+};
+
#define DMX_START_IO('o', 41)
#define DMX_STOP _IO('o', 42)
#define DMX_SET_FILTER _IOW('o', 43, struct dmx_sct_filter_params)
@@ -227,4 +285,10 @@ typedef enum dmx_ts_pes dmx_pes_type_t;
typedef struct dmx_filter dmx_filter_t;
+#define DMX_REQBUFS _IOWR('o', 60, struct dmx_requestbuffers)
+#define DMX_QUERYBUF _IOWR('o', 61, struct dmx_buffer)
+#define DMX_EXPBUF _IOWR('o', 62, struct dmx_exportbuffer)
+#define DMX_QBUF _IOWR('o', 63, struct dmx_buffer)
+#define DMX_DQBUF_IOWR('o', 64, struct dmx_buffer)
+
#endif /* _DVBDMX_H_ */
diff --git a/utils/dvb/dvbv5-zap.c b/utils/dvb/dvbv5-zap.c
index 5567736da3c7..de4c00e539c3 100644
--- a/utils/dvb/dvbv5-zap.c
+++ b/utils/dvb/dvbv5-zap.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
@@ -73,6 +74,7 @@ struct arguments {
enum dvb_file_formats input_format, output_format;
unsigned traffic_monitor, low_traffic, non_human, port;
char *search, *server;
+ unsigned streaming;
const char *cc;
/* Used by status print */
@@ -94,6 +96,7 @@ static const struct argp_option options[] = {
{"pat", 'p', NULL, 0, N_("add pat and pmt
to TS recording (implies -r)"), 0},
{"all-pids",'P', NULL, 0, N_("don't filter any
pids. Instead, outputs all of them"), 0 },
{"record", 'r', NULL, 0, N_("set up
/dev/dvb/adapterX/dvr0 for TS recording"), 0},
+ {"streaming", 'R', NULL, 0, N_("uses streaming
I/O for TS recording"), 0},
{"silence", 's', NULL, 0, N_("increase
Re: [PATCH v2 3/8] media: v4l2-async: simplify v4l2_async_subdev structure
Hi Mauro,
On 2017-12-19 09:18:19 -0200, Mauro Carvalho Chehab wrote:
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
>
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
>
> That makes easier to document it, as we don't need to document
> weird senseless structs.
>
> At drivers, this makes even clearer about the match criteria.
>
> Acked-by: Sylwester Nawrocki
> Acked-by: Benoit Parrot
> Acked-by: Alexandre Belloni
> Acked-by: Sakari Ailus
> Acked-by: Philipp Zabel
> Signed-off-by: Mauro Carvalho Chehab
> ---
> drivers/media/platform/am437x/am437x-vpfe.c| 6 +++---
> drivers/media/platform/atmel/atmel-isc.c | 2 +-
> drivers/media/platform/atmel/atmel-isi.c | 2 +-
> drivers/media/platform/davinci/vpif_capture.c | 4 ++--
> drivers/media/platform/exynos4-is/media-dev.c | 4 ++--
> drivers/media/platform/pxa_camera.c| 2 +-
> drivers/media/platform/qcom/camss-8x16/camss.c | 2 +-
> drivers/media/platform/rcar-vin/rcar-core.c| 2 +-
For rcar-vin:
Acked-by: Niklas Söderlund
> drivers/media/platform/rcar_drif.c | 4 ++--
> drivers/media/platform/soc_camera/soc_camera.c | 2 +-
> drivers/media/platform/stm32/stm32-dcmi.c | 2 +-
> drivers/media/platform/ti-vpe/cal.c| 2 +-
> drivers/media/platform/xilinx/xilinx-vipp.c| 2 +-
> drivers/media/v4l2-core/v4l2-async.c | 16
> drivers/media/v4l2-core/v4l2-fwnode.c | 10 +-
> drivers/staging/media/imx/imx-media-dev.c | 4 ++--
> include/media/v4l2-async.h | 8 ++--
> 17 files changed, 35 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c
> b/drivers/media/platform/am437x/am437x-vpfe.c
> index 0997c640191d..601ae6487617 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -2304,8 +2304,8 @@ vpfe_async_bound(struct v4l2_async_notifier *notifier,
> vpfe_dbg(1, vpfe, "vpfe_async_bound\n");
>
> for (i = 0; i < ARRAY_SIZE(vpfe->cfg->asd); i++) {
> - if (vpfe->cfg->asd[i]->match.fwnode.fwnode ==
> - asd[i].match.fwnode.fwnode) {
> + if (vpfe->cfg->asd[i]->match.fwnode ==
> + asd[i].match.fwnode) {
> sdinfo = &vpfe->cfg->sub_devs[i];
> vpfe->sd[i] = subdev;
> vpfe->sd[i]->grp_id = sdinfo->grp_id;
> @@ -2510,7 +2510,7 @@ vpfe_get_pdata(struct platform_device *pdev)
> }
>
> pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - pdata->asd[i]->match.fwnode.fwnode = of_fwnode_handle(rem);
> + pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);
> of_node_put(rem);
> }
>
> diff --git a/drivers/media/platform/atmel/atmel-isc.c
> b/drivers/media/platform/atmel/atmel-isc.c
> index 0c2635647f69..34676409ca08 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -2088,7 +2088,7 @@ static int isc_parse_dt(struct device *dev, struct
> isc_device *isc)
> subdev_entity->pfe_cfg0 |= ISC_PFE_CFG0_PPOL_LOW;
>
> subdev_entity->asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - subdev_entity->asd->match.fwnode.fwnode =
> + subdev_entity->asd->match.fwnode =
> of_fwnode_handle(rem);
> list_add_tail(&subdev_entity->list, &isc->subdev_entities);
> }
> diff --git a/drivers/media/platform/atmel/atmel-isi.c
> b/drivers/media/platform/atmel/atmel-isi.c
> index e900995143a3..9958918e2449 100644
> --- a/drivers/media/platform/atmel/atmel-isi.c
> +++ b/drivers/media/platform/atmel/atmel-isi.c
> @@ -1128,7 +1128,7 @@ static int isi_graph_parse(struct atmel_isi *isi,
> struct device_node *node)
> /* Remote node to connect */
> isi->entity.node = remote;
> isi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> - isi->entity.asd.match.fwnode.fwnode = of_fwnode_handle(remote);
> + isi->entity.asd.match.fwnode = of_fwnode_handle(remote);
> return 0;
> }
> }
> diff --git a/drivers/media/platform/davinci/vpif_capture.c
> b/drivers/media/platform/davinci/vpif_capture.c
> index e45916f69def..e1c273c8b9a6 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -1390,7 +1390,7 @@ static int vpif_async_bound(struct v4l2_async_notifier
> *notifier,
>
> for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
> struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
> - const struct fwnode_handle *fwnode =
Re: [PATCH 2/8] media: v4l2-ioctl.h: convert debug into an enum of bits
Hi Mauro, On Tuesday, 19 December 2017 17:34:46 EET Mauro Carvalho Chehab wrote: > Em Tue, 19 Dec 2017 16:05:46 +0200 Laurent Pinchart escreveu: > > On Tuesday, 19 December 2017 16:02:02 EET Laurent Pinchart wrote: > >> On Tuesday, 19 December 2017 13:39:27 EET Sakari Ailus wrote: > >>> On Mon, Dec 18, 2017 at 05:53:56PM -0200, Mauro Carvalho Chehab wrote: > The V4L2_DEV_DEBUG_IOCTL macros actually define a bitmask, > but without using Kernel's modern standards. Also, > documentation looks akward. > > So, convert them into an enum with valid bits, adding > the correspoinding kernel-doc documentation for it. > >>> > >>> The pattern of using bits for flags is a well established one and I > >>> wouldn't deviate from that by requiring the use of the BIT() macro. > >>> There are no benefits that I can see from here but the approach brings > >>> additional risks: misuse of the flags and mimicing the same risky > >>> pattern. > >>> > >>> I'd also like to echo Laurent's concern that code is being changed in > >>> odd ways and not for itself, but due to deficiencies in documentation > >>> tools. > >>> > >>> I believe the tooling has to be improved to address this properly. > >>> That only needs to done once, compared to changing all flag > >>> definitions to enums. > >> > >> That's my main concern too. We really must not sacrifice code > >> readability or writing ease in order to work around limitations of the > >> documentation system. For this reason I'm strongly opposed to patches 2 > >> and 5 in this series. > > > > And I forgot to mention patch 8/8. Let's drop those three and improve the > > documentation system instead. > > Are you volunteering yourself to write the kernel-doc patches? :-) I thought you were the expert in this field, given the number of documentation patches that you have merged in the kernel ? :-) -- Regards, Laurent Pinchart
Re: [PATCH v1 05/10] staging: atomisp: Remove non-ACPI leftovers
On Wed, 20 Dec 2017, Dan Carpenter wrote: > On Tue, Dec 19, 2017 at 10:59:52PM +0200, Andy Shevchenko wrote: > > @@ -914,9 +904,7 @@ static int lm3554_probe(struct i2c_client *client) > > dev_err(&client->dev, "gpio request/direction_output fail"); > > goto fail2; > > } > > - if (ACPI_HANDLE(&client->dev)) > > - err = atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); > > - return 0; > > + return atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); > > fail2: > > media_entity_cleanup(&flash->sd.entity); > > v4l2_ctrl_handler_free(&flash->ctrl_handler); > > Actually every place where we directly return a function call is wrong > and needs error handling added. I've been meaning to write a Smatch > check for this because it's a common anti-pattern we don't check the > last function call for errors. > > Someone could probably do the same in Coccinelle if they want. I'm not sure what you are suggesting. Is every case of return f(...); for any f wrong? Or is it a particular function that is of concern? Or would it be that every function call that has error handling somewhere should have error handling everywhere? Or is it related to what seems to be the problem in the above code that err is initialized but nothing happens to it? julia
Re: [PATCH v1 05/10] staging: atomisp: Remove non-ACPI leftovers
On Wed, Dec 20, 2017 at 6:54 AM, Dan Carpenter wrote: > On Tue, Dec 19, 2017 at 10:59:52PM +0200, Andy Shevchenko wrote: >> @@ -1147,10 +1145,8 @@ static int gc2235_probe(struct i2c_client *client) >> if (ret) >> gc2235_remove(client); > > This error handling is probably wrong... > Thanks for pointing to this, but I'm not going to fix this by the following reasons: 1. I admit the driver's code is ugly 2. It's staging code 3. My patch does not touch those lines 4. My purpose is to get it working first. Feel free to send a followup with a good clean up which I agree with. >> >> - if (ACPI_HANDLE(&client->dev)) >> - ret = atomisp_register_i2c_module(&dev->sd, gcpdev, >> RAW_CAMERA); >> + return atomisp_register_i2c_module(&dev->sd, gcpdev, RAW_CAMERA); > > In the end this should look something like: > > ret = atomisp_register_i2c_module(&dev->sd, gcpdev, RAW_CAMERA); > if (ret) > goto err_free_something; > > return 0; > >> >> - return ret; >> out_free: >> v4l2_device_unregister_subdev(&dev->sd); >> kfree(dev); > > regards, > dan carpenter > -- With Best Regards, Andy Shevchenko
[PATCH v4 2/5] media: ov5640: check chip id
Verify that chip identifier is correct when probing.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 95 ++
1 file changed, 79 insertions(+), 16 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 61071f5..9f031f3 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1547,24 +1547,58 @@ static void ov5640_reset(struct ov5640_dev *sensor)
usleep_range(5000, 1);
}
-static int ov5640_set_power(struct ov5640_dev *sensor, bool on)
+static int ov5640_set_power_on(struct ov5640_dev *sensor)
{
- int ret = 0;
+ struct i2c_client *client = sensor->i2c_client;
+ int ret;
- if (on) {
- clk_prepare_enable(sensor->xclk);
+ ret = clk_prepare_enable(sensor->xclk);
+ if (ret) {
+ dev_err(&client->dev, "%s: failed to enable clock\n",
+ __func__);
+ return ret;
+ }
- ret = regulator_bulk_enable(OV5640_NUM_SUPPLIES,
- sensor->supplies);
- if (ret)
- goto xclk_off;
+ ret = regulator_bulk_enable(OV5640_NUM_SUPPLIES,
+ sensor->supplies);
+ if (ret) {
+ dev_err(&client->dev, "%s: failed to enable regulators\n",
+ __func__);
+ goto xclk_off;
+ }
+
+ ov5640_reset(sensor);
+ ov5640_power(sensor, true);
+
+ ret = ov5640_init_slave_id(sensor);
+ if (ret)
+ goto power_off;
+
+ return 0;
+
+power_off:
+ ov5640_power(sensor, false);
+ regulator_bulk_disable(OV5640_NUM_SUPPLIES, sensor->supplies);
+xclk_off:
+ clk_disable_unprepare(sensor->xclk);
+ return ret;
+}
+
+static void ov5640_set_power_off(struct ov5640_dev *sensor)
+{
+ ov5640_power(sensor, false);
+ regulator_bulk_disable(OV5640_NUM_SUPPLIES, sensor->supplies);
+ clk_disable_unprepare(sensor->xclk);
+}
- ov5640_reset(sensor);
- ov5640_power(sensor, true);
+static int ov5640_set_power(struct ov5640_dev *sensor, bool on)
+{
+ int ret = 0;
- ret = ov5640_init_slave_id(sensor);
+ if (on) {
+ ret = ov5640_set_power_on(sensor);
if (ret)
- goto power_off;
+ return ret;
ret = ov5640_restore_mode(sensor);
if (ret)
@@ -1586,10 +1620,7 @@ static int ov5640_set_power(struct ov5640_dev *sensor,
bool on)
}
power_off:
- ov5640_power(sensor, false);
- regulator_bulk_disable(OV5640_NUM_SUPPLIES, sensor->supplies);
-xclk_off:
- clk_disable_unprepare(sensor->xclk);
+ ov5640_set_power_off(sensor);
return ret;
}
@@ -2202,6 +2233,34 @@ static int ov5640_get_regulators(struct ov5640_dev
*sensor)
sensor->supplies);
}
+static int ov5640_check_chip_id(struct ov5640_dev *sensor)
+{
+ struct i2c_client *client = sensor->i2c_client;
+ int ret = 0;
+ u16 chip_id;
+
+ ret = ov5640_set_power_on(sensor);
+ if (ret)
+ return ret;
+
+ ret = ov5640_read_reg16(sensor, OV5640_REG_CHIP_ID, &chip_id);
+ if (ret) {
+ dev_err(&client->dev, "%s: failed to read chip identifier\n",
+ __func__);
+ goto power_off;
+ }
+
+ if (chip_id != 0x5640) {
+ dev_err(&client->dev, "%s: wrong chip identifier, expected
0x5640, got 0x%x\n",
+ __func__, chip_id);
+ ret = -ENXIO;
+ }
+
+power_off:
+ ov5640_set_power_off(sensor);
+ return ret;
+}
+
static int ov5640_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -2284,6 +2343,10 @@ static int ov5640_probe(struct i2c_client *client,
mutex_init(&sensor->lock);
+ ret = ov5640_check_chip_id(sensor);
+ if (ret)
+ goto entity_cleanup;
+
ret = ov5640_init_controls(sensor);
if (ret)
goto entity_cleanup;
--
1.9.1
[PATCH v4 4/5] media: ov5640: add support of DVP parallel interface
Add support of DVP parallel mode in addition of
existing MIPI CSI mode. The choice between two modes
and configuration is made through device tree.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 148 +++--
1 file changed, 130 insertions(+), 18 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 9f031f3..a44b680 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -34,13 +34,19 @@
#define OV5640_DEFAULT_SLAVE_ID 0x3c
+#define OV5640_REG_SYS_CTRL0 0x3008
#define OV5640_REG_CHIP_ID 0x300a
+#define OV5640_REG_IO_MIPI_CTRL00 0x300e
+#define OV5640_REG_PAD_OUTPUT_ENABLE01 0x3017
+#define OV5640_REG_PAD_OUTPUT_ENABLE02 0x3018
#define OV5640_REG_PAD_OUTPUT000x3019
+#define OV5640_REG_SYSTEM_CONTROL1 0x302e
#define OV5640_REG_SC_PLL_CTRL00x3034
#define OV5640_REG_SC_PLL_CTRL10x3035
#define OV5640_REG_SC_PLL_CTRL20x3036
#define OV5640_REG_SC_PLL_CTRL30x3037
#define OV5640_REG_SLAVE_ID0x3100
+#define OV5640_REG_SCCB_SYS_CTRL1 0x3103
#define OV5640_REG_SYS_ROOT_DIVIDER0x3108
#define OV5640_REG_AWB_R_GAIN 0x3400
#define OV5640_REG_AWB_G_GAIN 0x3402
@@ -70,6 +76,7 @@
#define OV5640_REG_HZ5060_CTRL01 0x3c01
#define OV5640_REG_SIGMADELTA_CTRL0C 0x3c0c
#define OV5640_REG_FRAME_CTRL010x4202
+#define OV5640_REG_POLARITY_CTRL00 0x4740
#define OV5640_REG_MIPI_CTRL00 0x4800
#define OV5640_REG_DEBUG_MODE 0x4814
#define OV5640_REG_PRE_ISP_TEST_SET1 0x503d
@@ -982,7 +989,111 @@ static int ov5640_get_gain(struct ov5640_dev *sensor)
return gain & 0x3ff;
}
-static int ov5640_set_stream(struct ov5640_dev *sensor, bool on)
+static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
+{
+ int ret;
+ unsigned int flags = sensor->ep.bus.parallel.flags;
+ u8 pclk_pol = 0;
+ u8 hsync_pol = 0;
+ u8 vsync_pol = 0;
+
+ /*
+* Note about parallel port configuration.
+*
+* When configured in parallel mode, the OV5640 will
+* output 10 bits data on DVP data lines [9:0].
+* If only 8 bits data are wanted, the 8 bits data lines
+* of the camera interface must be physically connected
+* on the DVP data lines [9:2].
+*
+* Control lines polarity can be configured through
+* devicetree endpoint control lines properties.
+* If no endpoint control lines properties are set,
+* polarity will be as below:
+* - VSYNC: active high
+* - HREF: active low
+* - PCLK: active low
+*/
+
+ if (on) {
+ /*
+* reset MIPI PCLK/SERCLK divider
+*
+* SC PLL CONTRL1 0
+* - [3..0]:MIPI PCLK/SERCLK divider
+*/
+ ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL1, 0x0f, 0);
+ if (ret)
+ return ret;
+
+ /*
+* configure parallel port control lines polarity
+*
+* POLARITY CTRL0
+* - [5]: PCLK polarity (0: active low, 1: active high)
+* - [1]: HREF polarity (0: active low, 1: active high)
+* - [0]: VSYNC polarity (mismatch here between
+* datasheet and hardware, 0 is active high
+* and 1 is active low...)
+*/
+ if (flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
+ pclk_pol = 1;
+ if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
+ hsync_pol = 1;
+ if (flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
+ vsync_pol = 1;
+
+ ret = ov5640_write_reg(sensor,
+ OV5640_REG_POLARITY_CTRL00,
+ (pclk_pol << 5) |
+ (hsync_pol << 1) |
+ vsync_pol);
+
+ if (ret)
+ return ret;
+ }
+
+ /*
+* powerdown MIPI TX/RX PHY & disable MIPI
+*
+* MIPI CONTROL 00
+* 4:PWDN PHY TX
+* 3:PWDN PHY RX
+* 2:MIPI enable
+*/
+ ret = ov5640_write_reg(sensor,
+ OV5640_REG_IO_MIPI_CTRL00, on ? 0x18 : 0);
+ if (ret)
+ return ret;
+
+ /*
+* enable VSYNC/HREF/PCLK DVP control lines
+* & D[9:6] DVP data lines
+*
+* PAD OUTPUT ENABLE 01
+* - 6: VSYNC output enable
+* - 5: HREF output enable
+* - 4: PCLK output enable
+* - [3:0]: D[9:6] output enable
+
[PATCH v4 1/5] media: ov5640: switch to gpiod_set_value_cansleep()
Switch gpiod_set_value to gpiod_set_value_cansleep to avoid
warnings when powering sensor.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index c89ed66..61071f5 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1524,7 +1524,7 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
static void ov5640_power(struct ov5640_dev *sensor, bool enable)
{
- gpiod_set_value(sensor->pwdn_gpio, enable ? 0 : 1);
+ gpiod_set_value_cansleep(sensor->pwdn_gpio, enable ? 0 : 1);
}
static void ov5640_reset(struct ov5640_dev *sensor)
@@ -1532,7 +1532,7 @@ static void ov5640_reset(struct ov5640_dev *sensor)
if (!sensor->reset_gpio)
return;
- gpiod_set_value(sensor->reset_gpio, 0);
+ gpiod_set_value_cansleep(sensor->reset_gpio, 0);
/* camera power cycle */
ov5640_power(sensor, false);
@@ -1540,10 +1540,10 @@ static void ov5640_reset(struct ov5640_dev *sensor)
ov5640_power(sensor, true);
usleep_range(5000, 1);
- gpiod_set_value(sensor->reset_gpio, 1);
+ gpiod_set_value_cansleep(sensor->reset_gpio, 1);
usleep_range(1000, 2000);
- gpiod_set_value(sensor->reset_gpio, 0);
+ gpiod_set_value_cansleep(sensor->reset_gpio, 0);
usleep_range(5000, 1);
}
--
1.9.1
[PATCH v4 3/5] media: dt-bindings: ov5640: refine CSI-2 and add parallel interface
Refine CSI-2 endpoint documentation and add bindings
for DVP parallel interface support.
Signed-off-by: Hugues Fruchet
---
.../devicetree/bindings/media/i2c/ov5640.txt | 48 +-
1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
index 540b36c..e26a846 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
@@ -1,4 +1,4 @@
-* Omnivision OV5640 MIPI CSI-2 sensor
+* Omnivision OV5640 MIPI CSI-2 / parallel sensor
Required Properties:
- compatible: should be "ovti,ov5640"
@@ -18,7 +18,27 @@ The device node must contain one 'port' child node for its
digital output
video port, in accordance with the video interface bindings defined in
Documentation/devicetree/bindings/media/video-interfaces.txt.
-Example:
+OV5640 can be connected to a MIPI CSI-2 bus or a parallel bus endpoint.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0)
+- data-lanes: should be set to <1> or <1 2> (one or two CSI-2 lanes supported)
+
+Endpoint node required properties for parallel connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- bus-width: shall be set to <8> for 8 bits parallel bus
+or <10> for 10 bits parallel bus
+- data-shift: shall be set to <2> for 8 bits parallel bus
+ (lines 9:2 are used) or <0> for 10 bits parallel bus
+
+Endpoint node optional properties for parallel connection are:
+- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
respectively.
+- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
respectively.
+- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock
+ signal.
+
+Examples:
&i2c1 {
ov5640: camera@3c {
@@ -35,6 +55,7 @@ Example:
reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
port {
+ /* MIPI CSI-2 bus endpoint */
ov5640_to_mipi_csi2: endpoint {
remote-endpoint = <&mipi_csi2_from_ov5640>;
clock-lanes = <0>;
@@ -43,3 +64,26 @@ Example:
};
};
};
+
+&i2c1 {
+ ov5640: camera@3c {
+ compatible = "ovti,ov5640";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ov5640>;
+ reg = <0x3c>;
+ clocks = <&clk_ext_camera>;
+ clock-names = "xclk";
+
+ port {
+ /* Parallel bus endpoint */
+ ov5640_to_parallel: endpoint {
+ remote-endpoint = <¶llel_from_ov5640>;
+ bus-width = <8>;
+ data-shift = <2>; /* lines 9:2 are used */
+ hsync-active = <0>;
+ vsync-active = <0>;
+ pclk-sample = <1>;
+ };
+ };
+ };
+};
--
1.9.1
[PATCH v4 0/5] Add OV5640 parallel interface and RGB565/YUYV support
Enhance OV5640 CSI driver to support also DVP parallel interface. Add RGB565 (LE & BE) and YUV422 YUYV format in addition to existing YUV422 UYVY format. Some other improvements on chip identifier check and removal of warnings in powering phase around gpio handling. === = history = === version 4: - Refine bindings as per Sakari suggestion: https://www.mail-archive.com/linux-media@vger.kernel.org/msg123609.html - Parallel port control lines polarity can now be configured through devicetree version 3: - Move chip identifier check at probe according to Fabio Estevam comment: https://www.mail-archive.com/linux-media@vger.kernel.org/msg122575.html - Use 16 bits register read for this check as per Steve Longerbeam comment: https://www.mail-archive.com/linux-media@vger.kernel.org/msg122692.html - Update bindings to document parallel mode support as per Fabio Estevam comment: https://www.mail-archive.com/linux-media@vger.kernel.org/msg122576.html - Enable the whole 10 bits parallel output and document 8/10 bits support in ov5640_set_stream_dvp() to answer to Steve Longerbeam comment: https://www.mail-archive.com/linux-media@vger.kernel.org/msg122693.html version 2: - Fix comments from Sakari Ailus: https://www.mail-archive.com/linux-media@vger.kernel.org/msg122259.html - Revisit ov5640_set_stream_dvp() to only configure DVP at streamon - Revisit ov5640_set_stream_dvp() implementation with fewer register settings version 1: - Initial submission Hugues Fruchet (5): media: ov5640: switch to gpiod_set_value_cansleep() media: ov5640: check chip id media: dt-bindings: ov5640: refine CSI-2 and add parallel interface media: ov5640: add support of DVP parallel interface media: ov5640: add support of RGB565 and YUYV formats .../devicetree/bindings/media/i2c/ov5640.txt | 48 ++- drivers/media/i2c/ov5640.c | 325 ++--- 2 files changed, 326 insertions(+), 47 deletions(-) -- 1.9.1
[PATCH v4 5/5] media: ov5640: add support of RGB565 and YUYV formats
Add RGB565 (LE & BE) and YUV422 YUYV format in addition
to existing YUV422 UYVY format.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 74 +-
1 file changed, 67 insertions(+), 7 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index a44b680..f017742 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -76,9 +76,11 @@
#define OV5640_REG_HZ5060_CTRL01 0x3c01
#define OV5640_REG_SIGMADELTA_CTRL0C 0x3c0c
#define OV5640_REG_FRAME_CTRL010x4202
+#define OV5640_REG_FORMAT_CONTROL000x4300
#define OV5640_REG_POLARITY_CTRL00 0x4740
#define OV5640_REG_MIPI_CTRL00 0x4800
#define OV5640_REG_DEBUG_MODE 0x4814
+#define OV5640_REG_ISP_FORMAT_MUX_CTRL 0x501f
#define OV5640_REG_PRE_ISP_TEST_SET1 0x503d
#define OV5640_REG_SDE_CTRL0 0x5580
#define OV5640_REG_SDE_CTRL1 0x5581
@@ -106,6 +108,18 @@ enum ov5640_frame_rate {
OV5640_NUM_FRAMERATES,
};
+struct ov5640_pixfmt {
+ u32 code;
+ u32 colorspace;
+};
+
+static const struct ov5640_pixfmt ov5640_formats[] = {
+ { MEDIA_BUS_FMT_UYVY8_2X8, V4L2_COLORSPACE_SRGB, },
+ { MEDIA_BUS_FMT_YUYV8_2X8, V4L2_COLORSPACE_SRGB, },
+ { MEDIA_BUS_FMT_RGB565_2X8_LE, V4L2_COLORSPACE_SRGB, },
+ { MEDIA_BUS_FMT_RGB565_2X8_BE, V4L2_COLORSPACE_SRGB, },
+};
+
/*
* FIXME: remove this when a subdev API becomes available
* to set the MIPI CSI-2 virtual channel.
@@ -1837,17 +1851,23 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev
*sd,
{
struct ov5640_dev *sensor = to_ov5640_dev(sd);
const struct ov5640_mode_info *mode;
+ int i;
mode = ov5640_find_mode(sensor, fr, fmt->width, fmt->height, true);
if (!mode)
return -EINVAL;
-
fmt->width = mode->width;
fmt->height = mode->height;
- fmt->code = sensor->fmt.code;
if (new_mode)
*new_mode = mode;
+
+ for (i = 0; i < ARRAY_SIZE(ov5640_formats); i++)
+ if (ov5640_formats[i].code == fmt->code)
+ break;
+ if (i >= ARRAY_SIZE(ov5640_formats))
+ fmt->code = ov5640_formats[0].code;
+
return 0;
}
@@ -1890,6 +1910,45 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
return ret;
}
+static int ov5640_set_framefmt(struct ov5640_dev *sensor,
+ struct v4l2_mbus_framefmt *format)
+{
+ int ret = 0;
+ bool is_rgb = false;
+ u8 val;
+
+ switch (format->code) {
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ /* YUV422, UYVY */
+ val = 0x3f;
+ break;
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ /* YUV422, YUYV */
+ val = 0x30;
+ break;
+ case MEDIA_BUS_FMT_RGB565_2X8_LE:
+ /* RGB565 {g[2:0],b[4:0]},{r[4:0],g[5:3]} */
+ val = 0x6F;
+ is_rgb = true;
+ break;
+ case MEDIA_BUS_FMT_RGB565_2X8_BE:
+ /* RGB565 {r[4:0],g[5:3]},{g[2:0],b[4:0]} */
+ val = 0x61;
+ is_rgb = true;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* FORMAT CONTROL00: YUV and RGB formatting */
+ ret = ov5640_write_reg(sensor, OV5640_REG_FORMAT_CONTROL00, val);
+ if (ret)
+ return ret;
+
+ /* FORMAT MUX CONTROL: ISP YUV or RGB */
+ return ov5640_write_reg(sensor, OV5640_REG_ISP_FORMAT_MUX_CTRL,
+ is_rgb ? 0x01 : 0x00);
+}
/*
* Sensor Controls.
@@ -2275,15 +2334,12 @@ static int ov5640_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
{
- struct ov5640_dev *sensor = to_ov5640_dev(sd);
-
if (code->pad != 0)
return -EINVAL;
- if (code->index != 0)
+ if (code->index >= ARRAY_SIZE(ov5640_formats))
return -EINVAL;
- code->code = sensor->fmt.code;
-
+ code->code = ov5640_formats[code->index].code;
return 0;
}
@@ -2299,6 +2355,10 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int
enable)
ret = ov5640_set_mode(sensor, sensor->current_mode);
if (ret)
goto out;
+
+ ret = ov5640_set_framefmt(sensor, &sensor->fmt);
+ if (ret)
+ goto out;
}
if (sensor->ep.bus_type == V4L2_MBUS_CSI2)
--
1.9.1
Re: [-next PATCH 0/4] sysfs and DEVICE_ATTR_
Hi, Joe Perches writes: > drivers/usb/phy/phy-tahvo.c| 2 +- Acked-by: Felipe Balbi -- balbi
Re: [RFC PATCH 9/9] media: vim2m: add request support
On Tue, Dec 19, 2017 at 5:53 AM, Gustavo Padovan wrote:
> Hi Alex,
>
> 2017-12-15 Alexandre Courbot :
>
>> Set the necessary ops for supporting requests in vim2m.
>>
>> Signed-off-by: Alexandre Courbot
>> ---
>> drivers/media/platform/vim2m.c | 22 ++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
>> index a32e8a7950eb..ffe94ef9214d 100644
>> --- a/drivers/media/platform/vim2m.c
>> +++ b/drivers/media/platform/vim2m.c
>> @@ -30,6 +30,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> MODULE_DESCRIPTION("Virtual device for mem2mem framework testing");
>> MODULE_AUTHOR("Pawel Osciak, ");
>> @@ -142,6 +143,7 @@ struct vim2m_dev {
>> struct video_device vfd;
>> #ifdef CONFIG_MEDIA_CONTROLLER
>> struct media_device mdev;
>> + struct media_request_queue *req_queue;
>> #endif
>>
>> atomic_tnum_inst;
>> @@ -937,6 +939,11 @@ static int vim2m_open(struct file *file)
>> goto open_unlock;
>> }
>>
>> +#ifdef CONFIG_MEDIA_CONTROLLER
>> + v4l2_mem_ctx_request_init(ctx->fh.m2m_ctx, dev->req_queue,
>> + &dev->vfd.entity);
>> +#endif
>> +
>> v4l2_fh_add(&ctx->fh);
>> atomic_inc(&dev->num_inst);
>>
>> @@ -992,6 +999,12 @@ static const struct v4l2_m2m_ops m2m_ops = {
>> .job_abort = job_abort,
>> };
>>
>> +#ifdef CONFIG_MEDIA_CONTROLLER
>> +static const struct media_entity_operations vim2m_entity_ops = {
>> + .process_request = v4l2_m2m_process_request,
>> +};
>> +#endif
>> +
>> static int vim2m_probe(struct platform_device *pdev)
>> {
>> struct vim2m_dev *dev;
>> @@ -1006,6 +1019,10 @@ static int vim2m_probe(struct platform_device *pdev)
>>
>> #ifdef CONFIG_MEDIA_CONTROLLER
>> dev->mdev.dev = &pdev->dev;
>> + dev->req_queue = media_request_queue_generic_alloc(&dev->mdev);
>> + if (IS_ERR(dev->req_queue))
>> + return PTR_ERR(dev->req_queue);
>> + dev->mdev.req_queue = dev->req_queue;
>> strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model));
>> media_device_init(&dev->mdev);
>> dev->v4l2_dev.mdev = &dev->mdev;
>> @@ -1023,6 +1040,11 @@ static int vim2m_probe(struct platform_device *pdev)
>> vfd->lock = &dev->dev_mutex;
>> vfd->v4l2_dev = &dev->v4l2_dev;
>>
>> +#ifdef CONFIG_MEDIA_CONTROLLER
>> + vfd->entity.ops = &vim2m_entity_ops;
>> + vfd->entity.req_ops = &media_entity_request_generic_ops;
>> +#endif
>> +
>
> It seems to me that we can avoid most of this patch and just setup the
> request support automatically when the media device node is registered.
> The less change we impose to drivers the better I think.
I'd like to move things more towards that direction, if possible. The
main blocker for now is that I want to keep the ability for drivers to
overload the request ops to fit their needs. But maybe we can just
allow some level of specialization if specified, while falling back to
sane defaults otherwise.
Re: [RFC PATCH 0/9] media: base request API support
On Sat, Dec 16, 2017 at 6:04 AM, Nicolas Dufresne wrote: > Le vendredi 15 décembre 2017 à 16:56 +0900, Alexandre Courbot a écrit : >> Here is a new attempt at the request API, following the UAPI we agreed on in >> Prague. Hopefully this can be used as the basis to move forward. >> >> This series only introduces the very basics of how requests work: allocate a >> request, queue buffers to it, queue the request itself, wait for it to >> complete, >> reuse it. It does *not* yet use Hans' work with controls setting. I have >> preferred to submit it this way for now as it allows us to concentrate on the >> basic request/buffer flow, which was harder to get properly than I initially >> thought. I still have a gut feeling that it can be improved, with less >> back-and- >> forth into drivers. >> >> Plugging in controls support should not be too hard a task (basically just >> apply >> the saved controls when the request starts), and I am looking at it now. >> >> The resulting vim2m driver can be successfully used with requests, and my >> tests >> so far have been successful. >> >> There are still some rougher edges: >> >> * locking is currently quite coarse-grained >> * too many #ifdef CONFIG_MEDIA_CONTROLLER in the code, as the request API >> depends on it - I plan to craft the headers so that it becomes unnecessary. >> As it is, some of the code will probably not even compile if >> CONFIG_MEDIA_CONTROLLER is not set > > Would it be possible to explain why this relation between request and > the media controller ? Why couldn't request be created from video > devices ? Laurent replied on IRC, but for the record this is because the media node can act as an orchestrator to the devices it manages. Some devices may have particular needs in that respect, and we want to be able to hook somewhere and control that. Another reason is that in the future the media framework will probably take more importance, to the point where you could control the whole chain of devices through it instead of having to open every node individually. This is a different topic though, and we only touched the surface of it during the latest mini-summit.
Re: [RFC PATCH 0/9] media: base request API support
On Sat, Dec 16, 2017 at 6:02 AM, Nicolas Dufresne wrote: > Le vendredi 15 décembre 2017 à 16:56 +0900, Alexandre Courbot a écrit : >> Here is a new attempt at the request API, following the UAPI we agreed on in >> Prague. Hopefully this can be used as the basis to move forward. >> >> This series only introduces the very basics of how requests work: allocate a >> request, queue buffers to it, queue the request itself, wait for it to >> complete, >> reuse it. It does *not* yet use Hans' work with controls setting. I have >> preferred to submit it this way for now as it allows us to concentrate on the >> basic request/buffer flow, which was harder to get properly than I initially >> thought. I still have a gut feeling that it can be improved, with less >> back-and- >> forth into drivers. >> >> Plugging in controls support should not be too hard a task (basically just >> apply >> the saved controls when the request starts), and I am looking at it now. >> >> The resulting vim2m driver can be successfully used with requests, and my >> tests >> so far have been successful. >> >> There are still some rougher edges: >> >> * locking is currently quite coarse-grained >> * too many #ifdef CONFIG_MEDIA_CONTROLLER in the code, as the request API >> depends on it - I plan to craft the headers so that it becomes unnecessary. >> As it is, some of the code will probably not even compile if >> CONFIG_MEDIA_CONTROLLER is not set >> >> But all in all I think the request flow should be clear and easy to review, >> and >> the possibility of custom queue and entity support implementations should >> give >> us the flexibility we need to support more specific use-cases (I expect the >> generic implementations to be sufficient most of the time though). >> >> A very simple test program exercising this API is available here (don't >> forget >> to adapt the /dev/media0 hardcoding): >> https://gist.github.com/Gnurou/dbc3776ed97ea7d4ce6041ea15eb0438 > > It looks like the example uses Hans control work you just mention. > Notably, it uses v4l2_ext_controls ctrls.request. Oops, I indeed forgot to remove that bit. You will want to edit that line out if trying to compile.
Re: [PATCH] build: Disabled MEDIA_TUNER_TDA18250 for Kernels older than 4.3
Hi! I could test only the compilation for Kernel 3.13 due to my laptop I am using on vacation. It should compile now for all Kernels from 4.2 .. 3.13 also. BR, Jasmin
[PATCH] build: Disabled MEDIA_TUNER_TDA18250 for Kernels older than 4.3
From: Jasmin Jessich Due to recent changes the TDA18250 driver requires now regmap_write_bits, which is available since Kernel 4.3 only. Signed-off-by: Jasmin Jessich --- v4l/versions.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/v4l/versions.txt b/v4l/versions.txt index 072e7d6..6885a05 100644 --- a/v4l/versions.txt +++ b/v4l/versions.txt @@ -35,6 +35,8 @@ VIDEO_SOLO6X10 [4.2.0] # needs led_trigger_remove V4L2_FLASH_LED_CLASS +# needs regmap_write_bits +MEDIA_TUNER_TDA18250 [4.0.0] # needs of_property_read_u64_array @@ -86,8 +88,6 @@ VIDEO_COBALT [3.13.0] # needs gpio/consumer.h RADIO_SI4713 -# needs regmap_reg_range -MEDIA_TUNER_TDA18250 [3.12.0] # BIN_ATTR_RW was changed -- 2.7.4
Re: [PATCH v1 05/10] staging: atomisp: Remove non-ACPI leftovers
On Tue, Dec 19, 2017 at 10:59:52PM +0200, Andy Shevchenko wrote: > @@ -914,9 +904,7 @@ static int lm3554_probe(struct i2c_client *client) > dev_err(&client->dev, "gpio request/direction_output fail"); > goto fail2; > } > - if (ACPI_HANDLE(&client->dev)) > - err = atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); > - return 0; > + return atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); > fail2: > media_entity_cleanup(&flash->sd.entity); > v4l2_ctrl_handler_free(&flash->ctrl_handler); Actually every place where we directly return a function call is wrong and needs error handling added. I've been meaning to write a Smatch check for this because it's a common anti-pattern we don't check the last function call for errors. Someone could probably do the same in Coccinelle if they want. regards, dan carpenter
