[U-Boot] [PATCH 2/4] sunxi: Avoid any assumption between musb gadget and host but fallback to host

[Paul Kocialkowski]

Le samedi 21 mars 2015 à 14:14 +0100, Hans de Goede a écrit :
> Hi,
> 
> On 15-03-15 18:27, Paul Kocialkowski wrote:
> > musb might be used in gadget mode on sunxi, so we don't want to assume anything
> > related to host mode, especially USB keyboard support.
> > However, in case gadget mode is not explicitly used, fallback to host mode.
> >
> > Signed-off-by: Paul Kocialkowski <contact at paulk.fr>
> > ---
> >   board/sunxi/Kconfig            | 2 +-
> >   include/configs/sunxi-common.h | 5 ++++-
> >   2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> > index 9d0eb91..0b6a492 100644
> > --- a/board/sunxi/Kconfig
> > +++ b/board/sunxi/Kconfig
> > @@ -436,7 +436,7 @@ config USB_MUSB_SUNXI
> >
> >   config USB_KEYBOARD
> >   	boolean "Enable USB keyboard support"
> > -	default y
> > +	default n
> >   	---help---
> >   	Say Y here to add support for using a USB keyboard (typically used
> >   	in combination with a graphical console).
> 
> You've just disabled the usb keyboard support pretty much everywhere, please
> do not do that.
> 
> > diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> > index 1f7a1cb..ffd9f5c 100644
> > --- a/include/configs/sunxi-common.h
> > +++ b/include/configs/sunxi-common.h
> > @@ -297,13 +297,16 @@ extern int soft_i2c_gpio_scl;
> >   #endif
> >
> >   #ifdef CONFIG_USB_MUSB_SUNXI
> > +#ifndef CONFIG_MUSB_GADGET
> >   #define CONFIG_MUSB_HOST
> > +#endif
> >   #define CONFIG_MUSB_PIO_ONLY
> >   #endif
> >
> > -#if defined CONFIG_USB_EHCI || defined CONFIG_USB_MUSB_SUNXI
> > +#if defined CONFIG_USB_EHCI || defined CONFIG_MUSB_HOST
> >   #define CONFIG_CMD_USB
> >   #define CONFIG_USB_STORAGE
> > +#define CONFIG_USB_KEYBOARD
> 
> 
> And here you are overriding something set by Kconfig which is a big no no,
> if you want to disable the keyboard support on boards where you use
> the oth port in gadget mode jsut add:
> CONFIG_USB_KEYBOARD=n to the (def)config.

I understand that the logic is flawed here (disable a Kconfig option in
Kconfig and enable it back in the config header, which is weaker), but I
really don't think it should always be enabled.

IMO the best solution would be to remove USB_KEYBOARD from Kconfig
entirely and enable it in sunxi-common.h as I suggested. This way, we
don't even have to explicitely disable it for devices that don't have
USB support (MSI_Primo81/MSI_Primo73): it will only get enabled when
EHCI or host MUSB support is enabled.

I fail to see the point of using Kconfig here anyway, since nothing
depends on it (in the Kconfig sense) and other platforms have been using
CONFIG_USB_KEYBOARD in the config files without ever needing to have it
in Kconfig. In addition, it doesn't seem very relevant to me to have
that Kconfig option defined in the sunxi part of the code, while
USB_KEYBOARD is also used on other SoCs.

Please let me know what you think here, I'd be glad to make a patch
fixing this behaviour.

-- 
Paul Kocialkowski, Replicant developer

Replicant is a fully free Android distribution running on several
devices, a free software mobile operating system putting the emphasis on
freedom and privacy/security.

Website: http://www.replicant.us/
Blog: http://blog.replicant.us/
Wiki/tracker/forums: http://redmine.replicant.us/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150322/1f4047cd/attachment.sig>