[1/2] dt-bindings: dma: Ingenic: DT bindings for Ingenic PDMA

Message ID 20240228012420.4223-1-bin.yao@ingenic.com
State New
Headers
Series [1/2] dt-bindings: dma: Ingenic: DT bindings for Ingenic PDMA |

Commit Message

bin.yao Feb. 28, 2024, 1:24 a.m. UTC
  From: byao <bin.yao@ingenic.com>

Convert the textual documentation for the Ingenic SoCs PDMA
Controller devicetree binding to YAML.

Add a dt-bindings header, and convert the device trees to it.

Signed-off-by: byao <bin.yao@ingenic.com>
Signed-off-by: rick <rick.tyliu@ingenic.com>
---
 .../devicetree/bindings/dma/ingenic,pdma.yaml | 77 +++++++++++++++++++
 include/dt-bindings/dma/ingenic-pdma.h        | 51 ++++++++++++
 2 files changed, 128 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/ingenic,pdma.yaml
 create mode 100644 include/dt-bindings/dma/ingenic-pdma.h
  

Comments

Rob Herring Feb. 28, 2024, 2:20 a.m. UTC | #1
On Wed, 28 Feb 2024 06:24:19 +0500, bin.yao wrote:
> From: byao <bin.yao@ingenic.com>
> 
> Convert the textual documentation for the Ingenic SoCs PDMA
> Controller devicetree binding to YAML.
> 
> Add a dt-bindings header, and convert the device trees to it.
> 
> Signed-off-by: byao <bin.yao@ingenic.com>
> Signed-off-by: rick <rick.tyliu@ingenic.com>
> ---
>  .../devicetree/bindings/dma/ingenic,pdma.yaml | 77 +++++++++++++++++++
>  include/dt-bindings/dma/ingenic-pdma.h        | 51 ++++++++++++
>  2 files changed, 128 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/ingenic,pdma.yaml
>  create mode 100644 include/dt-bindings/dma/ingenic-pdma.h
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
/Documentation/devicetree/bindings/dma/ingenic,pdma.yaml:13:11: [error] string value is redundantly quoted with any quotes (quoted-strings)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dma/ingenic,pdma.yaml: title: 'Ingenic SoCs DMA Controller DT bindings' should not be valid under {'pattern': '([Bb]inding| [Ss]chema)'}
	hint: Everything is a binding/schema, no need to say it. Describe what hardware the binding is for.
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dma/ingenic,pdma.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename
 	 $id: http://devicetree.org/schemas/dma/ingenic,dma.yaml
 	file: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dma/ingenic,pdma.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dma/ingenic,dma.yaml: warning: ignoring duplicate '$id' value 'http://devicetree.org/schemas/dma/ingenic,dma.yaml#'
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dma/ingenic,pdma.yaml: interrupts-parent: missing type definition
Documentation/devicetree/bindings/dma/ingenic,dma.example.dtb: /example-0/dma-controller@13420000: failed to match any schema with compatible: ['ingenic,jz4780-dma']
Documentation/devicetree/bindings/dma/ingenic,pdma.example.dts:24:18: fatal error: dt-bindings/clock/ingenic,t33-cgu.h: No such file or directory
   24 |         #include <dt-bindings/clock/ingenic,t33-cgu.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/dma/ingenic,pdma.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1428: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240228012420.4223-1-bin.yao@ingenic.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
  
Krzysztof Kozlowski Feb. 28, 2024, 8:02 a.m. UTC | #2
On 28/02/2024 02:24, bin.yao wrote:
> From: byao <bin.yao@ingenic.com>
> 
> Convert the textual documentation for the Ingenic SoCs PDMA
> Controller devicetree binding to YAML.
> 
> Add a dt-bindings header, and convert the device trees to it.
> 
> Signed-off-by: byao <bin.yao@ingenic.com>
> Signed-off-by: rick <rick.tyliu@ingenic.com>

Use full names.

Except that, nothing here was tested, so limited review follows.

A nit, subject: drop second/last, redundant "DT bindings for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> ---
>  .../devicetree/bindings/dma/ingenic,pdma.yaml | 77 +++++++++++++++++++
>  include/dt-bindings/dma/ingenic-pdma.h        | 51 ++++++++++++
>  2 files changed, 128 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/ingenic,pdma.yaml
>  create mode 100644 include/dt-bindings/dma/ingenic-pdma.h
> 
> diff --git a/Documentation/devicetree/bindings/dma/ingenic,pdma.yaml b/Documentation/devicetree/bindings/dma/ingenic,pdma.yaml
> new file mode 100644
> index 000000000000..b3f3a8f0b813
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/ingenic,pdma.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/ingenic,dma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ingenic SoCs DMA Controller DT bindings
> +
> +maintainers:
> +  - byao <bin.yao@ingenic.com>
> +
> +allOf:
> +  - $ref: "dma-controller.yaml#"
> +
> +properties:
> +  compatible:
> +    oneOf:

Drop

