On Mon, Aug 20, 2018 at 5:03 PM, Maxime Ripard
<maxime.rip...@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.ch...@samsung.com>
>> Signed-off-by: Jagan Teki <ja...@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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
