2020-12-02 23:03:30

by Michael Walle

[permalink] [raw]
Subject: [PATCH v7 2/7] mtd: spi-nor: ignore errors in spi_nor_unlock_all()

Just try to unlock the whole SPI-NOR flash array. Don't abort the
probing in case of an error. Justifications:
(1) For some boards, this just works because
spi_nor_write_16bit_sr_and_check() is broken and just checks the
second half of the 16bit. Once that will be fixed, SPI probe will
fail for boards which has hardware-write protected SPI-NOR flashes.
(2) Until now, hardware write-protection was the only viable solution
to use the block protection bits. This is because this very
function spi_nor_unlock_all() will be called unconditionally on
every linux boot. Therefore, this bits only makes sense in
combination with the hardware write-protection. If we would fail
the SPI probe on an error in spi_nor_unlock_all() we'd break
virtually all users of the block protection bits.
(3) We should try hard to keep the MTD working even if the flash might
not be writable/erasable.

Fixes: 3e0930f109e7 ("mtd: spi-nor: Rework the disabling of block write protection")
Signed-off-by: Michael Walle <[email protected]>
---
changes since v6:
- new patch

drivers/mtd/spi-nor/core.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 5bee7c8da4dc..013198abe929 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3121,20 +3121,27 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
}

/**
- * spi_nor_unlock_all() - Unlocks the entire flash memory array.
+ * spi_nor_try_unlock_all() - Tries to unlock the entire flash memory array.
* @nor: pointer to a 'struct spi_nor'.
*
* Some SPI NOR flashes are write protected by default after a power-on reset
* cycle, in order to avoid inadvertent writes during power-up. Backward
* compatibility imposes to unlock the entire flash memory array at power-up
* by default.
+ *
+ * Unprotecting the entire flash array will fail for boards which are hardware
+ * write-protected. Thus any errors are ignored.
*/
-static int spi_nor_unlock_all(struct spi_nor *nor)
+static void spi_nor_try_unlock_all(struct spi_nor *nor)
{
- if (nor->flags & SNOR_F_HAS_LOCK)
- return spi_nor_unlock(&nor->mtd, 0, nor->params->size);
+ int ret;

- return 0;
+ if (!(nor->flags & SNOR_F_HAS_LOCK))
+ return;
+
+ ret = spi_nor_unlock(&nor->mtd, 0, nor->params->size);
+ if (ret)
+ dev_dbg(nor->dev, "Failed to unlock the entire flash memory array\n");
}

static int spi_nor_init(struct spi_nor *nor)
@@ -3153,11 +3160,7 @@ static int spi_nor_init(struct spi_nor *nor)
return err;
}

- err = spi_nor_unlock_all(nor);
- if (err) {
- dev_dbg(nor->dev, "Failed to unlock the entire flash memory array\n");
- return err;
- }
+ spi_nor_try_unlock_all(nor);

if (nor->addr_width == 4 &&
nor->read_proto != SNOR_PROTO_8_8_8_DTR &&
--
2.20.1


2020-12-03 14:39:44

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v7 2/7] mtd: spi-nor: ignore errors in spi_nor_unlock_all()

On 12/3/20 1:00 AM, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Just try to unlock the whole SPI-NOR flash array. Don't abort the
> probing in case of an error. Justifications:
> (1) For some boards, this just works because
> spi_nor_write_16bit_sr_and_check() is broken and just checks the
> second half of the 16bit. Once that will be fixed, SPI probe will
> fail for boards which has hardware-write protected SPI-NOR flashes.
> (2) Until now, hardware write-protection was the only viable solution
> to use the block protection bits. This is because this very
> function spi_nor_unlock_all() will be called unconditionally on
> every linux boot. Therefore, this bits only makes sense in
> combination with the hardware write-protection. If we would fail
> the SPI probe on an error in spi_nor_unlock_all() we'd break
> virtually all users of the block protection bits.
> (3) We should try hard to keep the MTD working even if the flash might
> not be writable/erasable.
>
> Fixes: 3e0930f109e7 ("mtd: spi-nor: Rework the disabling of block write protection")
> Signed-off-by: Michael Walle <[email protected]>

Reviewed-by: Tudor Ambarus <[email protected]>

> ---
> changes since v6:
> - new patch
>
> drivers/mtd/spi-nor/core.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 5bee7c8da4dc..013198abe929 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3121,20 +3121,27 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
> }
>
> /**
> - * spi_nor_unlock_all() - Unlocks the entire flash memory array.
> + * spi_nor_try_unlock_all() - Tries to unlock the entire flash memory array.
> * @nor: pointer to a 'struct spi_nor'.
> *
> * Some SPI NOR flashes are write protected by default after a power-on reset
> * cycle, in order to avoid inadvertent writes during power-up. Backward
> * compatibility imposes to unlock the entire flash memory array at power-up
> * by default.
> + *
> + * Unprotecting the entire flash array will fail for boards which are hardware
> + * write-protected. Thus any errors are ignored.
> */
> -static int spi_nor_unlock_all(struct spi_nor *nor)
> +static void spi_nor_try_unlock_all(struct spi_nor *nor)
> {
> - if (nor->flags & SNOR_F_HAS_LOCK)
> - return spi_nor_unlock(&nor->mtd, 0, nor->params->size);
> + int ret;
>
> - return 0;
> + if (!(nor->flags & SNOR_F_HAS_LOCK))
> + return;
> +
> + ret = spi_nor_unlock(&nor->mtd, 0, nor->params->size);
> + if (ret)
> + dev_dbg(nor->dev, "Failed to unlock the entire flash memory array\n");
> }
>
> static int spi_nor_init(struct spi_nor *nor)
> @@ -3153,11 +3160,7 @@ static int spi_nor_init(struct spi_nor *nor)
> return err;
> }
>
> - err = spi_nor_unlock_all(nor);
> - if (err) {
> - dev_dbg(nor->dev, "Failed to unlock the entire flash memory array\n");
> - return err;
> - }
> + spi_nor_try_unlock_all(nor);
>
> if (nor->addr_width == 4 &&
> nor->read_proto != SNOR_PROTO_8_8_8_DTR &&
> --
> 2.20.1
>