[PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Jacopo Mondi]

Add device tree bindings documentation for the Renesas R-Car Display
Unit Color Management Module.

CMM is the image enhancement module available on each R-Car DU video
channel on R-Car Gen2 and Gen3 SoCs (V3H and V3M excluded).

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 .../bindings/display/renesas,cmm.txt          | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/renesas,cmm.txt

diff --git a/Documentation/devicetree/bindings/display/renesas,cmm.txt b/Documentation/devicetree/bindings/display/renesas,cmm.txt
new file mode 100644
index 000000000000..bc599b69c278
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/renesas,cmm.txt
@@ -0,0 +1,33 @@
+* Renesas R-Car Color Management Module (CMM)
+
+Renesas R-Car image enhancement module connected to R-Car DU video channels.
+
+Required properties:
+ - compatible: shall be one or more of the following:
+   - "renesas,cmm-r8a7795": for R8A7795 (R-Car H3) compatible CMM.
+   - "renesas,cmm-r8a7796": for R8A7796 (R-Car M3-W) compatible CMM.
+   - "renesas,cmm-r8a77965": for R8A77965 (R-Car M3-N) compatible CMM.
+   - "renesas,cmm-r8a77990": for R8A77990 (R-Car E3) compatible CMM.
+   - "renesas,cmm-r8a77995": for R8A77995 (R-Car D3) compatible CMM.
+   - "renesas,rcar-gen3-cmm": for a generic R-Car Gen3 compatible CMM.
+   - "renesas,rcar-gen2-cmm": for a generic R-Car Gen2 compatible CMM.
+
+   When the generic compatible string is specified, the SoC-specific
+   version corresponding to the platform should be listed first.
+
+ - reg: the address base and length of the memory area where CMM control
+   registers are mapped to.
+
+ - clocks: phandle and clock-specifier pair to the CMM functional clock
+   supplier.
+
+Example:
+--------
+
+	cmm0: cmm@fea40000 {
+		compatible = "renesas,cmm-r8a7796";
+		reg = <0 0xfea40000 0 0x1000>;
+		power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+		clocks = <&cpg CPG_MOD 711>;
+		resets = <&cpg 711>;
+	};
--
2.22.0

Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Geert Uytterhoeven]

Hi Jacopo,

On Sun, Aug 25, 2019 at 3:50 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> Add device tree bindings documentation for the Renesas R-Car Display
> Unit Color Management Module.
>
> CMM is the image enhancement module available on each R-Car DU video
> channel on R-Car Gen2 and Gen3 SoCs (V3H and V3M excluded).
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/renesas,cmm.txt
> @@ -0,0 +1,33 @@
> +* Renesas R-Car Color Management Module (CMM)
> +
> +Renesas R-Car image enhancement module connected to R-Car DU video channels.
> +
> +Required properties:
> + - compatible: shall be one or more of the following:
> +   - "renesas,cmm-r8a7795": for R8A7795 (R-Car H3) compatible CMM.
> +   - "renesas,cmm-r8a7796": for R8A7796 (R-Car M3-W) compatible CMM.
> +   - "renesas,cmm-r8a77965": for R8A77965 (R-Car M3-N) compatible CMM.
> +   - "renesas,cmm-r8a77990": for R8A77990 (R-Car E3) compatible CMM.
> +   - "renesas,cmm-r8a77995": for R8A77995 (R-Car D3) compatible CMM.

Please use "renesas,<socype->-cmm" instead of "renesas,cmm-<soctype>".

> +   - "renesas,rcar-gen3-cmm": for a generic R-Car Gen3 compatible CMM.
> +   - "renesas,rcar-gen2-cmm": for a generic R-Car Gen2 compatible CMM.
> +
> +   When the generic compatible string is specified, the SoC-specific
> +   version corresponding to the platform should be listed first.
> +
> + - reg: the address base and length of the memory area where CMM control
> +   registers are mapped to.
> +
> + - clocks: phandle and clock-specifier pair to the CMM functional clock
> +   supplier.

Thinking about yaml validation:

power-domains?
resets?

> +Example:
> +--------
> +
> +       cmm0: cmm@fea40000 {
> +               compatible = "renesas,cmm-r8a7796";
> +               reg = <0 0xfea40000 0 0x1000>;
> +               power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
> +               clocks = <&cpg CPG_MOD 711>;
> +               resets = <&cpg 711>;
> +       };

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Jacopo Mondi]

[-- Attachment #1: Type: text/plain, Size: 2768 bytes --]

Hi Geert,

On Mon, Aug 26, 2019 at 09:34:41AM +0200, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> On Sun, Aug 25, 2019 at 3:50 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> > Add device tree bindings documentation for the Renesas R-Car Display
> > Unit Color Management Module.
> >
> > CMM is the image enhancement module available on each R-Car DU video
> > channel on R-Car Gen2 and Gen3 SoCs (V3H and V3M excluded).
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/renesas,cmm.txt
> > @@ -0,0 +1,33 @@
> > +* Renesas R-Car Color Management Module (CMM)
> > +
> > +Renesas R-Car image enhancement module connected to R-Car DU video channels.
> > +
> > +Required properties:
> > + - compatible: shall be one or more of the following:
> > +   - "renesas,cmm-r8a7795": for R8A7795 (R-Car H3) compatible CMM.
> > +   - "renesas,cmm-r8a7796": for R8A7796 (R-Car M3-W) compatible CMM.
> > +   - "renesas,cmm-r8a77965": for R8A77965 (R-Car M3-N) compatible CMM.
> > +   - "renesas,cmm-r8a77990": for R8A77990 (R-Car E3) compatible CMM.
> > +   - "renesas,cmm-r8a77995": for R8A77995 (R-Car D3) compatible CMM.
>
> Please use "renesas,<socype->-cmm" instead of "renesas,cmm-<soctype>".
>

I actually copied it from the r-car gpio bindings, and I liked
cmm-<soctype> better. If you prefer I can change it though.

> > +   - "renesas,rcar-gen3-cmm": for a generic R-Car Gen3 compatible CMM.
> > +   - "renesas,rcar-gen2-cmm": for a generic R-Car Gen2 compatible CMM.
> > +
> > +   When the generic compatible string is specified, the SoC-specific
> > +   version corresponding to the platform should be listed first.
> > +
> > + - reg: the address base and length of the memory area where CMM control
> > +   registers are mapped to.
> > +
> > + - clocks: phandle and clock-specifier pair to the CMM functional clock
> > +   supplier.
>
> Thinking about yaml validation:
>
> power-domains?
> resets?
>

They should indeed be documented.

Thanks
   j

> > +Example:
> > +--------
> > +
> > +       cmm0: cmm@fea40000 {
> > +               compatible = "renesas,cmm-r8a7796";
> > +               reg = <0 0xfea40000 0 0x1000>;
> > +               power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
> > +               clocks = <&cpg CPG_MOD 711>;
> > +               resets = <&cpg 711>;
> > +       };
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Geert Uytterhoeven]

Hi Jacopo,

On Mon, Aug 26, 2019 at 9:58 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
> On Mon, Aug 26, 2019 at 09:34:41AM +0200, Geert Uytterhoeven wrote:
> > On Sun, Aug 25, 2019 at 3:50 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> > > Add device tree bindings documentation for the Renesas R-Car Display
> > > Unit Color Management Module.
> > >
> > > CMM is the image enhancement module available on each R-Car DU video
> > > channel on R-Car Gen2 and Gen3 SoCs (V3H and V3M excluded).
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > Thanks for your patch!
> >
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/display/renesas,cmm.txt
> > > @@ -0,0 +1,33 @@
> > > +* Renesas R-Car Color Management Module (CMM)
> > > +
> > > +Renesas R-Car image enhancement module connected to R-Car DU video channels.
> > > +
> > > +Required properties:
> > > + - compatible: shall be one or more of the following:
> > > +   - "renesas,cmm-r8a7795": for R8A7795 (R-Car H3) compatible CMM.
> > > +   - "renesas,cmm-r8a7796": for R8A7796 (R-Car M3-W) compatible CMM.
> > > +   - "renesas,cmm-r8a77965": for R8A77965 (R-Car M3-N) compatible CMM.
> > > +   - "renesas,cmm-r8a77990": for R8A77990 (R-Car E3) compatible CMM.
> > > +   - "renesas,cmm-r8a77995": for R8A77995 (R-Car D3) compatible CMM.
> >
> > Please use "renesas,<socype->-cmm" instead of "renesas,cmm-<soctype>".
> >
>
> I actually copied it from the r-car gpio bindings, and I liked
> cmm-<soctype> better. If you prefer I can change it though.

We switched from "renesas,cmm-<soctype>" to "renesas,<socype->-cmm"
a few years ago, upon request from the DT people:

    vendor -> family/SoC -> IP core

Unfortunately we cannot change the old style in existing bindings, without
great effort and backwards compatibility ramifications.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Laurent Pinchart]

Hi Jacopo,

On Mon, Aug 26, 2019 at 09:59:43AM +0200, Jacopo Mondi wrote:
> On Mon, Aug 26, 2019 at 09:34:41AM +0200, Geert Uytterhoeven wrote:
> > On Sun, Aug 25, 2019 at 3:50 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> > > Add device tree bindings documentation for the Renesas R-Car Display
> > > Unit Color Management Module.
> > >
> > > CMM is the image enhancement module available on each R-Car DU video
> > > channel on R-Car Gen2 and Gen3 SoCs (V3H and V3M excluded).
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > Thanks for your patch!
> >
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/display/renesas,cmm.txt
> > > @@ -0,0 +1,33 @@
> > > +* Renesas R-Car Color Management Module (CMM)
> > > +
> > > +Renesas R-Car image enhancement module connected to R-Car DU video channels.
> > > +
> > > +Required properties:
> > > + - compatible: shall be one or more of the following:
> > > +   - "renesas,cmm-r8a7795": for R8A7795 (R-Car H3) compatible CMM.
> > > +   - "renesas,cmm-r8a7796": for R8A7796 (R-Car M3-W) compatible CMM.
> > > +   - "renesas,cmm-r8a77965": for R8A77965 (R-Car M3-N) compatible CMM.
> > > +   - "renesas,cmm-r8a77990": for R8A77990 (R-Car E3) compatible CMM.
> > > +   - "renesas,cmm-r8a77995": for R8A77995 (R-Car D3) compatible CMM.
> >
> > Please use "renesas,<socype->-cmm" instead of "renesas,cmm-<soctype>".
> 
> I actually copied it from the r-car gpio bindings, and I liked
> cmm-<soctype> better. If you prefer I can change it though.
> 
> > > +   - "renesas,rcar-gen3-cmm": for a generic R-Car Gen3 compatible CMM.
> > > +   - "renesas,rcar-gen2-cmm": for a generic R-Car Gen2 compatible CMM.
> > > +
> > > +   When the generic compatible string is specified, the SoC-specific
> > > +   version corresponding to the platform should be listed first.
> > > +
> > > + - reg: the address base and length of the memory area where CMM control
> > > +   registers are mapped to.
> > > +
> > > + - clocks: phandle and clock-specifier pair to the CMM functional clock
> > > +   supplier.
> >
> > Thinking about yaml validation:
> >
> > power-domains?
> > resets?
> 
> They should indeed be documented.

How about converting this binding to yaml alreay ? It should be fairly
simple.

> > > +Example:
> > > +--------
> > > +
> > > +       cmm0: cmm@fea40000 {
> > > +               compatible = "renesas,cmm-r8a7796";
> > > +               reg = <0 0xfea40000 0 0x1000>;
> > > +               power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
> > > +               clocks = <&cpg CPG_MOD 711>;
> > > +               resets = <&cpg 711>;
> > > +       };

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Jacopo Mondi]

