Re: [PATCH v2 6/6] dt-bindings: media: mt9m111: add pclk-sample property

[Rob Herring]

On Mon, 29 Oct 2018 19:24:10 +0100, Marco Felsch wrote:
> Add the pclk-sample property to the list of optional properties
> for the mt9m111 camera sensor.
> 
> Signed-off-by: Marco Felsch 
> ---
>  Documentation/devicetree/bindings/media/i2c/mt9m111.txt | 5 +
>  1 file changed, 5 insertions(+)
> 

Reviewed-by: Rob Herring 

Re: [PATCH v2 5/6] dt-bindings: media: mt9m111: adapt documentation to be more clear

[Rob Herring]

On Mon, Oct 29, 2018 at 07:24:09PM +0100, Marco Felsch wrote:
> Replace the vague binding by a more verbose. Remove the remote property
> from the example since the driver don't support such a property. Also
> remove the bus-width property from the endpoint since the driver don't
> take care of it.
> 
> Signed-off-by: Marco Felsch 
> ---
>  .../devicetree/bindings/media/i2c/mt9m111.txt | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/mt9m111.txt 
> b/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
> index 6b910036b57e..921cc48c488b 100644
> --- a/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
> @@ -9,8 +9,13 @@ Required Properties:
>  - clocks: reference to the master clock.
>  - clock-names: shall be "mclk".
>  
> -For further reading on port node refer to
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> +The device node must contain one 'port' child node with one 'endpoint' child
> +sub-node for its digital output video port, in accordance with the video
> +interface bindings defined in:
> +Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Required endpoint properties:
> +- remote-endpoint: For information see ../video-interfaces.txt.

You don't need to explicitly document this.

Otherwise,

Reviewed-by: Rob Herring 

Re: [PATCH v3 1/2] dt-bindings: media: Add Aspeed Video Engine binding documentation

[Rob Herring]

On Tue, 25 Sep 2018 14:27:08 -0500, Eddie James wrote:
> Document the bindings.
> 
> Signed-off-by: Eddie James 
> ---
>  .../devicetree/bindings/media/aspeed-video.txt | 26 
> ++
>  1 file changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt
> 

Reviewed-by: Rob Herring 

Re: [PATCH v4 3/7] [media] ad5820: DT new optional field enable-gpios

[Rob Herring]

On Thu, Sep 20, 2018 at 10:47:47PM +0200, Ricardo Ribalda Delgado wrote:
> Document new enable-gpio field. It can be used to disable the part

enable-gpios

> without turning down its regulator.
> 
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado 
> Acked-by: Pavel Machek 
> ---
>  Documentation/devicetree/bindings/media/i2c/ad5820.txt | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt 
> b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> index 5940ca11c021..9ccd96d3d5f0 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> @@ -8,6 +8,12 @@ Required Properties:
>  
>- VANA-supply: supply of voltage for VANA pin
>  
> +Optional properties:
> +
> +   - enable-gpios : GPIO spec for the XSHUTDOWN pin. Note that the polarity 
> of
> +the enable GPIO is the opposite of the XSHUTDOWN pin (asserting the enable
> +GPIO deasserts the XSHUTDOWN signal and vice versa).

shutdown-gpios is also standard and seems like it would make more sense 
here. Yes, it is a bit redundant to have both, but things just evolved 
that way and we don't want to totally abandon the hardware names (just 
all the variants).

Rob

Re: [PATCH v4 7/7] [media] ad5820: DT new compatible devices

[Rob Herring]

On Thu, 20 Sep 2018 22:47:51 +0200, Ricardo Ribalda Delgado wrote:
> Document new compatible devices.
> 
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado 
> ---
>  Documentation/devicetree/bindings/media/i2c/ad5820.txt | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring 

Re: [PATCH v3 4/9] media: dt-bindings: tvp5150: Add input port connectors DT bindings

[Rob Herring]

On Tue, 18 Sep 2018 15:14:48 +0200, Marco Felsch wrote:
> The TVP5150/1 decoders support different video input sources to their
> AIP1A/B pins.
> 
> Possible configurations are as follows:
>   - Analog Composite signal connected to AIP1A.
>   - Analog Composite signal connected to AIP1B.
>   - Analog S-Video Y (luminance) and C (chrominance)
> signals connected to AIP1A and AIP1B respectively.
> 
> This patch extends the device tree bindings documentation to describe
> how the input connectors for these devices should be defined in a DT.
> 
> Signed-off-by: Marco Felsch 
> ---
> Changelog:
> 
> v3:
> - remove examples for one and two inputs
> - replace space by tabs
> 
> v2:
> - adapt port layout in accordance with
>   https://www.spinics.net/lists/linux-media/msg138546.html with the
>   svideo-connector deviation (use only one endpoint)
> 
>  .../devicetree/bindings/media/i2c/tvp5150.txt | 92 +++++++++--
>  1 file changed, 85 insertions(+), 7 deletions(-)
> 

Reviewed-by: Rob Herring 

Re: [PATCH v3 1/9] partial revert of "[media] tvp5150: add HW input connectors support"

[Rob Herring]

On Tue, Sep 18, 2018 at 03:14:45PM +0200, Marco Felsch wrote:
> From: Javier Martinez Canillas 
> 
> Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support")
> added input signals support for the tvp5150, but the approach was found
> to be incorrect so the corresponding DT binding commit 82c2ffeb217a
> ("[media] tvp5150: document input connectors DT bindings") was reverted.
> 
> This left the driver with an undocumented (and wrong) DT parsing logic,
> so lets get rid of this code as well until the input connectors support
> is implemented properly.
> 
> It's a partial revert due other patches added on top of mentioned commit
> not allowing the commit to be reverted cleanly anymore. But all the code
> related to the DT parsing logic and input entities creation are removed.
> 
> Suggested-by: Laurent Pinchart 
> Signed-off-by: Javier Martinez Canillas 
> Acked-by: Laurent Pinchart 
> [m.fel...@pengutronix.de: rm TVP5150_INPUT_NUM define]
> Signed-off-by: Marco Felsch 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/i2c/tvp5150.c | 141 
>  include/dt-bindings/media/tvp5150.h |   2 -
>  2 files changed, 143 deletions(-)

Acked-by: Rob Herring 

Re: [PATCH v2 04/23] dt-bindings: media: Specify bus type for MIPI D-PHY, others, explicitly

[Rob Herring]

On Mon, 27 Aug 2018 12:29:41 +0300, Sakari Ailus wrote:
> Allow specifying the bus type explicitly for MIPI D-PHY, parallel and
> Bt.656 busses. This is useful for devices that can make use of different
> bus types. There are CSI-2 transmitters and receivers but the PHY
> selection needs to be made between C-PHY and D-PHY; many devices also
> support parallel and Bt.656 interfaces but the means to pass that
> information to software wasn't there.
> 
> Autodetection (value 0) is removed as an option as the property could be
> simply omitted in that case.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring 

Re: [PATCH 05/21] dt-bindings: media: Specify bus type for MIPI D-PHY, others, explicitly

[Rob Herring]

On Thu, Aug 16, 2018 at 3:17 AM Sakari Ailus
 wrote:
>
> Ping?
>
> On Wed, Aug 01, 2018 at 02:16:27PM +0300, Sakari Ailus wrote:
> > Hi Rob,
> >
> > Thanks for the review.
> >
> > On Tue, Jul 31, 2018 at 03:32:10PM -0600, Rob Herring wrote:
> > > On Mon, Jul 23, 2018 at 04:46:50PM +0300, Sakari Ailus wrote:
> > > > Allow specifying the bus type explicitly for MIPI D-PHY, parallel and
> > > > Bt.656 busses. This is useful for devices that can make use of different
> > > > bus types. There are CSI-2 transmitters and receivers but the PHY
> > > > selection needs to be made between C-PHY and D-PHY; many devices also
> > > > support parallel and Bt.656 interfaces but the means to pass that
> > > > information to software wasn't there.
> > > >
> > > > Autodetection (value 0) is removed as an option as the property could be
> > > > simply omitted in that case.
> > >
> > > Presumably there are users, so you can't remove it. But documenting
> > > behavior when absent would be good.
> >
> > Well, it's effectively the same as having no such property at all: the type
> > is not specified. Generally there are two possibilities: the hardware
> > supports just a single bus or it supports more than one. If there's just
> > one, the type can be known by the driver. In that case there's no use for
> > autodetection.
> >
> > The second case is a bit more complicated: the bus type detection is solely
> > based on properties available in the endpoint, and I think that may have
> > been feasible approach when there were just parallel and Bt.656 busses that
> > were supported, but with the additional busses, the V4L2 fwnode framework
> > may no longer guess the bus in any meaningful way from the available
> > properties. I'd think the only known-good option here is to specify the
> > type explicitly in that case: there's no room for guessing. (This patchset
> > makes it possible for drivers to explicitly define the bus type, but the
> > autodetection support is maintained for backwards compatibility.)
> >
> > One of the existing issues is that there are combined parallel/Bt.656
> > receivers that need to know the type of the bus. This is based on the
> > existence parallel interface only properties: if any of these exist, then
> > the interface is parallel, otherwise it is Bt.656. The DT bindings for the
> > same devices also define the defaults for the parallel interface. This
> > leaves the end result ambiguous: is it the parallel interface with the
> > default configuration or is it Bt.656?
> >
> > There will likely be similar issues for CSI-2 D-PHY and CSI-2 C-PHY. The
> > question there would be: is this CSI-2 C-PHY or CSI-2 D-PHY with default
> > clock lane configuration?
> >
> > In either case the autodetection option for the bus type provides no useful
> > information. If it exists in DT source, that's fine, there's just no use
> > for it.
> >
> > Let me know if you still think it should be maintained in binding
> > documentation.
>
> If you prefer to keep it, I'd propose to mark it as deprecated or something
> as it provides no information to software.

Looks like there's only one user in tree:

arch/arm/boot/dts/omap3-n900.dts:
bus-type = <3>; /* CCP2 */
arch/arm/boot/dts/omap3-n900.dts:
bus-type = <3>; /* CCP2 */

So I guess removing is okay.

Rob

Re: [PATCH 1/3] dt-bindings: media: Add i.MX Pixel Pipeline binding

[Rob Herring]

On Fri, 10 Aug 2018 17:18:20 +0200, Philipp Zabel wrote:
> Add DT binding documentation for the Pixel Pipeline (PXP) found on
> various NXP i.MX SoCs.
> 
> Signed-off-by: Philipp Zabel 
> ---
>  .../devicetree/bindings/media/fsl-pxp.txt | 26 +++
>  1 file changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
> 

Reviewed-by: Rob Herring 

Re: [PATCH v2 2/7] [media] dt-bindings: tvp5150: Add input port connectors DT bindings

[Rob Herring]

On Mon, Aug 13, 2018 at 11:25:03AM +0200, Marco Felsch wrote:
> The TVP5150/1 decoders support different video input sources to their
> AIP1A/B pins.
> 
> Possible configurations are as follows:
>   - Analog Composite signal connected to AIP1A.
>   - Analog Composite signal connected to AIP1B.
>   - Analog S-Video Y (luminance) and C (chrominance)
> signals connected to AIP1A and AIP1B respectively.
> 
> This patch extends the device tree bindings documentation to describe
> how the input connectors for these devices should be defined in a DT.
> 
> Signed-off-by: Marco Felsch 
> 
> ---
> Changelog:
> v2:
> - adapt port layout in accordance with
>   https://www.spinics.net/lists/linux-media/msg138546.html with the
>   svideo-connector deviation (use only one endpoint)
> ---
>  .../devicetree/bindings/media/i2c/tvp5150.txt | 191 +-
>  1 file changed, 185 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt 
> b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> index 8c0fc1a26bf0..d647d671f14a 100644
> --- a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> @@ -12,11 +12,31 @@ Optional Properties:
>  - pdn-gpios: phandle for the GPIO connected to the PDN pin, if any.
>  - reset-gpios: phandle for the GPIO connected to the RESETB pin, if any.
>  
> -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.
> +The device node must contain one 'port' child node per device physical input
> +and output port, in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
> +are numbered as follows
>  
> -Required Endpoint Properties for parallel synchronization:
> +   Name  TypePort
> + --
> +   AIP1A sink0
> +   AIP1B sink1
> +   Y-OUT src 2
> +
> +The device node must contain at least one sink port and the src port. Each 
> input
> +port must be linked to an endpoint defined in
> +Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt. 
> The
> +port/connector layout is as follows
> +
> +tvp-5150 port@0 (AIP1A)
> + endpoint@0 ---> Comp0-Con  port
> + endpoint@1 ---> Svideo-Con port
> +tvp-5150 port@1 (AIP1B)
> + endpoint   ---> Comp1-Con  port
> +tvp-5150 port@2
> + endpoint (video bitstream output at YOUT[0-7] parallel bus)
> +
> +Required Endpoint Properties for parallel synchronization on output port:
>  
>  - hsync-active: active state of the HSYNC signal. Must be <1> (HIGH).
>  - vsync-active: active state of the VSYNC signal. Must be <1> (HIGH).
> @@ -26,7 +46,140 @@ Required Endpoint Properties for parallel synchronization:
>  If none of hsync-active, vsync-active and field-even-active is specified,
>  the endpoint is assumed to use embedded BT.656 synchronization.
>  
> -Example:
> +Examples:

Is it really necessary to enumerate every possibility? Just show the 
most complicated case which is a superset of the rest.

