| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 22 Jul 2020 19:19:22 +0800
From: EastL <EastL.Lee@...iatek.com>
To: Rob Herring <robh@...nel.org>
CC: Sean Wang <sean.wang@...iatek.com>, <vkoul@...nel.org>,
<mark.rutland@....com>, <matthias.bgg@...il.com>,
<dmaengine@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-mediatek@...ts.infradead.org>, <devicetree@...r.kernel.org>,
<wsd_upstream@...iatek.com>, <cc.hwang@...iatek.com>
Subject: Re: [PATCH v6 1/4] dt-bindings: dmaengine: Add MediaTek
Command-Queue DMA controller bindings
On Thu, 2020-07-09 at 14:59 -0600, Rob Herring wrote:
> On Thu, Jul 02, 2020 at 03:06:01PM +0800, EastL Lee wrote:
> > Document the devicetree bindings for MediaTek Command-Queue DMA controller
> > which could be found on MT6779 SoC or other similar Mediatek SoCs.
> >
> > Signed-off-by: EastL Lee <EastL.Lee@...iatek.com>
> > ---
> > .../devicetree/bindings/dma/mtk-cqdma.yaml | 113 +++++++++++++++++++++
> > 1 file changed, 113 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml b/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
> > new file mode 100644
> > index 0000000..83ed742
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
> > @@ -0,0 +1,113 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: https://urldefense.com/v3/__http://devicetree.org/schemas/dma/mtk-cqdma.yaml*__;Iw!!CTRNKA9wMg0ARbw!1P_if3RiZOVpzN8n4EQI0IxZq0d07UksSgeHYA0h6HLylU9l4pu2eggq7eeVsF2H$
> > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!1P_if3RiZOVpzN8n4EQI0IxZq0d07UksSgeHYA0h6HLylU9l4pu2eggq7ZMPow23$
> > +
> > +title: MediaTek Command-Queue DMA controller Device Tree Binding
> > +
> > +maintainers:
> > + - EastL Lee <EastL.Lee@...iatek.com>
> > +
> > +description:
> > + MediaTek Command-Queue DMA controller (CQDMA) on Mediatek SoC
> > + is dedicated to memory-to-memory transfer through queue based
> > + descriptor management.
> > +
> > +allOf:
> > + - $ref: "dma-controller.yaml#"
> > +
> > +properties:
> > + "#dma-cells":
> > + minimum: 1
> > + maximum: 255
> > + description:
> > + Used to provide DMA controller specific information.
>
> No, for a specific binding like this, it should be 1 defined value.
OK.I'll fix it to const 1
>
> > +
> > + compatible:
> > + oneOf:
> > + - const: mediatek,mt6765-cqdma
> > + - const: mediatek,mt6779-cqdma
> > +
> > + reg:
> > + minItems: 1
> > + maxItems: 5
> > + description:
> > + A base address of MediaTek Command-Queue DMA controller,
> > + a channel will have a set of base address.
> > +
> > + interrupts:
> > + minItems: 1
> > + maxItems: 5
> > + description:
> > + A interrupt number of MediaTek Command-Queue DMA controller,
> > + one interrupt number per dma-channels.
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + clock-names:
> > + const: cqdma
> > +
> > + dma-channel-mask:
> > + $ref: /schemas/types.yaml#definitions/uint32
>
> Alreay has a type, don't redefine it here.
OK
>
> > + description:
> > + For DMA capability, We will know the addressing capability of
> > + MediaTek Command-Queue DMA controller through dma-channel-mask.
>
> This sounds like the kernel's DMA masks which is not what this property
> is.
Yes, this is for kernel's DMA mask.
Do I need to declare this member again?
>
> > + items:
> > + minItems: 1
> > + maxItems: 63
>
> An array of 63 elements?
>
> I think you want:
>
> minimum: 1
> maximum: 63
>
> Or:
>
> enum: [ 1, 3, 7, 0xf, 0x1f, 0x3f ]
>
> (Though if this works, then just 'dma-channels' is enough.)
>
> > +
> > + dma-channels:
> > + $ref: /schemas/types.yaml#definitions/uint32
> > + description:
> > + Number of DMA channels supported by MediaTek Command-Queue DMA
> > + controller, support up to five.
>
> Is it 5 or 6 channels? You're off by one somewhere.
Currently chip CQDMA only has three channels at most, I'll fix it to
maxnum 3
> > + items:
> > + minItems: 1
> > + maxItems: 5
> > +
> > + dma-requests:
> > + $ref: /schemas/types.yaml#definitions/uint32
> > + description:
> > + Number of DMA request (virtual channel) supported by MediaTek
> > + Command-Queue DMA controller, support up to 32.
> > + items:
> > + minItems: 1
> > + maxItems: 32
>
> You are describing how many elements in an array and this is a scalar.
OK I;ll fix to minnum & maxnum
>
> > +
> > +required:
> > + - "#dma-cells"
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - clock-names
> > + - dma-channel-mask
> > + - dma-channels
> > + - dma-requests
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/mt6779-clk.h>
> > + cqdma: dma-controller@...12000 {
> > + compatible = "mediatek,mt6779-cqdma";
> > + reg = <0x10212000 0x80>,
> > + <0x10212080 0x80>,
> > + <0x10212100 0x80>;
> > + interrupts = <GIC_SPI 139 IRQ_TYPE_LEVEL_LOW>,
> > + <GIC_SPI 140 IRQ_TYPE_LEVEL_LOW>,
> > + <GIC_SPI 141 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&infracfg_ao CLK_INFRA_CQ_DMA>;
> > + clock-names = "cqdma";
> > + dma-channel-mask = <63>;
> > + dma-channels = <3>;
> > + dma-requests = <32>;
> > + #dma-cells = <1>;
> > + };
> > +
> > +...
> > --
> > 1.9.1
Powered by blists - more mailing lists