[PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Mighty]

From: Mithil Bavishi <bavishimithil@gmail.com>

Convert the OMAP4+ McPDM bindings to DT schema.

Signed-off-by: Mithil Bavishi <bavishimithil@gmail.com>
---
Changelog v5:
- Add imports for constants
- Add desc to ti,hwmods

 .../devicetree/bindings/sound/omap-mcpdm.txt  | 30 ---------
 .../bindings/sound/ti,omap4-mcpdm.yaml        | 61 +++++++++++++++++++
 2 files changed, 61 insertions(+), 30 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/sound/omap-mcpdm.txt
 create mode 100644 Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml

diff --git a/Documentation/devicetree/bindings/sound/omap-mcpdm.txt b/Documentation/devicetree/bindings/sound/omap-mcpdm.txt
deleted file mode 100644
index ff98a0cb5..000000000
--- a/Documentation/devicetree/bindings/sound/omap-mcpdm.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-* Texas Instruments OMAP4+ McPDM
-
-Required properties:
-- compatible: "ti,omap4-mcpdm"
-- reg: Register location and size as an array:
-       <MPU access base address, size>,
-       <L3 interconnect address, size>;
-- interrupts: Interrupt number for McPDM
-- ti,hwmods: Name of the hwmod associated to the McPDM
-- clocks:  phandle for the pdmclk provider, likely <&twl6040>
-- clock-names: Must be "pdmclk"
-
-Example:
-
-mcpdm: mcpdm@40132000 {
-	compatible = "ti,omap4-mcpdm";
-	reg = <0x40132000 0x7f>, /* MPU private access */
-	      <0x49032000 0x7f>; /* L3 Interconnect */
-	interrupts = <0 112 0x4>;
-	interrupt-parent = <&gic>;
-	ti,hwmods = "mcpdm";
-};
-
-In board DTS file the pdmclk needs to be added:
-
-&mcpdm {
-	clocks = <&twl6040>;
-	clock-names = "pdmclk";
-	status = "okay";
-};
diff --git a/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml b/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml
new file mode 100644
index 000000000..966406078
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/ti,omap4-mcpdm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: OMAP McPDM
+
+maintainers:
+  - Misael Lopez Cruz <misael.lopez@ti.com>
+
+description:
+  OMAP ALSA SoC DAI driver using McPDM port used by TWL6040
+
+properties:
+  compatible:
+    const: ti,omap4-mcpdm
+
+  reg:
+    items:
+      - description: MPU access base address
+      - description: L3 interconnect address
+
+  interrupts:
+    maxItems: 1
+
+  ti,hwmods:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [mcpdm]
+    description: Name of the hwmod associated to the McPDM, likely "mcpdm"
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: pdmclk
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - ti,hwmods
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    pdm@40132000 {
+      compatible = "ti,omap4-mcpdm";
+      reg = <0x40132000 0x7f>, /* MPU private access */
+            <0x49032000 0x7f>; /* L3 Interconnect */
+      interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
+      interrupt-parent = <&gic>;
+      ti,hwmods = "mcpdm";
+      clocks = <&twl6040>;
+      clock-names = "pdmclk";
+    };
-- 
2.34.1

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Krzysztof Kozlowski]

On 22/05/2024 09:52, Mighty wrote:
> From: Mithil Bavishi <bavishimithil@gmail.com>
> 
> Convert the OMAP4+ McPDM bindings to DT schema.
> 
> Signed-off-by: Mithil Bavishi <bavishimithil@gmail.com>
> ---
> Changelog v5:
> - Add imports for constants
> - Add desc to ti,hwmods

You are not making it easier for us to review:
====
b4 diff '<20240522075245.388-1-bavishimithil@gmail.com>'
Grabbing thread from
lore.kernel.org/all/20240522075245.388-1-bavishimithil@gmail.com/t.mbox.gz
Checking for older revisions
Grabbing search results from lore.kernel.org
  Added from v4: 2 patches
---
Analyzing 15 messages in the thread
WARNING: duplicate messages found at index 1
   Subject 1: ASoC: dt-bindings: omap-mcpdm: Convert to DT schema
   Subject 2: ASoC: dt-bindings: omap-mcpdm: Convert to DT schema
  2 is not a reply... assume additional patch
Preparing fake-am for v4: ASoC: dt-bindings: omap-mcpdm: Convert to DT
schema
ERROR: Could not fake-am version v4
---
Could not create fake-am range for lower series v4

====


Looks good, but let's wait few hours to see if bot is happy as well.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Mithil]

> You are not making it easier for us to review:
> ====
> b4 diff '<20240522075245.388-1-bavishimithil@gmail.com>'
> Grabbing thread from
> lore.kernel.org/all/20240522075245.388-1-bavishimithil@gmail.com/t.mbox.gz
> Checking for older revisions
> Grabbing search results from lore.kernel.org
>   Added from v4: 2 patches
> ---
> Analyzing 15 messages in the thread
> WARNING: duplicate messages found at index 1
>    Subject 1: ASoC: dt-bindings: omap-mcpdm: Convert to DT schema
>    Subject 2: ASoC: dt-bindings: omap-mcpdm: Convert to DT schema
>   2 is not a reply... assume additional patch
> Preparing fake-am for v4: ASoC: dt-bindings: omap-mcpdm: Convert to DT
> schema
> ERROR: Could not fake-am version v4
> ---
> Could not create fake-am range for lower series v4
>
> ====
>
Hey, sorry about it, its my first time with lkml, could you explain
what seems to be the issue here?


-- 
Best Regards,
Mithil

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Krzysztof Kozlowski]