> +
> +One Input:
> +
> +connector {
> + compatible = "composite-video-connector";
> + label = "Composite0";
> +
> + port {
> + composite0_to_tvp5150: endpoint {
> + remote-endpoint = <_to_composite0>;
> + };
> + };
> +};
> +
> + {
> + ...
> + tvp5150@5c {
> + compatible = "ti,tvp5150";
> + reg = <0x5c>;
> + pdn-gpios = < 30 GPIO_ACTIVE_LOW>;
> + reset-gpios = < 7 GPIO_ACTIVE_LOW>;
> +
> + port@0 {
> + reg = <0>;
> +
> + tvp5150_to_composite0: endpoint {
> + remote-endpoint = <_to_tvp5150>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> +
> + tvp5150_1: endpoint {
> + remote-endpoint = <_ep>;
> + };
> + };
> + };
> +};
> +
> +Two Inputs:
> +
> +comp_connector_1 {
> + compatible = "composite-video-connector";
> + label = "Composite1";
> +
> + port {
> + composite1_to_tvp5150: endpoint {
> + remote-endpoint = <_to_composite1>;
> + };
> + };
> +};
> +
> +svid_connector {
> + compatible = "svideo-connector";
> + label = "S-Video";
> +
> + port {
> + svideo_to_tvp5150: endpoint {
> + remote-endpoint = <_to_svideo>;
> + };
> + };
> +};
> +
> + {
> + ...
> + tvp5150@5c {
> + compatible = "ti,tvp5150";
> + reg = <0x5c>;
> + 

Re: [PATCH v2 1/6] dt-bindings: Document the Rockchip VPU bindings

[Rob Herring]

On Thu, Aug 02, 2018 at 05:00:05PM -0300, Ezequiel Garcia wrote:
> Add devicetree binding documentation for Rockchip Video Processing
> Unit IP.
> 
> Signed-off-by: Ezequiel Garcia 
> ---
>  .../bindings/media/rockchip-vpu.txt   | 30 +++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/rockchip-vpu.txt

Reviewed-by: Rob Herring 

Re: [PATCH 05/21] dt-bindings: media: Specify bus type for MIPI D-PHY, others, explicitly

[Rob Herring]

On Mon, Jul 23, 2018 at 04:46:50PM +0300, Sakari Ailus wrote:
> Allow specifying the bus type explicitly for MIPI D-PHY, parallel and
> Bt.656 busses. This is useful for devices that can make use of different
> bus types. There are CSI-2 transmitters and receivers but the PHY
> selection needs to be made between C-PHY and D-PHY; many devices also
> support parallel and Bt.656 interfaces but the means to pass that
> information to software wasn't there.
> 
> Autodetection (value 0) is removed as an option as the property could be
> simply omitted in that case.

Presumably there are users, so you can't remove it. But documenting 
behavior when absent would be good.

> 
> Signed-off-by: Sakari Ailus 
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
> b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index baf9d9756b3c..f884ada0bffc 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -100,10 +100,12 @@ Optional endpoint properties
>slave device (data source) by the master device (data sink). In the master
>mode the data source device is also the source of the synchronization 
> signals.
>  - bus-type: data bus type. Possible values are:
> -  0 - autodetect based on other properties (MIPI CSI-2 D-PHY, parallel or 
> Bt656)
>1 - MIPI CSI-2 C-PHY
>2 - MIPI CSI1
>3 - CCP2
> +  4 - MIPI CSI-2 D-PHY
> +  5 - Parallel

Is that really specific enough to be useful?

> +  6 - Bt.656
>  - bus-width: number of data lines actively used, valid for the parallel 
> busses.
>  - data-shift: on the parallel data busses, if bus-width is used to specify 
> the
>number of data lines, data-shift can be used to specify which data lines 
> are
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] cec-gpio.txt: add v5-gpios for testing the 5V line

[Rob Herring]

On Tue, Jul 17, 2018 at 03:29:05PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> In order to debug the HDMI 5V line we need to add a new v5-gpios
> property.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  .../devicetree/bindings/media/cec-gpio.txt  | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/cec-gpio.txt 
> b/Documentation/devicetree/bindings/media/cec-gpio.txt
> index 12fcd55ed153..1d53ce89da74 100644
> --- a/Documentation/devicetree/bindings/media/cec-gpio.txt
> +++ b/Documentation/devicetree/bindings/media/cec-gpio.txt
> @@ -4,8 +4,8 @@ The HDMI CEC GPIO module supports CEC implementations where 
> the CEC line
>  is hooked up to a pull-up GPIO line and - optionally - the HPD line is
>  hooked up to another GPIO line.
>  
> -Please note: the maximum voltage for the CEC line is 3.63V, for the HPD
> -line it is 5.3V. So you may need some sort of level conversion circuitry
> +Please note: the maximum voltage for the CEC line is 3.63V, for the HPD and
> +5V lines it is 5.3V. So you may need some sort of level conversion circuitry
>  when connecting them to a GPIO line.
>  
>  Required properties:
> @@ -22,15 +22,18 @@ If the CEC line is not associated with an HDMI 
> receiver/transmitter, then
>  the following property is optional:
>  
>- hpd-gpios: gpio that the HPD line is connected to.
> +  - v5-gpios: gpio that the 5V line is connected to.

This is a bit strange without the context of debugging. So can you 
mention that here.

With that,

Reviewed-by: Rob Herring 

>  
>  Example for the Raspberry Pi 3 where the CEC line is connected to
> -pin 26 aka BCM7 aka CE1 on the GPIO pin header and the HPD line is
> -connected to pin 11 aka BCM17 (some level shifter is needed for this!):
> +pin 26 aka BCM7 aka CE1 on the GPIO pin header, the HPD line is
> +connected to pin 11 aka BCM17 and the 5V line is connected to pin
> +15 aka BCM22 (some level shifter is needed for the HPD and 5V lines!):
>  
>  #include 
>  
>  cec-gpio {
> -   compatible = "cec-gpio";
> -   cec-gpios = < 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> -   hpd-gpios = < 17 GPIO_ACTIVE_HIGH>;
> + compatible = "cec-gpio";
> + cec-gpios = < 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> + hpd-gpios = < 17 GPIO_ACTIVE_HIGH>;
> + v5-gpios = < 22 GPIO_ACTIVE_HIGH>;
>  };
> -- 
> 2.18.0
> 

Re: [PATCH v7 1/2] media: ov2680: dt: Add bindings for OV2680

[Rob Herring]

On Wed, Jul 4, 2018 at 9:52 AM Rui Miguel Silva  wrote:
>
> Hi Jacopo,
> Hope your fine.
> Thanks for the review.
>
> On Wed 04 Jul 2018 at 09:58, jacopo mondi wrote:
> > Hi Rui,
> >sorry, I'm a bit late, you're already at v7 and I don't want
> >to
> > slow down inclusion with a few minor comments.
> >
> > Please bear with me and see below...
> >
> > On Tue, Jul 03, 2018 at 03:08:02PM +0100, Rui Miguel Silva
> > wrote:
> >> Add device tree binding documentation for the OV2680 camera
> >> sensor.
> >>
> >> CC: devicet...@vger.kernel.org
> >> Signed-off-by: Rui Miguel Silva 
> >> ---
> >>  .../devicetree/bindings/media/i2c/ov2680.txt  | 46
> >>  +++
> >>  1 file changed, 46 insertions(+)
> >>  create mode 100644
> >>  Documentation/devicetree/bindings/media/i2c/ov2680.txt
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> >> b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> >> new file mode 100644
> >> index ..11e925ed9dad
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> >> @@ -0,0 +1,46 @@
> >> +* Omnivision OV2680 MIPI CSI-2 sensor
> >> +
> >> +Required Properties:
> >> +- compatible: should be "ovti,ov2680".
> >> +- clocks: reference to the xvclk input clock.
> >> +- clock-names: should be "xvclk".
> >
> > Having a single clock source I think you can omit 'clock-names'
> > (or at
> > least not marking it as required)
>
> yeah, I see you point, but really all other OV sensors share this
> and
> the bellow clock/data-lanes properties as required, I will let Rob
> or
> Sakari take a call in this one.

I generally tell folks that *-names is not needed when there's only 1,
but having alignment across bindings is good too.

Rob

Re: [PATCH 20/22] [media] tvp5150: Add input port connectors DT bindings

[Rob Herring]

On Thu, Jun 28, 2018 at 06:20:52PM +0200, Marco Felsch wrote:
> The TVP5150/1 decoders support different video input sources to their
> AIP1A/B pins.
> 
> Possible configurations are as follows:
>   - Analog Composite signal connected to AIP1A.
>   - Analog Composite signal connected to AIP1B.
>   - Analog S-Video Y (luminance) and C (chrominance)
> signals connected to AIP1A and AIP1B respectively.
> 
> This patch extends the device tree bindings documentation to describe
> how the input connectors for these devices should be defined in a DT.
> 
> Signed-off-by: Marco Felsch 
> ---
>  .../devicetree/bindings/media/i2c/tvp5150.txt | 118 +-
>  1 file changed, 113 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt 
> b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> index 8c0fc1a26bf0..feed8c911c5e 100644
> --- a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> @@ -12,11 +12,23 @@ Optional Properties:
>  - pdn-gpios: phandle for the GPIO connected to the PDN pin, if any.
>  - reset-gpios: phandle for the GPIO connected to the RESETB pin, if any.
>  
> -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.
> +The device node must contain one 'port' child node per device input and 
> output
> +port, in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
> +are numbered as follows
>  
> -Required Endpoint Properties for parallel synchronization:
> +   Name  TypePort
> + --
> +   AIP1A sink0
> +   AIP1B sink1
> +   S-VIDEO   sink2
> +   Y-OUT src 3
> +
> +The device node must contain at least the Y-OUT port. Each input port must be
> +linked to an endpoint defined in
> +Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt.
> +
> +Required Endpoint Properties for parallel synchronization on output port:
>  
>  - hsync-active: active state of the HSYNC signal. Must be <1> (HIGH).
>  - vsync-active: active state of the VSYNC signal. Must be <1> (HIGH).
> @@ -26,7 +38,9 @@ Required Endpoint Properties for parallel synchronization:
>  If none of hsync-active, vsync-active and field-even-active is specified,
>  the endpoint is assumed to use embedded BT.656 synchronization.
>  
> -Example:
> +Examples:
> +
> +Only Output:
>  
>   {
>   ...
> @@ -37,6 +51,100 @@ Example:
>   reset-gpios = < 7 GPIO_ACTIVE_LOW>;
>  
>   port {
> + reg = <3>;
> + tvp5150_1: endpoint {
> + remote-endpoint = <_ep>;
> + };
> + };
> + };
> +};
> +
> +One Input:
> +
> +connector@0 {

Drop the unit-address as there is no reg property.

> + compatible = "composite-video-connector";
> + label = "Composite0";
> +
> + port {
> + comp0_out: endpoint {
> + remote-endpoint = <_comp0_in>;
> + };
> + };
> +};
> +
> + {
> + ...
> + tvp5150@5c {
> + compatible = "ti,tvp5150";
> + reg = <0x5c>;
> + pdn-gpios = < 30 GPIO_ACTIVE_LOW>;
> + reset-gpios = < 7 GPIO_ACTIVE_LOW>;
> +
> + port@0 {
> + reg = <0>;
> + tvp5150_comp0_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> +
> + port@3 {
> + reg = <3>;
> + tvp5150_1: endpoint {
> + remote-endpoint = <_ep>;
> + };
> + };
> + };
> +};
> +
> +
> +Two Inputs, different connector 12 on input AIP1A:
> +
> +connector@1 {

ditto

> + compatible = "svideo-connector";
> + label = "S-Video";
> +
> + port {
> + svideo_out: endpoint {
> + remote-endpoint = <_svideo_in>;
> + };
> + };
> +};
> +
> +connector@12 {

ditto

> + compatible = "composite-video-connector";
> + label = "Composite12";
> +
> + port {
> + comp12_out: endpoint {
> + remote-endpoint = <_comp12_in>;
> + };
> + };
> +};
> +
> + {
> + ...
> + tvp5150@5c {
> + compatible = "ti,tvp5150";
> + reg = <0x5c>;
> + pdn-gpios = < 30 GPIO_ACTIVE_LOW>;
> + reset-gpios = < 7 GPIO_ACTIVE_LOW>;
> +
> + port@0 {
> + reg = <0>;
> + tvp5150_comp12_in: endpoint {
> + 

Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"

[Rob Herring]

On Thu, Jun 28, 2018 at 06:20:50PM +0200, Marco Felsch wrote:
> From: Javier Martinez Canillas 
> 
> Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support")
> added input signals support for the tvp5150, but the approach was found
> to be incorrect so the corresponding DT binding commit 82c2ffeb217a
> ("[media] tvp5150: document input connectors DT bindings") was reverted.
> 
> This left the driver with an undocumented (and wrong) DT parsing logic,
> so lets get rid of this code as well until the input connectors support
> is implemented properly.
> 
> It's a partial revert due other patches added on top of mentioned commit
> not allowing the commit to be reverted cleanly anymore. But all the code
> related to the DT parsing logic and input entities creation are removed.
> 
> Suggested-by: Laurent Pinchart 
> Signed-off-by: Javier Martinez Canillas 
> Acked-by: Laurent Pinchart 
> [m.fel...@pengutronix.de: rm TVP5150_INPUT_NUM define]
> Signed-off-by: Marco Felsch 
> ---
>  drivers/media/i2c/tvp5150.c | 141 ------------

>  include/dt-bindings/media/tvp5150.h |   2 -

Acked-by: Rob Herring 

>  2 files changed, 143 deletions(-)

Re: [PATCH v7 1/2] media: ov2680: dt: Add bindings for OV2680

[Rob Herring]

On Tue, Jul 03, 2018 at 03:08:02PM +0100, Rui Miguel Silva wrote:
> Add device tree binding documentation for the OV2680 camera sensor.
> 
> CC: devicet...@vger.kernel.org
> Signed-off-by: Rui Miguel Silva 
> ---
>  .../devicetree/bindings/media/i2c/ov2680.txt  | 46 +++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

Please add acks/reviews when posting new versions.

Rob

Re: [PATCH v2 2/3] dt-bindings: ov5640: Add "rotation" property

[Rob Herring]

On Mon, Jun 18, 2018 at 12:29:18PM +0200, Hugues Fruchet wrote:
> Add the rotation property to the list of optional properties
> for the OV5640 camera sensor.
> 
> Signed-off-by: Hugues Fruchet 
> ---
>  Documentation/devicetree/bindings/media/i2c/ov5640.txt | 5 +
>  1 file changed, 5 insertions(+)

Reviewed-by: Rob Herring 

Re: [PATCH 2/2] media: ov5640: add support of module orientation

[Rob Herring]

On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
> Add support of module being physically mounted upside down.
> In this case, mirror and flip are enabled to fix captured images
> orientation.
> 
> Signed-off-by: Hugues Fruchet 
> ---
>  .../devicetree/bindings/media/i2c/ov5640.txt   |  3 +++

Please split bindings to separate patches.

>  drivers/media/i2c/ov5640.c | 28 
> --
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> index 8e36da0..f76eb7e 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> @@ -13,6 +13,8 @@ Optional Properties:
>  This is an active low signal to the OV5640.
>  - powerdown-gpios: reference to the GPIO connected to the powerdown pin,
>  if any. This is an active high signal to the OV5640.
> +- rotation: integer property; valid values are 0 (sensor mounted upright)
> + and 180 (sensor mounted upside down).

Didn't we just add this as a common property? If so, just reference the 
common definition. If not, it needs a common definition.

Re: [PATCH 1/1] dw9807: Use the dongwoon,dw9807-vcm compatible string

[Rob Herring]

On Tue, May 29, 2018 at 03:25:54PM +0300, Sakari Ailus wrote:
> The original dw9807 DT bindings patch proposed the dongwoon,dw9807
> compatible string. However, the device also includes an EEPROM on a
> different I²C address. Indicate that this is just the VCM part of the
> entire device.
> 
> The EEPROM part is compatible with the at24c64 for read-only access, with
> 1 kiB page size.
> 
> Signed-off-by: Sakari Ailus 
> ---
> Hi Rob, others,
> 
> The original bindings were missing the EEPROM bit. This change recognises
> it's there, and allows adding more elaborate support for it later on if
> needed.
> 
> If this change is fine, I'll squash it to the original patches that are
> not yet merged:

Looks fine to me.

> 
> https://patchwork.linuxtv.org/patch/49613/>
> https://patchwork.linuxtv.org/patch/49614/>
> 
> Thanks.
> 
>  Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 2 +-
>  drivers/media/i2c/dw9807.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Re: [PATCH v2.2 2/2] smiapp: Support the "rotation" property

[Rob Herring]

On Fri, May 25, 2018 at 04:52:35PM +0300, Sakari Ailus wrote:
> Use the "rotation" property to tell that the sensor is mounted upside
> down. This reverses the behaviour of the VFLIP and HFLIP controls as well
> as the pixel order.
> 
> Signed-off-by: Sakari Ailus 
> ---
> since v2.2:
> 
> - Fix property name in code.
> 
>  .../devicetree/bindings/media/i2c/nokia,smia.txt |  2 ++

Acked-by: Rob Herring 

>  drivers/media/i2c/smiapp/smiapp-core.c   | 16 
> 
>  2 files changed, 18 insertions(+)

Re: [PATCH v2 1/2] dt-bindings: media: Define "rotation" property for sensors

[Rob Herring]

On Fri, May 25, 2018 at 03:27:25PM +0300, Sakari Ailus wrote:
> Sensors are occasionally mounted upside down to systems such as mobile
> phones or tablets. In order to use such a sensor without having to turn
> every image upside down, most camera sensors support reversing the readout
> order by setting both horizontal and vertical flipping.
> 
> This patch documents the "rotation" property for camera sensors, mirroring
> what is defined for displays in
> Documentation/devicetree/bindings/display/panel/panel.txt .
> 
> Signed-off-by: Sakari Ailus 
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 4 
>  1 file changed, 4 insertions(+)

Reviewed-by: Rob Herring 

Re: [RESEND PATCH V2 1/2] dt-bindings: Add bindings for AKM ak7375 voice coil lens

[Rob Herring]

On Fri, May 25, 2018 at 05:55:34PM +0800, bingbu@intel.com wrote:
> From: Bingbu Cao 
> 
> Add device tree bindings for AKM ak7375 voice coil lens
> driver. This chip is used to drive a lens in a camera module.
> 
> Signed-off-by: Tianshu Qiu 
> Signed-off-by: Bingbu Cao 
> ---
> Changes since v1:
>  - remove the vendor prefix change
> ---
>  Documentation/devicetree/bindings/media/i2c/ak7375.txt | 8 
>  1 file changed, 8 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ak7375.txt

Reviewed-by: Rob Herring 

Re: [PATCH v6 1/2] media: ov2680: dt: Add bindings for OV2680

[Rob Herring]

On Fri, May 18, 2018 at 10:27 AM, Rui Miguel Silva <rmf...@gmail.com> wrote:
> Hi Rob,
> On Fri 18 May 2018 at 14:18, Rob Herring wrote:
>>
>> On Wed, May 09, 2018 at 03:31:58PM +0100, Rui Miguel Silva wrote:
>>>
>>> Add device tree binding documentation for the OV2680 camera sensor.
>>>
>>> CC: devicet...@vger.kernel.org
>>> Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
>>> ---
>>>  .../devicetree/bindings/media/i2c/ov2680.txt  | 46  +++
>>>  1 file changed, 46 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/media/i2c/ov2680.txt
>>
>>
>> Please add acks/reviewed bys on new versions.
>
>
> I have add this to the cover letter [0]:
> - Removed Rob Herring Reviewed-by tag, since bindings have changed  since
> his
>  ack.
>
> But only now I notice that I did not CC the devicetree list for the all
> series, but only for this particular patch. Sorry about that.

NP. It's better to put this and revision history in the individual
patches. I don't always read cover letters anyways.

Rob

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Rob Herring]

On Wed, May 23, 2018 at 2:38 PM, Laurent Pinchart
<laurent.pinch...@ideasonboard.com> wrote:
> Hi Rob,
>
> On Wednesday, 23 May 2018 19:29:47 EEST Rob Herring wrote:
>> On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
>> > Describe the optional endpoint properties for endpoint nodes of port@0
>> > and port@1 of the R-Car VIN driver device tree bindings documentation.
>> >
>> > Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
>> > ---
>> >
>> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 -
>> >  1 file changed, 12 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > b/Documentation/devicetree/bindings/media/rcar_vin.txt index
>> > a19517e1..c53ce4e 100644
>> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on
>> > SoC.
>> >
>> >from external SoC pins described in video-interfaces.txt[1].
>> >Describing more then one endpoint in port 0 is invalid. Only VIN
>> >instances that are connected to external pins should have port 0.
>> >
>> > +
>> > +  - Optional properties for endpoint nodes of port@0:
>> > +- hsync-active: active state of the HSYNC signal, 0/1 for
>> > LOW/HIGH
>> > + respectively. Default is active high.
>> > +- vsync-active: active state of the VSYNC signal, 0/1 for
>> > LOW/HIGH
>> > + respectively. Default is active high.
>> > +
>> > +   If both HSYNC and VSYNC polarities are not specified, embedded
>> > +   synchronization is selected.
>>
>> No need to copy-n-paste from video-interfaces.txt. Just "see
>> video-interfaces.txt" for the description is fine.
>
> I would still explicitly list the properties that apply to this binding. I
> agree that there's no need to copy & paste the description of those properties
> though.

Yes, that's what I meant. List each property with "see
video-interfaces.txt" for the description of each.

Rob

Re: [PATCH] dt-bindings: media: rcar_vin: fix style for ports and endpoints

[Rob Herring]

On Thu, May 17, 2018 at 01:32:12AM +0200, Niklas Söderlund wrote:
> The style for referring to ports and endpoint are wrong. Refer to them
> using lowercase and a unit address, port@x and endpoint@x.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
> Reported-by: Geert Uytterhoeven <ge...@linux-m68k.org>
> ---
>  .../devicetree/bindings/media/rcar_vin.txt| 20 +--
>  1 file changed, 10 insertions(+), 10 deletions(-)

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Rob Herring]

On Wed, May 16, 2018 at 06:32:28PM +0200, Jacopo Mondi wrote:
> Document 'data-active' property in R-Car VIN device tree bindings.
> The property is optional when running with explicit synchronization
> (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> BT.656) as specified by the hardware manual.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
> b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index c53ce4e..17eac8a 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>   If both HSYNC and VSYNC polarities are not specified, embedded
>   synchronization is selected.
> 
> +- data-active: active state of data enable signal (CLOCKENB pin).
> +  0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> +  data enable signal. When using embedded synchronization this
> +  property is mandatory.

This doesn't match the common property's definition which AIUI is for 
the data lines' polarity. You need a new property. Perhaps a common one.

> +
>  - port 1 - sub-nodes describing one or more endpoints connected to
>the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>use the following schema.
> --
> 2.7.4
> 

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Rob Herring]

On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
> 
> Thanks for your work.
> 
> On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that used
> > them.
> > 
> > Signed-off-by: Jacopo Mondi 
> > ---
> >  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
> >  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
> >  arch/arm/boot/dts/r8a7793-gose.dts| 3 ---
> >  arch/arm/boot/dts/r8a7794-alt.dts | 1 -
> >  arch/arm/boot/dts/r8a7794-silk.dts| 1 -
> >  6 files changed, 12 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
> > b/arch/arm/boot/dts/r8a7790-lager.dts
> > index 063fdb6..b56b309 100644
> > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > @@ -873,10 +873,8 @@
> > port {
> > vin0ep2: endpoint {
> > remote-endpoint = <_out>;
> > -   bus-width = <24>;
> 
> I can't really make up my mind if this is a good thing or not. Device 
> tree describes the hardware and not what the drivers make use of. And 
> the fact is that this bus is 24 bits wide. So I'm not sure we should 
> remove these properties. But I would love to hear what others think 
> about this.

IMO, this property should only be present when all the pins are not 
connected. And by "all", I mean the minimum of what each end of the 
graph can support.

Rob

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Rob Herring]

On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
> Describe the optional endpoint properties for endpoint nodes of port@0
> and port@1 of the R-Car VIN driver device tree bindings documentation.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
> b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1..c53ce4e 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>from external SoC pins described in video-interfaces.txt[1].
>Describing more then one endpoint in port 0 is invalid. Only VIN
>instances that are connected to external pins should have port 0.
> +
> +  - Optional properties for endpoint nodes of port@0:
> +- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
> +   respectively. Default is active high.
> +- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
> +   respectively. Default is active high.
> +
> + If both HSYNC and VSYNC polarities are not specified, embedded
> + synchronization is selected.

No need to copy-n-paste from video-interfaces.txt. Just "see 
video-interfaces.txt" for the description is fine.

> +
>  - port 1 - sub-nodes describing one or more endpoints connected to
>the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>use the following schema.
> @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>  - Endpoint 2 - sub-node describing the endpoint connected to CSI40
>  - Endpoint 3 - sub-node describing the endpoint connected to CSI41
> 
> +  Endpoint nodes of port@1 do not support any optional endpoint property.
> +
>  Device node example for Gen2 platforms
>  --
> 
> @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite 
> video input)
> 
>  vin1ep0: endpoint {
>  remote-endpoint = <>;
> -bus-width = <8>;
>  };
>  };
>  };
> --
> 2.7.4
> 

Re: [PATCH] media: dt-bindings: media: rcar_vin: add support for r8a77965

[Rob Herring]

On Sun, May 13, 2018 at 08:58:18PM +0200, Niklas Söderlund wrote:
> Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <r...@kernel.org>

> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
> b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1c669eb35..c2c57dcf73f4851b 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -21,6 +21,7 @@ on Gen3 platforms to a CSI-2 receiver.
> - "renesas,vin-r8a7794" for the R8A7794 device
> - "renesas,vin-r8a7795" for the R8A7795 device
> - "renesas,vin-r8a7796" for the R8A7796 device
> +   - "renesas,vin-r8a77965" for the R8A77965 device
> - "renesas,vin-r8a77970" for the R8A77970 device
> - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
>   device.
> -- 
> 2.17.0
> 

Re: [PATCH] media: rcar-vin: Drop unnecessary register properties from example vin port

[Rob Herring]

On Wed, May 09, 2018 at 08:45:58PM +0200, Simon Horman wrote:
> The example vin port node does not have an address and thus does not
> need address-cells or address size-properties.
> 
> Signed-off-by: Simon Horman <horms+rene...@verge.net.au>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 ---
>  1 file changed, 3 deletions(-)

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v6 07/13] media: dt-bindings: add bindings for i.MX7 media driver

