[LEDE-DEV] [PATCH] kernel: clean up usb gadget support

[Tim Harvey]

On Sat, Sep 3, 2016 at 5:00 AM, Felix Fietkau <nbd at nbd.name> wrote:
> On 2016-09-01 16:52, Tim Harvey wrote:
>> clean up usb gadget support:
>> - remove unnecessary kmod-usb-lib-composite
>> - make kmod-usb-gadget a proper dependency vs a selection
>> - rename modules so that they match standard linux kernel module name
>>   and properly indent underneath usb-gadget in menuconfig
>>
>> Signed-off-by: Tim Harvey <tharvey at gateworks.com>
>> ---
>>  package/kernel/linux/modules/usb.mk | 48 +++++++++++++++----------------------
>>  1 file changed, 19 insertions(+), 29 deletions(-)
>>
>> diff --git a/package/kernel/linux/modules/usb.mk b/package/kernel/linux/modules/usb.mk
>> index 52c28c9..24b5d7b 100644
>> --- a/package/kernel/linux/modules/usb.mk
>> +++ b/package/kernel/linux/modules/usb.mk
>> @@ -219,20 +219,6 @@ endef
>>
>>  $(eval $(call KernelPackage,usb-gadget))
>>
>> -define KernelPackage/usb-lib-composite
>> -  TITLE:=USB lib composite
>> -  KCONFIG:=CONFIG_USB_LIBCOMPOSITE
>> -  DEPENDS:=+kmod-usb-gadget +kmod-fs-configfs
>> -  FILES:=$(LINUX_DIR)/drivers/usb/gadget/libcomposite.ko
>> -  AUTOLOAD:=$(call AutoLoad,50,libcomposite)
>> -  $(call AddDepends/usb)
>> -endef
>> -
>> -define KernelPackage/usb-lib-composite/description
>> - Lib Composite
>> -endef
>> -
>> -$(eval $(call KernelPackage,usb-lib-composite))
>>
>>  define KernelPackage/usb-ehci-debug-gadget
>>    TITLE:=USB EHCI debug port Gadget support
>> @@ -252,14 +238,15 @@ endef
>>
>>  $(eval $(call KernelPackage,usb-ehci-debug-gadget))
>>
>> -define KernelPackage/usb-eth-gadget
>> -  TITLE:=USB Ethernet Gadget support
>> +define KernelPackage/usb-g_ether
> Please don't use _ in package names, it is used to to separate the
> package name from the version in .ipk files.
> Also, I think the new name is less clear than the old one.

I was trying to stick with the kernel module names but as they have _
in them, I will remove this part of the patch.

>
>> +  TITLE:=USB Ethernet Gadget
>>    KCONFIG:= \
>>       CONFIG_USB_ETH \
>>       CONFIG_USB_ETH_RNDIS=y \
>>       CONFIG_USB_ETH_EEM=n
>> -  DEPENDS:=+kmod-usb-gadget +kmod-usb-lib-composite
>> +  DEPENDS:=kmod-usb-gadget +kmod-fs-configfs
>>    FILES:= \
>> +     $(LINUX_DIR)/drivers/usb/gadget/libcomposite.ko \
> Adding libcomposite.ko to multiple packages is a really bad idea.
> Why did you prefer that over having proper dependencies in place?

I don't believe I added any deps - I removed libcomposite from the
menu selection and included it directly in the gadgets that require it
because as far as I know there is no point in having libcomposite.ko
without the g_* drivers that use it.

On a related note, what do you think about replacing all gadget
drivers (g_*) with usb_f_* modules using CONFIG_USB_CONFIG_FS allowing
userspace configuration of gadgets? I can see an OpenWrt init script
that builds composite gadgets based off of UCI config options. I'm not
sure what the future holds for the various g_* modules as you can
re-create those easily in userspace now.

For details and examples see:
 - http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/usb/gadget_configfs.txt
 - http://trac.gateworks.com/wiki/linux/OTG#configfs

Tim