diff mbox

[RFC,16/22] USB: UHCI: clarify Kconfig dependencies

Message ID Pine.LNX.4.44L0.1305021304500.1436-100000@iolanthe.rowland.org (mailing list archive)
State New, archived
Headers show

Commit Message

Alan Stern May 2, 2013, 5:30 p.m. UTC
On Thu, 2 May 2013, Arnd Bergmann wrote:

> The UHCI driver currently gives a build error if the base driver is
> enabled but none of the three bus glues are turned on:
> 
> drivers/usb/host/uhci-hcd.c:857:2: error: #error "missing bus glue for uhci-hcd"
> 
> A better solution for this is to change the Kconfig statements to
> prevent getting into this situation. This adds a new USB_UHCI_CORE
> symbol in Kconfig, which is selected by each of the three bus glues.
> This way, the driver never gets built if all three of them are disabled.

I had already worked out a patch on my own for this (below).  It is
different from yours in several ways:

	It relies more on "depends" than "select".  I don't know how 
	important this is in the end.

	It doesn't add a new USB_UHCI_GRLIB symbol; instead it uses
	SPARC_LEON in several places.  I tend to think the new symbol
	is nicer.

	It doesn't add a new USB_UHCI_PCI symbol.

	It improves the dependency list for USB_UHCI_HCD.

	It removes the help text for USB_UHCI_PLATFORM, thereby making
	that symbol not user-configurable.  I don't see any reason why
	the user should need to worry about this -- the choice should
	be a very simple one: build UHCI support or don't build it.  If
	the user chooses to build it then it should include support for
	all the compatible bus glues.  (This last decision may need to 
	be changed if more bus glues get added.)

	It prevents situations where USB_UHCI_HCD is enabled but the
	driver doesn't get built.

	It creates a bunch of USB_UHCI_* symbols even when USB_UHCI_HCD
	is disabled.  This is a disadvantage, but I don't see any way
	around it.  Basically, we have to consider two separate but
	related questions:

		Does the supported hardware configuration allow for 
		UHCI?

		Which types of UHCI bus glue support should be included 
		in the kernel?

	If the answer to the first is Yes then creating these symbols
	seems unavoidable, even when the answer to the second is None.

Maybe the ideal solution is some sort of combination of the two
patches.

What do you think of my patch as compared to yours?  And what do you
think of the "depends" vs. "select" issue?

Alan Stern

Comments

Arnd Bergmann May 2, 2013, 5:52 p.m. UTC | #1
On Thursday 02 May 2013, Alan Stern wrote:
> On Thu, 2 May 2013, Arnd Bergmann wrote:
> 
> 	It relies more on "depends" than "select".  I don't know how 
> 	important this is in the end.

'select' is sometimes problematic, but the only use that I try to
avoid is having one Kconfig symbol that is both user-selectable
standalone and selected by something else.

> 	It doesn't add a new USB_UHCI_GRLIB symbol; instead it uses
> 	SPARC_LEON in several places.  I tend to think the new symbol
> 	is nicer.
> 
> 	It doesn't add a new USB_UHCI_PCI symbol.
> 
> 	It improves the dependency list for USB_UHCI_HCD.
> 
> 	It removes the help text for USB_UHCI_PLATFORM, thereby making
> 	that symbol not user-configurable.  I don't see any reason why
> 	the user should need to worry about this -- the choice should
> 	be a very simple one: build UHCI support or don't build it.  If
> 	the user chooses to build it then it should include support for
> 	all the compatible bus glues.  (This last decision may need to 
> 	be changed if more bus glues get added.)
>
> What do you think of my patch as compared to yours? 

I think in the end it comes down to the question where you want to
head with your driver. The way I did my version was going towards
making it similar to EHCI, with stand-alone bus glue drivers
and a core that is just a library module but does not register
a device_driver by itself.

Given that there are just three bus glues from UHCI, and at most
two of them enabled at the same time, I don't see a direct need
for UHCI to go down the same route as EHCI. If you want to just
leave this driver alone, your patch is simpler and has the same
effect in the end. Otherwise I think my patch avoids changing it
all again once the driver gets reworked.

Things might also get a little messy if we are seeing a lot
of other platforms beside VIA VT8500 use UHCI, but I think that
is rather unlikely.

	Arnd
Alan Stern May 2, 2013, 6:27 p.m. UTC | #2
On Thu, 2 May 2013, Arnd Bergmann wrote:

> > What do you think of my patch as compared to yours? 
> 
> I think in the end it comes down to the question where you want to
> head with your driver. The way I did my version was going towards
> making it similar to EHCI, with stand-alone bus glue drivers
> and a core that is just a library module but does not register
> a device_driver by itself.
> 
> Given that there are just three bus glues from UHCI, and at most
> two of them enabled at the same time, I don't see a direct need
> for UHCI to go down the same route as EHCI. If you want to just
> leave this driver alone, your patch is simpler and has the same
> effect in the end. Otherwise I think my patch avoids changing it
> all again once the driver gets reworked.

Okay then, I'll submit my patch to Greg after the merge window closes.

> Things might also get a little messy if we are seeing a lot
> of other platforms beside VIA VT8500 use UHCI, but I think that
> is rather unlikely.

Indeed.  UHCI is the oldest of the host controller standards; people
aren't about to start creating a whole bunch of new implementations of 
it.  Even Intel no longer uses UHCI on their more recent motherboards.

Alan Stern
diff mbox

Patch

Index: usb-3.9/drivers/usb/host/Kconfig
===================================================================
--- usb-3.9.orig/drivers/usb/host/Kconfig
+++ usb-3.9/drivers/usb/host/Kconfig
@@ -507,7 +507,7 @@  endif # USB_OHCI_HCD
 
 config USB_UHCI_HCD
 	tristate "UHCI HCD (most Intel and VIA) support"
-	depends on PCI || SPARC_LEON || ARCH_VT8500
+	depends on PCI || USB_UHCI_SUPPORT_NON_PCI_HC
 	---help---
 	  The Universal Host Controller Interface is a standard by Intel for
 	  accessing the USB hardware in the PC (which is also called the USB
@@ -524,26 +524,19 @@  config USB_UHCI_HCD
 
 config USB_UHCI_SUPPORT_NON_PCI_HC
 	bool
-	depends on USB_UHCI_HCD
-	default y if (SPARC_LEON || ARCH_VT8500)
+	default y if (SPARC_LEON || USB_UHCI_PLATFORM)
 
 config USB_UHCI_PLATFORM
-	bool "Generic UHCI Platform Driver support"
-	depends on USB_UHCI_SUPPORT_NON_PCI_HC
+	bool
 	default y if ARCH_VT8500
-	---help---
-	  Enable support for generic UHCI platform devices that require no
-	  additional configuration.
 
 config USB_UHCI_BIG_ENDIAN_MMIO
 	bool
-	depends on USB_UHCI_SUPPORT_NON_PCI_HC && SPARC_LEON
-	default y
+	default y if SPARC_LEON
 
 config USB_UHCI_BIG_ENDIAN_DESC
 	bool
-	depends on USB_UHCI_SUPPORT_NON_PCI_HC && SPARC_LEON
-	default y
+	default y if SPARC_LEON
 
 config USB_FHCI_HCD
 	tristate "Freescale QE USB Host Controller support"