diff mbox

[1/2] ARM: dts: sun9i: cubieboard4: Enable USB support

Message ID 1431017037-28331-2-git-send-email-wens@csie.org (mailing list archive)
State New, archived
Headers show

Commit Message

Chen-Yu Tsai May 7, 2015, 4:43 p.m. UTC
The Cubieboard4 has 4 USB ports. 3 of them are connected to a GL850G
USB hub chip on usb1. The fourth one, the lower port of 2 ports next
to the power barrel, is directly connected to usb3.

2 power enable GPIOs are used between the 2 port groups, 1 for each.
This raises the possibility of having no power for hub-connected port
next to the power barrel, if usb3 is not enabled.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun9i-a80-cubieboard4.dts | 60 +++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

Comments

Maxime Ripard May 8, 2015, 7:46 a.m. UTC | #1
Hi,

On Fri, May 08, 2015 at 12:43:56AM +0800, Chen-Yu Tsai wrote:
> The Cubieboard4 has 4 USB ports. 3 of them are connected to a GL850G
> USB hub chip on usb1. The fourth one, the lower port of 2 ports next
> to the power barrel, is directly connected to usb3.
> 
> 2 power enable GPIOs are used between the 2 port groups, 1 for each.
> This raises the possibility of having no power for hub-connected port
> next to the power barrel, if usb3 is not enabled.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  arch/arm/boot/dts/sun9i-a80-cubieboard4.dts | 60 +++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts b/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts
> index 6484dcf69873..42ddc046213c 100644
> --- a/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts
> +++ b/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts
> @@ -62,6 +62,30 @@
>  		stdout-path = "serial0:115200n8";
>  	};
>  
> +	reg_usb3_vbus: usb3-vbus {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&usb3_vbus_pin_cubieboard4>;
> +		regulator-name = "usb3-vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		enable-active-high;
> +		gpio = <&pio 7 15 GPIO_ACTIVE_HIGH>; /* PH15 */
> +	};
> +};
> +
> +&ehci0 {
> +	status = "okay";
> +};
> +
> +&ehci2 {
> +	status = "okay";
> +};
> +
> +/* usb1 is connected to a GL850G USB hub chip, so no need to enable OHCI */

You're mentionning usb1, but I don't see it enabled anywhere, is that
a typo?

> +
> +&ohci2 {
> +	status = "okay";
>  };
>  
>  &pio {
> @@ -71,6 +95,20 @@
>  		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>  		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>  	};
> +
> +	usb1_vbus_pin_cubieboard4: usb1_vbus_pin@1 {
> +		allwinner,pins = "PH14";
> +		allwinner,function = "gpio_out";
> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
> +	};
> +
> +	usb3_vbus_pin_cubieboard4: usb3_vbus_pin@1 {
> +		allwinner,pins = "PH15";
> +		allwinner,function = "gpio_out";
> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
> +	};
>  };
>  
>  &mmc0 {
> @@ -92,8 +130,30 @@
>  	status = "okay";
>  };
>  
> +&reg_usb1_vbus {
> +	pinctrl-0 = <&usb1_vbus_pin_cubieboard4>;
> +	gpio = <&pio 7 14 GPIO_ACTIVE_HIGH>; /* PH14 */
> +	status = "okay";
> +};
> +
>  &uart0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&uart0_pins_a>;
>  	status = "okay";
>  };
> +
> +&usbphy1 {
> +	phy-supply = <&reg_usb1_vbus>;
> +	status = "okay";
> +};
> +
> +/*
> + * Unfortunately reg_usb1_vbus also powers one of the ports from usb3's hub.
> + * One should always make sure both regulators are enabled and working for
> + * all USB ports to have power.
> + */

Can't we just provide the two regulators, and enable both of them so
that we know that we always have the needed regulators enabled,
disregarding which USB port is used?

