Re: [PATCH] ARM: DRA7-evm: Enable SATA PHY and USB PHY power supplies

From: Tero Kristo
Date: Thu Jun 26 2014 - 11:06:00 EST


On 06/26/2014 05:22 PM, Roger Quadros wrote:
+Tero

On 06/26/2014 12:36 PM, Roger Quadros wrote:
On 06/26/2014 10:31 AM, Tony Lindgren wrote:
* Nishanth Menon <nm@xxxxxx> [140625 15:29]:
On 06/25/2014 07:56 AM, Roger Quadros wrote:
The SATA and USB PHYs need the 1.8V and 3.3V supplies.
The PHY drivers/framework don't yet support regulator
supply so we have to keep these regulators always-on till
then.

Signed-off-by: Roger Quadros <rogerq@xxxxxx>
---
arch/arm/boot/dts/dra7-evm.dts | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 4adc280..99a1f79 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -241,6 +241,7 @@
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-boot-on;
+ regulator-always-on;
};

ldo9_reg: ldo9 {
@@ -266,6 +267,7 @@
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
regulator-boot-on;
+ regulator-always-on;
};
};
};


Why not fix phy driver/framework as needed? the trouble is people
always forget to remove always-on... who actually audits old logs and
fixes stuff back up?

Yes I agree let's not play with the regulator-always-on unless
absolutely necessary.

Agreed. PHY framework must deal with this. Till then USB, SATA and most likely PCIe as well will not work on DRA7-evm.


I tried the below patch to enable the relevant regulators in the PHY drivers but still couldn't manage to get USB to work. The same 1.8V regulator is used to supply 3 pins
-vdda_usb1: DPLL_USB and HS_USB1 analog supply
-vdda_usb2: HS_USB2 analog supply
-vdda_usb3: DPLL_USB_OTG_SS and USB3.0 Rx/Tx analog supply

It seems that the regulator auto disable kicks in before the phy driver enables the regulator thus causing some kind of malfunction. I'm not sure whether this is due to DPLL_USB supply glitch or HS_USB1 analog supply glitch.

In any case, the DPLL_USB (clock driver?) needs to enable the 1.8V regulator in addition to the HS_USB PHY driver to prevent this supply glitch.

Tero, any suggestions about this? If we are not concerned about disabling DPLL_USB anytime then the regulator might just as well be always-on. Alternatively can we place a regulator_get(), regulator_enable() in drivers/clk/ti/dpll.c?

I believe dpll_usb needs to go down for the core to idle. Also, we are working on getting suspend-resume working on this platform, so having the regulator always on is a no-no.

Also, I am heavily against adding regulator tweaks within the clock driver, there needs to be another way.

-Tero


cheers,
-roger
---

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 4adc280..23379ab 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -495,3 +495,11 @@
};
};
};
+
+&usb2_phy1 {
+ vdda3v3-supply = <&ldousb_reg>;
+};
+
+&usb3_phy1 {
+ vdda1v8-supply = <&ldo3_reg>;
+};
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 7007c11..bb1a768 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -30,6 +30,7 @@
#include <linux/phy/omap_control_phy.h>
#include <linux/phy/phy.h>
#include <linux/of_platform.h>
+#include <linux/regulator/consumer.h>

#define USB2PHY_DISCON_BYP_LATCH (1 << 31)
#define USB2PHY_ANA_CONFIG1 0x4c
@@ -107,6 +108,18 @@ static int omap_usb_power_off(struct phy *x)

omap_control_phy_power(phy->control_dev, 0);

+ if (phy->pwr) {
+ int ret;
+
+ ret = regulator_disable(phy->pwr);
+ if (ret) {
+ dev_err(phy->dev, "failed to disable regulator\n");
+ return ret;
+ } else {
+ dev_info(phy->dev, "disabled regulator\n");
+ }
+ }
+
return 0;
}

