diff mbox series

[1/7] arm64: dts: allwinner: H6/A64: properly connect USB PHY to port 0

Message ID 20210106150525.15403-2-andre.przywara@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: dts: allwinner: Various DT fixes | expand

Commit Message

Andre Przywara Jan. 6, 2021, 3:05 p.m. UTC
In recent Allwinner SoCs the first USB host controller (HCI0) shares
the first PHY with the MUSB controller. Probably to make this sharing
work, we were avoiding to declare this in the DT. This has two
shortcomings:
- U-Boot (which uses the same .dts) cannot use this port in host mode
  without a PHY linked, so we were loosing one USB port there.
- It requires the MUSB driver to be enabled and loaded, although we
  don't actually use it.

To avoid those issues, let's add this PHY link to the A64 and H6 .dtsi
files. After all PHY port 0 *is* connected to HCI0, so we should describe
it as this. Remove the part from the boards which were already doing
that.

This makes it work in U-Boot, also improves compatiblity when no MUSB
driver is loaded (for instance in distribution installers).

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts | 4 ----
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi         | 4 ++++
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi          | 4 ++++
 3 files changed, 8 insertions(+), 4 deletions(-)

Comments

Chen-Yu Tsai Jan. 6, 2021, 3:56 p.m. UTC | #1
Hi,

On Wed, Jan 6, 2021 at 11:05 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> In recent Allwinner SoCs the first USB host controller (HCI0) shares
> the first PHY with the MUSB controller. Probably to make this sharing
> work, we were avoiding to declare this in the DT. This has two
> shortcomings:
> - U-Boot (which uses the same .dts) cannot use this port in host mode
>   without a PHY linked, so we were loosing one USB port there.
> - It requires the MUSB driver to be enabled and loaded, although we
>   don't actually use it.
>
> To avoid those issues, let's add this PHY link to the A64 and H6 .dtsi
> files. After all PHY port 0 *is* connected to HCI0, so we should describe
> it as this. Remove the part from the boards which were already doing
> that.
>
> This makes it work in U-Boot, also improves compatiblity when no MUSB
> driver is loaded (for instance in distribution installers).
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts | 4 ----
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi         | 4 ++++
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi          | 4 ++++

It would be better if the H6 parts were separated out.

Please also add a fixes tag, which helps with backporting.

