[PATCH] Convert the Imagination Technologies SPDIF Input Controllerto DT schema.
Convert the Imagination Technologies SPDIF Input Controllerto DT schema.
Signed-off-by: Javier García javier.garcia.ta@udima.es --- .../bindings/sound/img,spdif-in.txt | 41 ---------- .../bindings/sound/img,spdif-in.yaml | 80 +++++++++++++++++++ 2 files changed, 80 insertions(+), 41 deletions(-) delete mode 100644 Documentation/devicetree/bindings/sound/img,spdif-in.txt create mode 100644 Documentation/devicetree/bindings/sound/img,spdif-in.yaml
diff --git a/Documentation/devicetree/bindings/sound/img,spdif-in.txt b/Documentation/devicetree/bindings/sound/img,spdif-in.txt deleted file mode 100644 index f7ea8c87bf34..000000000000 --- a/Documentation/devicetree/bindings/sound/img,spdif-in.txt +++ /dev/null @@ -1,41 +0,0 @@ -Imagination Technologies SPDIF Input Controller - -Required Properties: - - - compatible : Compatible list, must contain "img,spdif-in" - - - #sound-dai-cells : Must be equal to 0 - - - reg : Offset and length of the register set for the device - - - dmas: Contains an entry for each entry in dma-names. - - - dma-names: Must include the following entry: - "rx" - - - clocks : Contains an entry for each entry in clock-names - - - clock-names : Includes the following entries: - "sys" The system clock - -Optional Properties: - - - resets: Should contain a phandle to the spdif in reset signal, if any - - - reset-names: Should contain the reset signal name "rst", if a - reset phandle is given - - - interrupts : Contains the spdif in interrupt, if present - -Example: - -spdif_in: spdif-in@18100e00 { - compatible = "img,spdif-in"; - reg = <0x18100E00 0x100>; - interrupts = <GIC_SHARED 20 IRQ_TYPE_LEVEL_HIGH>; - dmas = <&mdc 15 0xffffffff 0>; - dma-names = "rx"; - clocks = <&cr_periph SYS_CLK_SPDIF_IN>; - clock-names = "sys"; - #sound-dai-cells = <0>; -}; diff --git a/Documentation/devicetree/bindings/sound/img,spdif-in.yaml b/Documentation/devicetree/bindings/sound/img,spdif-in.yaml new file mode 100644 index 000000000000..d201293d63c7 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/img,spdif-in.yaml @@ -0,0 +1,80 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/img,spdif-in.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Imagination Technologies SPDIF Input Controller + +maintainers: + - Rob Herring robh+dt@kernel.org + - Krzysztof Kozlowski krzysztof.kozlowski+dt@linaro.org + +properties: + compatible: + enum: + - img,spdif-in + + reg: + description: + Offset and length of the register set for the device. + maxItems: 1 + + interrupts: + maxItems: 1 + + dmas: + maxItems: 1 + + dma-names: + items: + - const: rx + + clocks: + items: + - description: The system clock + + clock-names: + items: + - const: sys + + '#sound-dai-cells': + const: 0 + + resets: + items: + - description: Should contain a phandle to the spdif in reset signal, if any + + reset-names: + items: + - const: rst + +required: + - compatible + - reg + - dmas + - dma-names + - clocks + - clock-names + - '#sound-dai-cells' + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/pistachio-clk.h> + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/interrupt-controller/mips-gic.h> + #include <dt-bindings/reset/pistachio-resets.h> + spdif_in: spdif-in@18100e00 { + compatible = "img,spdif-in"; + reg = <0x18100E00 0x100>; + interrupts = <GIC_SHARED 20 IRQ_TYPE_LEVEL_HIGH>; + dmas = <&mdc 15 0xffffffff 0>; + dma-names = "rx"; + clocks = <&cr_periph SYS_CLK_SPDIF_IN>; + clock-names = "sys"; + + #sound-dai-cells = <0>; + };
Few comments:
Subject should contain a prefix e.g:
ASoC: dt-bindings: img,spdif-in: Convert Imagination ....
Also do not add a '.' at the end of the subject line.
one more comment inline:
+examples:
- |
- #include <dt-bindings/clock/pistachio-clk.h>
- #include <dt-bindings/gpio/gpio.h>
- #include <dt-bindings/interrupt-controller/irq.h>
- #include <dt-bindings/interrupt-controller/mips-gic.h>
- #include <dt-bindings/reset/pistachio-resets.h>
- spdif_in: spdif-in@18100e00 {
Node name should be generic:
e.g spdif_in: spdif@18100....
On Tue, Feb 27, 2024 at 01:35:55PM +0100, Javier García wrote:
Convert the Imagination Technologies SPDIF Input Controllerto DT schema.
Please submit patches using subject lines reflecting the style for the subsystem, this makes it easier for people to identify relevant patches. Look at what existing commits in the area you're changing are doing and make sure your subject lines visually resemble what they're doing.
+title: Imagination Technologies SPDIF Input Controller
+maintainers:
- Rob Herring robh+dt@kernel.org
- Krzysztof Kozlowski krzysztof.kozlowski+dt@linaro.org
I'm not sure they'd agree with that...
On 27/02/2024 13:35, Javier García wrote:
Convert the Imagination Technologies SPDIF Input Controllerto DT schema.
Signed-off-by: Javier García javier.garcia.ta@udima.es
.../bindings/sound/img,spdif-in.txt | 41 ---------- .../bindings/sound/img,spdif-in.yaml | 80 +++++++++++++++++++
Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching.
Also no need for full stop in subject.
2 files changed, 80 insertions(+), 41 deletions(-) delete mode 100644 Documentation/devicetree/bindings/sound/img,spdif-in.txt create mode 100644 Documentation/devicetree/bindings/sound/img,spdif-in.yaml
diff --git a/Documentation/devicetree/bindings/sound/img,spdif-in.txt b/Documentation/devicetree/bindings/sound/img,spdif-in.txt deleted file mode 100644 index f7ea8c87bf34..000000000000 --- a/Documentation/devicetree/bindings/sound/img,spdif-in.txt +++ /dev/null @@ -1,41 +0,0 @@
...
diff --git a/Documentation/devicetree/bindings/sound/img,spdif-in.yaml b/Documentation/devicetree/bindings/sound/img,spdif-in.yaml new file mode 100644 index 000000000000..d201293d63c7 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/img,spdif-in.yaml @@ -0,0 +1,80 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/img,spdif-in.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Imagination Technologies SPDIF Input Controller
+maintainers:
- Rob Herring robh+dt@kernel.org
- Krzysztof Kozlowski krzysztof.kozlowski+dt@linaro.org
No, unfortunately this cannot be us. Choose someone responsible for hardware or Linux drivers at least.
+properties:
- compatible:
- enum:
- img,spdif-in
- reg:
- description:
Offset and length of the register set for the device.
Drop description
- maxItems: 1
- interrupts:
- maxItems: 1
- dmas:
- maxItems: 1
- dma-names:
- items:
- const: rx
- clocks:
- items:
- description: The system clock
- clock-names:
- items:
- const: sys
- '#sound-dai-cells':
- const: 0
- resets:
- items:
- description: Should contain a phandle to the spdif in reset signal, if any
Drop description, it's useless. You don't say anything valuable here. Just maxItems: 1
- reset-names:
- items:
- const: rst
+required:
- compatible
- reg
- dmas
- dma-names
- clocks
- clock-names
- '#sound-dai-cells'
+unevaluatedProperties: false
This alone does not make sense... so it is pointing you to missing $ref for dai-common.
+examples:
- |
- #include <dt-bindings/clock/pistachio-clk.h>
- #include <dt-bindings/gpio/gpio.h>
What for?
- #include <dt-bindings/interrupt-controller/irq.h>
- #include <dt-bindings/interrupt-controller/mips-gic.h>
- #include <dt-bindings/reset/pistachio-resets.h>
That's not used, so just add resets and reset-names.
- spdif_in: spdif-in@18100e00 {
compatible = "img,spdif-in";
reg = <0x18100E00 0x100>;
Lowercase hex
interrupts = <GIC_SHARED 20 IRQ_TYPE_LEVEL_HIGH>;
dmas = <&mdc 15 0xffffffff 0>;
dma-names = "rx";
clocks = <&cr_periph SYS_CLK_SPDIF_IN>;
clock-names = "sys";
#sound-dai-cells = <0>;
- };
Best regards, Krzysztof
Convert the Imagination Technologies SPDIF Input Controllerto DT schema.
Signed-off-by: Javier García javier.garcia.ta@udima.es --- .../bindings/sound/img,spdif-in.txt | 41 ---------- .../bindings/sound/img,spdif-in.yaml | 78 +++++++++++++++++++ 2 files changed, 78 insertions(+), 41 deletions(-) delete mode 100644 Documentation/devicetree/bindings/sound/img,spdif-in.txt create mode 100644 Documentation/devicetree/bindings/sound/img,spdif-in.yaml
diff --git a/Documentation/devicetree/bindings/sound/img,spdif-in.txt b/Documentation/devicetree/bindings/sound/img,spdif-in.txt deleted file mode 100644 index f7ea8c87bf34..000000000000 --- a/Documentation/devicetree/bindings/sound/img,spdif-in.txt +++ /dev/null @@ -1,41 +0,0 @@ -Imagination Technologies SPDIF Input Controller - -Required Properties: - - - compatible : Compatible list, must contain "img,spdif-in" - - - #sound-dai-cells : Must be equal to 0 - - - reg : Offset and length of the register set for the device - - - dmas: Contains an entry for each entry in dma-names. - - - dma-names: Must include the following entry: - "rx" - - - clocks : Contains an entry for each entry in clock-names - - - clock-names : Includes the following entries: - "sys" The system clock - -Optional Properties: - - - resets: Should contain a phandle to the spdif in reset signal, if any - - - reset-names: Should contain the reset signal name "rst", if a - reset phandle is given - - - interrupts : Contains the spdif in interrupt, if present - -Example: - -spdif_in: spdif-in@18100e00 { - compatible = "img,spdif-in"; - reg = <0x18100E00 0x100>; - interrupts = <GIC_SHARED 20 IRQ_TYPE_LEVEL_HIGH>; - dmas = <&mdc 15 0xffffffff 0>; - dma-names = "rx"; - clocks = <&cr_periph SYS_CLK_SPDIF_IN>; - clock-names = "sys"; - #sound-dai-cells = <0>; -}; diff --git a/Documentation/devicetree/bindings/sound/img,spdif-in.yaml b/Documentation/devicetree/bindings/sound/img,spdif-in.yaml new file mode 100644 index 000000000000..1d2e318b349f --- /dev/null +++ b/Documentation/devicetree/bindings/sound/img,spdif-in.yaml @@ -0,0 +1,78 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/img,spdif-in.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Imagination Technologies SPDIF Input Controller + +maintainers: + - Liam Girdwood lgirdwood@gmail.com + - Mark Brown broonie@kernel.org + +allOf: + - $ref: dai-common.yaml# + +properties: + compatible: + enum: + - img,spdif-in + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + dmas: + maxItems: 1 + + dma-names: + items: + - const: rx + + clocks: + items: + - description: The system clock + + clock-names: + items: + - const: sys + + '#sound-dai-cells': + const: 0 + + resets: + maxItems: 1 + + reset-names: + items: + - const: rst + +required: + - compatible + - reg + - dmas + - dma-names + - clocks + - clock-names + - '#sound-dai-cells' + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/pistachio-clk.h> + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/interrupt-controller/mips-gic.h> + spdif_in: spdif@18100e00 { + compatible = "img,spdif-in"; + reg = <0x18100e00 0x100>; + interrupts = <GIC_SHARED 20 IRQ_TYPE_LEVEL_HIGH>; + dmas = <&mdc 15 0xffffffff 0>; + dma-names = "rx"; + clocks = <&cr_periph SYS_CLK_SPDIF_IN>; + clock-names = "sys"; + + #sound-dai-cells = <0>; + };
This looks much better than v1. Make sure you have addressed all comments from the previous version and add a short log under the scissor line explaining what you have changed.
Few comments inline:
On Tue, Feb 27, 2024 at 6:13 PM Javier García javier.garcia.ta@udima.es wrote:
Convert the Imagination Technologies SPDIF Input Controllerto DT schema.
Signed-off-by: Javier García javier.garcia.ta@udima.es
^ this is the scissor line. Here you add the change log.
Changes since v1: - re-written the subject inline to include relevant prefix - removed header file as it is not used - ....etc
+$id: http://devicetree.org/schemas/sound/img,spdif-in.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Imagination Technologies SPDIF Input Controller
+maintainers:
- Liam Girdwood lgirdwood@gmail.com
- Mark Brown broonie@kernel.org
Please do not blindly add people here. The most proper candidate for this is the people who wrote the original file.
Using git log we can find Damien.Horsley Damien.Horsley@imgtec.com
Damien,
Is it OK to add you as a maintainer for this file as you wrote the original driver?
Other than this looks good to me as far as i can tell.
thanks, Daniel.
On Tue, Feb 27, 2024 at 6:27 PM Daniel Baluta daniel.baluta@gmail.com wrote:
This looks much better than v1. Make sure you have addressed all comments from the previous version and add a short log under the scissor line explaining what you have changed.
Few comments inline:
On Tue, Feb 27, 2024 at 6:13 PM Javier García javier.garcia.ta@udima.es wrote:
Convert the Imagination Technologies SPDIF Input Controllerto DT schema.
Signed-off-by: Javier García javier.garcia.ta@udima.es
^ this is the scissor line. Here you add the change log.
Changes since v1:
- re-written the subject inline to include relevant prefix
- removed header file as it is not used
- ....etc
+$id: http://devicetree.org/schemas/sound/img,spdif-in.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Imagination Technologies SPDIF Input Controller
+maintainers:
- Liam Girdwood lgirdwood@gmail.com
- Mark Brown broonie@kernel.org
Please do not blindly add people here. The most proper candidate for this is the people who wrote the original file.
Using git log we can find Damien.Horsley Damien.Horsley@imgtec.com
Looks like this address bounced back.
@Krzysztof Kozlowski @Mark Brown is it OK to add a mailing list as maintainer for a yml file?
On 27/02/2024 17:29, Daniel Baluta wrote:
On Tue, Feb 27, 2024 at 6:27 PM Daniel Baluta daniel.baluta@gmail.com wrote:
This looks much better than v1. Make sure you have addressed all comments from the previous version and add a short log under the scissor line explaining what you have changed.
Few comments inline:
On Tue, Feb 27, 2024 at 6:13 PM Javier García javier.garcia.ta@udima.es wrote:
Convert the Imagination Technologies SPDIF Input Controllerto DT schema.
Signed-off-by: Javier García javier.garcia.ta@udima.es
^ this is the scissor line. Here you add the change log.
Changes since v1:
- re-written the subject inline to include relevant prefix
- removed header file as it is not used
- ....etc
+$id: http://devicetree.org/schemas/sound/img,spdif-in.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Imagination Technologies SPDIF Input Controller
+maintainers:
- Liam Girdwood lgirdwood@gmail.com
- Mark Brown broonie@kernel.org
Please do not blindly add people here. The most proper candidate for this is the people who wrote the original file.
Using git log we can find Damien.Horsley Damien.Horsley@imgtec.com
Looks like this address bounced back.
@Krzysztof Kozlowski @Mark Brown is it OK to add a mailing list as maintainer for a yml file?
Yeah, works as a last resort, when we cannot find any responsible person.
Best regards, Krzysztof
Convert the Imagination Technologies SPDIF Input Controllerto DT schema.
Signed-off-by: Javier García javier.garcia.ta@udima.es --- Changes since v1 and v2: - re-written the subject inline to include relevant prefix
- Node name changed to spdif@18100e00 to be more generic
- Added maintainers from Documentation/devicetree/bindings/sound/ who should be added to the maintainers list?
- Drop reg description
- Drop resets description
- Added $ref for dai-common.yaml
- Removed unused dt-bindigs/gpio/gpio.h include
- Added reset and reset-names in the example
- changed example hex to be lowercase
.../bindings/sound/img,spdif-in.txt | 41 ---------- .../bindings/sound/img,spdif-in.yaml | 81 +++++++++++++++++++ 2 files changed, 81 insertions(+), 41 deletions(-) delete mode 100644 Documentation/devicetree/bindings/sound/img,spdif-in.txt create mode 100644 Documentation/devicetree/bindings/sound/img,spdif-in.yaml
diff --git a/Documentation/devicetree/bindings/sound/img,spdif-in.txt b/Documentation/devicetree/bindings/sound/img,spdif-in.txt deleted file mode 100644 index f7ea8c87bf34..000000000000 --- a/Documentation/devicetree/bindings/sound/img,spdif-in.txt +++ /dev/null @@ -1,41 +0,0 @@ -Imagination Technologies SPDIF Input Controller - -Required Properties: - - - compatible : Compatible list, must contain "img,spdif-in" - - - #sound-dai-cells : Must be equal to 0 - - - reg : Offset and length of the register set for the device - - - dmas: Contains an entry for each entry in dma-names. - - - dma-names: Must include the following entry: - "rx" - - - clocks : Contains an entry for each entry in clock-names - - - clock-names : Includes the following entries: - "sys" The system clock - -Optional Properties: - - - resets: Should contain a phandle to the spdif in reset signal, if any - - - reset-names: Should contain the reset signal name "rst", if a - reset phandle is given - - - interrupts : Contains the spdif in interrupt, if present - -Example: - -spdif_in: spdif-in@18100e00 { - compatible = "img,spdif-in"; - reg = <0x18100E00 0x100>; - interrupts = <GIC_SHARED 20 IRQ_TYPE_LEVEL_HIGH>; - dmas = <&mdc 15 0xffffffff 0>; - dma-names = "rx"; - clocks = <&cr_periph SYS_CLK_SPDIF_IN>; - clock-names = "sys"; - #sound-dai-cells = <0>; -}; diff --git a/Documentation/devicetree/bindings/sound/img,spdif-in.yaml b/Documentation/devicetree/bindings/sound/img,spdif-in.yaml new file mode 100644 index 000000000000..ab8f96cc37cd --- /dev/null +++ b/Documentation/devicetree/bindings/sound/img,spdif-in.yaml @@ -0,0 +1,81 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/img,spdif-in.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Imagination Technologies SPDIF Input Controller + +maintainers: + - Liam Girdwood lgirdwood@gmail.com + - Mark Brown broonie@kernel.org + +allOf: + - $ref: dai-common.yaml# + +properties: + compatible: + enum: + - img,spdif-in + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + dmas: + maxItems: 1 + + dma-names: + items: + - const: rx + + clocks: + items: + - description: The system clock + + clock-names: + items: + - const: sys + + '#sound-dai-cells': + const: 0 + + resets: + maxItems: 1 + + reset-names: + items: + - const: rst + +required: + - compatible + - reg + - dmas + - dma-names + - clocks + - clock-names + - '#sound-dai-cells' + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/pistachio-clk.h> + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/interrupt-controller/mips-gic.h> + #include <dt-bindings/reset/pistachio-resets.h> + spdif_in: spdif@18100e00 { + compatible = "img,spdif-in"; + reg = <0x18100e00 0x100>; + interrupts = <GIC_SHARED 20 IRQ_TYPE_LEVEL_HIGH>; + dmas = <&mdc 15 0xffffffff 0>; + dma-names = "rx"; + clocks = <&cr_periph SYS_CLK_SPDIF_IN>; + clock-names = "sys"; + resets = <&pistachio_reset PISTACHIO_RESET_SPDIF_IN>; + reset-names = "rst"; + + #sound-dai-cells = <0>; + };
On 27/02/2024 18:03, Javier García wrote:
Convert the Imagination Technologies SPDIF Input Controllerto DT schema.
Signed-off-by: Javier García javier.garcia.ta@udima.es
Do not attach (thread) your patchsets to some other threads (unrelated or older versions). This buries them deep in the mailbox and might interfere with applying entire sets.
Changes since v1 and v2:
re-written the subject inline to include relevant prefix
Node name changed to spdif@18100e00 to be more generic
...
diff --git a/Documentation/devicetree/bindings/sound/img,spdif-in.yaml b/Documentation/devicetree/bindings/sound/img,spdif-in.yaml new file mode 100644 index 000000000000..ab8f96cc37cd --- /dev/null +++ b/Documentation/devicetree/bindings/sound/img,spdif-in.yaml @@ -0,0 +1,81 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/img,spdif-in.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Imagination Technologies SPDIF Input Controller
+maintainers:
- Liam Girdwood lgirdwood@gmail.com
- Mark Brown broonie@kernel.org
That's still not good choice... that's the problem with converting bindings for very old, unmaintained hardware. No one cares. If you care about MIPS and Pistachio platform, then put your name there. If you do not care and we cannot find anyone responsible, then your effort has little impact. Better focus on bindings for actively used hardware.
Reviewed-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org
Best regards, Krzysztof
participants (4)
-
Daniel Baluta
-
Javier García
-
Krzysztof Kozlowski
-
Mark Brown