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

Banana Pi Router design BPI-R3/R3 Mini(MT7986)
21

So you think I’ll need mutex_lock(&phydev->mdio.bus->mdio_lock); also?

22

Thanks, I’ll definitely look into that.

23

https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/5e9bfbf223f19246e7ef8a991062e422bdbd1bfa

BTW mediatek uploaded new driver about 3 months ago, that one looks more clean and would be good to used as reference.

24

I saw that one before, but actually there are not so many differences. The resulting cleanup of that version would result in what I have now anyway. The code is pretty clean now, but I can still do more.

25

@castiel652

Do you have an example of how the devicetree would look like?

something like this? but how to set the trigger? Each of the three leds, 1 at 100, 1 at 1000 and 1 at 2500M

           ethernet-phy@14 {
                reg = <14>;

                leds {
                    #address-cells = <1>;
                    #size-cells = <0>;

                    led@0 {
                        reg = <0>;
                        color = <LED_COLOR_ID_GREEN>;
                        function = LED_FUNCTION_LAN;
                        default-state = "keep";
                    };
                };
            };

Edit

I guess should add

linux,default-trigger = "link_100";
linux,default-trigger = "link_100o";
linux,default-trigger = "link_2500";

Respectively to the three leds…

26

Device tree looks correct.

I haven’t thought about that yet. Didn’t have time to work on the driver more.

27

I’ve done this but have to try…

I needed these commits:

28

@castiel652

I added the led control functions and added the leds like so:

	phy14: ethernet-phy@14 {
		reg = <14>;
		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
		reset-assert-us = <10000>;
		reset-deassert-us = <20000>;
		phy-mode = "2500base-x";
		full-duplex;
		pause;
		leds {
			#address-cells = <1>;
			#size-cells = <0>;

			led@0 { /* en8811_a_gpio5 */
				reg = <0>;
				color = <LED_COLOR_ID_YELLOW>;
				function = LED_FUNCTION_LAN;
				function-enumerator = <1>;
				default-state = "keep";
			};
			led@1 { /* en8811_a_gpio4 */
				reg = <1>;
				color = <LED_COLOR_ID_GREEN>;
				function = LED_FUNCTION_LAN;
				function-enumerator = <2>;
				default-state = "keep";
			};
			led@2 { /*  en8811_a_gpio3, not connected */
				reg = <2>;
				color = <LED_COLOR_ID_WHITE>;
				function = LED_FUNCTION_LAN;
				function-enumerator = <3>;
				default-state = "keep";
			};
		};
	};

I can swtich the led on/off like so:

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

I have set CONFIG_LEDS_TRIGGERS=y

I see the triggers:

[root@bpir3m ~]# cat "/sys/class/leds/mdio-bus:0e:yellow:lan-1/trigger" 
none kbd-scrolllock kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock kbd-altgrlock kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock kbd-ctrlllock kbd-ctrlrlock timer oneshot heartbeat backlight cpu cpu0 cpu1 cpu2 cpu3 default-on netdev mmc0 rfkill-any rfkill-none phy0rx phy0tx phy0assoc phy0radio phy0tpt rfkill0 phy1rx phy1tx phy1assoc phy1radio phy1tpt rfkill1 mdio-bus:0e:link mdio-bus:0e:2.5Gbps [mdio-bus:0e:1Gbps] mdio-bus:0e:100Mbps mdio-bus:0f:link mdio-bus:0f:2.5Gbps mdio-bus:0f:1Gbps mdio-bus:0f:100Mbps

I can do this:

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

But never is .led_hw_is_supported() even being called. So then also .led_hw_control_set() and .led_hw_control_get() are not called.

What am I missing?

29

Can you check if these all are defined?

Is this called?

30 
CONFIG_LEDS_TRIGGERS=y

	.led_hw_is_supported 	= en8811h_led_hw_is_supported,
	.led_hw_control_set 	= en8811h_led_hw_control_set,
	.led_hw_control_get 	= en8811h_led_hw_control_get,

Yep. The leds are added and I can control them, just no hw_control on them

31

Did you set CONFIG_PHYLIB_LEDS

32 
CONFIG_LED_TRIGGER_PHY=y
CONFIG_PHYLIB_LEDS=y
33

Do you get a value in err?

34

Hang on @castiel652 @frank-w, I think I got something:

[root@bpir3m ~]# ls "/sys/class/leds/mdio-bus:0e:yellow:lan-1/"
brightness  color  device  max_brightness  power  subsystem  trigger  uevent

[root@bpir3m ~]# echo "netdev" > /sys/class/leds/mdio-bus\:0e\:yellow\:lan-1/trigger 

[root@bpir3m ~]# ls "/sys/class/leds/mdio-bus:0e:yellow:lan-1/"
brightness  device	 full_duplex  interval	link_10   link_1000  max_brightness  power  subsystem  tx
color	    device_name  half_duplex  link	link_100  link_2500  offloaded	     rx     trigger    uevent

Now I have link_1000, etc, which I was looking for in the first place… I need to use these… How?

Edit:

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

Seems to do the trick

But how to set this in the dts?

			led@0 { /* en8811_a_gpio5 */
				reg = <0>;
				color = <LED_COLOR_ID_YELLOW>;
				function = LED_FUNCTION_LAN;
				function-enumerator = <1>;
				default-state = "keep";
				linux,default-trigger = "netdev";
			};
			led@1 { /* en8811_a_gpio4 */
				reg = <1>;
				color = <LED_COLOR_ID_GREEN>;
				function = LED_FUNCTION_LAN;
				function-enumerator = <2>;
				default-state = "keep";
				linux,default-trigger = "netdev";
			};

But then how to do the echo 1 > link_1000 part in the devicetree?

Edit2:

Somehow, setting the netdev trigger in the devicetree does not work yet, maybe it is too early (firmware not loaded yet?).

35

The trigger is set to netdev in initialization,so just drop it

Which value do you have?

36

I also thought so, but I still have to set it.

37

So either the code is not executed or overidden afterwards …have you added debug there? I guess these callbacks are are not set

38

I think setting ‘hw_control_trigger’ is different then setting ‘trigger’

39

What’s the value of offloaded?

40

So I have cleaned up again, it was getting messy.

So I think only problem, is that the trigger to netdev in dts is too early, otherwise I think this is ok…

I have done the led_state in an array. That makes the code easier to read if there are more then 2 leds.

Implemented using the mdio.lock. Now I also solved the hanging problem.