[U-Boot] [PATCH 1/8] USB Consolidate descriptor definitions
Tom Rix
tom at bumblecow.com
Wed Oct 14 13:14:55 CEST 2009
Remy Bohmer wrote:
> Hi Tom,
>
> Sorry for not responding to this series very fast, because I wanted to
> look into it carefully.
> Below my comments on this patch
>
> 2009/9/28 Tom Rix <Tom.Rix at windriver.com>:
>> The header files usb.h and usbdescriptors.h have the same or
>> similarly named structure definitions for
>>
>> usb_device_descriptor
>> usb_string_descriptor
>> usb_endpoint_descriptor
>> usb_config_descriptor vs usb_configuration_descriptor
>> usb_interface_descriptor
>>
>> There should only be one definition of these structures.
>
> True, I agree with that.
>
>> Some of these structures are element-wise duplicates,
>> one has unused elements that can be reduced to a duplicate,
>> and some of the the structures have additional elements.
>>
>> These are the element-wise duplicates in usb.h
>>
>> usb_device_descriptor
>> usb_string_descriptor
>>
>> For these, the usb.h definition will be removed.
>>
>> This definition in usb.h has extra unused elements
>
> Can these be merged to one global structure?
>
No.
The size of the structure is kept in 'bLength' field.
Have bLength = sizeof(struct bla_descriptor)
is the right way to go.
>> usb_endpoint_descriptor
>>
>> unsigned char bRefresh
>> unsigned char bSynchAddress;
>>
>> The definition of usb_endpoint_descriptor in usb.h will
>> be removed.
>>
>> These definitions in usb.h have extra elements at the end of
>> the usb 2.0 specified descriptor:
>>
>> usb_config_descriptor
>> usb_interface_descriptor
>>
>> These structures will have their name shorted by removing the
>> '_descriptor' suffix.
>>
>> So
>>
>> usb_config_descriptor -> usb_config
>> usb_interface_descriptor -> usb_interface
>>
>> The common descriptor elements are now defined by the macros
>>
>> USB_CONFIG_DESCRIPTOR_DEFINITION
>> USB_INTERFACE_DESCRIPTOR_DEFINITION
>
> Personally I do not like the macros for these kind of definitions, I
> would prefer a solution by nesting the structures.
> What do you think?
I have an early version of this patch that does use nested structures.
After looking at the added '.desc.' that caused a good number of
changes in the c files, I went with the macro approach. It seemed like
the lesser of the evils as the changes seemed smaller and were limited
to the h files. What I really wanted to do was to use nameless
structures but had a problem getting them to work with gcc.
I do not mind reverting to the '.desc.' solution.
I will resend the patches when I have had a chance to test them.
Thanks
Tom
>
> Kind regards,
>
> Remy
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
More information about the U-Boot
mailing list