On Wed, 2024-10-30 at 22:46 +0100, Andrew Lunn wrote:

External email : Please do not click links or open attachments until
you have verified the sender or the content.

quoted

+void mtk_phy_leds_state_init(struct phy_device *phydev)
+{
+     int i;
+
+     for (i = 0; i < 2; ++i)
+             phydev->drv->led_hw_control_get(phydev, i, NULL);
+}

This does appear to be the same as what it is replacing, but it also
seems odd.

Why is an init function doing a get? I assume it is to do with
setting
priv->led_state? But led_state is not used in setting *rules, which
is
what led_hw_control_get() is all about. So maybe in a follow up
patch,
move the actual init code out of led_hw_control_get()?

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

I think I got your point. In a follow up patch, I'll do the following
change:

[Psuedo code]
/* Currently */
mtk_phy_led_hw_ctrl_get() {
	get_led_hw_settings(); //on & blink

	set/clear &priv->led_state according to led_hw_settings

	get *rules according to led_hw_settings
}

/* Change into */
get_led_hw_settings()

mtk_phy_leds_state_init() { /* Actual led_state init code */
	get_led_hw_settings(); //on & blink

	set/clear &priv->led_state according to led_hw_settings
}

mtk_phy_led_hw_ctrl_get() {
	get_led_hw_settings() in register; //on & blink

	get *rules according to led_hw_settings
}

BRs,
Sky