[linux-sunxi] Re: [PATCH v10 2/2] dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller
On Mon, Jul 28, 2014 at 05:09:11PM +0200, Maxime Ripard wrote: On Sun, Jul 27, 2014 at 07:49:42PM +0100, Mark Brown wrote: There's supposed to be facilities appearing in the generic clock code for specifying default clock tree configurations via the DT - it's quite a common requirement after all. It sounds like that should be able to do the job here. Not sure if these have gone in yet or not but there's quite a bit of demand. As far as I'm aware, it's not been merged yet. Well, another reason to push Mike about it then I guess - there are a lot of these case cropping up. signature.asc Description: Digital signature
[linux-sunxi] Re: [PATCH v10 2/2] dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller
On Tue, Jul 01, 2014 at 01:48:52PM +0100, Mark Rutland wrote: On Tue, Jul 01, 2014 at 08:21:19AM +0100, Maxime Ripard wrote: It feels a little fragile to rely on the organisation of the clock tree and the naming thereof. If the IP block is ever reused on an SoC with a different clock tree layout then we have to handle things differently. What do you suggest then? I will admit that I don't have a better suggestion. Without knowing which particular constraint on the mux parent clock we care about it's difficult to suggest anything useful. There's supposed to be facilities appearing in the generic clock code for specifying default clock tree configurations via the DT - it's quite a common requirement after all. It sounds like that should be able to do the job here. Not sure if these have gone in yet or not but there's quite a bit of demand. signature.asc Description: Digital signature
[linux-sunxi] Re: [PATCH v10 2/2] dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller
Hi Mark, On Sun, Jul 27, 2014 at 07:49:42PM +0100, Mark Brown wrote: On Tue, Jul 01, 2014 at 01:48:52PM +0100, Mark Rutland wrote: On Tue, Jul 01, 2014 at 08:21:19AM +0100, Maxime Ripard wrote: It feels a little fragile to rely on the organisation of the clock tree and the naming thereof. If the IP block is ever reused on an SoC with a different clock tree layout then we have to handle things differently. What do you suggest then? I will admit that I don't have a better suggestion. Without knowing which particular constraint on the mux parent clock we care about it's difficult to suggest anything useful. There's supposed to be facilities appearing in the generic clock code for specifying default clock tree configurations via the DT - it's quite a common requirement after all. It sounds like that should be able to do the job here. Not sure if these have gone in yet or not but there's quite a bit of demand. As far as I'm aware, it's not been merged yet. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
[linux-sunxi] Re: [PATCH v10 2/2] dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller
On Friday 04 July 2014, Maxime Ripard wrote: It feels a little fragile to rely on the organisation of the clock tree and the naming thereof. If the IP block is ever reused on an SoC with a different clock tree layout then we have to handle things differently. What do you suggest then? I will admit that I don't have a better suggestion. Without knowing which particular constraint on the mux parent clock we care about it's difficult to suggest anything useful. Well, I first made it into the mach- directory, and then was told to move it in the driver itself, so we're kind of running out of options :) How about having a property in the clock provider node that forces a specific value for the mux? I think that's generally the preferred solution for any clock settings that go beyond what an OS can be expected to figure out for itself. Arnd -- You received this message because you are subscribed to the Google Groups linux-sunxi group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v10 2/2] dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller
On Sun, Jul 06, 2014 at 05:22:00PM +0200, Arnd Bergmann wrote: On Friday 04 July 2014, Maxime Ripard wrote: It feels a little fragile to rely on the organisation of the clock tree and the naming thereof. If the IP block is ever reused on an SoC with a different clock tree layout then we have to handle things differently. What do you suggest then? I will admit that I don't have a better suggestion. Without knowing which particular constraint on the mux parent clock we care about it's difficult to suggest anything useful. Well, I first made it into the mach- directory, and then was told to move it in the driver itself, so we're kind of running out of options :) How about having a property in the clock provider node that forces a specific value for the mux? I think that's generally the preferred solution for any clock settings that go beyond what an OS can be expected to figure out for itself. Except that we don't really care about the parenting if the device isn't going to be used, so putting this property on the clock provider doesn't look that good. Plus, in the case where we have multiple clocks defined in a single node, it wouldn't work that great. However, I'm not convinced that it should be on the device node either, because we could end up having conflicts between devices. I don't know, some C code seems like the easier and more flexible solution here. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
[linux-sunxi] Re: [PATCH v10 2/2] dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller
Hi Mark,
On Tue, Jul 01, 2014 at 01:48:52PM +0100, Mark Rutland wrote:
On Tue, Jul 01, 2014 at 08:21:19AM +0100, Maxime Ripard wrote:
On Mon, Jun 30, 2014 at 04:33:05PM +0100, Mark Rutland wrote:
On Mon, Jun 30, 2014 at 04:19:06PM +0100, Maxime Ripard wrote:
On Mon, Jun 30, 2014 at 03:20:54PM +0100, Mark Rutland wrote:
Hi Maxime,
On Mon, Jun 30, 2014 at 02:20:54PM +0100, Maxime Ripard wrote:
The Allwinner A31 has a 16 channels DMA controller that it shares
with the
newer A23. Although sharing some similarities with the DMA
controller of the
older Allwinner SoCs, it's significantly different, I don't expect
it to be
possible to share the driver for these two.
The A31 Controller is able to memory-to-memory or memory-to-device
transfers on
the 16 channels in parallel.
Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com
Acked-by: Arnd Bergmann a...@arndb.de
---
drivers/dma/Kconfig |8 +
drivers/dma/Makefile|1 +
drivers/dma/sun6i-dma.c | 1058
+++
3 files changed, 1067 insertions(+)
create mode 100644 drivers/dma/sun6i-dma.c
[...]
+ sdc-clk = devm_clk_get(pdev-dev, NULL);
+ if (IS_ERR(sdc-clk)) {
+ dev_err(pdev-dev, No clock specified\n);
+ return PTR_ERR(sdc-clk);
+ }
+
+ mux = clk_get(NULL, ahb1_mux);
+ if (IS_ERR(mux)) {
+ dev_err(pdev-dev, Couldn't get AHB1 Mux\n);
+ return PTR_ERR(mux);
+ }
+
+ pll6 = clk_get(NULL, pll6);
+ if (IS_ERR(pll6)) {
+ dev_err(pdev-dev, Couldn't get PLL6\n);
+ clk_put(mux);
+ return PTR_ERR(pll6);
+ }
I'm slightly confused. The binding listed a single unnamed clock (the
AHB clock). What is going on here?
The device itself needs only a single clock to work...
+ ret = clk_set_parent(mux, pll6);
+ clk_put(pll6);
+ clk_put(mux);
+
+ if (ret) {
+ dev_err(pdev-dev, Couldn't reparent AHB1 on
PLL6\n);
+ return ret;
+ }
Why do we need to reparent the mux?
... but will function only if this clock is derived from PLL6.
Ok, but _why_ is that the case? Could we at least have a comment for
that?
I have no idea, sorry.
Oh no :(
Where did this requirement turn up? Did an issue crop up in testing and
using PLL6 just happened to work, or does some manual somewhere say that
the device must have its clock derived from PLL6 without further
explanation?
I first tried with the default clocks (the 24MHz oscillator) and only
transfers to devices and/or SRAM we working, but transfers from/to the
DRAM were not. Switching to PLL6 was making everything work fine. It
seems to be a bug in the A31 itself, but I don't have much details
yet.
Does PLL6 drive some other clock that the DMA clock needs to stay in
sync with, possibly?
PLL6 also drives some peripherals directly yes (such as the MMC or the
SPI controllers), but since I tested this using the SPI, with the main
oscillator as a source for both, I don't think it's related to
that. Or at least, not as directly related as what you were
suggesting.
Where does the driver get the named clocks from if they aren't provided
on the device node? Is there a clock-ranges somewhere?
No, it just looks up the global clock name.
Ok.
It feels a little fragile to rely on the organisation of the clock tree
and the naming thereof. If the IP block is ever reused on an SoC with a
different clock tree layout then we have to handle things differently.
What do you suggest then?
I will admit that I don't have a better suggestion.
Without knowing which particular constraint on the mux parent clock we
care about it's difficult to suggest anything useful.
Well, I first made it into the mach- directory, and then was told to
move it in the driver itself, so we're kind of running out of options
:)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
[linux-sunxi] Re: [PATCH v10 2/2] dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller
On Mon, Jun 30, 2014 at 04:33:05PM +0100, Mark Rutland wrote:
On Mon, Jun 30, 2014 at 04:19:06PM +0100, Maxime Ripard wrote:
On Mon, Jun 30, 2014 at 03:20:54PM +0100, Mark Rutland wrote:
Hi Maxime,
On Mon, Jun 30, 2014 at 02:20:54PM +0100, Maxime Ripard wrote:
The Allwinner A31 has a 16 channels DMA controller that it shares with
the
newer A23. Although sharing some similarities with the DMA controller
of the
older Allwinner SoCs, it's significantly different, I don't expect it
to be
possible to share the driver for these two.
The A31 Controller is able to memory-to-memory or memory-to-device
transfers on
the 16 channels in parallel.
Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com
Acked-by: Arnd Bergmann a...@arndb.de
---
drivers/dma/Kconfig |8 +
drivers/dma/Makefile|1 +
drivers/dma/sun6i-dma.c | 1058
+++
3 files changed, 1067 insertions(+)
create mode 100644 drivers/dma/sun6i-dma.c
[...]
+ sdc-clk = devm_clk_get(pdev-dev, NULL);
+ if (IS_ERR(sdc-clk)) {
+ dev_err(pdev-dev, No clock specified\n);
+ return PTR_ERR(sdc-clk);
+ }
+
+ mux = clk_get(NULL, ahb1_mux);
+ if (IS_ERR(mux)) {
+ dev_err(pdev-dev, Couldn't get AHB1 Mux\n);
+ return PTR_ERR(mux);
+ }
+
+ pll6 = clk_get(NULL, pll6);
+ if (IS_ERR(pll6)) {
+ dev_err(pdev-dev, Couldn't get PLL6\n);
+ clk_put(mux);
+ return PTR_ERR(pll6);
+ }
I'm slightly confused. The binding listed a single unnamed clock (the
AHB clock). What is going on here?
The device itself needs only a single clock to work...
+ ret = clk_set_parent(mux, pll6);
+ clk_put(pll6);
+ clk_put(mux);
+
+ if (ret) {
+ dev_err(pdev-dev, Couldn't reparent AHB1 on PLL6\n);
+ return ret;
+ }
Why do we need to reparent the mux?
... but will function only if this clock is derived from PLL6.
Ok, but _why_ is that the case? Could we at least have a comment for
that?
I have no idea, sorry.
Where does the driver get the named clocks from if they aren't provided
on the device node? Is there a clock-ranges somewhere?
No, it just looks up the global clock name.
It feels a little fragile to rely on the organisation of the clock tree
and the naming thereof. If the IP block is ever reused on an SoC with a
different clock tree layout then we have to handle things differently.
What do you suggest then?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
[linux-sunxi] Re: [PATCH v10 2/2] dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller
On Tue, Jul 01, 2014 at 08:21:19AM +0100, Maxime Ripard wrote:
On Mon, Jun 30, 2014 at 04:33:05PM +0100, Mark Rutland wrote:
On Mon, Jun 30, 2014 at 04:19:06PM +0100, Maxime Ripard wrote:
On Mon, Jun 30, 2014 at 03:20:54PM +0100, Mark Rutland wrote:
Hi Maxime,
On Mon, Jun 30, 2014 at 02:20:54PM +0100, Maxime Ripard wrote:
The Allwinner A31 has a 16 channels DMA controller that it shares
with the
newer A23. Although sharing some similarities with the DMA controller
of the
older Allwinner SoCs, it's significantly different, I don't expect it
to be
possible to share the driver for these two.
The A31 Controller is able to memory-to-memory or memory-to-device
transfers on
the 16 channels in parallel.
Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com
Acked-by: Arnd Bergmann a...@arndb.de
---
drivers/dma/Kconfig |8 +
drivers/dma/Makefile|1 +
drivers/dma/sun6i-dma.c | 1058
+++
3 files changed, 1067 insertions(+)
create mode 100644 drivers/dma/sun6i-dma.c
[...]
+ sdc-clk = devm_clk_get(pdev-dev, NULL);
+ if (IS_ERR(sdc-clk)) {
+ dev_err(pdev-dev, No clock specified\n);
+ return PTR_ERR(sdc-clk);
+ }
+
+ mux = clk_get(NULL, ahb1_mux);
+ if (IS_ERR(mux)) {
+ dev_err(pdev-dev, Couldn't get AHB1 Mux\n);
+ return PTR_ERR(mux);
+ }
+
+ pll6 = clk_get(NULL, pll6);
+ if (IS_ERR(pll6)) {
+ dev_err(pdev-dev, Couldn't get PLL6\n);
+ clk_put(mux);
+ return PTR_ERR(pll6);
+ }
I'm slightly confused. The binding listed a single unnamed clock (the
AHB clock). What is going on here?
The device itself needs only a single clock to work...
+ ret = clk_set_parent(mux, pll6);
+ clk_put(pll6);
+ clk_put(mux);
+
+ if (ret) {
+ dev_err(pdev-dev, Couldn't reparent AHB1 on
PLL6\n);
+ return ret;
+ }
Why do we need to reparent the mux?
... but will function only if this clock is derived from PLL6.
Ok, but _why_ is that the case? Could we at least have a comment for
that?
I have no idea, sorry.
Oh no :(
Where did this requirement turn up? Did an issue crop up in testing and
using PLL6 just happened to work, or does some manual somewhere say that
the device must have its clock derived from PLL6 without further
explanation?
Does PLL6 drive some other clock that the DMA clock needs to stay in
sync with, possibly?
Where does the driver get the named clocks from if they aren't provided
on the device node? Is there a clock-ranges somewhere?
No, it just looks up the global clock name.
Ok.
It feels a little fragile to rely on the organisation of the clock tree
and the naming thereof. If the IP block is ever reused on an SoC with a
different clock tree layout then we have to handle things differently.
What do you suggest then?
I will admit that I don't have a better suggestion.
Without knowing which particular constraint on the mux parent clock we
care about it's difficult to suggest anything useful.
Mark.
--
You received this message because you are subscribed to the Google Groups
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v10 2/2] dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller
Hi Maxime,
On Mon, Jun 30, 2014 at 02:20:54PM +0100, Maxime Ripard wrote:
The Allwinner A31 has a 16 channels DMA controller that it shares with the
newer A23. Although sharing some similarities with the DMA controller of the
older Allwinner SoCs, it's significantly different, I don't expect it to be
possible to share the driver for these two.
The A31 Controller is able to memory-to-memory or memory-to-device transfers
on
the 16 channels in parallel.
Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com
Acked-by: Arnd Bergmann a...@arndb.de
---
drivers/dma/Kconfig |8 +
drivers/dma/Makefile|1 +
drivers/dma/sun6i-dma.c | 1058
+++
3 files changed, 1067 insertions(+)
create mode 100644 drivers/dma/sun6i-dma.c
[...]
+ sdc-clk = devm_clk_get(pdev-dev, NULL);
+ if (IS_ERR(sdc-clk)) {
+ dev_err(pdev-dev, No clock specified\n);
+ return PTR_ERR(sdc-clk);
+ }
+
+ mux = clk_get(NULL, ahb1_mux);
+ if (IS_ERR(mux)) {
+ dev_err(pdev-dev, Couldn't get AHB1 Mux\n);
+ return PTR_ERR(mux);
+ }
+
+ pll6 = clk_get(NULL, pll6);
+ if (IS_ERR(pll6)) {
+ dev_err(pdev-dev, Couldn't get PLL6\n);
+ clk_put(mux);
+ return PTR_ERR(pll6);
+ }
I'm slightly confused. The binding listed a single unnamed clock (the
AHB clock). What is going on here?
+ ret = clk_set_parent(mux, pll6);
+ clk_put(pll6);
+ clk_put(mux);
+
+ if (ret) {
+ dev_err(pdev-dev, Couldn't reparent AHB1 on PLL6\n);
+ return ret;
+ }
Why do we need to reparent the mux?
Thanks,
Mark.
--
You received this message because you are subscribed to the Google Groups
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v10 2/2] dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller
On Mon, Jun 30, 2014 at 03:20:54PM +0100, Mark Rutland wrote:
Hi Maxime,
On Mon, Jun 30, 2014 at 02:20:54PM +0100, Maxime Ripard wrote:
The Allwinner A31 has a 16 channels DMA controller that it shares with the
newer A23. Although sharing some similarities with the DMA controller of the
older Allwinner SoCs, it's significantly different, I don't expect it to be
possible to share the driver for these two.
The A31 Controller is able to memory-to-memory or memory-to-device
transfers on
the 16 channels in parallel.
Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com
Acked-by: Arnd Bergmann a...@arndb.de
---
drivers/dma/Kconfig |8 +
drivers/dma/Makefile|1 +
drivers/dma/sun6i-dma.c | 1058
+++
3 files changed, 1067 insertions(+)
create mode 100644 drivers/dma/sun6i-dma.c
[...]
+ sdc-clk = devm_clk_get(pdev-dev, NULL);
+ if (IS_ERR(sdc-clk)) {
+ dev_err(pdev-dev, No clock specified\n);
+ return PTR_ERR(sdc-clk);
+ }
+
+ mux = clk_get(NULL, ahb1_mux);
+ if (IS_ERR(mux)) {
+ dev_err(pdev-dev, Couldn't get AHB1 Mux\n);
+ return PTR_ERR(mux);
+ }
+
+ pll6 = clk_get(NULL, pll6);
+ if (IS_ERR(pll6)) {
+ dev_err(pdev-dev, Couldn't get PLL6\n);
+ clk_put(mux);
+ return PTR_ERR(pll6);
+ }
I'm slightly confused. The binding listed a single unnamed clock (the
AHB clock). What is going on here?
The device itself needs only a single clock to work...
+ ret = clk_set_parent(mux, pll6);
+ clk_put(pll6);
+ clk_put(mux);
+
+ if (ret) {
+ dev_err(pdev-dev, Couldn't reparent AHB1 on PLL6\n);
+ return ret;
+ }
Why do we need to reparent the mux?
... but will function only if this clock is derived from PLL6.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
[linux-sunxi] Re: [PATCH v10 2/2] dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller
On Mon, Jun 30, 2014 at 04:19:06PM +0100, Maxime Ripard wrote:
On Mon, Jun 30, 2014 at 03:20:54PM +0100, Mark Rutland wrote:
Hi Maxime,
On Mon, Jun 30, 2014 at 02:20:54PM +0100, Maxime Ripard wrote:
The Allwinner A31 has a 16 channels DMA controller that it shares with the
newer A23. Although sharing some similarities with the DMA controller of
the
older Allwinner SoCs, it's significantly different, I don't expect it to
be
possible to share the driver for these two.
The A31 Controller is able to memory-to-memory or memory-to-device
transfers on
the 16 channels in parallel.
Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com
Acked-by: Arnd Bergmann a...@arndb.de
---
drivers/dma/Kconfig |8 +
drivers/dma/Makefile|1 +
drivers/dma/sun6i-dma.c | 1058
+++
3 files changed, 1067 insertions(+)
create mode 100644 drivers/dma/sun6i-dma.c
[...]
+ sdc-clk = devm_clk_get(pdev-dev, NULL);
+ if (IS_ERR(sdc-clk)) {
+ dev_err(pdev-dev, No clock specified\n);
+ return PTR_ERR(sdc-clk);
+ }
+
+ mux = clk_get(NULL, ahb1_mux);
+ if (IS_ERR(mux)) {
+ dev_err(pdev-dev, Couldn't get AHB1 Mux\n);
+ return PTR_ERR(mux);
+ }
+
+ pll6 = clk_get(NULL, pll6);
+ if (IS_ERR(pll6)) {
+ dev_err(pdev-dev, Couldn't get PLL6\n);
+ clk_put(mux);
+ return PTR_ERR(pll6);
+ }
I'm slightly confused. The binding listed a single unnamed clock (the
AHB clock). What is going on here?
The device itself needs only a single clock to work...
+ ret = clk_set_parent(mux, pll6);
+ clk_put(pll6);
+ clk_put(mux);
+
+ if (ret) {
+ dev_err(pdev-dev, Couldn't reparent AHB1 on PLL6\n);
+ return ret;
+ }
Why do we need to reparent the mux?
... but will function only if this clock is derived from PLL6.
Ok, but _why_ is that the case? Could we at least have a comment for
that?
Where does the driver get the named clocks from if they aren't provided
on the device node? Is there a clock-ranges somewhere?
It feels a little fragile to rely on the organisation of the clock tree
and the naming thereof. If the IP block is ever reused on an SoC with a
different clock tree layout then we have to handle things differently.
Cheers,
Mark.
--
You received this message because you are subscribed to the Google Groups
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
