Re: [PATCH net-next 2/5] net: phy: mediatek: Move LED helper functions into mtk phy lib
From: SkyLake Huang (黃啟澤)
Date: Thu Oct 31 2024 - 00:57:25 EST
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