On 28/02/2024 02:24, bin.yao wrote: > From: byao <bin.yao@xxxxxxxxxxx> > > 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@xxxxxxxxxxx> > Signed-off-by: rick <rick.tyliu@xxxxxxxxxxx> 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@xxxxxxxxxxx> > + > +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
