diff mbox series

Convert the Imagination Technologies SPDIF Input Controllerto DT schema.

Message ID 20240227123602.258190-1-javier.garcia.ta@udima.es
State Changes Requested
Headers show
Series Convert the Imagination Technologies SPDIF Input Controllerto DT schema. | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 2 warnings, 80 lines checked
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Javier García Feb. 27, 2024, 12:35 p.m. UTC 
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

Comments

Daniel Baluta Feb. 27, 2024, 12:54 p.m. UTC | #1 
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....
Mark Brown Feb. 27, 2024, 12:56 p.m. UTC | #2 
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...
Krzysztof Kozlowski Feb. 27, 2024, 1:28 p.m. UTC | #3 
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
diff mbox series

Patch

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