LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
@ 2015-01-19 18:08 Romain Perier
  2015-01-19 18:08 ` [PATCH v1 1/4] net: stmmac: dwmac-rk: Don't set the regulator voltage for phy from the driver Romain Perier
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
  To: peppe.cavallaro
  Cc: netdev, linux-kernel, heiko, linux-rockchip, linux-arm-kernel,
	devicetree, roger.chen

This series fixes few issues in dwmac-rk:

1. Voltage settings was hardcoded into the driver for the phy regulator.
   The driver now uses the default voltage settings found in the devicetree, which are
   applied throught the regulator framework.
2. The regulator name used to power on or power off the phy was put in the devicetree
   variable "phy_regulator", which is not standard and added a lot of code for nothing.
   The driver now uses the devicetree property "phy-supply" and the corresponding functions
   to manipulate this regulator.

The corresponding devicetree files are also updated. As, dwmac-rk was recently
pushed in the development tree, I don't care about devicetree backward compatibility
issues.

Romain Perier (4):
  net: stmmac: dwmac-rk: Don't set the regulator voltage for phy from
    the driver
  ARM: dts: Add regulator voltage settings for vcc_phy in
    rk3288-evb.dtsi
  net: stmmac: dwmac-rk: Use standard devicetree property for phy
    regulator
  ARM: dts: Convert gmac to phy-supply in rk3288-evb-rk808.dts

 arch/arm/boot/dts/rk3288-evb-rk808.dts         |  2 +-
 arch/arm/boot/dts/rk3288-evb.dtsi              |  2 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 62 ++++++++------------------
 3 files changed, 22 insertions(+), 44 deletions(-)

-- 
1.9.1


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

* [PATCH v1 1/4] net: stmmac: dwmac-rk: Don't set the regulator voltage for phy from the driver
  2015-01-19 18:08 [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues Romain Perier
@ 2015-01-19 18:08 ` Romain Perier
  2015-01-19 18:08 ` [PATCH v1 2/4] ARM: dts: Add regulator voltage settings for vcc_phy in rk3288-evb.dtsi Romain Perier
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
  To: peppe.cavallaro
  Cc: netdev, linux-kernel, heiko, linux-rockchip, linux-arm-kernel,
	devicetree, roger.chen

As these settings can be directly expressed from devicetree for both fixed
regulators and pmic-integrated regulators, it is more standard to set them
from dts and let the regulator framework use the right voltage informations
when it is used in the driver.

Signed-off-by: Romain Perier <romain.perier@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 35f9b86..06e1637 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -303,7 +303,6 @@ static int phy_power_on(struct rk_priv_data *bsp_priv, bool enable)
 	} else {
 		if (enable) {
 			if (!regulator_is_enabled(ldo)) {
-				regulator_set_voltage(ldo, 3300000, 3300000);
 				ret = regulator_enable(ldo);
 				if (ret != 0)
 					dev_err(dev, "%s: fail to enable %s\n",
-- 
1.9.1


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

* [PATCH v1 2/4] ARM: dts: Add regulator voltage settings for vcc_phy in rk3288-evb.dtsi
  2015-01-19 18:08 [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues Romain Perier
  2015-01-19 18:08 ` [PATCH v1 1/4] net: stmmac: dwmac-rk: Don't set the regulator voltage for phy from the driver Romain Perier
@ 2015-01-19 18:08 ` Romain Perier
  2015-01-19 18:08 ` [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator Romain Perier
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
  To: peppe.cavallaro
  Cc: netdev, linux-kernel, heiko, linux-rockchip, linux-arm-kernel,
	devicetree, roger.chen

Signed-off-by: Romain Perier <romain.perier@gmail.com>
---
 arch/arm/boot/dts/rk3288-evb.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi b/arch/arm/boot/dts/rk3288-evb.dtsi
index 489643c..1c08eb0 100644
--- a/arch/arm/boot/dts/rk3288-evb.dtsi
+++ b/arch/arm/boot/dts/rk3288-evb.dtsi
@@ -98,6 +98,8 @@
 		pinctrl-names = "default";
 		pinctrl-0 = <&eth_phy_pwr>;
 		regulator-name = "vcc_phy";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
 		regulator-always-on;
 		regulator-boot-on;
 	};
-- 
1.9.1


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

