Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> writes:
> After I moved the function from the header file to the c file I see:
>
> | $ size drivers/usb/gadget/gadget_chips.o
> | text data bss dec hex filename
> | 1048 0 0 1048 418 drivers/usb/gadget/gadget_chips.o
>
> That is almost a KiB which is removed from each user.
> As Felipe pointed out, the function / usage is very dumb actually. This is
> used for the following reasons:
> - epautoconf ep hint (could provide a per-gadget callback)
> - miss-features. currently the missing altsetting on pxa's and something
> ZLP related on musbhdrc (looks like an optimisation which could be
> implemented in musb itself if it is correct)
> - unique BCD accross all UDCs. Not sure how important this is.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
> diff --git a/drivers/usb/gadget/gadget_chips.c b/drivers/usb/gadget/gadget_chips.c
> new file mode 100644
> index 0000000..854c30e
> --- /dev/null
> +++ b/drivers/usb/gadget/gadget_chips.c
> @@ -0,0 +1,92 @@
> +/*
> + * USB device controllers have lots of quirks. Use these macros in
> + * gadget drivers or other code that needs to deal with them, and which
> + * autoconfigures instead of using early binding to the hardware.
> + *
> + * This SHOULD eventually work like the ARM mach_is_*() stuff, driven by
> + * some config file that gets updated as new hardware is supported.
> + * (And avoiding all runtime comparisons in typical one-choice configs!)
> + *
> + * NOTE: some of these controller drivers may not be available yet.
> + * Some are available on 2.4 kernels; several are available, but not
> + * yet pushed in the 2.6 mainline tree.
> + */
> +
> +#include <linux/usb/gadget.h>
> +#include <linux/module.h>
> +
> +#include "gadget_chips.h"
> +
> +/**
> + * usb_gadget_controller_number - support bcdDevice id convention
> + * @gadget: the controller being driven
> + *
> + * Return a 2-digit BCD value associated with the peripheral controller,
> + * suitable for use as part of a bcdDevice value, or a negative error code.
> + *
> + * NOTE: this convention is purely optional, and has no meaning in terms of
> + * any USB specification. If you want to use a different convention in your
> + * gadget driver firmware -- maybe a more formal revision ID -- feel free.
> + *
> + * Hosts see these bcdDevice numbers, and are allowed (but not encouraged!)
> + * to change their behavior accordingly. For example it might help avoiding
> + * some chip bug.
> + */
I wouldn't duplicate the documentation. One in header file should be
enough.
> +int usb_gadget_controller_number(struct usb_gadget *gadget)
> +{
> + if (gadget_is_net2280(gadget))
> + return 0x01;
> + else if (gadget_is_dummy(gadget))
> + return 0x02;
> + else if (gadget_is_pxa(gadget))
> + return 0x03;
> + else if (gadget_is_goku(gadget))
> + return 0x06;
> + else if (gadget_is_omap(gadget))
> + return 0x08;
> + else if (gadget_is_pxa27x(gadget))
> + return 0x11;
> + else if (gadget_is_s3c2410(gadget))
> + return 0x12;
> + else if (gadget_is_at91(gadget))
> + return 0x13;
> + else if (gadget_is_imx(gadget))
> + return 0x14;
> + else if (gadget_is_musbhdrc(gadget))
> + return 0x16;
> + else if (gadget_is_atmel_usba(gadget))
> + return 0x18;
> + else if (gadget_is_fsl_usb2(gadget))
> + return 0x19;
> + else if (gadget_is_amd5536udc(gadget))
> + return 0x20;
> + else if (gadget_is_m66592(gadget))
> + return 0x21;
> + else if (gadget_is_fsl_qe(gadget))
> + return 0x22;
> + else if (gadget_is_ci13xxx_pci(gadget))
> + return 0x23;
> + else if (gadget_is_langwell(gadget))
> + return 0x24;
> + else if (gadget_is_r8a66597(gadget))
> + return 0x25;
> + else if (gadget_is_s3c_hsotg(gadget))
> + return 0x26;
> + else if (gadget_is_pch(gadget))
> + return 0x27;
> + else if (gadget_is_ci13xxx_msm(gadget))
> + return 0x28;
> + else if (gadget_is_renesas_usbhs(gadget))
> + return 0x29;
> + else if (gadget_is_s3c_hsudc(gadget))
> + return 0x30;
> + else if (gadget_is_net2272(gadget))
> + return 0x31;
> + else if (gadget_is_dwc3(gadget))
> + return 0x32;
> + else if (gadget_is_lpc32xx(gadget))
> + return 0x33;
> +
> + return -ENOENT;
> +}
How about using a table with a loop. For instance something like:
int usb_gadget_controller_number(struct usb_gadget *gadget)
{
static const char *const map[] = {
"\x01net2280",
"\x02dummy_udc",
"\x03pxa25x_udc",
"\x06goku_udc",
"\x08omap_udc",
"\x11pxa27x_udc",
"\x12s3c2410_udc",
"\x13at91_udc",
"\x14imx_udc",
"\x16musb-hdrc",
"\x18atmel_usba_udc",
"\x19fsl-usb2-udc",
"\x20amd5536udc",
"\x21m66592_udc",
"\x22fsl_qe_udc",
"\x23ci13xxx_pci",
"\x24langwell_udc",
"\x25r8a66597_udc",
"\x26s3c-hsotg",
"\x27pch_udc",
"\x28ci13xxx_msm",
"\x29renesas_usbhs_udc",
"\x30s3c-hsudc",
"\x31net2272",
"\x32dwc3-gadget",
"\x33lpc32xx_udc",
};
const char *const *it = map, *const *end = map + ARRAY_SIZE(map);
const char *const name = gadget->name;
for (; it != end; ++it) {
if (!strcmp(name, *it + 1))
return (unsigned char)**it;
}
return -ENOENT
}
Something less fancy should also do.
> +EXPORT_SYMBOL_GPL(usb_gadget_controller_number);
> diff --git a/drivers/usb/gadget/gadget_chips.h b/drivers/usb/gadget/gadget_chips.h
> index 6458e95..caea3c7 100644
> --- a/drivers/usb/gadget/gadget_chips.h
> +++ b/drivers/usb/gadget/gadget_chips.h
> @@ -69,64 +69,7 @@
> * to change their behavior accordingly. For example it might help avoiding
> * some chip bug.
> */
> -static inline int usb_gadget_controller_number(struct usb_gadget *gadget)
> -{
> - if (gadget_is_net2280(gadget))
> - return 0x01;
> - else if (gadget_is_dummy(gadget))
> - return 0x02;
> - else if (gadget_is_pxa(gadget))
> - return 0x03;
> - else if (gadget_is_goku(gadget))
> - return 0x06;
> - else if (gadget_is_omap(gadget))
> - return 0x08;
> - else if (gadget_is_pxa27x(gadget))
> - return 0x11;
> - else if (gadget_is_s3c2410(gadget))
> - return 0x12;
> - else if (gadget_is_at91(gadget))
> - return 0x13;
> - else if (gadget_is_imx(gadget))
> - return 0x14;
> - else if (gadget_is_musbhdrc(gadget))
> - return 0x16;
> - else if (gadget_is_atmel_usba(gadget))
> - return 0x18;
> - else if (gadget_is_fsl_usb2(gadget))
> - return 0x19;
> - else if (gadget_is_amd5536udc(gadget))
> - return 0x20;
> - else if (gadget_is_m66592(gadget))
> - return 0x21;
> - else if (gadget_is_fsl_qe(gadget))
> - return 0x22;
> - else if (gadget_is_ci13xxx_pci(gadget))
> - return 0x23;
> - else if (gadget_is_langwell(gadget))
> - return 0x24;
> - else if (gadget_is_r8a66597(gadget))
> - return 0x25;
> - else if (gadget_is_s3c_hsotg(gadget))
> - return 0x26;
> - else if (gadget_is_pch(gadget))
> - return 0x27;
> - else if (gadget_is_ci13xxx_msm(gadget))
> - return 0x28;
> - else if (gadget_is_renesas_usbhs(gadget))
> - return 0x29;
> - else if (gadget_is_s3c_hsudc(gadget))
> - return 0x30;
> - else if (gadget_is_net2272(gadget))
> - return 0x31;
> - else if (gadget_is_dwc3(gadget))
> - return 0x32;
> - else if (gadget_is_lpc32xx(gadget))
> - return 0x33;
> -
> - return -ENOENT;
> -}
> -
> +int usb_gadget_controller_number(struct usb_gadget *gadget);
>
> /**
> * gadget_supports_altsettings - return true if altsettings work
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@xxxxxxxxxx>--------------ooO--(_)--Ooo--Attachment:
pgpeAEiYuSBq6.pgp
Description: PGP signature