@@ -114,6 +127,18 @@ static int omap_usb_power_on(struct phy *x)
{
struct omap_usb *phy = phy_get_drvdata(x);

+ if (phy->pwr) {
+ int ret;
+
+ ret = regulator_enable(phy->pwr);
+ if (ret) {
+ dev_err(phy->dev, "failed to enable regulator\n");
+ return ret;
+ } else {
+ dev_info(phy->dev, "enabled regulator\n");
+ }
+ }
+
omap_control_phy_power(phy->control_dev, 1);

return 0;
@@ -253,6 +278,14 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy->control_dev = &control_pdev->dev;
omap_control_phy_power(phy->control_dev, 0);

+ /* phy-supply */
+ phy->pwr = devm_regulator_get_optional(phy->dev, "vdda3v3");
+ if (IS_ERR(phy->pwr)) {
+ if (PTR_ERR(phy->pwr) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ phy->pwr = NULL;
+ }
+
otg->set_host = omap_usb_set_host;
otg->set_peripheral = omap_usb_set_peripheral;
if (phy_data->flags & OMAP_USB2_HAS_SET_VBUS)
diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c
index 5913676..1b46b0b 100644
--- a/drivers/phy/phy-ti-pipe3.c
+++ b/drivers/phy/phy-ti-pipe3.c
@@ -28,6 +28,7 @@
#include <linux/delay.h>
#include <linux/phy/omap_control_phy.h>
#include <linux/of_platform.h>
+#include <linux/regulator/consumer.h>

#define PLL_STATUS 0x00000004
#define PLL_GO 0x00000008
@@ -81,6 +82,7 @@ struct ti_pipe3 {
struct clk *sys_clk;
struct clk *refclk;
struct pipe3_dpll_map *dpll_map;
+ struct regulator *pwr_dpll;
};

static struct pipe3_dpll_map dpll_map_usb[] = {
@@ -215,6 +217,17 @@ static int ti_pipe3_init(struct phy *x)
u32 val;
int ret = 0;

+ /* Enable DPLL regulator */
+ if (phy->pwr_dpll) {
+ ret = regulator_enable(phy->pwr_dpll);
+ if (ret) {
+ dev_err(phy->dev, "failed to enable regulator\n");
+ return ret;
+ } else {
+ dev_info(phy->dev, "enabled regulator\n");
+ }
+ }
+
/* Bring it out of IDLE if it is IDLE */
val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
if (val & PLL_IDLE) {
@@ -262,6 +275,18 @@ static int ti_pipe3_exit(struct phy *x)
return -EBUSY;
}

+ /* Disable DPLL regulator */
+ if (phy->pwr_dpll) {
+ int ret;
+
+ ret = regulator_disable(phy->pwr_dpll);
+ if (ret) {
+ dev_err(phy->dev, "failed to disable regulator\n");
+ return ret;
+ } else {
+ dev_info(phy->dev, "disabled regulator\n");
+ }
+ }
return 0;
}
static struct phy_ops ops = {
@@ -308,7 +333,15 @@ static int ti_pipe3_probe(struct platform_device *pdev)
if (IS_ERR(phy->pll_ctrl_base))
return PTR_ERR(phy->pll_ctrl_base);

- phy->dev = &pdev->dev;
+ phy->dev = &pdev->dev;
+
+ /* dpll-supply */
+ phy->pwr_dpll = devm_regulator_get_optional(phy->dev, "vdda1v8");
+ if (IS_ERR(phy->pwr_dpll)) {
+ if (PTR_ERR(phy->pwr_dpll) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ phy->pwr_dpll = NULL;
+ }

if (!of_device_is_compatible(node, "ti,phy-pipe3-sata")) {

diff --git a/include/linux/phy/omap_usb.h b/include/linux/phy/omap_usb.h
index dc2c541..e2c46df 100644
--- a/include/linux/phy/omap_usb.h
+++ b/include/linux/phy/omap_usb.h
@@ -40,6 +40,7 @@ struct omap_usb {
struct clk *wkupclk;
struct clk *optclk;
u8 flags;
+ struct regulator *pwr;
};

struct usb_phy_data {


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/