Re: [PATCH 01/22] sunxi: Fix default-enablement of USB host drivers

From: Andre Przywara <andre.przywara@arm.com>
To: Samuel Holland <samuel@sholland.org>
Cc: Jagan Teki <jagan@amarulasolutions.com>, u-boot@lists.denx.de
Subject: Re: [PATCH 01/22] sunxi: Fix default-enablement of USB host drivers
Date: Fri, 11 Nov 2022 01:09:18 +0000	[thread overview]
Message-ID: <20221111010918.13faa3b9@slackpad.lan> (raw)
In-Reply-To: <20221101050835.2421-2-samuel@sholland.org>

On Tue,  1 Nov 2022 00:08:13 -0500
Samuel Holland <samuel@sholland.org> wrote:

Hi Samuel,

> We tried to enable USB_EHCI_GENERIC and USB_OHCI_GENERIC by default.

Well, I am not really sure that was the real intention, looking at
commit 29d280c88a1ff3, and the fact that those symbols are still
explicitly defined in many board config files.
My main concern is that this change enables USB on many boards which had
it disabled - at least some of them probably on purpose. I believe for
the Pinephone for instance it's off to decrease boot time.

> This did not work because those symbols depend on USB_EHCI_HCD and
> USB_OHCI_HCD, which were not enabled. Fix this by implying all four.

The change itself looks correct, but as mentioned, 47 out of 161 boards
now come with USB newly enabled. 25 of those don't have EHCI or OHCI
nodes in their DT, so enabling USB doesn't make much sense there.
For the others I don't know if USB was disabled on purpose, on some
tablets for instance it seems that the only USB HCI port is connected to
an on-board WiFi chip, which we don't support in U-Boot.

So in a first round I would like to skip this patch. If we want to have
it (I am not against it, since the majority of boards have USB), this
should be paired with:
# USB_EHCI_HCD is not set
# USB_OHCI_HCD is not set
in the defconfigs at least for those boards without HCI DT nodes (I
have a list), to keep it disabled there.
If someone cares, and it has been disabled by mistake, they can fix
that with an extra patch.
For the other boards we should remove the ?HCI symbols from their
defconfigs, since they would be redundant then.

Cheers,
Andre

> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
>  arch/arm/Kconfig         | 4 ++++
>  drivers/usb/host/Kconfig | 2 --
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 710f171f87..d3a1f03b36 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1185,7 +1185,11 @@ config ARCH_SUNXI
>  	imply SYSRESET
>  	imply SYSRESET_WATCHDOG
>  	imply SYSRESET_WATCHDOG_AUTO
> +	imply USB_EHCI_GENERIC
> +	imply USB_EHCI_HCD
>  	imply USB_GADGET
> +	imply USB_OHCI_GENERIC
> +	imply USB_OHCI_HCD
>  	imply WDT
>  
>  config ARCH_U8500
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 1aabe062fb..a4d62bc9e8 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -280,7 +280,6 @@ config USB_EHCI_ZYNQ
>  config USB_EHCI_GENERIC
>  	bool "Support for generic EHCI USB controller"
>  	depends on DM_USB
> -	default ARCH_SUNXI
>  	---help---
>  	  Enables support for generic EHCI controller.
>  
> @@ -342,7 +341,6 @@ config USB_OHCI_PCI
>  
>  config USB_OHCI_GENERIC
>  	bool "Support for generic OHCI USB controller"
> -	default ARCH_SUNXI
>  	---help---
>  	  Enables support for generic OHCI controller.
>  