Thanks,
Maxime
Chen-Yu Tsai May 8, 2015, 8:01 a.m. UTC | #2
On Fri, May 8, 2015 at 3:46 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Fri, May 08, 2015 at 12:43:56AM +0800, Chen-Yu Tsai wrote:
>> The Cubieboard4 has 4 USB ports. 3 of them are connected to a GL850G
>> USB hub chip on usb1. The fourth one, the lower port of 2 ports next
>> to the power barrel, is directly connected to usb3.
>>
>> 2 power enable GPIOs are used between the 2 port groups, 1 for each.
>> This raises the possibility of having no power for hub-connected port
>> next to the power barrel, if usb3 is not enabled.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  arch/arm/boot/dts/sun9i-a80-cubieboard4.dts | 60 +++++++++++++++++++++++++++++
>>  1 file changed, 60 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts b/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts
>> index 6484dcf69873..42ddc046213c 100644
>> --- a/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts
>> +++ b/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts
>> @@ -62,6 +62,30 @@
>>               stdout-path = "serial0:115200n8";
>>       };
>>
>> +     reg_usb3_vbus: usb3-vbus {
>> +             compatible = "regulator-fixed";
>> +             pinctrl-names = "default";
>> +             pinctrl-0 = <&usb3_vbus_pin_cubieboard4>;
>> +             regulator-name = "usb3-vbus";
>> +             regulator-min-microvolt = <5000000>;
>> +             regulator-max-microvolt = <5000000>;
>> +             enable-active-high;
>> +             gpio = <&pio 7 15 GPIO_ACTIVE_HIGH>; /* PH15 */
>> +     };
>> +};
>> +
>> +&ehci0 {
>> +     status = "okay";
>> +};
>> +
>> +&ehci2 {
>> +     status = "okay";
>> +};
>> +
>> +/* usb1 is connected to a GL850G USB hub chip, so no need to enable OHCI */
>
> You're mentionning usb1, but I don't see it enabled anywhere, is that
> a typo?

usb1 (or usbphy1) == ehci/ohci0. usb0 is otg.

This numbering matches the fex files, and (mostly) matches the specs.

>> +
>> +&ohci2 {
>> +     status = "okay";
>>  };
>>
>>  &pio {
>> @@ -71,6 +95,20 @@
>>               allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>>               allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>>       };
>> +
>> +     usb1_vbus_pin_cubieboard4: usb1_vbus_pin@1 {
>> +             allwinner,pins = "PH14";
>> +             allwinner,function = "gpio_out";
>> +             allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> +             allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>> +     };
>> +
>> +     usb3_vbus_pin_cubieboard4: usb3_vbus_pin@1 {
>> +             allwinner,pins = "PH15";
>> +             allwinner,function = "gpio_out";
>> +             allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> +             allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>> +     };
>>  };
>>
>>  &mmc0 {
>> @@ -92,8 +130,30 @@
>>       status = "okay";
>>  };
>>
>> +&reg_usb1_vbus {
>> +     pinctrl-0 = <&usb1_vbus_pin_cubieboard4>;
>> +     gpio = <&pio 7 14 GPIO_ACTIVE_HIGH>; /* PH14 */
>> +     status = "okay";
>> +};
>> +
>>  &uart0 {
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&uart0_pins_a>;
>>       status = "okay";
>>  };
>> +
>> +&usbphy1 {
>> +     phy-supply = <&reg_usb1_vbus>;
>> +     status = "okay";
>> +};
>> +
>> +/*
>> + * Unfortunately reg_usb1_vbus also powers one of the ports from usb3's hub.
>> + * One should always make sure both regulators are enabled and working for
>> + * all USB ports to have power.
>> + */
>
> Can't we just provide the two regulators, and enable both of them so
> that we know that we always have the needed regulators enabled,
> disregarding which USB port is used?

Would setting "always-on" for both regulators work for you?
Or maybe just the one that's used by both USB hosts?

ChenYu
Maxime Ripard May 8, 2015, 11:40 a.m. UTC | #3
On Fri, May 08, 2015 at 04:01:14PM +0800, Chen-Yu Tsai wrote:
> >> +&ehci0 {
> >> +     status = "okay";
> >> +};
> >> +
> >> +&ehci2 {
> >> +     status = "okay";
> >> +};
> >> +
> >> +/* usb1 is connected to a GL850G USB hub chip, so no need to enable OHCI */
> >
> > You're mentionning usb1, but I don't see it enabled anywhere, is that
> > a typo?
> 
> usb1 (or usbphy1) == ehci/ohci0. usb0 is otg.
> 
> This numbering matches the fex files, and (mostly) matches the
> specs.

Ok, having this comment between ehci2 and ohci2 is confusing then :)

It's not the first board that has a hub, we usually don't really care
as it's kind of obvious. Maybe we can just remove it?

> >>  &mmc0 {
> >> @@ -92,8 +130,30 @@
> >>       status = "okay";
> >>  };
> >>
> >> +&reg_usb1_vbus {
> >> +     pinctrl-0 = <&usb1_vbus_pin_cubieboard4>;
> >> +     gpio = <&pio 7 14 GPIO_ACTIVE_HIGH>; /* PH14 */
> >> +     status = "okay";
> >> +};
> >> +
> >>  &uart0 {
> >>       pinctrl-names = "default";
> >>       pinctrl-0 = <&uart0_pins_a>;
> >>       status = "okay";
> >>  };
> >> +
> >> +&usbphy1 {
> >> +     phy-supply = <&reg_usb1_vbus>;
> >> +     status = "okay";
> >> +};
> >> +
> >> +/*
> >> + * Unfortunately reg_usb1_vbus also powers one of the ports from usb3's hub.
> >> + * One should always make sure both regulators are enabled and working for
> >> + * all USB ports to have power.
> >> + */
> >
> > Can't we just provide the two regulators, and enable both of them so
> > that we know that we always have the needed regulators enabled,
> > disregarding which USB port is used?
> 
> Would setting "always-on" for both regulators work for you?
> Or maybe just the one that's used by both USB hosts?

