| Message ID | 1449225915-28879-1-git-send-email-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, Peter Ujfalusi <peter.ujfalusi@ti.com> writes: > @@ -174,12 +182,44 @@ > }; > > edma: edma@49000000 { > - compatible = "ti,edma3"; > - ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2"; > - reg = <0x49000000 0x10000>, > - <0x44e10f90 0x40>; > + compatible = "ti,edma3-tpcc"; > + ti,hwmods = "tpcc"; > + reg = <0x49000000 0x10000>; > + reg-names = "edma3_cc"; > interrupts = <12 13 14>; > - #dma-cells = <1>; > + interrupt-names = "edma3_ccint", "emda3_mperr", > + "edma3_ccerrint"; > + dma-requests = <64>; > + #dma-cells = <2>; > + > + ti,tptcs = <&edma_tptc0 7>, <&edma_tptc1 5>, > + <&edma_tptc2 0>; > + > + ti,edma-memcpy-channels = /bits/ 16 <20 21>; can you explain this property here ? Are you setting bits 20 and 21 on a 16-bit field ?
* Felipe Balbi <balbi@ti.com> [151204 09:23]: > > Hi, > > Peter Ujfalusi <peter.ujfalusi@ti.com> writes: > > @@ -174,12 +182,44 @@ > > }; > > > > edma: edma@49000000 { > > - compatible = "ti,edma3"; > > - ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2"; > > - reg = <0x49000000 0x10000>, > > - <0x44e10f90 0x40>; > > + compatible = "ti,edma3-tpcc"; > > + ti,hwmods = "tpcc"; > > + reg = <0x49000000 0x10000>; > > + reg-names = "edma3_cc"; > > interrupts = <12 13 14>; > > - #dma-cells = <1>; > > + interrupt-names = "edma3_ccint", "emda3_mperr", > > + "edma3_ccerrint"; > > + dma-requests = <64>; > > + #dma-cells = <2>; > > + > > + ti,tptcs = <&edma_tptc0 7>, <&edma_tptc1 5>, > > + <&edma_tptc2 0>; > > + > > + ti,edma-memcpy-channels = /bits/ 16 <20 21>; > > can you explain this property here ? Are you setting bits 20 and 21 on a > 16-bit field ? I think it's an arry of u16 dma channels.. But could it be just /bits/ 8 instead of /bits/ 16? Regards, Tony
On Friday 04 December 2015 10:47:07 Tony Lindgren wrote: > > Peter Ujfalusi <peter.ujfalusi@ti.com> writes: > > > @@ -174,12 +182,44 @@ > > > }; > > > > > > edma: edma@49000000 { > > > - compatible = "ti,edma3"; > > > - ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2"; > > > - reg = <0x49000000 0x10000>, > > > - <0x44e10f90 0x40>; > > > + compatible = "ti,edma3-tpcc"; > > > + ti,hwmods = "tpcc"; > > > + reg = <0x49000000 0x10000>; > > > + reg-names = "edma3_cc"; > > > interrupts = <12 13 14>; > > > - #dma-cells = <1>; > > > + interrupt-names = "edma3_ccint", "emda3_mperr", > > > + "edma3_ccerrint"; > > > + dma-requests = <64>; > > > + #dma-cells = <2>; > > > + > > > + ti,tptcs = <&edma_tptc0 7>, <&edma_tptc1 5>, > > > + <&edma_tptc2 0>; > > > + > > > + ti,edma-memcpy-channels = /bits/ 16 <20 21>; > > > > can you explain this property here ? Are you setting bits 20 and 21 on a > > 16-bit field ? > > I think it's an arry of u16 dma channels.. But could it be just /bits/ 8 > instead of /bits/ 16? > Please just drop the /bits/ 16 and use normal cells. Arnd
* Arnd Bergmann <arnd@arndb.de> [151204 13:38]: > On Friday 04 December 2015 10:47:07 Tony Lindgren wrote: > > > Peter Ujfalusi <peter.ujfalusi@ti.com> writes: > > > > @@ -174,12 +182,44 @@ > > > > }; > > > > > > > > edma: edma@49000000 { > > > > - compatible = "ti,edma3"; > > > > - ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2"; > > > > - reg = <0x49000000 0x10000>, > > > > - <0x44e10f90 0x40>; > > > > + compatible = "ti,edma3-tpcc"; > > > > + ti,hwmods = "tpcc"; > > > > + reg = <0x49000000 0x10000>; > > > > + reg-names = "edma3_cc"; > > > > interrupts = <12 13 14>; > > > > - #dma-cells = <1>; > > > > + interrupt-names = "edma3_ccint", "emda3_mperr", > > > > + "edma3_ccerrint"; > > > > + dma-requests = <64>; > > > > + #dma-cells = <2>; > > > > + > > > > + ti,tptcs = <&edma_tptc0 7>, <&edma_tptc1 5>, > > > > + <&edma_tptc2 0>; > > > > + > > > > + ti,edma-memcpy-channels = /bits/ 16 <20 21>; > > > > > > can you explain this property here ? Are you setting bits 20 and 21 on a > > > 16-bit field ? > > > > I think it's an arry of u16 dma channels.. But could it be just /bits/ 8 > > instead of /bits/ 16? > > > > Please just drop the /bits/ 16 and use normal cells. Yeah agreed, makes things less confusing for sure :) Tony
* Peter Ujfalusi <peter.ujfalusi@ti.com> [151204 02:46]: > + > + ti,tptcs = <&edma_tptc0 7>, <&edma_tptc1 5>, > + <&edma_tptc2 0>; > + > + ti,edma-memcpy-channels = /bits/ 16 <20 21>; Is this safe to change to just ti,edma-memcpy-channels = <20 21> as suggested by Arnd? Regards, Tony
On 12/04/2015 11:51 PM, Tony Lindgren wrote: > * Arnd Bergmann <arnd@arndb.de> [151204 13:38]: >> On Friday 04 December 2015 10:47:07 Tony Lindgren wrote: >>>> Peter Ujfalusi <peter.ujfalusi@ti.com> writes: >>>>> @@ -174,12 +182,44 @@ >>>>> }; >>>>> >>>>> edma: edma@49000000 { >>>>> - compatible = "ti,edma3"; >>>>> - ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2"; >>>>> - reg = <0x49000000 0x10000>, >>>>> - <0x44e10f90 0x40>; >>>>> + compatible = "ti,edma3-tpcc"; >>>>> + ti,hwmods = "tpcc"; >>>>> + reg = <0x49000000 0x10000>; >>>>> + reg-names = "edma3_cc"; >>>>> interrupts = <12 13 14>; >>>>> - #dma-cells = <1>; >>>>> + interrupt-names = "edma3_ccint", "emda3_mperr", >>>>> + "edma3_ccerrint"; >>>>> + dma-requests = <64>; >>>>> + #dma-cells = <2>; >>>>> + >>>>> + ti,tptcs = <&edma_tptc0 7>, <&edma_tptc1 5>, >>>>> + <&edma_tptc2 0>; >>>>> + >>>>> + ti,edma-memcpy-channels = /bits/ 16 <20 21>; >>>> >>>> can you explain this property here ? Are you setting bits 20 and 21 on a >>>> 16-bit field ? >>> >>> I think it's an arry of u16 dma channels.. But could it be just /bits/ 8 >>> instead of /bits/ 16? >>> >> >> Please just drop the /bits/ 16 and use normal cells. > > Yeah agreed, makes things less confusing for sure :) 4.4 will be the first kernel where we will have the new eDMA bindings. I have chosen to use 16bit array for specifying the channels used for memcpy (ti,edma-memcpy-channels) and for the reserving paRAM slots (ti,edma-reserved-slot-ranges). As of now we have maximum of 64 channels and 512 paRAM slots. 16bit is more than enough to store this information and it even gives us enough room if ever in the future these numbers are going to increase (which they are not). But in order to change them to 32bit the driver needs to be changed as well. Currently we do not have drivers (in 4.4) using the new bindings, 4.4 is not yet out, so it might be possible to change the binding document and the driver to use 32bit arrays. The driver internally uses 16bit type for these which I'm not going to change, but the code parsing the DT needs to be adjusted for the new data type. If Vinod is willing to take update for the DT binding of eDMA for 4.4-rc, I can cook up the patch(es) to do so.
On 12/08/2015 02:19 AM, Tony Lindgren wrote: > * Peter Ujfalusi <peter.ujfalusi@ti.com> [151204 02:46]: >> + >> + ti,tptcs = <&edma_tptc0 7>, <&edma_tptc1 5>, >> + <&edma_tptc2 0>; >> + >> + ti,edma-memcpy-channels = /bits/ 16 <20 21>; > > Is this safe to change to just ti,edma-memcpy-channels = <20 21> as > suggested by Arnd? The binding document for the eDMA and the driver also needs update to be able to handle this change.
On Tuesday 08 December 2015 09:42:26 Peter Ujfalusi wrote: > On 12/04/2015 11:51 PM, Tony Lindgren wrote: > >> > >> Please just drop the /bits/ 16 and use normal cells. > > > > Yeah agreed, makes things less confusing for sure > > 4.4 will be the first kernel where we will have the new eDMA bindings. I have > chosen to use 16bit array for specifying the channels used for memcpy > (ti,edma-memcpy-channels) and for the reserving paRAM slots > (ti,edma-reserved-slot-ranges). As of now we have maximum of 64 channels and > 512 paRAM slots. 16bit is more than enough to store this information and it > even gives us enough room if ever in the future these numbers are going to > increase (which they are not). > > But in order to change them to 32bit the driver needs to be changed as well. > Currently we do not have drivers (in 4.4) using the new bindings, 4.4 is not > yet out, so it might be possible to change the binding document and the driver > to use 32bit arrays. The driver internally uses 16bit type for these which I'm > not going to change, but the code parsing the DT needs to be adjusted for the > new data type. > > If Vinod is willing to take update for the DT binding of eDMA for 4.4-rc, I > can cook up the patch(es) to do so. I hadn't realized that it was already in 4.4-rc. The change should be trivial enough though, so I'd still do it. If Vinod would rather not change it now, it's not overly important though. Arnd
On 12/08/2015 11:51 AM, Arnd Bergmann wrote: > On Tuesday 08 December 2015 09:42:26 Peter Ujfalusi wrote: >> On 12/04/2015 11:51 PM, Tony Lindgren wrote: >>>> >>>> Please just drop the /bits/ 16 and use normal cells. >>> >>> Yeah agreed, makes things less confusing for sure >> >> 4.4 will be the first kernel where we will have the new eDMA bindings. I have >> chosen to use 16bit array for specifying the channels used for memcpy >> (ti,edma-memcpy-channels) and for the reserving paRAM slots >> (ti,edma-reserved-slot-ranges). As of now we have maximum of 64 channels and >> 512 paRAM slots. 16bit is more than enough to store this information and it >> even gives us enough room if ever in the future these numbers are going to >> increase (which they are not). >> >> But in order to change them to 32bit the driver needs to be changed as well. >> Currently we do not have drivers (in 4.4) using the new bindings, 4.4 is not >> yet out, so it might be possible to change the binding document and the driver >> to use 32bit arrays. The driver internally uses 16bit type for these which I'm >> not going to change, but the code parsing the DT needs to be adjusted for the >> new data type. >> >> If Vinod is willing to take update for the DT binding of eDMA for 4.4-rc, I >> can cook up the patch(es) to do so. > > I hadn't realized that it was already in 4.4-rc. The change should be trivial > enough though, so I'd still do it. If Vinod would rather not change it now, > it's not overly important though. But this change must be done before we have actual users of these properties, which is the am33xx, am437x and the da850 conversion series I have sent recently. We might want to have this changed for 4.4 since it is going to be an LTS release...
On Tuesday 08 December 2015 12:22:09 Peter Ujfalusi wrote: > On 12/08/2015 11:51 AM, Arnd Bergmann wrote: > > On Tuesday 08 December 2015 09:42:26 Peter Ujfalusi wrote: > >> On 12/04/2015 11:51 PM, Tony Lindgren wrote: > >>>> > >>>> Please just drop the /bits/ 16 and use normal cells. > >>> > >>> Yeah agreed, makes things less confusing for sure > >> > >> 4.4 will be the first kernel where we will have the new eDMA bindings. I have > >> chosen to use 16bit array for specifying the channels used for memcpy > >> (ti,edma-memcpy-channels) and for the reserving paRAM slots > >> (ti,edma-reserved-slot-ranges). As of now we have maximum of 64 channels and > >> 512 paRAM slots. 16bit is more than enough to store this information and it > >> even gives us enough room if ever in the future these numbers are going to > >> increase (which they are not). > >> > >> But in order to change them to 32bit the driver needs to be changed as well. > >> Currently we do not have drivers (in 4.4) using the new bindings, 4.4 is not > >> yet out, so it might be possible to change the binding document and the driver > >> to use 32bit arrays. The driver internally uses 16bit type for these which I'm > >> not going to change, but the code parsing the DT needs to be adjusted for the > >> new data type. > >> > >> If Vinod is willing to take update for the DT binding of eDMA for 4.4-rc, I > >> can cook up the patch(es) to do so. > > > > I hadn't realized that it was already in 4.4-rc. The change should be trivial > > enough though, so I'd still do it. If Vinod would rather not change it now, > > it's not overly important though. > > But this change must be done before we have actual users of these properties, > which is the am33xx, am437x and the da850 conversion series I have sent recently. > We might want to have this changed for 4.4 since it is going to be an LTS > release... Yes, that's what I meant: We either get the patch into 4.4 by creating a branch for Vinod to pull with this change, and base all other changes in your 4.5 series on the same branch, or we don't change it at all. Arnd
* Arnd Bergmann <arnd@arndb.de> [151208 02:26]: > On Tuesday 08 December 2015 12:22:09 Peter Ujfalusi wrote: > > On 12/08/2015 11:51 AM, Arnd Bergmann wrote: > > > On Tuesday 08 December 2015 09:42:26 Peter Ujfalusi wrote: > > >> On 12/04/2015 11:51 PM, Tony Lindgren wrote: > > >>>> > > >>>> Please just drop the /bits/ 16 and use normal cells. > > >>> > > >>> Yeah agreed, makes things less confusing for sure > > >> > > >> 4.4 will be the first kernel where we will have the new eDMA bindings. I have > > >> chosen to use 16bit array for specifying the channels used for memcpy > > >> (ti,edma-memcpy-channels) and for the reserving paRAM slots > > >> (ti,edma-reserved-slot-ranges). As of now we have maximum of 64 channels and > > >> 512 paRAM slots. 16bit is more than enough to store this information and it > > >> even gives us enough room if ever in the future these numbers are going to > > >> increase (which they are not). > > >> > > >> But in order to change them to 32bit the driver needs to be changed as well. > > >> Currently we do not have drivers (in 4.4) using the new bindings, 4.4 is not > > >> yet out, so it might be possible to change the binding document and the driver > > >> to use 32bit arrays. The driver internally uses 16bit type for these which I'm > > >> not going to change, but the code parsing the DT needs to be adjusted for the > > >> new data type. > > >> > > >> If Vinod is willing to take update for the DT binding of eDMA for 4.4-rc, I > > >> can cook up the patch(es) to do so. > > > > > > I hadn't realized that it was already in 4.4-rc. The change should be trivial > > > enough though, so I'd still do it. If Vinod would rather not change it now, > > > it's not overly important though. > > > > But this change must be done before we have actual users of these properties, > > which is the am33xx, am437x and the da850 conversion series I have sent recently. > > We might want to have this changed for 4.4 since it is going to be an LTS > > release... > > Yes, that's what I meant: We either get the patch into 4.4 by creating a > branch for Vinod to pull with this change, and base all other changes in your > 4.5 series on the same branch, or we don't change it at all. Sounds good to me. If there's an immutable branch against v4.4-rc1 with just that change in it will make our lives easier. Regards, Tony
diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts index 4caf074063fe..a55072fb6646 100644 --- a/arch/arm/boot/dts/am335x-evm.dts +++ b/arch/arm/boot/dts/am335x-evm.dts @@ -743,8 +743,8 @@ &mmc3 { /* these are on the crossbar and are outlined in the xbar-event-map element */ - dmas = <&edma 12 - &edma 13>; + dmas = <&edma_xbar 12 0 1 + &edma_xbar 13 0 2>; dma-names = "tx", "rx"; status = "okay"; vmmc-supply = <&wlan_en_reg>; @@ -766,11 +766,6 @@ }; }; -&edma { - ti,edma-xbar-event-map = /bits/ 16 <1 12 - 2 13>; -}; - &sham { status = "okay"; }; diff --git a/arch/arm/boot/dts/am335x-pepper.dts b/arch/arm/boot/dts/am335x-pepper.dts index 9cb77a120319..4dd4f71498e5 100644 --- a/arch/arm/boot/dts/am335x-pepper.dts +++ b/arch/arm/boot/dts/am335x-pepper.dts @@ -339,13 +339,6 @@ ti,non-removable; }; -&edma { - /* Map eDMA MMC2 Events from Crossbar */ - ti,edma-xbar-event-map = /bits/ 16 <1 12 - 2 13>; -}; - - &mmc3 { /* Wifi & Bluetooth on MMC #3 */ status = "okay"; @@ -354,8 +347,8 @@ vmmmc-supply = <&v3v3c_reg>; bus-width = <4>; ti,non-removable; - dmas = <&edma 12 - &edma 13>; + dmas = <&edma_xbar 12 0 1 + &edma_xbar 13 0 2>; dma-names = "tx", "rx"; }; diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index 9b8861891bf0..9d414ed31e99 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -161,6 +161,14 @@ mboxes = <&mailbox &mbox_wkupm3>; }; + edma_xbar: dma-router@f90 { + compatible = "ti,am335x-edma-crossbar"; + reg = <0xf90 0x40>; + #dma-cells = <3>; + dma-requests = <32>; + dma-masters = <&edma>; + }; + scm_clockdomains: clockdomains { }; }; @@ -174,12 +182,44 @@ }; edma: edma@49000000 { - compatible = "ti,edma3"; - ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2"; - reg = <0x49000000 0x10000>, - <0x44e10f90 0x40>; + compatible = "ti,edma3-tpcc"; + ti,hwmods = "tpcc"; + reg = <0x49000000 0x10000>; + reg-names = "edma3_cc"; interrupts = <12 13 14>; - #dma-cells = <1>; + interrupt-names = "edma3_ccint", "emda3_mperr", + "edma3_ccerrint"; + dma-requests = <64>; + #dma-cells = <2>; + + ti,tptcs = <&edma_tptc0 7>, <&edma_tptc1 5>, + <&edma_tptc2 0>; + + ti,edma-memcpy-channels = /bits/ 16 <20 21>; + }; + + edma_tptc0: tptc@49800000 { + compatible = "ti,edma3-tptc"; + ti,hwmods = "tptc0"; + reg = <0x49800000 0x100000>; + interrupts = <112>; + interrupt-names = "edma3_tcerrint"; + }; + + edma_tptc1: tptc@49900000 { + compatible = "ti,edma3-tptc"; + ti,hwmods = "tptc1"; + reg = <0x49900000 0x100000>; + interrupts = <113>; + interrupt-names = "edma3_tcerrint"; + }; + + edma_tptc2: tptc@49a00000 { + compatible = "ti,edma3-tptc"; + ti,hwmods = "tptc2"; + reg = <0x49a00000 0x100000>; + interrupts = <114>; + interrupt-names = "edma3_tcerrint"; }; gpio0: gpio@44e07000 { @@ -233,7 +273,7 @@ reg = <0x44e09000 0x2000>; interrupts = <72>; status = "disabled"; - dmas = <&edma 26>, <&edma 27>; + dmas = <&edma 26 0>, <&edma 27 0>; dma-names = "tx", "rx"; }; @@ -244,7 +284,7 @@ reg = <0x48022000 0x2000>; interrupts = <73>; status = "disabled"; - dmas = <&edma 28>, <&edma 29>; + dmas = <&edma 28 0>, <&edma 29 0>; dma-names = "tx", "rx"; }; @@ -255,7 +295,7 @@ reg = <0x48024000 0x2000>; interrupts = <74>; status = "disabled"; - dmas = <&edma 30>, <&edma 31>; + dmas = <&edma 30 0>, <&edma 31 0>; dma-names = "tx", "rx"; }; @@ -322,8 +362,8 @@ ti,dual-volt; ti,needs-special-reset; ti,needs-special-hs-handling; - dmas = <&edma 24 - &edma 25>; + dmas = <&edma_xbar 24 0 0 + &edma_xbar 25 0 0>; dma-names = "tx", "rx"; interrupts = <64>; interrupt-parent = <&intc>; @@ -335,8 +375,8 @@ compatible = "ti,omap4-hsmmc"; ti,hwmods = "mmc2"; ti,needs-special-reset; - dmas = <&edma 2 - &edma 3>; + dmas = <&edma 2 0 + &edma 3 0>; dma-names = "tx", "rx"; interrupts = <28>; interrupt-parent = <&intc>; @@ -474,10 +514,10 @@ interrupts = <65>; ti,spi-num-cs = <2>; ti,hwmods = "spi0"; - dmas = <&edma 16 - &edma 17 - &edma 18 - &edma 19>; + dmas = <&edma 16 0 + &edma 17 0 + &edma 18 0 + &edma 19 0>; dma-names = "tx0", "rx0", "tx1", "rx1"; status = "disabled"; }; @@ -490,10 +530,10 @@ interrupts = <125>; ti,spi-num-cs = <2>; ti,hwmods = "spi1"; - dmas = <&edma 42 - &edma 43 - &edma 44 - &edma 45>; + dmas = <&edma 42 0 + &edma 43 0 + &edma 44 0 + &edma 45 0>; dma-names = "tx0", "rx0", "tx1", "rx1"; status = "disabled"; }; @@ -833,7 +873,7 @@ ti,hwmods = "sham"; reg = <0x53100000 0x200>; interrupts = <109>; - dmas = <&edma 36>; + dmas = <&edma 36 0>; dma-names = "rx"; }; @@ -842,8 +882,8 @@ ti,hwmods = "aes"; reg = <0x53500000 0xa0>; interrupts = <103>; - dmas = <&edma 6>, - <&edma 5>; + dmas = <&edma 6 0>, + <&edma 5 0>; dma-names = "tx", "rx"; }; @@ -856,8 +896,8 @@ interrupts = <80>, <81>; interrupt-names = "tx", "rx"; status = "disabled"; - dmas = <&edma 8>, - <&edma 9>; + dmas = <&edma 8 2>, + <&edma 9 2>; dma-names = "tx", "rx"; }; @@ -870,8 +910,8 @@ interrupts = <82>, <83>; interrupt-names = "tx", "rx"; status = "disabled"; - dmas = <&edma 10>, - <&edma 11>; + dmas = <&edma 10 2>, + <&edma 11 2>; dma-names = "tx", "rx"; };
Switch to use the ti,edma3-tpcc and ti,edma3-tptc binding for the eDMA3 and enable the DMA even crossbar with ti,am335x-edma-crossbar. With the new bindings boards can customize and tweak the DMA channel priority to match their needs. With the new binding the memcpy is safe to be used since with the old binding it was not possible for a driver to know which channel is allowed to be used as non HW triggered channel. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- arch/arm/boot/dts/am335x-evm.dts | 9 +--- arch/arm/boot/dts/am335x-pepper.dts | 11 +---- arch/arm/boot/dts/am33xx.dtsi | 94 ++++++++++++++++++++++++++----------- 3 files changed, 71 insertions(+), 43 deletions(-)