[-- Attachment #1: Type: text/plain, Size: 4870 bytes --]

Hi Laurent,

On Mon, Aug 26, 2019 at 01:15:50PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Aug 26, 2019 at 09:59:43AM +0200, Jacopo Mondi wrote:
> > On Mon, Aug 26, 2019 at 09:34:41AM +0200, Geert Uytterhoeven wrote:
> > > On Sun, Aug 25, 2019 at 3:50 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> > > > Add device tree bindings documentation for the Renesas R-Car Display
> > > > Unit Color Management Module.
> > > >
> > > > CMM is the image enhancement module available on each R-Car DU video
> > > > channel on R-Car Gen2 and Gen3 SoCs (V3H and V3M excluded).
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > >
> > > Thanks for your patch!
> > >
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/display/renesas,cmm.txt
> > > > @@ -0,0 +1,33 @@
> > > > +* Renesas R-Car Color Management Module (CMM)
> > > > +
> > > > +Renesas R-Car image enhancement module connected to R-Car DU video channels.
> > > > +
> > > > +Required properties:
> > > > + - compatible: shall be one or more of the following:
> > > > +   - "renesas,cmm-r8a7795": for R8A7795 (R-Car H3) compatible CMM.
> > > > +   - "renesas,cmm-r8a7796": for R8A7796 (R-Car M3-W) compatible CMM.
> > > > +   - "renesas,cmm-r8a77965": for R8A77965 (R-Car M3-N) compatible CMM.
> > > > +   - "renesas,cmm-r8a77990": for R8A77990 (R-Car E3) compatible CMM.
> > > > +   - "renesas,cmm-r8a77995": for R8A77995 (R-Car D3) compatible CMM.
> > >
> > > Please use "renesas,<socype->-cmm" instead of "renesas,cmm-<soctype>".
> >
> > I actually copied it from the r-car gpio bindings, and I liked
> > cmm-<soctype> better. If you prefer I can change it though.
> >
> > > > +   - "renesas,rcar-gen3-cmm": for a generic R-Car Gen3 compatible CMM.
> > > > +   - "renesas,rcar-gen2-cmm": for a generic R-Car Gen2 compatible CMM.
> > > > +
> > > > +   When the generic compatible string is specified, the SoC-specific
> > > > +   version corresponding to the platform should be listed first.
> > > > +
> > > > + - reg: the address base and length of the memory area where CMM control
> > > > +   registers are mapped to.
> > > > +
> > > > + - clocks: phandle and clock-specifier pair to the CMM functional clock
> > > > +   supplier.
> > >
> > > Thinking about yaml validation:
> > >
> > > power-domains?
> > > resets?
> >
> > They should indeed be documented.
>
> How about converting this binding to yaml alreay ? It should be fairly
> simple.

I'm trying to, and I'm having my portion of fun time at it.

The definition of the schema itself seems good, but I wonder, is this
the first renesas schema we have? Because it seems to me the schema
validator is having an hard time to digest the examplea 'clocks' and
'power-domains' properties, which have 1 phandle and 2 specifiers and 1
phandle and 1 specifier respectively for Rensas SoCs.

In other words, if in the example I have:

 examples:
   - |
     cmm0: cmm@fea40000 {
          compatible = "renesas,r8a7796-cmm";
          reg = <0 0xfea40000 0 0x1000>;
          clocks = <&cpg 711>              <---- 1 phandle + 1 specifier
          resets = <&cpg 711>;
          power-domains = <&sysc>;         <---- 1 phandle
     };

The schema validation is good.

While if I use an actual example
   - |
     cmm0: cmm@fea40000 {
          compatible = "renesas,r8a7796-cmm";
          reg = <0 0xfea40000 0 0x1000>;
          clocks = <&cpg CPG_MOD 711>         <---- 1 phandle + 2 specifier
          resets = <&cpg 711>;
          power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; <---- 1 phandle
     };                                                       + 1 specfier

The schema validation fails...
Error: Documentation/devicetree/bindings/display/renesas,cmm.example.dts:20.29-30 syntax error
FATAL ERROR: Unable to parse input tree

Are clocks properties with > 2 entries and power-domains properties with
> 1 entries supported?

Because from what I read here:
https://github.com/robherring/yaml-bindings/blob/master/schemas/clock/clock.yaml
"The length of a clock specifier is defined by the value of a #clock-cells
property in the clock provider node."

And that's expected, but is the examples actually validated against the
clock provider pointed by the phandle? Because in that case, if we had a
yaml schema for the cpg-mssr provider, it would indeed specify clock-cells=2.

Do we need a schema for cpg-mssr first, or am I doing something else
wrong?

Thanks
   j

>
> > > > +Example:
> > > > +--------
> > > > +
> > > > +       cmm0: cmm@fea40000 {
> > > > +               compatible = "renesas,cmm-r8a7796";
> > > > +               reg = <0 0xfea40000 0 0x1000>;
> > > > +               power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
> > > > +               clocks = <&cpg CPG_MOD 711>;
> > > > +               resets = <&cpg 711>;
> > > > +       };
>
> --
> Regards,
>
> Laurent Pinchart

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Laurent Pinchart]

Hi Jacopo,

On Fri, Aug 30, 2019 at 08:01:09PM +0200, Jacopo Mondi wrote:
> On Mon, Aug 26, 2019 at 01:15:50PM +0300, Laurent Pinchart wrote:
> > On Mon, Aug 26, 2019 at 09:59:43AM +0200, Jacopo Mondi wrote:
> >> On Mon, Aug 26, 2019 at 09:34:41AM +0200, Geert Uytterhoeven wrote:
> >>> On Sun, Aug 25, 2019 at 3:50 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> >>>> Add device tree bindings documentation for the Renesas R-Car Display
> >>>> Unit Color Management Module.
> >>>>
> >>>> CMM is the image enhancement module available on each R-Car DU video
> >>>> channel on R-Car Gen2 and Gen3 SoCs (V3H and V3M excluded).
> >>>>
> >>>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >>>
> >>> Thanks for your patch!
> >>>
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/display/renesas,cmm.txt
> >>>> @@ -0,0 +1,33 @@
> >>>> +* Renesas R-Car Color Management Module (CMM)
> >>>> +
> >>>> +Renesas R-Car image enhancement module connected to R-Car DU video channels.
> >>>> +
> >>>> +Required properties:
> >>>> + - compatible: shall be one or more of the following:
> >>>> +   - "renesas,cmm-r8a7795": for R8A7795 (R-Car H3) compatible CMM.
> >>>> +   - "renesas,cmm-r8a7796": for R8A7796 (R-Car M3-W) compatible CMM.
> >>>> +   - "renesas,cmm-r8a77965": for R8A77965 (R-Car M3-N) compatible CMM.
> >>>> +   - "renesas,cmm-r8a77990": for R8A77990 (R-Car E3) compatible CMM.
> >>>> +   - "renesas,cmm-r8a77995": for R8A77995 (R-Car D3) compatible CMM.
> >>>
> >>> Please use "renesas,<socype->-cmm" instead of "renesas,cmm-<soctype>".
> >>
> >> I actually copied it from the r-car gpio bindings, and I liked
> >> cmm-<soctype> better. If you prefer I can change it though.
> >>
> >>>> +   - "renesas,rcar-gen3-cmm": for a generic R-Car Gen3 compatible CMM.
> >>>> +   - "renesas,rcar-gen2-cmm": for a generic R-Car Gen2 compatible CMM.
> >>>> +
> >>>> +   When the generic compatible string is specified, the SoC-specific
> >>>> +   version corresponding to the platform should be listed first.
> >>>> +
> >>>> + - reg: the address base and length of the memory area where CMM control
> >>>> +   registers are mapped to.
> >>>> +
> >>>> + - clocks: phandle and clock-specifier pair to the CMM functional clock
> >>>> +   supplier.
> >>>
> >>> Thinking about yaml validation:
> >>>
> >>> power-domains?
> >>> resets?
> >>
> >> They should indeed be documented.
> >
> > How about converting this binding to yaml alreay ? It should be fairly
> > simple.
> 
> I'm trying to, and I'm having my portion of fun time at it.
> 
> The definition of the schema itself seems good, but I wonder, is this
> the first renesas schema we have? Because it seems to me the schema
> validator is having an hard time to digest the examplea 'clocks' and
> 'power-domains' properties, which have 1 phandle and 2 specifiers and 1
> phandle and 1 specifier respectively for Rensas SoCs.
> 
> In other words, if in the example I have:
> 
>  examples:
>    - |
>      cmm0: cmm@fea40000 {
>           compatible = "renesas,r8a7796-cmm";
>           reg = <0 0xfea40000 0 0x1000>;
>           clocks = <&cpg 711>              <---- 1 phandle + 1 specifier
>           resets = <&cpg 711>;
>           power-domains = <&sysc>;         <---- 1 phandle
>      };
> 
> The schema validation is good.
> 
> While if I use an actual example
>    - |
>      cmm0: cmm@fea40000 {
>           compatible = "renesas,r8a7796-cmm";
>           reg = <0 0xfea40000 0 0x1000>;
>           clocks = <&cpg CPG_MOD 711>         <---- 1 phandle + 2 specifier
>           resets = <&cpg 711>;
>           power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; <---- 1 phandle
>      };                                                       + 1 specfier
> 
> The schema validation fails...
> Error: Documentation/devicetree/bindings/display/renesas,cmm.example.dts:20.29-30 syntax error
> FATAL ERROR: Unable to parse input tree
> 
> Are clocks properties with > 2 entries and power-domains properties with
> > 1 entries supported?
> 
> Because from what I read here:
> https://github.com/robherring/yaml-bindings/blob/master/schemas/clock/clock.yaml
> "The length of a clock specifier is defined by the value of a #clock-cells
> property in the clock provider node."
> 
> And that's expected, but is the examples actually validated against the
> clock provider pointed by the phandle? Because in that case, if we had a
> yaml schema for the cpg-mssr provider, it would indeed specify clock-cells=2.
> 
> Do we need a schema for cpg-mssr first, or am I doing something else
> wrong?

I think you just need to #include the headers that define CPG_MOD and
R8A7796_PD_ALWAYS_ON.

> >>>> +Example:
> >>>> +--------
> >>>> +
> >>>> +       cmm0: cmm@fea40000 {
> >>>> +               compatible = "renesas,cmm-r8a7796";
> >>>> +               reg = <0 0xfea40000 0 0x1000>;
> >>>> +               power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
> >>>> +               clocks = <&cpg CPG_MOD 711>;
> >>>> +               resets = <&cpg 711>;
> >>>> +       };

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Geert Uytterhoeven]

Hi Laurent,

