The AT25 protocol fits pretty well in the spi-mem model. Convert the
at25 spi driver to a spi-mem driver and use the dirmap API instead of
forging SPI messages manually.
This makes the driver compatible with spi-mem-only controllers
(controllers implementing only the spi_mem ops).
Cc: Geert Uytterhoeven <[email protected]>
Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/misc/eeprom/at25.c | 282 +++++++++++++++++++++++--------------
1 file changed, 176 insertions(+), 106 deletions(-)
diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index 99de6939cd5a..818853babbd0 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -17,7 +17,7 @@
#include <linux/sched.h>
#include <linux/nvmem-provider.h>
-#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
#include <linux/spi/eeprom.h>
#include <linux/property.h>
@@ -29,12 +29,17 @@
*/
struct at25_data {
- struct spi_device *spi;
+ struct spi_mem *spimem;
struct mutex lock;
struct spi_eeprom chip;
unsigned addrlen;
struct nvmem_config nvmem_config;
struct nvmem_device *nvmem;
+ void *scratchbuf;
+ struct {
+ struct spi_mem_dirmap_desc *rdesc[2];
+ struct spi_mem_dirmap_desc *wdesc[2];
+ } dirmap;
};
#define AT25_WREN 0x06 /* latch the write enable */
@@ -63,17 +68,89 @@ struct at25_data {
#define io_limit PAGE_SIZE /* bytes */
+static int at25_create_dirmaps(struct at25_data *at25)
+{
+ struct spi_mem_dirmap_info info = {
+ .op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_READ, 1),
+ SPI_MEM_OP_ADDR(at25->addrlen, 0, 1),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_IN(0, NULL, 1)),
+ .offset = 0,
+ .length = at25->chip.byte_len,
+ };
+ struct device *dev = &at25->spimem->spi->dev;
+
+ if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
+ info.length = 256;
+
+ at25->dirmap.rdesc[0] = devm_spi_mem_dirmap_create(dev, at25->spimem,
+ &info);
+ if (IS_ERR(at25->dirmap.rdesc[0]))
+ return PTR_ERR(at25->dirmap.rdesc[0]);
+
+ info.op_tmpl.cmd.opcode = AT25_WRITE;
+ info.op_tmpl.data.dir = SPI_MEM_DATA_OUT;
+
+ at25->dirmap.wdesc[0] = devm_spi_mem_dirmap_create(dev, at25->spimem,
+ &info);
+ if (IS_ERR(at25->dirmap.wdesc[0]))
+ return PTR_ERR(at25->dirmap.wdesc[0]);
+
+ if (!(at25->chip.flags & EE_INSTR_BIT3_IS_ADDR))
+ return 0;
+
+ info.length = at25->chip.byte_len - 256;
+ info.op_tmpl.cmd.opcode = AT25_READ | AT25_INSTR_BIT3;
+ info.op_tmpl.data.dir = SPI_MEM_DATA_IN;
+
+ at25->dirmap.rdesc[1] = devm_spi_mem_dirmap_create(dev, at25->spimem,
+ &info);
+ if (IS_ERR(at25->dirmap.rdesc[1]))
+ return PTR_ERR(at25->dirmap.rdesc[1]);
+
+ info.op_tmpl.cmd.opcode = AT25_WRITE | AT25_INSTR_BIT3;
+ info.op_tmpl.data.dir = SPI_MEM_DATA_OUT;
+
+ at25->dirmap.wdesc[1] = devm_spi_mem_dirmap_create(dev, at25->spimem,
+ &info);
+ if (IS_ERR(at25->dirmap.wdesc[1]))
+ return PTR_ERR(at25->dirmap.wdesc[1]);
+
+ return 0;
+}
+
+static int at25_rdsr(struct at25_data *at25)
+{
+ struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_RDSR, 1),
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_IN(1, at25->scratchbuf, 1));
+ int ret;
+
+ ret = spi_mem_exec_op(at25->spimem, &op);
+ if (ret)
+ return ret;
+
+ return *((u8 *)at25->scratchbuf);
+}
+
+static int at25_wren(struct at25_data *at25)
+{
+ struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_WREN, 1),
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_NO_DATA);
+
+ return spi_mem_exec_op(at25->spimem, &op);
+}
+
static int at25_ee_read(void *priv, unsigned int offset,
void *val, size_t count)
{
+ struct spi_mem_dirmap_desc *desc;
struct at25_data *at25 = priv;
- char *buf = val;
- u8 command[EE_MAXADDRLEN + 1];
- u8 *cp;
+ unsigned int dirmap_offset;
ssize_t status;
- struct spi_transfer t[2];
- struct spi_message m;
- u8 instr;
if (unlikely(offset >= at25->chip.byte_len))
return -EINVAL;
@@ -82,38 +159,14 @@ static int at25_ee_read(void *priv, unsigned int offset,
if (unlikely(!count))
return -EINVAL;
- cp = command;
-
- instr = AT25_READ;
- if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
- if (offset >= (1U << (at25->addrlen * 8)))
- instr |= AT25_INSTR_BIT3;
- *cp++ = instr;
-
- /* 8/16/24-bit address is written MSB first */
- switch (at25->addrlen) {
- default: /* case 3 */
- *cp++ = offset >> 16;
- /* fall through */
- case 2:
- *cp++ = offset >> 8;
- /* fall through */
- case 1:
- case 0: /* can't happen: for better codegen */
- *cp++ = offset >> 0;
+ if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && offset > 255) {
+ desc = at25->dirmap.rdesc[1];
+ dirmap_offset = offset - 256;
+ } else {
+ desc = at25->dirmap.rdesc[0];
+ dirmap_offset = offset;
}
- spi_message_init(&m);
- memset(t, 0, sizeof(t));
-
- t[0].tx_buf = command;
- t[0].len = at25->addrlen + 1;
- spi_message_add_tail(&t[0], &m);
-
- t[1].rx_buf = buf;
- t[1].len = count;
- spi_message_add_tail(&t[1], &m);
-
mutex_lock(&at25->lock);
/* Read it all at once.
@@ -122,8 +175,8 @@ static int at25_ee_read(void *priv, unsigned int offset,
* other devices on the bus need to be accessed regularly or
* this chip is clocked very slowly
*/
- status = spi_sync(at25->spi, &m);
- dev_dbg(&at25->spi->dev, "read %zu bytes at %d --> %zd\n",
+ status = spi_mem_dirmap_read(desc, dirmap_offset, count, val);
+ dev_dbg(&at25->spimem->spi->dev, "read %zu bytes at %d --> %zd\n",
count, offset, status);
mutex_unlock(&at25->lock);
@@ -149,7 +202,7 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
buf_size = at25->chip.page_size;
if (buf_size > io_limit)
buf_size = io_limit;
- bounce = kmalloc(buf_size + at25->addrlen + 1, GFP_KERNEL);
+ bounce = kmalloc(buf_size, GFP_KERNEL);
if (!bounce)
return -ENOMEM;
@@ -158,48 +211,37 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
*/
mutex_lock(&at25->lock);
do {
+ struct spi_mem_dirmap_desc *desc;
unsigned long timeout, retries;
unsigned segment;
- unsigned offset = (unsigned) off;
- u8 *cp = bounce;
+ unsigned int dirmap_offset;
int sr;
- u8 instr;
- *cp = AT25_WREN;
- status = spi_write(at25->spi, cp, 1);
+ status = at25_wren(at25);
if (status < 0) {
- dev_dbg(&at25->spi->dev, "WREN --> %d\n", status);
+ dev_dbg(&at25->spimem->spi->dev, "WREN --> %d\n",
+ status);
break;
}
- instr = AT25_WRITE;
- if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
- if (offset >= (1U << (at25->addrlen * 8)))
- instr |= AT25_INSTR_BIT3;
- *cp++ = instr;
-
- /* 8/16/24-bit address is written MSB first */
- switch (at25->addrlen) {
- default: /* case 3 */
- *cp++ = offset >> 16;
- /* fall through */
- case 2:
- *cp++ = offset >> 8;
- /* fall through */
- case 1:
- case 0: /* can't happen: for better codegen */
- *cp++ = offset >> 0;
+ if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && off > 255) {
+ desc = at25->dirmap.wdesc[1];
+ dirmap_offset = off - 256;
+ } else {
+ desc = at25->dirmap.wdesc[0];
+ dirmap_offset = off;
}
/* Write as much of a page as we can */
- segment = buf_size - (offset % buf_size);
+ segment = buf_size - (dirmap_offset % buf_size);
if (segment > count)
segment = count;
- memcpy(cp, buf, segment);
- status = spi_write(at25->spi, bounce,
- segment + at25->addrlen + 1);
- dev_dbg(&at25->spi->dev, "write %u bytes at %u --> %d\n",
- segment, offset, status);
+ memcpy(bounce, buf, segment);
+ status = spi_mem_dirmap_write(desc, dirmap_offset, segment,
+ bounce);
+ dev_dbg(&at25->spimem->spi->dev,
+ "write %u bytes at %u --> %d\n",
+ segment, off, status);
if (status < 0)
break;
@@ -211,10 +253,9 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
timeout = jiffies + msecs_to_jiffies(EE_TIMEOUT);
retries = 0;
do {
-
- sr = spi_w8r8(at25->spi, AT25_RDSR);
+ sr = at25_rdsr(at25);
if (sr < 0 || (sr & AT25_SR_nRDY)) {
- dev_dbg(&at25->spi->dev,
+ dev_dbg(&at25->spimem->spi->dev,
"rdsr --> %d (%02x)\n", sr, sr);
/* at HZ=100, this is sloooow */
msleep(1);
@@ -225,9 +266,9 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
} while (retries++ < 3 || time_before_eq(jiffies, timeout));
if ((sr < 0) || (sr & AT25_SR_nRDY)) {
- dev_err(&at25->spi->dev,
+ dev_err(&at25->spimem->spi->dev,
"write %u bytes offset %u, timeout after %u msecs\n",
- segment, offset,
+ segment, off,
jiffies_to_msecs(jiffies -
(timeout - EE_TIMEOUT)));
status = -ETIMEDOUT;
@@ -304,7 +345,7 @@ static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip)
return 0;
}
-static int at25_probe(struct spi_device *spi)
+static int at25_probe(struct spi_mem *spimem)
{
struct at25_data *at25 = NULL;
struct spi_eeprom chip;
@@ -313,12 +354,12 @@ static int at25_probe(struct spi_device *spi)
int addrlen;
/* Chip description */
- if (!spi->dev.platform_data) {
- err = at25_fw_to_chip(&spi->dev, &chip);
+ if (!spimem->spi->dev.platform_data) {
+ err = at25_fw_to_chip(&spimem->spi->dev, &chip);
if (err)
return err;
} else
- chip = *(struct spi_eeprom *)spi->dev.platform_data;
+ chip = *(struct spi_eeprom *)spimem->spi->dev.platform_data;
/* For now we only support 8/16/24 bit addressing */
if (chip.flags & EE_ADDR1)
@@ -328,37 +369,49 @@ static int at25_probe(struct spi_device *spi)
else if (chip.flags & EE_ADDR3)
addrlen = 3;
else {
- dev_dbg(&spi->dev, "unsupported address type\n");
+ dev_dbg(&spimem->spi->dev, "unsupported address type\n");
return -EINVAL;
}
- /* Ping the chip ... the status register is pretty portable,
- * unlike probing manufacturer IDs. We do expect that system
- * firmware didn't write it in the past few milliseconds!
- */
- sr = spi_w8r8(spi, AT25_RDSR);
- if (sr < 0 || sr & AT25_SR_nRDY) {
- dev_dbg(&spi->dev, "rdsr --> %d (%02x)\n", sr, sr);
- return -ENXIO;
- }
-
- at25 = devm_kzalloc(&spi->dev, sizeof(struct at25_data), GFP_KERNEL);
+ at25 = devm_kzalloc(&spimem->spi->dev, sizeof(struct at25_data),
+ GFP_KERNEL);
if (!at25)
return -ENOMEM;
mutex_init(&at25->lock);
at25->chip = chip;
- at25->spi = spi;
- spi_set_drvdata(spi, at25);
+ at25->spimem = spimem;
+ spi_mem_set_drvdata(spimem, at25);
at25->addrlen = addrlen;
- at25->nvmem_config.name = dev_name(&spi->dev);
- at25->nvmem_config.dev = &spi->dev;
+ /*
+ * Can't be allocated with devm_kmalloc() because we need a DMA-safe
+ * buffer.
+ */
+ at25->scratchbuf = kmalloc(1, GFP_KERNEL);
+
+ /* Ping the chip ... the status register is pretty portable,
+ * unlike probing manufacturer IDs. We do expect that system
+ * firmware didn't write it in the past few milliseconds!
+ */
+ sr = at25_rdsr(at25);
+ if (sr < 0 || sr & AT25_SR_nRDY) {
+ dev_dbg(&spimem->spi->dev, "rdsr --> %d (%02x)\n", sr, sr);
+ err = -ENXIO;
+ goto err_free_scratchbuf;
+ }
+
+ err = at25_create_dirmaps(at25);
+ if (err)
+ goto err_free_scratchbuf;
+
+ at25->nvmem_config.name = dev_name(&spimem->spi->dev);
+ at25->nvmem_config.dev = &spimem->spi->dev;
at25->nvmem_config.read_only = chip.flags & EE_READONLY;
at25->nvmem_config.root_only = true;
at25->nvmem_config.owner = THIS_MODULE;
at25->nvmem_config.compat = true;
- at25->nvmem_config.base_dev = &spi->dev;
+ at25->nvmem_config.base_dev = &spimem->spi->dev;
at25->nvmem_config.reg_read = at25_ee_read;
at25->nvmem_config.reg_write = at25_ee_write;
at25->nvmem_config.priv = at25;
@@ -366,17 +419,34 @@ static int at25_probe(struct spi_device *spi)
at25->nvmem_config.word_size = 1;
at25->nvmem_config.size = chip.byte_len;
- at25->nvmem = devm_nvmem_register(&spi->dev, &at25->nvmem_config);
- if (IS_ERR(at25->nvmem))
- return PTR_ERR(at25->nvmem);
+ at25->nvmem = devm_nvmem_register(&spimem->spi->dev,
+ &at25->nvmem_config);
+ if (IS_ERR(at25->nvmem)) {
+ err = PTR_ERR(at25->nvmem);
+ goto err_free_scratchbuf;
+ }
- dev_info(&spi->dev, "%d %s %s eeprom%s, pagesize %u\n",
+ dev_info(&spimem->spi->dev, "%d %s %s eeprom%s, pagesize %u\n",
(chip.byte_len < 1024) ? chip.byte_len : (chip.byte_len / 1024),
(chip.byte_len < 1024) ? "Byte" : "KByte",
at25->chip.name,
(chip.flags & EE_READONLY) ? " (readonly)" : "",
at25->chip.page_size);
return 0;
+
+err_free_scratchbuf:
+ kfree(at25->scratchbuf);
+
+ return err;
+}
+
+static int at25_remove(struct spi_mem *spimem)
+{
+ struct at25_data *at25 = spi_mem_get_drvdata(spimem);
+
+ kfree(at25->scratchbuf);
+
+ return 0;
}
/*-------------------------------------------------------------------------*/
@@ -387,15 +457,15 @@ static const struct of_device_id at25_of_match[] = {
};
MODULE_DEVICE_TABLE(of, at25_of_match);
-static struct spi_driver at25_driver = {
- .driver = {
- .name = "at25",
+static struct spi_mem_driver at25_driver = {
+ .spidrv.driver = {
+ .name = "at25",
.of_match_table = at25_of_match,
},
- .probe = at25_probe,
+ .probe = at25_probe,
+ .remove = at25_remove,
};
-
-module_spi_driver(at25_driver);
+module_spi_mem_driver(at25_driver);
MODULE_DESCRIPTION("Driver for most SPI EEPROMs");
MODULE_AUTHOR("David Brownell");
--
2.20.1
Hi Boris,
On Sat, Mar 30, 2019 at 3:16 PM Boris Brezillon
<[email protected]> wrote:
> The AT25 protocol fits pretty well in the spi-mem model. Convert the
> at25 spi driver to a spi-mem driver and use the dirmap API instead of
> forging SPI messages manually.
> This makes the driver compatible with spi-mem-only controllers
> (controllers implementing only the spi_mem ops).
Thanks for your patch!
> Signed-off-by: Boris Brezillon <[email protected]>
Tested-by: Geert Uytterhoeven <[email protected]>
> --- a/drivers/misc/eeprom/at25.c
> +++ b/drivers/misc/eeprom/at25.c
> @@ -63,17 +68,89 @@ struct at25_data {
>
> #define io_limit PAGE_SIZE /* bytes */
>
> +static int at25_create_dirmaps(struct at25_data *at25)
> +{
> + struct spi_mem_dirmap_info info = {
> + .op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_READ, 1),
> + SPI_MEM_OP_ADDR(at25->addrlen, 0, 1),
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_DATA_IN(0, NULL, 1)),
> + .offset = 0,
> + .length = at25->chip.byte_len,
> + };
> + struct device *dev = &at25->spimem->spi->dev;
> +
> + if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
> + info.length = 256;
Note that by hardcoding 256 you do loose the ability to support chips
where EE_INSTR_BIT3_IS_ADDR is set, _and_ more than one address byte is
used (i.e. extra bit is A16 or A24).
While the code parsing "address-width" doesn't handle that, the comment
for EE_INSTR_BIT3_IS_ADDR suggests such chips do exist (I doubt it,
though, but you may know better), and the flag can be enabled through
the "at25,addr-mode" property, or through platform_data.
> @@ -82,38 +159,14 @@ static int at25_ee_read(void *priv, unsigned int offset,
> if (unlikely(!count))
> return -EINVAL;
>
> - cp = command;
> -
> - instr = AT25_READ;
> - if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
> - if (offset >= (1U << (at25->addrlen * 8)))
> - instr |= AT25_INSTR_BIT3;
> - *cp++ = instr;
> -
> - /* 8/16/24-bit address is written MSB first */
> - switch (at25->addrlen) {
> - default: /* case 3 */
> - *cp++ = offset >> 16;
> - /* fall through */
> - case 2:
> - *cp++ = offset >> 8;
> - /* fall through */
> - case 1:
> - case 0: /* can't happen: for better codegen */
> - *cp++ = offset >> 0;
> + if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && offset > 255) {
Loss of support for address bit A16/A24.
> + desc = at25->dirmap.rdesc[1];
> + dirmap_offset = offset - 256;
> + } else {
> + desc = at25->dirmap.rdesc[0];
> + dirmap_offset = offset;
> }
> @@ -158,48 +211,37 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
> */
> mutex_lock(&at25->lock);
> do {
> + struct spi_mem_dirmap_desc *desc;
> unsigned long timeout, retries;
> unsigned segment;
> - unsigned offset = (unsigned) off;
> - u8 *cp = bounce;
> + unsigned int dirmap_offset;
> int sr;
> - u8 instr;
>
> - *cp = AT25_WREN;
> - status = spi_write(at25->spi, cp, 1);
> + status = at25_wren(at25);
> if (status < 0) {
> - dev_dbg(&at25->spi->dev, "WREN --> %d\n", status);
> + dev_dbg(&at25->spimem->spi->dev, "WREN --> %d\n",
> + status);
> break;
> }
>
> - instr = AT25_WRITE;
> - if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
> - if (offset >= (1U << (at25->addrlen * 8)))
> - instr |= AT25_INSTR_BIT3;
> - *cp++ = instr;
> -
> - /* 8/16/24-bit address is written MSB first */
> - switch (at25->addrlen) {
> - default: /* case 3 */
> - *cp++ = offset >> 16;
> - /* fall through */
> - case 2:
> - *cp++ = offset >> 8;
> - /* fall through */
> - case 1:
> - case 0: /* can't happen: for better codegen */
> - *cp++ = offset >> 0;
> + if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && off > 255) {
Loss of support for address bit A16/A24.
> + desc = at25->dirmap.wdesc[1];
> + dirmap_offset = off - 256;
Two spaces after minus sign.
> + } else {
> + desc = at25->dirmap.wdesc[0];
> + dirmap_offset = off;
> }
> @@ -211,10 +253,9 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
> timeout = jiffies + msecs_to_jiffies(EE_TIMEOUT);
> retries = 0;
> do {
> -
> - sr = spi_w8r8(at25->spi, AT25_RDSR);
> + sr = at25_rdsr(at25);
> if (sr < 0 || (sr & AT25_SR_nRDY)) {
> - dev_dbg(&at25->spi->dev,
> + dev_dbg(&at25->spimem->spi->dev,
> "rdsr --> %d (%02x)\n", sr, sr);
You might want to move the debug print inside the new function
at25_rdsr(), as there's another call plus debug print in at25_probe().
> @@ -328,37 +369,49 @@ static int at25_probe(struct spi_device *spi)
> else if (chip.flags & EE_ADDR3)
> addrlen = 3;
> else {
> - dev_dbg(&spi->dev, "unsupported address type\n");
> + dev_dbg(&spimem->spi->dev, "unsupported address type\n");
> return -EINVAL;
> }
>
> - /* Ping the chip ... the status register is pretty portable,
> - * unlike probing manufacturer IDs. We do expect that system
> - * firmware didn't write it in the past few milliseconds!
> - */
> - sr = spi_w8r8(spi, AT25_RDSR);
> - if (sr < 0 || sr & AT25_SR_nRDY) {
> - dev_dbg(&spi->dev, "rdsr --> %d (%02x)\n", sr, sr);
> - return -ENXIO;
> - }
> -
> - at25 = devm_kzalloc(&spi->dev, sizeof(struct at25_data), GFP_KERNEL);
> + at25 = devm_kzalloc(&spimem->spi->dev, sizeof(struct at25_data),
> + GFP_KERNEL);
> if (!at25)
> return -ENOMEM;
>
> mutex_init(&at25->lock);
> at25->chip = chip;
> - at25->spi = spi;
> - spi_set_drvdata(spi, at25);
> + at25->spimem = spimem;
> + spi_mem_set_drvdata(spimem, at25);
> at25->addrlen = addrlen;
>
> - at25->nvmem_config.name = dev_name(&spi->dev);
> - at25->nvmem_config.dev = &spi->dev;
> + /*
> + * Can't be allocated with devm_kmalloc() because we need a DMA-safe
> + * buffer.
> + */
That is no longer true as of commit a66d972465d15b1d ("devres: Align
data[] to ARCH_KMALLOC_MINALIGN") in v4.20-rc5, right?
> + at25->scratchbuf = kmalloc(1, GFP_KERNEL);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Boris,
I love your patch! Yet something to improve:
[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v5.1-rc3 next-20190401]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Boris-Brezillon/eeprom-at25-Convert-the-driver-to-the-spi-mem-interface/20190401-160450
config: i386-randconfig-m2-201913 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
>> ERROR: "spi_mem_driver_unregister" [drivers/misc/eeprom/at25.ko] undefined!
>> ERROR: "spi_mem_driver_register_with_owner" [drivers/misc/eeprom/at25.ko] undefined!
>> ERROR: "devm_spi_mem_dirmap_create" [drivers/misc/eeprom/at25.ko] undefined!
>> ERROR: "spi_mem_dirmap_write" [drivers/misc/eeprom/at25.ko] undefined!
>> ERROR: "spi_mem_dirmap_read" [drivers/misc/eeprom/at25.ko] undefined!
>> ERROR: "spi_mem_exec_op" [drivers/misc/eeprom/at25.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Boris,
I love your patch! Yet something to improve:
[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v5.1-rc3 next-20190401]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Boris-Brezillon/eeprom-at25-Convert-the-driver-to-the-spi-mem-interface/20190401-160450
config: x86_64-randconfig-m3-201913 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/misc/eeprom/at25.o: In function `at25_rdsr':
>> drivers/misc/eeprom/at25.c:130: undefined reference to `spi_mem_exec_op'
drivers/misc/eeprom/at25.o: In function `at25_ee_read':
>> drivers/misc/eeprom/at25.c:178: undefined reference to `spi_mem_dirmap_read'
drivers/misc/eeprom/at25.o: In function `at25_create_dirmaps':
>> drivers/misc/eeprom/at25.c:86: undefined reference to `devm_spi_mem_dirmap_create'
drivers/misc/eeprom/at25.c:94: undefined reference to `devm_spi_mem_dirmap_create'
drivers/misc/eeprom/at25.c:106: undefined reference to `devm_spi_mem_dirmap_create'
drivers/misc/eeprom/at25.c:114: undefined reference to `devm_spi_mem_dirmap_create'
drivers/misc/eeprom/at25.o: In function `at25_wren':
drivers/misc/eeprom/at25.c:144: undefined reference to `spi_mem_exec_op'
drivers/misc/eeprom/at25.o: In function `at25_ee_write':
>> drivers/misc/eeprom/at25.c:240: undefined reference to `spi_mem_dirmap_write'
drivers/misc/eeprom/at25.o: In function `at25_driver_init':
>> drivers/misc/eeprom/at25.c:468: undefined reference to `spi_mem_driver_register_with_owner'
drivers/misc/eeprom/at25.o: In function `at25_driver_exit':
>> drivers/misc/eeprom/at25.c:468: undefined reference to `spi_mem_driver_unregister'
vim +130 drivers/misc/eeprom/at25.c
70
71 static int at25_create_dirmaps(struct at25_data *at25)
72 {
73 struct spi_mem_dirmap_info info = {
74 .op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_READ, 1),
75 SPI_MEM_OP_ADDR(at25->addrlen, 0, 1),
76 SPI_MEM_OP_NO_DUMMY,
77 SPI_MEM_OP_DATA_IN(0, NULL, 1)),
78 .offset = 0,
79 .length = at25->chip.byte_len,
80 };
81 struct device *dev = &at25->spimem->spi->dev;
82
83 if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
84 info.length = 256;
85
> 86 at25->dirmap.rdesc[0] = devm_spi_mem_dirmap_create(dev, at25->spimem,
87 &info);
88 if (IS_ERR(at25->dirmap.rdesc[0]))
89 return PTR_ERR(at25->dirmap.rdesc[0]);
90
91 info.op_tmpl.cmd.opcode = AT25_WRITE;
92 info.op_tmpl.data.dir = SPI_MEM_DATA_OUT;
93
94 at25->dirmap.wdesc[0] = devm_spi_mem_dirmap_create(dev, at25->spimem,
95 &info);
96 if (IS_ERR(at25->dirmap.wdesc[0]))
97 return PTR_ERR(at25->dirmap.wdesc[0]);
98
99 if (!(at25->chip.flags & EE_INSTR_BIT3_IS_ADDR))
100 return 0;
101
102 info.length = at25->chip.byte_len - 256;
103 info.op_tmpl.cmd.opcode = AT25_READ | AT25_INSTR_BIT3;
104 info.op_tmpl.data.dir = SPI_MEM_DATA_IN;
105
> 106 at25->dirmap.rdesc[1] = devm_spi_mem_dirmap_create(dev, at25->spimem,
107 &info);
108 if (IS_ERR(at25->dirmap.rdesc[1]))
109 return PTR_ERR(at25->dirmap.rdesc[1]);
110
111 info.op_tmpl.cmd.opcode = AT25_WRITE | AT25_INSTR_BIT3;
112 info.op_tmpl.data.dir = SPI_MEM_DATA_OUT;
113
114 at25->dirmap.wdesc[1] = devm_spi_mem_dirmap_create(dev, at25->spimem,
115 &info);
116 if (IS_ERR(at25->dirmap.wdesc[1]))
117 return PTR_ERR(at25->dirmap.wdesc[1]);
118
119 return 0;
120 }
121
122 static int at25_rdsr(struct at25_data *at25)
123 {
124 struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_RDSR, 1),
125 SPI_MEM_OP_NO_ADDR,
126 SPI_MEM_OP_NO_DUMMY,
127 SPI_MEM_OP_DATA_IN(1, at25->scratchbuf, 1));
128 int ret;
129
> 130 ret = spi_mem_exec_op(at25->spimem, &op);
131 if (ret)
132 return ret;
133
134 return *((u8 *)at25->scratchbuf);
135 }
136
137 static int at25_wren(struct at25_data *at25)
138 {
139 struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_WREN, 1),
140 SPI_MEM_OP_NO_ADDR,
141 SPI_MEM_OP_NO_DUMMY,
142 SPI_MEM_OP_NO_DATA);
143
144 return spi_mem_exec_op(at25->spimem, &op);
145 }
146
147 static int at25_ee_read(void *priv, unsigned int offset,
148 void *val, size_t count)
149 {
150 struct spi_mem_dirmap_desc *desc;
151 struct at25_data *at25 = priv;
152 unsigned int dirmap_offset;
153 ssize_t status;
154
155 if (unlikely(offset >= at25->chip.byte_len))
156 return -EINVAL;
157 if ((offset + count) > at25->chip.byte_len)
158 count = at25->chip.byte_len - offset;
159 if (unlikely(!count))
160 return -EINVAL;
161
162 if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && offset > 255) {
163 desc = at25->dirmap.rdesc[1];
164 dirmap_offset = offset - 256;
165 } else {
166 desc = at25->dirmap.rdesc[0];
167 dirmap_offset = offset;
168 }
169
170 mutex_lock(&at25->lock);
171
172 /* Read it all at once.
173 *
174 * REVISIT that's potentially a problem with large chips, if
175 * other devices on the bus need to be accessed regularly or
176 * this chip is clocked very slowly
177 */
> 178 status = spi_mem_dirmap_read(desc, dirmap_offset, count, val);
179 dev_dbg(&at25->spimem->spi->dev, "read %zu bytes at %d --> %zd\n",
180 count, offset, status);
181
182 mutex_unlock(&at25->lock);
183 return status;
184 }
185
186 static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
187 {
188 struct at25_data *at25 = priv;
189 const char *buf = val;
190 int status = 0;
191 unsigned buf_size;
192 u8 *bounce;
193
194 if (unlikely(off >= at25->chip.byte_len))
195 return -EFBIG;
196 if ((off + count) > at25->chip.byte_len)
197 count = at25->chip.byte_len - off;
198 if (unlikely(!count))
199 return -EINVAL;
200
201 /* Temp buffer starts with command and address */
202 buf_size = at25->chip.page_size;
203 if (buf_size > io_limit)
204 buf_size = io_limit;
205 bounce = kmalloc(buf_size, GFP_KERNEL);
206 if (!bounce)
207 return -ENOMEM;
208
209 /* For write, rollover is within the page ... so we write at
210 * most one page, then manually roll over to the next page.
211 */
212 mutex_lock(&at25->lock);
213 do {
214 struct spi_mem_dirmap_desc *desc;
215 unsigned long timeout, retries;
216 unsigned segment;
217 unsigned int dirmap_offset;
218 int sr;
219
220 status = at25_wren(at25);
221 if (status < 0) {
222 dev_dbg(&at25->spimem->spi->dev, "WREN --> %d\n",
223 status);
224 break;
225 }
226
227 if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && off > 255) {
228 desc = at25->dirmap.wdesc[1];
229 dirmap_offset = off - 256;
230 } else {
231 desc = at25->dirmap.wdesc[0];
232 dirmap_offset = off;
233 }
234
235 /* Write as much of a page as we can */
236 segment = buf_size - (dirmap_offset % buf_size);
237 if (segment > count)
238 segment = count;
239 memcpy(bounce, buf, segment);
> 240 status = spi_mem_dirmap_write(desc, dirmap_offset, segment,
241 bounce);
242 dev_dbg(&at25->spimem->spi->dev,
243 "write %u bytes at %u --> %d\n",
244 segment, off, status);
245 if (status < 0)
246 break;
247
248 /* REVISIT this should detect (or prevent) failed writes
249 * to readonly sections of the EEPROM...
250 */
251
252 /* Wait for non-busy status */
253 timeout = jiffies + msecs_to_jiffies(EE_TIMEOUT);
254 retries = 0;
255 do {
256 sr = at25_rdsr(at25);
257 if (sr < 0 || (sr & AT25_SR_nRDY)) {
258 dev_dbg(&at25->spimem->spi->dev,
259 "rdsr --> %d (%02x)\n", sr, sr);
260 /* at HZ=100, this is sloooow */
261 msleep(1);
262 continue;
263 }
264 if (!(sr & AT25_SR_nRDY))
265 break;
266 } while (retries++ < 3 || time_before_eq(jiffies, timeout));
267
268 if ((sr < 0) || (sr & AT25_SR_nRDY)) {
269 dev_err(&at25->spimem->spi->dev,
270 "write %u bytes offset %u, timeout after %u msecs\n",
271 segment, off,
272 jiffies_to_msecs(jiffies -
273 (timeout - EE_TIMEOUT)));
274 status = -ETIMEDOUT;
275 break;
276 }
277
278 off += segment;
279 buf += segment;
280 count -= segment;
281
282 } while (count > 0);
283
284 mutex_unlock(&at25->lock);
285
286 kfree(bounce);
287 return status;
288 }
289
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Sat, Mar 30, 2019 at 03:16:37PM +0100, Boris Brezillon wrote:
> The AT25 protocol fits pretty well in the spi-mem model. Convert the
> at25 spi driver to a spi-mem driver and use the dirmap API instead of
> forging SPI messages manually.
> This makes the driver compatible with spi-mem-only controllers
> (controllers implementing only the spi_mem ops).
>
> Cc: Geert Uytterhoeven <[email protected]>
> Signed-off-by: Boris Brezillon <[email protected]>
> ---
> drivers/misc/eeprom/at25.c | 282 +++++++++++++++++++++++--------------
> 1 file changed, 176 insertions(+), 106 deletions(-)
Will there be a new version of this to fix up the problems that 0-day
found in it?
thanks,
greg k-h
On Thu, Apr 25, 2019 at 9:44 PM Greg Kroah-Hartman
<[email protected]> wrote:
> On Sat, Mar 30, 2019 at 03:16:37PM +0100, Boris Brezillon wrote:
> > The AT25 protocol fits pretty well in the spi-mem model. Convert the
> > at25 spi driver to a spi-mem driver and use the dirmap API instead of
> > forging SPI messages manually.
> > This makes the driver compatible with spi-mem-only controllers
> > (controllers implementing only the spi_mem ops).
> >
> > Cc: Geert Uytterhoeven <[email protected]>
> > Signed-off-by: Boris Brezillon <[email protected]>
> > ---
> > drivers/misc/eeprom/at25.c | 282 +++++++++++++++++++++++--------------
> > 1 file changed, 176 insertions(+), 106 deletions(-)
>
> Will there be a new version of this to fix up the problems that 0-day
> found in it?
gmail-whitespace-damaged fix:
diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
index f2abe27010eff133..98145d7d43d0c728 100644
--- a/drivers/misc/eeprom/Kconfig
+++ b/drivers/misc/eeprom/Kconfig
@@ -36,6 +36,7 @@ config EEPROM_AT25
depends on SPI && SYSFS
select NVMEM
select NVMEM_SYSFS
+ select SPI_MEM
help
Enable this driver to get read/write support to most SPI EEPROMs,
after you configure the board init code to know about each eeprom
Boris: what's the plan?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On Tue, 3 Sep 2019 17:06:52 +0200
Geert Uytterhoeven <[email protected]> wrote:
> On Thu, Apr 25, 2019 at 9:44 PM Greg Kroah-Hartman
> <[email protected]> wrote:
> > On Sat, Mar 30, 2019 at 03:16:37PM +0100, Boris Brezillon wrote:
> > > The AT25 protocol fits pretty well in the spi-mem model. Convert the
> > > at25 spi driver to a spi-mem driver and use the dirmap API instead of
> > > forging SPI messages manually.
> > > This makes the driver compatible with spi-mem-only controllers
> > > (controllers implementing only the spi_mem ops).
> > >
> > > Cc: Geert Uytterhoeven <[email protected]>
> > > Signed-off-by: Boris Brezillon <[email protected]>
> > > ---
> > > drivers/misc/eeprom/at25.c | 282 +++++++++++++++++++++++--------------
> > > 1 file changed, 176 insertions(+), 106 deletions(-)
> >
> > Will there be a new version of this to fix up the problems that 0-day
> > found in it?
>
> gmail-whitespace-damaged fix:
>
> diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
> index f2abe27010eff133..98145d7d43d0c728 100644
> --- a/drivers/misc/eeprom/Kconfig
> +++ b/drivers/misc/eeprom/Kconfig
> @@ -36,6 +36,7 @@ config EEPROM_AT25
> depends on SPI && SYSFS
> select NVMEM
> select NVMEM_SYSFS
> + select SPI_MEM
> help
> Enable this driver to get read/write support to most SPI EEPROMs,
> after you configure the board init code to know about each eeprom
>
> Boris: what's the plan?
Sorry, I don't have time to respin this patch. Feel free to send a new
version if you need it.
Regards,
Boris