Messages in this thread | | | Date | Mon, 1 Aug 2016 09:31:04 -0500 | From | Segher Boessenkool <> | Subject | Re: [PATCH] powerpc/32: fix csum_partial_copy_generic() |
| |
On Mon, Aug 01, 2016 at 03:45:59PM +0200, Christophe Leroy wrote: > >>--- a/arch/powerpc/lib/checksum_32.S > >>+++ b/arch/powerpc/lib/checksum_32.S > >>@@ -127,7 +127,8 @@ _GLOBAL(csum_partial_copy_generic) > >> stw r7,12(r1) > >> stw r8,8(r1) > >> > >>- andi. r0,r4,1 /* is destination address even ? */ > >>+ rlwinm r0,r4,3,0x8 /* is destination address even ? */ > >>+ rlwnm r6,r6,r0,0,31 /* swap bytes for odd destination */ > >> cmplwi cr7,r0,0 > >> addic r12,r6,0 > >> addi r6,r4,-4 > > > >That does not "swap bytes"; it shifts the word up by 8 bits, instead. > >That may or may not do what is intended. > > Indeed it does what is intended, similar to what is done at the end of > the function: > > ... > beqlr+ cr7 > rlwinm r3,r3,8,0,31 /* swap bytes for odd destination */ > blr > > Should I fix the (both) comment(s) ?
It's quite confusing comment, so yes please. The code is hard enough to understand as is!
Segher
| |