(I posted this bug at http://sourceforge.net/projects/tulip/, before I realized that that bug tracker was deprecated. So it probably belongs here instead.) Problem ======= There seems to be a problem when running the tulip driver with 6 bit eeprom address length: The tulip_read_eeprom() function sets up a read command like: int read_cmd = location | (EE_READ_CMD << addr_len); where location is the offset (within the eeprom) and addr_len is either 6 or 8 bits. However, with addr_len == 6 and location > 0x3f, the most significant location bits would overlap with the read command itself. Thus, with 6 bit addressing, tulip_read_eeprom() must not be called with location > 0x3f. Unfortunately, in tulip_core.c, it's called with location up to sizeof(tp->eeprom)/2, which should be 0x100 if I'm not mistaken. In the same way de2104x.c uses locations up to DE_EEPROM_WORDS, which is equal to 256 too. How-to-fix ========== Make sure that the location is masked corresponding to add_len in tulip_read_eeprom().
Please submit a patch to fix it?
Ok. A patch is on it's way. I just need to do some testing first. (it's really a one-liner as I described above - just mask the address before using it).
Created attachment 4887 [details] Patch to mask address in tulip_read_eeprom() Added patch.
Jeff, can you review the patch in this bug?
Any update on this please. Daniel, was the problem resolved for you in recent kernels? If not, maybe Grant can now take a look at it and the proposed patch. Well, if Daniel is not around the bug will have to be closed unfortunately. Thanks.
Patch looks good to me and based on previous email, I expect it fixes the problem. I'll verify against current linus tree and submit to akpm with signed-off-by line. thanks!
On second thought, patch isn't ok. We should NOT read some other "random" register instead of returning an error value. I'll attached a patch that I just submitted upstream that does two things: 1) tulip_read_eeprom will return zero (since we can't return -EINVAL) if this is attempted (defensive programming). 2) In tulip_core.c, fix the tulip_read_eeprom caller so they don't iterate past addr_len bits and make sure the entire tp->eeprom[] array is cleared. I'll attach the patch I submitted as well to this bug report.
Created attachment 15410 [details] Proposed tulip_read_eeprom patch sent to netdev today
Perhaps this should be a separate bug, but de2104x.c has the same bug since it defines it's own copy of tulip_read_eeprom(). I suspect the right fix there would be to modify de2104x.c to use eeprom.c instead of keeping it's own copy of the same code. But that might not be a trivial change...and I don't have any de2104x cards to test with (I need to double check, but I don't think so offhand).
Verified applied