net: phy: mediatek: fix compile-test dependencies

[PATCH] net: phy: mediatek: fix compile-test dependencies

Posted by Arnd Bergmann 1 year, 3 months ago

From: Arnd Bergmann <arnd@arndb.de>

The new phy driver attempts to select a driver from another subsystem,
but that fails when the NVMEM subsystem is disabled:

WARNING: unmet direct dependencies detected for NVMEM_MTK_EFUSE
  Depends on [n]: NVMEM [=n] && (ARCH_MEDIATEK [=n] || COMPILE_TEST [=y]) && HAS_IOMEM [=y]
  Selected by [y]:
  - MEDIATEK_GE_SOC_PHY [=y] && NETDEVICES [=y] && PHYLIB [=y] && (ARM64 && ARCH_MEDIATEK [=n] || COMPILE_TEST [=y])

I could not see an actual compile time dependency, so presumably this
is only needed for for working correctly but not technically a dependency
on that particular nvmem driver implementation, so it would likely
be safe to remove the select for compile testing.

To keep the spirit of the original 'select', just replace this with a
'depends on' that ensures that the driver will work but does not get in
the way of build testing.

Fixes: 98c485eaf509b ("net: phy: add driver for MediaTek SoC built-in GE PHYs")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/phy/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index a40269c175974..78e6981650d94 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -239,7 +239,7 @@ config MEDIATEK_GE_PHY
 config MEDIATEK_GE_SOC_PHY
 	tristate "MediaTek SoC Ethernet PHYs"
 	depends on (ARM64 && ARCH_MEDIATEK) || COMPILE_TEST
-	select NVMEM_MTK_EFUSE
+	depends on NVMEM_MTK_EFUSE
 	help
 	  Supports MediaTek SoC built-in Gigabit Ethernet PHYs.
 
-- 
2.39.2

Re: [PATCH] net: phy: mediatek: fix compile-test dependencies

Posted by patchwork-bot+netdevbpf@kernel.org 1 year, 3 months ago

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 16 Jun 2023 11:29:54 +0200 you wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The new phy driver attempts to select a driver from another subsystem,
> but that fails when the NVMEM subsystem is disabled:
> 
> WARNING: unmet direct dependencies detected for NVMEM_MTK_EFUSE
>   Depends on [n]: NVMEM [=n] && (ARCH_MEDIATEK [=n] || COMPILE_TEST [=y]) && HAS_IOMEM [=y]
>   Selected by [y]:
>   - MEDIATEK_GE_SOC_PHY [=y] && NETDEVICES [=y] && PHYLIB [=y] && (ARM64 && ARCH_MEDIATEK [=n] || COMPILE_TEST [=y])
> 
> [...]

Here is the summary with links:
  - net: phy: mediatek: fix compile-test dependencies
    https://git.kernel.org/netdev/net-next/c/462a3daad679

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] net: phy: mediatek: fix compile-test dependencies

Posted by Randy Dunlap 1 year, 3 months ago


On 6/16/23 02:29, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The new phy driver attempts to select a driver from another subsystem,
> but that fails when the NVMEM subsystem is disabled:
> 
> WARNING: unmet direct dependencies detected for NVMEM_MTK_EFUSE
>   Depends on [n]: NVMEM [=n] && (ARCH_MEDIATEK [=n] || COMPILE_TEST [=y]) && HAS_IOMEM [=y]
>   Selected by [y]:
>   - MEDIATEK_GE_SOC_PHY [=y] && NETDEVICES [=y] && PHYLIB [=y] && (ARM64 && ARCH_MEDIATEK [=n] || COMPILE_TEST [=y])
> 
> I could not see an actual compile time dependency, so presumably this
> is only needed for for working correctly but not technically a dependency
> on that particular nvmem driver implementation, so it would likely
> be safe to remove the select for compile testing.
> 
> To keep the spirit of the original 'select', just replace this with a
> 'depends on' that ensures that the driver will work but does not get in
> the way of build testing.
> 
> Fixes: 98c485eaf509b ("net: phy: add driver for MediaTek SoC built-in GE PHYs")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org> # build-tested

