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

Vignesh Raghavendra
2019-08-27T07:13:15Z

On 26/08/19 5:38 PM, Tudor.Ambarus@microchip.com wrote:
> From: Boris Brezillon <boris.brezillon@bootlin.com>
>
> 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@bootlin.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---

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

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