Kernel: New IO mapped Register API and a rework of the E1000 drivers #25521
Conversation
| // FIXME: This seems odd to me: | ||
| // We disable interrupts and then wait for and irq to happen... |
There was a problem hiding this comment.
Might be a long shot, but I feel like this tries to avoid getting into an interrupt while sending a packet, no?
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! |
|
This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions! |
|
|
||
| // https://www.intel.com/content/dam/doc/manual/pci-pci-x-family-gbe-controllers-software-dev-manual.pdf | ||
| // Table 13-2 | ||
| enum class Register { |
There was a problem hiding this comment.
nit: I think it could be nice to have this list sorted by category. In later datasheets you have an even clearer separation between types of registers.
i.e. https://www.mouser.com/pdfdocs/82579datasheetvol21.pdf
There was a problem hiding this comment.
Currently its in numeric order preferring the legacy aliases
In the future we may have to support the "array-like" register types, which are ordered a bit more nicely
For now I could add a few more line separations whenever we skip a register, which acheives something similar for now
| in32(REG_INTERRUPT_CAUSE_READ); | ||
| // FIXME: Do this properly, | ||
| // There's probably a way to find an ideal interrupt rate etc | ||
| m_registers.write<Register::InterruptThrottling>(6000); // Interrupt rate of 1.536 milliseconds |
There was a problem hiding this comment.
We should be able to calculate the ideal rate dynamically based on packet/byte counts during the last firing of the interrupt. We have to test what the ideal boundaries are - so it may be out of scope for this PR. Would be great though if there is some sort of function that runs after each interrupt though if we decide to go forward with this.
| } else { | ||
| out32(REG_EEPROM, ((u32)address << 2) | 1); | ||
| while (!((tmp = in32(REG_EEPROM)) & (1 << 1))) | ||
| // FIXME: Should this just return 0 then? |
There was a problem hiding this comment.
IMO yes. We could give an error back instead of crashing(?) the system, but that's personal preference.
For this we: - Introduce an operating mode - Allow getting the MAC address from firmware initialized hw-registers - Add support for advanced receive descriptors, as qemu does not support the legacy mode for these in `igb` mode - Remove the `E1000ENetworkAdapter` driver With this we can now boot on qemu with the `igb` network devices selected.
github-actions
bot
added
the
👀 pr-needs-review
Less defines, more type safety....