[PATCH 7/8] mtd: spi-nor: use spi-mem dirmap API

Chin-Ting Kuo chin-ting_kuo at aspeedtech.com
Wed Apr 20 11:38:56 CEST 2022


Hi Pratyush,

Thanks for your review.

> -----Original Message-----
> From: Pratyush Yadav <p.yadav at ti.com>
> Sent: Wednesday, April 20, 2022 4:21 PM
> To: Chin-Ting Kuo <chin-ting_kuo at aspeedtech.com>
> Subject: Re: [PATCH 7/8] mtd: spi-nor: use spi-mem dirmap API
> 
> On 14/04/22 07:23PM, Chin-Ting Kuo wrote:
> > This adds support for the dirmap API to the spi-nor subsystem, as
> > introduced in Linux commit df5c210 ("mtd: spi-nor: use spi-mem dirmap
> > API").
> >
> > This patch is synchronize from the following patch
> > https://patchwork.ozlabs.org/project/uboot/patch/20210205043924.149504
> > -4-seanga2 at gmail.com/
> >
> > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo at aspeedtech.com>
> > Signed-off-by: Sean Anderson <seanga2 at gmail.com>
> > ---
> >  drivers/mtd/spi/sf_probe.c     | 82
> ++++++++++++++++++++++++++++++++++
> >  drivers/mtd/spi/spi-nor-core.c | 55 ++++++++++++++++-------
> >  include/linux/mtd/spi-nor.h    | 18 ++++++++
> >  3 files changed, 139 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> > index f461082e03..a3b38b6a29 100644
> > --- a/drivers/mtd/spi/sf_probe.c
> > +++ b/drivers/mtd/spi/sf_probe.c
> > @@ -10,13 +10,81 @@
> >  #include <common.h>
> >  #include <dm.h>
> >  #include <errno.h>
> > +#include <linux/mtd/spi-nor.h>
> >  #include <log.h>
> >  #include <malloc.h>
> >  #include <spi.h>
> >  #include <spi_flash.h>
> > +#include <spi-mem.h>
> >
> >  #include "sf_internal.h"
> >
> > +#if CONFIG_IS_ENABLED(SPI_DIRMAP)
> > +static int spi_nor_create_read_dirmap(struct spi_nor *nor) {
> > +	struct spi_mem_dirmap_info info = {
> > +		.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 0),
> > +				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 0),
> > +				      SPI_MEM_OP_DUMMY(nor->read_dummy, 0),
> > +				      SPI_MEM_OP_DATA_IN(0, NULL, 0)),
> > +		.offset = 0,
> > +		.length = nor->mtd.size,
> > +	};
> > +	struct spi_mem_op *op = &info.op_tmpl;
> > +
> > +	/* get transfer protocols. */
> > +	spi_nor_setup_op(nor, op, nor->read_proto);
> > +	op->data.buswidth =
> > +spi_nor_get_protocol_data_nbits(nor->read_proto);
> > +
> > +	/* convert the dummy cycles to the number of bytes */
> > +	op->dummy.nbytes = (nor->read_dummy * op->dummy.buswidth) / 8;
> > +	if (spi_nor_protocol_is_dtr(nor->read_proto))
> > +		op->dummy.nbytes *= 2;
> > +
> > +	nor->dirmap.rdesc = spi_mem_dirmap_create(nor->spi, &info);
> > +	if (IS_ERR(nor->dirmap.rdesc))
> > +		return PTR_ERR(nor->dirmap.rdesc);
> > +
> > +	return 0;
> > +}
> > +
> > +static int spi_nor_create_write_dirmap(struct spi_nor *nor) {
> > +	struct spi_mem_dirmap_info info = {
> > +		.op_tmpl =
> SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 0),
> > +				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 0),
> > +				      SPI_MEM_OP_NO_DUMMY,
> > +				      SPI_MEM_OP_DATA_OUT(0, NULL, 0)),
> > +		.offset = 0,
> > +		.length = nor->mtd.size,
> > +	};
> > +	struct spi_mem_op *op = &info.op_tmpl;
> > +
> > +	/* get transfer protocols. */
> > +	spi_nor_setup_op(nor, op, nor->write_proto);
> > +	op->data.buswidth =
> > +spi_nor_get_protocol_data_nbits(nor->write_proto);
> > +
> > +	if (nor->program_opcode == SPINOR_OP_AAI_WP &&
> nor->sst_write_second)
> > +		op->addr.nbytes = 0;
> > +
> > +	nor->dirmap.wdesc = spi_mem_dirmap_create(nor->spi, &info);
> > +	if (IS_ERR(nor->dirmap.wdesc))
> > +		return PTR_ERR(nor->dirmap.wdesc);
> > +
> > +	return 0;
> > +}
> > +#else
> > +static int spi_nor_create_read_dirmap(struct spi_nor *nor) {
> > +	return 0;
> > +}
> > +
> > +static int spi_nor_create_write_dirmap(struct spi_nor *nor) {
> > +	return 0;
> > +}
> > +#endif /* CONFIG_SPI_DIRMAP */
> > +
> 
> Instead of wrapping these in #ifdefs...
> 
> >  /**
> >   * spi_flash_probe_slave() - Probe for a SPI flash device on a bus
> >   *
> > @@ -45,6 +113,14 @@ static int spi_flash_probe_slave(struct spi_flash
> *flash)
> >  	if (ret)
> >  		goto err_read_id;
> >
> > +	ret = spi_nor_create_read_dirmap(flash);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = spi_nor_create_write_dirmap(flash);
> > +	if (ret)
> > +		return ret;
> > +
> 
> ... wrap these in a
> 
> 	if (CONFIG_IS_ENABLED(SPI_DIRMAP)) {
> 		// Create read and write dirmap
> 	}
> 
> Then at compile time if the config is not enabled, this is a dead branch and the
> compiler should not look at spi_nor_create_{read,write}_dirmap()
> at all.

Okay, this will be updated in the next version.


Best Wishes,
Chin-Ting

> 
> >  	if (CONFIG_IS_ENABLED(SPI_FLASH_MTD))
> >  		ret = spi_flash_mtd_register(flash);
> >
> > @@ -83,6 +159,9 @@ struct spi_flash *spi_flash_probe(unsigned int
> > busnum, unsigned int cs,
> >
> >  void spi_flash_free(struct spi_flash *flash)  {
> > +	spi_mem_dirmap_destroy(flash->dirmap.wdesc);
> > +	spi_mem_dirmap_destroy(flash->dirmap.rdesc);
> > +
> >  	if (CONFIG_IS_ENABLED(SPI_FLASH_MTD))
> >  		spi_flash_mtd_unregister(flash);
> >
> > @@ -153,6 +232,9 @@ static int spi_flash_std_remove(struct udevice *dev)
> >  	struct spi_flash *flash = dev_get_uclass_priv(dev);
> >  	int ret;
> >
> > +	spi_mem_dirmap_destroy(flash->dirmap.wdesc);
> > +	spi_mem_dirmap_destroy(flash->dirmap.rdesc);
> > +
> >  	ret = spi_nor_remove(flash);
> >  	if (ret)
> >  		return ret;
> > diff --git a/drivers/mtd/spi/spi-nor-core.c
> > b/drivers/mtd/spi/spi-nor-core.c index 3b7c817c02..0c6262b7fd 100644
> > --- a/drivers/mtd/spi/spi-nor-core.c
> > +++ b/drivers/mtd/spi/spi-nor-core.c
> > @@ -239,9 +239,9 @@ static u8 spi_nor_get_cmd_ext(const struct spi_nor
> *nor,
> >   *			need to be initialized.
> >   * @proto:		the protocol from which the properties need to be set.
> >   */
> > -static void spi_nor_setup_op(const struct spi_nor *nor,
> > -			     struct spi_mem_op *op,
> > -			     const enum spi_nor_protocol proto)
> > +void spi_nor_setup_op(const struct spi_nor *nor,
> > +		      struct spi_mem_op *op,
> > +		      const enum spi_nor_protocol proto)
> >  {
> >  	u8 ext;
> >
> > @@ -362,13 +362,29 @@ static ssize_t spi_nor_read_data(struct spi_nor
> > *nor, loff_t from, size_t len,
> >
> >  	while (remaining) {
> >  		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
> > -		ret = spi_mem_adjust_op_size(nor->spi, &op);
> > -		if (ret)
> > -			return ret;
> >
> > -		ret = spi_mem_exec_op(nor->spi, &op);
> > -		if (ret)
> > -			return ret;
> > +		if (CONFIG_IS_ENABLED(SPI_DIRMAP) && nor->dirmap.rdesc) {
> > +			/*
> > +			 * Record current operation information which may be used
> > +			 * when the address or data length exceeds address mapping.
> > +			 */
> > +			memcpy(&nor->dirmap.rdesc->info.op_tmpl, &op,
> > +			       sizeof(struct spi_mem_op));
> > +			ret = spi_mem_dirmap_read(nor->dirmap.rdesc,
> > +						  op.addr.val, op.data.nbytes,
> > +						  op.data.buf.in);
> > +			if (ret < 0)
> > +				return ret;
> > +			op.data.nbytes = ret;
> > +		} else {
> > +			ret = spi_mem_adjust_op_size(nor->spi, &op);
> > +			if (ret)
> > +				return ret;
> > +
> > +			ret = spi_mem_exec_op(nor->spi, &op);
> > +			if (ret)
> > +				return ret;
> > +		}
> >
> >  		op.addr.val += op.data.nbytes;
> >  		remaining -= op.data.nbytes;
> > @@ -393,14 +409,21 @@ static ssize_t spi_nor_write_data(struct spi_nor
> > *nor, loff_t to, size_t len,
> >
> >  	spi_nor_setup_op(nor, &op, nor->write_proto);
> >
> > -	ret = spi_mem_adjust_op_size(nor->spi, &op);
> > -	if (ret)
> > -		return ret;
> > -	op.data.nbytes = len < op.data.nbytes ? len : op.data.nbytes;
> > +	if (CONFIG_IS_ENABLED(SPI_DIRMAP) && nor->dirmap.wdesc) {
> > +		memcpy(&nor->dirmap.wdesc->info.op_tmpl, &op,
> > +		       sizeof(struct spi_mem_op));
> > +		op.data.nbytes = spi_mem_dirmap_write(nor->dirmap.wdesc,
> op.addr.val,
> > +						      op.data.nbytes, op.data.buf.out);
> > +	} else {
> > +		ret = spi_mem_adjust_op_size(nor->spi, &op);
> > +		if (ret)
> > +			return ret;
> > +		op.data.nbytes = len < op.data.nbytes ? len : op.data.nbytes;
> >
> > -	ret = spi_mem_exec_op(nor->spi, &op);
> > -	if (ret)
> > -		return ret;
> > +		ret = spi_mem_exec_op(nor->spi, &op);
> > +		if (ret)
> > +			return ret;
> > +	}
> >
> >  	return op.data.nbytes;
> >  }
> > diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> > index 4ceeae623d..2a5ad09625 100644
> > --- a/include/linux/mtd/spi-nor.h
> > +++ b/include/linux/mtd/spi-nor.h
> > @@ -11,6 +11,7 @@
> >  #include <linux/bitops.h>
> >  #include <linux/mtd/cfi.h>
> >  #include <linux/mtd/mtd.h>
> > +#include <spi-mem.h>
> >
> >  /*
> >   * Manufacturer IDs
> > @@ -511,6 +512,7 @@ struct spi_flash {
> >   * @quad_enable:	[FLASH-SPECIFIC] enables SPI NOR quad mode
> >   * @octal_dtr_enable:	[FLASH-SPECIFIC] enables SPI NOR octal DTR
> mode.
> >   * @ready:		[FLASH-SPECIFIC] check if the flash is ready
> > + * @dirmap:		pointers to struct spi_mem_dirmap_desc for
> reads/writes.
> >   * @priv:		the private data
> >   */
> >  struct spi_nor {
> > @@ -561,6 +563,11 @@ struct spi_nor {
> >  	int (*octal_dtr_enable)(struct spi_nor *nor);
> >  	int (*ready)(struct spi_nor *nor);
> >
> > +	struct {
> > +		struct spi_mem_dirmap_desc *rdesc;
> > +		struct spi_mem_dirmap_desc *wdesc;
> > +	} dirmap;
> > +
> >  	void *priv;
> >  	char mtd_name[MTD_NAME_SIZE(MTD_DEV_TYPE_NOR)];
> >  /* Compatibility for spi_flash, remove once sf layer is merged with
> > mtd */ @@ -584,6 +591,17 @@ device_node
> *spi_nor_get_flash_node(struct
> > spi_nor *nor)  }  #endif /* __UBOOT__ */
> >
> > +/**
> > + * spi_nor_setup_op() - Set up common properties of a spi-mem op.
> > + * @nor:		pointer to a 'struct spi_nor'
> > + * @op:			pointer to the 'struct spi_mem_op' whose properties
> > + *			need to be initialized.
> > + * @proto:		the protocol from which the properties need to be set.
> > + */
> > +void spi_nor_setup_op(const struct spi_nor *nor,
> > +		      struct spi_mem_op *op,
> > +		      const enum spi_nor_protocol proto);
> > +
> >  /**
> >   * spi_nor_scan() - scan the SPI NOR
> >   * @nor:	the spi_nor structure
> > --
> > 2.25.1
> >
> 
> --
> Regards,
> Pratyush Yadav
> Texas Instruments Inc.


More information about the U-Boot mailing list