Re: [PATCH 4.4 03/34] mtd: nand: gpmi: Fix failure when a erased page has a bitflip at BBM

2018-03-07 Thread Sasha Levin
On Wed, Mar 07, 2018 at 09:12:36AM +0100, Boris Brezillon wrote:
>On Tue, 06 Mar 2018 21:22:30 +
>Ben Hutchings  wrote:
>
>> On Fri, 2018-03-02 at 09:50 +0100, Greg Kroah-Hartman wrote:
>> > 4.4-stable review patch.  If anyone has any objections, please let me know.
>> >
>> > --
>> >
>> > From: Sascha Hauer 
>> >
>> >
>> > [ Upstream commit fdf2e821052958a114618a95ab18a300d0b080cb ]
>> >
>> > When erased subpages are read then the BCH decoder returns STATUS_ERASED
>> > if they are all empty, or STATUS_UNCORRECTABLE if there are bitflips.
>> > When there are bitflips, we have to set these bits again to show the
>> > upper layers a completely erased page. When a bitflip happens in the
>> > exact byte where the bad block marker is, then this byte is swapped
>> > with another byte in block_mark_swapping(). The correction code then
>> > detects a bitflip in another subpage and no longer corrects the bitflip
>> > where it really happens.
>> [...]
>>
>> This seesm to be a bug fix for commit bd2e778c9ee3 "gpmi-nand: Handle
>> ECC Errors in erased pages".  That's not in 4.4 so the bug fix is not
>> needed, though it doesn't appear to do any harm.
>
>I wonder why the fix was backported to stable releases in the first
>place. AFAICS, there's no Cc-stable or Fixes tag in the original
>commit. It's probably something in the backport-to-stable process I'm
>not aware of.

It's an attempt to mine kernel commits for commits that should go into
stable trees but were not marked as such.

Thanks again for the review!

-- 

Thanks,
Sasha

Re: [PATCH 4.4 03/34] mtd: nand: gpmi: Fix failure when a erased page has a bitflip at BBM

2018-03-07 Thread Richard Weinberger
Boris,

Am Mittwoch, 7. März 2018, 09:12:36 CET schrieb Boris Brezillon:
> On Tue, 06 Mar 2018 21:22:30 +
> 
> Ben Hutchings  wrote:
> > On Fri, 2018-03-02 at 09:50 +0100, Greg Kroah-Hartman wrote:
> > > 4.4-stable review patch.  If anyone has any objections, please let me
> > > know.
> > > 
> > > --
> > > 
> > > From: Sascha Hauer 
> > > 
> > > 
> > > [ Upstream commit fdf2e821052958a114618a95ab18a300d0b080cb ]
> > > 
> > > When erased subpages are read then the BCH decoder returns STATUS_ERASED
> > > if they are all empty, or STATUS_UNCORRECTABLE if there are bitflips.
> > > When there are bitflips, we have to set these bits again to show the
> > > upper layers a completely erased page. When a bitflip happens in the
> > > exact byte where the bad block marker is, then this byte is swapped
> > > with another byte in block_mark_swapping(). The correction code then
> > > detects a bitflip in another subpage and no longer corrects the bitflip
> > > where it really happens.
> > 
> > [...]
> > 
> > This seesm to be a bug fix for commit bd2e778c9ee3 "gpmi-nand: Handle
> > ECC Errors in erased pages".  That's not in 4.4 so the bug fix is not
> > needed, though it doesn't appear to do any harm.
> 
> I wonder why the fix was backported to stable releases in the first
> place. AFAICS, there's no Cc-stable or Fixes tag in the original
> commit. It's probably something in the backport-to-stable process I'm
> not aware of.

It was auto selected by a stable script.

Thanks,
//richard


Re: [PATCH 4.4 03/34] mtd: nand: gpmi: Fix failure when a erased page has a bitflip at BBM

2018-03-07 Thread Boris Brezillon
On Tue, 06 Mar 2018 21:22:30 +
Ben Hutchings  wrote:

