Prevent Nand page writes on Power failure

Prevent Nand page writes on Power failure

[Sascha Hauer]

Hi All,

I have hardware here for which the normal way to turn off is just to cut
the power. When the powercut happens during a NAND page write then we
get more or less completely written pages during next boot. Very rarely
it seems to happen that such a half written page with only very few
flipped bits is erroneously detected as empty and written again which
then results in ECC errors when reading the data.

The Nand in question is a Micron MT29F4G08ABADAH4 and in TN2917 Micron
clearly states:

| Power loss during NAND array operations (especially Program/Erase) is a
| violation of the NAND voltage specifications, which is not supported and
| should be avoided

Micron suggests to make the capacitors on the Nand chips supply input
big enough that every started operation will be finished before the
power goes down. Now we don't have that situation here, what I have
though is a power good status GPIO, so my job is to wire that up to the
Nand write operations.

Now my question is how could that be done? I assume for some people a
power good failure means that we should write all important data away,
rather than preventing any Nand access. Given it's a policy decision I
assume user space should be involved, right?  An option might be to
introcude some sysfs entry to switch mtd devices to readonly mode. Would
that be fine? Other suggestions?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Richard Weinberger]

On Wed, Feb 20, 2019 at 2:58 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
>
> Hi All,
>
> I have hardware here for which the normal way to turn off is just to cut
> the power. When the powercut happens during a NAND page write then we
> get more or less completely written pages during next boot. Very rarely
> it seems to happen that such a half written page with only very few
> flipped bits is erroneously detected as empty and written again which
> then results in ECC errors when reading the data.
>
> The Nand in question is a Micron MT29F4G08ABADAH4 and in TN2917 Micron
> clearly states:
>
> | Power loss during NAND array operations (especially Program/Erase) is a
> | violation of the NAND voltage specifications, which is not supported and
> | should be avoided
>
> Micron suggests to make the capacitors on the Nand chips supply input
> big enough that every started operation will be finished before the
> power goes down. Now we don't have that situation here, what I have
> though is a power good status GPIO, so my job is to wire that up to the
> Nand write operations.
>
> Now my question is how could that be done? I assume for some people a
> power good failure means that we should write all important data away,
> rather than preventing any Nand access. Given it's a policy decision I
> assume user space should be involved, right?  An option might be to
> introcude some sysfs entry to switch mtd devices to readonly mode. Would
> that be fine? Other suggestions?

Well, we need to make sure that no new write/erase command is issued and
the running one can complete. So, in nand_base.c you can (ab)use a lock
to ensure that.
Regarding important data, users that care need to use fsync() anyway, so
there is no need to trigger whatever writeback upon power failure.
Or what else important data do you have in mind?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Sascha Hauer]

On Thu, Feb 21, 2019 at 01:05:04AM +0100, Richard Weinberger wrote:
> On Wed, Feb 20, 2019 at 2:58 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> >
> > Hi All,
> >
> > I have hardware here for which the normal way to turn off is just to cut
> > the power. When the powercut happens during a NAND page write then we
> > get more or less completely written pages during next boot. Very rarely
> > it seems to happen that such a half written page with only very few
> > flipped bits is erroneously detected as empty and written again which
> > then results in ECC errors when reading the data.
> >
> > The Nand in question is a Micron MT29F4G08ABADAH4 and in TN2917 Micron
> > clearly states:
> >
> > | Power loss during NAND array operations (especially Program/Erase) is a
> > | violation of the NAND voltage specifications, which is not supported and
> > | should be avoided
> >
> > Micron suggests to make the capacitors on the Nand chips supply input
> > big enough that every started operation will be finished before the
> > power goes down. Now we don't have that situation here, what I have
> > though is a power good status GPIO, so my job is to wire that up to the
> > Nand write operations.
> >
> > Now my question is how could that be done? I assume for some people a
> > power good failure means that we should write all important data away,
> > rather than preventing any Nand access. Given it's a policy decision I
> > assume user space should be involved, right?  An option might be to
> > introcude some sysfs entry to switch mtd devices to readonly mode. Would
> > that be fine? Other suggestions?
> 
> Well, we need to make sure that no new write/erase command is issued and
> the running one can complete. So, in nand_base.c you can (ab)use a lock
> to ensure that.

Nice idea.

