| Message ID | 88969938-961b-973b-0c13-3792c394b1d3@cogentembedded.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [REPOST,v3] mtd: spi-nor: use spi-mem dirmap API | expand |
Hi, Sergei, On 11/22/19 11:02 PM, Sergei Shtylyov wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Make use of the spi-mem direct mapping API to let advanced controllers > optimize read/write operations when they support direct mapping. > > Based on the original patch by Boris Brezillon <boris.brezillon@bootlin.com>. > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > > --- > The patch is against the 'spi-nor/next' branch of the MTD 'linux.git repo... > Forgot to post to 'linux-mtd' 1st time... :-( > > Changes in version 3: > - simplified the way spi_mem_dirmap_{read|write}() are called; > - refreshed the patch; > - added Boris' tag. > > Changes in version 2: > - moved the spi_mem_dirmap_{read|write}() calls from spi_nor_{read|write}() to > spi_nor_spimem_{read|write}_data(). > > drivers/mtd/spi-nor/spi-nor.c | 66 ++++++++++++++++++++++++++++++++++++++++++ > include/linux/mtd/spi-nor.h | 5 +++ > 2 files changed, 71 insertions(+) > > Index: linux/drivers/mtd/spi-nor/spi-nor.c > =================================================================== > --- linux.orig/drivers/mtd/spi-nor/spi-nor.c > +++ linux/drivers/mtd/spi-nor/spi-nor.c > @@ -311,6 +311,9 @@ static ssize_t spi_nor_spimem_read_data( > SPI_MEM_OP_DUMMY(nor->read_dummy, 1), > SPI_MEM_OP_DATA_IN(len, buf, 1)); > > + if (nor->dirmap.rdesc) > + return spi_mem_dirmap_read(nor->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); > @@ -360,6 +363,9 @@ static ssize_t spi_nor_spimem_write_data > SPI_MEM_OP_NO_DUMMY, > SPI_MEM_OP_DATA_OUT(len, buf, 1)); > > + if (nor->dirmap.wdesc) This check is always true. The dirmap API is available just for the spi-mem drivers, and we are already in the nor->spimem case. > + return spi_mem_dirmap_write(nor->dirmap.wdesc, to, len, buf); > + This doesn't guarantee that the buffer is DMA-able. You should instead modify the spi_nor_spimem_xfer_data() method. Cheers, ta
Hello! On 12/23/2019 04:45 PM, Tudor.Ambarus@microchip.com wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> Make use of the spi-mem direct mapping API to let advanced controllers >> optimize read/write operations when they support direct mapping. >> >> Based on the original patch by Boris Brezillon <boris.brezillon@bootlin.com>. >> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> >> >> --- >> The patch is against the 'spi-nor/next' branch of the MTD 'linux.git repo... >> Forgot to post to 'linux-mtd' 1st time... :-( >> >> Changes in version 3: >> - simplified the way spi_mem_dirmap_{read|write}() are called; >> - refreshed the patch; >> - added Boris' tag. >> >> Changes in version 2: >> - moved the spi_mem_dirmap_{read|write}() calls from spi_nor_{read|write}() to >> spi_nor_spimem_{read|write}_data(). >> >> drivers/mtd/spi-nor/spi-nor.c | 66 ++++++++++++++++++++++++++++++++++++++++++ >> include/linux/mtd/spi-nor.h | 5 +++ >> 2 files changed, 71 insertions(+) >> >> Index: linux/drivers/mtd/spi-nor/spi-nor.c >> =================================================================== >> --- linux.orig/drivers/mtd/spi-nor/spi-nor.c >> +++ linux/drivers/mtd/spi-nor/spi-nor.c >> @@ -311,6 +311,9 @@ static ssize_t spi_nor_spimem_read_data( >> SPI_MEM_OP_DUMMY(nor->read_dummy, 1), >> SPI_MEM_OP_DATA_IN(len, buf, 1)); >> >> + if (nor->dirmap.rdesc) >> + return spi_mem_dirmap_read(nor->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); >> @@ -360,6 +363,9 @@ static ssize_t spi_nor_spimem_write_data >> SPI_MEM_OP_NO_DUMMY, >> SPI_MEM_OP_DATA_OUT(len, buf, 1)); >> >> + if (nor->dirmap.wdesc) > > This check is always true. Not really, the rdesc/wdesc pointers are NULL during probing: spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000 spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000 spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000 spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000 spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000 spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000 spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000 spi-nor spi0.0: Failed to parse optional parameter table: ff81 spi_nor_spimem_read_data(): nor->dirmap.rdesc = 0000000000000000 spi-nor spi0.0: s25fs512s (65536 Kbytes) 11 fixed-partitions partitions found on MTD device spi0.0 Creating 11 MTD partitions on "spi0.0": 0x000000000000-0x000000040000 : "bootparam" 0x000000040000-0x0000000c0000 : "cr7" 0x0000000c0000-0x000000140000 : "cert_header_sa3" 0x000000140000-0x000000180000 : "bl2" 0x000000180000-0x0000001c0000 : "cert_header_sa6" 0x0000001c0000-0x000000620000 : "bl31" 0x000000640000-0x000000700000 : "uboot" 0x000000700000-0x000000740000 : "uboot-env" 0x000000740000-0x0000007c0000 : "dtb" 0x0000007c0000-0x000001bc0000 : "kernel" 0x000001bc0000-0x000004000000 : "user" Perhaps the order of calls during probe should be changed? > The dirmap API is available just for the spi-mem > drivers, and we are already in the nor->spimem case. > >> + return spi_mem_dirmap_write(nor->dirmap.wdesc, to, len, buf); >> + > This doesn't guarantee that the buffer is DMA-able. You should instead modify > the spi_nor_spimem_xfer_data() method. Still looking into this... > Cheers, > ta MBR, Sergei
Index: linux/drivers/mtd/spi-nor/spi-nor.c =================================================================== --- linux.orig/drivers/mtd/spi-nor/spi-nor.c +++ linux/drivers/mtd/spi-nor/spi-nor.c @@ -311,6 +311,9 @@ static ssize_t spi_nor_spimem_read_data( SPI_MEM_OP_DUMMY(nor->read_dummy, 1), SPI_MEM_OP_DATA_IN(len, buf, 1)); + if (nor->dirmap.rdesc) + return spi_mem_dirmap_read(nor->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); @@ -360,6 +363,9 @@ static ssize_t spi_nor_spimem_write_data SPI_MEM_OP_NO_DUMMY, SPI_MEM_OP_DATA_OUT(len, buf, 1)); + if (nor->dirmap.wdesc) + return spi_mem_dirmap_write(nor->dirmap.wdesc, to, len, buf); + 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.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto); @@ -5204,6 +5210,58 @@ int spi_nor_scan(struct spi_nor *nor, co } EXPORT_SYMBOL_GPL(spi_nor_scan); +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, 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 = 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; + + nor->dirmap.rdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem, + &info); + return PTR_ERR_OR_ZERO(nor->dirmap.rdesc); +} + +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, 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 = 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; + + nor->dirmap.wdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem, + &info); + return PTR_ERR_OR_ZERO(nor->dirmap.wdesc); +} + static int spi_nor_probe(struct spi_mem *spimem) { struct spi_device *spi = spimem->spi; @@ -5265,6 +5323,14 @@ static int spi_nor_probe(struct spi_mem return -ENOMEM; } + ret = spi_nor_create_read_dirmap(nor); + if (ret) + return ret; + + ret = spi_nor_create_write_dirmap(nor); + if (ret) + return ret; + return mtd_device_register(&nor->mtd, data ? data->parts : NULL, data ? data->nr_parts : 0); } Index: linux/include/linux/mtd/spi-nor.h =================================================================== --- linux.orig/include/linux/mtd/spi-nor.h +++ linux/include/linux/mtd/spi-nor.h @@ -610,6 +610,11 @@ struct spi_nor { struct spi_nor_flash_parameter params; + struct { + struct spi_mem_dirmap_desc *rdesc; + struct spi_mem_dirmap_desc *wdesc; + } dirmap; + void *priv; };