eeprom: at25: convert to spi-mem API

Replace the RAW SPI accesses with spi-mem API. The latter will fall back to
RAW SPI accesses if spi-mem callbacks are not implemented by a controller
driver.

Notable advantages:
- read function now allocates a bounce buffer for SPI DMA compatibility,
  similar to write function;
- the driver can now be used in conjunction with SPI controller drivers
  providing spi-mem API only, e.g. spi-nxp-fspi.
- during the initial probe the driver polls busy/ready status bit for 25ms
  instead of giving up instantly and hoping that the FW didn't write the
  EEPROM

Notes:
- mutex_lock() has been dropped from fm25_aux_read() because the latter is
  only being called in probe phase and therefore cannot race with
  at25_ee_read() or at25_ee_write()

Quick 4KB block size test with CY15B102Q 256KB F-RAM over spi_omap2_mcspi
driver (no spi-mem ops provided, fallback to raw SPI inside spi-mem):

OP	| throughput, KB/s	| change
--------+-----------------------+-------
write	| 1717.847 -> 1656.684	| -3.6%
read	| 1115.868 -> 1059.367	| -5.1%

The lower throughtput probably comes from the 3 messages per SPI transfer
inside spi-mem instead of hand-crafted 2 messages per transfer in the
former at25 code. However, if the raw SPI access is not preserved, then
the driver doesn't grow from the lines-of-code perspective and subjectively
could be considered even a bit simpler.

Higher performance impact on the read operation could be explained by the
newly introduced bounce buffer in read operation. I didn't find any
explanation or guarantee, why would a bounce buffer be not needed on the
read side, so I assume it's a pure luck that nobody read EEPROM into
some variable on stack on an architecture where kernel stack would be
not DMA-able.

Cc: Michael Walle <mwalle@kernel.org>
Cc: Hui Wang <hui.wang@canonical.com>
Link: https://lore.kernel.org/all/28ab8b72afee1af59b628f7389f0d7f5@kernel.org/
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
Link: https://lore.kernel.org/r/20250702222823.864803-1-alexander.sverdlin@siemens.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Alexander Sverdlin
2025-07-03 00:28:22 +02:00
committed by Greg Kroah-Hartman
parent 8282013b56
commit 8ad6249c51
2 changed files with 165 additions and 163 deletions

View File

@@ -37,6 +37,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
and Cypress FRAMs,

View File

