All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
Date: Wed, 16 Nov 2022 11:21:31 -0600	[thread overview]
Message-ID: <20221116172131.GA233356-robh@kernel.org> (raw)
In-Reply-To: <20221110094945.191100-1-u.kleine-koenig@pengutronix.de>

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 <u.kleine-koenig@pengutronix.de>
> ---
> 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 000000000000..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 <s.hauer@pengutronix.de>
> +  - Pengutronix Kernel Team <kernel@pengutronix.de>
> +
> +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 = <&display0>;
> +        clocks = <&clks 103>, <&clks 49>, <&clks 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 = <&timing_disp0>;
> +            timing_disp0: timing0 {
> +                hactive = <640>;
> +                vactive = <480>;
> +                hback-porch = <112>;
> +                hfront-porch = <36>;
> +                hsync-len = <32>;
> +                vback-porch = <33>;
> +                vfront-porch = <33>;
> +                vsync-len = <2>;
> +                clock-frequency = <25000000>;
> +            };
> +        };
> +    };
> -- 
> 2.38.1
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
Date: Wed, 16 Nov 2022 11:21:31 -0600	[thread overview]
Message-ID: <20221116172131.GA233356-robh@kernel.org> (raw)
In-Reply-To: <20221110094945.191100-1-u.kleine-koenig@pengutronix.de>

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 <u.kleine-koenig@pengutronix.de>
> ---
> 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 000000000000..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 <s.hauer@pengutronix.de>
> +  - Pengutronix Kernel Team <kernel@pengutronix.de>
> +
> +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 = <&display0>;
> +        clocks = <&clks 103>, <&clks 49>, <&clks 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 = <&timing_disp0>;
> +            timing_disp0: timing0 {
> +                hactive = <640>;
> +                vactive = <480>;
> +                hback-porch = <112>;
> +                hfront-porch = <36>;
> +                hsync-len = <32>;
> +                vback-porch = <33>;
> +                vfront-porch = <33>;
> +                vsync-len = <2>;
> +                clock-frequency = <25000000>;
> +            };
> +        };
> +    };
> -- 
> 2.38.1
> 
> 

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: devicetree@vger.kernel.org,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	dri-devel@lists.freedesktop.org,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
Date: Wed, 16 Nov 2022 11:21:31 -0600	[thread overview]
Message-ID: <20221116172131.GA233356-robh@kernel.org> (raw)
In-Reply-To: <20221110094945.191100-1-u.kleine-koenig@pengutronix.de>

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 <u.kleine-koenig@pengutronix.de>
> ---
> 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 000000000000..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 <s.hauer@pengutronix.de>
> +  - Pengutronix Kernel Team <kernel@pengutronix.de>
> +
> +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 = <&display0>;
> +        clocks = <&clks 103>, <&clks 49>, <&clks 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 = <&timing_disp0>;
> +            timing_disp0: timing0 {
> +                hactive = <640>;
> +                vactive = <480>;
> +                hback-porch = <112>;
> +                hfront-porch = <36>;
> +                hsync-len = <32>;
> +                vback-porch = <33>;
> +                vfront-porch = <33>;
> +                vsync-len = <2>;
> +                clock-frequency = <25000000>;
> +            };
> +        };
> +    };
> -- 
> 2.38.1
> 
> 

  reply	other threads:[~2022-11-16 17:48 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-10  9:49 [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema Uwe Kleine-König
2022-11-10  9:49 ` Uwe Kleine-König
2022-11-10  9:49 ` Uwe Kleine-König
2022-11-16 17:21 ` Rob Herring [this message]
2022-11-16 17:21   ` Rob Herring
2022-11-16 17:21   ` Rob Herring
2022-11-28 17:18   ` Uwe Kleine-König
2022-11-28 17:18     ` Uwe Kleine-König
2022-11-28 17:18     ` Uwe Kleine-König
2022-11-16 17:49 ` Philipp Zabel
2022-11-16 17:49   ` Philipp Zabel
2022-11-16 17:49   ` Philipp Zabel
2022-11-17 15:09   ` Rob Herring
2022-11-17 15:09     ` [PATCH v1] dt-bindings: display: Convert fsl, imx-fb.txt " Rob Herring
2022-11-17 15:09     ` [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt " Rob Herring
2022-11-17 17:49   ` Krzysztof Kozlowski
2022-11-17 17:49     ` Krzysztof Kozlowski
2022-11-17 17:49     ` Krzysztof Kozlowski
2022-11-28 17:42     ` Uwe Kleine-König
2022-11-28 17:42       ` Uwe Kleine-König
2022-11-28 17:42       ` Uwe Kleine-König
2022-11-29 10:03       ` Krzysztof Kozlowski
2022-11-29 10:03         ` Krzysztof Kozlowski
2022-11-29 10:03         ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221116172131.GA233356-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=p.zabel@pengutronix.de \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.