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 <rog...@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 <rog...@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

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

Reply via email to