On Fri, Dec 13, 2024 at 11:15:09PM +0100, Marek Vasut wrote:
> On 10/3/24 2:06 PM, Andrew Lunn wrote:
> > On Tue, Oct 01, 2024 at 04:45:23AM +0200, Marek Vasut wrote:
> > > The trigger_data->hw_control indicates whether the LED is controlled by HW
> > > offload, i.e. the PHY. The trigger_data->hw_control = can_hw_control() is
> > > currently called only from netdev_led_attr_store(), i.e. when writing any
> > > sysfs attribute of the netdev trigger instance associated with a PHY LED.
> > >
> > > The can_hw_control() calls validate_net_dev() which internally calls
> > > led_cdev->hw_control_get_device(), which is phy_led_hw_control_get_device()
> > > for PHY LEDs. The phy_led_hw_control_get_device() returns NULL if the PHY
> > > is not attached.
> > >
> > > At least in case of DWMAC (STM32MP, iMX8M, ...), the PHY device is attached
> > > only when the interface is brought up and is detached again when the
> > > interface is brought down. In case e.g. udev rules configure the netdev
> > > LED trigger sysfs attributes before the interface is brought up, then when
> > > the interface is brought up, the LEDs are not blinking.
> > >
> > > This is because trigger_data->hw_control = can_hw_control() was called
> > > when udev wrote the sysfs attribute files, before the interface was up,
> > > so can_hw_control() resp. validate_net_dev() returned false, and the
> > > trigger_data->hw_control = can_hw_control() was never called again to
> > > update the trigger_data->hw_control content and let the offload take
> > > over the LED blinking.
> > >
> > > Call data->hw_control = can_hw_control() from netdev_trig_notify() to
> > > update the offload capability of the LED when the UP notification arrives.
> > > This makes the LEDs blink after the interface is brought up.
> > >
> > > On STM32MP13xx with RTL8211F, it is enough to have the following udev rule
> > > in place, boot the machine with cable plugged in, and the LEDs won't work
> > > without this patch once the interface is brought up, even if they should:
> > > "
> > > ACTION=="add", SUBSYSTEM=="leds", KERNEL=="stmmac-0:01:green:wan", ATTR{trigger}="netdev", ATTR{link_10}="1", ATTR{link_100}="1", ATTR{link_1000}="1", ATTR{device_name}="end0"
> > > ACTION=="add", SUBSYSTEM=="leds", KERNEL=="stmmac-0:01:yellow:wan", ATTR{trigger}="netdev", ATTR{rx}="1", ATTR{tx}="1", ATTR{device_name}="end0"
> > > "
> > >
> > > Signed-off-by: Marek Vasut <marex@xxxxxxx>
> >
> > Reviewed-by: Andrew Lunn <andrew@xxxxxxx>
> Is there anything blocking this patch from being picked up ?
I think this should be going via the LED Maintainer. Please check with
Lee.
Andrew
