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@xxxxxx>
>>
>> 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@xxxxxx>
>> Signed-off-by: Roger Quadros <rogerq@xxxxxx>
>> [Sricharan R <r.sricharan@xxxxxx>: Replaced constants with preprocessor macros]
>> Signed-off-by: Sricharan R <r.sricharan@xxxxxx>
>> ---
>> 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
--
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
