[U-Boot] [linux-sunxi] [PATCH 2/9] sunxi: clk: A80: add MMC clock support

[Chen-Yu Tsai]

On Mon, Jan 21, 2019 at 6:12 PM Andre Przywara <andre.przywara at arm.com> wrote:
>
> On Mon, 21 Jan 2019 18:02:17 +0800
> Chen-Yu Tsai <wens at csie.org> wrote:
>
> > On Mon, Jan 21, 2019 at 5:39 PM Andre Przywara
> > <andre.przywara at arm.com> wrote:
> > >
> > > On Mon, 21 Jan 2019 17:34:25 +0800
> > > Chen-Yu Tsai <wens at csie.org> wrote:
> > >
> > > > On Mon, Jan 21, 2019 at 5:32 PM Jagan Teki
> > > > <jagan at amarulasolutions.com> wrote:
> > > > >
> > > > > On Sat, Jan 19, 2019 at 7:02 AM Andre Przywara
> > > > > <andre.przywara at arm.com> wrote:
> > > > > >
> > > > > > The A80 handles resets and clock gates for the MMC devices
> > > > > > differently, outside of the CCU IP block. Consequently we
> > > > > > have a separate clock device with a separate binding for that.
> > > > > >
> > > > > > Implement that with the respective clock gates and resets to
> > > > > > allow the A80 taking part in the DM_MMC game.
> > > > > >
> > > > > > Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> > > > > > ---
> > > > > >  drivers/clk/sunxi/clk_a80.c | 28 +++++++++++++++++++++++++++-
> > > > > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/clk/sunxi/clk_a80.c
> > > > > > b/drivers/clk/sunxi/clk_a80.c index d6dd6a1fa1..2bfc2ca9f5
> > > > > > 100644 --- a/drivers/clk/sunxi/clk_a80.c
> > > > > > +++ b/drivers/clk/sunxi/clk_a80.c
> > > > > > @@ -30,19 +30,45 @@ static const struct ccu_reset
> > > > > > a80_resets[] = { [RST_BUS_UART5]         = RESET(0x5b4,
> > > > > > BIT(21)), };
> > > > > >
> > > > > > +static const struct ccu_clk_gate a80_mmc_gates[] = {
> > > > > > +       [0]                     = GATE(0x0, BIT(16)),
> > > > > > +       [1]                     = GATE(0x4, BIT(16)),
> > > > > > +       [2]                     = GATE(0x8, BIT(16)),
> > > > > > +       [3]                     = GATE(0xc, BIT(16)),
> > > > > > +};
> > > > > > +
> > > > > > +static const struct ccu_reset a80_mmc_resets[] = {
> > > > > > +       [0]                     = GATE(0x0, BIT(18)),
> > > > > > +       [1]                     = GATE(0x4, BIT(18)),
> > > > > > +       [2]                     = GATE(0x8, BIT(18)),
> > > > > > +       [3]                     = GATE(0xc, BIT(18)),
> > > > > > +};
> > > > > > +
> > > > > >  static const struct ccu_desc a80_ccu_desc = {
> > > > > >         .gates = a80_gates,
> > > > > >         .resets = a80_resets,
> > > > > >  };
> > > > > >
> > > > > > +static const struct ccu_desc a80_mmc_clk_desc = {
> > > > > > +       .gates = a80_mmc_gates,
> > > > > > +       .resets = a80_mmc_resets,
> > > > > > +};
> > > > > > +
> > > > > >  static int a80_clk_bind(struct udevice *dev)
> > > > > >  {
> > > > > > -       return sunxi_reset_bind(dev, ARRAY_SIZE(a80_resets));
> > > > > > +       ulong count = ARRAY_SIZE(a80_resets);
> > > > > > +
> > > > > > +       if (device_is_compatible(dev,
> > > > > > "allwinner,allwinner,sun9i-a80-mmc"))
> > > > > > +               count = ARRAY_SIZE(a80_mmc_resets);
> > > > > > +
> > > > > > +       return sunxi_reset_bind(dev, count);
> > > > > >  }
> > > > > >
> > > > > >  static const struct udevice_id a80_ccu_ids[] = {
> > > > > >         { .compatible = "allwinner,sun9i-a80-ccu",
> > > > > >           .data = (ulong)&a80_ccu_desc },
> > > > > > +       { .compatible = "allwinner,allwinner,sun9i-a80-mmc",
> > > > >
> > > > > This can be "allwinner,sun9i-a80-mmc-config-clk"
> > > >
> > > > *Must*, not can. :)
> > >
> > > Indeed, thanks for spotting this, Jagan.
> > >
> > > Also I just figured that we need to enable the parent clocks and
> > > resets of this clock itself. I hope we can do this unconditionally,
> > > even for the complex CCU, as enabling fixed-clocks should be
> > > covered by the clock framework already.
> >
> > You need to deassert the reset control for the MMC clock config block,
> > otherwise none of the toggles are going to work. And you can't assert
> > it afterwards, otherwise the bits revert to default. Also, you need to
> > enable the bus clock to be able to access the bits.
>
> Yeah, that was what I was thinking of.
>
> > So yeah, you can do this unconditionally. The Linux driver simply
> > tries to be smarter and only enables the bus clock when toggling the
> > bits.
>
> With "unconditionally" I meant doing this in the probe routine, which
> right now I *share* between the CCU clocks and this config clock. The
> patch is pretty small because of this.
>
> The CCU has only fixed parent clocks and no resets, so I think we can
> add parent clock and optional reset handling there and should cover
> this config clock as well, without providing a separate probe routine
> (and the rest of the boilerplate).

I think I'm missing some context here. You want to do this in the shared
*bind* function in this file? Or the shared *probe* function in
drivers/clk/sunxi/clk_sunxi.c ?

I assume it's the former? In which case you already have a conditional
to check the compatible string. So you could just expand that block and
put it there?

Either way I think it's not up to me about what's acceptable. :)

ChenYu