All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
To: Li Jun <b47624-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: Li Jun <jun.li-KZfg59tc24xl57MIdRCFDg@public.gmane.org>,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	balbi-l0cyMroinI0@public.gmane.org,
	peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	pawel.moll-5wv7dgnIgG8@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	macpaul-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Subject: Re: [PATCH v5 08/23] usb: chipidea: set usb gadeget's otg capabilities
Date: Thu, 18 Jun 2015 15:06:27 +0300	[thread overview]
Message-ID: <20150618150627.d7e81ca1d6bdf7dba94c4eff@ti.com> (raw)
In-Reply-To: <20150618082029.GB15280@shlinux2>

On Thu, 18 Jun 2015 16:20:30 +0800
Li Jun <b47624-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:

> On Thu, Jun 18, 2015 at 10:39:35AM +0300, Roger Quadros wrote:
> > Typo in subject
> > gadeget's -> gadget's
> > 
> > On Wed, 17 Jun 2015 19:40:15 +0800
> > Li Jun <jun.li-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > 
> > > Set gadget's otg capabilities according to controller's capability and otg
> > > properties in device tree.
> > > 
> > > Signed-off-by: Li Jun <jun.li-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > ---
> > >  drivers/usb/chipidea/core.c  | 8 ++++++++
> > >  drivers/usb/chipidea/udc.c   | 7 ++++++-
> > >  include/linux/usb/chipidea.h | 1 +
> > >  3 files changed, 15 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > > index 74fea4f..2abecbc 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -588,6 +588,14 @@ static int ci_get_platdata(struct device *dev,
> > >  				of_usb_host_tpl_support(dev->of_node);
> > >  	}
> > >  
> > > +	if (platdata->dr_mode == USB_DR_MODE_OTG) {
> > > +		/* We can support HNP and SRP */
> > > +		platdata->ci_otg_caps.hnp_support = true;
> > > +		platdata->ci_otg_caps.srp_support = true;
> > > +		/* Update otg capabilities by DT properties */
> > > +		of_usb_set_otg_caps(dev->of_node, &platdata->ci_otg_caps);
> > 
> > we're not tackling the legacy platform case.
> > i.e. when otg-rev is not present in DT.
> > 
> Legacy platforms with chipidea controller can use otg HNP and SRP
> without any otg properties passed, so I keep hnp and srp to be true.

I was thinking that controller should set whatever it supports regardless of
legacy platforms.
e.g. if chipidea controller supports ADP then it should set it to true here
before calling of_usb_update_otg_caps().

And it should also set the best OTG version it supports and not leave it to 0.

of_usb_update_otg_caps() should then take the decision based on
controller otg_caps and DT otg flags as to what must be done.

> 
> > one way could be that of_usb_set_otg_caps sets otg-rev to 0 if
> > it is not present in DT.
> > 
> otg-rev should be 0 as initial value if it's not present in DT, isn't it?

That's because you didn't set the otg-rev that chipidea controller supports.

This will break for example if DT passes otg-rev 3.0 and controller supports
only otg-rev 2.0 but we didn't bother to check :)

Let's do whatever common functionality we can in of_usb_update_otg_caps()
rather than depending on controller drivers and inviting more errors.

cheers,
-roger

> 
> > > +	}
> > > +
> > >  	if (of_usb_get_maximum_speed(dev->of_node) == USB_SPEED_FULL)
> > >  		platdata->flags |= CI_HDRC_FORCE_FULLSPEED;
> > >  
> > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > > index 764f668..858bd21 100644
> > > --- a/drivers/usb/chipidea/udc.c
> > > +++ b/drivers/usb/chipidea/udc.c
> > > @@ -1827,6 +1827,7 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci)
> > >  static int udc_start(struct ci_hdrc *ci)
> > >  {
> > >  	struct device *dev = ci->dev;
> > > +	struct usb_otg_caps *otg_caps = &ci->platdata->ci_otg_caps;
> > >  	int retval = 0;
> > >  
> > >  	spin_lock_init(&ci->lock);
> > > @@ -1834,8 +1835,12 @@ static int udc_start(struct ci_hdrc *ci)
> > >  	ci->gadget.ops          = &usb_gadget_ops;
> > >  	ci->gadget.speed        = USB_SPEED_UNKNOWN;
> > >  	ci->gadget.max_speed    = USB_SPEED_HIGH;
> > > -	ci->gadget.is_otg       = ci->is_otg ? 1 : 0;
> > >  	ci->gadget.name         = ci->platdata->name;
> > > +	ci->gadget.otg_caps	= otg_caps;
> > > +
> > > +	if (otg_caps->otg_rev && (otg_caps->hnp_support ||
> > > +		otg_caps->srp_support || otg_caps->adp_support))
> > > +		ci->gadget.is_otg = 1;
> > >  
> > >  	INIT_LIST_HEAD(&ci->gadget.ep_list);
> > >  
> > > diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
> > > index ab94f78..e10cefc 100644
> > > --- a/include/linux/usb/chipidea.h
> > > +++ b/include/linux/usb/chipidea.h
> > > @@ -34,6 +34,7 @@ struct ci_hdrc_platform_data {
> > >  #define CI_HDRC_CONTROLLER_STOPPED_EVENT	1
> > >  	void	(*notify_event) (struct ci_hdrc *ci, unsigned event);
> > >  	struct regulator	*reg_vbus;
> > > +	struct usb_otg_caps	ci_otg_caps;
> > >  	bool			tpl_support;
> > >  };
> > >  
> > > -- 
> > > 1.9.1
> > > 
> > 
> > cheers,
> > -roger
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2015-06-18 12:06 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-17 11:40 [PATCH v5 00/23] usb gadget update for OTG 2.0 Li Jun
     [not found] ` <1434541230-15090-1-git-send-email-jun.li-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
2015-06-17 11:40   ` [PATCH v5 01/23] usb: add usb_otg20_descriptor for OTG 2.0 and above Li Jun
2015-06-17 11:40   ` [PATCH v5 02/23] usb: add USB_OTG_ADP definition Li Jun
2015-06-17 11:40   ` [PATCH v5 03/23] usb: otg: add usb_otg_caps structure for otg capabilities Li Jun
2015-06-17 11:40   ` [PATCH v5 04/23] usb: add usb_otg_caps to usb_gadget structure Li Jun
2015-06-17 11:40   ` [PATCH v5 05/23] usb: gadget: composite: add USB_DT_OTG request handling Li Jun
2015-06-17 11:40   ` [PATCH v5 06/23] doc: dt-binding: usb: add otg related properties Li Jun
2015-06-17 11:40   ` [PATCH v5 07/23] usb: common: add API to set usb otg capabilities by device tree Li Jun
2015-06-17 11:40   ` [PATCH v5 08/23] usb: chipidea: set usb gadeget's otg capabilities Li Jun
     [not found]     ` <1434541230-15090-9-git-send-email-jun.li-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
2015-06-18  7:39       ` Roger Quadros
     [not found]         ` <20150618103935.cfbd94c8c37c5c3298c55272-l0cyMroinI0@public.gmane.org>
2015-06-18  8:20           ` Li Jun
2015-06-18 12:06             ` Roger Quadros [this message]
     [not found]               ` <20150618150627.d7e81ca1d6bdf7dba94c4eff-l0cyMroinI0@public.gmane.org>
2015-06-18 13:10                 ` Li Jun
2015-06-24  3:04       ` Peter Chen
2015-06-24  3:34         ` Li Jun
2015-06-24  3:49           ` Peter Chen
2015-06-24  7:22             ` Li Jun
2015-06-24  7:44               ` Peter Chen
2015-06-17 11:40   ` [PATCH v5 09/23] usb: chipidea: update ci_otg_is_fsm_mode conditions Li Jun
2015-06-17 11:40   ` [PATCH v5 10/23] usb: gadget: add usb otg descriptor allocate and init interface Li Jun
2015-06-17 11:40   ` [PATCH v5 11/23] usb: gadget: configfs: allocate and init otg descriptor by otg capabilities Li Jun
2015-06-17 11:40   ` [PATCH v5 12/23] usb: gadget: ether: " Li Jun
     [not found]     ` <1434541230-15090-13-git-send-email-jun.li-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
2015-06-18  7:43       ` Roger Quadros
     [not found]         ` <20150618104302.3da49bc854834b0257e3f703-l0cyMroinI0@public.gmane.org>
2015-06-18  7:51           ` Roger Quadros
     [not found]             ` <20150618105148.cbf486831982d0244d499461-l0cyMroinI0@public.gmane.org>
2015-06-18  8:30               ` Li Jun
2015-06-17 11:40   ` [PATCH v5 13/23] usb: gadget: acm_ms: " Li Jun
2015-06-17 11:40   ` [PATCH v5 14/23] usb: gadget: audio: " Li Jun
2015-06-17 11:40   ` [PATCH v5 15/23] usb: gadget: cdc2: " Li Jun
2015-06-17 11:40   ` [PATCH v5 16/23] usb: gadget: g_ffs: " Li Jun
2015-06-17 11:40   ` [PATCH v5 17/23] usb: gadget: hid: " Li Jun
2015-06-17 11:40   ` [PATCH v5 18/23] usb: gadget: mass_storage: " Li Jun
2015-06-17 11:40   ` [PATCH v5 19/23] usb: gadget: multi: " Li Jun
2015-06-17 11:40   ` [PATCH v5 20/23] usb: gadget: ncm: " Li Jun
2015-06-17 11:40   ` [PATCH v5 21/23] usb: gadget: printer: " Li Jun
2015-06-17 11:40   ` [PATCH v5 22/23] usb: gadget: serial: " Li Jun
2015-06-17 11:40   ` [PATCH v5 23/23] usb: gadget: zero: " Li Jun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150618150627.d7e81ca1d6bdf7dba94c4eff@ti.com \
    --to=rogerq-l0cymroini0@public.gmane.org \
    --cc=b47624-KZfg59tc24xl57MIdRCFDg@public.gmane.org \
    --cc=balbi-l0cyMroinI0@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
    --cc=jun.li-KZfg59tc24xl57MIdRCFDg@public.gmane.org \
    --cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=macpaul-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
    --cc=peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.