| Message ID | 1401799074-3801-2-git-send-email-akshay.s@samsung.com |
|---|---|
| State | Superseded |
| Delegated to: | Minkyu Kang |
| Headers | show |
HI Akshay, On 3 June 2014 06:37, Akshay Saraswat <akshay.s@samsung.com> wrote: > SPI recieve and transfer code in exynos_spi driver has a logical bug. > We read data in a variable which can hold an integer. Then we assign > this integer 32 bit value to another variable which has data type uchar. > Latter represents a unit of our recieve buffer. Everytime when we write > a value to our recieve buffer we step ahead by 4 units when actually we > wrote to one unit. This results in the loss of 3 bytes out of every 4 > bytes recieved. This patch intends to fix this bug. > > Signed-off-by: Akshay Saraswat <akshay.s@samsung.com> Acked-by: Simon Glass <sjg@chromium.org> (see comment below - could update this patch or do a follow-on to improve speed of both tx and rx) > --- > Changes since v1: > - Added check for step. > > drivers/spi/exynos_spi.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c > index 4d5def2..68d1206 100644 > --- a/drivers/spi/exynos_spi.c > +++ b/drivers/spi/exynos_spi.c > @@ -302,6 +302,11 @@ static int spi_rx_tx(struct exynos_spi_slave *spi_slave, int todo, > } > } else { > if (rxp || stopping) { > + if (step == 4) { > + *(rxp + 3) = temp >> 24; > + *(rxp + 2) = temp >> 16; > + *(rxp + 1) = temp >> 8; > + } BTW I just had another look at the code - sorry I didn't notice this before, but you know that rxp is word-aligned so you can just do *((uint32_t *)rxp) = temp; > *rxp = temp; > rxp += step; > } > -- > 1.7.12.4 >
On 3 June 2014 08:23, Simon Glass <sjg@chromium.org> wrote: > HI Akshay, > > On 3 June 2014 06:37, Akshay Saraswat <akshay.s@samsung.com> wrote: >> SPI recieve and transfer code in exynos_spi driver has a logical bug. >> We read data in a variable which can hold an integer. Then we assign >> this integer 32 bit value to another variable which has data type uchar. >> Latter represents a unit of our recieve buffer. Everytime when we write >> a value to our recieve buffer we step ahead by 4 units when actually we >> wrote to one unit. This results in the loss of 3 bytes out of every 4 >> bytes recieved. This patch intends to fix this bug. >> >> Signed-off-by: Akshay Saraswat <akshay.s@samsung.com> > > Acked-by: Simon Glass <sjg@chromium.org> Tested on pit using saveenv Tested-by: Simon Glass <sjg@chromium.org>
diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c index 4d5def2..68d1206 100644 --- a/drivers/spi/exynos_spi.c +++ b/drivers/spi/exynos_spi.c @@ -302,6 +302,11 @@ static int spi_rx_tx(struct exynos_spi_slave *spi_slave, int todo, } } else { if (rxp || stopping) { + if (step == 4) { + *(rxp + 3) = temp >> 24; + *(rxp + 2) = temp >> 16; + *(rxp + 1) = temp >> 8; + } *rxp = temp; rxp += step; }
SPI recieve and transfer code in exynos_spi driver has a logical bug. We read data in a variable which can hold an integer. Then we assign this integer 32 bit value to another variable which has data type uchar. Latter represents a unit of our recieve buffer. Everytime when we write a value to our recieve buffer we step ahead by 4 units when actually we wrote to one unit. This results in the loss of 3 bytes out of every 4 bytes recieved. This patch intends to fix this bug. Signed-off-by: Akshay Saraswat <akshay.s@samsung.com> --- Changes since v1: - Added check for step. drivers/spi/exynos_spi.c | 5 +++++ 1 file changed, 5 insertions(+)