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

2022-04-20 Thread Chin-Ting Kuo
Hi Pratyush,

Thanks for your review.

> -Original Message-
> From: Pratyush Yadav 
> Sent: Wednesday, April 20, 2022 4:21 PM
> To: Chin-Ting Kuo 
> 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-sean...@gmail.com/
> >
> > Signed-off-by: Chin-Ting Kuo 
> > Signed-off-by: Sean Anderson 
> > ---
> >  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 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #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 = _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, );
> > +   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 = _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, );
> > +   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)
> &

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

2022-04-20 Thread Pratyush Yadav
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-sean...@gmail.com/
> 
> Signed-off-by: Chin-Ting Kuo 
> Signed-off-by: Sean Anderson 
> ---
>  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 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #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 = _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, );
> + 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 = _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, );
> + 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.

>   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