> Regarding important data, users that care need to use fsync() anyway, so
> there is no need to trigger whatever writeback upon power failure.
> Or what else important data do you have in mind?

What you want to do on a power failure probably depends on the time
you have left. When you have a second left then you get other ideas than
when you have just a few milliseconds. If you have enough time then you
maybe even want to do an ordered shutdown which would include writing to
the nand.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Richard Weinberger]

Am Donnerstag, 21. Februar 2019, 09:39:14 CET schrieb Sascha Hauer:
> > Regarding important data, users that care need to use fsync() anyway, so
> > there is no need to trigger whatever writeback upon power failure.
> > Or what else important data do you have in mind?
> 
> What you want to do on a power failure probably depends on the time
> you have left. When you have a second left then you get other ideas than
> when you have just a few milliseconds. If you have enough time then you
> maybe even want to do an ordered shutdown which would include writing to
> the nand.

Well, these are two distinct use cases. Both are sane but have different goals.
If you have a signal that tells "I'm out of power supply and have plenty of time"
you can notify/wake userspace and it is no big deal if the userspace action takes too
long and the board browns out.

But for the NAND stuff you need something that tells you "I'm out of of power
and you need to take action immediately".
Here the action needs to happen in any case because the hardware requires so.

Thanks,
//richard




______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Boris Brezillon]

Hi Sascha,

On Wed, 20 Feb 2019 14:58:20 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Hi All,
> 
> I have hardware here for which the normal way to turn off is just to cut
> the power. When the powercut happens during a NAND page write then we
> get more or less completely written pages during next boot. Very rarely
> it seems to happen that such a half written page with only very few
> flipped bits is erroneously detected as empty and written again which
> then results in ECC errors when reading the data.

This should definitely be fixed, maybe by lowering the bitflip
threshold when doing the empty check. Do you know the ECC strength and
the number of bitflips you have when that problem occurs?

Regards,

Boris

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Sascha Hauer]

Hi Boris,

On Thu, Feb 21, 2019 at 09:10:55AM +0100, Boris Brezillon wrote:
> Hi Sascha,
> 
> On Wed, 20 Feb 2019 14:58:20 +0100
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Hi All,
> > 
> > I have hardware here for which the normal way to turn off is just to cut
> > the power. When the powercut happens during a NAND page write then we
> > get more or less completely written pages during next boot. Very rarely
> > it seems to happen that such a half written page with only very few
> > flipped bits is erroneously detected as empty and written again which
> > then results in ECC errors when reading the data.
> 
> This should definitely be fixed, maybe by lowering the bitflip
> threshold when doing the empty check. Do you know the ECC strength and
> the number of bitflips you have when that problem occurs?

The problem is that these half written pages do not seem to be very
stable. It happens that the number of bitflips change with each read.
I have seen pages which can be read sometimes and sometimes not. It
really seems that half written pages must be avoided entirely.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Boris Brezillon]

On Thu, 21 Feb 2019 11:17:47 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Hi Boris,
> 
> On Thu, Feb 21, 2019 at 09:10:55AM +0100, Boris Brezillon wrote:
> > Hi Sascha,
> > 
> > On Wed, 20 Feb 2019 14:58:20 +0100
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >   
> > > Hi All,
> > > 
> > > I have hardware here for which the normal way to turn off is just to cut
> > > the power. When the powercut happens during a NAND page write then we
> > > get more or less completely written pages during next boot. Very rarely
> > > it seems to happen that such a half written page with only very few
> > > flipped bits is erroneously detected as empty and written again which
> > > then results in ECC errors when reading the data.  
> > 
> > This should definitely be fixed, maybe by lowering the bitflip
> > threshold when doing the empty check. Do you know the ECC strength and
> > the number of bitflips you have when that problem occurs?  
> 
> The problem is that these half written pages do not seem to be very
> stable. It happens that the number of bitflips change with each read.
> I have seen pages which can be read sometimes and sometimes not. It
> really seems that half written pages must be avoided entirely.

But when they are correctly read, do you know how many bitflips they
have? To be honest, I fear not all users will be able to be informed
that powercuts are about to happen, and we need a way to fix that for
everyone.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Sascha Hauer]

