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 |
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>
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 --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;