Re: [PATCH v2 02/25] mtd: spi-nor: Emphasise which is the generic set_4byte_addr_mode() method

[Vignesh Raghavendra <vigneshr@xxxxxx>]

On 14/03/20 1:12 am, Tudor.Ambarus@xxxxxxxxxxxxx wrote:
> From: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>
> 
> Rename (*set_4byte)() to (*set_4byte_addr_mode)() for a better
> differentiation between the 4 byte address mode and opcodes.
> 
> Rename macronix_set_4byte() to spi_nor_set_4byte_addr_mode(), it will be
> the only 4 byte address mode method exposed to the manufacturer drivers.
> 
> Here's how the manufacturers enter and exit the 4 byte address mode:
> - eon, gidadevice, issi, macronix, xmc use EN4B/EX4B
> - micron-st needs WEN. st_micron_set_4byte_addr_mode() will become
>   a private method, as they are the only ones that need WEN before the
>   EN4B/EX4B commands.
> - newer spansion have a 4BAM opcode (this translates to a new, public
>   command). Older spansion  flashes use the BRWR command (legacy in
>   core.c -> spansion_set_4byte_addr_mode())
> - winbond's method is hackish and may be reason for just a flash
>   fixup hook -> private method
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>

Reviewed-by: Vignesh Raghavendra <vigneshr@xxxxxx>

spansion_set_4byte_addr() is the only function with manuf specific
prefix in core.c.

This could probably renamed to a generic one in a separate change

Regards
Vignesh

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 34 ++++++++++++++++++----------------
>  include/linux/mtd/spi-nor.h   |  4 ++--
>  2 files changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 0b8fac0b0299..8616673ddb7c 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -568,14 +568,14 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
>  }
>  
>  /**
> - * macronix_set_4byte() - Set 4-byte address mode for Macronix flashes.
> + * spi_nor_set_4byte_addr_mode() - Enter/Exit 4-byte address mode.
>   * @nor:	pointer to 'struct spi_nor'.
>   * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
>   *		address mode.
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int macronix_set_4byte(struct spi_nor *nor, bool enable)
> +static int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
>  {
>  	int ret;
>  
> @@ -604,14 +604,15 @@ static int macronix_set_4byte(struct spi_nor *nor, bool enable)
>  }
>  
>  /**
> - * st_micron_set_4byte() - Set 4-byte address mode for ST and Micron flashes.
> + * st_micron_set_4byte_addr_mode() - Set 4-byte address mode for ST and Micron
> + * flashes.
>   * @nor:	pointer to 'struct spi_nor'.
>   * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
>   *		address mode.
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
> +static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
>  {
>  	int ret;
>  
> @@ -619,7 +620,7 @@ static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
>  	if (ret)
>  		return ret;
>  
> -	ret = macronix_set_4byte(nor, enable);
> +	ret = spi_nor_set_4byte_addr_mode(nor, enable);
>  	if (ret)
>  		return ret;
>  
> @@ -627,14 +628,15 @@ static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
>  }
>  
>  /**
> - * spansion_set_4byte() - Set 4-byte address mode for Spansion flashes.
> + * spansion_set_4byte_addr_mode() - Set 4-byte address mode for Spansion
> + * flashes.
>   * @nor:	pointer to 'struct spi_nor'.
>   * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
>   *		address mode.
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int spansion_set_4byte(struct spi_nor *nor, bool enable)
> +static int spansion_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
>  {
>  	int ret;
>  
> @@ -692,18 +694,18 @@ static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
>  }
>  
>  /**
> - * winbond_set_4byte() - Set 4-byte address mode for Winbond flashes.
> + * winbond_set_4byte_addr_mode() - Set 4-byte address mode for Winbond flashes.
>   * @nor:	pointer to 'struct spi_nor'.
>   * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
>   *		address mode.
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int winbond_set_4byte(struct spi_nor *nor, bool enable)
> +static int winbond_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
>  {
>  	int ret;
>  
> -	ret = macronix_set_4byte(nor, enable);
> +	ret = spi_nor_set_4byte_addr_mode(nor, enable);
>  	if (ret || enable)
>  		return ret;
>  
> @@ -4655,7 +4657,7 @@ static void issi_set_default_init(struct spi_nor *nor)
>  static void macronix_set_default_init(struct spi_nor *nor)
>  {
>  	nor->params.quad_enable = spi_nor_sr1_bit6_quad_enable;
> -	nor->params.set_4byte = macronix_set_4byte;
> +	nor->params.set_4byte_addr_mode = spi_nor_set_4byte_addr_mode;
>  }
>  
>  static void sst_set_default_init(struct spi_nor *nor)
> @@ -4668,12 +4670,12 @@ static void st_micron_set_default_init(struct spi_nor *nor)
>  	nor->flags |= SNOR_F_HAS_LOCK;
>  	nor->flags &= ~SNOR_F_HAS_16BIT_SR;
>  	nor->params.quad_enable = NULL;
> -	nor->params.set_4byte = st_micron_set_4byte;
> +	nor->params.set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
>  }
>  
>  static void winbond_set_default_init(struct spi_nor *nor)
>  {
> -	nor->params.set_4byte = winbond_set_4byte;
> +	nor->params.set_4byte_addr_mode = winbond_set_4byte_addr_mode;
>  }
>  
>  /**
> @@ -4759,7 +4761,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
>  
>  	/* Initialize legacy flash parameters and settings. */
>  	params->quad_enable = spi_nor_sr2_bit1_quad_enable;
> -	params->set_4byte = spansion_set_4byte;
> +	params->set_4byte_addr_mode = spansion_set_4byte_addr_mode;
>  	params->setup = spi_nor_default_setup;
>  	/* Default to 16-bit Write Status (01h) Command */
>  	nor->flags |= SNOR_F_HAS_16BIT_SR;
> @@ -5011,7 +5013,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");
> -		nor->params.set_4byte(nor, true);
> +		nor->params.set_4byte_addr_mode(nor, true);
>  	}
>  
>  	return 0;
> @@ -5035,7 +5037,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)
> -		nor->params.set_4byte(nor, false);
> +		nor->params.set_4byte_addr_mode(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 de90724f62f1..2b9717b0cd62 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -520,7 +520,7 @@ struct spi_nor_locking_ops {
>   * @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.
> + * @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
>   * @convert_addr:	converts an absolute address into something the flash
>   *                      will understand. Particularly useful when pagesize is
>   *                      not a power-of-2.
> @@ -541,7 +541,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);
> +	int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
>  	u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
>  	int (*setup)(struct spi_nor *nor, const struct spi_nor_hwcaps *hwcaps);
>  
> 

-- 
Regards
Vignesh

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/