diff mbox series

[net-next,v2,2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver

Message ID 20250410100410.348-2-ansuelsmth@gmail.com (mailing list archive)
State Accepted
Commit 6a325aed130bb68790e765f923e76ec5669d2da7
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v2,1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 15 of 15 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 4 this patch: 4
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 98 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2025-04-11--03-00 (tests: 900)

Commit Message

Christian Marangi April 10, 2025, 10:04 a.m. UTC 
Airoha AN7581 SoC ship with a Switch based on the MT753x Switch embedded
in other SoC like the MT7581 and the MT7988. Similar to these they
require configuring some pin to enable LED PHYs.

Add support for the PHY ID for the Airoha embedded Switch and define a
simple probe function to toggle these pins. Also fill the LED functions
and add dedicated function to define LED polarity.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Changes v2:
- Add Reviewed-by tag
- Address suggested dependency from Russell

 drivers/net/phy/mediatek/Kconfig      |  4 +-
 drivers/net/phy/mediatek/mtk-ge-soc.c | 62 +++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)

Comments

Arnd Bergmann April 10, 2025, 10:33 a.m. UTC | #1 
On Thu, Apr 10, 2025, at 12:04, Christian Marangi wrote:
> 
>  config MEDIATEK_GE_SOC_PHY
>  	tristate "MediaTek SoC Ethernet PHYs"
> -	depends on (ARM64 && ARCH_MEDIATEK && NVMEM_MTK_EFUSE) || COMPILE_TEST
> +	depends on ARM64 || COMPILE_TEST
> +	depends on ARCH_AIROHA || (ARCH_MEDIATEK && NVMEM_MTK_EFUSE) || \
> +		   COMPILE_TEST
>  	select MTK_NET_PHYLIB
>  	help
>  	  Supports MediaTek SoC built-in Gigabit Ethernet PHYs.

This now also fails for non-compile-test builds with
NVMEM=m, ARCH_MEDIATEK=n, ARCH_AIROHA=y and MEDIATEK_GE_SOC_PHY=y.

     Arnd
Simon Horman April 10, 2025, 7:07 p.m. UTC | #2 
On Thu, Apr 10, 2025 at 12:04:04PM +0200, Christian Marangi wrote:
> Airoha AN7581 SoC ship with a Switch based on the MT753x Switch embedded
> in other SoC like the MT7581 and the MT7988. Similar to these they
> require configuring some pin to enable LED PHYs.
> 
> Add support for the PHY ID for the Airoha embedded Switch and define a
> simple probe function to toggle these pins. Also fill the LED functions
> and add dedicated function to define LED polarity.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

...

> diff --git a/drivers/net/phy/mediatek/mtk-ge-soc.c b/drivers/net/phy/mediatek/mtk-ge-soc.c

...

> +static int an7581_phy_led_polarity_set(struct phy_device *phydev, int index,
> +				       unsigned long modes)
> +{
> +	u32 mode;
> +	u16 val;
> +
> +	if (index >= MTK_PHY_MAX_LEDS)
> +		return -EINVAL;
> +
> +	for_each_set_bit(mode, &modes, __PHY_LED_MODES_NUM) {
> +		switch (mode) {
> +		case PHY_LED_ACTIVE_LOW:
> +			val = MTK_PHY_LED_ON_POLARITY;
> +			break;
> +		case PHY_LED_ACTIVE_HIGH:
> +			val = 0;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return phy_modify_mmd(phydev, MDIO_MMD_VEND2, index ?
> +			      MTK_PHY_LED1_ON_CTRL : MTK_PHY_LED0_ON_CTRL,
> +			      MTK_PHY_LED_ON_POLARITY, val);

Hi Christian,

Perhaps this cannot occur in practice, but if the for_each_set_bit
loop iterates zero times then val will be used uninitialised here.

Flagged by Smatch.

> +}

...
Christian Marangi April 14, 2025, 10:30 a.m. UTC | #3 
On Thu, Apr 10, 2025 at 08:07:33PM +0100, Simon Horman wrote:
> On Thu, Apr 10, 2025 at 12:04:04PM +0200, Christian Marangi wrote:
> > Airoha AN7581 SoC ship with a Switch based on the MT753x Switch embedded
> > in other SoC like the MT7581 and the MT7988. Similar to these they
> > require configuring some pin to enable LED PHYs.
> > 
> > Add support for the PHY ID for the Airoha embedded Switch and define a
> > simple probe function to toggle these pins. Also fill the LED functions
> > and add dedicated function to define LED polarity.
> > 
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> 
> ...
> 
> > diff --git a/drivers/net/phy/mediatek/mtk-ge-soc.c b/drivers/net/phy/mediatek/mtk-ge-soc.c
> 
> ...
> 
> > +static int an7581_phy_led_polarity_set(struct phy_device *phydev, int index,
> > +				       unsigned long modes)
> > +{
> > +	u32 mode;
> > +	u16 val;
> > +
> > +	if (index >= MTK_PHY_MAX_LEDS)
> > +		return -EINVAL;
> > +
> > +	for_each_set_bit(mode, &modes, __PHY_LED_MODES_NUM) {
> > +		switch (mode) {
> > +		case PHY_LED_ACTIVE_LOW:
> > +			val = MTK_PHY_LED_ON_POLARITY;
> > +			break;
> > +		case PHY_LED_ACTIVE_HIGH:
> > +			val = 0;
> > +			break;
> > +		default:
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	return phy_modify_mmd(phydev, MDIO_MMD_VEND2, index ?
> > +			      MTK_PHY_LED1_ON_CTRL : MTK_PHY_LED0_ON_CTRL,
> > +			      MTK_PHY_LED_ON_POLARITY, val);
> 
> Hi Christian,
> 
> Perhaps this cannot occur in practice, but if the for_each_set_bit
> loop iterates zero times then val will be used uninitialised here.
> 
> Flagged by Smatch.
> 
> > +}
> 
> ...

Almost impossible but yes I will post a follow-up patch fixing this!
diff mbox series

Patch

diff --git a/drivers/net/phy/mediatek/Kconfig b/drivers/net/phy/mediatek/Kconfig
index 6a4c2b328c41..4308002bb82c 100644
--- a/drivers/net/phy/mediatek/Kconfig
+++ b/drivers/net/phy/mediatek/Kconfig
@@ -15,7 +15,9 @@  config MEDIATEK_GE_PHY
 
 config MEDIATEK_GE_SOC_PHY
 	tristate "MediaTek SoC Ethernet PHYs"
-	depends on (ARM64 && ARCH_MEDIATEK && NVMEM_MTK_EFUSE) || COMPILE_TEST
+	depends on ARM64 || COMPILE_TEST
+	depends on ARCH_AIROHA || (ARCH_MEDIATEK && NVMEM_MTK_EFUSE) || \
+		   COMPILE_TEST
 	select MTK_NET_PHYLIB
 	help
 	  Supports MediaTek SoC built-in Gigabit Ethernet PHYs.
diff --git a/drivers/net/phy/mediatek/mtk-ge-soc.c b/drivers/net/phy/mediatek/mtk-ge-soc.c
index 175cf5239bba..fd0e447ffce7 100644
--- a/drivers/net/phy/mediatek/mtk-ge-soc.c
+++ b/drivers/net/phy/mediatek/mtk-ge-soc.c
@@ -11,8 +11,11 @@ 
 #include "../phylib.h"
 #include "mtk.h"
 
+#define MTK_PHY_MAX_LEDS			2
+
 #define MTK_GPHY_ID_MT7981			0x03a29461
 #define MTK_GPHY_ID_MT7988			0x03a29481
+#define MTK_GPHY_ID_AN7581			0x03a294c1
 
 #define MTK_EXT_PAGE_ACCESS			0x1f
 #define MTK_PHY_PAGE_STANDARD			0x0000
@@ -1406,6 +1409,53 @@  static int mt7981_phy_probe(struct phy_device *phydev)
 	return mt798x_phy_calibration(phydev);
 }
 
+static int an7581_phy_probe(struct phy_device *phydev)
+{
+	struct mtk_socphy_priv *priv;
+	struct pinctrl *pinctrl;
+
+	/* Toggle pinctrl to enable PHY LED */
+	pinctrl = devm_pinctrl_get_select(&phydev->mdio.dev, "gbe-led");
+	if (IS_ERR(pinctrl))
+		dev_err(&phydev->mdio.bus->dev,
+			"Failed to setup PHY LED pinctrl\n");
+
+	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	phydev->priv = priv;
+
+	return 0;
+}
+
+static int an7581_phy_led_polarity_set(struct phy_device *phydev, int index,
+				       unsigned long modes)
+{
+	u32 mode;
+	u16 val;
+
+	if (index >= MTK_PHY_MAX_LEDS)
+		return -EINVAL;
+
+	for_each_set_bit(mode, &modes, __PHY_LED_MODES_NUM) {
+		switch (mode) {
+		case PHY_LED_ACTIVE_LOW:
+			val = MTK_PHY_LED_ON_POLARITY;
+			break;
+		case PHY_LED_ACTIVE_HIGH:
+			val = 0;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return phy_modify_mmd(phydev, MDIO_MMD_VEND2, index ?
+			      MTK_PHY_LED1_ON_CTRL : MTK_PHY_LED0_ON_CTRL,
+			      MTK_PHY_LED_ON_POLARITY, val);
+}
+
 static struct phy_driver mtk_socphy_driver[] = {
 	{
 		PHY_ID_MATCH_EXACT(MTK_GPHY_ID_MT7981),
@@ -1441,6 +1491,17 @@  static struct phy_driver mtk_socphy_driver[] = {
 		.led_hw_control_set = mt798x_phy_led_hw_control_set,
 		.led_hw_control_get = mt798x_phy_led_hw_control_get,
 	},
+	{
+		PHY_ID_MATCH_EXACT(MTK_GPHY_ID_AN7581),
+		.name		= "Airoha AN7581 PHY",
+		.probe		= an7581_phy_probe,
+		.led_blink_set	= mt798x_phy_led_blink_set,
+		.led_brightness_set = mt798x_phy_led_brightness_set,
+		.led_hw_is_supported = mt798x_phy_led_hw_is_supported,
+		.led_hw_control_set = mt798x_phy_led_hw_control_set,
+		.led_hw_control_get = mt798x_phy_led_hw_control_get,
+		.led_polarity_set = an7581_phy_led_polarity_set,
+	},
 };
 
 module_phy_driver(mtk_socphy_driver);
@@ -1448,6 +1509,7 @@  module_phy_driver(mtk_socphy_driver);
 static const struct mdio_device_id __maybe_unused mtk_socphy_tbl[] = {
 	{ PHY_ID_MATCH_EXACT(MTK_GPHY_ID_MT7981) },
 	{ PHY_ID_MATCH_EXACT(MTK_GPHY_ID_MT7988) },
+	{ PHY_ID_MATCH_EXACT(MTK_GPHY_ID_AN7581) },
 	{ }
 };