On Thu, Feb 21, 2019 at 11:36:46AM +0100, Boris Brezillon wrote:
> On Thu, 21 Feb 2019 11:17:47 +0100
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Hi Boris,
> > 
> > On Thu, Feb 21, 2019 at 09:10:55AM +0100, Boris Brezillon wrote:
> > > Hi Sascha,
> > > 
> > > On Wed, 20 Feb 2019 14:58:20 +0100
> > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > >   
> > > > Hi All,
> > > > 
> > > > I have hardware here for which the normal way to turn off is just to cut
> > > > the power. When the powercut happens during a NAND page write then we
> > > > get more or less completely written pages during next boot. Very rarely
> > > > it seems to happen that such a half written page with only very few
> > > > flipped bits is erroneously detected as empty and written again which
> > > > then results in ECC errors when reading the data.  
> > > 
> > > This should definitely be fixed, maybe by lowering the bitflip
> > > threshold when doing the empty check. Do you know the ECC strength and
> > > the number of bitflips you have when that problem occurs?  
> > 
> > The problem is that these half written pages do not seem to be very
> > stable. It happens that the number of bitflips change with each read.
> > I have seen pages which can be read sometimes and sometimes not. It
> > really seems that half written pages must be avoided entirely.
> 
> But when they are correctly read, do you know how many bitflips they
> have?

Yes, I know this number, but as said, it varies when reading the same
page again.

> To be honest, I fear not all users will be able to be informed
> that powercuts are about to happen, and we need a way to fix that for
> everyone.

If you want that I'm afraid we can't fix this on this level. You can't
rely on the data when a powercut happens during write. It may look ok at
first, but bitflips can develop later. I just found an interesting read
here:
https://www.datalight.com/blog/2017/03/08/enemy-of-nand-flash-memory/

If you want to fix it for all users you have to track somehow which
pages you have written last and discard the data or copy it to another
block.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Boris Brezillon]

On Thu, 21 Feb 2019 12:21:38 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Thu, Feb 21, 2019 at 11:36:46AM +0100, Boris Brezillon wrote:
> > On Thu, 21 Feb 2019 11:17:47 +0100
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >   
> > > Hi Boris,
> > > 
> > > On Thu, Feb 21, 2019 at 09:10:55AM +0100, Boris Brezillon wrote:  
> > > > Hi Sascha,
> > > > 
> > > > On Wed, 20 Feb 2019 14:58:20 +0100
> > > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > >     
> > > > > Hi All,
> > > > > 
> > > > > I have hardware here for which the normal way to turn off is just to cut
> > > > > the power. When the powercut happens during a NAND page write then we
> > > > > get more or less completely written pages during next boot. Very rarely
> > > > > it seems to happen that such a half written page with only very few
> > > > > flipped bits is erroneously detected as empty and written again which
> > > > > then results in ECC errors when reading the data.    
> > > > 
> > > > This should definitely be fixed, maybe by lowering the bitflip
> > > > threshold when doing the empty check. Do you know the ECC strength and
> > > > the number of bitflips you have when that problem occurs?    
> > > 
> > > The problem is that these half written pages do not seem to be very
> > > stable. It happens that the number of bitflips change with each read.
> > > I have seen pages which can be read sometimes and sometimes not. It
> > > really seems that half written pages must be avoided entirely.  
> > 
> > But when they are correctly read, do you know how many bitflips they
> > have?  
> 
> Yes, I know this number,

Is it high? I mean, can we tweak the threshold to detect such cases?

> but as said, it varies when reading the same
> page again.

Yes, I can imagine, as cells were weakly programmed, their state might
change from one read to another. What I'm mainly interested in is the
state on the first read after the powercut.

> 
> > To be honest, I fear not all users will be able to be informed
> > that powercuts are about to happen, and we need a way to fix that for
> > everyone.  
> 
> If you want that I'm afraid we can't fix this on this level. You can't
> rely on the data when a powercut happens during write. It may look ok at
> first, but bitflips can develop later. I just found an interesting read
> here:
> https://www.datalight.com/blog/2017/03/08/enemy-of-nand-flash-memory/
> 
> If you want to fix it for all users you have to track somehow which
> pages you have written last and discard the data or copy it to another
> block.

Did you have the problem that pages written with valid data turn into
uncorrectable pages on the second read attempt or is it just
erased/empty pages that turn out to be not so empty/erased and next
time you write to them you end up with uncorrectable errors?

Other than that, yes, tracking which block was written last so that you
can move the data somewhere else in case of an unclean detach/unmount
is an option. Don't know if UBIFS has this information (UBI doesn't,
that's for sure).

