2013-06-18 16:05:21

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v2 0/4] ARM: OMAP2+: Panda USB Host support and DVI EDID fix

Hi Benoit & Tony,

The first two patches make changes to dts files to get USB host support
and DVI EDID to work on Panda.

The third patch should get USB host functional on uEVM.

The fourth patch is a temporary workaround to create a clock alias to
the USB PHY clock as it is not possible to define this via device tree.
This patch is required for USB host to work on Panda and uEVM.

The first 3 patches are for Benoit and the last one is for Tony.

Changes in v2:
- Fixed subject for patch 4
- Added "regulator-always-on" for Panda's USB Hub regulator

cheers,
-roger

Roger Quadros (4):
ARM: dts: omap4-panda: Add USB Host support
ARM: dts: omap4-panda: Fix DVI EDID reads
ARM: dts: omap5-uevm: Provide USB Host PHY clock frequency
ARM: OMAP2+: Provide alias to USB PHY clock

arch/arm/boot/dts/omap4-panda-common.dtsi | 69 +++++++++++++++++++++++++++++
arch/arm/boot/dts/omap5-uevm.dts | 7 +++
arch/arm/mach-omap2/board-generic.c | 23 +++++++++-
3 files changed, 98 insertions(+), 1 deletions(-)

--
1.7.4.1


2013-06-18 16:05:31

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v2 2/4] ARM: dts: omap4-panda: Fix DVI EDID reads

On Panda the +5V supply for DVI EDID is supplied by the
same regulator that poweres the USB Hub. Currently, the
DSS/DVI subsystem doesn't know how to manage this regulator
and so DVI EDID reads will fail if USB Hub is not enabled.

As a temporary fix we keep this regulator permanently enabled
on boot. This fixes the DVI EDID read problem.

CC: Tomi Valkeinen <[email protected]>
Signed-off-by: Roger Quadros <[email protected]>
---
arch/arm/boot/dts/omap4-panda-common.dtsi | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
index 7a21e8e..e6e6c39 100644
--- a/arch/arm/boot/dts/omap4-panda-common.dtsi
+++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
@@ -80,6 +80,13 @@
gpio = <&gpio1 1 0>; /* gpio_1 */
startup-delay-us = <70000>;
enable-active-high;
+ /*
+ * boot-on is required along with always-on as the
+ * regulator framework doesn't enable the regulator
+ * if boot-on is not there.
+ */
+ regulator-always-on;
+ regulator-boot-on;
};

/* HS USB Host PHY on PORT 1 */
--
1.7.4.1

2013-06-18 16:05:39

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v2 4/4] ARM: OMAP2+: Provide alias to USB PHY clock

Till the OMAP clocks are correctly defined in device tree, use
this temporary hack to provide clock alias to the USB PHY clocks.

Without this, USB Host & Ethernet will not be functional with
device tree boots on Panda and uEVM.

Signed-off-by: Roger Quadros <[email protected]>
---
arch/arm/mach-omap2/board-generic.c | 23 ++++++++++++++++++++++-
1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 88aa6b1..7428733 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -15,6 +15,7 @@
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/irqdomain.h>
+#include <linux/clk.h>

#include <asm/mach/arch.h>

@@ -35,6 +36,21 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
{ }
};

+/*
+ * Create alias for USB host PHY clock.
+ * Remove this when clock phandle can be provided via DT
+ */
+static void __init legacy_init_ehci_clk(char *clkname)
+{
+ int ret;
+
+ ret = clk_add_alias("main_clk", NULL, clkname, NULL);
+ if (ret) {
+ pr_err("%s:Failed to add main_clk alias to %s :%d\n",
+ __func__, clkname, ret);
+ }
+}
+
static void __init omap_generic_init(void)
{
omap_sdrc_init(NULL, NULL);
@@ -45,10 +61,15 @@ static void __init omap_generic_init(void)
* HACK: call display setup code for selected boards to enable omapdss.
* This will be removed when omapdss supports DT.
*/
- if (of_machine_is_compatible("ti,omap4-panda"))
+ if (of_machine_is_compatible("ti,omap4-panda")) {
omap4_panda_display_init_of();
+ legacy_init_ehci_clk("auxclk3_ck");
+
+ }
else if (of_machine_is_compatible("ti,omap4-sdp"))
omap_4430sdp_display_init_of();
+ else if (of_machine_is_compatible("ti,omap5-uevm"))
+ legacy_init_ehci_clk("auxclk1_ck");
}

#ifdef CONFIG_SOC_OMAP2420
--
1.7.4.1

2013-06-18 16:05:37

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v2 3/4] ARM: dts: omap5-uevm: Provide USB Host PHY clock frequency

USB Host PHY clock on port 2 must be configured to 19.2MHz.
Provide this information.

CC: Sricharan R <[email protected]>
Signed-off-by: Roger Quadros <[email protected]>
---
arch/arm/boot/dts/omap5-uevm.dts | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts
index 927db1e..c0ad472 100644
--- a/arch/arm/boot/dts/omap5-uevm.dts
+++ b/arch/arm/boot/dts/omap5-uevm.dts
@@ -40,6 +40,13 @@
hsusb2_phy: hsusb2_phy {
compatible = "usb-nop-xceiv";
reset-supply = <&hsusb2_reset>;
+ /**
+ * FIXME
+ * Put the right clock phandle here when available
+ * clocks = <&auxclk1>;
+ * clock-names = "main_clk";
+ */
+ clock-frequency = <19200000>;
};

/* HS USB Port 3 RESET */
--
1.7.4.1

2013-06-18 16:06:29

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

Provide the RESET and Power regulators for the USB PHY,
the USB Host port mode and the PHY device.

Also provide pin multiplexer information for the USB host
pins.

Signed-off-by: Roger Quadros <[email protected]>
---
arch/arm/boot/dts/omap4-panda-common.dtsi | 62 +++++++++++++++++++++++++++++
1 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
index 00cbaa5..7a21e8e 100644
--- a/arch/arm/boot/dts/omap4-panda-common.dtsi
+++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
@@ -59,6 +59,42 @@
"AFML", "Line In",
"AFMR", "Line In";
};
+
+ /* HS USB Port 1 RESET */
+ hsusb1_reset: hsusb1_reset_reg {
+ compatible = "regulator-fixed";
+ regulator-name = "hsusb1_reset";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio2 30 0>; /* gpio_62 */
+ startup-delay-us = <70000>;
+ enable-active-high;
+ };
+
+ /* HS USB Port 1 Power */
+ hsusb1_power: hsusb1_power_reg {
+ compatible = "regulator-fixed";
+ regulator-name = "hsusb1_vbus";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio1 1 0>; /* gpio_1 */
+ startup-delay-us = <70000>;
+ enable-active-high;
+ };
+
+ /* HS USB Host PHY on PORT 1 */
+ hsusb1_phy: hsusb1_phy {
+ compatible = "usb-nop-xceiv";
+ reset-supply = <&hsusb1_reset>;
+ vcc-supply = <&hsusb1_power>;
+ /**
+ * FIXME:
+ * put the right clock phandle here when available
+ * clocks = <&auxclk3>;
+ * clock-names = "main_clk";
+ */
+ clock-frequency = <19200000>;
+ };
};

