Intel Ethernet i217 needs edits?
Posted: Thu Aug 18, 2016 11:21 am
I can't seem to create a wiki account (clicking the link from Vivaldi just brings me back to the top page) so I don't know how to fix this myself.
I'm just finishing up an Intel GBE driver (works! thank you yet again OSDev!) and found the Intel Ethernet i217 page invaluable (my whole problem was that the descriptor head/tail registers count by 1, not by the descriptor size, which is either obvious or not at all depending on who you are). But it has this whole discussion of EPROM vs. flash which does not match its own code. What the code really does is distinguish between two different layouts (old and new) for the EERD EEPROM access register. detectEEProm checks for the old "done" bit (at EERD.3 instead of EERD.1) and sets eerprom_exists based on its existence. eepromRead correctly uses this flag to decide which register format to use ... but it doesn't matter because readMACAddress uses that flag just to decide whether to call eepromRead *at all* (it does only in the old case -- for the new case it assumes the first slot of the perfect address filter has been auto-loaded at reset).
The references to flash memory that I've found in the (newer) data sheets refer only to the PXE BIOS, which obviously has nothing to do with the MAC address.
So for one thing I'm curious whether the author's "problem" NICs (based on i217 and i82577LM) would actually work OK with the code corrected. I have no trouble reading the EEPROM on my i82574-based NIC, which uses the newer EERD format that the sample code almost supports but not quite. (My code just waits for either "done" bit, with a timeout, and then increments the address field by either 4 or 100h depending on which bit came on, until it has all three words of the MAC address.)
Also the sample code uses the CMD_RPS bit in transmit descriptors, which if I understand right, is supported by only a very few (older) chips. It modifies transmit behavior to report the status only after transmission is complete (vs. when the packet is accepted by the FIFO), so the status field adds two bits about collisions (which this code doesn't check). Seems risky to set what later became a reserved bit, is all I'm saying.
I'm just finishing up an Intel GBE driver (works! thank you yet again OSDev!) and found the Intel Ethernet i217 page invaluable (my whole problem was that the descriptor head/tail registers count by 1, not by the descriptor size, which is either obvious or not at all depending on who you are). But it has this whole discussion of EPROM vs. flash which does not match its own code. What the code really does is distinguish between two different layouts (old and new) for the EERD EEPROM access register. detectEEProm checks for the old "done" bit (at EERD.3 instead of EERD.1) and sets eerprom_exists based on its existence. eepromRead correctly uses this flag to decide which register format to use ... but it doesn't matter because readMACAddress uses that flag just to decide whether to call eepromRead *at all* (it does only in the old case -- for the new case it assumes the first slot of the perfect address filter has been auto-loaded at reset).
The references to flash memory that I've found in the (newer) data sheets refer only to the PXE BIOS, which obviously has nothing to do with the MAC address.
So for one thing I'm curious whether the author's "problem" NICs (based on i217 and i82577LM) would actually work OK with the code corrected. I have no trouble reading the EEPROM on my i82574-based NIC, which uses the newer EERD format that the sample code almost supports but not quite. (My code just waits for either "done" bit, with a timeout, and then increments the address field by either 4 or 100h depending on which bit came on, until it has all three words of the MAC address.)
Also the sample code uses the CMD_RPS bit in transmit descriptors, which if I understand right, is supported by only a very few (older) chips. It modifies transmit behavior to report the status only after transmission is complete (vs. when the packet is accepted by the FIFO), so the status field adds two bits about collisions (which this code doesn't check). Seems risky to set what later became a reserved bit, is all I'm saying.