[PATCH 1/2] dt-bindings: display: bridge: ldb: Implement simple NXP i.MX8M LDB bridge

[PATCH 1/2] dt-bindings: display: bridge: ldb: Implement simple NXP i.MX8M LDB bridge

[Marek Vasut]

The i.MX8MP contains two syscon registers which are responsible
for configuring the on-SoC DPI-to-LVDS serializer. Add DT binding
which represents this serializer as a bridge.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Maxime Ripard <maxime@cerno.tech>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Robby Cai <robby.cai@nxp.com>
Cc: Robert Foss <robert.foss@linaro.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
To: dri-devel@lists.freedesktop.org
---
 .../bindings/display/bridge/nxp,ldb.yaml      | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
new file mode 100644
index 0000000000000..a05dd05547836
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/nxp,ldb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8M DPI to LVDS bridge chip
+
+maintainers:
+  - Marek Vasut <marex@denx.de>
+
+description: |
+  The i.MX8MP contains two syscon registers which are responsible
+  for configuring the on-SoC DPI-to-LVDS serializer. This describes
+  those registers as bridge within the DT.
+
+properties:
+  compatible:
+    items:
+      - const: nxp,imx8mp-ldb
+
+  clocks:
+    minItems: 1
+
+  clock-names:
+    const: "ldb"
+
+  syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: A phandle to media block controller.
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Video port for DPI input.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Video port for LVDS Channel-A output (panel or bridge).
+
+      port@2:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Video port for LVDS Channel-B output (panel or bridge).
+
+    required:
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - clocks
+  - syscon
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8mp-clock.h>
+
+    lvds-ldb {
+        #address-cells = <0>;
+        #size-cells = <0>;
+        compatible = "fsl,imx8mp-ldb";
+        clocks = <&clk IMX8MP_CLK_MEDIA_LDB>;
+        clock-names = "ldb";
+        syscon = <&media_blk_ctrl>;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+
+                ldb_from_lcdif2: endpoint {
+                    remote-endpoint = <&lcdif2_to_ldb>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+
+                ldb_lvds_ch0: endpoint {
+                    remote-endpoint = <&ldb_to_lvdsx4panel>;
+                };
+            };
+
+            port@2 {
+                reg = <2>;
+
+                ldb_lvds_ch1: endpoint {
+                };
+            };
+        };
+    };
-- 
2.35.1

Re: [PATCH 1/2] dt-bindings: display: bridge: ldb: Implement simple NXP i.MX8M LDB bridge

[Krzysztof Kozlowski]

On 13/03/2022 13:38, Marek Vasut wrote:
> The i.MX8MP contains two syscon registers which are responsible
> for configuring the on-SoC DPI-to-LVDS serializer. Add DT binding
> which represents this serializer as a bridge.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Maxime Ripard <maxime@cerno.tech>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Robby Cai <robby.cai@nxp.com>
> Cc: Robert Foss <robert.foss@linaro.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
>  .../bindings/display/bridge/nxp,ldb.yaml      | 99 +++++++++++++++++++
>  1 file changed, 99 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
> new file mode 100644
> index 0000000000000..a05dd05547836
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/nxp,ldb.yaml#

In title, description and commit msg you point this is specific to
i.MX8M, so maybe reflect it in the file name as well.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX8M DPI to LVDS bridge chip
> +
> +maintainers:
> +  - Marek Vasut <marex@denx.de>
> +
> +description: |
> +  The i.MX8MP contains two syscon registers which are responsible
> +  for configuring the on-SoC DPI-to-LVDS serializer. This describes
> +  those registers as bridge within the DT.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: nxp,imx8mp-ldb

No items.

> +
> +  clocks:
> +    minItems: 1

maxItems

> +
> +  clock-names:
> +    const: "ldb"

No quotes

> +
> +  syscon:

You need a specific name with vendor prefix.

