Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/2] net: phy: qcom: add qca808x support
@ 2026-10-15 14:38 Alex Elder
  2026-10-15 14:38 ` [PATCH net-next v2 1/2] dt-bindings: net: qca,qca808x: Add regulator properties Alex Elder
  2026-10-15 14:38 ` [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management Alex Elder
  0 siblings, 2 replies; 12+ messages in thread
From: Alex Elder @ 2026-10-15 14:38 UTC (permalink / raw)
  To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, hkallweit1, linux
  Cc: ansuelsmth, donggeunyoo.kernel, mohd.anwar, lorenzo.bianconi,
	daniel, devicetree, netdev, linux-arm-msm, linux-kernel

The Qualcomm QCA8081 PHY (and compatible devices) has four power inputs
available.  Not all designs need to control them, and some might connect
multiple power inputs to the same rail.  This series makes it possible
to optionally specify these regulators.

Between version 1 and version 2:
- Add a devicetree binding update that was not provided in v1
- Remove #ifdef CONFIG_OF from the body of qca808x_power_on()

Version 1 is available here:
  https://lore.kernel.org/lkml/20261014172300.292324-1-elder@riscstar.com/

					-Alex

Daniel Thompson (2):
  dt-bindings: net: qca,qca808x: Add regulator properties
  net: phy: qcom: qca808x: Add regulator management

 .../devicetree/bindings/net/qca,qca808x.yaml  | 16 ++++++++++
 drivers/net/phy/qcom/qca808x.c                | 29 +++++++++++++++++++
 2 files changed, 45 insertions(+)


base-commit: 73022f605c447f31610b94ed4a983d2b4bf2c721
-- 
2.53.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 1/2] dt-bindings: net: qca,qca808x: Add regulator properties
  2026-10-15 14:38 [PATCH net-next v2 0/2] net: phy: qcom: add qca808x support Alex Elder
@ 2026-10-15 14:38 ` Alex Elder
  2026-10-17 16:54   ` Krzysztof Kozlowski
  2026-10-15 14:38 ` [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management Alex Elder
  1 sibling, 1 reply; 12+ messages in thread
From: Alex Elder @ 2026-10-15 14:38 UTC (permalink / raw)
  To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, hkallweit1, linux
  Cc: Daniel Thompson, ansuelsmth, donggeunyoo.kernel, mohd.anwar,
	lorenzo.bianconi, devicetree, netdev, linux-arm-msm, linux-kernel

From: Daniel Thompson <daniel@riscstar.com>

QCA808x has four different power rails (although in many board designs
the different rails share a regulator). Add each of these to the
corresponding DT binding as optional supplies.

Signed-off-by: Daniel Thompson <daniel@riscstar.com>
Co-developed-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 .../devicetree/bindings/net/qca,qca808x.yaml     | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qca,qca808x.yaml b/Documentation/devicetree/bindings/net/qca,qca808x.yaml
index e2552655902a3..03daef1c7a4e6 100644
--- a/Documentation/devicetree/bindings/net/qca,qca808x.yaml
+++ b/Documentation/devicetree/bindings/net/qca,qca808x.yaml
@@ -17,6 +17,8 @@ description:
 
   By default this special PIN is set to LED function.
 
+  Additionally, up to four power supplies may be separately specified.
+
 allOf:
   - $ref: ethernet-phy.yaml#
 
@@ -25,6 +27,18 @@ properties:
     enum:
       - ethernet-phy-id004d.d101
 
+  avdd18-supply:
+    description: 1.8v analog power supply.
+
+  vdd-supply:
+    description: 1.05v power supply.
+
+  vdd18-supply:
+    description: 1.8v power supply.
+
+  vdd125-supply:
+    description: (1.25v to 1.98v) LDO regulator power supply.
+
 unevaluatedProperties: false
 
 examples:
@@ -39,6 +53,8 @@ examples:
             compatible = "ethernet-phy-id004d.d101";
             reg = <0>;
 
+            vdd18-supply = <&phy_vreg_1v8>;
+
             leds {
                 #address-cells = <1>;
                 #size-cells = <0>;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management
  2026-10-15 14:38 [PATCH net-next v2 0/2] net: phy: qcom: add qca808x support Alex Elder
  2026-10-15 14:38 ` [PATCH net-next v2 1/2] dt-bindings: net: qca,qca808x: Add regulator properties Alex Elder