&omap4_pmx_wkup {
@@ -83,6 +119,7 @@
&mcbsp1_pins
&dss_hdmi_pins
&tpd12s015_pins
+ &hsusbb1_pins
>;

twl6030_pins: pinmux_twl6030_pins {
@@ -133,6 +170,23 @@
>;
};

+ hsusbb1_pins: pinmux_hsusbb1_pins {
+ pinctrl-single,pins = <
+ 0x82 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_clk.usbb1_ulpiphy_clk */
+ 0x84 (PIN_OUTPUT | MUX_MODE4) /* usbb1_ulpitll_stp.usbb1_ulpiphy_stp */
+ 0x86 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dir.usbb1_ulpiphy_dir */
+ 0x88 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_nxt.usbb1_ulpiphy_nxt */
+ 0x8a (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat0.usbb1_ulpiphy_dat0 */
+ 0x8c (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat1.usbb1_ulpiphy_dat1 */
+ 0x8e (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat2.usbb1_ulpiphy_dat2 */
+ 0x90 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat3.usbb1_ulpiphy_dat3 */
+ 0x92 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat4.usbb1_ulpiphy_dat4 */
+ 0x94 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat5.usbb1_ulpiphy_dat5 */
+ 0x96 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat6.usbb1_ulpiphy_dat6 */
+ 0x98 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat7.usbb1_ulpiphy_dat7 */
+ >;
+ };
+
i2c1_pins: pinmux_i2c1_pins {
pinctrl-single,pins = <
0xe2 (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c1_scl */
@@ -283,3 +337,11 @@
mode = <3>;
power = <50>;
};
+
+&usbhshost {
+ port1-mode = "ehci-phy";
+};
+
+&usbhsehci {
+ phys = <&hsusb1_phy>;
+};
--
1.7.4.1

2013-06-19 01:23:22

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

Hi Roger,

On 06/18/2013 11:04 AM, Roger Quadros wrote:
> Provide the RESET and Power regulators for the USB PHY,
> the USB Host port mode and the PHY device.
>
> Also provide pin multiplexer information for the USB host
> pins.
>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
> arch/arm/boot/dts/omap4-panda-common.dtsi | 62 +++++++++++++++++++++++++++++
> 1 files changed, 62 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
> index 00cbaa5..7a21e8e 100644
> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
> @@ -59,6 +59,42 @@
> "AFML", "Line In",
> "AFMR", "Line In";
> };
> +
> + /* HS USB Port 1 RESET */
> + hsusb1_reset: hsusb1_reset_reg {
> + compatible = "regulator-fixed";
> + regulator-name = "hsusb1_reset";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpio = <&gpio2 30 0>; /* gpio_62 */
> + startup-delay-us = <70000>;
> + enable-active-high;
> + };

Is this really a regulator? Or just a GPIO line used to reset the USB PHY?

If this is the case, I don't think it should be represented as a regulator.

Regards,
Benoit

> +
> + /* HS USB Port 1 Power */
> + hsusb1_power: hsusb1_power_reg {
> + compatible = "regulator-fixed";
> + regulator-name = "hsusb1_vbus";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpio = <&gpio1 1 0>; /* gpio_1 */
> + startup-delay-us = <70000>;
> + enable-active-high;
> + };
> +
> + /* HS USB Host PHY on PORT 1 */
> + hsusb1_phy: hsusb1_phy {
> + compatible = "usb-nop-xceiv";
> + reset-supply = <&hsusb1_reset>;
> + vcc-supply = <&hsusb1_power>;
> + /**
> + * FIXME:
> + * put the right clock phandle here when available
> + * clocks = <&auxclk3>;
> + * clock-names = "main_clk";
> + */
> + clock-frequency = <19200000>;
> + };
> };
>
> &omap4_pmx_wkup {
> @@ -83,6 +119,7 @@
> &mcbsp1_pins
> &dss_hdmi_pins
> &tpd12s015_pins
> + &hsusbb1_pins
> >;
>
> twl6030_pins: pinmux_twl6030_pins {
> @@ -133,6 +170,23 @@
> >;
> };
>
> + hsusbb1_pins: pinmux_hsusbb1_pins {
> + pinctrl-single,pins = <
> + 0x82 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_clk.usbb1_ulpiphy_clk */
> + 0x84 (PIN_OUTPUT | MUX_MODE4) /* usbb1_ulpitll_stp.usbb1_ulpiphy_stp */
> + 0x86 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dir.usbb1_ulpiphy_dir */
> + 0x88 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_nxt.usbb1_ulpiphy_nxt */
> + 0x8a (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat0.usbb1_ulpiphy_dat0 */
> + 0x8c (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat1.usbb1_ulpiphy_dat1 */
> + 0x8e (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat2.usbb1_ulpiphy_dat2 */
> + 0x90 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat3.usbb1_ulpiphy_dat3 */
> + 0x92 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat4.usbb1_ulpiphy_dat4 */
> + 0x94 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat5.usbb1_ulpiphy_dat5 */
> + 0x96 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat6.usbb1_ulpiphy_dat6 */
> + 0x98 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* usbb1_ulpitll_dat7.usbb1_ulpiphy_dat7 */
> + >;
> + };
> +
> i2c1_pins: pinmux_i2c1_pins {
> pinctrl-single,pins = <
> 0xe2 (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c1_scl */
> @@ -283,3 +337,11 @@
> mode = <3>;
> power = <50>;
> };
> +
> +&usbhshost {
> + port1-mode = "ehci-phy";
> +};
> +
> +&usbhsehci {
> + phys = <&hsusb1_phy>;
> +};
>

2013-06-19 07:37:05

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

Hi Benoit,

On 06/19/2013 04:17 AM, Benoit Cousson wrote:
> Hi Roger,
>
> On 06/18/2013 11:04 AM, Roger Quadros wrote:
>> Provide the RESET and Power regulators for the USB PHY,
>> the USB Host port mode and the PHY device.
>>
>> Also provide pin multiplexer information for the USB host
>> pins.
>>
>> Signed-off-by: Roger Quadros <[email protected]>
>> ---
>> arch/arm/boot/dts/omap4-panda-common.dtsi | 62 +++++++++++++++++++++++++++++
>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
>> index 00cbaa5..7a21e8e 100644
>> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
>> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
>> @@ -59,6 +59,42 @@
>> "AFML", "Line In",
>> "AFMR", "Line In";
>> };
>> +
>> + /* HS USB Port 1 RESET */
>> + hsusb1_reset: hsusb1_reset_reg {
>> + compatible = "regulator-fixed";
>> + regulator-name = "hsusb1_reset";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + gpio = <&gpio2 30 0>; /* gpio_62 */
>> + startup-delay-us = <70000>;
>> + enable-active-high;
>> + };
>
> Is this really a regulator? Or just a GPIO line used to reset the USB PHY?

It is in fact a GPIO line used as reset.
>
> If this is the case, I don't think it should be represented as a regulator.

Why not? I think it fits very well in the regulator device model. I couldn't find a better
way to represent this. It gives us a way to specify not only the GPIO line but also
the polarity. I know mostly reset is active low but still there is flexibility as you never
know for sure.

Do you have any better ideas?

FYI. The USB PHY driver is already treating reset as a regulator and is into 3.10. Reworking that
will take some time. Not getting these in will prevent USB host/ethernet support on panda.

cheers,
-roger

2013-06-19 07:46:16

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

* Roger Quadros <[email protected]> [130619 00:42]:
> Hi Benoit,
>
> On 06/19/2013 04:17 AM, Benoit Cousson wrote:
> > Hi Roger,
> >
> > On 06/18/2013 11:04 AM, Roger Quadros wrote:
> >> Provide the RESET and Power regulators for the USB PHY,
> >> the USB Host port mode and the PHY device.
> >>
> >> Also provide pin multiplexer information for the USB host
> >> pins.
> >>
> >> Signed-off-by: Roger Quadros <[email protected]>
> >> ---
> >> arch/arm/boot/dts/omap4-panda-common.dtsi | 62 +++++++++++++++++++++++++++++
> >> 1 files changed, 62 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
> >> index 00cbaa5..7a21e8e 100644
> >> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
> >> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
> >> @@ -59,6 +59,42 @@
> >> "AFML", "Line In",
> >> "AFMR", "Line In";
> >> };
> >> +
> >> + /* HS USB Port 1 RESET */
> >> + hsusb1_reset: hsusb1_reset_reg {
> >> + compatible = "regulator-fixed";
> >> + regulator-name = "hsusb1_reset";
> >> + regulator-min-microvolt = <3300000>;
> >> + regulator-max-microvolt = <3300000>;
> >> + gpio = <&gpio2 30 0>; /* gpio_62 */
> >> + startup-delay-us = <70000>;
> >> + enable-active-high;
> >> + };
> >
> > Is this really a regulator? Or just a GPIO line used to reset the USB PHY?
>
> It is in fact a GPIO line used as reset.
> >
> > If this is the case, I don't think it should be represented as a regulator.
>
> Why not? I think it fits very well in the regulator device model. I couldn't find a better
> way to represent this. It gives us a way to specify not only the GPIO line but also
> the polarity. I know mostly reset is active low but still there is flexibility as you never
> know for sure.

I think it's really a mux + a comparator. But from Linux driver point of view
a regulator fits well as we don't have anything better. After all, the pin
voltage changes, and then something can be done based on the comparator
value.

> Do you have any better ideas?

We have a similar issue with the MMC1 PBIAS. I think in the long run we
should expand regulator (and possibly pinctrl) framework(s) to handle
comparators. We could just assume that a comparatator is a regulator,
and have a comparator binding that just uses the regulator code.

> FYI. The USB PHY driver is already treating reset as a regulator and is into 3.10. Reworking that
> will take some time. Not getting these in will prevent USB host/ethernet support on panda.

Yes and we need to have some solution for v3.11 as we've dropped the
legacy data for omap4. Otherwise things won't work properly.

Regards,

Tony

2013-06-19 10:11:49

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

On 06/19/2013 02:46 AM, Tony Lindgren wrote:
> * Roger Quadros <[email protected]> [130619 00:42]:
>> Hi Benoit,
>>
>> On 06/19/2013 04:17 AM, Benoit Cousson wrote:
>>> Hi Roger,
>>>
>>> On 06/18/2013 11:04 AM, Roger Quadros wrote:
>>>> Provide the RESET and Power regulators for the USB PHY,
>>>> the USB Host port mode and the PHY device.
>>>>
>>>> Also provide pin multiplexer information for the USB host
>>>> pins.
>>>>
>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>> ---
>>>> arch/arm/boot/dts/omap4-panda-common.dtsi | 62 +++++++++++++++++++++++++++++
>>>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>> index 00cbaa5..7a21e8e 100644
>>>> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>> @@ -59,6 +59,42 @@
>>>> "AFML", "Line In",
>>>> "AFMR", "Line In";
>>>> };
>>>> +
>>>> + /* HS USB Port 1 RESET */
>>>> + hsusb1_reset: hsusb1_reset_reg {
>>>> + compatible = "regulator-fixed";
>>>> + regulator-name = "hsusb1_reset";
>>>> + regulator-min-microvolt = <3300000>;
>>>> + regulator-max-microvolt = <3300000>;
>>>> + gpio = <&gpio2 30 0>; /* gpio_62 */
>>>> + startup-delay-us = <70000>;
>>>> + enable-active-high;
>>>> + };
>>>
>>> Is this really a regulator? Or just a GPIO line used to reset the USB PHY?
>>
>> It is in fact a GPIO line used as reset.
>>>
>>> If this is the case, I don't think it should be represented as a regulator.
>>
>> Why not? I think it fits very well in the regulator device model.

I'm not sure fitting very well is the correct term.
It works, for sure, but it is no different than when we were trying to
abuse the regulator fmwk to enable the 32k clock in phoenix.
It is just a hack.

>>I couldn't find a better
>> way to represent this. It gives us a way to specify not only the GPIO line but also
>> the polarity. I know mostly reset is active low but still there is flexibility as you never
>> know for sure.

Mmm, and what about just controlling the gpio from the driver?

> I think it's really a mux + a comparator. But from Linux driver point of view
> a regulator fits well as we don't have anything better. After all, the pin
> voltage changes, and then something can be done based on the comparator
> value.
>
>> Do you have any better ideas?
>
> We have a similar issue with the MMC1 PBIAS. I think in the long run we
> should expand regulator (and possibly pinctrl) framework(s) to handle
> comparators. We could just assume that a comparatator is a regulator,
> and have a comparator binding that just uses the regulator code.

In the case of pbias, the pinctrl seems to be a much better fit for my
point of view. pinctrl can handle pin configuration and this is what the
pbias is in the case of MMC pins.

>> FYI. The USB PHY driver is already treating reset as a regulator and is into 3.10. Reworking that
>> will take some time. Not getting these in will prevent USB host/ethernet support on panda.

That's not because we did some mistake in the past that we have to keep
doing that :-)

> Yes and we need to have some solution for v3.11 as we've dropped the
> legacy data for omap4. Otherwise things won't work properly.

I'm fine taking it as soon as big disclaimer is added to avoid mis-using
the regulator in the future for controlling a gpio line.

Regards,
Benoit

2013-06-19 11:04:16

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

On 06/19/2013 01:10 PM, Benoit Cousson wrote:
> On 06/19/2013 02:46 AM, Tony Lindgren wrote:
>> * Roger Quadros <[email protected]> [130619 00:42]:
>>> Hi Benoit,
>>>
>>> On 06/19/2013 04:17 AM, Benoit Cousson wrote:
>>>> Hi Roger,
>>>>
>>>> On 06/18/2013 11:04 AM, Roger Quadros wrote:
>>>>> Provide the RESET and Power regulators for the USB PHY,
>>>>> the USB Host port mode and the PHY device.
>>>>>
>>>>> Also provide pin multiplexer information for the USB host
>>>>> pins.
>>>>>
>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>> ---
>>>>> arch/arm/boot/dts/omap4-panda-common.dtsi | 62 +++++++++++++++++++++++++++++
>>>>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>> index 00cbaa5..7a21e8e 100644
>>>>> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>> @@ -59,6 +59,42 @@
>>>>> "AFML", "Line In",
>>>>> "AFMR", "Line In";
>>>>> };
>>>>> +
>>>>> + /* HS USB Port 1 RESET */
>>>>> + hsusb1_reset: hsusb1_reset_reg {
>>>>> + compatible = "regulator-fixed";
>>>>> + regulator-name = "hsusb1_reset";
>>>>> + regulator-min-microvolt = <3300000>;
>>>>> + regulator-max-microvolt = <3300000>;
>>>>> + gpio = <&gpio2 30 0>; /* gpio_62 */
>>>>> + startup-delay-us = <70000>;
>>>>> + enable-active-high;
>>>>> + };
>>>>
>>>> Is this really a regulator? Or just a GPIO line used to reset the USB PHY?
>>>
>>> It is in fact a GPIO line used as reset.
>>>>
>>>> If this is the case, I don't think it should be represented as a regulator.
>>>
>>> Why not? I think it fits very well in the regulator device model.
>
> I'm not sure fitting very well is the correct term.
> It works, for sure, but it is no different than when we were trying to abuse the regulator fmwk to enable the 32k clock in phoenix.
> It is just a hack.
>

The only difference is there is a dedicated framework for clocks. Since there is nothing specific to
handle reset lines it is left to the driver writer how he wants to manage it.

>>> I couldn't find a better
>>> way to represent this. It gives us a way to specify not only the GPIO line but also
>>> the polarity. I know mostly reset is active low but still there is flexibility as you never
>>> know for sure.
>
> Mmm, and what about just controlling the gpio from the driver?

Yes gpio is possible. But then you need to add additional code in the driver to parse GPIO number and polarity.
Using regulator_get() was plain simple for me.

>
>> I think it's really a mux + a comparator. But from Linux driver point of view
>> a regulator fits well as we don't have anything better. After all, the pin
>> voltage changes, and then something can be done based on the comparator
>> value.
>>
>>> Do you have any better ideas?
>>
>> We have a similar issue with the MMC1 PBIAS. I think in the long run we
>> should expand regulator (and possibly pinctrl) framework(s) to handle
>> comparators. We could just assume that a comparatator is a regulator,
>> and have a comparator binding that just uses the regulator code.
>
> In the case of pbias, the pinctrl seems to be a much better fit for my point of view. pinctrl can handle pin configuration and this is what the pbias is in the case of MMC pins.
>
>>> FYI. The USB PHY driver is already treating reset as a regulator and is into 3.10. Reworking that
>>> will take some time. Not getting these in will prevent USB host/ethernet support on panda.
>
> That's not because we did some mistake in the past that we have to keep doing that :-)
>

I had actually thought it well and don't consider it as a mistake. It is just a difference in opinion.

>> Yes and we need to have some solution for v3.11 as we've dropped the
>> legacy data for omap4. Otherwise things won't work properly.
>
> I'm fine taking it as soon as big disclaimer is added to avoid mis-using the regulator in the future for controlling a gpio line.
>

I can re-work the phy driver. No problem. But I'm still not convinced
what it will improve. IMHO it just adds more code in the phy driver without any benefits.

cheers,
-roger

2013-06-19 11:30:59

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

On 06/19/2013 06:03 AM, Roger Quadros wrote:
> On 06/19/2013 01:10 PM, Benoit Cousson wrote:
>> On 06/19/2013 02:46 AM, Tony Lindgren wrote:
>>> * Roger Quadros <[email protected]> [130619 00:42]:
>>>> Hi Benoit,
>>>>
>>>> On 06/19/2013 04:17 AM, Benoit Cousson wrote:
>>>>> Hi Roger,
>>>>>
>>>>> On 06/18/2013 11:04 AM, Roger Quadros wrote:
>>>>>> Provide the RESET and Power regulators for the USB PHY,
>>>>>> the USB Host port mode and the PHY device.
>>>>>>
>>>>>> Also provide pin multiplexer information for the USB host
>>>>>> pins.
>>>>>>
>>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>>> ---
>>>>>> arch/arm/boot/dts/omap4-panda-common.dtsi | 62 +++++++++++++++++++++++++++++
>>>>>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>> index 00cbaa5..7a21e8e 100644
>>>>>> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>> @@ -59,6 +59,42 @@
>>>>>> "AFML", "Line In",
>>>>>> "AFMR", "Line In";
>>>>>> };
>>>>>> +
>>>>>> + /* HS USB Port 1 RESET */
>>>>>> + hsusb1_reset: hsusb1_reset_reg {
>>>>>> + compatible = "regulator-fixed";
>>>>>> + regulator-name = "hsusb1_reset";
>>>>>> + regulator-min-microvolt = <3300000>;
>>>>>> + regulator-max-microvolt = <3300000>;
>>>>>> + gpio = <&gpio2 30 0>; /* gpio_62 */
>>>>>> + startup-delay-us = <70000>;
>>>>>> + enable-active-high;
>>>>>> + };
>>>>>
>>>>> Is this really a regulator? Or just a GPIO line used to reset the USB PHY?
>>>>
>>>> It is in fact a GPIO line used as reset.
>>>>>
>>>>> If this is the case, I don't think it should be represented as a regulator.
>>>>
>>>> Why not? I think it fits very well in the regulator device model.
>>
>> I'm not sure fitting very well is the correct term.
>> It works, for sure, but it is no different than when we were trying to abuse the regulator fmwk to enable the 32k clock in phoenix.
>> It is just a hack.
>>
>
> The only difference is there is a dedicated framework for clocks. Since there is nothing specific to
> handle reset lines it is left to the driver writer how he wants to manage it.

Well, at that time, it was not available either. The point is still that
using a existing fmwk that has nothing to do with the signal you need to
handle just because it works is not a very good justification.

>>>> I couldn't find a better
>>>> way to represent this. It gives us a way to specify not only the GPIO line but also
>>>> the polarity. I know mostly reset is active low but still there is flexibility as you never
>>>> know for sure.
>>
>> Mmm, and what about just controlling the gpio from the driver?
>
> Yes gpio is possible. But then you need to add additional code in the driver to parse GPIO number and polarity.
> Using regulator_get() was plain simple for me.

Maybe, but it is not the right thing to do.
Can you explain me the commonality between a reset line and a regulator???

>>> I think it's really a mux + a comparator. But from Linux driver point of view
>>> a regulator fits well as we don't have anything better. After all, the pin
>>> voltage changes, and then something can be done based on the comparator
>>> value.
>>>
>>>> Do you have any better ideas?
>>>
>>> We have a similar issue with the MMC1 PBIAS. I think in the long run we
>>> should expand regulator (and possibly pinctrl) framework(s) to handle
>>> comparators. We could just assume that a comparatator is a regulator,
>>> and have a comparator binding that just uses the regulator code.
>>
>> In the case of pbias, the pinctrl seems to be a much better fit for my point of view. pinctrl can handle pin configuration and this is what the pbias is in the case of MMC pins.
>>
>>>> FYI. The USB PHY driver is already treating reset as a regulator and is into 3.10. Reworking that
>>>> will take some time. Not getting these in will prevent USB host/ethernet support on panda.
>>
>> That's not because we did some mistake in the past that we have to keep doing that :-)
>
> I had actually thought it well and don't consider it as a mistake. It is just a difference in opinion.

Well, I don't think so. Again, you are abusing the regulator fmwk to
enable at boot time a GPIO line that should reset the IP. I doubt the
regulator fmwk was done for that. Otherwise Mark would have named it
"miscellaneous fmwk" or "regulator & reset" fmwk.

>>> Yes and we need to have some solution for v3.11 as we've dropped the
>>> legacy data for omap4. Otherwise things won't work properly.
>>
>> I'm fine taking it as soon as big disclaimer is added to avoid mis-using the regulator in the future for controlling a gpio line.
>>
>
> I can re-work the phy driver. No problem. But I'm still not convinced
> what it will improve. IMHO it just adds more code in the phy driver without any benefits.

Yes, it will. It will give explicitly the reset control to the driver
that knows and needs it. Moreover, it will avoid abusing a fmwk that was
not done for that purpose.

Regards,
Benoit

2013-06-19 12:05:07

by Florian Vaussard

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

Hello,

On 06/19/2013 01:03 PM, Roger Quadros wrote:
> On 06/19/2013 01:10 PM, Benoit Cousson wrote:
>> On 06/19/2013 02:46 AM, Tony Lindgren wrote:
>>> * Roger Quadros <[email protected]> [130619 00:42]:
>>>> Hi Benoit,
>>>>
>>>> On 06/19/2013 04:17 AM, Benoit Cousson wrote:
>>>>> Hi Roger,
>>>>>
>>>>> On 06/18/2013 11:04 AM, Roger Quadros wrote:
>>>>>> Provide the RESET and Power regulators for the USB PHY,
>>>>>> the USB Host port mode and the PHY device.
>>>>>>
>>>>>> Also provide pin multiplexer information for the USB host
>>>>>> pins.
>>>>>>
>>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>>> ---
>>>>>> arch/arm/boot/dts/omap4-panda-common.dtsi | 62 +++++++++++++++++++++++++++++
>>>>>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>> index 00cbaa5..7a21e8e 100644
>>>>>> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>> @@ -59,6 +59,42 @@
>>>>>> "AFML", "Line In",
>>>>>> "AFMR", "Line In";
>>>>>> };
>>>>>> +
>>>>>> + /* HS USB Port 1 RESET */
>>>>>> + hsusb1_reset: hsusb1_reset_reg {
>>>>>> + compatible = "regulator-fixed";
>>>>>> + regulator-name = "hsusb1_reset";
>>>>>> + regulator-min-microvolt = <3300000>;
>>>>>> + regulator-max-microvolt = <3300000>;
>>>>>> + gpio = <&gpio2 30 0>; /* gpio_62 */
>>>>>> + startup-delay-us = <70000>;
>>>>>> + enable-active-high;
>>>>>> + };
>>>>>
>>>>> Is this really a regulator? Or just a GPIO line used to reset the USB PHY?
>>>>
>>>> It is in fact a GPIO line used as reset.
>>>>>
>>>>> If this is the case, I don't think it should be represented as a regulator.
>>>>
>>>> Why not? I think it fits very well in the regulator device model.
>>
>> I'm not sure fitting very well is the correct term.
>> It works, for sure, but it is no different than when we were trying to abuse the regulator fmwk to enable the 32k clock in phoenix.
>> It is just a hack.
>>
>
> The only difference is there is a dedicated framework for clocks. Since there is nothing specific to
> handle reset lines it is left to the driver writer how he wants to manage it.
>

There is a proposed binding for gpio-controlled reset lines, but it is
not yet merged [1].
I guess it can fit most usage patterns, and it can be an interesting
move in the future.

Regards,

Florian

[1] http://thread.gmane.org/gmane.linux.drivers.devicetree/36830

2013-06-19 12:24:07

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

On 06/19/2013 07:05 AM, Florian Vaussard wrote:
> Hello,
>
> On 06/19/2013 01:03 PM, Roger Quadros wrote:
>> On 06/19/2013 01:10 PM, Benoit Cousson wrote:
>>> On 06/19/2013 02:46 AM, Tony Lindgren wrote:
>>>> * Roger Quadros <[email protected]> [130619 00:42]:
>>>>> Hi Benoit,
>>>>>
>>>>> On 06/19/2013 04:17 AM, Benoit Cousson wrote:
>>>>>> Hi Roger,
>>>>>>
>>>>>> On 06/18/2013 11:04 AM, Roger Quadros wrote:
>>>>>>> Provide the RESET and Power regulators for the USB PHY,
>>>>>>> the USB Host port mode and the PHY device.
>>>>>>>
>>>>>>> Also provide pin multiplexer information for the USB host
>>>>>>> pins.
>>>>>>>
>>>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>>>> ---
>>>>>>> arch/arm/boot/dts/omap4-panda-common.dtsi | 62
>>>>>>> +++++++++++++++++++++++++++++
>>>>>>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>> b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>> index 00cbaa5..7a21e8e 100644
>>>>>>> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>> @@ -59,6 +59,42 @@
>>>>>>> "AFML", "Line In",
>>>>>>> "AFMR", "Line In";
>>>>>>> };
>>>>>>> +
>>>>>>> + /* HS USB Port 1 RESET */
>>>>>>> + hsusb1_reset: hsusb1_reset_reg {
>>>>>>> + compatible = "regulator-fixed";
>>>>>>> + regulator-name = "hsusb1_reset";
>>>>>>> + regulator-min-microvolt = <3300000>;
>>>>>>> + regulator-max-microvolt = <3300000>;
>>>>>>> + gpio = <&gpio2 30 0>; /* gpio_62 */
>>>>>>> + startup-delay-us = <70000>;
>>>>>>> + enable-active-high;
>>>>>>> + };
>>>>>>
>>>>>> Is this really a regulator? Or just a GPIO line used to reset the
>>>>>> USB PHY?
>>>>>
>>>>> It is in fact a GPIO line used as reset.
>>>>>>
>>>>>> If this is the case, I don't think it should be represented as a
>>>>>> regulator.
>>>>>
>>>>> Why not? I think it fits very well in the regulator device model.
>>>
>>> I'm not sure fitting very well is the correct term.
>>> It works, for sure, but it is no different than when we were trying
>>> to abuse the regulator fmwk to enable the 32k clock in phoenix.
>>> It is just a hack.
>>>
>>
>> The only difference is there is a dedicated framework for clocks.
>> Since there is nothing specific to
>> handle reset lines it is left to the driver writer how he wants to
>> manage it.
>>
>
> There is a proposed binding for gpio-controlled reset lines, but it is
> not yet merged [1].
> I guess it can fit most usage patterns, and it can be an interesting
> move in the future.

I'm glad to see that I was not the only one thinking that the regulator
was not the right fmwk to do that :-)

Thanks for the pointer Florian.

I guess that series should be merged for 3.11? Based on the thread, it
should to through arm-soc.

Roger,

It might be tricky to have dependency on that series, but if this is
possible, you should try. Otherwise, just keep the existing one, adding
that a new solution will be available soon as a disclaimer.

Regards,
Benoit

2013-06-19 12:27:19

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

* Benoit Cousson <[email protected]> [130619 03:17]:
> On 06/19/2013 02:46 AM, Tony Lindgren wrote:
> >
> >We have a similar issue with the MMC1 PBIAS. I think in the long run we
> >should expand regulator (and possibly pinctrl) framework(s) to handle
> >comparators. We could just assume that a comparatator is a regulator,
> >and have a comparator binding that just uses the regulator code.
>
> In the case of pbias, the pinctrl seems to be a much better fit for
> my point of view. pinctrl can handle pin configuration and this is
> what the pbias is in the case of MMC pins.

Well just recently Linus W specifically wanted us to use regulator
framework for the MMC1 PBIAS rather than pinctrl. That's because
from consumer driver point of view, it changes voltage and there's
a delay involved. So I guess no conclusion yet, and it's best to
do stand alone drivers to deal with those that use pinctrl for the
pinctrl specific parts and export it as a regulator for the consumer
devices. That's pretty much along the lines what Roger has done,
except the transceiver could use the pinctrl-single,bits for the
muxing and pinconf.

Regards,

Tony

2013-06-19 12:32:24

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

* Benoit Cousson <[email protected]> [130619 05:30]:
> On 06/19/2013 07:05 AM, Florian Vaussard wrote:
> >>>>>>>+
> >>>>>>>+ /* HS USB Port 1 RESET */
> >>>>>>>+ hsusb1_reset: hsusb1_reset_reg {
> >>>>>>>+ compatible = "regulator-fixed";
> >>>>>>>+ regulator-name = "hsusb1_reset";
> >>>>>>>+ regulator-min-microvolt = <3300000>;
> >>>>>>>+ regulator-max-microvolt = <3300000>;
> >>>>>>>+ gpio = <&gpio2 30 0>; /* gpio_62 */
> >>>>>>>+ startup-delay-us = <70000>;
> >>>>>>>+ enable-active-high;
> >>>>>>>+ };
> >>>>>>
> >>>>>>Is this really a regulator? Or just a GPIO line used to reset the
> >>>>>>USB PHY?
> >>>>>
> >>>>>It is in fact a GPIO line used as reset.
> >>>>>>
> >>>>>>If this is the case, I don't think it should be represented as a
> >>>>>>regulator.
> >>>>>
> >>>>>Why not? I think it fits very well in the regulator device model.
> >>>
> >>>I'm not sure fitting very well is the correct term.
> >>>It works, for sure, but it is no different than when we were trying
> >>>to abuse the regulator fmwk to enable the 32k clock in phoenix.
> >>>It is just a hack.