> +    $ref: /schemas/types.yaml#/definitions/phandle> +    description: A phandle to media block controller.
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Video port for DPI input.
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Video port for LVDS Channel-A output (panel or bridge).
> +
> +      port@2:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Video port for LVDS Channel-B output (panel or bridge).
> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - clocks
> +  - syscon
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8mp-clock.h>
> +
> +    lvds-ldb {

Generic node name, so "bridge" or "display-bridge"

> +        #address-cells = <0>;
> +        #size-cells = <0>;

Why do you need address and size cells? This will complain if you test
your bindings with proper compatible.

> +        compatible = "fsl,imx8mp-ldb";

This does not look the same as documented here.


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: display: bridge: ldb: Implement simple NXP i.MX8M LDB bridge

[Marek Vasut]

On 3/13/22 16:47, Krzysztof Kozlowski wrote:

Hi,

[...]

>> diff --git a/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
>> new file mode 100644
>> index 0000000000000..a05dd05547836
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
>> @@ -0,0 +1,99 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/bridge/nxp,ldb.yaml#
> 
> In title, description and commit msg you point this is specific to
> i.MX8M, so maybe reflect it in the file name as well.

That's how it is so far, however NXP does recycle their IPs quite a bit 
so I don't want to encode the SoC type into the bindings file name. I do 
expect them to re-use this bridge somewhere else sooner rather than later.

[...]

The rest is fixed.

[...]

>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/imx8mp-clock.h>
>> +
>> +    lvds-ldb {
> 
> Generic node name, so "bridge" or "display-bridge"
> 
>> +        #address-cells = <0>;
>> +        #size-cells = <0>;
> 
> Why do you need address and size cells? This will complain if you test
> your bindings with proper compatible.

Because the subnodes of this bridge have no dimension, so address/size 
cells = 0.

I don't understand the second part about "proper compatible", can you 
elaborate ?

>> +        compatible = "fsl,imx8mp-ldb";
> 
> This does not look the same as documented here.

This is the right compatible string, fixed.

Re: [PATCH 1/2] dt-bindings: display: bridge: ldb: Implement simple NXP i.MX8M LDB bridge

[Krzysztof Kozlowski]

On 13/03/2022 18:09, Marek Vasut wrote:
> On 3/13/22 16:47, Krzysztof Kozlowski wrote:
> 
> Hi,
> 
> [...]
> 
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
>>> new file mode 100644
>>> index 0000000000000..a05dd05547836
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
>>> @@ -0,0 +1,99 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/display/bridge/nxp,ldb.yaml#
>>
>> In title, description and commit msg you point this is specific to
>> i.MX8M, so maybe reflect it in the file name as well.
> 
> That's how it is so far, however NXP does recycle their IPs quite a bit 
> so I don't want to encode the SoC type into the bindings file name. I do 
> expect them to re-use this bridge somewhere else sooner rather than later.
> 
> [...]
> 
> The rest is fixed.
> 
> [...]
> 
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/clock/imx8mp-clock.h>
>>> +
>>> +    lvds-ldb {
>>
>> Generic node name, so "bridge" or "display-bridge"
>>
>>> +        #address-cells = <0>;
>>> +        #size-cells = <0>;
>>
>> Why do you need address and size cells? This will complain if you test
>> your bindings with proper compatible.
> 
> Because the subnodes of this bridge have no dimension, so address/size 
> cells = 0.
> 
> I don't understand the second part about "proper compatible", can you 
> elaborate ?

You have wrong compatible in example and in bindings, so you do not see
the error of address/size cells.

They are also not required in your example, since you do not have unit
address. Otherwise your lvds-ldb node would be wrong (see its
address/size cells).

If you supply address-cells you should see a W=1 warning:
#address-cells/#size-cells without "ranges" or child "reg" property

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: display: bridge: ldb: Implement simple NXP i.MX8M LDB bridge

[Marek Vasut]

On 3/13/22 18:50, Krzysztof Kozlowski wrote:
> On 13/03/2022 18:09, Marek Vasut wrote:
>> On 3/13/22 16:47, Krzysztof Kozlowski wrote:
>>
>> Hi,
>>
>> [...]
>>
>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
>>>> new file mode 100644
>>>> index 0000000000000..a05dd05547836
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
>>>> @@ -0,0 +1,99 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/display/bridge/nxp,ldb.yaml#
>>>
>>> In title, description and commit msg you point this is specific to
>>> i.MX8M, so maybe reflect it in the file name as well.
>>
>> That's how it is so far, however NXP does recycle their IPs quite a bit
>> so I don't want to encode the SoC type into the bindings file name. I do
>> expect them to re-use this bridge somewhere else sooner rather than later.
>>
>> [...]
>>
>> The rest is fixed.
>>
>> [...]
>>
>>>> +examples:
>>>> +  - |
>>>> +    #include <dt-bindings/clock/imx8mp-clock.h>
>>>> +
>>>> +    lvds-ldb {
>>>
>>> Generic node name, so "bridge" or "display-bridge"
>>>
>>>> +        #address-cells = <0>;
>>>> +        #size-cells = <0>;
>>>
>>> Why do you need address and size cells? This will complain if you test
>>> your bindings with proper compatible.
>>
>> Because the subnodes of this bridge have no dimension, so address/size
>> cells = 0.
>>
>> I don't understand the second part about "proper compatible", can you
>> elaborate ?
> 
> You have wrong compatible in example and in bindings, so you do not see
> the error of address/size cells.
> 
> They are also not required in your example, since you do not have unit
> address. Otherwise your lvds-ldb node would be wrong (see its
> address/size cells).
> 
> If you supply address-cells you should see a W=1 warning:
> #address-cells/#size-cells without "ranges" or child "reg" property

Ah, right, both are fixed now. I'll wait a bit for more feedback esp. on 
the bridge driver and send a V2.

Re: [PATCH 1/2] dt-bindings: display: bridge: ldb: Implement simple NXP i.MX8M LDB bridge

[Rob Herring]

On Sun, 13 Mar 2022 13:38:51 +0100, Marek Vasut wrote:
> The i.MX8MP contains two syscon registers which are responsible
> for configuring the on-SoC DPI-to-LVDS serializer. Add DT binding
> which represents this serializer as a bridge.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Maxime Ripard <maxime@cerno.tech>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Robby Cai <robby.cai@nxp.com>
> Cc: Robert Foss <robert.foss@linaro.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
>  .../bindings/display/bridge/nxp,ldb.yaml      | 99 +++++++++++++++++++
>  1 file changed, 99 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/display/bridge/nxp,ldb.example.dt.yaml:0:0: /example-0/lvds-ldb: failed to match any schema with compatible: ['fsl,imx8mp-ldb']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1604767

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.