diff mbox series

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

Message ID 20240621122045.30732-7-SkyLake.Huang@mediatek.com (mailing list archive)
State New
Headers show
Series net: phy: mediatek: Introduce mtk-phy-lib and add 2.5Gphy support | expand

Commit Message

SkyLake Huang (黃啟澤) June 21, 2024, 12:20 p.m. UTC 
From: "SkyLake.Huang" <skylake.huang@mediatek.com>

We have mtk-phy-lib.c now so that we can use LED helper functions in
mtk-ge.c(mt7531 part). It also means that mt7531/mt7981/mt7988's
Giga ethernet phys share almost the same HW LED controller design.
Also, add probe function for mt7531 so that it can initialize LED state.

Signed-off-by: SkyLake.Huang <skylake.huang@mediatek.com>
---
 drivers/net/phy/mediatek/mtk-ge.c | 105 ++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

Comments

Andrew Lunn June 22, 2024, 5:29 p.m. UTC | #1 
> +static int mt753x_phy_led_blink_set(struct phy_device *phydev, u8 index,
> +				    unsigned long *delay_on,
> +				    unsigned long *delay_off)
> +{
> +	struct mtk_gephy_priv *priv = phydev->priv;
> +	bool blinking = false;
> +	int err = 0;
> +
> +	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.

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

Do the different hardware variants have different blink speeds? If
not, maybe also move this into the common code. Otherwise maybe add a
comment in the commit message explaining the differences between the
hardware variants.

    Andrew

---
pw-bot: cr
Daniel Golle June 22, 2024, 9:03 p.m. UTC | #2 
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.
SkyLake Huang (黃啟澤) June 24, 2024, 10:38 a.m. UTC | #3 
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
diff mbox series

Patch

diff --git a/drivers/net/phy/mediatek/mtk-ge.c b/drivers/net/phy/mediatek/mtk-ge.c
index 9122899..c338bba 100644
--- a/drivers/net/phy/mediatek/mtk-ge.c
+++ b/drivers/net/phy/mediatek/mtk-ge.c
@@ -13,6 +13,10 @@ 
 #define MTK_PHY_PAGE_EXTENDED_2A30	0x2a30
 #define MTK_PHY_PAGE_EXTENDED_52B5	0x52b5
 
+struct mtk_gephy_priv {
+	unsigned long		led_state;
+};
+
 static void mtk_gephy_config_init(struct phy_device *phydev)
 {
 	/* Enable HW auto downshift */
@@ -57,6 +61,101 @@  static int mt7531_phy_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int mt7531_phy_probe(struct phy_device *phydev)
+{
+	struct mtk_gephy_priv *priv;
+
+	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(struct mtk_gephy_priv),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	phydev->priv = priv;
+
+	mtk_phy_leds_state_init(phydev);
+
+	return 0;
+}
+
+static int mt753x_phy_led_blink_set(struct phy_device *phydev, u8 index,
+				    unsigned long *delay_on,
+				    unsigned long *delay_off)
+{
+	struct mtk_gephy_priv *priv = phydev->priv;
+	bool blinking = false;
+	int err = 0;
+
+	if (index > 1)
+		return -EINVAL;
+
+	if (delay_on && delay_off && (*delay_on > 0) && (*delay_off > 0)) {
+		blinking = true;
+		*delay_on = 50;
+		*delay_off = 50;
+	}
+
+	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);
+}
+
+static int mt753x_phy_led_brightness_set(struct phy_device *phydev,
+					 u8 index, enum led_brightness value)
+{
+	struct mtk_gephy_priv *priv = phydev->priv;
+	int err;
+
+	err = mtk_phy_hw_led_blink_set(phydev, index, &priv->led_state, false);
+	if (err)
+		return err;
+
+	return mtk_phy_hw_led_on_set(phydev, index, &priv->led_state,
+				     MTK_GPHY_LED_ON_MASK, (value != LED_OFF));
+}
+
+static const unsigned long supported_triggers =
+	(BIT(TRIGGER_NETDEV_FULL_DUPLEX) |
+	 BIT(TRIGGER_NETDEV_HALF_DUPLEX) |
+	 BIT(TRIGGER_NETDEV_LINK)        |
+	 BIT(TRIGGER_NETDEV_LINK_10)     |
+	 BIT(TRIGGER_NETDEV_LINK_100)    |
+	 BIT(TRIGGER_NETDEV_LINK_1000)   |
+	 BIT(TRIGGER_NETDEV_RX)          |
+	 BIT(TRIGGER_NETDEV_TX));
+
+static int mt753x_phy_led_hw_is_supported(struct phy_device *phydev, u8 index,
+					  unsigned long rules)
+{
+	return mtk_phy_led_hw_is_supported(phydev, index, rules,
+					   supported_triggers);
+}
+
+static int mt753x_phy_led_hw_control_get(struct phy_device *phydev, u8 index,
+					 unsigned long *rules)
+{
+	struct mtk_gephy_priv *priv = phydev->priv;
+
+	return mtk_phy_led_hw_ctrl_get(phydev, index, rules, &priv->led_state,
+				       MTK_GPHY_LED_ON_SET,
+				       MTK_GPHY_LED_RX_BLINK_SET,
+				       MTK_GPHY_LED_TX_BLINK_SET);
+};
+
+static int mt753x_phy_led_hw_control_set(struct phy_device *phydev, u8 index,
+					 unsigned long rules)
+{
+	struct mtk_gephy_priv *priv = phydev->priv;
+
+	return mtk_phy_led_hw_ctrl_set(phydev, index, rules, &priv->led_state,
+				       MTK_GPHY_LED_ON_SET,
+				       MTK_GPHY_LED_RX_BLINK_SET,
+				       MTK_GPHY_LED_TX_BLINK_SET);
+};
+
 static struct phy_driver mtk_gephy_driver[] = {
 	{
 		PHY_ID_MATCH_EXACT(0x03a29412),
@@ -75,6 +174,7 @@  static struct phy_driver mtk_gephy_driver[] = {
 	{
 		PHY_ID_MATCH_EXACT(0x03a29441),
 		.name		= "MediaTek MT7531 PHY",
+		.probe		= mt7531_phy_probe,
 		.config_init	= mt7531_phy_config_init,
 		/* Interrupts are handled by the switch, not the PHY
 		 * itself.
@@ -85,6 +185,11 @@  static struct phy_driver mtk_gephy_driver[] = {
 		.resume		= genphy_resume,
 		.read_page	= mtk_phy_read_page,
 		.write_page	= mtk_phy_write_page,
+		.led_blink_set	= mt753x_phy_led_blink_set,
+		.led_brightness_set = mt753x_phy_led_brightness_set,
+		.led_hw_is_supported = mt753x_phy_led_hw_is_supported,
+		.led_hw_control_set = mt753x_phy_led_hw_control_set,
+		.led_hw_control_get = mt753x_phy_led_hw_control_get,
 	},
 };