On Tue, Mar 17, 2015 at 12:09:22PM -0500, Felipe Balbi wrote:
> 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 ?
>
gadget_is_otg_13 is another big topic. At otg 2.0 spec, it introduces bcdOTG
entry at descriptor to indicate otg release number, bcdOTG should be
controller driver property, some driver follows 1.3 spec, some may follow 2.0
spec or later.
But currently, we define otg descriptor at gadget driver, and it defines
bmAttributes as USB_OTG_SRP | USB_OTG_HNP unconditionally. From my
point, both bmAttributes and bcdOTG should be decided by controller
driver, we can have several entries at struct usb_gadget to indicate
these, and fill the otg descriptor at gadget driver.
I go back to review Jun's hnp polling patches, your suggestion (add
hpn_polling_support flag at struct usb_gadget) is more suitable, the
legacy otg device should stall polling request instead of responding it.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