@@ -7,8 +7,10 @@
*/
#include <linux/bits.h>
#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/iopoll.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/property.h>
@@ -17,6 +19,7 @@
#include <linux/spi/eeprom.h>
#include <linux/spi/spi.h>
#include <linux/spi/spi-mem.h>
#include <linux/nvmem-provider.h>
@@ -35,13 +38,12 @@
struct at25_data {
struct spi_eeprom chip;
struct spi_device *spi;
struct spi_mem *spimem;
struct mutex lock;
unsigned addrlen;
struct nvmem_config nvmem_config;
struct nvmem_device *nvmem;
u8 sernum[FM25_SN_LEN];
u8 command[EE_MAXADDRLEN + 1];
};
#define AT25_WREN 0x06 /* latch the write enable */
@@ -74,20 +76,29 @@ struct at25_data {
#define io_limit PAGE_SIZE /* bytes */
/* Handle the address MSB as part of instruction byte */
static u8 at25_instr(struct at25_data *at25, u8 instr, unsigned int off)
{
if (!(at25->chip.flags & EE_INSTR_BIT3_IS_ADDR))
return instr;
if (off < BIT(at25->addrlen * 8))
return instr;
return instr | AT25_INSTR_BIT3;
}
static int at25_ee_read(void *priv, unsigned int offset,
void *val, size_t count)
{
u8 *bounce __free(kfree) = kmalloc(min(count, io_limit), GFP_KERNEL);
struct at25_data *at25 = priv;
char *buf = val;
size_t max_chunk = spi_max_transfer_size(at25->spi);
unsigned int msg_offset = offset;
size_t bytes_left = count;
size_t segment;
u8 *cp;
ssize_t status;
struct spi_transfer t[2];
struct spi_message m;
u8 instr;
int status;
if (!bounce)
return -ENOMEM;
if (unlikely(offset >= at25->chip.byte_len))
return -EINVAL;
@@ -97,87 +108,67 @@ static int at25_ee_read(void *priv, unsigned int offset,
return -EINVAL;
do {
segment = min(bytes_left, max_chunk);
cp = at25->command;
struct spi_mem_op op;
instr = AT25_READ;
if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
if (msg_offset >= BIT(at25->addrlen * 8))
instr |= AT25_INSTR_BIT3;
segment = min(bytes_left, io_limit);
mutex_lock(&at25->lock);
*cp++ = instr;
/* 8/16/24-bit address is written MSB first */
switch (at25->addrlen) {
default: /* case 3 */
*cp++ = msg_offset >> 16;
fallthrough;
case 2:
*cp++ = msg_offset >> 8;
fallthrough;
case 1:
case 0: /* can't happen: for better code generation */
*cp++ = msg_offset >> 0;
}
spi_message_init(&m);
memset(t, 0, sizeof(t));
t[0].tx_buf = at25->command;
t[0].len = at25->addrlen + 1;
spi_message_add_tail(&t[0], &m);
t[1].rx_buf = buf;
t[1].len = segment;
spi_message_add_tail(&t[1], &m);
status = spi_sync(at25->spi, &m);
mutex_unlock(&at25->lock);
op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(at25_instr(at25, AT25_READ,
msg_offset), 1),
SPI_MEM_OP_ADDR(at25->addrlen, msg_offset, 1),
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(segment, bounce, 1));
status = spi_mem_adjust_op_size(at25->spimem, &op);
if (status)
return status;
segment = op.data.nbytes;
mutex_lock(&at25->lock);
status = spi_mem_exec_op(at25->spimem, &op);
mutex_unlock(&at25->lock);
if (status)
return status;
memcpy(buf, bounce, segment);
msg_offset += segment;
buf += segment;
bytes_left -= segment;
} while (bytes_left > 0);
dev_dbg(&at25->spi->dev, "read %zu bytes at %d\n",
dev_dbg(&at25->spimem->spi->dev, "read %zu bytes at %d\n",
count, offset);
return 0;
}
/* Read extra registers as ID or serial number */
/*
* Read extra registers as ID or serial number
*
* Allow for the callers to provide @buf on stack (not necessary DMA-capable)
* by allocating a bounce buffer internally.
*/
static int fm25_aux_read(struct at25_data *at25, u8 *buf, uint8_t command,
int len)
{
u8 *bounce __free(kfree) = kmalloc(len, GFP_KERNEL);
struct spi_mem_op op;
int status;
struct spi_transfer t[2];
struct spi_message m;
spi_message_init(&m);
memset(t, 0, sizeof(t));
if (!bounce)
return -ENOMEM;
t[0].tx_buf = at25->command;
t[0].len = 1;
spi_message_add_tail(&t[0], &m);
op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(command, 1),
SPI_MEM_OP_NO_ADDR,
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(len, bounce, 1));
t[1].rx_buf = buf;
t[1].len = len;
spi_message_add_tail(&t[1], &m);
status = spi_mem_exec_op(at25->spimem, &op);
dev_dbg(&at25->spimem->spi->dev, "read %d aux bytes --> %d\n", len, status);
if (status)
return status;
mutex_lock(&at25->lock);
memcpy(buf, bounce, len);
at25->command[0] = command;
status = spi_sync(at25->spi, &m);
dev_dbg(&at25->spi->dev, "read %d aux bytes --> %d\n", len, status);
mutex_unlock(&at25->lock);
return status;
return 0;
}
static ssize_t sernum_show(struct device *dev, struct device_attribute *attr, char *buf)
@@ -195,14 +186,47 @@ static struct attribute *sernum_attrs[] = {
};
ATTRIBUTE_GROUPS(sernum);
/*
* Poll Read Status Register with timeout
*
* Return:
* 0, if the chip is ready
* [positive] Status Register value as-is, if the chip is busy
* [negative] error code in case of read failure
*/
static int at25_wait_ready(struct at25_data *at25)
{
u8 *bounce __free(kfree) = kmalloc(1, GFP_KERNEL);
struct spi_mem_op op;
int status;
if (!bounce)
return -ENOMEM;
op = (struct spi_mem_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, bounce, 1));
read_poll_timeout(spi_mem_exec_op, status,
status || !(bounce[0] & AT25_SR_nRDY), false,
USEC_PER_MSEC, USEC_PER_MSEC * EE_TIMEOUT,
at25->spimem, &op);
if (status < 0)
return status;
if (!(bounce[0] & AT25_SR_nRDY))
return 0;
return bounce[0];
}
static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
{
u8 *bounce __free(kfree) = kmalloc(min(count, io_limit), GFP_KERNEL);
struct at25_data *at25 = priv;
size_t maxsz = spi_max_transfer_size(at25->spi);
const char *buf = val;
int status = 0;
unsigned buf_size;
u8 *bounce;
unsigned int buf_size;
int status;
if (unlikely(off >= at25->chip.byte_len))
return -EFBIG;
@@ -211,11 +235,8 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
if (unlikely(!count))
return -EINVAL;
/* Temp buffer starts with command and address */
buf_size = at25->chip.page_size;
if (buf_size > io_limit)
buf_size = io_limit;
bounce = kmalloc(buf_size + at25->addrlen + 1, GFP_KERNEL);
if (!bounce)
return -ENOMEM;
@@ -223,85 +244,64 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
* For write, rollover is within the page ... so we write at
* most one page, then manually roll over to the next page.
*/
mutex_lock(&at25->lock);
guard(mutex)(&at25->lock);
do {
unsigned long timeout, retries;
unsigned segment;
unsigned offset = off;
u8 *cp = bounce;
int sr;
u8 instr;
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);
unsigned int segment;
*cp = AT25_WREN;
status = spi_write(at25->spi, cp, 1);
status = spi_mem_exec_op(at25->spimem, &op);
if (status < 0) {
dev_dbg(&at25->spi->dev, "WREN --> %d\n", status);
break;
}
instr = AT25_WRITE;
if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
if (offset >= BIT(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;
fallthrough;
case 2:
*cp++ = offset >> 8;
fallthrough;
case 1:
case 0: /* can't happen: for better code generation */
*cp++ = offset >> 0;
dev_dbg(&at25->spimem->spi->dev, "WREN --> %d\n", status);
return status;
}
/* Write as much of a page as we can */
segment = buf_size - (offset % buf_size);
segment = buf_size - (off % buf_size);
if (segment > count)
segment = count;
if (segment > maxsz)
segment = maxsz;
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);
if (status < 0)
break;
if (segment > io_limit)
segment = io_limit;
op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(at25_instr(at25, AT25_WRITE, off),
1),
SPI_MEM_OP_ADDR(at25->addrlen, off, 1),
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_OUT(segment, bounce, 1));
status = spi_mem_adjust_op_size(at25->spimem, &op);
if (status)
return status;
segment = op.data.nbytes;
memcpy(bounce, buf, segment);
status = spi_mem_exec_op(at25->spimem, &op);
dev_dbg(&at25->spimem->spi->dev, "write %u bytes at %u --> %d\n",
segment, off, status);
if (status)
return status;
/*
* REVISIT this should detect (or prevent) failed writes
* to read-only sections of the EEPROM...
*/
/* Wait for non-busy status */
timeout = jiffies + msecs_to_jiffies(EE_TIMEOUT);
retries = 0;
do {
sr = spi_w8r8(at25->spi, AT25_RDSR);
if (sr < 0 || (sr & AT25_SR_nRDY)) {
dev_dbg(&at25->spi->dev,
"rdsr --> %d (%02x)\n", sr, sr);
/* at HZ=100, this is sloooow */
msleep(1);
continue;
}
if (!(sr & AT25_SR_nRDY))
break;
} while (retries++ < 3 || time_before_eq(jiffies, timeout));
if ((sr < 0) || (sr & AT25_SR_nRDY)) {
dev_err(&at25->spi->dev,
status = at25_wait_ready(at25);
if (status < 0) {
dev_err_probe(&at25->spimem->spi->dev, status,
"Read Status Redister command failed\n");
return status;
}
if (status) {
dev_dbg(&at25->spimem->spi->dev,
"Status %02x\n", status);
dev_err(&at25->spimem->spi->dev,
"write %u bytes offset %u, timeout after %u msecs\n",
segment, offset,
jiffies_to_msecs(jiffies -
(timeout - EE_TIMEOUT)));
status = -ETIMEDOUT;
break;
segment, off, EE_TIMEOUT);
return -ETIMEDOUT;
}
off += segment;
@@ -310,9 +310,6 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
} while (count > 0);
mutex_unlock(&at25->lock);
kfree(bounce);
return status;
}
@@ -445,31 +442,33 @@ static const struct spi_device_id at25_spi_ids[] = {
};
MODULE_DEVICE_TABLE(spi, at25_spi_ids);
static int at25_probe(struct spi_device *spi)
static int at25_probe(struct spi_mem *mem)
{
struct at25_data *at25 = NULL;
int err;
int sr;
struct spi_device *spi = mem->spi;
struct spi_eeprom *pdata;
struct at25_data *at25;
bool is_fram;
/*
* 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;
}
int err;
at25 = devm_kzalloc(&spi->dev, sizeof(*at25), GFP_KERNEL);
if (!at25)
return -ENOMEM;
at25->spimem = mem;
/*
* Ping the chip ... the status register is pretty portable,
* unlike probing manufacturer IDs.
*/
err = at25_wait_ready(at25);
if (err < 0)
return dev_err_probe(&spi->dev, err, "Read Status Register command failed\n");
if (err) {
dev_err(&spi->dev, "Not ready (%02x)\n", err);
return -ENXIO;
}
mutex_init(&at25->lock);
at25->spi = spi;
spi_set_drvdata(spi, at25);
is_fram = fwnode_device_is_compatible(dev_fwnode(&spi->dev), "cypress,fm25");
@@ -530,17 +529,19 @@ static int at25_probe(struct spi_device *spi)
/*-------------------------------------------------------------------------*/
static struct spi_driver at25_driver = {
.driver = {
.name = "at25",
.of_match_table = at25_of_match,
.dev_groups = sernum_groups,
static struct spi_mem_driver at25_driver = {
.spidrv = {
.driver = {
.name = "at25",
.of_match_table = at25_of_match,
.dev_groups = sernum_groups,
},
.id_table = at25_spi_ids,
},
.probe = at25_probe,
.id_table = at25_spi_ids,
};
module_spi_driver(at25_driver);
module_spi_mem_driver(at25_driver);
MODULE_DESCRIPTION("Driver for most SPI EEPROMs");
MODULE_AUTHOR("David Brownell");