| Message ID | 1371571487-14389-2-git-send-email-rogerq@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <rogerq@ti.com> > --- > 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>; > +}; >
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 <rogerq@ti.com> >> --- >> 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
* Roger Quadros <rogerq@ti.com> [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 <rogerq@ti.com> > >> --- > >> 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
On 06/19/2013 02:46 AM, Tony Lindgren wrote: > * Roger Quadros <rogerq@ti.com> [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 <rogerq@ti.com> >>>> --- >>>> 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
On 06/19/2013 01:10 PM, Benoit Cousson wrote: > On 06/19/2013 02:46 AM, Tony Lindgren wrote: >> * Roger Quadros <rogerq@ti.com> [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 <rogerq@ti.com> >>>>> --- >>>>> 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
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 <rogerq@ti.com> [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 <rogerq@ti.com> >>>>>> --- >>>>>> 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
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 <rogerq@ti.com> [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 <rogerq@ti.com> >>>>>> --- >>>>>> 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
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 <rogerq@ti.com> [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 <rogerq@ti.com> >>>>>>> --- >>>>>>> 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
* Benoit Cousson <b-cousson@ti.com> [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
* Benoit Cousson <b-cousson@ti.com> [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
Hi Tony, On 06/19/2013 07:27 AM, Tony Lindgren wrote: > * Benoit Cousson <b-cousson@ti.com> [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
* Benoit Cousson <b-cousson@ti.com> [130619 05:41]: > Hi Tony, > > On 06/19/2013 07:27 AM, Tony Lindgren wrote: > >* Benoit Cousson <b-cousson@ti.com> [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
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 <rogerq@ti.com> [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 <rogerq@ti.com> >>>>>>> --- >>>>>>> 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
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 <rogerq@ti.com> [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 <rogerq@ti.com> >>>>>>>> --- >>>>>>>> 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
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 <rogerq@ti.com> [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 <rogerq@ti.com> >>>>>>>>> --- >>>>>>>>> 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
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 <rogerq@ti.com> [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 <rogerq@ti.com> >>>>>>>>> --- >>>>>>>>> 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
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 <rogerq@ti.com> [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 <rogerq@ti.com> >>>>>>>>>> --- >>>>>>>>>> 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
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>; +};
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 <rogerq@ti.com> --- arch/arm/boot/dts/omap4-panda-common.dtsi | 62 +++++++++++++++++++++++++++++ 1 files changed, 62 insertions(+), 0 deletions(-)