> +      - enum:
> +          - ingenic,m200-pdma
> +          - ingenic,x1000-pdma
> +          - ingenic,t40-pdma
> +          - ingenic,t41-pdma
> +          - ingenic,t33-pdma
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts-parent:
> +    maxItems: 1

Drop interrupts-parent

> +
> +  interrupts-names:
> +    items:
> +      - const: pdam
> +      - const: pdmam
> +
> +  interrupts:
> +    maxItems: 1

Nope, you have two items. Test your DTS.

> +
> +  dma-channels:
> +    const: 32
> +
> +  "#dma-cells":
> +    const: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: gate_pdma
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-parent
> +  - interrupt-names
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/ingenic,t33-cgu.h>
> +    pdma:dma@13420000 {
> +      compatible = "ingenic,t33-pdma";
> +      reg = <0x13420000 0x10000>;
> +      interrupt-parent = <&plic>;
> +      interrupt-names = "pdma", "pdmam";
> +      interrupts = <10 61>;
> +      #dma-channels = <0x20>;
> +      #dma-cells = <0x1>;
> +      clocks = <&cgu T33_CLK_DMA>;
> +      clock-names = "gate_pdma";
> +    };
> +
> diff --git a/include/dt-bindings/dma/ingenic-pdma.h b/include/dt-bindings/dma/ingenic-pdma.h
> new file mode 100644
> index 000000000000..99c871bc0ea8
> --- /dev/null
> +++ b/include/dt-bindings/dma/ingenic-pdma.h

Same filename as binding.

> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
> +
> +#ifndef __INGENIC_PDMA_H__
> +#define __INGENIC_PDMA_H__
> +
> +#define INGENIC_DMA_REQ_AIC_LOOP_RX	0x5

Indexes start from 0. If these are not indexes, then these are neither
suitable for bindings. Hex for sure is questionable.

> +#define INGENIC_DMA_REQ_AIC_TX		0x6
> +#define INGENIC_DMA_REQ_AIC_F_RX	0x7
> +#define INGENIC_DMA_REQ_AUTO_TX		0x8
> +#define INGENIC_DMA_REQ_SADC_RX		0x9
> +#define INGENIC_DMA_REQ_UART5_TX	0xa
> +#define INGENIC_DMA_REQ_UART5_RX	0xb
> +#define INGENIC_DMA_REQ_UART4_TX	0xc
> +#define INGENIC_DMA_REQ_UART4_RX	0xd
> +#define INGENIC_DMA_REQ_UART3_TX	0xe
> +#define INGENIC_DMA_REQ_UART3_RX	0xf
> +#define INGENIC_DMA_REQ_UART2_TX	0x10
> +#define INGENIC_DMA_REQ_UART2_RX	0x11
> +#define INGENIC_DMA_REQ_UART1_TX	0x12
> +#define INGENIC_DMA_REQ_UART1_RX	0x13
> +#define INGENIC_DMA_REQ_UART0_TX	0x14
> +#define INGENIC_DMA_REQ_UART0_RX	0x15
> +#define INGENIC_DMA_REQ_SSI0_TX		0x16
> +#define INGENIC_DMA_REQ_SSI0_RX		0x17
> +#define INGENIC_DMA_REQ_SSI1_TX		0x18
> +#define INGENIC_DMA_REQ_SSI1_RX		0x19
> +#define INGENIC_DMA_REQ_SLV_TX		0x1a
> +#define INGENIC_DMA_REQ_SLV_RX		0x1b
> +#define INGENIC_DMA_REQ_I2C0_TX		0x24
> +#define INGENIC_DMA_REQ_I2C0_RX		0x25
> +#define INGENIC_DMA_REQ_I2C1_TX		0x26
> +#define INGENIC_DMA_REQ_I2C1_RX		0x27
> +#define INGENIC_DMA_REQ_I2C2_TX		0x28
> +#define INGENIC_DMA_REQ_I2C2_RX		0x29
> +#define INGENIC_DMA_REQ_DES_TX		0x2e
> +#define INGENIC_DMA_REQ_DES_RX		0x2f
> +
> +#define INGENIC_DMA_TYPE_REQ_MSK	0xff

Nope, not a binding.

> +#define INGENIC_DMA_TYPE_CH_SFT		8
> +#define INGENIC_DMA_TYPE_CH_MSK		(0xff << INGENIC_DMA_TYPE_CH_SFT)

Drop entire file.

Best regards,
Krzysztof
  

Patch