> 
> Sascha
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Richard Weinberger]

On Thu, Feb 21, 2019 at 12:51 PM Boris Brezillon
<boris.brezillon@collabora.com> wrote:
> Other than that, yes, tracking which block was written last so that you
> can move the data somewhere else in case of an unclean detach/unmount
> is an option. Don't know if UBIFS has this information (UBI doesn't,
> that's for sure).

Yes, neither UBI nor UBIFS have this information.
With reasonable effort we might compute this information using the
seqeuence numbers.

-- 
Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Sascha Hauer]

On Thu, Feb 21, 2019 at 12:37:09PM +0100, Boris Brezillon wrote:
> On Thu, 21 Feb 2019 12:21:38 +0100
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > On Thu, Feb 21, 2019 at 11:36:46AM +0100, Boris Brezillon wrote:
> > > On Thu, 21 Feb 2019 11:17:47 +0100
> > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > >   
> > > > Hi Boris,
> > > > 
> > > > On Thu, Feb 21, 2019 at 09:10:55AM +0100, Boris Brezillon wrote:  
> > > > > Hi Sascha,
> > > > > 
> > > > > On Wed, 20 Feb 2019 14:58:20 +0100
> > > > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > > >     
> > > > > > Hi All,
> > > > > > 
> > > > > > I have hardware here for which the normal way to turn off is just to cut
> > > > > > the power. When the powercut happens during a NAND page write then we
> > > > > > get more or less completely written pages during next boot. Very rarely
> > > > > > it seems to happen that such a half written page with only very few
> > > > > > flipped bits is erroneously detected as empty and written again which
> > > > > > then results in ECC errors when reading the data.    
> > > > > 
> > > > > This should definitely be fixed, maybe by lowering the bitflip
> > > > > threshold when doing the empty check. Do you know the ECC strength and
> > > > > the number of bitflips you have when that problem occurs?    
> > > > 
> > > > The problem is that these half written pages do not seem to be very
> > > > stable. It happens that the number of bitflips change with each read.
> > > > I have seen pages which can be read sometimes and sometimes not. It
> > > > really seems that half written pages must be avoided entirely.  
> > > 
> > > But when they are correctly read, do you know how many bitflips they
> > > have?  
> > 
> > Yes, I know this number,
> 
> Is it high? I mean, can we tweak the threshold to detect such cases?

I haven't told you the number because I have seen pages with a single
bitflip, pages with 3 bitflips, pages with 8 bitflips, anything you
like.
Funny enough, once I write to following pages up to the end of the block
then the page which previously had only a few bitflips turns into a
nearly random pattern.

> 
> > but as said, it varies when reading the same
> > page again.
> 
> Yes, I can imagine, as cells were weakly programmed, their state might
> change from one read to another. What I'm mainly interested in is the
> state on the first read after the powercut.
> 
> > 
> > > To be honest, I fear not all users will be able to be informed
> > > that powercuts are about to happen, and we need a way to fix that for
> > > everyone.  
> > 
> > If you want that I'm afraid we can't fix this on this level. You can't
> > rely on the data when a powercut happens during write. It may look ok at
> > first, but bitflips can develop later. I just found an interesting read
> > here:
> > https://www.datalight.com/blog/2017/03/08/enemy-of-nand-flash-memory/
> > 
> > If you want to fix it for all users you have to track somehow which
> > pages you have written last and discard the data or copy it to another
> > block.
> 
> Did you have the problem that pages written with valid data turn into
> uncorrectable pages on the second read attempt or is it just
> erased/empty pages that turn out to be not so empty/erased and next
> time you write to them you end up with uncorrectable errors?

Both (seemingly) valid pages turned into uncorrectable pages, but as
said above pages turned into garbage once I wrote other pages in the
same block.

I haven't made any tests with power cuts during erase. Power cuts during
erase are probably not that critical because blocks without a valid
erase marker will be erased again next time.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Sascha Hauer]

