[U-Boot] [PATCH v4 1/9] sunxi: clk: enable clk and reset for CCU devices
Andre Przywara
andre.przywara at arm.com
Wed Jan 30 13:42:52 UTC 2019
On Wed, 30 Jan 2019 18:16:44 +0530
Jagan Teki <jagan at amarulasolutions.com> wrote:
> On Wed, Jan 30, 2019 at 4:26 PM Andre Przywara
> <andre.przywara at arm.com> wrote:
> >
> > On Wed, 30 Jan 2019 16:08:14 +0530
> > Jagan Teki <jagan at amarulasolutions.com> wrote:
> >
> > > On Wed, Jan 30, 2019 at 4:04 PM Andre Przywara
> > > <andre.przywara at arm.com> wrote:
> > > >
> > > > 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?
> > >
> > > IP can be any peripheral like USB, MMC, UART and it those drivers
> > > job to get and enable the clock isn't it?
> >
> > Yes, using the DM_CLK framework. This is what we do: the A80 MMC DT
> > node refers to the MMC config clock (instead of the generic CCU),
> > and the MMC driver doesn't care about any requirement this clock has
> > *itself*.
> > This is the responsibility of the *A80 MMC config clock driver*,
> > which we introduce in patch 3/9. So in *this* driver's probe
> > function you would need to enable the parent clocks, which is
> > exactly what we do. Just by re-using the existing sunxi_clk_probe()
> > function.
> > > I assume this code would do the same thing what these peripheral
> > > driver do?
> >
> > It does, it's just one layer in between.
> > -----------------------
> > A64 MMC driver A64 CCU driver fixed clock driver
> > sunxi_clk_probe()
> > clk_enable() -> (NULL)
>
> Ahh.. It didn't effect other clk drivers except A80?
It affects every CCU device, but most (all?) of them just don't
specify any resets in the DT and the only clocks most of them have
there are fixed clocks.
And even if they would, enabling clocks and de-asserting resets sounds
like a reasonable thing to do anyway.
So the reset_get_bulk() call will fail (which we don't care about), and
the clk_get_bulk() call will return the two fixed clocks, for which
enable calls are a NOP.
So it works.
Cheers,
Andre.
>
> > clk_enable() -> sunxi_clk_enable()
> > -----------------------
> > A80 MMC driver A80 MMC cfg clk driver A80 CCU driver fixed
> > clk sunxi_clk_probe()
> > clk_enable() -> (NULL)
> > sunxi_clk_probe()
> > clk_enable() -> sunxi_clk_enable()
> > clk_enable() -> sunxi_clk_enable()
> > -----------------------
>
> So these bulk enable /deasserts in sunxi_clk_probe will call
> respective ops for A80 only and for other the behavior is as before.
> it it?
More information about the U-Boot
mailing list