>  3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> index 896f34fd9fc3..d07cf05549c3 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> @@ -126,8 +126,6 @@
>  };
>
>  &ehci0 {
> -       phys = <&usbphy 0>;
> -       phy-names = "usb";
>         status = "okay";
>  };
>
> @@ -177,8 +175,6 @@
>  };
>
>  &ohci0 {
> -       phys = <&usbphy 0>;
> -       phy-names = "usb";
>         status = "okay";
>  };
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 51cc30e84e26..19e9b8ca8432 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -593,6 +593,8 @@
>                                  <&ccu CLK_USB_OHCI0>;
>                         resets = <&ccu RST_BUS_OHCI0>,
>                                  <&ccu RST_BUS_EHCI0>;
> +                       phys = <&usbphy 0>;
> +                       phy-names = "usb";
>                         status = "disabled";
>                 };
>
> @@ -603,6 +605,8 @@
>                         clocks = <&ccu CLK_BUS_OHCI0>,
>                                  <&ccu CLK_USB_OHCI0>;
>                         resets = <&ccu RST_BUS_OHCI0>;
> +                       phys = <&usbphy 0>;
> +                       phy-names = "usb";
>                         status = "disabled";
>                 };
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index 8a62a9fbe347..f593cfeaecc9 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -680,6 +680,8 @@
>                                  <&ccu CLK_USB_OHCI0>;
>                         resets = <&ccu RST_BUS_OHCI0>,
>                                  <&ccu RST_BUS_EHCI0>;
> +                       phys = <&usb2phy 0>;
> +                       phy-names = "usb";
>                         status = "disabled";
>                 };
>
> @@ -690,6 +692,8 @@
>                         clocks = <&ccu CLK_BUS_OHCI0>,
>                                  <&ccu CLK_USB_OHCI0>;
>                         resets = <&ccu RST_BUS_OHCI0>;
> +                       phys = <&usb2phy 0>;
> +                       phy-names = "usb";
>                         status = "disabled";
>                 };
>
> --
> 2.17.5
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20210106150525.15403-2-andre.przywara%40arm.com.
Andre Przywara Jan. 13, 2021, 10:30 a.m. UTC | #2
On 06/01/2021 15:56, Chen-Yu Tsai wrote:
> Hi,
> 
> On Wed, Jan 6, 2021 at 11:05 PM Andre Przywara <andre.przywara@arm.com> wrote:
>>
>> In recent Allwinner SoCs the first USB host controller (HCI0) shares
>> the first PHY with the MUSB controller. Probably to make this sharing
>> work, we were avoiding to declare this in the DT. This has two
>> shortcomings:
>> - U-Boot (which uses the same .dts) cannot use this port in host mode
>>   without a PHY linked, so we were loosing one USB port there.
>> - It requires the MUSB driver to be enabled and loaded, although we
>>   don't actually use it.
>>
>> To avoid those issues, let's add this PHY link to the A64 and H6 .dtsi
>> files. After all PHY port 0 *is* connected to HCI0, so we should describe
>> it as this. Remove the part from the boards which were already doing
>> that.
>>
>> This makes it work in U-Boot, also improves compatiblity when no MUSB
>> driver is loaded (for instance in distribution installers).
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts | 4 ----
>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi         | 4 ++++
>>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi          | 4 ++++
> 
> It would be better if the H6 parts were separated out.

Sure.

> 
> Please also add a fixes tag, which helps with backporting.

The whole concept of DT backports raises some eyebrows here, but it's
surely easy enough to do.

Thanks,
Andre.

> 
>>  3 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
>> index 896f34fd9fc3..d07cf05549c3 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
>> @@ -126,8 +126,6 @@
>>  };
>>
>>  &ehci0 {
>> -       phys = <&usbphy 0>;
>> -       phy-names = "usb";
>>         status = "okay";
>>  };
>>
>> @@ -177,8 +175,6 @@
>>  };
>>
>>  &ohci0 {
>> -       phys = <&usbphy 0>;
>> -       phy-names = "usb";
>>         status = "okay";
>>  };
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> index 51cc30e84e26..19e9b8ca8432 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> @@ -593,6 +593,8 @@
>>                                  <&ccu CLK_USB_OHCI0>;
>>                         resets = <&ccu RST_BUS_OHCI0>,
>>                                  <&ccu RST_BUS_EHCI0>;
>> +                       phys = <&usbphy 0>;
>> +                       phy-names = "usb";
>>                         status = "disabled";
>>                 };
>>
>> @@ -603,6 +605,8 @@
>>                         clocks = <&ccu CLK_BUS_OHCI0>,
>>                                  <&ccu CLK_USB_OHCI0>;
>>                         resets = <&ccu RST_BUS_OHCI0>;
>> +                       phys = <&usbphy 0>;
>> +                       phy-names = "usb";
>>                         status = "disabled";
>>                 };
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>> index 8a62a9fbe347..f593cfeaecc9 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>> @@ -680,6 +680,8 @@
>>                                  <&ccu CLK_USB_OHCI0>;
>>                         resets = <&ccu RST_BUS_OHCI0>,
>>                                  <&ccu RST_BUS_EHCI0>;
>> +                       phys = <&usb2phy 0>;
>> +                       phy-names = "usb";
>>                         status = "disabled";
>>                 };
>>
>> @@ -690,6 +692,8 @@
>>                         clocks = <&ccu CLK_BUS_OHCI0>,
>>                                  <&ccu CLK_USB_OHCI0>;
>>                         resets = <&ccu RST_BUS_OHCI0>;
>> +                       phys = <&usb2phy 0>;
>> +                       phy-names = "usb";
>>                         status = "disabled";
>>                 };
>>
>> --
>> 2.17.5
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
>> To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20210106150525.15403-2-andre.przywara%40arm.com.
Chen-Yu Tsai Jan. 13, 2021, 10:43 a.m. UTC | #3
On Wed, Jan 13, 2021 at 6:30 PM André Przywara <andre.przywara@arm.com> wrote:
>
> On 06/01/2021 15:56, Chen-Yu Tsai wrote:
> > Hi,
> >
> > On Wed, Jan 6, 2021 at 11:05 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >>
> >> In recent Allwinner SoCs the first USB host controller (HCI0) shares
> >> the first PHY with the MUSB controller. Probably to make this sharing
> >> work, we were avoiding to declare this in the DT. This has two
> >> shortcomings:
> >> - U-Boot (which uses the same .dts) cannot use this port in host mode
> >>   without a PHY linked, so we were loosing one USB port there.
> >> - It requires the MUSB driver to be enabled and loaded, although we
> >>   don't actually use it.
> >>
> >> To avoid those issues, let's add this PHY link to the A64 and H6 .dtsi
> >> files. After all PHY port 0 *is* connected to HCI0, so we should describe
> >> it as this. Remove the part from the boards which were already doing
> >> that.
> >>
> >> This makes it work in U-Boot, also improves compatiblity when no MUSB
> >> driver is loaded (for instance in distribution installers).
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >> ---
> >>  arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts | 4 ----
> >>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi         | 4 ++++
> >>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi          | 4 ++++
> >
> > It would be better if the H6 parts were separated out.
>
> Sure.
>
> >
> > Please also add a fixes tag, which helps with backporting.
>
> The whole concept of DT backports raises some eyebrows here, but it's
> surely easy enough to do.

