linux-kernel - Re: [PATCH v5 1/5] dt-bindings: media: add mediatek ISP3.0 sensor interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEHHSvZad9sSo+ubKdgwvdzjVz7mf9gqL551J+n831q+uoJOSA@mail.gmail.com>
Date: Fri, 5 Jul 2024 11:35:27 +0200
From: Julien Stephan <jstephan@...libre.com>
To: Conor Dooley <conor.dooley@...rochip.com>
Cc: Conor Dooley <conor@...nel.org>, Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
	Andy Hsieh <andy.hsieh@...iatek.com>, Mauro Carvalho Chehab <mchehab@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, linux-media@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org, 
	Louis Kuo <louis.kuo@...iatek.com>, Phi-Bang Nguyen <pnguyen@...libre.com>
Subject: Re: [PATCH v5 1/5] dt-bindings: media: add mediatek ISP3.0 sensor interface

Le ven. 5 juil. 2024 à 11:24, Conor Dooley
<conor.dooley@...rochip.com> a écrit :
>
> On Fri, Jul 05, 2024 at 09:50:59AM +0200, Julien Stephan wrote:
> > Le jeu. 4 juil. 2024 à 18:27, Conor Dooley <conor@...nel.org> a écrit :
> > >
> > > On Thu, Jul 04, 2024 at 03:36:40PM +0200, Julien Stephan wrote:
> > > > From: Louis Kuo <louis.kuo@...iatek.com>
> > > >
> > > > This adds the bindings, for the mediatek ISP3.0 SENINF module embedded in
> > > > some Mediatek SoC, such as the mt8365
> > > >
> > > > Signed-off-by: Louis Kuo <louis.kuo@...iatek.com>
> > > > Signed-off-by: Phi-Bang Nguyen <pnguyen@...libre.com>
> > > > Link: https://lore.kernel.org/r/20230807094940.329165-2-jstephan@baylibre.com
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> > > > Signed-off-by: Julien Stephan <jstephan@...libre.com>
> > >
> > > I'm really confused by the link tag here. At first glance this looked
> > > like you were sending out something that had been applied by Laurent,
> > > given the Link, Rb and SoB from him. Why does he have a SoB on this
> > > patch? What did Phi-Bang Nguyen do with this patch, and should they have
> > > a Co-developed-by tag?
> >
> > I was not using b4 for the previous revisions of this series, so maybe
> > I messed something up here :(
>
> b4 am has an option to add a link to a patch you apply from the mailing
> list (-l, --add-link) but you should not be using that as a contributor.
> In this case, that link provides no value and is just confusing.
>
> > About Phi-Bang, this series has been in our internal tree for a long
> > time, and Phi-Bang has his SoB on it, so I kept it.
> >
> > About Laurent's tags, they were already on v4. But maybe it was an
> > error ? Should I remove them?
>
> They were also on v1. Did Laurent write part of these bindings, and
> should he have a Co-developed-by?
>

Got it! I understood where I messed up :)
I'll remove the link and add the Co-developed-by tag of Laurent

> > > > +additionalProperties: false
> > > > +
> > > > +if:
> > > > +  properties:
> > > > +    compatible:
> > > > +      contains:
> > > > +        const: mediatek,mt8365-seninf
> > >
> > > The binding supports only a single compatible, why is this complexity
> > > required? I don't see other devices being added in this series.
> >
> > Right. The idea is that the number of PHYs depends on the SoC. In the
> > previous revision of the series,
> > the number of PHYs was not fixed, and Krzysztof asked me to fix it by
> > SoC. So I wanted to make it clear
> > that the number of PHYs depends on SoC but maybe I don't need that
> > complexity for that?
> >
> > Is something like the following enough? And if complexity is added
> > later if some other SoC are added?
>
> Yes, that looks reasonable to me. Adding conditional stuff can be done
> iff another soc re-uses the binding.

Will do in the next series.
Thank you for your  feedback on this!

Cheers
Julien
>
> Thanks,
> Conor.
>
> >     phys:
> >       minItems: 2
> >       maxItems: 2
> >       description:
> >         phandle to the PHYs connected to CSI0/A, CSI1, CSI0B
> >
> >     phy-names:
> >       description:
> >         list of PHYs names
> >       minItems: 2
> >       maxItems: 2
> >       items:
> >         type: string
> >         enum:
> >           - csi0
> >           - csi1
> >           - csi0b
> >       uniqueItems: true