| Message ID | 20230928121953.524608-2-keguang.zhang@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add Loongson1 dmaengine driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
Hey, On Thu, Sep 28, 2023 at 08:19:52PM +0800, Keguang Zhang wrote: > Add devicetree binding document for Loongson-1 DMA. > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com> > --- > V4 -> V5: > A newly added patch > > .../bindings/dma/loongson,ls1x-dma.yaml | 64 +++++++++++++++++++ > 1 file changed, 64 insertions(+) > create mode 100644 Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml > > diff --git a/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml b/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml > new file mode 100644 > index 000000000000..51b45d998a58 > --- /dev/null > +++ b/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml > @@ -0,0 +1,64 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/dma/loongson,ls1x-dma.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Loongson-1 DMA Controller > + > +maintainers: > + - Keguang Zhang <keguang.zhang@gmail.com> > + > +description: | This | isn't required as you have no formatting to preserve here. > + Loongson-1 DMA controller provides 3 independent channels for > + peripherals such as NAND and AC97. > + > +properties: > + compatible: > + enum: > + - loongson,ls1b-dma > + - loongson,ls1c-dma From a skim of the driver, these two devices seem to be compatible, and therefore one should fall back to the other. > + > + reg: > + maxItems: 1 > + > + interrupts: > + description: Each channel has a dedicated interrupt line. > + minItems: 1 > + maxItems: 3 > + > + interrupt-names: > + minItems: 1 > + items: > + - pattern: ch0 > + - pattern: ch1 > + - pattern: ch2 > + > + '#dma-cells': > + description: The single cell represents the channel index. This description is unnecessary. > + const: 1 > + > +required: > + - compatible > + - reg > + - interrupts > + - interrupt-names > + - '#dma-cells' > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + dma: dma-controller@1fd01160 { This label is unused. > + compatible = "loongson,ls1b-dma"; > + reg = <0x1fd01160 0x18>; > + > + interrupt-parent = <&intc0>; > + interrupts = <13 IRQ_TYPE_EDGE_RISING>, > + <14 IRQ_TYPE_EDGE_RISING>, > + <15 IRQ_TYPE_EDGE_RISING>; Odd mixed indentation here. Bindings use spaces only. Thanks, Conor. > + interrupt-names = "ch0", "ch1", "ch2"; > + > + #dma-cells = <1>; > + }; > -- > 2.39.2 >
On Fri, Sep 29, 2023 at 12:42 AM Conor Dooley <conor@kernel.org> wrote: > > Hey, > > On Thu, Sep 28, 2023 at 08:19:52PM +0800, Keguang Zhang wrote: > > Add devicetree binding document for Loongson-1 DMA. > > > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com> > > --- > > V4 -> V5: > > A newly added patch > > > > .../bindings/dma/loongson,ls1x-dma.yaml | 64 +++++++++++++++++++ > > 1 file changed, 64 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml > > > > diff --git a/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml b/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml > > new file mode 100644 > > index 000000000000..51b45d998a58 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml > > @@ -0,0 +1,64 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/dma/loongson,ls1x-dma.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Loongson-1 DMA Controller > > + > > +maintainers: > > + - Keguang Zhang <keguang.zhang@gmail.com> > > + > > +description: | > > This | isn't required as you have no formatting to preserve here. > Will remove '|'. > > + Loongson-1 DMA controller provides 3 independent channels for > > + peripherals such as NAND and AC97. > > + > > +properties: > > + compatible: > > + enum: > > + - loongson,ls1b-dma > > + - loongson,ls1c-dma > > From a skim of the driver, these two devices seem to be compatible, > and therefore one should fall back to the other. > Got it. How about changing to one const? compatible: const: loongson,ls1-dma > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + description: Each channel has a dedicated interrupt line. > > + minItems: 1 > > + maxItems: 3 > > + > > + interrupt-names: > > + minItems: 1 > > + items: > > + - pattern: ch0 > > + - pattern: ch1 > > + - pattern: ch2 > > + > > + '#dma-cells': > > + description: The single cell represents the channel index. > > This description is unnecessary. > Will delete the description. > > + const: 1 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - interrupt-names > > + - '#dma-cells' > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/irq.h> > > + dma: dma-controller@1fd01160 { > > This label is unused. > Will delete the lablel. > > + compatible = "loongson,ls1b-dma"; > > + reg = <0x1fd01160 0x18>; > > + > > + interrupt-parent = <&intc0>; > > + interrupts = <13 IRQ_TYPE_EDGE_RISING>, > > + <14 IRQ_TYPE_EDGE_RISING>, > > + <15 IRQ_TYPE_EDGE_RISING>; > > Odd mixed indentation here. Bindings use spaces only. > Will fix the indentation. > Thanks, > Conor. > > > + interrupt-names = "ch0", "ch1", "ch2"; > > + > > + #dma-cells = <1>; > > + }; > > -- > > 2.39.2 > > -- Best regards, Keguang Zhang
On Sat, Oct 07, 2023 at 01:45:23PM +0800, Keguang Zhang wrote: > On Fri, Sep 29, 2023 at 12:42 AM Conor Dooley <conor@kernel.org> wrote: > > > > Hey, > > > > On Thu, Sep 28, 2023 at 08:19:52PM +0800, Keguang Zhang wrote: > > > Add devicetree binding document for Loongson-1 DMA. > > > > > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com> > > > --- > > > V4 -> V5: > > > A newly added patch > > > > > > .../bindings/dma/loongson,ls1x-dma.yaml | 64 +++++++++++++++++++ > > > 1 file changed, 64 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml b/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml > > > new file mode 100644 > > > index 000000000000..51b45d998a58 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml > > > @@ -0,0 +1,64 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/dma/loongson,ls1x-dma.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Loongson-1 DMA Controller > > > + > > > +maintainers: > > > + - Keguang Zhang <keguang.zhang@gmail.com> > > > + > > > +description: | > > > > This | isn't required as you have no formatting to preserve here. > > > Will remove '|'. > > > > + Loongson-1 DMA controller provides 3 independent channels for > > > + peripherals such as NAND and AC97. > > > + > > > +properties: > > > + compatible: > > > + enum: > > > + - loongson,ls1b-dma > > > + - loongson,ls1c-dma > > > > From a skim of the driver, these two devices seem to be compatible, > > and therefore one should fall back to the other. > > > Got it. How about changing to one const? > compatible: > const: loongson,ls1-dma No, keep the compatibles you have & set up a fallback please. Too many of these loongson bindings have got in with overly generic compatibles...
On 07/10/2023 07:45, Keguang Zhang wrote: > >>> + Loongson-1 DMA controller provides 3 independent channels for >>> + peripherals such as NAND and AC97. >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - loongson,ls1b-dma >>> + - loongson,ls1c-dma >> >> From a skim of the driver, these two devices seem to be compatible, >> and therefore one should fall back to the other. >> > Got it. How about changing to one const? > compatible: > const: loongson,ls1-dma No, because there is no fallback here. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml b/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml new file mode 100644 index 000000000000..51b45d998a58 --- /dev/null +++ b/Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml @@ -0,0 +1,64 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/dma/loongson,ls1x-dma.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Loongson-1 DMA Controller + +maintainers: + - Keguang Zhang <keguang.zhang@gmail.com> + +description: | + Loongson-1 DMA controller provides 3 independent channels for + peripherals such as NAND and AC97. + +properties: + compatible: + enum: + - loongson,ls1b-dma + - loongson,ls1c-dma + + reg: + maxItems: 1 + + interrupts: + description: Each channel has a dedicated interrupt line. + minItems: 1 + maxItems: 3 + + interrupt-names: + minItems: 1 + items: + - pattern: ch0 + - pattern: ch1 + - pattern: ch2 + + '#dma-cells': + description: The single cell represents the channel index. + const: 1 + +required: + - compatible + - reg + - interrupts + - interrupt-names + - '#dma-cells' + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + dma: dma-controller@1fd01160 { + compatible = "loongson,ls1b-dma"; + reg = <0x1fd01160 0x18>; + + interrupt-parent = <&intc0>; + interrupts = <13 IRQ_TYPE_EDGE_RISING>, + <14 IRQ_TYPE_EDGE_RISING>, + <15 IRQ_TYPE_EDGE_RISING>; + interrupt-names = "ch0", "ch1", "ch2"; + + #dma-cells = <1>; + };
Add devicetree binding document for Loongson-1 DMA. Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com> --- V4 -> V5: A newly added patch .../bindings/dma/loongson,ls1x-dma.yaml | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 Documentation/devicetree/bindings/dma/loongson,ls1x-dma.yaml