On Tue, Mar 17, 2015 at 10:06:44AM +0800, Peter Chen wrote:
> On Mon, Mar 16, 2015 at 12:21:53PM -0500, Felipe Balbi wrote:
> > On Mon, Mar 16, 2015 at 05:34:43PM +0800, Li Jun wrote:
> > > On Mon, Mar 16, 2015 at 05:03:17PM +0800, Peter Chen wrote:
> > > > On Mon, Mar 16, 2015 at 04:15:22PM +0800, Li Jun wrote:
> > > > > On Mon, Mar 16, 2015 at 09:44:30AM +0800, Peter Chen wrote:
> > > > > > On Fri, Mar 13, 2015 at 10:34:59AM -0500, Felipe Balbi wrote:
> > > > > > > On Fri, Mar 13, 2015 at 11:13:11AM +0800, Peter Chen wrote:
> > > > > > > > On Thu, Mar 12, 2015 at 11:04:09AM -0500, Felipe Balbi wrote:
> > > > > > > > > 
> > > > > > > > > this could still be done generically in composite.c
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > I suggested it at v1, but after thinking more, we have handled
> > > > > > > > DEVICE request type at individual udc driver, like remote 
> > > > > > > > wakeup,
> > > > > > > > self-power support, so it is reasonable we handle get_status 
> > > > > > > > that
> > > > > > > > if we support hnp polling at udc driver, since it is controller
> > > > > > > > driver's capabilities.
> > > > > > > 
> > > > > > > right, right.. it is controller's capabilities, but all 
> > > > > > > controller needs
> > > > > > > to do is a set a flag in struct usb_gadget, which it already 
> > > > > > > does. Then,
> > > > > > > every single udc will get free implementation after setting that 
> > > > > > > flag,
> > > > > > > right ?
> > > > > > > 
> > > > > > 
> > > > > > Great, then the udc driver which set b_hnp_enable can get the hnp
> > > > > > polling capabilities automatically, in fact, hnp polling support
> > > > > > is a software implement, I don't think it will affect old v1.3 otg
> > > > > > driver.
> > > > > > 
> > > > > 
> > > > > Existing flags in usb_gadget cannot exactly be used for judge if HNP 
> > > > > polling
> > > > > is supported or not, support HNP does not necessarily means HNP 
> > > > > polling is
> > > > > supported, in current code you can see b_hnp_enable is set for some 
> > > > > controllers
> > > > > but they don't support HNP polling yet;
> > > > 
> > > > Why HNP polling can't apply for all hnp enabled gadget? It is just a
> > > > software feature, once the gadget is at device mode, it should be
> > > > ready for hnp, meanwhile, you have already added host_request_flag
> > > > at usb_gadget which the user can choose when to enable hnp polling.
> > > > 
> > > 
> > > It can, just because in current code, some controllers support HNP but do 
> > > not
> > > use OTG FSM driver, in this case I say they do not support HNP polling but
> > > b_hnp_enable is set.
> > 
> > then add a different flag, if you have to in order to maintain backwards
> > compatibility. The fact that hnp polling is a completely SW
> > implementation remains and we don't want to duplicate that all over the
> > place.
> > 
> 
> Felipe/Jun, since otg spec has changed, how about adding version check in code
> first, it can avoid we add some feature selectors which is not defined
> at otg spec, see:
> 
> composite.c
> 
> if (gadget_is_otg(gadget)) {
>       if (gadget_is_otg_13(gadget)) {
>               if (gadget->b_hnp_support)
>                       gadget->host_requst_flag = true;
>       } else if (gadget_is_otg_20(gadget)) {
>                       gadget->host_requst_flag = true;
>       }
> }
> 
> Then, every otg driver can use hnp polling, and 1.3 otg b device
> can work with 2.0 otg a device well.

sounds reasonable to me. But where would gadget_is_otg_13() be defined ?

-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to