| Message ID | 1370414770-1485-3-git-send-email-r.sricharan@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello, Some very minor comments. On 06/05/2013 08:46 AM, Sricharan R wrote: > From: Roger Quadros <rogerq@ti.com> > > Provide the RESET regulators for the USB PHYs, the USB Host > port modes and the PHY devices. > > Also provide pin multiplexer information for the USB host > pins. > > Cc: Roger Quadros <rogerq@ti.com> > Signed-off-by: Roger Quadros <rogerq@ti.com> > [Sricharan R <r.sricharan@ti.com>: Replaced constants with preprocessor macros] > Signed-off-by: Sricharan R <r.sricharan@ti.com> > --- > arch/arm/boot/dts/omap5-uevm.dts | 77 ++++++++++++++++++++++++++++++++++++++ > arch/arm/boot/dts/omap5.dtsi | 30 +++++++++++++++ > 2 files changed, 107 insertions(+) > > diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts > index 843a001..cf862df 100644 > --- a/arch/arm/boot/dts/omap5-uevm.dts > +++ b/arch/arm/boot/dts/omap5-uevm.dts > @@ -25,6 +25,47 @@ > regulator-max-microvolt = <3000000>; > }; > > + /* HS USB Port 2 RESET */ > + hsusb2_reset: hsusb2_reset_reg { > + compatible = "regulator-fixed"; > + regulator-name = "hsusb2_reset"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpio = <&gpio3 16 GPIO_ACTIVE_HIGH>; /* gpio3_80 HUB_NRESET */ > + startup-delay-us = <70000>; > + enable-active-high; > + }; > + > + /* HS USB Host PHY on PORT 2 */ > + hsusb2_phy: hsusb2_phy { > + compatible = "usb-nop-xceiv"; > + reset-supply = <&hsusb2_reset>; > + }; > + > + /* HS USB Port 3 RESET */ > + hsusb3_reset: hsusb3_reset_reg { > + compatible = "regulator-fixed"; > + regulator-name = "hsusb3_reset"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpio = <&gpio3 15 GPIO_ACTIVE_HIGH>; /* gpio3_79 ETH_NRESET */ > + startup-delay-us = <70000>; > + enable-active-high; > + }; > + > + /* HS USB Host PHY on PORT 3 */ > + hsusb3_phy: hsusb3_phy { > + compatible = "usb-nop-xceiv"; > + reset-supply = <&hsusb3_reset>; > + }; > + > + /* hsusb2_phy is clocked by FREF_CLK1 i.e. auxclk1 */ > + clock_alias { > + clock-name = "auxclk1_ck"; > + clock-alias = "main_clk"; > + device = <&hsusb2_phy>; > + clock-frequency = <19200000>; /* 19.2 MHz */ > + }; > }; > > &omap5_pmx_core { > @@ -35,6 +76,7 @@ > &dmic_pins > &mcbsp1_pins > &mcbsp2_pins > + &usbhost_pins > >; > > twl6040_pins: pinmux_twl6040_pins { > @@ -120,6 +162,32 @@ > 0x16c (PIN_INPUT | MUX_MODE1) /* mcspi2_cs */ > >; > }; > + > + usbhost_pins: pinmux_usbhost_pins { > + pinctrl-single,pins = < > + 0x84 (PIN_INPUT | MUX_MODE0) /* usbb2_hsic_strobe INPUT | MODE 0 */ > + 0x86 (PIN_INPUT | MUX_MODE0) /* usbb2_hsic_data INPUT | MODE 0 */ Comments are redundant with the constants, so maybe you can leave this part out. Same for a few others below. > + > + 0x19e (PIN_INPUT | MUX_MODE0) /* usbb3_hsic_strobe INPUT | MODE 0 */ > + 0x1a0 (PIN_INPUT | MUX_MODE0) /* usbb3_hsic_data INPUT | MODE 0 */ > + > + 0x70 (PIN_OUTPUT | MUX_MODE6) /* gpio3_80 OUTPUT | MODE 6 HUB_NRESET */ > + 0x6e (PIN_OUTPUT | MUX_MODE6) /* gpio3_79 OUTPUT | MODE 6 ETH_NRESET */ > + >; > + }; > +}; > + > +&omap5_pmx_wkup { > + pinctrl-names = "default"; > + pinctrl-0 = < > + &usbhost_wkup_pins > + >; > + > + usbhost_wkup_pins: pinmux_usbhost_wkup_pins { > + pinctrl-single,pins = < > + 0x1A (PIN_OUTPUT | MUX_MODE0) /* fref_clk1_out OUTPUT | MODE 7 for USB hub clk */ Mismatch between constants and comments, which mode should it be? > + >; > + }; > }; > > &mmc1 { > @@ -164,6 +232,15 @@ > status = "disabled"; > }; > > +&usbhshost { > + port2-mode = "ehci-hsic"; > + port3-mode = "ehci-hsic"; > +}; > + > +&usbhsehci { > + phys = <0 &hsusb2_phy &hsusb3_phy>; > +}; > + > &mcspi1 { > > }; > diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi > index 1e84db8..67d6e1f 100644 > --- a/arch/arm/boot/dts/omap5.dtsi > +++ b/arch/arm/boot/dts/omap5.dtsi > @@ -666,5 +666,35 @@ > ctrl-module = <&omap_control_usb>; > }; > }; > + > + usbhstll: usbhstll@4a062000 { > + compatible = "ti,usbhs-tll"; > + reg = <0x4a062000 0x1000>; > + interrupts = <0 78 IRQ_TYPE_LEVEL_HIGH>; I guess that here you can replace '0' with GIC_SPI. > + ti,hwmods = "usb_tll_hs"; > + }; > + > + usbhshost: usbhshost@4a064000 { > + compatible = "ti,usbhs-host"; > + reg = <0x4a064000 0x800>; > + ti,hwmods = "usb_host_hs"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + usbhsohci: ohci@4a064800 { > + compatible = "ti,ohci-omap3", "usb-ohci"; > + reg = <0x4a064800 0x400>; > + interrupt-parent = <&gic>; > + interrupts = <0 76 IRQ_TYPE_LEVEL_HIGH>; Same here. > + }; > + > + usbhsehci: ehci@4a064c00 { > + compatible = "ti,ehci-omap", "usb-ehci"; > + reg = <0x4a064c00 0x400>; > + interrupt-parent = <&gic>; > + interrupts = <0 77 IRQ_TYPE_LEVEL_HIGH>; And here. > + }; > + }; > }; > }; > Regards, Florian
Hi, On Wednesday 05 June 2013 01:29 PM, Florian Vaussard wrote: > Hello, > > Some very minor comments. > > On 06/05/2013 08:46 AM, Sricharan R wrote: >> From: Roger Quadros <rogerq@ti.com> >> >> Provide the RESET regulators for the USB PHYs, the USB Host >> port modes and the PHY devices. >> >> Also provide pin multiplexer information for the USB host >> pins. >> >> Cc: Roger Quadros <rogerq@ti.com> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> [Sricharan R <r.sricharan@ti.com>: Replaced constants with preprocessor macros] >> Signed-off-by: Sricharan R <r.sricharan@ti.com> >> --- >> arch/arm/boot/dts/omap5-uevm.dts | 77 ++++++++++++++++++++++++++++++++++++++ >> arch/arm/boot/dts/omap5.dtsi | 30 +++++++++++++++ >> 2 files changed, 107 insertions(+) >> >> diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts >> index 843a001..cf862df 100644 >> --- a/arch/arm/boot/dts/omap5-uevm.dts >> +++ b/arch/arm/boot/dts/omap5-uevm.dts >> @@ -25,6 +25,47 @@ >> regulator-max-microvolt = <3000000>; >> }; >> >> + /* HS USB Port 2 RESET */ >> + hsusb2_reset: hsusb2_reset_reg { >> + compatible = "regulator-fixed"; >> + regulator-name = "hsusb2_reset"; >> + regulator-min-microvolt = <3300000>; >> + regulator-max-microvolt = <3300000>; >> + gpio = <&gpio3 16 GPIO_ACTIVE_HIGH>; /* gpio3_80 HUB_NRESET */ >> + startup-delay-us = <70000>; >> + enable-active-high; >> + }; >> + >> + /* HS USB Host PHY on PORT 2 */ >> + hsusb2_phy: hsusb2_phy { >> + compatible = "usb-nop-xceiv"; >> + reset-supply = <&hsusb2_reset>; >> + }; >> + >> + /* HS USB Port 3 RESET */ >> + hsusb3_reset: hsusb3_reset_reg { >> + compatible = "regulator-fixed"; >> + regulator-name = "hsusb3_reset"; >> + regulator-min-microvolt = <3300000>; >> + regulator-max-microvolt = <3300000>; >> + gpio = <&gpio3 15 GPIO_ACTIVE_HIGH>; /* gpio3_79 ETH_NRESET */ >> + startup-delay-us = <70000>; >> + enable-active-high; >> + }; >> + >> + /* HS USB Host PHY on PORT 3 */ >> + hsusb3_phy: hsusb3_phy { >> + compatible = "usb-nop-xceiv"; >> + reset-supply = <&hsusb3_reset>; >> + }; >> + >> + /* hsusb2_phy is clocked by FREF_CLK1 i.e. auxclk1 */ >> + clock_alias { >> + clock-name = "auxclk1_ck"; >> + clock-alias = "main_clk"; >> + device = <&hsusb2_phy>; >> + clock-frequency = <19200000>; /* 19.2 MHz */ >> + }; >> }; >> >> &omap5_pmx_core { >> @@ -35,6 +76,7 @@ >> &dmic_pins >> &mcbsp1_pins >> &mcbsp2_pins >> + &usbhost_pins >> >; >> >> twl6040_pins: pinmux_twl6040_pins { >> @@ -120,6 +162,32 @@ >> 0x16c (PIN_INPUT | MUX_MODE1) /* mcspi2_cs */ >> >; >> }; >> + >> + usbhost_pins: pinmux_usbhost_pins { >> + pinctrl-single,pins = < >> + 0x84 (PIN_INPUT | MUX_MODE0) /* usbb2_hsic_strobe INPUT | MODE 0 */ >> + 0x86 (PIN_INPUT | MUX_MODE0) /* usbb2_hsic_data INPUT | MODE 0 */ > > Comments are redundant with the constants, so maybe you can leave this part out. > Same for a few others below. > Ok, I agree here for attributes like pulls, i/o etc, comments are now not required. But comment is still good to describe just the mux mode functionality ? One module will use different pins, like data, clk, gpios, etc. Just MUX_MODEX does not complete it.. >> + >> + 0x19e (PIN_INPUT | MUX_MODE0) /* usbb3_hsic_strobe INPUT | MODE 0 */ >> + 0x1a0 (PIN_INPUT | MUX_MODE0) /* usbb3_hsic_data INPUT | MODE 0 */ >> + >> + 0x70 (PIN_OUTPUT | MUX_MODE6) /* gpio3_80 OUTPUT | MODE 6 HUB_NRESET */ >> + 0x6e (PIN_OUTPUT | MUX_MODE6) /* gpio3_79 OUTPUT | MODE 6 ETH_NRESET */ >> + >; >> + }; >> +}; >> + >> +&omap5_pmx_wkup { >> + pinctrl-names = "default"; >> + pinctrl-0 = < >> + &usbhost_wkup_pins >> + >; >> + >> + usbhost_wkup_pins: pinmux_usbhost_wkup_pins { >> + pinctrl-single,pins = < >> + 0x1A (PIN_OUTPUT | MUX_MODE0) /* fref_clk1_out OUTPUT | MODE 7 for USB hub clk */ > > Mismatch between constants and comments, which mode should it be? > MODE 0. I see safe mode for 7. Comment should be corrected. >> + >; >> + }; >> }; >> >> &mmc1 { >> @@ -164,6 +232,15 @@ >> status = "disabled"; >> }; >> >> +&usbhshost { >> + port2-mode = "ehci-hsic"; >> + port3-mode = "ehci-hsic"; >> +}; >> + >> +&usbhsehci { >> + phys = <0 &hsusb2_phy &hsusb3_phy>; >> +}; >> + >> &mcspi1 { >> >> }; >> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi >> index 1e84db8..67d6e1f 100644 >> --- a/arch/arm/boot/dts/omap5.dtsi >> +++ b/arch/arm/boot/dts/omap5.dtsi >> @@ -666,5 +666,35 @@ >> ctrl-module = <&omap_control_usb>; >> }; >> }; >> + >> + usbhstll: usbhstll@4a062000 { >> + compatible = "ti,usbhs-tll"; >> + reg = <0x4a062000 0x1000>; >> + interrupts = <0 78 IRQ_TYPE_LEVEL_HIGH>; > > I guess that here you can replace '0' with GIC_SPI. > Thanks. Will replace here and below. Regards, Sricharan
Hi Sricharan, Thanks for sending this, but some parts are outdated. See below. On 06/05/2013 09:46 AM, Sricharan R wrote: > From: Roger Quadros <rogerq@ti.com> > > Provide the RESET regulators for the USB PHYs, the USB Host > port modes and the PHY devices. > > Also provide pin multiplexer information for the USB host > pins. > > Cc: Roger Quadros <rogerq@ti.com> > Signed-off-by: Roger Quadros <rogerq@ti.com> > [Sricharan R <r.sricharan@ti.com>: Replaced constants with preprocessor macros] > Signed-off-by: Sricharan R <r.sricharan@ti.com> > --- > arch/arm/boot/dts/omap5-uevm.dts | 77 ++++++++++++++++++++++++++++++++++++++ > arch/arm/boot/dts/omap5.dtsi | 30 +++++++++++++++ > 2 files changed, 107 insertions(+) > > diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts > index 843a001..cf862df 100644 > --- a/arch/arm/boot/dts/omap5-uevm.dts > +++ b/arch/arm/boot/dts/omap5-uevm.dts > @@ -25,6 +25,47 @@ > regulator-max-microvolt = <3000000>; > }; > > + /* HS USB Port 2 RESET */ > + hsusb2_reset: hsusb2_reset_reg { > + compatible = "regulator-fixed"; > + regulator-name = "hsusb2_reset"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpio = <&gpio3 16 GPIO_ACTIVE_HIGH>; /* gpio3_80 HUB_NRESET */ > + startup-delay-us = <70000>; > + enable-active-high; > + }; > + > + /* HS USB Host PHY on PORT 2 */ > + hsusb2_phy: hsusb2_phy { > + compatible = "usb-nop-xceiv"; > + reset-supply = <&hsusb2_reset>; > + }; > + > + /* HS USB Port 3 RESET */ > + hsusb3_reset: hsusb3_reset_reg { > + compatible = "regulator-fixed"; > + regulator-name = "hsusb3_reset"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpio = <&gpio3 15 GPIO_ACTIVE_HIGH>; /* gpio3_79 ETH_NRESET */ > + startup-delay-us = <70000>; > + enable-active-high; > + }; > + > + /* HS USB Host PHY on PORT 3 */ > + hsusb3_phy: hsusb3_phy { > + compatible = "usb-nop-xceiv"; > + reset-supply = <&hsusb3_reset>; > + }; > + > + /* hsusb2_phy is clocked by FREF_CLK1 i.e. auxclk1 */ > + clock_alias { > + clock-name = "auxclk1_ck"; > + clock-alias = "main_clk"; > + device = <&hsusb2_phy>; > + clock-frequency = <19200000>; /* 19.2 MHz */ > + }; clock_alias node is not required. Instead we need to rely on proper clock binding and provide reference to the clock phandle in the PHY node. as an example please see here https://lkml.org/lkml/2013/4/19/124 However this clock binding technique is still under discussion/transition. https://patchwork.kernel.org/patch/2541331/ So, I suggest you leave the auxclk1 part out for now and we can get it in later once the clock binding stuff is sorted out. The side effect of this is that PORT 2 (i.e. USB Hub) will not work. USB Ethernet should still work IMO. cheers, -roger
Hi Roger, On Wednesday 05 June 2013 04:02 PM, Roger Quadros wrote: > Hi Sricharan, > > Thanks for sending this, but some parts are outdated. See below. > > On 06/05/2013 09:46 AM, Sricharan R wrote: >> From: Roger Quadros <rogerq@ti.com> >> >> Provide the RESET regulators for the USB PHYs, the USB Host >> port modes and the PHY devices. >> >> Also provide pin multiplexer information for the USB host >> pins. >> >> Cc: Roger Quadros <rogerq@ti.com> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> [Sricharan R <r.sricharan@ti.com>: Replaced constants with preprocessor macros] >> Signed-off-by: Sricharan R <r.sricharan@ti.com> >> --- >> arch/arm/boot/dts/omap5-uevm.dts | 77 ++++++++++++++++++++++++++++++++++++++ >> arch/arm/boot/dts/omap5.dtsi | 30 +++++++++++++++ >> 2 files changed, 107 insertions(+) >> >> diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts >> index 843a001..cf862df 100644 >> --- a/arch/arm/boot/dts/omap5-uevm.dts >> +++ b/arch/arm/boot/dts/omap5-uevm.dts >> @@ -25,6 +25,47 @@ >> regulator-max-microvolt = <3000000>; >> }; >> >> + /* HS USB Port 2 RESET */ >> + hsusb2_reset: hsusb2_reset_reg { >> + compatible = "regulator-fixed"; >> + regulator-name = "hsusb2_reset"; >> + regulator-min-microvolt = <3300000>; >> + regulator-max-microvolt = <3300000>; >> + gpio = <&gpio3 16 GPIO_ACTIVE_HIGH>; /* gpio3_80 HUB_NRESET */ >> + startup-delay-us = <70000>; >> + enable-active-high; >> + }; >> + >> + /* HS USB Host PHY on PORT 2 */ >> + hsusb2_phy: hsusb2_phy { >> + compatible = "usb-nop-xceiv"; >> + reset-supply = <&hsusb2_reset>; >> + }; >> + >> + /* HS USB Port 3 RESET */ >> + hsusb3_reset: hsusb3_reset_reg { >> + compatible = "regulator-fixed"; >> + regulator-name = "hsusb3_reset"; >> + regulator-min-microvolt = <3300000>; >> + regulator-max-microvolt = <3300000>; >> + gpio = <&gpio3 15 GPIO_ACTIVE_HIGH>; /* gpio3_79 ETH_NRESET */ >> + startup-delay-us = <70000>; >> + enable-active-high; >> + }; >> + >> + /* HS USB Host PHY on PORT 3 */ >> + hsusb3_phy: hsusb3_phy { >> + compatible = "usb-nop-xceiv"; >> + reset-supply = <&hsusb3_reset>; >> + }; >> + >> + /* hsusb2_phy is clocked by FREF_CLK1 i.e. auxclk1 */ >> + clock_alias { >> + clock-name = "auxclk1_ck"; >> + clock-alias = "main_clk"; >> + device = <&hsusb2_phy>; >> + clock-frequency = <19200000>; /* 19.2 MHz */ >> + }; > clock_alias node is not required. Instead we need to rely on proper clock binding > and provide reference to the clock phandle in the PHY node. > > as an example please see here > https://lkml.org/lkml/2013/4/19/124 > > However this clock binding technique is still under discussion/transition. > https://patchwork.kernel.org/patch/2541331/ > > So, I suggest you leave the auxclk1 part out for now and we can get it in later once > the clock binding stuff is sorted out. The side effect of this is that PORT 2 (i.e. USB Hub) > will not work. USB Ethernet should still work IMO. Ok will drop that part then. Thanks, Sricharan
On 06/05/2013 11:12 AM, Sricharan R wrote: > Hi, > On Wednesday 05 June 2013 01:29 PM, Florian Vaussard wrote: >> Hello, >> >> Some very minor comments. >> >> On 06/05/2013 08:46 AM, Sricharan R wrote: >>> From: Roger Quadros <rogerq@ti.com> >>> >>> Provide the RESET regulators for the USB PHYs, the USB Host >>> port modes and the PHY devices. >>> >>> Also provide pin multiplexer information for the USB host >>> pins. >>> >>> Cc: Roger Quadros <rogerq@ti.com> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> [Sricharan R <r.sricharan@ti.com>: Replaced constants with preprocessor macros] >>> Signed-off-by: Sricharan R <r.sricharan@ti.com> >>> --- >>> arch/arm/boot/dts/omap5-uevm.dts | 77 ++++++++++++++++++++++++++++++++++++++ >>> arch/arm/boot/dts/omap5.dtsi | 30 +++++++++++++++ >>> 2 files changed, 107 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts >>> index 843a001..cf862df 100644 >>> --- a/arch/arm/boot/dts/omap5-uevm.dts >>> +++ b/arch/arm/boot/dts/omap5-uevm.dts >>> @@ -25,6 +25,47 @@ >>> regulator-max-microvolt = <3000000>; >>> }; >>> >>> + /* HS USB Port 2 RESET */ >>> + hsusb2_reset: hsusb2_reset_reg { >>> + compatible = "regulator-fixed"; >>> + regulator-name = "hsusb2_reset"; >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + gpio = <&gpio3 16 GPIO_ACTIVE_HIGH>; /* gpio3_80 HUB_NRESET */ >>> + startup-delay-us = <70000>; >>> + enable-active-high; >>> + }; >>> + >>> + /* HS USB Host PHY on PORT 2 */ >>> + hsusb2_phy: hsusb2_phy { >>> + compatible = "usb-nop-xceiv"; >>> + reset-supply = <&hsusb2_reset>; >>> + }; >>> + >>> + /* HS USB Port 3 RESET */ >>> + hsusb3_reset: hsusb3_reset_reg { >>> + compatible = "regulator-fixed"; >>> + regulator-name = "hsusb3_reset"; >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + gpio = <&gpio3 15 GPIO_ACTIVE_HIGH>; /* gpio3_79 ETH_NRESET */ >>> + startup-delay-us = <70000>; >>> + enable-active-high; >>> + }; >>> + >>> + /* HS USB Host PHY on PORT 3 */ >>> + hsusb3_phy: hsusb3_phy { >>> + compatible = "usb-nop-xceiv"; >>> + reset-supply = <&hsusb3_reset>; >>> + }; >>> + >>> + /* hsusb2_phy is clocked by FREF_CLK1 i.e. auxclk1 */ >>> + clock_alias { >>> + clock-name = "auxclk1_ck"; >>> + clock-alias = "main_clk"; >>> + device = <&hsusb2_phy>; >>> + clock-frequency = <19200000>; /* 19.2 MHz */ >>> + }; >>> }; >>> >>> &omap5_pmx_core { >>> @@ -35,6 +76,7 @@ >>> &dmic_pins >>> &mcbsp1_pins >>> &mcbsp2_pins >>> + &usbhost_pins >>> >; >>> >>> twl6040_pins: pinmux_twl6040_pins { >>> @@ -120,6 +162,32 @@ >>> 0x16c (PIN_INPUT | MUX_MODE1) /* mcspi2_cs */ >>> >; >>> }; >>> + >>> + usbhost_pins: pinmux_usbhost_pins { >>> + pinctrl-single,pins = < >>> + 0x84 (PIN_INPUT | MUX_MODE0) /* usbb2_hsic_strobe INPUT | MODE 0 */ >>> + 0x86 (PIN_INPUT | MUX_MODE0) /* usbb2_hsic_data INPUT | MODE 0 */ >> >> Comments are redundant with the constants, so maybe you can leave this part out. >> Same for a few others below. >> > Ok, I agree here for attributes like pulls, i/o etc, comments are now not required. > But comment is still good to describe just the mux mode functionality ? > One module will use different pins, like data, clk, gpios, etc. Just MUX_MODEX does not > complete it.. Yes, the 'usbb2_hsic_strobe' part must stay, only 'INPUT | MODE 0' should be removed from comments. >>> + >>> + 0x19e (PIN_INPUT | MUX_MODE0) /* usbb3_hsic_strobe INPUT | MODE 0 */ >>> + 0x1a0 (PIN_INPUT | MUX_MODE0) /* usbb3_hsic_data INPUT | MODE 0 */ >>> + >>> + 0x70 (PIN_OUTPUT | MUX_MODE6) /* gpio3_80 OUTPUT | MODE 6 HUB_NRESET */ >>> + 0x6e (PIN_OUTPUT | MUX_MODE6) /* gpio3_79 OUTPUT | MODE 6 ETH_NRESET */ >>> + >; >>> + }; >>> +}; >>> + >>> +&omap5_pmx_wkup { >>> + pinctrl-names = "default"; >>> + pinctrl-0 = < >>> + &usbhost_wkup_pins >>> + >; >>> + >>> + usbhost_wkup_pins: pinmux_usbhost_wkup_pins { >>> + pinctrl-single,pins = < >>> + 0x1A (PIN_OUTPUT | MUX_MODE0) /* fref_clk1_out OUTPUT | MODE 7 for USB hub clk */ >> >> Mismatch between constants and comments, which mode should it be? >> > MODE 0. I see safe mode for 7. Comment should be corrected. s/corrected/removed/. This will avoid this kind of inconsistency. Regards, Florian
On 12:16-20130605, Sricharan R wrote: > From: Roger Quadros <rogerq@ti.com> [...] > diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts > index 843a001..cf862df 100644 > --- a/arch/arm/boot/dts/omap5-uevm.dts > +++ b/arch/arm/boot/dts/omap5-uevm.dts <snip> > + /* hsusb2_phy is clocked by FREF_CLK1 i.e. auxclk1 */ > + clock_alias { > + clock-name = "auxclk1_ck"; > + clock-alias = "main_clk"; is'nt this better to use with clock node? > + device = <&hsusb2_phy>; > + clock-frequency = <19200000>; /* 19.2 MHz */ > + }; > }; <snip>
Hi, On Wednesday 05 June 2013 07:27 PM, Nishanth Menon wrote: > On 12:16-20130605, Sricharan R wrote: >> From: Roger Quadros <rogerq@ti.com> > [...] >> diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts >> index 843a001..cf862df 100644 >> --- a/arch/arm/boot/dts/omap5-uevm.dts >> +++ b/arch/arm/boot/dts/omap5-uevm.dts > <snip> >> + /* hsusb2_phy is clocked by FREF_CLK1 i.e. auxclk1 */ >> + clock_alias { >> + clock-name = "auxclk1_ck"; >> + clock-alias = "main_clk"; > is'nt this better to use with clock node? Roger was suggesting to remove this for now. Posted V2 following that. Are you suggesting to go with your clock-binding patch ? Regards, Sricharan
On 23:21-20130606, Sricharan R wrote: > Hi, > On Wednesday 05 June 2013 07:27 PM, Nishanth Menon wrote: > > On 12:16-20130605, Sricharan R wrote: > >> From: Roger Quadros <rogerq@ti.com> > > [...] > >> diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts > >> index 843a001..cf862df 100644 > >> --- a/arch/arm/boot/dts/omap5-uevm.dts > >> +++ b/arch/arm/boot/dts/omap5-uevm.dts > > <snip> > >> + /* hsusb2_phy is clocked by FREF_CLK1 i.e. auxclk1 */ > >> + clock_alias { > >> + clock-name = "auxclk1_ck"; > >> + clock-alias = "main_clk"; > > is'nt this better to use with clock node? > Roger was suggesting to remove this for now. Posted > V2 following that. > Are you suggesting to go with your clock-binding patch ? you cannot. Mike has already rejected that approach and I have dropped that series and wont be following up. See discussion https://patchwork.kernel.org/patch/2541331/ instead the new approach being discussed https://patchwork.kernel.org/patch/2657351/ and related discussions is the way to go. That said, it practically blocks introduction of all code that depend on dts nodes :(, no matter how I dont like being blocked and my many series delayed, I believe Mike, Tero, Rajendra are doing the right architecture and we should wait for it's upstream introduction. That is just my 2 cents.
On Friday 07 June 2013 12:16 AM, Nishanth Menon wrote: > On 23:21-20130606, Sricharan R wrote: >> Hi, >> On Wednesday 05 June 2013 07:27 PM, Nishanth Menon wrote: >>> On 12:16-20130605, Sricharan R wrote: >>>> From: Roger Quadros <rogerq@ti.com> >>> [...] >>>> diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts >>>> index 843a001..cf862df 100644 >>>> --- a/arch/arm/boot/dts/omap5-uevm.dts >>>> +++ b/arch/arm/boot/dts/omap5-uevm.dts >>> <snip> >>>> + /* hsusb2_phy is clocked by FREF_CLK1 i.e. auxclk1 */ >>>> + clock_alias { >>>> + clock-name = "auxclk1_ck"; >>>> + clock-alias = "main_clk"; >>> is'nt this better to use with clock node? >> Roger was suggesting to remove this for now. Postedhen >> V2 following that. >> Are you suggesting to go with your clock-binding patch ? > you cannot. Mike has already rejected that approach and I have dropped > that series and wont be following up. > See discussion https://patchwork.kernel.org/patch/2541331/ > > instead the new approach being discussed > https://patchwork.kernel.org/patch/2657351/ and related discussions is > the way to go. > > That said, it practically blocks introduction of all code that depend on > dts nodes :(, no matter how I dont like being blocked and my many series > delayed, I believe Mike, Tero, Rajendra are doing the right architecture > and we should wait for it's upstream introduction. > > That is just my 2 cents. > Thanks for the details. So dropping the alias for now, should be fine. Regards, Sricharan
diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts index 843a001..cf862df 100644 --- a/arch/arm/boot/dts/omap5-uevm.dts +++ b/arch/arm/boot/dts/omap5-uevm.dts @@ -25,6 +25,47 @@ regulator-max-microvolt = <3000000>; }; + /* HS USB Port 2 RESET */ + hsusb2_reset: hsusb2_reset_reg { + compatible = "regulator-fixed"; + regulator-name = "hsusb2_reset"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + gpio = <&gpio3 16 GPIO_ACTIVE_HIGH>; /* gpio3_80 HUB_NRESET */ + startup-delay-us = <70000>; + enable-active-high; + }; + + /* HS USB Host PHY on PORT 2 */ + hsusb2_phy: hsusb2_phy { + compatible = "usb-nop-xceiv"; + reset-supply = <&hsusb2_reset>; + }; + + /* HS USB Port 3 RESET */ + hsusb3_reset: hsusb3_reset_reg { + compatible = "regulator-fixed"; + regulator-name = "hsusb3_reset"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + gpio = <&gpio3 15 GPIO_ACTIVE_HIGH>; /* gpio3_79 ETH_NRESET */ + startup-delay-us = <70000>; + enable-active-high; + }; + + /* HS USB Host PHY on PORT 3 */ + hsusb3_phy: hsusb3_phy { + compatible = "usb-nop-xceiv"; + reset-supply = <&hsusb3_reset>; + }; + + /* hsusb2_phy is clocked by FREF_CLK1 i.e. auxclk1 */ + clock_alias { + clock-name = "auxclk1_ck"; + clock-alias = "main_clk"; + device = <&hsusb2_phy>; + clock-frequency = <19200000>; /* 19.2 MHz */ + }; }; &omap5_pmx_core { @@ -35,6 +76,7 @@ &dmic_pins &mcbsp1_pins &mcbsp2_pins + &usbhost_pins >; twl6040_pins: pinmux_twl6040_pins { @@ -120,6 +162,32 @@ 0x16c (PIN_INPUT | MUX_MODE1) /* mcspi2_cs */ >; }; + + usbhost_pins: pinmux_usbhost_pins { + pinctrl-single,pins = < + 0x84 (PIN_INPUT | MUX_MODE0) /* usbb2_hsic_strobe INPUT | MODE 0 */ + 0x86 (PIN_INPUT | MUX_MODE0) /* usbb2_hsic_data INPUT | MODE 0 */ + + 0x19e (PIN_INPUT | MUX_MODE0) /* usbb3_hsic_strobe INPUT | MODE 0 */ + 0x1a0 (PIN_INPUT | MUX_MODE0) /* usbb3_hsic_data INPUT | MODE 0 */ + + 0x70 (PIN_OUTPUT | MUX_MODE6) /* gpio3_80 OUTPUT | MODE 6 HUB_NRESET */ + 0x6e (PIN_OUTPUT | MUX_MODE6) /* gpio3_79 OUTPUT | MODE 6 ETH_NRESET */ + >; + }; +}; + +&omap5_pmx_wkup { + pinctrl-names = "default"; + pinctrl-0 = < + &usbhost_wkup_pins + >; + + usbhost_wkup_pins: pinmux_usbhost_wkup_pins { + pinctrl-single,pins = < + 0x1A (PIN_OUTPUT | MUX_MODE0) /* fref_clk1_out OUTPUT | MODE 7 for USB hub clk */ + >; + }; }; &mmc1 { @@ -164,6 +232,15 @@ status = "disabled"; }; +&usbhshost { + port2-mode = "ehci-hsic"; + port3-mode = "ehci-hsic"; +}; + +&usbhsehci { + phys = <0 &hsusb2_phy &hsusb3_phy>; +}; + &mcspi1 { }; diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi index 1e84db8..67d6e1f 100644 --- a/arch/arm/boot/dts/omap5.dtsi +++ b/arch/arm/boot/dts/omap5.dtsi @@ -666,5 +666,35 @@ ctrl-module = <&omap_control_usb>; }; }; + + usbhstll: usbhstll@4a062000 { + compatible = "ti,usbhs-tll"; + reg = <0x4a062000 0x1000>; + interrupts = <0 78 IRQ_TYPE_LEVEL_HIGH>; + ti,hwmods = "usb_tll_hs"; + }; + + usbhshost: usbhshost@4a064000 { + compatible = "ti,usbhs-host"; + reg = <0x4a064000 0x800>; + ti,hwmods = "usb_host_hs"; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + usbhsohci: ohci@4a064800 { + compatible = "ti,ohci-omap3", "usb-ohci"; + reg = <0x4a064800 0x400>; + interrupt-parent = <&gic>; + interrupts = <0 76 IRQ_TYPE_LEVEL_HIGH>; + }; + + usbhsehci: ehci@4a064c00 { + compatible = "ti,ehci-omap", "usb-ehci"; + reg = <0x4a064c00 0x400>; + interrupt-parent = <&gic>; + interrupts = <0 77 IRQ_TYPE_LEVEL_HIGH>; + }; + }; }; };