From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Bean Huo <huobean@gmail.com>
Cc: vigneshr@ti.com, richard@nod.at, s.hauer@pengutronix.de,
linux-kernel@vger.kernel.org, derosier@gmail.com,
boris.brezillon@collabora.com, linux-mtd@lists.infradead.org,
Bean Huo <beanhuo@micron.com>
Subject: Re: [PATCH v4 0/5] Micron SLC NAND filling block
Date: Tue, 19 May 2020 11:08:40 +0200 [thread overview]
Message-ID: <20200519110840.2302987f@xps13> (raw)
In-Reply-To: <8de0911281b4c03671841027ec165422789b63f2.camel@gmail.com>
Hi Bean,
Bean Huo <huobean@gmail.com> wrote on Tue, 19 May 2020 11:04:15 +0200:
> hi, Miquel
>
> On Mon, 2020-05-18 at 17:22 +0200, Miquel Raynal wrote:
> > Hi Bean,
> >
> > Bean Huo <huobean@gmail.com> wrote on Mon, 18 May 2020 15:59:38
> > +0200:
> >
> > > From: Bean Huo <beanhuo@micron.com>
> > >
> > > After submission of patch V1 [1] and V2 [2], we stopped its update
> > > since we get
> > > stuck in the solution on how to avoid the power-loss issue in case
> > > power-cut
> > > hits the block filling. In the v1 and v2, to avoid this issue, we
> > > always damaged
> > > page0, page1, this's based on the hypothesis that NAND FS is UBIFS.
> > > This
> > > FS-specifical code is unacceptable in the MTD layer. Also, it
> > > cannot cover all
> > > NAND based file system. Based on the current discussion, seems that
> > > re-write all
> > > first 15 page from page0 is a satisfactory solution.
> >
> > We have a layering problem now. Maybe we should just have an MTD
> > internal variable like min_written_pages_before_erase that the Micron
> > driver could set to a !0 value.
> >
> > Then, the handling could be done by the user (UBI/UBIFS, JFFS2, MTD
> > user if exported).
> >
>
> This is NAND its own problem, if no significant adantage, I don't think
> it's a good solution to extend the problem to the upper FS layer.
> also, in the FS erase path, doesn't have the programmed pages counter.
> we should repeat the same approach as we did in MTD layer.
The problem is that if the filesystem is not aware, it breaks the
"power cut safe" assertion.
There is a problem with JFFS2 and a problem with UBIFS because of that.
We can certainly keep a default implementation like this one for other
users though.
>
> > >
> > > Meanwhile, I borrowed one idea from Miquel Raynal patchset [3], in
> > > which keeps
> > > a recode of programmed pages, base on it, for most of the cases, we
> > > don't need
> > > to read every page to see if current erasing block is a partially
> > > programmed
> > > block.
> > >
> > > Changelog:
> > >
> > > v3 - v4:
> > > 1. In the patch 4/5, change to directly use ecc.strength to
> > > judge the page
> > > is a empty page or not, rather than max_bitflips < mtd-
> > > >bitflip_threshold
> > > 2. In the patch 5/5, for the powerloss case, from the next time
> > > boot up,
> > > lots of page will be programmed from >page15 address, if
> > > still using
> > > first_p as GENMASK() bitmask starting position, writtenp
> > > will be always 0,
> > > fix it by changing its bitmask starting at bit position 0.
> > >
> > > v2 - v3:
> > > 1. Rebase patch to the latest MTD git tree
> > > 2. Add a record that keeps tracking the programmed pages in the
> > > first 16
> > > pages
> > > 3. Change from program odd pages, damage page 0 and page 1, to
> > > program all
> > > first 15 pages
> > > 4. Address issues which exist in the V2.
> > >
> > > v1 - v2:
> > > 1. Rebased V1 to latest Linux kernel.
> > > 2. Add erase preparation function pointer in
> > > nand_manufacturer_ops.
> > >
> > >
> > > [1] https://www.spinics.net/lists/linux-mtd/msg04112.html
> > > [2] https://www.spinics.net/lists/linux-mtd/msg04450.html
> > > [3] https://www.spinics.net/lists/linux-mtd/msg13083.html
> > >
> > >
> > > Bean Huo (5):
> > > mtd: rawnand: group all NAND specific ops into new nand_chip_ops
> > > mtd: rawnand: Add {pre,post}_erase hooks in nand_chip_ops
> > > mtd: rawnand: Add write_oob hook in nand_chip_ops
> > > mtd: rawnand: Introduce a new function
> > > nand_check_is_erased_page()
> > > mtd: rawnand: micron: Micron SLC NAND filling block
> >
> > When you take my patches in your series, especially when not touching
> > them at all, you should keep my Authorship and SoB first, then add
> > your
> > SoB.
> >
>
> sorry for my fault, I thought adding your Signed-off-by in 3/5 is
> suffient. you mean I should add your signed-off-by in 5/5 as well?
> I will do that in next version.
You should keep my Authorship and SoB for both patches + add your SoB
after mine.
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Bean Huo <huobean@gmail.com>
Cc: richard@nod.at, vigneshr@ti.com, s.hauer@pengutronix.de,
boris.brezillon@collabora.com, derosier@gmail.com,
linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
Bean Huo <beanhuo@micron.com>
Subject: Re: [PATCH v4 0/5] Micron SLC NAND filling block
Date: Tue, 19 May 2020 11:08:40 +0200 [thread overview]
Message-ID: <20200519110840.2302987f@xps13> (raw)
In-Reply-To: <8de0911281b4c03671841027ec165422789b63f2.camel@gmail.com>
Hi Bean,
Bean Huo <huobean@gmail.com> wrote on Tue, 19 May 2020 11:04:15 +0200:
> hi, Miquel
>
> On Mon, 2020-05-18 at 17:22 +0200, Miquel Raynal wrote:
> > Hi Bean,
> >
> > Bean Huo <huobean@gmail.com> wrote on Mon, 18 May 2020 15:59:38
> > +0200:
> >
> > > From: Bean Huo <beanhuo@micron.com>
> > >
> > > After submission of patch V1 [1] and V2 [2], we stopped its update
> > > since we get
> > > stuck in the solution on how to avoid the power-loss issue in case
> > > power-cut
> > > hits the block filling. In the v1 and v2, to avoid this issue, we
> > > always damaged
> > > page0, page1, this's based on the hypothesis that NAND FS is UBIFS.
> > > This
> > > FS-specifical code is unacceptable in the MTD layer. Also, it
> > > cannot cover all
> > > NAND based file system. Based on the current discussion, seems that
> > > re-write all
> > > first 15 page from page0 is a satisfactory solution.
> >
> > We have a layering problem now. Maybe we should just have an MTD
> > internal variable like min_written_pages_before_erase that the Micron
> > driver could set to a !0 value.
> >
> > Then, the handling could be done by the user (UBI/UBIFS, JFFS2, MTD
> > user if exported).
> >
>
> This is NAND its own problem, if no significant adantage, I don't think
> it's a good solution to extend the problem to the upper FS layer.
> also, in the FS erase path, doesn't have the programmed pages counter.
> we should repeat the same approach as we did in MTD layer.
The problem is that if the filesystem is not aware, it breaks the
"power cut safe" assertion.
There is a problem with JFFS2 and a problem with UBIFS because of that.
We can certainly keep a default implementation like this one for other
users though.
>
> > >
> > > Meanwhile, I borrowed one idea from Miquel Raynal patchset [3], in
> > > which keeps
> > > a recode of programmed pages, base on it, for most of the cases, we
> > > don't need
> > > to read every page to see if current erasing block is a partially
> > > programmed
> > > block.
> > >
> > > Changelog:
> > >
> > > v3 - v4:
> > > 1. In the patch 4/5, change to directly use ecc.strength to
> > > judge the page
> > > is a empty page or not, rather than max_bitflips < mtd-
> > > >bitflip_threshold
> > > 2. In the patch 5/5, for the powerloss case, from the next time
> > > boot up,
> > > lots of page will be programmed from >page15 address, if
> > > still using
> > > first_p as GENMASK() bitmask starting position, writtenp
> > > will be always 0,
> > > fix it by changing its bitmask starting at bit position 0.
> > >
> > > v2 - v3:
> > > 1. Rebase patch to the latest MTD git tree
> > > 2. Add a record that keeps tracking the programmed pages in the
> > > first 16
> > > pages
> > > 3. Change from program odd pages, damage page 0 and page 1, to
> > > program all
> > > first 15 pages
> > > 4. Address issues which exist in the V2.
> > >
> > > v1 - v2:
> > > 1. Rebased V1 to latest Linux kernel.
> > > 2. Add erase preparation function pointer in
> > > nand_manufacturer_ops.
> > >
> > >
> > > [1] https://www.spinics.net/lists/linux-mtd/msg04112.html
> > > [2] https://www.spinics.net/lists/linux-mtd/msg04450.html
> > > [3] https://www.spinics.net/lists/linux-mtd/msg13083.html
> > >
> > >
> > > Bean Huo (5):
> > > mtd: rawnand: group all NAND specific ops into new nand_chip_ops
> > > mtd: rawnand: Add {pre,post}_erase hooks in nand_chip_ops
> > > mtd: rawnand: Add write_oob hook in nand_chip_ops
> > > mtd: rawnand: Introduce a new function
> > > nand_check_is_erased_page()
> > > mtd: rawnand: micron: Micron SLC NAND filling block
> >
> > When you take my patches in your series, especially when not touching
> > them at all, you should keep my Authorship and SoB first, then add
> > your
> > SoB.
> >
>
> sorry for my fault, I thought adding your Signed-off-by in 3/5 is
> suffient. you mean I should add your signed-off-by in 5/5 as well?
> I will do that in next version.
You should keep my Authorship and SoB for both patches + add your SoB
after mine.
Thanks,
Miquèl
next prev parent reply other threads:[~2020-05-19 9:08 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-18 13:59 [PATCH v4 0/5] Micron SLC NAND filling block Bean Huo
2020-05-18 13:59 ` Bean Huo
2020-05-18 13:59 ` [PATCH v4 1/5] mtd: rawnand: group all NAND specific ops into new nand_chip_ops Bean Huo
2020-05-18 13:59 ` Bean Huo
2020-05-18 13:59 ` [PATCH v4 2/5] mtd: rawnand: Add {pre, post}_erase hooks in nand_chip_ops Bean Huo
2020-05-18 13:59 ` [PATCH v4 2/5] mtd: rawnand: Add {pre,post}_erase " Bean Huo
2020-05-18 13:59 ` [PATCH v4 3/5] mtd: rawnand: Add write_oob hook " Bean Huo
2020-05-18 13:59 ` Bean Huo
2020-05-18 13:59 ` [PATCH v4 4/5] mtd: rawnand: Introduce a new function nand_check_is_erased_page() Bean Huo
2020-05-18 13:59 ` Bean Huo
2020-05-18 15:11 ` kbuild test robot
2020-05-18 15:11 ` kbuild test robot
2020-05-18 15:11 ` kbuild test robot
2020-05-18 13:59 ` [PATCH v4 5/5] mtd: rawnand: micron: Micron SLC NAND filling block Bean Huo
2020-05-18 13:59 ` Bean Huo
2020-05-18 18:32 ` Steve deRosier
2020-05-18 18:32 ` Steve deRosier
2020-05-19 9:16 ` Bean Huo
2020-05-19 9:16 ` Bean Huo
2020-05-18 15:22 ` [PATCH v4 0/5] " Miquel Raynal
2020-05-18 15:22 ` Miquel Raynal
2020-05-19 9:04 ` Bean Huo
2020-05-19 9:04 ` Bean Huo
2020-05-19 9:08 ` Miquel Raynal [this message]
2020-05-19 9:08 ` Miquel Raynal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200519110840.2302987f@xps13 \
--to=miquel.raynal@bootlin.com \
--cc=beanhuo@micron.com \
--cc=boris.brezillon@collabora.com \
--cc=derosier@gmail.com \
--cc=huobean@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=richard@nod.at \
--cc=s.hauer@pengutronix.de \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.