* [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
@ 2015-01-19 18:08 ` Romain Perier
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
@ 2015-01-19 18:08 ` Romain Perier
0 siblings, 0 replies; 23+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
To: peppe.cavallaro-qxv4g6HH51o
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
heiko-4mtYJXux2i+zQB+pC5nmwQ,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
roger.chen-TNX95d0MmH7DzftRWevZcw
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
@ 2015-01-19 18:08 ` Romain Perier
0 siblings, 0 replies; 23+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
To: linux-arm-kernel
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] 23+ 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 ` Romain Perier
@ 2015-01-19 18:08 ` Romain Perier
-1 siblings, 0 replies; 23+ 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 related [flat|nested] 23+ 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 ` Romain Perier
0 siblings, 0 replies; 23+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
To: linux-arm-kernel
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 related [flat|nested] 23+ 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 ` Romain Perier
(?)
@ 2015-01-19 18:08 ` Romain Perier
-1 siblings, 0 replies; 23+ 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 = <ð_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 related [flat|nested] 23+ 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 ` Romain Perier
0 siblings, 0 replies; 23+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
To: peppe.cavallaro
Cc: devicetree, heiko, netdev, linux-kernel, linux-rockchip,
roger.chen, linux-arm-kernel
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 = <ð_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 related [flat|nested] 23+ 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 ` Romain Perier
0 siblings, 0 replies; 23+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
To: linux-arm-kernel
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 = <ð_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 related [flat|nested] 23+ messages in thread
* [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator
2015-01-19 18:08 ` Romain Perier
@ 2015-01-19 18:08 ` Romain Perier
-1 siblings, 0 replies; 23+ 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 related [flat|nested] 23+ messages in thread
* [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator
@ 2015-01-19 18:08 ` Romain Perier
0 siblings, 0 replies; 23+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
To: linux-arm-kernel
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 related [flat|nested] 23+ messages in thread
* [PATCH v1 4/4] ARM: dts: Convert gmac to phy-supply in rk3288-evb-rk808.dts
2015-01-19 18:08 ` Romain Perier
@ 2015-01-19 18:08 ` Romain Perier
-1 siblings, 0 replies; 23+ 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 related [flat|nested] 23+ messages in thread
* [PATCH v1 4/4] ARM: dts: Convert gmac to phy-supply in rk3288-evb-rk808.dts
@ 2015-01-19 18:08 ` Romain Perier
0 siblings, 0 replies; 23+ messages in thread
From: Romain Perier @ 2015-01-19 18:08 UTC (permalink / raw)
To: linux-arm-kernel
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 related [flat|nested] 23+ 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
0 siblings, 0 replies; 23+ 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] 23+ 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
0 siblings, 0 replies; 23+ messages in thread
From: Heiko Stübner @ 2015-01-19 20:12 UTC (permalink / raw)
To: Romain Perier
Cc: peppe.cavallaro-qxv4g6HH51o, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
roger.chen-TNX95d0MmH7DzftRWevZcw
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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
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);
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator
@ 2015-01-19 20:12 ` Heiko Stübner
0 siblings, 0 replies; 23+ messages in thread
From: Heiko Stübner @ 2015-01-19 20:12 UTC (permalink / raw)
To: linux-arm-kernel
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] 23+ messages in thread
* Re: [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
2015-01-19 18:08 ` Romain Perier
@ 2015-01-19 20:19 ` Heiko Stübner
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
@ 2015-01-19 20:19 ` Heiko Stübner
0 siblings, 0 replies; 23+ messages in thread
From: Heiko Stübner @ 2015-01-19 20:19 UTC (permalink / raw)
To: linux-arm-kernel
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] 23+ 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
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator
@ 2015-01-20 6:55 ` Romain Perier
0 siblings, 0 replies; 23+ messages in thread
From: Romain Perier @ 2015-01-20 6:55 UTC (permalink / raw)
To: Heiko Stübner
Cc: peppe.cavallaro-qxv4g6HH51o, netdev, Linux Kernel Mailing List,
open list:ARM/Rockchip SoC...,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
roger.chen-TNX95d0MmH7DzftRWevZcw
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-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>:
> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> 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);
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v1 3/4] net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator
@ 2015-01-20 6:55 ` Romain Perier
0 siblings, 0 replies; 23+ messages in thread
From: Romain Perier @ 2015-01-20 6:55 UTC (permalink / raw)
To: linux-arm-kernel
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] 23+ messages in thread
* Re: [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
2015-01-19 20:19 ` Heiko Stübner
(?)
@ 2015-01-20 6:58 ` Romain Perier
-1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
@ 2015-01-20 6:58 ` Romain Perier
0 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
@ 2015-01-20 6:58 ` Romain Perier
0 siblings, 0 replies; 23+ messages in thread
From: Romain Perier @ 2015-01-20 6:58 UTC (permalink / raw)
To: linux-arm-kernel
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] 23+ messages in thread
end of thread, other threads:[~2015-01-20 6:58 UTC | newest]
Thread overview: 23+ 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 ` Romain Perier
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
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
2015-01-19 18:08 ` 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
2015-01-19 18:08 ` Romain Perier
2015-01-19 20:12 ` Heiko Stübner
2015-01-19 20:12 ` Heiko Stübner
2015-01-19 20:12 ` Heiko Stübner
2015-01-20 6:55 ` Romain Perier
2015-01-20 6:55 ` Romain Perier
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 18:08 ` Romain Perier
2015-01-19 20:19 ` [PATCH v1 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues Heiko Stübner
2015-01-19 20:19 ` Heiko Stübner
2015-01-20 6:58 ` Romain Perier
2015-01-20 6:58 ` Romain Perier
2015-01-20 6:58 ` Romain Perier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.