[U-Boot] [PATCH v3 19/58] clk: sunxi: Implement direct MMC clocks

[Jagan Teki]

On Mon, Aug 20, 2018 at 5:03 PM, Maxime Ripard
<maxime.ripard at bootlin.com> wrote:
> On Sun, Aug 19, 2018 at 07:26:36PM +0530, Jagan Teki wrote:
>> Implement direct MMC clocks for all Allwinner SoC
>> clock drivers via clock map descriptor table.
>>
>> This includes adding ccu_clk_set_rate function pointer,
>> which indeed support CLK set_rate API, so update clock
>> handling in sunxi_mmc driver to support both no-dm and dm code.
>>
>> Cc: Jaehoon Chung <jh80.chung at samsung.com>
>> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
>> ---
>>  arch/arm/include/asm/arch-sunxi/ccu.h | 10 +++++
>>  drivers/clk/sunxi/clk_a10.c           |  5 +++
>>  drivers/clk/sunxi/clk_a10s.c          |  6 +++
>>  drivers/clk/sunxi/clk_a23.c           |  6 +++
>>  drivers/clk/sunxi/clk_a31.c           |  5 +++
>>  drivers/clk/sunxi/clk_a64.c           |  4 ++
>>  drivers/clk/sunxi/clk_a83t.c          |  4 ++
>>  drivers/clk/sunxi/clk_h3.c            |  4 ++
>>  drivers/clk/sunxi/clk_r40.c           |  4 ++
>>  drivers/clk/sunxi/clk_sunxi.c         | 19 +++++++++
>>  drivers/clk/sunxi/clk_v3s.c           |  4 ++
>>  drivers/mmc/sunxi_mmc.c               | 58 +++++++++++++++++----------
>>  12 files changed, 107 insertions(+), 22 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-sunxi/ccu.h b/arch/arm/include/asm/arch-sunxi/ccu.h
>> index bacd052ef3..4e30ab330c 100644
>> --- a/arch/arm/include/asm/arch-sunxi/ccu.h
>> +++ b/arch/arm/include/asm/arch-sunxi/ccu.h
>> @@ -60,6 +60,16 @@ struct sunxi_clk_priv {
>>
>>  extern struct clk_ops sunxi_clk_ops;
>>
>> +/**
>> + * mmc_clk_set_rate - mmc clock set rate
>> + *
>> + * @base:    clock register base address
>> + * @bit:     clock bit value
>> + * @rate:    clock input rate in Hz
>> + * @return 0, or -ve error code.
>> + */
>> +int mmc_clk_set_rate(void *base, u32 bit, ulong rate);
>> +
>>  /**
>>   * sunxi_reset_bind() - reset binding
>>   *
>> diff --git a/drivers/clk/sunxi/clk_a10.c b/drivers/clk/sunxi/clk_a10.c
>> index fb11231dd1..55176bc174 100644
>> --- a/drivers/clk/sunxi/clk_a10.c
>> +++ b/drivers/clk/sunxi/clk_a10.c
>> @@ -23,6 +23,11 @@ static struct ccu_clk_map a10_clks[] = {
>>       [CLK_AHB_MMC2]          = { 0x060, BIT(10), NULL },
>>       [CLK_AHB_MMC3]          = { 0x060, BIT(11), NULL },
>>
>> +     [CLK_MMC0]              = { 0x088, BIT(31), &mmc_clk_set_rate },
>> +     [CLK_MMC1]              = { 0x08c, BIT(31), &mmc_clk_set_rate },
>> +     [CLK_MMC2]              = { 0x090, BIT(31), &mmc_clk_set_rate },
>> +     [CLK_MMC3]              = { 0x094, BIT(31), &mmc_clk_set_rate },
>> +
>>       [CLK_USB_OHCI0]         = { 0x0cc, BIT(6), NULL },
>>       [CLK_USB_OHCI1]         = { 0x0cc, BIT(7), NULL },
>>       [CLK_USB_PHY]           = { 0x0cc, BIT(8), NULL },
>> diff --git a/drivers/clk/sunxi/clk_a10s.c b/drivers/clk/sunxi/clk_a10s.c
>> index bc4ae7352b..fbac0ad751 100644
>> --- a/drivers/clk/sunxi/clk_a10s.c
>> +++ b/drivers/clk/sunxi/clk_a10s.c
>> @@ -20,6 +20,12 @@ static struct ccu_clk_map a10s_clks[] = {
>>       [CLK_AHB_MMC1]          = { 0x060, BIT(9), NULL },
>>       [CLK_AHB_MMC2]          = { 0x060, BIT(10), NULL },
>>
>> +#ifdef CONFIG_MMC
>> +     [CLK_MMC0]              = { 0x088, BIT(31), &mmc_clk_set_rate },
>> +     [CLK_MMC1]              = { 0x08c, BIT(31), &mmc_clk_set_rate },
>> +     [CLK_MMC2]              = { 0x090, BIT(31), &mmc_clk_set_rate },
>> +#endif
>> +
>
> I'm not too sure about the ifdef here. Or at least, we should be
> consistent, and if we do it for the MMC, we should do it for all the
> SoCs (including the A10), and for all the controllers (including USB,
> for example).

because few of sun5i boards not using MMC, example CHIP and CHIP_pro
otherwise we need to use intermediate wrapper to call
mmc_clk_set_rate.

>
>> diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
>> index 39f15eb423..bf82014a64 100644
>> --- a/drivers/mmc/sunxi_mmc.c
>> +++ b/drivers/mmc/sunxi_mmc.c
>> @@ -13,6 +13,7 @@
>>  #include <malloc.h>
>>  #include <mmc.h>
>>  #include <asm/io.h>
>> +#include <asm/arch/ccu.h>
>>  #include <asm/arch/clock.h>
>>  #include <asm/arch/cpu.h>
>>  #include <asm/arch/gpio.h>
>> @@ -34,6 +35,8 @@ struct sunxi_mmc_priv {
>>       struct mmc_config cfg;
>>  };
>>
>> +bool new_mode;
>> +
>>  #if !CONFIG_IS_ENABLED(DM_MMC)
>>  /* support 4 mmc hosts */
>>  struct sunxi_mmc_priv mmc_host[4];
>> @@ -95,23 +98,19 @@ static int mmc_resource_init(int sdc_no)
>>  }
>>  #endif
>>
>> -static int mmc_set_mod_clk(struct sunxi_mmc_priv *priv, unsigned int hz)
>> +int mmc_clk_set_rate(void *base, u32 bit, ulong rate)
>>  {
>>       unsigned int pll, pll_hz, div, n, oclk_dly, sclk_dly;
>> -     bool new_mode = false;
>>       u32 val = 0;
>>
>> -     if (IS_ENABLED(CONFIG_MMC_SUNXI_HAS_NEW_MODE) && (priv->mmc_no == 2))
>> -             new_mode = true;
>> -
>
> [..]
>
>> +static int mmc_set_mod_clk(struct sunxi_mmc_priv *priv, unsigned int hz)
>> +{
>> +#if CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(CLK)
>> +#else
>> +     if (IS_ENABLED(CONFIG_MMC_SUNXI_HAS_NEW_MODE) && (priv->mmc_no == 2))
>> +             new_mode = true;
>> +
>
> I'm not sure why you need the global variable.
>
> The A83t emmc case you have below is caught in this condition, and
> therefore, the scope doesn't need to be global.

Since mmc_set_rate calling with base and reg bits, which doesn't have
any possibility know private data of driver we need a global to check
to update the same in dm and non-dm.