| Message ID | 1444744848-720-2-git-send-email-cedric.madianga@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 13, 2015 at 04:00:45PM +0200, M'boumba Cedric Madianga wrote: > This patch adds documentation of device tree bindings for the STM32 dma > controller. > > Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com> > --- > .../devicetree/bindings/dma/stm32-dma.txt | 98 ++++++++++++++++++++++ > 1 file changed, 98 insertions(+) > create mode 100644 Documentation/devicetree/bindings/dma/stm32-dma.txt > > diff --git a/Documentation/devicetree/bindings/dma/stm32-dma.txt b/Documentation/devicetree/bindings/dma/stm32-dma.txt > new file mode 100644 > index 0000000..9ce0d49 > --- /dev/null > +++ b/Documentation/devicetree/bindings/dma/stm32-dma.txt > @@ -0,0 +1,98 @@ > +* STMicroelectronics STM32 DMA controller > + > +The STM32 DMA is a general-purpose direct memory access controller capable of > +supporting 8 independent DMA channels. Each channel can have up to 8 requests. > + > +Required properties: > +- compatible: Should be "st,stm32-dma" > +- reg: Should contain DMA registers location and length. This should include > + all of the per-channel registers. > +- interrupts: Should contain all of the per-channel DMA interrupts. Please specify the order they must be in. > +- clocks: Should contain the input clock of the DMA instance. > +- #dma-cells : Must be <4>. See DMA client paragraph for more details. > + > +Optional properties: > +- resets: Reference to a reset controller asserting the DMA controller > +- st,mem2mem: boolean; if defined, it indicates that the controller supports > + memory-to-memory transfer > + > +Example: > + > + dma2: dma-controller@40026400 { > + compatible = "st,stm32-dma"; > + reg = <0x40026400 0x400>; > + interrupts = <56>, > + <57>, > + <58>, > + <59>, > + <60>, > + <68>, > + <69>, > + <70>; > + clocks = <&clk_hclk>; > + #dma-cells = <4>; > + st,mem2mem; > + resets = <&rcc 150>; > + }; > + > +* DMA client > + > +Required properties: > +- dmas: Comma separated list of dma channel requests > +- dma-names: Names of the aforementioned requested channels > + > +Each dmas request consists of 5 cells: > +1. A phandle pointing to the STM32 DMA controller > +2. The channel id > +3. The request line number > +4. A 32bit mask specifying the DMA channel configuration > + -bit 1: Direct Mode Error Interrupt > + 0x0: disabled > + 0x1: enabled > + -bit 2: Transfer Error Interrupt > + 0x0: disabled > + 0x1: enabled > + -bit 3: Half Transfer Mode Error Interrupt > + 0x0: disabled > + 0x1: enabled > + -bit 4: Transfer Complete Interrupt > + 0x0: disabled > + 0x1: enabled Why should this be in the DT? Surely the driver should be in charge of deciding when to use these? > + -bit 9: Peripheral Increment Address > + 0x0: no address increment between transfers > + 0x1: increment address between transfers > + -bit 10: Memory Increment Address > + 0x0: no address increment between transfers > + 0x1: increment address between transfers These don't seem like they belong either. Surely this would depend on the request made, rather than being a fixed property? > + -bit 15: Peripheral Increment Offset Size > + 0x0: offset size is linked to the peripheral bus width > + 0x1: offset size is fixed to 4 (32-bit alignment) This sounds like it might belong in the DT. > + -bit 16-17: Priority level > + 0x0: low > + 0x1: medium > + 0x2: high > + 0x3: very high What do we do elsewhere in terms of describing prioritisation? It feels like it would be a dynamic property of the system. > +5. A 32bit mask specifying the DMA FIFO configuration > + -bit 0-1: Fifo threshold > + 0x0: 1/4 full FIFO > + 0x1: 1/2 full FIFO > + 0x2: 3/4 full FIFO > + 0x3:full FIFO What does this mean? > + -bit 2: Direct mode > + 0x0: enabled > + 0x1: disabled What does this mean? > + -bit 7: FIFO Error Interrupt > + 0x0: disabled > + 0x1: enabled As with the other interrupt configuration, this does not look like it belongs in the DT. Thanks, Mark.
Hi Mark, 2015-10-13 16:22 GMT+02:00 Mark Rutland <mark.rutland@arm.com>: > On Tue, Oct 13, 2015 at 04:00:45PM +0200, M'boumba Cedric Madianga wrote: >> This patch adds documentation of device tree bindings for the STM32 dma >> controller. >> >> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com> >> --- >> .../devicetree/bindings/dma/stm32-dma.txt | 98 ++++++++++++++++++++++ >> 1 file changed, 98 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/dma/stm32-dma.txt >> >> diff --git a/Documentation/devicetree/bindings/dma/stm32-dma.txt b/Documentation/devicetree/bindings/dma/stm32-dma.txt >> new file mode 100644 >> index 0000000..9ce0d49 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/dma/stm32-dma.txt >> @@ -0,0 +1,98 @@ >> +* STMicroelectronics STM32 DMA controller >> + >> +The STM32 DMA is a general-purpose direct memory access controller capable of >> +supporting 8 independent DMA channels. Each channel can have up to 8 requests. >> + >> +Required properties: >> +- compatible: Should be "st,stm32-dma" >> +- reg: Should contain DMA registers location and length. This should include >> + all of the per-channel registers. >> +- interrupts: Should contain all of the per-channel DMA interrupts. > > Please specify the order they must be in. Ok, I will do it in the next version. Thanks > >> +- clocks: Should contain the input clock of the DMA instance. >> +- #dma-cells : Must be <4>. See DMA client paragraph for more details. >> + >> +Optional properties: >> +- resets: Reference to a reset controller asserting the DMA controller >> +- st,mem2mem: boolean; if defined, it indicates that the controller supports >> + memory-to-memory transfer >> + >> +Example: >> + >> + dma2: dma-controller@40026400 { >> + compatible = "st,stm32-dma"; >> + reg = <0x40026400 0x400>; >> + interrupts = <56>, >> + <57>, >> + <58>, >> + <59>, >> + <60>, >> + <68>, >> + <69>, >> + <70>; >> + clocks = <&clk_hclk>; >> + #dma-cells = <4>; >> + st,mem2mem; >> + resets = <&rcc 150>; >> + }; >> + >> +* DMA client >> + >> +Required properties: >> +- dmas: Comma separated list of dma channel requests >> +- dma-names: Names of the aforementioned requested channels >> + >> +Each dmas request consists of 5 cells: >> +1. A phandle pointing to the STM32 DMA controller >> +2. The channel id >> +3. The request line number >> +4. A 32bit mask specifying the DMA channel configuration >> + -bit 1: Direct Mode Error Interrupt >> + 0x0: disabled >> + 0x1: enabled >> + -bit 2: Transfer Error Interrupt >> + 0x0: disabled >> + 0x1: enabled >> + -bit 3: Half Transfer Mode Error Interrupt >> + 0x0: disabled >> + 0x1: enabled >> + -bit 4: Transfer Complete Interrupt >> + 0x0: disabled >> + 0x1: enabled > > Why should this be in the DT? > > Surely the driver should be in charge of deciding when to use these? For interrupt configuration the driver could set default configuration and don't let the DMA client set it. The goal here is to offer for each DMA client a way to choose his level of information when an error occured on DMA bus or when a DMA transfer is complete. For channel and request ids, these inputs are really mandatory as DMA mapping (couple channel/request) is fixed. So each DMA peripheral has it own channel/request ids. > >> + -bit 9: Peripheral Increment Address >> + 0x0: no address increment between transfers >> + 0x1: increment address between transfers >> + -bit 10: Memory Increment Address >> + 0x0: no address increment between transfers >> + 0x1: increment address between transfers > > These don't seem like they belong either. Surely this would depend on > the request made, rather than being a fixed property? It is really specific to the DMA client. Many clients transfer DMA from peripheral at fixed register address so in this use case PINC=0. (It is the most common case) But others clients could do it by incrementing an memory area after each transfer and in this case PINC=1. I don't have any example in mind for the second use case but as the DMA supports it I would like to keep it. > >> + -bit 15: Peripheral Increment Offset Size >> + 0x0: offset size is linked to the peripheral bus width >> + 0x1: offset size is fixed to 4 (32-bit alignment) > > This sounds like it might belong in the DT. Agree > >> + -bit 16-17: Priority level >> + 0x0: low >> + 0x1: medium >> + 0x2: high >> + 0x3: very high > > What do we do elsewhere in terms of describing prioritisation? It feels > like it would be a dynamic property of the system. You're right it could be a dynamic property of the system but we don't have any way to set it via the dmaengine API. So, we decide to set it before requesting a DMA transfer and keep it during all peripheral life. > >> +5. A 32bit mask specifying the DMA FIFO configuration >> + -bit 0-1: Fifo threshold >> + 0x0: 1/4 full FIFO >> + 0x1: 1/2 full FIFO >> + 0x2: 3/4 full FIFO >> + 0x3:full FIFO > > What does this mean? The STM32 DMA has a 4 words FIFO to temporarily stores data from source. The threshold level is used to know at each which FIFO filling rate the data stores in the FIFO will be really sent to the destination. For example, with FIFO threshold = 1/2 full FIFO, we wait at least 2 words of data from source before storing it into to the destination. > >> + -bit 2: Direct mode >> + 0x0: enabled >> + 0x1: disabled > > What does this mean? The Direct mode bit is used to choose if you want to use FIFO or not. In direct mode (Direct mode = 0), the data coming from source are not temporarily stored in the DMA FIFO and are directly stored into the destination. In FIFO mode ((Direct mode = 1), the data are temporarily stored in the DMA FIFO and then in the destination according to the FIFO threshold level as explained above. > >> + -bit 7: FIFO Error Interrupt >> + 0x0: disabled >> + 0x1: enabled > > As with the other interrupt configuration, this does not look like it > belongs in the DT. Again the driver could set default configuration and don't let the DMA client set it. The goal here is to offer for each DMA client a way to choose his level of information when an error occured on DMA bus. > > Thanks, > Mark. Thanks for your review and comments. BR, Cedric
> >> +Each dmas request consists of 5 cells: > >> +1. A phandle pointing to the STM32 DMA controller > >> +2. The channel id > >> +3. The request line number Whoops, I meant to clip these lines out of my original reply. Sorry for the confusion below resulting from this. > >> +4. A 32bit mask specifying the DMA channel configuration > >> + -bit 1: Direct Mode Error Interrupt > >> + 0x0: disabled > >> + 0x1: enabled > >> + -bit 2: Transfer Error Interrupt > >> + 0x0: disabled > >> + 0x1: enabled > >> + -bit 3: Half Transfer Mode Error Interrupt > >> + 0x0: disabled > >> + 0x1: enabled > >> + -bit 4: Transfer Complete Interrupt > >> + 0x0: disabled > >> + 0x1: enabled > > > > Why should this be in the DT? > > > > Surely the driver should be in charge of deciding when to use these? > For interrupt configuration the driver could set default configuration > and don't let the DMA client set it. > The goal here is to offer for each DMA client a way to choose his > level of information when an error occured on DMA bus or when a DMA > transfer is complete. The DT is separate from what the client driver might want, so if different clients want different things, that should be requested in a a generic and dnyamic fashion through the dmaengine API, with the physical details left to the DMA controller driver. I really don't see why the DTS author should be in charge of how the DMA controller driver chooses to use interrupts in this fashion. > For channel and request ids, these inputs are really mandatory as DMA > mapping (couple channel/request) is fixed. > So each DMA peripheral has it own channel/request ids. As above, I only meant to quote the interrupt bits here. The channel and request line numbers should be in the DT as they are fixed HW details. > >> + -bit 9: Peripheral Increment Address > >> + 0x0: no address increment between transfers > >> + 0x1: increment address between transfers > >> + -bit 10: Memory Increment Address > >> + 0x0: no address increment between transfers > >> + 0x1: increment address between transfers > > > > These don't seem like they belong either. Surely this would depend on > > the request made, rather than being a fixed property? > It is really specific to the DMA client. > Many clients transfer DMA from peripheral at fixed register address so > in this use case PINC=0. (It is the most common case) I'd have expected other DMA controllers to also need to handle this, but from a quick skim of bindings I couldn't spot anything similar, so I assume that this gets handled somehow dynamically. Do you know what other DMA controllers do for cases like this in Linux? > But others clients could do it by incrementing an memory area after > each transfer and in this case PINC=1. > I don't have any example in mind for the second use case but as the > DMA supports it I would like to keep it. Similarly this seems odd. What do other DMA controllers do? > >> + -bit 16-17: Priority level > >> + 0x0: low > >> + 0x1: medium > >> + 0x2: high > >> + 0x3: very high > > > > What do we do elsewhere in terms of describing prioritisation? It feels > > like it would be a dynamic property of the system. > You're right it could be a dynamic property of the system but we don't > have any way to set it via the dmaengine API. > So, we decide to set it before requesting a DMA transfer and keep it > during all peripheral life. I see that there is precedent with existing bindings. > >> +5. A 32bit mask specifying the DMA FIFO configuration > >> + -bit 0-1: Fifo threshold > >> + 0x0: 1/4 full FIFO > >> + 0x1: 1/2 full FIFO > >> + 0x2: 3/4 full FIFO > >> + 0x3:full FIFO > > > > What does this mean? > The STM32 DMA has a 4 words FIFO to temporarily stores data from source. > The threshold level is used to know at each which FIFO filling rate > the data stores in the FIFO will be really sent to the destination. > For example, with FIFO threshold = 1/2 full FIFO, we wait at least 2 > words of data from source before storing it into to the destination. Likewise there seems to be precedent for this. I don't know whether it really makes sense for this to be in the DT. > >> + -bit 2: Direct mode > >> + 0x0: enabled > >> + 0x1: disabled > > > > What does this mean? > The Direct mode bit is used to choose if you want to use FIFO or not. > In direct mode (Direct mode = 0), the data coming from source are not > temporarily stored in the DMA FIFO and are directly stored into the > destination. > In FIFO mode ((Direct mode = 1), the data are temporarily stored in > the DMA FIFO and then in the destination according to the FIFO > threshold level as explained above. I guess this is effectively an extension of the previous FIFO config bits, so if those make sense I guess this does... > >> + -bit 7: FIFO Error Interrupt > >> + 0x0: disabled > >> + 0x1: enabled > > > > As with the other interrupt configuration, this does not look like it > > belongs in the DT. > Again the driver could set default configuration and don't let the DMA > client set it. > The goal here is to offer for each DMA client a way to choose his > level of information when an error occured on DMA bus. As with the other interrupts, I still don't believe this should be in the DT. Thanks, Mark.
Hi Mark, 2015-10-13 18:21 GMT+02:00 Mark Rutland <mark.rutland@arm.com>: >> >> +4. A 32bit mask specifying the DMA channel configuration >> >> + -bit 1: Direct Mode Error Interrupt >> >> + 0x0: disabled >> >> + 0x1: enabled >> >> + -bit 2: Transfer Error Interrupt >> >> + 0x0: disabled >> >> + 0x1: enabled >> >> + -bit 3: Half Transfer Mode Error Interrupt >> >> + 0x0: disabled >> >> + 0x1: enabled >> >> + -bit 4: Transfer Complete Interrupt >> >> + 0x0: disabled >> >> + 0x1: enabled >> > > The DT is separate from what the client driver might want, so if > different clients want different things, that should be requested in a a > generic and dnyamic fashion through the dmaengine API, with the physical > details left to the DMA controller driver. > > I really don't see why the DTS author should be in charge of how the DMA > controller driver chooses to use interrupts in this fashion. > Ok got it. As enabling all interrupts by default does not influence DMA behavior I will remove these inputs from DT. Thanks for this explanation. > >> >> + -bit 9: Peripheral Increment Address >> >> + 0x0: no address increment between transfers >> >> + 0x1: increment address between transfers >> >> + -bit 10: Memory Increment Address >> >> + 0x0: no address increment between transfers >> >> + 0x1: increment address between transfers >> > > I'd have expected other DMA controllers to also need to handle this, but > from a quick skim of bindings I couldn't spot anything similar, so I > assume that this gets handled somehow dynamically. > > Do you know what other DMA controllers do for cases like this in Linux? After quick search I don't find any other DMA controller that handles this problematic. For sure, there is no way to handle it thanks to dmaengine API. Then, I know that previously many DMA clients could pass specific information during channel request thanks to *fn_param as below: struct dma_chan *__dma_request_channel(const dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param) But now, this API seems obsolete has new platform has to request channel thanks to DT inputs via dma_request_slave_channel(struct device *dev, const char *name) API. In that way, all specific information has to be retrieved from DT. > >> But others clients could do it by incrementing an memory area after >> each transfer and in this case PINC=1. >> I don't have any example in mind for the second use case but as the >> DMA supports it I would like to keep it. > > Similarly this seems odd. What do other DMA controllers do? Same remark as above > > >> >> +5. A 32bit mask specifying the DMA FIFO configuration >> >> + -bit 0-1: Fifo threshold >> >> + 0x0: 1/4 full FIFO >> >> + 0x1: 1/2 full FIFO >> >> + 0x2: 3/4 full FIFO >> >> + 0x3:full FIFO >> > > Likewise there seems to be precedent for this. I don't know whether it > really makes sense for this to be in the DT. Same remark as above. We abolutely need to retrieve it from DT. Moreover, this point really influence DMA controller behavior in term of performance. > >> >> + -bit 7: FIFO Error Interrupt >> >> + 0x0: disabled >> >> + 0x1: enabled >> > >> > As with the other interrupt configuration, this does not look like it >> > belongs in the DT. >> Again the driver could set default configuration and don't let the DMA >> client set it. >> The goal here is to offer for each DMA client a way to choose his >> level of information when an error occured on DMA bus. > > As with the other interrupts, I still don't believe this should be in > the DT. As other interrupt configuration, I will remove it from DT. Thanks, Cedric
diff --git a/Documentation/devicetree/bindings/dma/stm32-dma.txt b/Documentation/devicetree/bindings/dma/stm32-dma.txt new file mode 100644 index 0000000..9ce0d49 --- /dev/null +++ b/Documentation/devicetree/bindings/dma/stm32-dma.txt @@ -0,0 +1,98 @@ +* STMicroelectronics STM32 DMA controller + +The STM32 DMA is a general-purpose direct memory access controller capable of +supporting 8 independent DMA channels. Each channel can have up to 8 requests. + +Required properties: +- compatible: Should be "st,stm32-dma" +- reg: Should contain DMA registers location and length. This should include + all of the per-channel registers. +- interrupts: Should contain all of the per-channel DMA interrupts. +- clocks: Should contain the input clock of the DMA instance. +- #dma-cells : Must be <4>. See DMA client paragraph for more details. + +Optional properties: +- resets: Reference to a reset controller asserting the DMA controller +- st,mem2mem: boolean; if defined, it indicates that the controller supports + memory-to-memory transfer + +Example: + + dma2: dma-controller@40026400 { + compatible = "st,stm32-dma"; + reg = <0x40026400 0x400>; + interrupts = <56>, + <57>, + <58>, + <59>, + <60>, + <68>, + <69>, + <70>; + clocks = <&clk_hclk>; + #dma-cells = <4>; + st,mem2mem; + resets = <&rcc 150>; + }; + +* DMA client + +Required properties: +- dmas: Comma separated list of dma channel requests +- dma-names: Names of the aforementioned requested channels + +Each dmas request consists of 5 cells: +1. A phandle pointing to the STM32 DMA controller +2. The channel id +3. The request line number +4. A 32bit mask specifying the DMA channel configuration + -bit 1: Direct Mode Error Interrupt + 0x0: disabled + 0x1: enabled + -bit 2: Transfer Error Interrupt + 0x0: disabled + 0x1: enabled + -bit 3: Half Transfer Mode Error Interrupt + 0x0: disabled + 0x1: enabled + -bit 4: Transfer Complete Interrupt + 0x0: disabled + 0x1: enabled + -bit 9: Peripheral Increment Address + 0x0: no address increment between transfers + 0x1: increment address between transfers + -bit 10: Memory Increment Address + 0x0: no address increment between transfers + 0x1: increment address between transfers + -bit 15: Peripheral Increment Offset Size + 0x0: offset size is linked to the peripheral bus width + 0x1: offset size is fixed to 4 (32-bit alignment) + -bit 16-17: Priority level + 0x0: low + 0x1: medium + 0x2: high + 0x3: very high +5. A 32bit mask specifying the DMA FIFO configuration + -bit 0-1: Fifo threshold + 0x0: 1/4 full FIFO + 0x1: 1/2 full FIFO + 0x2: 3/4 full FIFO + 0x3:full FIFO + -bit 2: Direct mode + 0x0: enabled + 0x1: disabled + -bit 7: FIFO Error Interrupt + 0x0: disabled + 0x1: enabled + +Example: + + usart1: serial@40011000 { + compatible = "st,stm32-usart", "st,stm32-uart"; + reg = <0x40011000 0x400>; + interrupts = <37>; + clocks = <&clk_pclk2>; + dmas = <&dma2 2 4 0x20610 0x3>, + <&dma2 7 5 0x20610 0x3>; + dma-names = "rx", "tx"; + };
This patch adds documentation of device tree bindings for the STM32 dma controller. Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com> --- .../devicetree/bindings/dma/stm32-dma.txt | 98 ++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 Documentation/devicetree/bindings/dma/stm32-dma.txt