On Thu, Sep 5, 2019 at 1:50 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Aug 30, 2019 at 08:01:09PM +0200, Jacopo Mondi wrote:
> > On Mon, Aug 26, 2019 at 01:15:50PM +0300, Laurent Pinchart wrote:
> > > How about converting this binding to yaml alreay ? It should be fairly
> > > simple.
> >
> > I'm trying to, and I'm having my portion of fun time at it.
> >
> > The definition of the schema itself seems good, but I wonder, is this
> > the first renesas schema we have? Because it seems to me the schema
> > validator is having an hard time to digest the examplea 'clocks' and
> > 'power-domains' properties, which have 1 phandle and 2 specifiers and 1
> > phandle and 1 specifier respectively for Rensas SoCs.
> >
> > In other words, if in the example I have:
> >
> >  examples:
> >    - |
> >      cmm0: cmm@fea40000 {
> >           compatible = "renesas,r8a7796-cmm";
> >           reg = <0 0xfea40000 0 0x1000>;
> >           clocks = <&cpg 711>              <---- 1 phandle + 1 specifier
> >           resets = <&cpg 711>;
> >           power-domains = <&sysc>;         <---- 1 phandle
> >      };
> >
> > The schema validation is good.
> >
> > While if I use an actual example
> >    - |
> >      cmm0: cmm@fea40000 {
> >           compatible = "renesas,r8a7796-cmm";
> >           reg = <0 0xfea40000 0 0x1000>;
> >           clocks = <&cpg CPG_MOD 711>         <---- 1 phandle + 2 specifier
> >           resets = <&cpg 711>;
> >           power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; <---- 1 phandle
> >      };                                                       + 1 specfier
> >
> > The schema validation fails...
> > Error: Documentation/devicetree/bindings/display/renesas,cmm.example.dts:20.29-30 syntax error
> > FATAL ERROR: Unable to parse input tree
> >
> > Are clocks properties with > 2 entries and power-domains properties with
> > > 1 entries supported?
> >
> > Because from what I read here:
> > https://github.com/robherring/yaml-bindings/blob/master/schemas/clock/clock.yaml
> > "The length of a clock specifier is defined by the value of a #clock-cells
> > property in the clock provider node."
> >
> > And that's expected, but is the examples actually validated against the
> > clock provider pointed by the phandle? Because in that case, if we had a
> > yaml schema for the cpg-mssr provider, it would indeed specify clock-cells=2.
> >
> > Do we need a schema for cpg-mssr first, or am I doing something else
> > wrong?
>
> I think you just need to #include the headers that define CPG_MOD and
> R8A7796_PD_ALWAYS_ON.

If that helps, you might want to replace the symbols in the examples by their
actual values (1 resp. 32).

And perhaps keep the symbols as comments?

        clocks = <&cpg 1 /* CPG_MOD */ 711>;
        power-domains = <&sysc 32 /* R8A7796_PD_ALWAYS_ON */>;

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Laurent Pinchart]

Hi Geert,

On Thu, Sep 05, 2019 at 02:05:34PM +0200, Geert Uytterhoeven wrote:
> On Thu, Sep 5, 2019 at 1:50 PM Laurent Pinchart wrote:
> > On Fri, Aug 30, 2019 at 08:01:09PM +0200, Jacopo Mondi wrote:
> > > On Mon, Aug 26, 2019 at 01:15:50PM +0300, Laurent Pinchart wrote:
> > > > How about converting this binding to yaml alreay ? It should be fairly
> > > > simple.
> > >
> > > I'm trying to, and I'm having my portion of fun time at it.
> > >
> > > The definition of the schema itself seems good, but I wonder, is this
> > > the first renesas schema we have? Because it seems to me the schema
> > > validator is having an hard time to digest the examplea 'clocks' and
> > > 'power-domains' properties, which have 1 phandle and 2 specifiers and 1
> > > phandle and 1 specifier respectively for Rensas SoCs.
> > >
> > > In other words, if in the example I have:
> > >
> > >  examples:
> > >    - |
> > >      cmm0: cmm@fea40000 {
> > >           compatible = "renesas,r8a7796-cmm";
> > >           reg = <0 0xfea40000 0 0x1000>;
> > >           clocks = <&cpg 711>              <---- 1 phandle + 1 specifier
> > >           resets = <&cpg 711>;
> > >           power-domains = <&sysc>;         <---- 1 phandle
> > >      };
> > >
> > > The schema validation is good.
> > >
> > > While if I use an actual example
> > >    - |
> > >      cmm0: cmm@fea40000 {
> > >           compatible = "renesas,r8a7796-cmm";
> > >           reg = <0 0xfea40000 0 0x1000>;
> > >           clocks = <&cpg CPG_MOD 711>         <---- 1 phandle + 2 specifier
> > >           resets = <&cpg 711>;
> > >           power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; <---- 1 phandle
> > >      };                                                       + 1 specfier
> > >
> > > The schema validation fails...
> > > Error: Documentation/devicetree/bindings/display/renesas,cmm.example.dts:20.29-30 syntax error
> > > FATAL ERROR: Unable to parse input tree
> > >
> > > Are clocks properties with > 2 entries and power-domains properties with
> > > > 1 entries supported?
> > >
> > > Because from what I read here:
> > > https://github.com/robherring/yaml-bindings/blob/master/schemas/clock/clock.yaml
> > > "The length of a clock specifier is defined by the value of a #clock-cells
> > > property in the clock provider node."
> > >
> > > And that's expected, but is the examples actually validated against the
> > > clock provider pointed by the phandle? Because in that case, if we had a
> > > yaml schema for the cpg-mssr provider, it would indeed specify clock-cells=2.
> > >
> > > Do we need a schema for cpg-mssr first, or am I doing something else
> > > wrong?
> >
> > I think you just need to #include the headers that define CPG_MOD and
> > R8A7796_PD_ALWAYS_ON.
> 
> If that helps, you might want to replace the symbols in the examples by their
> actual values (1 resp. 32).
> 
> And perhaps keep the symbols as comments?
> 
>         clocks = <&cpg 1 /* CPG_MOD */ 711>;
>         power-domains = <&sysc 32 /* R8A7796_PD_ALWAYS_ON */>;

