[U-Boot] [PATCH v4 1/9] sunxi: clk: enable clk and reset for CCU devices

[Andre Przywara]

On Tue, 29 Jan 2019 23:56:44 +0530
Jagan Teki <jagan at amarulasolutions.com> wrote:

Hi,

> On Tue, Jan 29, 2019 at 11:47 PM Andre Przywara
> <andre.przywara at foss.arm.com> wrote:
> >
> > On Tue, 29 Jan 2019 23:40:26 +0530
> > Jagan Teki <jagan at amarulasolutions.com> wrote:
> >  
> > > On Tue, Jan 29, 2019 at 9:25 PM Andre Przywara
> > > <andre.przywara at arm.com> wrote:  
> > > >
> > > > Some Allwinner clock devices have parent clocks and reset gates
> > > > itself, which need to be activated for them to work.
> > > >
> > > > Add some code to just assert all resets and enable all clocks
> > > > given. This should enable the A80 MMC config clock, which
> > > > requires both to be activated. The full CCU devices typically
> > > > don't require resets, and have just fixed clocks as their
> > > > parents. Since we treat both as optional and enabling fixed
> > > > clocks is a NOP, this works for all cases, without the need to
> > > > differentiate between those clock types.
> > > >
> > > > Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> > > > ---
> > > >  drivers/clk/sunxi/clk_sunxi.c | 12 ++++++++++++
> > > >  1 file changed, 12 insertions(+)
> > > >
> > > > diff --git a/drivers/clk/sunxi/clk_sunxi.c
> > > > b/drivers/clk/sunxi/clk_sunxi.c index 62ce2994e4..6d4aeb5315
> > > > 100644 --- a/drivers/clk/sunxi/clk_sunxi.c
> > > > +++ b/drivers/clk/sunxi/clk_sunxi.c
> > > > @@ -8,6 +8,7 @@
> > > >  #include <clk-uclass.h>
> > > >  #include <dm.h>
> > > >  #include <errno.h>
> > > > +#include <reset.h>
> > > >  #include <asm/io.h>
> > > >  #include <asm/arch/ccu.h>
> > > >  #include <linux/log2.h>
> > > > @@ -61,6 +62,9 @@ struct clk_ops sunxi_clk_ops = {
> > > >  int sunxi_clk_probe(struct udevice *dev)
> > > >  {
> > > >         struct ccu_priv *priv = dev_get_priv(dev);
> > > > +       struct clk_bulk clk_bulk;
> > > > +       struct reset_ctl_bulk rst_bulk;
> > > > +       int ret;
> > > >
> > > >         priv->base = dev_read_addr_ptr(dev);
> > > >         if (!priv->base)
> > > > @@ -70,5 +74,13 @@ int sunxi_clk_probe(struct udevice *dev)
> > > >         if (!priv->desc)
> > > >                 return -EINVAL;
> > > >
> > > > +       ret = clk_get_bulk(dev, &clk_bulk);
> > > > +       if (!ret)
> > > > +               clk_enable_bulk(&clk_bulk);
> > > > +
> > > > +       ret = reset_get_bulk(dev, &rst_bulk);
> > > > +       if (!ret)
> > > > +               reset_deassert_bulk(&rst_bulk);
> > > > +  
> > >
> > > Can't we do this locally to clk_a80 probe?  
> >
> > That's the point: there is no such thing. For all SoCs we use the
> > shared sunxi_clk_probe() function. Doing this only for the A80
> > would mean to split this up, which is duplicating a lot of code for
> > very little effect. The code here just enables every clock and
> > reset given, which is generic and should always be the right
> > thing.  
> 
> But enable and dessert of clock and reset is job respective IP driver
> isn't it?

Which IP driver are you thinking about? This is "the IP driver"
for those clock, isn't it?
So you could come up with a completely separate driver for this tiny
A80 MMC config clock, and would find that this is actually almost the
same as the "big" CCU driver (just gates and resets). That's why this
idea of piggy backing on what we already have, just adding the clock
enablement and reset deasserting, which doesn't hurt the "big" driver.

Cheers,
Andre.