Re: [PATCH 1/9] drivers:nand:mtd: add support for UBI bakvol in mtd layer

[Boris Brezillon <boris.brezillon@free-electrons.com>]

Hi Bean,

On Wed, 30 Sep 2015 06:05:44 +0000
Bean Huo 霍斌斌 (beanhuo) <beanhuo@micron.com> wrote:


> > > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index
> > > f17fa75..cfcb3a68 100644
> > > --- a/include/linux/mtd/mtd.h
> > > +++ b/include/linux/mtd/mtd.h
> > > @@ -204,6 +204,9 @@ struct mtd_info {
> > >  			  struct mtd_oob_ops *ops);
> > >  	int (*_write_oob) (struct mtd_info *mtd, loff_t to,
> > >  			   struct mtd_oob_ops *ops);
> > > +	int (*_dual_plane_write_oob) (struct mtd_info *mtd, loff_t to_plane0,
> > > +			struct mtd_oob_ops *ops_plane0, loff_t to_plane1,
> > > +					struct mtd_oob_ops *ops_plane1);
> > 
> > 
> > IMHO, if we were about to allow parallel write operations this should be
> > exposed as a more generic API, something like:
> > 
> > struct mtd_write_op {
> > 	loff_t to;
> > 	struct mtd_oob_ops ops;
> > };
> > 
> > struct mtd_multi_write_ops {
> > 	struct list_head writes;
> > };
> > 
> > int (*_multi_write)(struct mtd_info *mtd,
> > 		    struct mtd_multi_write_ops *ops);
> > 
> > Then the NAND layer could optimize that if the NAND chip supports "two-plane
> > page program", and if 2 pages in the write list are fulfilling the requirements.
> > 
> Good suggestion, I can improve it for next version patch. Thanks.
> 

Please wait for other reviews before reworking that.

> > > index 1e271cb..1da3418 100644
> > > --- a/include/linux/mtd/ubi.h
> > > +++ b/include/linux/mtd/ubi.h
> > > @@ -35,6 +35,15 @@
> > >   */
> > >  #define UBI_MAX_SG_COUNT 64
> > >
> > > +enum {
> > > +	UBI_BAKVOL_UNONE,
> > > +	UBI_BAKVOL_INIT_INFO,
> > > +	UBI_BAKVOL_INIT_INFO_DONE,
> > > +	UBI_BAKVOL_INIT_VOLUME,
> > > +	UBI_BAKVOL_INIT_VOLUME_DONE,
> > > +	UBI_BAKVOL_RUN
> > > +};
> > > +
> > 
> > Are those changes related to this patch?
> > 
> 
> Yes, maybe can simplify more.

Actually that was a rhetorical question. My point was that this enum
definition has nothing to do in this patch, and you're doing that
(mixing unrelated changes in the same commit) a lot in your other
patches.
So please make sure you correctly split your changes next time you send
a patch set.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com