Donald Becker wrote:
No. See what set_bit() (and set_bit_le() in this case) does. It worksWhere? The register writes are identical to your driver:
on a bit arry and handles any byte arithmetic/shifting internally.
The dp83815 chip is quirky:
the filter index is a byte address,
the filter entries must be written as pairs of bytes
the filter entry write must be a 32 bit operation.
You may note that the natsemi.c driver in the kernel does not write
registers using the correct bit width.
>
>void set_bit_le(int offset, unsigned char * data)
>{
> data[offset >> 3] |= (1 << (offset & 0x07));
>}
[...]
> u8 mc_filter[64]; /* Multicast hash filter */
> set_bit_le(
> dp83815_crc(ETH_ALEN, mclist->dmi_addr) & 0x1ff,
> mc_filter);
[...]
> writew(HASH_TABLE + i, ioaddr + RxFilterAddr);
> writew((mc_filter[i+1]<<8) + mc_filter[i],
> ioaddr + RxFilterData);
[...]
> writel(rx_mode, ioaddr + RxFilterAddr);
The only change I've done was the introduction of set_bit_le that operates on bytes: set_bit() is only defined on 'unsigned long' quantities. It could generate access violation if the compiler doesn't 'long' align the array on the stack and the cpu doesn't support locked operations on misaligned variables.
And it was wrong on bit-endian systems.
Back to the bug: Is the crc function correct? I remember that I tested multicast both on little and big endian, but that was long ago.
Who wrote the comment
/**
* dp83815_crc - computer CRC for hash table entries
*
* Note - this is, for some reason, *not* the same function
* as ether_crc_le() or ether_crc(), though it uses the
* same big-endian polynomial.
*/
--
Manfred
-
: send the line "unsubscribe linux-net" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
