* [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver
@ 2025-04-10 10:04 Christian Marangi
2025-04-10 10:04 ` [net-next PATCH v2 2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver Christian Marangi
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Christian Marangi @ 2025-04-10 10:04 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Daniel Golle,
Qingfang Deng, SkyLake Huang, Matthias Brugger,
AngeloGioacchino Del Regno, Christian Marangi, Randy Dunlap,
Simon Horman, Arnd Bergmann, netdev, linux-kernel,
linux-arm-kernel, linux-mediatek
When commit 462a3daad679 ("net: phy: mediatek: fix compile-test
dependencies") fixed the dependency, it should have also introduced
an or on COMPILE_TEST to permit this driver to be compile-tested even if
NVMEM_MTK_EFUSE wasn't selected. The driver makes use of NVMEM API that
are always compiled (return error) so the driver can actually be
compiled even without that config.
Fix and simplify the dependency condition of this kernel config.
Fixes: 462a3daad679 ("net: phy: mediatek: fix compile-test dependencies")
Acked-by: Daniel Golle <daniel@makrotopia.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Changes v2:
- Add Ack and Reviewed-by tag
- Address suggested dependency from Russell
drivers/net/phy/mediatek/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/phy/mediatek/Kconfig b/drivers/net/phy/mediatek/Kconfig
index 2a8ac5aed0f8..6a4c2b328c41 100644
--- a/drivers/net/phy/mediatek/Kconfig
+++ b/drivers/net/phy/mediatek/Kconfig
@@ -15,8 +15,7 @@ config MEDIATEK_GE_PHY
config MEDIATEK_GE_SOC_PHY
tristate "MediaTek SoC Ethernet PHYs"
- depends on (ARM64 && ARCH_MEDIATEK) || COMPILE_TEST
- depends on NVMEM_MTK_EFUSE
+ depends on (ARM64 && ARCH_MEDIATEK && NVMEM_MTK_EFUSE) || COMPILE_TEST
select MTK_NET_PHYLIB
help
Supports MediaTek SoC built-in Gigabit Ethernet PHYs.
--
2.48.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [net-next PATCH v2 2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver
2025-04-10 10:04 [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver Christian Marangi
@ 2025-04-10 10:04 ` Christian Marangi
2025-04-10 10:33 ` Arnd Bergmann
2025-04-10 19:07 ` Simon Horman
2025-04-10 10:31 ` [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver Arnd Bergmann
2025-04-12 3:30 ` patchwork-bot+netdevbpf
2 siblings, 2 replies; 11+ messages in thread
From: Christian Marangi @ 2025-04-10 10:04 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Daniel Golle,
Qingfang Deng, SkyLake Huang, Matthias Brugger,
AngeloGioacchino Del Regno, Christian Marangi, Randy Dunlap,
Simon Horman, Arnd Bergmann, netdev, linux-kernel,
linux-arm-kernel, linux-mediatek
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(-)
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) },
{ }
};
--
2.48.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver
2025-04-10 10:04 [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver Christian Marangi
2025-04-10 10:04 ` [net-next PATCH v2 2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver Christian Marangi
@ 2025-04-10 10:31 ` Arnd Bergmann
2025-04-10 10:40 ` Christian Marangi
2025-04-12 3:30 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2025-04-10 10:31 UTC (permalink / raw)
To: Christian Marangi, Andrew Lunn, Heiner Kallweit, Russell King,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Daniel Golle, Qingfang Deng, SkyLake Huang, Matthias Brugger,
AngeloGioacchino Del Regno, Randy Dunlap, Simon Horman, Netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
On Thu, Apr 10, 2025, at 12:04, Christian Marangi wrote:
> When commit 462a3daad679 ("net: phy: mediatek: fix compile-test
> dependencies") fixed the dependency, it should have also introduced
> an or on COMPILE_TEST to permit this driver to be compile-tested even if
> NVMEM_MTK_EFUSE wasn't selected
Why does this matter? NVMEM_MTK_EFUSE can be enabled for both
allmodconfig and randconfig builds on any architecture, so you
get build coverage either way, it's just a little less likely
to be enabled in randconfig I guess?
> diff --git a/drivers/net/phy/mediatek/Kconfig b/drivers/net/phy/mediatek/Kconfig
> index 2a8ac5aed0f8..6a4c2b328c41 100644
> --- a/drivers/net/phy/mediatek/Kconfig
> +++ b/drivers/net/phy/mediatek/Kconfig
> @@ -15,8 +15,7 @@ config MEDIATEK_GE_PHY
>
> config MEDIATEK_GE_SOC_PHY
> tristate "MediaTek SoC Ethernet PHYs"
> - depends on (ARM64 && ARCH_MEDIATEK) || COMPILE_TEST
> - depends on NVMEM_MTK_EFUSE
> + depends on (ARM64 && ARCH_MEDIATEK && NVMEM_MTK_EFUSE) || COMPILE_TEST
> select MTK_NET_PHYLIB
> help
> Supports MediaTek SoC built-in Gigabit Ethernet PHYs.
> --
I would expect this to break the build with CONFIG_NVMEM=m
and MEDIATEK_GE_SOC_PHY=y.
The normal thing here would be to have a dependency on
CONFIG_NVMEM in place of the NVMEM_MTK_EFUSE dependency,
or possible on 'NVMEM || !NVMEM' if you want to make it
more likely to be enabled in randconfig builds.
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [net-next PATCH v2 2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver
2025-04-10 10:04 ` [net-next PATCH v2 2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver Christian Marangi
@ 2025-04-10 10:33 ` Arnd Bergmann
2025-04-10 19:07 ` Simon Horman
1 sibling, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2025-04-10 10:33 UTC (permalink / raw)
To: Christian Marangi, Andrew Lunn, Heiner Kallweit, Russell King,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Daniel Golle, Qingfang Deng, SkyLake Huang, Matthias Brugger,
AngeloGioacchino Del Regno, Randy Dunlap, Simon Horman, Netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver
2025-04-10 10:31 ` [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver Arnd Bergmann
@ 2025-04-10 10:40 ` Christian Marangi
2025-04-10 10:44 ` Christian Marangi
2025-04-10 12:46 ` Arnd Bergmann
0 siblings, 2 replies; 11+ messages in thread
From: Christian Marangi @ 2025-04-10 10:40 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Daniel Golle,
Qingfang Deng, SkyLake Huang, Matthias Brugger,
AngeloGioacchino Del Regno, Randy Dunlap, Simon Horman, Netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
On Thu, Apr 10, 2025 at 12:31:05PM +0200, Arnd Bergmann wrote:
> On Thu, Apr 10, 2025, at 12:04, Christian Marangi wrote:
> > When commit 462a3daad679 ("net: phy: mediatek: fix compile-test
> > dependencies") fixed the dependency, it should have also introduced
> > an or on COMPILE_TEST to permit this driver to be compile-tested even if
> > NVMEM_MTK_EFUSE wasn't selected
>
> Why does this matter? NVMEM_MTK_EFUSE can be enabled for both
> allmodconfig and randconfig builds on any architecture, so you
> get build coverage either way, it's just a little less likely
> to be enabled in randconfig I guess?
>
If we base stuff on the fact that everything is selected or that a
random config by luck selects it, then COMPILE_TEST doesn't make sense
at all.
For my personal test, I wanted to test the driver on a simple x86 build
without having to depend on ARCH or having to cross compile. Won't
happen on real world scenario? Totally. I should be able to compile it?
Yes.
> > diff --git a/drivers/net/phy/mediatek/Kconfig b/drivers/net/phy/mediatek/Kconfig
> > index 2a8ac5aed0f8..6a4c2b328c41 100644
> > --- a/drivers/net/phy/mediatek/Kconfig
> > +++ b/drivers/net/phy/mediatek/Kconfig
> > @@ -15,8 +15,7 @@ config MEDIATEK_GE_PHY
> >
> > config MEDIATEK_GE_SOC_PHY
> > tristate "MediaTek SoC Ethernet PHYs"
> > - depends on (ARM64 && ARCH_MEDIATEK) || COMPILE_TEST
> > - depends on NVMEM_MTK_EFUSE
> > + depends on (ARM64 && ARCH_MEDIATEK && NVMEM_MTK_EFUSE) || COMPILE_TEST
> > select MTK_NET_PHYLIB
> > help
> > Supports MediaTek SoC built-in Gigabit Ethernet PHYs.
> > --
>
> I would expect this to break the build with CONFIG_NVMEM=m
> and MEDIATEK_GE_SOC_PHY=y.
>
> The normal thing here would be to have a dependency on
> CONFIG_NVMEM in place of the NVMEM_MTK_EFUSE dependency,
> or possible on 'NVMEM || !NVMEM' if you want to make it
> more likely to be enabled in randconfig builds.
>
The big idea of these dependency is that... In MTK the internal PHY of
the switch needs calibration or it won't work hence it doesn't make
sense to select the PHY as it won't ever work without the NVMEM driver.
But from a compile test view where we only evaluate if the driver have
compilation error or other kind of warning, we should not care...
--
Ansuel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver
2025-04-10 10:40 ` Christian Marangi
@ 2025-04-10 10:44 ` Christian Marangi
2025-04-10 12:01 ` Arnd Bergmann
2025-04-10 12:46 ` Arnd Bergmann
1 sibling, 1 reply; 11+ messages in thread
From: Christian Marangi @ 2025-04-10 10:44 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Daniel Golle,
Qingfang Deng, SkyLake Huang, Matthias Brugger,
AngeloGioacchino Del Regno, Randy Dunlap, Simon Horman, Netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
On Thu, Apr 10, 2025 at 12:40:15PM +0200, Christian Marangi wrote:
> On Thu, Apr 10, 2025 at 12:31:05PM +0200, Arnd Bergmann wrote:
> > On Thu, Apr 10, 2025, at 12:04, Christian Marangi wrote:
> > > When commit 462a3daad679 ("net: phy: mediatek: fix compile-test
> > > dependencies") fixed the dependency, it should have also introduced
> > > an or on COMPILE_TEST to permit this driver to be compile-tested even if
> > > NVMEM_MTK_EFUSE wasn't selected
> >
> > Why does this matter? NVMEM_MTK_EFUSE can be enabled for both
> > allmodconfig and randconfig builds on any architecture, so you
> > get build coverage either way, it's just a little less likely
> > to be enabled in randconfig I guess?
> >
>
> If we base stuff on the fact that everything is selected or that a
> random config by luck selects it, then COMPILE_TEST doesn't make sense
> at all.
>
> For my personal test, I wanted to test the driver on a simple x86 build
> without having to depend on ARCH or having to cross compile. Won't
> happen on real world scenario? Totally. I should be able to compile it?
> Yes.
>
> > > diff --git a/drivers/net/phy/mediatek/Kconfig b/drivers/net/phy/mediatek/Kconfig
> > > index 2a8ac5aed0f8..6a4c2b328c41 100644
> > > --- a/drivers/net/phy/mediatek/Kconfig
> > > +++ b/drivers/net/phy/mediatek/Kconfig
> > > @@ -15,8 +15,7 @@ config MEDIATEK_GE_PHY
> > >
> > > config MEDIATEK_GE_SOC_PHY
> > > tristate "MediaTek SoC Ethernet PHYs"
> > > - depends on (ARM64 && ARCH_MEDIATEK) || COMPILE_TEST
> > > - depends on NVMEM_MTK_EFUSE
> > > + depends on (ARM64 && ARCH_MEDIATEK && NVMEM_MTK_EFUSE) || COMPILE_TEST
> > > select MTK_NET_PHYLIB
> > > help
> > > Supports MediaTek SoC built-in Gigabit Ethernet PHYs.
> > > --
> >
> > I would expect this to break the build with CONFIG_NVMEM=m
> > and MEDIATEK_GE_SOC_PHY=y.
> >
> > The normal thing here would be to have a dependency on
> > CONFIG_NVMEM in place of the NVMEM_MTK_EFUSE dependency,
> > or possible on 'NVMEM || !NVMEM' if you want to make it
> > more likely to be enabled in randconfig builds.
> >
>
> The big idea of these dependency is that... In MTK the internal PHY of
> the switch needs calibration or it won't work hence it doesn't make
> sense to select the PHY as it won't ever work without the NVMEM driver.
>
> But from a compile test view where we only evaluate if the driver have
> compilation error or other kind of warning, we should not care...
>
Also 99% I could be wrong but from what I can see in NVMEM kconfig,
NVMEM is not tristate but only bool? So NVMEM=m is not a thing?
--
Ansuel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver
2025-04-10 10:44 ` Christian Marangi
@ 2025-04-10 12:01 ` Arnd Bergmann
0 siblings, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2025-04-10 12:01 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Daniel Golle,
Qingfang Deng, SkyLake Huang, Matthias Brugger,
AngeloGioacchino Del Regno, Randy Dunlap, Simon Horman, Netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
On Thu, Apr 10, 2025, at 12:44, Christian Marangi wrote:
> On Thu, Apr 10, 2025 at 12:40:15PM +0200, Christian Marangi wrote:
>
> Also 99% I could be wrong but from what I can see in NVMEM kconfig,
> NVMEM is not tristate but only bool? So NVMEM=m is not a thing?
Ah, I missed that. In this case your patches are both fine.
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver
2025-04-10 10:40 ` Christian Marangi
2025-04-10 10:44 ` Christian Marangi
@ 2025-04-10 12:46 ` Arnd Bergmann
1 sibling, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2025-04-10 12:46 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Daniel Golle,
Qingfang Deng, SkyLake Huang, Matthias Brugger,
AngeloGioacchino Del Regno, Randy Dunlap, Simon Horman, Netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
On Thu, Apr 10, 2025, at 12:40, Christian Marangi wrote:
> On Thu, Apr 10, 2025 at 12:31:05PM +0200, Arnd Bergmann wrote:
>> On Thu, Apr 10, 2025, at 12:04, Christian Marangi wrote:
>> > When commit 462a3daad679 ("net: phy: mediatek: fix compile-test
>> > dependencies") fixed the dependency, it should have also introduced
>> > an or on COMPILE_TEST to permit this driver to be compile-tested even if
>> > NVMEM_MTK_EFUSE wasn't selected
>>
>> Why does this matter? NVMEM_MTK_EFUSE can be enabled for both
>> allmodconfig and randconfig builds on any architecture, so you
>> get build coverage either way, it's just a little less likely
>> to be enabled in randconfig I guess?
>>
>
> If we base stuff on the fact that everything is selected or that a
> random config by luck selects it, then COMPILE_TEST doesn't make sense
> at all.
>
> For my personal test, I wanted to test the driver on a simple x86 build
> without having to depend on ARCH or having to cross compile. Won't
> happen on real world scenario? Totally. I should be able to compile it?
> Yes.
Being able to compile test the driver yourself is clearly
a good idea, I just don't think that requires the dependency to
be conditional here.
>> I would expect this to break the build with CONFIG_NVMEM=m
>> and MEDIATEK_GE_SOC_PHY=y.
>>
>> The normal thing here would be to have a dependency on
>> CONFIG_NVMEM in place of the NVMEM_MTK_EFUSE dependency,
>> or possible on 'NVMEM || !NVMEM' if you want to make it
>> more likely to be enabled in randconfig builds.
>>
>
> The big idea of these dependency is that... In MTK the internal PHY of
> the switch needs calibration or it won't work hence it doesn't make
> sense to select the PHY as it won't ever work without the NVMEM driver.
That's not how dependencies normally work: the driver also fails
to work correctly if you are missing the correct pinctrl or led
driver, or the syscon, yet you don't list them explicitly because
you just need them for the machine to work correctly.
The driver dependencies should just list what you need to build,
regardless of whether you are compile-testing or building a driver
that is going to be used.
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [net-next PATCH v2 2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver
2025-04-10 10:04 ` [net-next PATCH v2 2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver Christian Marangi
2025-04-10 10:33 ` Arnd Bergmann
@ 2025-04-10 19:07 ` Simon Horman
2025-04-14 10:30 ` Christian Marangi
1 sibling, 1 reply; 11+ messages in thread
From: Simon Horman @ 2025-04-10 19:07 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Daniel Golle,
Qingfang Deng, SkyLake Huang, Matthias Brugger,
AngeloGioacchino Del Regno, Randy Dunlap, Arnd Bergmann, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
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.
> +}
...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver
2025-04-10 10:04 [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver Christian Marangi
2025-04-10 10:04 ` [net-next PATCH v2 2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver Christian Marangi
2025-04-10 10:31 ` [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver Arnd Bergmann
@ 2025-04-12 3:30 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-04-12 3:30 UTC (permalink / raw)
To: Christian Marangi
Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, daniel,
dqfext, SkyLake.Huang, matthias.bgg, angelogioacchino.delregno,
rdunlap, horms, arnd, netdev, linux-kernel, linux-arm-kernel,
linux-mediatek
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 10 Apr 2025 12:04:03 +0200 you wrote:
> When commit 462a3daad679 ("net: phy: mediatek: fix compile-test
> dependencies") fixed the dependency, it should have also introduced
> an or on COMPILE_TEST to permit this driver to be compile-tested even if
> NVMEM_MTK_EFUSE wasn't selected. The driver makes use of NVMEM API that
> are always compiled (return error) so the driver can actually be
> compiled even without that config.
>
> [...]
Here is the summary with links:
- [net-next,v2,1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver
https://git.kernel.org/netdev/net-next/c/e5566162af8b
- [net-next,v2,2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver
https://git.kernel.org/netdev/net-next/c/6a325aed130b
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [net-next PATCH v2 2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver
2025-04-10 19:07 ` Simon Horman
@ 2025-04-14 10:30 ` Christian Marangi
0 siblings, 0 replies; 11+ messages in thread
From: Christian Marangi @ 2025-04-14 10:30 UTC (permalink / raw)
To: Simon Horman
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Daniel Golle,
Qingfang Deng, SkyLake Huang, Matthias Brugger,
AngeloGioacchino Del Regno, Randy Dunlap, Arnd Bergmann, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
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!
--
Ansuel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-04-14 10:31 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-10 10:04 [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver Christian Marangi
2025-04-10 10:04 ` [net-next PATCH v2 2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver Christian Marangi
2025-04-10 10:33 ` Arnd Bergmann
2025-04-10 19:07 ` Simon Horman
2025-04-14 10:30 ` Christian Marangi
2025-04-10 10:31 ` [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver Arnd Bergmann
2025-04-10 10:40 ` Christian Marangi
2025-04-10 10:44 ` Christian Marangi
2025-04-10 12:01 ` Arnd Bergmann
2025-04-10 12:46 ` Arnd Bergmann
2025-04-12 3:30 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).