Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
On 28/11/2022 18:42, Uwe Kleine-König wrote: > So I'd go for putting into the legacy binding what is currently done in > arch/arm/boot/dts and the driver allowing exactly: > > compatible = "fsl,imx27-fb", "fsl,imx21-fb"; > compatible = "fsl,imx25-fb", "fsl,imx21-fb"; > compatible = "fsl,imx21-fb"; > compatible = "fsl,imx1-fb"; > > I thinks this is accomplished using: > > compatible: > oneOf: > - enum: > - fsl,imx1-fb > - fsl,imx21-fb > - items > - enum: > - fsl,imx25-fb > - fsl,imx27-fb > - const: fsl,imx21-fb > > right? Yes, looks correct. Best regards, Krzysztof
Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
On Thu, Nov 17, 2022 at 06:49:02PM +0100, Krzysztof Kozlowski wrote: > On 16/11/2022 18:49, Philipp Zabel wrote: > > On Thu, Nov 10, 2022 at 10:49:45AM +0100, Uwe Kleine-König wrote: > > [...] > >> new file mode 100644 > >> index ..c3cf6f92a766 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml > >> @@ -0,0 +1,110 @@ > >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >> +%YAML 1.2 > >> +--- > >> +$id: http://devicetree.org/schemas/display/imx/fsl,imx-lcdc.yaml# > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >> + > >> +title: Freescale i.MX LCD Controller, found on i.MX1, i.MX21, i.MX25 and > >> i.MX27 > >> + > >> +maintainers: > >> + - Sascha Hauer > >> + - Pengutronix Kernel Team > >> + > >> +properties: > >> + compatible: > >> +oneOf: > >> + - items: > >> + - enum: > >> + - fsl,imx1-fb > >> + - fsl,imx21-fb > > > > Are the items/enum keywords superfluous here? Couldn't this just be two > > > > - const: fsl,imx1-fb > > - const: fsl,imx21-fb > > > > entries? > > Only "items" is, so should be dropped. Status quo are the following settings: imx25.dtsi uses: compatible = "fsl,imx25-fb", "fsl,imx21-fb"; imx27.dtsi uses: compatible = "fsl,imx27-fb", "fsl,imx21-fb"; The fb driver (drivers/video/fbdev/imxfb.c) supports devices with "fsl,imx1-fb" and "fsl,imx21-fb" in their comaptible list. So my best guess is to assume an i.MX21 would use compatible = "fsl,imx21-fb"; and an i.MX1 would use: compatible = "fsl,imx1-fb"; . Looking at the driver it might be that it works in i.MX1 mode on an i.MX2x. The latter has some additional registers, higher y-resolution and supports 16, 18 and 24 bpp. However my actual plan was to support the drm driver with the saner binding on i.MX25 and not cleanup the driver and binding I want to deprecate. So I'd go for putting into the legacy binding what is currently done in arch/arm/boot/dts and the driver allowing exactly: compatible = "fsl,imx27-fb", "fsl,imx21-fb"; compatible = "fsl,imx25-fb", "fsl,imx21-fb"; compatible = "fsl,imx21-fb"; compatible = "fsl,imx1-fb"; I thinks this is accomplished using: compatible: oneOf: - enum: - fsl,imx1-fb - fsl,imx21-fb - items - enum: - fsl,imx25-fb - fsl,imx27-fb - const: fsl,imx21-fb right? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
Hello, On Wed, Nov 16, 2022 at 11:21:31AM -0600, Rob Herring wrote: > On Thu, Nov 10, 2022 at 10:49:45AM +0100, Uwe Kleine-König wrote: > > This is a straight forward conversion. Note that fsl,imx-lcdc was picked > > as the new name as this is the compatible that should supersede the > > legacy fb binding. > > > > Signed-off-by: Uwe Kleine-König > > --- > > Hello, > > > > the eventual goal is to add drm support for this hardware. That one will > > use a different (and more sensible) binding. However fsl,imx*-fb won't > > go away directly, and Rob requested to describe both bindings in the > > same file given that it describes a single hardware type. > > > > As a first step I convert the old binding to yaml. I tried to put the > > new binding on top of that but I'm not sure about a few things in this > > patch and so post only this first patch and once it's accepted add the > > new binding which I guess is less overall work. > > > > What I'm unsure about is the description of the display node (Is there a > > better description? I didn't find a schema for that.) > > That's going to be a challenge to describe because every panel binding > will need a reference to those custom properties. It's a similar problem > that spi-peripheral-props.yaml solved. But here, there may not be enough > instances to do such a general solution. Do the panels used even have > schemas yet? Looking at the dts files in the tree[1] I only found sharp,lq035q7db03 in simple-panel which might match the display used in arch/arm/boot/dts/imx27-phytec-phycore-rdk.dts. > It's kind of a separate problem. You could start with just creating a > schema just listing the custom properties. Understood that. Will try it. > > Further I didn't find documentation about additionalProperties and > > unevaluatedProperties. Did I pick the right one here? > > example-schema.yaml talks about it some. In general, if there's a > $ref to other properties for a node not defined locally, then you need > unevaluatedProperties. Otherwise, additionalProperties is fine. Not sure I got the complete picture. I'll stick to additionalProperties and rely on people and tools to tell me if I'm wrong :-) Best regards and thanks for the feedback, Uwe [1] in arch/arm/boot/dts/imx25-pdk.dts _qvga in arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-cmo-qvga.dts _svga in arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-dvi-svga.dts _vga in arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-dvi-vga.dts in arch/arm/boot/dts/imx27-eukrea-mbimxsd27-baseboard.dts in arch/arm/boot/dts/imx27-apf27dev.dts in arch/arm/boot/dts/imx27-eukrea-mbimxsd27-baseboard.dts in arch/arm/boot/dts/imx27-phytec-phycard-s-rdk.dts in arch/arm/boot/dts/imx27-phytec-phycore-rdk.dts -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
On 16/11/2022 18:49, Philipp Zabel wrote: > On Thu, Nov 10, 2022 at 10:49:45AM +0100, Uwe Kleine-König wrote: > [...] >> new file mode 100644 >> index ..c3cf6f92a766 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml >> @@ -0,0 +1,110 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/display/imx/fsl,imx-lcdc.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Freescale i.MX LCD Controller, found on i.MX1, i.MX21, i.MX25 and >> i.MX27 >> + >> +maintainers: >> + - Sascha Hauer >> + - Pengutronix Kernel Team >> + >> +properties: >> + compatible: >> +oneOf: >> + - items: >> + - enum: >> + - fsl,imx1-fb >> + - fsl,imx21-fb > > Are the items/enum keywords superfluous here? Couldn't this just be two > > - const: fsl,imx1-fb > - const: fsl,imx21-fb > > entries? Only "items" is, so should be dropped. Best regards, Krzysztof
Re: [PATCH v1] dt-bindings: display: Convert fsl, imx-fb.txt to dt-schema
On Wed, Nov 16, 2022 at 11:49 AM Philipp Zabel wrote: > > On Thu, Nov 10, 2022 at 10:49:45AM +0100, Uwe Kleine-König wrote: > [...] > > new file mode 100644 > > index ..c3cf6f92a766 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml > > @@ -0,0 +1,110 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/display/imx/fsl,imx-lcdc.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Freescale i.MX LCD Controller, found on i.MX1, i.MX21, i.MX25 and > > i.MX27 > > + > > +maintainers: > > + - Sascha Hauer > > + - Pengutronix Kernel Team > > + > > +properties: > > + compatible: > > +oneOf: > > + - items: > > + - enum: > > + - fsl,imx1-fb > > + - fsl,imx21-fb > > Are the items/enum keywords superfluous here? Couldn't this just be two > > - const: fsl,imx1-fb > - const: fsl,imx21-fb > > entries? mx1 is backwards compatible with mx21? No. Rob
Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
On Thu, Nov 10, 2022 at 10:49:45AM +0100, Uwe Kleine-König wrote: [...] > new file mode 100644 > index ..c3cf6f92a766 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml > @@ -0,0 +1,110 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/imx/fsl,imx-lcdc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale i.MX LCD Controller, found on i.MX1, i.MX21, i.MX25 and > i.MX27 > + > +maintainers: > + - Sascha Hauer > + - Pengutronix Kernel Team > + > +properties: > + compatible: > +oneOf: > + - items: > + - enum: > + - fsl,imx1-fb > + - fsl,imx21-fb Are the items/enum keywords superfluous here? Couldn't this just be two - const: fsl,imx1-fb - const: fsl,imx21-fb entries? > + - items: > + - enum: > + - fsl,imx25-fb > + - fsl,imx27-fb > + - const: fsl,imx21-fb > + > + clocks: > +maxItems: 3 > + > + clock-names: > +items: > + - const: ipg > + - const: ahb > + - const: per clocks and clock-names are new, so this is a little bit more than a straight forward conversion. I'd mention this in the commit description. regards Philipp
Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
On Thu, Nov 10, 2022 at 10:49:45AM +0100, Uwe Kleine-König wrote:
> This is a straight forward conversion. Note that fsl,imx-lcdc was picked
> as the new name as this is the compatible that should supersede the
> legacy fb binding.
>
> Signed-off-by: Uwe Kleine-König
> ---
> Hello,
>
> the eventual goal is to add drm support for this hardware. That one will
> use a different (and more sensible) binding. However fsl,imx*-fb won't
> go away directly, and Rob requested to describe both bindings in the
> same file given that it describes a single hardware type.
>
> As a first step I convert the old binding to yaml. I tried to put the
> new binding on top of that but I'm not sure about a few things in this
> patch and so post only this first patch and once it's accepted add the
> new binding which I guess is less overall work.
>
> What I'm unsure about is the description of the display node (Is there a
> better description? I didn't find a schema for that.)
That's going to be a challenge to describe because every panel binding
will need a reference to those custom properties. It's a similar problem
that spi-peripheral-props.yaml solved. But here, there may not be enough
instances to do such a general solution. Do the panels used even have
schemas yet?
It's kind of a separate problem. You could start with just creating a
schema just listing the custom properties.
> Further I didn't find documentation about additionalProperties and
> unevaluatedProperties. Did I pick the right one here?
example-schema.yaml talks about it some. In general, if there's a
$ref to other properties for a node not defined locally, then you need
unevaluatedProperties. Otherwise, additionalProperties is fine.
> Best regards
> Uwe
>
> .../bindings/display/imx/fsl,imx-fb.txt | 57 -
> .../bindings/display/imx/fsl,imx-lcdc.yaml| 110 ++
> 2 files changed, 110 insertions(+), 57 deletions(-)
> delete mode 100644
> Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt
> create mode 100644
> Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
[...]
> diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
> b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
> new file mode 100644
> index ..c3cf6f92a766
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/imx/fsl,imx-lcdc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX LCD Controller, found on i.MX1, i.MX21, i.MX25 and
> i.MX27
> +
> +maintainers:
> + - Sascha Hauer
> + - Pengutronix Kernel Team
> +
> +properties:
> + compatible:
> +oneOf:
> + - items:
> + - enum:
> + - fsl,imx1-fb
> + - fsl,imx21-fb
> + - items:
> + - enum:
> + - fsl,imx25-fb
> + - fsl,imx27-fb
> + - const: fsl,imx21-fb
> +
> + clocks:
> +maxItems: 3
> +
> + clock-names:
> +items:
> + - const: ipg
> + - const: ahb
> + - const: per
> +
> + display:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description: |
> + Display hardware node
> + It needs to define the following properties:
> +- bits-per-pixel
> +- fsl,pcr: LCDC PCR value
> + And optionally:
> +- fsl,aus-mode: boolean to enable AUS mode (only for imx21)
> +
> + interrupts:
> +maxItems: 1
> +
> + reg:
> +maxItems: 1
> +
> + lcd-supply:
> +description:
> + Regulator for LCD supply voltage.
> +
> + fsl,dmacr:
> +$ref: '/schemas/types.yaml#/definitions/uint32'
Drop quotes.
> +description:
> + Override value for DMA Control Register
> +
> + fsl,lpccr:
> +$ref: '/schemas/types.yaml#/definitions/uint32'
Drop quotes.
> +description:
> + Contrast Control Register value.
> +
> + fsl,lscr1:
> +$ref: '/schemas/types.yaml#/definitions/uint32'
Drop quotes.
> +description:
> + LCDC Sharp Configuration Register value.
> +
> +required:
> + - compatible
> + - clocks
> + - clock-names
> + - display
> + - interrupts
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +imxfb: fb@10021000 {
lcd-controller@...
> +compatible = "fsl,imx21-fb";
> +interrupts = <61>;
> +reg = <0x10021000 0x1000>;
> +display = <>;
> +clocks = < 103>, < 49>, < 66>;
> +clock-names = "ipg", "ahb", "per";
> +};
> +
> +display0: display0 {
> +model = "Primeview-PD050VL1";
> +bits-per-pixel = <16>;
> +fsl,pcr = <0xf0c88080>; /* non-standard but required */
> +
> +display-timings {
> +native-mode = <_disp0>;
> +timing_disp0: timing0 {
> +hactive = <640>;
> +
[PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
This is a straight forward conversion. Note that fsl,imx-lcdc was picked
as the new name as this is the compatible that should supersede the
legacy fb binding.
Signed-off-by: Uwe Kleine-König
---
Hello,
the eventual goal is to add drm support for this hardware. That one will
use a different (and more sensible) binding. However fsl,imx*-fb won't
go away directly, and Rob requested to describe both bindings in the
same file given that it describes a single hardware type.
As a first step I convert the old binding to yaml. I tried to put the
new binding on top of that but I'm not sure about a few things in this
patch and so post only this first patch and once it's accepted add the
new binding which I guess is less overall work.
What I'm unsure about is the description of the display node (Is there a
better description? I didn't find a schema for that.)
Further I didn't find documentation about additionalProperties and
unevaluatedProperties. Did I pick the right one here?
Best regards
Uwe
.../bindings/display/imx/fsl,imx-fb.txt | 57 -
.../bindings/display/imx/fsl,imx-lcdc.yaml| 110 ++
2 files changed, 110 insertions(+), 57 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt
create mode 100644
Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt
b/Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt
deleted file mode 100644
index f4df9e83bcd2..
--- a/Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt
+++ /dev/null
@@ -1,57 +0,0 @@
-Freescale imx21 Framebuffer
-
-This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
-
-Required properties:
-- compatible : "fsl,-fb", chip should be imx1 or imx21
-- reg : Should contain 1 register ranges(address and length)
-- interrupts : One interrupt of the fb dev
-
-Required nodes:
-- display: Phandle to a display node as described in
- Documentation/devicetree/bindings/display/panel/display-timing.txt
- Additional, the display node has to define properties:
- - bits-per-pixel: Bits per pixel
- - fsl,pcr: LCDC PCR value
- A display node may optionally define
- - fsl,aus-mode: boolean to enable AUS mode (only for imx21)
-
-Optional properties:
-- lcd-supply: Regulator for LCD supply voltage.
-- fsl,dmacr: DMA Control Register value. This is optional. By default, the
- register is not modified as recommended by the datasheet.
-- fsl,lpccr: Contrast Control Register value. This property provides the
- default value for the contrast control register.
- If that property is omitted, the register is zeroed.
-- fsl,lscr1: LCDC Sharp Configuration Register value.
-
-Example:
-
- imxfb: fb@10021000 {
- compatible = "fsl,imx21-fb";
- interrupts = <61>;
- reg = <0x10021000 0x1000>;
- display = <>;
- };
-
- ...
-
- display0: display0 {
- model = "Primeview-PD050VL1";
- bits-per-pixel = <16>;
- fsl,pcr = <0xf0c88080>; /* non-standard but required */
- display-timings {
- native-mode = <_disp0>;
- timing_disp0: 640x480 {
- hactive = <640>;
- vactive = <480>;
- hback-porch = <112>;
- hfront-porch = <36>;
- hsync-len = <32>;
- vback-porch = <33>;
- vfront-porch = <33>;
- vsync-len = <2>;
- clock-frequency = <2500>;
- };
- };
- };
diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
new file mode 100644
index ..c3cf6f92a766
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/imx/fsl,imx-lcdc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale i.MX LCD Controller, found on i.MX1, i.MX21, i.MX25 and i.MX27
+
+maintainers:
+ - Sascha Hauer
+ - Pengutronix Kernel Team
+
+properties:
+ compatible:
+oneOf:
+ - items:
+ - enum:
+ - fsl,imx1-fb
+ - fsl,imx21-fb
+ - items:
+ - enum:
+ - fsl,imx25-fb
+ - fsl,imx27-fb
+ - const: fsl,imx21-fb
+
+ clocks:
+maxItems: 3
+
+ clock-names:
+items:
+ - const: ipg
+ - const: ahb
+ - const: per
+
+ display:
+$ref: /schemas/types.yaml#/definitions/phandle
+description: |
+
