Sebastian Andrzej Siewior <bige...@linutronix.de> writes:
> On 08/24/2012 11:53 PM, Michal Nazarewicz wrote:
>> I wouldn't duplicate the documentation. One in header file should be
>> enough.
>
> I think I go for the .c file where this is implemented. If you change
> the .c file most people don't care about the header file.
>
>>> +int usb_gadget_controller_number(struct usb_gadget *gadget)
>>> +{
>>> + if (gadget_is_net2280(gadget))
>>> + return 0x01;
>
> .. snip...
>
>>> + 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",
>
> ..snip..
>
>> "\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.
>
> I am a friend of explicit naming and less hacks. And if you sort the
> map then you have less reads to get to the last entry. This can be
> limited even further if make it a member of a struct.
Yeah, that's why I wrote that something less fancy should do too, like:
int usb_gadget_controller_number(struct usb_gadget *gadget)
{
static const struct name_to_num {
const *name;
int id;
} map[] = {
{ "net2280", 0x01 },
{ "dummy_udc", 0x02 },
{ "pxa25x_udc", 0x03 },
{ "goku_udc", 0x06 },
{ "omap_udc", 0x08 },
{ "pxa27x_udc", 0x11 },
{ "s3c2410_udc", 0x12 },
{ "at91_udc", 0x13 },
{ "imx_udc", 0x14 },
{ "musb-hdrc", 0x16 },
{ "atmel_usba_udc", 0x18 },
{ "fsl-usb2-udc", 0x19 },
{ "amd5536udc", 0x20 },
{ "m66592_udc", 0x21 },
{ "fsl_qe_udc", 0x22 },
{ "ci13xxx_pci", 0x23 },
{ "langwell_udc", 0x24 },
{ "r8a66597_udc", 0x25 },
{ "s3c-hsotg", 0x26 },
{ "pch_udc", 0x27 },
{ "ci13xxx_msm", 0x28 },
{ "renesas_usbhs_udc", 0x29 },
{ "s3c-hsudc", 0x30 },
{ "net2272", 0x31 },
{ "dwc3-gadget", 0x32 },
{ "lpc32xx_udc", 0x33 },
{ NULL, -ENOENT }
};
const struct name_to_num *it = map;
const char *const name = gadget->name;
while (it->name && strcmp(name, it->name))
++it;
return it->num;
}
> Which brings me to: How important is this function?
> - hacks / limitations should be implement differently i.e. a feature
> bitmap
I totally agree.
> - do we need a unique bcd value across all udcs? Is somebody using
> this? Useful?
> Most of my device have either 1.00 or 2.00.
> We set this to 0x200 + UDC number. Why? Why the 0x200? The spec says
> "device release number" and "release number" is defined as 0xJJMN
> (with J major, M minor, N sub minor minor). I don't say we have to
> follow this I just for a sane reason and a use case.
Yeah, I was always wondering about that myself.
> Using here something like 0x305 for kernel version 3.5 would be a
> little more useful. We would know that the kernel on this device is
> based on v3.5 (plus a big number of patches on top).
Worth noting that version is also included in the default manufacturer.
--
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--
pgpvSYVDiO31H.pgp
Description: PGP signature
