[U-Boot] [PATCH v2 01/12] spi/ich.c: Fix a bug of reading from a non-64 bytes aligned address
The ich spi controller driver spi_xfer() tries to align reading address to 64 bytes when doing spi data in, which causes a bug of either infinite loop or a huge size memcpy().
Actually the ich spi controller does not have such requirement of 64 bytes alignment when reading data from spi slave devices.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org --- drivers/spi/ich.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c index f5c6f3e..c4d3a29 100644 --- a/drivers/spi/ich.c +++ b/drivers/spi/ich.c @@ -483,8 +483,6 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, struct spi_trans *trans = &ich->trans; unsigned type = flags & (SPI_XFER_BEGIN | SPI_XFER_END); int using_cmd = 0; - /* Align read transactions to 64-byte boundaries */ - char buff[ctlr.databytes];
/* Ee don't support writing partial bytes. */ if (bitlen % 8) { @@ -632,14 +630,9 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, */ while (trans->bytesout || trans->bytesin) { uint32_t data_length; - uint32_t aligned_offset; - uint32_t diff; - - aligned_offset = trans->offset & ~(ctlr.databytes - 1); - diff = trans->offset - aligned_offset;
/* SPI addresses are 24 bit only */ - ich_writel(aligned_offset & 0x00FFFFFF, ctlr.addr); + ich_writel(trans->offset & 0x00FFFFFF, ctlr.addr);
if (trans->bytesout) data_length = min(trans->bytesout, ctlr.databytes); @@ -673,13 +666,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, }
if (trans->bytesin) { - if (diff) { - data_length -= diff; - read_reg(ctlr.data, buff, ctlr.databytes); - memcpy(trans->in, buff + diff, data_length); - } else { - read_reg(ctlr.data, trans->in, data_length); - } + read_reg(ctlr.data, trans->in, data_length); spi_use_in(trans, data_length); if (with_address) trans->offset += data_length;
Hi,
On 1 November 2014 at 02:53, Bin Meng bmeng.cn@gmail.com wrote:
The ich spi controller driver spi_xfer() tries to align reading address to 64 bytes when doing spi data in, which causes a bug of either infinite loop or a huge size memcpy().
Actually the ich spi controller does not have such requirement of 64 bytes alignment when reading data from spi slave devices.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
drivers/spi/ich.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
Any update on this series?
If the series is acceptable I could bring it in through the x86 tree since it relates to that SPI controller.
Regards, Simon
Hi Simon
On Tue, Nov 25, 2014 at 11:10 AM, Simon Glass sjg@chromium.org wrote:
Hi,
On 1 November 2014 at 02:53, Bin Meng bmeng.cn@gmail.com wrote:
The ich spi controller driver spi_xfer() tries to align reading address to 64 bytes when doing spi data in, which causes a bug of either infinite loop or a huge size memcpy().
Actually the ich spi controller does not have such requirement of 64 bytes alignment when reading data from spi slave devices.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
drivers/spi/ich.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
Any update on this series?
If the series is acceptable I could bring it in through the x86 tree since it relates to that SPI controller.
Thanks for tracking this series. I was asking the same question to Jagan yesterday.
Regards, Bin
Hi Jagan,
On 24 November 2014 at 20:13, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon
On Tue, Nov 25, 2014 at 11:10 AM, Simon Glass sjg@chromium.org wrote:
Hi,
On 1 November 2014 at 02:53, Bin Meng bmeng.cn@gmail.com wrote:
The ich spi controller driver spi_xfer() tries to align reading address to 64 bytes when doing spi data in, which causes a bug of either infinite loop or a huge size memcpy().
Actually the ich spi controller does not have such requirement of 64 bytes alignment when reading data from spi slave devices.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
drivers/spi/ich.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
Any update on this series?
If the series is acceptable I could bring it in through the x86 tree since it relates to that SPI controller.
Thanks for tracking this series. I was asking the same question to Jagan yesterday.
Let me know if you want to pull this through the SPI tree, otherwise I'll go ahead (iwc I'll need to test once more first).
Regards, Simon
On 26 November 2014 at 21:33, Simon Glass sjg@chromium.org wrote:
Hi Jagan,
On 24 November 2014 at 20:13, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon
On Tue, Nov 25, 2014 at 11:10 AM, Simon Glass sjg@chromium.org wrote:
Hi,
On 1 November 2014 at 02:53, Bin Meng bmeng.cn@gmail.com wrote:
The ich spi controller driver spi_xfer() tries to align reading address to 64 bytes when doing spi data in, which causes a bug of either infinite loop or a huge size memcpy().
Actually the ich spi controller does not have such requirement of 64 bytes alignment when reading data from spi slave devices.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
drivers/spi/ich.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
Any update on this series?
If the series is acceptable I could bring it in through the x86 tree since it relates to that SPI controller.
Thanks for tracking this series. I was asking the same question to Jagan yesterday.
Let me know if you want to pull this through the SPI tree, otherwise I'll go ahead (iwc I'll need to test once more first).
This series change the sf generic code stuff, I will send my review comments. Please hold on this, once everything goes fine I will pick.
thanks!
Hi Jagan,
On 26 November 2014 at 09:43, Jagan Teki jagannadh.teki@gmail.com wrote:
On 26 November 2014 at 21:33, Simon Glass sjg@chromium.org wrote:
Hi Jagan,
On 24 November 2014 at 20:13, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon
On Tue, Nov 25, 2014 at 11:10 AM, Simon Glass sjg@chromium.org wrote:
Hi,
On 1 November 2014 at 02:53, Bin Meng bmeng.cn@gmail.com wrote:
The ich spi controller driver spi_xfer() tries to align reading address to 64 bytes when doing spi data in, which causes a bug of either infinite loop or a huge size memcpy().
Actually the ich spi controller does not have such requirement of 64 bytes alignment when reading data from spi slave devices.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
drivers/spi/ich.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
Any update on this series?
If the series is acceptable I could bring it in through the x86 tree since it relates to that SPI controller.
Thanks for tracking this series. I was asking the same question to Jagan yesterday.
Let me know if you want to pull this through the SPI tree, otherwise I'll go ahead (iwc I'll need to test once more first).
This series change the sf generic code stuff, I will send my review comments. Please hold on this, once everything goes fine I will pick.
Sounds good, thanks. Let me know if you want me to test again as a lot has changed since this series was sent.
Regards, Simon
participants (3)
-
Bin Meng -
Jagan Teki -
Simon Glass