From mboxrd@z Thu Jan 1 00:00:00 1970 From: Troy Kisky Date: Fri, 20 Sep 2013 14:33:20 -0700 Subject: [U-Boot] [PATCH V4 06/17] usb: udc: add udc.h include file In-Reply-To: <201309202320.15717.marex@denx.de> References: <1379647780-2623-1-git-send-email-troy.kisky@boundarydevices.com> <201309202153.44729.marex@denx.de> <523CBAEE.4080709@boundarydevices.com> <201309202320.15717.marex@denx.de> Message-ID: <523CBF20.3010902@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 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. Troy