[PATCH 0/3] i.MX8M Nano ISI single port support

[PATCH 0/3] i.MX8M Nano ISI single port support

[Alexander Stein]

Hi,

I'm trying to fix the dtbs_check warning:
/soc@0/bus@32c00000/isi@32e20000/ports: graph node has single child node
'port@0', #address-cells/#size-cells are not necessary

OF graphs with a single port should be placed inside a 'port' node, rather than
a single 'port' node inside a 'ports' node. Bindings were designed for the
latter case.
Adjust the bindings so a seconds port (port@1) is not allowed for
single-pipeline models and allow both 'ports' (current one) and 'port' based
OF graph connections for single-pipeline models.

Best regards,
Alexander

Alexander Stein (3):
  media: dt-bindings: nxp,imx8-isi: Refuse port@1 for single pipeline
    models
  media: dt-bindings: nxp,imx8-isi: Allow single port for single
    pipeline models
  arm64: dts: imx8mn: Use single port for ISI

 .../devicetree/bindings/media/nxp,imx8-isi.yaml   | 15 ++++++++++++++-
 arch/arm64/boot/dts/freescale/imx8mn.dtsi         | 12 +++---------
 2 files changed, 17 insertions(+), 10 deletions(-)

-- 
2.34.1

[PATCH 1/3] media: dt-bindings: nxp,imx8-isi: Refuse port@1 for single pipeline models

[Alexander Stein]

In case the hardware only supports just one pipeline, explicitly refuse
port@1 in ports node.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml
index e4665469a86c3..4d5348d456a1f 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml
@@ -84,6 +84,7 @@ allOf:
           properties:
             port@0:
               description: MIPI CSI-2 RX
+            port@1: false
           required:
             - port@0
 
-- 
2.34.1

Re: [PATCH 1/3] media: dt-bindings: nxp,imx8-isi: Refuse port@1 for single pipeline models

[Krzysztof Kozlowski]

On 23/02/2024 15:04, Alexander Stein wrote:
> In case the hardware only supports just one pipeline, explicitly refuse
> port@1 in ports node.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
>  Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH 2/3] media: dt-bindings: nxp,imx8-isi: Allow single port for single pipeline models

[Alexander Stein]

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(-)

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
 
-- 
2.34.1

Re: [PATCH 2/3] media: dt-bindings: nxp,imx8-isi: Allow single port for single pipeline models

[Laurent Pinchart]

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
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] media: dt-bindings: nxp,imx8-isi: Allow single port for single pipeline models

[Laurent Pinchart]

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
> >  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] media: dt-bindings: nxp,imx8-isi: Allow single port for single pipeline models

[Krzysztof Kozlowski]

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

Re: [PATCH 2/3] media: dt-bindings: nxp,imx8-isi: Allow single port for single pipeline models

[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".

Best regards,
Krzysztof

Re: [PATCH 2/3] media: dt-bindings: nxp, imx8-isi: Allow single port for single pipeline models

[Alexander Stein]

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
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 2/3] media: dt-bindings: nxp, imx8-isi: Allow single port for single pipeline models

[Krzysztof Kozlowski]

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

Re: [PATCH 2/3] media: dt-bindings: nxp,imx8-isi: Allow single port for single pipeline models

[Rob Herring]

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 

[PATCH 3/3] arm64: dts: imx8mn: Use single port for ISI

[Alexander Stein]

The ISI on i.MX8M Nano only has one port/channel, so use the single port
node. Fixes the DTBS_CHECK warning:
/soc@0/bus@32c00000/isi@32e20000/ports: graph node has single child node
'port@0', #address-cells/#size-cells are not necessary

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 arch/arm64/boot/dts/freescale/imx8mn.dtsi | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index 136e75c51251a..de7704aafbc1a 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -1118,15 +1118,9 @@ isi: isi@32e20000 {
 				power-domains = <&disp_blk_ctrl IMX8MN_DISPBLK_PD_ISI>;
 				status = "disabled";
 
-				ports {
-					#address-cells = <1>;
-					#size-cells = <0>;
-
-					port@0 {
-						reg = <0>;
-						isi_in: endpoint {
-							remote-endpoint = <&mipi_csi_out>;
-						};
+				port {
+					isi_in: endpoint {
+						remote-endpoint = <&mipi_csi_out>;
 					};
 				};
 			};
-- 
2.34.1

Re: [PATCH 3/3] arm64: dts: imx8mn: Use single port for ISI

[Krzysztof Kozlowski]

On 23/02/2024 16:29, Adam Ford wrote:
> On Fri, Feb 23, 2024 at 8:09 AM Alexander Stein <
> alexander.stein@ew.tq-group.com> wrote:
> 
>> The ISI on i.MX8M Nano only has one port/channel, so use the single port
>> node. Fixes the DTBS_CHECK warning:
>> /soc@0/bus@32c00000/isi@32e20000/ports: graph node has single child node
>> 'port@0', #address-cells/#size-cells are not necessary
>>
>>
> Should we add  fixes tags to both the bindings and the dts?
> 
> adam

No, because original code is correct and we are just choosing here one
style over another. "style" fixes should not be backported.

Best regards,
Krzysztof