Re: [RESEND PATCH v3 09/20] mtd: spi-nor: Create a ->set_4byte() method

Vignesh Raghavendra
2019-08-27T06:07:11Z

On 26/08/19 5:38 PM, Tudor.Ambarus@microchip.com wrote:
> From: Boris Brezillon <boris.brezillon@bootlin.com>
>
> The procedure used to enable 4 byte addressing mode depends on the NOR
> device, so let's provide a hook so that manufacturer specific handling
> can be implemented in a sane way.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> [tudor.ambarus@microchip.com: use nor->params.set_4byte() instead of
> nor->set_4byte()]
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

> v3: no changes>
> drivers/mtd/spi-nor/spi-nor.c | 76 ++++++++++++++++++++++---------------------
> include/linux/mtd/spi-nor.h | 2 ++
> 2 files changed, 41 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 1e7f8dc3457d..235e82a121a1 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -633,6 +633,17 @@ static int macronix_set_4byte(struct spi_nor *nor, bool enable)
> NULL, 0);
> }
>
> +static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
> +{
> + int ret;
> +
> + write_enable(nor);
> + ret = macronix_set_4byte(nor, enable);
> + write_disable(nor);
> +
> + return ret;
> +}
> +
> static int spansion_set_4byte(struct spi_nor *nor, bool enable)
> {
> nor->bouncebuf[0] = enable << 7;
> @@ -667,45 +678,24 @@ static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
> return nor->write_reg(nor, SPINOR_OP_WREAR, nor->bouncebuf, 1);
> }
>
> -/* Enable/disable 4-byte addressing mode. */
> -static int set_4byte(struct spi_nor *nor, bool enable)
> +static int winbond_set_4byte(struct spi_nor *nor, bool enable)
> {
> - int status;
> - bool need_wren = false;
> -
> - switch (JEDEC_MFR(nor->info)) {
> - case SNOR_MFR_ST:
> - case SNOR_MFR_MICRON:
> - /* Some Micron need WREN command; all will accept it */
> - need_wren = true;
> - /* fall through */
> - case SNOR_MFR_MACRONIX:
> - case SNOR_MFR_WINBOND:
> - if (need_wren)
> - write_enable(nor);
> + int ret;
>
> - status = macronix_set_4byte(nor, enable);
> - if (need_wren)
> - write_disable(nor);
> + ret = macronix_set_4byte(nor, enable);
> + if (ret || enable)
> + return ret;
>
> - if (!status && !enable &&
> - JEDEC_MFR(nor->info) == SNOR_MFR_WINBOND) {
> - /*
> - * On Winbond W25Q256FV, leaving 4byte mode causes
> - * the Extended Address Register to be set to 1, so all
> - * 3-byte-address reads come from the second 16M.
> - * We must clear the register to enable normal behavior.
> - */
> - write_enable(nor);
> - spi_nor_write_ear(nor, 0);
> - write_disable(nor);
> - }
> + /*
> + * On Winbond W25Q256FV, leaving 4byte mode causes the Extended Address
> + * Register to be set to 1, so all 3-byte-address reads come from the
> + * second 16M. We must clear the register to enable normal behavior.
> + */
> + write_enable(nor);
> + ret = spi_nor_write_ear(nor, 0);
> + write_disable(nor);
>
> - return status;
> - default:
> - /* Spansion style */
> - return spansion_set_4byte(nor, enable);
> - }
> + return ret;
> }
>
> static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
> @@ -4153,11 +4143,18 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
> static void macronix_set_default_init(struct spi_nor *nor)
> {
> nor->params.quad_enable = macronix_quad_enable;
> + nor->params.set_4byte = macronix_set_4byte;
> }
>
> static void st_micron_set_default_init(struct spi_nor *nor)
> {
> nor->params.quad_enable = NULL;
> + nor->params.set_4byte = st_micron_set_4byte;
> +}
> +
> +static void winbond_set_default_init(struct spi_nor *nor)
> +{
> + nor->params.set_4byte = winbond_set_4byte;
> }
>
> /**
> @@ -4178,6 +4175,10 @@ static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
> st_micron_set_default_init(nor);
> break;
>
> + case SNOR_MFR_WINBOND:
> + winbond_set_default_init(nor);
> + break;
> +
> default:
> break;
> }
> @@ -4222,6 +4223,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
>
> /* Initialize legacy flash parameters and settings. */
> params->quad_enable = spansion_quad_enable;
> + params->set_4byte = spansion_set_4byte;
>
> /* Set SPI NOR sizes. */
> params->size = (u64)info->sector_size * info->n_sectors;
> @@ -4587,7 +4589,7 @@ static int spi_nor_init(struct spi_nor *nor)
> */
> WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
> "enabling reset hack; may not recover from unexpected reboots\n");
> - set_4byte(nor, true);
> + nor->params.set_4byte(nor, true);
> }
>
> return 0;
> @@ -4611,7 +4613,7 @@ void spi_nor_restore(struct spi_nor *nor)
> /* restore the addressing mode */
> if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES) &&
> nor->flags & SNOR_F_BROKEN_RESET)
> - set_4byte(nor, false);
> + nor->params.set_4byte(nor, false);
> }
> EXPORT_SYMBOL_GPL(spi_nor_restore);
>
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index a86c0d9fb01d..7da89dd483cb 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -482,6 +482,7 @@ struct spi_nor;
> * @erase_map: the erase map parsed from the SFDP Sector Map Parameter
> * Table.
> * @quad_enable: enables SPI NOR quad mode.
> + * @set_4byte: puts the SPI NOR in 4 byte addressing mode.
> */
> struct spi_nor_flash_parameter {
> u64 size;
> @@ -494,6 +495,7 @@ struct spi_nor_flash_parameter {
> struct spi_nor_erase_map erase_map;
>
> int (*quad_enable)(struct spi_nor *nor);
> + int (*set_4byte)(struct spi_nor *nor, bool enable);
> };
>
> /**
>

--
Regards
Vignesh