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

Marek Vasut
2013-09-20T21:20:15Z

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