[RFC v4] V4L DT bindings

[RFC v4] V4L DT bindings

[Guennadi Liakhovetski]

Hi all

After an initial RFC [1] and taking into consideration an even earlier 
patch-set [2], Sylwester and I have spent some time discussing V4L DT 
bindings, below is a result of those discussions.

We have chosen to try to design a DT example, documentation and 
implementation should follow. I'll try to bring together just several most 
important points, that might not be immediately obvious from the example.

1. Sylwester has initially designed his patches around a concept of a 
central "video" node, that contains (references to) all video devices on 
the system. This might make finding all relevant components easier and 
should make power management more readily available. In the below example 
such a node is missing. For now we decided not to require one, but systems 
may choose to use them. Support for them might be added to the V4L DT 
subsystem later.

2. The below example contains the following 4 components:
   (a) an SoC bridge (CEU node "ceu0@0xfe910000"), note, that the bridge 
       is also providing a master clock "mclk: master_clock" to sensors
   (b) a CSI-2 interface "csi2: csi2@0xffc90000", that can be used with 
       the above bridge
   (c) an I2C parallel camera sensor "ov772x_1: ov772x@0x21"
   (d) an I2C serial (MIPI CSI-2) camera sensor "imx074: imx074@0x1a"

3. Linking of various components follows the V4L2 MC concept: each video 
node can contain "xxx: videolink@x" child nodes. These nodes specify the 
opposite end of the link and a local pad configuration. This is required, 
because two linked pads might require different configuration. E.g., if 
the board contains an inverter in the camera vertical sync line, 
respective pads have to be configured with opposite vsync polarities.

4. In the below example the following links are defined:
   (a) "ov772x_1_1: videolink@1" is a child of the CEU node, it links the 
       CEU with the ov772x sensor.
   (b) "csi2_0: videolink@0" is also child of the CEU node, it links the 
       CEU with the CSI-2 module. Note, that this link might not be 
       necessary, since this is an SoC internal connection and drivers 
       will know themselves how to configure it
   (c) "ceu0_1: videolink@0" is a chile of the OV772x node
   (d) "csi2_0_1: videolink@0" is a child of the IMX074 camera sensor node
   (e) "imx074_1: videolink@1" is a child of the CSI-2 node
   (f) "ceu0: videolink@0" is a child of the CSI-2 node - also might not 
       be required

5. Remote node references in videolinks are unidirectional. I.e., 
videolink nodes on downstream devices (e.g., the bridge) reference 
phandles of upstream nodes (e.g., sensors), but not the other way round. 
This should be sufficient for the proposed probing method:
   (a) external subdevices like sensors are children on their respective 
       busses (e.g., I2C) and can be probed before the bridge. In this 
       case probing can fail, because the master clock is not supplied 
       yet. Therefore the sensor driver will have to request deferred 
       probing.
   (b) the bridge device is probed, the driver instantiates the clock, 
       before returning the driver registers a notifier (in this case on 
       the I2C bus)
   (c) sensor .probe() is tried again, this time the clock is available, 
       so, this time probing succeeds
   (d) the bridge driver notifier is called, it scans videolink child 
       nodes, finds a match, gets a link to the subdevice

6. In the below example we are using the "reg" property to enumerate 
videolink child nodes. Doubts have been expressed previous in thread [1] 
about validity of such use. If it's proven, that "reg" shouldn't be used 
in this case, a new property shall be introduced.

7. Concerning data lines. We have chosen to use "bus-width" and 
"data-shift" for parallel busses and new properties "data-lanes" and 
"clock-lanes" to describe pin assignment on MIPI CSI-2 devices and 
additionally a "bus-width" property per videolink child of CSI-2 
controllers to specify how many data lanes are actually used for this 
link.

Any comments welcome.

It's been a pleasure working on this together with Sylwester, it is a pity 
he won't be coming to the KS this time, hopefully, we'll continue this 
cooperation during upcoming discussion and implementation phases.

