Re: [RESEND PATCH v3 13/20] mtd: spi-nor: Add a ->convert_addr() method

[Vignesh Raghavendra <vigneshr@xxxxxx>]

On 26/08/19 5:38 PM, Tudor.Ambarus@xxxxxxxxxxxxx wrote:
> From: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>
> 
> In order to separate manufacturer quirks from the core we need to get
> rid of all the manufacturer specific flags, like the
> SNOR_F_S3AN_ADDR_DEFAULT one.
> 
> This can easily be replaced by a ->convert_addr() hook, which when
> implemented will provide the core with an easy way to convert an
> absolute address into something the flash understands.
> 
> Right now the only user are the S3AN chips, but other manufacturers
> can implement it if needed.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>
> ---

Reviewed-by: Vignesh Raghavendra <vigneshr@xxxxxx>

Regards
Vignesh

> v3: no changes, rebase on previous commits
> 
>  drivers/mtd/spi-nor/spi-nor.c | 24 ++++++++++++++----------
>  include/linux/mtd/spi-nor.h   | 17 ++++++++++-------
>  2 files changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index c862a59ce9df..b96a7066a36c 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -899,10 +899,9 @@ static void spi_nor_unlock_and_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
>   * Addr can safely be unsigned int, the biggest S3AN device is smaller than
>   * 4 MiB.
>   */
> -static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
> +static u32 s3an_convert_addr(struct spi_nor *nor, u32 addr)
>  {
> -	unsigned int offset;
> -	unsigned int page;
> +	u32 offset, page;
>  
>  	offset = addr % nor->page_size;
>  	page = addr / nor->page_size;
> @@ -911,6 +910,14 @@ static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
>  	return page | offset;
>  }
>  
> +static u32 spi_nor_convert_addr(struct spi_nor *nor, loff_t addr)
> +{
> +	if (!nor->params.convert_addr)
> +		return addr;
> +
> +	return nor->params.convert_addr(nor, addr);
> +}
> +
>  /*
>   * Initiate the erasure of a single sector
>   */
> @@ -918,8 +925,7 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
>  {
>  	int i;
>  
> -	if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
> -		addr = spi_nor_s3an_addr_convert(nor, addr);
> +	addr = spi_nor_convert_addr(nor, addr);
>  
>  	if (nor->erase)
>  		return nor->erase(nor, addr);
> @@ -2535,8 +2541,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>  	while (len) {
>  		loff_t addr = from;
>  
> -		if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
> -			addr = spi_nor_s3an_addr_convert(nor, addr);
> +		addr = spi_nor_convert_addr(nor, addr);
>  
>  		ret = spi_nor_read_data(nor, addr, len, buf);
>  		if (ret == 0) {
> @@ -2680,8 +2685,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>  		page_remain = min_t(size_t,
>  				    nor->page_size - page_offset, len - i);
>  
> -		if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
> -			addr = spi_nor_s3an_addr_convert(nor, addr);
> +		addr = spi_nor_convert_addr(nor, addr);
>  
>  		write_enable(nor);
>  		ret = spi_nor_write_data(nor, addr, page_remain, buf + i);
> @@ -2748,7 +2752,7 @@ static int s3an_nor_scan(struct spi_nor *nor)
>  		nor->mtd.erasesize = 8 * nor->page_size;
>  	} else {
>  		/* Flash in Default addressing mode */
> -		nor->flags |= SNOR_F_S3AN_ADDR_DEFAULT;
> +		nor->params.convert_addr = s3an_convert_addr;
>  	}
>  
>  	return 0;
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index ea3bcac54dc2..35aad92a4ff8 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -237,13 +237,12 @@ enum spi_nor_option_flags {
>  	SNOR_F_USE_FSR		= BIT(0),
>  	SNOR_F_HAS_SR_TB	= BIT(1),
>  	SNOR_F_NO_OP_CHIP_ERASE	= BIT(2),
> -	SNOR_F_S3AN_ADDR_DEFAULT = BIT(3),
> -	SNOR_F_READY_XSR_RDY	= BIT(4),
> -	SNOR_F_USE_CLSR		= BIT(5),
> -	SNOR_F_BROKEN_RESET	= BIT(6),
> -	SNOR_F_4B_OPCODES	= BIT(7),
> -	SNOR_F_HAS_4BAIT	= BIT(8),
> -	SNOR_F_HAS_LOCK		= BIT(9),
> +	SNOR_F_READY_XSR_RDY	= BIT(3),
> +	SNOR_F_USE_CLSR		= BIT(4),
> +	SNOR_F_BROKEN_RESET	= BIT(5),
> +	SNOR_F_4B_OPCODES	= BIT(6),
> +	SNOR_F_HAS_4BAIT	= BIT(7),
> +	SNOR_F_HAS_LOCK		= BIT(8),
>  };
>  
>  /**
> @@ -496,6 +495,9 @@ struct spi_nor_locking_ops {
>   *                      Table.
>   * @quad_enable:	enables SPI NOR quad mode.
>   * @set_4byte:		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.
>   * @locking_ops:	SPI NOR locking methods.
>   */
>  struct spi_nor_flash_parameter {
> @@ -510,6 +512,7 @@ struct spi_nor_flash_parameter {
>  
>  	int (*quad_enable)(struct spi_nor *nor);
>  	int (*set_4byte)(struct spi_nor *nor, bool enable);
> +	u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
>  
>  	const struct spi_nor_locking_ops *locking_ops;
>  };
> 

-- 
Regards
Vignesh

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