Re: [PATCH net-next 2/5] net: phy: mediatek: Move LED helper functions into mtk phy lib

[SkyLake Huang (黃啟澤)]

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.
> 
> 
> > +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@xxxxxxx>
> 
>     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