ASoC: dt-bindings: omap-mcpdm: Convert to DT schema
Subsystems (3)
PATCH v4 1/1 ASoC: dt-bindings: omap-mcpdm: Convert to DT schema
From: Mithil Bavishi <[email protected]>
Convert the OMAP4+ McPDM bindings to DT schema.
--- .../devicetree/bindings/sound/omap-mcpdm.txt | 30 ---------- .../bindings/sound/ti,omap4-mcpdm.yaml | 58 +++++++++++++++++++ 2 files changed, 58 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
@@ -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 +58 -0
58 lines not on this page · raw message
Signature · 1 line
2.34.1
Quoted 6 lines · Mighty
From: Mithil Bavishi <[email protected]>
Convert the OMAP4+ McPDM bindings to DT schema.
Signed-off-by Mithil Bavishi <[email protected]>---
Please provide the changelog under ---.
Quoted 48 lines
.../devicetree/bindings/sound/omap-mcpdm.txt | 30 ---------- .../bindings/sound/ti,omap4-mcpdm.yaml | 58 +++++++++++++++++++ 2 files changed, 58 insertions(+), 30 deletions(-) delete mode 100644 Documentation/devicetree/bindings/sound/omap-mcpdm.txt create mode 100644 Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yamlDocumentation/devicetree/bindings/sound/omap-mcpdm.txt deleted +0
-30@@ -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 +0
-0
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 10
Quoted 10 lines
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +10
-0@@ -0,0 +1,58 @@ +# 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: + - Krzysztof Kozlowski <[email protected]>
How did this happen? I see this was in v1, but I am quite surprised to be listed here. I am for sure not a maintainer of this binding. Choose driver maintainers or platform maintainers, worse case.
Quoted 8 lines
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +8
-0+ +description: + OMAP ALSA SoC DAI driver using McPDM port used by TWL6040 + +properties: + compatible: + const: ti,omap4-mcpdm +
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 28
Quoted 10 lines
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +10
-0+ reg: + items: + - description: MPU access base address + - description: L3 interconnect address + + interrupts: + maxItems: 1 + + ti,hwmods: + description: Name of the hwmod associated to the McPDM, likely "mcpdm"
Not much improved here. You miss $ref and optionally constraints.
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 31
Quoted 3 lines
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +3
-0+ + clocks: + description: phandle for the pdmclk provider, likely <&twl6040>
Missing constraints, so replace it with maxItems: 1
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 34
Quoted 3 lines
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +3
-0+ + clock-names: + description: Must be "pdmclk"
List the items. I asked to open existing bindings and take a look how it is there. Existing bindings would show you how we code this part.
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 36
Quoted 2 lines
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +2
-0+ +
Just one blank line.
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 49
Quoted 13 lines
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +13
-0+required: + - compatible + - reg + - interrupts + - ti,hwmods + - clocks + - clock-names + +additionalProperties: false + +examples: + - | + pdm@0 {
That's wrong address. Old code does not have 0. Please do no change parts of code without reason. If there is a reason, explain it in the changelog.
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml line 53
Quoted 4 lines
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +4
-0+ compatible = "ti,omap4-mcpdm"; + reg = <0x40132000 0x7f>, /* MPU private access */ + <0x49032000 0x7f>; /* L3 Interconnect */ + interrupts = <0 112 0x4>;
Include header and use common defines for flags. Just like all other recent bindings.
Best regards,
Krzysztof
Quoted 3 lines · Krzysztof Kozlowski
How did this happen? I see this was in v1, but I am quite surprised to be listed here. I am for sure not a maintainer of this binding. Choose driver maintainers or platform maintainers, worse case.
I might have overlooked this, will fix it. There is no driver maintainer for it as far as I know.
Should I include the module author?
Quoted 1 line
Not much improved here. You miss $ref and optionally constraints.
Something like this
$ref: /schemas/types.yaml#/definitions/string
enum: [mcpdm]
Didnt really understand the "optionally constraints" part.
Quoted 1 line
Missing constraints, so replace it with maxItems: 1
Similar to how clock-names are handled?
Quoted 2 lines
List the items. I asked to open existing bindings and take a look how it is there. Existing bindings would show you how we code this part.
clock-names:
items:
- const: pdmclk
minItems: 1
maxItems: 1
Something like this?
Quoted 1 line
Just one blank line.
Removed.
Quoted 4 lines
That's wrong address. Old code does not have 0. Please do no change parts of code without reason. If there is a reason, explain it in the changelog.
The checks were giving a warning if 0 was not included hence, I'll put the real address if needed then.
Quoted 3 lines
Include header and use common defines for flags. Just like all other recent bindings.
There's no defines for them, this is how it is in the dts :(
Best regards,
Mithil
Quoted 9 lines · Mithil
Quoted 3 lines · Krzysztof Kozlowski
How did this happen? I see this was in v1, but I am quite surprised to be listed here. I am for sure not a maintainer of this binding. Choose driver maintainers or platform maintainers, worse case.
I might have overlooked this, will fix it. There is no driver maintainer for it as far as I know.
Should I include the module author?
Or platform maintainers or whoever is interested in this hardware.
Quoted 6 lines
Quoted 2 lines
Not much improved here. You miss $ref and optionally constraints.
Something like this
$ref: /schemas/types.yaml#/definitions/string
enum: [mcpdm]
Didnt really understand the "optionally constraints" part.
Sorry, you stripped out *entire* context. No clue what you refer to.
Quoted 4 lines
Quoted 2 lines
Missing constraints, so replace it with maxItems: 1
Similar to how clock-names are handled?
Quoted 8 lines
Quoted 2 lines
List the items. I asked to open existing bindings and take a look how it is there. Existing bindings would show you how we code this part.
clock-names:
items:
- const: pdmclk
minItems: 1
maxItems: 1
Something like this?
No. Do you see code like this anywhere? Please only list the items, although without context impossible to judge.
Quoted 8 lines
Quoted 2 lines
Just one blank line.
Removed.
Quoted 4 lines
That's wrong address. Old code does not have 0. Please do no change parts of code without reason. If there is a reason, explain it in the changelog.
Quoted 7 lines
The checks were giving a warning if 0 was not included hence, I'll put the real address if needed then.
Quoted 3 lines
Include header and use common defines for flags. Just like all other recent bindings.
There's no defines for them, this is how it is in the dts :(
It does not matter whether some particular DTS uses values or defines, if these are the well known constants. Again, stripping entire context and replying after 2-3 weeks does not help me to understand this at all. Between these 2-3 weeks I got another 200 patches to review.
Best regards,
Krzysztof
Quoted 1 line
Or platform maintainers or whoever is interested in this hardware.
Aight will do it in the next patch.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Not much improved here. You miss $ref and optionally constraints.
Something like this
$ref: /schemas/types.yaml#/definitions/string
enum: [mcpdm]
Didnt really understand the "optionally constraints" part.Sorry, you stripped out *entire* context. No clue what you refer to.
Its regarding the ti,hwmods prop
ti,hwmods:
description: Name of the hwmod associated to the McPDM, likely "mcpdm"
Quoted 12 lines
Quoted 11 lines
Quoted 1 line
Missing constraints, so replace it with maxItems: 1
Similar to how clock-names are handled?
Quoted 2 lines
List the items. I asked to open existing bindings and take a look how it is there. Existing bindings would show you how we code this part.
clock-names:
items:
- const: pdmclk
minItems: 1
maxItems: 1
Something like this?
Quoted 3 lines
No. Do you see code like this anywhere? Please only list the items, although without context impossible to judge.
Quick search on sources gave meDocumentation/devicetree/bindings/usb/dwc2.yaml
which I used as reference for this prop
clock-names:
description: Must be "pdmclk"
Quoted 16 lines
Quoted 8 lines
Quoted 2 lines
Just one blank line.
Removed.
Quoted 4 lines
That's wrong address. Old code does not have 0. Please do no change parts of code without reason. If there is a reason, explain it in the changelog.
Quoted 7 lines
The checks were giving a warning if 0 was not included hence, I'll put the real address if needed then.
Quoted 3 lines
Include header and use common defines for flags. Just like all other recent bindings.
There's no defines for them, this is how it is in the dts :(
Quoted 7 lines
It does not matter whether some particular DTS uses values or defines, if these are the well known constants. Again, stripping entire context and replying after 2-3 weeks does not help me to understand this at all. Between these 2-3 weeks I got another 200 patches to review.
Best regards,
Krzysztof
compatible = "ti,omap4-mcpdm";
reg = <0x40132000 0x7f>, /* MPU private access */
<0x49032000 0x7f>; /* L3 Interconnect */
interrupts = <0 112 0x4>;
Not really constants as they do change with platforms (omap4 vs 5 for example) but
So do i just make up the constants for it then? Those just seem like magic numbers.
Regards,
Mithil
Quoted 12 lines · Mithil
Quoted 12 lines
Quoted 11 lines
Quoted 1 line
Missing constraints, so replace it with maxItems: 1
Similar to how clock-names are handled?
Quoted 2 lines
List the items. I asked to open existing bindings and take a look how it is there. Existing bindings would show you how we code this part.
clock-names:
items:
- const: pdmclk
minItems: 1
maxItems: 1
Something like this?
Quoted 5 lines
Quoted 3 lines
No. Do you see code like this anywhere? Please only list the items, although without context impossible to judge.
Quick search on sources gave me
Documentation/devicetree/bindings/usb/dwc2.yaml
Above code is different - it does not have maxItems, which you want to add.
Quoted 3 lines
which I used as reference for this prop
clock-names:
description: Must be "pdmclk"
Again, no, please list the items.
items:
- const: foo
Quoted 7 lines
compatible = "ti,omap4-mcpdm";
reg = <0x401320000x7f>, /* MPU private access */<0x49032000 0x7f>; /* L3 Interconnect */interrupts = <0 112
0x4>;
Not really constants as they do change with platforms (omap4 vs 5 for example) but
That is not really relevant... This is not pi or other math constant.
Quoted 2 lines
So do i just make up the constants for it then? Those just seem like magic numbers.
Hm? Did you look around for other SoC nodes? 0 looks like SPI, 4 like LEVEL_HIGH, but it depends on number of meaning of the interrupt cells, so who is parent interrupt controller.
To work with DT bindings it is necessary to have minimum basic knowledge about DTS. Maybe it would be good to start with some guides/tutorials about DTS. elinux has quite some tutorial and also resources pointing to various conference talks.
Best regards,
Krzysztof
Quoted 22 lines · Krzysztof Kozlowski
Quoted 12 lines · Mithil
Quoted 12 lines
Quoted 11 lines
Quoted 1 line
Missing constraints, so replace it with maxItems: 1
Similar to how clock-names are handled?
Quoted 2 lines
List the items. I asked to open existing bindings and take a look how it is there. Existing bindings would show you how we code this part.
clock-names:
items:
- const: pdmclk
minItems: 1
maxItems: 1
Something like this?Quoted 5 lines
Quoted 3 lines
No. Do you see code like this anywhere? Please only list the items, although without context impossible to judge.
Quick search on sources gave me
Documentation/devicetree/bindings/usb/dwc2.yamlAbove code is different - it does not have maxItems, which you want to add.
Quoted 7 lines
Quoted 3 lines
which I used as reference for this prop
clock-names:
description: Must be "pdmclk"Again, no, please list the items.
items:
- const: foo
Yep that was the old code for reference. Just items (no maxItems/minItems as well)
Quoted 7 lines
Quoted 6 lines
compatible = "ti,omap4-mcpdm";
reg = <0x401320000x7f>, /* MPU private access */<0x49032000 0x7f>; /* L3 Interconnect */interrupts = <0 112
0x4>;
Not really constants as they do change with platforms (omap4 vs 5 for example) but
Quoted 9 lines
That is not really relevant... This is not pi or other math constant.
Quoted 2 lines
So do i just make up the constants for it then? Those just seem like magic numbers.
Hm? Did you look around for other SoC nodes? 0 looks like SPI, 4 like
LEVEL_HIGH, but it depends on number of meaning of the interrupt cells, so who is parent interrupt controller.
Ah the irq values, correct, I thought you meant the reg values. It should be <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>; not sure about 112 though.
I suppose these changes are enough right?
Best regards,
Mithil
Hey, before making a new patch I'll just list the changes that need to be done according to this discussion.
Change maintainer
Change clocks to only include maxItems: 1
Change clock-names to include
items:
- const: pdmclk
Use correct address in example
Use flags for interrupt in example
Best regards,
Mithil
Quoted 57 lines · Mithil
Quoted 22 lines · Krzysztof Kozlowski
Quoted 12 lines · Mithil
Quoted 12 lines
Quoted 11 lines
Quoted 1 line
Missing constraints, so replace it with maxItems: 1
Similar to how clock-names are handled?
Quoted 2 lines
List the items. I asked to open existing bindings and take a look how it is there. Existing bindings would show you how we code this part.
clock-names:
items:
- const: pdmclk
minItems: 1
maxItems: 1
Something like this?Quoted 5 lines
Quoted 3 lines
No. Do you see code like this anywhere? Please only list the items, although without context impossible to judge.
Quick search on sources gave me
Documentation/devicetree/bindings/usb/dwc2.yamlAbove code is different - it does not have maxItems, which you want to add.
Quoted 7 lines
Quoted 3 lines
which I used as reference for this prop
clock-names:
description: Must be "pdmclk"Again, no, please list the items.
items:
- const: fooYep that was the old code for reference. Just items (no maxItems/minItems as well)
Quoted 7 lines
Quoted 6 lines
compatible = "ti,omap4-mcpdm";
reg = <0x401320000x7f>, /* MPU private access */<0x49032000 0x7f>; /* L3 Interconnect */interrupts = <0 112
0x4>;
Not really constants as they do change with platforms (omap4 vs 5 for example) butQuoted 9 lines
That is not really relevant... This is not pi or other math constant.
Quoted 2 lines
So do i just make up the constants for it then? Those just seem like magic numbers.
Hm? Did you look around for other SoC nodes? 0 looks like SPI, 4 like
LEVEL_HIGH, but it depends on number of meaning of the interrupt cells, so who is parent interrupt controller.Ah the irq values, correct, I thought you meant the reg values. It should be <
GIC_SPI112IRQ_TYPE_LEVEL_HIGH>; not sure about 112 though.I suppose these changes are enough right?
Best regards,
Mithil
Quoted 8 lines · Mithil
Hey, before making a new patch I'll just list the changes that need to be done according to this discussion.
Change maintainer
Change clocks to only include maxItems: 1
Change clock-names to include
items:
- const: pdmclk
Use correct address in example
If you meant unit address, then yes.
Quoted 1 line
Use flags for interrupt in example
Everything else yes.
Best regards,
Krzysztof