[PATCH 3/3] mtd: gpmi: change the code for clocks

[Lothar Waßmann]

Hi,

Dong Aisheng writes:
> 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.
> 
Even if it had fewer clocks you would have to change the driver!


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________