[PATCH 3/3] mtd: gpmi: change the code for clocks
Dong Aisheng
aisheng.dong at freescale.com
Fri Jun 29 05:34:38 EDT 2012
On Fri, Jun 29, 2012 at 05:29:26PM +0800, Lothar Waßmann wrote:
> Hi,
>
> Dong Aisheng writes:
> > On Fri, Jun 29, 2012 at 10:06:52AM +0800, Shawn Guo wrote:
> > > On Thu, Jun 28, 2012 at 11:52:05PM -0400, Huang Shijie wrote:
> > > > From: Huang Shijie <b32955 at freescale.com>
> > > >
> > > > The gpmi nand driver may needs several clocks(MX6Q needs five clocks).
> > > >
> > > > In the old clock framework, all these clocks are chained together,
> > > > all you need is to manipulate the first clock.
> > > >
> > > > But the kernel uses the common clk framework now, which forces us to
> > > > get the clocks one by one. When we use them, we have to enable them
> > > > one by one too.
> > > >
> [...]
> > > > +static char *extra_clks_for_mx6q[] = {
> > > > + "gpmi_apb", "gpmi_bch", "gpmi_bch_apb", "per1_bch",
> > > > +};
> > > > +
> > > > +static int __devinit gpmi_get_clks(struct gpmi_nand_data *this)
> > > > +{
> > > > + struct resources *r = &this->resources;
> > > > + char **extra_clks = NULL;
> > > > + struct clk *clk;
> > > > + int i;
> > > > +
> > > > + r->clock[0] = clk_get(&this->pdev->dev, NULL);
> > > > + if (IS_ERR(r->clock[0]))
> > > > + goto err_clock;
> > > > +
> > > > + /* Get extra clocks */
> > > > + if (GPMI_IS_MX6Q(this))
> > > > + extra_clks = extra_clks_for_mx6q;
> > >
> > > We probably do not need this tweaking. We can have the driver always
> > > take all those 5 clocks, and I think the current imx28 clock driver
> > > can just work with it, because the gpmi-nand clkdev lookup has NULL
> > > con_id and all those 5 clocks can match the same one on imx28.
> > >
> > Will mx28 fail if doing like that?
> > clk_get will fail if no clock found.
> > struct clk *clk_get_sys(const char *dev_id, const char *con_id)
> > {
> > struct clk_lookup *cl;
> >
> > mutex_lock(&clocks_mutex);
> > cl = clk_find(dev_id, con_id);
> > if (cl && !__clk_get(cl->clk))
> > cl = NULL;
> > mutex_unlock(&clocks_mutex);
> >
> > return cl ? cl->clk : ERR_PTR(-ENOENT);
> > }
> > EXPORT_SYMBOL(clk_get_sys);
> >
> > Furthermore, find unnecessary clock for mx28 seems not a good choice.
> > Probably a better way is to define each SoC required clocks and get them
> > respectively. It's explicit and clear.
> >
> No, that's silly. You would have to change the driver for each
> new platform that the driver can support.
>
If the new platform is fully compatible with exist platforms, then no.
If need more clocks, then in either way, we have to add support in driver.
> Instead the driver should request the maximum number of clocks that
> the existing set of platforms provide and all platforms with fewer
> clocks should provide an appropriate number of dummy clocks.
>
I wish we can not use dummy since it's easy causing misleading until
we have no other better way to go.
> This way new platforms can be supported without any change to the
> driver and only if a platform requires even more clocks (like in this
> particular case) would some code outside that platform have to be
> changed at all.
Regards
Dong Aisheng
More information about the linux-mtd
mailing list