[PATCH net-next] net: phy: qcom: qca808x: Add regulator management

[PATCH net-next] net: phy: qcom: qca808x: Add regulator management

[Alex Elder]

From: Daniel Thompson <daniel@riscstar.com>

QCA8081 appears in embedded board designs paired with GPIO controlled
regulators for its power rails. Add logic to allow these regulators to
be turned on during a probe.

In order to avoid the complexity of tracking state for suspend with and
without WoL we take a tremendously simple "always-on" approach to
regulator management, essentially relying on BMCR_PDOWN to conserve
power when the phy device exists.

Signed-off-by: Daniel Thompson <daniel@riscstar.com>
Co-developed-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 drivers/net/phy/qcom/qca808x.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/phy/qcom/qca808x.c b/drivers/net/phy/qcom/qca808x.c
index bab26e4c61408..6e2bd753257d9 100644
--- a/drivers/net/phy/qcom/qca808x.c
+++ b/drivers/net/phy/qcom/qca808x.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 
 #include <linux/phy.h>
+#include <linux/regulator/consumer.h>
 #include <linux/module.h>
 
 #include "qcom.h"
@@ -181,15 +182,44 @@ static void qca808x_fill_possible_interfaces(struct phy_device *phydev)
 		__set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
 }
 
+static int qca808x_power_on(struct phy_device *phydev)
+{
+#ifdef CONFIG_OF
+	static const char * const regulator_names[] = {
+		"avdd18", "vdd", "vdd18", "vdd125"
+	};
+	struct device *dev = &phydev->mdio.dev;
+	u32 count = 0;
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(regulator_names); i++) {
+		ret = devm_regulator_get_enable_optional(dev, regulator_names[i]);
+		if (!ret)
+			count++;
+		else if (ret != -ENODEV)
+			return ret;
+	}
+
+	if (count)
+		fsleep(phydev->mdio.reset_assert_delay);
+#endif
+	return 0;
+}
+
 static int qca808x_probe(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
 	struct qca808x_priv *priv;
+	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
+	ret = qca808x_power_on(phydev);
+	if (ret)
+		return ret;
+
 	priv->led_polarity_mode = QCA808X_PHY_LED_UNSET;
 
 	phydev->priv = priv;
-- 
2.53.0

Re: [PATCH net-next] net: phy: qcom: qca808x: Add regulator management

[Andrew Lunn]

On Wed, Oct 14, 2026 at 12:22:59PM -0500, Alex Elder wrote:
> From: Daniel Thompson <daniel@riscstar.com>
> 
> QCA8081 appears in embedded board designs paired with GPIO controlled
> regulators for its power rails. Add logic to allow these regulators to
> be turned on during a probe.
> 
> In order to avoid the complexity of tracking state for suspend with and
> without WoL we take a tremendously simple "always-on" approach to
> regulator management, essentially relying on BMCR_PDOWN to conserve
> power when the phy device exists.
> 
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> Co-developed-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
>  drivers/net/phy/qcom/qca808x.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/net/phy/qcom/qca808x.c b/drivers/net/phy/qcom/qca808x.c
> index bab26e4c61408..6e2bd753257d9 100644
> --- a/drivers/net/phy/qcom/qca808x.c
> +++ b/drivers/net/phy/qcom/qca808x.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  
>  #include <linux/phy.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/module.h>
>  
>  #include "qcom.h"
> @@ -181,15 +182,44 @@ static void qca808x_fill_possible_interfaces(struct phy_device *phydev)
>  		__set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
>  }
>  
> +static int qca808x_power_on(struct phy_device *phydev)
> +{
> +#ifdef CONFIG_OF

Does this need to be conditional? It seems like
devm_regulator_get_enable_optional() should return ENODEV, which you
already handle.

Making code this like conditional makes build testing less useful,
more combinations which need testing.

Another option is

	if (IS_ENABLED(CONFIG_OF)) {


	}

That at least compiles the code, and then the optimizier throws it
away when it sees  if (0) { }

     Andrew

Re: [PATCH net-next] net: phy: qcom: qca808x: Add regulator management

[Alex Elder]

On 10/14/26 12:36 PM, Andrew Lunn wrote:
> On Wed, Oct 14, 2026 at 12:22:59PM -0500, Alex Elder wrote:
>> From: Daniel Thompson <daniel@riscstar.com>
>>
>> QCA8081 appears in embedded board designs paired with GPIO controlled
>> regulators for its power rails. Add logic to allow these regulators to
>> be turned on during a probe.
>>
>> In order to avoid the complexity of tracking state for suspend with and
>> without WoL we take a tremendously simple "always-on" approach to
>> regulator management, essentially relying on BMCR_PDOWN to conserve
>> power when the phy device exists.
>>
>> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
>> Co-developed-by: Alex Elder <elder@riscstar.com>
>> Signed-off-by: Alex Elder <elder@riscstar.com>
>> ---
>>   drivers/net/phy/qcom/qca808x.c | 30 ++++++++++++++++++++++++++++++
>>   1 file changed, 30 insertions(+)
>>
>> diff --git a/drivers/net/phy/qcom/qca808x.c b/drivers/net/phy/qcom/qca808x.c
>> index bab26e4c61408..6e2bd753257d9 100644
>> --- a/drivers/net/phy/qcom/qca808x.c
>> +++ b/drivers/net/phy/qcom/qca808x.c
>> @@ -1,6 +1,7 @@
>>   // SPDX-License-Identifier: GPL-2.0+
>>   
>>   #include <linux/phy.h>
>> +#include <linux/regulator/consumer.h>
>>   #include <linux/module.h>
>>   
>>   #include "qcom.h"
>> @@ -181,15 +182,44 @@ static void qca808x_fill_possible_interfaces(struct phy_device *phydev)
>>   		__set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
>>   }
>>   
>> +static int qca808x_power_on(struct phy_device *phydev)
>> +{
>> +#ifdef CONFIG_OF
> 
> Does this need to be conditional? It seems like
> devm_regulator_get_enable_optional() should return ENODEV, which you
> already handle.
> 
> Making code this like conditional makes build testing less useful,
> more combinations which need testing.

This is a really good point.

> Another option is
> 
> 	if (IS_ENABLED(CONFIG_OF)) {
> 
> 
> 	}
> 
> That at least compiles the code, and then the optimizier throws it
> away when it sees  if (0) { }

Yes I like doing it this way better.  I'll modify it
to be similar to this for v2, which I'll plan to send
tomorrow.

Thanks a lot.

					-Alex

> 
>       Andrew

Re: [PATCH net-next] net: phy: qcom: qca808x: Add regulator management

[Daniel Thompson]

On Wed, Oct 14, 2026 at 03:41:20PM -0500, Alex Elder wrote:
> On 10/14/26 12:36 PM, Andrew Lunn wrote:
> > On Wed, Oct 14, 2026 at 12:22:59PM -0500, Alex Elder wrote:
> > > From: Daniel Thompson <daniel@riscstar.com>
> > >
> > > QCA8081 appears in embedded board designs paired with GPIO controlled
> > > regulators for its power rails. Add logic to allow these regulators to
> > > be turned on during a probe.
> > >
> > > In order to avoid the complexity of tracking state for suspend with and
> > > without WoL we take a tremendously simple "always-on" approach to
> > > regulator management, essentially relying on BMCR_PDOWN to conserve
> > > power when the phy device exists.
> > >
> > > Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> > > Co-developed-by: Alex Elder <elder@riscstar.com>
> > > Signed-off-by: Alex Elder <elder@riscstar.com>
> > > ---
> > >   drivers/net/phy/qcom/qca808x.c | 30 ++++++++++++++++++++++++++++++
> > >   1 file changed, 30 insertions(+)
> > >
> > > diff --git a/drivers/net/phy/qcom/qca808x.c b/drivers/net/phy/qcom/qca808x.c
> > > index bab26e4c61408..6e2bd753257d9 100644
> > > --- a/drivers/net/phy/qcom/qca808x.c
> > > +++ b/drivers/net/phy/qcom/qca808x.c
> > > @@ -1,6 +1,7 @@
> > >   // SPDX-License-Identifier: GPL-2.0+
> > >   #include <linux/phy.h>
> > > +#include <linux/regulator/consumer.h>
> > >   #include <linux/module.h>
> > >   #include "qcom.h"
> > > @@ -181,15 +182,44 @@ static void qca808x_fill_possible_interfaces(struct phy_device *phydev)
> > >   		__set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
> > >   }
> > > +static int qca808x_power_on(struct phy_device *phydev)
> > > +{
> > > +#ifdef CONFIG_OF
> >
> > Does this need to be conditional? It seems like
> > devm_regulator_get_enable_optional() should return ENODEV, which you
> > already handle.
> >
> > Making code this like conditional makes build testing less useful,
> > more combinations which need testing.
>
> This is a really good point.
>
> > Another option is
> >
> > 	if (IS_ENABLED(CONFIG_OF)) {
> >
> >
> > 	}
> >
> > That at least compiles the code, and then the optimizier throws it
> > away when it sees  if (0) { }
>
> Yes I like doing it this way better.  I'll modify it
> to be similar to this for v2, which I'll plan to send
> tomorrow.

Reading things back, I think Andrew's first suggestion of having no
conditional might be better.

devm_regulator_get_enable_optional() is log-silent on failure. I suspect
the reason this code was conditional is that an early version used
devm_regulator_get_enable() which is not log-silent when it falls back
to a dummy reg.

I suspect I was more influenced by the log-silence than the marginal
code size cost on a CONFIG_REGULATOR && !CONFIG_OF system.


Daniel.