[U-Boot] [PATCH V4 06/17] usb: udc: add udc.h include file

Troy Kisky
2013-09-20T21:33:20Z

On 9/20/2013 2:20 PM, Marek Vasut wrote:
> Dear Troy Kisky,
>
>> On 9/20/2013 12:53 PM, Marek Vasut wrote:
>>> Dear Troy Kisky,
>>>
>>>> On 9/20/2013 11:52 AM, Marek Vasut wrote:
>>>>> Dear Troy Kisky,
>>>>>
>>>>>> On 9/20/2013 3:55 AM, Marek Vasut wrote:
>>>>>>> Dear Troy Kisky,
>>>>>>>
>>>>>>>> Move common definitions to udc.h
>>>>>>>> This allows musb_udc.h to be removed as well.
>>>>>>>>
>>>>>>>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>>>>>>>>
>>>>>>>> ---
>>>>>>>> v4: updated commit message
>>>>>>>> removed ifdef UDC_BULK_HS_PACKET_SIZE since 512
>>>>>>>> is the only legal value, it shouldn't be overridden.
>>>>>>> [...]
>>>>>>>
>>>>>>>> #endif
>>>>>>>>
>>>>>>>> diff --git a/include/usb/udc.h b/include/usb/udc.h
>>>>>>>> new file mode 100644
>>>>>>>> index 0000000..3bcbbbc
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/include/usb/udc.h
>>>>>>>> @@ -0,0 +1,61 @@
>>>>>>>> +/*
>>>>>>>> + * SPDX-License-Identifier: GPL-2.0+
>>>>>>>> + */
>>>>>>>> +#ifndef USB_UDC_H
>>>>>>>> +#define USB_UDC_H
>>>>>>>> +
>>>>>>>> +#ifndef EP0_MAX_PACKET_SIZE
>>>>>>>> +#define EP0_MAX_PACKET_SIZE 64
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>> +#ifndef EP_MAX_PACKET_SIZE
>>>>>>>> +#define EP_MAX_PACKET_SIZE 64
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>> +#ifndef UDC_OUT_PACKET_SIZE
>>>>>>>> +#define UDC_OUT_PACKET_SIZE EP_MAX_PACKET_SIZE
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>> +#ifndef UDC_IN_PACKET_SIZE
>>>>>>>> +#define UDC_IN_PACKET_SIZE EP_MAX_PACKET_SIZE
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>> +#ifndef UDC_INT_PACKET_SIZE
>>>>>>>> +#define UDC_INT_PACKET_SIZE EP_MAX_PACKET_SIZE
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>> +#ifndef UDC_BULK_PACKET_SIZE
>>>>>>>> +#define UDC_BULK_PACKET_SIZE EP_MAX_PACKET_SIZE
>>>>>>>> +#endif
>>>>>>> Do you expect these values to change on per-controller basis? Or why
>>>>>>> do you have these ifndefs here ?
>>>>>> I don't know why they change but
>>>>>>
>>>>>> include/usb/mpc8xx_udc.h:#define UDC_BULK_PACKET_SIZE
>>>>>> EP_MIN_PACKET_SIZE /* 8 */
>>>>>> include/usb/omap1510_udc.h:#define UDC_BULK_PACKET_SIZE 16
>>>>>>
>>>>>> include/usb/mpc8xx_udc.h:#define UDC_INT_PACKET_SIZE
>>>>>> UDC_IN_PACKET_SIZE /* 8 */
>>>>>> include/usb/omap1510_udc.h:#define UDC_INT_PACKET_SIZE 16
>>>>>>
>>>>>> include/usb/mpc8xx_udc.h:#define UDC_OUT_PACKET_SIZE
>>>>>> EP_MIN_PACKET_SIZE /* */
>>>>> Are you sure this is not OHCI ?
>>>>>
>>>>> Best regards,
>>>>> Marek Vasut
>>>> I don't know.
>>>> I don't understand the relevance of the question. Can you explain the
>>>> issue a little more
>>>> for me.
>>> OMAP1510 has only OHCI controller in it, dunno about MPC8xx, but that
>>> seems to be the case as well. Therefore, in OHCI case, the max packet is
>>> 16 and in ehci it's 64 . Check the specs ;-)
>>>
>>> Best regards,
>>> Marek Vasut
>> Ok, I checked the spec for the OMAP1510, and found "Table 14?23.
>> Endpoint n Size Values"
>> lists maximum packet sizes of 8, 16, 32, or 64 bytes for Non-isochronous
>> endpoints, and 8, 16, 32, 64, 128, 256, 512 for Isochronous endpoints
>>
>> So, I still don't understand the limit of 16, but that isn't required.
>>
>>
>> So, are you saying I should edit omap1510_udc.h and add
>>
>> #define EP_MAX_PACKET_SIZE 16
>>
>>
>> and remove
>> #ifndef UDC_BULK_PACKET_SIZE
>>
>> in udc.h and do
>>
>> #define UDC_BULK_PACKET_SIZE EP_MAX_PACKET_SIZE
>>
>> unconditionally?
> I'd say you should check if the controller is OHCI or EHCI , check what kind of
> endpoint it is and based on that , configure the max packet size. Or is this
> really controller specific ? What do the OHCI and EHCI specs say ?
>
> Best regards,
> Marek Vasut
>

The omap1510 does have an OHCI host controller, I don't know how that
relates to the device
controller so I think a check would just be confusing.

Troy