[Rob Herring]

On Tue, May 22, 2018 at 03:52:39PM +0100, Rui Miguel Silva wrote:
> Add bindings documentation for i.MX7 media drivers.
> The imx7 MIPI CSI2 and imx7 CMOS Sensor Interface.
> 
> Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
> ---
>  .../devicetree/bindings/media/imx7-csi.txt| 44 ++
>  .../bindings/media/imx7-mipi-csi2.txt | 82 +++
>  2 files changed, 126 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/imx7-csi.txt
>  create mode 100644 Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v5 06/12] media: dt-bindings: add bindings for i.MX7 media driver

[Rob Herring]

On Fri, May 18, 2018 at 10:28:00AM +0100, Rui Miguel Silva wrote:
> Add bindings documentation for i.MX7 media drivers.
> 
> Signed-off-by: Rui Miguel Silva 
> ---
>  .../devicetree/bindings/media/imx7.txt| 125 ++
>  1 file changed, 125 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/imx7.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/imx7.txt 
> b/Documentation/devicetree/bindings/media/imx7.txt
> new file mode 100644
> index ..a26372630377
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx7.txt
> @@ -0,0 +1,125 @@
> +Freescale i.MX7 Media Video Device
> +==
> +
> +mipi_csi2 node
> +--
> +
> +This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
> +compatible with previous version of Samsung D-phy.
> +
> +Required properties:
> +
> +- compatible: "fsl,imx7-mipi-csi2";
> +- reg   : base address and length of the register set for the device;
> +- interrupts: should contain MIPI CSIS interrupt;
> +- clocks: list of clock specifiers, see
> +Documentation/devicetree/bindings/clock/clock-bindings.txt for 
> details;
> +- clock-names   : must contain "pclk", "wrap" and "phy" entries, matching
> +  entries in the clock property;
> +- power-domains : a phandle to the power domain, see
> +  Documentation/devicetree/bindings/power/power_domain.txt for 
> details.
> +- reset-names   : should include following entry "mrst";
> +- resets: a list of phandle, should contain reset entry of
> +  reset-names;
> +- phy-supply: from the generic phy bindings, a phandle to a regulator 
> that
> +   provides power to MIPI CSIS core;
> +- bus-width : maximum number of data lanes supported (SoC specific);

This property is for parallel buses. Either just rely on data-lanes in 
the graph or this should be implied by the compatible.

> +
> +Optional properties:
> +
> +- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
> + value when this property is not specified is 166 MHz;
> +
> +port node
> +-
> +
> +- reg  : (required) can take the values 0 or 1, where 0 is 
> the
> + related sink port and port 1 should be the source one;
> +
> +endpoint node
> +-
> +
> +- data-lanes: (required) an array specifying active physical MIPI-CSI2
> + data input lanes and their mapping to logical lanes; the
> + array's content is unused, only its length is meaningful;
> +
> +- fsl,csis-hs-settle : (optional) differential receiver (HS-RX) settle time;

units and unit-suffix?

> +
> +example:
> +
> +mipi_csi: mipi-csi@3075 {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +compatible = "fsl,imx7-mipi-csi2";
> +reg = <0x3075 0x1>;
> +interrupts = ;
> +clocks = < IMX7D_IPG_ROOT_CLK>,
> +< IMX7D_MIPI_CSI_ROOT_CLK>,
> +< IMX7D_MIPI_DPHY_ROOT_CLK>;

> +clock-names = "pclk", "wrap", "phy";
> +clock-names = "mipi", "phy";

Err...

> +clock-frequency = <16600>;
> +power-domains = <_mipi_phy>;
> +phy-supply = <_1p0d>;
> +resets = < IMX7_RESET_MIPI_PHY_MRST>;
> +reset-names = "mrst";
> +bus-width = <4>;
> +fsl,csis-hs-settle = <3>;
> +fsl,csis-clk-settle = <0>;

Not documented.

> +
> +port@0 {
> +reg = <0>;
> +
> +mipi_from_sensor: endpoint {
> +remote-endpoint = <_to_mipi>;
> +data-lanes = <1>;
> +};
> +};
> +
> +port@1 {
> +reg = <1>;
> +
> +mipi_vc0_to_csi_mux: endpoint {
> +remote-endpoint = <_mux_from_mipi_vc0>;
> +};
> +};
> +};
> +
> +
> +csi node
> +

I'd prefer these split into 2 files.

> +
> +This is device node for the CMOS Sensor Interface (CSI) which enables the 
> chip
> +to connect directly to external CMOS image sensors.
> +
> +Required properties:
> +
> +- compatible: "fsl,imx7-csi";
> +- reg   : base address and length of the register set for the device;
> +- interrupts: should contain CSI interrupt;
> +- clocks: list of clock specifiers, see
> +Documentation/devicetree/bindings/clock/clock-bindings.txt for 
> details;
> +- clock-names   : must contain "axi", "mclk" and "dcic" entries, matching
> + entries in the clock property;

Re: [PATCH v5 07/12] ARM: dts: imx7s: add mipi phy power domain

[Rob Herring]

On Fri, May 18, 2018 at 10:28:01AM +0100, Rui Miguel Silva wrote:
> Add power domain index 0 related with mipi-phy to imx7s.
> 
> Signed-off-by: Rui Miguel Silva 
> ---
>  arch/arm/boot/dts/imx7s.dtsi | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 4d42335c0dee..67450ad89940 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -636,6 +636,12 @@
>   #address-cells = <1>;
>   #size-cells = <0>;
>  
> + pgc_mipi_phy: pgc-power-domain@0 {

power-domain@0

> + #power-domain-cells = <0>;
> + reg = <0>;
> + power-supply = <_1p0d>;
> + };
> +
>   pgc_pcie_phy: pgc-power-domain@1 {

ditto.

>   #power-domain-cells = <0>;
>   reg = <1>;
> -- 
> 2.17.0
> 

Re: [PATCH v5 08/12] ARM: dts: imx7s: add multiplexer controls

[Rob Herring]

On Fri, May 18, 2018 at 10:28:02AM +0100, Rui Miguel Silva wrote:
> The IOMUXC General Purpose Register has bitfield to control video bus
> multiplexer to control the CSI input between the MIPI-CSI2 and parallel
> interface. Add that register and mask.
> 
> Signed-off-by: Rui Miguel Silva 
> Reviewed-by: Philipp Zabel 
> ---
>  arch/arm/boot/dts/imx7s.dtsi | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 67450ad89940..3590dab529f9 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -520,8 +520,14 @@
>  
>   gpr: iomuxc-gpr@3034 {
>   compatible = "fsl,imx7d-iomuxc-gpr",
> - "fsl,imx6q-iomuxc-gpr", "syscon";
> + "fsl,imx6q-iomuxc-gpr", "syscon", 
> "simple-mfd";
>   reg = <0x3034 0x1>;
> +
> + mux: mux-controller {
> + compatible = "mmio-mux";
> + #mux-control-cells = <1>;
> + mux-reg-masks = <0x14 0x0010>;

If 1 bit control, then #mux-control-cells can be 0.

> + };
>   };
>  
>   ocotp: ocotp-ctrl@3035 {
> -- 
> 2.17.0
> 

Re: [PATCH v6 1/2] media: ov2680: dt: Add bindings for OV2680

[Rob Herring]

On Wed, May 09, 2018 at 03:31:58PM +0100, Rui Miguel Silva wrote:
> Add device tree binding documentation for the OV2680 camera sensor.
> 
> CC: devicet...@vger.kernel.org
> Signed-off-by: Rui Miguel Silva 
> ---
>  .../devicetree/bindings/media/i2c/ov2680.txt  | 46 +++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

Please add acks/reviewed bys on new versions.

Rob

Re: [PATCH 1/2] media: dt-bindings: add binding for TI SCAN921226H video deserializer

[Rob Herring]

On Fri, May 04, 2018 at 02:49:02PM +0200, Jan Luebbe wrote:
> This deserializer can be used with sensors that directly produce a
> 10-bit LVDS stream and converts it to a parallel bus.
> 
> Controlling it via the optional GPIOs is mainly useful for avoiding
> conflicts when another parallel sensor is connected to the same data bus
> as the deserializer.
> 
> Signed-off-by: Jan Luebbe 
> ---
>  .../bindings/media/ti,scan921226h.txt | 59 +++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/ti,scan921226h.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/ti,scan921226h.txt 
> b/Documentation/devicetree/bindings/media/ti,scan921226h.txt
> new file mode 100644
> index ..4e475672d7bf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/ti,scan921226h.txt
> @@ -0,0 +1,59 @@
> +TI SCAN921226H Video Deserializer
> +-
> +
> +The SCAN921226H receives a LVDS serial data stream with embedded clock and
> +converts it to a 10-bit wide parallel data bus and recovers parallel clock.
> +Some CMOS sensors such as the ON Semiconductor MT9V024 produce a LVDS signal
> +compatible with this deserializer.
> +
> +Required properties:
> +- compatible : should be "ti,scan921226h"
> +- #address-cells: should be <1>
> +- #size-cells: should be <0>
> +- port@0: serial (LVDS) input
> +- port@1: parallel output
> +
> +The device node should contain two 'port' child nodes (one each for input and
> +output), in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +Optional Properties:
> +- enable-gpios: reference to the GPIO connected to the REN (output enable) 
> pin,
> +  if any.
> +- npwrdn-gpios: reference to the GPIO connected to the nPWRDN pin, if any.

Use the standard powerdown-gpios here.

Both should state the active level.

> +
> +Optionally, #address-cells, #size-cells, and port nodes can be grouped under 
> a
> +ports node as described in Documentation/devicetree/bindings/graph.txt.
> +
> +Example:
> +
> +  csi0_deserializer: csi0_deserializer {

Don't use '_' in node names.

> +  compatible = "ti,scan921226h";
> +
> +  enable-gpios = < 20 GPIO_ACTIVE_HIGH>;
> +  npwrdn-gpios = < 24 GPIO_ACTIVE_HIGH>;
> +
> +  #address-cells = <1>;
> +  #size-cells = <0>;
> +
> +  /* serial sink interface */
> +  port@0 {
> +  reg = <0>;
> +
> +  des0_in: endpoint {
> +  remote-endpoint = <_0_out>;
> +  };
> +  };
> +
> +  /* parallel source interface */
> +  port@1 {
> +  reg = <1>;
> +
> +  des0_out: endpoint {
> +  remote-endpoint = 
> <_csi0_mux_from_parallel_sensor>;
> +  bus-width = <8>;
> +  hsync-active = <1>;
> +  vsync-active = <1>;
> +  };
> +  };
> +  };
> -- 
> 2.17.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 10/14] dt-bindings: media: Document bindings for the Sunxi-Cedrus VPU driver

[Rob Herring]

On Mon, May 07, 2018 at 02:44:56PM +0200, Paul Kocialkowski wrote:
> This adds a device-tree binding document that specifies the properties
> used by the Sunxi-Cedurs VPU driver, as well as examples.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkow...@bootlin.com>
> ---
>  .../devicetree/bindings/media/sunxi-cedrus.txt | 58 
> ++
>  1 file changed, 58 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/sunxi-cedrus.txt

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH 1/2] dt-bindings: media: i2c: Add mt9t111 image sensor

[Rob Herring]

On Wed, Apr 25, 2018 at 01:00:13PM +0200, Jacopo Mondi wrote:
> Add device tree bindings documentation for Micron MT9T111/MT9T112 image
> sensors.
> 
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
> ---
>  Documentation/devicetree/bindings/mt9t112.txt | 41 
> +++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mt9t112.txt
> 
> diff --git a/Documentation/devicetree/bindings/mt9t112.txt 
> b/Documentation/devicetree/bindings/mt9t112.txt
> new file mode 100644
> index 000..cbad475
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mt9t112.txt
> @@ -0,0 +1,41 @@
> +Micron 3.1Mp CMOS Digital Image Sensor
> +--
> +
> +The Micron MT9T111 and MT9T112 are 1/4 inch 3.1Mp System-On-A-Chip (SOC) CMOS
> +digital image sensors which support up to QXGA (2048x1536) image resolution 
> in
> +4/3 format.
> +
> +The sensors can be programmed through a two-wire serial interface and can
> +work both in parallel data output mode as well as in MIPI CSI-2 mode.
> +
> +Required Properties:
> +- compatible: shall be one of the following values
> + "micron,mt9t111" for MT9T111 sensors
> + "micron,mt9t112" for MT9T112 sensors
> +
> +Optional properties:
> +- powerdown-gpios: reference to powerdown input GPIO signal. Pin name 
> "STANDBY".
> +  Active level is high.
> +
> +The device node shall contain one 'port' sub-node with one 'endpoint' child
> +node, modeled accordingly to bindings described in:
> +Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> +
> + mt9t112@3d {

camera-sensor@3d

Otherwise,

Reviewed-by: Rob Herring <r...@kernel.org>

> + compatible = "micron,mt9t112";
> + reg = <0x3d>;
> +
> + powerdown-gpios = < 2 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + mt9t112_out: endpoint {
> + pclk-sample = <1>;
> + remote-endpoint = <_in>;
> + };
> + };
> + };
> +
> +
> -- 
> 2.7.4
> 

Re: [PATCH] rcar-vin: remove generic gen3 compatible string

[Rob Herring]

On Wed, Apr 25, 2018 at 01:43:21AM +0200, Niklas Söderlund wrote:
> The compatible string "renesas,rcar-gen3-vin" was added before the
> Gen3 driver code was added but it's not possible to use. Each SoC in the
> Gen3 series require SoC specific knowledge in the driver to function.
> Remove it before it is added to any device tree descriptions.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH 27/28] venus: add sdm845 compatible and resource data

[Rob Herring]

On Tue, Apr 24, 2018 at 03:44:35PM +0300, Stanimir Varbanov wrote:
> This adds sdm845 DT compatible string with it's resource
> data table.
> 
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Stanimir Varbanov <stanimir.varba...@linaro.org>
> ---
>  .../devicetree/bindings/media/qcom,venus.txt   |  1 +
>  drivers/media/platform/qcom/venus/core.c   | 22 
> ++
>  2 files changed, 23 insertions(+)

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v2 1/2] dt-bindings: media: renesas-ceu: Add R-Mobile R8A7740

[Rob Herring]

On Thu, Apr 26, 2018 at 08:24:42PM +0200, Jacopo Mondi wrote:
> Add R-Mobile A1 R8A7740 SoC to the list of compatible values for the CEU
> unit.
> 
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/renesas,ceu.txt | 7 ---
>  drivers/media/platform/renesas-ceu.c| 1 +
>  2 files changed, 5 insertions(+), 3 deletions(-)

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v2 08/15] media: dt-bindings: add bindings for i.MX7 media driver

[Rob Herring]

On Mon, Apr 23, 2018 at 02:47:43PM +0100, Rui Miguel Silva wrote:
> Add bindings documentation for i.MX7 media drivers.
> 
> Signed-off-by: Rui Miguel Silva 
> ---
>  .../devicetree/bindings/media/imx7.txt| 158 ++
>  1 file changed, 158 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/imx7.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/imx7.txt 
> b/Documentation/devicetree/bindings/media/imx7.txt
> new file mode 100644
> index ..7e058ea25102
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx7.txt
> @@ -0,0 +1,158 @@
> +Freescale i.MX7 Media Video Device
> +==
> +
> +Video Media Controller node
> +---
> +
> +This is the media controller node for video capture support. It is a
> +virtual device that lists the camera serial interface nodes that the
> +media device will control.
> +
> +Required properties:
> +- compatible : "fsl,imx7-capture-subsystem";
> +- ports  : Should contain a list of phandles pointing to camera
> + sensor interface port of CSI
> +
> +example:
> +
> +capture-subsystem {
> + compatible = "fsl,imx7-capture-subsystem";
> + ports = <>;

Why do you need this node? Just have the driver match on the CSI node.

> +};
> +
> +
> +mipi_csi2 node
> +--
> +
> +This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
> +compatible with previous version of Samsung D-phy.

Compatible with Samsung?

> +
> +Required properties:
> +
> +- compatible: "fsl,imx7-mipi-csi2";
> +- reg   : base address and length of the register set for the device;
> +- interrupts: should contain MIPI CSIS interrupt;
> +- clocks: list of clock specifiers, see
> +Documentation/devicetree/bindings/clock/clock-bindings.txt for 
> details;
> +- clock-names   : must contain "mipi" and "phy" entries, matching entries in 
> the
> +  clock property;
> +- power-domains : a phandle to the power domain, see
> +  Documentation/devicetree/bindings/power/power_domain.txt for 
> details.
> +- reset-names   : should include following entry "mrst";
> +- resets: a list of phandle, should contain reset entry of
> +  reset-names;
> +- phy-supply: from the generic phy bindings, a phandle to a regulator 
> that
> +   provides power to VBUS;

VBUS? Copy-n-paste from USB something?

> +- bus-width : maximum number of data lanes supported (SoC specific);

Don't we have a standard lanes property for CSI (or DSI)? bus-width is 
for parallel buses and goes in endpoint nodes. (But maybe it got used 
here too).

> +
> +Optional properties:
> +
> +- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
> + value when this property is not specified is 166 MHz;
> +
> +port node
> +-
> +
> +- reg  : (required) can take the values 0 or 1, where 0 is 
> the
> + related sink port and port 1 should be the source one;
> +
> +endpoint node
> +-
> +
> +- data-lanes: (required) an array specifying active physical MIPI-CSI2
> + data input lanes and their mapping to logical lanes; the
> + array's content is unused, only its length is meaningful;

Ah yes, like this. :) So why do you need bus-width too?

> +
> +- csis-hs-settle : (optional) differential receiver (HS-RX) settle time;

units?

> +- csis-clk-settle : (optional) D-PHY control register;
> +- csis-wclk : CSI-2 wrapper clock selection. If this property is present
> +   external clock from CMU will be used, or the bus clock if
> +   if it's not specified.

boolean?

These 3 need vendor properties.

> +
> +example:
> +
> + mipi_csi: mipi-csi@3075 {
> +clock-frequency = <16600>;
> +status = "okay";
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> + compatible = "fsl,imx7-mipi-csi2";
> + reg = <0x3075 0x1>;
> + interrupts = ;
> + clocks = < IMX7D_MIPI_CSI_ROOT_CLK>,
> + < IMX7D_MIPI_DPHY_ROOT_CLK>;
> + clock-names = "mipi", "phy";
> + power-domains = <_mipi_phy>;
> + phy-supply = <_1p0d>;
> + resets = < IMX7_RESET_MIPI_PHY_MRST>;
> + reset-names = "mrst";
> + bus-width = <4>;
> + status = "disabled";
> +
> +port@0 {
> +reg = <0>;
> +
> +mipi_from_sensor: endpoint {
> +remote-endpoint = <_to_mipi>;
> +data-lanes = <1>;
> +csis-hs-settle = <3>;
> +csis-clk-settle = <0>;
> +

Re: [PATCH v2 08/10] dt-bindings: media: Document bindings for the Sunxi-Cedrus VPU driver

[Rob Herring]

On Thu, Apr 19, 2018 at 05:45:34PM +0200, Paul Kocialkowski wrote:
> This adds a device-tree binding document that specifies the properties
> used by the Sunxi-Cedurs VPU driver, as well as examples.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkow...@bootlin.com>
> ---
>  .../devicetree/bindings/media/sunxi-cedrus.txt | 50 
> ++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/sunxi-cedrus.txt

Other than the one nit about the node name,

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v2 08/10] dt-bindings: media: Document bindings for the Sunxi-Cedrus VPU driver

[Rob Herring]

On Fri, Apr 20, 2018 at 09:22:20AM +0200, Paul Kocialkowski wrote:
> Hi and thanks for the review,
> 
> On Fri, 2018-04-20 at 01:31 +, Tomasz Figa wrote:
> > Hi Paul, Philipp,
> > 
> > On Fri, Apr 20, 2018 at 1:04 AM Philipp Zabel 
> > wrote:
> > 
> > > Hi Paul,
> > > On Thu, 2018-04-19 at 17:45 +0200, Paul Kocialkowski wrote:
> > > > This adds a device-tree binding document that specifies the
> > > > properties
> > > > used by the Sunxi-Cedurs VPU driver, as well as examples.
> > > > 
> > > > Signed-off-by: Paul Kocialkowski 
> > > > ---
> > > >  .../devicetree/bindings/media/sunxi-cedrus.txt | 50
> > 
> > ++
> > > >  1 file changed, 50 insertions(+)
> > > >  create mode 100644
> > 
> > Documentation/devicetree/bindings/media/sunxi-cedrus.txt
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/media/sunxi-
> > > > cedrus.txt
> > 
> > b/Documentation/devicetree/bindings/media/sunxi-cedrus.txt
> > > > new file mode 100644
> > > > index ..71ad3f9c3352
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/sunxi-cedrus.txt
> > > > @@ -0,0 +1,50 @@
> > > > +Device-tree bindings for the VPU found in Allwinner SoCs,
> > > > referred to
> > 
> > as the
> > > > +Video Engine (VE) in Allwinner literature.
> > > > +
> > > > +The VPU can only access the first 256 MiB of DRAM, that are DMA-
> > > > mapped
> > 
> > starting
> > > > +from the DRAM base. This requires specific memory allocation and
> > 
> > handling.
> > 
> > And no IOMMU? Brings back memories.
> 
> Exactly, no IOMMU so we don't have much choice but cope with that
> hardware limitation...
> 
> > > > +
> > > > +Required properties:
> > > > +- compatible : "allwinner,sun4i-a10-video-engine";
> > > > +- memory-region : DMA pool for buffers allocation;
> > > > +- clocks : list of clock specifiers, corresponding to
> > 
> > entries in
> > > > +  the clock-names property;
> > > > +- clock-names: should contain "ahb", "mod" and
> > > > "ram"
> > 
> > entries;
> > > > +- assigned-clocks   : list of clocks assigned to the VE;
> > > > +- assigned-clocks-rates : list of clock rates for the clocks
> > > > assigned
> > 
> > to the VE;
> > > > +- resets : phandle for reset;
> > > > +- interrupts : should contain VE interrupt number;
> > > > +- reg: should contain register base and
> > > > length
> > 
> > of VE.
> > > > +
> > > > +Example:
> > > > +
> > > > +reserved-memory {
> > > > + #address-cells = <1>;
> > > > + #size-cells = <1>;
> > > > + ranges;
> > > > +
> > > > + /* Address must be kept in the lower 256 MiBs of DRAM for
> > > > VE. */
> > > > + ve_memory: cma@4a00 {
> > > > + compatible = "shared-dma-pool";
> > > > + reg = <0x4a00 0x600>;
> > > > + no-map;
> > > > + linux,cma-default;
> > > > + };
> > > > +};
> > > > +
> > > > +video-engine@1c0e000 {
> > > This is not really required by any specification, and not as common
> > > as
> > > gpu@..., but could this reasonably be called "vpu@1c0e000" to follow
> > > somewhat-common practice?
> > 
> > AFAIR the name is supposed to be somewhat readable for someone that
> > doesn't know the hardware. To me, "video-engine" sounds more obvious
> > than "vpu", but we actually use "codec" already, in case of MFC and
> > JPEG codec on Exynos. If encode/decode is the only functionality of
> > this block, I'd personally go with "codec". If it can do other things,
> > e.g. scaling/rotation without encode/decode, I'd probably call it
> > "video-processor".
> 
> I agree that the term VPU is more commonly associated with video
> decoding, while video engine could mean a number of things.
> 
> The reason I went with "video-engine" here (while still presenting the
> driver as a VPU driver) is that Video Engine is the term used in
> Allwinner's litterature. Other nodes in Allwinner device-trees generally
> stick to these terms (for instance, we have "display-engine" nodes).
> This also makes it easier to find the matching parts in the
> documentation.

'video-codec' is what is defined in the DT spec.

Rob

Re: [PATCH 2/8] dt-bindings: display: bridge: thc63lvd1024: Add lvds map property

[Rob Herring]

On Thu, Apr 19, 2018 at 11:31:03AM +0200, Jacopo Mondi wrote:
> The THC63LVD1024 LVDS to RGB bridge supports two different input mapping
> modes, selectable by means of an external pin.
> 
> Describe the LVDS mode map through a newly defined mandatory property in
> device tree bindings.
> 
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
> ---
>  .../devicetree/bindings/display/bridge/thine,thc63lvd1024.txt  | 3 
> +++
>  1 file changed, 3 insertions(+)

+1 for what Laurent said.

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v2 06/10] media: dt-bindings: ov772x: add device tree binding

[Rob Herring]

On Mon, Apr 16, 2018 at 11:51:47AM +0900, Akinobu Mita wrote:
> This adds a device tree binding documentation for OV7720/OV7725 sensor.
> 
> Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> Cc: Hans Verkuil <hans.verk...@cisco.com>
> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
> Cc: Rob Herring <robh...@kernel.org>
> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
> ---
> * v2
> - Add "dt-bindings:" in the subject
> - Add a brief description of the sensor
> - Update the GPIO names
> - Indicate the GPIO active level
> 
>  .../devicetree/bindings/media/i2c/ov772x.txt   | 42 
> ++
>  MAINTAINERS|  1 +
>  2 files changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [RESEND PATCH v7 1/2] media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil

[Rob Herring]

On Tue, Apr 10, 2018 at 11:48:43PM +0800, Andy Yeh wrote:
> From: Alan Chiang 
> 
> Dongwoon DW9807 is a voice coil lens driver.
> 
> Signed-off-by: Andy Yeh 
> ---
>  Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 +
>  1 file changed, 9 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt

Please add acks/reviewed-bys when posting new versions.

Rob

Re: [PATCHv2 1/3] dt-bindings: display: dw_hdmi.txt: add cec-disable property

[Rob Herring]

On Fri, Mar 23, 2018 at 01:59:13PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Some boards have both a DesignWare and their own CEC controller.
> The CEC pin is only hooked up to their own CEC controller and not
> to the DW controller.
> 
> Add the cec-disable property to disable the DW CEC controller.
> 
> This particular situation happens on Amlogic boards that have their
> own meson CEC controller.

Seems like we could avoid this by describing how the CEC line is hooked 
up which could be needed for other reasons.

> 
> Signed-off-by: Hans Verkuil 
> Acked-by: Neil Armstrong 
> ---
>  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++
>  1 file changed, 3 insertions(+)

Re: RESEND[PATCH v6 1/2] media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil

[Rob Herring]

On Sat, Mar 17, 2018 at 01:05:25AM +0800, Andy Yeh wrote:
> From: Alan Chiang <alanx.chi...@intel.com>
> 
> Dongwoon DW9807 is a voice coil lens driver.
> 
> Signed-off-by: Andy Yeh <andy@intel.com>
> ---
>  Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 +
>  1 file changed, 9 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH 6/9] sunxi-cedrus: Add device tree binding document

[Rob Herring]

On Fri, Mar 09, 2018 at 11:14:42AM +0100, Paul Kocialkowski wrote:
> From: Florent Revest 

"device tree binding document" can all be summarized with the subject 
prefix "dt-bindings: media: ".

Also, email should be updated to @bootlin.com?

> 
> Device Tree bindings for the Allwinner's video engine
> 
> Signed-off-by: Florent Revest 
> ---
>  .../devicetree/bindings/media/sunxi-cedrus.txt | 44 
> ++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/sunxi-cedrus.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/sunxi-cedrus.txt 
> b/Documentation/devicetree/bindings/media/sunxi-cedrus.txt
> new file mode 100644
> index ..138581113c49
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/sunxi-cedrus.txt
> @@ -0,0 +1,44 @@
> +Device-Tree bindings for SUNXI video engine found in sunXi SoC family
> +
> +Required properties:
> +- compatible : "allwinner,sun4i-a10-video-engine";
> +- memory-region : DMA pool for buffers allocation;

Why do you need this linkage? Many drivers use CMA and don't need this.

> +- clocks : list of clock specifiers, corresponding to
> +   entries in clock-names property;
> +- clock-names: should contain "ahb", "mod" and "ram" entries;
> +- resets : phandle for reset;
> +- interrupts : should contain VE interrupt number;
> +- reg: should contain register base and length of VE.
> +
> +Example:
> +
> +reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + ve_reserved: cma {
> + compatible = "shared-dma-pool";
> + reg = <0x43d0 0x900>;
> + no-map;
> + linux,cma-default;
> + };
> +};
> +
> +video-engine {
> + compatible = "allwinner,sun4i-a10-video-engine";
> + memory-region = <_reserved>;
> +
> + clocks = <_gates 32>, < CLK_VE>,
> +  <_gates 0>;
> + clock-names = "ahb", "mod", "ram";
> +
> + assigned-clocks = < CLK_VE>;
> + assigned-clock-rates = <32000>;

Not documented.

> +
> + resets = < RST_VE>;
> +
> + interrupts = <53>;
> +
> + reg = <0x01c0e000 4096>;
> +};
> -- 
> 2.16.2
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] dt-bindings: media: rcar_vin: Use status "okay"

