On 2020/07/17 3:48 Rob Herring <robh@xxxxxxxxxx> wrote:
> > +
> > + reg:
> > + minItems: 2
> > + maxItems: 32
>
> Needs some sort of description as to what each region is.
Okay, will add it in v3.
>
> > +
> > + interrupts:
> > + minItems: 2
> > + maxItems: 32
>
> ditto
Will add it in v3.
>
> > +
> > + interrupt-names:
> > + minItems: 2
> > + maxItems: 32
> > + items:
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
>
> What about interrupts 8-31? If you want a pattern for all entries, you
Here 8 lines matches with the 8 channel used in dts (same example as the
bottom of this yaml file below), otherwise, below warning comes out with
'make dt_binding_check' if I remove the above last line:
dma-controller@5a1f0000: interrupt-names: Additional items are not allowed ('edma2-chan15-tx' was unexpected)
Maybe there something I miss, please correct me if I'm wrong, thanks.
> do:
>
> items:
> pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
>
> (If 'items' is a schema instead of a list, then it applies to all
> entries.)
>
> What does edma[0-2] correspond to? The names should be local to the
> instance.
There are 3 edma controllers on i.mx8QXP/QM, [0-2] is controller index.
Clear controller index here may make dts readable since every edma channel
has unique register map, interrupt number, power domain.
>
> Really, what's the point of names that just have the channel number in them?
> The driver can create names dynamically if needed. We already have
> properties to define how many channels and a mask of present channels.
interrupt-names/power-domain-names here just for better readable in dts, besides, the clear interrupt name for every channel including edma controller
id which passed by dts could be used to register irq.
>
> > +
> > + '#dma-cells':
> > + const: 3
> > + description: |
> > + The 1st cell specifies the channel ID.
> > +
> > + The 2nd cell specifies the channel priority.
> > +
> > + The 3rd cell specifies the channel attributes:
> > + bit0 transmit or receive.
> > + 0 = transmit
> > + 1 = receive
> > + bit1 local or remote access.
> > + 0 = local
> > + 1 = remote
> > + bit2 dualfifo case or not(only in Audio cyclic now).
> > + 0 = not dual fifo case
> > + 1 = dualfifo case.
> > +
> > + dma-channels:
> > + minimum: 2
> > + maximum: 32
> > +
> > + power-domains:
> > + minItems: 2
> > + maxItems: 32
> > +
> > + power-domain-names:
> > + minItems: 2
> > + maxItems: 32
> > + items:
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> > + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
>
> Again, why do you need names here?
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - interrupt-names
> > + - '#dma-cells'
> > + - dma-channels
> > + - power-domains
> > + - power-domain-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/firmware/imx/rsrc.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > + edma2: dma-controller@5a1f0000 {
> > + compatible = "fsl,imx8qm-edma";
> > + reg = <0x5a280000 0x10000>, /* channel8 UART0 rx */
> > + <0x5a290000 0x10000>, /* channel9 UART0 tx */
> > + <0x5a2a0000 0x10000>, /* channel10 UART1 rx */
> > + <0x5a2b0000 0x10000>, /* channel11 UART1 tx */
> > + <0x5a2c0000 0x10000>, /* channel12 UART2 rx */
> > + <0x5a2d0000 0x10000>, /* channel13 UART2 tx */
> > + <0x5a2e0000 0x10000>, /* channel14 UART3 rx */
> > + <0x5a2f0000 0x10000>; /* channel15 UART3 tx */
> > + #dma-cells = <3>;
> > + dma-channels = <8>;
> > + interrupts = <GIC_SPI 434 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 435 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 437 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 438 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 439 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 440 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 441 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-names = "edma2-chan8-rx", "edma2-chan9-tx",
> > + "edma2-chan10-rx", "edma2-chan11-tx",
> > + "edma2-chan12-rx", "edma2-chan13-tx",
> > + "edma2-chan14-rx", "edma2-chan15-tx";
> > + power-domains = <&pd IMX_SC_R_DMA_2_CH8>,
> > + <&pd IMX_SC_R_DMA_2_CH9>,
> > + <&pd IMX_SC_R_DMA_2_CH10>,
> > + <&pd IMX_SC_R_DMA_2_CH11>,
> > + <&pd IMX_SC_R_DMA_2_CH12>,
> > + <&pd IMX_SC_R_DMA_2_CH13>,
> > + <&pd IMX_SC_R_DMA_2_CH14>,
> > + <&pd IMX_SC_R_DMA_2_CH15>;
> > + power-domain-names = "edma2-chan8", "edma2-chan9",
> > + "edma2-chan10", "edma2-chan11",
> > + "edma2-chan12", "edma2-chan13",
> > + "edma2-chan14", "edma2-chan15";
> > + };
> > --
> > 2.7.4
> >