Well, whether people choose to backport is their decision. But providing
information used to decide whether to do so, and if yes, what else is
needed, is IMHO something the author can help with.

Likely backport scenarios are distribution kernels AFAICT.

ChenYu

> Thanks,
> Andre.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
index 896f34fd9fc3..d07cf05549c3 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -126,8 +126,6 @@ 
 };
 
 &ehci0 {
-	phys = <&usbphy 0>;
-	phy-names = "usb";
 	status = "okay";
 };
 
@@ -177,8 +175,6 @@ 
 };
 
 &ohci0 {
-	phys = <&usbphy 0>;
-	phy-names = "usb";
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 51cc30e84e26..19e9b8ca8432 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -593,6 +593,8 @@ 
 				 <&ccu CLK_USB_OHCI0>;
 			resets = <&ccu RST_BUS_OHCI0>,
 				 <&ccu RST_BUS_EHCI0>;
+			phys = <&usbphy 0>;
+			phy-names = "usb";
 			status = "disabled";
 		};
 
@@ -603,6 +605,8 @@ 
 			clocks = <&ccu CLK_BUS_OHCI0>,
 				 <&ccu CLK_USB_OHCI0>;
 			resets = <&ccu RST_BUS_OHCI0>;
+			phys = <&usbphy 0>;
+			phy-names = "usb";
 			status = "disabled";
 		};
 
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 8a62a9fbe347..f593cfeaecc9 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -680,6 +680,8 @@ 
 				 <&ccu CLK_USB_OHCI0>;
 			resets = <&ccu RST_BUS_OHCI0>,
 				 <&ccu RST_BUS_EHCI0>;
+			phys = <&usb2phy 0>;
+			phy-names = "usb";
 			status = "disabled";
 		};
 
@@ -690,6 +692,8 @@ 
 			clocks = <&ccu CLK_BUS_OHCI0>,
 				 <&ccu CLK_USB_OHCI0>;
 			resets = <&ccu RST_BUS_OHCI0>;
+			phys = <&usb2phy 0>;
+			phy-names = "usb";
 			status = "disabled";
 		};