Hi Sascha, On Mon, Feb 21, 2011 at 05:21:54PM +0100, Sascha Hauer wrote: > On Mon, Feb 21, 2011 at 05:54:52PM +0800, Richard Zhao wrote: > > Signed-off-by: Richard Zhao <[email protected]> > > > > diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c > > b/arch/arm/mach-mx5/clock-mx51-mx53.c > > index 8164b1d..8ac61d5 100644 > > --- a/arch/arm/mach-mx5/clock-mx51-mx53.c > > +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c > > @@ -42,6 +42,7 @@ static struct clk usboh3_clk; > > static struct clk emi_fast_clk; > > static struct clk ipu_clk; > > static struct clk mipi_hsc1_clk; > > +static struct clk esdhc1_clk; > > > > #define MAX_DPLL_WAIT_TRIES 1000 /* 1000 * udelay(1) = 1ms */ > > > > @@ -1147,6 +1148,34 @@ CLK_GET_RATE(esdhc2, 1, ESDHC2_MSHC2) > > CLK_SET_PARENT(esdhc2, 1, ESDHC2_MSHC2) > > CLK_SET_RATE(esdhc2, 1, ESDHC2_MSHC2) > > > > +static int clk_esdhc3_set_parent(struct clk *clk, struct clk *parent) > > +{ > > + u32 reg; > > + > > + reg = __raw_readl(MXC_CCM_CSCMR1); > > + if (parent == &esdhc1_clk) > > + reg &= ~MXC_CCM_CSCMR1_ESDHC3_CLK_SEL; > > + else > > + reg |= MXC_CCM_CSCMR1_ESDHC3_CLK_SEL; > > parent != &esdhc1_clk doesn't mean parent is a valid parent for this > clock. Please do the parameter checking properly. I can add a check here, but it will not share the same function between mx51 and mx53. for mx51, partents can be sdhc1_clk and sdhc2_clk, but for mx53, it can be sdhc1_clk and sdhc3_clk. > > > + __raw_writel(reg, MXC_CCM_CSCMR1); > > + > > + return 0; > > +} > > + > > +static int clk_esdhc4_set_parent(struct clk *clk, struct clk *parent) > > +{ > > + u32 reg; > > + > > + reg = __raw_readl(MXC_CCM_CSCMR1); > > + if (parent == &esdhc1_clk) > > + reg &= ~MXC_CCM_CSCMR1_ESDHC4_CLK_SEL; > > + else > > + reg |= MXC_CCM_CSCMR1_ESDHC4_CLK_SEL; > > ditto. > > > + __raw_writel(reg, MXC_CCM_CSCMR1); > > + > > + return 0; > > +} > > + > > #define DEFINE_CLOCK_FULL(name, i, er, es, gr, sr, e, d, p, s) > > \ > > static struct clk name = { \ > > .id = i, \ > > @@ -1253,6 +1282,32 @@ DEFINE_CLOCK_FULL(esdhc2_ipg_clk, 1, MXC_CCM_CCGR3, > > MXC_CCM_CCGRx_CG2_OFFSET, > > NULL, NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL); > > DEFINE_CLOCK_MAX(esdhc2_clk, 1, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG3_OFFSET, > > clk_esdhc2, &pll2_sw_clk, &esdhc2_ipg_clk); > > +DEFINE_CLOCK_FULL(esdhc3_ipg_clk, 2, MXC_CCM_CCGR3, > > MXC_CCM_CCGRx_CG4_OFFSET, > > + NULL, NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL); > > +DEFINE_CLOCK_FULL(esdhc4_ipg_clk, 3, MXC_CCM_CCGR3, > > MXC_CCM_CCGRx_CG6_OFFSET, > > + NULL, NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL); > > + > > +static struct clk esdhc3_clk = { > > + .id = 2, > > + .parent = &esdhc1_clk, > > + .set_parent = clk_esdhc3_set_parent, > > + .enable_reg = MXC_CCM_CCGR3, > > + .enable_shift = MXC_CCM_CCGRx_CG5_OFFSET, > > + .enable = _clk_max_enable, > > + .disable = _clk_max_disable, > > + .secondary = &esdhc3_ipg_clk, > > +}; > > + > > +static struct clk esdhc4_clk = { > > + .id = 3, > > + .parent = &esdhc1_clk, > > + .set_parent = clk_esdhc4_set_parent, > > + .enable_reg = MXC_CCM_CCGR3, > > + .enable_shift = MXC_CCM_CCGRx_CG7_OFFSET, > > + .enable = _clk_max_enable, > > + .disable = _clk_max_disable, > > + .secondary = &esdhc4_ipg_clk, > > +}; > > > > DEFINE_CLOCK(mipi_esc_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG5_OFFSET, > > NULL, NULL, NULL, &pll2_sw_clk); > > DEFINE_CLOCK(mipi_hsc2_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG4_OFFSET, > > NULL, NULL, &mipi_esc_clk, &pll2_sw_clk); > > @@ -1312,6 +1367,8 @@ static struct clk_lookup mx51_lookups[] = { > > _REGISTER_CLOCK("imx51-cspi.0", NULL, cspi_clk) > > _REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk) > > _REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk) > > + _REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_clk) > > + _REGISTER_CLOCK("sdhci-esdhc-imx.3", NULL, esdhc4_clk) > > _REGISTER_CLOCK(NULL, "cpu_clk", cpu_clk) > > _REGISTER_CLOCK(NULL, "iim_clk", iim_clk) > > _REGISTER_CLOCK("imx2-wdt.0", NULL, dummy_clk) > > @@ -1333,6 +1390,8 @@ static struct clk_lookup mx53_lookups[] = { > > _REGISTER_CLOCK("imx-i2c.1", NULL, i2c2_clk) > > _REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk) > > _REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk) > > + _REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_clk) > > + _REGISTER_CLOCK("sdhci-esdhc-imx.3", NULL, esdhc4_clk) > > _REGISTER_CLOCK("imx53-ecspi.0", NULL, ecspi1_clk) > > _REGISTER_CLOCK("imx53-ecspi.1", NULL, ecspi2_clk) > > _REGISTER_CLOCK("imx53-cspi.0", NULL, cspi_clk) > > @@ -1412,6 +1471,14 @@ int __init mx53_clocks_init(unsigned long ckil, > > unsigned long osc, > > ckih2_reference = ckih2; > > oscillator_reference = osc; > > > > + esdhc2_clk.get_rate = NULL; > > + esdhc2_clk.set_rate = NULL; > > + esdhc2_clk.set_parent = clk_esdhc3_set_parent; > > + esdhc2_clk.parent = &esdhc1_clk; > > + esdhc3_clk.get_rate = clk_esdhc2_get_rate; > > + esdhc3_clk.set_rate = clk_esdhc2_set_rate; > > + esdhc3_clk.set_parent = clk_esdhc2_set_parent; > > + > > This is just too confusing and will be hard to cleanup when we get > clkops for i.MX. Please make SoC specific clocks from this and do not > reassign the function pointers. ok.
Thanks Richard > > Sascha > > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
