Re: [PATCH v4 07/13] usb: otg: add OTG core

[Peter Chen]

On Wed, Sep 09, 2015 at 01:21:50PM +0300, Roger Quadros wrote:
> On 09/09/15 11:45, Peter Chen wrote:
> > On Wed, Sep 09, 2015 at 12:33:20PM +0300, Roger Quadros wrote:
> >> On 09/09/15 11:13, Peter Chen wrote:
> >>> On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote:
>  On 09/09/15 05:21, Peter Chen wrote:
> > On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:
> >>
> >>
> >> On 08/09/15 11:31, Peter Chen wrote:
> >>> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
>  On 07/09/15 04:23, Peter Chen wrote:
> > On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
> >> + * This is used by the USB Host stack to register the Host 
> >> controller
> >> + * to the OTG core. Host controller must not be started by the
> >> + * caller as it is left upto the OTG state machine to do so.
> >> + *
> >> + * Returns: 0 on success, error value otherwise.
> >> + */
> >> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> >> +   unsigned long irqflags, struct otg_hcd_ops 
> >> *ops)
> >> +{
> >> +  struct usb_otg *otgd;
> >> +  struct device *hcd_dev = hcd->self.controller;
> >> +  struct device *otg_dev = usb_otg_get_device(hcd_dev);
> >> +
> >
> > One big problem here is: there are two designs for current (IP) 
> > driver
> > code, one creates dedicated hcd device as roothub's parent, like 
> > dwc3.
> > Another one doesn't do this, roothub's parent is core device (or 
> > otg device
> > in your patch), like chipidea and dwc2.
> >
> > Then, otg_dev will be glue layer device for chipidea after that.
> 
>  OK. Let's add a way for the otg controller driver to provide the 
>  host and gadget
>  information to the otg core for such devices like chipidea and dwc2.
> 
> >>>
> >>> Roger, not only chipidea and dwc2, I think the musb uses the same
> >>> hierarchy. If the host, device, and otg share the same register
> >>> region, host part can't be a platform driver since we don't want
> >>> to remap the same register region again.
> >>>
> >>> So, in the design, we may need to consider both situations, one
> >>> is otg/host/device has its own register region, and host is a
> >>> separate platform device (A), the other is three parts share the
> >>> same register region, there is only one platform driver (B).
> >>>
> >>> A:
> >>>
> >>>   IP core device 
> >>>   |
> >>>   |
> >>> |-|-|
> >>> gadget   host platform device 
> >>>   |
> >>>   roothub
> >>>
> >>> B:
> >>>
> >>>   IP core device
> >>>   |
> >>>   |
> >>> |-|-|
> >>> gadget roothub
> >>>   
> >>>
>  This API must be called before the hcd/gadget-driver is added so 
>  that the otg
>  core knows it's linked to an OTG controller.
> 
>  Any better idea?
> 
> >>>
> >>> A flag stands for this hcd controller is the same with otg controller
> >>> can be used, this flag can be stored at struct usb_otg_config.
> >>
> >> What if there is another architecture like so?
> >>
> >> C:
> >>[Parent]
> >>   |
> >>   |
> >>|--|--|
> >>[OTG core]  [gadget][host]
> >>
> >> We need a more flexible mechanism to link the gadget and
> >> host device to the otg core for non DT case.
> >>
> >> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
> >>
> >> e.g.
> >> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
> >>
> >> If otg is NULL it will try DT otg-controller property or parent to
> >> get the otg controller.
> >
> > How usb_otg_register_hcd get struct usb_otg, from where?
> 
>  This only works when the parent driver creating the hcd has registered 
>  the
>  otg controller too.
> 
> >>>
> >>> Sorry? So we need to find another way to solve this issue, right?
> >>
> >> For existing cases this is sufficient.
> >> The otg device is either the one supplied during usb_otg_register_hcd
> >> (cases B and C) or it is the parent device (case A).
> > 
> > How we differentiate case A and case B at usb_otg_register_hcd?
> > Would you show me the sample code?
> 
> Case A:

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Li Jun]

On Wed, Sep 09, 2015 at 01:01:14PM +0300, Roger Quadros wrote:
... ...

>  +return -EINVAL;
> >>>
> >>> Return non-zero, then if err, do we need call usb_otg_add_hcd() after
> >>> usb_otg_register_hcd() fails?
> >>
> >> You should not call usb_otg_register_hcd() but usb_add_hcd().
> >> If that fails then you fail as ususal.
> > 
> > My point is if we use usb_add_hcd(), but failed in usb_otg_register_hcd(),
> > then usb_otg_add_hcd() will be called in *all* error case, is this your
> > expectation?
> > if (usb_otg_register_hcd(hcd, irqnum, irqflags, _hcd_intf))
> > return usb_otg_add_hcd(hcd, irqnum, irqflags);
> > 
> 
> Yes, my intention was that if otg fails then it is a non otg HCD so register 
> normally.
> Let me correct my previous statement. If you are absolutely sure
> that the HCD is for otg/dual-role usage then you should call 
> usb_otg_register_hcd().
> 

I think this is not just about a statement, in your usb_otg_register_hcd()
implementation, there are several places will return error, I think only
the first two are for a non-otg HCD case, the following error cases seems
mean this is for otg usage, but it fails in middle of registration, if that
is the case, is it reasonable to call usb_otg_add_hcd()?

>  + * @fsm_running: state machine running/stopped indicator
>  + */
>   struct usb_otg {
>   u8  default_a;
>   
>   struct phy  *phy;
>   /* old usb_phy interface */
>   struct usb_phy  *usb_phy;
>  +
> >>>
> >>> add a blank line?
> >>>
> > 
> > You missed this.
> 
> Sorry. Did you suggest to remove that blank line
> or add a new one before usb_phy?
> 

Remove it.

> > 
>   struct usb_bus  *host;
>   struct usb_gadget   *gadget;
>   
>   enum usb_otg_state  state;
>   
>  +struct device *dev;
>  +struct usb_otg_caps *caps;
>  +struct otg_fsm fsm;
>  +struct otg_fsm_ops fsm_ops;
>  +
>  +/* internal use only */
>  +struct otg_hcd primary_hcd;
>  +struct otg_hcd shared_hcd;
>  +struct otg_gadget_ops *gadget_ops;
>  +struct otg_timer timers[NUM_OTG_FSM_TIMERS];
>  +struct list_head list;
>  +struct work_struct work;
>  -- 
>  2.1.4
> > 
> 
> --
> cheers,
> -roger
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

On 10/09/15 08:35, Peter Chen wrote:
> On Wed, Sep 09, 2015 at 01:21:50PM +0300, Roger Quadros wrote:
>> On 09/09/15 11:45, Peter Chen wrote:
>>> On Wed, Sep 09, 2015 at 12:33:20PM +0300, Roger Quadros wrote:
 On 09/09/15 11:13, Peter Chen wrote:
> On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote:
>> On 09/09/15 05:21, Peter Chen wrote:
>>> On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:


 On 08/09/15 11:31, Peter Chen wrote:
> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
>> On 07/09/15 04:23, Peter Chen wrote:
>>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
 + * This is used by the USB Host stack to register the Host 
 controller
 + * to the OTG core. Host controller must not be started by the
 + * caller as it is left upto the OTG state machine to do so.
 + *
 + * Returns: 0 on success, error value otherwise.
 + */
 +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
 +   unsigned long irqflags, struct otg_hcd_ops 
 *ops)
 +{
 +  struct usb_otg *otgd;
 +  struct device *hcd_dev = hcd->self.controller;
 +  struct device *otg_dev = usb_otg_get_device(hcd_dev);
 +
>>>
>>> One big problem here is: there are two designs for current (IP) 
>>> driver
>>> code, one creates dedicated hcd device as roothub's parent, like 
>>> dwc3.
>>> Another one doesn't do this, roothub's parent is core device (or 
>>> otg device
>>> in your patch), like chipidea and dwc2.
>>>
>>> Then, otg_dev will be glue layer device for chipidea after that.
>>
>> OK. Let's add a way for the otg controller driver to provide the 
>> host and gadget
>> information to the otg core for such devices like chipidea and dwc2.
>>
>
> Roger, not only chipidea and dwc2, I think the musb uses the same
> hierarchy. If the host, device, and otg share the same register
> region, host part can't be a platform driver since we don't want
> to remap the same register region again.
>
> So, in the design, we may need to consider both situations, one
> is otg/host/device has its own register region, and host is a
> separate platform device (A), the other is three parts share the
> same register region, there is only one platform driver (B).
>
> A:
>
>   IP core device 
>   |
>   |
> |-|-|
> gadget   host platform device 
>   |
>   roothub
>
> B:
>
>   IP core device
>   |
>   |
> |-|-|
> gadget roothub
>   
>
>> This API must be called before the hcd/gadget-driver is added so 
>> that the otg
>> core knows it's linked to an OTG controller.
>>
>> Any better idea?
>>
>
> A flag stands for this hcd controller is the same with otg controller
> can be used, this flag can be stored at struct usb_otg_config.

 What if there is another architecture like so?

 C:
[Parent]
   |
   |
|--|--|
[OTG core]  [gadget][host]

 We need a more flexible mechanism to link the gadget and
 host device to the otg core for non DT case.

 How about adding struct usb_otg parameter to usb_otg_register_hcd()?

 e.g.
 int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)

 If otg is NULL it will try DT otg-controller property or parent to
 get the otg controller.
>>>
>>> How usb_otg_register_hcd get struct usb_otg, from where?
>>
>> This only works when the parent driver creating the hcd has registered 
>> the
>> otg controller too.
>>
>
> Sorry? So we need to find another way to solve this issue, right?

 For existing cases this is sufficient.
 The otg device is either the one supplied during usb_otg_register_hcd
 (cases B and C) or it is the parent device (case A).
>>>
>>> How we differentiate case A and case B at usb_otg_register_hcd?
>>> Would you 

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

On 10/09/15 12:28, Li Jun wrote:
> On Wed, Sep 09, 2015 at 01:01:14PM +0300, Roger Quadros wrote:
> ... ...
> 
>> +return -EINVAL;
>
> Return non-zero, then if err, do we need call usb_otg_add_hcd() after
> usb_otg_register_hcd() fails?

 You should not call usb_otg_register_hcd() but usb_add_hcd().
 If that fails then you fail as ususal.
>>>
>>> My point is if we use usb_add_hcd(), but failed in usb_otg_register_hcd(),
>>> then usb_otg_add_hcd() will be called in *all* error case, is this your
>>> expectation?
>>> if (usb_otg_register_hcd(hcd, irqnum, irqflags, _hcd_intf))
>>> return usb_otg_add_hcd(hcd, irqnum, irqflags);
>>>
>>
>> Yes, my intention was that if otg fails then it is a non otg HCD so register 
>> normally.
>> Let me correct my previous statement. If you are absolutely sure
>> that the HCD is for otg/dual-role usage then you should call 
>> usb_otg_register_hcd().
>>
> 
> I think this is not just about a statement, in your usb_otg_register_hcd()
> implementation, there are several places will return error, I think only
> the first two are for a non-otg HCD case, the following error cases seems
> mean this is for otg usage, but it fails in middle of registration, if that
> is the case, is it reasonable to call usb_otg_add_hcd()?

OK. We need to check the return value then and differentiate if it is non-otg
or otg with failure.
If it is non-otg then only we must call usb_otg_add_hcd().

I will fix usb_add_hcd().