[Rob Herring]

On Fri, Mar 09, 2018 at 10:34:40AM +0100, Geert Uytterhoeven wrote:
> According to the Devicetree Specification, "ok" is not a valid status.

Correct.

> Fixes: 47c71bd61b772cd7 ("[media] rcar_vin: add devicetree support")
> Signed-off-by: Geert Uytterhoeven 
> ---
> For the checkpatch TODO list?
> https://www.devicetree.org/
> 
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
> b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 68c5c497b7fa5551..a19517e1c669eb35 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -81,7 +81,7 @@ Board setup example for Gen2 platforms (vin1 composite 
> video input)
>  ---
>  
> {
> -status = "ok";
> +status = "okay";

However, I prefer that status not be in examples as it applies to any 
node and the SoC/board split is not relevant to binding docs. I'd 
cleaned all these up except for the cases with SoC/board split.

>  pinctrl-0 = <_pins>;
>  pinctrl-names = "default";
>  
> @@ -104,7 +104,7 @@ Board setup example for Gen2 platforms (vin1 composite 
> video input)
>  pinctrl-0 = <_pins>;
>  pinctrl-names = "default";
>  
> -status = "ok";
> +status = "okay";
>  
>  port {
>  #address-cells = <1>;
> -- 
> 2.7.4
> 

Re: [PATCH 1/4] dt-bindings: media: sun6i: Add A31 and H3 compatibles

[Rob Herring]

On Mon, Mar 05, 2018 at 11:04:29AM +0100, Maxime Ripard wrote:
> The H3 has a slightly different CSI controller (no BT656, no CCI) which
> looks a lot like the original A31 controller. Add a compatible for the A31,
> and more specific compatible the for the H3 to be used in combination for
> the A31.
> 
> Signed-off-by: Maxime Ripard <maxime.rip...@bootlin.com>
> ---
>  Documentation/devicetree/bindings/media/sun6i-csi.txt | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v2 1/2] media: ov2680: dt: Add bindings for OV2680

[Rob Herring]

On Wed, Feb 28, 2018 at 03:27:22PM +, Rui Miguel Silva wrote:
> Add device tree binding documentation for the OV2680 camera sensor.
> 
> CC: devicet...@vger.kernel.org
> Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
> ---
>  .../devicetree/bindings/media/i2c/ov2680.txt   | 40 
> ++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v4 1/2] media: dt-bindings: Add bindings for panasonic,amg88xx

[Rob Herring]

On Mon, Feb 26, 2018 at 10:11:35PM -0800, Matt Ranostay wrote:
> Define the device tree bindings for the panasonic,amg88xx i2c
> video driver.
> 
> Cc: Rob Herring <r...@kernel.org>
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Matt Ranostay <matt.ranos...@konsulko.com>
> ---
>  .../bindings/media/i2c/panasonic,amg88xx.txt  | 19 
> +++
>  1 file changed, 19 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/i2c/panasonic,amg88xx.txt

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [v5 2/2] media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil

[Rob Herring]

On Wed, Feb 28, 2018 at 03:31:26PM +0200, Sakari Ailus wrote:
> Hi Rob,
> 
> Thanks for the review.
> 
> On Tue, Feb 27, 2018 at 04:10:31PM -0600, Rob Herring wrote:
> > On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh <andy@intel.com> wrote:
> > > From: Alan Chiang <alanx.chi...@intel.com>
> > >
> > > Dongwoon DW9807 is a voice coil lens driver.
> > >
> > > Also add a vendor prefix for Dongwoon for one did not exist previously.
> > 
> > Where's that?
> 
> Added by aece98a912d92444ea9da03b04269407d1308f1f . So that line isn't
> relevant indeed and should be removed.
> 
> > 
> > >
> > > Signed-off-by: Andy Yeh <andy@intel.com>
> > > ---
> > >  Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 
> > > +
> > 
> > DACs generally go in bindings/iio/dac/
> 
> We have quite a few lens voice coil drivers under bindings/media/i2c now. I
> don't really object to putting this one to bindings/iio/dac but then the
> rest should be moved as well.
> 
> The camera LED flash drivers are under bindings/leds so this would actually
> be analoguous to that. The lens voice coil drivers are perhaps still a bit
> more bound to the domain (camera) than the LED flash drivers.

The h/w is bound to that function or just the s/w?

> I can send a patch if you think the existing bindings should be moved; let
> me know.

I'm okay if they are separate as long as we're not going to see the 
same device show up in both places. However, "i2c" is not the best 
directory choice. It should be by function, so we can find common 
properties.

Rob

Re: [v5 2/2] media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil

[Rob Herring]

On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh  wrote:
> From: Alan Chiang 
>
> Dongwoon DW9807 is a voice coil lens driver.
>
> Also add a vendor prefix for Dongwoon for one did not exist previously.

Where's that?

>
> Signed-off-by: Andy Yeh 
> ---
>  Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 +

DACs generally go in bindings/iio/dac/

>  1 file changed, 9 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt 
> b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
> new file mode 100644
> index 000..0a1a860
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
> @@ -0,0 +1,9 @@
> +Dongwoon Anatech DW9807 voice coil lens driver
> +
> +DW9807 is a 10-bit DAC with current sink capability. It is intended for
> +controlling voice coil lenses.
> +
> +Mandatory properties:
> +
> +- compatible: "dongwoon,dw9807"
> +- reg: I2C slave address
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] dt-bindings: media: Binding document for OV7251 camera sensor

[Rob Herring]

On Thu, Feb 08, 2018 at 10:53:37AM +0200, Todor Tomov wrote:
> Add the document for ov7251 device tree binding.
> 
> CC: Rob Herring <robh...@kernel.org>
> CC: Mark Rutland <mark.rutl...@arm.com>
> CC: devicet...@vger.kernel.org
> Signed-off-by: Todor Tomov <todor.to...@linaro.org>
> ---
>  .../devicetree/bindings/media/i2c/ov7251.txt   | 51 
> ++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7251.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov7251.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov7251.txt
> new file mode 100644
> index 000..c807646
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov7251.txt
> @@ -0,0 +1,51 @@
> +* Omnivision 1/7.5-Inch B VGA CMOS Digital Image Sensor
> +
> +The Omnivision OV7251 is a 1/7.5-Inch CMOS active pixel digital image sensor 
> with
> +an active array size of 640H x 480V. It is programmable through a serial I2C
> +interface.
> +
> +Required Properties:
> +- compatible: Value should be "ovti,ov7251".
> +- clocks: Reference to the xclk clock.
> +- clock-names: Should be "xclk".
> +- clock-frequency: Frequency of the xclk clock.
> +- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This 
> corresponds
> +  to the hardware pin XSHUTDOWN which is physically active low.
> +- vdddo-supply: Chip digital IO regulator.
> +- vdda-supply: Chip analog regulator.
> +- vddd-supply: Chip digital core regulator.
> +
> +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:
> +
> +  {
> + ...
> +
> + ov7251: ov7251@60 {

camera-sensor@60

With that,

Reviewed-by: Rob Herring <r...@kernel.org>

> + compatible = "ovti,ov7251";
> + reg = <0x60>;
> +
> + enable-gpios = < 6 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_bw_default>;
> +
> + clocks = < 200>;
> + clock-names = "xclk";
> + clock-frequency = <2400>;
> +
> + vdddo-supply = <_dovdd_1v8>;
> + vdda-supply = <_avdd_2v8>;
> + vddd-supply = <_dvdd_1v2>;
> +
> + port {
> + ov7251_ep: endpoint {
> + clock-lanes = <1>;
> + data-lanes = <0>;
> + remote-endpoint = <_ep>;
> + };
> + };
> + };
> + };
> -- 
> 2.7.4
> 

Re: [PATCH v4 1/2] media: dt-bindings: Add OF properties to ov7670

[Rob Herring]

On Wed, Jan 24, 2018 at 10:30:49AM +0100, Jacopo Mondi wrote:
> Describe newly introduced OF properties for ov7670 image sensor.
> The driver supports two standard properties to configure synchronism
> signals polarities and one custom property already supported as
> platform data options to suppress pixel clock during horizontal
> blankings.
> 
> Re-phrase child nodes description while at there.
> 
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/i2c/ov7670.txt | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH 1/2] media: adv7604: Add support for i2c_new_secondary_device

[Rob Herring]

On Mon, Jan 22, 2018 at 12:49:56PM +, Kieran Bingham wrote:
> From: Jean-Michel Hautbois <jean-michel.hautb...@vodalys.com>
> 
> The ADV7604 has thirteen 256-byte maps that can be accessed via the main
> I²C ports. Each map has it own I²C address and acts as a standard slave
> device on the I²C bus.
> 
> Allow a device tree node to override the default addresses so that
> address conflicts with other devices on the same bus may be resolved at
> the board description level.
> 
> Signed-off-by: Jean-Michel Hautbois <jean-michel.hautb...@vodalys.com>
> [Kieran: Re-adapted for mainline]
> Signed-off-by: Kieran Bingham <kieran.bing...@ideasonboard.com>
> ---
> Based upon the original posting :
>   https://lkml.org/lkml/2014/10/22/469
> ---
>  .../devicetree/bindings/media/i2c/adv7604.txt  | 18 ++-

Reviewed-by: Rob Herring <r...@kernel.org>

In the future, please split bindings to separate patch.

>  drivers/media/i2c/adv7604.c| 60 
> ++
>  2 files changed, 55 insertions(+), 23 deletions(-)

Re: [PATCH 2/2] drm: adv7511: Add support for i2c_new_secondary_device

[Rob Herring]

On Mon, Jan 22, 2018 at 12:50:01PM +, Kieran Bingham wrote:
> The ADV7511 has four 256-byte maps that can be accessed via the main I²C
> ports. Each map has it own I²C address and acts as a standard slave
> device on the I²C bus.
> 
> Allow a device tree node to override the default addresses so that
> address conflicts with other devices on the same bus may be resolved at
> the board description level.
> 
> Signed-off-by: Kieran Bingham <kieran.bing...@ideasonboard.com>
> ---
>  .../bindings/display/bridge/adi,adv7511.txt    | 10 +-

Reviewed-by: Rob Herring <r...@kernel.org>

>  drivers/gpu/drm/bridge/adv7511/adv7511.h   |  4 +++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 36 
> ++
>  3 files changed, 37 insertions(+), 13 deletions(-)

Re: [PATCH 2/2] drm: adv7511: Add support for i2c_new_secondary_device

[Rob Herring]

On Mon, Jan 29, 2018 at 12:26:00PM +0200, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Monday, 22 January 2018 14:50:00 EET Kieran Bingham wrote:
> > The ADV7511 has four 256-byte maps that can be accessed via the main I²C
> > ports. Each map has it own I²C address and acts as a standard slave
> > device on the I²C bus.
> > 
> > Allow a device tree node to override the default addresses so that
> > address conflicts with other devices on the same bus may be resolved at
> > the board description level.
> > 
> > Signed-off-by: Kieran Bingham 
> > ---
> >  .../bindings/display/bridge/adi,adv7511.txt| 10 +-
> 
> I don't mind personally, but device tree maintainers usually ask for DT 
> bindings changes to be split to a separate patch.

Or perfect bindings, then I won't ask to split it just for that 
(usually).

> >  drivers/gpu/drm/bridge/adv7511/adv7511.h   |  4 +++
> >  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 36 ---
> >  3 files changed, 37 insertions(+), 13 deletions(-)

Re: [PATCH v6 3/4] dt-bindings: media: Add bindings for OV2685

[Rob Herring]

On Tue, Jan 16, 2018 at 05:22:00PM +0800, Shunqian Zheng wrote:
> Add device tree binding documentation for the OV2685 sensor.
> 
> Signed-off-by: Shunqian Zheng <zhen...@rock-chips.com>
> Reviewed-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
> ---
>  .../devicetree/bindings/media/i2c/ov2685.txt   | 41 
> ++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2685.txt

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v6 1/4] dt-bindings: media: Add bindings for OV5695

[Rob Herring]

On Tue, Jan 16, 2018 at 05:21:58PM +0800, Shunqian Zheng wrote:
> Add device tree binding documentation for the OV5695 sensor.
> 
> Signed-off-by: Shunqian Zheng <zhen...@rock-chips.com>
> Reviewed-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
> ---
>  .../devicetree/bindings/media/i2c/ov5695.txt   | 41 
> ++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5695.txt

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v5 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)

[Rob Herring]

On Thu, Jan 11, 2018 at 11:03:43AM +0800, Yong Deng wrote:
> Add binding documentation for Allwinner V3s CSI.
> 
> Signed-off-by: Yong Deng <yong.d...@magewell.com>
> ---
>  .../devicetree/bindings/media/sun6i-csi.txt| 59 
> ++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v4 1/9] dt-bindings: media: Add Renesas CEU bindings

[Rob Herring]

On Tue, Jan 09, 2018 at 05:25:23PM +0100, Jacopo Mondi wrote:
> Add bindings documentation for Renesas Capture Engine Unit (CEU).
> 
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
> ---
>  .../devicetree/bindings/media/renesas,ceu.txt  | 81 
> ++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v2 2/2] media: ov9650: add device tree binding

