[BPi-R3-Mini] Airoha EN8811H 2.5G PHY driver

Those are legacy/deprecated triggers. Use netdev instead, there you will then be able to control on which speeds and duplex the LED should be on and for which traffic (RX/TX) it should blink – and if possible, offload that to the hardware like it’s down for mediatek-ge-soc.

Thanks, I found out already. I can do this:

echo 1 > "/sys/class/leds/mdio-bus:0e:yellow:lan-1/link_1000"

But is there a way to do this in the dts? Or should I set default some triggers on in the driver?

Edit: probably best to inherit from u-boot…

While there is a way to define the default trigger in device tree (via the linux,default-trigger attribute) unfortunately that won’t work well for hardware-offloaded netdev – simply because the default trigger is setup at the time the LED is setup, and that’s when the PHY driver is being probed and before the netdev is actually registered… More work on kernel logic to setup default trigger will be needed to resolve this. Apart from that, the linux,default-trigger attribute merely allows setting the name of the default trigger – but not its parameters. For those reasons we resort to setup the default trigger in userspace in OpenWrt.

A few questions.

  1. Why did you move en8811h_load_firmware to config_init?
  2. Can you describe more on the driver hanging when using C45? Didn’t encounter that with my board after I convert the driver to use C45.
	/**
	 * @probe: Called during discovery.  Used to set
	 * up device-specific structures, if any
	 */
	 
	 /**
	 * @config_init: Called to initialize the PHY,
	 * including after a reset
	 */
  1. When the driver is built-in the kernel image, .probe() is called before the filesystem is ready, thus will fail loading the firmware.

  2. The power to the en8811h’s on the BPI-R3-mini can be individually controlled. I still have to figure out how to set this in the dts to switch it when eth0/1 link is set up/down. If the power is switched off and on, it would mean the driver needs to be re-probed…

I actually solved that problem yesterday, it was a mdio.lock issue.

Did you use real C45 mdio communication, or C45 over C22?

Does the en8811h work well with the genphy_c45_xxx() functions?

I think I solved it by using EPROBE_DEFER

Yeah I have it on my board and it’s running for a month now but I accidentally deleted the source code from my hard drive because of stupid reason. :sweat_smile:

firmware is on the way :wink:

https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/

and it uses the airoha subfolder like you suggested

1 Like

Thanks. Hopefully I can test it some time these days soon. I hope the 2 bugs I mentioned are dealt with…

Would be great,but even if it is the currently used one it can be updated. But having the firmware accepted allows upstreaming the driver.

Btw. You can also builtin firmware files in kernel to have driver builtin too to not wait for rootfs ready. I have done it for r3 or r4 some time ago,but seems i had not documented it in my wiki :frowning: i hope i’m able to find the tree in my git or at least the defconfig with the options

Afair it was this option,but it took me some attempts because of folder structure

https://www.kernel.org/doc/html/v4.14/driver-api/firmware/built-in-fw.html

https://cateee.net/lkddb/web-lkddb/EXTRA_FIRMWARE.html

I’m not sure eprobedefer is enough when driver is builtin.

@ericwoud is the wip branch the latest?

I would like to convert it to full C45

Yes it is, but there are many changes I still need to test.

Great, me too, but I’m converting it one step at a time… As much as possible C45. Would have done it earlier, but someone had the great idea of implementing the led functionality. :wink: which I did first.

After I tested current changes, I will start with removing air_mii_cl45_read() and air_mii_cl45_write()

But what do you suggest for air_buckpbus_reg_read() air_buckpbus_reg_read() and air_write_buf() ? They still use C22 read/write.

And do you mean by ‘full C45’ replacing genphy_read_status() with genphy_c45_read_status() and such? I would like that too, if the hardware is ok with it… I have done this already on the rtl8221b.

No, they have a newer version…

My end goal is to switch the power on/off to the chip with ip set link eth0/1 up/down. The hardware is capable of doing this.

This would also be a reason to have the firmware loading, where it is now.

I also want to confirm to the documentation as much as possible:

Keep those. I didn’t replace them.

Yes but there’s a prerequisite. Firmware must be loaded during probe.

Sadly, they are not…

1 Like

It is done. Check it in the wip branch.

Then how do you propose handling the firmware when switching the power on/off/on to the phy chip with link up/down/up?

Why is it a prerequisite?

So we can use genphy_c45_pma_read_abilities

Do you mean suspend/resume? I think it doesn’t matter as long as the PHY doesn’t get reseted. I might be wrong though.

It will be a fixed outcome anyway.

I mean, the BPI-R3 has a gpio pin which can switch on/off the power to the chip. It can be used to save power on any port not in use. I think, if the hardware is designed like this, then I should try to make this work.

No it’s not and setting the feature manually probably won’t get accepted upstream.

Oh that’s interesting. What’s the dmesg output when your turn it off and on when the board is running? Also I think that should be something to worry about after the driver is finalized.

Hmz, ok, had to also adjust en8811h_load_firmware() a bit. First check if both firmwares can be loaded, before doing anything.

So it would be something like it is now in branch wip

Look at the powering on/off issue later then.

Still not 100%, loading fails sometimes… will look at it later. May have to hold the mdio lock during the entire call to en8811h_load_firmware() perhapse…

Mhm am i wrong or is the airoha/EthMD32.dm.bin from the patch above empty? Seems it is a bug in patchwork…mail contains it