[1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/50755
[2] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/11143

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

// Describe an imaginary configuration: a CEU serving either a parallel ov7725
// sensor, or a serial imx074 sensor, connected over a CSI-2 SoC interface
//
// Any vendor-specific properties here are only provided as examples. The
// emphasis is on common media properties. If any of mentioned here vendor-
// specific properties seem to be common enough, they can be promoted to
// generic ones.

	ceu0@0xfe910000 {
		compatible = "renesas,sh-mobile-ceu";
		reg = <0xfe910000 0xa0>;
		interrupts = <0x880>;
		bus-width = <16>;		/* #lines routed on the board */
		#address-cells = <1>;
		#size-cells = <0>;

		mclk: master_clock {
			compatible = "renesas,ceu-clock";
			#clock-cells = <1>;
			clock-frequency = <50000000>;	/* max clock frequency */
			clock-output-names = "mclk";
		};

		...
		ov772x_1_1: videolink@1 {
			reg = <1>;		/* local pad # */
			client = <&ov772x_1 0>; /* remote phandle and pad # */
			bus-width = <8>;	/* used data lines */
			data-shift = <0>;	/* lines 7:0 are used */

			/* If [hv]sync-active are missing, embedded bt.605 sync is used */
			hsync-active = <1>;	/* active high */
			vsync-active = <1>;	/* active high */
			pclk-sample = <1>;	/* rising */
		};
		csi2_0: videolink@0 {
			reg = <0>;
			client = <&csi2 0>;
			immutable;
		};
	};

	i2c0: i2c@0xfff20000 {
		...
		ov772x_1: ov772x@0x21 {
			compatible = "omnivision,ov772x";
			reg = <0x21>;
			vddio-supply = <&regulator1>;
			vddcore-supply = <&regulator2>;
			bus-width = <10>;

			clock-frequency = <20000000>;
			clocks = <&mclk 0>;
			clock-names = "mclk"            

			#address-cells = <1>;
			#size-cells = <0>;
			...
			ceu0_1: videolink@0 {
				reg = <0>;		/* link configuration to local pad #0 */
				bus-width = <8>;
				hsync-active = <1>;
				hsync-active = <0>;	/* who came up with an inverter here?... */
				pclk-sample = <1>;
			};
		};

		imx074: imx074@0x1a {
			compatible = "sony,imx074";
			reg = <0x1a>;
			vddio-supply = <&regulator1>;
			vddcore-supply = <&regulator2>;
			clock-lanes = <0>;
			data-lanes = <1>, <2>;

			clock-frequency = <30000000>;	/* shared clock with ov772x_1 */
			clocks = <&mclk 0>;
			clock-names = "mclk"            

			#address-cells = <1>;
			#size-cells = <0>;
			...
			csi2_0_1: videolink@0 {
				reg = <0>;		/* link configuration to local pad #0 */
				bus-width = <2>;	/* 2 lanes, fixed roles, also described above */
			};
		};
		...
	};

	csi2: csi2@0xffc90000 {
		compatible = "renesas,sh-mobile-csi2";
		reg = <0xffc90000 0x1000>;
		interrupts = <0x17a0>;
		#address-cells = <1>;
		#size-cells = <0>;

		/* Ok to have them global? */
		clock-lanes = <0>;
		data-lanes = <2>, <1>;

		...
		imx074_1: videolink@1 {
			reg = <1>;
			client = <&imx074 0>;
			bus-width = <2>;

			csi2-ecc;
			csi2-crc;

			renesas,csi2-phy = <0>;
		};
		ceu0: videolink@0 {
			reg = <0>;
			immutable;
		};
	};

Re: [RFC v4] V4L DT bindings

[Nicolas THERY]

Hello,

I've got a couple of questions regarding lane swapping and
polarity inversion.

On 2012-08-25 01:27, Guennadi Liakhovetski wrote:
> Hi all
> 
> After an initial RFC [1] and taking into consideration an even earlier 
> patch-set [2], Sylwester and I have spent some time discussing V4L DT 
> bindings, below is a result of those discussions.
> 
> We have chosen to try to design a DT example, documentation and 
> implementation should follow. I'll try to bring together just several most 
> important points, that might not be immediately obvious from the example.
> 
> 1. Sylwester has initially designed his patches around a concept of a 
> central "video" node, that contains (references to) all video devices on 
> the system. This might make finding all relevant components easier and 
> should make power management more readily available. In the below example 
> such a node is missing. For now we decided not to require one, but systems 
> may choose to use them. Support for them might be added to the V4L DT 
> subsystem later.
> 
> 2. The below example contains the following 4 components:
>    (a) an SoC bridge (CEU node "ceu0@0xfe910000"), note, that the bridge 
>        is also providing a master clock "mclk: master_clock" to sensors
>    (b) a CSI-2 interface "csi2: csi2@0xffc90000", that can be used with 
>        the above bridge
>    (c) an I2C parallel camera sensor "ov772x_1: ov772x@0x21"
>    (d) an I2C serial (MIPI CSI-2) camera sensor "imx074: imx074@0x1a"
> 
> 3. Linking of various components follows the V4L2 MC concept: each video 
> node can contain "xxx: videolink@x" child nodes. These nodes specify the 
> opposite end of the link and a local pad configuration. This is required, 
> because two linked pads might require different configuration. E.g., if 
> the board contains an inverter in the camera vertical sync line, 
> respective pads have to be configured with opposite vsync polarities.
> 
> 4. In the below example the following links are defined:
>    (a) "ov772x_1_1: videolink@1" is a child of the CEU node, it links the 
>        CEU with the ov772x sensor.
>    (b) "csi2_0: videolink@0" is also child of the CEU node, it links the 
>        CEU with the CSI-2 module. Note, that this link might not be 
>        necessary, since this is an SoC internal connection and drivers 
>        will know themselves how to configure it
>    (c) "ceu0_1: videolink@0" is a chile of the OV772x node
>    (d) "csi2_0_1: videolink@0" is a child of the IMX074 camera sensor node
>    (e) "imx074_1: videolink@1" is a child of the CSI-2 node
>    (f) "ceu0: videolink@0" is a child of the CSI-2 node - also might not 
>        be required
> 
> 5. Remote node references in videolinks are unidirectional. I.e., 
> videolink nodes on downstream devices (e.g., the bridge) reference 
> phandles of upstream nodes (e.g., sensors), but not the other way round. 
> This should be sufficient for the proposed probing method:
>    (a) external subdevices like sensors are children on their respective 
>        busses (e.g., I2C) and can be probed before the bridge. In this 
>        case probing can fail, because the master clock is not supplied 
>        yet. Therefore the sensor driver will have to request deferred 
>        probing.
>    (b) the bridge device is probed, the driver instantiates the clock, 
>        before returning the driver registers a notifier (in this case on 
>        the I2C bus)
>    (c) sensor .probe() is tried again, this time the clock is available, 
>        so, this time probing succeeds
>    (d) the bridge driver notifier is called, it scans videolink child 
>        nodes, finds a match, gets a link to the subdevice
> 
> 6. In the below example we are using the "reg" property to enumerate 
> videolink child nodes. Doubts have been expressed previous in thread [1] 
> about validity of such use. If it's proven, that "reg" shouldn't be used 
> in this case, a new property shall be introduced.
> 
> 7. Concerning data lines. We have chosen to use "bus-width" and 
> "data-shift" for parallel busses and new properties "data-lanes" and 
> "clock-lanes" to describe pin assignment on MIPI CSI-2 devices and 
> additionally a "bus-width" property per videolink child of CSI-2 
> controllers to specify how many data lanes are actually used for this 
> link.
> 
> Any comments welcome.
> 
> It's been a pleasure working on this together with Sylwester, it is a pity 
> he won't be coming to the KS this time, hopefully, we'll continue this 
> cooperation during upcoming discussion and implementation phases.
> 
> [1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/50755
> [2] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/11143
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 
> // Describe an imaginary configuration: a CEU serving either a parallel ov7725
> // sensor, or a serial imx074 sensor, connected over a CSI-2 SoC interface
> //
> // Any vendor-specific properties here are only provided as examples. The
> // emphasis is on common media properties. If any of mentioned here vendor-
> // specific properties seem to be common enough, they can be promoted to
> // generic ones.
> 
> 	ceu0@0xfe910000 {
> 		compatible = "renesas,sh-mobile-ceu";
> 		reg = <0xfe910000 0xa0>;
> 		interrupts = <0x880>;
> 		bus-width = <16>;		/* #lines routed on the board */
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		mclk: master_clock {
> 			compatible = "renesas,ceu-clock";
> 			#clock-cells = <1>;
> 			clock-frequency = <50000000>;	/* max clock frequency */
> 			clock-output-names = "mclk";
> 		};
> 
> 		...
> 		ov772x_1_1: videolink@1 {
> 			reg = <1>;		/* local pad # */
> 			client = <&ov772x_1 0>; /* remote phandle and pad # */
> 			bus-width = <8>;	/* used data lines */
> 			data-shift = <0>;	/* lines 7:0 are used */
> 
> 			/* If [hv]sync-active are missing, embedded bt.605 sync is used */
> 			hsync-active = <1>;	/* active high */
> 			vsync-active = <1>;	/* active high */
> 			pclk-sample = <1>;	/* rising */
> 		};
> 		csi2_0: videolink@0 {
> 			reg = <0>;
> 			client = <&csi2 0>;
> 			immutable;
> 		};
> 	};
> 
> 	i2c0: i2c@0xfff20000 {
> 		...
> 		ov772x_1: ov772x@0x21 {
> 			compatible = "omnivision,ov772x";
> 			reg = <0x21>;
> 			vddio-supply = <&regulator1>;
> 			vddcore-supply = <&regulator2>;
> 			bus-width = <10>;
> 
> 			clock-frequency = <20000000>;
> 			clocks = <&mclk 0>;
> 			clock-names = "mclk"            
> 
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 			...
> 			ceu0_1: videolink@0 {
> 				reg = <0>;		/* link configuration to local pad #0 */
> 				bus-width = <8>;
> 				hsync-active = <1>;
> 				hsync-active = <0>;	/* who came up with an inverter here?... */
> 				pclk-sample = <1>;
> 			};
> 		};
> 
> 		imx074: imx074@0x1a {
> 			compatible = "sony,imx074";
> 			reg = <0x1a>;
> 			vddio-supply = <&regulator1>;
> 			vddcore-supply = <&regulator2>;
> 			clock-lanes = <0>;
> 			data-lanes = <1>, <2>;
> 
> 			clock-frequency = <30000000>;	/* shared clock with ov772x_1 */
> 			clocks = <&mclk 0>;
> 			clock-names = "mclk"            
> 
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 			...
> 			csi2_0_1: videolink@0 {
> 				reg = <0>;		/* link configuration to local pad #0 */
> 				bus-width = <2>;	/* 2 lanes, fixed roles, also described above */
> 			};
> 		};
> 		...
> 	};
> 
> 	csi2: csi2@0xffc90000 {
> 		compatible = "renesas,sh-mobile-csi2";
> 		reg = <0xffc90000 0x1000>;
> 		interrupts = <0x17a0>;
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		/* Ok to have them global? */
> 		clock-lanes = <0>;
> 		data-lanes = <2>, <1>;

In imx074@0x1a above, the data-lanes property is <1>, <2>.  Is it
reversed here to show that lanes are swapped between the sensor and the
CSI rx?  If not, how to express lane swapping?

> 		...
> 		imx074_1: videolink@1 {
> 			reg = <1>;
> 			client = <&imx074 0>;
> 			bus-width = <2>;
> 
> 			csi2-ecc;
> 			csi2-crc;
> 
> 			renesas,csi2-phy = <0>;
> 		};
> 		ceu0: videolink@0 {
> 			reg = <0>;
> 			immutable;
> 		};
> 	};

How to express that the positive and negative signals of a given
clock/data lane are inversed?  Is it somehow with the hsync-active
property?

Actually there may be two positive/negative inversion cases to consider:

- the positive/negative signals are inversed both in low-power and
  high-speed modes (e.g. physical lines between sensor module and SoC
  are swapped on the PCB);

- the positive/negative signals are inversed in high-speed mode only
  (the sensor and CSI rx use opposite polarities in high-speed mode).

Thanks in advance.

Best regards,
Nicolas

Re: [RFC v4] V4L DT bindings

[Sylwester Nawrocki]

On 08/30/2012 05:19 PM, Nicolas THERY wrote:
>> 	i2c0: i2c@0xfff20000 {
>> 		...
>> 		ov772x_1: ov772x@0x21 {
>> 			compatible = "omnivision,ov772x";
>> 			reg =<0x21>;
>> 			vddio-supply =<&regulator1>;
>> 			vddcore-supply =<&regulator2>;
>> 			bus-width =<10>;
>>
>> 			clock-frequency =<20000000>;
>> 			clocks =<&mclk 0>;
>> 			clock-names = "mclk"
>>
>> 			#address-cells =<1>;
>> 			#size-cells =<0>;
>> 			...
>> 			ceu0_1: videolink@0 {
>> 				reg =<0>;		/* link configuration to local pad #0 */
>> 				bus-width =<8>;
>> 				hsync-active =<1>;
>> 				hsync-active =<0>;	/* who came up with an inverter here?... */
>> 				pclk-sample =<1>;
>> 			};
>> 		};
>>
>> 		imx074: imx074@0x1a {
>> 			compatible = "sony,imx074";
>> 			reg =<0x1a>;
>> 			vddio-supply =<&regulator1>;
>> 			vddcore-supply =<&regulator2>;
>> 			clock-lanes =<0>;
>> 			data-lanes =<1>,<2>;
>>
>> 			clock-frequency =<30000000>;	/* shared clock with ov772x_1 */
>> 			clocks =<&mclk 0>;
>> 			clock-names = "mclk"
>>
>> 			#address-cells =<1>;
>> 			#size-cells =<0>;
>> 			...
>> 			csi2_0_1: videolink@0 {
>> 				reg =<0>;		/* link configuration to local pad #0 */
>> 				bus-width =<2>;	/* 2 lanes, fixed roles, also described above */
>> 			};
>> 		};
>> 		...
>> 	};
>>
>> 	csi2: csi2@0xffc90000 {
>> 		compatible = "renesas,sh-mobile-csi2";
>> 		reg =<0xffc90000 0x1000>;
>> 		interrupts =<0x17a0>;
>> 		#address-cells =<1>;
>> 		#size-cells =<0>;
>>
>> 		/* Ok to have them global? */

I'm not sure, maybe it's better to move it under videolink@1 node,
to keep it together with 'bus-width' property ?

>> 		clock-lanes =<0>;
>> 		data-lanes =<2>,<1>;
> 
> In imx074@0x1a above, the data-lanes property is<1>,<2>.  Is it
> reversed here to show that lanes are swapped between the sensor and the
> CSI rx?  If not, how to express lane swapping?

Yes, this indicates lanes remapping at the receiver.

Probably we could make it a single value with length determined by
'bus-width', since we're going to use 'bus-width' for CSI buses as well, 
(optionally) in addition to 'clock-lanes' and 'data-lanes' ?

>> 		...
>> 		imx074_1: videolink@1 {
>> 			reg =<1>;
>> 			client =<&imx074 0>;
>> 			bus-width =<2>;
>>
>> 			csi2-ecc;
>> 			csi2-crc;
>>
>> 			renesas,csi2-phy =<0>;
>> 		};
>> 		ceu0: videolink@0 {
>> 			reg =<0>;
>> 			immutable;
>> 		};
>> 	};
> 
> How to express that the positive and negative signals of a given
> clock/data lane are inversed?  Is it somehow with the hsync-active
> property?

Hmm, I don't think this is covered in this RFC. hsync-active is mostly
intended for the parallel buses. We need to come up with new properties
to handle CSI data/clock lane polarity swapping. There was a short
discussion about that already:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg41724.html

> Actually there may be two positive/negative inversion cases to consider:
> 
> - the positive/negative signals are inversed both in low-power and
>    high-speed modes (e.g. physical lines between sensor module and SoC
>    are swapped on the PCB);
> 
> - the positive/negative signals are inversed in high-speed mode only
>    (the sensor and CSI rx use opposite polarities in high-speed mode).

Then is this positive/negative LVDS lines swapping separately configurable
in hardware for low-power and high-speed mode ? What is an advantage of it ?

One possible solution would be to have a one to two elements array property,
e.g.

lanes-polarity = <0 0 0 0 0>, <1 1 1 1 1>;

where the first entry would indicate lanes polarity for high speed mode and
the second one for low power mode. For receivers/transmitters that don't
allow to configure the polarities separately for different bus states there
could be just one entry. The width of each element could be determined by 
value of the 'bus-width' property + 1.

Would it make sense ?

--

Regards,
Sylwester

V4L DT @ plumbers (was Re: [RFC v4] V4L DT bindings)

[Guennadi Liakhovetski]

Hi all

Do we still want to try to organise a short discussion of this while at 
plumbers? Maybe tomorrow during or around lunch? Or any other time for 
that matter?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: V4L DT @ plumbers (was Re: [RFC v4] V4L DT bindings)

[Laurent Pinchart]

Hi Guennadi,

On Thursday 30 August 2012 22:58:17 Guennadi Liakhovetski wrote:
> Hi all
> 
> Do we still want to try to organise a short discussion of this while at
> plumbers? Maybe tomorrow during or around lunch? Or any other time for
> that matter?

I'm certainly interested. I have a meeting tomorrow at a still unknown type, 
apart from that I'm free.

-- 
Regards,

Laurent Pinchart

Re: V4L DT @ plumbers (was Re: [RFC v4] V4L DT bindings)

[Guennadi Liakhovetski]

On Fri, 31 Aug 2012, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Thursday 30 August 2012 22:58:17 Guennadi Liakhovetski wrote:
> > Hi all
> > 
> > Do we still want to try to organise a short discussion of this while at
> > plumbers? Maybe tomorrow during or around lunch? Or any other time for
> > that matter?
> 
> I'm certainly interested. I have a meeting tomorrow at a still unknown type, 
> apart from that I'm free.

How about beginning tomorrow at 11:55 (Nautilus 3 & 4 are still free 
according to the schedule) and then continuing after the lunch for another 
hour if needed (first talk in the afternoon tomorrow is at 14:10).

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [RFC v4] V4L DT bindings

[Nicolas THERY]

Hello,

Thanks for the feedback.

On 2012-08-30 22:21, Sylwester Nawrocki wrote:
> On 08/30/2012 05:19 PM, Nicolas THERY wrote:

[snip]

>> In imx074@0x1a above, the data-lanes property is<1>,<2>.  Is it
>> reversed here to show that lanes are swapped between the sensor and the
>> CSI rx?  If not, how to express lane swapping?
> 
> Yes, this indicates lanes remapping at the receiver.
> 
> Probably we could make it a single value with length determined by
> 'bus-width', since we're going to use 'bus-width' for CSI buses as well, 
> (optionally) in addition to 'clock-lanes' and 'data-lanes' ?

Looks good to me.

[snip]

>> How to express that the positive and negative signals of a given
>> clock/data lane are inversed?  Is it somehow with the hsync-active
>> property?
> 
> Hmm, I don't think this is covered in this RFC. hsync-active is mostly
> intended for the parallel buses. We need to come up with new properties
> to handle CSI data/clock lane polarity swapping. There was a short
> discussion about that already:
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg41724.html
> 
>> Actually there may be two positive/negative inversion cases to consider:
>>
>> - the positive/negative signals are inversed both in low-power and
>>    high-speed modes (e.g. physical lines between sensor module and SoC
>>    are swapped on the PCB);
>>
>> - the positive/negative signals are inversed in high-speed mode only
>>    (the sensor and CSI rx use opposite polarities in high-speed mode).
> 
> Then is this positive/negative LVDS lines swapping separately configurable
> in hardware for low-power and high-speed mode ? 

Yes.

> What is an advantage of it ?

I suspect our hardware people after years of experience with
not-so-compliant sensors and weird PCBs have adopted a
belt-and-suspenders approach and made configurable everything they
thought could go wrong.

In the "inversion in high-speed mode only" case, that could be because
the sensor does not use the standard-specified polarity.

> One possible solution would be to have a one to two elements array property,
> e.g.
> 
> lanes-polarity = <0 0 0 0 0>, <1 1 1 1 1>;
> 
> where the first entry would indicate lanes polarity for high speed mode and
> the second one for low power mode. For receivers/transmitters that don't
> allow to configure the polarities separately for different bus states there
> could be just one entry. The width of each element could be determined by 
> value of the 'bus-width' property + 1.
> 
> Would it make sense ?

Yes, that looks fine.

Incidentally is it okay to extend DT nodes with manufacturer-specific
properties?  I'm asking because our CSI rx supports other esoteric
lane-related configuration knobs, e.g. for impedance tuning.  We'd like
to put them in the DT but they probably don't warrant an official
property.

Thanks a lot again.

Best regards,
Nicolas

Re: [RFC v4] V4L DT bindings

[Nicolas THERY]

Hello,

On 2012-08-25 01:27, Guennadi Liakhovetski wrote:

[snip]

> 	csi2: csi2@0xffc90000 {
> 		compatible = "renesas,sh-mobile-csi2";
> 		reg = <0xffc90000 0x1000>;
> 		interrupts = <0x17a0>;
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		/* Ok to have them global? */
> 		clock-lanes = <0>;
> 		data-lanes = <2>, <1>;
> 
> 		...
> 		imx074_1: videolink@1 {
> 			reg = <1>;
> 			client = <&imx074 0>;
> 			bus-width = <2>;
> 
> 			csi2-ecc;
> 			csi2-crc;
> 
> 			renesas,csi2-phy = <0>;
> 		};
> 		ceu0: videolink@0 {
> 			reg = <0>;
> 			immutable;
> 		};
> 	};

videolink@1 makes the description of the CSI-2 rx board-specific.  Would it be
possible to keep the description of the SoC nodes board-agnostic to ease reuse
of the SoC description in multiple board DTs?

Would this be as simple as replacing &imx074 with a generic well-known name
defined in the board part of the DT?

Best regards,
Nicolas

Re: [RFC v4] V4L DT bindings

[Sylwester Nawrocki]

On 08/31/2012 08:46 AM, Nicolas THERY wrote:
> Hello,
> 
> Thanks for the feedback.
> 
> On 2012-08-30 22:21, Sylwester Nawrocki wrote:
>> On 08/30/2012 05:19 PM, Nicolas THERY wrote:
> 
> [snip]
> 
>>> In imx074@0x1a above, the data-lanes property is<1>,<2>.  Is it
>>> reversed here to show that lanes are swapped between the sensor and the
>>> CSI rx?  If not, how to express lane swapping?
>>
>> Yes, this indicates lanes remapping at the receiver.
>>
>> Probably we could make it a single value with length determined by
>> 'bus-width', since we're going to use 'bus-width' for CSI buses as well,
>> (optionally) in addition to 'clock-lanes' and 'data-lanes' ?
> 
> Looks good to me.
> 
> [snip]
> 
>>> How to express that the positive and negative signals of a given
>>> clock/data lane are inversed?  Is it somehow with the hsync-active
>>> property?
>>
>> Hmm, I don't think this is covered in this RFC. hsync-active is mostly
>> intended for the parallel buses. We need to come up with new properties
>> to handle CSI data/clock lane polarity swapping. There was a short
>> discussion about that already:
>> http://www.mail-archive.com/linux-media@vger.kernel.org/msg41724.html
>>
>>> Actually there may be two positive/negative inversion cases to consider:
>>>
>>> - the positive/negative signals are inversed both in low-power and
>>>     high-speed modes (e.g. physical lines between sensor module and SoC
>>>     are swapped on the PCB);
>>>
>>> - the positive/negative signals are inversed in high-speed mode only
>>>     (the sensor and CSI rx use opposite polarities in high-speed mode).
>>
>> Then is this positive/negative LVDS lines swapping separately configurable
>> in hardware for low-power and high-speed mode ?
> 
> Yes.
> 
>> What is an advantage of it ?
> 
> I suspect our hardware people after years of experience with
> not-so-compliant sensors and weird PCBs have adopted a
> belt-and-suspenders approach and made configurable everything they
> thought could go wrong.
> 
> In the "inversion in high-speed mode only" case, that could be because
> the sensor does not use the standard-specified polarity.

OK, thanks for the clarification.

>> One possible solution would be to have a one to two elements array property,
>> e.g.
>>
>> lanes-polarity =<0 0 0 0 0>,<1 1 1 1 1>;
>>
>> where the first entry would indicate lanes polarity for high speed mode and
>> the second one for low power mode. For receivers/transmitters that don't
>> allow to configure the polarities separately for different bus states there
>> could be just one entry. The width of each element could be determined by
>> value of the 'bus-width' property + 1.
>>
>> Would it make sense ?
> 
> Yes, that looks fine.
> 
> Incidentally is it okay to extend DT nodes with manufacturer-specific
> properties?  I'm asking because our CSI rx supports other esoteric
> lane-related configuration knobs, e.g. for impedance tuning.  We'd like
> to put them in the DT but they probably don't warrant an official
> property.

My understanding is that it's just fine to have uncommon DT properties, 
as long as they describe hardware in OS agnostic way. I believe our goal 
is to define as many common bindings as possible, in order to make it 
possible to have devices from various manufacturers and their drivers 
interworking seamlessly, possibly without a need for S/W interfaces 
modifications. Hence it's useful to list in these discussions as many 
hardware properties, that may seem device-specific at first glance, 
as possible. 

I would expect some share of device specific bindings in most of 
the cases. We're mostly trying to avoid having same things defined 
in different ways.

--

Regards,
Sylwester

[RFC v5] V4L DT bindings

[Guennadi Liakhovetski]

Hi all

Version 5 of this RFC is a result of a discussion of its version 4, which 
took place during the recent Linux Plumbers conference in San Diego. 
Changes are:

(1) remove bus-width properties from device (bridge and client) top level. 
This has actually already been decided upon during our discussions with 
Sylwester, I just forgot to actually remove them, sorry.

(2) links are now grouped under "ports." This should help better describe 
device connection topology by making clearer, which interfaces links are 
attached to. (help needed: in my notes I see "port" optional if only one 
port is present, but I seem to remember, that the final decision was - 
make ports compulsory for uniformity. Which one is true?)

(3) "videolink" is renamed to just "link."

(4) "client" is renamed to "remote" and is now used on both sides of 
links.

(5) clock-names in clock consumer nodes (e.g., camera sensors) should 
reflect clock input pin names from respective datasheets

(6) also serial bus description should be placed under respective link 
nodes.

(7) reference remote link DT nodes in "remote" properties, not to the 
parent.

(8) use standard names for "SoC-external" (e.g., i2c) devices on their 
respective busses. "Sensor" has been proposed, maybe "camera" is a better 
match though.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

// Describe an imaginary configuration: a CEU serving either a parallel ov7725
// sensor, or a serial imx074 sensor, connected over a CSI-2 SoC interface

	ceu0: ceu@0xfe910000 {
		compatible = "renesas,sh-mobile-ceu";
		reg = <0xfe910000 0xa0>;
		interrupts = <0x880>;

		mclk: master_clock {
			compatible = "renesas,ceu-clock";
			#clock-cells = <1>;
			clock-frequency = <50000000>;	/* max clock frequency */
			clock-output-names = "mclk";
		};

		...
		port@0 {
			#address-cells = <1>;
			#size-cells = <0>;

			ov772x_1_1: link@1 {
				reg = <1>;		/* local pad # */
				remote = <&ceu0_1>;	/* remote phandle and pad # */
				bus-width = <8>;	/* used data lines */
				data-shift = <0>;	/* lines 7:0 are used */
	
				/* If [hv]sync-active are missing, embedded bt.605 sync is used */
				hsync-active = <1>;	/* active high */
				vsync-active = <1>;	/* active high */
				pclk-sample = <1>;	/* rising */
			};

			csi2_0: link@0 {
				reg = <0>;
				remote = <&ceu0_2>;
				immutable;
			};
		};
	};

	i2c0: i2c@0xfff20000 {
		...
		ov772x_1: camera@0x21 {
			compatible = "omnivision,ov772x";
			reg = <0x21>;
			vddio-supply = <&regulator1>;
			vddcore-supply = <&regulator2>;

			clock-frequency = <20000000>;
			clocks = <&mclk 0>;
			clock-names = "xclk";

			...
			port {
				/* With 1 link per port no need in addresses */
				ceu0_1: link@0 {
					bus-width = <8>;
					remote = <&ov772x_1_1>;
					hsync-active = <1>;
					hsync-active = <0>;	/* who came up with an inverter here?... */
					pclk-sample = <1>;
				};
			};
		};

		imx074: camera@0x1a {
			compatible = "sony,imx074";
			reg = <0x1a>;
			vddio-supply = <&regulator1>;
			vddcore-supply = <&regulator2>;

			clock-frequency = <30000000>;	/* shared clock with ov772x_1 */
			clocks = <&mclk 0>;
			clock-names = "sysclk";		/* assuming this is the name in the datasheet */
			...
			port {
				csi2_1: link@0 {
					clock-lanes = <0>;
					data-lanes = <1>, <2>;
					remote = <&imx074_1>;
				};
			};
		};
		...
	};

	csi2: csi2@0xffc90000 {
		compatible = "renesas,sh-mobile-csi2";
		reg = <0xffc90000 0x1000>;
		interrupts = <0x17a0>;
		#address-cells = <1>;
		#size-cells = <0>;
		...
		port {
			compatible = "renesas,csi2c";	/* one of CSI2I and CSI2C */
			reg = <1>;			/* CSI-2 PHY #1 of 2: PHY_S, PHY_M has port address 0, is unused */

			imx074_1: link@1 {
				client = <&imx074 0>;
				clock-lanes = <0>;
				data-lanes = <2>, <1>;
				remote = <&csi2_1>;
			};
		};
		port {
			reg = <2>;			/* port 2: link to the CEU */
			ceu0_2: link@0 {
				immutable;
				remote = <&csi2_0>;
			};
		};
	};

Re: [RFC v5] V4L DT bindings

[Stephen Warren]

On 09/05/2012 04:57 AM, Guennadi Liakhovetski wrote:
> Hi all
> 
> Version 5 of this RFC is a result of a discussion of its version 4, which 
> took place during the recent Linux Plumbers conference in San Diego. 
> Changes are:
> 
> (1) remove bus-width properties from device (bridge and client) top level. 
> This has actually already been decided upon during our discussions with 
> Sylwester, I just forgot to actually remove them, sorry.
> 
> (2) links are now grouped under "ports." This should help better describe 
> device connection topology by making clearer, which interfaces links are 
> attached to. (help needed: in my notes I see "port" optional if only one 
> port is present, but I seem to remember, that the final decision was - 
> make ports compulsory for uniformity. Which one is true?)

I'd tend to make the port node compulsory.

A related question: What code is going to parse all the port/link
sub-nodes in a device? And, how does it know which sub-nodes are ports,
and which are something else entirely? Perhaps the algorithm is that all
port nodes must be named "port"?

If there were (optionally) no port node, I think the answer to that
question would be a lot more complex, hence why I advocate for it always
being there.

> (3) "videolink" is renamed to just "link."
> 
> (4) "client" is renamed to "remote" and is now used on both sides of 
> links.
> 
> (5) clock-names in clock consumer nodes (e.g., camera sensors) should 
> reflect clock input pin names from respective datasheets
> 
> (6) also serial bus description should be placed under respective link 
> nodes.
> 
> (7) reference remote link DT nodes in "remote" properties, not to the 
> parent.
> 
> (8) use standard names for "SoC-external" (e.g., i2c) devices on their 
> respective busses. "Sensor" has been proposed, maybe "camera" is a better 
> match though.
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 
> // Describe an imaginary configuration: a CEU serving either a parallel ov7725
> // sensor, or a serial imx074 sensor, connected over a CSI-2 SoC interface
> 
> 	ceu0: ceu@0xfe910000 {
> 		compatible = "renesas,sh-mobile-ceu";
> 		reg = <0xfe910000 0xa0>;
> 		interrupts = <0x880>;
> 
> 		mclk: master_clock {
> 			compatible = "renesas,ceu-clock";
> 			#clock-cells = <1>;
> 			clock-frequency = <50000000>;	/* max clock frequency */
> 			clock-output-names = "mclk";
> 		};
> 
> 		...
> 		port@0 {

Since there's only 1 port node here, you can drop the "@0" here.

> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			ov772x_1_1: link@1 {

This isn't a comment on the binding definition, but on the example
itself. The label names ("ov772x_1_1" here and "csi2_0" below) feel
backwards to me; I'd personally use label names that describe the object
being labelled, rather than the object that's linked to the node being
labelled. In other words, "ceu0_1" here, and "ov772x_1" at the far end
of this link. But, these are just arbitrary names, so you can name/label
everything whatever you want and it'll still work.

> 				reg = <1>;		/* local pad # */
> 				remote = <&ceu0_1>;	/* remote phandle and pad # */
> 				bus-width = <8>;	/* used data lines */
> 				data-shift = <0>;	/* lines 7:0 are used */
> 	
> 				/* If [hv]sync-active are missing, embedded bt.605 sync is used */
> 				hsync-active = <1>;	/* active high */
> 				vsync-active = <1>;	/* active high */
> 				pclk-sample = <1>;	/* rising */
> 			};
> 
> 			csi2_0: link@0 {
> 				reg = <0>;
> 				remote = <&ceu0_2>;
> 				immutable;
> 			};
> 		};
> 	};
> 
> 	i2c0: i2c@0xfff20000 {
> 		...
> 		ov772x_1: camera@0x21 {
> 			compatible = "omnivision,ov772x";
> 			reg = <0x21>;
> 			vddio-supply = <&regulator1>;
> 			vddcore-supply = <&regulator2>;
> 
> 			clock-frequency = <20000000>;
> 			clocks = <&mclk 0>;
> 			clock-names = "xclk";
> 
> 			...
> 			port {
> 				/* With 1 link per port no need in addresses */
> 				ceu0_1: link@0 {

You can drop "@0" there too.

> 					bus-width = <8>;
> 					remote = <&ov772x_1_1>;
> 					hsync-active = <1>;
> 					hsync-active = <0>;	/* who came up with an inverter here?... */
> 					pclk-sample = <1>;
> 				};
> 			};
> 		};
> 
> 		imx074: camera@0x1a {
> 			compatible = "sony,imx074";
> 			reg = <0x1a>;
> 			vddio-supply = <&regulator1>;
> 			vddcore-supply = <&regulator2>;
> 
> 			clock-frequency = <30000000>;	/* shared clock with ov772x_1 */
> 			clocks = <&mclk 0>;
> 			clock-names = "sysclk";		/* assuming this is the name in the datasheet */
> 			...
> 			port {
> 				csi2_1: link@0 {

You can drop "@0" there too.

> 					clock-lanes = <0>;
> 					data-lanes = <1>, <2>;
> 					remote = <&imx074_1>;
> 				};
> 			};
> 		};
> 		...
> 	};
> 
> 	csi2: csi2@0xffc90000 {
> 		compatible = "renesas,sh-mobile-csi2";
> 		reg = <0xffc90000 0x1000>;
> 		interrupts = <0x17a0>;
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		...
> 		port {
> 			compatible = "renesas,csi2c";	/* one of CSI2I and CSI2C */
> 			reg = <1>;			/* CSI-2 PHY #1 of 2: PHY_S, PHY_M has port address 0, is unused */
> 
> 			imx074_1: link@1 {

You can drop "@1" there too.

> 				client = <&imx074 0>;
> 				clock-lanes = <0>;
> 				data-lanes = <2>, <1>;
> 				remote = <&csi2_1>;
> 			};
> 		};
> 		port {

There are two nodes named "port" here; I think they should be "port@1"
and "port@2" based on the reg properties.

> 			reg = <2>;			/* port 2: link to the CEU */
> 			ceu0_2: link@0 {

You can drop "@0" there too.

> 				immutable;
> 				remote = <&csi2_0>;
> 			};
> 		};
> 	};

Aside from those minor comments, I think the overall structure of the
bindings looks good.

Re: [RFC v5] V4L DT bindings

[Guennadi Liakhovetski]

Hi Stephen

Thanks for the review.

On Wed, 5 Sep 2012, Stephen Warren wrote:

> On 09/05/2012 04:57 AM, Guennadi Liakhovetski wrote:
> > Hi all
> > 
> > Version 5 of this RFC is a result of a discussion of its version 4, which 
> > took place during the recent Linux Plumbers conference in San Diego. 
> > Changes are:
> > 
> > (1) remove bus-width properties from device (bridge and client) top level. 
> > This has actually already been decided upon during our discussions with 
> > Sylwester, I just forgot to actually remove them, sorry.
> > 
> > (2) links are now grouped under "ports." This should help better describe 
> > device connection topology by making clearer, which interfaces links are 
> > attached to. (help needed: in my notes I see "port" optional if only one 
> > port is present, but I seem to remember, that the final decision was - 
> > make ports compulsory for uniformity. Which one is true?)
> 
> I'd tend to make the port node compulsory.
> 
> A related question: What code is going to parse all the port/link
> sub-nodes in a device?

I think we'll have to make a generic V4L DT parser. We certainly don't 
want each driver reimplement this.

> And, how does it know which sub-nodes are ports,
> and which are something else entirely? Perhaps the algorithm is that all
> port nodes must be named "port"?

Yes, that was the idea. Is anything speaking against it?

> If there were (optionally) no port node, I think the answer to that
> question would be a lot more complex, hence why I advocate for it always
> being there.

Ok, fine with me.

All other your comments address various issues with specific DT node 
instances, not with the design itself. I'll address them in the next 
version, which I'm also planning to accompany with a proper 
Documentation/devicetree/bindings patch.

Thanks
Guennadi

> > (3) "videolink" is renamed to just "link."
> > 
> > (4) "client" is renamed to "remote" and is now used on both sides of 
> > links.
> > 
> > (5) clock-names in clock consumer nodes (e.g., camera sensors) should 
> > reflect clock input pin names from respective datasheets
> > 
> > (6) also serial bus description should be placed under respective link 
> > nodes.
> > 
> > (7) reference remote link DT nodes in "remote" properties, not to the 
> > parent.
> > 
> > (8) use standard names for "SoC-external" (e.g., i2c) devices on their 
> > respective busses. "Sensor" has been proposed, maybe "camera" is a better 
> > match though.
> > 
> > Thanks
> > Guennadi
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
> > 
> > // Describe an imaginary configuration: a CEU serving either a parallel ov7725
> > // sensor, or a serial imx074 sensor, connected over a CSI-2 SoC interface
> > 
> > 	ceu0: ceu@0xfe910000 {
> > 		compatible = "renesas,sh-mobile-ceu";
> > 		reg = <0xfe910000 0xa0>;
> > 		interrupts = <0x880>;
> > 
> > 		mclk: master_clock {
> > 			compatible = "renesas,ceu-clock";
> > 			#clock-cells = <1>;
> > 			clock-frequency = <50000000>;	/* max clock frequency */
> > 			clock-output-names = "mclk";
> > 		};
> > 
> > 		...
> > 		port@0 {
> 
> Since there's only 1 port node here, you can drop the "@0" here.
> 
> > 			#address-cells = <1>;
> > 			#size-cells = <0>;
> > 
> > 			ov772x_1_1: link@1 {
> 
> This isn't a comment on the binding definition, but on the example
> itself. The label names ("ov772x_1_1" here and "csi2_0" below) feel
> backwards to me; I'd personally use label names that describe the object
> being labelled, rather than the object that's linked to the node being
> labelled. In other words, "ceu0_1" here, and "ov772x_1" at the far end
> of this link. But, these are just arbitrary names, so you can name/label
> everything whatever you want and it'll still work.
> 
> > 				reg = <1>;		/* local pad # */
> > 				remote = <&ceu0_1>;	/* remote phandle and pad # */
> > 				bus-width = <8>;	/* used data lines */
> > 				data-shift = <0>;	/* lines 7:0 are used */
> > 	
> > 				/* If [hv]sync-active are missing, embedded bt.605 sync is used */
> > 				hsync-active = <1>;	/* active high */
> > 				vsync-active = <1>;	/* active high */
> > 				pclk-sample = <1>;	/* rising */
> > 			};
> > 
> > 			csi2_0: link@0 {
> > 				reg = <0>;
> > 				remote = <&ceu0_2>;
> > 				immutable;
> > 			};
> > 		};
> > 	};
> > 
> > 	i2c0: i2c@0xfff20000 {
> > 		...
> > 		ov772x_1: camera@0x21 {
> > 			compatible = "omnivision,ov772x";
> > 			reg = <0x21>;
> > 			vddio-supply = <&regulator1>;
> > 			vddcore-supply = <&regulator2>;
> > 
> > 			clock-frequency = <20000000>;
> > 			clocks = <&mclk 0>;
> > 			clock-names = "xclk";
> > 
> > 			...
> > 			port {
> > 				/* With 1 link per port no need in addresses */
> > 				ceu0_1: link@0 {
> 
> You can drop "@0" there too.
> 
> > 					bus-width = <8>;
> > 					remote = <&ov772x_1_1>;
> > 					hsync-active = <1>;
> > 					hsync-active = <0>;	/* who came up with an inverter here?... */
> > 					pclk-sample = <1>;
> > 				};
> > 			};
> > 		};
> > 
> > 		imx074: camera@0x1a {
> > 			compatible = "sony,imx074";
> > 			reg = <0x1a>;
> > 			vddio-supply = <&regulator1>;
> > 			vddcore-supply = <&regulator2>;
> > 
> > 			clock-frequency = <30000000>;	/* shared clock with ov772x_1 */
> > 			clocks = <&mclk 0>;
> > 			clock-names = "sysclk";		/* assuming this is the name in the datasheet */
> > 			...
> > 			port {
> > 				csi2_1: link@0 {
> 
> You can drop "@0" there too.
> 
> > 					clock-lanes = <0>;
> > 					data-lanes = <1>, <2>;
> > 					remote = <&imx074_1>;
> > 				};
> > 			};
> > 		};
> > 		...
> > 	};
> > 
> > 	csi2: csi2@0xffc90000 {
> > 		compatible = "renesas,sh-mobile-csi2";
> > 		reg = <0xffc90000 0x1000>;
> > 		interrupts = <0x17a0>;
> > 		#address-cells = <1>;
> > 		#size-cells = <0>;
> > 		...
> > 		port {
> > 			compatible = "renesas,csi2c";	/* one of CSI2I and CSI2C */
> > 			reg = <1>;			/* CSI-2 PHY #1 of 2: PHY_S, PHY_M has port address 0, is unused */
> > 
> > 			imx074_1: link@1 {
> 
> You can drop "@1" there too.
> 
> > 				client = <&imx074 0>;
> > 				clock-lanes = <0>;
> > 				data-lanes = <2>, <1>;
> > 				remote = <&csi2_1>;
> > 			};
> > 		};
> > 		port {
> 
> There are two nodes named "port" here; I think they should be "port@1"
> and "port@2" based on the reg properties.
> 
> > 			reg = <2>;			/* port 2: link to the CEU */
> > 			ceu0_2: link@0 {
> 
> You can drop "@0" there too.
> 
> > 				immutable;
> > 				remote = <&csi2_0>;
> > 			};
> > 		};
> > 	};
> 
> Aside from those minor comments, I think the overall structure of the
> bindings looks good.
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [RFC v5] V4L DT bindings

[Stephen Warren]

On 09/11/2012 08:02 AM, Guennadi Liakhovetski wrote:
> Hi Stephen
> 
> Thanks for the review.
> 
> On Wed, 5 Sep 2012, Stephen Warren wrote:
> 
>> On 09/05/2012 04:57 AM, Guennadi Liakhovetski wrote:
>>> Hi all
>>>
>>> Version 5 of this RFC is a result of a discussion of its version 4, which 
>>> took place during the recent Linux Plumbers conference in San Diego. 
>>> Changes are:
>>>
>>> (1) remove bus-width properties from device (bridge and client) top level. 
>>> This has actually already been decided upon during our discussions with 
>>> Sylwester, I just forgot to actually remove them, sorry.
>>>
>>> (2) links are now grouped under "ports." This should help better describe 
>>> device connection topology by making clearer, which interfaces links are 
>>> attached to. (help needed: in my notes I see "port" optional if only one 
>>> port is present, but I seem to remember, that the final decision was - 
>>> make ports compulsory for uniformity. Which one is true?)
>>
>> I'd tend to make the port node compulsory.
>>
>> A related question: What code is going to parse all the port/link
>> sub-nodes in a device?
> 
> I think we'll have to make a generic V4L DT parser. We certainly don't 
> want each driver reimplement this.
> 
>> And, how does it know which sub-nodes are ports,
>> and which are something else entirely? Perhaps the algorithm is that all
>> port nodes must be named "port"?
> 
> Yes, that was the idea. Is anything speaking against it?

I think that's fine; it's certainly a nice and simple requirement. It's
just a rule that will have to be thought about when designing bindings
for all the devices that use this feature, to make sure they don't
define any other kind of "port" node that would confuse the parser.

I suppose if this ever becomes a problem, an individual binding could
choose to avoid conflicts by placing the "port" nodes in some specific
child node of its device node, and the driver would pass the name of
that node into the common parsing code, which would default to using the
device's main node when not otherwise specified. However, we should
avoid the conflicts if we can. In other words:

Normal:

/foo {
    port@0 { ... };
    port@1 { ... };
};

If there's ever a need to resolve some conflict with that standard layout:

/foo {
    media-ports {
        port@0 { ... };
        port@1 { ... };
    };
};