[PATCH net-next 0/2] net: phy: fixed_phy: add helper fixed_phy_register_100fd

[PATCH net-next 0/2] net: phy: fixed_phy: add helper fixed_phy_register_100fd

[Heiner Kallweit]

In few places a 100FD fixed PHY is used. Create a helper so that users
don't have to define the struct fixed_phy_status. First user is the
dsa loop driver. A follow-up series will remove the usage of
fixed_phy_add() from bcm47xx and coldfire/m5272, then this helper
will be used too.

Heiner Kallweit (2):
  net: phy: fixed_phy: add helper fixed_phy_register_100fd
  net: dsa: loop: use new helper fixed_phy_register_100fd

 drivers/net/dsa/dsa_loop.c  |  7 +------
 drivers/net/phy/fixed_phy.c | 12 ++++++++++++
 include/linux/phy_fixed.h   |  6 ++++++
 3 files changed, 19 insertions(+), 6 deletions(-)

-- 
2.51.1.dirty

[PATCH net-next 1/2] net: phy: fixed_phy: add helper fixed_phy_register_100fd

[Heiner Kallweit]

In few places a 100FD fixed PHY is used. Create a helper so that users
don't have to define the struct fixed_phy_status.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/fixed_phy.c | 12 ++++++++++++
 include/linux/phy_fixed.h   |  6 ++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 0e1b28f06..bdc3a4bff 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -227,6 +227,18 @@ struct phy_device *fixed_phy_register(const struct fixed_phy_status *status,
 }
 EXPORT_SYMBOL_GPL(fixed_phy_register);
 
+struct phy_device *fixed_phy_register_100fd(void)
+{
+	static const struct fixed_phy_status status = {
+		.link	= 1,
+		.speed	= SPEED_100,
+		.duplex	= DUPLEX_FULL,
+	};
+
+	return fixed_phy_register(&status, NULL);
+}
+EXPORT_SYMBOL_GPL(fixed_phy_register_100fd);
+
 void fixed_phy_unregister(struct phy_device *phy)
 {
 	phy_device_remove(phy);
diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
index d17ff750c..08275ef64 100644
--- a/include/linux/phy_fixed.h
+++ b/include/linux/phy_fixed.h
@@ -20,6 +20,7 @@ extern int fixed_phy_change_carrier(struct net_device *dev, bool new_carrier);
 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);