* [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator
  2015-01-19 18:08 [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues Romain Perier
  2015-01-19 18:08 ` [PATCH v1 1/4] net: stmmac: dwmac-rk: Don't set the regulator voltage for phy from the driver Romain Perier
  2015-01-19 18:08 ` [PATCH v1 2/4] ARM: dts: Add regulator voltage settings for vcc_phy in rk3288-evb.dtsi Romain Perier
@ 2015-01-19 18:08 ` Romain Perier
  2015-01-19 20:12   ` Heiko Stübner
  2015-01-19 18:08 ` [PATCH v1 4/4] ARM: dts: Convert gmac to phy-supply in rk3288-evb-rk808.dts Romain Perier
  2015-01-19 20:19 ` [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues Heiko Stübner
  4 siblings, 1 reply; 9+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
  To: peppe.cavallaro
  Cc: netdev, linux-kernel, heiko, linux-rockchip, linux-arm-kernel,
	devicetree, roger.chen

Currently, dwmac-rk uses a custom propety "phy_regulator" to get the name of the
right regulator to use to power on or power off the phy. This commit converts the
driver to use phy-supply devicetree property and the corresponding API, it cleans
the code a bit and make it simpler to maintain.

Signed-off-by: Romain Perier <romain.perier@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 61 ++++++++------------------
 1 file changed, 19 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 06e1637..8a8091c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -32,7 +32,7 @@
 struct rk_priv_data {
 	struct platform_device *pdev;
 	int phy_iface;
-	char regulator[32];
+	struct regulator *regulator;
 
 	bool clk_enabled;
 	bool clock_input;
@@ -287,46 +287,25 @@ static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)
 
 static int phy_power_on(struct rk_priv_data *bsp_priv, bool enable)
 {
-	struct regulator *ldo;
-	char *ldostr = bsp_priv->regulator;
+	struct regulator *ldo = bsp_priv->regulator;
 	int ret;
 	struct device *dev = &bsp_priv->pdev->dev;
 
-	if (!ldostr) {
-		dev_err(dev, "%s: no ldo found\n", __func__);
+	if (!ldo) {
+		dev_err(dev, "%s: no regulator found\n", __func__);
 		return -1;
 	}
 
-	ldo = regulator_get(NULL, ldostr);
-	if (!ldo) {
-		dev_err(dev, "\n%s get ldo %s failed\n", __func__, ldostr);
+	if (enable) {
+		ret = regulator_enable(ldo);
+		if (ret)
+			dev_err(dev, "%s: fail to enable phy-supply\n",
+				__func__);
 	} else {
-		if (enable) {
-			if (!regulator_is_enabled(ldo)) {
-				ret = regulator_enable(ldo);
-				if (ret != 0)
-					dev_err(dev, "%s: fail to enable %s\n",
-						__func__, ldostr);
-				else
-					dev_info(dev, "turn on ldo done.\n");
-			} else {
-				dev_warn(dev, "%s is enabled before enable",
-					 ldostr);
-			}
-		} else {
-			if (regulator_is_enabled(ldo)) {
-				ret = regulator_disable(ldo);
-				if (ret != 0)
-					dev_err(dev, "%s: fail to disable %s\n",
-						__func__, ldostr);
-				else
-					dev_info(dev, "turn off ldo done.\n");
-			} else {
-				dev_warn(dev, "%s is disabled before disable",
-					 ldostr);
-			}
-		}
-		regulator_put(ldo);
+		ret = regulator_disable(ldo);
+		if (ret)
+			dev_err(dev, "%s: fail to disable phy-supply\n",
+				__func__);
 	}
 
 	return 0;
@@ -346,14 +325,12 @@ static void *rk_gmac_setup(struct platform_device *pdev)
 
 	bsp_priv->phy_iface = of_get_phy_mode(dev->of_node);
 
-	ret = of_property_read_string(dev->of_node, "phy_regulator", &strings);
-	if (ret) {
-		dev_warn(dev, "%s: Can not read property: phy_regulator.\n",
-			 __func__);
-	} else {
-		dev_info(dev, "%s: PHY power controlled by regulator(%s).\n",
-			 __func__, strings);
-		strcpy(bsp_priv->regulator, strings);
+	bsp_priv->regulator = devm_regulator_get_optional(dev, "phy");
+	if (IS_ERR(bsp_priv->regulator)) {
+		if (PTR_ERR(bsp_priv->regulator) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		dev_err(dev, "no regulator found\n");
+		bsp_priv->regulator = NULL;
 	}
 
 	ret = of_property_read_string(dev->of_node, "clock_in_out", &strings);
-- 
1.9.1


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

* [PATCH v1 4/4] ARM: dts: Convert gmac to phy-supply in rk3288-evb-rk808.dts
  2015-01-19 18:08 [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues Romain Perier
                   ` (2 preceding siblings ...)
  2015-01-19 18:08 ` [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator Romain Perier
@ 2015-01-19 18:08 ` Romain Perier
  2015-01-19 20:19 ` [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues Heiko Stübner
  4 siblings, 0 replies; 9+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
  To: peppe.cavallaro
  Cc: netdev, linux-kernel, heiko, linux-rockchip, linux-arm-kernel,
	devicetree, roger.chen

This replaces the property phy_regulator by the standard devicetree property
phy-supply and references the device_node vcc_phy from it.

Signed-off-by: Romain Perier <romain.perier@gmail.com>
---
 arch/arm/boot/dts/rk3288-evb-rk808.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/rk3288-evb-rk808.dts b/arch/arm/boot/dts/rk3288-evb-rk808.dts
index 831a7aa..e1d3eeb 100644
--- a/arch/arm/boot/dts/rk3288-evb-rk808.dts
+++ b/arch/arm/boot/dts/rk3288-evb-rk808.dts
@@ -161,7 +161,7 @@
 };
 
 &gmac {
-	phy_regulator = "vcc_phy";
+	phy-supply = <&vcc_phy>;
 	phy-mode = "rgmii";
 	clock_in_out = "input";
 	snps,reset-gpio = <&gpio4 7 0>;
-- 
1.9.1


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

* Re: [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator
  2015-01-19 18:08 ` [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator Romain Perier
@ 2015-01-19 20:12   ` Heiko Stübner
  2015-01-20  6:55     ` Romain Perier
  0 siblings, 1 reply; 9+ messages in thread
From: Heiko Stübner @ 2015-01-19 20:12 UTC (permalink / raw)
  To: Romain Perier
  Cc: peppe.cavallaro, netdev, linux-kernel, linux-rockchip,
	linux-arm-kernel, devicetree, roger.chen

Am Montag, 19. Januar 2015, 18:08:08 schrieb Romain Perier:
> Currently, dwmac-rk uses a custom propety "phy_regulator" to get the name of
> the right regulator to use to power on or power off the phy. This commit
> converts the driver to use phy-supply devicetree property and the
> corresponding API, it cleans the code a bit and make it simpler to
> maintain.
> 
> Signed-off-by: Romain Perier <romain.perier@gmail.com>

I don't see updated binding documentation in here.

Secondly I think patch4 which changes the property in the evb-rk808 could be 
in here. The change is simple enough and would keep it bisectable without 
needing transistional patches.

And there is the one warning down below.


> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 61
> ++++++++------------------ 1 file changed, 19 insertions(+), 42
> deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index 06e1637..8a8091c
> 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -32,7 +32,7 @@
>  struct rk_priv_data {
>  	struct platform_device *pdev;
>  	int phy_iface;
> -	char regulator[32];
> +	struct regulator *regulator;
> 
>  	bool clk_enabled;
>  	bool clock_input;
> @@ -287,46 +287,25 @@ static int gmac_clk_enable(struct rk_priv_data
> *bsp_priv, bool enable)
> 
>  static int phy_power_on(struct rk_priv_data *bsp_priv, bool enable)
>  {
> -	struct regulator *ldo;
> -	char *ldostr = bsp_priv->regulator;
> +	struct regulator *ldo = bsp_priv->regulator;
>  	int ret;
>  	struct device *dev = &bsp_priv->pdev->dev;
> 
> -	if (!ldostr) {
> -		dev_err(dev, "%s: no ldo found\n", __func__);
> +	if (!ldo) {
> +		dev_err(dev, "%s: no regulator found\n", __func__);
>  		return -1;
>  	}
> 
> -	ldo = regulator_get(NULL, ldostr);
> -	if (!ldo) {
> -		dev_err(dev, "\n%s get ldo %s failed\n", __func__, ldostr);
> +	if (enable) {
> +		ret = regulator_enable(ldo);
> +		if (ret)
> +			dev_err(dev, "%s: fail to enable phy-supply\n",
> +				__func__);
>  	} else {
> -		if (enable) {
> -			if (!regulator_is_enabled(ldo)) {
> -				ret = regulator_enable(ldo);
> -				if (ret != 0)
> -					dev_err(dev, "%s: fail to enable %s\n",
> -						__func__, ldostr);
> -				else
> -					dev_info(dev, "turn on ldo done.\n");
> -			} else {
> -				dev_warn(dev, "%s is enabled before enable",
> -					 ldostr);
> -			}
> -		} else {
> -			if (regulator_is_enabled(ldo)) {
> -				ret = regulator_disable(ldo);
> -				if (ret != 0)
> -					dev_err(dev, "%s: fail to disable %s\n",
> -						__func__, ldostr);
> -				else
> -					dev_info(dev, "turn off ldo done.\n");
> -			} else {
> -				dev_warn(dev, "%s is disabled before disable",
> -					 ldostr);
> -			}
> -		}
> -		regulator_put(ldo);
> +		ret = regulator_disable(ldo);
> +		if (ret)
> +			dev_err(dev, "%s: fail to disable phy-supply\n",
> +				__func__);
>  	}
> 
>  	return 0;
> @@ -346,14 +325,12 @@ static void *rk_gmac_setup(struct platform_device
> *pdev)
> 
>  	bsp_priv->phy_iface = of_get_phy_mode(dev->of_node);
> 
> -	ret = of_property_read_string(dev->of_node, "phy_regulator", &strings);
> -	if (ret) {
> -		dev_warn(dev, "%s: Can not read property: phy_regulator.\n",
> -			 __func__);
> -	} else {
> -		dev_info(dev, "%s: PHY power controlled by regulator(%s).\n",
> -			 __func__, strings);
> -		strcpy(bsp_priv->regulator, strings);
> +	bsp_priv->regulator = devm_regulator_get_optional(dev, "phy");
> +	if (IS_ERR(bsp_priv->regulator)) {
> +		if (PTR_ERR(bsp_priv->regulator) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;

this should be return ERR_PTR(-EPROBE_DEFER) and produces a warning in its 
current state


> +		dev_err(dev, "no regulator found\n");
> +		bsp_priv->regulator = NULL;
>  	}
> 
>  	ret = of_property_read_string(dev->of_node, "clock_in_out", &strings);


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

* Re: [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
  2015-01-19 18:08 [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues Romain Perier
                   ` (3 preceding siblings ...)
  2015-01-19 18:08 ` [PATCH v1 4/4] ARM: dts: Convert gmac to phy-supply in rk3288-evb-rk808.dts Romain Perier
@ 2015-01-19 20:19 ` Heiko Stübner
  2015-01-20  6:58   ` Romain Perier
  4 siblings, 1 reply; 9+ messages in thread
From: Heiko Stübner @ 2015-01-19 20:19 UTC (permalink / raw)
  To: Romain Perier, David Miller
  Cc: peppe.cavallaro, netdev, linux-kernel, linux-rockchip,
	linux-arm-kernel, devicetree, roger.chen

Hi Romain

Am Montag, 19. Januar 2015, 18:08:05 schrieb Romain Perier:
> This series fixes few issues in dwmac-rk:
> 
> 1. Voltage settings was hardcoded into the driver for the phy regulator.
>    The driver now uses the default voltage settings found in the devicetree,
> which are applied throught the regulator framework.
> 2. The regulator name used to power on or power off the phy was put in the
> devicetree variable "phy_regulator", which is not standard and added a lot
> of code for nothing. The driver now uses the devicetree property
> "phy-supply" and the corresponding functions to manipulate this regulator.
> 
> The corresponding devicetree files are also updated. As, dwmac-rk was
> recently pushed in the development tree, I don't care about devicetree
> backward compatibility issues.

This last sentence is slightly misleading :-) .

The actual fact is, that these new bindings for the rk3288 gmac have not been 
released with any official kernel release yet ... i.e. the will be released with 
3.20 in whatever form, so we don't _need_ to care about keeping compatibility 
still for the next 2.5 weeks or so.

@Dave: it would be good if this series (when fixed) could still go into the 
3.20 material so we don't get stuck with the non-standard regulator property.


As we'll probably need a v2 due to at the issue in patch3, could you also 
switch places of patch1 and 2, which would keep bisecatbility (i.e. regulator 
property before removing the voltage setting from the driver).


Otherwise this series:
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Heiko Stuebner <heiko@sntech.de>


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

* Re: [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator
  2015-01-19 20:12   ` Heiko Stübner
@ 2015-01-20  6:55     ` Romain Perier
  0 siblings, 0 replies; 9+ messages in thread
From: Romain Perier @ 2015-01-20  6:55 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: peppe.cavallaro, netdev, Linux Kernel Mailing List,
	open list:ARM/Rockchip SoC...,
	linux-arm-kernel, devicetree, roger.chen

Hi,

Ok I will do these changes. However, the property "phy_regulator" was
not documented in
Documentation/devicetree/bindings/net/rockchip-dwmac.txt ... I can
document it anyway, it has probably been forgotten.

Thanks !

2015-01-19 21:12 GMT+01:00 Heiko Stübner <heiko@sntech.de>:
> Am Montag, 19. Januar 2015, 18:08:08 schrieb Romain Perier:
>> Currently, dwmac-rk uses a custom propety "phy_regulator" to get the name of
>> the right regulator to use to power on or power off the phy. This commit
>> converts the driver to use phy-supply devicetree property and the
>> corresponding API, it cleans the code a bit and make it simpler to
>> maintain.
>>
>> Signed-off-by: Romain Perier <romain.perier@gmail.com>
>
> I don't see updated binding documentation in here.
>
> Secondly I think patch4 which changes the property in the evb-rk808 could be
> in here. The change is simple enough and would keep it bisectable without
> needing transistional patches.
>
> And there is the one warning down below.
>
>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 61
>> ++++++++------------------ 1 file changed, 19 insertions(+), 42
>> deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index 06e1637..8a8091c
>> 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> @@ -32,7 +32,7 @@
>>  struct rk_priv_data {
>>       struct platform_device *pdev;
>>       int phy_iface;
>> -     char regulator[32];
>> +     struct regulator *regulator;
>>
>>       bool clk_enabled;
>>       bool clock_input;
>> @@ -287,46 +287,25 @@ static int gmac_clk_enable(struct rk_priv_data
>> *bsp_priv, bool enable)
>>
>>  static int phy_power_on(struct rk_priv_data *bsp_priv, bool enable)
>>  {
>> -     struct regulator *ldo;
>> -     char *ldostr = bsp_priv->regulator;
>> +     struct regulator *ldo = bsp_priv->regulator;
>>       int ret;
>>       struct device *dev = &bsp_priv->pdev->dev;
>>
>> -     if (!ldostr) {
>> -             dev_err(dev, "%s: no ldo found\n", __func__);
>> +     if (!ldo) {
>> +             dev_err(dev, "%s: no regulator found\n", __func__);
>>               return -1;
>>       }
>>
>> -     ldo = regulator_get(NULL, ldostr);
>> -     if (!ldo) {
>> -             dev_err(dev, "\n%s get ldo %s failed\n", __func__, ldostr);
>> +     if (enable) {
>> +             ret = regulator_enable(ldo);
>> +             if (ret)
>> +                     dev_err(dev, "%s: fail to enable phy-supply\n",
>> +                             __func__);
>>       } else {
>> -             if (enable) {
>> -                     if (!regulator_is_enabled(ldo)) {
>> -                             ret = regulator_enable(ldo);
>> -                             if (ret != 0)
>> -                                     dev_err(dev, "%s: fail to enable %s\n",
>> -                                             __func__, ldostr);
>> -                             else
>> -                                     dev_info(dev, "turn on ldo done.\n");
>> -                     } else {
>> -                             dev_warn(dev, "%s is enabled before enable",
>> -                                      ldostr);
>> -                     }
>> -             } else {
>> -                     if (regulator_is_enabled(ldo)) {
>> -                             ret = regulator_disable(ldo);
>> -                             if (ret != 0)
>> -                                     dev_err(dev, "%s: fail to disable %s\n",
>> -                                             __func__, ldostr);
>> -                             else
>> -                                     dev_info(dev, "turn off ldo done.\n");
>> -                     } else {
>> -                             dev_warn(dev, "%s is disabled before disable",
>> -                                      ldostr);
>> -                     }
>> -             }
>> -             regulator_put(ldo);
>> +             ret = regulator_disable(ldo);
>> +             if (ret)
>> +                     dev_err(dev, "%s: fail to disable phy-supply\n",
>> +                             __func__);
>>       }
>>
>>       return 0;
>> @@ -346,14 +325,12 @@ static void *rk_gmac_setup(struct platform_device
>> *pdev)
>>
>>       bsp_priv->phy_iface = of_get_phy_mode(dev->of_node);
>>
>> -     ret = of_property_read_string(dev->of_node, "phy_regulator", &strings);
>> -     if (ret) {
>> -             dev_warn(dev, "%s: Can not read property: phy_regulator.\n",
>> -                      __func__);
>> -     } else {
>> -             dev_info(dev, "%s: PHY power controlled by regulator(%s).\n",
>> -                      __func__, strings);
>> -             strcpy(bsp_priv->regulator, strings);
>> +     bsp_priv->regulator = devm_regulator_get_optional(dev, "phy");
>> +     if (IS_ERR(bsp_priv->regulator)) {
>> +             if (PTR_ERR(bsp_priv->regulator) == -EPROBE_DEFER)
>> +                     return -EPROBE_DEFER;
>
> this should be return ERR_PTR(-EPROBE_DEFER) and produces a warning in its
> current state
>
>
>> +             dev_err(dev, "no regulator found\n");
>> +             bsp_priv->regulator = NULL;
>>       }
>>
>>       ret = of_property_read_string(dev->of_node, "clock_in_out", &strings);
>

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

* Re: [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
  2015-01-19 20:19 ` [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues Heiko Stübner
@ 2015-01-20  6:58   ` Romain Perier
  0 siblings, 0 replies; 9+ messages in thread
From: Romain Perier @ 2015-01-20  6:58 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: David Miller, peppe.cavallaro, netdev, Linux Kernel Mailing List,
	open list:ARM/Rockchip SoC...,
	linux-arm-kernel, devicetree, roger.chen

Hi all,

2015-01-19 21:19 GMT+01:00 Heiko Stübner <heiko@sntech.de>:
> Hi Romain
>
> Am Montag, 19. Januar 2015, 18:08:05 schrieb Romain Perier:
>> This series fixes few issues in dwmac-rk:
>>
>> 1. Voltage settings was hardcoded into the driver for the phy regulator.
>>    The driver now uses the default voltage settings found in the devicetree,
>> which are applied throught the regulator framework.
>> 2. The regulator name used to power on or power off the phy was put in the
>> devicetree variable "phy_regulator", which is not standard and added a lot
>> of code for nothing. The driver now uses the devicetree property
>> "phy-supply" and the corresponding functions to manipulate this regulator.
>>
>> The corresponding devicetree files are also updated. As, dwmac-rk was
>> recently pushed in the development tree, I don't care about devicetree
>> backward compatibility issues.
>
> This last sentence is slightly misleading :-) .
>

Yes, I meant that I don't need to care about it, as you explain it well below.
Sorry for my misleading sentence ;) . I will fix it in my second serie.

> The actual fact is, that these new bindings for the rk3288 gmac have not been
> released with any official kernel release yet ... i.e. the will be released with
> 3.20 in whatever form, so we don't _need_ to care about keeping compatibility
> still for the next 2.5 weeks or so.
>
> @Dave: it would be good if this series (when fixed) could still go into the
> 3.20 material so we don't get stuck with the non-standard regulator property.
>
>
> As we'll probably need a v2 due to at the issue in patch3, could you also
> switch places of patch1 and 2, which would keep bisecatbility (i.e. regulator
> property before removing the voltage setting from the driver).

This sentence about re-ordering patches is for Dave or for me ?

Thanks,
Romain

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

end of thread, other threads:[~2015-01-20  6:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-19 18:08 [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues Romain Perier
2015-01-19 18:08 ` [PATCH v1 1/4] net: stmmac: dwmac-rk: Don't set the regulator voltage for phy from the driver Romain Perier
2015-01-19 18:08 ` [PATCH v1 2/4] ARM: dts: Add regulator voltage settings for vcc_phy in rk3288-evb.dtsi Romain Perier
2015-01-19 18:08 ` [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator Romain Perier
2015-01-19 20:12   ` Heiko Stübner
2015-01-20  6:55     ` Romain Perier
2015-01-19 18:08 ` [PATCH v1 4/4] ARM: dts: Convert gmac to phy-supply in rk3288-evb-rk808.dts Romain Perier
2015-01-19 20:19 ` [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues Heiko Stübner
2015-01-20  6:58   ` Romain Perier

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).