If it's a reset, then yeah it's not a regulator. If the GPIO line is
really used to control the regulator in the external device, then it
makes sense to set it up as a regulator.

> >>The only difference is there is a dedicated framework for clocks.
> >>Since there is nothing specific to
> >>handle reset lines it is left to the driver writer how he wants to
> >>manage it.
> >>
> >
> >There is a proposed binding for gpio-controlled reset lines, but it is
> >not yet merged [1].
> >I guess it can fit most usage patterns, and it can be an interesting
> >move in the future.
>
> I'm glad to see that I was not the only one thinking that the
> regulator was not the right fmwk to do that :-)
>
> Thanks for the pointer Florian.

Good to hear about the gpio-controlled reset lines :)

> I guess that series should be merged for 3.11? Based on the thread,
> it should to through arm-soc.
>
> Roger,
>
> It might be tricky to have dependency on that series, but if this is
> possible, you should try. Otherwise, just keep the existing one,
> adding that a new solution will be available soon as a disclaimer.

We need some solution for v3.11 for omap4 USB on panda so people can
use it with DT.

Regards,

Tony

2013-06-19 12:35:32

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

Hi Tony,

On 06/19/2013 07:27 AM, Tony Lindgren wrote:
> * Benoit Cousson <[email protected]> [130619 03:17]:
>> On 06/19/2013 02:46 AM, Tony Lindgren wrote:
>>>
>>> We have a similar issue with the MMC1 PBIAS. I think in the long run we
>>> should expand regulator (and possibly pinctrl) framework(s) to handle
>>> comparators. We could just assume that a comparatator is a regulator,
>>> and have a comparator binding that just uses the regulator code.
>>
>> In the case of pbias, the pinctrl seems to be a much better fit for
>> my point of view. pinctrl can handle pin configuration and this is
>> what the pbias is in the case of MMC pins.
>
> Well just recently Linus W specifically wanted us to use regulator
> framework for the MMC1 PBIAS rather than pinctrl. That's because
> from consumer driver point of view, it changes voltage and there's
> a delay involved. So I guess no conclusion yet, and it's best to
> do stand alone drivers to deal with those that use pinctrl for the
> pinctrl specific parts and export it as a regulator for the consumer
> devices.

In the case of pbias, the boundary is not that clear, and it is true
that writing a complete pinctrl driver is really overkill.
I've tried in the past, and gave up due to the complexity of fmwk and
the lack of time. I still think, it is a much better fmwk to handle pin
configuration than the regulator fmwk.

> That's pretty much along the lines what Roger has done,
> except the transceiver could use the pinctrl-single,bits for the
> muxing and pinconf.

Well, in that case, this is a reset line, so it does not have anything
to do with voltage :-)

Anyway, thanks to Florian, we know that there is a real solution to that
problem. It is just not merged now :-(

Regards,
Benoit

2013-06-19 12:44:19

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

* Benoit Cousson <[email protected]> [130619 05:41]:
> Hi Tony,
>
> On 06/19/2013 07:27 AM, Tony Lindgren wrote:
> >* Benoit Cousson <[email protected]> [130619 03:17]:
> >>On 06/19/2013 02:46 AM, Tony Lindgren wrote:
> >>>
> >>>We have a similar issue with the MMC1 PBIAS. I think in the long run we
> >>>should expand regulator (and possibly pinctrl) framework(s) to handle
> >>>comparators. We could just assume that a comparatator is a regulator,
> >>>and have a comparator binding that just uses the regulator code.
> >>
> >>In the case of pbias, the pinctrl seems to be a much better fit for
> >>my point of view. pinctrl can handle pin configuration and this is
> >>what the pbias is in the case of MMC pins.
> >
> >Well just recently Linus W specifically wanted us to use regulator
> >framework for the MMC1 PBIAS rather than pinctrl. That's because
> >from consumer driver point of view, it changes voltage and there's
> >a delay involved. So I guess no conclusion yet, and it's best to
> >do stand alone drivers to deal with those that use pinctrl for the
> >pinctrl specific parts and export it as a regulator for the consumer
> >devices.
>
> In the case of pbias, the boundary is not that clear, and it is true
> that writing a complete pinctrl driver is really overkill.
> I've tried in the past, and gave up due to the complexity of fmwk
> and the lack of time. I still think, it is a much better fmwk to
> handle pin configuration than the regulator fmwk.

For omaps, these kind of registers can already be handled by
pinctrl-single,bits. What's missing is the capability to create
a regulator out of the voltage mux + comparator bits.

> >That's pretty much along the lines what Roger has done,
> >except the transceiver could use the pinctrl-single,bits for the
> >muxing and pinconf.
>
> Well, in that case, this is a reset line, so it does not have
> anything to do with voltage :-)
>
> Anyway, thanks to Florian, we know that there is a real solution to
> that problem. It is just not merged now :-(

Right. Meanwhile, sounds like the transceiver driver needs to
deal with the GPIO directly.

Regards,

Tony

2013-06-19 14:02:45

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

On 06/19/2013 02:30 PM, Benoit Cousson wrote:
> On 06/19/2013 06:03 AM, Roger Quadros wrote:
>> On 06/19/2013 01:10 PM, Benoit Cousson wrote:
>>> On 06/19/2013 02:46 AM, Tony Lindgren wrote:
>>>> * Roger Quadros <[email protected]> [130619 00:42]:
>>>>> Hi Benoit,
>>>>>
>>>>> On 06/19/2013 04:17 AM, Benoit Cousson wrote:
>>>>>> Hi Roger,
>>>>>>
>>>>>> On 06/18/2013 11:04 AM, Roger Quadros wrote:
>>>>>>> Provide the RESET and Power regulators for the USB PHY,
>>>>>>> the USB Host port mode and the PHY device.
>>>>>>>
>>>>>>> Also provide pin multiplexer information for the USB host
>>>>>>> pins.
>>>>>>>
>>>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>>>> ---
>>>>>>> arch/arm/boot/dts/omap4-panda-common.dtsi | 62 +++++++++++++++++++++++++++++
>>>>>>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>> index 00cbaa5..7a21e8e 100644
>>>>>>> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>> @@ -59,6 +59,42 @@
>>>>>>> "AFML", "Line In",
>>>>>>> "AFMR", "Line In";
>>>>>>> };
>>>>>>> +
>>>>>>> + /* HS USB Port 1 RESET */
>>>>>>> + hsusb1_reset: hsusb1_reset_reg {
>>>>>>> + compatible = "regulator-fixed";
>>>>>>> + regulator-name = "hsusb1_reset";
>>>>>>> + regulator-min-microvolt = <3300000>;
>>>>>>> + regulator-max-microvolt = <3300000>;
>>>>>>> + gpio = <&gpio2 30 0>; /* gpio_62 */
>>>>>>> + startup-delay-us = <70000>;
>>>>>>> + enable-active-high;
>>>>>>> + };
>>>>>>
>>>>>> Is this really a regulator? Or just a GPIO line used to reset the USB PHY?
>>>>>
>>>>> It is in fact a GPIO line used as reset.
>>>>>>
>>>>>> If this is the case, I don't think it should be represented as a regulator.
>>>>>
>>>>> Why not? I think it fits very well in the regulator device model.
>>>
>>> I'm not sure fitting very well is the correct term.
>>> It works, for sure, but it is no different than when we were trying to abuse the regulator fmwk to enable the 32k clock in phoenix.
>>> It is just a hack.
>>>
>>
>> The only difference is there is a dedicated framework for clocks. Since there is nothing specific to
>> handle reset lines it is left to the driver writer how he wants to manage it.
>
> Well, at that time, it was not available either. The point is still that using a existing fmwk that has nothing to do with the signal you need to handle just because it works is not a very good justification.
>
>>>>> I couldn't find a better
>>>>> way to represent this. It gives us a way to specify not only the GPIO line but also
>>>>> the polarity. I know mostly reset is active low but still there is flexibility as you never
>>>>> know for sure.
>>>
>>> Mmm, and what about just controlling the gpio from the driver?
>>
>> Yes gpio is possible. But then you need to add additional code in the driver to parse GPIO number and polarity.
>> Using regulator_get() was plain simple for me.
>
> Maybe, but it is not the right thing to do.
> Can you explain me the commonality between a reset line and a regulator???
>

I cannot prove that a reset line is a regulator, because it is not. However, the necessary features
required to manage a reset line were provided by the regulator framework e.g. gpio control, polarity,
enable delay time. It was much less hassle for me to use the regulator framework than manage
this in the phy driver.

Now that we have something specific for reset gpio I will migrate the PHY driver to that, when it is
merged.

cheers,
-roger

2013-06-19 14:05:32

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

On 06/19/2013 03:23 PM, Benoit Cousson wrote:
> On 06/19/2013 07:05 AM, Florian Vaussard wrote:
>> Hello,
>>
>> On 06/19/2013 01:03 PM, Roger Quadros wrote:
>>> On 06/19/2013 01:10 PM, Benoit Cousson wrote:
>>>> On 06/19/2013 02:46 AM, Tony Lindgren wrote:
>>>>> * Roger Quadros <[email protected]> [130619 00:42]:
>>>>>> Hi Benoit,
>>>>>>
>>>>>> On 06/19/2013 04:17 AM, Benoit Cousson wrote:
>>>>>>> Hi Roger,
>>>>>>>
>>>>>>> On 06/18/2013 11:04 AM, Roger Quadros wrote:
>>>>>>>> Provide the RESET and Power regulators for the USB PHY,
>>>>>>>> the USB Host port mode and the PHY device.
>>>>>>>>
>>>>>>>> Also provide pin multiplexer information for the USB host
>>>>>>>> pins.
>>>>>>>>
>>>>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>>>>> ---
>>>>>>>> arch/arm/boot/dts/omap4-panda-common.dtsi | 62
>>>>>>>> +++++++++++++++++++++++++++++
>>>>>>>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>> b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>> index 00cbaa5..7a21e8e 100644
>>>>>>>> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>> @@ -59,6 +59,42 @@
>>>>>>>> "AFML", "Line In",
>>>>>>>> "AFMR", "Line In";
>>>>>>>> };
>>>>>>>> +
>>>>>>>> + /* HS USB Port 1 RESET */
>>>>>>>> + hsusb1_reset: hsusb1_reset_reg {
>>>>>>>> + compatible = "regulator-fixed";
>>>>>>>> + regulator-name = "hsusb1_reset";
>>>>>>>> + regulator-min-microvolt = <3300000>;
>>>>>>>> + regulator-max-microvolt = <3300000>;
>>>>>>>> + gpio = <&gpio2 30 0>; /* gpio_62 */
>>>>>>>> + startup-delay-us = <70000>;
>>>>>>>> + enable-active-high;
>>>>>>>> + };
>>>>>>>
>>>>>>> Is this really a regulator? Or just a GPIO line used to reset the
>>>>>>> USB PHY?
>>>>>>
>>>>>> It is in fact a GPIO line used as reset.
>>>>>>>
>>>>>>> If this is the case, I don't think it should be represented as a
>>>>>>> regulator.
>>>>>>
>>>>>> Why not? I think it fits very well in the regulator device model.
>>>>
>>>> I'm not sure fitting very well is the correct term.
>>>> It works, for sure, but it is no different than when we were trying
>>>> to abuse the regulator fmwk to enable the 32k clock in phoenix.
>>>> It is just a hack.
>>>>
>>>
>>> The only difference is there is a dedicated framework for clocks.
>>> Since there is nothing specific to
>>> handle reset lines it is left to the driver writer how he wants to
>>> manage it.
>>>
>>
>> There is a proposed binding for gpio-controlled reset lines, but it is
>> not yet merged [1].
>> I guess it can fit most usage patterns, and it can be an interesting
>> move in the future.
>
> I'm glad to see that I was not the only one thinking that the regulator was not the right fmwk to do that :-)
>
> Thanks for the pointer Florian.

Thanks again Florian.
>
> I guess that series should be merged for 3.11? Based on the thread, it should to through arm-soc.
>
> Roger,
>
> It might be tricky to have dependency on that series, but if this is possible, you should try. Otherwise, just keep the existing one, adding that a new solution will be available soon as a disclaimer.
>

I will rework the PHY driver to use the new gpio-reset driver. But for 3.11 let's proceed the way it is.
I'll resend this one with a disclaimer.

cheers,
-roger

2013-06-19 18:18:44

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

On 06/19/2013 09:05 AM, Roger Quadros wrote:
> On 06/19/2013 03:23 PM, Benoit Cousson wrote:
>> On 06/19/2013 07:05 AM, Florian Vaussard wrote:
>>> Hello,
>>>
>>> On 06/19/2013 01:03 PM, Roger Quadros wrote:
>>>> On 06/19/2013 01:10 PM, Benoit Cousson wrote:
>>>>> On 06/19/2013 02:46 AM, Tony Lindgren wrote:
>>>>>> * Roger Quadros <[email protected]> [130619 00:42]:
>>>>>>> Hi Benoit,
>>>>>>>
>>>>>>> On 06/19/2013 04:17 AM, Benoit Cousson wrote:
>>>>>>>> Hi Roger,
>>>>>>>>
>>>>>>>> On 06/18/2013 11:04 AM, Roger Quadros wrote:
>>>>>>>>> Provide the RESET and Power regulators for the USB PHY,
>>>>>>>>> the USB Host port mode and the PHY device.
>>>>>>>>>
>>>>>>>>> Also provide pin multiplexer information for the USB host
>>>>>>>>> pins.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>>>>>> ---
>>>>>>>>> arch/arm/boot/dts/omap4-panda-common.dtsi | 62
>>>>>>>>> +++++++++++++++++++++++++++++
>>>>>>>>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>> b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>> index 00cbaa5..7a21e8e 100644
>>>>>>>>> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>> @@ -59,6 +59,42 @@
>>>>>>>>> "AFML", "Line In",
>>>>>>>>> "AFMR", "Line In";
>>>>>>>>> };
>>>>>>>>> +
>>>>>>>>> + /* HS USB Port 1 RESET */
>>>>>>>>> + hsusb1_reset: hsusb1_reset_reg {
>>>>>>>>> + compatible = "regulator-fixed";
>>>>>>>>> + regulator-name = "hsusb1_reset";
>>>>>>>>> + regulator-min-microvolt = <3300000>;
>>>>>>>>> + regulator-max-microvolt = <3300000>;
>>>>>>>>> + gpio = <&gpio2 30 0>; /* gpio_62 */
>>>>>>>>> + startup-delay-us = <70000>;
>>>>>>>>> + enable-active-high;
>>>>>>>>> + };
>>>>>>>>
>>>>>>>> Is this really a regulator? Or just a GPIO line used to reset the
>>>>>>>> USB PHY?
>>>>>>>
>>>>>>> It is in fact a GPIO line used as reset.
>>>>>>>>
>>>>>>>> If this is the case, I don't think it should be represented as a
>>>>>>>> regulator.
>>>>>>>
>>>>>>> Why not? I think it fits very well in the regulator device model.
>>>>>
>>>>> I'm not sure fitting very well is the correct term.
>>>>> It works, for sure, but it is no different than when we were trying
>>>>> to abuse the regulator fmwk to enable the 32k clock in phoenix.
>>>>> It is just a hack.
>>>>>
>>>>
>>>> The only difference is there is a dedicated framework for clocks.
>>>> Since there is nothing specific to
>>>> handle reset lines it is left to the driver writer how he wants to
>>>> manage it.
>>>>
>>>
>>> There is a proposed binding for gpio-controlled reset lines, but it is
>>> not yet merged [1].
>>> I guess it can fit most usage patterns, and it can be an interesting
>>> move in the future.
>>
>> I'm glad to see that I was not the only one thinking that the regulator was not the right fmwk to do that :-)
>>
>> Thanks for the pointer Florian.
>
> Thanks again Florian.
>>
>> I guess that series should be merged for 3.11? Based on the thread, it should to through arm-soc.
>>
>> Roger,
>>
>> It might be tricky to have dependency on that series, but if this is possible, you should try. Otherwise, just keep the existing one, adding that a new solution will be available soon as a disclaimer.
>>
>
> I will rework the PHY driver to use the new gpio-reset driver. But for 3.11 let's proceed the way it is.
> I'll resend this one with a disclaimer.

OK, sounds good.

Thanks,
Benoit

2013-06-19 22:41:14

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

On 06/19/2013 09:05 AM, Roger Quadros wrote:
> On 06/19/2013 03:23 PM, Benoit Cousson wrote:
>> On 06/19/2013 07:05 AM, Florian Vaussard wrote:
>>> Hello,
>>>
>>> On 06/19/2013 01:03 PM, Roger Quadros wrote:
>>>> On 06/19/2013 01:10 PM, Benoit Cousson wrote:
>>>>> On 06/19/2013 02:46 AM, Tony Lindgren wrote:
>>>>>> * Roger Quadros <[email protected]> [130619 00:42]:
>>>>>>> Hi Benoit,
>>>>>>>
>>>>>>> On 06/19/2013 04:17 AM, Benoit Cousson wrote:
>>>>>>>> Hi Roger,
>>>>>>>>
>>>>>>>> On 06/18/2013 11:04 AM, Roger Quadros wrote:
>>>>>>>>> Provide the RESET and Power regulators for the USB PHY,
>>>>>>>>> the USB Host port mode and the PHY device.
>>>>>>>>>
>>>>>>>>> Also provide pin multiplexer information for the USB host
>>>>>>>>> pins.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>>>>>> ---
>>>>>>>>> arch/arm/boot/dts/omap4-panda-common.dtsi | 62
>>>>>>>>> +++++++++++++++++++++++++++++
>>>>>>>>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>> b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>> index 00cbaa5..7a21e8e 100644
>>>>>>>>> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>> @@ -59,6 +59,42 @@
>>>>>>>>> "AFML", "Line In",
>>>>>>>>> "AFMR", "Line In";
>>>>>>>>> };
>>>>>>>>> +
>>>>>>>>> + /* HS USB Port 1 RESET */
>>>>>>>>> + hsusb1_reset: hsusb1_reset_reg {
>>>>>>>>> + compatible = "regulator-fixed";
>>>>>>>>> + regulator-name = "hsusb1_reset";
>>>>>>>>> + regulator-min-microvolt = <3300000>;
>>>>>>>>> + regulator-max-microvolt = <3300000>;
>>>>>>>>> + gpio = <&gpio2 30 0>; /* gpio_62 */
>>>>>>>>> + startup-delay-us = <70000>;
>>>>>>>>> + enable-active-high;
>>>>>>>>> + };
>>>>>>>>
>>>>>>>> Is this really a regulator? Or just a GPIO line used to reset the
>>>>>>>> USB PHY?
>>>>>>>
>>>>>>> It is in fact a GPIO line used as reset.
>>>>>>>>
>>>>>>>> If this is the case, I don't think it should be represented as a
>>>>>>>> regulator.
>>>>>>>
>>>>>>> Why not? I think it fits very well in the regulator device model.
>>>>>
>>>>> I'm not sure fitting very well is the correct term.
>>>>> It works, for sure, but it is no different than when we were trying
>>>>> to abuse the regulator fmwk to enable the 32k clock in phoenix.
>>>>> It is just a hack.
>>>>>
>>>>
>>>> The only difference is there is a dedicated framework for clocks.
>>>> Since there is nothing specific to
>>>> handle reset lines it is left to the driver writer how he wants to
>>>> manage it.
>>>>
>>>
>>> There is a proposed binding for gpio-controlled reset lines, but it is
>>> not yet merged [1].
>>> I guess it can fit most usage patterns, and it can be an interesting
>>> move in the future.
>>
>> I'm glad to see that I was not the only one thinking that the regulator was not the right fmwk to do that :-)
>>
>> Thanks for the pointer Florian.
>
> Thanks again Florian.
>>
>> I guess that series should be merged for 3.11? Based on the thread, it should to through arm-soc.
>>
>> Roger,
>>
>> It might be tricky to have dependency on that series, but if this is possible, you should try. Otherwise, just keep the existing one, adding that a new solution will be available soon as a disclaimer.
>>
>
> I will rework the PHY driver to use the new gpio-reset driver. But for 3.11 let's proceed the way it is.
> I'll resend this one with a disclaimer.

OK, I've just done it myself while applying your series.

Regards,
Benoit

2013-06-20 11:50:23

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] ARM: dts: omap4-panda: Add USB Host support

On 06/20/2013 01:40 AM, Benoit Cousson wrote:
> On 06/19/2013 09:05 AM, Roger Quadros wrote:
>> On 06/19/2013 03:23 PM, Benoit Cousson wrote:
>>> On 06/19/2013 07:05 AM, Florian Vaussard wrote:
>>>> Hello,
>>>>
>>>> On 06/19/2013 01:03 PM, Roger Quadros wrote:
>>>>> On 06/19/2013 01:10 PM, Benoit Cousson wrote:
>>>>>> On 06/19/2013 02:46 AM, Tony Lindgren wrote:
>>>>>>> * Roger Quadros <[email protected]> [130619 00:42]:
>>>>>>>> Hi Benoit,
>>>>>>>>
>>>>>>>> On 06/19/2013 04:17 AM, Benoit Cousson wrote:
>>>>>>>>> Hi Roger,
>>>>>>>>>
>>>>>>>>> On 06/18/2013 11:04 AM, Roger Quadros wrote:
>>>>>>>>>> Provide the RESET and Power regulators for the USB PHY,
>>>>>>>>>> the USB Host port mode and the PHY device.
>>>>>>>>>>
>>>>>>>>>> Also provide pin multiplexer information for the USB host
>>>>>>>>>> pins.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>>>>>>> ---
>>>>>>>>>> arch/arm/boot/dts/omap4-panda-common.dtsi | 62
>>>>>>>>>> +++++++++++++++++++++++++++++
>>>>>>>>>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>>> b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>>> index 00cbaa5..7a21e8e 100644
>>>>>>>>>> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>>> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
>>>>>>>>>> @@ -59,6 +59,42 @@
>>>>>>>>>> "AFML", "Line In",
>>>>>>>>>> "AFMR", "Line In";
>>>>>>>>>> };
>>>>>>>>>> +
>>>>>>>>>> + /* HS USB Port 1 RESET */
>>>>>>>>>> + hsusb1_reset: hsusb1_reset_reg {
>>>>>>>>>> + compatible = "regulator-fixed";
>>>>>>>>>> + regulator-name = "hsusb1_reset";
>>>>>>>>>> + regulator-min-microvolt = <3300000>;
>>>>>>>>>> + regulator-max-microvolt = <3300000>;
>>>>>>>>>> + gpio = <&gpio2 30 0>; /* gpio_62 */
>>>>>>>>>> + startup-delay-us = <70000>;
>>>>>>>>>> + enable-active-high;
>>>>>>>>>> + };
>>>>>>>>>
>>>>>>>>> Is this really a regulator? Or just a GPIO line used to reset the
>>>>>>>>> USB PHY?
>>>>>>>>
>>>>>>>> It is in fact a GPIO line used as reset.
>>>>>>>>>
>>>>>>>>> If this is the case, I don't think it should be represented as a
>>>>>>>>> regulator.
>>>>>>>>
>>>>>>>> Why not? I think it fits very well in the regulator device model.
>>>>>>
>>>>>> I'm not sure fitting very well is the correct term.
>>>>>> It works, for sure, but it is no different than when we were trying
>>>>>> to abuse the regulator fmwk to enable the 32k clock in phoenix.
>>>>>> It is just a hack.
>>>>>>
>>>>>
>>>>> The only difference is there is a dedicated framework for clocks.
>>>>> Since there is nothing specific to
>>>>> handle reset lines it is left to the driver writer how he wants to
>>>>> manage it.
>>>>>
>>>>
>>>> There is a proposed binding for gpio-controlled reset lines, but it is
>>>> not yet merged [1].
>>>> I guess it can fit most usage patterns, and it can be an interesting
>>>> move in the future.
>>>
>>> I'm glad to see that I was not the only one thinking that the regulator was not the right fmwk to do that :-)
>>>
>>> Thanks for the pointer Florian.
>>
>> Thanks again Florian.
>>>
>>> I guess that series should be merged for 3.11? Based on the thread, it should to through arm-soc.
>>>
>>> Roger,
>>>
>>> It might be tricky to have dependency on that series, but if this is possible, you should try. Otherwise, just keep the existing one, adding that a new solution will be available soon as a disclaimer.
>>>
>>
>> I will rework the PHY driver to use the new gpio-reset driver. But for 3.11 let's proceed the way it is.
>> I'll resend this one with a disclaimer.
>
> OK, I've just done it myself while applying your series.
>

Great !! Thanks.

There is a similar patch for beagle-xm. But I will resend it to you with the disclaimer.

cheers,
-roger

2013-07-15 14:05:46

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] ARM: OMAP2+: Provide alias to USB PHY clock

Hi Tony,

On 06/18/2013 07:04 PM, Roger Quadros wrote:
> Till the OMAP clocks are correctly defined in device tree, use
> this temporary hack to provide clock alias to the USB PHY clocks.
>
> Without this, USB Host & Ethernet will not be functional with
> device tree boots on Panda and uEVM.

Looks like this one got missed out.

USB host no longer works on Panda with DT boot.
Could you please queue this for next 3.11-rc? Thanks.

cheers,
-roger

>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
> arch/arm/mach-omap2/board-generic.c | 23 ++++++++++++++++++++++-
> 1 files changed, 22 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> index 88aa6b1..7428733 100644
> --- a/arch/arm/mach-omap2/board-generic.c
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -15,6 +15,7 @@
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <linux/irqdomain.h>
> +#include <linux/clk.h>
>
> #include <asm/mach/arch.h>
>
> @@ -35,6 +36,21 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
> { }
> };
>
> +/*
> + * Create alias for USB host PHY clock.
> + * Remove this when clock phandle can be provided via DT
> + */
> +static void __init legacy_init_ehci_clk(char *clkname)
> +{
> + int ret;
> +
> + ret = clk_add_alias("main_clk", NULL, clkname, NULL);
> + if (ret) {
> + pr_err("%s:Failed to add main_clk alias to %s :%d\n",
> + __func__, clkname, ret);
> + }
> +}
> +
> static void __init omap_generic_init(void)
> {
> omap_sdrc_init(NULL, NULL);
> @@ -45,10 +61,15 @@ static void __init omap_generic_init(void)
> * HACK: call display setup code for selected boards to enable omapdss.
> * This will be removed when omapdss supports DT.
> */
> - if (of_machine_is_compatible("ti,omap4-panda"))
> + if (of_machine_is_compatible("ti,omap4-panda")) {
> omap4_panda_display_init_of();
> + legacy_init_ehci_clk("auxclk3_ck");
> +
> + }
> else if (of_machine_is_compatible("ti,omap4-sdp"))
> omap_4430sdp_display_init_of();
> + else if (of_machine_is_compatible("ti,omap5-uevm"))
> + legacy_init_ehci_clk("auxclk1_ck");
> }
>
> #ifdef CONFIG_SOC_OMAP2420
>

2013-07-16 12:33:01

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] ARM: OMAP2+: Provide alias to USB PHY clock

* Roger Quadros <[email protected]> [130715 07:11]:
> Hi Tony,
>
> On 06/18/2013 07:04 PM, Roger Quadros wrote:
> > Till the OMAP clocks are correctly defined in device tree, use
> > this temporary hack to provide clock alias to the USB PHY clocks.
> >
> > Without this, USB Host & Ethernet will not be functional with
> > device tree boots on Panda and uEVM.
>
> Looks like this one got missed out.
>
> USB host no longer works on Panda with DT boot.
> Could you please queue this for next 3.11-rc? Thanks.

OK thanks for checking, applying into omap-for-v3.11/fixes.

Looks like I also have some pending the .dts changes in
omap-for-v3.11/dt branch:

4cf2198b1e3cee47a1cccbb500b67782c36615d5 ARM: dts: omap3-beagle-xm: Add USB host supprot for Rev. Ax/Bx
9bc44515819a00c0ffaf751ad497ddf275089ede ARM: dts: omap3-beagle-xm: Add USB Host support
fd65d7df4598c1d2ad27d664c719611a1a070edc ARM: dts: omap3-beagle: Make USB host pin naming consistent
39ea891d9da6ae1824bc9f689db3f306f5ce0724 ARM: dts: omap4-panda: Add USB Host support

Those should get merged as fixes, right? Can you please
check and see if they're still OK or if we also need
something else?

Regards,

Tony

2013-07-16 13:13:05

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] ARM: OMAP2+: Provide alias to USB PHY clock

On 07/15/2013 04:05 PM, Roger Quadros wrote:
> Hi Tony,
>
> On 06/18/2013 07:04 PM, Roger Quadros wrote:
>> Till the OMAP clocks are correctly defined in device tree, use
>> this temporary hack to provide clock alias to the USB PHY clocks.
>>
>> Without this, USB Host & Ethernet will not be functional with
>> device tree boots on Panda and uEVM.
>
> Looks like this one got missed out.

Could it be because you promised to resend it (see [1]). I was upgrading
our test stuff to 3.11-rc1 and made a DT appended image. Boot went fine,
but no USB/ethernet. Should I pickup all four patches?

Regards,
Arend

[1] http://mid.gmane.org/[email protected]

> USB host no longer works on Panda with DT boot.
> Could you please queue this for next 3.11-rc? Thanks.
>
> cheers,
> -roger
>

2013-07-16 13:19:07

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] ARM: OMAP2+: Provide alias to USB PHY clock

On 07/16/2013 03:12 PM, Arend van Spriel wrote:
> On 07/15/2013 04:05 PM, Roger Quadros wrote:
>> Hi Tony,
>>
>> On 06/18/2013 07:04 PM, Roger Quadros wrote:
>>> Till the OMAP clocks are correctly defined in device tree, use
>>> this temporary hack to provide clock alias to the USB PHY clocks.
>>>
>>> Without this, USB Host & Ethernet will not be functional with
>>> device tree boots on Panda and uEVM.
>>
>> Looks like this one got missed out.
>
> Could it be because you promised to resend it (see [1]). I was upgrading
> our test stuff to 3.11-rc1 and made a DT appended image. Boot went fine,
> but no USB/ethernet. Should I pickup all four patches?

Nevermind. The first three seem to be in already.

> Regards,
> Arend
>
> [1] http://mid.gmane.org/[email protected]
>
>> USB host no longer works on Panda with DT boot.
>> Could you please queue this for next 3.11-rc? Thanks.
>>
>> cheers,
>> -roger
>>
>

2013-07-16 13:39:57

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] ARM: OMAP2+: Provide alias to USB PHY clock

On 07/16/2013 03:32 PM, Tony Lindgren wrote:
> * Roger Quadros <[email protected]> [130715 07:11]:
>> Hi Tony,
>>
>> On 06/18/2013 07:04 PM, Roger Quadros wrote:
>>> Till the OMAP clocks are correctly defined in device tree, use
>>> this temporary hack to provide clock alias to the USB PHY clocks.
>>>
>>> Without this, USB Host & Ethernet will not be functional with
>>> device tree boots on Panda and uEVM.
>>
>> Looks like this one got missed out.
>>
>> USB host no longer works on Panda with DT boot.
>> Could you please queue this for next 3.11-rc? Thanks.
>
> OK thanks for checking, applying into omap-for-v3.11/fixes.

Thanks.

>
> Looks like I also have some pending the .dts changes in
> omap-for-v3.11/dt branch:
>
> 4cf2198b1e3cee47a1cccbb500b67782c36615d5 ARM: dts: omap3-beagle-xm: Add USB host supprot for Rev. Ax/Bx
> 9bc44515819a00c0ffaf751ad497ddf275089ede ARM: dts: omap3-beagle-xm: Add USB Host support
> fd65d7df4598c1d2ad27d664c719611a1a070edc ARM: dts: omap3-beagle: Make USB host pin naming consistent

Please disregard those. They are old versions.

Benoit had agreed to take the newer version of those for 3.12.

Please pick this one to get usb host working on 3.11 with DT.

http://www.spinics.net/lists/arm-kernel/msg253612.html

> 39ea891d9da6ae1824bc9f689db3f306f5ce0724 ARM: dts: omap4-panda: Add USB Host support

This one is already in through Benoit.

cheers,
-roger

2013-07-16 13:43:56

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] ARM: OMAP2+: Provide alias to USB PHY clock

* Roger Quadros <[email protected]> [130716 06:45]:
> On 07/16/2013 03:32 PM, Tony Lindgren wrote:
> > * Roger Quadros <[email protected]> [130715 07:11]:
> >> Hi Tony,
> >>
> >> On 06/18/2013 07:04 PM, Roger Quadros wrote:
> >>> Till the OMAP clocks are correctly defined in device tree, use
> >>> this temporary hack to provide clock alias to the USB PHY clocks.
> >>>
> >>> Without this, USB Host & Ethernet will not be functional with
> >>> device tree boots on Panda and uEVM.
> >>
> >> Looks like this one got missed out.
> >>
> >> USB host no longer works on Panda with DT boot.
> >> Could you please queue this for next 3.11-rc? Thanks.
> >
> > OK thanks for checking, applying into omap-for-v3.11/fixes.
>
> Thanks.
>
> >
> > Looks like I also have some pending the .dts changes in
> > omap-for-v3.11/dt branch:
> >
> > 4cf2198b1e3cee47a1cccbb500b67782c36615d5 ARM: dts: omap3-beagle-xm: Add USB host supprot for Rev. Ax/Bx
> > 9bc44515819a00c0ffaf751ad497ddf275089ede ARM: dts: omap3-beagle-xm: Add USB Host support
> > fd65d7df4598c1d2ad27d664c719611a1a070edc ARM: dts: omap3-beagle: Make USB host pin naming consistent
>
> Please disregard those. They are old versions.

OK, poof that branch is now gone.

> Benoit had agreed to take the newer version of those for 3.12.
>
> Please pick this one to get usb host working on 3.11 with DT.
>
> http://www.spinics.net/lists/arm-kernel/msg253612.html
>
> > 39ea891d9da6ae1824bc9f689db3f306f5ce0724 ARM: dts: omap4-panda: Add USB Host support
>
> This one is already in through Benoit.

It's best that Benoit queue all those to avoid more confusion :)

Regards,

Tony