[PATCH net-next] net: phy: fixed_phy: replace list of fixed PHYs with static array

[PATCH net-next] net: phy: fixed_phy: replace list of fixed PHYs with static array

[Heiner Kallweit]

Due to max 32 PHY addresses being available per mii bus, using a list
can't support more fixed PHY's. And there's no known use case for as
much as 32 fixed PHY's on a system. 8 should be plenty of fixed PHY's,
so use an array of that size instead of a list. This allows to
significantly reduce the code size and complexity.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/fixed_phy.c | 69 +++++++++----------------------------
 1 file changed, 17 insertions(+), 52 deletions(-)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 50684271f81..7d6078d1570 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -10,7 +10,6 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/list.h>
 #include <linux/mii.h>
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
@@ -22,27 +21,24 @@
 
 #include "swphy.h"
 
+/* The DSA loop driver may allocate 4 fixed PHY's, and 4 additional
+ * fixed PHY's for a system should be sufficient.
+ */
+#define NUM_FP	8
+
 struct fixed_phy {
-	int addr;
 	struct phy_device *phydev;
 	struct fixed_phy_status status;
 	int (*link_update)(struct net_device *, struct fixed_phy_status *);
-	struct list_head node;
 };
 
+static struct fixed_phy fmb_fixed_phys[NUM_FP];
 static struct mii_bus *fmb_mii_bus;
-static LIST_HEAD(fmb_phys);
+static DEFINE_IDA(phy_fixed_ida);
 
 static struct fixed_phy *fixed_phy_find(int addr)
 {
-	struct fixed_phy *fp;
-
-	list_for_each_entry(fp, &fmb_phys, node) {
-		if (fp->addr == addr)
-			return fp;
-	}
-
-	return NULL;
+	return ida_exists(&phy_fixed_ida, addr) ? fmb_fixed_phys + addr : NULL;
 }
 
 int fixed_phy_change_carrier(struct net_device *dev, bool new_carrier)
@@ -108,31 +104,6 @@ int fixed_phy_set_link_update(struct phy_device *phydev,
 }
 EXPORT_SYMBOL_GPL(fixed_phy_set_link_update);
 
-static int __fixed_phy_add(int phy_addr,
-			   const struct fixed_phy_status *status)
-{
-	struct fixed_phy *fp;
-	int ret;
-
-	ret = swphy_validate_state(status);
-	if (ret < 0)
-		return ret;
-
-	fp = kzalloc(sizeof(*fp), GFP_KERNEL);
-	if (!fp)
-		return -ENOMEM;
-
-	fp->addr = phy_addr;
-	fp->status = *status;
-	fp->status.link = true;
-
-	list_add_tail(&fp->node, &fmb_phys);
-
-	return 0;
-}
-
-static DEFINE_IDA(phy_fixed_ida);
-
 static void fixed_phy_del(int phy_addr)
 {
 	struct fixed_phy *fp;
@@ -141,8 +112,7 @@ static void fixed_phy_del(int phy_addr)
 	if (!fp)
 		return;
 
-	list_del(&fp->node);
-	kfree(fp);
+	memset(fp, 0, sizeof(*fp));
 	ida_free(&phy_fixed_ida, phy_addr);
 }
 
