| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 26 Jun 2014 18:06:21 +0300
From: Tero Kristo <t-kristo@...com>
To: Roger Quadros <rogerq@...com>, Tony Lindgren <tony@...mide.com>,
Nishanth Menon <nm@...com>
CC: <rnayak@...com>, <balbi@...com>, <nsekhar@...com>, <kishon@...com>,
<george.cherian@...com>, <linux-omap@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ARM: DRA7-evm: Enable SATA PHY and USB PHY power supplies
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@...com> [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@...com>
>>>>> ---
>>>>> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists