On Tue, Aug 28, 2018 at 3:21 PM, Maxime Ripard <maxime.rip...@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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot