Hi Jacopo, Thanks for your feedback. On 2022-01-17 11:00:40 +0100, Jacopo Mondi wrote: > Hi Niklas, > > On Mon, Jan 17, 2022 at 10:23:28AM +0100, Niklas Söderlund wrote: > > Hello Jacopo, > > > > On 2022-01-17 09:11:10 +0100, Jacopo Mondi wrote: > > > Hello Prabhakar, > > > > > > On Thu, Jan 13, 2022 at 10:32:14AM +0000, Lad Prabhakar wrote: > > > > CSI-2 (CSI4LNK0) on R-Car and RZ/G2 supports 4-lane mode which is already > > > > handled by rcar-csi2.c driver. This patch updates the data-lanes property > > > > to describe the same. > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > > --- > > > > .../devicetree/bindings/media/renesas,csi2.yaml | 9 ++++++++- > > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/renesas,csi2.yaml b/Documentation/devicetree/bindings/media/renesas,csi2.yaml > > > > index e6a036721082..064a0a4c5737 100644 > > > > --- a/Documentation/devicetree/bindings/media/renesas,csi2.yaml > > > > +++ b/Documentation/devicetree/bindings/media/renesas,csi2.yaml > > > > @@ -67,7 +67,14 @@ properties: > > > > maxItems: 1 > > > > > > > > data-lanes: > > > > - maxItems: 1 > > > > + items: > > > > + minItems: 1 > > > > + maxItems: 4 > > > > + items: > > > > + - const: 1 > > > > + - const: 2 > > > > + - const: 3 > > > > + - const: 4 > > > > > > Seeing "maxItems: 1" there confuses me too, as the property is an > > > array of data-lanes, but I'm afraid your change does not what you > > > intend as it would allow you to specify the number of data lanes as an > > > integer rather than as an array. > > > > > > I think it would probably be correct to set > > > > > > data-lanes: true > > > > > > (maybe maxItems: 1 is correct already) > > > > > > And restrict the number of valid combinations in the board DTS file > > > with a construct like: > > > > > > data-lanes: > > > oneOf: > > > - items: > > > - const: 1 > > > - const: 2 > > > - const: 3 > > > - const: 4 > > > - items: > > > - const: 1 > > > - const: 2 > > > > I don't think this is correct, what if data lanes 2 and 3 are used? > > > > These were examples that allow you to accept <1 2> and <1 2 3 4> as > valid properties. If other combinations are accepted they can be > specified there, in your example, <2 3> with > > - items: > - const: 2 > - const: 3 > > As lane re-reordering is quite unusual as a feature (afaik) there are > usually just an handful of supported combinations for 1, 2 and 4 data > lanes setups. R-Car CSI-2 hardware and driver supports full lane swapping, see the LSWAP register and usage of struct rcar_csi2.lane_swap. I think it's a good idea to extend the binding description to limit the data-lanes property to an array of max 4 items where each value use is ether a 1, 2, 3 or 4. But it must allow for any combination of the values. > > If full lane re-ordering is supported then it's enough to set > data-lanes: true and accepts all combinations. > > Also, the reason why imho the property should go in the board DTS and > not in the SoC .dtsi is that not all the available data lanes of the > IP-core might be routed out on a specific board. > > That's at least my understanding which I would be glad to be disproved > as specifying the valid combinations in each board dts is rather > un-convenient. > > Thanks > j > > > > > > > Thanks > > > j > > > > > > > > > > > required: > > > > - clock-lanes > > > > -- > > > > 2.17.1 > > > > > > > > -- > > Kind Regards, > > Niklas Söderlund -- Kind Regards, Niklas Söderlund
