| Message ID | 20240223140445.1885083-3-alexander.stein@ew.tq-group.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i.MX8M Nano ISI single port support | expand |
Hi Alexander, Thank you for the patch. On Fri, Feb 23, 2024 at 03:04:44PM +0100, Alexander Stein wrote: > In case the hardware only supports just one pipeline, allow using a > single port node as well. This is frowned upon in DT bindings, as it makes them more complicated for little gain. The recommendation is to always use a ports node if a device can have multiple ports for at least one of its compatibles. > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > .../devicetree/bindings/media/nxp,imx8-isi.yaml | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml > index 4d5348d456a1f..f855f3cc91fea 100644 > --- a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml > +++ b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml > @@ -53,6 +53,12 @@ properties: > power-domains: > maxItems: 1 > > + port: > + $ref: /schemas/graph.yaml#/properties/port > + description: | > + Port representing the Pixel Link input to the ISI. Used for > + single-pipeline models. The port shall have a single endpoint. > + > ports: > $ref: /schemas/graph.yaml#/properties/ports > description: | > @@ -66,7 +72,6 @@ required: > - clocks > - clock-names > - fsl,blk-ctrl > - - ports > > allOf: > - if: > @@ -87,6 +92,11 @@ allOf: > port@1: false > required: > - port@0 > + oneOf: > + - required: > + - port > + - required: > + - ports > > - if: > properties: > @@ -106,6 +116,8 @@ allOf: > required: > - port@0 > - port@1 > + required: > + - ports > > additionalProperties: false >
On Fri, Feb 23, 2024 at 04:16:31PM +0200, Laurent Pinchart wrote: > Hi Alexander, > > Thank you for the patch. > > On Fri, Feb 23, 2024 at 03:04:44PM +0100, Alexander Stein wrote: > > In case the hardware only supports just one pipeline, allow using a > > single port node as well. > > This is frowned upon in DT bindings, as it makes them more complicated > for little gain. The recommendation is to always use a ports node if a > device can have multiple ports for at least one of its compatibles. And reading the cover letter, I see this causes warnings. I think we need guidance from Rob on this. > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > > --- > > .../devicetree/bindings/media/nxp,imx8-isi.yaml | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml > > index 4d5348d456a1f..f855f3cc91fea 100644 > > --- a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml > > @@ -53,6 +53,12 @@ properties: > > power-domains: > > maxItems: 1 > > > > + port: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: | > > + Port representing the Pixel Link input to the ISI. Used for > > + single-pipeline models. The port shall have a single endpoint. > > + > > ports: > > $ref: /schemas/graph.yaml#/properties/ports > > description: | > > @@ -66,7 +72,6 @@ required: > > - clocks > > - clock-names > > - fsl,blk-ctrl > > - - ports > > > > allOf: > > - if: > > @@ -87,6 +92,11 @@ allOf: > > port@1: false > > required: > > - port@0 > > + oneOf: > > + - required: > > + - port > > + - required: > > + - ports > > > > - if: > > properties: > > @@ -106,6 +116,8 @@ allOf: > > required: > > - port@0 > > - port@1 > > + required: > > + - ports > > > > additionalProperties: false > >
On 23/02/2024 15:17, Laurent Pinchart wrote: > On Fri, Feb 23, 2024 at 04:16:31PM +0200, Laurent Pinchart wrote: >> Hi Alexander, >> >> Thank you for the patch. >> >> On Fri, Feb 23, 2024 at 03:04:44PM +0100, Alexander Stein wrote: >>> In case the hardware only supports just one pipeline, allow using a >>> single port node as well. >> >> This is frowned upon in DT bindings, as it makes them more complicated >> for little gain. The recommendation is to always use a ports node if a >> device can have multiple ports for at least one of its compatibles. > > And reading the cover letter, I see this causes warnings. I think we > need guidance from Rob on this. Here was similar case: https://lore.kernel.org/all/20240227142440.GA3863852-robh@kernel.org/ and @Rob recommendation was to just use ports. It's true it causes warnings... or I should say - it was causing warnings (one of my last warnings in Samsung DTS for W=1). I wonder what's the base of this patchset? https://lore.kernel.org/linux-samsung-soc/20231122-dtc-warnings-v2-1-bd4087325392@kernel.org/ Best regards, Krzysztof
On 29/02/2024 19:14, Krzysztof Kozlowski wrote: > On 23/02/2024 15:17, Laurent Pinchart wrote: >> On Fri, Feb 23, 2024 at 04:16:31PM +0200, Laurent Pinchart wrote: >>> Hi Alexander, >>> >>> Thank you for the patch. >>> >>> On Fri, Feb 23, 2024 at 03:04:44PM +0100, Alexander Stein wrote: >>>> In case the hardware only supports just one pipeline, allow using a >>>> single port node as well. >>> >>> This is frowned upon in DT bindings, as it makes them more complicated >>> for little gain. The recommendation is to always use a ports node if a >>> device can have multiple ports for at least one of its compatibles. >> >> And reading the cover letter, I see this causes warnings. I think we >> need guidance from Rob on this. > > Here was similar case: > https://lore.kernel.org/all/20240227142440.GA3863852-robh@kernel.org/ > and @Rob recommendation was to just use ports. > > It's true it causes warnings... or I should say - it was causing > warnings (one of my last warnings in Samsung DTS for W=1). > > I wonder what's the base of this patchset? > > https://lore.kernel.org/linux-samsung-soc/20231122-dtc-warnings-v2-1-bd4087325392@kernel.org/ Uh, wait, this was not merged, so the warning still appears. Anyway the preference is simpler schema, so just "ports". Best regards, Krzysztof
Hi Krzysztof, Am Donnerstag, 29. Februar 2024, 19:18:14 CET schrieb Krzysztof Kozlowski: > On 29/02/2024 19:14, Krzysztof Kozlowski wrote: > > On 23/02/2024 15:17, Laurent Pinchart wrote: > >> On Fri, Feb 23, 2024 at 04:16:31PM +0200, Laurent Pinchart wrote: > >>> Hi Alexander, > >>> > >>> Thank you for the patch. > >>> > >>> On Fri, Feb 23, 2024 at 03:04:44PM +0100, Alexander Stein wrote: > >>>> In case the hardware only supports just one pipeline, allow using a > >>>> single port node as well. > >>> > >>> This is frowned upon in DT bindings, as it makes them more complicated > >>> for little gain. The recommendation is to always use a ports node if a > >>> device can have multiple ports for at least one of its compatibles. > >> > >> And reading the cover letter, I see this causes warnings. I think we > >> need guidance from Rob on this. > > > > Here was similar case: > > https://lore.kernel.org/all/20240227142440.GA3863852-robh@kernel.org/ > > and @Rob recommendation was to just use ports. > > > > It's true it causes warnings... or I should say - it was causing > > warnings (one of my last warnings in Samsung DTS for W=1). > > > > I wonder what's the base of this patchset? > > > > https://lore.kernel.org/linux-samsung-soc/20231122-dtc-warnings-v2-1-bd4087325392@kernel.org/ > > Uh, wait, this was not merged, so the warning still appears. Anyway the > preference is simpler schema, so just "ports". Okay, thanks for that information. I'll drop this patch then. Just to be on the same side, this implies that using a single port in this case ( see patch 3) is not necessary/wanted, no? If so, I'll drop patch 3 as well. Best regards, Alexander
On 01/03/2024 09:32, Alexander Stein wrote: > Hi Krzysztof, > > Am Donnerstag, 29. Februar 2024, 19:18:14 CET schrieb Krzysztof Kozlowski: >> On 29/02/2024 19:14, Krzysztof Kozlowski wrote: >>> On 23/02/2024 15:17, Laurent Pinchart wrote: >>>> On Fri, Feb 23, 2024 at 04:16:31PM +0200, Laurent Pinchart wrote: >>>>> Hi Alexander, >>>>> >>>>> Thank you for the patch. >>>>> >>>>> On Fri, Feb 23, 2024 at 03:04:44PM +0100, Alexander Stein wrote: >>>>>> In case the hardware only supports just one pipeline, allow using a >>>>>> single port node as well. >>>>> >>>>> This is frowned upon in DT bindings, as it makes them more complicated >>>>> for little gain. The recommendation is to always use a ports node if a >>>>> device can have multiple ports for at least one of its compatibles. >>>> >>>> And reading the cover letter, I see this causes warnings. I think we >>>> need guidance from Rob on this. >>> >>> Here was similar case: >>> https://lore.kernel.org/all/20240227142440.GA3863852-robh@kernel.org/ >>> and @Rob recommendation was to just use ports. >>> >>> It's true it causes warnings... or I should say - it was causing >>> warnings (one of my last warnings in Samsung DTS for W=1). >>> >>> I wonder what's the base of this patchset? >>> >>> https://lore.kernel.org/linux-samsung-soc/20231122-dtc-warnings-v2-1-bd4087325392@kernel.org/ >> >> Uh, wait, this was not merged, so the warning still appears. Anyway the >> preference is simpler schema, so just "ports". > > Okay, thanks for that information. I'll drop this patch then. > Just to be on the same side, this implies that using a single port > in this case ( see patch 3) is not necessary/wanted, no? > If so, I'll drop patch 3 as well. Both patches are related, so if you drop this one, you cannot have #3. Drop this and #3 as well. Best regards, Krzysztof
On Fri, Feb 23, 2024 at 04:17:31PM +0200, Laurent Pinchart wrote: > On Fri, Feb 23, 2024 at 04:16:31PM +0200, Laurent Pinchart wrote: > > Hi Alexander, > > > > Thank you for the patch. > > > > On Fri, Feb 23, 2024 at 03:04:44PM +0100, Alexander Stein wrote: > > > In case the hardware only supports just one pipeline, allow using a > > > single port node as well. > > > > This is frowned upon in DT bindings, as it makes them more complicated > > for little gain. The recommendation is to always use a ports node if a > > device can have multiple ports for at least one of its compatibles. > > And reading the cover letter, I see this causes warnings. I think we > need guidance from Rob on this. The warning is for: ports { port@0 {}; }; It should/could be changed like this to fix it: ports { port {}; }; But I've also said some warnings are guidance, not absolute. This is one of them. Some devices have optional port@1. In those cases, switching between 'port' and 'port@0' depending on 'port@1' makes little sense. Rob
diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml index 4d5348d456a1f..f855f3cc91fea 100644 --- a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml +++ b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml @@ -53,6 +53,12 @@ properties: power-domains: maxItems: 1 + port: + $ref: /schemas/graph.yaml#/properties/port + description: | + Port representing the Pixel Link input to the ISI. Used for + single-pipeline models. The port shall have a single endpoint. + ports: $ref: /schemas/graph.yaml#/properties/ports description: | @@ -66,7 +72,6 @@ required: - clocks - clock-names - fsl,blk-ctrl - - ports allOf: - if: @@ -87,6 +92,11 @@ allOf: port@1: false required: - port@0 + oneOf: + - required: + - port + - required: + - ports - if: properties: @@ -106,6 +116,8 @@ allOf: required: - port@0 - port@1 + required: + - ports additionalProperties: false
In case the hardware only supports just one pipeline, allow using a single port node as well. Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> --- .../devicetree/bindings/media/nxp,imx8-isi.yaml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)