diff --git a/Documentation/devicetree/bindings/dma/ingenic,pdma.yaml b/Documentation/devicetree/bindings/dma/ingenic,pdma.yaml
new file mode 100644
index 000000000000..b3f3a8f0b813
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/ingenic,pdma.yaml
@@ -0,0 +1,77 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/ingenic,dma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ingenic SoCs DMA Controller DT bindings
+
+maintainers:
+  - byao <bin.yao@ingenic.com>
+
+allOf:
+  - $ref: "dma-controller.yaml#"
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - ingenic,m200-pdma
+          - ingenic,x1000-pdma
+          - ingenic,t40-pdma
+          - ingenic,t41-pdma
+          - ingenic,t33-pdma
+
+  reg:
+    maxItems: 1
+
+  interrupts-parent:
+    maxItems: 1
+
+  interrupts-names:
+    items:
+      - const: pdam
+      - const: pdmam
+
+  interrupts:
+    maxItems: 1
+
+  dma-channels:
+    const: 32
+
+  "#dma-cells":
+    const: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: gate_pdma
+
+required:
+  - compatible
+  - reg
+  - interrupt-parent
+  - interrupt-names
+  - interrupts
+  - clocks
+  - clock-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/ingenic,t33-cgu.h>
+    pdma:dma@13420000 {
+      compatible = "ingenic,t33-pdma";
+      reg = <0x13420000 0x10000>;
+      interrupt-parent = <&plic>;
+      interrupt-names = "pdma", "pdmam";
+      interrupts = <10 61>;
+      #dma-channels = <0x20>;
+      #dma-cells = <0x1>;
+      clocks = <&cgu T33_CLK_DMA>;
+      clock-names = "gate_pdma";
+    };
+
diff --git a/include/dt-bindings/dma/ingenic-pdma.h b/include/dt-bindings/dma/ingenic-pdma.h
new file mode 100644
index 000000000000..99c871bc0ea8
--- /dev/null
+++ b/include/dt-bindings/dma/ingenic-pdma.h
@@ -0,0 +1,51 @@ 
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
+
+#ifndef __INGENIC_PDMA_H__
+#define __INGENIC_PDMA_H__
+
+#define INGENIC_DMA_REQ_AIC_LOOP_RX	0x5
+#define INGENIC_DMA_REQ_AIC_TX		0x6
+#define INGENIC_DMA_REQ_AIC_F_RX	0x7
+#define INGENIC_DMA_REQ_AUTO_TX		0x8
+#define INGENIC_DMA_REQ_SADC_RX		0x9
+#define INGENIC_DMA_REQ_UART5_TX	0xa
+#define INGENIC_DMA_REQ_UART5_RX	0xb
+#define INGENIC_DMA_REQ_UART4_TX	0xc
+#define INGENIC_DMA_REQ_UART4_RX	0xd
+#define INGENIC_DMA_REQ_UART3_TX	0xe
+#define INGENIC_DMA_REQ_UART3_RX	0xf
+#define INGENIC_DMA_REQ_UART2_TX	0x10
+#define INGENIC_DMA_REQ_UART2_RX	0x11
+#define INGENIC_DMA_REQ_UART1_TX	0x12
+#define INGENIC_DMA_REQ_UART1_RX	0x13
+#define INGENIC_DMA_REQ_UART0_TX	0x14
+#define INGENIC_DMA_REQ_UART0_RX	0x15
+#define INGENIC_DMA_REQ_SSI0_TX		0x16
+#define INGENIC_DMA_REQ_SSI0_RX		0x17
+#define INGENIC_DMA_REQ_SSI1_TX		0x18
+#define INGENIC_DMA_REQ_SSI1_RX		0x19
+#define INGENIC_DMA_REQ_SLV_TX		0x1a
+#define INGENIC_DMA_REQ_SLV_RX		0x1b
+#define INGENIC_DMA_REQ_I2C0_TX		0x24
+#define INGENIC_DMA_REQ_I2C0_RX		0x25
+#define INGENIC_DMA_REQ_I2C1_TX		0x26
+#define INGENIC_DMA_REQ_I2C1_RX		0x27
+#define INGENIC_DMA_REQ_I2C2_TX		0x28
+#define INGENIC_DMA_REQ_I2C2_RX		0x29
+#define INGENIC_DMA_REQ_DES_TX		0x2e
+#define INGENIC_DMA_REQ_DES_RX		0x2f
+
+#define INGENIC_DMA_TYPE_REQ_MSK	0xff
+#define INGENIC_DMA_TYPE_CH_SFT		8
+#define INGENIC_DMA_TYPE_CH_MSK		(0xff << INGENIC_DMA_TYPE_CH_SFT)
+#define INGENIC_DMA_TYPE_CH_EN		(1 << 16)
+#define INGENIC_DMA_TYPE_PROG		(1 << 17)
+#define INGENIC_DMA_TYPE_SPEC		(1 << 18)
+
+#define INGENIC_DMA_CH(ch)		\
+	((((ch) << INGENIC_DMA_TYPE_CH_SFT) & INGENIC_DMA_TYPE_CH_MSK) | INGENIC_DMA_TYPE_CH_EN)
+
+#define INGENIC_DMA_TYPE(type)	     ((type) & INGENIC_DMA_TYPE_REQ_MSK)
+#define INGENIC_DMA_TYPE_CH(type, ch) (INGENIC_DMA_TYPE((type)) | INGENIC_DMA_CH((ch)))
+#define INGENIC_DMA_PG_CH(type, ch) (INGENIC_DMA_TYPE_CH((type), (ch)) | INGENIC_DMA_TYPE_PROG_MSK)
+#endif