Re: [PATCH v5 1/5] dt-bindings: media: add mediatek ISP3.0 sensor interface
From: Julien Stephan <jstephan@baylibre.com>
Date: 2024-07-05 09:35:40
Also in:
linux-devicetree, linux-media, linux-mediatek, lkml
Le ven. 5 juil. 2024 à 11:24, Conor Dooley [off-list ref] a écrit :
On Fri, Jul 05, 2024 at 09:50:59AM +0200, Julien Stephan wrote:quoted
Le jeu. 4 juil. 2024 à 18:27, Conor Dooley [off-list ref] a écrit :quoted
On Thu, Jul 04, 2024 at 03:36:40PM +0200, Julien Stephan wrote:quoted
From: Louis Kuo <redacted> 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 <redacted> Signed-off-by: Phi-Bang Nguyen <redacted> Link: https://lore.kernel.org/r/20230807094940.329165-2-jstephan@baylibre.com (local) Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Julien Stephan <jstephan@baylibre.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.quoted
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
quoted
quoted
quoted
+additionalProperties: false + +if: + properties: + compatible: + contains: + const: mediatek,mt8365-seninfThe 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.quoted
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