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.b...@aribaud.net> wrote: > > Hi Tom, > > > > On Thu, 18 Apr 2013 12:58:54 -0400, Tom Rini <tr...@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 <w...@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
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot