diff mbox series

[2/3] UBI: Do not zero out EC and VID when multi-pass writes are not supported

Message ID 20201012180404.6476-3-p.yadav@ti.com
State Changes Requested
Delegated to: Richard Weinberger
Headers show
Series mtd: Make sure UBIFS does not do multi-pass page programming on flashes that don't support it | expand

Commit Message

Pratyush Yadav Oct. 12, 2020, 6:04 p.m. UTC 
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 Cypress Semper S28 SPI 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 when multi-pass writes are not supported.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 drivers/mtd/ubi/io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vignesh Raghavendra Nov. 3, 2020, 11:48 a.m. UTC | #1 
On 10/12/20 11:34 PM, Pratyush Yadav wrote:
> 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 Cypress Semper S28 SPI 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 when multi-pass writes are not supported.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
>  drivers/mtd/ubi/io.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
> index 14d890b00d2c..4023fc4886e3 100644
> --- a/drivers/mtd/ubi/io.c
> +++ b/drivers/mtd/ubi/io.c
> @@ -535,7 +535,7 @@ int ubi_io_sync_erase(struct ubi_device *ubi, int pnum, int torture)
>  		return -EROFS;
>  	}
>  
> -	if (ubi->nor_flash) {
> +	if (ubi->nor_flash && !(ubi->mtd->flags & MTD_NO_MULTI_PASS_WRITE)) {
>  		err = nor_erase_prepare(ubi, pnum);
>  		if (err)
>  			return err;
> 

You may want to get rid of assertion for mtd->writesize != 1 in case of
MTD_NORFLASH.

See drivers/mtd/ubi/build.c::631

        if (ubi->mtd->type == MTD_NORFLASH) {
                ubi_assert(ubi->mtd->writesize == 1);
                ubi->nor_flash = 1;
        }

Regards
Vignesh
Richard Weinberger Nov. 3, 2020, 11:59 a.m. UTC | #2 
----- Ursprüngliche Mail -----
> Von: "Vignesh Raghavendra" <vigneshr@ti.com>
>
> You may want to get rid of assertion for mtd->writesize != 1 in case of
> MTD_NORFLASH.

Agreed. I hope nothing else breaks if NOR has suddenly a writesize >= 1.

Thanks,
//richard
Pratyush Yadav Nov. 3, 2020, 12:47 p.m. UTC | #3 
On 03/11/20 12:59PM, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "Vignesh Raghavendra" <vigneshr@ti.com>
> >
> > You may want to get rid of assertion for mtd->writesize != 1 in case of
> > MTD_NORFLASH.
> 
> Agreed. I hope nothing else breaks if NOR has suddenly a writesize >= 1.

Please see my response on the cover letter which explains why I think 
setting mtd->writesize = nor->page_size is wrong.
diff mbox series

Patch

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 14d890b00d2c..4023fc4886e3 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -535,7 +535,7 @@  int ubi_io_sync_erase(struct ubi_device *ubi, int pnum, int torture)
 		return -EROFS;
 	}
 
-	if (ubi->nor_flash) {
+	if (ubi->nor_flash && !(ubi->mtd->flags & MTD_NO_MULTI_PASS_WRITE)) {
 		err = nor_erase_prepare(ubi, pnum);
 		if (err)
 			return err;