Re: [PATCH v2] staging/ccg: Allow to overwrite composite's setup function

[Michal Nazarewicz]

Sebastian Andrzej Siewior <bige...@linutronix.de> writes:
> I'm not going to swear here. The Android gadget includes composite.c
> from the main tree and overwrites functions. This would still work if I
> rename it and remove the const attribute but the problem rises again
> once we stop including composite.c and use it as a library function.
>
> Cc: de...@driverdev.osuosl.org
> Signed-off-by: Sebastian Andrzej Siewior <bige...@linutronix.de>
> ---
> v1..v2: different overwrite hack
>
> Guys, it is getting difficult to keep up fixing Android code while
> hacking on composite. Is anyone against a copy of composite.c and
> friends in drivers/staging/ccg/ like it has been with transport.c or
> scsiglue.c for drivers/staging/keucr?
>
>  drivers/staging/ccg/ccg.c      |    9 +++++----
>  drivers/usb/gadget/composite.c |   11 ++++++++++-
>  2 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/ccg/ccg.c b/drivers/staging/ccg/ccg.c
> index 8fb8663..78ccfe9 100644
> --- a/drivers/staging/ccg/ccg.c
> +++ b/drivers/staging/ccg/ccg.c
> @@ -47,6 +47,11 @@
>  #include "../../usb/gadget/usbstring.c"
>  #include "../../usb/gadget/config.c"
>  #include "../../usb/gadget/epautoconf.c"
> +
> +#define USB_COMPOSITE_INCLUDED_BY_CCG        1
> +static int ccg_setup(struct usb_gadget *gadget,
> +             const struct usb_ctrlrequest *c);
> +static void ccg_disconnect(struct usb_gadget *gadget);
>  #include "../../usb/gadget/composite.c"
>  
>  #include "../../usb/gadget/f_mass_storage.c"
> @@ -1272,10 +1277,6 @@ static int __init init(void)
>  
>       _ccg_dev = dev;
>  
> -     /* Override composite driver functions */
> -     composite_driver.setup = ccg_setup;
> -     composite_driver.disconnect = ccg_disconnect;
> -
>       err = usb_composite_probe(&ccg_usb_driver);
>       if (err) {
>               class_destroy(ccg_class);
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 598df69..790fc4d 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -1598,9 +1598,18 @@ static const struct usb_gadget_driver 
> composite_driver_template = {
>  
>       .bind           = composite_bind,
>       .unbind         = composite_unbind,
> -
> +#ifdef USB_COMPOSITE_INCLUDED_BY_CCG
> +     /*
> +      * This is to keep the Android code in staging working. The current
> +      * Kconfig dependencies don't allow any other gadgets besides the
> +      * Android one so it is not even a limitation.
> +      */
> +     .setup          = ccg_setup,
> +     .disconnect     = ccg_disconnect,
> +#else
>       .setup          = composite_setup,
>       .disconnect     = composite_disconnect,
> +#endif

Would it make sense to instead of doing that, wrap composite_setup() and
composite_disconnect() inside ifdef and make ccg.c define those two
functions?

>       .suspend        = composite_suspend,
>       .resume         = composite_resume,

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: m...@google.com>--------------ooO--(_)--Ooo--

pgprVg0wMeitL.pgp
Description: PGP signature