@@ -153,19 +123,20 @@ struct phy_device *fixed_phy_register(const struct fixed_phy_status *status,
 	int phy_addr;
 	int ret;
 
+	ret = swphy_validate_state(status);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
 	if (!fmb_mii_bus || fmb_mii_bus->state != MDIOBUS_REGISTERED)
 		return ERR_PTR(-EPROBE_DEFER);
 
-	/* Get the next available PHY address, up to PHY_MAX_ADDR */
-	phy_addr = ida_alloc_max(&phy_fixed_ida, PHY_MAX_ADDR - 1, GFP_KERNEL);
+	/* Get the next available PHY address, up to NUM_FP */
+	phy_addr = ida_alloc_max(&phy_fixed_ida, NUM_FP - 1, GFP_KERNEL);
 	if (phy_addr < 0)
 		return ERR_PTR(phy_addr);
 
-	ret = __fixed_phy_add(phy_addr, status);
-	if (ret < 0) {
-		ida_free(&phy_fixed_ida, phy_addr);
-		return ERR_PTR(ret);
-	}
+	fmb_fixed_phys[phy_addr].status = *status;
+	fmb_fixed_phys[phy_addr].status.link = true;
 
 	phy = get_phy_device(fmb_mii_bus, phy_addr, false);
 	if (IS_ERR(phy)) {
@@ -237,15 +208,9 @@ module_init(fixed_mdio_bus_init);
 
 static void __exit fixed_mdio_bus_exit(void)
 {
-	struct fixed_phy *fp, *tmp;
-
 	mdiobus_unregister(fmb_mii_bus);
 	mdiobus_free(fmb_mii_bus);
 
-	list_for_each_entry_safe(fp, tmp, &fmb_phys, node) {
-		list_del(&fp->node);
-		kfree(fp);
-	}
 	ida_destroy(&phy_fixed_ida);
 }
 module_exit(fixed_mdio_bus_exit);
-- 
2.52.0

Re: [PATCH net-next] net: phy: fixed_phy: replace list of fixed PHYs with static array

[Russell King (Oracle)]

On Tue, Jan 06, 2026 at 05:56:26PM +0100, Heiner Kallweit wrote:
> Due to max 32 PHY addresses being available per mii bus, using a list
> can't support more fixed PHY's. And there's no known use case for as
> much as 32 fixed PHY's on a system. 8 should be plenty of fixed PHY's,
> so use an array of that size instead of a list. This allows to
> significantly reduce the code size and complexity.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: phy: fixed_phy: replace list of fixed PHYs with static array

[Jakub Kicinski]

On Tue, 6 Jan 2026 17:56:26 +0100 Heiner Kallweit wrote:
> +/* The DSA loop driver may allocate 4 fixed PHY's, and 4 additional
> + * fixed PHY's for a system should be sufficient.
> + */
> +#define NUM_FP	8
> +
>  struct fixed_phy {
> -	int addr;
>  	struct phy_device *phydev;
>  	struct fixed_phy_status status;
>  	int (*link_update)(struct net_device *, struct fixed_phy_status *);
> -	struct list_head node;
>  };
>  
> +static struct fixed_phy fmb_fixed_phys[NUM_FP];
>  static struct mii_bus *fmb_mii_bus;
> -static LIST_HEAD(fmb_phys);
> +static DEFINE_IDA(phy_fixed_ida);

Isn't IDA an overkill for a range this tiny?
IDA is useful if the ID range is large and may be sparse.
Here a bitmap would suffice.

DECLARE_BITMAP(phy_fixed_ids, NUM_FP); 

id = find_first_zero_bit(phy_fixed_ids, NUM_FP);
if (id >= NUM_FP)
	return -ENOSPC;

set_bit(id, phy_fixed_ids);
...

Re: [PATCH net-next] net: phy: fixed_phy: replace list of fixed PHYs with static array

[Heiner Kallweit]

On 1/9/2026 3:11 AM, Jakub Kicinski wrote:
> On Tue, 6 Jan 2026 17:56:26 +0100 Heiner Kallweit wrote:
>> +/* The DSA loop driver may allocate 4 fixed PHY's, and 4 additional
>> + * fixed PHY's for a system should be sufficient.
>> + */
>> +#define NUM_FP	8
>> +
>>  struct fixed_phy {
>> -	int addr;
>>  	struct phy_device *phydev;
>>  	struct fixed_phy_status status;
>>  	int (*link_update)(struct net_device *, struct fixed_phy_status *);
>> -	struct list_head node;
>>  };
>>  
>> +static struct fixed_phy fmb_fixed_phys[NUM_FP];
>>  static struct mii_bus *fmb_mii_bus;
>> -static LIST_HEAD(fmb_phys);
>> +static DEFINE_IDA(phy_fixed_ida);
> 
> Isn't IDA an overkill for a range this tiny?
> IDA is useful if the ID range is large and may be sparse.
> Here a bitmap would suffice.
> 

Thanks for the suggestion! The IDA has been there forever, just the definition
has been moved now. I think switching to a bitmap is a good option.
Can we handle this as a follow-up?

> DECLARE_BITMAP(phy_fixed_ids, NUM_FP); 
> 
> id = find_first_zero_bit(phy_fixed_ids, NUM_FP);
> if (id >= NUM_FP)
> 	return -ENOSPC;
> 
> set_bit(id, phy_fixed_ids);
> ...

Re: [PATCH net-next] net: phy: fixed_phy: replace list of fixed PHYs with static array

[Jakub Kicinski]

On Fri, 9 Jan 2026 12:00:41 +0100 Heiner Kallweit wrote:
> >> +static struct fixed_phy fmb_fixed_phys[NUM_FP];
> >>  static struct mii_bus *fmb_mii_bus;
> >> -static LIST_HEAD(fmb_phys);
> >> +static DEFINE_IDA(phy_fixed_ida);  
> > 
> > Isn't IDA an overkill for a range this tiny?
> > IDA is useful if the ID range is large and may be sparse.
> > Here a bitmap would suffice.
> 
> Thanks for the suggestion! The IDA has been there forever, just the definition
> has been moved now. I think switching to a bitmap is a good option.
> Can we handle this as a follow-up?

I see.. Still I think that deleting the IDA in the same patch makes
sense. IDA makes no sense for a small fixed-size array and we're
touching a number of the relevant lines, anyway.

Re: [PATCH net-next] net: phy: fixed_phy: replace list of fixed PHYs with static array

[Russell King (Oracle)]

On Thu, Jan 08, 2026 at 06:11:02PM -0800, Jakub Kicinski wrote:
> On Tue, 6 Jan 2026 17:56:26 +0100 Heiner Kallweit wrote:
> > +/* The DSA loop driver may allocate 4 fixed PHY's, and 4 additional
> > + * fixed PHY's for a system should be sufficient.
> > + */
> > +#define NUM_FP	8
> > +
> >  struct fixed_phy {
> > -	int addr;
> >  	struct phy_device *phydev;
> >  	struct fixed_phy_status status;
> >  	int (*link_update)(struct net_device *, struct fixed_phy_status *);
> > -	struct list_head node;
> >  };
> >  
> > +static struct fixed_phy fmb_fixed_phys[NUM_FP];
> >  static struct mii_bus *fmb_mii_bus;
> > -static LIST_HEAD(fmb_phys);
> > +static DEFINE_IDA(phy_fixed_ida);
> 
> Isn't IDA an overkill for a range this tiny?
> IDA is useful if the ID range is large and may be sparse.
> Here a bitmap would suffice.
> 
> DECLARE_BITMAP(phy_fixed_ids, NUM_FP); 
> 
> id = find_first_zero_bit(phy_fixed_ids, NUM_FP);
> if (id >= NUM_FP)
> 	return -ENOSPC;
> 
> set_bit(id, phy_fixed_ids);

Racy without locking.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: phy: fixed_phy: replace list of fixed PHYs with static array

[Jakub Kicinski]

On Fri, 9 Jan 2026 15:10:18 +0000 Russell King (Oracle) wrote:
> > Isn't IDA an overkill for a range this tiny?
> > IDA is useful if the ID range is large and may be sparse.
> > Here a bitmap would suffice.
> > 
> > DECLARE_BITMAP(phy_fixed_ids, NUM_FP); 
> > 
> > id = find_first_zero_bit(phy_fixed_ids, NUM_FP);
> > if (id >= NUM_FP)
> > 	return -ENOSPC;
> > 
> > set_bit(id, phy_fixed_ids);  
> 
> Racy without locking.

True, if there's no existing locking wrap it in a do {} while and use
test_and_set_bit() as the condition for repeat.