Thread (5 messages) 5 messages, 3 authors, 2025-10-04

Re: [PATCH net-next] net: phy: fixed: let fixed_phy_add always use addr 0 and remove return value

From: Hauke Mehrtens <hauke@hauke-m.de>
Date: 2025-08-24 16:05:36
Also in: linux-m68k, linux-mips

On 8/22/25 22:36, Heiner Kallweit wrote:
quoted hunk ↗ jump to hunk
We have only two users of fixed_phy_add(), both use address 0 and
ignore the return value. So simplify fixed_phy_add() accordingly.

Whilst at it, constify the fixed_phy_status configs.

Note:
fixed_phy_add() is a legacy function which shouldn't be used in new
code, as it's use may be problematic:
- No check whether a fixed phy exists already at the given address
- If fixed_phy_register() is called afterwards by any other driver,
   then it will also use phy_addr 0, because fixed_phy_add() ignores
   the ida which manages address assignment
Drivers using a fixed phy created by fixed_phy_add() in platform code,
should dynamically create a fixed phy with fixed_phy_register()
instead.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
  arch/m68k/coldfire/m5272.c  | 4 ++--
  arch/mips/bcm47xx/setup.c   | 4 ++--
  drivers/net/phy/fixed_phy.c | 4 ++--
  include/linux/phy_fixed.h   | 8 ++------
  4 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/arch/m68k/coldfire/m5272.c b/arch/m68k/coldfire/m5272.c
index 5b70dfdab..918e2a323 100644
--- a/arch/m68k/coldfire/m5272.c
+++ b/arch/m68k/coldfire/m5272.c
@@ -108,7 +108,7 @@ void __init config_BSP(char *commandp, int size)
   * an ethernet switch. In this case we need to use the fixed phy type,
   * and we need to declare it early in boot.
   */
-static struct fixed_phy_status nettel_fixed_phy_status __initdata = {
+static const struct fixed_phy_status nettel_fixed_phy_status __initconst = {
  	.link	= 1,
  	.speed	= 100,
  	.duplex	= 0,
@@ -119,7 +119,7 @@ static struct fixed_phy_status nettel_fixed_phy_status __initdata = {
  static int __init init_BSP(void)
  {
  	m5272_uarts_init();
-	fixed_phy_add(0, &nettel_fixed_phy_status);
+	fixed_phy_add(&nettel_fixed_phy_status);
  	clkdev_add_table(m5272_clk_lookup, ARRAY_SIZE(m5272_clk_lookup));
  	return 0;
  }
diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c
index de426a474..a93a4266d 100644
--- a/arch/mips/bcm47xx/setup.c
+++ b/arch/mips/bcm47xx/setup.c
@@ -256,7 +256,7 @@ static int __init bcm47xx_cpu_fixes(void)
  }
  arch_initcall(bcm47xx_cpu_fixes);
  
-static struct fixed_phy_status bcm47xx_fixed_phy_status __initdata = {
+static const struct fixed_phy_status bcm47xx_fixed_phy_status __initconst = {
  	.link	= 1,
  	.speed	= SPEED_100,
  	.duplex	= DUPLEX_FULL,
@@ -282,7 +282,7 @@ static int __init bcm47xx_register_bus_complete(void)
  	bcm47xx_leds_register();
  	bcm47xx_workarounds();
  
-	fixed_phy_add(0, &bcm47xx_fixed_phy_status);
+	fixed_phy_add(&bcm47xx_fixed_phy_status);
  	return 0;
  }
  device_initcall(bcm47xx_register_bus_complete);
diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 7902b35c5..b39532abf 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -153,9 +153,9 @@ static int fixed_phy_add_gpiod(unsigned int irq, int phy_addr,
  	return 0;
  }
  
-int fixed_phy_add(int phy_addr, const struct fixed_phy_status *status)
+void fixed_phy_add(const struct fixed_phy_status *status)
  {
-	return fixed_phy_add_gpiod(PHY_POLL, phy_addr, status, NULL);
+	fixed_phy_add_gpiod(PHY_POLL, 0, status, NULL);
  }
  EXPORT_SYMBOL_GPL(fixed_phy_add);
  
diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
index 5399b9e41..6227a1bde 100644
--- a/include/linux/phy_fixed.h
+++ b/include/linux/phy_fixed.h
@@ -17,7 +17,7 @@ struct net_device;
  
  #if IS_ENABLED(CONFIG_FIXED_PHY)
  extern int fixed_phy_change_carrier(struct net_device *dev, bool new_carrier);
-int fixed_phy_add(int phy_id, const struct fixed_phy_status *status);
+void fixed_phy_add(const struct fixed_phy_status *status);
  struct phy_device *fixed_phy_register(const struct fixed_phy_status *status,
  				      struct device_node *np);
  
@@ -26,11 +26,7 @@ extern int fixed_phy_set_link_update(struct phy_device *phydev,
  			int (*link_update)(struct net_device *,
  					   struct fixed_phy_status *));
  #else
-static inline int fixed_phy_add(int phy_id,
-				const struct fixed_phy_status *status)
-{
-	return -ENODEV;
-}
+static inline void fixed_phy_add(const struct fixed_phy_status *status) {}
  static inline struct phy_device *
  fixed_phy_register(const struct fixed_phy_status *status,
  		   struct device_node *np)
Hi,

I do not use this hardware any more, but Takumi reported that 
fixed_phy_add() is not working for the PHY registration on brcm47xx any 
more and we have to use fixed_phy_register(), see:
https://github.com/openwrt/openwrt/pull/19610

Does this need a bigger refactoring anyway?

Hauke
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help