@ 2026-10-15 14:38 ` Alex Elder
  2026-10-15 15:16   ` Maxime Chevallier
                     ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Alex Elder @ 2026-10-15 14:38 UTC (permalink / raw)
  To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, hkallweit1, linux
  Cc: Daniel Thompson, ansuelsmth, donggeunyoo.kernel, mohd.anwar,
	lorenzo.bianconi, devicetree, netdev, linux-arm-msm, linux-kernel

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 | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/net/phy/qcom/qca808x.c b/drivers/net/phy/qcom/qca808x.c
index bab26e4c61408..4aba513b9a512 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,43 @@ 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)
+{
+	static const char * const regulator_names[] = {
+		"avdd18", "vdd", "vdd18", "vdd125"
+	};
+	struct device *dev = &phydev->mdio.dev;
+	u32 count = 0;
+	int ret;
+
+	for (u32 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);
+
+	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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management
  2026-10-15 14:38 ` [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management Alex Elder
@ 2026-10-15 15:16   ` Maxime Chevallier
  2026-10-15 15:40   ` Lorenzo Bianconi
  2026-10-19 14:38   ` netdev-bot+sashiko
  2 siblings, 0 replies; 12+ messages in thread
From: Maxime Chevallier @ 2026-10-15 15:16 UTC (permalink / raw)
  To: Alex Elder, andrew+netdev, davem, edumazet, kuba, pabeni, robh,
	krzk+dt, conor+dt, hkallweit1, linux
  Cc: Daniel Thompson, ansuelsmth, donggeunyoo.kernel, mohd.anwar,
	lorenzo.bianconi, devicetree, netdev, linux-arm-msm, linux-kernel

Hi,

On 10/15/26 16:38, 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 | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/net/phy/qcom/qca808x.c b/drivers/net/phy/qcom/qca808x.c
> index bab26e4c61408..4aba513b9a512 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,43 @@ 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)
> +{
> +	static const char * const regulator_names[] = {
> +		"avdd18", "vdd", "vdd18", "vdd125"
> +	};
> +	struct device *dev = &phydev->mdio.dev;
> +	u32 count = 0;
> +	int ret;
> +
> +	for (u32 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);

Kinda makes sense to use that delay as a basis, it may be too short in
some odd cases but I guess it's good enough of an approximation here,

> +
> +	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;

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management
  2026-10-15 14:38 ` [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management Alex Elder
  2026-10-15 15:16   ` Maxime Chevallier
@ 2026-10-15 15:40   ` Lorenzo Bianconi
  2026-10-15 16:01     ` Alex Elder
  2026-10-19 14:38   ` netdev-bot+sashiko
  2 siblings, 1 reply; 12+ messages in thread
From: Lorenzo Bianconi @ 2026-10-15 15:40 UTC (permalink / raw)
  To: Alex Elder
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, hkallweit1, linux, Daniel Thompson, ansuelsmth,
	donggeunyoo.kernel, mohd.anwar, devicetree, netdev, linux-arm-msm,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2458 bytes --]

On Oct 15, 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 | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/net/phy/qcom/qca808x.c b/drivers/net/phy/qcom/qca808x.c
> index bab26e4c61408..4aba513b9a512 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,43 @@ 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)
> +{
> +	static const char * const regulator_names[] = {
> +		"avdd18", "vdd", "vdd18", "vdd125"
> +	};
> +	struct device *dev = &phydev->mdio.dev;
> +	u32 count = 0;
> +	int ret;
> +
> +	for (u32 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);

nit: should we check reset_assert_delay field is actually set?

Regards,
Lorenzo

> +
> +	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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management
  2026-10-15 15:40   ` Lorenzo Bianconi
@ 2026-10-15 16:01     ` Alex Elder
  2026-10-15 16:27       ` Andrew Lunn
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2026-10-15 16:01 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, hkallweit1, linux, Daniel Thompson, ansuelsmth,
	donggeunyoo.kernel, mohd.anwar, devicetree, netdev, linux-arm-msm,
	linux-kernel

On 10/15/26 10:40 AM, Lorenzo Bianconi wrote:
>> +	if (count)
>> +		fsleep(phydev->mdio.reset_assert_delay);
> nit: should we check reset_assert_delay field is actually set?

Are you saying there should be an alternative delay used
if it is not set?  Do you have a suggestion for what to
use in that case?

					-Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management
  2026-10-15 16:01     ` Alex Elder
@ 2026-10-15 16:27       ` Andrew Lunn
  2026-10-15 16:37         ` Mohd Ayaan Anwar
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2026-10-15 16:27 UTC (permalink / raw)
  To: Alex Elder
  Cc: Lorenzo Bianconi, andrew+netdev, davem, edumazet, kuba, pabeni,
	robh, krzk+dt, conor+dt, hkallweit1, linux, Daniel Thompson,
	ansuelsmth, donggeunyoo.kernel, mohd.anwar, devicetree, netdev,
	linux-arm-msm, linux-kernel

On Thu, Oct 15, 2026 at 11:01:43AM -0500, Alex Elder wrote:
> On 10/15/26 10:40 AM, Lorenzo Bianconi wrote:
> > > +	if (count)
> > > +		fsleep(phydev->mdio.reset_assert_delay);
> > nit: should we check reset_assert_delay field is actually set?
> 
> Are you saying there should be an alternative delay used
> if it is not set?  Do you have a suggestion for what to
> use in that case?

Does the binding require the value is set?

What happens it if is 0? I think fsleep() is happy to delay for 0 uS,
but does the hardware still work? Does the datasheet say anything
about power on delays?

      Andrew

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management
  2026-10-15 16:27       ` Andrew Lunn
@ 2026-10-15 16:37         ` Mohd Ayaan Anwar
  2026-10-16 15:17           ` Andrew Lunn
  0 siblings, 1 reply; 12+ messages in thread
From: Mohd Ayaan Anwar @ 2026-10-15 16:37 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Alex Elder, Lorenzo Bianconi, andrew+netdev, davem, edumazet,
	kuba, pabeni, robh, krzk+dt, conor+dt, hkallweit1, linux,
	Daniel Thompson, ansuelsmth, donggeunyoo.kernel, devicetree,
	netdev, linux-arm-msm, linux-kernel

On Thu, Oct 15, 2026 at 06:27:01PM +0200, Andrew Lunn wrote:
> On Thu, Oct 15, 2026 at 11:01:43AM -0500, Alex Elder wrote:
> > On 10/15/26 10:40 AM, Lorenzo Bianconi wrote:
> > > > +	if (count)
> > > > +		fsleep(phydev->mdio.reset_assert_delay);
> > > nit: should we check reset_assert_delay field is actually set?
> > 
> > Are you saying there should be an alternative delay used
> > if it is not set?  Do you have a suggestion for what to
> > use in that case?
> 
> Does the binding require the value is set?
> 
> What happens it if is 0? I think fsleep() is happy to delay for 0 uS,
> but does the hardware still work? Does the datasheet say anything
> about power on delays?
> 

I was going to share a text version of the power-up diagram given in
this PHY's datasheet. This might help.

Power-Up Sequence

1. DVDD asserted
   |
   |<-------- 10 ms -------->|   (required for external power)
   |
2. DVDD18 / AVDD18 asserted

3. VDD1P25 asserted
   |
   |<-------- 0.2 ms ------->|   (required for LDO to settle)
   |
4. VDD_LDO asserted

5. AVDD asserted
   |
   |<--------- 2 ms -------->|   (required for XTL to oscillate)
   |
6. Crystal 50 MHz starts oscillating

7. PAD reset deasserted
   |
   |<------- 10 ms --------->|   (deglitch time)


So, maybe just fsleep(22200) or something a bit higher?

	Ayaan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management
  2026-10-15 16:37         ` Mohd Ayaan Anwar
@ 2026-10-16 15:17           ` Andrew Lunn
  2026-10-17  8:12             ` Mohd Ayaan Anwar
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2026-10-16 15:17 UTC (permalink / raw)
  To: Mohd Ayaan Anwar
  Cc: Alex Elder, Lorenzo Bianconi, andrew+netdev, davem, edumazet,
	kuba, pabeni, robh, krzk+dt, conor+dt, hkallweit1, linux,
	Daniel Thompson, ansuelsmth, donggeunyoo.kernel, devicetree,
	netdev, linux-arm-msm, linux-kernel

> I was going to share a text version of the power-up diagram given in
> this PHY's datasheet. This might help.
> 
> Power-Up Sequence
> 
> 1. DVDD asserted
>    |
>    |<-------- 10 ms -------->|   (required for external power)
>    |
> 2. DVDD18 / AVDD18 asserted
> 
> 3. VDD1P25 asserted
>    |
>    |<-------- 0.2 ms ------->|   (required for LDO to settle)
>    |
> 4. VDD_LDO asserted
> 
> 5. AVDD asserted
>    |
>    |<--------- 2 ms -------->|   (required for XTL to oscillate)
>    |
> 6. Crystal 50 MHz starts oscillating
> 
> 7. PAD reset deasserted
>    |
>    |<------- 10 ms --------->|   (deglitch time)
> 

Are these supposed to be staggered? Or can you turn them all on at
once.

23ms does seem like the worst case.

	Andrew

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management
  2026-10-16 15:17           ` Andrew Lunn
@ 2026-10-17  8:12             ` Mohd Ayaan Anwar
  0 siblings, 0 replies; 12+ messages in thread
From: Mohd Ayaan Anwar @ 2026-10-17  8:12 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Alex Elder, Lorenzo Bianconi, andrew+netdev, davem, edumazet,
	kuba, pabeni, robh, krzk+dt, conor+dt, hkallweit1, linux,
	Daniel Thompson, ansuelsmth, donggeunyoo.kernel, devicetree,
	netdev, linux-arm-msm, linux-kernel

On Fri, Oct 16, 2026 at 05:17:20PM +0200, Andrew Lunn wrote:
> > I was going to share a text version of the power-up diagram given in
> > this PHY's datasheet. This might help.
> > 
> > Power-Up Sequence
> > 
> > 1. DVDD asserted
> >    |
> >    |<-------- 10 ms -------->|   (required for external power)
> >    |
> > 2. DVDD18 / AVDD18 asserted
> > 
> > 3. VDD1P25 asserted
> >    |
> >    |<-------- 0.2 ms ------->|   (required for LDO to settle)
> >    |
> > 4. VDD_LDO asserted
> > 
> > 5. AVDD asserted
> >    |
> >    |<--------- 2 ms -------->|   (required for XTL to oscillate)
> >    |
> > 6. Crystal 50 MHz starts oscillating
> > 
> > 7. PAD reset deasserted
> >    |
> >    |<------- 10 ms --------->|   (deglitch time)
> > 
> 
> Are these supposed to be staggered? Or can you turn them all on at
> once.

Its a confusing diagram but from what I gather: there's no delay
required between 1., 3., and 5. They are shown as partially overlapping.
This might be a better representation of the diagram:


DVDD          ..../-------------------------------------------------
                  |<--------- 10 ms ----------->|
                  |    (external power)         |
DVDD18/AVDD18 ................................../-------------------

VDD1P25       ............../---------------------------------------
                            |<0.2 ms>|
                            | LDO settling
VDD_LDO       ......................../-----------------------------

AVDD          ......................../-----------------------------
                                                |< 2 ms >|
                                                | crystal startup
Crystal 50 MHz ........................................../\/\/\/\/\/

PAD reset     ..................................................../---
                                                          |<10 ms>|
                                                          deglitch

> 
> 23ms does seem like the worst case.
> 

Yes, I thought that 23ms would be a safe value after looking at the diagram.

	Ayaan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 1/2] dt-bindings: net: qca,qca808x: Add regulator properties
  2026-10-15 14:38 ` [PATCH net-next v2 1/2] dt-bindings: net: qca,qca808x: Add regulator properties Alex Elder
@ 2026-10-17 16:54   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-10-17 16:54 UTC (permalink / raw)
  To: Alex Elder
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, hkallweit1, linux, Daniel Thompson, ansuelsmth,
	donggeunyoo.kernel, mohd.anwar, lorenzo.bianconi, devicetree,
	netdev, linux-arm-msm, linux-kernel

On Thu, Oct 15, 2026 at 09:38:46AM -0500, Alex Elder wrote:
> From: Daniel Thompson <daniel@riscstar.com>
> 
> QCA808x has four different power rails (although in many board designs
> the different rails share a regulator). Add each of these to the
> corresponding DT binding as optional supplies.
> 
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> Co-developed-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
>  .../devicetree/bindings/net/qca,qca808x.yaml     | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management
  2026-10-15 14:38 ` [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management Alex Elder
  2026-10-15 15:16   ` Maxime Chevallier
  2026-10-15 15:40   ` Lorenzo Bianconi
@ 2026-10-19 14:38   ` netdev-bot+sashiko
  2 siblings, 0 replies; 12+ messages in thread
From: netdev-bot+sashiko @ 2026-10-19 14:38 UTC (permalink / raw)
  To: elder
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, hkallweit1, linux, daniel, ansuelsmth,
	donggeunyoo.kernel, mohd.anwar, lorenzo.bianconi, devicetree,
	netdev, linux-arm-msm, linux-kernel

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] qca808x_power_on() (drivers/net/phy/qcom/qca808x.c) turns on
  the QCA8081 supplies only after phylib has already released the PHY…
- [Low] Kept only because requires_validation=true; the baseline decision
  is a false positive.

--

net: phy: qcom: qca808x: Add regulator management

This commit adds optional regulator handling to the qca808x PHY driver.
qca808x_probe() now enables the avdd18, vdd, vdd18 and vdd125 supplies
with devm_regulator_get_enable_optional() and leaves them on for the
lifetime of the device. If any supply was found, the driver then waits
for reset_assert_delay before it continues.

> diff --git a/drivers/net/phy/qcom/qca808x.c b/drivers/net/phy/qcom/qca808x.c
> index bab26e4c61408..4aba513b9a512 100644
> --- a/drivers/net/phy/qcom/qca808x.c
> +++ b/drivers/net/phy/qcom/qca808x.c

[ ... ]

> @@ -181,15 +182,43 @@ 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)
> +{
> +	static const char * const regulator_names[] = {
> +		"avdd18", "vdd", "vdd18", "vdd125"
> +	};
> +	struct device *dev = &phydev->mdio.dev;
> +	u32 count = 0;
> +	int ret;
> +
> +	for (u32 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);

[Severity: Medium]
Is reset_assert_delay the right delay to use here?

This value comes from the reset-assert-us DT property. ethernet-phy.yaml
describes it as:

  reset-assert-us:
    description:
      Delay after the reset was asserted in microseconds. If this
      property is missing the delay will be skipped.

mdio_device_reset() only uses it when reset is being asserted:

    d = value ? mdiodev->reset_assert_delay : mdiodev->reset_deassert_delay;

The qca,qca808x.yaml example has vdd18-supply but no reset-assert-us. For
a board described that way, this call becomes fsleep(0).

The ordering is also a concern. By the time qca808x_power_on() runs,
phylib has already released the reset line, and the PHY had no power at
that point:

phy_device_register()
    phy_device_reset(phydev, 0);
    ...
    device_add()
        phy_probe()
            phy_device_reset(phydev, 0);
            phydev->drv->probe(phydev)
                qca808x_probe()
                    qca808x_power_on()
                        devm_regulator_get_enable_optional()
                        fsleep(phydev->mdio.reset_assert_delay);
            phy_disable_interrupts(phydev);
            phydrv->get_features(phydev);
            ...
            genphy_c45_read_eee_adv(phydev, ...);

So the reset-deassert-us wait has already run out before the rails come
up. MDIO accesses then start as soon as qca808x_probe() returns. The same
thing happens again on unbind and rebind.

Can this lead to MDIO accesses before the PHY has finished powering up?
That could make probe fail, or leave phylib with the wrong supported and
EEE link modes. The exact QCA8081 power-on timing isn't visible from the
code, so this depends on the datasheet.

Existing QCA8081 descriptions such as lemans-evk.dts set the two delays
to different values:

    reset-assert-us = <11000>;
    reset-deassert-us = <70000>;

So the time the PHY needs after reset release is separate from the time
reset is held asserted. If a board raised reset-assert-us to cover
power-up, every later reset assertion would also get slower.

Would it work to assert reset with phy_device_reset(phydev, 1) before
enabling the supplies, then release it with phy_device_reset(phydev, 0)?
That way both documented delays apply in the right order. A fixed,
documented power-up settle time would be another option. The commit
message doesn't mention this sequencing either.

> +
> +	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;

[Severity: Low]
This isn't a bug, but a note in the binding or the commit message might
help. The supplies are only enabled after a phy_device has been created
and matched to this driver.

If the DT node lacks the ethernet-phy-id004d.d101 compatible, or is
marked c45, fwnode_mdiobus_register_phy() reads the ID registers from a
PHY that has no power:

    if (is_c45 || fwnode_get_phy_id(child, &phy_id))
        phy = get_phy_device(bus, addr, is_c45);

get_phy_c22_id() would then read all-ones and return -ENODEV. In that
case qca808x_probe() never runs and the regulators are never turned on.

ethernet-phy.yaml already says the explicit PHY ID compatible is needed
when supplies must be sequenced before the ID can be read. qca,qca808x.yaml
only allows ethernet-phy-id004d.d101, and all in-tree QCA8081 DTs already
use it. So this is mostly about making the documentation clearer.

>  	priv->led_polarity_mode = QCA808X_PHY_LED_UNSET;
>  
>  	phydev->priv = priv;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015143849.483339-1-elder%40riscstar.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-10-19 14:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15 14:38 [PATCH net-next v2 0/2] net: phy: qcom: add qca808x support Alex Elder
2026-10-15 14:38 ` [PATCH net-next v2 1/2] dt-bindings: net: qca,qca808x: Add regulator properties Alex Elder
2026-10-17 16:54   ` Krzysztof Kozlowski
2026-10-15 14:38 ` [PATCH net-next v2 2/2] net: phy: qcom: qca808x: Add regulator management Alex Elder
2026-10-15 15:16   ` Maxime Chevallier
2026-10-15 15:40   ` Lorenzo Bianconi
2026-10-15 16:01     ` Alex Elder
2026-10-15 16:27       ` Andrew Lunn
2026-10-15 16:37         ` Mohd Ayaan Anwar
2026-10-16 15:17           ` Andrew Lunn
2026-10-17  8:12             ` Mohd Ayaan Anwar
2026-10-19 14:38   ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox