diff mbox series

net: phy: fix circular LEDS_CLASS dependencies

Message ID 20230420084624.3005701-1-arnd@kernel.org (mailing list archive)
State Accepted
Commit 4bb7aac70b5d8a4bddf4ee0791b834f9f56883d2
Delegated to: Netdev Maintainers
Headers show
Series net: phy: fix circular LEDS_CLASS dependencies | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
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: 8 this patch: 8
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
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 Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch warning WARNING: please write a help paragraph that fully describes the config symbol
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Arnd Bergmann April 20, 2023, 8:45 a.m. UTC 
From: Arnd Bergmann <arnd@arndb.de>

The CONFIG_PHYLIB symbol is selected by a number of device drivers that
need PHY support, but it now has a dependency on CONFIG_LEDS_CLASS,
which may not be enabled, causing build failures.

Avoid the risk of missing and circular dependencies by guarding the
phylib LED support itself in another Kconfig symbol that can only be
enabled if the dependency is met.

This could be made a hidden symbol and always enabled when both CONFIG_OF
and CONFIG_LEDS_CLASS are reachable from the phylib, but there may be an
advantage in having users see this option when they have a misconfigured
kernel without built-in LED support.

Fixes: 01e5b728e9e4 ("net: phy: Add a binding for PHY LEDs")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/phy/Kconfig      | 9 ++++++++-
 drivers/net/phy/phy_device.c | 3 ++-
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Andrew Lunn April 20, 2023, 3:29 p.m. UTC | #1 
On Thu, Apr 20, 2023 at 10:45:51AM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The CONFIG_PHYLIB symbol is selected by a number of device drivers that
> need PHY support, but it now has a dependency on CONFIG_LEDS_CLASS,
> which may not be enabled, causing build failures.
> 
> Avoid the risk of missing and circular dependencies by guarding the
> phylib LED support itself in another Kconfig symbol that can only be
> enabled if the dependency is met.
> 
> This could be made a hidden symbol and always enabled when both CONFIG_OF
> and CONFIG_LEDS_CLASS are reachable from the phylib, but there may be an
> advantage in having users see this option when they have a misconfigured
> kernel without built-in LED support.
> 
> Fixes: 01e5b728e9e4 ("net: phy: Add a binding for PHY LEDs")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

This is for net-next.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Jakub Kicinski April 21, 2023, 2:09 a.m. UTC | #2 
On Thu, 20 Apr 2023 10:45:51 +0200 Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The CONFIG_PHYLIB symbol is selected by a number of device drivers that
> need PHY support, but it now has a dependency on CONFIG_LEDS_CLASS,
> which may not be enabled, causing build failures.
> 
> Avoid the risk of missing and circular dependencies by guarding the
> phylib LED support itself in another Kconfig symbol that can only be
> enabled if the dependency is met.
> 
> This could be made a hidden symbol and always enabled when both CONFIG_OF
> and CONFIG_LEDS_CLASS are reachable from the phylib, but there may be an
> advantage in having users see this option when they have a misconfigured
> kernel without built-in LED support.

The problem is breaking build for the config I use in testing,
so let me apply without waiting full review period. Thanks!
patchwork-bot+netdevbpf@kernel.org April 21, 2023, 2:20 a.m. UTC | #3 
Hello:

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

On Thu, 20 Apr 2023 10:45:51 +0200 you wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The CONFIG_PHYLIB symbol is selected by a number of device drivers that
> need PHY support, but it now has a dependency on CONFIG_LEDS_CLASS,
> which may not be enabled, causing build failures.
> 
> Avoid the risk of missing and circular dependencies by guarding the
> phylib LED support itself in another Kconfig symbol that can only be
> enabled if the dependency is met.
> 
> [...]

Here is the summary with links:
  - net: phy: fix circular LEDS_CLASS dependencies
    https://git.kernel.org/netdev/net-next/c/4bb7aac70b5d

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index b8cc49820ced..513675ae4dd2 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -18,7 +18,6 @@  menuconfig PHYLIB
 	depends on NETDEVICES
 	select MDIO_DEVICE
 	select MDIO_DEVRES
-	depends on LEDS_CLASS || LEDS_CLASS=n
 	help
 	  Ethernet controllers are usually attached to PHY
 	  devices.  This option provides infrastructure for
@@ -45,6 +44,14 @@  config LED_TRIGGER_PHY
 		<Speed in megabits>Mbps OR <Speed in gigabits>Gbps OR link
 		for any speed known to the PHY.
 
+config PHYLIB_LEDS
+	bool "Support probing LEDs from device tree"
+	depends on LEDS_CLASS=y || LEDS_CLASS=PHYLIB
+	depends on OF
+	default y
+	help
+	  When LED class support is enabled, phylib can automatically
+	  probe LED setting from device tree.
 
 config FIXED_PHY
 	tristate "MDIO Bus/PHY emulation with fixed speed/link PHYs"
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 538523a7cd51..d373446ab5ac 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3284,7 +3284,8 @@  static int phy_probe(struct device *dev)
 	/* Get the LEDs from the device tree, and instantiate standard
 	 * LEDs for them.
 	 */
-	err = of_phy_leds(phydev);
+	if (IS_ENABLED(CONFIG_PHYLIB_LEDS))
+		err = of_phy_leds(phydev);
 
 out:
 	/* Re-assert the reset signal on error */