I was more thinking of giving to the phy an additional regulator, so
that it would enable both the regulators needed to power up all ports.

Maxime
Chen-Yu Tsai May 11, 2015, 8:26 a.m. UTC | #4
On Fri, May 8, 2015 at 7:40 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Fri, May 08, 2015 at 04:01:14PM +0800, Chen-Yu Tsai wrote:
>> >> +&ehci0 {
>> >> +     status = "okay";
>> >> +};
>> >> +
>> >> +&ehci2 {
>> >> +     status = "okay";
>> >> +};
>> >> +
>> >> +/* usb1 is connected to a GL850G USB hub chip, so no need to enable OHCI */
>> >
>> > You're mentionning usb1, but I don't see it enabled anywhere, is that
>> > a typo?
>>
>> usb1 (or usbphy1) == ehci/ohci0. usb0 is otg.
>>
>> This numbering matches the fex files, and (mostly) matches the
>> specs.
>
> Ok, having this comment between ehci2 and ohci2 is confusing then :)
>
> It's not the first board that has a hub, we usually don't really care
> as it's kind of obvious. Maybe we can just remove it?

Why not. :)

>> >>  &mmc0 {
>> >> @@ -92,8 +130,30 @@
>> >>       status = "okay";
>> >>  };
>> >>
>> >> +&reg_usb1_vbus {
>> >> +     pinctrl-0 = <&usb1_vbus_pin_cubieboard4>;
>> >> +     gpio = <&pio 7 14 GPIO_ACTIVE_HIGH>; /* PH14 */
>> >> +     status = "okay";
>> >> +};
>> >> +
>> >>  &uart0 {
>> >>       pinctrl-names = "default";
>> >>       pinctrl-0 = <&uart0_pins_a>;
>> >>       status = "okay";
>> >>  };
>> >> +
>> >> +&usbphy1 {
>> >> +     phy-supply = <&reg_usb1_vbus>;
>> >> +     status = "okay";
>> >> +};
>> >> +
>> >> +/*
>> >> + * Unfortunately reg_usb1_vbus also powers one of the ports from usb3's hub.
>> >> + * One should always make sure both regulators are enabled and working for
>> >> + * all USB ports to have power.
>> >> + */
>> >
>> > Can't we just provide the two regulators, and enable both of them so
>> > that we know that we always have the needed regulators enabled,
>> > disregarding which USB port is used?
>>
>> Would setting "always-on" for both regulators work for you?
>> Or maybe just the one that's used by both USB hosts?
>
> I was more thinking of giving to the phy an additional regulator, so
> that it would enable both the regulators needed to power up all ports.

That would require adding back all the regulator-related code I
removed from the phy driver before it was merged. (sigh) It's not
like the regulator bindings takes a list.

I see this as more of a hardware design flaw, and we should label
it as such. And it might still work for self-powered devices even
if VBUS is off. The USB hub chip is always on.


ChenYu
Maxime Ripard May 12, 2015, 2:57 p.m. UTC | #5
On Mon, May 11, 2015 at 04:26:58PM +0800, Chen-Yu Tsai wrote:
> >> >> +&usbphy1 {
> >> >> +     phy-supply = <&reg_usb1_vbus>;
> >> >> +     status = "okay";
> >> >> +};
> >> >> +
> >> >> +/*
> >> >> + * Unfortunately reg_usb1_vbus also powers one of the ports from usb3's hub.
> >> >> + * One should always make sure both regulators are enabled and working for
> >> >> + * all USB ports to have power.
> >> >> + */
> >> >
> >> > Can't we just provide the two regulators, and enable both of them so
> >> > that we know that we always have the needed regulators enabled,
> >> > disregarding which USB port is used?
> >>
> >> Would setting "always-on" for both regulators work for you?
> >> Or maybe just the one that's used by both USB hosts?
> >
> > I was more thinking of giving to the phy an additional regulator, so
> > that it would enable both the regulators needed to power up all ports.
> 
> That would require adding back all the regulator-related code I
> removed from the phy driver before it was merged. (sigh) It's not
> like the regulator bindings takes a list.

Yeah, but maybe we can just add an optional device-supply property or
something like that to power up the devices connected on the bus.

> I see this as more of a hardware design flaw, and we should label
> it as such.