[Rob Herring]

On Mon, Jan 08, 2018 at 01:54:24AM +0900, Akinobu Mita wrote:
> Now the ov9650 driver supports device tree probing.  So this adds a
> device tree binding documentation.
> 
> Cc: Jacopo Mondi <jac...@jmondi.org>
> Cc: H. Nikolaus Schaller <h...@goldelico.com>
> Cc: Hugues Fruchet <hugues.fruc...@st.com>
> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
> Cc: Rob Herring <r...@kernel.org>
> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
> ---
> * Changelog v2
> - Split binding documentation, suggested by Rob Herring and Jacopo Mondi
> - Improve the wording for compatible property in the binding documentation,
>   suggested by Jacopo Mondi
> - Improve the description for the device node in the binding documentation,
>   suggested by Sakari Ailus
> 
>  .../devicetree/bindings/media/i2c/ov9650.txt   | 36 
> ++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v2 2/2] media: dt-bindings: Add OF properties to ov7670

[Rob Herring]

On Tue, Jan 9, 2018 at 2:01 AM, jacopo mondi <jac...@jmondi.org> wrote:
> Hi Rob,
>thanks for comments
>
> On Mon, Jan 08, 2018 at 09:35:55PM -0600, Rob Herring wrote:
>> On Thu, Jan 04, 2018 at 10:52:33AM +0100, Jacopo Mondi wrote:
>> > Describe newly introduced OF properties for ov7670 image sensor.
>> > The driver supports two standard properties to configure synchronism
>> > signals polarities and two custom properties already supported as
>> > platform data options by the driver.
>>
>> Missing S-o-b.
>>
>
> Ups, that was trivial, sorry about that
>
>> > ---
>> >  Documentation/devicetree/bindings/media/i2c/ov7670.txt | 14 ++
>> >  1 file changed, 14 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov7670.txt 
>> > b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
>> > index 826b656..57ded18 100644
>> > --- a/Documentation/devicetree/bindings/media/i2c/ov7670.txt
>> > +++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
>> > @@ -9,11 +9,22 @@ Required Properties:
>> >  - clocks: reference to the xclk input clock.
>> >  - clock-names: should be "xclk".
>> >
>> > +The following properties, as defined by video interfaces OF bindings
>> > +"Documentation/devicetree/bindings/media/video-interfaces.txt" are 
>> > supported:
>> > +
>> > +- 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.
>>
>> Don't these go in the endpoint? Not sure offhand.
>>
>
> Yes they do. I will list them as "Optional endpoint properties", and
>
>> > +
>> > +Default is high active state for both vsync and hsync signals.
>> > +
>> >  Optional Properties:
>> >  - reset-gpios: reference to the GPIO connected to the resetb pin, if any.
>> >Active is low.
>> >  - powerdown-gpios: reference to the GPIO connected to the pwdn pin, if 
>> > any.
>> >Active is high.
>> > +- ov7670,pll-bypass: set to 1 to bypass PLL for pixel clock generation.
>>
>> Boolean instead?
>>
>
> Do we have booleans? I had a look at device tree specs and grep for
> "true"/"false" in arch/arm*/boot/dts, and didn't find that much.
> Seems like they're actually strings, am I wrong?

Properties with no value are boolean. Present is true, absent is
false. "foo = <0>" is also treated as true, but not recommended.

Rob

Re: [PATCH v2 2/2] media: dt-bindings: Add OF properties to ov7670

[Rob Herring]

On Thu, Jan 04, 2018 at 10:52:33AM +0100, Jacopo Mondi wrote:
> Describe newly introduced OF properties for ov7670 image sensor.
> The driver supports two standard properties to configure synchronism
> signals polarities and two custom properties already supported as
> platform data options by the driver.

Missing S-o-b.

> ---
>  Documentation/devicetree/bindings/media/i2c/ov7670.txt | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov7670.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
> index 826b656..57ded18 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov7670.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
> @@ -9,11 +9,22 @@ Required Properties:
>  - clocks: reference to the xclk input clock.
>  - clock-names: should be "xclk".
>  
> +The following properties, as defined by video interfaces OF bindings
> +"Documentation/devicetree/bindings/media/video-interfaces.txt" are supported:
> +
> +- 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.

Don't these go in the endpoint? Not sure offhand.

> +
> +Default is high active state for both vsync and hsync signals.
> +
>  Optional Properties:
>  - reset-gpios: reference to the GPIO connected to the resetb pin, if any.
>Active is low.
>  - powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
>Active is high.
> +- ov7670,pll-bypass: set to 1 to bypass PLL for pixel clock generation.

Boolean instead?

> +- ov7670,pclk-hb-disable: set to 1 to suppress pixel clock output signal 
> during
> +  horizontal blankings.

ditto

