From: "Niklas Söderlund" <niklas.soderlund@ragnatech.se>
To: Jacopo Mondi <jacopo@jmondi.org>
Cc: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Prabhakar <prabhakar.csengg@gmail.com>,
Biju Das <biju.das.jz@bp.renesas.com>,
linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] media: dt-bindings: media: renesas,csi2: Update data-lanes property
Date: Tue, 18 Jan 2022 11:33:08 +0100 [thread overview]
Message-ID: <YeaXZO+3C/fUM7ex@oden.dyn.berto.se> (raw)
In-Reply-To: <20220117100040.wa3ple6meahebtni@uno.localdomain>
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@bp.renesas.com>
> > > > ---
> > > > .../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
next prev parent reply other threads:[~2022-01-18 10:33 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-13 10:32 [PATCH] media: dt-bindings: media: renesas,csi2: Update data-lanes property Lad Prabhakar
2022-01-17 8:11 ` Jacopo Mondi
2022-01-17 9:23 ` Niklas Söderlund
2022-01-17 10:00 ` Jacopo Mondi
2022-01-18 10:33 ` Niklas Söderlund [this message]
2022-01-18 16:22 ` Jacopo Mondi
2022-01-18 9:11 ` Lad, Prabhakar
2022-01-18 10:55 ` Lad, Prabhakar
2022-01-18 16:34 ` Jacopo Mondi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YeaXZO+3C/fUM7ex@oden.dyn.berto.se \
--to=niklas.soderlund@ragnatech.se \
--cc=biju.das.jz@bp.renesas.com \
--cc=devicetree@vger.kernel.org \
--cc=geert+renesas@glider.be \
--cc=jacopo@jmondi.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=prabhakar.csengg@gmail.com \
--cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).