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

[Marek Vasut]

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 at 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