diff mbox series

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

Message ID 20240223140445.1885083-3-alexander.stein@ew.tq-group.com
State New
Headers show
Series i.MX8M Nano ISI single port support | expand

Commit Message

Alexander Stein Feb. 23, 2024, 2:04 p.m. UTC
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(-)

Comments

Krzysztof Kozlowski Feb. 29, 2024, 6:14 p.m. UTC | #1
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
Krzysztof Kozlowski Feb. 29, 2024, 6:18 p.m. UTC | #2
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
Alexander Stein March 1, 2024, 8:32 a.m. UTC | #3
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
Krzysztof Kozlowski March 1, 2024, 8:49 a.m. UTC | #4
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
Rob Herring (Arm) March 1, 2024, 9:11 p.m. UTC | #5
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 mbox series

Patch

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