diff mbox series

[v2,1/6] mtd: ubi: Do not zero out EC and VID on ECC-ed NOR flashes

Message ID b92b89d130f8075cebf74bc02eb6efe41afc6d75.1714020303.git.Takahiro.Kuwano@infineon.com
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series mtd: Make sure UBIFS does not do multi-pass page programming on flashes that don't support it | expand

Commit Message

Takahiro Kuwano April 25, 2024, 4:52 a.m. UTC
From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

For NOR flashes EC and VID are zeroed out before an erase is issued to
make sure UBI does not mistakenly treat the PEB as used and associate it
with an LEB.

But on some flashes, like the Infineon Semper NOR flash family,
multi-pass page programming is not allowed on the default ECC scheme.
This means zeroing out these magic numbers will result in the flash
throwing a page programming error.

Do not zero out EC and VID for such flashes. A writesize > 1 is an
indication of an ECC-ed flash.

This patch replicates the following upstream linux commit:
f669e74be820 ("ubi: Do not zero out EC and VID on ECC-ed NOR flashes")

Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/ubi/build.c | 4 +---
 drivers/mtd/ubi/io.c    | 9 ++++++++-
 2 files changed, 9 insertions(+), 4 deletions(-)

Comments

Pratyush Yadav April 29, 2024, 2:17 p.m. UTC | #1
On Thu, Apr 25 2024, tkuw584924@gmail.com wrote:

> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

I wonder how authorship should work for such patches. Patches 1, 2, and
6 in this series are very close to what my patches did for Linux. So I
wonder who should get authorship in this case: the person porting the
patch or the person who wrote the original one. Same for patch 5 but
that is still a little more changed from its Linux counterpart.

>
> For NOR flashes EC and VID are zeroed out before an erase is issued to
> make sure UBI does not mistakenly treat the PEB as used and associate it
> with an LEB.
>
> But on some flashes, like the Infineon Semper NOR flash family,
> multi-pass page programming is not allowed on the default ECC scheme.
> This means zeroing out these magic numbers will result in the flash
> throwing a page programming error.
>
> Do not zero out EC and VID for such flashes. A writesize > 1 is an
> indication of an ECC-ed flash.
>
> This patch replicates the following upstream linux commit:
> f669e74be820 ("ubi: Do not zero out EC and VID on ECC-ed NOR flashes")
>
> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Apart from my comment above,

Acked-by: Pratyush Yadav <pratyush@kernel.org>
Tudor Ambarus April 29, 2024, 2:44 p.m. UTC | #2
On 4/29/24 15:17, Pratyush Yadav wrote:
> On Thu, Apr 25 2024, tkuw584924@gmail.com wrote:
> 
>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> I wonder how authorship should work for such patches. Patches 1, 2, and
> 6 in this series are very close to what my patches did for Linux. So I
> wonder who should get authorship in this case: the person porting the
> patch or the person who wrote the original one. Same for patch 5 but
> that is still a little more changed from its Linux counterpart.
> 

I saw it's the one that ports the patch to u-boot even when it's a
1-to-1 match. I guess it's because u-boot has its own dedicated repo.

cut

>>
>> This patch replicates the following upstream linux commit:
>> f669e74be820 ("ubi: Do not zero out EC and VID on ECC-ed NOR flashes")

This paragraph shall give credit to the linux author and at the same
time help others review the changes.
diff mbox series

Patch

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index a1941b8eb8..81c1b7bdbc 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -679,10 +679,8 @@  static int io_init(struct ubi_device *ubi, int max_beb_per1024)
 		ubi->bad_peb_limit = get_bad_peb_limit(ubi, max_beb_per1024);
 	}
 
-	if (ubi->mtd->type == MTD_NORFLASH) {
-		ubi_assert(ubi->mtd->writesize == 1);
+	if (ubi->mtd->type == MTD_NORFLASH)
 		ubi->nor_flash = 1;
-	}
 
 	ubi->min_io_size = ubi->mtd->writesize;
 	ubi->hdrs_min_io_size = ubi->mtd->writesize >> ubi->mtd->subpage_sft;
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 14be95b74b..45699b4a47 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -563,7 +563,14 @@  int ubi_io_sync_erase(struct ubi_device *ubi, int pnum, int torture)
 		return -EROFS;
 	}
 
-	if (ubi->nor_flash) {
+	/*
+	 * If the flash is ECC-ed then we have to erase the ECC block before we
+	 * can write to it. But the write is in preparation to an erase in the
+	 * first place. This means we cannot zero out EC and VID before the
+	 * erase and we just have to hope the flash starts erasing from the
+	 * start of the page.
+	 */
+	if (ubi->nor_flash && ubi->mtd->writesize == 1) {
 		err = nor_erase_prepare(ubi, pnum);
 		if (err)
 			return err;