On Wed, Feb 20, 2019 at 02:58:20PM +0100, Sascha Hauer wrote:
> 
> Hi All,
> 
> I have hardware here for which the normal way to turn off is just to cut
> the power. When the powercut happens during a NAND page write then we
> get more or less completely written pages during next boot. Very rarely
> it seems to happen that such a half written page with only very few
> flipped bits is erroneously detected as empty and written again which
> then results in ECC errors when reading the data.
> 
> The Nand in question is a Micron MT29F4G08ABADAH4 and in TN2917 Micron
> clearly states:
> 
> | Power loss during NAND array operations (especially Program/Erase) is a
> | violation of the NAND voltage specifications, which is not supported and
> | should be avoided
> 
> Micron suggests to make the capacitors on the Nand chips supply input
> big enough that every started operation will be finished before the
> power goes down. Now we don't have that situation here, what I have
> though is a power good status GPIO, so my job is to wire that up to the
> Nand write operations.
> 
> Now my question is how could that be done? I assume for some people a
> power good failure means that we should write all important data away,
> rather than preventing any Nand access. Given it's a policy decision I
> assume user space should be involved, right?  An option might be to
> introcude some sysfs entry to switch mtd devices to readonly mode. Would
> that be fine? Other suggestions?
> 

FYI the attached patch is what I have ended up with for now. Not
upstreamable of course, but works for me. I have the feeling that it's
not the last time I have this problem, so an upstreamable solution is
really desired.

Sascha

-----------------------------8<--------------------------------

From 04e76aaea00d2f92b51904e2b1611a3d64081e71 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Thu, 21 Feb 2019 12:30:29 +0100
Subject: [PATCH] mtd: nand: raw: Allow to prevent write operations on power
 failures

When doing write operations the Nand chips supply must be within
specified ranges, otherwise written pages can be corrupted in various
ways, including cases where a page looks empty, but gets corrupted when
written to, or pages which first look good, but get corrupted over time.

This adds a power_good mutex to the nand chip which is acquired before
write/erase and released afterwards. Board code can acquire this mutex
on a power failure to prevent further write accesses to the Nand chip.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 50 ++++++++++++++++++++++
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h |  2 +
 drivers/mtd/nand/raw/nand_base.c           | 22 +++++++---
 include/linux/mtd/rawnand.h                |  2 +
 4 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index 1c1ebbc82824..b13ddf7b7b75 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -13,6 +13,7 @@
 #include <linux/mtd/partitions.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/gpio/consumer.h>
 #include "gpmi-nand.h"
 #include "bch-regs.h"
 
@@ -628,6 +629,50 @@ err_clock:
 	return err;
 }
 