I think adding the required #include at the beginning of the example is
a better solution.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation

[Jacopo Mondi]

[-- Attachment #1: Type: text/plain, Size: 3923 bytes --]

Hi Laurent, Geert,


On Thu, Sep 05, 2019 at 03:20:59PM +0300, Laurent Pinchart wrote:
> Hi Geert,
>
> On Thu, Sep 05, 2019 at 02:05:34PM +0200, Geert Uytterhoeven wrote:
> > On Thu, Sep 5, 2019 at 1:50 PM Laurent Pinchart wrote:
> > > On Fri, Aug 30, 2019 at 08:01:09PM +0200, Jacopo Mondi wrote:
> > > > On Mon, Aug 26, 2019 at 01:15:50PM +0300, Laurent Pinchart wrote:
> > > > > How about converting this binding to yaml alreay ? It should be fairly
> > > > > simple.
> > > >
> > > > I'm trying to, and I'm having my portion of fun time at it.
> > > >
> > > > The definition of the schema itself seems good, but I wonder, is this
> > > > the first renesas schema we have? Because it seems to me the schema
> > > > validator is having an hard time to digest the examplea 'clocks' and
> > > > 'power-domains' properties, which have 1 phandle and 2 specifiers and 1
> > > > phandle and 1 specifier respectively for Rensas SoCs.
> > > >
> > > > In other words, if in the example I have:
> > > >
> > > >  examples:
> > > >    - |
> > > >      cmm0: cmm@fea40000 {
> > > >           compatible = "renesas,r8a7796-cmm";
> > > >           reg = <0 0xfea40000 0 0x1000>;
> > > >           clocks = <&cpg 711>              <---- 1 phandle + 1 specifier
> > > >           resets = <&cpg 711>;
> > > >           power-domains = <&sysc>;         <---- 1 phandle
> > > >      };
> > > >
> > > > The schema validation is good.
> > > >
> > > > While if I use an actual example
> > > >    - |
> > > >      cmm0: cmm@fea40000 {
> > > >           compatible = "renesas,r8a7796-cmm";
> > > >           reg = <0 0xfea40000 0 0x1000>;
> > > >           clocks = <&cpg CPG_MOD 711>         <---- 1 phandle + 2 specifier
> > > >           resets = <&cpg 711>;
> > > >           power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; <---- 1 phandle
> > > >      };                                                       + 1 specfier
> > > >
> > > > The schema validation fails...
> > > > Error: Documentation/devicetree/bindings/display/renesas,cmm.example.dts:20.29-30 syntax error
> > > > FATAL ERROR: Unable to parse input tree
> > > >
> > > > Are clocks properties with > 2 entries and power-domains properties with
> > > > > 1 entries supported?
> > > >
> > > > Because from what I read here:
> > > > https://github.com/robherring/yaml-bindings/blob/master/schemas/clock/clock.yaml
> > > > "The length of a clock specifier is defined by the value of a #clock-cells
> > > > property in the clock provider node."
> > > >
> > > > And that's expected, but is the examples actually validated against the
> > > > clock provider pointed by the phandle? Because in that case, if we had a
> > > > yaml schema for the cpg-mssr provider, it would indeed specify clock-cells=2.
> > > >
> > > > Do we need a schema for cpg-mssr first, or am I doing something else
> > > > wrong?
> > >
> > > I think you just need to #include the headers that define CPG_MOD and
> > > R8A7796_PD_ALWAYS_ON.
> >
> > If that helps, you might want to replace the symbols in the examples by their
> > actual values (1 resp. 32).
> >
> > And perhaps keep the symbols as comments?
> >
> >         clocks = <&cpg 1 /* CPG_MOD */ 711>;
> >         power-domains = <&sysc 32 /* R8A7796_PD_ALWAYS_ON */>;
>
> I think adding the required #include at the beginning of the example is
> a better solution.

I didn't realize that, but it actually makes sense, as the example is
extracted and actually compiled from the yaml schema.. brilliant!

With a simple:

--- a/Documentation/devicetree/bindings/display/renesas,cmm.yaml
+++ b/Documentation/devicetree/bindings/display/renesas,cmm.yaml
@@ -51,6 +51,9 @@ additionalProperties: false

 examples:
   - |
+    #include <dt-bindings/clock/r8a7796-cpg-mssr.h>
+    #include <dt-bindings/power/r8a7796-sysc.h>

The example now compiles.

Thanks, I will submit the bindings in yaml format in next iteration.

>
> --
> Regards,
>
> Laurent Pinchart

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 02/14] dt-bindings: display, renesas,du: Document cmms property

[Jacopo Mondi]

