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.ripard@xxxxxxxxxxxxxxxxxx> > > > > Acked-by: Arnd Bergmann <arnd@xxxxxxxx> > > > > --- > > > > 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
Attachment:
signature.asc
Description: Digital signature