linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] riscv: dts: starfive: Add VisionFive 2 PHY supplies
@ 2023-09-23 15:26 Daniel Maslowski
  2023-09-24 12:22 ` Emil Renner Berthing
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Maslowski @ 2023-09-23 15:26 UTC (permalink / raw)
  To: linux-riscv, devicetree, linux-kernel
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski,
	Emil Renner Berthing, Daniel Maslowski

I checked the schematics to see what power supplies are needed.
This allows for using ethernet without other drivers enabled
that would coincidentally enable the same power supply. ⚡

Signed-off-by: Daniel Maslowski <cyrevolt@gmail.com>
---
 .../boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts    | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
index d4ea4a2c0b9b..bc1ac12bc71b 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
@@ -16,12 +16,14 @@ &gmac0 {
 	starfive,tx-use-rgmii-clk;
 	assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
 	assigned-clock-parents = <&aoncrg JH7110_AONCLK_GMAC0_RMII_RTX>;
+	phy-supply = <&vcc_3v3>;
 };
 
 &gmac1 {
 	starfive,tx-use-rgmii-clk;
 	assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>;
 	assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>;
+	phy-supply = <&vcc_3v3>;
 };
 
 &phy0 {
-- 
2.34.1


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

* Re: [PATCH] riscv: dts: starfive: Add VisionFive 2 PHY supplies
  2023-09-23 15:26 [PATCH] riscv: dts: starfive: Add VisionFive 2 PHY supplies Daniel Maslowski
@ 2023-09-24 12:22 ` Emil Renner Berthing
  2023-09-27  3:46   ` Walker Chen
  0 siblings, 1 reply; 3+ messages in thread
From: Emil Renner Berthing @ 2023-09-24 12:22 UTC (permalink / raw)
  To: Daniel Maslowski, Walker Chen, linux-riscv, devicetree, linux-kernel
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski,
	Emil Renner Berthing, Daniel Maslowski

Daniel Maslowski wrote:
> I checked the schematics to see what power supplies are needed.
> This allows for using ethernet without other drivers enabled
> that would coincidentally enable the same power supply. ⚡
>
> Signed-off-by: Daniel Maslowski <cyrevolt@gmail.com>
> ---
>  .../boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts    | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
> index d4ea4a2c0b9b..bc1ac12bc71b 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
> @@ -16,12 +16,14 @@ &gmac0 {
>  	starfive,tx-use-rgmii-clk;
>  	assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
>  	assigned-clock-parents = <&aoncrg JH7110_AONCLK_GMAC0_RMII_RTX>;
> +	phy-supply = <&vcc_3v3>;
>  };
>
>  &gmac1 {
>  	starfive,tx-use-rgmii-clk;
>  	assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>;
>  	assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>;
> +	phy-supply = <&vcc_3v3>;
>  };

Hi Daniel,

Thank you for the patch! I've looked into this a bit now and I found 3 things:

1) The vcc_3v3 regulator (DCDC1 on the PMIC) already has the
   regulator-always-on property, so should never be turned off even if there
   are no consumers.

2) As far as I can tell the schematic for both the 1.2A and 1.3B says the PHYs
   are powered by DCDC6 (and not DCDC1) on the PMIC via the VDD33_IO line.

3) There doesn't seem to be any generic code in the dwmac driver handling the
   phy-supply property, only the Rockchip wrapper (dwmac-rk.c) does that.

So all in all I'm a little confused how this change can make a difference for
you.

@Walker does the above and 2) in particular seem correct to you?

I'd expect something like the following instead (although split in 3 patches).
Daniel, does this work for you?

diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
index 5e7cfbbebce6..9ce877782419 100644
--- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
@@ -54,6 +54,9 @@ properties:
     minItems: 3
     maxItems: 3

+  phy-supply:
+    description: PHY regulator
+
   resets:
     items:
       - description: MAC Reset signal.
diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index 2209d5348928..6633c8d29be7 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -101,6 +101,7 @@ &tdm_ext {
 &gmac0 {
 	phy-handle = <&phy0>;
 	phy-mode = "rgmii-id";
+	phy-supply = <&vdd33_io>;
 	status = "okay";

 	mdio {
@@ -117,6 +118,7 @@ phy0: ethernet-phy@0 {
 &gmac1 {
 	phy-handle = <&phy1>;
 	phy-mode = "rgmii-id";
+	phy-supply = <&vdd33_io>;
 	status = "okay";

 	mdio {
@@ -182,6 +184,13 @@ vdd_cpu: dcdc2 {
 				regulator-name = "vdd-cpu";
 			};

+			vdd33_io: dcdc6 {
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vdd33_io";
+			};
+
 			emmc_vdd: aldo4 {
 				regulator-boot-on;
 				regulator-always-on;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
index 9289bb87c3e3..2ce349421981 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
@@ -12,6 +12,7 @@
 #include <linux/property.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>

 #include "stmmac_platform.h"

@@ -97,6 +98,7 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
 	struct plat_stmmacenet_data *plat_dat;
 	struct stmmac_resources stmmac_res;
 	struct starfive_dwmac *dwmac;
+	struct regulator *reg_phy;
 	struct clk *clk_gtx;
 	int err;

@@ -124,6 +126,11 @@ static int starfive_dwmac_probe(struct
platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(clk_gtx),
 				     "error getting gtx clock\n");

+	reg_phy = devm_regulator_get_enable_optional(&pdev->dev, "phy");
+	if (IS_ERR(reg_phy))
+		return dev_err_probe(&pdev->dev, PTR_ERR(reg_phy),
+				     "error getting phy regulator\n");
+
 	/* Generally, the rgmii_tx clock is provided by the internal clock,
 	 * which needs to match the corresponding clock frequency according
 	 * to different speeds. If the rgmii_tx clock is provided by the

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

* Re: [PATCH] riscv: dts: starfive: Add VisionFive 2 PHY supplies
  2023-09-24 12:22 ` Emil Renner Berthing
