Re: [PATCH v4] mtd: devices: m25p80: Use the spi-mem dirmap API

[Vignesh Raghavendra <vigneshr@xxxxxx>]

Hi,

On 01-Jun-19 12:01 AM, Sergei Shtylyov wrote:
[...]
> 
>  drivers/mtd/devices/m25p80.c |  102 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 99 insertions(+), 3 deletions(-)
> 
> Index: linux/drivers/mtd/devices/m25p80.c
> ===================================================================
> --- linux.orig/drivers/mtd/devices/m25p80.c
> +++ linux/drivers/mtd/devices/m25p80.c
> @@ -31,8 +31,70 @@
>  struct m25p {
>  	struct spi_mem		*spimem;
>  	struct spi_nor		spi_nor;
> +	struct {
> +		struct spi_mem_dirmap_desc *rdesc;
> +		struct spi_mem_dirmap_desc *wdesc;
> +	} dirmap;
>  };
>  
> +static int m25p_create_write_dirmap(struct m25p *flash)
> +{
> +	struct spi_nor *nor = &flash->spi_nor;
> +	struct spi_mem_dirmap_info info = {
> +		.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
> +				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 1),
> +				      SPI_MEM_OP_NO_DUMMY,
> +				      SPI_MEM_OP_DATA_OUT(0, NULL, 1)),
> +		.offset = 0,
> +		.length = flash->spi_nor.mtd.size,
> +	};
> +	struct spi_mem_op *op = &info.op_tmpl;
> +
> +	/* get transfer protocols. */
> +	op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
> +	op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
> +	op->dummy.buswidth = op->addr.buswidth;
> +	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;
> +
> +	flash->dirmap.wdesc = spi_mem_dirmap_create(flash->spimem, &info);

Could you change this and elsewhere to devm_spi_mem_dirmap_create() and
thus simplify error handling?

Regards
Vignesh


> +	if (IS_ERR(flash->dirmap.wdesc))
> +		return PTR_ERR(flash->dirmap.wdesc);
> +
> +	return 0;
> +}
> +
> +static int m25p_create_read_dirmap(struct m25p *flash)
> +{
> +	struct spi_nor *nor = &flash->spi_nor;
> +	struct spi_mem_dirmap_info info = {
> +		.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
> +				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 1),
> +				      SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
> +				      SPI_MEM_OP_DATA_IN(0, NULL, 1)),
> +		.offset = 0,
> +		.length = flash->spi_nor.mtd.size,
> +	};
> +	struct spi_mem_op *op = &info.op_tmpl;
> +
> +	/* get transfer protocols. */
> +	op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
> +	op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
> +	op->dummy.buswidth = op->addr.buswidth;
> +	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;
> +
> +	flash->dirmap.rdesc = spi_mem_dirmap_create(flash->spimem, &info);
> +	if (IS_ERR(flash->dirmap.rdesc))
> +		return PTR_ERR(flash->dirmap.rdesc);
> +
> +	return 0;
> +}
> +
>  static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
>  {
>  	struct m25p *flash = nor->priv;
> @@ -92,6 +154,9 @@ static ssize_t m25p80_write(struct spi_n
>  				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
>  	int ret;
>  
> +	if (flash->dirmap.wdesc)
> +		return spi_mem_dirmap_write(flash->dirmap.wdesc, to, len, buf);
> +
>  	/* get transfer protocols. */
>  	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
>  	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
> @@ -128,6 +193,9 @@ static ssize_t m25p80_read(struct spi_no
>  	size_t remaining = len;
>  	int ret;
>  
> +	if (flash->dirmap.rdesc)
> +		return spi_mem_dirmap_read(flash->dirmap.rdesc, from, len, buf);
> +
>  	/* get transfer protocols. */
>  	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
>  	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
> @@ -238,19 +306,47 @@ static int m25p_probe(struct spi_mem *sp
>  	if (ret)
>  		return ret;
>  
> -	return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
> -				   data ? data->nr_parts : 0);
> +	ret = m25p_create_write_dirmap(flash);
> +	if (ret)
> +		return ret;
> +
> +	ret = m25p_create_read_dirmap(flash);
> +	if (ret)
> +		goto err_destroy_write_dirmap;
> +
> +	ret = mtd_device_register(&nor->mtd, data ? data->parts : NULL,
> +				  data ? data->nr_parts : 0);
> +	if (ret)
> +		goto err_destroy_read_dirmap;
> +
> +	return 0;
> +
> +err_destroy_read_dirmap:
> +	spi_mem_dirmap_destroy(flash->dirmap.rdesc);
> +
> +err_destroy_write_dirmap:
> +	spi_mem_dirmap_destroy(flash->dirmap.wdesc);
> +
> +	return ret;
>  }
>  
>  
>  static int m25p_remove(struct spi_mem *spimem)
>  {
>  	struct m25p	*flash = spi_mem_get_drvdata(spimem);
> +	int ret;
>  
>  	spi_nor_restore(&flash->spi_nor);
>  
>  	/* Clean up MTD stuff. */
> -	return mtd_device_unregister(&flash->spi_nor.mtd);
> +	ret = mtd_device_unregister(&flash->spi_nor.mtd);
> +	if (ret)
> +		return ret;
> +
> +	spi_mem_dirmap_destroy(flash->dirmap.rdesc);
> +	spi_mem_dirmap_destroy(flash->dirmap.wdesc);
> +
> +	return 0;
>  }
>  
>  static void m25p_shutdown(struct spi_mem *spimem)
> 

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