Le ven. 5 juil. 2024 à 11:24, Conor Dooley <conor.dooley@xxxxxxxxxxxxx> 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@xxxxxxxxxx> a écrit : > > > > > > On Thu, Jul 04, 2024 at 03:36:40PM +0200, Julien Stephan wrote: > > > > From: Louis Kuo <louis.kuo@xxxxxxxxxxxx> > > > > > > > > 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@xxxxxxxxxxxx> > > > > Signed-off-by: Phi-Bang Nguyen <pnguyen@xxxxxxxxxxxx> > > > > Link: https://lore.kernel.org/r/20230807094940.329165-2-jstephan@xxxxxxxxxxxx > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > > > Signed-off-by: Julien Stephan <jstephan@xxxxxxxxxxxx> > > > > > > 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
