diff mbox series

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

Message ID 20190119013055.28023-3-andre.przywara@arm.com
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series sunxi: enable DM_MMC | expand

Commit Message

Andre Przywara Jan. 19, 2019, 1:30 a.m. UTC
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@arm.com>
---
 drivers/clk/sunxi/clk_a80.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

Comments

Jagan Teki Jan. 21, 2019, 9:31 a.m. UTC | #1
On Sat, Jan 19, 2019 at 7:02 AM Andre Przywara <andre.przywara@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@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"
Chen-Yu Tsai Jan. 21, 2019, 9:34 a.m. UTC | #2
On Mon, Jan 21, 2019 at 5:32 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Sat, Jan 19, 2019 at 7:02 AM Andre Przywara <andre.przywara@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@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. :)

ChenYu
Andre Przywara Jan. 21, 2019, 9:38 a.m. UTC | #3
On Mon, 21 Jan 2019 17:34:25 +0800
Chen-Yu Tsai <wens@csie.org> wrote:

> On Mon, Jan 21, 2019 at 5:32 PM Jagan Teki
> <jagan@amarulasolutions.com> wrote:
> >
> > On Sat, Jan 19, 2019 at 7:02 AM Andre Przywara
> > <andre.przywara@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@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.

Cheers,
Andre.
Chen-Yu Tsai Jan. 21, 2019, 10:02 a.m. UTC | #4
On Mon, Jan 21, 2019 at 5:39 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Mon, 21 Jan 2019 17:34:25 +0800
> Chen-Yu Tsai <wens@csie.org> wrote:
>
> > On Mon, Jan 21, 2019 at 5:32 PM Jagan Teki
> > <jagan@amarulasolutions.com> wrote:
> > >
> > > On Sat, Jan 19, 2019 at 7:02 AM Andre Przywara
> > > <andre.przywara@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@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.

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.

Regards
ChenYu
Andre Przywara Jan. 21, 2019, 10:12 a.m. UTC | #5
On Mon, 21 Jan 2019 18:02:17 +0800
Chen-Yu Tsai <wens@csie.org> wrote:

> On Mon, Jan 21, 2019 at 5:39 PM Andre Przywara
> <andre.przywara@arm.com> wrote:
> >
> > On Mon, 21 Jan 2019 17:34:25 +0800
> > Chen-Yu Tsai <wens@csie.org> wrote:
> >  
> > > On Mon, Jan 21, 2019 at 5:32 PM Jagan Teki
> > > <jagan@amarulasolutions.com> wrote:  
> > > >
> > > > On Sat, Jan 19, 2019 at 7:02 AM Andre Przywara
> > > > <andre.przywara@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@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).

Cheers,
Andre.
Chen-Yu Tsai Jan. 21, 2019, 10:29 a.m. UTC | #6
On Mon, Jan 21, 2019 at 6:12 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Mon, 21 Jan 2019 18:02:17 +0800
> Chen-Yu Tsai <wens@csie.org> wrote:
>
> > On Mon, Jan 21, 2019 at 5:39 PM Andre Przywara
> > <andre.przywara@arm.com> wrote:
> > >
> > > On Mon, 21 Jan 2019 17:34:25 +0800
> > > Chen-Yu Tsai <wens@csie.org> wrote:
> > >
> > > > On Mon, Jan 21, 2019 at 5:32 PM Jagan Teki
> > > > <jagan@amarulasolutions.com> wrote:
> > > > >
> > > > > On Sat, Jan 19, 2019 at 7:02 AM Andre Przywara
> > > > > <andre.przywara@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@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
diff mbox series

Patch

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",
+	  .data = (ulong)&a80_mmc_clk_desc },
 	{ }
 };