>  
>  The device node must contain one 'port' child node for its digital output
>  video port, in accordance with the video interface bindings defined in
> @@ -34,6 +45,9 @@ Example:
>   assigned-clocks = <>;
>   assigned-clock-rates = <2500>;
>  
> + vsync-active = <0>;
> + ov7670,pclk-hb-disable = <1>;
> +
>   port {
>   ov7670_0: endpoint {
>   remote-endpoint = <_0>;
> -- 
> 2.7.4
> 

Re: [PATCH v5 3/5] media: dt-bindings: ov5640: refine CSI-2 and add parallel interface

[Rob Herring]

On Wed, Jan 03, 2018 at 10:57:30AM +0100, Hugues Fruchet wrote:
> Refine CSI-2 endpoint documentation and add bindings
> for DVP parallel interface support.
> 
> Signed-off-by: Hugues Fruchet <hugues.fruc...@st.com>
> ---
>  .../devicetree/bindings/media/i2c/ov5640.txt   | 46 
> +-
>  1 file changed, 44 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <r...@kernel.org> 

Re: [PATCH] media: ov9650: support device tree probing

[Rob Herring]

On Fri, Jan 5, 2018 at 11:49 AM, Akinobu Mita <akinobu.m...@gmail.com> wrote:
> The ov9650 driver currently only supports legacy platform data probe.
> This change adds device tree probing.
>
> There has been an attempt to add device tree support for ov9650 driver
> by Hugues Fruchet as a part of the patchset that adds support of OV9655
> camera (http://www.spinics.net/lists/linux-media/msg117903.html), but
> it wasn't merged into mainline because creating a separate driver for
> OV9655 is preferred.
>
> This is very similar to Hugues's patch, but not supporting new device.
>
> Cc: H. Nikolaus Schaller <h...@goldelico.com>
> Cc: Hugues Fruchet <hugues.fruc...@st.com>
> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
> Cc: Rob Herring <r...@kernel.org>
> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
> ---
>  .../devicetree/bindings/media/i2c/ov9650.txt   |  35 +++

Please split bindings and send to the DT list.

>  drivers/media/i2c/ov9650.c | 107 
> -
>  2 files changed, 117 insertions(+), 25 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt

Re: [PATCH 4/4] [media] dt/bindings: Add bindings for OV2685

[Rob Herring]

On Mon, Dec 25, 2017 at 10:11:26PM +0800, Shunqian Zheng wrote:
> Add device tree binding documentation for the OV2685 sensor.
> 
> Signed-off-by: Shunqian Zheng 
> ---
>  .../devicetree/bindings/media/i2c/ov2685.txt   | 35 
> ++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2685.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2685.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov2685.txt
> new file mode 100644
> index 000..c62db9c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov2685.txt
> @@ -0,0 +1,35 @@
> +* Omnivision OV2685 MIPI CSI-2 sensor
> +
> +Required Properties:
> +- compatible: should be "ovti,ov2685"
> +- clocks: reference to the 24M xvclk input clock.
> +- clock-names: should be "xvclk".
> +- avdd-supply: Analog voltage supply, 2.8 volts
> +- dvdd-supply: Digital core voltage supply, 1.2 volts
> +- reset-gpios: Low active reset gpio
> +
> +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:
> + ucam: ov2680@3c {

camera-sensor@3c

> + compatible = "ovti,ov2685";
> + reg = <0x3c>;

> + pinctrl-names = "default";
> + pinctrl-0 = <_24m_cam _rst>;

Not documented.

> +
> + clocks = < SCLK_TESTCLKOUT1>;
> + clock-names = "xvclk";
> +
> + avdd-supply = <_cam>;
> + dovdd-supply = <>;
> + reset-gpios = < 3 GPIO_ACTIVE_LOW>;
> +
> + port {
> + ucam_out: endpoint {
> + remote-endpoint = <_in_ucam>;
> + data-lanes = <1>;
> + };
> + };
> + };
> -- 
> 1.9.1
> 

Re: [PATCH 2/4] [media] dt/bindings: Add bindings for OV5695

[Rob Herring]

On Mon, Dec 25, 2017 at 10:11:24PM +0800, Shunqian Zheng wrote:
> Add device tree binding documentation for the OV5695 sensor.
> 
> Signed-off-by: Shunqian Zheng <zhen...@rock-chips.com>
> ---
>  .../devicetree/bindings/media/i2c/ov5695.txt   | 38 
> ++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5695.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5695.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
> new file mode 100644
> index 000..ed27eb1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
> @@ -0,0 +1,38 @@
> +* Omnivision OV5695 MIPI CSI-2 sensor
> +
> +Required Properties:
> +- compatible: should be "ovti,ov5695"
> +- clocks: reference to the 24M xvclk input clock.
> +- clock-names: should be "xvclk".
> +- dovdd-supply: Digital I/O voltage supply, 1.8 volts
> +- avdd-supply: Analog voltage supply, 2.8 volts
> +- dvdd-supply: Digital core voltage supply, 1.2 volts
> +- reset-gpios: Low active reset gpio
> +
> +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:
> + {

Presumably, i2c1 is your I2C controller, so you are missing a child node 
here: camera-sensor@36

With that,

Reviewed-by: Rob Herring <r...@kernel.org>

> + compatible = "ovti,ov5695";
> + reg = <0x36>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_24m_cam _rst>;
> +
> + clocks = < SCLK_TESTCLKOUT1>;
> + clock-names = "xvclk";
> +
> + avdd-supply = <_cam>;
> + dvdd-supply = <_cam>;
> + dovdd-supply = <>;
> +
> + reset-gpios = < 5 GPIO_ACTIVE_LOW>;
> +
> + port {
> + wcam_out: endpoint {
> + remote-endpoint = <_in_wcam>;
> + data-lanes = <1 2>;
> + };
> + };
> +};
> -- 
> 1.9.1
> 

Re: [PATCH v4 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)

[Rob Herring]

On Fri, Dec 22, 2017 at 05:41:29PM +0800, Yong Deng wrote:
> Add binding documentation for Allwinner V3s CSI.
> 
> Signed-off-by: Yong Deng 
> ---
>  .../devicetree/bindings/media/sun6i-csi.txt| 51 
> ++
>  1 file changed, 51 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..b5bfe3f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> @@ -0,0 +1,51 @@
> +Allwinner V3s Camera Sensor Interface
> +--
> +
> +Required properties:
> +  - compatible: value must be "allwinner,sun8i-v3s-csi"
> +  - reg: base address and size of the memory-mapped region.
> +  - interrupts: interrupt associated to this IP
> +  - clocks: phandles to the clocks feeding the CSI
> +* bus: 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.
> +  Currently, the driver only support the parallel interface. So, a single 
> port
> +  node with one endpoint and parallel bus is supported.

What the driver supports is not relevant. Please document what the h/w 
has.

> +
> +Example:
> +
> + csi1: csi@1cb4000 {
> + compatible = "allwinner,sun8i-v3s-csi";
> + reg = <0x01cb4000 0x1000>;
> + interrupts = ;
> + clocks = < CLK_BUS_CSI>,
> +  < CLK_CSI1_SCLK>,
> +  < CLK_DRAM_CSI>;
> + clock-names = "bus", "mod", "ram";
> + resets = < RST_BUS_CSI>;
> +
> + port {

> + #address-cells = <1>;
> + #size-cells = <0>;

These are not needed with a single endpoint.

> +
> + /* Parallel bus endpoint */
> + csi1_ep: endpoint {
> + remote-endpoint = <_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 */
> + };
> + };
> + };
> +
> -- 
> 1.8.3.1
> 

Re: [linux-sunxi] [PATCH v4 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)

[Rob Herring]

On Fri, Dec 22, 2017 at 10:00:08AM +, Priit Laes wrote:
> On Fri, Dec 22, 2017 at 05:41:29PM +0800, Yong Deng wrote:
> > Add binding documentation for Allwinner V3s CSI.
> > 
> > Signed-off-by: Yong Deng 
> > ---
> >  .../devicetree/bindings/media/sun6i-csi.txt| 51 
> > ++
> >  1 file changed, 51 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..b5bfe3f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> > @@ -0,0 +1,51 @@
> > +Allwinner V3s Camera Sensor Interface
> > +--
> 
> Not sure whether syntax for these files is proper reStructuredText/Markdown,
> but the underline-ish style expects the title and underline having same 
> length.

The binding files are not rst/md format, but still the comment is just 
good style.

Rob

Re: [PATCH v5 3/6] media: dt-bindings: Add bindings for TDA1997X

[Rob Herring]

On Sat, Dec 16, 2017 at 10:00:27AM -0800, Tim Harvey wrote:
> Cc: Rob Herring <r...@kernel.org>
> Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Signed-off-by: Tim Harvey <thar...@gateworks.com>
> ---
> v5:
>  - added Sakari's ack
> 
> v4:
>  - move include/dt-bindings/media/tda1997x.h to bindings patch
>  - clarify port node details
> 
> v3:
>  - fix typo
> 
> v2:
>  - add vendor prefix and remove _ from vidout-portcfg
>  - remove _ from labels
>  - remove max-pixel-rate property
>  - describe and provide example for single output port
>  - update to new audio port bindings
> ---
>  .../devicetree/bindings/media/i2c/tda1997x.txt | 179 
> +
>  include/dt-bindings/media/tda1997x.h   |  78 +
>  2 files changed, 257 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/tda1997x.txt
>  create mode 100644 include/dt-bindings/media/tda1997x.h

Acked-by: Rob Herring <r...@kernel.org>

Re: [PATCH v3 2/6] media: dt: bindings: Update binding documentation for sunxi IR controller

[Rob Herring]

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 <embe...@gmail.com>
> ---
>  Documentation/devicetree/bindings/media/sunxi-ir.txt | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v4 11/16] dt-bindings: Document the Rockchip MIPI RX D-PHY bindings

[Rob Herring]

On Mon, Dec 18, 2017 at 08:14:40PM +0800, Jacob Chen wrote:
> From: Jacob Chen <jacob2.c...@rock-chips.com>
> 
> Add DT bindings documentation for Rockchip MIPI D-PHY RX
> 
> Signed-off-by: Jacob Chen <jacob2.c...@rock-chips.com>
> ---
>  .../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 <r...@kernel.org>

Re: [PATCH v4 10/16] dt-bindings: Document the Rockchip ISP1 bindings

[Rob Herring]

On Mon, Dec 18, 2017 at 08:14:39PM +0800, Jacob Chen wrote:
> From: Jacob Chen <jacob2.c...@rock-chips.com>
> 
> Add DT bindings documentation for Rockchip ISP1
> 
> Signed-off-by: Jacob Chen <jacob2.c...@rock-chips.com>
> ---
>  .../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 <r...@kernel.org>

Re: [PATCH for v4.15] dt-bindings/media/cec-gpio.txt: mention the CEC/HPD max voltages

[Rob Herring]

On Sat, Dec 16, 2017 at 11:44:13AM +0100, Hans Verkuil wrote:
> Mention the maximum voltages of the CEC and HPD lines. Since in the example
> these lines are connected to a Raspberry Pi and the Rpi GPIO lines are 3.3V
> it is a good idea to warn against directly connecting the HPD to the Raspberry
> Pi's GPIO line.
> 
> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [RFC 2/5] [media] dt: bindings: Update binding documentation for sunxi IR controller

[Rob Herring]

gOn Sat, Dec 16, 2017 at 03:49:11AM +0100, Philipp Rossak wrote:
> This patch updates documentation for Device-Tree bindings for sunxi IR
> controller and adds the new requiered property for the base clock frequency.
> 
> Signed-off-by: Philipp Rossak 
> ---
>  Documentation/devicetree/bindings/media/sunxi-ir.txt | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt 
> b/Documentation/devicetree/bindings/media/sunxi-ir.txt
> index 91648c569b1e..5f4960c61245 100644
> --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt
> +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt
> @@ -1,12 +1,13 @@
>  Device-Tree bindings for SUNXI IR controller found in sunXi SoC family
>  
>  Required properties:
> -- compatible : "allwinner,sun4i-a10-ir" or "allwinner,sun5i-a13-ir"
> -- clocks : list of clock specifiers, corresponding to
> -   entries in clock-names property;
> -- clock-names: should contain "apb" and "ir" entries;
> -- interrupts : should contain IR IRQ number;
> -- reg: should contain IO map address for IR.
> +- compatible   : "allwinner,sun4i-a10-ir" or "allwinner,sun5i-a13-ir"
> +- clocks   : list of clock specifiers, corresponding to
> + entries in clock-names property;
> +- clock-names  : should contain "apb" and "ir" entries;
> +- interrupts   : should contain IR IRQ number;
> +- reg  : should contain IO map address for IR.
> +- base-clk-frequency  : should contain the base clock frequency

Use clock-frequency or assigned-clocks.

Rob

Re: [PATCH 1/2] media: dt-bindings: coda: Add compatible for CodaHx4 on i.MX51

[Rob Herring]

On Wed, Dec 13, 2017 at 03:09:17PM +0100, Philipp Zabel wrote:
> Add a compatible for the CodaHx4 VPU used on i.MX51.
> 
> Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/media/coda.txt | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Rob Herring <r...@kernel.org>

Re: [PATCH v3 07/12] dt-bindings: Document the Rockchip MIPI RX D-PHY bindings

[Rob Herring]

On Wed, Dec 06, 2017 at 07:19:34PM +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  | 71 
> ++
>  1 file changed, 71 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt 
> b/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
> new file mode 100644
> index ..cef9450db051
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
> @@ -0,0 +1,71 @@
> +Rockchip SoC MIPI RX D-PHY
> +-
> +
> +Required properties:
> +
> +- compatible: value should be one of the following
> +"rockchip,rk3288-mipi-dphy";
> +"rockchip,rk3399-mipi-dphy";

Drop the ';'

> +- rockchip,grf: GRF regs.
> +- bus-width : maximum number of data lanes supported (SoC specific);
> +- clocks : list of clock specifiers, corresponding to entries in
> + clock-names property;
> +- clock-names: required clock name.
> +
> +The device node should contain two 'port' child node, according to the 
> bindings
> +defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
> +The first port should be connected to sensor nodes, and the second port 
> should be
> +connected to isp node. The following are properties specific to those nodes.

Need to list how many endpoints if there are more than 1.

> +
> +endpoint node
> +-
> +
> +- data-lanes : (required) an array specifying active physical MIPI-CSI2
> + data input lanes and their mapping to logical lanes; the
> + array's content is unused, only its length is meaningful;
> +
> +Device node example
> +---
> +
> +mipi_dphy_rx0: mipi-dphy-rx0 {
> +compatible = "rockchip,rk3399-mipi-dphy";
> +clocks = < SCLK_MIPIDPHY_REF>,
> +< SCLK_DPHY_RX0_CFG>,
> +< PCLK_VIO_GRF>;
> +clock-names = "dphy-ref", "dphy-cfg", "grf";
> +power-domains = < RK3399_PD_VIO>;
> +bus-width = <4>;

rockchip,grf?

No other registers? Can you just make this a child of the grf block with 
a proper reg property for the range of dphy registers?

> +
> +ports {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +port@0 {
> +reg = <0>;
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +mipi_in_wcam: endpoint@0 {
> +reg = <0>;
> +remote-endpoint = <_out>;
> +data-lanes = <1 2>;
> +};
> +mipi_in_ucam: endpoint@1 {
> +reg = <1>;
> +remote-endpoint = <_out>;
> +data-lanes = <1>;
> +};
> +};
> +
> +port@1 {
> +reg = <1>;
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +dphy_rx0_out: endpoint@0 {
> +reg = <0>;

Don't need reg and everything associated with it if there's only 1 
endpoint. 

> +remote-endpoint = <_mipi_in>;
> +};
> +};
> +};
> +};
> \ No newline at end of file

Please fix.

Rob

Re: [PATCH v3 06/12] dt-bindings: Document the Rockchip ISP1 bindings

[Rob Herring]

On Wed, Dec 06, 2017 at 07:19:33PM +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| 57 
> ++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/rockchip-isp1.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip-isp1.txt 
> b/Documentation/devicetree/bindings/media/rockchip-isp1.txt
> new file mode 100644
> index ..0971ed94ed69
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rockchip-isp1.txt
> @@ -0,0 +1,57 @@
> +Rockchip SoC Image Signal Processing unit v1
> +--
> +
> +Rockchip ISP1 is the Camera interface for the Rockchip series of SoCs
> +which contains image processing, scaling, and compression funcitons.
> +
> +Required properties:
> +  - compatible: value should be one of the following
> +  "rockchip,rk3288-cif-isp";
> +  "rockchip,rk3399-cif-isp";
> +  - reg : offset and length of the register set for the device.
> +  - interrupts: should contain ISP interrupt.
> +  - clocks: phandle to the required clocks.
> +  - clock-names: required clock name.
> +  - iommus: required a iommu node.
> +
> +The device node should contain one 'port' child node with child 'endpoint'
> +nodes, according to the bindings defined in 
> Documentation/devicetree/bindings/
> +media/video-interfaces.txt.
> +
> +For sensor with a parallel video bus, it could be linked directly to the isp.
> +For sensor with a MIPI CSI-2 video bus, it should be linked through the
> +MIPI-DPHY, which is defined in rockchip-mipi-dphy.txt.

As I mentioned on the last version, you need to list that there are 2 
endpoints for the port and what their assignment is.

Rob

Re: [PATCH v6 1/2] media: ov7740: Document device tree bindings

[Rob Herring]

On Mon, Dec 04, 2017 at 02:58:57PM +0800, Wenyou Yang wrote:
> Add the device tree binding documentation for the ov7740 sensor driver.
> 
> Signed-off-by: Wenyou Yang 
> ---
> 
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3:
>  - Explicitly document the "remote-endpoint" property.
> 
> Changes in v2: None
> 
>  .../devicetree/bindings/media/i2c/ov7740.txt   | 47 
> ++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7740.txt

Please add acks when posting new versions.

Rob

Re: [PATCH v8 28/28] rcar-vin: enable support for r8a77970

[Rob Herring]

On Wed, Nov 29, 2017 at 08:43:42PM +0100, Niklas Söderlund wrote:
> Add the SoC specific information for Renesas r8a77970.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
> ---
>  .../devicetree/bindings/media/rcar_vin.txt     |  1 +

Acked-by: Rob Herring <r...@kernel.org>

>  drivers/media/platform/rcar-vin/rcar-core.c| 40 
> ++
>  2 files changed, 41 insertions(+)

Re: [PATCH v8 27/28] rcar-vin: enable support for r8a7796

[Rob Herring]

On Wed, Nov 29, 2017 at 08:43:41PM +0100, Niklas Söderlund wrote:
> Add the SoC specific information for Renesas r8a7796.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
> Reviewed-by: Hans Verkuil <hans.verk...@cisco.com>
> ---
>  .../devicetree/bindings/media/rcar_vin.txt     |  1 +

Acked-by: Rob Herring <r...@kernel.org>

>  drivers/media/platform/rcar-vin/rcar-core.c| 64 
> ++
>  2 files changed, 65 insertions(+)

Re: [PATCH v5 1/2] media: ov7740: Document device tree bindings

[Rob Herring]

On Tue, Nov 28, 2017 at 01:22:58PM +0800, Wenyou Yang wrote:
> Add the device tree binding documentation for the ov7740 sensor driver.
> 
> Signed-off-by: Wenyou Yang <wenyou.y...@microchip.com>
> ---
> 
> Changes in v5: None
> Changes in v4: None
> Changes in v3:
>  - Explicitly document the "remote-endpoint" property.
> 
> Changes in v2: None
> 
>  .../devicetree/bindings/media/i2c/ov7740.txt   | 47 
> ++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7740.txt

Acked-by: Rob Herring <r...@kernel.org>

Re: [PATCH v2 06/11] dt-bindings: Document the Rockchip MIPI RX D-PHY bindings

[Rob Herring]

On Fri, Nov 24, 2017 at 10:37:01AM +0800, Jacob Chen wrote:
> Add DT bindings documentation for Rockchip MIPI D-PHY RX
> 
> Signed-off-by: Jacob Chen 
> ---
>  .../bindings/media/rockchip-mipi-dphy.txt  | 77 
> ++
>  1 file changed, 77 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt

Same comments apply to this one.

Re: [PATCH v2 05/11] dt-bindings: Document the Rockchip ISP1 bindings

[Rob Herring]

On Fri, Nov 24, 2017 at 10:37:00AM +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| 61 
> ++
>  1 file changed, 61 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/rockchip-isp1.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip-isp1.txt 
> b/Documentation/devicetree/bindings/media/rockchip-isp1.txt
> new file mode 100644
> index ..5e5b72edcf81
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rockchip-isp1.txt
> @@ -0,0 +1,61 @@
> +Rockchip SoC Image Signal Processing unit v1
> +--
> +
> +Rockchip ISP1 is the Camera interface for the Rockchip series of SoCs
> +which contains image processing, scaling, and compression funcitons.
> +
> +Currently device tree nodes for the Rockchip ISP1 driver includes:

Bindings describe the h/w, not drivers.

> +MIPI D-PHY, ISP.
> +
> +Required properties:
> +  - compatible: value should be one of the following
> +  "rockchip,rk3288-cif-isp";
> +  "rockchip,rk3399-cif-isp";
> +  - reg : offset and length of the register set for the device.
> +  - interrupts: should contain ISP interrupt.
> +  - clocks: phandle to the required clocks.
> +  - clock-names: required clock name.
> +  - iommus: required a iommu node.
> +
> +The device node should contain one 'port' child node with child 'endpoint'
> +nodes, according to the bindings defined in 
> Documentation/devicetree/bindings/
> +media/video-interfaces.txt.

You need to enumerate the endpoints (mipi and parallel), too.
 
> +
> +Example:
> +SoC-specific DT entry:
> + isp0: isp0@ff91 {
> + compatible = "rockchip,rk3399-cif-isp";
> + reg = <0x0 0xff91 0x0 0x4000>;
> + interrupts = ;
> + clocks = < SCLK_ISP0>,
> +  < ACLK_ISP0>, < ACLK_ISP0_WRAPPER>,
> +  < HCLK_ISP0>, < HCLK_ISP0_WRAPPER>;
> + clock-names = "clk_isp",
> +   "aclk_isp", "aclk_isp_wrap",
> +   "hclk_isp", "hclk_isp_wrap";
> + power-domains = < RK3399_PD_ISP0>;
> + iommus = <_mmu>;
> + status = "disabled";
> + };
> +
> +Board-specific:

Just show the complete example. The SoC and board split is purely source 
level convention.

> + isp0: isp0@ff91 {
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* mipi */
> + isp0_mipi_in: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <_rx0_out>;
> + };
> +
> + /* parallel */
> + isp0_parallel_in: endpoint@1 {
> + reg = <1>;
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +
> +The MIPI-DPHY device binding is defined in rockchip-mipi-dphy.txt.
> -- 
> 2.15.0
> 

Re: [PATCH v3 2/4] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver

[Rob Herring]

On Tue, Nov 21, 2017 at 1:26 PM, Eric Anholt <e...@anholt.net> wrote:
> Dave Stevenson <dave.steven...@raspberrypi.org> writes:
>
>> Hi Rob
>>
>> On 27 September 2017 at 22:51, Rob Herring <r...@kernel.org> wrote:
>>> On Fri, Sep 22, 2017 at 05:07:22PM +0100, Dave Stevenson wrote:
>>>> Hi Stefan
>>>>
>>>> On 22 September 2017 at 07:45, Stefan Wahren <stefan.wah...@i2se.com> 
>>>> wrote:
>>>> > Hi Dave,
>>>> >
>>>> >> Dave Stevenson <dave.steven...@raspberrypi.org> hat am 20. September 
>>>> >> 2017 um 18:07 geschrieben:
>>>> >>
>>>> >>
>>>> >> Document the DT bindings for the CSI2/CCP2 receiver peripheral
>>>> >> (known as Unicam) on BCM283x SoCs.
>>>> >>
>>>> >> Signed-off-by: Dave Stevenson <dave.steven...@raspberrypi.org>
>>>> >> ---
>>>> >>
>>>> >> Changes since v2
>>>> >> - Removed all references to Linux drivers.
>>>> >> - Reworded section about disabling the firmware driver.
>>>> >> - Renamed clock from "lp_clock" to "lp" in description and example.
>>>> >> - Referred to video-interfaces.txt and stated requirements on 
>>>> >> remote-endpoint
>>>> >>   and data-lanes.
>>>> >> - Corrected typo in example from csi to csi1.
>>>> >> - Removed unnecessary #address-cells and #size-cells in example.
>>>> >> - Removed setting of status from the example.
>>>> >>
>>>> >>  .../devicetree/bindings/media/bcm2835-unicam.txt   | 85 
>>>> >> ++
>>>> >>  1 file changed, 85 insertions(+)
>>>> >>  create mode 100644 
>>>> >> Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>>> >>
>>>> >> diff --git a/Documentation/devicetree/bindings/media/bcm2835-unicam.txt 
>>>> >> b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>>> >> new file mode 100644
>>>> >> index 000..7714fb3
>>>> >> --- /dev/null
>>>> >> +++ b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>>> >> @@ -0,0 +1,85 @@
>>>> >> +Broadcom BCM283x Camera Interface (Unicam)
>>>> >> +--
>>>> >> +
>>>> >> +The Unicam block on BCM283x SoCs is the receiver for either
>>>> >> +CSI-2 or CCP2 data from image sensors or similar devices.
>>>> >> +
>>>> >> +The main platform using this SoC is the Raspberry Pi family of boards.
>>>> >> +On the Pi the VideoCore firmware can also control this hardware block,
>>>> >> +and driving it from two different processors will cause issues.
>>>> >> +To avoid this, the firmware checks the device tree configuration
>>>> >> +during boot. If it finds device tree nodes called csi0 or csi1 then
>>>> >> +it will stop the firmware accessing the block, and it can then
>>>> >> +safely be used via the device tree binding.
>>>> >> +
>>>> >> +Required properties:
>>>> >> +===
>>>> >> +- compatible : must be "brcm,bcm2835-unicam".
>>>> >> +- reg: physical base address and length of the 
>>>> >> register sets for the
>>>> >> +   device.
>>>> >> +- interrupts : should contain the IRQ line for this Unicam instance.
>>>> >> +- clocks : list of clock specifiers, corresponding to entries in
>>>> >> +   clock-names property.
>>>> >> +- clock-names: must contain an "lp" entry, matching entries in 
>>>> >> the
>>>> >> +   clocks property.
>>>> >> +
>>>> >> +Unicam supports a single port node. It should contain one 'port' child 
>>>> >> node
>>>> >> +with child 'endpoint' node. Please refer to the bindings defined in
>>>> >> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>>>> >> +
>>>> >> +Within the endpoint node the "remote-endpoint" and "data-lanes" 
>>>> >> prope

Re: [PATCH v2 2/4] dt-bindings: media: rcar_vin: add device tree support for r8a774[35]

[Rob Herring]

On Thu, Nov 16, 2017 at 06:22:49PM +, Fabrizio Castro wrote:
> Add compatible strings for r8a7743 and r8a7745. No driver change
> is needed as "renesas,rcar-gen2-vin" will activate the right code.
> However, it is good practice to document compatible strings for the
> specific SoC as this allows SoC specific changes to the driver if
> needed, in addition to document SoC support and therefore allow
> checkpatch.pl to validate compatible string values.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.cas...@bp.renesas.com>
> Reviewed-by: Biju Das <biju@bp.renesas.com>
> ---
> v1->v2:
> * Fixed double "change" in changelog
> 
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <r...@kernel.org>

Re: [PATCH v2 1/4] dt-bindings: media: rcar_vin: Reverse SoC part number list

[Rob Herring]

On Thu, Nov 16, 2017 at 06:22:48PM +, Fabrizio Castro wrote:
> Change the sorting of the part numbers from descending to ascending to
> match with other documentation.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.cas...@bp.renesas.com>
> Reviewed-by: Biju Das <biju@bp.renesas.com>
> ---
> v1->v2:
> * new patch triggered by Geert's comment, see the below link for details:
>   https://www.mail-archive.com/linux-media@vger.kernel.org/msg121992.html
> 
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)

Acked-by: Rob Herring <r...@kernel.org>

Re: [PATCH v7 01/25] rcar-vin: add Gen3 devicetree bindings documentation

[Rob Herring]

On Wed, Nov 15, 2017 at 4:58 PM, Niklas Söderlund
<niklas.soderl...@ragnatech.se> wrote:
> Hi Rob,
>
> Thanks for your feedback, much appreciated!
>
> On 2017-11-15 14:02:26 -0600, Rob Herring wrote:
>> On Sat, Nov 11, 2017 at 01:38:11AM +0100, Niklas Söderlund wrote:
>> > Document the devicetree bindings for the CSI-2 inputs available on Gen3.
>> >
>> > There is a need to add a custom property 'renesas,id' and to define
>> > which CSI-2 input is described in which endpoint under the port@1 node.
>> > This information is needed since there are a set of predefined routes
>> > between each VIN and CSI-2 block. This routing table will be kept
>> > inside the driver but in order for it to act on it it must know which
>> > VIN and CSI-2 is which.
>> >
>> > Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
>> > ---
>> >  .../devicetree/bindings/media/rcar_vin.txt | 116 
>> > ++---
>> >  1 file changed, 104 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
>> > b/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > index 6e4ef8caf759e5d3..df1abd0fb20386f8 100644
>> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > @@ -2,8 +2,12 @@ Renesas R-Car Video Input driver (rcar_vin)
>> >  ---
>> >
>> >  The rcar_vin device provides video input capabilities for the Renesas 
>> > R-Car
>> > -family of devices. The current blocks are always slaves and suppot one 
>> > input
>> > -channel which can be either RGB, YUYV or BT656.
>> > +family of devices.
>> > +
>> > +Each VIN instance has a single parallel input that supports RGB and YUV 
>> > video,
>> > +with both external synchronization and BT.656 synchronization for the 
>> > latter.
>> > +Depending on the instance the VIN input is connected to external SoC 
>> > pins, or
>> > +on Gen3 to a CSI-2 receiver.
>> >
>> >   - compatible: Must be one or more of the following
>> > - "renesas,vin-r8a7795" for the R8A7795 device
>> > @@ -28,21 +32,38 @@ channel which can be either RGB, YUYV or BT656.
>> >  Additionally, an alias named vinX will need to be created to specify
>> >  which video input device this is.
>> >
>> > -The per-board settings:
>> > +The per-board settings Gen2:
>> >   - port sub-node describing a single endpoint connected to the vin
>> > as described in video-interfaces.txt[1]. Only the first one will
>> > be considered as each vin interface has one input port.
>> >
>> > -   These settings are used to work out video input format and widths
>> > -   into the system.
>> > +The per-board settings Gen3:
>> > +
>> > +Gen3 can support both a single connected parallel input source from
>> > +external SoC pins (port0) and/or multiple parallel input sources from
>> > +local SoC CSI-2 receivers (port1) depending on SoC.
>> >
>> > +- renesas,id - ID number of the VIN, VINx in the documentation.
>>
>> Why is this needed? We try to avoid indexes unless that's the only way a
>> device is addressed (and then we use reg).
>
> This is unfortunately needed (or something similar) as there is a
> register in one VIN instance which controls the routing of the incoming
> CSI-2 video streams, not only to itself, but also to other VIN instances
> inside the same SoC.
>
> To be more specific I will try to clarify this using the R-Car H3 as an
> example. On the H3 there are 8 instances of the capture hardware (VIN0 -
> VIN7) and 3 instances off CSI-2 receivers (CSI20, CSI40 and CSI41) which
> receives CSI-2 streams, split the possible multiple virtual channels
> (VC) encoded in CSI-2 streams and forwards it to the VIN's.
>
> The problem is that VIN0 and VIN4 are different from the other VIN's,
> they have one register (CHSEL) which controls the limited number of
> possible routes of video streams between a CSI-2 + VC to a specific VIN.
> The CHSEL register in VIN0 controls the routing for VIN0-3 and the one
> in VIN4 controls VIN4-7 (the two subgroups are similar so lets only
> consider VIN0-3).
>
> There are only a handful of routes possible and the kicker is that
> changing the CHSEL value in VIN0, directly reflects all routes for
> VIN0-VIN3 per this table:
>
> CHSEL reg in VIN0:  0

Re: [PATCH 3/5] media: i2c: Add TDA1997x HDMI receiver driver

[Rob Herring]

On Wed, Nov 15, 2017 at 10:31:14AM -0800, Tim Harvey wrote:
> On Wed, Nov 15, 2017 at 7:52 AM, Rob Herring <r...@kernel.org> wrote:
> > On Thu, Nov 09, 2017 at 10:45:34AM -0800, Tim Harvey wrote:
> >> Add support for the TDA1997x HDMI receivers.
> >>
> >> Cc: Hans Verkuil <hverk...@xs4all.nl>
> >> Signed-off-by: Tim Harvey <thar...@gateworks.com>
> >> ---
> >> v3:
> >>  - use V4L2_DV_BT_FRAME_WIDTH/HEIGHT macros
> >>  - fixed missing break
> >>  - use only hdmi_infoframe_log for infoframe logging
> >>  - simplify tda1997x_s_stream error handling
> >>  - add delayed work proc to handle hotplug enable/disable
> >>  - fix set_edid (disable HPD before writing, enable after)
> >>  - remove enabling edid by default
> >>  - initialize timings
> >>  - take quant range into account in colorspace conversion
> >>  - remove vendor/product tracking (we provide this in log_status via 
> >> infoframes)
> >>  - add v4l_controls
> >>  - add more detail to log_status
> >>  - calculate vhref generator timings
> >>  - timing detection fixes (rounding errors, hswidth errors)
> >>  - rename configure_input/configure_conv functions
> >>
> >> v2:
> >>  - implement dv timings enum/cap
> >>  - remove deprecated g_mbus_config op
> >>  - fix dv_query_timings
> >>  - add EDID get/set handling
> >>  - remove max-pixel-rate support
> >>  - add audio codec DAI support
> >>  - change audio bindings
> >> ---
> >>  drivers/media/i2c/Kconfig|9 +
> >>  drivers/media/i2c/Makefile   |1 +
> >>  drivers/media/i2c/tda1997x.c | 3485 
> >> ++
> >>  include/dt-bindings/media/tda1997x.h |   78 +
> >
> > This belongs with the binding documentation patch.
> >
> 
> Rob,
> 
> Thanks - missed that. I will move it for v4.
> 
> Regarding your previous comment to the v2 series:
> > The rest of the binding looks fine, but I have some reservations about
> > this. I think this should be common probably. There's been a few
> > bindings for display recently that deal with the interface format. Maybe
> > some vendor property is needed here to map a standard interface format
> > back to pin configuration.
> 
> I take it this is not an 'Ack' for the bindings?
> 
> Which did you feel should be made common? I admit I was surprised
> there wasn't a common binding for audio bus format (i2s|spdif) but if
> you were referring to the video data that would probably be much more
> complicated.

The video data. Either you have to try to come up with some way to map 
color components to signals/pins (and even cycles) or you just enumerate 
the formats and keep adding to them when new ones appear. There's h/w 
that allows the former, but in the end you have to interoperate, so 
enumerating the formats is probably enough.

> I was hoping one of the media/driver maintainers would respond to your
> comment with thoughts as I'm not familiar with a very wide variety of
> receivers.

I am hoping, too.

Rob

Re: [PATCH v7 01/25] rcar-vin: add Gen3 devicetree bindings documentation

[Rob Herring]

On Sat, Nov 11, 2017 at 01:38:11AM +0100, Niklas Söderlund wrote:
> Document the devicetree bindings for the CSI-2 inputs available on Gen3.
> 
> There is a need to add a custom property 'renesas,id' and to define
> which CSI-2 input is described in which endpoint under the port@1 node.
> This information is needed since there are a set of predefined routes
> between each VIN and CSI-2 block. This routing table will be kept
> inside the driver but in order for it to act on it it must know which
> VIN and CSI-2 is which.
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  .../devicetree/bindings/media/rcar_vin.txt | 116 
> ++---
>  1 file changed, 104 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
> b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 6e4ef8caf759e5d3..df1abd0fb20386f8 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -2,8 +2,12 @@ Renesas R-Car Video Input driver (rcar_vin)
>  ---
>  
>  The rcar_vin device provides video input capabilities for the Renesas R-Car
> -family of devices. The current blocks are always slaves and suppot one input
> -channel which can be either RGB, YUYV or BT656.
> +family of devices.
> +
> +Each VIN instance has a single parallel input that supports RGB and YUV 
> video,
> +with both external synchronization and BT.656 synchronization for the latter.
> +Depending on the instance the VIN input is connected to external SoC pins, or
> +on Gen3 to a CSI-2 receiver.
>  
>   - compatible: Must be one or more of the following
> - "renesas,vin-r8a7795" for the R8A7795 device
> @@ -28,21 +32,38 @@ channel which can be either RGB, YUYV or BT656.
>  Additionally, an alias named vinX will need to be created to specify
>  which video input device this is.
>  
> -The per-board settings:
> +The per-board settings Gen2:
>   - port sub-node describing a single endpoint connected to the vin
> as described in video-interfaces.txt[1]. Only the first one will
> be considered as each vin interface has one input port.
>  
> -   These settings are used to work out video input format and widths
> -   into the system.
> +The per-board settings Gen3:
> +
> +Gen3 can support both a single connected parallel input source from
> +external SoC pins (port0) and/or multiple parallel input sources from
> +local SoC CSI-2 receivers (port1) depending on SoC.
>  
> +- renesas,id - ID number of the VIN, VINx in the documentation.

Why is this needed? We try to avoid indexes unless that's the only way a 
device is addressed (and then we use reg).

Re: [PATCH v11 1/2] media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation

[Rob Herring]

On Sat, Nov 11, 2017 at 01:25:25AM +0100, Niklas Söderlund wrote:
> Documentation for Renesas R-Car MIPI CSI-2 receiver. The CSI-2 receivers
> are located between the video sources (CSI-2 transmitters) and the video
> grabbers (VIN) on Gen3 of Renesas R-Car SoC.
> 
> Each CSI-2 device is connected to more then one VIN device which
> simultaneously can receive video from the same CSI-2 device. Each VIN
> device can also be connected to more then one CSI-2 device. The routing
> of which link are used are controlled by the VIN devices. There are only
> a few possible routes which are set by hardware limitations, which are
> different for each SoC in the Gen3 family.
> 
> To work with the limitations of routing possibilities it is necessary
> for the DT bindings to describe which VIN device is connected to which
> CSI-2 device. This is why port 1 needs to to assign reg numbers for each
> VIN device that be connected to it. To setup and to know which links are
> valid for each SoC is the responsibility of the VIN driver since the
> register to configure it belongs to the VIN hardware.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
> ---
>  .../bindings/media/renesas,rcar-csi2.txt   | 104 
> +
>  MAINTAINERS|   1 +
>  2 files changed, 105 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt 
> b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> new file mode 100644
> index ..24705d8997b14a10
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> @@ -0,0 +1,104 @@
> +Renesas R-Car MIPI CSI-2
> +
> +
> +The rcar-csi2 device provides MIPI CSI-2 capabilities for the Renesas R-Car
> +family of devices. It is to be used in conjunction with the R-Car VIN module,
> +which provides the video capture capabilities.
> +
> +Mandatory properties
> +
> + - compatible: Must be one or more of the following
> +   - "renesas,r8a7795-csi2" for the R8A7795 device.
> +   - "renesas,r8a7796-csi2" for the R8A7796 device.
> +
> + - reg: the register base and size for the device registers
> + - interrupts: the interrupt for the device
> + - clocks: Reference to the parent clock
> +
> +The device node shall contain two 'port' child nodes according to the
> +bindings defined in Documentation/devicetree/bindings/media/
> +video-interfaces.txt. Port 0 shall connect the node that is the video
> +source for to the CSI-2. Port 1 shall connect all the R-Car VIN
> +modules, which can make use of the CSI-2 module.
> +
> +- Port 0 - Video source (Mandatory)
> + - Endpoint 0 - sub-node describing the endpoint that is the video source
> +
> +- Port 1 - VIN instances (Mandatory for all VIN present in the SoC)
> + - Endpoint 0 - sub-node describing the endpoint that is VIN0
> + - Endpoint 1 - sub-node describing the endpoint that is VIN1
> + - Endpoint 2 - sub-node describing the endpoint that is VIN2
> + - Endpoint 3 - sub-node describing the endpoint that is VIN3
> + - Endpoint 4 - sub-node describing the endpoint that is VIN4
> + - Endpoint 5 - sub-node describing the endpoint that is VIN5
> + - Endpoint 6 - sub-node describing the endpoint that is VIN6
> + - Endpoint 7 - sub-node describing the endpoint that is VIN7
> +
> +Example:
> +
> + csi20: csi2@fea8 {
> + compatible = "renesas,r8a7796-csi2", "renesas,rcar-gen3-csi2";
> + reg = <0 0xfea8 0 0x1>;
> + interrupts = <0 184 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = < CPG_MOD 714>;
> + power-domains = < R8A7796_PD_ALWAYS_ON>;
> + resets = < 714>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <0>;
> +
> + csi20_in: endpoint@0 {

unit-address without reg property is not valid.

Otherwise,

Acked-by: Rob Herring <r...@kernel.org>


> + clock-lanes = <0>;
> + data-lanes = <1>;
> + remote-endpoint = <_txb>;
> 

Re: [PATCH 3/5] media: i2c: Add TDA1997x HDMI receiver driver

[Rob Herring]

On Thu, Nov 09, 2017 at 10:45:34AM -0800, Tim Harvey wrote:
> Add support for the TDA1997x HDMI receivers.
> 
> Cc: Hans Verkuil 
> Signed-off-by: Tim Harvey 
> ---
> v3:
>  - use V4L2_DV_BT_FRAME_WIDTH/HEIGHT macros
>  - fixed missing break
>  - use only hdmi_infoframe_log for infoframe logging
>  - simplify tda1997x_s_stream error handling
>  - add delayed work proc to handle hotplug enable/disable
>  - fix set_edid (disable HPD before writing, enable after)
>  - remove enabling edid by default
>  - initialize timings
>  - take quant range into account in colorspace conversion
>  - remove vendor/product tracking (we provide this in log_status via 
> infoframes)
>  - add v4l_controls
>  - add more detail to log_status
>  - calculate vhref generator timings
>  - timing detection fixes (rounding errors, hswidth errors)
>  - rename configure_input/configure_conv functions
> 
> v2:
>  - implement dv timings enum/cap
>  - remove deprecated g_mbus_config op
>  - fix dv_query_timings
>  - add EDID get/set handling
>  - remove max-pixel-rate support
>  - add audio codec DAI support
>  - change audio bindings
> ---
>  drivers/media/i2c/Kconfig|9 +
>  drivers/media/i2c/Makefile   |1 +
>  drivers/media/i2c/tda1997x.c | 3485 
> ++
>  include/dt-bindings/media/tda1997x.h |   78 +

This belongs with the binding documentation patch.

>  include/media/i2c/tda1997x.h |   53 +
>  5 files changed, 3626 insertions(+)
>  create mode 100644 drivers/media/i2c/tda1997x.c
>  create mode 100644 include/dt-bindings/media/tda1997x.h
>  create mode 100644 include/media/i2c/tda1997x.h

Re: [RESEND PATCH 1/1] of: Make return types of to_of_node and of_fwnode_handle macros consistent

[Rob Herring]

On Thu, Nov 02, 2017 at 11:59:18AM +0200, Sakari Ailus wrote:
> (Fixed Mauro's e-mail.)
> 
> to_of_node() macro checks whether the fwnode_handle passed to it is not an
> OF node, and if so, returns NULL in order to be NULL-safe. Otherwise it
> returns the pointer to the OF node which the fwnode_handle contains.
> 
> The problem with returning NULL is that its type was void *, which
> sometimes matters. Explicitly return struct device_node * instead.
> 
> Make a similar change to of_fwnode_handle() as well.
> 
> Fixes: d20dc1493db4 ("of: Support const and non-const use for to_of_node()")
> Fixes: debd3a3b27c7 ("of: Make of_fwnode_handle() safer")
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> ---
> Hi Mauro,
> 
> Could you check whether this addresses the smatch issue with the xilinx
> driver?
> 
> Thanks.
> 
>  include/linux/of.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Acked-by: Rob Herring <r...@kernel.org>