* [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: 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 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 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: 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: 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 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
* 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
* [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 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
* 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
* [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 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 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 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 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
* [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
* 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
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.