On 22/05/2024 15:46, Mithil wrote:
>> You are not making it easier for us to review:
>> ====
>> b4 diff '<20240522075245.388-1-bavishimithil@gmail.com>'
>> Grabbing thread from
>> lore.kernel.org/all/20240522075245.388-1-bavishimithil@gmail.com/t.mbox.gz
>> Checking for older revisions
>> Grabbing search results from lore.kernel.org
>>   Added from v4: 2 patches
>> ---
>> Analyzing 15 messages in the thread
>> WARNING: duplicate messages found at index 1
>>    Subject 1: ASoC: dt-bindings: omap-mcpdm: Convert to DT schema
>>    Subject 2: ASoC: dt-bindings: omap-mcpdm: Convert to DT schema
>>   2 is not a reply... assume additional patch
>> Preparing fake-am for v4: ASoC: dt-bindings: omap-mcpdm: Convert to DT
>> schema
>> ERROR: Could not fake-am version v4
>> ---
>> Could not create fake-am range for lower series v4
>>
>> ====
>>
> Hey, sorry about it, its my first time with lkml, could you explain
> what seems to be the issue here?

You sent multiple same versions, I think more than one v4. You can try
by yourself - does b4 work on this patchset?

Best regards,
Krzysztof

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Péter Ujfalusi]

Hi,

On 22/05/2024 10:52, Mighty wrote:
> From: Mithil Bavishi <bavishimithil@gmail.com>
> 
> Convert the OMAP4+ McPDM bindings to DT schema.
> 
> Signed-off-by: Mithil Bavishi <bavishimithil@gmail.com>
> ---
> Changelog v5:
> - Add imports for constants
> - Add desc to ti,hwmods
> 
>  .../devicetree/bindings/sound/omap-mcpdm.txt  | 30 ---------
>  .../bindings/sound/ti,omap4-mcpdm.yaml        | 61 +++++++++++++++++++
>  2 files changed, 61 insertions(+), 30 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/sound/omap-mcpdm.txt
>  create mode 100644 Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/omap-mcpdm.txt b/Documentation/devicetree/bindings/sound/omap-mcpdm.txt
> deleted file mode 100644
> index ff98a0cb5..000000000
> --- a/Documentation/devicetree/bindings/sound/omap-mcpdm.txt
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -* Texas Instruments OMAP4+ McPDM
> -
> -Required properties:
> -- compatible: "ti,omap4-mcpdm"
> -- reg: Register location and size as an array:
> -       <MPU access base address, size>,
> -       <L3 interconnect address, size>;
> -- interrupts: Interrupt number for McPDM
> -- ti,hwmods: Name of the hwmod associated to the McPDM
> -- clocks:  phandle for the pdmclk provider, likely <&twl6040>
> -- clock-names: Must be "pdmclk"
> -
> -Example:
> -
> -mcpdm: mcpdm@40132000 {
> -	compatible = "ti,omap4-mcpdm";
> -	reg = <0x40132000 0x7f>, /* MPU private access */
> -	      <0x49032000 0x7f>; /* L3 Interconnect */
> -	interrupts = <0 112 0x4>;
> -	interrupt-parent = <&gic>;
> -	ti,hwmods = "mcpdm";
> -};
> -
> -In board DTS file the pdmclk needs to be added:
> -
> -&mcpdm {
> -	clocks = <&twl6040>;
> -	clock-names = "pdmclk";
> -	status = "okay";
> -};
> diff --git a/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml b/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml
> new file mode 100644
> index 000000000..966406078
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/ti,omap4-mcpdm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OMAP McPDM
> +
> +maintainers:
> +  - Misael Lopez Cruz <misael.lopez@ti.com>
> +
> +description:
> +  OMAP ALSA SoC DAI driver using McPDM port used by TWL6040
> +
> +properties:
> +  compatible:
> +    const: ti,omap4-mcpdm
> +
> +  reg:
> +    items:
> +      - description: MPU access base address
> +      - description: L3 interconnect address
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  ti,hwmods:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [mcpdm]
> +    description: Name of the hwmod associated to the McPDM, likely "mcpdm"
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: pdmclk
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - ti,hwmods
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    pdm@40132000 {

The original label and name is preferred to be used.

> +      compatible = "ti,omap4-mcpdm";
> +      reg = <0x40132000 0x7f>, /* MPU private access */
> +            <0x49032000 0x7f>; /* L3 Interconnect */
> +      interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
> +      interrupt-parent = <&gic>;
> +      ti,hwmods = "mcpdm";
> +      clocks = <&twl6040>;
> +      clock-names = "pdmclk";

The clocks cannot be added at the time when the node is defined, it is
board specific. This way you imply that it is OK to have it in main dtsi
file. It is not.

> +    };

-- 
Péter

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Krzysztof Kozlowski]

On 22/05/2024 15:56, Péter Ujfalusi wrote:
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - ti,hwmods
>> +  - clocks
>> +  - clock-names
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    pdm@40132000 {
> 
> The original label and name is preferred to be used.

Label is not used here.

About node name, not:

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> 
>> +      compatible = "ti,omap4-mcpdm";
>> +      reg = <0x40132000 0x7f>, /* MPU private access */
>> +            <0x49032000 0x7f>; /* L3 Interconnect */
>> +      interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
>> +      interrupt-parent = <&gic>;
>> +      ti,hwmods = "mcpdm";
>> +      clocks = <&twl6040>;
>> +      clock-names = "pdmclk";
> 
> The clocks cannot be added at the time when the node is defined, it is
> board specific. This way you imply that it is OK to have it in main dtsi
> file. It is not.

Wait, what? That's example and pretty standard. Example should be
complete. This is not an exceptional binding.

Best regards,
Krzysztof

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Péter Ujfalusi]

On 22/05/2024 17:16, Krzysztof Kozlowski wrote:
> On 22/05/2024 15:56, Péter Ujfalusi wrote:
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - interrupts
>>> +  - ti,hwmods
>>> +  - clocks
>>> +  - clock-names
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    pdm@40132000 {
>>
>> The original label and name is preferred to be used.
> 
> Label is not used here.
> 
> About node name, not:
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> 
>>
>>> +      compatible = "ti,omap4-mcpdm";
>>> +      reg = <0x40132000 0x7f>, /* MPU private access */
>>> +            <0x49032000 0x7f>; /* L3 Interconnect */
>>> +      interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
>>> +      interrupt-parent = <&gic>;
>>> +      ti,hwmods = "mcpdm";
>>> +      clocks = <&twl6040>;
>>> +      clock-names = "pdmclk";
>>
>> The clocks cannot be added at the time when the node is defined, it is
>> board specific. This way you imply that it is OK to have it in main dtsi
>> file. It is not.
> 
> Wait, what? That's example and pretty standard. Example should be
> complete. This is not an exceptional binding.

The fclk for the McPDM is coming from external source, and the McPDM is
designed in pair with twl6040/6041, there were plan for other codecs to
support the McPDM protocol and in those cases the clock would come from
the connected codec.

The example (as the original binding was bit rot) is missing reg-names,
dmas and dma-names to be complete.

> 
> Best regards,
> Krzysztof
> 

-- 
Péter

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Krzysztof Kozlowski]

On 22/05/2024 16:43, Péter Ujfalusi wrote:
>>>
>>>> +      compatible = "ti,omap4-mcpdm";
>>>> +      reg = <0x40132000 0x7f>, /* MPU private access */
>>>> +            <0x49032000 0x7f>; /* L3 Interconnect */
>>>> +      interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
>>>> +      interrupt-parent = <&gic>;
>>>> +      ti,hwmods = "mcpdm";
>>>> +      clocks = <&twl6040>;
>>>> +      clock-names = "pdmclk";
>>>
>>> The clocks cannot be added at the time when the node is defined, it is
>>> board specific. This way you imply that it is OK to have it in main dtsi
>>> file. It is not.
>>
>> Wait, what? That's example and pretty standard. Example should be
>> complete. This is not an exceptional binding.
> 
> The fclk for the McPDM is coming from external source, and the McPDM is
> designed in pair with twl6040/6041, there were plan for other codecs to
> support the McPDM protocol and in those cases the clock would come from
> the connected codec.
> 
> The example (as the original binding was bit rot) is missing reg-names,
> dmas and dma-names to be complete.

None of these properties are allowed by the binding and during these
five/six revisions of the patchset no one raised missing properties.

I assume the DTS was validated with the binding. Isn't the case here?

Best regards,
Krzysztof

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Péter Ujfalusi]

On 22/05/2024 18:22, Krzysztof Kozlowski wrote:
> On 22/05/2024 16:43, Péter Ujfalusi wrote:
>>>>
>>>>> +      compatible = "ti,omap4-mcpdm";
>>>>> +      reg = <0x40132000 0x7f>, /* MPU private access */
>>>>> +            <0x49032000 0x7f>; /* L3 Interconnect */
>>>>> +      interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
>>>>> +      interrupt-parent = <&gic>;
>>>>> +      ti,hwmods = "mcpdm";
>>>>> +      clocks = <&twl6040>;
>>>>> +      clock-names = "pdmclk";
>>>>
>>>> The clocks cannot be added at the time when the node is defined, it is
>>>> board specific. This way you imply that it is OK to have it in main dtsi
>>>> file. It is not.
>>>
>>> Wait, what? That's example and pretty standard. Example should be
>>> complete. This is not an exceptional binding.
>>
>> The fclk for the McPDM is coming from external source, and the McPDM is
>> designed in pair with twl6040/6041, there were plan for other codecs to
>> support the McPDM protocol and in those cases the clock would come from
>> the connected codec.
>>
>> The example (as the original binding was bit rot) is missing reg-names,
>> dmas and dma-names to be complete.
> 
> None of these properties are allowed by the binding and during these
> five/six revisions of the patchset no one raised missing properties.

I just by accident spotted this patch, I was not in Cc.

The reg-names must be set to 'mpu' and 'dma'
The dma-names should be 'up_link' and 'dn_link'

These names go back for a long time (~2012) and have been mandatory ever
since.

Yes, the binding document was neglected pretty badly but when converting
to yaml it has to be correct since that will have ripple effect on
existing dts/dtsi files.

> I assume the DTS was validated with the binding. Isn't the case here?
> 
> Best regards,
> Krzysztof
> 

-- 
Péter

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Krzysztof Kozlowski]

On 22/05/2024 18:01, Péter Ujfalusi wrote:
> 
> 
> On 22/05/2024 18:22, Krzysztof Kozlowski wrote:
>> On 22/05/2024 16:43, Péter Ujfalusi wrote:
>>>>>
>>>>>> +      compatible = "ti,omap4-mcpdm";
>>>>>> +      reg = <0x40132000 0x7f>, /* MPU private access */
>>>>>> +            <0x49032000 0x7f>; /* L3 Interconnect */
>>>>>> +      interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
>>>>>> +      interrupt-parent = <&gic>;
>>>>>> +      ti,hwmods = "mcpdm";
>>>>>> +      clocks = <&twl6040>;
>>>>>> +      clock-names = "pdmclk";
>>>>>
>>>>> The clocks cannot be added at the time when the node is defined, it is
>>>>> board specific. This way you imply that it is OK to have it in main dtsi
>>>>> file. It is not.
>>>>
>>>> Wait, what? That's example and pretty standard. Example should be
>>>> complete. This is not an exceptional binding.
>>>
>>> The fclk for the McPDM is coming from external source, and the McPDM is
>>> designed in pair with twl6040/6041, there were plan for other codecs to
>>> support the McPDM protocol and in those cases the clock would come from
>>> the connected codec.
>>>
>>> The example (as the original binding was bit rot) is missing reg-names,
>>> dmas and dma-names to be complete.
>>
>> None of these properties are allowed by the binding and during these
>> five/six revisions of the patchset no one raised missing properties.
> 
> I just by accident spotted this patch, I was not in Cc.
> 
> The reg-names must be set to 'mpu' and 'dma'
> The dma-names should be 'up_link' and 'dn_link'
> 
> These names go back for a long time (~2012) and have been mandatory ever
> since.
> 
> Yes, the binding document was neglected pretty badly but when converting
> to yaml it has to be correct since that will have ripple effect on
> existing dts/dtsi files.

Yep. And testing DTS should clearly show that conversion leads to
incomplete binding.

> 
>> I assume the DTS was validated with the binding. Isn't the case here?

Mithil Bavishi,
Are you sure you tested the DTS?

Best regards,
Krzysztof

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Mithil]

> Yep. And testing DTS should clearly show that conversion leads to
> incomplete binding.
>
> >
> >> I assume the DTS was validated with the binding. Isn't the case here?
>
> Mithil Bavishi,
> Are you sure you tested the DTS?

dt_binding_check did not give me any errors. Yeah the example is
different from how it is implemented in the kernel ie board specific
(omap4, omap5 etc). Should the example be changed according to that
dtsi then?


-- 
Best Regards,
Mithil

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Mithil]

Something along the lines of,
mcpdm: mcpdm@0 {
    compatible = "ti,omap4-mcpdm";
    reg = <0x0 0x7f>, /* MPU private access */
            <0x49032000 0x7f>; /* L3 Interconnect */
    reg-names = "mpu", "dma";
    interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
    dmas = <&sdma 65>,
                <&sdma 66>;
     dma-names = "up_link", "dn_link";
};

Might also need to add clocks?
    clocks = <&twl6040>;
    clock-names = "pdmclk";
But those are usually defined in board specific files.

And add reg-names, dmas, dma-names information to the docs?

-- 
Best Regards,
Mithil

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Krzysztof Kozlowski]

On 22/05/2024 19:02, Mithil wrote:
>> Yep. And testing DTS should clearly show that conversion leads to
>> incomplete binding.
>>
>>>
>>>> I assume the DTS was validated with the binding. Isn't the case here?
>>
>> Mithil Bavishi,
>> Are you sure you tested the DTS?
> 
> dt_binding_check did not give me any errors. Yeah the example is
> different from how it is implemented in the kernel ie board specific
> (omap4, omap5 etc). Should the example be changed according to that
> dtsi then?

Binding needs to be adapted to match DTS or DTS has to be fixed to match
binding, depending which one is correct. Mention any changes done in the
binding which deviate from pure conversion of TXT->DT schema.

https://social.kernel.org/notice/Ai9hYRUKo8suzX3zNY

Best regards,
Krzysztof

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Mithil]

> Binding needs to be adapted to match DTS or DTS has to be fixed to match
> binding, depending which one is correct. Mention any changes done in the
> binding which deviate from pure conversion of TXT->DT schema.
The DTS is correct so will base the example on that (and get a better
changelog in the next version)
So the checks will be 1) dt_bindings_check and 2) dtbs_check

> https://social.kernel.org/notice/Ai9hYRUKo8suzX3zNY
Noted, but here I'd assume omap2plus_defconfig would be more relevant.

arch/arm/boot/dts/ti/omap/omap4-duovero-parlor.dtb: mcpdm@0:
'ti,hwmods' is a required property
        from schema $id:
http://devicetree.org/schemas/sound/ti,omap4-mcpdm.yaml#
We already have ti,hwmods still its asking for it?

arch/arm/boot/dts/ti/omap/omap4-duovero-parlor.dtb: mcpdm@0:
'dma-names', 'dmas', 'reg-names' do not match any of the regexes:
'pinctrl-[0-9]+'
        from schema $id:
http://devicetree.org/schemas/sound/ti,omap4-mcpdm.yaml#
It also requires a pinctrl subnode which isnt used anywhere, the
parent node of mcpdm that is mcpdm_module has a pinctrl how would we
go about implementing that?

-- 
Best Regards,
Mithil

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Mithil]

My apologies, misunderstood the error.
Proposed changes for the next version,
Add dma, dma-names, reg-names properties, and do the changes in
example (rename node to mcpdm since it is different from generic pdm).
  reg-names:
    items:
      - const: mpu
      - const: dma

  dmas:
    maxItems: 2

  dma-names:
    items:
      - const: up_link
      - const: dn_link

examples:
    - |
    #include <dt-bindings/interrupt-controller/arm-gic.h>
    mcpdm@0 {
      compatible = "ti,omap4-mcpdm";
      reg = <0x0 0x7f>, /* MPU private access */
            <0x49032000 0x7f>; /* L3 Interconnect */
      reg-names = "mpu", "dma";
      interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
      interrupt-parent = <&gic>;
      dmas = <&sdma 65>,
             <&sdma 66>;
      dma-names = "up_link", "dn_link";
      ti,hwmods = "mcpdm";
      clocks = <&twl6040>;
      clock-names = "pdmclk";
    };

Remove ti.hwmods from required since some dts like
omap4-duovero-parlor, omap4-panda etc do not use it which causes
dtbs_check to not pass.

-- 
Best Regards,
Mithil

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Péter Ujfalusi]

Hi,

On 22/05/2024 20:47, Mithil wrote:
> My apologies, misunderstood the error.
> Proposed changes for the next version,
> Add dma, dma-names, reg-names properties, and do the changes in
> example (rename node to mcpdm since it is different from generic pdm).
>   reg-names:
>     items:
>       - const: mpu
>       - const: dma
> 
>   dmas:
>     maxItems: 2
> 
>   dma-names:
>     items:
>       - const: up_link
>       - const: dn_link
> 
> examples:
>     - |
>     #include <dt-bindings/interrupt-controller/arm-gic.h>
>     mcpdm@0 {
>       compatible = "ti,omap4-mcpdm";
>       reg = <0x0 0x7f>, /* MPU private access */
>             <0x49032000 0x7f>; /* L3 Interconnect */
>       reg-names = "mpu", "dma";
>       interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
>       interrupt-parent = <&gic>;
>       dmas = <&sdma 65>,
>              <&sdma 66>;
These can be in one line to make it nice and tidy

>       dma-names = "up_link", "dn_link";
>       ti,hwmods = "mcpdm";

The ti,hwmods no longer needed since the sysc conversion

>       clocks = <&twl6040>;
>       clock-names = "pdmclk";
>     };
> 
> Remove ti.hwmods from required since some dts like
> omap4-duovero-parlor, omap4-panda etc do not use it which causes
> dtbs_check to not pass.
> 

-- 
Péter

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Péter Ujfalusi]

On 22/05/2024 20:07, Krzysztof Kozlowski wrote:
> On 22/05/2024 19:02, Mithil wrote:
>>> Yep. And testing DTS should clearly show that conversion leads to
>>> incomplete binding.
>>>
>>>>
>>>>> I assume the DTS was validated with the binding. Isn't the case here?
>>>
>>> Mithil Bavishi,
>>> Are you sure you tested the DTS?
>>
>> dt_binding_check did not give me any errors. Yeah the example is
>> different from how it is implemented in the kernel ie board specific
>> (omap4, omap5 etc). Should the example be changed according to that
>> dtsi then?
> 
> Binding needs to be adapted to match DTS or DTS has to be fixed to match
> binding, depending which one is correct.

Normally the DTS is written based on the binding document and the driver
is written also to follow the binding document.
However in this case we have a broken/inaccurate binding document and
the existing DTS files and binaries in wild have deviated (there are
boards out there using qnx or BSD and use this binding), or to be
precise the binding document was not updated.

The existing DTS files are the ABI, so we cannot deviate from them,
unfortunately.

In this case the DTS / driver needs to be reverse engineered to create a
binding document.

To note: I'm also guilty of not updating the .txt file.

> Mention any changes done in the
> binding which deviate from pure conversion of TXT->DT schema.
> 
> https://social.kernel.org/notice/Ai9hYRUKo8suzX3zNY
> 
> Best regards,
> Krzysztof
> 

-- 
Péter

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Krzysztof Kozlowski]

On 22/05/2024 20:39, Péter Ujfalusi wrote:
> 
> 
> On 22/05/2024 20:07, Krzysztof Kozlowski wrote:
>> On 22/05/2024 19:02, Mithil wrote:
>>>> Yep. And testing DTS should clearly show that conversion leads to
>>>> incomplete binding.
>>>>
>>>>>
>>>>>> I assume the DTS was validated with the binding. Isn't the case here?
>>>>
>>>> Mithil Bavishi,
>>>> Are you sure you tested the DTS?
>>>
>>> dt_binding_check did not give me any errors. Yeah the example is
>>> different from how it is implemented in the kernel ie board specific
>>> (omap4, omap5 etc). Should the example be changed according to that
>>> dtsi then?
>>
>> Binding needs to be adapted to match DTS or DTS has to be fixed to match
>> binding, depending which one is correct.
> 
> Normally the DTS is written based on the binding document and the driver
> is written also to follow the binding document.
> However in this case we have a broken/inaccurate binding document and
> the existing DTS files and binaries in wild have deviated (there are
> boards out there using qnx or BSD and use this binding), or to be
> precise the binding document was not updated.
> 
> The existing DTS files are the ABI, so we cannot deviate from them,
> unfortunately.
> 
> In this case the DTS / driver needs to be reverse engineered to create a
> binding document.

Ah, yes, the third option - ABI should not be broken and sometimes
binding and DTS needs fixes.

Best regards,
Krzysztof

DT schema bindings conversion mentorships (was Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema)

[Krzysztof Kozlowski]

On 22/05/2024 18:42, Krzysztof Kozlowski wrote:
>> Yes, the binding document was neglected pretty badly but when converting
>> to yaml it has to be correct since that will have ripple effect on
>> existing dts/dtsi files.
> 
> Yep. And testing DTS should clearly show that conversion leads to
> incomplete binding.
> 
>>
>>> I assume the DTS was validated with the binding. Isn't the case here?
> 
> Mithil Bavishi,
> Are you sure you tested the DTS?

Dear Daniel, Shuah, Julia, Javier and other mentorship managers,

I see some contributions regarding Devicetree bindings which look like
efforts of some mentorship programs. It's great, I really like it. Only
sadness is that no one ever asked us, Devicetree maintainers, about some
sort of guidelines. This leads to sub-optimal allocation of tasks and
quite a strain on reviewers side: for example we receive contributions
which were never tested (tested as in make target - make
dt_binding_check). Or people converted bindings which really do not
matter thus their work soon will become obsolete.

If there are still such active programs, please be sure that mentees
follow these guidelines:

https://social.kernel.org/notice/Ai9hYRUKo8suzX3zNY

1. Please convert bindings which have active DTS users. First choose
bindings with DTS built by arm64 defconfig, then next choice by arm
multi_v7 defconfig. Then any other ARM or different architecture DTS.

2. Be sure dt_bindings_check (including yamllint) and checkpatch pass
without any warnings. See writing-schema.rst document.

3. Be sure DTS using these bindings passes dtbs_check validation. If
this means binding needs to be adapted during conversion, mention
briefly in the commit message changes done comparing to pure TXT->DT
schema conversion.

Best regards,
Krzysztof

Re: DT schema bindings conversion mentorships (was Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema)

[Javier Carrasco]

On 22/05/2024 19:05, Krzysztof Kozlowski wrote:
> On 22/05/2024 18:42, Krzysztof Kozlowski wrote:
>>> Yes, the binding document was neglected pretty badly but when converting
>>> to yaml it has to be correct since that will have ripple effect on
>>> existing dts/dtsi files.
>>
>> Yep. And testing DTS should clearly show that conversion leads to
>> incomplete binding.
>>
>>>
>>>> I assume the DTS was validated with the binding. Isn't the case here?
>>
>> Mithil Bavishi,
>> Are you sure you tested the DTS?
> 
> Dear Daniel, Shuah, Julia, Javier and other mentorship managers,
> 
> I see some contributions regarding Devicetree bindings which look like
> efforts of some mentorship programs. It's great, I really like it. Only
> sadness is that no one ever asked us, Devicetree maintainers, about some
> sort of guidelines. This leads to sub-optimal allocation of tasks and
> quite a strain on reviewers side: for example we receive contributions
> which were never tested (tested as in make target - make
> dt_binding_check). Or people converted bindings which really do not
> matter thus their work soon will become obsolete.
> 
> If there are still such active programs, please be sure that mentees
> follow these guidelines:
> 
> https://social.kernel.org/notice/Ai9hYRUKo8suzX3zNY
> 
> 1. Please convert bindings which have active DTS users. First choose
> bindings with DTS built by arm64 defconfig, then next choice by arm
> multi_v7 defconfig. Then any other ARM or different architecture DTS.
> 
> 2. Be sure dt_bindings_check (including yamllint) and checkpatch pass
> without any warnings. See writing-schema.rst document.
> 
> 3. Be sure DTS using these bindings passes dtbs_check validation. If
> this means binding needs to be adapted during conversion, mention
> briefly in the commit message changes done comparing to pure TXT->DT
> schema conversion.
> 
> Best regards,
> Krzysztof
> 


Hello Krzysztof,

Several mentees from the Linux Kernel Mentorship Program have been
converting bindings within the last weeks, but it was not a programmed
task from the mentorship as such. They are free to choose the areas
where they want to contribute, and some of them chose that one.
Therefore no direct contact with the subsystem maintainers was
established. We will keep an eye on that too, so we can anticipate such
misunderstandings and additional work for the maintainers.

Nonetheless, I saw that some our mentees sent such faulty/pointless
conversions a few days ago, and they received some guidelines and links
to the official documentation yesterday. All points you mentioned were
covered, so the next patches should look better.

Usually their patches are sent to the mentors first for a preliminary
review, but sometimes that step gets "bypassed". We will insist on the
preliminary review, at least for the first conversions.

Apologies for any faulty patch they might still send directly/not taking
those points into account.

Thank you so much for your patience and feedback.

Best regards,
Javier Carrasco

Re: DT schema bindings conversion mentorships (was Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema)

[Krzysztof Kozlowski]

On 22/05/2024 19:47, Javier Carrasco wrote:
>> https://social.kernel.org/notice/Ai9hYRUKo8suzX3zNY
>>
>> 1. Please convert bindings which have active DTS users. First choose
>> bindings with DTS built by arm64 defconfig, then next choice by arm
>> multi_v7 defconfig. Then any other ARM or different architecture DTS.
>>
>> 2. Be sure dt_bindings_check (including yamllint) and checkpatch pass
>> without any warnings. See writing-schema.rst document.
>>
>> 3. Be sure DTS using these bindings passes dtbs_check validation. If
>> this means binding needs to be adapted during conversion, mention
>> briefly in the commit message changes done comparing to pure TXT->DT
>> schema conversion.
>>
>> Best regards,
>> Krzysztof
>>
> 
> 
> Hello Krzysztof,
> 
> Several mentees from the Linux Kernel Mentorship Program have been
> converting bindings within the last weeks, but it was not a programmed
> task from the mentorship as such. They are free to choose the areas
> where they want to contribute, and some of them chose that one.
> Therefore no direct contact with the subsystem maintainers was
> established. We will keep an eye on that too, so we can anticipate such
> misunderstandings and additional work for the maintainers.
> 
> Nonetheless, I saw that some our mentees sent such faulty/pointless
> conversions a few days ago, and they received some guidelines and links
> to the official documentation yesterday. All points you mentioned were
> covered, so the next patches should look better.
> 
> Usually their patches are sent to the mentors first for a preliminary
> review, but sometimes that step gets "bypassed". We will insist on the
> preliminary review, at least for the first conversions.
> 
> Apologies for any faulty patch they might still send directly/not taking
> those points into account.
> 
> Thank you so much for your patience and feedback.

Sounds good, thanks Javier!

Best regards,
Krzysztof

Re: DT schema bindings conversion mentorships (was Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema)

[Conor Dooley]

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

On Wed, May 22, 2024 at 07:05:24PM +0200, Krzysztof Kozlowski wrote:

> Dear Daniel,
> 
> I see some contributions regarding Devicetree bindings which look like
> efforts of some mentorship programs.

I noticed that the 2024 dt-bindings gsoc wiki suggests using the wrong
dual license for bindings:
https://wiki.linuxfoundation.org/gsoc/2024-gsoc-device-tree-bindings#proposal_1convert_device_tree_bindings_to_dt_schema

The correct dual license is "GPL-2.0-only OR BSD-2-Clause", not
BSD-3-Clause.

Thanks,
Conor.

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

Re: DT schema bindings conversion mentorships (was Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema)

[Daniel Baluta]

On 5/22/24 21:36, Conor Dooley wrote:
> I noticed that the 2024 dt-bindings gsoc wiki suggests using the wrong
> dual license for bindings:
> https://wiki.linuxfoundation.org/gsoc/2024-gsoc-device-tree-bindings#proposal_1convert_device_tree_bindings_to_dt_schema
>
> The correct dual license is "GPL-2.0-only OR BSD-2-Clause", not
> BSD-3-Clause.


Thanks for pointing this out, Conor! Fixed now!

Re: DT schema bindings conversion mentorships (was Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema)

[Daniel Baluta]

On 5/22/24 20:05, Krzysztof Kozlowski wrote:
> Dear Daniel, Shuah, Julia, Javier and other mentorship managers,
>
> I see some contributions regarding Devicetree bindings which look like
> efforts of some mentorship programs. It's great, I really like it. Only
> sadness is that no one ever asked us, Devicetree maintainers, about some
> sort of guidelines. This leads to sub-optimal allocation of tasks and
> quite a strain on reviewers side: for example we receive contributions
> which were never tested (tested as in make target - make
> dt_binding_check). Or people converted bindings which really do not
> matter thus their work soon will become obsolete.
>

Hi Krzysztof,

Some of the faulty patches are on me! Sorry for that. We had an 
unexpected high

number of people sending contributions for Google Summer of Code and I 
couldn't watch them all.

Now, the application period has ended and we have 1 intern working for 
the summer!

Will follow your guidance! Thanks a lot for your help!

Re: DT schema bindings conversion mentorships (was Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema)

[Rob Herring]

On Thu, May 23, 2024 at 7:30 AM Daniel Baluta <daniel.baluta@nxp.com> wrote:
>
>
> On 5/22/24 20:05, Krzysztof Kozlowski wrote:
> > Dear Daniel, Shuah, Julia, Javier and other mentorship managers,
> >
> > I see some contributions regarding Devicetree bindings which look like
> > efforts of some mentorship programs. It's great, I really like it. Only
> > sadness is that no one ever asked us, Devicetree maintainers, about some
> > sort of guidelines. This leads to sub-optimal allocation of tasks and
> > quite a strain on reviewers side: for example we receive contributions
> > which were never tested (tested as in make target - make
> > dt_binding_check). Or people converted bindings which really do not
> > matter thus their work soon will become obsolete.
> >
>
> Hi Krzysztof,
>
> Some of the faulty patches are on me! Sorry for that. We had an
> unexpected high
>
> number of people sending contributions for Google Summer of Code and I
> couldn't watch them all.
>
> Now, the application period has ended and we have 1 intern working for
> the summer!
>
> Will follow your guidance! Thanks a lot for your help!

