From mboxrd@z Thu Jan 1 00:00:00 1970 From: Troy Kisky Date: Fri, 20 Sep 2013 17:25:09 -0700 Subject: [U-Boot] [PATCH V4 06/17] usb: udc: add udc.h include file In-Reply-To: <201309210001.54431.marex@denx.de> References: <1379647780-2623-1-git-send-email-troy.kisky@boundarydevices.com> <201309202320.15717.marex@denx.de> <523CBF20.3010902@boundarydevices.com> <201309210001.54431.marex@denx.de> Message-ID: <523CE765.1070104@boundarydevices.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 9/20/2013 3:01 PM, Marek Vasut wrote: > Dear Troy Kisky, > >> 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 >>>>>>>>>> >>>>>>>>>> --- >>>>>>>>>> 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. > I see the difference now. For EHCI UDC, the max packet sizes are as they are. > For OMAP/MPC8xx, these are non-standard arbitrary values, it's just that the > name is the same. > > This is pretty ugly and confusing, ick. I'd say you #ifdef this stuff like this: > #ifdef OMAP1510 ... > #elif MPC8xx > #elif .... > #endif > > And put a comment around it that the old UDCs simply follow no standard. > > Best regards, > I suspect omap1510 will work with a value of 64 for the sizes, since it and mpc8xx were added by Bryan O'Donoghue - 29 May 2006 I've added him to the CC. Bryan, can you test changing include/usb/omap1510_udc.h #define UDC_INT_PACKET_SIZE 16 #define UDC_BULK_PACKET_SIZE 16 -------- or on a very old u-boot include/usbdcore_omap1510.h #define UDC_INT_PKTSIZE 16 #define UDC_BULK_PKTSIZE 16 from 16 to 64? Or maybe you remember the reason 16 was needed? Also, do your remember anything about the MPC885 family and why endpoint 0 could be 16 bytes, but bulk endpoints are limited to 8? Thanks, I'd like my comments to be accurate... Troy