dt-bindings: omap-mcpdm: Convert to DT schema
Subsystems (2)
PATCH v2 1/1 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,omap-mcpdm.yaml | 59 +++++++++++++++++++ 2 files changed, 59 insertions(+), 30 deletions(-) delete mode 100644 Documentation/devicetree/bindings/sound/omap-mcpdm.txt create mode 100644 Documentation/devicetree/bindings/sound/ti,omap-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,omap-mcpdm.yaml new file +59 -0
59 lines not on this page · raw message
Signature · 1 line
2.34.1
Quoted 5 lines · Mighty
From: Mithil Bavishi <[email protected]>
Convert the OMAP4+ McPDM bindings to DT schema.
Signed-off-by Mighty <[email protected]>
This does not match SoB. Can you respond to comments you receive?
Subject: nothing improved.
Rest... also did not improve. so you ignored entire feedback?
This is a friendly reminder during the review process.
It seems my or other reviewer's previous comments were not fully addressed. Maybe the feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them.
Thank you.
Best regards,
Krzysztof
So sorry about the 2nd patch being sent as a new mail, here is a new patch with the changes as suggested
Quoted 1 line
Please use subject prefixes matching the subsystem
Changed the patch name to match the folder history.
Quoted 1 line
Is it your full name?
Fixed it, my apologies.
Quoted 1 line
Filename like compatible.
Fixed.
Quoted 1 line
Please open existing bindings and look how it is done there.
Changed it, is it fine now?
Quoted 1 line
Same problem. Drop useless description but provide maxItems.
Removed descriptions for interrupts and hwmods.
Quoted 2 lines
It does not look like you tested the bindings, at least after quick look. Please run
make dt_binding_check
I did run it and it didnt produce any errors henceforth i submitted the patch.
Quoted 1 line
Node names should be generic
Changed as said.
From c24a42724e870822d50ac6857ba9f32d0dce02ae Mon Sep 17 00:00:00 2001
From: Mithil Bavishi <[email protected]>
Date: Mon, 1 Apr 2024 21:10:15 +0530
Subject: [PATCH v2] dt-bindings: omap-mcpdm: Convert to DT schema
Convert the OMAP4+ McPDM bindings to DT schema.
--- .../devicetree/bindings/sound/omap-mcpdm.txt | 30 ---------- .../bindings/sound/ti,omap-mcpdm.yaml | 59 +++++++++++++++++++ 2 files changed, 59 insertions(+), 30 deletions(-) delete mode 100644 Documentation/devicetree/bindings/sound/omap-mcpdm.txt create mode 100644 Documentation/devicetree/bindings/sound/ti,omap-mcpdm.yaml
(quoted diff) +0 -0
b/Documentation/devicetree/bindings/sound/omap-mcpdm.txt deleted file mode 100644
index ff98a0cb5..000000000
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"; -};
(quoted diff) +0 -0
b/Documentation/devicetree/bindings/sound/ti,omap-mcpdm.yaml new file mode 100644
index 000000000..4d5d37e98
Documentation/devicetree/bindings/sound/ti,omap-mcpdm.yaml new file +59 -0
59 lines not on this page · raw message
--
2.34.1
Best regards,
Mithil
Quoted 26 lines · Krzysztof Kozlowski
Quoted 5 lines · Mighty
From: Mithil Bavishi <[email protected]>
Convert the OMAP4+ McPDM bindings to DT schema.
Signed-off-by Mighty <[email protected]>This does not match SoB. Can you respond to comments you receive?
Subject: nothing improved.
Rest... also did not improve. so you ignored entire feedback?
This is a friendly reminder during the review process.
It seems my or other reviewer's previous comments were not fully addressed. Maybe the feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them.
Thank you.
Best regards,
Krzysztof
Quoted 5 lines · Mithil
So sorry about the 2nd patch being sent as a new mail, here is a new patch with the changes as suggested
Quoted 1 line
Please use subject prefixes matching the subsystem
Changed the patch name to match the folder history.
Nothing improved. What the history tells you?
Quoted 6 lines
Quoted 2 lines
Is it your full name?
Fixed it, my apologies.
Quoted 1 line
Filename like compatible.
Fixed.
Still not, compatible is omap4.
Quoted 3 lines
Quoted 2 lines
Please open existing bindings and look how it is done there.
Changed it, is it fine now?
You mean v2? I have no clue to what you are responding here, but no, v2 did not improve much.
Best regards,
Krzysztof
Quoted 10 lines · Krzysztof Kozlowski
Quoted 5 lines · Mithil
So sorry about the 2nd patch being sent as a new mail, here is a new patch with the changes as suggested
Quoted 1 line
Please use subject prefixes matching the subsystem
Changed the patch name to match the folder history.
Nothing improved. What the history tells you?
Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema" Not really sure what else I should change.
Quoted 9 lines
Quoted 6 lines
Quoted 2 lines
Is it your full name?
Fixed it, my apologies.
Quoted 1 line
Filename like compatible.
Fixed.
Still not, compatible is omap4.
Sorry, seems like I was sending the old file again. Will fix this.
Quoted 7 lines
Quoted 3 lines
Quoted 2 lines
Please open existing bindings and look how it is done there.
Changed it, is it fine now?
You mean v2? I have no clue to what you are responding here, but no, v2 did not improve much.
Again, could you guide me to what needs to be done? Description for reg should be fine as this is how it is done in other files as well.
Interrupts and hwmods use maxItems now.
Changed nodename to be generic in example as well.
Those were the suggested changes previously.
Best regards,
Mithil
Quoted 7 lines · Mithil
Quoted 5 lines · Krzysztof Kozlowski
Quoted 3 lines · Mithil
So sorry about the 2nd patch being sent as a new mail, here is a new patch with the changes as suggested
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Quoted 1 line
Please use subject prefixes matching the subsystem
Changed the patch name to match the folder history.
Nothing improved. What the history tells you?
Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema" Not really sure what else I should change.
But the subject you wrote here is "dt-bindings: omap-mcpdm: Convert to DT schema"?
Where is the ASoC?
Quoted 18 lines
Quoted 10 lines
Quoted 7 lines
Quoted 3 lines
Is it your full name?
Fixed it, my apologies.
Quoted 1 line
Filename like compatible.
Fixed.
Still not, compatible is omap4.
Sorry, seems like I was sending the old file again. Will fix this.
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
Please open existing bindings and look how it is done there.
Changed it, is it fine now?
Quoted 7 lines
Quoted 3 lines
You mean v2? I have no clue to what you are responding here, but no, v2 did not improve much.
Again, could you guide me to what needs to be done? Description for reg should be fine as this is how it is done in other files as well.
reg is not correct. Please point me to files doing that way, so I can fix them.
You need items with description.
Quoted 1 line
Interrupts and hwmods use maxItems now.
hwmods lost description, why?
Quoted 1 line
Changed nodename to be generic in example as well.
"mcpdm" does not feel generic. What is mcpdm? Google finds nothing. Maybe just "pdm"?
Anyway, this patch has exactly the same name as v1, so what did you improve? Your v2 is almost the same as v1.
Quoted 1 line
Those were the suggested changes previously.
Best regards,
Krzysztof
Quoted 15 lines · Krzysztof Kozlowski
Quoted 13 lines · Mithil
Quoted 10 lines · Krzysztof Kozlowski
Quoted 5 lines · Mithil
So sorry about the 2nd patch being sent as a new mail, here is a new patch with the changes as suggested
Quoted 1 line
Please use subject prefixes matching the subsystem
Changed the patch name to match the folder history.
Nothing improved. What the history tells you?
Quoted 8 lines
Quoted 2 lines
Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema" Not really sure what else I should change.
But the subject you wrote here is "dt-bindings: omap-mcpdm: Convert to DT schema"?
Where is the ASoC?
I did change it, will send the patch again.
Quoted 6 lines
reg is not correct. Please point me to files doing that way, so I can fix them.
You need items with description.
Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
I referred here for the description, but will add items for the 2 regs
Quoted 3 lines
Quoted 1 line
Interrupts and hwmods use maxItems now.
hwmods lost description, why?
Seems self explanatory.
Quoted 5 lines
Quoted 1 line
Changed nodename to be generic in example as well.
"mcpdm" does not feel generic. What is mcpdm? Google finds nothing. Maybe just "pdm"?
Multichannel PDM Controller. Kept it like that since the node is also called as mcpdm in the devicetree. Calling it pdm might cause confusion.
Best Regards,
Mithil
Quoted 28 lines · Mithil
Quoted 15 lines · Krzysztof Kozlowski
Quoted 13 lines · Mithil
Quoted 10 lines · Krzysztof Kozlowski
Quoted 5 lines · Mithil
So sorry about the 2nd patch being sent as a new mail, here is a new patch with the changes as suggested
Quoted 1 line
Please use subject prefixes matching the subsystem
Changed the patch name to match the folder history.
Nothing improved. What the history tells you?
Quoted 8 lines
Quoted 2 lines
Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema" Not really sure what else I should change.
But the subject you wrote here is "dt-bindings: omap-mcpdm: Convert to DT schema"?
Where is the ASoC?
I did change it, will send the patch again.
Quoted 1 line
Quoted 7 lines
Quoted 5 lines
reg is not correct. Please point me to files doing that way, so I can fix them.
You need items with description.
Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
I referred here for the description, but will add items for the 2 regs
I don't see at all the code you are using. It's entirely different! Where in this file is that type of "reg" property?
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
Interrupts and hwmods use maxItems now.
hwmods lost description, why?
Seems self explanatory.
Really? Not to me. I have no clue what this is. Also, you need description for (almost) every non-standard, vendor property.
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
Changed nodename to be generic in example as well.
"mcpdm" does not feel generic. What is mcpdm? Google finds nothing. Maybe just "pdm"?
Multichannel PDM Controller. Kept it like that since the node is also
You said you "changed nodename". So from what did you change to what?
Quoted 1 line
called as mcpdm in the devicetree. Calling it pdm might cause
Poor DTS is not the example...
Quoted 1 line
confusion.
So far I am confused. Often name of SoC block is specific, not generic. Anyway, that's not important part, so if you claim mcpdm is generic name of a class of devices, I am fine.
Best regards,
Krzysztof
Quoted 33 lines · Krzysztof Kozlowski
Quoted 31 lines · Mithil
Quoted 15 lines · Krzysztof Kozlowski
Quoted 13 lines · Mithil
Quoted 10 lines · Krzysztof Kozlowski
Quoted 5 lines · Mithil
So sorry about the 2nd patch being sent as a new mail, here is a new patch with the changes as suggested
Quoted 1 line
Please use subject prefixes matching the subsystem
Changed the patch name to match the folder history.
Nothing improved. What the history tells you?
Quoted 8 lines
Quoted 2 lines
Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema" Not really sure what else I should change.
But the subject you wrote here is "dt-bindings: omap-mcpdm: Convert to DT schema"?
Where is the ASoC?
I did change it, will send the patch again.
Quoted 4 lines
reg is not correct. Please point me to files doing that way, so I can fix them.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
You need items with description.
Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
I referred here for the description, but will add items for the 2 regsI don't see at all the code you are using. It's entirely different! Where in this file is that type of "reg" property?
Changed it to use items and description. Was not aware about this format apologies.
Quoted 9 lines
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
Interrupts and hwmods use maxItems now.
hwmods lost description, why?
Seems self explanatory.
Really? Not to me. I have no clue what this is. Also, you need description for (almost) every non-standard, vendor property.
Re-added it as it was previously.
Quoted 12 lines
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
Changed nodename to be generic in example as well.
"mcpdm" does not feel generic. What is mcpdm? Google finds nothing. Maybe just "pdm"?
Multichannel PDM Controller. Kept it like that since the node is also
You said you "changed nodename". So from what did you change to what?
Quoted 1 line
called as mcpdm in the devicetree. Calling it pdm might cause
Quoted 8 lines
Poor DTS is not the example...
Quoted 1 line
confusion.
So far I am confused. Often name of SoC block is specific, not generic. Anyway, that's not important part, so if you claim mcpdm is generic name of a class of devices, I am fine.
Changed to pdm.
Here's the patch,
From 9fb94e551da1ff06d489f60d52335001a9de9976 Mon Sep 17 00:00:00 2001
From: Mithil Bavishi <[email protected]>
Date: Mon, 1 Apr 2024 21:10:15 +0530
Subject: [PATCH] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema
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
(quoted diff) +0 -0
b/Documentation/devicetree/bindings/sound/omap-mcpdm.txt deleted file mode 100644
index ff98a0cb5..000000000
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"; -};
(quoted diff) +0 -0
b/Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file mode 100644
index 000000000..73fcfaf6e
Documentation/devicetree/bindings/sound/ti,omap4-mcpdm.yaml new file +58 -0
58 lines not on this page · raw message
--
2.34.1
Best regards,
Mithil
Hello,
Is the patch better now? I'd still like to learn from where i did the mistakes (which seem very silly now)
Best Regards,
Mithil
Quoted 3 lines · Mithil
Hello,
Is the patch better now? I'd still like to learn from where i did the mistakes (which seem very silly now)
I do not recall any new patch from you. Please follow submitting patches document. Each patch is a new submission (new, not reply to something), with versioning, with changelog under ---. For example generated with git format-patch -v3 -1.
Best regards,
Krzysztof