That patch fixes handling SPI messages with two writing transfers. It's important when using e.g. by m25p80 driver which uses one transfer for opcode and another one for data. Thanks to that fix we can now drop m25p80 workaround patch. It means one less hack and also a better flash writing performance as there is no more data buf copying. Signed-off-by: Rafał Miłecki <rafal@milecki.pl>master
Rafał Miłecki
7 years ago
parent
d40a358136
commit
9c312ef628
@ -0,0 +1,107 @@ |
||||
From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <rafal@milecki.pl>
|
||||
Date: Fri, 29 Dec 2017 14:44:09 +0100
|
||||
Subject: [PATCH] spi: bcm53xx: simplify reading SPI data
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
This commit makes transfer function use spi_transfer_is_last to
|
||||
determine if currently processed transfer is the last one. Thanks to
|
||||
that we finally set hardware registers properly and it makes controller
|
||||
behave the way it's expected to.
|
||||
|
||||
This allows simplifying read function which can now simply start reading
|
||||
from the slot 0 instead of the last saved offset. It has been
|
||||
successfully tested using spi_write_then_read.
|
||||
|
||||
Moreover this change fixes handling messages with two writing transfers.
|
||||
It's important for SPI flash devices as their drivers commonly use one
|
||||
transfer for a command and another one for data.
|
||||
|
||||
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
|
||||
---
|
||||
drivers/spi/spi-bcm53xx.c | 26 ++++++++++----------------
|
||||
1 file changed, 10 insertions(+), 16 deletions(-)
|
||||
|
||||
--- a/drivers/spi/spi-bcm53xx.c
|
||||
+++ b/drivers/spi/spi-bcm53xx.c
|
||||
@@ -27,8 +27,6 @@ struct bcm53xxspi {
|
||||
struct bcma_device *core;
|
||||
struct spi_master *master;
|
||||
void __iomem *mmio_base;
|
||||
-
|
||||
- size_t read_offset;
|
||||
bool bspi; /* Boot SPI mode with memory mapping */
|
||||
};
|
||||
|
||||
@@ -172,8 +170,6 @@ static void bcm53xxspi_buf_write(struct
|
||||
|
||||
if (!cont)
|
||||
bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 0);
|
||||
-
|
||||
- b53spi->read_offset = len;
|
||||
}
|
||||
|
||||
static void bcm53xxspi_buf_read(struct bcm53xxspi *b53spi, u8 *r_buf,
|
||||
@@ -182,10 +178,10 @@ static void bcm53xxspi_buf_read(struct b
|
||||
u32 tmp;
|
||||
int i;
|
||||
|
||||
- for (i = 0; i < b53spi->read_offset + len; i++) {
|
||||
+ for (i = 0; i < len; i++) {
|
||||
tmp = B53SPI_CDRAM_CONT | B53SPI_CDRAM_PCS_DISABLE_ALL |
|
||||
B53SPI_CDRAM_PCS_DSCK;
|
||||
- if (!cont && i == b53spi->read_offset + len - 1)
|
||||
+ if (!cont && i == len - 1)
|
||||
tmp &= ~B53SPI_CDRAM_CONT;
|
||||
tmp &= ~0x1;
|
||||
/* Command Register File */
|
||||
@@ -194,8 +190,7 @@ static void bcm53xxspi_buf_read(struct b
|
||||
|
||||
/* Set queue pointers */
|
||||
bcm53xxspi_write(b53spi, B53SPI_MSPI_NEWQP, 0);
|
||||
- bcm53xxspi_write(b53spi, B53SPI_MSPI_ENDQP,
|
||||
- b53spi->read_offset + len - 1);
|
||||
+ bcm53xxspi_write(b53spi, B53SPI_MSPI_ENDQP, len - 1);
|
||||
|
||||
if (cont)
|
||||
bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 1);
|
||||
@@ -214,13 +209,11 @@ static void bcm53xxspi_buf_read(struct b
|
||||
bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 0);
|
||||
|
||||
for (i = 0; i < len; ++i) {
|
||||
- int offset = b53spi->read_offset + i;
|
||||
+ u16 reg = B53SPI_MSPI_RXRAM + 4 * (1 + i * 2);
|
||||
|
||||
/* Data stored in the transmit register file LSB */
|
||||
- r_buf[i] = (u8)bcm53xxspi_read(b53spi, B53SPI_MSPI_RXRAM + 4 * (1 + offset * 2));
|
||||
+ r_buf[i] = (u8)bcm53xxspi_read(b53spi, reg);
|
||||
}
|
||||
-
|
||||
- b53spi->read_offset = 0;
|
||||
}
|
||||
|
||||
static int bcm53xxspi_transfer_one(struct spi_master *master,
|
||||
@@ -238,7 +231,8 @@ static int bcm53xxspi_transfer_one(struc
|
||||
left = t->len;
|
||||
while (left) {
|
||||
size_t to_write = min_t(size_t, 16, left);
|
||||
- bool cont = left - to_write > 0;
|
||||
+ bool cont = !spi_transfer_is_last(master, t) ||
|
||||
+ left - to_write > 0;
|
||||
|
||||
bcm53xxspi_buf_write(b53spi, buf, to_write, cont);
|
||||
left -= to_write;
|
||||
@@ -250,9 +244,9 @@ static int bcm53xxspi_transfer_one(struc
|
||||
buf = (u8 *)t->rx_buf;
|
||||
left = t->len;
|
||||
while (left) {
|
||||
- size_t to_read = min_t(size_t, 16 - b53spi->read_offset,
|
||||
- left);
|
||||
- bool cont = left - to_read > 0;
|
||||
+ size_t to_read = min_t(size_t, 16, left);
|
||||
+ bool cont = !spi_transfer_is_last(master, t) ||
|
||||
+ left - to_read > 0;
|
||||
|
||||
bcm53xxspi_buf_read(b53spi, buf, to_read, cont);
|
||||
left -= to_read;
|
||||
@ -1,59 +0,0 @@ |
||||
From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <zajec5@gmail.com>
|
||||
Subject: [PATCH] mtd: m25p80: use single SPI message for writing data
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
On all 3 tested Northstar devices with following flash memories:
|
||||
mx25l6405d (8192 Kbytes)
|
||||
mx25l12805d (16384 Kbytes)
|
||||
mx25l25635e (32768 Kbytes)
|
||||
I noticed writing to be broken. Not a single bit was changed leaving all
|
||||
bytes set to 0xff.
|
||||
|
||||
This is most likely some problem related to the SPI controller or its
|
||||
driver. Using a single SPI message seems to workaround this. Of course
|
||||
it's not perfect solution as copying whole data into a new buffer makes
|
||||
writing slower.
|
||||
|
||||
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
|
||||
---
|
||||
|
||||
--- a/drivers/mtd/devices/m25p80.c
|
||||
+++ b/drivers/mtd/devices/m25p80.c
|
||||
@@ -78,6 +78,7 @@ static void m25p80_write(struct spi_nor
|
||||
{
|
||||
struct m25p *flash = nor->priv;
|
||||
struct spi_device *spi = flash->spi;
|
||||
+ u8 *command = kzalloc(MAX_CMD_SIZE + len, GFP_KERNEL);
|
||||
struct spi_transfer t[2] = {};
|
||||
struct spi_message m;
|
||||
int cmd_sz = m25p_cmdsz(nor);
|
||||
@@ -87,20 +88,19 @@ static void m25p80_write(struct spi_nor
|
||||
if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
|
||||
cmd_sz = 1;
|
||||
|
||||
- flash->command[0] = nor->program_opcode;
|
||||
- m25p_addr2cmd(nor, to, flash->command);
|
||||
+ command[0] = nor->program_opcode;
|
||||
+ m25p_addr2cmd(nor, to, command);
|
||||
+ memcpy(&command[cmd_sz], buf, len);
|
||||
|
||||
- t[0].tx_buf = flash->command;
|
||||
- t[0].len = cmd_sz;
|
||||
+ t[0].tx_buf = command;
|
||||
+ t[0].len = cmd_sz + len;
|
||||
spi_message_add_tail(&t[0], &m);
|
||||
|
||||
- t[1].tx_buf = buf;
|
||||
- t[1].len = len;
|
||||
- spi_message_add_tail(&t[1], &m);
|
||||
-
|
||||
spi_sync(spi, &m);
|
||||
|
||||
*retlen += m.actual_length - cmd_sz;
|
||||
+
|
||||
+ kfree(command);
|
||||
}
|
||||
|
||||
static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor)
|
||||
@ -0,0 +1,107 @@ |
||||
From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <rafal@milecki.pl>
|
||||
Date: Fri, 29 Dec 2017 14:44:09 +0100
|
||||
Subject: [PATCH] spi: bcm53xx: simplify reading SPI data
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
This commit makes transfer function use spi_transfer_is_last to
|
||||
determine if currently processed transfer is the last one. Thanks to
|
||||
that we finally set hardware registers properly and it makes controller
|
||||
behave the way it's expected to.
|
||||
|
||||
This allows simplifying read function which can now simply start reading
|
||||
from the slot 0 instead of the last saved offset. It has been
|
||||
successfully tested using spi_write_then_read.
|
||||
|
||||
Moreover this change fixes handling messages with two writing transfers.
|
||||
It's important for SPI flash devices as their drivers commonly use one
|
||||
transfer for a command and another one for data.
|
||||
|
||||
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
|
||||
---
|
||||
drivers/spi/spi-bcm53xx.c | 26 ++++++++++----------------
|
||||
1 file changed, 10 insertions(+), 16 deletions(-)
|
||||
|
||||
--- a/drivers/spi/spi-bcm53xx.c
|
||||
+++ b/drivers/spi/spi-bcm53xx.c
|
||||
@@ -27,8 +27,6 @@ struct bcm53xxspi {
|
||||
struct bcma_device *core;
|
||||
struct spi_master *master;
|
||||
void __iomem *mmio_base;
|
||||
-
|
||||
- size_t read_offset;
|
||||
bool bspi; /* Boot SPI mode with memory mapping */
|
||||
};
|
||||
|
||||
@@ -172,8 +170,6 @@ static void bcm53xxspi_buf_write(struct
|
||||
|
||||
if (!cont)
|
||||
bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 0);
|
||||
-
|
||||
- b53spi->read_offset = len;
|
||||
}
|
||||
|
||||
static void bcm53xxspi_buf_read(struct bcm53xxspi *b53spi, u8 *r_buf,
|
||||
@@ -182,10 +178,10 @@ static void bcm53xxspi_buf_read(struct b
|
||||
u32 tmp;
|
||||
int i;
|
||||
|
||||
- for (i = 0; i < b53spi->read_offset + len; i++) {
|
||||
+ for (i = 0; i < len; i++) {
|
||||
tmp = B53SPI_CDRAM_CONT | B53SPI_CDRAM_PCS_DISABLE_ALL |
|
||||
B53SPI_CDRAM_PCS_DSCK;
|
||||
- if (!cont && i == b53spi->read_offset + len - 1)
|
||||
+ if (!cont && i == len - 1)
|
||||
tmp &= ~B53SPI_CDRAM_CONT;
|
||||
tmp &= ~0x1;
|
||||
/* Command Register File */
|
||||
@@ -194,8 +190,7 @@ static void bcm53xxspi_buf_read(struct b
|
||||
|
||||
/* Set queue pointers */
|
||||
bcm53xxspi_write(b53spi, B53SPI_MSPI_NEWQP, 0);
|
||||
- bcm53xxspi_write(b53spi, B53SPI_MSPI_ENDQP,
|
||||
- b53spi->read_offset + len - 1);
|
||||
+ bcm53xxspi_write(b53spi, B53SPI_MSPI_ENDQP, len - 1);
|
||||
|
||||
if (cont)
|
||||
bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 1);
|
||||
@@ -214,13 +209,11 @@ static void bcm53xxspi_buf_read(struct b
|
||||
bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 0);
|
||||
|
||||
for (i = 0; i < len; ++i) {
|
||||
- int offset = b53spi->read_offset + i;
|
||||
+ u16 reg = B53SPI_MSPI_RXRAM + 4 * (1 + i * 2);
|
||||
|
||||
/* Data stored in the transmit register file LSB */
|
||||
- r_buf[i] = (u8)bcm53xxspi_read(b53spi, B53SPI_MSPI_RXRAM + 4 * (1 + offset * 2));
|
||||
+ r_buf[i] = (u8)bcm53xxspi_read(b53spi, reg);
|
||||
}
|
||||
-
|
||||
- b53spi->read_offset = 0;
|
||||
}
|
||||
|
||||
static int bcm53xxspi_transfer_one(struct spi_master *master,
|
||||
@@ -238,7 +231,8 @@ static int bcm53xxspi_transfer_one(struc
|
||||
left = t->len;
|
||||
while (left) {
|
||||
size_t to_write = min_t(size_t, 16, left);
|
||||
- bool cont = left - to_write > 0;
|
||||
+ bool cont = !spi_transfer_is_last(master, t) ||
|
||||
+ left - to_write > 0;
|
||||
|
||||
bcm53xxspi_buf_write(b53spi, buf, to_write, cont);
|
||||
left -= to_write;
|
||||
@@ -250,9 +244,9 @@ static int bcm53xxspi_transfer_one(struc
|
||||
buf = (u8 *)t->rx_buf;
|
||||
left = t->len;
|
||||
while (left) {
|
||||
- size_t to_read = min_t(size_t, 16 - b53spi->read_offset,
|
||||
- left);
|
||||
- bool cont = left - to_read > 0;
|
||||
+ size_t to_read = min_t(size_t, 16, left);
|
||||
+ bool cont = !spi_transfer_is_last(master, t) ||
|
||||
+ left - to_read > 0;
|
||||
|
||||
bcm53xxspi_buf_read(b53spi, buf, to_read, cont);
|
||||
left -= to_read;
|
||||
@ -1,68 +0,0 @@ |
||||
From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <zajec5@gmail.com>
|
||||
Subject: [PATCH] mtd: m25p80: use single SPI message for writing data
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
On all 3 tested Northstar devices with following flash memories:
|
||||
mx25l6405d (8192 Kbytes)
|
||||
mx25l12805d (16384 Kbytes)
|
||||
mx25l25635e (32768 Kbytes)
|
||||
I noticed writing to be broken. Not a single bit was changed leaving all
|
||||
bytes set to 0xff.
|
||||
|
||||
This is most likely some problem related to the SPI controller or its
|
||||
driver. Using a single SPI message seems to workaround this. Of course
|
||||
it's not perfect solution as copying whole data into a new buffer makes
|
||||
writing slower.
|
||||
|
||||
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
|
||||
---
|
||||
|
||||
--- a/drivers/mtd/devices/m25p80.c
|
||||
+++ b/drivers/mtd/devices/m25p80.c
|
||||
@@ -78,6 +78,7 @@ static ssize_t m25p80_write(struct spi_n
|
||||
{
|
||||
struct m25p *flash = nor->priv;
|
||||
struct spi_device *spi = flash->spi;
|
||||
+ u8 *command = kzalloc(MAX_CMD_SIZE + len, GFP_KERNEL);
|
||||
struct spi_transfer t[2] = {};
|
||||
struct spi_message m;
|
||||
int cmd_sz = m25p_cmdsz(nor);
|
||||
@@ -88,24 +89,26 @@ static ssize_t m25p80_write(struct spi_n
|
||||
if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
|
||||
cmd_sz = 1;
|
||||
|
||||
- flash->command[0] = nor->program_opcode;
|
||||
- m25p_addr2cmd(nor, to, flash->command);
|
||||
+ command[0] = nor->program_opcode;
|
||||
+ m25p_addr2cmd(nor, to, command);
|
||||
+ memcpy(&command[cmd_sz], buf, len);
|
||||
|
||||
- t[0].tx_buf = flash->command;
|
||||
- t[0].len = cmd_sz;
|
||||
+ t[0].tx_buf = command;
|
||||
+ t[0].len = cmd_sz + len;
|
||||
spi_message_add_tail(&t[0], &m);
|
||||
|
||||
- t[1].tx_buf = buf;
|
||||
- t[1].len = len;
|
||||
- spi_message_add_tail(&t[1], &m);
|
||||
-
|
||||
ret = spi_sync(spi, &m);
|
||||
- if (ret)
|
||||
+ if (ret) {
|
||||
+ kfree(command);
|
||||
return ret;
|
||||
+ }
|
||||
|
||||
ret = m.actual_length - cmd_sz;
|
||||
- if (ret < 0)
|
||||
+ if (ret < 0) {
|
||||
+ kfree(command);
|
||||
return -EIO;
|
||||
+ }
|
||||
+ kfree(command);
|
||||
return ret;
|
||||
}
|
||||
|
||||
Loading…
Reference in new issue