Re: [RFC][PATCH v2 00/13] USB: OTG/DRD Core functionality

[Roger Quadros <rogerq@ti.com>]

On 23/04/15 04:52, Peter Chen wrote:
> On Wed, Apr 22, 2015 at 03:42:32PM +0300, Roger Quadros wrote:
>>> So we will have a separate drd fsm file, and the CONFIG_USB_OTG
>>> and CONFIG_USB_OTG_FSM are not needed to be defined, right?
>>>
>>
>> for drd case CONFIG_USB_OTG_FSM is definitely not needed.
>> I'm not sure if we can operate dual-role without CONFIG_USB_OTG.
>> I was thinking of combining the OTG core functionality (drivers/usb/common/usb-otg.c)
>> with CONFIG_USB_OTG.
>>
> 
> Ok, let's choose CONFIG_USB_OTG for both drd and usb fsm case.
> And we need to patch for hcd that only hnp supported hcd needs
> to request otg descriptor, etc.

Agreed. It makes things much simpler.

> 
> For yesterday's back-compatible old otg device, we can add otg
> features to these drivers, the current behavior for these drivers
> is: if CONFIG_USB_OTG is defined, it is an otg device, we can just
> keep the behavior unchanging. If these drivers need to use OTG framework
> in future, it needs to update its platform data or dts.

Right.

> 
> So, I prefer:
> 
> - For switching the role through the ID pin devices, we doesn't need any otg
> features, so no otg dts properties are needed.(expect dr_mode = "otg")
> - For adp/srp/hnp supported devices, we need (partial) otg features, and
> the fsm (hardware or software) are needed, we need some otg dts
> properties we discussed before.

Agreed.

I will try to incorporate these points in v3 of this series.

cheers,
-roger
> 
> 
>>>
>>> static const struct usb_descriptor_header *otg_desc_20[] = {
>>> 	(struct usb_descriptor_header *) &(struct usb_otg_descriptor_20){
>>> 		.bLength =		sizeof(struct usb_otg_descriptor_20),
>>> 		.bDescriptorType =	USB_DT_OTG,
>>>
>>> 		/*
>>> 		 * REVISIT SRP-only hardware is possible, although
>>> 		 * it would not be called "OTG" ...
>>> 		 */
>>> 		.bmAttributes =		USB_OTG_SRP | USB_OTG_HNP,
>>> 		.bcdOTG =               cpu_to_le16(0x0200),
>>> 	},
>>> 	NULL,
>>> };
>>
>> instead of hardcoding bmAttributes field, it must be obtained from the
>> appropriate data structure that was set by the controller driver
>> by reading DT and OTG hardware info.
>>
>>>
>>> During bind process:
>>>
>>> 	if (gadget_is_otg_13(c->cdev->gadget))
>>> 		c->descriptors = otg_desc_13;
>>> 	else if (gadget_is_otg_20(c->cdev->gadget))
>>> 		c->descriptors = otg_desc_20;
>>
>> Probably a helper utitily can do the necessary checks and build the
>> otg descriptor for us.
>> 		usb_otg_get_descriptor(c->descriptors);
>>
>> e.g. for OTG3.0 we have a new flag USB_OTG_RSP, and this helper
>> can be upgraded in the future.
> 
> ok, it is the implementation detail.
>