| Message ID | ef173d36-c918-4636-ace6-f6ba82dac672@cogentembedded.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [v2] mtd: spi-nor: use spi-mem dirmap API | expand |
On 11/07/2019 11:49 PM, Sergei Shtylyov wrote: > 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> > > --- Oope, forgot to mention the base. Was generated again mtd/fixes by mistake, should still apply to spi-nor/next with only small offsets. > 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 | 125 +++++++++++++++++++++++++++++++++--------- > include/linux/mtd/spi-nor.h | 5 + > 2 files changed, 104 insertions(+), 26 deletions(-) [...] MBR, Sergei
On 11/07/2019 11:49 PM, Sergei Shtylyov wrote: > 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> > > --- Oops, forgot to mention the base. Was generated again mtd/fixes by mistake, should still apply to spi-nor/next with only small offsets. > 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 | 125 +++++++++++++++++++++++++++++++++--------- > include/linux/mtd/spi-nor.h | 5 + > 2 files changed, 104 insertions(+), 26 deletions(-) [...] MBR, Sergei
On Thu, 7 Nov 2019 23:49:12 +0300 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > 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> > > --- > 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 | 125 +++++++++++++++++++++++++++++++++--------- > include/linux/mtd/spi-nor.h | 5 + > 2 files changed, 104 insertions(+), 26 deletions(-) > > 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 > @@ -305,22 +305,28 @@ static ssize_t spi_nor_spimem_xfer_data( > static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from, > size_t len, u8 *buf) > { > - struct spi_mem_op op = > - SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1), > - SPI_MEM_OP_ADDR(nor->addr_width, from, 1), > - SPI_MEM_OP_DUMMY(nor->read_dummy, 1), > - SPI_MEM_OP_DATA_IN(len, buf, 1)); > - > - /* 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); > + if (!nor->dirmap.rdesc) { > + struct spi_mem_op op = > + SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1), > + SPI_MEM_OP_ADDR(nor->addr_width, from, 1), > + SPI_MEM_OP_DUMMY(nor->read_dummy, 1), > + SPI_MEM_OP_DATA_IN(len, buf, 1)); > + > + /* 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; > + /* convert the dummy cycles to the number of bytes */ > + op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8; > > - return spi_nor_spimem_xfer_data(nor, &op); > + return spi_nor_spimem_xfer_data(nor, &op); > + } > + return spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf); Can we put the spi_mem_dirmap_read() in the if() branch instead of having an extra level of indentation for the most complex block. if (nor->dirmap.rdesc) return spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf); ... (same comment applies to the write path BTW). With this addressed, you can add Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> Thanks, Boris
Hello! On 11/09/2019 10:35 PM, Boris Brezillon wrote: >> 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> >> >> --- >> 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 | 125 +++++++++++++++++++++++++++++++++--------- >> include/linux/mtd/spi-nor.h | 5 + >> 2 files changed, 104 insertions(+), 26 deletions(-) >> >> 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 >> @@ -305,22 +305,28 @@ static ssize_t spi_nor_spimem_xfer_data( >> static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from, >> size_t len, u8 *buf) >> { >> - struct spi_mem_op op = >> - SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1), >> - SPI_MEM_OP_ADDR(nor->addr_width, from, 1), >> - SPI_MEM_OP_DUMMY(nor->read_dummy, 1), >> - SPI_MEM_OP_DATA_IN(len, buf, 1)); >> - >> - /* 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); >> + if (!nor->dirmap.rdesc) { >> + struct spi_mem_op op = >> + SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1), >> + SPI_MEM_OP_ADDR(nor->addr_width, from, 1), >> + SPI_MEM_OP_DUMMY(nor->read_dummy, 1), >> + SPI_MEM_OP_DATA_IN(len, buf, 1)); >> + >> + /* 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; >> + /* convert the dummy cycles to the number of bytes */ >> + op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8; >> >> - return spi_nor_spimem_xfer_data(nor, &op); >> + return spi_nor_spimem_xfer_data(nor, &op); >> + } >> + return spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf); > > Can we put the spi_mem_dirmap_read() in the if() branch instead of > having an extra level of indentation for the most complex block. Have you noticed a complex variable initializer in that block? Do you want it to be done on the simple branch as well? > if (nor->dirmap.rdesc) > return spi_mem_dirmap_read(nor->dirmap.rdesc, from, > len, buf); > > ... > > (same comment applies to the write path BTW). > > With this addressed, you can add > > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> Thank you. :-) > > Thanks, > > Boris 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 @@ -305,22 +305,28 @@ static ssize_t spi_nor_spimem_xfer_data( static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from, size_t len, u8 *buf) { - struct spi_mem_op op = - SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1), - SPI_MEM_OP_ADDR(nor->addr_width, from, 1), - SPI_MEM_OP_DUMMY(nor->read_dummy, 1), - SPI_MEM_OP_DATA_IN(len, buf, 1)); - - /* 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); + if (!nor->dirmap.rdesc) { + struct spi_mem_op op = + SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1), + SPI_MEM_OP_ADDR(nor->addr_width, from, 1), + SPI_MEM_OP_DUMMY(nor->read_dummy, 1), + SPI_MEM_OP_DATA_IN(len, buf, 1)); + + /* 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; + /* convert the dummy cycles to the number of bytes */ + op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8; - return spi_nor_spimem_xfer_data(nor, &op); + return spi_nor_spimem_xfer_data(nor, &op); + } + return spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf); } /** @@ -354,20 +360,27 @@ static ssize_t spi_nor_read_data(struct static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to, size_t len, const u8 *buf) { - struct spi_mem_op op = - SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1), - SPI_MEM_OP_ADDR(nor->addr_width, to, 1), - SPI_MEM_OP_NO_DUMMY, - SPI_MEM_OP_DATA_OUT(len, buf, 1)); - - 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); + if (!nor->dirmap.wdesc) { + struct spi_mem_op op = + SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1), + SPI_MEM_OP_ADDR(nor->addr_width, to, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(len, buf, 1)); - if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second) - op.addr.nbytes = 0; + 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); + + if (nor->program_opcode == SPINOR_OP_AAI_WP && + nor->sst_write_second) + op.addr.nbytes = 0; - return spi_nor_spimem_xfer_data(nor, &op); + return spi_nor_spimem_xfer_data(nor, &op); + } + return spi_mem_dirmap_write(nor->dirmap.wdesc, to, len, buf); } /** @@ -4979,6 +4992,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; @@ -5040,6 +5105,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 @@ -602,6 +602,11 @@ struct spi_nor { int (*clear_sr_bp)(struct spi_nor *nor); struct spi_nor_flash_parameter params; + struct { + struct spi_mem_dirmap_desc *rdesc; + struct spi_mem_dirmap_desc *wdesc; + } dirmap; + void *priv; };
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> --- 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 | 125 +++++++++++++++++++++++++++++++++--------- include/linux/mtd/spi-nor.h | 5 + 2 files changed, 104 insertions(+), 26 deletions(-)