Re: [PATCH v2] usb: chipidea: fix and improve dependencies if usb host or gadget support is built as module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/17/2012 07:47 PM, Greg KH wrote:
> On Mon, Jul 16, 2012 at 10:18:20PM +0200, Marc Kleine-Budde wrote:
>> Since commit "5e0aa49 usb: chipidea: use generic map/unmap routines",
>> the udc part of the chipidea driver needs the generic usb gadget helper
>> functions. If the chipidea driver with udc support is built into the
>> kernel and usb gadget is built a module, the linking of the kernel
>> fails with:
>>
>> drivers/built-in.o: In function `_hardware_dequeue':
>> drivers/usb/chipidea/udc.c:527:
>>     undefined reference to `usb_gadget_unmap_request'
>> drivers/usb/chipidea/udc.c:1269:
>>     undefined reference to `usb_gadget_unmap_request'
>> drivers/usb/chipidea/udc.c:1821:
>>     undefined reference to `usb_del_gadget_udc'
>> drivers/usb/chipidea/udc.c:443:
>>     undefined reference to `usb_gadget_map_request'
>> drivers/usb/chipidea/udc.c:1774:
>>     undefined reference to `usb_add_gadget_udc'
>>
>> This patch changes the dependencies, so that udc support can only be
>> activated if the linux gadget support (USB_GADGET) is builtin or both
>> chipidea driver and USB_GADGET are modular. Same dependencies for the
>> chipidea host support and the linux host side USB support (USB).
> 
> Shouldn't you just properly export the above functions?

These function are exported.

> Hm, they are already exported, so you really shouldn't have to do these
> Kconfig gyrations.

The problem is, one half of the functions comes from the USB host
subsystem, the other half from the USB gadget subsystem.So the driver
can only be built into the kernel (with host+udc support) if both USB
host and USB gadget are built in.

>> While there, fix the indention of chipidea the help text.
>>
>> Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
>> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
>> ---
>>
>> Changes since v1
>> - Do not depend on USB host support anymore (tnx Felipe for the input)
>> - Fix help text indention
>>
>>  drivers/usb/chipidea/Kconfig |    9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
>> index fd36dc8..38305b2 100644
>> --- a/drivers/usb/chipidea/Kconfig
>> +++ b/drivers/usb/chipidea/Kconfig
>> @@ -1,9 +1,9 @@
>>  config USB_CHIPIDEA
>>  	tristate "ChipIdea Highspeed Dual Role Controller"
>> -	depends on USB
>> +	depends on USB || USB_GADGET
> 
> This should be all that is needed, right?

yes, but....

> 
>>  	help
>> -          Say Y here if your system has a dual role high speed USB
>> -          controller based on ChipIdea silicon IP. Currently, only the
>> +	  Say Y here if your system has a dual role high speed USB
>> +	  controller based on ChipIdea silicon IP. Currently, only the
>>  	  peripheral mode is supported.
>>  
>>  	  When compiled dynamically, the module will be called ci-hdrc.ko.
>> @@ -12,7 +12,7 @@ if USB_CHIPIDEA
>>  
>>  config USB_CHIPIDEA_UDC
>>  	bool "ChipIdea device controller"
>> -	depends on USB_GADGET
>> +	depends on USB_GADGET=y || USB_GADGET=USB_CHIPIDEA
> 
> Ick, is this really necessary?

I can built in the the chipidea driver, if USB_GADGET _or_ USB (=host
stuff) is built in. So if USB_GADGET is modular and USB_CHIPIDEA is
built in, I will get a linking error, because the USB_GADGET functions
are missing in the kernel binary. This more or less ugly construct
disables chipidea udc support in this case.

> 
>>  	select USB_GADGET_DUALSPEED
>>  	help
>>  	  Say Y here to enable device controller functionality of the
>> @@ -20,6 +20,7 @@ config USB_CHIPIDEA_UDC
>>  
>>  config USB_CHIPIDEA_HOST
>>  	bool "ChipIdea host controller"
>> +	depends on USB=y || USB=USB_CHIPIDEA
> 
> This seems to not be needed with the first change above, right?

This covers the same case but with host modular and USB_GADGET builtin.

> Something odd is going on here, I don't see how these weird Kconfig
> depends fix this properly.  What am I missing?

Hope my explanation helps. I'm open for better solutions.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux