Re: [PATCH 3/4 v2] lib: add crc16_le helper

[Florian Fainelli <ffainelli@freebox.fr>]

Hi Joakim,

On Thursday 19 August 2010 11:41:13 Joakim Tjernlund wrote:
> Matthieu CASTET <matthieu.castet@parrot.com> wrote on 2010/08/19 09:42:49:
> > Hi,
> > 
> > Joakim Tjernlund a écrit :
> > > Matthieu CASTET <matthieu.castet@parrot.com> wrote on 2010/08/18 
18:10:11:
> > >>> if so, 0x8005 looks
> > >>> wrong too. see CRCPOLY_LE resp. CRCPOLY_BE for an idea.
> > >> 
> > >> Why ?
> > > 
> > > Because changing endian of crc also require the POLY
> > > 
> > > to be bit reversed as well. See:
> > >  #define CRCPOLY_LE 0xedb88320
> > >  #define CRCPOLY_BE 0x04c11db7
> > > 
> > > So I assume you need to do that as well for crc16_be, otherwise it
> > > won't be a BE version of the standard crc16 LE
> > 
> > Well it match the crc provided in onfi nand [1]. But it may be not a
> > real BE version.
> > 
> > >>> What is this crc sum used for?
> > >> 
> > >> Onfi flash parsing in mtd.
> > > 
> > > And this is a one time operation of fairly small amount of
> > > data? I ask because you impl. is really slow.
> > 
> > Yes it checks only 253 bytes at startup time (one time only). So it is
> > not speed critical.
> 
> OK.
> 
> > > Why can't you use the crc16 LE version?
> > 
> > Because it doesn't compute the crc provided by onfi spec. Or is there
> > some magic maths to convert LE version to our version ?
> 
> I suspect the onfi spec got it wrong and isn't computing a real
> crc16_be. I suggest you rename this to onfi_crc16_be and move it
> inside the onfi subsystem. I supposed you are stuck with
> the onfi version?

I thought we could somehow genericize this version of the crc16, but you are 
right, we cannot, so let's put it back to the mtd subsystem where it is used.

Thanks for your comments.

> Just for the record, I think the crc16_be should be like this:
> 
> u16 crc16_le(u16 crc, u8 const *p, size_t len)
> {
>    int i;
>    while (len--) {
>       crc ^= *p++ << 8;
>       i = 8;
>       do {
>          crc = (crc << 1) ^ ((crc & 0x8000) ? 0xa001 : 0);
>       } while(--i);
>    }
>    return crc;
> }
> 
> Don't know of an easy way to go between LE and BE versions but
> have a look at lib/crc32.c, there is a unit test in there
> that suggests that on can byte reverse the data to go between
> LE and BE.

Ok, I will give it a try.
--
Florian