[U-Boot] [PATCH] crc32: Correct endianness of crc32 result

[Tom Rini <trini@ti.com>]

On Thu, Apr 18, 2013 at 12:06:47PM -0700, Simon Glass wrote:
> Hi,
> 
> On Thu, Apr 18, 2013 at 11:43 AM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
> > Hi Tom,
> >
> > On Thu, 18 Apr 2013 12:58:54 -0400, Tom Rini <trini@ti.com> wrote:
> >
> >> On Thu, Apr 18, 2013 at 06:39:29PM +0200, Albert ARIBAUD wrote:
> >> > Hi Wolfgang,
> >> >
> >> > On Thu, 18 Apr 2013 16:39:09 +0200, Wolfgang Denk <wd@denx.de> wrote:
> >> >
> >> > > > So how about changing the element type of output in the definition of
> >> > > > hash_func_ws, adapting the corresponding implementations sha1_csum_wd,
> >> > > > sha256_csum_wd and crc32_wd_buf, and adapting the output argument
> >> > > > of the sole call to hash_func_ws, that is, the local "u8
> >> > > > output[HASH_MAX_DIGEST_SIZE];" in hash.c? Then we'be done with
> >> > > > alignment.
> >> > >
> >> > > OK, but that would be a totally different approach (which appears to
> >> > > be a good one, here).
> >> >
> >> > Indeed; I would suggest splitting this change in two independent ones:
> >> >
> >> > - fix the endianness issue: change the endianness of crc through the
> >> >   use of htonl() but leave the existing memcpy() in place as it is,
> >> >   even though it is not speed-optimized. That's what Simon's patch
> >> >   does if the HOSTCC part is ignored;
> >> >
> >> > - fix the unalignment issue by changing parameter 'output' of function
> >> >   type 'hash_func_ws' from u8* to u32* and adjusting the rest of the
> >> >   code accordingly -- which would lead to replacing the crc32 final
> >> >   memcpy() with a single indirect assignment.
> >> >
> >> > These two changes could be submitted either separately, or as a series.
> >>
> >> Now so that I'm clear, if we don't do anything about the unaligned issue
> >> today, it's just slow, not an unaligned access that leads to abort,
> >> right?  So part one today for release, part two next week after release.
> >
> > Yes, the current code is just slower than it could be, but works, and
> > so would it with Simon's patch as long as it keeps the memcpy().
> 
> Yes the alignment issue is manufactured IMO by the char * used for the
> function signature and I did not feel comfortable declaring that it
> must be word aligned. To be it seems that the type should be naturally
> aligned. What do people think about that?
> 
> Anyway, that's why I ended up with this patch, which I felt was small
> enough to be accepted as a bug fix for the release.
> 
> Albert's email exactly mirrors my thinking on this - and yes I would
> be much more comfortable not having to create part 2 for the release.
> 
> Please let me know what I should do with this patch for now.

Lets follow the outline Albert made above, two patches, #2 depends on #1
and #1 is now, #2 is next week.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130418/f1147d70/attachment.pgp>