@ 2023-09-27  3:46   ` Walker Chen
  0 siblings, 0 replies; 3+ messages in thread
From: Walker Chen @ 2023-09-27  3:46 UTC (permalink / raw)
  To: Emil Renner Berthing, Daniel Maslowski, linux-riscv, devicetree,
	linux-kernel
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski,
	Emil Renner Berthing, Daniel Maslowski



On 2023/9/24 20:22, Emil Renner Berthing wrote:
> Daniel Maslowski wrote:
>> I checked the schematics to see what power supplies are needed.
>> This allows for using ethernet without other drivers enabled
>> that would coincidentally enable the same power supply. ⚡
>>
>> Signed-off-by: Daniel Maslowski <cyrevolt@gmail.com>
>> ---
>>  .../boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts    | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
>> index d4ea4a2c0b9b..bc1ac12bc71b 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
>> @@ -16,12 +16,14 @@ &gmac0 {
>>  	starfive,tx-use-rgmii-clk;
>>  	assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
>>  	assigned-clock-parents = <&aoncrg JH7110_AONCLK_GMAC0_RMII_RTX>;
>> +	phy-supply = <&vcc_3v3>;
>>  };
>>
>>  &gmac1 {
>>  	starfive,tx-use-rgmii-clk;
>>  	assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>;
>>  	assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>;
>> +	phy-supply = <&vcc_3v3>;
>>  };
> 
> Hi Daniel,
> 
> Thank you for the patch! I've looked into this a bit now and I found 3 things:
> 
> 1) The vcc_3v3 regulator (DCDC1 on the PMIC) already has the
>    regulator-always-on property, so should never be turned off even if there
>    are no consumers.
> 
> 2) As far as I can tell the schematic for both the 1.2A and 1.3B says the PHYs
>    are powered by DCDC6 (and not DCDC1) on the PMIC via the VDD33_IO line.
> 
> 3) There doesn't seem to be any generic code in the dwmac driver handling the
>    phy-supply property, only the Rockchip wrapper (dwmac-rk.c) does that.
> 
> So all in all I'm a little confused how this change can make a difference for
> you.
> 
> @Walker does the above and 2) in particular seem correct to you?

I checked the schematic of VisionFive2 again, the GMAC PHY is powered by DCDC6 via the VDD33_IO line,
not DCDC1. So there's no need to add the property 'phy-supply' and handler code for regulator in probe().

> 
> I'd expect something like the following instead (although split in 3 patches).
> Daniel, does this work for you?
> 
> diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> index 5e7cfbbebce6..9ce877782419 100644
> --- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> @@ -54,6 +54,9 @@ properties:
>      minItems: 3
>      maxItems: 3
> 
> +  phy-supply:
> +    description: PHY regulator
> +
>    resets:
>      items:
>        - description: MAC Reset signal.
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index 2209d5348928..6633c8d29be7 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -101,6 +101,7 @@ &tdm_ext {
>  &gmac0 {
>  	phy-handle = <&phy0>;
>  	phy-mode = "rgmii-id";
> +	phy-supply = <&vdd33_io>;
>  	status = "okay";
> 
>  	mdio {
> @@ -117,6 +118,7 @@ phy0: ethernet-phy@0 {
>  &gmac1 {
>  	phy-handle = <&phy1>;
>  	phy-mode = "rgmii-id";
> +	phy-supply = <&vdd33_io>;
>  	status = "okay";
> 
>  	mdio {
> @@ -182,6 +184,13 @@ vdd_cpu: dcdc2 {
>  				regulator-name = "vdd-cpu";
>  			};
> 
> +			vdd33_io: dcdc6 {
> +				regulator-boot-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vdd33_io";
> +			};
> +
>  			emmc_vdd: aldo4 {
>  				regulator-boot-on;
>  				regulator-always-on;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> index 9289bb87c3e3..2ce349421981 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> @@ -12,6 +12,7 @@
>  #include <linux/property.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> 
>  #include "stmmac_platform.h"
> 
> @@ -97,6 +98,7 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
>  	struct plat_stmmacenet_data *plat_dat;
>  	struct stmmac_resources stmmac_res;
>  	struct starfive_dwmac *dwmac;
> +	struct regulator *reg_phy;
>  	struct clk *clk_gtx;
>  	int err;
> 
> @@ -124,6 +126,11 @@ static int starfive_dwmac_probe(struct
> platform_device *pdev)
>  		return dev_err_probe(&pdev->dev, PTR_ERR(clk_gtx),
>  				     "error getting gtx clock\n");
> 
> +	reg_phy = devm_regulator_get_enable_optional(&pdev->dev, "phy");
> +	if (IS_ERR(reg_phy))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(reg_phy),
> +				     "error getting phy regulator\n");
> +
>  	/* Generally, the rgmii_tx clock is provided by the internal clock,
>  	 * which needs to match the corresponding clock frequency according
>  	 * to different speeds. If the rgmii_tx clock is provided by the

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

end of thread, other threads:[~2023-09-27  4:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-23 15:26 [PATCH] riscv: dts: starfive: Add VisionFive 2 PHY supplies Daniel Maslowski
2023-09-24 12:22 ` Emil Renner Berthing
2023-09-27  3:46   ` Walker Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).