ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

From: Mithil Bavishi <[email protected]>

Convert the OMAP4+ McPDM bindings to DT schema.

Signed-off-by Mithil Bavishi <[email protected]>

---
Changelog v5:
- Add imports for constants
- Add desc to ti,hwmods

You are not making it easier for us to review:
====
b4 diff '<[email protected]>' Grabbing thread from lore.kernel.org/all/[email protected]/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

====

Quoted 20 lines

You are not making it easier for us to review:
====
b4 diff '<[email protected]>' Grabbing thread from lore.kernel.org/all/[email protected]/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

Quoted 2 lines

====

Hey, sorry about it, its my first time with lkml, could you explain what seems to be the issue here?

From: Mithil Bavishi <[email protected]>

Convert the OMAP4+ McPDM bindings to DT schema.

Signed-off-by Mithil Bavishi <[email protected]>

---
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

Documentation/devicetree/bindings/sound/omap-mcpdm.txt deleted +0 -30

deleted file mode 100644
index ff98a0cb5..000000000
@@ -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";
-};

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +42 -0

new file mode 100644
index 000000000..966406078
@@ -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 <[email protected]>
+
+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

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +1 -0

+    };

Quoted 9 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +9 -0

+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - ti,hwmods
+  - clocks
+  - clock-names
+

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 52

Quoted 6 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +6 -0

+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    pdm@40132000 {

The original label and name is preferred to be used.

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 60

Quoted 9 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +8 -0

+      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.

Quoted 9 lines · Péter Ujfalusi

Quoted 9 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +9 -0

+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - ti,hwmods
+  - clocks
+  - clock-names
+

Quoted 8 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 52

Quoted 6 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +6 -0

+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

Quoted 10 lines

Quoted 9 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +8 -0

+      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";

Quoted 3 lines

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

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +8 -0

+      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";

Quoted 6 lines

Quoted 3 lines

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.

Quoted 22 lines · Péter Ujfalusi

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +8 -0

+      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";

Quoted 6 lines

Quoted 3 lines

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.

Quoted 2 lines

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

Quoted 23 lines · Krzysztof Kozlowski

Quoted 22 lines · Péter Ujfalusi

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +8 -0

+      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";

Quoted 6 lines

Quoted 3 lines

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 2 lines

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

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

Quoted 2 lines

Quoted 2 lines

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

Mithil Bavishi,
Are you sure you tested the DTS?

Quoted 6 lines

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

Quoted 2 lines

Quoted 2 lines

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

Quoted 2 lines

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

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.

Quoted 9 lines · Mithil

Quoted 8 lines

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

Quoted 2 lines

Quoted 2 lines

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

Mithil Bavishi,
Are you sure you tested the DTS?

Quoted 4 lines

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

Quoted 10 lines · Krzysztof Kozlowski

Quoted 9 lines · Mithil

Quoted 8 lines

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

Quoted 2 lines

Quoted 2 lines

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

Mithil Bavishi,
Are you sure you tested the DTS?

Quoted 7 lines

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 2 lines

Quoted 2 lines

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

Mithil Bavishi,
Are you sure you tested the DTS?

Quoted 4 lines · Krzysztof Kozlowski

Quoted 3 lines

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.

Quoted 8 lines

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

Quoted 2 lines

Quoted 2 lines

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

Quoted 9 lines

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.

Quoted 8 lines

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.

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.

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.

Quoted 11 lines · Krzysztof Kozlowski

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!

Quoted 20 lines · Daniel Baluta

Quoted 11 lines · Krzysztof Kozlowski

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

Quoted 7 lines

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

Hi,

Quoted 104 lines · Mighty

From: Mithil Bavishi <[email protected]>

Convert the OMAP4+ McPDM bindings to DT schema.

Signed-off-by Mithil Bavishi <[email protected]>

---
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

Documentation/devicetree/bindings/sound/omap-mcpdm.txt deleted +0 -30

deleted file mode 100644
index ff98a0cb5..000000000
@@ -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";
-};

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +46 -0

new file mode 100644
index 000000000..966406078
@@ -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 <[email protected]>
+
+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
+

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 52

Quoted 6 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +6 -0

+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    pdm@40132000 {

The original label and name is preferred to be used.

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 60

Quoted 8 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +8 -0

+      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.

Quoted 107 lines · Péter Ujfalusi

Hi,

Quoted 104 lines · Mighty

From: Mithil Bavishi <[email protected]>

Convert the OMAP4+ McPDM bindings to DT schema.

Signed-off-by Mithil Bavishi <[email protected]>

---
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

Documentation/devicetree/bindings/sound/omap-mcpdm.txt deleted +0 -30

deleted file mode 100644
index ff98a0cb5..000000000
@@ -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";
-};

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +46 -0

new file mode 100644
index 000000000..966406078
@@ -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 <[email protected]>
+
+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
+

Quoted 8 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 52

Quoted 6 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +6 -0

+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.

Quoted 10 lines

Quoted 9 lines

Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +8 -0

+      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";

Quoted 3 lines

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