To be specific, there are several ways to prioritize what to work on.

- There's a list maintained in CI of number of occurrences of
undocumented (by schema) compatibles[1]. Start at the top.
- Pick a platform (or family of platform) and get the warnings down to
0 or close. There's a grouping of warnings and undocumented
compatibles by platform family at the same link.
- Prioritize newer platforms over older (arm64 rather than
arm32(though there's still new arm32 stuff)).
- Fix warnings treewide from common schemas (i.e. from dtschema).
That's not conversions, but related.

Rob

[1] https://gitlab.com/robherring/linux-dt/-/jobs/6918723853

Re: DT schema bindings conversion mentorships (was Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema)

[Javier Carrasco]

On 23/05/2024 18:24, Rob Herring wrote:
> On Thu, May 23, 2024 at 7:30 AM Daniel Baluta <daniel.baluta@nxp.com> wrote:
>>
>>
>> On 5/22/24 20:05, Krzysztof Kozlowski wrote:
>>> Dear Daniel, Shuah, Julia, Javier and other mentorship managers,
>>>
>>> I see some contributions regarding Devicetree bindings which look like
>>> efforts of some mentorship programs. It's great, I really like it. Only
>>> sadness is that no one ever asked us, Devicetree maintainers, about some
>>> sort of guidelines. This leads to sub-optimal allocation of tasks and
>>> quite a strain on reviewers side: for example we receive contributions
>>> which were never tested (tested as in make target - make
>>> dt_binding_check). Or people converted bindings which really do not
>>> matter thus their work soon will become obsolete.
>>>
>>
>> Hi Krzysztof,
>>
>> Some of the faulty patches are on me! Sorry for that. We had an
>> unexpected high
>>
>> number of people sending contributions for Google Summer of Code and I
>> couldn't watch them all.
>>
>> Now, the application period has ended and we have 1 intern working for
>> the summer!
>>
>> Will follow your guidance! Thanks a lot for your help!
> 
> To be specific, there are several ways to prioritize what to work on.
> 
> - There's a list maintained in CI of number of occurrences of
> undocumented (by schema) compatibles[1]. Start at the top.
> - Pick a platform (or family of platform) and get the warnings down to
> 0 or close. There's a grouping of warnings and undocumented
> compatibles by platform family at the same link.
> - Prioritize newer platforms over older (arm64 rather than
> arm32(though there's still new arm32 stuff)).
> - Fix warnings treewide from common schemas (i.e. from dtschema).
> That's not conversions, but related.
> 
> Rob
> 
> [1] https://gitlab.com/robherring/linux-dt/-/jobs/6918723853

Thank you Rob, I forwarded your recommendations to the LFX mentees.

Best regards,
Javier Carrasco

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Rob Herring]

On Wed, May 22, 2024 at 04:56:11PM +0300, Péter Ujfalusi wrote:
> Hi,
> 
> On 22/05/2024 10:52, Mighty wrote:
> > From: Mithil Bavishi <bavishimithil@gmail.com>
> > 
> > Convert the OMAP4+ McPDM bindings to DT schema.
> > 
> > Signed-off-by: Mithil Bavishi <bavishimithil@gmail.com>
> > ---
> > Changelog v5:
> > - Add imports for constants
> > - Add desc to ti,hwmods
> > 
> >  .../devicetree/bindings/sound/omap-mcpdm.txt  | 30 ---------
> >  .../bindings/sound/ti,omap4-mcpdm.yaml        | 61 +++++++++++++++++++
> >  2 files changed, 61 insertions(+), 30 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/sound/omap-mcpdm.txt
> >  create mode 100644 Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/omap-mcpdm.txt b/Documentation/devicetree/bindings/sound/omap-mcpdm.txt
> > deleted file mode 100644
> > index ff98a0cb5..000000000
> > --- a/Documentation/devicetree/bindings/sound/omap-mcpdm.txt
> > +++ /dev/null
> > @@ -1,30 +0,0 @@
> > -* Texas Instruments OMAP4+ McPDM
> > -
> > -Required properties:
> > -- compatible: "ti,omap4-mcpdm"
> > -- reg: Register location and size as an array:
> > -       <MPU access base address, size>,
> > -       <L3 interconnect address, size>;
> > -- interrupts: Interrupt number for McPDM
> > -- ti,hwmods: Name of the hwmod associated to the McPDM
> > -- clocks:  phandle for the pdmclk provider, likely <&twl6040>
> > -- clock-names: Must be "pdmclk"
> > -
> > -Example:
> > -
> > -mcpdm: mcpdm@40132000 {
> > -	compatible = "ti,omap4-mcpdm";
> > -	reg = <0x40132000 0x7f>, /* MPU private access */
> > -	      <0x49032000 0x7f>; /* L3 Interconnect */
> > -	interrupts = <0 112 0x4>;
> > -	interrupt-parent = <&gic>;
> > -	ti,hwmods = "mcpdm";
> > -};
> > -
> > -In board DTS file the pdmclk needs to be added:
> > -
> > -&mcpdm {
> > -	clocks = <&twl6040>;
> > -	clock-names = "pdmclk";
> > -	status = "okay";
> > -};
> > diff --git a/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml b/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml
> > new file mode 100644
> > index 000000000..966406078
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml
> > @@ -0,0 +1,61 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/sound/ti,omap4-mcpdm.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: OMAP McPDM
> > +
> > +maintainers:
> > +  - Misael Lopez Cruz <misael.lopez@ti.com>
> > +
> > +description:
> > +  OMAP ALSA SoC DAI driver using McPDM port used by TWL6040
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,omap4-mcpdm
> > +
> > +  reg:
> > +    items:
> > +      - description: MPU access base address
> > +      - description: L3 interconnect address
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  ti,hwmods:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    enum: [mcpdm]
> > +    description: Name of the hwmod associated to the McPDM, likely "mcpdm"
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: pdmclk
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - ti,hwmods
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    pdm@40132000 {
> 
> The original label and name is preferred to be used.

I imagine both were review comments. I can only imagine given the poor 
changelog.

Unused labels in examples should be dropped.

Node names should be generic. Though if we haven't defined the name in 
the spec or a schema, I don't care too much what is used.

> > +      compatible = "ti,omap4-mcpdm";
> > +      reg = <0x40132000 0x7f>, /* MPU private access */
> > +            <0x49032000 0x7f>; /* L3 Interconnect */
> > +      interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
> > +      interrupt-parent = <&gic>;
> > +      ti,hwmods = "mcpdm";
> > +      clocks = <&twl6040>;
> > +      clock-names = "pdmclk";
> 
> The clocks cannot be added at the time when the node is defined, it is
> board specific. This way you imply that it is OK to have it in main dtsi
> file. It is not.

That's a .dtsi structuring decision which is irrelevant to the 
binding.

Rob

Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Péter Ujfalusi]

On 22/05/2024 17:22, Rob Herring wrote:
> On Wed, May 22, 2024 at 04:56:11PM +0300, Péter Ujfalusi wrote:
>> Hi,
>>
>> On 22/05/2024 10:52, Mighty wrote:
>>> From: Mithil Bavishi <bavishimithil@gmail.com>
>>>
>>> Convert the OMAP4+ McPDM bindings to DT schema.
>>>
>>> Signed-off-by: Mithil Bavishi <bavishimithil@gmail.com>
>>> ---
>>> Changelog v5:
>>> - Add imports for constants
>>> - Add desc to ti,hwmods
>>>
>>>  .../devicetree/bindings/sound/omap-mcpdm.txt  | 30 ---------
>>>  .../bindings/sound/ti,omap4-mcpdm.yaml        | 61 +++++++++++++++++++
>>>  2 files changed, 61 insertions(+), 30 deletions(-)
>>>  delete mode 100644 Documentation/devicetree/bindings/sound/omap-mcpdm.txt
>>>  create mode 100644 Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/omap-mcpdm.txt b/Documentation/devicetree/bindings/sound/omap-mcpdm.txt
>>> deleted file mode 100644
>>> index ff98a0cb5..000000000
>>> --- a/Documentation/devicetree/bindings/sound/omap-mcpdm.txt
>>> +++ /dev/null
>>> @@ -1,30 +0,0 @@
>>> -* Texas Instruments OMAP4+ McPDM
>>> -
>>> -Required properties:
>>> -- compatible: "ti,omap4-mcpdm"
>>> -- reg: Register location and size as an array:
>>> -       <MPU access base address, size>,
>>> -       <L3 interconnect address, size>;
>>> -- interrupts: Interrupt number for McPDM
>>> -- ti,hwmods: Name of the hwmod associated to the McPDM
>>> -- clocks:  phandle for the pdmclk provider, likely <&twl6040>
>>> -- clock-names: Must be "pdmclk"
>>> -
>>> -Example:
>>> -
>>> -mcpdm: mcpdm@40132000 {
>>> -	compatible = "ti,omap4-mcpdm";
>>> -	reg = <0x40132000 0x7f>, /* MPU private access */
>>> -	      <0x49032000 0x7f>; /* L3 Interconnect */
>>> -	interrupts = <0 112 0x4>;
>>> -	interrupt-parent = <&gic>;
>>> -	ti,hwmods = "mcpdm";
>>> -};
>>> -
>>> -In board DTS file the pdmclk needs to be added:
>>> -
>>> -&mcpdm {
>>> -	clocks = <&twl6040>;
>>> -	clock-names = "pdmclk";
>>> -	status = "okay";
>>> -};
>>> diff --git a/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml b/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml
>>> new file mode 100644
>>> index 000000000..966406078
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml
>>> @@ -0,0 +1,61 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/sound/ti,omap4-mcpdm.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: OMAP McPDM
>>> +
>>> +maintainers:
>>> +  - Misael Lopez Cruz <misael.lopez@ti.com>
>>> +
>>> +description:
>>> +  OMAP ALSA SoC DAI driver using McPDM port used by TWL6040
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: ti,omap4-mcpdm
>>> +
>>> +  reg:
>>> +    items:
>>> +      - description: MPU access base address
>>> +      - description: L3 interconnect address
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  ti,hwmods:
>>> +    $ref: /schemas/types.yaml#/definitions/string
>>> +    enum: [mcpdm]
>>> +    description: Name of the hwmod associated to the McPDM, likely "mcpdm"
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: pdmclk
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - interrupts
>>> +  - ti,hwmods
>>> +  - clocks
>>> +  - clock-names
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    pdm@40132000 {
>>
>> The original label and name is preferred to be used.
> 
> I imagine both were review comments. I can only imagine given the poor 
> changelog.
> 
> Unused labels in examples should be dropped.
> 
> Node names should be generic. Though if we haven't defined the name in 
> the spec or a schema, I don't care too much what is used.

McPDM uses it's own flavor of PDM, it is not the normal PDM as we all
know, I don't know what other generic name can be used.

> 
>>> +      compatible = "ti,omap4-mcpdm";
>>> +      reg = <0x40132000 0x7f>, /* MPU private access */
>>> +            <0x49032000 0x7f>; /* L3 Interconnect */
>>> +      interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
>>> +      interrupt-parent = <&gic>;
>>> +      ti,hwmods = "mcpdm";
>>> +      clocks = <&twl6040>;
>>> +      clock-names = "pdmclk";
>>
>> The clocks cannot be added at the time when the node is defined, it is
>> board specific. This way you imply that it is OK to have it in main dtsi
>> file. It is not.
> 
> That's a .dtsi structuring decision which is irrelevant to the 
> binding.

I see, but then the dmas/dma-names should also be in here somewhere, yes
it was not part of the original binding doc, but it is mandatory.

Looking at the code and DT files, the reg-names also mandatory.

> 
> Rob

-- 
Péter