public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.u.boot@aribaud.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] spi: mxc: Fix data loss for non aligned write buffers.
Date: Wed, 16 Oct 2013 16:41:24 +0200	[thread overview]
Message-ID: <20131016164124.7e82993c@lilith> (raw)
In-Reply-To: <CAD6G_RSEozcGb-uvpeEjTHcXhn5zhy=4ei3cGPU8XzSWenuNzQ@mail.gmail.com>

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,
-- 
Albert.

      parent reply	other threads:[~2013-10-16 14:41 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-15 17:57 [U-Boot] [PATCH] spi: mxc: Fix data loss for non aligned write buffers Martin Fuzzey
2013-10-16  5:46 ` Jagan Teki
2013-10-16  7:35   ` Albert ARIBAUD
2013-10-16 10:50     ` Jagan Teki
2013-10-16 12:05       ` Albert ARIBAUD
2013-10-16 13:22         ` Jagan Teki
2013-10-16 13:56           ` Martin Fuzzey
2013-10-16 14:41           ` Albert ARIBAUD [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131016164124.7e82993c@lilith \
    --to=albert.u.boot@aribaud.net \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox