| Message ID | 1414832010-13353-1-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
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
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;