This can be seen as one, and we can debate it for some time I guess,
but if the hardware guys were not making crazy stuff like that, we
would run out of work pretty quickly :)

What we really need to do is find a proper and reliable way to handle
this case. Whether we declare it as a flaw or not is a separate
debate.

> And it might still work for self-powered devices even if VBUS is
> off. The USB hub chip is always on.

That still leaves a significant amount of devices out and non
functional, especially very standard devices like USB keys, keyboards
or headsets that you would expect to just work.

Maxime
Chen-Yu Tsai May 12, 2015, 3:49 p.m. UTC | #6
On Tue, May 12, 2015 at 10:57 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Mon, May 11, 2015 at 04:26:58PM +0800, Chen-Yu Tsai wrote:
>> >> >> +&usbphy1 {
>> >> >> +     phy-supply = <&reg_usb1_vbus>;
>> >> >> +     status = "okay";
>> >> >> +};
>> >> >> +
>> >> >> +/*
>> >> >> + * Unfortunately reg_usb1_vbus also powers one of the ports from usb3's hub.
>> >> >> + * One should always make sure both regulators are enabled and working for
>> >> >> + * all USB ports to have power.
>> >> >> + */
>> >> >
>> >> > Can't we just provide the two regulators, and enable both of them so
>> >> > that we know that we always have the needed regulators enabled,
>> >> > disregarding which USB port is used?
>> >>
>> >> Would setting "always-on" for both regulators work for you?
>> >> Or maybe just the one that's used by both USB hosts?
>> >
>> > I was more thinking of giving to the phy an additional regulator, so
>> > that it would enable both the regulators needed to power up all ports.
>>
>> That would require adding back all the regulator-related code I
>> removed from the phy driver before it was merged. (sigh) It's not
>> like the regulator bindings takes a list.
>
> Yeah, but maybe we can just add an optional device-supply property or
> something like that to power up the devices connected on the bus.

(CC-ed Kishon)
Do we think this generic enough to go into the generic phy core?

>> I see this as more of a hardware design flaw, and we should label
>> it as such.
>
> This can be seen as one, and we can debate it for some time I guess,
> but if the hardware guys were not making crazy stuff like that, we
> would run out of work pretty quickly :)

Ah yes, but the users would be happier. :)

> What we really need to do is find a proper and reliable way to handle
> this case. Whether we declare it as a flaw or not is a separate
> debate.
>
>> And it might still work for self-powered devices even if VBUS is
>> off. The USB hub chip is always on.
>
> That still leaves a significant amount of devices out and non
> functional, especially very standard devices like USB keys, keyboards
> or headsets that you would expect to just work.

I agree. So the question is where should this go in.

ChenYu
diff mbox

Patch

diff --git a/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts b/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts
index 6484dcf69873..42ddc046213c 100644
--- a/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts
+++ b/arch/arm/boot/dts/sun9i-a80-cubieboard4.dts
@@ -62,6 +62,30 @@ 
 		stdout-path = "serial0:115200n8";
 	};
 
+	reg_usb3_vbus: usb3-vbus {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb3_vbus_pin_cubieboard4>;
+		regulator-name = "usb3-vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		gpio = <&pio 7 15 GPIO_ACTIVE_HIGH>; /* PH15 */
+	};
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&ehci2 {
+	status = "okay";
+};
+
+/* usb1 is connected to a GL850G USB hub chip, so no need to enable OHCI */
+
+&ohci2 {
+	status = "okay";
 };
 
 &pio {
@@ -71,6 +95,20 @@ 
 		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
 		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
 	};
+
+	usb1_vbus_pin_cubieboard4: usb1_vbus_pin@1 {
+		allwinner,pins = "PH14";
+		allwinner,function = "gpio_out";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+	};
+
+	usb3_vbus_pin_cubieboard4: usb3_vbus_pin@1 {
+		allwinner,pins = "PH15";
+		allwinner,function = "gpio_out";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+	};
 };
 
 &mmc0 {
@@ -92,8 +130,30 @@ 
 	status = "okay";
 };
 
+&reg_usb1_vbus {
+	pinctrl-0 = <&usb1_vbus_pin_cubieboard4>;
+	gpio = <&pio 7 14 GPIO_ACTIVE_HIGH>; /* PH14 */
+	status = "okay";
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins_a>;
 	status = "okay";
 };
+
+&usbphy1 {
+	phy-supply = <&reg_usb1_vbus>;
+	status = "okay";
+};
+
+/*
+ * Unfortunately reg_usb1_vbus also powers one of the ports from usb3's hub.
+ * One should always make sure both regulators are enabled and working for
+ * all USB ports to have power.
+ */
+
+&usbphy3 {
+	phy-supply = <&reg_usb3_vbus>;
+	status = "okay";
+};