Document the newly added 'cmms' property which accepts a list of phandle
and channel index pairs that point to the CMM units available for each
Display Unit output video channel.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/devicetree/bindings/display/renesas,du.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
index c97dfacad281..c2265e2a1af2 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.txt
+++ b/Documentation/devicetree/bindings/display/renesas,du.txt
@@ -45,6 +45,10 @@ Required Properties:
     instance that serves the DU channel, and the channel index identifies the
     LIF instance in that VSP.

+  - cmms: A list of phandles to the CMM instances present in the SoC, one
+    for each available DU channel. The property shall not be specified for
+    SoCs that do not provide any CMM (such as V3M and V3H).
+
 Required nodes:

 The connections to the DU output video ports are modeled using the OF graph
@@ -91,6 +95,7 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
 			 <&cpg CPG_MOD 721>;
 		clock-names = "du.0", "du.1", "du.2", "du.3";
 		vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>;
+		cmms = <&cmm0 &cmm1 &cmm2 &cmm3>;

 		ports {
 			#address-cells = <1>;
--
2.22.0

Re: [PATCH v3 02/14] dt-bindings: display, renesas,du: Document cmms property

[Rob Herring]

On Sun, Aug 25, 2019 at 03:51:42PM +0200, Jacopo Mondi wrote:
> Document the newly added 'cmms' property which accepts a list of phandle
> and channel index pairs that point to the CMM units available for each
> Display Unit output video channel.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  Documentation/devicetree/bindings/display/renesas,du.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
> index c97dfacad281..c2265e2a1af2 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> @@ -45,6 +45,10 @@ Required Properties:
>      instance that serves the DU channel, and the channel index identifies the
>      LIF instance in that VSP.
> 
> +  - cmms: A list of phandles to the CMM instances present in the SoC, one
> +    for each available DU channel. The property shall not be specified for
> +    SoCs that do not provide any CMM (such as V3M and V3H).

renesas,cmms

Perhaps define what CMM is.

> +
>  Required nodes:
> 
>  The connections to the DU output video ports are modeled using the OF graph
> @@ -91,6 +95,7 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
>  			 <&cpg CPG_MOD 721>;
>  		clock-names = "du.0", "du.1", "du.2", "du.3";
>  		vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>;
> +		cmms = <&cmm0 &cmm1 &cmm2 &cmm3>;
> 
>  		ports {
>  			#address-cells = <1>;
> --
> 2.22.0
> 

Re: [PATCH v3 02/14] dt-bindings: display, renesas, du: Document cmms property

[Geert Uytterhoeven]

On Tue, Aug 27, 2019 at 10:29 PM Rob Herring <robh@kernel.org> wrote:
> On Sun, Aug 25, 2019 at 03:51:42PM +0200, Jacopo Mondi wrote:
> > Document the newly added 'cmms' property which accepts a list of phandle
> > and channel index pairs that point to the CMM units available for each
> > Display Unit output video channel.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  Documentation/devicetree/bindings/display/renesas,du.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
> > index c97dfacad281..c2265e2a1af2 100644
> > --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> > +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> > @@ -45,6 +45,10 @@ Required Properties:
> >      instance that serves the DU channel, and the channel index identifies the
> >      LIF instance in that VSP.
> >
> > +  - cmms: A list of phandles to the CMM instances present in the SoC, one
> > +    for each available DU channel. The property shall not be specified for
> > +    SoCs that do not provide any CMM (such as V3M and V3H).
>
> renesas,cmms

So I guess we really wanted to have the prefix for the vsps property, too?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 02/14] dt-bindings: display, renesas,du: Document cmms property

[Laurent Pinchart]

On Wed, Aug 28, 2019 at 09:32:23AM +0200, Geert Uytterhoeven wrote:
> On Tue, Aug 27, 2019 at 10:29 PM Rob Herring <robh@kernel.org> wrote:
> > On Sun, Aug 25, 2019 at 03:51:42PM +0200, Jacopo Mondi wrote:
> > > Document the newly added 'cmms' property which accepts a list of phandle
> > > and channel index pairs that point to the CMM units available for each
> > > Display Unit output video channel.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  Documentation/devicetree/bindings/display/renesas,du.txt | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
> > > index c97dfacad281..c2265e2a1af2 100644
> > > --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> > > +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> > > @@ -45,6 +45,10 @@ Required Properties:
> > >      instance that serves the DU channel, and the channel index identifies the
> > >      LIF instance in that VSP.
> > >
> > > +  - cmms: A list of phandles to the CMM instances present in the SoC, one
> > > +    for each available DU channel. The property shall not be specified for
> > > +    SoCs that do not provide any CMM (such as V3M and V3H).
> >
> > renesas,cmms
> 
> So I guess we really wanted to have the prefix for the vsps property, too?

Yes, we should have :-( My bad.

-- 
Regards,

Laurent Pinchart