+struct phy_device *fixed_phy_register_100fd(void);
 
 extern void fixed_phy_unregister(struct phy_device *phydev);
 extern int fixed_phy_set_link_update(struct phy_device *phydev,
@@ -34,6 +35,11 @@ fixed_phy_register(const struct fixed_phy_status *status,
 	return ERR_PTR(-ENODEV);
 }
 
+static inline struct phy_device *fixed_phy_register_100fd(void)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline void fixed_phy_unregister(struct phy_device *phydev)
 {
 }
-- 
2.51.1.dirty

[PATCH net-next 2/2] net: dsa: loop: use new helper fixed_phy_register_100fd

[Heiner Kallweit]

Use new helper fixed_phy_register_100fd() to simplify the code.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/dsa/dsa_loop.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index 650d93226..4a416f271 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -441,11 +441,6 @@ static int __init dsa_loop_create_switch_mdiodev(void)
 
 static int __init dsa_loop_init(void)
 {
-	struct fixed_phy_status status = {
-		.link = 1,
-		.speed = SPEED_100,
-		.duplex = DUPLEX_FULL,
-	};
 	unsigned int i;
 	int ret;
 
@@ -454,7 +449,7 @@ static int __init dsa_loop_init(void)
 		return ret;
 
 	for (i = 0; i < NUM_FIXED_PHYS; i++)
-		phydevs[i] = fixed_phy_register(&status, NULL);
+		phydevs[i] = fixed_phy_register_100fd();
 
 	ret = mdio_driver_register(&dsa_loop_drv);
 	if (ret) {
-- 
2.51.1.dirty

Re: [PATCH net-next 1/2] net: phy: fixed_phy: add helper fixed_phy_register_100fd

[Gerhard Engleder]

On 17.10.25 22:12, Heiner Kallweit wrote:
> In few places a 100FD fixed PHY is used. Create a helper so that users
> don't have to define the struct fixed_phy_status.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>   drivers/net/phy/fixed_phy.c | 12 ++++++++++++
>   include/linux/phy_fixed.h   |  6 ++++++
>   2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
> index 0e1b28f06..bdc3a4bff 100644
> --- a/drivers/net/phy/fixed_phy.c
> +++ b/drivers/net/phy/fixed_phy.c
> @@ -227,6 +227,18 @@ struct phy_device *fixed_phy_register(const struct fixed_phy_status *status,
>   }
>   EXPORT_SYMBOL_GPL(fixed_phy_register);
>   
> +struct phy_device *fixed_phy_register_100fd(void)
> +{
> +	static const struct fixed_phy_status status = {
> +		.link	= 1,
> +		.speed	= SPEED_100,
> +		.duplex	= DUPLEX_FULL,
> +	};
> +
> +	return fixed_phy_register(&status, NULL);
> +}
> +EXPORT_SYMBOL_GPL(fixed_phy_register_100fd);
> +
>   void fixed_phy_unregister(struct phy_device *phy)
>   {
>   	phy_device_remove(phy);
> diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
> index d17ff750c..08275ef64 100644
> --- a/include/linux/phy_fixed.h
> +++ b/include/linux/phy_fixed.h
> @@ -20,6 +20,7 @@ extern int fixed_phy_change_carrier(struct net_device *dev, bool new_carrier);
>   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);
> +struct phy_device *fixed_phy_register_100fd(void);
>   
>   extern void fixed_phy_unregister(struct phy_device *phydev);
>   extern int fixed_phy_set_link_update(struct phy_device *phydev,
> @@ -34,6 +35,11 @@ fixed_phy_register(const struct fixed_phy_status *status,
>   	return ERR_PTR(-ENODEV);
>   }
>   
> +static inline struct phy_device *fixed_phy_register_100fd(void)
> +{
> +	return ERR_PTR(-ENODEV);
> +}
> +
>   static inline void fixed_phy_unregister(struct phy_device *phydev)
>   {
>   }

Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>

Re: [PATCH net-next 2/2] net: dsa: loop: use new helper fixed_phy_register_100fd

[Gerhard Engleder]

On 17.10.25 22:14, Heiner Kallweit wrote:
> Use new helper fixed_phy_register_100fd() to simplify the code.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>   drivers/net/dsa/dsa_loop.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
> index 650d93226..4a416f271 100644
> --- a/drivers/net/dsa/dsa_loop.c
> +++ b/drivers/net/dsa/dsa_loop.c
> @@ -441,11 +441,6 @@ static int __init dsa_loop_create_switch_mdiodev(void)
>   
>   static int __init dsa_loop_init(void)
>   {
> -	struct fixed_phy_status status = {
> -		.link = 1,
> -		.speed = SPEED_100,
> -		.duplex = DUPLEX_FULL,
> -	};
>   	unsigned int i;
>   	int ret;
>   
> @@ -454,7 +449,7 @@ static int __init dsa_loop_init(void)
>   		return ret;
>   
>   	for (i = 0; i < NUM_FIXED_PHYS; i++)
> -		phydevs[i] = fixed_phy_register(&status, NULL);
> +		phydevs[i] = fixed_phy_register_100fd();
>   
>   	ret = mdio_driver_register(&dsa_loop_drv);
>   	if (ret) {

Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>

Re: [PATCH net-next 1/2] net: phy: fixed_phy: add helper fixed_phy_register_100fd

[Florian Fainelli]

On 10/17/25 13:12, Heiner Kallweit wrote:
> In few places a 100FD fixed PHY is used. Create a helper so that users
> don't have to define the struct fixed_phy_status.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>   drivers/net/phy/fixed_phy.c | 12 ++++++++++++
>   include/linux/phy_fixed.h   |  6 ++++++
>   2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
> index 0e1b28f06..bdc3a4bff 100644
> --- a/drivers/net/phy/fixed_phy.c
> +++ b/drivers/net/phy/fixed_phy.c
> @@ -227,6 +227,18 @@ struct phy_device *fixed_phy_register(const struct fixed_phy_status *status,
>   }
>   EXPORT_SYMBOL_GPL(fixed_phy_register);
>   
> +struct phy_device *fixed_phy_register_100fd(void)
> +{
> +	static const struct fixed_phy_status status = {
> +		.link	= 1,
> +		.speed	= SPEED_100,
> +		.duplex	= DUPLEX_FULL,
> +	};
> +
> +	return fixed_phy_register(&status, NULL);
> +}
> +EXPORT_SYMBOL_GPL(fixed_phy_register_100fd);

Would not you want this to be a static inline helper directly?
-- 
Florian

Re: [PATCH net-next 1/2] net: phy: fixed_phy: add helper fixed_phy_register_100fd

[Heiner Kallweit]

On 10/17/2025 10:55 PM, Florian Fainelli wrote:
> On 10/17/25 13:12, Heiner Kallweit wrote:
>> In few places a 100FD fixed PHY is used. Create a helper so that users
>> don't have to define the struct fixed_phy_status.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>   drivers/net/phy/fixed_phy.c | 12 ++++++++++++
>>   include/linux/phy_fixed.h   |  6 ++++++
>>   2 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
>> index 0e1b28f06..bdc3a4bff 100644
>> --- a/drivers/net/phy/fixed_phy.c
>> +++ b/drivers/net/phy/fixed_phy.c
>> @@ -227,6 +227,18 @@ struct phy_device *fixed_phy_register(const struct fixed_phy_status *status,
>>   }
>>   EXPORT_SYMBOL_GPL(fixed_phy_register);
>>   +struct phy_device *fixed_phy_register_100fd(void)
>> +{
>> +    static const struct fixed_phy_status status = {
>> +        .link    = 1,
>> +        .speed    = SPEED_100,
>> +        .duplex    = DUPLEX_FULL,
>> +    };
>> +
>> +    return fixed_phy_register(&status, NULL);
>> +}
>> +EXPORT_SYMBOL_GPL(fixed_phy_register_100fd);
> 
> Would not you want this to be a static inline helper directly?

Wouldn't then again each user allocate its own struct fixed_phy_status?
As-is we allocate it only once.

Re: [PATCH net-next 0/2] net: phy: fixed_phy: add helper fixed_phy_register_100fd

[Jakub Kicinski]

On Fri, 17 Oct 2025 22:11:44 +0200 Heiner Kallweit wrote:
> In few places a 100FD fixed PHY is used. Create a helper so that users
> don't have to define the struct fixed_phy_status. First user is the
> dsa loop driver. A follow-up series will remove the usage of
> fixed_phy_add() from bcm47xx and coldfire/m5272, then this helper
> will be used too.

Not knowing the area too well it looks like struct fixed_phy_status
is an argument struct to make it easier to add / pass thru extra
attrs without having to modify all the callers. This series goes
in the opposite direction trying to make the callers not have to
declare the argument struct.

When reading the code it may also be easier to graps the code if 
the definition is local vs having to look at fixed_phy_register_100fd()
Granted the function name kinda makes it obvious what it does.

Lastly: 19 insertions(+), 6 deletions(-)

So the improvement here is not immediately obvious to me.
Maybe it'd be easier to appreciate this series if it was in one
piece with what you mentioned as a follow up?

Re: [PATCH net-next 0/2] net: phy: fixed_phy: add helper fixed_phy_register_100fd

[Heiner Kallweit]

On 10/22/2025 3:20 AM, Jakub Kicinski wrote:
> On Fri, 17 Oct 2025 22:11:44 +0200 Heiner Kallweit wrote:
>> In few places a 100FD fixed PHY is used. Create a helper so that users
>> don't have to define the struct fixed_phy_status. First user is the
>> dsa loop driver. A follow-up series will remove the usage of
>> fixed_phy_add() from bcm47xx and coldfire/m5272, then this helper
>> will be used too.
> 
> Not knowing the area too well it looks like struct fixed_phy_status
> is an argument struct to make it easier to add / pass thru extra
> attrs without having to modify all the callers. This series goes
> in the opposite direction trying to make the callers not have to
> declare the argument struct.
> 
> When reading the code it may also be easier to graps the code if 
> the definition is local vs having to look at fixed_phy_register_100fd()
> Granted the function name kinda makes it obvious what it does.
> 
> Lastly: 19 insertions(+), 6 deletions(-)
> 
> So the improvement here is not immediately obvious to me.
> Maybe it'd be easier to appreciate this series if it was in one
> piece with what you mentioned as a follow up?

Makes sense, I'll prepare and submit the full series.