On Tue, Aug 28, 2018 at 3:21 PM, Maxime Ripard maxime.ripard@bootlin.com wrote:
On Mon, Aug 27, 2018 at 03:34:20PM +0530, Jagan Teki wrote:
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.
Well, yes, but you can make the same argument for other SoCs, and other features. So really, I think this is a good idea, but if we remain consistent between SoCs and features.
True, I see adding wrapper make consistent on these descriptor table, so the wrapper will call the actual set_rate, of course we need to add ifdef in wrapper for non-MMC targets or __weak function.