[PATCH 2/3 v3] P4080/mtd: Only make elbc nand driver detect nand flash partitions

Anton Vorontsov cbouatmailru at gmail.com
Thu Sep 16 12:40:44 EDT 2010


On Thu, Sep 16, 2010 at 11:14:48AM -0500, Scott Wood wrote:
> > > DEFINE_MUTEX(fsl_elbc_mutex);
> > 
> > I'd place the mutex inside the fsl_lbc_ctrl_dev,
> > i.e. fsl_lbc_ctrl_dev->nand_lock. This is to avoid more
> > global variables.
> 
> I wouldn't.  If the lock is only meaningful to the NAND driver, it
> should be declared in the NAND driver.
> 
> Besides, it's not any less of a global just because it's sitting inside
> a singleton struct.
> 
> Perhaps it should be declared as a static local inside the probe
> function, if it's just to guard against this one race.

OK, in that case better be persistent and not introduce
fsl_lbc_ctrl_dev->nand at all, as it isn't used outside
of the driver.

Having fsl_lbc_ctrl_dev->nand and its lock elsewhere in
the code makes no sense.

> > Btw, even before this patch, it seems that the driver had
> > all these bugs/races, i.e. ctrl->controller.lock was not
> > used at all. Ugh.
> 
> It is used, search nand_base.c for controller->lock.

OK, now I see, the driver implements its own chip->controller
(which is exactly what ctrl->controller is). Then we're fine.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2



More information about the linux-mtd mailing list