On 10/09/2011 12:51 AM, Michael Büsch wrote:
> On Sat, 08 Oct 2011 17:28:42 -0500
> Larry Finger <Larry.Finger@xxxxxxxxxxxx> wrote:
>
>> The kernel now contains library routines to establish crc8 tables and
>> to calculate the appropriate sums. Use them for ssb.
>
>> +static u8 srom_crc8_table[CRC8_TABLE_SIZE];
>> +
>> +/* Polynomial: x^8 + x^7 + x^6 + x^4 + x^2 + 1 */
>> +#define SROM_CRC8_POLY 0xAB
>> +
>> +static inline void ltoh16_buf(u16 *buf, unsigned int size)
>> {
>> + size /= 2;
>> + while (size--)
>> + *(buf + size) = le16_to_cpu(*(__le16 *)(buf + size));
>> +}
>>
>> - return crc;
>> +static inline void htol16_buf(u16 *buf, unsigned int size)
>> +{
>> + size /= 2;
>> + while (size--)
>> + *(__le16 *)(buf + size) = cpu_to_le16(*(buf + size));
>> }
>
>> return -ENOMEM;
>> + crc8_populate_lsb(srom_crc8_table, SROM_CRC8_POLY);
>> bus->sprom_size = SSB_SPROMSIZE_WORDS_R123;
>> sprom_do_read(bus, buf);
>> + /* convert to le */
>> + htol16_buf(buf, 2 * bus->sprom_size);
>
>> bus->sprom_size = SSB_SPROMSIZE_WORDS_R4;
>> sprom_do_read(bus, buf);
>> + htol16_buf(buf, 2 * bus->sprom_size);
>> err = sprom_check_crc(buf, bus->sprom_size);
>
>> + /* restore endianess */
>> + ltoh16_buf(buf, 2 * bus->sprom_size);
>> err = sprom_extract(bus, sprom, buf, bus->sprom_size);
>
> This endianness stuff is _really_ ugly.
It may seem ugly, but is not new. Choosing a 8-bit crc to check a 16-bit
array is not very efficient considering host endianess. The endianess
was also dealt with in the old version:
- for (word = 0; word < size - 1; word++) {
- crc = ssb_crc8(crc, sprom[word] & 0x00FF);
- crc = ssb_crc8(crc, (sprom[word] & 0xFF00) >> 8);
- }
It is a bit easier on the eye. I guess the ugliness comes from the fact
that there are two calls to htol16_buf.
A better approach would be to read sprom as bytes and run the crc8 over
the byte array. When ok do ltoh16_buf once.
> Does this patch decrease the code size, at least? I'll almost doubt it.
> If it doesn't, why are we actually doing this?
Probably for the same reason why struct list_head and related functions
are used. Trying to use what is commonly available in the kernel.
> It doesn't even decrease the .data size. Worse, it converts a .const
> table to a .data table.
True. .code size became .data size, because of the flexibility that the
table is generated for a given polynomial. Every 'bility' comes with a
price and this seems not too pricy.
> Just my 2 cents.
>
Gr. AvS
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