> 
>> + * @fsm_running: state machine running/stopped indicator
>> + */
>>  struct usb_otg {
>>  u8  default_a;
>>  
>>  struct phy  *phy;
>>  /* old usb_phy interface */
>>  struct usb_phy  *usb_phy;
>> +
>
> add a blank line?
>
>>>
>>> You missed this.
>>
>> Sorry. Did you suggest to remove that blank line
>> or add a new one before usb_phy?
>>
> 
> Remove it.

OK.

> 
>>>
>>  struct usb_bus  *host;
>>  struct usb_gadget   *gadget;
>>  
>>  enum usb_otg_state  state;
>>  
>> +struct device *dev;
>> +struct usb_otg_caps *caps;
>> +struct otg_fsm fsm;
>> +struct otg_fsm_ops fsm_ops;
>> +
>> +/* internal use only */
>> +struct otg_hcd primary_hcd;
>> +struct otg_hcd shared_hcd;
>> +struct otg_gadget_ops *gadget_ops;
>> +struct otg_timer timers[NUM_OTG_FSM_TIMERS];
>> +struct list_head list;
>> +struct work_struct work;
>> -- 
>> 2.1.4
>>>
--
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Peter Chen]

On Thu, Sep 10, 2015 at 05:17:36PM +0300, Roger Quadros wrote:
> On 10/09/15 08:35, Peter Chen wrote:
> > On Wed, Sep 09, 2015 at 01:21:50PM +0300, Roger Quadros wrote:
> >> On 09/09/15 11:45, Peter Chen wrote:
> >>> On Wed, Sep 09, 2015 at 12:33:20PM +0300, Roger Quadros wrote:
>  On 09/09/15 11:13, Peter Chen wrote:
> > On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote:
> >> On 09/09/15 05:21, Peter Chen wrote:
> >>> On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:
> 
> 
>  On 08/09/15 11:31, Peter Chen wrote:
> > On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
> >> On 07/09/15 04:23, Peter Chen wrote:
> >>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
>  + * This is used by the USB Host stack to register the Host 
>  controller
>  + * to the OTG core. Host controller must not be started by the
>  + * caller as it is left upto the OTG state machine to do so.
>  + *
>  + * Returns: 0 on success, error value otherwise.
>  + */
>  +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int 
>  irqnum,
>  + unsigned long irqflags, struct 
>  otg_hcd_ops *ops)
>  +{
>  +struct usb_otg *otgd;
>  +struct device *hcd_dev = hcd->self.controller;
>  +struct device *otg_dev = usb_otg_get_device(hcd_dev);
>  +
> >>>
> >>> One big problem here is: there are two designs for current (IP) 
> >>> driver
> >>> code, one creates dedicated hcd device as roothub's parent, like 
> >>> dwc3.
> >>> Another one doesn't do this, roothub's parent is core device (or 
> >>> otg device
> >>> in your patch), like chipidea and dwc2.
> >>>
> >>> Then, otg_dev will be glue layer device for chipidea after that.
> >>
> >> OK. Let's add a way for the otg controller driver to provide the 
> >> host and gadget
> >> information to the otg core for such devices like chipidea and 
> >> dwc2.
> >>
> >
> > Roger, not only chipidea and dwc2, I think the musb uses the same
> > hierarchy. If the host, device, and otg share the same register
> > region, host part can't be a platform driver since we don't want
> > to remap the same register region again.
> >
> > So, in the design, we may need to consider both situations, one
> > is otg/host/device has its own register region, and host is a
> > separate platform device (A), the other is three parts share the
> > same register region, there is only one platform driver (B).
> >
> > A:
> >
> > IP core device 
> > |
> > |
> >   |-|-|
> >   gadget   host platform device 
> > |
> > roothub
> >
> > B:
> >
> > IP core device
> > |
> > |
> >   |-|-|
> >   gadget roothub
> > 
> >
> >> This API must be called before the hcd/gadget-driver is added so 
> >> that the otg
> >> core knows it's linked to an OTG controller.
> >>
> >> Any better idea?
> >>
> >
> > A flag stands for this hcd controller is the same with otg 
> > controller
> > can be used, this flag can be stored at struct usb_otg_config.
> 
>  What if there is another architecture like so?
> 
>  C:
>   [Parent]
>  |
>  |
>   |--|--|
>   [OTG core]  [gadget][host]
> 
>  We need a more flexible mechanism to link the gadget and
>  host device to the otg core for non DT case.
> 
>  How about adding struct usb_otg parameter to usb_otg_register_hcd()?
> 
>  e.g.
>  int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, 
>  ..)
> 
>  If otg is NULL it will try DT otg-controller property or parent to
>  get the otg controller.
> >>>
> >>> How usb_otg_register_hcd get struct usb_otg, from where?
> >>
> >> This only works when the parent driver creating the hcd has registered 
> >> the
> >> otg controller too.
> >>
> >
> > 

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Li Jun]

On Mon, Sep 07, 2015 at 01:53:19PM +0300, Roger Quadros wrote:
> On 07/09/15 10:40, Li Jun wrote:
> > On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
> >> The OTG core instantiates the OTG Finite State Machine
> >> per OTG controller and manages starting/stopping the
> >> host and gadget controllers based on the bus state.
> >>
> >> It provides APIs for the following tasks
> >>
> >> - Registering an OTG capable controller
> >> - Registering Host and Gadget controllers to OTG core
> >> - Providing inputs to and kicking the OTG state machine
> >>
> >> Signed-off-by: Roger Quadros 
> >> ---
> >>  MAINTAINERS  |4 +-
> >>  drivers/usb/Kconfig  |2 +-
> >>  drivers/usb/Makefile |1 +
> >>  drivers/usb/common/Makefile  |3 +-
> >>  drivers/usb/common/usb-otg.c | 1061 
> >> ++
> >>  drivers/usb/common/usb-otg.h |   71 +++
> >>  drivers/usb/core/Kconfig |   11 +-
> >>  include/linux/usb/otg.h  |  189 +++-
> >>  8 files changed, 1321 insertions(+), 21 deletions(-)
> >>  create mode 100644 drivers/usb/common/usb-otg.c
> >>  create mode 100644 drivers/usb/common/usb-otg.h
> >>
> > 
> > ... ...
> > 
> >> +
> >> +/**
> >> + * Get OTG device from host or gadget device.
> >> + *
> >> + * For non device tree boot, the OTG controller is assumed to be
> >> + * the parent of the host/gadget device.
> > 
> > This assumption/restriction maybe a problem, as I pointed in your previous
> > version, usb_create_hcd() use the passed dev as its dev, but,
> > usb_add_gadget_udc() use the passed dev as its parent dev, so often the
> > host and gadget don't share the same parent device, at least it doesn't
> > apply to chipidea case.
> 
> Let's provide a way for OTG driver to provide the OTG core exactly which is
> the related host/gadget device.
> 
> > 
> >> + * For device tree boot, the OTG controller is derived from the
> >> + * "otg-controller" property.
> >> + */
> >> +static struct device *usb_otg_get_device(struct device *hcd_gcd_dev)
> >> +{
> >> +  struct device *otg_dev;
> >> +
> >> +  if (!hcd_gcd_dev)
> >> +  return NULL;
> >> +
> >> +  if (hcd_gcd_dev->of_node) {
> >> +  struct device_node *np;
> >> +  struct platform_device *pdev;
> >> +
> >> +  np = of_parse_phandle(hcd_gcd_dev->of_node, "otg-controller",
> >> +0);
> >> +  if (!np)
> >> +  goto legacy;/* continue legacy way */
> >> +
> >> +  pdev = of_find_device_by_node(np);
> >> +  of_node_put(np);
> >> +  if (!pdev) {
> >> +  dev_err(>dev, "couldn't get otg-controller 
> >> device\n");
> >> +  return NULL;
> >> +  }
> >> +
> >> +  otg_dev = >dev;
> >> +  return otg_dev;
> >> +  }
> >> +
> >> +legacy:
> >> +  /* otg device is parent and must be registered */
> >> +  otg_dev = hcd_gcd_dev->parent;
> >> +  if (!usb_otg_get_data(otg_dev))
> >> +  return NULL;
> >> +
> >> +  return otg_dev;
> >> +}
> >> +
> > 
> > ... ...
> > 
> >> +static void usb_otg_init_timers(struct usb_otg *otgd, unsigned *timeouts)
> >> +{
> >> +  struct otg_fsm *fsm = >fsm;
> >> +  unsigned long tmouts[NUM_OTG_FSM_TIMERS];
> >> +  int i;
> >> +
> >> +  /* set default timeouts */
> >> +  tmouts[A_WAIT_VRISE] = TA_WAIT_VRISE;
> >> +  tmouts[A_WAIT_VFALL] = TA_WAIT_VFALL;
> >> +  tmouts[A_WAIT_BCON] = TA_WAIT_BCON;
> >> +  tmouts[A_AIDL_BDIS] = TA_AIDL_BDIS;
> >> +  tmouts[A_BIDL_ADIS] = TA_BIDL_ADIS;
> >> +  tmouts[B_ASE0_BRST] = TB_ASE0_BRST;
> >> +  tmouts[B_SE0_SRP] = TB_SE0_SRP;
> >> +  tmouts[B_SRP_FAIL] = TB_SRP_FAIL;
> >> +
> >> +  /* set controller provided timeouts */
> >> +  if (timeouts) {
> >> +  for (i = 0; i < NUM_OTG_FSM_TIMERS; i++) {
> >> +  if (timeouts[i])
> >> +  tmouts[i] = timeouts[i];
> >> +  }
> >> +  }
> >> +
> >> +  otg_timer_init(A_WAIT_VRISE, otgd, set_tmout, TA_WAIT_VRISE,
> >> + >a_wait_vrise_tmout);
> >> +  otg_timer_init(A_WAIT_VFALL, otgd, set_tmout, TA_WAIT_VFALL,
> >> + >a_wait_vfall_tmout);
> >> +  otg_timer_init(A_WAIT_BCON, otgd, set_tmout, TA_WAIT_BCON,
> >> + >a_wait_bcon_tmout);
> >> +  otg_timer_init(A_AIDL_BDIS, otgd, set_tmout, TA_AIDL_BDIS,
> >> + >a_aidl_bdis_tmout);
> >> +  otg_timer_init(A_BIDL_ADIS, otgd, set_tmout, TA_BIDL_ADIS,
> >> + >a_bidl_adis_tmout);
> >> +  otg_timer_init(B_ASE0_BRST, otgd, set_tmout, TB_ASE0_BRST,
> >> + >b_ase0_brst_tmout);
> >> +
> >> +  otg_timer_init(B_SE0_SRP, otgd, set_tmout, TB_SE0_SRP,
> >> + >b_se0_srp);
> >> +  otg_timer_init(B_SRP_FAIL, otgd, set_tmout, TB_SRP_FAIL,
> >> + >b_srp_done);
> >> +
> >> +  /* FIXME: what about A_WAIT_ENUM? */
> > 
> > Either you init it as other timers, or you remove all of it, otherwise
> > there will be 

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

On 09/09/15 05:21, Peter Chen wrote:
> On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:
>>
>>
>> On 08/09/15 11:31, Peter Chen wrote:
>>> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
 On 07/09/15 04:23, Peter Chen wrote:
> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
>> + * This is used by the USB Host stack to register the Host controller
>> + * to the OTG core. Host controller must not be started by the
>> + * caller as it is left upto the OTG state machine to do so.
>> + *
>> + * Returns: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>> + unsigned long irqflags, struct otg_hcd_ops 
>> *ops)
>> +{
>> +struct usb_otg *otgd;
>> +struct device *hcd_dev = hcd->self.controller;
>> +struct device *otg_dev = usb_otg_get_device(hcd_dev);
>> +
>
> One big problem here is: there are two designs for current (IP) driver
> code, one creates dedicated hcd device as roothub's parent, like dwc3.
> Another one doesn't do this, roothub's parent is core device (or otg 
> device
> in your patch), like chipidea and dwc2.
>
> Then, otg_dev will be glue layer device for chipidea after that.

 OK. Let's add a way for the otg controller driver to provide the host and 
 gadget
 information to the otg core for such devices like chipidea and dwc2.

>>>
>>> Roger, not only chipidea and dwc2, I think the musb uses the same
>>> hierarchy. If the host, device, and otg share the same register
>>> region, host part can't be a platform driver since we don't want
>>> to remap the same register region again.
>>>
>>> So, in the design, we may need to consider both situations, one
>>> is otg/host/device has its own register region, and host is a
>>> separate platform device (A), the other is three parts share the
>>> same register region, there is only one platform driver (B).
>>>
>>> A:
>>>
>>> IP core device 
>>> |
>>> |
>>>   |-|-|
>>>   gadget   host platform device 
>>> |
>>> roothub
>>>
>>> B:
>>>
>>> IP core device
>>> |
>>> |
>>>   |-|-|
>>>   gadget roothub
>>> 
>>>
 This API must be called before the hcd/gadget-driver is added so that the 
 otg
 core knows it's linked to an OTG controller.

 Any better idea?

>>>
>>> A flag stands for this hcd controller is the same with otg controller
>>> can be used, this flag can be stored at struct usb_otg_config.
>>
>> What if there is another architecture like so?
>>
>> C:
>>  [Parent]
>> |
>> |
>>  |--|--|
>>  [OTG core]  [gadget][host]
>>
>> We need a more flexible mechanism to link the gadget and
>> host device to the otg core for non DT case.
>>
>> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
>>
>> e.g.
>> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
>>
>> If otg is NULL it will try DT otg-controller property or parent to
>> get the otg controller.
> 
> How usb_otg_register_hcd get struct usb_otg, from where?

This only works when the parent driver creating the hcd has registered the
otg controller too.

> 
>>
>>>
>>> Peter
>>>
>>> P.S: I still read your code, I find not all APIs in this file are used
>>> in your dwc3 example. 
>>
>> Which ones? The ones for registering/unregistered host/gadget are used
>> by hcd/udc core as part of usb_add/remove_hcd() and
>> udc_bind_to_driver()/usb_gadget_remove_driver()
>>
> 
> Ok, now I understand your design, usb_create_hcd must be called before
> the fsm kicks off. The call flow like below:
> 
> usb_otg_register->usb_create_hcd->usb_add_hcd->usb_otg_register_hcd->
> usb_otg_start_host->usb_otg_add_hcd

Actually these are the discrete steps
- usb_otg_register
- usb_create_hcd
- usb_add_hcd->usb_otg_register_hcd  (HCD is not really added till FSM in host 
role)

Above 3 are prerequisite for FSM to start in addition to gadget controller and
driver being ready.

When FSM enters in host role
- usb_otg_start_host(true)->usb_otg_add_hcd (Now HCD is added)

When FSM exits host role
- usb_otg_start_host(false)->usb_otg_remove_hcd (Now HCD is removed, but not 
unregistered)

If HCD hardware really goes away
- usb_remove_hcd->usb_otg_unregister_hcd (Now FSM stops as host resource not 
available)

> 
> We need to make some changes to let chipidea work since usb_create_hcd
> is included at host->start.
> 

Yes, you just need to do the usb_add_hcd() in probe
and 

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

On 09/09/15 11:13, Peter Chen wrote:
> On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote:
>> On 09/09/15 05:21, Peter Chen wrote:
>>> On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:


 On 08/09/15 11:31, Peter Chen wrote:
> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
>> On 07/09/15 04:23, Peter Chen wrote:
>>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
 + * This is used by the USB Host stack to register the Host controller
 + * to the OTG core. Host controller must not be started by the
 + * caller as it is left upto the OTG state machine to do so.
 + *
 + * Returns: 0 on success, error value otherwise.
 + */
 +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
 +   unsigned long irqflags, struct otg_hcd_ops 
 *ops)
 +{
 +  struct usb_otg *otgd;
 +  struct device *hcd_dev = hcd->self.controller;
 +  struct device *otg_dev = usb_otg_get_device(hcd_dev);
 +
>>>
>>> One big problem here is: there are two designs for current (IP) driver
>>> code, one creates dedicated hcd device as roothub's parent, like dwc3.
>>> Another one doesn't do this, roothub's parent is core device (or otg 
>>> device
>>> in your patch), like chipidea and dwc2.
>>>
>>> Then, otg_dev will be glue layer device for chipidea after that.
>>
>> OK. Let's add a way for the otg controller driver to provide the host 
>> and gadget
>> information to the otg core for such devices like chipidea and dwc2.
>>
>
> Roger, not only chipidea and dwc2, I think the musb uses the same
> hierarchy. If the host, device, and otg share the same register
> region, host part can't be a platform driver since we don't want
> to remap the same register region again.
>
> So, in the design, we may need to consider both situations, one
> is otg/host/device has its own register region, and host is a
> separate platform device (A), the other is three parts share the
> same register region, there is only one platform driver (B).
>
> A:
>
>   IP core device 
>   |
>   |
> |-|-|
> gadget   host platform device 
>   |
>   roothub
>
> B:
>
>   IP core device
>   |
>   |
> |-|-|
> gadget roothub
>   
>
>> This API must be called before the hcd/gadget-driver is added so that 
>> the otg
>> core knows it's linked to an OTG controller.
>>
>> Any better idea?
>>
>
> A flag stands for this hcd controller is the same with otg controller
> can be used, this flag can be stored at struct usb_otg_config.

 What if there is another architecture like so?

 C:
[Parent]
   |
   |
|--|--|
[OTG core]  [gadget][host]

 We need a more flexible mechanism to link the gadget and
 host device to the otg core for non DT case.

 How about adding struct usb_otg parameter to usb_otg_register_hcd()?

 e.g.
 int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)

 If otg is NULL it will try DT otg-controller property or parent to
 get the otg controller.
>>>
>>> How usb_otg_register_hcd get struct usb_otg, from where?
>>
>> This only works when the parent driver creating the hcd has registered the
>> otg controller too.
>>
> 
> Sorry? So we need to find another way to solve this issue, right?

For existing cases this is sufficient.
The otg device is either the one supplied during usb_otg_register_hcd
(cases B and C) or it is the parent device (case A).

It does not work when the 3 devices are totally independent and get registered
at different times.
I don't think there is such a case for non-DT yet, but let's not have this
limitation. So yes, we need to look for better solution :).

--
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

(adding back folks in cc)

On 08/09/15 20:35, Alan Stern wrote:
> On Tue, 8 Sep 2015, Roger Quadros wrote:
> 
 What if there is another architecture like so?

 C:
[Parent]
   |
   |
|--|--|
[OTG core]  [gadget][host]

 We need a more flexible mechanism to link the gadget and
 host device to the otg core for non DT case.

 How about adding struct usb_otg parameter to usb_otg_register_hcd()?

 e.g.
 int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)

 If otg is NULL it will try DT otg-controller property or parent to
 get the otg controller.
>>>
>>> This seems a lot like something Peter and I discussed recently.  See
>>>
>>> http://marc.info/?l=linux-usb=143977568021328=2
>>>
>>> and the following messages in that thread.
>>>
>>
>> If I understood right, your proposal was to add a usb_pointers data
>> struct to the device's drvdata?
> 
> Right.
> 
>> This is fine only if the otg/gadget/host share the same device.
>> It does not solve the problem where each have different platform devices.
> 
> Why not?  Each of the platform devices can store a pointer to the same 
> usb_pointers structure.  Wouldn't that be suitable?
> 

I didn't understand how all of them get the reference to the
same usb_pointer structure (or contents) when their respective platform devices
get created so that they can store it in their respective drvdata.

Worst case, the 3 devices could be totally independent of each other without a
common parent, and get registered at different times.

The common resource here is the physical USB port for which the 3 drivers
otg/host/gadget are being registered.
There should be a mechanism for each device to tell that it is part of
this particular USB port. (or usb_pointers struct)

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Peter Chen]

On Wed, Sep 09, 2015 at 12:33:20PM +0300, Roger Quadros wrote:
> On 09/09/15 11:13, Peter Chen wrote:
> > On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote:
> >> On 09/09/15 05:21, Peter Chen wrote:
> >>> On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:
> 
> 
>  On 08/09/15 11:31, Peter Chen wrote:
> > On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
> >> On 07/09/15 04:23, Peter Chen wrote:
> >>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
>  + * This is used by the USB Host stack to register the Host 
>  controller
>  + * to the OTG core. Host controller must not be started by the
>  + * caller as it is left upto the OTG state machine to do so.
>  + *
>  + * Returns: 0 on success, error value otherwise.
>  + */
>  +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>  + unsigned long irqflags, struct otg_hcd_ops 
>  *ops)
>  +{
>  +struct usb_otg *otgd;
>  +struct device *hcd_dev = hcd->self.controller;
>  +struct device *otg_dev = usb_otg_get_device(hcd_dev);
>  +
> >>>
> >>> One big problem here is: there are two designs for current (IP) driver
> >>> code, one creates dedicated hcd device as roothub's parent, like dwc3.
> >>> Another one doesn't do this, roothub's parent is core device (or otg 
> >>> device
> >>> in your patch), like chipidea and dwc2.
> >>>
> >>> Then, otg_dev will be glue layer device for chipidea after that.
> >>
> >> OK. Let's add a way for the otg controller driver to provide the host 
> >> and gadget
> >> information to the otg core for such devices like chipidea and dwc2.
> >>
> >
> > Roger, not only chipidea and dwc2, I think the musb uses the same
> > hierarchy. If the host, device, and otg share the same register
> > region, host part can't be a platform driver since we don't want
> > to remap the same register region again.
> >
> > So, in the design, we may need to consider both situations, one
> > is otg/host/device has its own register region, and host is a
> > separate platform device (A), the other is three parts share the
> > same register region, there is only one platform driver (B).
> >
> > A:
> >
> > IP core device 
> > |
> > |
> >   |-|-|
> >   gadget   host platform device 
> > |
> > roothub
> >
> > B:
> >
> > IP core device
> > |
> > |
> >   |-|-|
> >   gadget roothub
> > 
> >
> >> This API must be called before the hcd/gadget-driver is added so that 
> >> the otg
> >> core knows it's linked to an OTG controller.
> >>
> >> Any better idea?
> >>
> >
> > A flag stands for this hcd controller is the same with otg controller
> > can be used, this flag can be stored at struct usb_otg_config.
> 
>  What if there is another architecture like so?
> 
>  C:
>   [Parent]
>  |
>  |
>   |--|--|
>   [OTG core]  [gadget][host]
> 
>  We need a more flexible mechanism to link the gadget and
>  host device to the otg core for non DT case.
> 
>  How about adding struct usb_otg parameter to usb_otg_register_hcd()?
> 
>  e.g.
>  int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
> 
>  If otg is NULL it will try DT otg-controller property or parent to
>  get the otg controller.
> >>>
> >>> How usb_otg_register_hcd get struct usb_otg, from where?
> >>
> >> This only works when the parent driver creating the hcd has registered the
> >> otg controller too.
> >>
> > 
> > Sorry? So we need to find another way to solve this issue, right?
> 
> For existing cases this is sufficient.
> The otg device is either the one supplied during usb_otg_register_hcd
> (cases B and C) or it is the parent device (case A).

How we differentiate case A and case B at usb_otg_register_hcd?
Would you show me the sample code?

> 
> It does not work when the 3 devices are totally independent and get registered
> at different times.
> I don't think there is such a case for non-DT yet, but let's not have this
> limitation. So yes, we need to look for better solution :).
> 

Yes, we need to find some places to store gadget/host/otg information,
Alan's suggestion to save them at device drvdata may be a 

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

On 09/09/15 11:45, Peter Chen wrote:
> On Wed, Sep 09, 2015 at 12:33:20PM +0300, Roger Quadros wrote:
>> On 09/09/15 11:13, Peter Chen wrote:
>>> On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote:
 On 09/09/15 05:21, Peter Chen wrote:
> On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:
>>
>>
>> On 08/09/15 11:31, Peter Chen wrote:
>>> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
 On 07/09/15 04:23, Peter Chen wrote:
> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
>> + * This is used by the USB Host stack to register the Host 
>> controller
>> + * to the OTG core. Host controller must not be started by the
>> + * caller as it is left upto the OTG state machine to do so.
>> + *
>> + * Returns: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>> + unsigned long irqflags, struct otg_hcd_ops 
>> *ops)
>> +{
>> +struct usb_otg *otgd;
>> +struct device *hcd_dev = hcd->self.controller;
>> +struct device *otg_dev = usb_otg_get_device(hcd_dev);
>> +
>
> One big problem here is: there are two designs for current (IP) driver
> code, one creates dedicated hcd device as roothub's parent, like dwc3.
> Another one doesn't do this, roothub's parent is core device (or otg 
> device
> in your patch), like chipidea and dwc2.
>
> Then, otg_dev will be glue layer device for chipidea after that.

 OK. Let's add a way for the otg controller driver to provide the host 
 and gadget
 information to the otg core for such devices like chipidea and dwc2.

>>>
>>> Roger, not only chipidea and dwc2, I think the musb uses the same
>>> hierarchy. If the host, device, and otg share the same register
>>> region, host part can't be a platform driver since we don't want
>>> to remap the same register region again.
>>>
>>> So, in the design, we may need to consider both situations, one
>>> is otg/host/device has its own register region, and host is a
>>> separate platform device (A), the other is three parts share the
>>> same register region, there is only one platform driver (B).
>>>
>>> A:
>>>
>>> IP core device 
>>> |
>>> |
>>>   |-|-|
>>>   gadget   host platform device 
>>> |
>>> roothub
>>>
>>> B:
>>>
>>> IP core device
>>> |
>>> |
>>>   |-|-|
>>>   gadget roothub
>>> 
>>>
 This API must be called before the hcd/gadget-driver is added so that 
 the otg
 core knows it's linked to an OTG controller.

 Any better idea?

>>>
>>> A flag stands for this hcd controller is the same with otg controller
>>> can be used, this flag can be stored at struct usb_otg_config.
>>
>> What if there is another architecture like so?
>>
>> C:
>>  [Parent]
>> |
>> |
>>  |--|--|
>>  [OTG core]  [gadget][host]
>>
>> We need a more flexible mechanism to link the gadget and
>> host device to the otg core for non DT case.
>>
>> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
>>
>> e.g.
>> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
>>
>> If otg is NULL it will try DT otg-controller property or parent to
>> get the otg controller.
>
> How usb_otg_register_hcd get struct usb_otg, from where?

 This only works when the parent driver creating the hcd has registered the
 otg controller too.

>>>
>>> Sorry? So we need to find another way to solve this issue, right?
>>
>> For existing cases this is sufficient.
>> The otg device is either the one supplied during usb_otg_register_hcd
>> (cases B and C) or it is the parent device (case A).
> 
> How we differentiate case A and case B at usb_otg_register_hcd?
> Would you show me the sample code?

Case A:

hcd platform driver doesn't know about otg device so it calls

usb_add_hcd(hcd,..)->usb_otg_register_hcd(NULL, hcd,..);

Case B:

core driver knows about both otg and hcd so it calls
usb_otg_register_hcd(otg, hcd,...);

code:

usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ...)
{
if 

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Peter Chen]

On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote:
> On 09/09/15 05:21, Peter Chen wrote:
> > On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:
> >>
> >>
> >> On 08/09/15 11:31, Peter Chen wrote:
> >>> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
>  On 07/09/15 04:23, Peter Chen wrote:
> > On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
> >> + * This is used by the USB Host stack to register the Host controller
> >> + * to the OTG core. Host controller must not be started by the
> >> + * caller as it is left upto the OTG state machine to do so.
> >> + *
> >> + * Returns: 0 on success, error value otherwise.
> >> + */
> >> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> >> +   unsigned long irqflags, struct otg_hcd_ops 
> >> *ops)
> >> +{
> >> +  struct usb_otg *otgd;
> >> +  struct device *hcd_dev = hcd->self.controller;
> >> +  struct device *otg_dev = usb_otg_get_device(hcd_dev);
> >> +
> >
> > One big problem here is: there are two designs for current (IP) driver
> > code, one creates dedicated hcd device as roothub's parent, like dwc3.
> > Another one doesn't do this, roothub's parent is core device (or otg 
> > device
> > in your patch), like chipidea and dwc2.
> >
> > Then, otg_dev will be glue layer device for chipidea after that.
> 
>  OK. Let's add a way for the otg controller driver to provide the host 
>  and gadget
>  information to the otg core for such devices like chipidea and dwc2.
> 
> >>>
> >>> Roger, not only chipidea and dwc2, I think the musb uses the same
> >>> hierarchy. If the host, device, and otg share the same register
> >>> region, host part can't be a platform driver since we don't want
> >>> to remap the same register region again.
> >>>
> >>> So, in the design, we may need to consider both situations, one
> >>> is otg/host/device has its own register region, and host is a
> >>> separate platform device (A), the other is three parts share the
> >>> same register region, there is only one platform driver (B).
> >>>
> >>> A:
> >>>
> >>>   IP core device 
> >>>   |
> >>>   |
> >>> |-|-|
> >>> gadget   host platform device 
> >>>   |
> >>>   roothub
> >>>
> >>> B:
> >>>
> >>>   IP core device
> >>>   |
> >>>   |
> >>> |-|-|
> >>> gadget roothub
> >>>   
> >>>
>  This API must be called before the hcd/gadget-driver is added so that 
>  the otg
>  core knows it's linked to an OTG controller.
> 
>  Any better idea?
> 
> >>>
> >>> A flag stands for this hcd controller is the same with otg controller
> >>> can be used, this flag can be stored at struct usb_otg_config.
> >>
> >> What if there is another architecture like so?
> >>
> >> C:
> >>[Parent]
> >>   |
> >>   |
> >>|--|--|
> >>[OTG core]  [gadget][host]
> >>
> >> We need a more flexible mechanism to link the gadget and
> >> host device to the otg core for non DT case.
> >>
> >> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
> >>
> >> e.g.
> >> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
> >>
> >> If otg is NULL it will try DT otg-controller property or parent to
> >> get the otg controller.
> > 
> > How usb_otg_register_hcd get struct usb_otg, from where?
> 
> This only works when the parent driver creating the hcd has registered the
> otg controller too.
> 

Sorry? So we need to find another way to solve this issue, right?

> > 
> >>
> >>>
> >>> Peter
> >>>
> >>> P.S: I still read your code, I find not all APIs in this file are used
> >>> in your dwc3 example. 
> >>
> >> Which ones? The ones for registering/unregistered host/gadget are used
> >> by hcd/udc core as part of usb_add/remove_hcd() and
> >> udc_bind_to_driver()/usb_gadget_remove_driver()
> >>
> > 
> > Ok, now I understand your design, usb_create_hcd must be called before
> > the fsm kicks off. The call flow like below:
> > 
> > usb_otg_register->usb_create_hcd->usb_add_hcd->usb_otg_register_hcd->
> > usb_otg_start_host->usb_otg_add_hcd
> 
> Actually these are the discrete steps
> - usb_otg_register
> - usb_create_hcd
> - usb_add_hcd->usb_otg_register_hcd  (HCD is not really added till FSM in 
> host role)
> 
> Above 3 are prerequisite for FSM to start in addition to gadget controller and
> driver being ready.
> 
> When FSM enters in host role
> - usb_otg_start_host(true)->usb_otg_add_hcd (Now HCD is added)
> 
> When FSM exits host role
> - 

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

On 09/09/15 09:20, Li Jun wrote:
> On Mon, Sep 07, 2015 at 01:53:19PM +0300, Roger Quadros wrote:
>> On 07/09/15 10:40, Li Jun wrote:
>>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
 The OTG core instantiates the OTG Finite State Machine
 per OTG controller and manages starting/stopping the
 host and gadget controllers based on the bus state.

 It provides APIs for the following tasks

 - Registering an OTG capable controller
 - Registering Host and Gadget controllers to OTG core
 - Providing inputs to and kicking the OTG state machine

 Signed-off-by: Roger Quadros 
 ---
  MAINTAINERS  |4 +-
  drivers/usb/Kconfig  |2 +-
  drivers/usb/Makefile |1 +
  drivers/usb/common/Makefile  |3 +-
  drivers/usb/common/usb-otg.c | 1061 
 ++
  drivers/usb/common/usb-otg.h |   71 +++
  drivers/usb/core/Kconfig |   11 +-
  include/linux/usb/otg.h  |  189 +++-
  8 files changed, 1321 insertions(+), 21 deletions(-)
  create mode 100644 drivers/usb/common/usb-otg.c
  create mode 100644 drivers/usb/common/usb-otg.h

>>>
>>> ... ...
>>>
 +
 +/**
 + * Get OTG device from host or gadget device.
 + *
 + * For non device tree boot, the OTG controller is assumed to be
 + * the parent of the host/gadget device.
>>>
>>> This assumption/restriction maybe a problem, as I pointed in your previous
>>> version, usb_create_hcd() use the passed dev as its dev, but,
>>> usb_add_gadget_udc() use the passed dev as its parent dev, so often the
>>> host and gadget don't share the same parent device, at least it doesn't
>>> apply to chipidea case.
>>
>> Let's provide a way for OTG driver to provide the OTG core exactly which is
>> the related host/gadget device.
>>
>>>
 + * For device tree boot, the OTG controller is derived from the
 + * "otg-controller" property.
 + */
 +static struct device *usb_otg_get_device(struct device *hcd_gcd_dev)
 +{
 +  struct device *otg_dev;
 +
 +  if (!hcd_gcd_dev)
 +  return NULL;
 +
 +  if (hcd_gcd_dev->of_node) {
 +  struct device_node *np;
 +  struct platform_device *pdev;
 +
 +  np = of_parse_phandle(hcd_gcd_dev->of_node, "otg-controller",
 +0);
 +  if (!np)
 +  goto legacy;/* continue legacy way */
 +
 +  pdev = of_find_device_by_node(np);
 +  of_node_put(np);
 +  if (!pdev) {
 +  dev_err(>dev, "couldn't get otg-controller 
 device\n");
 +  return NULL;
 +  }
 +
 +  otg_dev = >dev;
 +  return otg_dev;
 +  }
 +
 +legacy:
 +  /* otg device is parent and must be registered */
 +  otg_dev = hcd_gcd_dev->parent;
 +  if (!usb_otg_get_data(otg_dev))
 +  return NULL;
 +
 +  return otg_dev;
 +}
 +
>>>
>>> ... ...
>>>
 +static void usb_otg_init_timers(struct usb_otg *otgd, unsigned *timeouts)
 +{
 +  struct otg_fsm *fsm = >fsm;
 +  unsigned long tmouts[NUM_OTG_FSM_TIMERS];
 +  int i;
 +
 +  /* set default timeouts */
 +  tmouts[A_WAIT_VRISE] = TA_WAIT_VRISE;
 +  tmouts[A_WAIT_VFALL] = TA_WAIT_VFALL;
 +  tmouts[A_WAIT_BCON] = TA_WAIT_BCON;
 +  tmouts[A_AIDL_BDIS] = TA_AIDL_BDIS;
 +  tmouts[A_BIDL_ADIS] = TA_BIDL_ADIS;
 +  tmouts[B_ASE0_BRST] = TB_ASE0_BRST;
 +  tmouts[B_SE0_SRP] = TB_SE0_SRP;
 +  tmouts[B_SRP_FAIL] = TB_SRP_FAIL;
 +
 +  /* set controller provided timeouts */
 +  if (timeouts) {
 +  for (i = 0; i < NUM_OTG_FSM_TIMERS; i++) {
 +  if (timeouts[i])
 +  tmouts[i] = timeouts[i];
 +  }
 +  }
 +
 +  otg_timer_init(A_WAIT_VRISE, otgd, set_tmout, TA_WAIT_VRISE,
 + >a_wait_vrise_tmout);
 +  otg_timer_init(A_WAIT_VFALL, otgd, set_tmout, TA_WAIT_VFALL,
 + >a_wait_vfall_tmout);
 +  otg_timer_init(A_WAIT_BCON, otgd, set_tmout, TA_WAIT_BCON,
 + >a_wait_bcon_tmout);
 +  otg_timer_init(A_AIDL_BDIS, otgd, set_tmout, TA_AIDL_BDIS,
 + >a_aidl_bdis_tmout);
 +  otg_timer_init(A_BIDL_ADIS, otgd, set_tmout, TA_BIDL_ADIS,
 + >a_bidl_adis_tmout);
 +  otg_timer_init(B_ASE0_BRST, otgd, set_tmout, TB_ASE0_BRST,
 + >b_ase0_brst_tmout);
 +
 +  otg_timer_init(B_SE0_SRP, otgd, set_tmout, TB_SE0_SRP,
 + >b_se0_srp);
 +  otg_timer_init(B_SRP_FAIL, otgd, set_tmout, TB_SRP_FAIL,
 + >b_srp_done);
 +
 +  /* FIXME: what about A_WAIT_ENUM? */
>>>
>>> Either you init it as other timers, or you remove all of 

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Alan Stern]

On Wed, 9 Sep 2015, Roger Quadros wrote:

> (adding back folks in cc)
> 
> On 08/09/15 20:35, Alan Stern wrote:
> > On Tue, 8 Sep 2015, Roger Quadros wrote:
> > 
>  What if there is another architecture like so?
> 
>  C:
>   [Parent]
>  |
>  |
>   |--|--|
>   [OTG core]  [gadget][host]
> 
>  We need a more flexible mechanism to link the gadget and
>  host device to the otg core for non DT case.
> 
>  How about adding struct usb_otg parameter to usb_otg_register_hcd()?
> 
>  e.g.
>  int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
> 
>  If otg is NULL it will try DT otg-controller property or parent to
>  get the otg controller.
> >>>
> >>> This seems a lot like something Peter and I discussed recently.  See
> >>>
> >>>   http://marc.info/?l=linux-usb=143977568021328=2
> >>>
> >>> and the following messages in that thread.
> >>>
> >>
> >> If I understood right, your proposal was to add a usb_pointers data
> >> struct to the device's drvdata?
> > 
> > Right.
> > 
> >> This is fine only if the otg/gadget/host share the same device.
> >> It does not solve the problem where each have different platform devices.
> > 
> > Why not?  Each of the platform devices can store a pointer to the same 
> > usb_pointers structure.  Wouldn't that be suitable?
> > 
> 
> I didn't understand how all of them get the reference to the
> same usb_pointer structure (or contents) when their respective platform 
> devices
> get created so that they can store it in their respective drvdata.
> 
> Worst case, the 3 devices could be totally independent of each other without a
> common parent, and get registered at different times.
> 
> The common resource here is the physical USB port for which the 3 drivers
> otg/host/gadget are being registered.
> There should be a mechanism for each device to tell that it is part of
> this particular USB port. (or usb_pointers struct)

True, it's not clear how to set up the common relationship among the 
three drivers.  But there must be some way to do it if they all are 
talking to the same port or PHY.

The details are an issue for the platform-specific code to handle.  
I'm not sure what would be involved; maybe you can invent something.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Peter Chen]

On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
> On 07/09/15 04:23, Peter Chen wrote:
> > On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
> >> + * This is used by the USB Host stack to register the Host controller
> >> + * to the OTG core. Host controller must not be started by the
> >> + * caller as it is left upto the OTG state machine to do so.
> >> + *
> >> + * Returns: 0 on success, error value otherwise.
> >> + */
> >> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> >> +   unsigned long irqflags, struct otg_hcd_ops *ops)
> >> +{
> >> +  struct usb_otg *otgd;
> >> +  struct device *hcd_dev = hcd->self.controller;
> >> +  struct device *otg_dev = usb_otg_get_device(hcd_dev);
> >> +
> > 
> > One big problem here is: there are two designs for current (IP) driver
> > code, one creates dedicated hcd device as roothub's parent, like dwc3.
> > Another one doesn't do this, roothub's parent is core device (or otg device
> > in your patch), like chipidea and dwc2.
> > 
> > Then, otg_dev will be glue layer device for chipidea after that.
> 
> OK. Let's add a way for the otg controller driver to provide the host and 
> gadget
> information to the otg core for such devices like chipidea and dwc2.
> 

Roger, not only chipidea and dwc2, I think the musb uses the same
hierarchy. If the host, device, and otg share the same register
region, host part can't be a platform driver since we don't want
to remap the same register region again.

So, in the design, we may need to consider both situations, one
is otg/host/device has its own register region, and host is a
separate platform device (A), the other is three parts share the
same register region, there is only one platform driver (B).

A:

IP core device 
|
|
  |-|-|
  gadget   host platform device 
|
roothub

B:

IP core device
|
|
  |-|-|
  gadget roothub


> This API must be called before the hcd/gadget-driver is added so that the otg
> core knows it's linked to an OTG controller.
> 
> Any better idea?
> 

A flag stands for this hcd controller is the same with otg controller
can be used, this flag can be stored at struct usb_otg_config.

Peter

P.S: I still read your code, I find not all APIs in this file are used
in your dwc3 example. 
> > 
> >> +  if (!otg_dev)
> >> +  return -EINVAL; /* we're definitely not OTG */
> >> +
> >> +  /* we're otg but otg controller might not yet be registered */
> >> +  mutex_lock(_list_mutex);
> >> +  otgd = usb_otg_get_data(otg_dev);
> >> +  mutex_unlock(_list_mutex);
> >> +  if (!otgd) {
> >> +  dev_dbg(hcd_dev,
> >> +  "otg: controller not yet registered. waiting..\n");
> >> +  /*
> >> +   * otg controller might register later. Put the hcd in
> >> +   * wait list and call us back when ready
> >> +   */
> >> +  if (usb_otg_hcd_wait_add(otg_dev, hcd, irqnum, irqflags, ops)) {
> >> +  dev_dbg(hcd_dev, "otg: failed to add to wait list\n");
> >> +  return -EINVAL;
> >> +  }
> >> +
> >> +  return 0;
> >> +  }
> >> +
> >> +  /* HCD will be started by OTG fsm when needed */
> >> +  mutex_lock(>fsm.lock);
> >> +  if (otgd->primary_hcd.hcd) {
> >> +  /* probably a shared HCD ? */
> >> +  if (usb_otg_hcd_is_primary_hcd(hcd)) {
> >> +  dev_err(otg_dev, "otg: primary host already 
> >> registered\n");
> >> +  goto err;
> >> +  }
> >> +
> >> +  if (hcd->shared_hcd == otgd->primary_hcd.hcd) {
> >> +  if (otgd->shared_hcd.hcd) {
> >> +  dev_err(otg_dev, "otg: shared host already 
> >> registered\n");
> >> +  goto err;
> >> +  }
> >> +
> >> +  otgd->shared_hcd.hcd = hcd;
> >> +  otgd->shared_hcd.irqnum = irqnum;
> >> +  otgd->shared_hcd.irqflags = irqflags;
> >> +  otgd->shared_hcd.ops = ops;
> >> +  dev_info(otg_dev, "otg: shared host %s registered\n",
> >> +   dev_name(hcd->self.controller));
> >> +  } else {
> >> +  dev_err(otg_dev, "otg: invalid shared host %s\n",
> >> +  dev_name(hcd->self.controller));
> >> +  goto err;
> >> +  }
> >> +  } else {
> >> +  if (!usb_otg_hcd_is_primary_hcd(hcd)) {
> >> +  dev_err(otg_dev, "otg: primary host must be registered 
> >> first\n");
> >> +  goto err;
> >> +  }
> >> +
> >> +  

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

On 08/09/15 11:31, Peter Chen wrote:
> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
>> On 07/09/15 04:23, Peter Chen wrote:
>>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
 + * This is used by the USB Host stack to register the Host controller
 + * to the OTG core. Host controller must not be started by the
 + * caller as it is left upto the OTG state machine to do so.
 + *
 + * Returns: 0 on success, error value otherwise.
 + */
 +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
 +   unsigned long irqflags, struct otg_hcd_ops *ops)
 +{
 +  struct usb_otg *otgd;
 +  struct device *hcd_dev = hcd->self.controller;
 +  struct device *otg_dev = usb_otg_get_device(hcd_dev);
 +
>>>
>>> One big problem here is: there are two designs for current (IP) driver
>>> code, one creates dedicated hcd device as roothub's parent, like dwc3.
>>> Another one doesn't do this, roothub's parent is core device (or otg device
>>> in your patch), like chipidea and dwc2.
>>>
>>> Then, otg_dev will be glue layer device for chipidea after that.
>>
>> OK. Let's add a way for the otg controller driver to provide the host and 
>> gadget
>> information to the otg core for such devices like chipidea and dwc2.
>>
> 
> Roger, not only chipidea and dwc2, I think the musb uses the same
> hierarchy. If the host, device, and otg share the same register
> region, host part can't be a platform driver since we don't want
> to remap the same register region again.
> 
> So, in the design, we may need to consider both situations, one
> is otg/host/device has its own register region, and host is a
> separate platform device (A), the other is three parts share the
> same register region, there is only one platform driver (B).
> 
> A:
> 
>   IP core device 
>   |
>   |
> |-|-|
> gadget   host platform device 
>   |
>   roothub
> 
> B:
> 
>   IP core device
>   |
>   |
> |-|-|
> gadget roothub
>   
> 
>> This API must be called before the hcd/gadget-driver is added so that the otg
>> core knows it's linked to an OTG controller.
>>
>> Any better idea?
>>
> 
> A flag stands for this hcd controller is the same with otg controller
> can be used, this flag can be stored at struct usb_otg_config.

What if there is another architecture like so?

C:
[Parent]
   |
   |
|--|--|
[OTG core]  [gadget][host]

We need a more flexible mechanism to link the gadget and
host device to the otg core for non DT case.

How about adding struct usb_otg parameter to usb_otg_register_hcd()?

e.g.
int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)

If otg is NULL it will try DT otg-controller property or parent to
get the otg controller.

> 
> Peter
> 
> P.S: I still read your code, I find not all APIs in this file are used
> in your dwc3 example. 

Which ones? The ones for registering/unregistered host/gadget are used
by hcd/udc core as part of usb_add/remove_hcd() and
udc_bind_to_driver()/usb_gadget_remove_driver()

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Alan Stern]

On Tue, 8 Sep 2015, Roger Quadros wrote:

> On 08/09/15 11:31, Peter Chen wrote:
> > On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
> >> On 07/09/15 04:23, Peter Chen wrote:
> >>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
>  + * This is used by the USB Host stack to register the Host controller
>  + * to the OTG core. Host controller must not be started by the
>  + * caller as it is left upto the OTG state machine to do so.
>  + *
>  + * Returns: 0 on success, error value otherwise.
>  + */
>  +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>  + unsigned long irqflags, struct otg_hcd_ops 
>  *ops)
>  +{
>  +struct usb_otg *otgd;
>  +struct device *hcd_dev = hcd->self.controller;
>  +struct device *otg_dev = usb_otg_get_device(hcd_dev);
>  +
> >>>
> >>> One big problem here is: there are two designs for current (IP) driver
> >>> code, one creates dedicated hcd device as roothub's parent, like dwc3.
> >>> Another one doesn't do this, roothub's parent is core device (or otg 
> >>> device
> >>> in your patch), like chipidea and dwc2.
> >>>
> >>> Then, otg_dev will be glue layer device for chipidea after that.
> >>
> >> OK. Let's add a way for the otg controller driver to provide the host and 
> >> gadget
> >> information to the otg core for such devices like chipidea and dwc2.
> >>
> > 
> > Roger, not only chipidea and dwc2, I think the musb uses the same
> > hierarchy. If the host, device, and otg share the same register
> > region, host part can't be a platform driver since we don't want
> > to remap the same register region again.
> > 
> > So, in the design, we may need to consider both situations, one
> > is otg/host/device has its own register region, and host is a
> > separate platform device (A), the other is three parts share the
> > same register region, there is only one platform driver (B).
> > 
> > A:
> > 
> > IP core device 
> > |
> > |
> >   |-|-|
> >   gadget   host platform device 
> > |
> > roothub
> > 
> > B:
> > 
> > IP core device
> > |
> > |
> >   |-|-|
> >   gadget roothub
> > 
> > 
> >> This API must be called before the hcd/gadget-driver is added so that the 
> >> otg
> >> core knows it's linked to an OTG controller.
> >>
> >> Any better idea?
> >>
> > 
> > A flag stands for this hcd controller is the same with otg controller
> > can be used, this flag can be stored at struct usb_otg_config.
> 
> What if there is another architecture like so?
> 
> C:
>   [Parent]
>  |
>  |
>   |--|--|
>   [OTG core]  [gadget][host]
> 
> We need a more flexible mechanism to link the gadget and
> host device to the otg core for non DT case.
> 
> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
> 
> e.g.
> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
> 
> If otg is NULL it will try DT otg-controller property or parent to
> get the otg controller.

This seems a lot like something Peter and I discussed recently.  See

http://marc.info/?l=linux-usb=143977568021328=2

and the following messages in that thread.

Alan Stern


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Peter Chen]

On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:
> 
> 
> On 08/09/15 11:31, Peter Chen wrote:
> > On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
> >> On 07/09/15 04:23, Peter Chen wrote:
> >>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
>  + * This is used by the USB Host stack to register the Host controller
>  + * to the OTG core. Host controller must not be started by the
>  + * caller as it is left upto the OTG state machine to do so.
>  + *
>  + * Returns: 0 on success, error value otherwise.
>  + */
>  +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>  + unsigned long irqflags, struct otg_hcd_ops 
>  *ops)
>  +{
>  +struct usb_otg *otgd;
>  +struct device *hcd_dev = hcd->self.controller;
>  +struct device *otg_dev = usb_otg_get_device(hcd_dev);
>  +
> >>>
> >>> One big problem here is: there are two designs for current (IP) driver
> >>> code, one creates dedicated hcd device as roothub's parent, like dwc3.
> >>> Another one doesn't do this, roothub's parent is core device (or otg 
> >>> device
> >>> in your patch), like chipidea and dwc2.
> >>>
> >>> Then, otg_dev will be glue layer device for chipidea after that.
> >>
> >> OK. Let's add a way for the otg controller driver to provide the host and 
> >> gadget
> >> information to the otg core for such devices like chipidea and dwc2.
> >>
> > 
> > Roger, not only chipidea and dwc2, I think the musb uses the same
> > hierarchy. If the host, device, and otg share the same register
> > region, host part can't be a platform driver since we don't want
> > to remap the same register region again.
> > 
> > So, in the design, we may need to consider both situations, one
> > is otg/host/device has its own register region, and host is a
> > separate platform device (A), the other is three parts share the
> > same register region, there is only one platform driver (B).
> > 
> > A:
> > 
> > IP core device 
> > |
> > |
> >   |-|-|
> >   gadget   host platform device 
> > |
> > roothub
> > 
> > B:
> > 
> > IP core device
> > |
> > |
> >   |-|-|
> >   gadget roothub
> > 
> > 
> >> This API must be called before the hcd/gadget-driver is added so that the 
> >> otg
> >> core knows it's linked to an OTG controller.
> >>
> >> Any better idea?
> >>
> > 
> > A flag stands for this hcd controller is the same with otg controller
> > can be used, this flag can be stored at struct usb_otg_config.
> 
> What if there is another architecture like so?
> 
> C:
>   [Parent]
>  |
>  |
>   |--|--|
>   [OTG core]  [gadget][host]
> 
> We need a more flexible mechanism to link the gadget and
> host device to the otg core for non DT case.
> 
> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
> 
> e.g.
> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
> 
> If otg is NULL it will try DT otg-controller property or parent to
> get the otg controller.

How usb_otg_register_hcd get struct usb_otg, from where?

> 
> > 
> > Peter
> > 
> > P.S: I still read your code, I find not all APIs in this file are used
> > in your dwc3 example. 
> 
> Which ones? The ones for registering/unregistered host/gadget are used
> by hcd/udc core as part of usb_add/remove_hcd() and
> udc_bind_to_driver()/usb_gadget_remove_driver()
> 

Ok, now I understand your design, usb_create_hcd must be called before
the fsm kicks off. The call flow like below:

usb_otg_register->usb_create_hcd->usb_add_hcd->usb_otg_register_hcd->
usb_otg_start_host->usb_otg_add_hcd

We need to make some changes to let chipidea work since usb_create_hcd
is included at host->start.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

Alan,

On 08/09/15 17:34, Alan Stern wrote:
> On Tue, 8 Sep 2015, Roger Quadros wrote:
> 
>> On 08/09/15 11:31, Peter Chen wrote:
>>> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
 On 07/09/15 04:23, Peter Chen wrote:
> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
>> + * This is used by the USB Host stack to register the Host controller
>> + * to the OTG core. Host controller must not be started by the
>> + * caller as it is left upto the OTG state machine to do so.
>> + *
>> + * Returns: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>> + unsigned long irqflags, struct otg_hcd_ops 
>> *ops)
>> +{
>> +struct usb_otg *otgd;
>> +struct device *hcd_dev = hcd->self.controller;
>> +struct device *otg_dev = usb_otg_get_device(hcd_dev);
>> +
>
> One big problem here is: there are two designs for current (IP) driver
> code, one creates dedicated hcd device as roothub's parent, like dwc3.
> Another one doesn't do this, roothub's parent is core device (or otg 
> device
> in your patch), like chipidea and dwc2.
>
> Then, otg_dev will be glue layer device for chipidea after that.

 OK. Let's add a way for the otg controller driver to provide the host and 
 gadget
 information to the otg core for such devices like chipidea and dwc2.

>>>
>>> Roger, not only chipidea and dwc2, I think the musb uses the same
>>> hierarchy. If the host, device, and otg share the same register
>>> region, host part can't be a platform driver since we don't want
>>> to remap the same register region again.
>>>
>>> So, in the design, we may need to consider both situations, one
>>> is otg/host/device has its own register region, and host is a
>>> separate platform device (A), the other is three parts share the
>>> same register region, there is only one platform driver (B).
>>>
>>> A:
>>>
>>> IP core device 
>>> |
>>> |
>>>   |-|-|
>>>   gadget   host platform device 
>>> |
>>> roothub
>>>
>>> B:
>>>
>>> IP core device
>>> |
>>> |
>>>   |-|-|
>>>   gadget roothub
>>> 
>>>
 This API must be called before the hcd/gadget-driver is added so that the 
 otg
 core knows it's linked to an OTG controller.

 Any better idea?

>>>
>>> A flag stands for this hcd controller is the same with otg controller
>>> can be used, this flag can be stored at struct usb_otg_config.
>>
>> What if there is another architecture like so?
>>
>> C:
>>  [Parent]
>> |
>> |
>>  |--|--|
>>  [OTG core]  [gadget][host]
>>
>> We need a more flexible mechanism to link the gadget and
>> host device to the otg core for non DT case.
>>
>> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
>>
>> e.g.
>> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
>>
>> If otg is NULL it will try DT otg-controller property or parent to
>> get the otg controller.
> 
> This seems a lot like something Peter and I discussed recently.  See
> 
>   http://marc.info/?l=linux-usb=143977568021328=2
> 
> and the following messages in that thread.
> 

If I understood right, your proposal was to add a usb_pointers data
struct to the device's drvdata?

This is fine only if the otg/gadget/host share the same device.
It does not solve the problem where each have different platform devices.

cheers,
-roger

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Li Jun]

On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
> The OTG core instantiates the OTG Finite State Machine
> per OTG controller and manages starting/stopping the
> host and gadget controllers based on the bus state.
> 
> It provides APIs for the following tasks
> 
> - Registering an OTG capable controller
> - Registering Host and Gadget controllers to OTG core
> - Providing inputs to and kicking the OTG state machine
> 
> Signed-off-by: Roger Quadros 
> ---
>  MAINTAINERS  |4 +-
>  drivers/usb/Kconfig  |2 +-
>  drivers/usb/Makefile |1 +
>  drivers/usb/common/Makefile  |3 +-
>  drivers/usb/common/usb-otg.c | 1061 
> ++
>  drivers/usb/common/usb-otg.h |   71 +++
>  drivers/usb/core/Kconfig |   11 +-
>  include/linux/usb/otg.h  |  189 +++-
>  8 files changed, 1321 insertions(+), 21 deletions(-)
>  create mode 100644 drivers/usb/common/usb-otg.c
>  create mode 100644 drivers/usb/common/usb-otg.h
> 

... ...

> +
> +/**
> + * Get OTG device from host or gadget device.
> + *
> + * For non device tree boot, the OTG controller is assumed to be
> + * the parent of the host/gadget device.

This assumption/restriction maybe a problem, as I pointed in your previous
version, usb_create_hcd() use the passed dev as its dev, but,
usb_add_gadget_udc() use the passed dev as its parent dev, so often the
host and gadget don't share the same parent device, at least it doesn't
apply to chipidea case.

> + * For device tree boot, the OTG controller is derived from the
> + * "otg-controller" property.
> + */
> +static struct device *usb_otg_get_device(struct device *hcd_gcd_dev)
> +{
> + struct device *otg_dev;
> +
> + if (!hcd_gcd_dev)
> + return NULL;
> +
> + if (hcd_gcd_dev->of_node) {
> + struct device_node *np;
> + struct platform_device *pdev;
> +
> + np = of_parse_phandle(hcd_gcd_dev->of_node, "otg-controller",
> +   0);
> + if (!np)
> + goto legacy;/* continue legacy way */
> +
> + pdev = of_find_device_by_node(np);
> + of_node_put(np);
> + if (!pdev) {
> + dev_err(>dev, "couldn't get otg-controller 
> device\n");
> + return NULL;
> + }
> +
> + otg_dev = >dev;
> + return otg_dev;
> + }
> +
> +legacy:
> + /* otg device is parent and must be registered */
> + otg_dev = hcd_gcd_dev->parent;
> + if (!usb_otg_get_data(otg_dev))
> + return NULL;
> +
> + return otg_dev;
> +}
> +

... ...

> +static void usb_otg_init_timers(struct usb_otg *otgd, unsigned *timeouts)
> +{
> + struct otg_fsm *fsm = >fsm;
> + unsigned long tmouts[NUM_OTG_FSM_TIMERS];
> + int i;
> +
> + /* set default timeouts */
> + tmouts[A_WAIT_VRISE] = TA_WAIT_VRISE;
> + tmouts[A_WAIT_VFALL] = TA_WAIT_VFALL;
> + tmouts[A_WAIT_BCON] = TA_WAIT_BCON;
> + tmouts[A_AIDL_BDIS] = TA_AIDL_BDIS;
> + tmouts[A_BIDL_ADIS] = TA_BIDL_ADIS;
> + tmouts[B_ASE0_BRST] = TB_ASE0_BRST;
> + tmouts[B_SE0_SRP] = TB_SE0_SRP;
> + tmouts[B_SRP_FAIL] = TB_SRP_FAIL;
> +
> + /* set controller provided timeouts */
> + if (timeouts) {
> + for (i = 0; i < NUM_OTG_FSM_TIMERS; i++) {
> + if (timeouts[i])
> + tmouts[i] = timeouts[i];
> + }
> + }
> +
> + otg_timer_init(A_WAIT_VRISE, otgd, set_tmout, TA_WAIT_VRISE,
> +>a_wait_vrise_tmout);
> + otg_timer_init(A_WAIT_VFALL, otgd, set_tmout, TA_WAIT_VFALL,
> +>a_wait_vfall_tmout);
> + otg_timer_init(A_WAIT_BCON, otgd, set_tmout, TA_WAIT_BCON,
> +>a_wait_bcon_tmout);
> + otg_timer_init(A_AIDL_BDIS, otgd, set_tmout, TA_AIDL_BDIS,
> +>a_aidl_bdis_tmout);
> + otg_timer_init(A_BIDL_ADIS, otgd, set_tmout, TA_BIDL_ADIS,
> +>a_bidl_adis_tmout);
> + otg_timer_init(B_ASE0_BRST, otgd, set_tmout, TB_ASE0_BRST,
> +>b_ase0_brst_tmout);
> +
> + otg_timer_init(B_SE0_SRP, otgd, set_tmout, TB_SE0_SRP,
> +>b_se0_srp);
> + otg_timer_init(B_SRP_FAIL, otgd, set_tmout, TB_SRP_FAIL,
> +>b_srp_done);
> +
> + /* FIXME: what about A_WAIT_ENUM? */

Either you init it as other timers, or you remove all of it, otherwise
there will be NULL pointer crash.

> +}
> +
> +/**
> + * OTG FSM ops function to add timer
> + */
> +static void usb_otg_add_timer(struct otg_fsm *fsm, enum otg_fsm_timer id)
> +{
> + struct usb_otg *otgd = container_of(fsm, struct usb_otg, fsm);
> + struct otg_timer *otgtimer = >timers[id];
> + struct hrtimer *timer = >timer;
> +
> + if (!otgd->fsm_running)
> + return;
> +
> + /* if timer is already active, exit */
> +   

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

On 07/09/15 04:23, Peter Chen wrote:
> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
>> + * This is used by the USB Host stack to register the Host controller
>> + * to the OTG core. Host controller must not be started by the
>> + * caller as it is left upto the OTG state machine to do so.
>> + *
>> + * Returns: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>> + unsigned long irqflags, struct otg_hcd_ops *ops)
>> +{
>> +struct usb_otg *otgd;
>> +struct device *hcd_dev = hcd->self.controller;
>> +struct device *otg_dev = usb_otg_get_device(hcd_dev);
>> +
> 
> One big problem here is: there are two designs for current (IP) driver
> code, one creates dedicated hcd device as roothub's parent, like dwc3.
> Another one doesn't do this, roothub's parent is core device (or otg device
> in your patch), like chipidea and dwc2.
> 
> Then, otg_dev will be glue layer device for chipidea after that.

OK. Let's add a way for the otg controller driver to provide the host and gadget
information to the otg core for such devices like chipidea and dwc2.

This API must be called before the hcd/gadget-driver is added so that the otg
core knows it's linked to an OTG controller.

Any better idea?

cheers,
-roger

> 
> Peter
> 
>> +if (!otg_dev)
>> +return -EINVAL; /* we're definitely not OTG */
>> +
>> +/* we're otg but otg controller might not yet be registered */
>> +mutex_lock(_list_mutex);
>> +otgd = usb_otg_get_data(otg_dev);
>> +mutex_unlock(_list_mutex);
>> +if (!otgd) {
>> +dev_dbg(hcd_dev,
>> +"otg: controller not yet registered. waiting..\n");
>> +/*
>> + * otg controller might register later. Put the hcd in
>> + * wait list and call us back when ready
>> + */
>> +if (usb_otg_hcd_wait_add(otg_dev, hcd, irqnum, irqflags, ops)) {
>> +dev_dbg(hcd_dev, "otg: failed to add to wait list\n");
>> +return -EINVAL;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +/* HCD will be started by OTG fsm when needed */
>> +mutex_lock(>fsm.lock);
>> +if (otgd->primary_hcd.hcd) {
>> +/* probably a shared HCD ? */
>> +if (usb_otg_hcd_is_primary_hcd(hcd)) {
>> +dev_err(otg_dev, "otg: primary host already 
>> registered\n");
>> +goto err;
>> +}
>> +
>> +if (hcd->shared_hcd == otgd->primary_hcd.hcd) {
>> +if (otgd->shared_hcd.hcd) {
>> +dev_err(otg_dev, "otg: shared host already 
>> registered\n");
>> +goto err;
>> +}
>> +
>> +otgd->shared_hcd.hcd = hcd;
>> +otgd->shared_hcd.irqnum = irqnum;
>> +otgd->shared_hcd.irqflags = irqflags;
>> +otgd->shared_hcd.ops = ops;
>> +dev_info(otg_dev, "otg: shared host %s registered\n",
>> + dev_name(hcd->self.controller));
>> +} else {
>> +dev_err(otg_dev, "otg: invalid shared host %s\n",
>> +dev_name(hcd->self.controller));
>> +goto err;
>> +}
>> +} else {
>> +if (!usb_otg_hcd_is_primary_hcd(hcd)) {
>> +dev_err(otg_dev, "otg: primary host must be registered 
>> first\n");
>> +goto err;
>> +}
>> +
>> +otgd->primary_hcd.hcd = hcd;
>> +otgd->primary_hcd.irqnum = irqnum;
>> +otgd->primary_hcd.irqflags = irqflags;
>> +otgd->primary_hcd.ops = ops;
>> +dev_info(otg_dev, "otg: primary host %s registered\n",
>> + dev_name(hcd->self.controller));
>> +}
>> +
>> +/*
>> + * we're ready only if we have shared HCD
>> + * or we don't need shared HCD.
>> + */
>> +if (otgd->shared_hcd.hcd || !otgd->primary_hcd.hcd->shared_hcd) {
>> +otgd->fsm.otg->host = hcd_to_bus(hcd);
>> +/* FIXME: set bus->otg_port if this is true OTG port with HNP */
>> +
>> +/* start FSM */
>> +usb_otg_start_fsm(>fsm);
>> +} else {
>> +dev_dbg(otg_dev, "otg: can't start till shared host 
>> registers\n");
>> +}
>> +
>> +mutex_unlock(>fsm.lock);
>> +
>> +return 0;
>> +
>> +err:
>> +mutex_unlock(>fsm.lock);
>> +return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_register_hcd);
>> +
>> +/**
>> + * usb_otg_unregister_hcd - Unregister Host controller from OTG core
>> + * @hcd:Host controller device
>> + *
>> + * This is used by the USB Host stack to unregister the Host controller
>> + * from the OTG core. Ensures that Host controller is not running
>> + * on successful return.
>> + *
>> + * 

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

On 07/09/15 10:40, Li Jun wrote:
> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
>> The OTG core instantiates the OTG Finite State Machine
>> per OTG controller and manages starting/stopping the
>> host and gadget controllers based on the bus state.
>>
>> It provides APIs for the following tasks
>>
>> - Registering an OTG capable controller
>> - Registering Host and Gadget controllers to OTG core
>> - Providing inputs to and kicking the OTG state machine
>>
>> Signed-off-by: Roger Quadros 
>> ---
>>  MAINTAINERS  |4 +-
>>  drivers/usb/Kconfig  |2 +-
>>  drivers/usb/Makefile |1 +
>>  drivers/usb/common/Makefile  |3 +-
>>  drivers/usb/common/usb-otg.c | 1061 
>> ++
>>  drivers/usb/common/usb-otg.h |   71 +++
>>  drivers/usb/core/Kconfig |   11 +-
>>  include/linux/usb/otg.h  |  189 +++-
>>  8 files changed, 1321 insertions(+), 21 deletions(-)
>>  create mode 100644 drivers/usb/common/usb-otg.c
>>  create mode 100644 drivers/usb/common/usb-otg.h
>>
> 
> ... ...
> 
>> +
>> +/**
>> + * Get OTG device from host or gadget device.
>> + *
>> + * For non device tree boot, the OTG controller is assumed to be
>> + * the parent of the host/gadget device.
> 
> This assumption/restriction maybe a problem, as I pointed in your previous
> version, usb_create_hcd() use the passed dev as its dev, but,
> usb_add_gadget_udc() use the passed dev as its parent dev, so often the
> host and gadget don't share the same parent device, at least it doesn't
> apply to chipidea case.

Let's provide a way for OTG driver to provide the OTG core exactly which is
the related host/gadget device.

> 
>> + * For device tree boot, the OTG controller is derived from the
>> + * "otg-controller" property.
>> + */
>> +static struct device *usb_otg_get_device(struct device *hcd_gcd_dev)
>> +{
>> +struct device *otg_dev;
>> +
>> +if (!hcd_gcd_dev)
>> +return NULL;
>> +
>> +if (hcd_gcd_dev->of_node) {
>> +struct device_node *np;
>> +struct platform_device *pdev;
>> +
>> +np = of_parse_phandle(hcd_gcd_dev->of_node, "otg-controller",
>> +  0);
>> +if (!np)
>> +goto legacy;/* continue legacy way */
>> +
>> +pdev = of_find_device_by_node(np);
>> +of_node_put(np);
>> +if (!pdev) {
>> +dev_err(>dev, "couldn't get otg-controller 
>> device\n");
>> +return NULL;
>> +}
>> +
>> +otg_dev = >dev;
>> +return otg_dev;
>> +}
>> +
>> +legacy:
>> +/* otg device is parent and must be registered */
>> +otg_dev = hcd_gcd_dev->parent;
>> +if (!usb_otg_get_data(otg_dev))
>> +return NULL;
>> +
>> +return otg_dev;
>> +}
>> +
> 
> ... ...
> 
>> +static void usb_otg_init_timers(struct usb_otg *otgd, unsigned *timeouts)
>> +{
>> +struct otg_fsm *fsm = >fsm;
>> +unsigned long tmouts[NUM_OTG_FSM_TIMERS];
>> +int i;
>> +
>> +/* set default timeouts */
>> +tmouts[A_WAIT_VRISE] = TA_WAIT_VRISE;
>> +tmouts[A_WAIT_VFALL] = TA_WAIT_VFALL;
>> +tmouts[A_WAIT_BCON] = TA_WAIT_BCON;
>> +tmouts[A_AIDL_BDIS] = TA_AIDL_BDIS;
>> +tmouts[A_BIDL_ADIS] = TA_BIDL_ADIS;
>> +tmouts[B_ASE0_BRST] = TB_ASE0_BRST;
>> +tmouts[B_SE0_SRP] = TB_SE0_SRP;
>> +tmouts[B_SRP_FAIL] = TB_SRP_FAIL;
>> +
>> +/* set controller provided timeouts */
>> +if (timeouts) {
>> +for (i = 0; i < NUM_OTG_FSM_TIMERS; i++) {
>> +if (timeouts[i])
>> +tmouts[i] = timeouts[i];
>> +}
>> +}
>> +
>> +otg_timer_init(A_WAIT_VRISE, otgd, set_tmout, TA_WAIT_VRISE,
>> +   >a_wait_vrise_tmout);
>> +otg_timer_init(A_WAIT_VFALL, otgd, set_tmout, TA_WAIT_VFALL,
>> +   >a_wait_vfall_tmout);
>> +otg_timer_init(A_WAIT_BCON, otgd, set_tmout, TA_WAIT_BCON,
>> +   >a_wait_bcon_tmout);
>> +otg_timer_init(A_AIDL_BDIS, otgd, set_tmout, TA_AIDL_BDIS,
>> +   >a_aidl_bdis_tmout);
>> +otg_timer_init(A_BIDL_ADIS, otgd, set_tmout, TA_BIDL_ADIS,
>> +   >a_bidl_adis_tmout);
>> +otg_timer_init(B_ASE0_BRST, otgd, set_tmout, TB_ASE0_BRST,
>> +   >b_ase0_brst_tmout);
>> +
>> +otg_timer_init(B_SE0_SRP, otgd, set_tmout, TB_SE0_SRP,
>> +   >b_se0_srp);
>> +otg_timer_init(B_SRP_FAIL, otgd, set_tmout, TB_SRP_FAIL,
>> +   >b_srp_done);
>> +
>> +/* FIXME: what about A_WAIT_ENUM? */
> 
> Either you init it as other timers, or you remove all of it, otherwise
> there will be NULL pointer crash.

I want to initialize it but was not sure about the timeout value.
What timeout value I must use?

> 
>> +}
>> +
>> +/**
>> + * OTG FSM ops function to add timer
>> + */
>> +static void 

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Peter Chen]

On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
> + * This is used by the USB Host stack to register the Host controller
> + * to the OTG core. Host controller must not be started by the
> + * caller as it is left upto the OTG state machine to do so.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> +  unsigned long irqflags, struct otg_hcd_ops *ops)
> +{
> + struct usb_otg *otgd;
> + struct device *hcd_dev = hcd->self.controller;
> + struct device *otg_dev = usb_otg_get_device(hcd_dev);
> +

One big problem here is: there are two designs for current (IP) driver
code, one creates dedicated hcd device as roothub's parent, like dwc3.
Another one doesn't do this, roothub's parent is core device (or otg device
in your patch), like chipidea and dwc2.

Then, otg_dev will be glue layer device for chipidea after that.

Peter

> + if (!otg_dev)
> + return -EINVAL; /* we're definitely not OTG */
> +
> + /* we're otg but otg controller might not yet be registered */
> + mutex_lock(_list_mutex);
> + otgd = usb_otg_get_data(otg_dev);
> + mutex_unlock(_list_mutex);
> + if (!otgd) {
> + dev_dbg(hcd_dev,
> + "otg: controller not yet registered. waiting..\n");
> + /*
> +  * otg controller might register later. Put the hcd in
> +  * wait list and call us back when ready
> +  */
> + if (usb_otg_hcd_wait_add(otg_dev, hcd, irqnum, irqflags, ops)) {
> + dev_dbg(hcd_dev, "otg: failed to add to wait list\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> + }
> +
> + /* HCD will be started by OTG fsm when needed */
> + mutex_lock(>fsm.lock);
> + if (otgd->primary_hcd.hcd) {
> + /* probably a shared HCD ? */
> + if (usb_otg_hcd_is_primary_hcd(hcd)) {
> + dev_err(otg_dev, "otg: primary host already 
> registered\n");
> + goto err;
> + }
> +
> + if (hcd->shared_hcd == otgd->primary_hcd.hcd) {
> + if (otgd->shared_hcd.hcd) {
> + dev_err(otg_dev, "otg: shared host already 
> registered\n");
> + goto err;
> + }
> +
> + otgd->shared_hcd.hcd = hcd;
> + otgd->shared_hcd.irqnum = irqnum;
> + otgd->shared_hcd.irqflags = irqflags;
> + otgd->shared_hcd.ops = ops;
> + dev_info(otg_dev, "otg: shared host %s registered\n",
> +  dev_name(hcd->self.controller));
> + } else {
> + dev_err(otg_dev, "otg: invalid shared host %s\n",
> + dev_name(hcd->self.controller));
> + goto err;
> + }
> + } else {
> + if (!usb_otg_hcd_is_primary_hcd(hcd)) {
> + dev_err(otg_dev, "otg: primary host must be registered 
> first\n");
> + goto err;
> + }
> +
> + otgd->primary_hcd.hcd = hcd;
> + otgd->primary_hcd.irqnum = irqnum;
> + otgd->primary_hcd.irqflags = irqflags;
> + otgd->primary_hcd.ops = ops;
> + dev_info(otg_dev, "otg: primary host %s registered\n",
> +  dev_name(hcd->self.controller));
> + }
> +
> + /*
> +  * we're ready only if we have shared HCD
> +  * or we don't need shared HCD.
> +  */
> + if (otgd->shared_hcd.hcd || !otgd->primary_hcd.hcd->shared_hcd) {
> + otgd->fsm.otg->host = hcd_to_bus(hcd);
> + /* FIXME: set bus->otg_port if this is true OTG port with HNP */
> +
> + /* start FSM */
> + usb_otg_start_fsm(>fsm);
> + } else {
> + dev_dbg(otg_dev, "otg: can't start till shared host 
> registers\n");
> + }
> +
> + mutex_unlock(>fsm.lock);
> +
> + return 0;
> +
> +err:
> + mutex_unlock(>fsm.lock);
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_register_hcd);
> +
> +/**
> + * usb_otg_unregister_hcd - Unregister Host controller from OTG core
> + * @hcd: Host controller device
> + *
> + * This is used by the USB Host stack to unregister the Host controller
> + * from the OTG core. Ensures that Host controller is not running
> + * on successful return.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_unregister_hcd(struct usb_hcd *hcd)
> +{
> + struct usb_otg *otgd;
> + struct device *hcd_dev = hcd_to_bus(hcd)->controller;
> + struct device *otg_dev = usb_otg_get_device(hcd_dev);
> +
> + if (!otg_dev)
> + return -EINVAL; /* we're definitely not OTG */
> +
> + mutex_lock(_list_mutex);
> + otgd = 

[PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

The OTG core instantiates the OTG Finite State Machine
per OTG controller and manages starting/stopping the
host and gadget controllers based on the bus state.

It provides APIs for the following tasks

- Registering an OTG capable controller
- Registering Host and Gadget controllers to OTG core
- Providing inputs to and kicking the OTG state machine

Signed-off-by: Roger Quadros rog...@ti.com
---
 MAINTAINERS  |4 +-
 drivers/usb/Kconfig  |2 +-
 drivers/usb/Makefile |1 +
 drivers/usb/common/Makefile  |3 +-
 drivers/usb/common/usb-otg.c | 1061 ++
 drivers/usb/common/usb-otg.h |   71 +++
 drivers/usb/core/Kconfig |   11 +-
 include/linux/usb/otg.h  |  189 +++-
 8 files changed, 1321 insertions(+), 21 deletions(-)
 create mode 100644 drivers/usb/common/usb-otg.c
 create mode 100644 drivers/usb/common/usb-otg.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a9ae6c1..4bc1279 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10667,12 +10667,14 @@ S:Maintained
 F: Documentation/usb/ohci.txt
 F: drivers/usb/host/ohci*
 
-USB OTG FSM (Finite State Machine)
+USB OTG/DRD core and FSM (Finite State Machine)
 M: Peter Chen peter.c...@freescale.com
+M: Roger Quadros rog...@ti.com
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
 L: linux-...@vger.kernel.org
 S: Maintained
 F: drivers/usb/common/usb-otg-fsm.c
+F: drivers/usb/common/usb-otg.c
 
 USB OVER IP DRIVER
 M: Valentina Manea valentina.mane...@gmail.com
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 8ed451d..5b625e2 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -32,7 +32,7 @@ if USB_SUPPORT
 config USB_COMMON
tristate
default y
-   depends on USB || USB_GADGET
+   depends on USB || USB_GADGET || USB_OTG
 
 config USB_ARCH_HAS_HCD
def_bool y
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index d8926c6..769d13b 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -60,5 +60,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS)   += renesas_usbhs/
 obj-$(CONFIG_USB_GADGET)   += gadget/
 
 obj-$(CONFIG_USB_COMMON)   += common/
+obj-$(CONFIG_USB_OTG)  += common/
 
 obj-$(CONFIG_USBIP_CORE)   += usbip/
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 6bbb3ec..730d928 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -6,5 +6,6 @@ obj-$(CONFIG_USB_COMMON)  += usb-common.o
 usb-common-y += common.o
 usb-common-$(CONFIG_USB_LED_TRIG) += led.o
 
-obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
 obj-$(CONFIG_USB_ULPI_BUS) += ulpi.o
+usbotg-y   := usb-otg.o usb-otg-fsm.o
+obj-$(CONFIG_USB_OTG)  += usbotg.o
diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
new file mode 100644
index 000..930c2fe
--- /dev/null
+++ b/drivers/usb/common/usb-otg.c
@@ -0,0 +1,1061 @@
+/**
+ * drivers/usb/common/usb-otg.c - USB OTG core
+ *
+ * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
+ * Author: Roger Quadros rog...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/kernel.h
+#include linux/ktime.h
+#include linux/hrtimer.h
+#include linux/list.h
+#include linux/of.h
+#include linux/of_platform.h
+#include linux/usb/otg.h
+#include linux/usb/phy.h /* enum usb_otg_state */
+#include linux/usb/gadget.h
+#include linux/workqueue.h
+
+#include usb-otg.h
+
+struct otg_gcd {
+   struct usb_gadget *gadget;
+   struct otg_gadget_ops *ops;
+};
+
+/* OTG device list */
+LIST_HEAD(otg_list);
+static DEFINE_MUTEX(otg_list_mutex);
+
+/* Hosts and Gadgets waiting for OTG controller */
+struct otg_wait_data {
+   struct device *dev; /* OTG controller device */
+
+   struct otg_hcd primary_hcd;
+   struct otg_hcd shared_hcd;
+   struct otg_gcd gcd;
+   struct list_head list;
+};
+
+LIST_HEAD(wait_list);
+static DEFINE_MUTEX(wait_list_mutex);
+
+static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
+{
+   if (!hcd-primary_hcd)
+   return 1;
+   return hcd == hcd-primary_hcd;
+}
+
+/**
+ * Check if the OTG device is in our wait list and return
+ * otg_wait_data, else NULL.
+ *
+ * wait_list_mutex must be held.
+ */
+static struct otg_wait_data *usb_otg_get_wait(struct device *otg_dev)
+{
+   struct otg_wait_data *wait;
+
+   if (!otg_dev)
+   return NULL;
+
+   /* is there an entry for this otg_dev ?*/
+