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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150930091545.57a4047e@bbrezillon>
Date:	Wed, 30 Sep 2015 09:15:45 +0200
From:	Boris Brezillon <boris.brezillon@...e-electrons.com>
To:	"Bean Huo 霍斌斌 (beanhuo)" <beanhuo@...ron.com>
Cc:	"dedekind1@...il.com" <dedekind1@...il.com>,
	"adrian.hunter@...el.com" <adrian.hunter@...el.com>,
	"computersforpeace@...il.com" <computersforpeace@...il.com>,
	"baruch@...s.co.il" <baruch@...s.co.il>,
	"asierra@...-inc.com" <asierra@...-inc.com>,
	"guz.fnst@...fujitsu.com" <guz.fnst@...fujitsu.com>,
	"gsi@...x.de" <gsi@...x.de>, "richard@....at" <richard@....at>,
	David Woodhouse <dwmw2@...radead.org>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	"Frank Liu 刘群 (frankliu)" <frankliu@...ron.com>,
	Andrea Scian <rnd4@...e-tech.it>,
	"Peter Pan 潘栋 (peterpandong)" 
	<peterpandong@...ron.com>,
	"Karl Zhang 张双锣 (karlzhang)" 
	<karlzhang@...ron.com>,
	Iwo Mergler <Iwo.Mergler@...commwireless.com>,
	"Jeff Lauruhn (jlauruhn)" <jlauruhn@...ron.com>,
	Stefan Roese <sr@...x.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/9] drivers:nand:mtd: add support for UBI bakvol in mtd
 layer

Hi Bean,

On Wed, 30 Sep 2015 06:05:44 +0000
Bean Huo 霍斌斌 (beanhuo) <beanhuo@...ron.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/