Re: [PATCH net-next v8 06/13] net: phy: mediatek: Hook LED helper functions in mtk-ge.c

["SkyLake Huang (黃啟澤)" <SkyLake.Huang@mediatek.com>]

On Sat, 2024-06-22 at 22:03 +0100, Daniel Golle wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On Sat, Jun 22, 2024 at 07:29:45PM +0200, Andrew Lunn wrote:
> > > [...]
> > > +if (index > 1)
> > > +return -EINVAL;
> > > +
> > 
> > It looks like this test could be moved into the common code. It
> seems
> > like all variants have a single LED.
> 
> Exactly two LEDs, which is what index > 1 checks for, but yes, it
> should be moved to common code.
> 
I'll add mtk_phy_led_num_dly_cfg() in mtk-phy-lib.c like this:

int mtk_phy_led_num_dly_cfg(u8 index, unsigned long *delay_on,
			    unsigned long *delay_off, bool *blinking)
{
	if (index > 1)
		return -EINVAL;

	if (delay_on && delay_off && (*delay_on > 0) && (*delay_off >
0)) {
		*blinking = true;
		*delay_on = 50;
		*delay_off = 50;
	}

	return 0;
}
EXPORT_SYMBOL_GPL(mtk_phy_led_num_dly_cfg);

Also fix *_phy_led_blink_set() in mtk-ge.c/mtk-ge-soc.c/mtk-2p5ge.c
like this:

static int mt798x_phy_led_blink_set(struct phy_device *phydev, u8
index,
				    unsigned long *delay_on,
				    unsigned long *delay_off)
{
	struct mtk_socphy_priv *priv = phydev->priv;
	bool blinking = false;
	int err = 0;

	err = mtk_phy_led_num_dly_cfg(index, delay_on, delay_off,
&blinking);
	if (err < 0)
		return err;

	err = mtk_phy_hw_led_blink_set(phydev, index, &priv->led_state,
				       blinking);
	if (err)
		return err;

	return mtk_phy_hw_led_on_set(phydev, index, &priv->led_state,
				     MTK_GPHY_LED_ON_MASK, false);
}

BRs,
Sky