Thanks.

> ---
>  drivers/net/phy/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index a40269c175974..78e6981650d94 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -239,7 +239,7 @@ config MEDIATEK_GE_PHY
>  config MEDIATEK_GE_SOC_PHY
>  	tristate "MediaTek SoC Ethernet PHYs"
>  	depends on (ARM64 && ARCH_MEDIATEK) || COMPILE_TEST
> -	select NVMEM_MTK_EFUSE
> +	depends on NVMEM_MTK_EFUSE
>  	help
>  	  Supports MediaTek SoC built-in Gigabit Ethernet PHYs.
>  

-- 
~Randy

Re: [PATCH] net: phy: mediatek: fix compile-test dependencies

Posted by Simon Horman 1 year, 3 months ago

On Fri, Jun 16, 2023 at 11:29:54AM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The new phy driver attempts to select a driver from another subsystem,
> but that fails when the NVMEM subsystem is disabled:
> 
> WARNING: unmet direct dependencies detected for NVMEM_MTK_EFUSE
>   Depends on [n]: NVMEM [=n] && (ARCH_MEDIATEK [=n] || COMPILE_TEST [=y]) && HAS_IOMEM [=y]
>   Selected by [y]:
>   - MEDIATEK_GE_SOC_PHY [=y] && NETDEVICES [=y] && PHYLIB [=y] && (ARM64 && ARCH_MEDIATEK [=n] || COMPILE_TEST [=y])
> 
> I could not see an actual compile time dependency, so presumably this
> is only needed for for working correctly but not technically a dependency

nit: for for -> for
     or
     for for working correctly -> for correct operation

> on that particular nvmem driver implementation, so it would likely
> be safe to remove the select for compile testing.
> 
> To keep the spirit of the original 'select', just replace this with a
> 'depends on' that ensures that the driver will work but does not get in
> the way of build testing.
> 
> Fixes: 98c485eaf509b ("net: phy: add driver for MediaTek SoC built-in GE PHYs")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I don't know the answer to the question of if this dependency is needed or
not. But I do agree that it does what it says on the box.

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH] net: phy: mediatek: fix compile-test dependencies

Posted by Daniel Golle 1 year, 3 months ago

On Fri, Jun 16, 2023 at 01:47:35PM +0200, Simon Horman wrote:
> On Fri, Jun 16, 2023 at 11:29:54AM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > The new phy driver attempts to select a driver from another subsystem,
> > but that fails when the NVMEM subsystem is disabled:
> > 
> > WARNING: unmet direct dependencies detected for NVMEM_MTK_EFUSE
> >   Depends on [n]: NVMEM [=n] && (ARCH_MEDIATEK [=n] || COMPILE_TEST [=y]) && HAS_IOMEM [=y]
> >   Selected by [y]:
> >   - MEDIATEK_GE_SOC_PHY [=y] && NETDEVICES [=y] && PHYLIB [=y] && (ARM64 && ARCH_MEDIATEK [=n] || COMPILE_TEST [=y])
> > 
> > I could not see an actual compile time dependency, so presumably this
> > is only needed for for working correctly but not technically a dependency
> 
> nit: for for -> for
>      or
>      for for working correctly -> for correct operation
> 
> > on that particular nvmem driver implementation, so it would likely
> > be safe to remove the select for compile testing.
> > 
> > To keep the spirit of the original 'select', just replace this with a
> > 'depends on' that ensures that the driver will work but does not get in
> > the way of build testing.
> > 
> > Fixes: 98c485eaf509b ("net: phy: add driver for MediaTek SoC built-in GE PHYs")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> I don't know the answer to the question of if this dependency is needed or
> not. But I do agree that it does what it says on the box.

It's not needed to build or load the driver, but the PHY won't function
at all without reading values from the SoCs efuse, and for that the
nvmem driver is required.

Using a simple dependency instead of select will fix it.

Reviewed-by: Daniel Golle <daniel@makrotopia.org>

> 
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> 
>