[PATCH] spi: dw: Fix broken dw_spi_mem_ops()

From: Niklas Cassel niklas.cassel@wdc.com
The driver is currently using sizeof(op->cmd.opcode) in the op_len calculation. Commit d15de623013c ("spi: spi-mem: allow specifying a command's extension") changed op->cmd.opcode from one byte to two.
Instead, a new struct member op->cmd.nbytes is supposed to be used. For regular commands op->cmd.nbytes will be one.
Commit d15de623013c ("spi: spi-mem: allow specifying a command's extension") did update some drivers that overload the generic mem_ops() implementation, but forgot to update dw_spi_mem_ops().
Calculating op_len incorrectly causes dw_spi_mem_ops() to misbehave, since op_len is used to determine how many bytes that should be read/written.
On the canaan k210 board, this causes the probe of the SPI flash to fail.
Fix the op_len calculation in dw_spi_mem_ops(). Doing so results in working SPI flash on the canaan k210 board.
Fixes: d15de623013c ("spi: spi-mem: allow specifying a command's extension") Signed-off-by: Niklas Cassel niklas.cassel@wdc.com --- drivers/spi/designware_spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index 742121140d..fc22f540fe 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -572,7 +572,7 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) int pos, i, ret = 0; struct udevice *bus = slave->dev->parent; struct dw_spi_priv *priv = dev_get_priv(bus); - u8 op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes; + u8 op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes; u8 op_buf[op_len]; u32 cr0;

On 08/02/22 10:52PM, Niklas Cassel wrote:
From: Niklas Cassel niklas.cassel@wdc.com
The driver is currently using sizeof(op->cmd.opcode) in the op_len calculation. Commit d15de623013c ("spi: spi-mem: allow specifying a command's extension") changed op->cmd.opcode from one byte to two.
Instead, a new struct member op->cmd.nbytes is supposed to be used. For regular commands op->cmd.nbytes will be one.
Commit d15de623013c ("spi: spi-mem: allow specifying a command's extension") did update some drivers that overload the generic mem_ops() implementation, but forgot to update dw_spi_mem_ops().
Calculating op_len incorrectly causes dw_spi_mem_ops() to misbehave, since op_len is used to determine how many bytes that should be read/written.
On the canaan k210 board, this causes the probe of the SPI flash to fail.
Fix the op_len calculation in dw_spi_mem_ops(). Doing so results in working SPI flash on the canaan k210 board.
Fixes: d15de623013c ("spi: spi-mem: allow specifying a command's extension") Signed-off-by: Niklas Cassel niklas.cassel@wdc.com
Reviewed-by: Pratyush Yadav p.yadav@ti.com

On 2/9/22 07:52, Niklas Cassel wrote:
From: Niklas Cassel niklas.cassel@wdc.com
The driver is currently using sizeof(op->cmd.opcode) in the op_len calculation. Commit d15de623013c ("spi: spi-mem: allow specifying a command's extension") changed op->cmd.opcode from one byte to two.
Instead, a new struct member op->cmd.nbytes is supposed to be used. For regular commands op->cmd.nbytes will be one.
Commit d15de623013c ("spi: spi-mem: allow specifying a command's extension") did update some drivers that overload the generic mem_ops() implementation, but forgot to update dw_spi_mem_ops().
Calculating op_len incorrectly causes dw_spi_mem_ops() to misbehave, since op_len is used to determine how many bytes that should be read/written.
On the canaan k210 board, this causes the probe of the SPI flash to fail.
Fix the op_len calculation in dw_spi_mem_ops(). Doing so results in working SPI flash on the canaan k210 board.
Fixes: d15de623013c ("spi: spi-mem: allow specifying a command's extension") Signed-off-by: Niklas Cassel niklas.cassel@wdc.com
drivers/spi/designware_spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index 742121140d..fc22f540fe 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -572,7 +572,7 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) int pos, i, ret = 0; struct udevice *bus = slave->dev->parent; struct dw_spi_priv *priv = dev_get_priv(bus);
- u8 op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
- u8 op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes; u8 op_buf[op_len]; u32 cr0;
Tested-by: Damien Le Moal damien.lemoal@opensource.wdc.com

On Wed, Feb 9, 2022 at 4:22 AM Niklas Cassel Niklas.Cassel@wdc.com wrote:
From: Niklas Cassel niklas.cassel@wdc.com
The driver is currently using sizeof(op->cmd.opcode) in the op_len calculation. Commit d15de623013c ("spi: spi-mem: allow specifying a command's extension") changed op->cmd.opcode from one byte to two.
Instead, a new struct member op->cmd.nbytes is supposed to be used. For regular commands op->cmd.nbytes will be one.
Commit d15de623013c ("spi: spi-mem: allow specifying a command's extension") did update some drivers that overload the generic mem_ops() implementation, but forgot to update dw_spi_mem_ops().
Calculating op_len incorrectly causes dw_spi_mem_ops() to misbehave, since op_len is used to determine how many bytes that should be read/written.
On the canaan k210 board, this causes the probe of the SPI flash to fail.
Fix the op_len calculation in dw_spi_mem_ops(). Doing so results in working SPI flash on the canaan k210 board.
Fixes: d15de623013c ("spi: spi-mem: allow specifying a command's extension") Signed-off-by: Niklas Cassel niklas.cassel@wdc.com
Applied to u-boot-spi/master

On Thu, Feb 24, 2022 at 01:41:38PM +0530, Jagan Teki wrote:
On Wed, Feb 9, 2022 at 4:22 AM Niklas Cassel Niklas.Cassel@wdc.com wrote:
From: Niklas Cassel niklas.cassel@wdc.com
The driver is currently using sizeof(op->cmd.opcode) in the op_len calculation. Commit d15de623013c ("spi: spi-mem: allow specifying a command's extension") changed op->cmd.opcode from one byte to two.
Instead, a new struct member op->cmd.nbytes is supposed to be used. For regular commands op->cmd.nbytes will be one.
Commit d15de623013c ("spi: spi-mem: allow specifying a command's extension") did update some drivers that overload the generic mem_ops() implementation, but forgot to update dw_spi_mem_ops().
Calculating op_len incorrectly causes dw_spi_mem_ops() to misbehave, since op_len is used to determine how many bytes that should be read/written.
On the canaan k210 board, this causes the probe of the SPI flash to fail.
Fix the op_len calculation in dw_spi_mem_ops(). Doing so results in working SPI flash on the canaan k210 board.
Fixes: d15de623013c ("spi: spi-mem: allow specifying a command's extension") Signed-off-by: Niklas Cassel niklas.cassel@wdc.com
Applied to u-boot-spi/master
Hello Jagan,
I still don't see this in git://git.denx.de/u-boot-spi.git master branch.
I can see that we are in v2022.04-rc3.
Any chance that this will make it into v2022.04 final?
Kind regards, Niklas
participants (4)
-
Damien Le Moal
-
Jagan Teki
-
Niklas Cassel
-
Pratyush Yadav