On 03/10/2014 05:13 PM, Florian Vaussard wrote: > Hi Roger, > > On 03/10/2014 11:30 AM, Roger Quadros wrote: >> Hi Florian, >> >> On 03/07/2014 09:22 PM, Florian Vaussard wrote: >>> Add the High-Speed USB PHY. >>> >>> Signed-off-by: Florian Vaussard <florian.vaussard@xxxxxxx> >>> --- >>> arch/arm/boot/dts/omap3-overo-base.dtsi | 44 ++++++++++++++++++++++++++++++++ >>> arch/arm/boot/dts/omap3-overo-storm.dtsi | 16 ++++++++++++ >>> arch/arm/boot/dts/omap3-overo.dtsi | 16 ++++++++++++ >>> 3 files changed, 76 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/omap3-overo-base.dtsi b/arch/arm/boot/dts/omap3-overo-base.dtsi >>> index edac70e..13d1ad2 100644 >>> --- a/arch/arm/boot/dts/omap3-overo-base.dtsi >>> +++ b/arch/arm/boot/dts/omap3-overo-base.dtsi >>> @@ -30,6 +30,24 @@ >>> ti,codec = <&twl_audio>; >>> }; >>> >>> + /* HS USB Port 2 Power */ >>> + hsusb2_power: hsusb2_power_reg { >>> + compatible = "regulator-fixed"; >>> + regulator-name = "hsusb2_vbus"; >>> + regulator-min-microvolt = <5000000>; >>> + regulator-max-microvolt = <5000000>; >>> + gpio = <&gpio6 8 0>; /* gpio_168: vbus enable */ >>> + startup-delay-us = <70000>; >>> + enable-active-high; >>> + }; >>> + >>> + /* HS USB Host PHY on PORT 2 */ >>> + hsusb2_phy: hsusb2_phy { >>> + compatible = "usb-nop-xceiv"; >>> + reset-gpios = <&gpio6 23 GPIO_ACTIVE_LOW>; /* gpio_183 */ >>> + vcc-supply = <&hsusb2_power>; >>> + }; >>> + >>> /* Regulator to trigger the nPoweron signal of the Wifi module */ >>> w3cbw003c_npoweron: regulator-w3cbw003c-npoweron { >>> compatible = "regulator-fixed"; >>> @@ -64,6 +82,11 @@ >>> }; >>> >>> &omap3_pmx_core { >>> + pinctrl-names = "default"; >>> + pinctrl-0 = < >>> + &hsusb2_pins >>> + >; >>> + >>> uart2_pins: pinmux_uart2_pins { >>> pinctrl-single,pins = < >>> OMAP3_CORE1_IOPAD(0x216c, PIN_INPUT | MUX_MODE1) /* mcbsp3_dx.uart2_cts */ >>> @@ -116,6 +139,19 @@ >>> OMAP3_CORE1_IOPAD(0x219c, PIN_OUTPUT | MUX_MODE4) /* uart3_rts_sd.gpio_164 */ >>> >; >>> }; >>> + >>> + hsusb2_pins: pinmux_hsusb2_pins { >>> + pinctrl-single,pins = < >>> + OMAP3_CORE1_IOPAD(0x21d4, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi1_cs3.hsusb2_data2 */ >>> + OMAP3_CORE1_IOPAD(0x21d6, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi2_clk.hsusb2_data7 */ >>> + OMAP3_CORE1_IOPAD(0x21d8, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi2_simo.hsusb2_data4 */ >>> + OMAP3_CORE1_IOPAD(0x21da, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi2_somi.hsusb2_data5 */ >>> + OMAP3_CORE1_IOPAD(0x21dc, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi2_cs0.hsusb2_data6 */ >>> + OMAP3_CORE1_IOPAD(0x21de, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi2_cs1.hsusb2_data3 */ >>> + OMAP3_CORE1_IOPAD(0x21be, PIN_OUTPUT | MUX_MODE4) /* i2c2_scl.gpio_168 */ >>> + OMAP3_CORE1_IOPAD(0x21c0, PIN_OUTPUT | MUX_MODE4) /* i2c2_sda.gpio_183 */ >>> + >; >>> + }; >>> }; >>> >>> &i2c1 { >>> @@ -177,6 +213,14 @@ >>> power = <50>; >>> }; >>> >>> +&usbhshost { >>> + port2-mode = "ehci-phy"; >>> +}; >>> + >>> +&usbhsehci { >>> + phys = <0 &hsusb2_phy>; >>> +}; >>> + >>> &uart2 { >>> pinctrl-names = "default"; >>> pinctrl-0 = <&uart2_pins>; >>> diff --git a/arch/arm/boot/dts/omap3-overo-storm.dtsi b/arch/arm/boot/dts/omap3-overo-storm.dtsi >>> index c235ae8..6cb418b 100644 >>> --- a/arch/arm/boot/dts/omap3-overo-storm.dtsi >>> +++ b/arch/arm/boot/dts/omap3-overo-storm.dtsi >>> @@ -10,6 +10,22 @@ >>> #include "omap3-overo-base.dtsi" >>> >>> &omap3_pmx_core2 { >>> + pinctrl-names = "default"; >>> + pinctrl-0 = < >>> + &hsusb2_2_pins >>> + >; >>> + >>> + hsusb2_2_pins: pinmux_hsusb2_2_pins { >>> + pinctrl-single,pins = < >>> + OMAP3630_CORE2_IOPAD(0x25f0, PIN_OUTPUT | MUX_MODE3) /* etk_d10.hsusb2_clk */ >>> + OMAP3630_CORE2_IOPAD(0x25f2, PIN_OUTPUT | MUX_MODE3) /* etk_d11.hsusb2_stp */ >>> + OMAP3630_CORE2_IOPAD(0x25f4, PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d12.hsusb2_dir */ >>> + OMAP3630_CORE2_IOPAD(0x25f6, PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d13.hsusb2_nxt */ >>> + OMAP3630_CORE2_IOPAD(0x25f8, PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d14.hsusb2_data0 */ >>> + OMAP3630_CORE2_IOPAD(0x25fa, PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d15.hsusb2_data1 */ >> >> If you don't use the OMAP3x30_CORE2_IOPAD() macro then these definitions can fit in omap3-overo-base.dtsi. >> The offsets will be taken care of if you place them within &omap3_pmx_core2 {} . e.g. >> >> + 0x50 (PIN_OUTPUT | MUX_MODE3) /* etk_d10.hsusb2_clk */ >> + 0x52 (PIN_OUTPUT | MUX_MODE3) /* etk_d11.hsusb2_stp */ >> + 0x54 (PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d12.hsusb2_dir */ >> + 0x56 (PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d13.hsusb2_nxt */ >> + 0x58 (PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d14.hsusb2_data0 */ >> + 0x5a (PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d15.hsusb2_data1 */ >> >> Do you think this is better? >> > > I do not think that this could work. Let's take for example hsusb2_clk > et let's do the math. Here, 0x50 is the offset inside omap3_pmx_core2 > pinctrl, so: > > hsusb2_clk omap3_pmx_core2 hsusb2_clk > offset base addr. PADCONF addr. > ---- ---------- ---------- > OMAP34xx: 0x50 0x480025d8 0x48002628 *NO* > OMAP36xx: 0x50 0x480025a0 0x480025F0 *ok* > > Looking at the TRM, the PADCONF address for hsusb2_clk is 0x480025F0 in > both cases. Thus we will be wrong on OMAP34xx (offset should be 0x18 > instead of 0x50). Thus so far, I do not see any magical solution apart > putting the omap3_pmx_core2 pins in a SoC-dependant .dtsi file. > > I already expressed my concerns [1] about having a different base > address for omap3_pmx_core2 depending on the SoC (although I do > understand the rational behind). This makes cross-SoC platforms more > difficult to maintain. Any sane design would maintain register offsets but it doesn't seem so with omap34xx vs omap36xx. So my assumption was wrong. I'll ack this patch. cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html