From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Thu, 18 Apr 2013 15:18:32 -0400 Subject: [U-Boot] [PATCH] crc32: Correct endianness of crc32 result In-Reply-To: References: <20130417212326.4f803bfb@lilith> <20130418082027.4b5ea191@lilith> <20130418103600.8E7FB20019A@gemini.denx.de> <20130418131810.38c916b8@lilith> <20130418143909.192F720019A@gemini.denx.de> <20130418183929.4ec73ae3@lilith> <20130418165854.GE14952@bill-the-cat> <20130418204339.34f58991@lilith> Message-ID: <20130418191832.GF14952@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > wrote: > > Hi Tom, > > > > On Thu, 18 Apr 2013 12:58:54 -0400, Tom Rini 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 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: