| Message ID | 20131015175715.457.29765.stgit@localhost |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
Hi, On Tue, Oct 15, 2013 at 11:27 PM, Martin Fuzzey <mfuzzey@parkeon.com> wrote: > When writing buffers that are not 32 bit aligned data loss occurs. > > This can also occur when the total transfer size is not a multiple of 4 bytes > since the extra bytes are written first causing the rest to be unaligned. > > This can be seen by writing to SPI flash a 57 byte file containing 00 01 .. 38: > > U-Boot > dhcp data.bin > ... > Bytes transferred = 57 (39 hex) > U-Boot > md.b $loadaddr > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ > 72000020: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f !"#$%&'()*+,-./ > 72000030: 30 31 32 33 34 35 36 37 38 ff ff ff ff ff ff ff 012345678....... > U-Boot > sf write $loadaddr 0 $filesize > > U-Boot > mw.b $loadaddr ff 100 > U-Boot > sf read $loadaddr 0 100 > U-Boot > md.b $loadaddr > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ > 72000020: 20 21 22 23 24 29 2a 2b 2c 31 32 33 34 ff ff ff !"#$)*+,1234... > 72000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > > [bytes 25-28, 2d-30, 35-38 are missing] > > Activating debug in the write command gave: > spi_xchg_single: bitlen 256 dout 0x72000000 din 0x0 > Sending SPI 0x10203 > Sending SPI 0x4050607 > Sending SPI 0x8090a0b > Sending SPI 0xc0d0e0f > Sending SPI 0x10111213 > Sending SPI 0x14151617 > Sending SPI 0x18191a1b > Sending SPI 0x1c1d1e1f > SPI Rx: 0xffffffff 0xffffffff > ... > SPI Rx: 0xffffffff 0xffffffff > spi_xchg_single: bitlen 200 dout 0x72000020 din 0x0 > Sending SPI 0x20 > Sending SPI 0x21222324 > Sending SPI 0x292a2b2c <=== What happened to 25262728? > Sending SPI 0x31323334 > Sending SPI 0xffffffff > ... > SF: program success 57 bytes @ 0x0 Thanks for your log, for showing the exact issues. > > > The problem was that the pointer was being incremented twice for the non aligned case. > > Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com> > --- > drivers/spi/mxc_spi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c > index 859c43f..964a2b7 100644 > --- a/drivers/spi/mxc_spi.c > +++ b/drivers/spi/mxc_spi.c > @@ -274,8 +274,8 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen, > } else { > data = *(u32 *)dout; > data = cpu_to_be32(data); > + dout += 4; > } > - dout += 4; > } > debug("Sending SPI 0x%x\n", data); > reg_write(®s->txdata, data); > This patch pushed on master, please check it on http://git.denx.de/?p=u-boot.git;a=commitdiff;h=6d5ce1bd0048617d48c05de1a84fae8696081127
Hi Jagan, On Wed, 16 Oct 2013 11:16:49 +0530, Jagan Teki <jagannadh.teki@gmail.com> wrote: > Hi, > > On Tue, Oct 15, 2013 at 11:27 PM, Martin Fuzzey <mfuzzey@parkeon.com> wrote: > > When writing buffers that are not 32 bit aligned data loss occurs. > > > > This can also occur when the total transfer size is not a multiple of 4 bytes > > since the extra bytes are written first causing the rest to be unaligned. > > > > This can be seen by writing to SPI flash a 57 byte file containing 00 01 .. 38: > > > > U-Boot > dhcp data.bin > > ... > > Bytes transferred = 57 (39 hex) > > U-Boot > md.b $loadaddr > > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ > > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ > > 72000020: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f !"#$%&'()*+,-./ > > 72000030: 30 31 32 33 34 35 36 37 38 ff ff ff ff ff ff ff 012345678....... > > U-Boot > sf write $loadaddr 0 $filesize > > > > U-Boot > mw.b $loadaddr ff 100 > > U-Boot > sf read $loadaddr 0 100 > > U-Boot > md.b $loadaddr > > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ > > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ > > 72000020: 20 21 22 23 24 29 2a 2b 2c 31 32 33 34 ff ff ff !"#$)*+,1234... > > 72000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > > > > [bytes 25-28, 2d-30, 35-38 are missing] > > > > Activating debug in the write command gave: > > spi_xchg_single: bitlen 256 dout 0x72000000 din 0x0 > > Sending SPI 0x10203 > > Sending SPI 0x4050607 > > Sending SPI 0x8090a0b > > Sending SPI 0xc0d0e0f > > Sending SPI 0x10111213 > > Sending SPI 0x14151617 > > Sending SPI 0x18191a1b > > Sending SPI 0x1c1d1e1f > > SPI Rx: 0xffffffff 0xffffffff > > ... > > SPI Rx: 0xffffffff 0xffffffff > > spi_xchg_single: bitlen 200 dout 0x72000020 din 0x0 > > Sending SPI 0x20 > > Sending SPI 0x21222324 > > Sending SPI 0x292a2b2c <=== What happened to 25262728? > > Sending SPI 0x31323334 > > Sending SPI 0xffffffff > > ... > > SF: program success 57 bytes @ 0x0 > Thanks for your log, for showing the exact issues. Agreed, but I think this log could go in a comment below the '---' line, so that the commit message focuses on describing what the patch does, rather than how it was found out necessary. Amicalement,
Hi Amicalement, On Wed, Oct 16, 2013 at 1:05 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Jagan, > > On Wed, 16 Oct 2013 11:16:49 +0530, Jagan Teki > <jagannadh.teki@gmail.com> wrote: > >> Hi, >> >> On Tue, Oct 15, 2013 at 11:27 PM, Martin Fuzzey <mfuzzey@parkeon.com> wrote: >> > When writing buffers that are not 32 bit aligned data loss occurs. >> > >> > This can also occur when the total transfer size is not a multiple of 4 bytes >> > since the extra bytes are written first causing the rest to be unaligned. >> > >> > This can be seen by writing to SPI flash a 57 byte file containing 00 01 .. 38: >> > >> > U-Boot > dhcp data.bin >> > ... >> > Bytes transferred = 57 (39 hex) >> > U-Boot > md.b $loadaddr >> > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ >> > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ >> > 72000020: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f !"#$%&'()*+,-./ >> > 72000030: 30 31 32 33 34 35 36 37 38 ff ff ff ff ff ff ff 012345678....... >> > U-Boot > sf write $loadaddr 0 $filesize >> > >> > U-Boot > mw.b $loadaddr ff 100 >> > U-Boot > sf read $loadaddr 0 100 >> > U-Boot > md.b $loadaddr >> > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ >> > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ >> > 72000020: 20 21 22 23 24 29 2a 2b 2c 31 32 33 34 ff ff ff !"#$)*+,1234... >> > 72000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ >> > >> > [bytes 25-28, 2d-30, 35-38 are missing] >> > >> > Activating debug in the write command gave: >> > spi_xchg_single: bitlen 256 dout 0x72000000 din 0x0 >> > Sending SPI 0x10203 >> > Sending SPI 0x4050607 >> > Sending SPI 0x8090a0b >> > Sending SPI 0xc0d0e0f >> > Sending SPI 0x10111213 >> > Sending SPI 0x14151617 >> > Sending SPI 0x18191a1b >> > Sending SPI 0x1c1d1e1f >> > SPI Rx: 0xffffffff 0xffffffff >> > ... >> > SPI Rx: 0xffffffff 0xffffffff >> > spi_xchg_single: bitlen 200 dout 0x72000020 din 0x0 >> > Sending SPI 0x20 >> > Sending SPI 0x21222324 >> > Sending SPI 0x292a2b2c <=== What happened to 25262728? >> > Sending SPI 0x31323334 >> > Sending SPI 0xffffffff >> > ... >> > SF: program success 57 bytes @ 0x0 >> Thanks for your log, for showing the exact issues. > > Agreed, but I think this log could go in a comment below the '---' > line, so that the commit message focuses on describing what the patch > does, rather than how it was found out necessary. I too thought the same, but the commit is already in master, any possibility to alter?
Hi Jagan, On Wed, 16 Oct 2013 16:20:17 +0530, Jagan Teki <jagannadh.teki@gmail.com> wrote: > Hi Amicalement, > > On Wed, Oct 16, 2013 at 1:05 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: > > Hi Jagan, > > > > On Wed, 16 Oct 2013 11:16:49 +0530, Jagan Teki > > <jagannadh.teki@gmail.com> wrote: > > > >> Hi, > >> > >> On Tue, Oct 15, 2013 at 11:27 PM, Martin Fuzzey <mfuzzey@parkeon.com> wrote: > >> > When writing buffers that are not 32 bit aligned data loss occurs. > >> > > >> > This can also occur when the total transfer size is not a multiple of 4 bytes > >> > since the extra bytes are written first causing the rest to be unaligned. > >> > > >> > This can be seen by writing to SPI flash a 57 byte file containing 00 01 .. 38: > >> > > >> > U-Boot > dhcp data.bin > >> > ... > >> > Bytes transferred = 57 (39 hex) > >> > U-Boot > md.b $loadaddr > >> > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ > >> > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ > >> > 72000020: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f !"#$%&'()*+,-./ > >> > 72000030: 30 31 32 33 34 35 36 37 38 ff ff ff ff ff ff ff 012345678....... > >> > U-Boot > sf write $loadaddr 0 $filesize > >> > > >> > U-Boot > mw.b $loadaddr ff 100 > >> > U-Boot > sf read $loadaddr 0 100 > >> > U-Boot > md.b $loadaddr > >> > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ > >> > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ > >> > 72000020: 20 21 22 23 24 29 2a 2b 2c 31 32 33 34 ff ff ff !"#$)*+,1234... > >> > 72000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > >> > > >> > [bytes 25-28, 2d-30, 35-38 are missing] > >> > > >> > Activating debug in the write command gave: > >> > spi_xchg_single: bitlen 256 dout 0x72000000 din 0x0 > >> > Sending SPI 0x10203 > >> > Sending SPI 0x4050607 > >> > Sending SPI 0x8090a0b > >> > Sending SPI 0xc0d0e0f > >> > Sending SPI 0x10111213 > >> > Sending SPI 0x14151617 > >> > Sending SPI 0x18191a1b > >> > Sending SPI 0x1c1d1e1f > >> > SPI Rx: 0xffffffff 0xffffffff > >> > ... > >> > SPI Rx: 0xffffffff 0xffffffff > >> > spi_xchg_single: bitlen 200 dout 0x72000020 din 0x0 > >> > Sending SPI 0x20 > >> > Sending SPI 0x21222324 > >> > Sending SPI 0x292a2b2c <=== What happened to 25262728? > >> > Sending SPI 0x31323334 > >> > Sending SPI 0xffffffff > >> > ... > >> > SF: program success 57 bytes @ 0x0 > >> Thanks for your log, for showing the exact issues. > > > > Agreed, but I think this log could go in a comment below the '---' > > line, so that the commit message focuses on describing what the patch > > does, rather than how it was found out necessary. > > I too thought the same, but the commit is already in master, any > possibility to alter? Er... Martin's patch is already in patchwork as 283794, but its state is still "New" with no delegate, and I don't see it in any of the repos I usually read, nor does it appear on u-boot-spi. Where do you see it "in master" exactly? Besides, it is less than 24 hours old; this is not enough time for review, and therefore, it should not be accepted yet. Amicalement,
Hi Albert, On Wed, Oct 16, 2013 at 5:35 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Jagan, > > On Wed, 16 Oct 2013 16:20:17 +0530, Jagan Teki > <jagannadh.teki@gmail.com> wrote: > >> Hi Amicalement, >> >> On Wed, Oct 16, 2013 at 1:05 PM, Albert ARIBAUD >> <albert.u.boot@aribaud.net> wrote: >> > Hi Jagan, >> > >> > On Wed, 16 Oct 2013 11:16:49 +0530, Jagan Teki >> > <jagannadh.teki@gmail.com> wrote: >> > >> >> Hi, >> >> >> >> On Tue, Oct 15, 2013 at 11:27 PM, Martin Fuzzey <mfuzzey@parkeon.com> wrote: >> >> > When writing buffers that are not 32 bit aligned data loss occurs. >> >> > >> >> > This can also occur when the total transfer size is not a multiple of 4 bytes >> >> > since the extra bytes are written first causing the rest to be unaligned. >> >> > >> >> > This can be seen by writing to SPI flash a 57 byte file containing 00 01 .. 38: >> >> > >> >> > U-Boot > dhcp data.bin >> >> > ... >> >> > Bytes transferred = 57 (39 hex) >> >> > U-Boot > md.b $loadaddr >> >> > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ >> >> > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ >> >> > 72000020: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f !"#$%&'()*+,-./ >> >> > 72000030: 30 31 32 33 34 35 36 37 38 ff ff ff ff ff ff ff 012345678....... >> >> > U-Boot > sf write $loadaddr 0 $filesize >> >> > >> >> > U-Boot > mw.b $loadaddr ff 100 >> >> > U-Boot > sf read $loadaddr 0 100 >> >> > U-Boot > md.b $loadaddr >> >> > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ >> >> > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ >> >> > 72000020: 20 21 22 23 24 29 2a 2b 2c 31 32 33 34 ff ff ff !"#$)*+,1234... >> >> > 72000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ >> >> > >> >> > [bytes 25-28, 2d-30, 35-38 are missing] >> >> > >> >> > Activating debug in the write command gave: >> >> > spi_xchg_single: bitlen 256 dout 0x72000000 din 0x0 >> >> > Sending SPI 0x10203 >> >> > Sending SPI 0x4050607 >> >> > Sending SPI 0x8090a0b >> >> > Sending SPI 0xc0d0e0f >> >> > Sending SPI 0x10111213 >> >> > Sending SPI 0x14151617 >> >> > Sending SPI 0x18191a1b >> >> > Sending SPI 0x1c1d1e1f >> >> > SPI Rx: 0xffffffff 0xffffffff >> >> > ... >> >> > SPI Rx: 0xffffffff 0xffffffff >> >> > spi_xchg_single: bitlen 200 dout 0x72000020 din 0x0 >> >> > Sending SPI 0x20 >> >> > Sending SPI 0x21222324 >> >> > Sending SPI 0x292a2b2c <=== What happened to 25262728? >> >> > Sending SPI 0x31323334 >> >> > Sending SPI 0xffffffff >> >> > ... >> >> > SF: program success 57 bytes @ 0x0 >> >> Thanks for your log, for showing the exact issues. >> > >> > Agreed, but I think this log could go in a comment below the '---' >> > line, so that the commit message focuses on describing what the patch >> > does, rather than how it was found out necessary. >> >> I too thought the same, but the commit is already in master, any >> possibility to alter? > > Er... Martin's patch is already in patchwork as 283794, but its state > is still "New" with no delegate, and I don't see it in any of the > repos I usually read, nor does it appear on u-boot-spi. Where do you > see it "in master" exactly? > > Besides, it is less than 24 hours old; this is not enough time for > review, and therefore, it should not be accepted yet. I got the similar patch from Timo Herbrecher couple of weeks back, I just reviewed and sent back to v3 before apllied on u-boot-spi. Yesterday I sent this patch as part of spi PR. You can find the patch in master: http://git.denx.de/?p=u-boot.git;a=commitdiff;h=6d5ce1bd0048617d48c05de1a84fae8696081127 Please let me know you still have any concerns, I agreed your concern that the commit message have enough info for this fix. But right know i think we can't do as it's part of master. If any possibility to alter the commit msg, please let me know.
On 16/10/13 15:22, Jagan Teki wrote: > Hi Albert, > > On Wed, Oct 16, 2013 at 5:35 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: >> Hi Jagan, >> >> On Wed, 16 Oct 2013 16:20:17 +0530, Jagan Teki >> <jagannadh.teki@gmail.com> wrote: >> >>> Hi Amicalement, >>> >>> On Wed, Oct 16, 2013 at 1:05 PM, Albert ARIBAUD >>> <albert.u.boot@aribaud.net> wrote: >>>> Hi Jagan, >>>> >>>> On Wed, 16 Oct 2013 11:16:49 +0530, Jagan Teki >>>> <jagannadh.teki@gmail.com> wrote: >>>> >>>>> Hi, >>>>> >>>>> On Tue, Oct 15, 2013 at 11:27 PM, Martin Fuzzey <mfuzzey@parkeon.com> wrote: >>>>>> When writing buffers that are not 32 bit aligned data loss occurs. >>>>>> >>>>>> This can also occur when the total transfer size is not a multiple of 4 bytes >>>>>> since the extra bytes are written first causing the rest to be unaligned. >>>>>> >>>>>> This can be seen by writing to SPI flash a 57 byte file containing 00 01 .. 38: >>>>>> >>>>>> U-Boot > dhcp data.bin >>>>>> ... >>>>>> Bytes transferred = 57 (39 hex) >>>>>> U-Boot > md.b $loadaddr >>>>>> 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ >>>>>> 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ >>>>>> 72000020: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f !"#$%&'()*+,-./ >>>>>> 72000030: 30 31 32 33 34 35 36 37 38 ff ff ff ff ff ff ff 012345678....... >>>>>> U-Boot > sf write $loadaddr 0 $filesize >>>>>> >>>>>> U-Boot > mw.b $loadaddr ff 100 >>>>>> U-Boot > sf read $loadaddr 0 100 >>>>>> U-Boot > md.b $loadaddr >>>>>> 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ >>>>>> 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ >>>>>> 72000020: 20 21 22 23 24 29 2a 2b 2c 31 32 33 34 ff ff ff !"#$)*+,1234... >>>>>> 72000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ >>>>>> >>>>>> [bytes 25-28, 2d-30, 35-38 are missing] >>>>>> >>>>>> Activating debug in the write command gave: >>>>>> spi_xchg_single: bitlen 256 dout 0x72000000 din 0x0 >>>>>> Sending SPI 0x10203 >>>>>> Sending SPI 0x4050607 >>>>>> Sending SPI 0x8090a0b >>>>>> Sending SPI 0xc0d0e0f >>>>>> Sending SPI 0x10111213 >>>>>> Sending SPI 0x14151617 >>>>>> Sending SPI 0x18191a1b >>>>>> Sending SPI 0x1c1d1e1f >>>>>> SPI Rx: 0xffffffff 0xffffffff >>>>>> ... >>>>>> SPI Rx: 0xffffffff 0xffffffff >>>>>> spi_xchg_single: bitlen 200 dout 0x72000020 din 0x0 >>>>>> Sending SPI 0x20 >>>>>> Sending SPI 0x21222324 >>>>>> Sending SPI 0x292a2b2c <=== What happened to 25262728? >>>>>> Sending SPI 0x31323334 >>>>>> Sending SPI 0xffffffff >>>>>> ... >>>>>> SF: program success 57 bytes @ 0x0 >>>>> Thanks for your log, for showing the exact issues. >>>> Agreed, but I think this log could go in a comment below the '---' >>>> line, so that the commit message focuses on describing what the patch >>>> does, rather than how it was found out necessary. >>> I too thought the same, but the commit is already in master, any >>> possibility to alter? >> Er... Martin's patch is already in patchwork as 283794, but its state >> is still "New" with no delegate, and I don't see it in any of the >> repos I usually read, nor does it appear on u-boot-spi. Where do you >> see it "in master" exactly? >> >> Besides, it is less than 24 hours old; this is not enough time for >> review, and therefore, it should not be accepted yet. > I got the similar patch from Timo Herbrecher couple of weeks back, > I just reviewed and sent back to v3 before apllied on u-boot-spi. > > Yesterday I sent this patch as part of spi PR. > > You can find the patch in master: > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=6d5ce1bd0048617d48c05de1a84fae8696081127 > > Please let me know you still have any concerns, I agreed your concern > that the commit message > have enough info for this fix. But right know i think we can't do as > it's part of master. > > If any possibility to alter the commit msg, please let me know. > Yes looks like two patches doing the same thing. The one in master is from Timo Herbrecher not me. I checked git master before sending my patch and it wasn't there yet but didin't check the mailing list archives. Anyway the code is identical just the commit message changes. Cheers, Martin
Hi Jagan, On Wed, 16 Oct 2013 18:52:34 +0530, Jagan Teki <jagannadh.teki@gmail.com> wrote: > Hi Albert, > > On Wed, Oct 16, 2013 at 5:35 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: > > Hi Jagan, > > > > On Wed, 16 Oct 2013 16:20:17 +0530, Jagan Teki > > <jagannadh.teki@gmail.com> wrote: > > > >> Hi Amicalement, > >> > >> On Wed, Oct 16, 2013 at 1:05 PM, Albert ARIBAUD > >> <albert.u.boot@aribaud.net> wrote: > >> > Hi Jagan, > >> > > >> > On Wed, 16 Oct 2013 11:16:49 +0530, Jagan Teki > >> > <jagannadh.teki@gmail.com> wrote: > >> > > >> >> Hi, > >> >> > >> >> On Tue, Oct 15, 2013 at 11:27 PM, Martin Fuzzey <mfuzzey@parkeon.com> wrote: > >> >> > When writing buffers that are not 32 bit aligned data loss occurs. > >> >> > > >> >> > This can also occur when the total transfer size is not a multiple of 4 bytes > >> >> > since the extra bytes are written first causing the rest to be unaligned. > >> >> > > >> >> > This can be seen by writing to SPI flash a 57 byte file containing 00 01 .. 38: > >> >> > > >> >> > U-Boot > dhcp data.bin > >> >> > ... > >> >> > Bytes transferred = 57 (39 hex) > >> >> > U-Boot > md.b $loadaddr > >> >> > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ > >> >> > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ > >> >> > 72000020: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f !"#$%&'()*+,-./ > >> >> > 72000030: 30 31 32 33 34 35 36 37 38 ff ff ff ff ff ff ff 012345678....... > >> >> > U-Boot > sf write $loadaddr 0 $filesize > >> >> > > >> >> > U-Boot > mw.b $loadaddr ff 100 > >> >> > U-Boot > sf read $loadaddr 0 100 > >> >> > U-Boot > md.b $loadaddr > >> >> > 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ > >> >> > 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ > >> >> > 72000020: 20 21 22 23 24 29 2a 2b 2c 31 32 33 34 ff ff ff !"#$)*+,1234... > >> >> > 72000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > >> >> > > >> >> > [bytes 25-28, 2d-30, 35-38 are missing] > >> >> > > >> >> > Activating debug in the write command gave: > >> >> > spi_xchg_single: bitlen 256 dout 0x72000000 din 0x0 > >> >> > Sending SPI 0x10203 > >> >> > Sending SPI 0x4050607 > >> >> > Sending SPI 0x8090a0b > >> >> > Sending SPI 0xc0d0e0f > >> >> > Sending SPI 0x10111213 > >> >> > Sending SPI 0x14151617 > >> >> > Sending SPI 0x18191a1b > >> >> > Sending SPI 0x1c1d1e1f > >> >> > SPI Rx: 0xffffffff 0xffffffff > >> >> > ... > >> >> > SPI Rx: 0xffffffff 0xffffffff > >> >> > spi_xchg_single: bitlen 200 dout 0x72000020 din 0x0 > >> >> > Sending SPI 0x20 > >> >> > Sending SPI 0x21222324 > >> >> > Sending SPI 0x292a2b2c <=== What happened to 25262728? > >> >> > Sending SPI 0x31323334 > >> >> > Sending SPI 0xffffffff > >> >> > ... > >> >> > SF: program success 57 bytes @ 0x0 > >> >> Thanks for your log, for showing the exact issues. > >> > > >> > Agreed, but I think this log could go in a comment below the '---' > >> > line, so that the commit message focuses on describing what the patch > >> > does, rather than how it was found out necessary. > >> > >> I too thought the same, but the commit is already in master, any > >> possibility to alter? > > > > Er... Martin's patch is already in patchwork as 283794, but its state > > is still "New" with no delegate, and I don't see it in any of the > > repos I usually read, nor does it appear on u-boot-spi. Where do you > > see it "in master" exactly? > > > > Besides, it is less than 24 hours old; this is not enough time for > > review, and therefore, it should not be accepted yet. > > I got the similar patch from Timo Herbrecher couple of weeks back, > I just reviewed and sent back to v3 before apllied on u-boot-spi. > > Yesterday I sent this patch as part of spi PR. > > You can find the patch in master: > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=6d5ce1bd0048617d48c05de1a84fae8696081127 > > Please let me know you still have any concerns, I agreed your concern > that the commit message > have enough info for this fix. But right know i think we can't do as > it's part of master. > > If any possibility to alter the commit msg, please let me know. If it's gone in a repo, especially the main one, then you can't roll it back -- BTW Timo's commit message is ok for me. Amicalement,
diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 859c43f..964a2b7 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -274,8 +274,8 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen, } else { data = *(u32 *)dout; data = cpu_to_be32(data); + dout += 4; } - dout += 4; } debug("Sending SPI 0x%x\n", data); reg_write(®s->txdata, data);
When writing buffers that are not 32 bit aligned data loss occurs. This can also occur when the total transfer size is not a multiple of 4 bytes since the extra bytes are written first causing the rest to be unaligned. This can be seen by writing to SPI flash a 57 byte file containing 00 01 .. 38: U-Boot > dhcp data.bin ... Bytes transferred = 57 (39 hex) U-Boot > md.b $loadaddr 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ 72000020: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f !"#$%&'()*+,-./ 72000030: 30 31 32 33 34 35 36 37 38 ff ff ff ff ff ff ff 012345678....... U-Boot > sf write $loadaddr 0 $filesize U-Boot > mw.b $loadaddr ff 100 U-Boot > sf read $loadaddr 0 100 U-Boot > md.b $loadaddr 72000000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ 72000010: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................ 72000020: 20 21 22 23 24 29 2a 2b 2c 31 32 33 34 ff ff ff !"#$)*+,1234... 72000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ [bytes 25-28, 2d-30, 35-38 are missing] Activating debug in the write command gave: spi_xchg_single: bitlen 256 dout 0x72000000 din 0x0 Sending SPI 0x10203 Sending SPI 0x4050607 Sending SPI 0x8090a0b Sending SPI 0xc0d0e0f Sending SPI 0x10111213 Sending SPI 0x14151617 Sending SPI 0x18191a1b Sending SPI 0x1c1d1e1f SPI Rx: 0xffffffff 0xffffffff ... SPI Rx: 0xffffffff 0xffffffff spi_xchg_single: bitlen 200 dout 0x72000020 din 0x0 Sending SPI 0x20 Sending SPI 0x21222324 Sending SPI 0x292a2b2c <=== What happened to 25262728? Sending SPI 0x31323334 Sending SPI 0xffffffff ... SF: program success 57 bytes @ 0x0 The problem was that the pointer was being incremented twice for the non aligned case. Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com> --- drivers/spi/mxc_spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)