-
Notifications
You must be signed in to change notification settings - Fork 8.2k
drivers: ethernet: add support for microchip lan9250 #77035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
31ad344 to
3b70da4
Compare
drivers/ethernet/eth_lan9250.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There exists WAIT_FOR macro that could perhaps used here and the other wait loops in other functions, could you check if it would make sense to use that here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a look into it and I tried to implement it. I don't know if I did something wrong but I noticed a performance hit. Receiving an IP took double the time after implementing WAIT_FOR
WAIT_FOR:
[00:00:00.019,000] <inf> eth_lan9250: LAN9250 Initialized
*** Booting Zephyr OS build v3.7.0-945-ga7dd4cb9ce3d ***
[00:00:00.019,000] <inf> net_dhcpv4_client_sample: Run dhcpv4 client
[00:00:00.019,000] <inf> net_dhcpv4_client_sample: Start on eth_lan9250@0: index=1
[00:00:10.669,000] <inf> net_dhcpv4: Received: 192.168.8.242
[00:00:10.669,000] <inf> net_dhcpv4_client_sample: Address[1]: 192.168.8.242
[00:00:10.669,000] <inf> net_dhcpv4_client_sample: Subnet[1]: 255.255.255.0
[00:00:10.669,000] <inf> net_dhcpv4_client_sample: Router[1]: 192.168.8.1
[00:00:10.669,000] <inf> net_dhcpv4_client_sample: Lease time[1]: 43200 seconds
uart:~$
while loop:
[00:00:00.074,000] <inf> eth_lan9250: LAN9250 Initialized
*** Booting Zephyr OS build v3.7.0-945-ga7dd4cb9ce3d ***
[00:00:00.074,000] <inf> net_dhcpv4_client_sample: Run dhcpv4 client
[00:00:00.074,000] <inf> net_dhcpv4_client_sample: Start on eth_lan9250@0: index=1
[00:00:05.659,000] <inf> net_dhcpv4: Received: 192.168.8.242
[00:00:05.659,000] <inf> net_dhcpv4_client_sample: Address[1]: 192.168.8.242
[00:00:05.659,000] <inf> net_dhcpv4_client_sample: Subnet[1]: 255.255.255.0
[00:00:05.659,000] <inf> net_dhcpv4_client_sample: Router[1]: 192.168.8.1
[00:00:05.659,000] <inf> net_dhcpv4_client_sample: Lease time[1]: 43200 seconds
drivers/ethernet/eth_lan9250.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment line is a bit lonely, perhaps you could add more information what it means (I see it is being used in next line so the comment refers to that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to give a bit more information this time
drivers/ethernet/eth_lan9250.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation is wrong
drivers/ethernet/eth_lan9250.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
drivers/ethernet/eth_lan9250.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here and other similar lines too, could you check them all
drivers/ethernet/eth_lan9250.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please explicitly check the return code (i.e. do not OR the ret)
drivers/ethernet/eth_lan9250.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My overall impression is that you do use a lot of busy waiting. Is there any special rationale for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the datasheet that specifies that there are timing rules back to back read-write operations 5.2.1 BACK-TO-BACK WRITE-READ CYCLES, and then I took as a baseline the delays in the ethernet-lan9250 driver.
drivers/ethernet/eth_lan9250.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to check if CONFIG_NET_BUF_DATA_SIZE is in sync with your setup as you use net_buf_add()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is needed because it is already checked how much space is available at the end of the buffer with net_buf_tailroom(). Similar to the other SPI Ethernet modules (w5500, enc28j60, eth_enc424j600)
drivers/ethernet/eth_lan9250.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LOG_DBG() shall be after declarations
drivers/ethernet/eth_lan9250.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you followed the paradigm from LAN8651 (T1S) driver. The rationale for half-duplex transmission there was the issue with driver complexity versus potential gain.
I'm just wondering if the LAN9250 has the opportunity to work full duplex (as other drivers connected via SPI)?
And another question - out of curiosity - have you tested the driver with zperf Zephyr's test program?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And just to point out - the SPI transmission with chained different buffers was the reason for considerable slow down for STM32 based SoC.
Last but not least - the LAN9250 seems to be 10/100 Mbps - I do assume that it works with 100 Mbps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And another question - out of curiosity - have you tested the driver with zperf Zephyr's test program?
No I did not test it, but I can do that too, to test it out.
However, I had a short look on the switch and the device is working at 100Mbps (FE):

Last but not least - the LAN9250 seems to be 10/100 Mbps - I do assume that it works with 100 Mbps?
I have enabled auto-negotiation & advertisement capability for both 10/100 Mbps HD/FD
drivers/ethernet/Kconfig.lan9250
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a bit off-topic. It looks like second MicroClick modules are used as based board. Is there any plan to add them to zephyr as "capes"? IIRC for LAN8651 I also wanted to add it - in a way similar to:
https://github.com/zephyrproject-rtos/zephyr/tree/main/boards/shields/mikroe_wifi_bt_click
or
https://github.com/zephyrproject-rtos/zephyr/tree/main/boards/shields/mikroe_eth_click
but the reply was that we cannot tie to any specific board (like with microe_wifi_bt_click.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lmajewski I was also thinking adding it as a shield in a second PR. At the moment I want to pass only a minimal implementation of the driver.
but the reply was that we cannot tie to any specific board (like with microe_wifi_bt_click.
I know that we should not tie drivers to specific shields but also Microship itself is using this module to demonstrate LAN9250 on their Quick Start Guide
01f1782 to
1d35288
Compare
|
Converted to draft because I noticed some unexpected behaviour, while connected to different switches/routers |
fd55383 to
117d071
Compare
|
Issue mentioned earlier is fixed xiao esp32s3 overlay: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some very minor style nits from me, other than that looks reasonable.
bc1038c to
edcbb6e
Compare
|
@auroramisic I noticed that restarting the board after flashing helps of fix the BTW this is only a basic implementation of this SPI Ethernet Controller and many features are still missing I disabled the following to be able to read the log better
LOG: |
|
@mariopaja restarting the dk did not fix the issue. And the log is still the same |
I could replicate the issue, and it was If you are using the nrf52840dk_nrf52840, please check the overlay you are using. The following overlay which is included with the provided sample: does not match nrf52840dk_nrf52840-pinctrl.dtsi |
b962452 to
e314c2c
Compare
a308018 to
85ab349
Compare
This PR adds support for LAN9250 spi ethernet controller. This driver is tested on the Mikroe ETH Click 3 https://www.mikroe.com/eth-3-click Signed-off-by: Mario Paja <[email protected]>
Add build test for microchip lan9250 Signed-off-by: Mario Paja <[email protected]>
@mariopaja please contribute a new shield definition for it when you have a chance :) |
@kartben I will do it soon :) |
|
@mariopaja If someone has the same issue with nordic in the future, the fix for the system was to add |


This PR adds support for LAN9250 spi ethernet controller.
This driver is tested on the Mikroe ETH Click 3
Console:
Nucleo-H563 Overlay:
Test application: lan9250_test_app.zip