| Message ID | 20240115132009.101718-2-raphael.gallais-pou@foss.st.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Introduce STM32 LVDS driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
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.
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.
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.
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
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 --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 = <<dc_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
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