+static irqreturn_t gpmi_pwr_good_irq(int irq, void *dev_id)
+{
+	struct gpmi_nand_data *this = dev_id;
+	int level;
+
+	level = gpiod_get_value_cansleep(this->pwrgood_gpio);
+	if (level) {
+		mutex_lock(&this->nand.power_lock);
+	} else {
+		mutex_unlock(&this->nand.power_lock);
+		dev_info(this->dev, "Spurious Power Failure detected\n");
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int gpmi_get_pwrgood(struct gpmi_nand_data *this)
+{
+	struct gpio_desc *desc;
+	int irq, ret;
+
+	desc = devm_gpiod_get_index_optional(this->dev, "pwrgood", 0, GPIOD_IN);
+	if (!desc)
+		return 0;
+
+	if (IS_ERR(desc))
+		return PTR_ERR(desc);
+
+	irq = gpiod_to_irq(desc);
+
+	ret = devm_request_threaded_irq(this->dev, irq,
+			NULL, gpmi_pwr_good_irq,
+			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+			"pwrgood", this);
+	if (ret)
+		return ret;
+
+	this->pwrgood_gpio = desc;
+
+	dev_info(this->dev, "using irq %d as power good input\n", irq);
+
+	return 0;
+}
+
 static int acquire_resources(struct gpmi_nand_data *this)
 {
 	int ret;
@@ -651,6 +696,11 @@ static int acquire_resources(struct gpmi_nand_data *this)
 	ret = gpmi_get_clks(this);
 	if (ret)
 		goto exit_clock;
+
+	ret = gpmi_get_pwrgood(this);
+	if (ret)
+		goto exit_clock;
+
 	return 0;
 
 exit_clock:
diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h
index 69cd0cbde4f2..251e02bb4338 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h
@@ -148,6 +148,8 @@ struct gpmi_nand_data {
 	void			*auxiliary_virt;
 	dma_addr_t		auxiliary_phys;
 
+	struct gpio_desc	*pwrgood_gpio;
+
 	void			*raw_buffer;
 
 	/* DMA channels */
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index d527e448ce19..d9487503bec0 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -2115,6 +2115,8 @@ int nand_erase_op(struct nand_chip *chip, unsigned int eraseblock)
 	int ret;
 	u8 status;
 
+	mutex_lock(&chip->power_lock);
+
 	if (chip->exec_op) {
 		const struct nand_sdr_timings *sdr =
 			nand_get_sdr_timings(&chip->data_interface);
@@ -2133,26 +2135,30 @@ int nand_erase_op(struct nand_chip *chip, unsigned int eraseblock)
 
 		ret = nand_exec_op(chip, &op);
 		if (ret)
-			return ret;
+			goto out;
 
 		ret = nand_status_op(chip, &status);
 		if (ret)
-			return ret;
+			goto out;
 	} else {
 		chip->cmdfunc(mtd, NAND_CMD_ERASE1, -1, page);
 		chip->cmdfunc(mtd, NAND_CMD_ERASE2, -1, -1);
 
 		ret = chip->waitfunc(mtd, chip);
 		if (ret < 0)
-			return ret;
+			goto out;
 
 		status = ret;
 	}
 
 	if (status & NAND_STATUS_FAIL)
-		return -EIO;
+		ret = -EIO;
+	else
+		ret = 0;
+out:
+	mutex_unlock(&chip->power_lock);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(nand_erase_op);
 
@@ -4330,6 +4336,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	else
 		subpage = 0;
 
+	mutex_lock(&chip->power_lock);
+
 	if (unlikely(raw))
 		status = chip->ecc.write_page_raw(mtd, chip, buf,
 						  oob_required, page);
@@ -4340,6 +4348,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 		status = chip->ecc.write_page(mtd, chip, buf, oob_required,
 					      page);
 
+	mutex_unlock(&chip->power_lock);
+
 	if (status < 0)
 		return status;
 
@@ -6786,6 +6796,8 @@ int nand_scan_with_ids(struct mtd_info *mtd, int maxchips,
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	int ret;
 
+	mutex_init(&chip->power_lock);
+
 	if (maxchips) {
 		ret = nand_scan_ident(mtd, maxchips, ids);
 		if (ret)
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index efb2345359bb..638b187eb198 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1357,6 +1357,8 @@ struct nand_chip {
 
 	struct nand_bbt_descr *badblock_pattern;
 
+	struct mutex power_lock;
+
 	void *priv;
 
 	struct {
-- 
2.20.1


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Richard Weinberger]

On Mon, Feb 25, 2019 at 9:56 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> +static irqreturn_t gpmi_pwr_good_irq(int irq, void *dev_id)
> +{
> +       struct gpmi_nand_data *this = dev_id;
> +       int level;
> +
> +       level = gpiod_get_value_cansleep(this->pwrgood_gpio);
> +       if (level) {
> +               mutex_lock(&this->nand.power_lock);
> +       } else {
> +               mutex_unlock(&this->nand.power_lock);
> +               dev_info(this->dev, "Spurious Power Failure detected\n");
> +       }

Hmm, not sure if it is wise to unlock after the power-low pin was toggled.
Do you have a guarantee that the system, and most important the NAND chip,
is in a well defined state after the power returns?

-- 
Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Prevent Nand page writes on Power failure

[Sascha Hauer]

On Mon, Feb 25, 2019 at 12:28:15PM +0100, Richard Weinberger wrote:
> On Mon, Feb 25, 2019 at 9:56 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > +static irqreturn_t gpmi_pwr_good_irq(int irq, void *dev_id)
> > +{
> > +       struct gpmi_nand_data *this = dev_id;
> > +       int level;
> > +
> > +       level = gpiod_get_value_cansleep(this->pwrgood_gpio);
> > +       if (level) {
> > +               mutex_lock(&this->nand.power_lock);
> > +       } else {
> > +               mutex_unlock(&this->nand.power_lock);
> > +               dev_info(this->dev, "Spurious Power Failure detected\n");
> > +       }
> 
> Hmm, not sure if it is wise to unlock after the power-low pin was toggled.
> Do you have a guarantee that the system, and most important the NAND chip,
> is in a well defined state after the power returns?

No, not at all ;)

I could remove the mutex_unlock() in the spurious power failure path,
but I think I'll figure out what to do when I see the message in the
logs.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/