> On Fri, 2018-03-02 at 09:50 +0100, Greg Kroah-Hartman wrote:
> > 4.4-stable review patch.  If anyone has any objections, please let me know.
> > 
> > --
> > 
> > From: Sascha Hauer 
> > 
> > 
> > [ Upstream commit fdf2e821052958a114618a95ab18a300d0b080cb ]
> > 
> > When erased subpages are read then the BCH decoder returns STATUS_ERASED
> > if they are all empty, or STATUS_UNCORRECTABLE if there are bitflips.
> > When there are bitflips, we have to set these bits again to show the
> > upper layers a completely erased page. When a bitflip happens in the
> > exact byte where the bad block marker is, then this byte is swapped
> > with another byte in block_mark_swapping(). The correction code then
> > detects a bitflip in another subpage and no longer corrects the bitflip
> > where it really happens.  
> [...]
> 
> This seesm to be a bug fix for commit bd2e778c9ee3 "gpmi-nand: Handle
> ECC Errors in erased pages".  That's not in 4.4 so the bug fix is not
> needed, though it doesn't appear to do any harm.

I wonder why the fix was backported to stable releases in the first
place. AFAICS, there's no Cc-stable or Fixes tag in the original
commit. It's probably something in the backport-to-stable process I'm
not aware of.

Anyway, not an issues since the changes seems to be harmless.

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH 4.4 03/34] mtd: nand: gpmi: Fix failure when a erased page has a bitflip at BBM

2018-03-06 Thread Ben Hutchings
On Fri, 2018-03-02 at 09:50 +0100, Greg Kroah-Hartman wrote:
> 4.4-stable review patch.  If anyone has any objections, please let me know.
> 
> --
> 
> From: Sascha Hauer 
> 
> 
> [ Upstream commit fdf2e821052958a114618a95ab18a300d0b080cb ]
> 
> When erased subpages are read then the BCH decoder returns STATUS_ERASED
> if they are all empty, or STATUS_UNCORRECTABLE if there are bitflips.
> When there are bitflips, we have to set these bits again to show the
> upper layers a completely erased page. When a bitflip happens in the
> exact byte where the bad block marker is, then this byte is swapped
> with another byte in block_mark_swapping(). The correction code then
> detects a bitflip in another subpage and no longer corrects the bitflip
> where it really happens.
[...]

This seesm to be a bug fix for commit bd2e778c9ee3 "gpmi-nand: Handle
ECC Errors in erased pages".  That's not in 4.4 so the bug fix is not
needed, though it doesn't appear to do any harm.

Ben.

-- 
Ben Hutchings
Software Developer, Codethink Ltd.



[PATCH 4.4 03/34] mtd: nand: gpmi: Fix failure when a erased page has a bitflip at BBM

2018-03-02 Thread Greg Kroah-Hartman
4.4-stable review patch.  If anyone has any objections, please let me know.

--

From: Sascha Hauer 


[ Upstream commit fdf2e821052958a114618a95ab18a300d0b080cb ]

When erased subpages are read then the BCH decoder returns STATUS_ERASED
if they are all empty, or STATUS_UNCORRECTABLE if there are bitflips.
When there are bitflips, we have to set these bits again to show the
upper layers a completely erased page. When a bitflip happens in the
exact byte where the bad block marker is, then this byte is swapped
with another byte in block_mark_swapping(). The correction code then
detects a bitflip in another subpage and no longer corrects the bitflip
where it really happens.

Correct this behaviour by calling block_mark_swapping() after the
bitflips have been corrected.

In our case UBIFS failed with this bug because it expects erased
pages to be really empty:

UBIFS error (pid 187): ubifs_scan: corrupt empty space at LEB 36:118735
UBIFS error (pid 187): ubifs_scanned_corruption: corruption at LEB 36:118735
UBIFS error (pid 187): ubifs_scanned_corruption: first 8192 bytes from LEB 
36:118735
UBIFS error (pid 187): ubifs_scan: LEB 36 scanning failed
UBIFS error (pid 187): do_commit: commit failed, error -117

Signed-off-by: Sascha Hauer 
Reviewed-by: Richard Weinberger 
Acked-by: Boris Brezillon 
Signed-off-by: Richard Weinberger 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1029,9 +1029,6 @@ static int gpmi_ecc_read_page(struct mtd
return ret;
}
 
-   /* handle the block mark swapping */
-   block_mark_swapping(this, payload_virt, auxiliary_virt);
-
/* Loop over status bytes, accumulating ECC status. */
status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
 
@@ -1047,6 +1044,9 @@ static int gpmi_ecc_read_page(struct mtd
max_bitflips = max_t(unsigned int, max_bitflips, *status);
}
 
+   /* handle the block mark swapping */
+   block_mark_swapping(this, buf, auxiliary_virt);
+
if (oob_required) {
/*
 * It's time to deliver the OOB bytes. See gpmi_ecc_read_oob()