diff mbox series

[v3,1/6] dt-bindings: display: add dt-bindings for STM32 LVDS device

Message ID 20240115132009.101718-2-raphael.gallais-pou@foss.st.com (mailing list archive)
State New, archived
Headers show
Series Introduce STM32 LVDS driver | expand

Commit Message

Raphael Gallais-Pou Jan. 15, 2024, 1:20 p.m. UTC
Add "st,stm32mp25-lvds" compatible.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
Depends on: "dt-bindings: stm32: add clocks and reset binding for
	    stm32mp25 platform" by Gabriel Fernandez

Changes in v3:
	- Clarify commit dependency
	- Fix includes in the example
	- Fix YAML
	- Add "clock-cells" description
	- s/regroups/is composed of/
	- Changed compatible to show SoC specificity

Changes in v2:
	- Switch compatible and clock-cells related areas
	- Remove faulty #include in the example.
	- Add entry in MAINTAINERS
---
 .../bindings/display/st,stm32-lvds.yaml       | 119 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 120 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/st,stm32-lvds.yaml

Comments

Rob Herring (Arm) Jan. 15, 2024, 2:32 p.m. UTC | #1
On Mon, 15 Jan 2024 14:20:04 +0100, Raphael Gallais-Pou wrote:
> Add "st,stm32mp25-lvds" compatible.
> 
> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> ---
> Depends on: "dt-bindings: stm32: add clocks and reset binding for
> 	    stm32mp25 platform" by Gabriel Fernandez
> 
> Changes in v3:
> 	- Clarify commit dependency
> 	- Fix includes in the example
> 	- Fix YAML
> 	- Add "clock-cells" description
> 	- s/regroups/is composed of/
> 	- Changed compatible to show SoC specificity
> 
> Changes in v2:
> 	- Switch compatible and clock-cells related areas
> 	- Remove faulty #include in the example.
> 	- Add entry in MAINTAINERS
> ---
>  .../bindings/display/st,stm32-lvds.yaml       | 119 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  2 files changed, 120 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/st,stm32-lvds.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/st,stm32-lvds.example.dts:18:18: fatal error: dt-bindings/clock/st,stm32mp25-rcc.h: No such file or directory
   18 |         #include <dt-bindings/clock/st,stm32mp25-rcc.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/display/st,stm32-lvds.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2
make: *** [Makefile:234: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240115132009.101718-2-raphael.gallais-pou@foss.st.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Rob Herring (Arm) Jan. 15, 2024, 3:46 p.m. UTC | #2
On Mon, Jan 15, 2024 at 02:20:04PM +0100, Raphael Gallais-Pou wrote:
> Add "st,stm32mp25-lvds" compatible.
> 
> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> ---
> Depends on: "dt-bindings: stm32: add clocks and reset binding for
> 	    stm32mp25 platform" by Gabriel Fernandez
> 
> Changes in v3:
> 	- Clarify commit dependency
> 	- Fix includes in the example
> 	- Fix YAML
> 	- Add "clock-cells" description
> 	- s/regroups/is composed of/
> 	- Changed compatible to show SoC specificity
> 
> Changes in v2:
> 	- Switch compatible and clock-cells related areas
> 	- Remove faulty #include in the example.
> 	- Add entry in MAINTAINERS
> ---
>  .../bindings/display/st,stm32-lvds.yaml       | 119 ++++++++++++++++++

Filename matching compatible.

> +properties:
> +  compatible:
> +    const: st,stm32mp25-lvds


> +examples:
> +  - |
> +    #include <dt-bindings/clock/st,stm32mp25-rcc.h>
> +    #include <dt-bindings/reset/st,stm32mp25-rcc.h>
> +
> +    lvds: lvds@48060000 {
> +        compatible = "st,stm32-lvds";

Wrong compatible.
Raphael Gallais-Pou Jan. 15, 2024, 4:51 p.m. UTC | #3
On 1/15/24 16:46, Rob Herring wrote:
> On Mon, Jan 15, 2024 at 02:20:04PM +0100, Raphael Gallais-Pou wrote:
>> Add "st,stm32mp25-lvds" compatible.
>>
>> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
>> ---
>> Depends on: "dt-bindings: stm32: add clocks and reset binding for
>> 	    stm32mp25 platform" by Gabriel Fernandez
>>
>> Changes in v3:
>> 	- Clarify commit dependency
>> 	- Fix includes in the example
>> 	- Fix YAML
>> 	- Add "clock-cells" description
>> 	- s/regroups/is composed of/
>> 	- Changed compatible to show SoC specificity
>>
>> Changes in v2:
>> 	- Switch compatible and clock-cells related areas
>> 	- Remove faulty #include in the example.
>> 	- Add entry in MAINTAINERS
>> ---
>>  .../bindings/display/st,stm32-lvds.yaml       | 119 ++++++++++++++++++
> Filename matching compatible.

Hi Rob,


I was unsure about this.

The driver will eventually support several SoCs with different compatibles,
wouldn't this be more confusing ?

I also wanted to keep the similarity with the "st,stm32-<ip>.yaml" name for the
DRM STM drivers. Would that be possible ?


Regards,

Raphaël



>
>> +properties:
>> +  compatible:
>> +    const: st,stm32mp25-lvds
>
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/st,stm32mp25-rcc.h>
>> +    #include <dt-bindings/reset/st,stm32mp25-rcc.h>
>> +
>> +    lvds: lvds@48060000 {
>> +        compatible = "st,stm32-lvds";
> Wrong compatible.
Krzysztof Kozlowski Jan. 16, 2024, 7:42 a.m. UTC | #4
On 15/01/2024 17:51, Raphael Gallais-Pou wrote:
> 
> On 1/15/24 16:46, Rob Herring wrote:
>> On Mon, Jan 15, 2024 at 02:20:04PM +0100, Raphael Gallais-Pou wrote:
>>> Add "st,stm32mp25-lvds" compatible.
>>>

A nit, subject: drop second/last, redundant "dt-bindings for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

>>> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
>>> ---
>>> Depends on: "dt-bindings: stm32: add clocks and reset binding for
>>> 	    stm32mp25 platform" by Gabriel Fernandez
>>>
>>> Changes in v3:
>>> 	- Clarify commit dependency
>>> 	- Fix includes in the example
>>> 	- Fix YAML
>>> 	- Add "clock-cells" description
>>> 	- s/regroups/is composed of/
>>> 	- Changed compatible to show SoC specificity
>>>
>>> Changes in v2:
>>> 	- Switch compatible and clock-cells related areas
>>> 	- Remove faulty #include in the example.
>>> 	- Add entry in MAINTAINERS
>>> ---
>>>  .../bindings/display/st,stm32-lvds.yaml       | 119 ++++++++++++++++++
>> Filename matching compatible.
> 
> Hi Rob,
> 
> 
> I was unsure about this.
> 
> The driver will eventually support several SoCs with different compatibles,
> wouldn't this be more confusing ?

No. "Eventually" might never happen.

> I also wanted to keep the similarity with the "st,stm32-<ip>.yaml" name for the
> DRM STM drivers. Would that be possible ?

But why? The consistency we want is the filename matching compatible,
not matching other filenames. If you have here multiple devices,
document them *now*.

> 
> 
> Regards,
> 
> Raphaël

I hope you did not ignore rest of the comments... We expect some sort of
"ack/ok/I'll fix/whatever" message and you wrote nothing further.

Best regards,
Krzysztof
Raphael Gallais-Pou Jan. 18, 2024, 3:06 p.m. UTC | #5
On 1/16/24 08:42, Krzysztof Kozlowski wrote:
> On 15/01/2024 17:51, Raphael Gallais-Pou wrote:
>> On 1/15/24 16:46, Rob Herring wrote:
>>> On Mon, Jan 15, 2024 at 02:20:04PM +0100, Raphael Gallais-Pou wrote:
>>>> Add "st,stm32mp25-lvds" compatible.
>>>>
> A nit, subject: drop second/last, redundant "dt-bindings for". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
>
>>>> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
>>>> ---
>>>> Depends on: "dt-bindings: stm32: add clocks and reset binding for
>>>> 	    stm32mp25 platform" by Gabriel Fernandez
>>>>
>>>> Changes in v3:
>>>> 	- Clarify commit dependency
>>>> 	- Fix includes in the example
>>>> 	- Fix YAML
>>>> 	- Add "clock-cells" description
>>>> 	- s/regroups/is composed of/
>>>> 	- Changed compatible to show SoC specificity
>>>>
>>>> Changes in v2:
>>>> 	- Switch compatible and clock-cells related areas
>>>> 	- Remove faulty #include in the example.
>>>> 	- Add entry in MAINTAINERS
>>>> ---
>>>>  .../bindings/display/st,stm32-lvds.yaml       | 119 ++++++++++++++++++
>>> Filename matching compatible.
>> Hi Rob,
>>
>>
>> I was unsure about this.
>>
>> The driver will eventually support several SoCs with different compatibles,
>> wouldn't this be more confusing ?
> No. "Eventually" might never happen.
>
>> I also wanted to keep the similarity with the "st,stm32-<ip>.yaml" name for the
>> DRM STM drivers. Would that be possible ?
> But why? The consistency we want is the filename matching compatible,
> not matching other filenames. If you have here multiple devices,
> document them *now*.


Hi Krzysztof,

|There is no multiple devices, so I will stick to the "st,stm32mp25-lvds"
pattern for now.|

>
>>
>> Regards,
>>
>> Raphaël
> I hope you did not ignore rest of the comments... We expect some sort of
> "ack/ok/I'll fix/whatever" message and you wrote nothing further.

Although I did not acknowledged what has been said previously, I always take
into account every comment on my patches.  I understand that it can lead to some confusion.  So rest assured that I did not
ignore Rob's and Dmitry's review.


Regards,

Raphaël

>
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/st,stm32-lvds.yaml b/Documentation/devicetree/bindings/display/st,stm32-lvds.yaml
new file mode 100644
index 000000000000..a6383858cd2c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/st,stm32-lvds.yaml
@@ -0,0 +1,119 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/st,stm32-lvds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STM32 LVDS Display Interface Transmitter
+
+maintainers:
+  - Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
+  - Yannick Fertre <yannick.fertre@foss.st.com>
+
+description: |
+  The STMicroelectronics STM32 LVDS Display Interface Transmitter handles the
+  LVDS protocol: it maps the pixels received from the upstream Pixel-DMA (LTDC)
+  onto the LVDS PHY.
+
+  It is composed of three sub blocks:
+    - LVDS host: handles the LVDS protocol (FPD / OpenLDI) and maps its input
+      pixels onto the data lanes of the PHY
+    - LVDS PHY: parallelize the data and drives the LVDS data lanes
+    - LVDS wrapper: handles top-level settings
+
+  The LVDS controller driver supports the following high-level features:
+    - FDP-Link-I and OpenLDI (v0.95) protocols
+    - Single-Link or Dual-Link operation
+    - Single-Display or Double-Display (with the same content duplicated on both)
+    - Flexible Bit-Mapping, including JEIDA and VESA
+    - RGB888 or RGB666 output
+    - Synchronous design, with one input pixel per clock cycle
+
+properties:
+  compatible:
+    const: st,stm32mp25-lvds
+
+  "#clock-cells":
+    const: 0
+    description:
+      Provides the internal LVDS PHY clock to the framework.
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: APB peripheral clock
+      - description: Reference clock for the internal PLL
+
+  clock-names:
+    items:
+      - const: pclk
+      - const: ref
+
+  resets:
+    maxItems: 1
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          LVDS input port node, connected to the LTDC RGB output port.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          LVDS output port node, connected to a panel or bridge input port.
+
+    required:
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - "#clock-cells"
+  - reg
+  - clocks
+  - clock-names
+  - resets
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/st,stm32mp25-rcc.h>
+    #include <dt-bindings/reset/st,stm32mp25-rcc.h>
+
+    lvds: lvds@48060000 {
+        compatible = "st,stm32-lvds";
+        reg = <0x48060000 0x2000>;
+        #clock-cells = <0>;
+        clocks = <&rcc CK_BUS_LVDS>, <&rcc CK_KER_LVDSPHY>;
+        clock-names = "pclk", "ref";
+        resets = <&rcc LVDS_R>;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                lvds_in: endpoint {
+                   remote-endpoint = <&ltdc_ep1_out>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                lvds_out0: endpoint {
+                   remote-endpoint = <&lvds_panel_in>;
+                };
+            };
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 9d959a6881f7..0b6ec5347195 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7193,6 +7193,7 @@  L:	dri-devel@lists.freedesktop.org
 S:	Maintained
 T:	git git://anongit.freedesktop.org/drm/drm-misc
 F:	Documentation/devicetree/bindings/display/st,stm32-ltdc.yaml
+F:	Documentation/devicetree/bindings/display/st,stm32-lvds.yaml
 F:	drivers/gpu/drm/stm
 
 DRM DRIVERS FOR TI KEYSTONE