Conversation
|
Depending on time frame, we could leave this open as a draft PR until you think it is fully. Or if you think it'll be a bit longer, an in-repo branch here would also be Ok. |
The IMX.6 ethernet driver is compatible with the IMX.8 but our implementation assumes the IMX.6 OCOTP which stores the MAC address at boot and is not compatible with the IMX.8. It's also possible that the ethernet device already has the register initialised by the bootloader already at this point... Signed-off-by: Curtis Millar <[email protected]> Signed-off-by: Lucy <[email protected]>
Signed-off-by: Lucy <[email protected]>
Signed-off-by: Lucy <[email protected]>
Signed-off-by: Lucy <[email protected]>
Signed-off-by: Lucy <[email protected]>
Signed-off-by: Lucy <[email protected]>
Signed-off-by: Lucy <[email protected]>
Signed-off-by: Lucy <[email protected]>
|
@lsf37 It might be a month or two before I'll have time to sort through it again. I also would appreciate feedback from the systems guys on the imx8mm specific changes. |
| add_config_library(ethdrivers "${configure_string}") | ||
|
|
||
| if("${KernelPlatform}" MATCHES "imx8mq-evk") | ||
| if(("${KernelPlatform}" MATCHES "imx8mq-evk") OR ("${KernelPlatform}" MATCHES "imx8mm-evk")) |
There was a problem hiding this comment.
I just wonder, was there any reason we say MATCHES here and not STREQ? We could simply use the flags here also if this selects well define platforms: if(KernelPlatformImx8mq-evk OR KernelPlatformImx8mm-evk)
| #ifdef CONFIG_PLAT_IMX8MQ_EVK | ||
| #define CCM_PADDR 0x30380000 | ||
| #if (defined(CONFIG_PLAT_IMX8MQ_EVK) || defined(CONFIG_PLAT_IMX8MM_EVK)) | ||
| #define CCM_PADDR 0x30380000 // from: root/arch/arm/include/asm/arch-imx8m/imx-regs-imx8mm.h |
| enet_tx_enable(dev->enet); | ||
| } | ||
| return; | ||
| if (d->stat & TXD_READY) return; |
There was a problem hiding this comment.
please use
| if (d->stat & TXD_READY) return; | |
| if (d->stat & TXD_READY) { | |
| return; | |
| } |
| // ethif_print_state(netif_get_eth_driver(netif)); | ||
| assert(0); | ||
| while (1); | ||
| while (e & IRQ_MASK) { |
There was a problem hiding this comment.
Should we merge this as a potential bug fix in a separate PR? Looping to consume all pending interrupt seems a good general pattern and I can't remember any reason not to do it here.
| * hardware MAC address. */ | ||
| uint64_t mac = 0; | ||
| ZF_LOGI("using MAC configured by bootloader"); | ||
| #else |
There was a problem hiding this comment.
I wonder if we should make this a conditional else, so we handle all known platform explicitly and don't have a default behavior? Ir would fail compilation for unknown platforms then and requires explicit action.
| switch (id) { | ||
| #if defined(CONFIG_PLAT_IMX8MM_EVK) || defined(CONFIG_IMX8MQ_EVK) | ||
| case 0: | ||
| mac = ((uint64_t)((uint16_t)regs->res37[4]) << 32) | regs->res37[0]; |
There was a problem hiding this comment.
Please give the registers a name in the definition, reading a MAC from reserved registers is a bit odd.
4 participants
This is an extension to the imx6 driver to support imx8mq and imx8mm dev boards.
Ideally the systems will actually have their own files (Todo), and in my opinion there is a tidy up to be done on this folder (I can get onto this in the near future). Perhaps this update should go somewhere else for the time being?