Re: [PATCH v4 00/13] USB: OTG/DRD Core functionality
w what host and gadget controllers are > linked to the OTG controller. For DT boots we can provide that > information by adding "otg-controller" property to the host and > gadget controller nodes that points to the right otg controller. > For legacy boot we assume that OTG controller is the parent > of the host and gadget controllers. For DT if "otg-controller" > property is not present then parent child relationship constraint > applies. > > - The OTG controller driver must call usb_otg_register() to register > itself with the OTG core. It must also provide the required > OTG configuration, fsm operations and timer timeouts (optional) > via struct usb_otg_config. The fsm operations will be called > depending on the OTG bus state. > > - The host/gadget core stacks are modified to inform the OTG core > whenever a new host/gadget device is added. The OTG core then > checks if the host/gadget is part of the OTG controller and if yes > then prevents the host/gadget from starting till both host and > gadget are registered, OTG state machine is running and the > USB bus state is appropriate to start host/gadget. > For this, APIs have been added to host/gadget stacks to start/stop > the controllers from the OTG core. > For DT boots, If the OTG controller hasn't yet been registered > while the host/gadget are added, the OTG core will hold it in a wait list > and register them when the OTG controller registers. > > - No modification is needed for the host/gadget controller drivers. > They must ensure that their start/stop methods can be called repeatedly > and any shared resources between host & gadget are properly managed. > The OTG core ensures that both are not started simultaneously. > > - The OTG core instantiates one OTG state machine per OTG controller > and the necessary OTG timers to manage OTG state timeouts. > If none of the otg features are set during usb_otg_register() then it > instanciates a DRD (dual-role device) state machine instead. > The state machine is started when both host & gadget register and > stopped when either of them unregisters. The controllers are started > and stopped depending on bus state. > > - During the lifetime of the OTG state machine, inputs can be > provided to it by modifying the appropriate members of 'struct otg_fsm' > and calling usb_otg_sync_inputs(). This is typically done by the > OTG controller driver that called usb_otg_register(). > > -- > cheers, > -roger > > Roger Quadros (13): > usb: otg-fsm: Add documentation for struct otg_fsm > usb: otg-fsm: support multiple instances > usb: otg-fsm: Prevent build warning "VDBG" redefined > otg-fsm: move usb_bus_start_enum into otg-fsm->ops > usb: hcd.h: Add OTG to HCD interface > usb: gadget.h: Add OTG to gadget interface > usb: otg: add OTG core > usb: doc: dt-binding: Add otg-controller property > usb: chipidea: move from CONFIG_USB_OTG_FSM to CONFIG_USB_OTG > usb: hcd: Adapt to OTG core > usb: core: hub: Notify OTG fsm when A device sets b_hnp_enable > usb: gadget: udc: adapt to OTG core > usb: otg: Add dual-role device (DRD) support > > Documentation/devicetree/bindings/usb/generic.txt |5 + > Documentation/usb/chipidea.txt|2 +- > MAINTAINERS |4 +- > drivers/usb/Kconfig |2 +- > drivers/usb/Makefile |1 + > drivers/usb/chipidea/Makefile |2 +- > drivers/usb/chipidea/ci.h |2 +- > drivers/usb/chipidea/otg_fsm.c|1 + > drivers/usb/chipidea/otg_fsm.h|2 +- > drivers/usb/common/Makefile |3 +- > drivers/usb/common/usb-otg-fsm.c | 26 +- > drivers/usb/common/usb-otg.c | 1223 > + > drivers/usb/common/usb-otg.h | 71 ++ > drivers/usb/core/Kconfig | 11 +- > drivers/usb/core/hcd.c| 55 +- > drivers/usb/core/hub.c| 10 +- > drivers/usb/gadget/udc/udc-core.c | 124 ++- > drivers/usb/phy/Kconfig |2 +- > drivers/usb/phy/phy-fsl-usb.c |3 + > include/linux/usb/gadget.h| 14 + > include/linux/usb/hcd.h | 14 + > include/linux/usb/otg-fsm.h | 116 +- > include/linux/usb/otg.h | 191 +++- > 23 files changed, 1808 insertions(+), 76 deletions(-) > create mode 100644 drivers/usb/common/usb-otg.c > create mode 100644 drivers/usb/common/usb-otg.h > > -- > 2.1.4 > -- 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
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 > >>>>>>> | > >>>>>>> | > >>>>>>> |-|-| > >>>>>>
Re: [PATCH v4 07/13] usb: otg: add OTG core
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 > >>>>>>>>> | > >>>>>>>
Re: [PATCH v4 07/13] usb: otg: add OTG core
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. > >>>> > >
Re: [PATCH v4 07/13] usb: otg: add OTG core
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 controll
Re: [PATCH v4 07/13] usb: otg: add OTG core
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; > >&g
Re: [PATCH v4 04/13] otg-fsm: move usb_bus_start_enum into otg-fsm->ops
On Mon, Sep 07, 2015 at 12:57:21PM +0300, Roger Quadros wrote: > On 07/09/15 04:24, Peter Chen wrote: > > On Mon, Aug 24, 2015 at 04:21:15PM +0300, Roger Quadros wrote: > >> This is to prevent missing symbol build error if OTG is > >> enabled (built-in) and HCD core (CONFIG_USB) is module. > >> > >> Signed-off-by: Roger Quadros <rog...@ti.com> > >> Acked-by: Peter Chen <peter.c...@freescale.com> > >> --- > >> drivers/usb/common/usb-otg-fsm.c | 6 -- > >> drivers/usb/phy/phy-fsl-usb.c| 2 ++ > >> include/linux/usb/otg-fsm.h | 1 + > >> 3 files changed, 7 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/usb/common/usb-otg-fsm.c > >> b/drivers/usb/common/usb-otg-fsm.c > >> index a46f29a..6e56c8c 100644 > >> --- a/drivers/usb/common/usb-otg-fsm.c > >> +++ b/drivers/usb/common/usb-otg-fsm.c > >> @@ -165,8 +165,10 @@ static int otg_set_state(struct otg_fsm *fsm, enum > >> usb_otg_state new_state) > >>otg_loc_conn(fsm, 0); > >>otg_loc_sof(fsm, 1); > >>otg_set_protocol(fsm, PROTO_HOST); > >> - usb_bus_start_enum(fsm->otg->host, > >> - fsm->otg->host->otg_port);usb_bus_start_enum > >> + if (fsm->ops->start_enum) { > >> + fsm->ops->start_enum(fsm->otg->host, > >> + fsm->otg->host->otg_port); > >> + } > >>break; > >>case OTG_STATE_A_IDLE: > >>otg_drv_vbus(fsm, 0); > >> diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c > >> index ee3f2c2..19541ed 100644 > >> --- a/drivers/usb/phy/phy-fsl-usb.c > >> +++ b/drivers/usb/phy/phy-fsl-usb.c > >> @@ -783,6 +783,8 @@ static struct otg_fsm_ops fsl_otg_ops = { > >> > >>.start_host = fsl_otg_start_host, > >>.start_gadget = fsl_otg_start_gadget, > >> + > >> + .start_enum = usb_bus_start_enum, > >> }; > >> > >> /* Initialize the global variable fsl_otg_dev and request IRQ for OTG */ > >> diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h > >> index 672551c..75e82cc 100644 > >> --- a/include/linux/usb/otg-fsm.h > >> +++ b/include/linux/usb/otg-fsm.h > >> @@ -199,6 +199,7 @@ struct otg_fsm_ops { > >>void(*del_timer)(struct otg_fsm *fsm, enum otg_fsm_timer timer); > >>int (*start_host)(struct otg_fsm *fsm, int on); > >>int (*start_gadget)(struct otg_fsm *fsm, int on); > >> + int (*start_enum)(struct usb_bus *bus, unsigned port_num); > >> }; > >> > >> > > > > Get one build warning: > > > > In file included from > > /u/home/b29397/work/projects/usb/drivers/usb/chipidea/udc.c:23:0: > > /u/home/b29397/work/projects/usb/include/linux/usb/otg-fsm.h:207:27: > > warning: 'struct usb_bus' declared inside parameter list > > int (*start_enum)(struct usb_bus *bus, unsigned port_num); > > ^ > > /u/home/b29397/work/projects/usb/include/linux/usb/otg-fsm.h:207:27: > > warning: its scope is only this definition or declaration, which is > > probably not what you want > > > > It probably dues to we should not have struct usb_bus* at udc driver > > > How about changing it to struct otg_fsm instead like the other APIs? > And do we leave usb_bus_start_enum() as it is? > You have defined struct otg_hcd_ops to let otg visit hcd stuff, how about move this to otg_hcd_ops? > cheers, > -roger -- 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
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 10/13] usb: hcd: Adapt to OTG core
On Mon, Aug 24, 2015 at 04:21:21PM +0300, Roger Quadros wrote: > The existing usb_add/remove_hcd() functionality > remains unchanged for non-OTG devices. For OTG > devices they only register the HCD with the OTG core. > > Introduce usb_otg_add/remove_hcd() for use by OTG core. > These functions actually add/remove the HCD. > > Signed-off-by: Roger Quadros <rog...@ti.com> > --- > drivers/usb/core/hcd.c | 55 > +- > 1 file changed, 50 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > index c0fd1f6..4851682 100644 > --- a/drivers/usb/core/hcd.c > +++ b/drivers/usb/core/hcd.c > @@ -46,6 +46,7 @@ > #include > #include > #include > +#include > > #include "usb.h" > > @@ -2625,8 +2626,8 @@ static void usb_put_invalidate_rhdev(struct usb_hcd > *hcd) > * buffers of consistent memory, register the bus, request the IRQ line, > * and call the driver's reset() and start() routines. > */ > -int usb_add_hcd(struct usb_hcd *hcd, > - unsigned int irqnum, unsigned long irqflags) > +static int usb_otg_add_hcd(struct usb_hcd *hcd, > +unsigned int irqnum, unsigned long irqflags) You may change the kernel doc to this name too. > { > int retval; > struct usb_device *rhdev; > @@ -2839,17 +2840,16 @@ err_phy: > } > return retval; > } > -EXPORT_SYMBOL_GPL(usb_add_hcd); > > /** > - * usb_remove_hcd - shutdown processing for generic HCDs > + * usb_otg_remove_hcd - shutdown processing for generic HCDs > * @hcd: the usb_hcd structure to remove > * Context: !in_interrupt() > * > * Disconnects the root hub, then reverses the effects of usb_add_hcd(), > * invoking the HCD's stop() method. > */ > -void usb_remove_hcd(struct usb_hcd *hcd) > +static void usb_otg_remove_hcd(struct usb_hcd *hcd) > { > struct usb_device *rhdev = hcd->self.root_hub; > > @@ -2923,6 +2923,51 @@ void usb_remove_hcd(struct usb_hcd *hcd) > > usb_put_invalidate_rhdev(hcd); > } > + > +static struct otg_hcd_ops otg_hcd_intf = { > + .add = usb_otg_add_hcd, > + .remove = usb_otg_remove_hcd, > +}; > + > +/** > + * usb_add_hcd - finish generic HCD structure initialization and register > + * @hcd: the usb_hcd structure to initialize > + * @irqnum: Interrupt line to allocate > + * @irqflags: Interrupt type flags > + * > + * Finish the remaining parts of generic HCD initialization: allocate the > + * buffers of consistent memory, register the bus, request the IRQ line, > + * and call the driver's reset() and start() routines. > + * If it is an OTG device then it only registers the HCD with OTG core. > + * > + */ > +int usb_add_hcd(struct usb_hcd *hcd, > + unsigned int irqnum, unsigned long irqflags) > +{ > + /* If OTG device, OTG core takes care of adding HCD */ > + if (usb_otg_register_hcd(hcd, irqnum, irqflags, _hcd_intf)) > + return usb_otg_add_hcd(hcd, irqnum, irqflags); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(usb_add_hcd); > + > +/** > + * usb_remove_hcd - shutdown processing for generic HCDs > + * @hcd: the usb_hcd structure to remove > + * Context: !in_interrupt() > + * > + * Disconnects the root hub, then reverses the effects of usb_add_hcd(), > + * invoking the HCD's stop() method. > + * If it is an OTG device then it unregisters the HCD from OTG core > + * as well. > + */ > +void usb_remove_hcd(struct usb_hcd *hcd) > +{ > + /* If OTG device, OTG core takes care of stopping HCD */ > + if (usb_otg_unregister_hcd(hcd)) > + usb_otg_remove_hcd(hcd); > +} > EXPORT_SYMBOL_GPL(usb_remove_hcd); > > void > -- > 2.1.4 > -- 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
*/ > }; > > /** > @@ -56,8 +137,105 @@ struct usb_otg_caps { > bool adp_support; > }; > > +/** > + * struct usb_otg_config - otg controller configuration > + * @caps: otg capabilities of the controller > + * @ops: otg fsm operations > + * @otg_timeouts: override default otg fsm timeouts > + */ > +struct usb_otg_config { > + struct usb_otg_caps otg_caps; > + struct otg_fsm_ops *fsm_ops; > + unsigned otg_timeouts[NUM_OTG_FSM_TIMERS]; > +}; > + > extern const char *usb_otg_state_string(enum usb_otg_state state); > > +enum usb_dr_mode { > + USB_DR_MODE_UNKNOWN, > + USB_DR_MODE_HOST, > + USB_DR_MODE_PERIPHERAL, > + USB_DR_MODE_OTG, > +}; > + > +#if IS_ENABLED(CONFIG_USB_OTG) > +struct otg_fsm *usb_otg_register(struct device *dev, > + struct usb_otg_config *config); > +int usb_otg_unregister(struct device *dev); > +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum, > + unsigned long irqflags, struct otg_hcd_ops *ops); > +int usb_otg_unregister_hcd(struct usb_hcd *hcd); > +int usb_otg_register_gadget(struct usb_gadget *gadget, > + struct otg_gadget_ops *ops); > +int usb_otg_unregister_gadget(struct usb_gadget *gadget); > +void usb_otg_sync_inputs(struct otg_fsm *fsm); > +int usb_otg_kick_fsm(struct device *hcd_gcd_device); > +struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm); > +int usb_otg_start_host(struct otg_fsm *fsm, int on); > +int usb_otg_start_gadget(struct otg_fsm *fsm, int on); > + > +#else /* CONFIG_USB_OTG */ > + > +static inline struct otg_fsm *usb_otg_register(struct device *dev, > +struct usb_otg_config *config) > +{ > + return ERR_PTR(-ENOTSUPP); > +} > + > +static inline int usb_otg_unregister(struct device *dev) > +{ > + return -ENOTSUPP; > +} > + > +static inline int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int > irqnum, > +unsigned long irqflags, > +struct otg_hcd_ops *ops) > +{ > + return -ENOTSUPP; > +} > + > +static inline int usb_otg_unregister_hcd(struct usb_hcd *hcd) > +{ > + return -ENOTSUPP; > +} > + > +static inline int usb_otg_register_gadget(struct usb_gadget *gadget, > + struct otg_gadget_ops *ops) > +{ > + return -ENOTSUPP; > +} > + > +static inline int usb_otg_unregister_gadget(struct usb_gadget *gadget) > +{ > + return -ENOTSUPP; > +} > + > +static inline void usb_otg_sync_inputs(struct otg_fsm *fsm) > +{ > +} > + > +static inline int usb_otg_kick_fsm(struct device *hcd_gcd_device) > +{ > + return -ENOTSUPP; > +} > + > +static inline struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm) > +{ > + return NULL; > +} > + > +static inline int usb_otg_start_host(struct otg_fsm *fsm, int on) > +{ > + return -ENOTSUPP; > +} > + > +static inline int usb_otg_start_gadget(struct otg_fsm *fsm, int on) > +{ > + return -ENOTSUPP; > +} > +#endif /* CONFIG_USB_OTG */ > + > +/*- deprecated interface -*/ > /* Context: can sleep */ > static inline int > otg_start_hnp(struct usb_otg *otg) > @@ -109,14 +287,9 @@ otg_start_srp(struct usb_otg *otg) > return -ENOTSUPP; > } > > +/*---*/ > + > /* for OTG controller drivers (and maybe other stuff) */ > extern int usb_bus_start_enum(struct usb_bus *bus, unsigned port_num); > > -enum usb_dr_mode { > - USB_DR_MODE_UNKNOWN, > - USB_DR_MODE_HOST, > - USB_DR_MODE_PERIPHERAL, > - USB_DR_MODE_OTG, > -}; > - > #endif /* __LINUX_USB_OTG_H */ > -- > 2.1.4 > -- 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 04/13] otg-fsm: move usb_bus_start_enum into otg-fsm->ops
On Mon, Aug 24, 2015 at 04:21:15PM +0300, Roger Quadros wrote: > This is to prevent missing symbol build error if OTG is > enabled (built-in) and HCD core (CONFIG_USB) is module. > > Signed-off-by: Roger Quadros <rog...@ti.com> > Acked-by: Peter Chen <peter.c...@freescale.com> > --- > drivers/usb/common/usb-otg-fsm.c | 6 -- > drivers/usb/phy/phy-fsl-usb.c| 2 ++ > include/linux/usb/otg-fsm.h | 1 + > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/common/usb-otg-fsm.c > b/drivers/usb/common/usb-otg-fsm.c > index a46f29a..6e56c8c 100644 > --- a/drivers/usb/common/usb-otg-fsm.c > +++ b/drivers/usb/common/usb-otg-fsm.c > @@ -165,8 +165,10 @@ static int otg_set_state(struct otg_fsm *fsm, enum > usb_otg_state new_state) > otg_loc_conn(fsm, 0); > otg_loc_sof(fsm, 1); > otg_set_protocol(fsm, PROTO_HOST); > - usb_bus_start_enum(fsm->otg->host, > - fsm->otg->host->otg_port); > + if (fsm->ops->start_enum) { > + fsm->ops->start_enum(fsm->otg->host, > + fsm->otg->host->otg_port); > + } > break; > case OTG_STATE_A_IDLE: > otg_drv_vbus(fsm, 0); > diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c > index ee3f2c2..19541ed 100644 > --- a/drivers/usb/phy/phy-fsl-usb.c > +++ b/drivers/usb/phy/phy-fsl-usb.c > @@ -783,6 +783,8 @@ static struct otg_fsm_ops fsl_otg_ops = { > > .start_host = fsl_otg_start_host, > .start_gadget = fsl_otg_start_gadget, > + > + .start_enum = usb_bus_start_enum, > }; > > /* Initialize the global variable fsl_otg_dev and request IRQ for OTG */ > diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h > index 672551c..75e82cc 100644 > --- a/include/linux/usb/otg-fsm.h > +++ b/include/linux/usb/otg-fsm.h > @@ -199,6 +199,7 @@ struct otg_fsm_ops { > void(*del_timer)(struct otg_fsm *fsm, enum otg_fsm_timer timer); > int (*start_host)(struct otg_fsm *fsm, int on); > int (*start_gadget)(struct otg_fsm *fsm, int on); > + int (*start_enum)(struct usb_bus *bus, unsigned port_num); > }; > > Get one build warning: In file included from /u/home/b29397/work/projects/usb/drivers/usb/chipidea/udc.c:23:0: /u/home/b29397/work/projects/usb/include/linux/usb/otg-fsm.h:207:27: warning: 'struct usb_bus' declared inside parameter list int (*start_enum)(struct usb_bus *bus, unsigned port_num); ^ /u/home/b29397/work/projects/usb/include/linux/usb/otg-fsm.h:207:27: warning: its scope is only this definition or declaration, which is probably not what you want It probably dues to we should not have struct usb_bus* at udc driver -- 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 02/13] usb: otg-fsm: support multiple instances
On Mon, Aug 24, 2015 at 04:21:13PM +0300, Roger Quadros wrote: > Move the state_changed variable into struct otg_fsm > so that we can support multiple instances. > > Signed-off-by: Roger Quadros <rog...@ti.com> > --- > drivers/usb/common/usb-otg-fsm.c | 10 -- > include/linux/usb/otg-fsm.h | 1 + > 2 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/common/usb-otg-fsm.c > b/drivers/usb/common/usb-otg-fsm.c > index 61d538a..32862a4 100644 > --- a/drivers/usb/common/usb-otg-fsm.c > +++ b/drivers/usb/common/usb-otg-fsm.c > @@ -61,8 +61,6 @@ static int otg_set_protocol(struct otg_fsm *fsm, int > protocol) > return 0; > } > > -static int state_changed; > - > /* Called when leaving a state. Do state clean up jobs here */ > static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state > old_state) > { > @@ -123,7 +121,6 @@ static void otg_leave_state(struct otg_fsm *fsm, enum > usb_otg_state old_state) > /* Called when entering a state */ > static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state) > { > - state_changed = 1; > if (fsm->otg->state == new_state) > return 0; > VDBG("Set state: %s\n", usb_otg_state_string(new_state)); > @@ -237,6 +234,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum > usb_otg_state new_state) > } > > fsm->otg->state = new_state; > + fsm->state_changed = 1; > return 0; > } > > @@ -248,7 +246,7 @@ int otg_statemachine(struct otg_fsm *fsm) > mutex_lock(>lock); > > state = fsm->otg->state; > - state_changed = 0; > + fsm->state_changed = 0; > /* State machine state change judgement */ > > switch (state) { > @@ -361,7 +359,7 @@ int otg_statemachine(struct otg_fsm *fsm) > } > mutex_unlock(>lock); > > - VDBG("quit statemachine, changed = %d\n", state_changed); > - return state_changed; > + VDBG("quit statemachine, changed = %d\n", fsm->state_changed); > + return fsm->state_changed; > } > EXPORT_SYMBOL_GPL(otg_statemachine); > diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h > index fc5b4d9..20c8219 100644 > --- a/include/linux/usb/otg-fsm.h > +++ b/include/linux/usb/otg-fsm.h > @@ -195,6 +195,7 @@ struct otg_fsm { > /* Current usb protocol used: 0:undefine; 1:host; 2:client */ > int protocol; > struct mutex lock; > + bool state_changed; > }; > > struct otg_fsm_ops { > -- > 2.1.4 > Acked-by: Peter Chen <peter.c...@freescale.com> -- 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 00/13] USB: OTG/DRD Core functionality
; int usb_otg_start_gadget(struct otg_fsm *fsm, int on); > > Usage model: > --- > > - The OTG core needs to know what host and gadget controllers are > linked to the OTG controller. For DT boots we can provide that > information by adding "otg-controller" property to the host and > gadget controller nodes that points to the right otg controller. > For legacy boot we assume that OTG controller is the parent > of the host and gadget controllers. For DT if "otg-controller" > property is not present then parent child relationship constraint > applies. > > - The OTG controller driver must call usb_otg_register() to register > itself with the OTG core. It must also provide the required > OTG configuration, fsm operations and timer timeouts (optional) > via struct usb_otg_config. The fsm operations will be called > depending on the OTG bus state. > > - The host/gadget core stacks are modified to inform the OTG core > whenever a new host/gadget device is added. The OTG core then > checks if the host/gadget is part of the OTG controller and if yes > then prevents the host/gadget from starting till both host and > gadget are registered, OTG state machine is running and the > USB bus state is appropriate to start host/gadget. > For this, APIs have been added to host/gadget stacks to start/stop > the controllers from the OTG core. > For DT boots, If the OTG controller hasn't yet been registered > while the host/gadget are added, the OTG core will hold it in a wait list > and register them when the OTG controller registers. > > - No modification is needed for the host/gadget controller drivers. > They must ensure that their start/stop methods can be called repeatedly > and any shared resources between host & gadget are properly managed. > The OTG core ensures that both are not started simultaneously. > > - The OTG core instantiates one OTG state machine per OTG controller > and the necessary OTG timers to manage OTG state timeouts. > If none of the otg features are set during usb_otg_register() then it > instanciates a DRD (dual-role device) state machine instead. > The state machine is started when both host & gadget register and > stopped when either of them unregisters. The controllers are started > and stopped depending on bus state. > > - During the lifetime of the OTG state machine, inputs can be > provided to it by modifying the appropriate members of 'struct otg_fsm' > and calling usb_otg_sync_inputs(). This is typically done by the > OTG controller driver that called usb_otg_register(). > > -- > cheers, > -roger > > Roger Quadros (13): > usb: otg-fsm: Add documentation for struct otg_fsm > usb: otg-fsm: support multiple instances > usb: otg-fsm: Prevent build warning "VDBG" redefined > otg-fsm: move usb_bus_start_enum into otg-fsm->ops > usb: hcd.h: Add OTG to HCD interface > usb: gadget.h: Add OTG to gadget interface > usb: otg: add OTG core > usb: doc: dt-binding: Add otg-controller property > usb: chipidea: move from CONFIG_USB_OTG_FSM to CONFIG_USB_OTG > usb: hcd: Adapt to OTG core > usb: core: hub: Notify OTG fsm when A device sets b_hnp_enable > usb: gadget: udc: adapt to OTG core > usb: otg: Add dual-role device (DRD) support > > Documentation/devicetree/bindings/usb/generic.txt |5 + > Documentation/usb/chipidea.txt|2 +- > MAINTAINERS |4 +- > drivers/usb/Kconfig |2 +- > drivers/usb/Makefile |1 + > drivers/usb/chipidea/Makefile |2 +- > drivers/usb/chipidea/ci.h |2 +- > drivers/usb/chipidea/otg_fsm.c|1 + > drivers/usb/chipidea/otg_fsm.h|2 +- > drivers/usb/common/Makefile |3 +- > drivers/usb/common/usb-otg-fsm.c | 26 +- > drivers/usb/common/usb-otg.c | 1223 > + > drivers/usb/common/usb-otg.h | 71 ++ > drivers/usb/core/Kconfig | 11 +- > drivers/usb/core/hcd.c| 55 +- > drivers/usb/core/hub.c| 10 +- > drivers/usb/gadget/udc/udc-core.c | 124 ++- > drivers/usb/phy/Kconfig |2 +- > drivers/usb/phy/phy-fsl-usb.c |3 + > include/linux/usb/gadget.h| 14 + > include/linux/usb/hcd.h | 14 + > include/linux/usb/otg-fsm.h | 116 +- > include/linux/usb/otg.h | 191 +++- > 23 files changed, 1808 insertions(+), 76 deletions(-) > create mode 100644 drivers/usb/common/usb-otg.c > create mode 100644 drivers/usb/common/usb-otg.h > > -- > 2.1.4 > -- 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 1/9] usb: dwc3: add dual-role support
On Wed, Sep 02, 2015 at 05:24:16PM +0300, Roger Quadros wrote: > Register with the USB OTG core. Since we don't support > OTG yet we just work as a dual-role device even > if device tree says "otg". > > + > +static int dwc3_drd_init(struct dwc3 *dwc) > +{ > + int ret, id, vbus; > + struct usb_otg_caps *otgcaps = >otg_config.otg_caps; > + > + otgcaps->otg_rev = 0; > + otgcaps->hnp_support = false; > + otgcaps->srp_support = false; > + otgcaps->adp_support = false; > + dwc->otg_config.fsm_ops = _drd_ops; > + > + if (!dwc->edev) { > + dev_err(dwc->dev, "No extcon device found for OTG mode\n"); > + return -ENODEV; > + } > + Do All dwc3 platforms id/vbus need to get through extcon? Do the SoCs have id/vbus pin? -- 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 5/9] usb: dwc3: core: make dual-role work with OTG irq
On Wed, Sep 02, 2015 at 09:43:38AM -0500, Felipe Balbi wrote: > Hi, > > > + > > +static irqreturn_t dwc3_otg_irq(int irq, void *_dwc) > > +{ > > + struct dwc3 *dwc = _dwc; > > + irqreturn_t ret = IRQ_NONE; > > + u32 reg; > > + > > + spin_lock(>lock); > > this seems unnecessary, we're already in hardirq with IRQs disabled. > What sort of race could we have ? (in fact, this also needs change in > dwc3/gadget.c). > Is it possible the kernel process is accessing the content you will access? -- 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 00/13] USB: OTG/DRD Core functionality
relationship constraint applies. - The OTG controller driver must call usb_otg_register() to register itself with the OTG core. It must also provide the required OTG configuration, fsm operations and timer timeouts (optional) via struct usb_otg_config. The fsm operations will be called depending on the OTG bus state. - The host/gadget core stacks are modified to inform the OTG core whenever a new host/gadget device is added. The OTG core then checks if the host/gadget is part of the OTG controller and if yes then prevents the host/gadget from starting till both host and gadget are registered, OTG state machine is running and the USB bus state is appropriate to start host/gadget. For this, APIs have been added to host/gadget stacks to start/stop the controllers from the OTG core. For DT boots, If the OTG controller hasn't yet been registered while the host/gadget are added, the OTG core will hold it in a wait list and register them when the OTG controller registers. - No modification is needed for the host/gadget controller drivers. They must ensure that their start/stop methods can be called repeatedly and any shared resources between host gadget are properly managed. The OTG core ensures that both are not started simultaneously. - The OTG core instantiates one OTG state machine per OTG controller and the necessary OTG timers to manage OTG state timeouts. If none of the otg features are set during usb_otg_register() then it instanciates a DRD (dual-role device) state machine instead. The state machine is started when both host gadget register and stopped when either of them unregisters. The controllers are started and stopped depending on bus state. - During the lifetime of the OTG state machine, inputs can be provided to it by modifying the appropriate members of 'struct otg_fsm' and calling usb_otg_sync_inputs(). This is typically done by the OTG controller driver that called usb_otg_register(). -- cheers, -roger Roger Quadros (13): usb: otg-fsm: Add documentation for struct otg_fsm usb: otg-fsm: support multiple instances usb: otg-fsm: Prevent build warning VDBG redefined otg-fsm: move usb_bus_start_enum into otg-fsm-ops usb: hcd.h: Add OTG to HCD interface usb: gadget.h: Add OTG to gadget interface usb: otg: add OTG core usb: doc: dt-binding: Add otg-controller property usb: chipidea: move from CONFIG_USB_OTG_FSM to CONFIG_USB_OTG usb: hcd: Adapt to OTG core usb: core: hub: Notify OTG fsm when A device sets b_hnp_enable usb: gadget: udc: adapt to OTG core usb: otg: Add dual-role device (DRD) support Documentation/devicetree/bindings/usb/generic.txt |5 + Documentation/usb/chipidea.txt|2 +- MAINTAINERS |4 +- drivers/usb/Kconfig |2 +- drivers/usb/Makefile |1 + drivers/usb/chipidea/Makefile |2 +- drivers/usb/chipidea/ci.h |2 +- drivers/usb/chipidea/otg_fsm.c|1 + drivers/usb/chipidea/otg_fsm.h|2 +- drivers/usb/common/Makefile |3 +- drivers/usb/common/usb-otg-fsm.c | 26 +- drivers/usb/common/usb-otg.c | 1223 + drivers/usb/common/usb-otg.h | 71 ++ drivers/usb/core/Kconfig | 11 +- drivers/usb/core/hcd.c| 55 +- drivers/usb/core/hub.c| 10 +- drivers/usb/gadget/udc/udc-core.c | 124 ++- drivers/usb/phy/Kconfig |2 +- drivers/usb/phy/phy-fsl-usb.c |3 + include/linux/usb/gadget.h| 14 + include/linux/usb/hcd.h | 14 + include/linux/usb/otg-fsm.h | 116 +- include/linux/usb/otg.h | 191 +++- 23 files changed, 1808 insertions(+), 76 deletions(-) create mode 100644 drivers/usb/common/usb-otg.c create mode 100644 drivers/usb/common/usb-otg.h -- 2.1.4 -- 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 v3 07/11] usb: otg: add OTG core
On Fri, Aug 14, 2015 at 12:42:38PM +0300, Roger Quadros wrote: Hi Peter, On 13/07/15 13:20, Roger Quadros wrote: On 13/07/15 05:14, Peter Chen wrote: On Wed, Jul 08, 2015 at 01:19:33PM +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 rog...@ti.com --- MAINTAINERS | 4 +- drivers/usb/Kconfig | 2 +- drivers/usb/Makefile | 1 + drivers/usb/common/Makefile | 3 +- drivers/usb/common/usb-otg.c | 768 +++ drivers/usb/common/usb-otg.h | 71 drivers/usb/core/Kconfig | 11 +- include/linux/usb/otg.h | 91 - 8 files changed, 930 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 8133cef..b21278e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10640,12 +10640,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 USB_OTG can depends on USB || UB_GADGET? I didn't understand. The above is for USB_COMMON. 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/ The comment like above. What should it look like? Can you please clarify what you meant at the above two comments? Thanks. Forget them, I had thought the USB_OTG could be module. -- 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 v3 07/11] usb: otg: add OTG core
On Fri, Jul 17, 2015 at 03:06:18PM +0300, Roger Quadros wrote: + +/** + * OTG FSM ops function to start/stop host + */ +static int usb_otg_start_host(struct otg_fsm *fsm, int on) +{ + struct otg_data *otgd = container_of(fsm, struct otg_data, fsm); + struct otg_hcd_ops *hcd_ops; + + dev_dbg(otgd-dev, otg: %s %d\n, __func__, on); + if (!fsm-otg-host) { + WARN_ONCE(1, otg: fsm running without host\n); + return 0; + } + + if (on) { + /* OTG device operations */ + if (otgd-start_host) + otgd-start_host(fsm, on); + + /* start host */ + hcd_ops = otgd-primary_hcd.ops; + hcd_ops-add(otgd-primary_hcd.hcd, otgd-primary_hcd.irqnum, + otgd-primary_hcd.irqflags); + if (otgd-shared_hcd.hcd) { + hcd_ops = otgd-shared_hcd.ops; + hcd_ops-add(otgd-shared_hcd.hcd, + otgd-shared_hcd.irqnum, + otgd-shared_hcd.irqflags); + } + } else { + /* stop host */ + if (otgd-shared_hcd.hcd) { + hcd_ops = otgd-shared_hcd.ops; + hcd_ops-remove(otgd-shared_hcd.hcd); + } + hcd_ops = otgd-primary_hcd.ops; + hcd_ops-remove(otgd-primary_hcd.hcd); + + /* OTG device operations */ + if (otgd-start_host) + otgd-start_host(fsm, on); + } + + return 0; +} I do not see much benefit by this override function, usb_add/remove_hcd can be simply included by controller's start_host function, also there maybe some additional operations after usb_add_hcd, but this override function limit usb_add_hcd() is the last step. I had tried host start/stop way before but Alan's suggestion was to use bind/unbind the host controller completely as that is much simpler [1] http://article.gmane.org/gmane.linux.usb.general/123842 Jun may want we have some ways to override otg_fsm-ops by platform drivers, I think we all agree to call usb_add_hcd/usb_remove_hcd to start/stop host roles, just some platforms may need more than just call them. -- 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 v3 10/11] usb: otg: Add dual-role device (DRD) support
On Fri, Jul 17, 2015 at 01:47:12PM +0300, Roger Quadros wrote: + * DRD mode hardware Inputs + * + * @id: TRUE for B-device, FALSE for A-device. + * @vbus: VBUS voltage in regulation. + * * OTG hardware Inputs * *Common inputs for A and B device @@ -122,7 +127,8 @@ enum otg_fsm_timer { */ struct otg_fsm { /* Input */ - int id; + int id; /* DRD + OTG */ + int vbus; /* DRD only */ Existing b_sess_vld can be also used for drd only case, no need create a new flag. b_sess_vld is a bit confusing to people not familiar with OTG. My suggestion is to use dedicated 'vbus' flag for DRD case for simplicity. Since OTG DRD is the subset in OTG FSM (FSM, data structure, APIs, etc), I agree with Jun to reuse existing variables, and we can add some comments for b_sess_vld if needed. int adp_change; int power_up; int a_srp_det; cheers, -roger -- 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 v3 06/11] usb: gadget.h: Add OTG to gadget interface
On Wed, Jul 08, 2015 at 01:19:32PM +0300, Roger Quadros wrote: The OTG core will use struct otg_gadget_ops to start/stop the gadget controller. The main purpose of this interface is to avoid directly calling usb_gadget_start/stop() from the OTG core as they wouldn't be defined in the built-in symbol table if CONFIG_USB_GADGET is m. Signed-off-by: Roger Quadros rog...@ti.com --- include/linux/usb/gadget.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index 4f3dfb7..0b4b341 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -887,6 +887,20 @@ struct usb_gadget_driver { }; +/*-*/ + +/** + * struct otg_gadget_ops - Interface between OTG core and gadget + * + * Provided by the gadget core to allow the OTG core to start/stop the gadget + * + * @start: function to start the gadget + * @stop: function to stop the gadget + */ +struct otg_gadget_ops { + int (*start)(struct usb_gadget *gadget); + int (*stop)(struct usb_gadget *gadget); +}; /*-*/ -- 2.1.4 Reviewed-by: Peter Chen peter.c...@freescale.com -- 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 v3 05/11] usb: hcd.h: Add OTG to HCD interface
On Wed, Jul 08, 2015 at 01:19:31PM +0300, Roger Quadros wrote: The OTG core will use struct otg_hcd_ops to add/remove the HCD controller. The main purpose of this interface is to avoid directly calling usb_add/remove_hcd() from the OTG core as they wouldn't be defined in the built-in symbol table if CONFIG_USB is m. Signed-off-by: Roger Quadros rog...@ti.com --- include/linux/usb/hcd.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index c9aa779..4108288 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -386,6 +386,20 @@ struct hc_driver { }; +/** + * struct otg_hcd_ops - Interface between OTG core and HCD + * + * Provided by the HCD core to allow the OTG core to start/stop the HCD + * + * @add: function to add the HCD + * @remove: function to remove the HCD + */ +struct otg_hcd_ops { + int (*add)(struct usb_hcd *hcd, +unsigned int irqnum, unsigned long irqflags); + void (*remove)(struct usb_hcd *hcd); +}; + static inline int hcd_giveback_urb_in_bh(struct usb_hcd *hcd) { return hcd-driver-flags HCD_BH; -- 2.1.4 Reviewed-by: Peter Chen peter.c...@freescale.com -- 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 v3 04/11] otg-fsm: move usb_bus_start_enum into otg-fsm-ops
On Wed, Jul 08, 2015 at 01:19:30PM +0300, Roger Quadros wrote: This is to prevent missing symbol build error if OTG is enabled (built-in) and HCD core (CONFIG_USB) is module. Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/common/usb-otg-fsm.c | 6 -- drivers/usb/phy/phy-fsl-usb.c| 2 ++ include/linux/usb/otg-fsm.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index 1873eb3..156fd25 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -166,8 +166,10 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state) otg_loc_conn(fsm, 0); otg_loc_sof(fsm, 1); otg_set_protocol(fsm, PROTO_HOST); - usb_bus_start_enum(fsm-otg-host, - fsm-otg-host-otg_port); + if (fsm-ops-start_enum) { + fsm-ops-start_enum(fsm-otg-host, + fsm-otg-host-otg_port); + } break; case OTG_STATE_A_IDLE: otg_drv_vbus(fsm, 0); diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c index ee3f2c2..19541ed 100644 --- a/drivers/usb/phy/phy-fsl-usb.c +++ b/drivers/usb/phy/phy-fsl-usb.c @@ -783,6 +783,8 @@ static struct otg_fsm_ops fsl_otg_ops = { .start_host = fsl_otg_start_host, .start_gadget = fsl_otg_start_gadget, + + .start_enum = usb_bus_start_enum, }; /* Initialize the global variable fsl_otg_dev and request IRQ for OTG */ diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index c631dde..22d8baa 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -198,6 +198,7 @@ struct otg_fsm_ops { void(*del_timer)(struct otg_fsm *fsm, enum otg_fsm_timer timer); int (*start_host)(struct otg_fsm *fsm, int on); int (*start_gadget)(struct otg_fsm *fsm, int on); + int (*start_enum)(struct usb_bus *bus, unsigned port_num); }; -- 2.1.4 Acked-by: Peter Chen peter.c...@freescale.com -- 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 v3 04/11] otg-fsm: move usb_bus_start_enum into otg-fsm-ops
On Wed, Jul 15, 2015 at 04:30:27PM +0300, Roger Quadros wrote: On 14/07/15 03:34, Peter Chen wrote: On Mon, Jul 13, 2015 at 01:13:54PM +0300, Roger Quadros wrote: Peter, On 13/07/15 04:58, Peter Chen wrote: On Wed, Jul 08, 2015 at 01:19:30PM +0300, Roger Quadros wrote: This is to prevent missing symbol build error if OTG is enabled (built-in) and HCD core (CONFIG_USB) is module. We may let the OTG-DRD/OTG-FSM depends on CONFIG_USB to fix it. CONFIG_OTG already depends on CONFIG_USB as it is a sub-option of CONFIG_USB. It doesn't depend on CONFIG_USB_GADGET and that can be fixed. But dependency is not the problem here. Symbols not available to OTG driver when USB/GADGET is 'm' is the problem. e.g. CONFIG_USB_OTG is always built-in. we need to work if CONFIG_USB is 'm'/'y' _and_ if CONFIG_USB_GADGET is 'm'/'y' below should fix this issue, but we may need to make some changes for code which are defined by CONFIG_USB_OTG. diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig index a99c89e..5e374ad 100644 --- a/drivers/usb/core/Kconfig +++ b/drivers/usb/core/Kconfig @@ -42,8 +42,9 @@ config USB_DYNAMIC_MINORS If you are unsure about this, say N here. config USB_OTG - bool OTG support + tristate OTG support depends on PM + depends on USB USB_GADGET default n help The most notable feature of USB OTG is support for a With this USB_OTG will become 'm' when either USB or USB_GADGET is m and will break if either USB or USB_GADGET is made y as all OTG core API symbols won't be available. :) Ok, after thinking more, seems we can't handle properly if USB_OTG as 'm', your idea that using host/gadget/fsm-ops to call hcd/gadget API and the controller driver will defines these ops (due to it will use hcd/gadget function) is proper way currently. -- 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 v3 00/11] USB: OTG/DRD Core functionality
On Tue, Jul 14, 2015 at 11:18:30AM -0700, Andrew Bresticker wrote: Hi Peter, On Mon, Jul 13, 2015 at 5:59 PM, Peter Chen peter.c...@freescale.com wrote: On Mon, Jul 13, 2015 at 12:14:43PM -0700, Andrew Bresticker wrote: Hi Roger, On Wed, Jul 8, 2015 at 3:19 AM, Roger Quadros rog...@ti.com wrote: Usage model: --- - The OTG controller device is assumed to be the parent of the host and gadget controller. It must call usb_otg_register() before populating the host and gadget devices so that the OTG core is aware that it is an OTG device before the host gadget register. The OTG controller must provide struct otg_fsm_ops * which will be called by the OTG core depending on OTG bus state. I'm wondering if the requirement that the OTG controller be the parent of the USB host/device-controllers makes sense. For some context, I'm working on adding dual-role support for Tegra210, specifically on a system with USB Type-C. On Tegra, the USB host-controller and USB device-controller are two separate IP blocks (XUSB host and XUSB device) with another, separate, IP block (XUSB padctl) for the USB PHY and OTG support. In the non-Type-C case, your OTG framework could work well, though it's debatable as to whether or not the XUSB padctl device should be a parent to the XUSB host/device-controller devices (currently it isn't - it's just a PHY provider). But in the Type-C case, it's an off-chip embedded controller that determines the dual-role status of the Type-C port, so the above requirement doesn't make sense at all. Hi Andrew, I think your problem is how to add your core driver to manage device and host functionality together, and once you find how (through padctl/type-c controller) to do it based on current code, it will be clear how to use roger proposal framework at that time. Most of current core drivers, we use extcon driver (through gpio) or USB vbus/id pin (through internal registers) to manager roles. Right, currently I'm modeling the Type-C controller as an extcon device and handle the role-changes in the core drivers, but that doesn't really make sense for the non-Type-C case where we use the XUSB padctl controller and need a full OTG state-machine. The full OTG FSM is only applied if your board needs it, you can disable it through dts. Jun [1] and Roger's patchset are for it. Roger's new OTG/DRD framework would fit my situation perfectly since it makes the host/device-controller drivers independent from all the OTG/role-changing logic. The only issue is the requirement that the OTG/DRD controller be the parent device of the host/device controllers. The core device is the parent for host/device device, the OTG core just use the pointer of it, Roger does an example using dwc3 [2]. [1] http://www.spinics.net/lists/linux-usb/msg127110.html [2] http://www.spinics.net/lists/linux-usb/msg126999.html -- 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 v3 00/11] USB: OTG/DRD Core functionality
On Mon, Jul 13, 2015 at 12:14:43PM -0700, Andrew Bresticker wrote: Hi Roger, On Wed, Jul 8, 2015 at 3:19 AM, Roger Quadros rog...@ti.com wrote: Usage model: --- - The OTG controller device is assumed to be the parent of the host and gadget controller. It must call usb_otg_register() before populating the host and gadget devices so that the OTG core is aware that it is an OTG device before the host gadget register. The OTG controller must provide struct otg_fsm_ops * which will be called by the OTG core depending on OTG bus state. I'm wondering if the requirement that the OTG controller be the parent of the USB host/device-controllers makes sense. For some context, I'm working on adding dual-role support for Tegra210, specifically on a system with USB Type-C. On Tegra, the USB host-controller and USB device-controller are two separate IP blocks (XUSB host and XUSB device) with another, separate, IP block (XUSB padctl) for the USB PHY and OTG support. In the non-Type-C case, your OTG framework could work well, though it's debatable as to whether or not the XUSB padctl device should be a parent to the XUSB host/device-controller devices (currently it isn't - it's just a PHY provider). But in the Type-C case, it's an off-chip embedded controller that determines the dual-role status of the Type-C port, so the above requirement doesn't make sense at all. Hi Andrew, I think your problem is how to add your core driver to manage device and host functionality together, and once you find how (through padctl/type-c controller) to do it based on current code, it will be clear how to use roger proposal framework at that time. Most of current core drivers, we use extcon driver (through gpio) or USB vbus/id pin (through internal registers) to manager roles. My idea was to have the OTG/DRD controller explicitly specify its host and device controllers, so in DT, something like: otg-controller { ... device-controller = usb_device; host-controller = usb_host; ... }; usb_device: usb-device@ { ... }; usb_host: usb-host@... { ... }; What do you think? Thanks, Andrew -- 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 v3 04/11] otg-fsm: move usb_bus_start_enum into otg-fsm-ops
On Mon, Jul 13, 2015 at 01:13:54PM +0300, Roger Quadros wrote: Peter, On 13/07/15 04:58, Peter Chen wrote: On Wed, Jul 08, 2015 at 01:19:30PM +0300, Roger Quadros wrote: This is to prevent missing symbol build error if OTG is enabled (built-in) and HCD core (CONFIG_USB) is module. We may let the OTG-DRD/OTG-FSM depends on CONFIG_USB to fix it. CONFIG_OTG already depends on CONFIG_USB as it is a sub-option of CONFIG_USB. It doesn't depend on CONFIG_USB_GADGET and that can be fixed. But dependency is not the problem here. Symbols not available to OTG driver when USB/GADGET is 'm' is the problem. e.g. CONFIG_USB_OTG is always built-in. we need to work if CONFIG_USB is 'm'/'y' _and_ if CONFIG_USB_GADGET is 'm'/'y' below should fix this issue, but we may need to make some changes for code which are defined by CONFIG_USB_OTG. diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig index a99c89e..5e374ad 100644 --- a/drivers/usb/core/Kconfig +++ b/drivers/usb/core/Kconfig @@ -42,8 +42,9 @@ config USB_DYNAMIC_MINORS If you are unsure about this, say N here. config USB_OTG - bool OTG support + tristate OTG support depends on PM + depends on USB USB_GADGET default n help The most notable feature of USB OTG is support for a Peter Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/common/usb-otg-fsm.c | 6 -- drivers/usb/phy/phy-fsl-usb.c| 2 ++ include/linux/usb/otg-fsm.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index 1873eb3..156fd25 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -166,8 +166,10 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state) otg_loc_conn(fsm, 0); otg_loc_sof(fsm, 1); otg_set_protocol(fsm, PROTO_HOST); - usb_bus_start_enum(fsm-otg-host, - fsm-otg-host-otg_port); + if (fsm-ops-start_enum) { + fsm-ops-start_enum(fsm-otg-host, + fsm-otg-host-otg_port); + } break; case OTG_STATE_A_IDLE: otg_drv_vbus(fsm, 0); diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c index ee3f2c2..19541ed 100644 --- a/drivers/usb/phy/phy-fsl-usb.c +++ b/drivers/usb/phy/phy-fsl-usb.c @@ -783,6 +783,8 @@ static struct otg_fsm_ops fsl_otg_ops = { .start_host = fsl_otg_start_host, .start_gadget = fsl_otg_start_gadget, + + .start_enum = usb_bus_start_enum, }; /* Initialize the global variable fsl_otg_dev and request IRQ for OTG */ diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index c631dde..22d8baa 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -198,6 +198,7 @@ struct otg_fsm_ops { void(*del_timer)(struct otg_fsm *fsm, enum otg_fsm_timer timer); int (*start_host)(struct otg_fsm *fsm, int on); int (*start_gadget)(struct otg_fsm *fsm, int on); + int (*start_enum)(struct usb_bus *bus, unsigned port_num); }; -- 2.1.4 -- 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 v3 01/11] usb: otg-fsm: Add documentation for struct otg_fsm
On Wed, Jul 08, 2015 at 01:19:27PM +0300, Roger Quadros wrote: struct otg_fsm is the interface to the OTG state machine. Document the input, output and internal state variables. Definations are taken from Table 7-2 and Table 7-4 of the USB OTG EH Specification Rev.2.0 Re-arrange some of the members as per use case for more clarity. Signed-off-by: Roger Quadros rog...@ti.com --- include/linux/usb/otg-fsm.h | 89 + 1 file changed, 82 insertions(+), 7 deletions(-) diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index f728f18..ca508c2 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -59,37 +59,112 @@ enum otg_fsm_timer { NUM_OTG_FSM_TIMERS, }; -/* OTG state machine according to the OTG spec */ +/** + * struct otg_fsm - OTG state machine according to the OTG spec + * + * OTG hardware Inputs + * + * Common inputs for A and B device + * @id: TRUE for B-device, FALSE for A-device. + * @adp_change: TRUE when current ADP measurement (n) value, compared to the + * ADP measurement taken at n-2, differs by more than CADP_THR + * @power_up:TRUE when the OTG device first powers up its USB system and + * ADP measurement taken if ADP capable + * + * A-Device state inputs + * @a_srp_det: TRUE if the A-device detects SRP + * @a_vbus_vld: TRUE when VBUS voltage is in regulation + * @b_conn: TRUE if the A-device detects connection from the B-device + * @a_bus_resume: TRUE when the B-device detects that the A-device is signaling + * a resume (K state) + * B-Device state inputs + * @a_bus_suspend: TRUE when the B-device detects that the A-device has put the + * bus into suspend + * @a_conn: TRUE if the B-device detects a connection from the A-device + * @b_se0_srp: TRUE when the line has been at SE0 for more than the minimum + * time before generating SRP + * @b_ssend_srp: TRUE when the VBUS has been below VOTG_SESS_VLD for more than + *the minimum time before generating SRP + * @b_sess_vld: TRUE when the B-device detects that the voltage on VBUS is + * above VOTG_SESS_VLD + * @test_device: TRUE when the B-device switches to B-Host and detects an OTG + * test device. This must be set by host/hub driver + * + * Application inputs (A-Device) + * @a_bus_drop: TRUE when A-device application needs to power down the bus + * @a_bus_req: TRUE when A-device application wants to use the bus. + * FALSE to suspend the bus + * + * Application inputs (B-Device) + * @b_bus_req: TRUE during the time that the Application running on the + * B-device wants to use the bus + * + * Auxilary inputs (OTG v1.3 only. Obsolete now.) + * @a_sess_vld: TRUE if the A-device detects that VBUS is above VA_SESS_VLD + * @b_bus_suspend: TRUE when the A-device detects that the B-device has put + * the bus into suspend + * @b_bus_resume: TRUE when the A-device detects that the B-device is signaling + *resume on the bus + * + * OTG Output status. Read only for users. updated by otg_ops() helpers updated by OTG FSM helpers defined in this file Only one tiny comment, others are ok. Acked-by: Peter Chen peter.c...@freescale.com -- 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 v3 03/11] usb: otg-fsm: Prevent build warning VDBG redefined
On Wed, Jul 08, 2015 at 01:19:29PM +0300, Roger Quadros wrote: If usb/otg-fsm.h and usb/composite.h are included together then it results in the build warning [1]. Prevent that by using dev_vdbg() instead. Also get rid of MPC_LOC which doesn't seem to be used by anyone. [1] - warning fixed by this patch: In file included from drivers/usb/dwc3/core.h:33, from drivers/usb/dwc3/ep0.c:33: include/linux/usb/otg-fsm.h:30:1: warning: VDBG redefined In file included from drivers/usb/dwc3/ep0.c:31: include/linux/usb/composite.h:615:1: warning: this is the location of the previous definition Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/chipidea/otg_fsm.c | 1 + drivers/usb/common/usb-otg-fsm.c | 12 +++- drivers/usb/phy/phy-fsl-usb.c| 1 + include/linux/usb/otg-fsm.h | 19 --- 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c index 19d655a..6e67f94 100644 --- a/drivers/usb/chipidea/otg_fsm.c +++ b/drivers/usb/chipidea/otg_fsm.c @@ -777,6 +777,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci) ci-fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0; ci-fsm.otg-state = OTG_STATE_UNDEFINED; ci-fsm.ops = ci_otg_ops; + ci-fsm.dev = ci-dev; mutex_init(ci-fsm.lock); diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index 42c6376..1873eb3 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -36,8 +36,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol) int ret = 0; if (fsm-protocol != protocol) { - VDBG(Changing role fsm-protocol= %d; new protocol= %d\n, - fsm-protocol, protocol); + dev_vdbg(fsm-dev, + Changing role fsm-protocol= %d; new protocol= %d\n, + fsm-protocol, protocol); /* stop old protocol */ if (fsm-protocol == PROTO_HOST) ret = otg_start_host(fsm, 0); @@ -124,7 +125,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state) fsm-state_changed = 1; if (fsm-otg-state == new_state) return 0; - VDBG(Set state: %s\n, usb_otg_state_string(new_state)); + dev_vdbg(fsm-dev, Set state: %s\n, usb_otg_state_string(new_state)); otg_leave_state(fsm, fsm-otg-state); switch (new_state) { case OTG_STATE_B_IDLE: @@ -251,7 +252,7 @@ int otg_statemachine(struct otg_fsm *fsm) switch (state) { case OTG_STATE_UNDEFINED: - VDBG(fsm-id = %d\n, fsm-id); + dev_vdbg(fsm-dev, fsm-id = %d\n, fsm-id); if (fsm-id) otg_set_state(fsm, OTG_STATE_B_IDLE); else @@ -359,7 +360,8 @@ int otg_statemachine(struct otg_fsm *fsm) } mutex_unlock(fsm-lock); - VDBG(quit statemachine, changed = %d\n, fsm-state_changed); + dev_vdbg(fsm-dev, quit statemachine, changed = %d\n, + fsm-state_changed); return fsm-state_changed; } EXPORT_SYMBOL_GPL(otg_statemachine); diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c index 94eb292..ee3f2c2 100644 --- a/drivers/usb/phy/phy-fsl-usb.c +++ b/drivers/usb/phy/phy-fsl-usb.c @@ -817,6 +817,7 @@ static int fsl_otg_conf(struct platform_device *pdev) /* Set OTG state machine operations */ fsl_otg_tc-fsm.ops = fsl_otg_ops; + fsl_otg_tc-fsm.dev = pdev-dev; /* initialize the otg structure */ fsl_otg_tc-phy.label = DRIVER_DESC; diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index 243274f..c631dde 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -18,24 +18,10 @@ #ifndef __LINUX_USB_OTG_FSM_H #define __LINUX_USB_OTG_FSM_H +#include linux/device.h #include linux/mutex.h #include linux/errno.h -#undef VERBOSE - -#ifdef VERBOSE -#define VDBG(fmt, args...) pr_debug([%s] fmt , \ - __func__, ## args) -#else -#define VDBG(stuff...) do {} while (0) -#endif - -#ifdef VERBOSE -#define MPC_LOC printk(Current Location [%s]:[%d]\n, __FILE__, __LINE__) -#else -#define MPC_LOC do {} while (0) -#endif - #define PROTO_UNDEF (0) #define PROTO_HOST (1) #define PROTO_GADGET (2) @@ -195,6 +181,9 @@ struct otg_fsm { int protocol; struct mutex lock; bool state_changed; + + /* for debug prints */ + struct device *dev; }; struct otg_fsm_ops { -- 2.1.4 Acked-by: Peter Chen peter.c...@freescale.com -- 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
Re: [PATCH v3 04/11] otg-fsm: move usb_bus_start_enum into otg-fsm-ops
On Wed, Jul 08, 2015 at 01:19:30PM +0300, Roger Quadros wrote: This is to prevent missing symbol build error if OTG is enabled (built-in) and HCD core (CONFIG_USB) is module. We may let the OTG-DRD/OTG-FSM depends on CONFIG_USB to fix it. Peter Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/common/usb-otg-fsm.c | 6 -- drivers/usb/phy/phy-fsl-usb.c| 2 ++ include/linux/usb/otg-fsm.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index 1873eb3..156fd25 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -166,8 +166,10 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state) otg_loc_conn(fsm, 0); otg_loc_sof(fsm, 1); otg_set_protocol(fsm, PROTO_HOST); - usb_bus_start_enum(fsm-otg-host, - fsm-otg-host-otg_port); + if (fsm-ops-start_enum) { + fsm-ops-start_enum(fsm-otg-host, + fsm-otg-host-otg_port); + } break; case OTG_STATE_A_IDLE: otg_drv_vbus(fsm, 0); diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c index ee3f2c2..19541ed 100644 --- a/drivers/usb/phy/phy-fsl-usb.c +++ b/drivers/usb/phy/phy-fsl-usb.c @@ -783,6 +783,8 @@ static struct otg_fsm_ops fsl_otg_ops = { .start_host = fsl_otg_start_host, .start_gadget = fsl_otg_start_gadget, + + .start_enum = usb_bus_start_enum, }; /* Initialize the global variable fsl_otg_dev and request IRQ for OTG */ diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index c631dde..22d8baa 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -198,6 +198,7 @@ struct otg_fsm_ops { void(*del_timer)(struct otg_fsm *fsm, enum otg_fsm_timer timer); int (*start_host)(struct otg_fsm *fsm, int on); int (*start_gadget)(struct otg_fsm *fsm, int on); + int (*start_enum)(struct usb_bus *bus, unsigned port_num); }; -- 2.1.4 -- 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 v3 02/11] usb: otg-fsm: support multiple instances
On Fri, Jul 10, 2015 at 04:06:43PM +0800, Li Jun wrote: On Wed, Jul 08, 2015 at 01:19:28PM +0300, Roger Quadros wrote: Move the state_changed variable into struct otg_fsm so that we can support multiple instances. I am not sure if multiple instances may happen since OTG protocol requires only one OTG port can be equipped on OTG device. It is ok the software can support more than spec requires, the user can only use one and the spec may change in future :) Li Jun Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/common/usb-otg-fsm.c | 10 -- include/linux/usb/otg-fsm.h | 1 + 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index 61d538a..42c6376 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -61,8 +61,6 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol) return 0; } -static int state_changed; - /* Called when leaving a state. Do state clean up jobs here */ static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state old_state) { @@ -123,7 +121,7 @@ static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state old_state) /* Called when entering a state */ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state) { - state_changed = 1; + fsm-state_changed = 1; if (fsm-otg-state == new_state) return 0; fsm-state_changed = 1; should be put here, I think. Yes, seems it is the problem at current code. Li Jun VDBG(Set state: %s\n, usb_otg_state_string(new_state)); @@ -248,7 +246,7 @@ int otg_statemachine(struct otg_fsm *fsm) mutex_lock(fsm-lock); state = fsm-otg-state; - state_changed = 0; + fsm-state_changed = 0; /* State machine state change judgement */ switch (state) { @@ -361,7 +359,7 @@ int otg_statemachine(struct otg_fsm *fsm) } mutex_unlock(fsm-lock); - VDBG(quit statemachine, changed = %d\n, state_changed); - return state_changed; + VDBG(quit statemachine, changed = %d\n, fsm-state_changed); + return fsm-state_changed; } EXPORT_SYMBOL_GPL(otg_statemachine); diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index ca508c2..243274f 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -194,6 +194,7 @@ struct otg_fsm { /* Current usb protocol used: 0:undefine; 1:host; 2:client */ int protocol; struct mutex lock; + bool state_changed; }; struct otg_fsm_ops { -- 2.1.4 -- 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 v3 07/11] usb: otg: add OTG core
On Wed, Jul 08, 2015 at 01:19:33PM +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 rog...@ti.com --- MAINTAINERS | 4 +- drivers/usb/Kconfig | 2 +- drivers/usb/Makefile | 1 + drivers/usb/common/Makefile | 3 +- drivers/usb/common/usb-otg.c | 768 +++ drivers/usb/common/usb-otg.h | 71 drivers/usb/core/Kconfig | 11 +- include/linux/usb/otg.h | 91 - 8 files changed, 930 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 8133cef..b21278e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10640,12 +10640,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 USB_OTG can depends on USB || UB_GADGET? 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/ The comment like above. 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..0379034 --- /dev/null +++ b/drivers/usb/common/usb-otg.c @@ -0,0 +1,768 @@ +/** + * 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/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 + +/* to link timer with callback data */ +struct otg_timer { + struct hrtimer timer; + ktime_t timeout; + /* callback data */ + int *timeout_bit; + struct otg_data *otgd; +}; + +struct otg_hcd { + struct usb_hcd *hcd; + unsigned int irqnum; + unsigned long irqflags; + struct otg_hcd_ops *ops; +}; + +struct otg_data { + struct device *dev; /* HCD GCD's parent device */ + + struct otg_fsm fsm; + /* HCD, GCD and usb_otg_state are present in otg_fsm-otg + * HCD is bus_to_hcd(fsm-otg-host) + * GCD is fsm-otg-gadget + */ + struct otg_fsm_ops fsm_ops; /* private copy for override */ + struct usb_otg otg; /* allocator for fsm-otg */ + + struct otg_hcd primary_hcd; + struct otg_hcd shared_hcd; + + struct otg_gadget_ops *gadget_ops; /* interface to gadget f/w
Re: [PATCH v3 05/11] usb: hcd.h: Add OTG to HCD interface
On Wed, Jul 08, 2015 at 01:19:31PM +0300, Roger Quadros wrote: The OTG core will use struct otg_hcd_ops to add/remove the HCD controller. The main purpose of this interface is to avoid directly calling usb_add/remove_hcd() from the OTG core as they wouldn't be defined in the built-in symbol table if CONFIG_USB is m. Like patch 4, Would you let the OTG depends on HCD? OTG needs dual-role, it is reasonable. Peter Signed-off-by: Roger Quadros rog...@ti.com --- include/linux/usb/hcd.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index c9aa779..4108288 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -386,6 +386,20 @@ struct hc_driver { }; +/** + * struct otg_hcd_ops - Interface between OTG core and HCD + * + * Provided by the HCD core to allow the OTG core to start/stop the HCD + * + * @add: function to add the HCD + * @remove: function to remove the HCD + */ +struct otg_hcd_ops { + int (*add)(struct usb_hcd *hcd, +unsigned int irqnum, unsigned long irqflags); + void (*remove)(struct usb_hcd *hcd); +}; + static inline int hcd_giveback_urb_in_bh(struct usb_hcd *hcd) { return hcd-driver-flags HCD_BH; -- 2.1.4 -- 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: [RFC][PATCH v2 00/13] USB: OTG/DRD Core functionality
On Wed, Apr 22, 2015 at 03:42:32PM +0300, Roger Quadros wrote: So we will have a separate drd fsm file, and the CONFIG_USB_OTG and CONFIG_USB_OTG_FSM are not needed to be defined, right? for drd case CONFIG_USB_OTG_FSM is definitely not needed. I'm not sure if we can operate dual-role without CONFIG_USB_OTG. I was thinking of combining the OTG core functionality (drivers/usb/common/usb-otg.c) with CONFIG_USB_OTG. Ok, let's choose CONFIG_USB_OTG for both drd and usb fsm case. And we need to patch for hcd that only hnp supported hcd needs to request otg descriptor, etc. For yesterday's back-compatible old otg device, we can add otg features to these drivers, the current behavior for these drivers is: if CONFIG_USB_OTG is defined, it is an otg device, we can just keep the behavior unchanging. If these drivers need to use OTG framework in future, it needs to update its platform data or dts. So, I prefer: - For switching the role through the ID pin devices, we doesn't need any otg features, so no otg dts properties are needed.(expect dr_mode = otg) - For adp/srp/hnp supported devices, we need (partial) otg features, and the fsm (hardware or software) are needed, we need some otg dts properties we discussed before. static const struct usb_descriptor_header *otg_desc_20[] = { (struct usb_descriptor_header *) (struct usb_otg_descriptor_20){ .bLength = sizeof(struct usb_otg_descriptor_20), .bDescriptorType = USB_DT_OTG, /* * REVISIT SRP-only hardware is possible, although * it would not be called OTG ... */ .bmAttributes = USB_OTG_SRP | USB_OTG_HNP, .bcdOTG = cpu_to_le16(0x0200), }, NULL, }; instead of hardcoding bmAttributes field, it must be obtained from the appropriate data structure that was set by the controller driver by reading DT and OTG hardware info. During bind process: if (gadget_is_otg_13(c-cdev-gadget)) c-descriptors = otg_desc_13; else if (gadget_is_otg_20(c-cdev-gadget)) c-descriptors = otg_desc_20; Probably a helper utitily can do the necessary checks and build the otg descriptor for us. usb_otg_get_descriptor(c-descriptors); e.g. for OTG3.0 we have a new flag USB_OTG_RSP, and this helper can be upgraded in the future. ok, it is the implementation detail. -- 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: [RFC][PATCH v2 00/13] USB: OTG/DRD Core functionality
On Wed, Apr 22, 2015 at 10:33:24AM +0300, Roger Quadros wrote: On 22/04/15 05:17, Peter Chen wrote: On Tue, Apr 21, 2015 at 10:34:01AM +0300, Roger Quadros wrote: On 21/04/15 09:04, Peter Chen wrote: On 20/04/15 06:05, Peter Chen wrote: On Tue, Apr 14, 2015 at 01:41:47PM +0300, Roger Quadros wrote: This is an attempt to centralize OTG/Dual-role functionality in the kernel. As of now I've got Dual-role functionality working pretty reliably on dra7-evm. xhci side of things for OTG/DRD use are fixed in http://thread.gmane.org/gmane.linux.kernel/1923161 Hi Roger, Currently, there are two main problems for DRD/OTG framework. - For multi-platform supports, we may define CONFIG_USB_OTG, but the gadget should not add its otg descriptor to its configuration descriptors if it does not support one of otg features (SRP/HNP/ADP). Macpaul Lin's patch set [1] is the right way to do it. Agreed. That check (whether OTG descriptor can be added and which version of it) has to be done at runtime and it must be added only if hardware supports OTG _and_ kernel OTG support is enabled. Ok, let's put this patch set in mainline first, since your patch set may need some information from it. - We are lack of framework to handle OTG (DRD) switch, it is great you are designing it. The main problem for this framework is how to handle DRD/OTG FSM unify. My thought is we add two paths for them separate. For easy, I suggest if the platform supports one of otg features, then it goes to fully otg fsm, else it goes to simply otg fsm (like your drd fsm). If you agree with it too, you may not need to add another dr_mode value. It would be nice that way but unfortunately it does't work in all cases. e.g. What if the SoC itself supports all OTG features but the board is not designed for OTG. Or the product designer simply is not interested in full OTG support but just dual-role. So we need some flexibility for the device tree/platform-data to specify that. This is where a new dr_mode == dual- role is needed. Since dr_mode has been widely used now, if we add a new property for it, we need to change all drivers. Your OTG/DRD framework needs to (partial) use otg fsm, and we will not teach old driver to use it since there are some driver related stuffs. fair enough. Let's not change dr_mode then and decide based on other parameters. SRP/HNP/ADP support can be board level capabilities, and we may consider the otg device which does not support otg fsm (hardware finishes fsm). So I suggest we have below properties at dts: - otg-support /* fully otg support */ - otg-fsm-support /* fully otg fsm support */ what is the difference between otg-support and otg-fsm-support? Like I mentioned at above, the hardware finishes HNP/SRP which does not use otg fsm code (usb-otg-fsm.c), most of legacy otg platforms (musb?) use this way, for these platforms, only need to set otg-support = 1 So dr_mode = otg _and_ otg-support = 1? Yes Again wouldn't this involve changes to dts for musb like platforms supporting full OTG? Instead we could add a new field saying otg-fsm-type otg-hw, otg-sw, drd-sw If the field is absent it defaults to otg-hw. This also means we don't need otg-fsm-support flag. Yes, that's we doesn't need to change for old platforms, but we keep our default value as small possibilities, most of current platforms are only drd platform, it is hard choice. My original thought was nothing need to add at dts for drd device. Now the pseudo-code to decide fsm is if (dr_mode == otg CONFIG_USB_OTG) if (otg-fsm-type == otg-sw) { if (CONFIG_USB_OTG_FSM) full otg fsm support via sw else error CONFIG_USB_OTG_FSM not set } else if (otg-fsm-type == drd-sw) { dual role fsm support } else { full otg support via hw } if (otg-fsm-type == otg else error CONFIG_USB_OTG not set So we will have a separate drd fsm file, and the CONFIG_USB_OTG and CONFIG_USB_OTG_FSM are not needed to be defined, right? For platforms which software finishes HNP/SRP using otg fsm code, need to set both flags. For platforms which only do role switch through id pin, do not need to set both. OK. I get it now. - otg-ver /* eh otg supplement version */ we can get otg version from the OTG controller. What exactly is the otg-ver in dts for? Since most of otg stuffs are software's, eg, for otg descriptor, we will only use otg 2.0 descriptor when both CONFIG_USB_OTG20 and otg-ver = 20 are set. CONFIG_USB_OTG20 is redundant now. Plus I mentioned in the respective thread that it is not suitable for booting single image on different platforms. Ok, agree. Then we need to define two usb otg descriptors for both 1.3
RE: [RFC][PATCH v2 00/13] USB: OTG/DRD Core functionality
On 20/04/15 06:05, Peter Chen wrote: On Tue, Apr 14, 2015 at 01:41:47PM +0300, Roger Quadros wrote: This is an attempt to centralize OTG/Dual-role functionality in the kernel. As of now I've got Dual-role functionality working pretty reliably on dra7-evm. xhci side of things for OTG/DRD use are fixed in http://thread.gmane.org/gmane.linux.kernel/1923161 Hi Roger, Currently, there are two main problems for DRD/OTG framework. - For multi-platform supports, we may define CONFIG_USB_OTG, but the gadget should not add its otg descriptor to its configuration descriptors if it does not support one of otg features (SRP/HNP/ADP). Macpaul Lin's patch set [1] is the right way to do it. Agreed. That check (whether OTG descriptor can be added and which version of it) has to be done at runtime and it must be added only if hardware supports OTG _and_ kernel OTG support is enabled. Ok, let's put this patch set in mainline first, since your patch set may need some information from it. - We are lack of framework to handle OTG (DRD) switch, it is great you are designing it. The main problem for this framework is how to handle DRD/OTG FSM unify. My thought is we add two paths for them separate. For easy, I suggest if the platform supports one of otg features, then it goes to fully otg fsm, else it goes to simply otg fsm (like your drd fsm). If you agree with it too, you may not need to add another dr_mode value. It would be nice that way but unfortunately it does't work in all cases. e.g. What if the SoC itself supports all OTG features but the board is not designed for OTG. Or the product designer simply is not interested in full OTG support but just dual-role. So we need some flexibility for the device tree/platform-data to specify that. This is where a new dr_mode == dual- role is needed. Since dr_mode has been widely used now, if we add a new property for it, we need to change all drivers. Your OTG/DRD framework needs to (partial) use otg fsm, and we will not teach old driver to use it since there are some driver related stuffs. SRP/HNP/ADP support can be board level capabilities, and we may consider the otg device which does not support otg fsm (hardware finishes fsm). So I suggest we have below properties at dts: - otg-support /* fully otg support */ - otg-fsm-support /* fully otg fsm support */ - otg-ver /* eh otg supplement version */ - adp-support /* board adp support */ - srp-support /* board srp support */ - hnp-support /* board hnp support */ Currently, if CONFIG_USB_OTG and CONFIG_USB_OTG_FSM are enabled, we will have otg fsm code (usb-otg-fsm.c). if (otg-support otg-fsm-support) this device has fully otg support, and will follow full otg fsm transitions. else this device is drd, and will follow simple otg fsm transtions. Peter -- 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: [RFC][PATCH v2 00/13] USB: OTG/DRD Core functionality
On Tue, Apr 21, 2015 at 10:34:01AM +0300, Roger Quadros wrote: On 21/04/15 09:04, Peter Chen wrote: On 20/04/15 06:05, Peter Chen wrote: On Tue, Apr 14, 2015 at 01:41:47PM +0300, Roger Quadros wrote: This is an attempt to centralize OTG/Dual-role functionality in the kernel. As of now I've got Dual-role functionality working pretty reliably on dra7-evm. xhci side of things for OTG/DRD use are fixed in http://thread.gmane.org/gmane.linux.kernel/1923161 Hi Roger, Currently, there are two main problems for DRD/OTG framework. - For multi-platform supports, we may define CONFIG_USB_OTG, but the gadget should not add its otg descriptor to its configuration descriptors if it does not support one of otg features (SRP/HNP/ADP). Macpaul Lin's patch set [1] is the right way to do it. Agreed. That check (whether OTG descriptor can be added and which version of it) has to be done at runtime and it must be added only if hardware supports OTG _and_ kernel OTG support is enabled. Ok, let's put this patch set in mainline first, since your patch set may need some information from it. - We are lack of framework to handle OTG (DRD) switch, it is great you are designing it. The main problem for this framework is how to handle DRD/OTG FSM unify. My thought is we add two paths for them separate. For easy, I suggest if the platform supports one of otg features, then it goes to fully otg fsm, else it goes to simply otg fsm (like your drd fsm). If you agree with it too, you may not need to add another dr_mode value. It would be nice that way but unfortunately it does't work in all cases. e.g. What if the SoC itself supports all OTG features but the board is not designed for OTG. Or the product designer simply is not interested in full OTG support but just dual-role. So we need some flexibility for the device tree/platform-data to specify that. This is where a new dr_mode == dual- role is needed. Since dr_mode has been widely used now, if we add a new property for it, we need to change all drivers. Your OTG/DRD framework needs to (partial) use otg fsm, and we will not teach old driver to use it since there are some driver related stuffs. fair enough. Let's not change dr_mode then and decide based on other parameters. SRP/HNP/ADP support can be board level capabilities, and we may consider the otg device which does not support otg fsm (hardware finishes fsm). So I suggest we have below properties at dts: - otg-support /* fully otg support */ - otg-fsm-support /* fully otg fsm support */ what is the difference between otg-support and otg-fsm-support? Like I mentioned at above, the hardware finishes HNP/SRP which does not use otg fsm code (usb-otg-fsm.c), most of legacy otg platforms (musb?) use this way, for these platforms, only need to set otg-support = 1 For platforms which software finishes HNP/SRP using otg fsm code, need to set both flags. For platforms which only do role switch through id pin, do not need to set both. - otg-ver /* eh otg supplement version */ we can get otg version from the OTG controller. What exactly is the otg-ver in dts for? Since most of otg stuffs are software's, eg, for otg descriptor, we will only use otg 2.0 descriptor when both CONFIG_USB_OTG20 and otg-ver = 20 are set. - adp-support /* board adp support */ - srp-support /* board srp support */ - hnp-support /* board hnp support */ So if these options are not provided in DTS but the OTG core supports them then we keep the respective feature disabled? Yes. Won't this need dts change for existing boards? Does you know any dts based platform supports hnp/srp? For chipidea platform, currently, we depends on kernel configurations (CONFIG_USB_OTG_FSM), but it is incorrect way. Instead how about having disable flags instead. - adp-disable/* board doesn't support adp */ - srp-disable/* board doesn't support srp */ - hnp-disable/* board doesn't support hnp */ Now, if the flags are not provided in dts we use the OTG core's flags. How the OTG core's know if it supports these? Currently, if CONFIG_USB_OTG and CONFIG_USB_OTG_FSM are enabled, we will have otg fsm code (usb-otg-fsm.c). if (otg-support otg-fsm-support) this device has fully otg support, and will follow full otg fsm transitions. else this device is drd, and will follow simple otg fsm transtions. cheers, -roger -- 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: [RFC][PATCH v2 00/13] USB: OTG/DRD Core functionality
and the necessary OTG timers to manage OTG state timeouts. The state machine is started when both host gadget register and stopped when either of them unregisters. The controllers are started and stopped depending on bus state. - During the lifetime of the OTG state machine, inputs can be provided to it by modifying the appropriate members of 'struct otg_fsm' and calling usb_otg_sync_inputs(). This is typically done by the OTG controller driver that called usb_otg_register() since it is the only external component that has the 'struct otg_fsm' handle. Pending items: - We probably need a otg class. - sysfs interface for application OTG inputs and OTG status information - resolve symbol dependency for module use. -- cheers, -roger Roger Quadros (13): usb: otg-fsm: Add documentation for struct otg_fsm usb: otg-fsm: support multiple instances usb: otg-fsm: Prevent build warning VDBG redefined usb: gadget: add usb_gadget_start/stop() usb: otg: add OTG core usb: hcd: Add hcd add/remove functions for OTG use usb: otg: Add dual-role device (DRD) support usb: otg: hub: Notify OTG fsm when A device sets b_hnp_enable usb: gadget: udc: adapt to OTG udc-core: fix lock circular dependency on udc_lock usb: add dual-role mode to dr_mode device tree helper usb: dwc3: add dual-role support ARM: dts: dra7x-evm: Enable dual-role for usb1 Documentation/devicetree/bindings/usb/generic.txt | 2 +- arch/arm/boot/dts/dra7-evm.dts| 2 +- arch/arm/boot/dts/dra72-evm.dts | 2 +- drivers/usb/Makefile | 1 + drivers/usb/common/Makefile | 1 + drivers/usb/common/common.c | 1 + drivers/usb/common/usb-otg-fsm.c | 19 +- drivers/usb/common/usb-otg.c | 902 ++ drivers/usb/common/usb-otg.h | 71 ++ drivers/usb/core/Kconfig | 8 + drivers/usb/core/hcd.c| 24 +- drivers/usb/core/hub.c| 11 +- drivers/usb/dwc3/core.c | 146 +++- drivers/usb/dwc3/core.h | 6 + drivers/usb/dwc3/platform_data.h | 1 + drivers/usb/gadget/udc/udc-core.c | 119 ++- include/linux/usb/gadget.h| 3 + include/linux/usb/hcd.h | 3 + include/linux/usb/otg-fsm.h | 104 ++- include/linux/usb/otg.h | 1 + include/linux/usb/usb-otg.h | 95 +++ 21 files changed, 1476 insertions(+), 46 deletions(-) create mode 100644 drivers/usb/common/usb-otg.c create mode 100644 drivers/usb/common/usb-otg.h create mode 100644 include/linux/usb/usb-otg.h -- 2.1.0 -- 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: [RFC][PATCH v2 05/13] usb: otg: add OTG core
of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __LINUX_USB_OTG_CORE_H +#define __LINUX_USB_OTG_CORE_H + +#include linux/device.h +#include linux/usb.h +#include linux/usb/hcd.h +#include linux/usb/gadget.h +#include linux/usb/otg-fsm.h + +#if IS_ENABLED(CONFIG_USB_OTG_CORE) +struct otg_fsm *usb_otg_register(struct device *parent_dev, + struct otg_fsm_ops *fsm_ops); +int usb_otg_unregister(struct device *parent_dev); +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum, + unsigned long irqflags); +int usb_otg_unregister_hcd(struct usb_hcd *hcd); +int usb_otg_register_gadget(struct usb_gadget *gadget); +int usb_otg_unregister_gadget(struct usb_gadget *gadget); +void usb_otg_sync_inputs(struct otg_fsm *fsm); +int usb_otg_kick_fsm(struct device *hcd_gcd_device); +bool usb_otg_gadget_can_start(struct usb_gadget *gadget); +struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm); + +#else /* CONFIG_USB_OTG_CORE */ + +static inline struct otg_fsm *usb_otg_register(struct device *parent_dev, +struct otg_fsm_ops *fsm_ops) +{ + return ERR_PTR(-ENOSYS); +} + +static inline int usb_otg_unregister(struct device *parent_dev) +{ + return -ENOSYS; +} + +static inline int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum, +unsigned long irqflags) +{ + return -ENOSYS; +} + +int usb_otg_unregister_hcd(struct usb_hcd *hcd) +{ + return -ENOSYS; +} + +static inline int usb_otg_register_gadget(struct usb_gadget *gadget) +{ + return -ENOSYS; +} + +static inline int usb_otg_unregister_gadget(struct usb_gadget *gadget) +{ + return -ENOSYS; +} + +static inline void usb_otg_sync_inputs(struct otg_fsm *fsm) +{ +} + +int usb_otg_kick_fsm(struct device *hcd_gcd_device) +{ + return -ENOSYS; +} + +bool usb_otg_gadget_can_start(struct usb_gadget *gadget) +{ + return true; +} + +static inline struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm) +{ + return NULL; +} +#endif /* CONFIG_USB_OTG_CORE */ + +#endif /* __LINUX_USB_OTG_CORE */ -- 2.1.0 -- 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: [RFC][PATCH v2 04/13] usb: gadget: add usb_gadget_start/stop()
On Tue, Apr 14, 2015 at 01:41:51PM +0300, Roger Quadros wrote: The OTG state machine needs a mechanism to start and stop the gadget controller. Add usb_gadget_start() and usb_gadget_stop(). Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/gadget/udc/udc-core.c | 74 +++ include/linux/usb/gadget.h| 3 ++ 2 files changed, 77 insertions(+) diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 5a81cb0..3aa5dd5 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -187,6 +187,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_udc_reset); */ static inline int usb_gadget_udc_start(struct usb_udc *udc) { + dev_dbg(udc-dev, %s\n, __func__); return udc-gadget-ops-udc_start(udc-gadget, udc-driver); } @@ -204,10 +205,83 @@ static inline int usb_gadget_udc_start(struct usb_udc *udc) */ static inline void usb_gadget_udc_stop(struct usb_udc *udc) { + dev_dbg(udc-dev, %s\n, __func__); udc-gadget-ops-udc_stop(udc-gadget); } /** + * usb_gadget_start - start the usb gadget controller and connect to bus + * @gadget: the gadget device to start + * + * This is external API for use by OTG core. + * + * Start the usb device controller and connect to bus (enable pull). + */ +int usb_gadget_start(struct usb_gadget *gadget) +{ + int ret; + struct usb_udc *udc = NULL; + + dev_dbg(gadget-dev, %s\n, __func__); + mutex_lock(udc_lock); + list_for_each_entry(udc, udc_list, list) + if (udc-gadget == gadget) + goto found; + + dev_err(gadget-dev.parent, %s: gadget not registered.\n, + __func__); + mutex_unlock(udc_lock); + return -EINVAL; + +found: + ret = usb_gadget_udc_start(udc); + if (ret) + dev_err(udc-dev, USB Device Controller didn't start: %d\n, + ret); + else + usb_gadget_connect(udc-gadget); OTG FSM manages its pullup/pulldown itself through its fsm-ops-loc_conn + + mutex_unlock(udc_lock); + + return ret; +} +EXPORT_SYMBOL_GPL(usb_gadget_start); + +/** + * usb_gadget_stop - disconnect from bus and stop the usb gadget + * @gadget: The gadget device we want to stop + * + * This is external API for use by OTG core. + * + * Disconnect from the bus (disable pull) and stop the + * gadget controller. + */ +int usb_gadget_stop(struct usb_gadget *gadget) +{ + struct usb_udc *udc = NULL; + + dev_dbg(gadget-dev, %s\n, __func__); + mutex_lock(udc_lock); + list_for_each_entry(udc, udc_list, list) + if (udc-gadget == gadget) + goto found; + + dev_err(gadget-dev.parent, %s: gadget not registered.\n, + __func__); + mutex_unlock(udc_lock); + return -EINVAL; + +found: + usb_gadget_disconnect(udc-gadget); + udc-driver-disconnect(udc-gadget); + usb_gadget_udc_stop(udc); + mutex_unlock(udc_lock); + + return 0; +} +EXPORT_SYMBOL_GPL(usb_gadget_stop); + +/** * usb_udc_release - release the usb_udc struct * @dev: the dev member within usb_udc * diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index e2f00fd..7ada7e6 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -922,6 +922,9 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver); */ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver); +int usb_gadget_start(struct usb_gadget *gadget); +int usb_gadget_stop(struct usb_gadget *gadget); + extern int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget, void (*release)(struct device *dev)); extern int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget); -- 2.1.0 -- 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: [RFC][PATCH v2 02/13] usb: otg-fsm: support multiple instances
On Tue, Apr 14, 2015 at 01:41:49PM +0300, Roger Quadros wrote: Move the state_changed variable into struct otg_fsm so that we can support multiple instances. OTG device has only one port. See 3.1.1. If you go to pass OTG Certification, the lab requires that there is only one port at your board. Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/common/usb-otg-fsm.c | 10 -- include/linux/usb/otg-fsm.h | 1 + 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index 1736bbe..0caa37b 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -61,8 +61,6 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol) return 0; } -static int state_changed; - /* Called when leaving a state. Do state clean up jobs here */ static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state old_state) { @@ -123,7 +121,7 @@ static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state old_state) /* Called when entering a state */ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state) { - state_changed = 1; + fsm-state_changed = 1; if (fsm-otg-state == new_state) return 0; VDBG(Set state: %s\n, usb_otg_state_string(new_state)); @@ -255,7 +253,7 @@ int otg_statemachine(struct otg_fsm *fsm) mutex_lock(fsm-lock); state = fsm-otg-state; - state_changed = 0; + fsm-state_changed = 0; /* State machine state change judgement */ switch (state) { @@ -368,7 +366,7 @@ int otg_statemachine(struct otg_fsm *fsm) } mutex_unlock(fsm-lock); - VDBG(quit statemachine, changed = %d\n, state_changed); - return state_changed; + VDBG(quit statemachine, changed = %d\n, fsm-state_changed); + return fsm-state_changed; } EXPORT_SYMBOL_GPL(otg_statemachine); diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index c5b74c5..85a9150 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -183,6 +183,7 @@ struct otg_fsm { /* Current usb protocol used: 0:undefine; 1:host; 2:client */ int protocol; struct mutex lock; + bool state_changed; }; struct otg_fsm_ops { -- 2.1.0 -- 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: [RFC][PATCH v2 04/13] usb: gadget: add usb_gadget_start/stop()
On Thu, Apr 16, 2015 at 03:07:41PM +0300, Roger Quadros wrote: On 16/04/15 14:48, Peter Chen wrote: On Tue, Apr 14, 2015 at 01:41:51PM +0300, Roger Quadros wrote: The OTG state machine needs a mechanism to start and stop the gadget controller. Add usb_gadget_start() and usb_gadget_stop(). Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/gadget/udc/udc-core.c | 74 +++ include/linux/usb/gadget.h| 3 ++ 2 files changed, 77 insertions(+) diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 5a81cb0..3aa5dd5 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -187,6 +187,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_udc_reset); */ static inline int usb_gadget_udc_start(struct usb_udc *udc) { + dev_dbg(udc-dev, %s\n, __func__); return udc-gadget-ops-udc_start(udc-gadget, udc-driver); } @@ -204,10 +205,83 @@ static inline int usb_gadget_udc_start(struct usb_udc *udc) */ static inline void usb_gadget_udc_stop(struct usb_udc *udc) { + dev_dbg(udc-dev, %s\n, __func__); udc-gadget-ops-udc_stop(udc-gadget); } /** + * usb_gadget_start - start the usb gadget controller and connect to bus + * @gadget: the gadget device to start + * + * This is external API for use by OTG core. + * + * Start the usb device controller and connect to bus (enable pull). + */ +int usb_gadget_start(struct usb_gadget *gadget) +{ + int ret; + struct usb_udc *udc = NULL; + + dev_dbg(gadget-dev, %s\n, __func__); + mutex_lock(udc_lock); + list_for_each_entry(udc, udc_list, list) + if (udc-gadget == gadget) + goto found; + + dev_err(gadget-dev.parent, %s: gadget not registered.\n, + __func__); + mutex_unlock(udc_lock); + return -EINVAL; + +found: + ret = usb_gadget_udc_start(udc); + if (ret) + dev_err(udc-dev, USB Device Controller didn't start: %d\n, + ret); + else + usb_gadget_connect(udc-gadget); OTG FSM manages its pullup/pulldown itself through its fsm-ops-loc_conn That API usb_gadget_udc_start() is used by DRD (dual-role) FSM as well and it doesn't use ops-loc_conn. So i'm wondering how to make this work for both. I could probably remove the pull up control from usb_gadget_start/stop() and move it into the DRD FSM. Just like I comment your at your 5th patch, if we want both DRD and OTG FSM devices run the same code. From our experience, it can cover both at runtime if we can handle otg descriptor well (no otg descriptor for DRD device). Peter + + mutex_unlock(udc_lock); + + return ret; +} +EXPORT_SYMBOL_GPL(usb_gadget_start); + +/** + * usb_gadget_stop - disconnect from bus and stop the usb gadget + * @gadget: The gadget device we want to stop + * + * This is external API for use by OTG core. + * + * Disconnect from the bus (disable pull) and stop the + * gadget controller. + */ +int usb_gadget_stop(struct usb_gadget *gadget) +{ + struct usb_udc *udc = NULL; + + dev_dbg(gadget-dev, %s\n, __func__); + mutex_lock(udc_lock); + list_for_each_entry(udc, udc_list, list) + if (udc-gadget == gadget) + goto found; + + dev_err(gadget-dev.parent, %s: gadget not registered.\n, + __func__); + mutex_unlock(udc_lock); + return -EINVAL; + +found: + usb_gadget_disconnect(udc-gadget); + udc-driver-disconnect(udc-gadget); + usb_gadget_udc_stop(udc); + mutex_unlock(udc_lock); + + return 0; +} +EXPORT_SYMBOL_GPL(usb_gadget_stop); + +/** * usb_udc_release - release the usb_udc struct * @dev: the dev member within usb_udc * diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index e2f00fd..7ada7e6 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -922,6 +922,9 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver); */ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver); +int usb_gadget_start(struct usb_gadget *gadget); +int usb_gadget_stop(struct usb_gadget *gadget); + extern int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget, void (*release)(struct device *dev)); extern int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget); -- 2.1.0 -- 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: [RFC][PATCH v2 03/13] usb: otg-fsm: Prevent build warning VDBG redefined
On Tue, Apr 14, 2015 at 01:41:50PM +0300, Roger Quadros wrote: If usb/otg-fsm.h and usb/composite.h are included together then it results in the build warning [1]. Prevent that by moving the VDBG defination into the usb-otg-fsm.c file where it is used. Also get rid of MPC_LOC which doesn't seem to be used by anyone. [1] - warning fixed by this patch: In file included from drivers/usb/dwc3/core.h:33, from drivers/usb/dwc3/ep0.c:33: include/linux/usb/otg-fsm.h:30:1: warning: VDBG redefined In file included from drivers/usb/dwc3/ep0.c:31: include/linux/usb/composite.h:615:1: warning: this is the location of the previous definition Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/common/usb-otg-fsm.c | 9 + include/linux/usb/otg-fsm.h | 15 --- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index 0caa37b..35f311a 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -30,6 +30,15 @@ #include linux/usb/otg.h #include linux/usb/otg-fsm.h +#undef VDBG + +#ifdef VERBOSE +#define VDBG(fmt, args...) pr_debug([%s] fmt , \ + __func__, ## args) +#else +#define VDBG(stuff...) do {} while (0) +#endif + It is obsolete too, we may use dev_vdbg instead of it. /* Change USB protocol when there is a protocol change */ static int otg_set_protocol(struct otg_fsm *fsm, int protocol) { diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index 85a9150..73136aa 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -21,21 +21,6 @@ #include linux/mutex.h #include linux/errno.h -#undef VERBOSE - -#ifdef VERBOSE -#define VDBG(fmt, args...) pr_debug([%s] fmt , \ - __func__, ## args) -#else -#define VDBG(stuff...) do {} while (0) -#endif - -#ifdef VERBOSE -#define MPC_LOC printk(Current Location [%s]:[%d]\n, __FILE__, __LINE__) -#else -#define MPC_LOC do {} while (0) -#endif - #define PROTO_UNDEF (0) #define PROTO_HOST (1) #define PROTO_GADGET (2) -- 2.1.0 -- 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: [RFC][PATCH v2 01/13] usb: otg-fsm: Add documentation for struct otg_fsm
*/ int a_sess_vld; int b_bus_resume; int b_bus_suspend; /* Output */ - int data_pulse; int drv_vbus; int loc_conn; int loc_sof; int adp_prb; int adp_sns; + int data_pulse; /* Internal variables */ int a_set_b_hnp_en; @@ -95,7 +161,7 @@ struct otg_fsm { int b_hnp_enable; int a_clr_err; - /* Informative variables */ + /* Informative variables. All unused as of now */ int a_bus_drop_inf; int a_bus_req_inf; int a_clr_err_inf; -- 2.1.0 -- 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: [RFC][PATCH v2 02/13] usb: otg-fsm: support multiple instances
On Thu, Apr 16, 2015 at 02:58:20PM +0300, Roger Quadros wrote: On 16/04/15 14:36, Peter Chen wrote: On Tue, Apr 14, 2015 at 01:41:49PM +0300, Roger Quadros wrote: Move the state_changed variable into struct otg_fsm so that we can support multiple instances. OTG device has only one port. See 3.1.1. If you go to pass OTG Certification, the lab requires that there is only one port at your board. We're not breaking OTG compliance by this ;). Moreover this structure is not limited to OTG but for DRD (dual-role) use as well. We can have multiple DRD ports per board. Ok, the code is ok for keeping multiple instances. Peter Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/common/usb-otg-fsm.c | 10 -- include/linux/usb/otg-fsm.h | 1 + 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index 1736bbe..0caa37b 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -61,8 +61,6 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol) return 0; } -static int state_changed; - /* Called when leaving a state. Do state clean up jobs here */ static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state old_state) { @@ -123,7 +121,7 @@ static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state old_state) /* Called when entering a state */ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state) { - state_changed = 1; + fsm-state_changed = 1; if (fsm-otg-state == new_state) return 0; VDBG(Set state: %s\n, usb_otg_state_string(new_state)); @@ -255,7 +253,7 @@ int otg_statemachine(struct otg_fsm *fsm) mutex_lock(fsm-lock); state = fsm-otg-state; - state_changed = 0; + fsm-state_changed = 0; /* State machine state change judgement */ switch (state) { @@ -368,7 +366,7 @@ int otg_statemachine(struct otg_fsm *fsm) } mutex_unlock(fsm-lock); - VDBG(quit statemachine, changed = %d\n, state_changed); - return state_changed; + VDBG(quit statemachine, changed = %d\n, fsm-state_changed); + return fsm-state_changed; } EXPORT_SYMBOL_GPL(otg_statemachine); diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index c5b74c5..85a9150 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -183,6 +183,7 @@ struct otg_fsm { /* Current usb protocol used: 0:undefine; 1:host; 2:client */ int protocol; struct mutex lock; + bool state_changed; }; struct otg_fsm_ops { -- 2.1.0 -- 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: [RFC][PATCH v2 03/13] usb: otg-fsm: Prevent build warning VDBG redefined
On Thu, Apr 16, 2015 at 02:59:12PM +0300, Roger Quadros wrote: On 16/04/15 14:41, Peter Chen wrote: On Tue, Apr 14, 2015 at 01:41:50PM +0300, Roger Quadros wrote: If usb/otg-fsm.h and usb/composite.h are included together then it results in the build warning [1]. Prevent that by moving the VDBG defination into the usb-otg-fsm.c file where it is used. Also get rid of MPC_LOC which doesn't seem to be used by anyone. [1] - warning fixed by this patch: In file included from drivers/usb/dwc3/core.h:33, from drivers/usb/dwc3/ep0.c:33: include/linux/usb/otg-fsm.h:30:1: warning: VDBG redefined In file included from drivers/usb/dwc3/ep0.c:31: include/linux/usb/composite.h:615:1: warning: this is the location of the previous definition Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/common/usb-otg-fsm.c | 9 + include/linux/usb/otg-fsm.h | 15 --- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index 0caa37b..35f311a 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -30,6 +30,15 @@ #include linux/usb/otg.h #include linux/usb/otg-fsm.h +#undef VDBG + +#ifdef VERBOSE +#define VDBG(fmt, args...) pr_debug([%s] fmt , \ + __func__, ## args) +#else +#define VDBG(stuff...)do {} while (0) +#endif + It is obsolete too, we may use dev_vdbg instead of it. ok, we need to add 'struct device *' to 'struct otg_fsm' then. I am not sure if we will keep struct device * for otg device at last, if it is, you can use otg's. Peter cheers, -roger /* Change USB protocol when there is a protocol change */ static int otg_set_protocol(struct otg_fsm *fsm, int protocol) { diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index 85a9150..73136aa 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -21,21 +21,6 @@ #include linux/mutex.h #include linux/errno.h -#undef VERBOSE - -#ifdef VERBOSE -#define VDBG(fmt, args...) pr_debug([%s] fmt , \ - __func__, ## args) -#else -#define VDBG(stuff...)do {} while (0) -#endif - -#ifdef VERBOSE -#define MPC_LOC printk(Current Location [%s]:[%d]\n, __FILE__, __LINE__) -#else -#define MPC_LOC do {} while (0) -#endif - #define PROTO_UNDEF (0) #define PROTO_HOST(1) #define PROTO_GADGET (2) -- 2.1.0 -- 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: [RFC][PATCH v2 08/13] usb: otg: hub: Notify OTG fsm when A device sets b_hnp_enable
On Tue, Apr 14, 2015 at 01:41:55PM +0300, Roger Quadros wrote: This is the a_set_b_hnp_enable flag in the OTG state machine diagram and must be set when the A-Host has successfully set the b_hnp_enable feature of the OTG-B-Peripheral attached to it. When this bit changes we kick our OTG FSM to make note of the change and act accordingly. Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/core/hub.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index d7c3d5a..ab0d498 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -22,6 +22,7 @@ #include linux/usb/hcd.h #include linux/usb/otg.h #include linux/usb/quirks.h +#include linux/usb/usb-otg.h #include linux/workqueue.h #include linux/mutex.h #include linux/random.h @@ -2270,6 +2271,9 @@ static int usb_enumerate_device_otg(struct usb_device *udev) can't set HNP mode: %d\n, err); bus-b_hnp_enable = 0; + } else { + /* notify OTG fsm about a_set_b_hnp_enable */ + usb_otg_kick_fsm(udev-bus-controller); } } } @@ -4244,8 +4248,13 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1, */ if (!hdev-parent) { delay = HUB_ROOT_RESET_TIME; - if (port1 == hdev-bus-otg_port) + if (port1 == hdev-bus-otg_port) { hdev-bus-b_hnp_enable = 0; +#ifdef CONFIG_USB_OTG + /* notify OTG fsm about a_set_b_hnp_enable change */ + usb_otg_kick_fsm(hdev-bus-controller); +#endif + } } /* Some low speed devices have problems with the quick delay, so */ -- 2.1.0 Since the fsm has already kicked, the only thing we need is update fsm.a_set_b_hnp_enable, but this flag is missing at current fsm structure. -- 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: [RFC][PATCH v2 06/13] usb: hcd: Add hcd add/remove functions for OTG use
On Tue, Apr 14, 2015 at 01:41:53PM +0300, Roger Quadros wrote: The existing usb_add/remove_hcd() functionality remains unchanged for non-OTG devices. For OTG devices they only register the HCD with the OTG core. Introduce _usb_add/remove_hcd() for use by OTG core. These functions actually add/remove the HCD. Would you please explain why additional _usb_add/remove_hcd are needed? Peter Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/common/usb-otg.c | 14 +++--- drivers/usb/core/hcd.c | 24 ++-- include/linux/usb/hcd.h | 3 +++ 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c index e848e08..860e2e7 100644 --- a/drivers/usb/common/usb-otg.c +++ b/drivers/usb/common/usb-otg.c @@ -198,18 +198,18 @@ static int usb_otg_start_host(struct otg_fsm *fsm, int on) otgd-start_host(fsm, on); /* start host */ - usb_add_hcd(otgd-primary_hcd.hcd, otgd-primary_hcd.irqnum, - otgd-primary_hcd.irqflags); + _usb_add_hcd(otgd-primary_hcd.hcd, otgd-primary_hcd.irqnum, + otgd-primary_hcd.irqflags); if (otgd-shared_hcd.hcd) { - usb_add_hcd(otgd-shared_hcd.hcd, - otgd-shared_hcd.irqnum, - otgd-shared_hcd.irqflags); + _usb_add_hcd(otgd-shared_hcd.hcd, + otgd-shared_hcd.irqnum, + otgd-shared_hcd.irqflags); } } else { /* stop host */ if (otgd-shared_hcd.hcd) - usb_remove_hcd(otgd-shared_hcd.hcd); - usb_remove_hcd(otgd-primary_hcd.hcd); + _usb_remove_hcd(otgd-shared_hcd.hcd); + _usb_remove_hcd(otgd-primary_hcd.hcd); /* OTG device operations */ if (otgd-start_host) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 45a915c..9a9c0f7 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -46,6 +46,7 @@ #include linux/usb.h #include linux/usb/hcd.h #include linux/usb/phy.h +#include linux/usb/usb-otg.h #include usb.h @@ -2622,7 +2623,7 @@ static void usb_put_invalidate_rhdev(struct usb_hcd *hcd) * buffers of consistent memory, register the bus, request the IRQ line, * and call the driver's reset() and start() routines. */ -int usb_add_hcd(struct usb_hcd *hcd, +int _usb_add_hcd(struct usb_hcd *hcd, unsigned int irqnum, unsigned long irqflags) { int retval; @@ -2827,6 +2828,17 @@ err_phy: } return retval; } +EXPORT_SYMBOL_GPL(_usb_add_hcd); + +int usb_add_hcd(struct usb_hcd *hcd, + unsigned int irqnum, unsigned long irqflags) +{ + /* If OTG device, OTG core takes care of adding HCD */ + if (usb_otg_register_hcd(hcd, irqnum, irqflags)) + return _usb_add_hcd(hcd, irqnum, irqflags); + + return 0; +} EXPORT_SYMBOL_GPL(usb_add_hcd); /** @@ -2837,7 +2849,7 @@ EXPORT_SYMBOL_GPL(usb_add_hcd); * Disconnects the root hub, then reverses the effects of usb_add_hcd(), * invoking the HCD's stop() method. */ -void usb_remove_hcd(struct usb_hcd *hcd) +void _usb_remove_hcd(struct usb_hcd *hcd) { struct usb_device *rhdev = hcd-self.root_hub; @@ -2911,6 +2923,14 @@ void usb_remove_hcd(struct usb_hcd *hcd) usb_put_invalidate_rhdev(hcd); } +EXPORT_SYMBOL_GPL(_usb_remove_hcd); + +void usb_remove_hcd(struct usb_hcd *hcd) +{ + /* If OTG device, OTG core takes care of stopping HCD */ + if (usb_otg_unregister_hcd(hcd)) + _usb_remove_hcd(hcd); +} EXPORT_SYMBOL_GPL(usb_remove_hcd); void diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 68b1e83..7993ae7 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -433,7 +433,10 @@ extern void usb_put_hcd(struct usb_hcd *hcd); extern int usb_hcd_is_primary_hcd(struct usb_hcd *hcd); extern int usb_add_hcd(struct usb_hcd *hcd, unsigned int irqnum, unsigned long irqflags); +extern int _usb_add_hcd(struct usb_hcd *hcd, + unsigned int irqnum, unsigned long irqflags); extern void usb_remove_hcd(struct usb_hcd *hcd); +extern void _usb_remove_hcd(struct usb_hcd *hcd); extern int usb_hcd_find_raw_port_number(struct usb_hcd *hcd, int port1); struct platform_device; -- 2.1.0 -- 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: [RFC][PATCH 3/9] usb: otg: add OTG core
On Thu, Mar 19, 2015 at 12:18:55PM +0200, Roger Quadros wrote: On 19/03/15 05:40, Peter Chen wrote: On Wed, Mar 18, 2015 at 03:55:57PM +0200, 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 TODO: - sysfs interface to allow application inputs to OTG state machine - otg class? Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/Makefile | 1 + drivers/usb/common/Makefile | 1 + drivers/usb/common/usb-otg.c | 732 +++ drivers/usb/common/usb-otg.h | 71 + drivers/usb/core/Kconfig | 8 + include/linux/usb/usb-otg.h | 86 + 6 files changed, 899 insertions(+) create mode 100644 drivers/usb/common/usb-otg.c create mode 100644 drivers/usb/common/usb-otg.h create mode 100644 include/linux/usb/usb-otg.h diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile index 2f1e2aa..07f59a5 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_CORE)+= common/ obj-$(CONFIG_USBIP_CORE) += usbip/ diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile index ca2f8bd..573fc75 100644 --- a/drivers/usb/common/Makefile +++ b/drivers/usb/common/Makefile @@ -7,3 +7,4 @@ 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_OTG_CORE) += usb-otg.o diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c new file mode 100644 index 000..1433fc9 --- /dev/null +++ b/drivers/usb/common/usb-otg.c @@ -0,0 +1,732 @@ +/** + * 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/list.h +#include linux/timer.h +#include linux/usb/otg.h +#include linux/usb/phy.h/* enum usb_otg_state */ +#include linux/usb/gadget.h +#include linux/usb/usb-otg.h +#include linux/workqueue.h + +#include usb-otg.h + +/* to link timer with callback data */ +struct otg_timer { + struct timer_list timer; + /* callback data */ + int *timeout_bit; + struct otg_data *otgd; +}; + +struct otg_data { + struct device *dev; /* HCD GCD's parent device */ + + struct otg_fsm fsm; + /* HCD, GCD and usb_otg_state are present in otg_fsm-otg + * HCD is bus_to_hcd(fsm-otg-host) + * GCD is fsm-otg-gadget + */ + struct otg_fsm_ops fsm_ops; /* private copy for override */ + struct usb_otg otg; + struct usb_hcd *shared_hcd; /* if shared HCD registered */ + + /* saved hooks to OTG device */ + int (*start_host)(struct otg_fsm *fsm, int on); + int (*start_gadget)(struct otg_fsm *fsm, int on); + + struct list_head list; + + struct work_struct work;/* OTG FSM work */ + struct workqueue_struct *wq; + + struct otg_timer timers[NUM_OTG_FSM_TIMERS]; + + bool fsm_running; + bool gadget_can_start; /* OTG FSM says gadget can start */ + bool host_can_start;/* OTG FSM says host can start */ + + /* use otg-fsm.lock for serializing access */ +}; What's the relationship between struct usb_otg otg and this one? Did you mean why struct usb_otg otg is there in struct otg_data? Just for easy allocation. fsm_ops only contains the pointer to struct usb_otg. The reason why I ask this question is the most structures at usb_otg (only enum usb_otg_state)are useless for otg_fsm, this structure may only for hardware otg fsm driver, so your OTG framework should only for software FSM drivers, right? Peter + +/* OTG device list */ +LIST_HEAD(otg_list); +static DEFINE_MUTEX(otg_list_mutex); + +/** + * check if device is in our OTG list and return + * otg_data, else NULL. + * + * otg_list_mutex must be held. + */ +static struct otg_data *usb_otg_device_get_otgd
Re: [RFC][PATCH 1/9] usb: hcd: Introduce usb_start/stop_hcd()
On Thu, Mar 19, 2015 at 01:38:32PM +0200, Roger Quadros wrote: On 18/03/15 21:49, Alan Stern wrote: On Wed, 18 Mar 2015, Roger Quadros wrote: To support OTG we want a mechanism to start and stop the HCD from the OTG state machine. Add usb_start_hcd() and usb_stop_hcd(). Signed-off-by: Roger Quadros rog...@ti.com There are a few problems in this proposed patch. +int usb_start_hcd(struct usb_hcd *hcd) +{ + int retval; + struct usb_device *rhdev = hcd-self.root_hub; + + if (hcd-state != HC_STATE_HALT) { + dev_err(hcd-self.controller, not starting a running HCD\n); + return -EINVAL; + } + + hcd-state = HC_STATE_RUNNING; + retval = hcd-driver-start(hcd); + if (retval 0) { + dev_err(hcd-self.controller, startup error %d\n, retval); + hcd-state = HC_STATE_HALT; + return retval; + } + + /* starting here, usbcore will pay attention to this root hub */ + if ((retval = register_root_hub(hcd)) != 0) + goto err_register_root_hub; If the host controller is started more than once, you will end up unregistering and re-registering the root hub. The device core does not allow this. Once a device has been unregistered, you must not try to register it again -- you have to allocate a new device and register it instead. Understood. Also, although you call the driver's -start method multiple times, the -reset method is called only once, when the controller is first probed. It's not clear that this will work in a situation where the HC and the UDC share hardware state; after the UDC is stopped it may be necessary to reset the HC before it can run again. Yes, good point. It might be possible to make this work, but I suspect quite a few drivers would need rewriting first. As another example of the problems you face, consider how stopping a host controller will interact with the driver's PM support (both system suspend and runtime suspend). Right. This needs more work than I thought. It would be a lot simpler to unbind the host controller driver completely when switching to device mode and rebind it when switching back. I guess that is the sort of heavy-duty approach you want to avoid, but it may be the only practical way forward. So you mean directly calling usb_add/remove_hcd() from the OTG core? I don't see any issues with that other than it being a heavy-duty operation like you said and hope that it doesn't violate the OTG spec timing. Looking at Figure 5-3: HNP Sequence of Events (FS) of the OTG 2.0 spec we have about 150ms (X10) to switch from B-Device detected A connect (b_wait_acon state) to driving bus reset (b_host state). I don't think this should be an issue in modern SoCs but I'm not very sure. It is not related toadd/remove hcd, it is the time from receiving PCD to issue BUS_RESET, the Linux stack can't satisfy OTG spec (150ms) due to there are some de-bounce waitings. In any case I can migrate to the add/remove hcd approach to simplify things. It should be no problem, we use it more than 1 years. -- 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: [RFC][PATCH 2/9] usb: gadget: add usb_gadget_start/stop()
On Thu, Mar 19, 2015 at 04:50:31PM +0200, Roger Quadros wrote: On 19/03/15 16:09, Li Jun wrote: On Thu, Mar 19, 2015 at 12:14:39PM +0200, Roger Quadros wrote: On 19/03/15 05:30, Peter Chen wrote: On Wed, Mar 18, 2015 at 03:55:56PM +0200, Roger Quadros wrote: The OTG state machine needs a mechanism to start and stop the gadget controller. Add usb_gadget_start() and usb_gadget_stop(). Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/gadget/udc/udc-core.c | 166 +++--- include/linux/usb/gadget.h| 3 + 2 files changed, 158 insertions(+), 11 deletions(-) diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 5a81cb0..69b4123 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -35,6 +35,8 @@ * @dev - the child device to the actual controller * @gadget - the gadget. For use by the class code * @list - for use by the udc class driver + * @running - udc is running Doesn't OTG FSM should know it? Not really, as the gadget driver might not have been loaded yet or userspace might have disabled softconnect when the OTG FSM wants UDC to start. So only UDC knows if it has really started or not based on this flag. why this can not be known by check the otg fsm state? i.e. if the device in b_peripheral or a_peripheral state, udc should had started, isn't it? If gadget function driver (which is different from UDC driver) is not yet loaded then we can't start UDC even if otg fsm is in b_peripheral. Also, if userspace has disabled softconnect we can't start UDC. So, b_peripheral != UDC_started. I've tried to address this issue by adding the checks in usb_gadget_start(). Ok, maybe we have different understanding for 'B-Device' at software, In spec, it says the Micro-AB receptacle with nothing connected can be 'B-Device', but in fact, we may not enable device mode before loading gadget driver, in chipidea fsm design, if the gadget driver is not loaded, the FSM will not start, and it is at OTG_STATE_UNDEFINED. One more thing is we may need to find a place to issue SRP when we load gadget driver, since we may at b_idle at that time due to host closes the vbus (timeout for a_wait_bcon). What is the softconnect used for? In otg fsm, we use b_bus_req for FSM. -- 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: [RFC][PATCH 3/9] usb: otg: add OTG core
- Registering an OTG capable controller - Registering Host and Gadget controllers to OTG core - Providing inputs to and kicking the OTG state machine TODO: - sysfs interface to allow application inputs to OTG state machine - otg class? Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/Makefile | 1 + drivers/usb/common/Makefile | 1 + drivers/usb/common/usb-otg.c | 732 +++ drivers/usb/common/usb-otg.h | 71 + drivers/usb/core/Kconfig | 8 + include/linux/usb/usb-otg.h | 86 + 6 files changed, 899 insertions(+) create mode 100644 drivers/usb/common/usb-otg.c create mode 100644 drivers/usb/common/usb-otg.h create mode 100644 include/linux/usb/usb-otg.h diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile index 2f1e2aa..07f59a5 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_CORE) += common/ obj-$(CONFIG_USBIP_CORE)+= usbip/ diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile index ca2f8bd..573fc75 100644 --- a/drivers/usb/common/Makefile +++ b/drivers/usb/common/Makefile @@ -7,3 +7,4 @@ 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_OTG_CORE) += usb-otg.o diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c new file mode 100644 index 000..1433fc9 --- /dev/null +++ b/drivers/usb/common/usb-otg.c @@ -0,0 +1,732 @@ +/** + * 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/list.h +#include linux/timer.h +#include linux/usb/otg.h +#include linux/usb/phy.h /* enum usb_otg_state */ +#include linux/usb/gadget.h +#include linux/usb/usb-otg.h +#include linux/workqueue.h + +#include usb-otg.h + +/* to link timer with callback data */ struct otg_timer { +struct timer_list timer; +/* callback data */ +int *timeout_bit; +struct otg_data *otgd; +}; + +struct otg_data { +struct device *dev; /* HCD GCD's parent device */ + +struct otg_fsm fsm; +/* HCD, GCD and usb_otg_state are present in otg_fsm-otg + * HCD is bus_to_hcd(fsm-otg-host) + * GCD is fsm-otg-gadget + */ +struct otg_fsm_ops fsm_ops; /* private copy for override */ +struct usb_otg otg; +struct usb_hcd *shared_hcd; /* if shared HCD registered */ + +/* saved hooks to OTG device */ +int (*start_host)(struct otg_fsm *fsm, int on); +int (*start_gadget)(struct otg_fsm *fsm, int on); + +struct list_head list; + +struct work_struct work;/* OTG FSM work */ +struct workqueue_struct *wq; + +struct otg_timer timers[NUM_OTG_FSM_TIMERS]; + +bool fsm_running; +bool gadget_can_start; /* OTG FSM says gadget can start */ +bool host_can_start;/* OTG FSM says host can start */ + +/* use otg-fsm.lock for serializing access */ }; What's the relationship between struct usb_otg otg and this one? Did you mean why struct usb_otg otg is there in struct otg_data? Just for easy allocation. fsm_ops only contains the pointer to struct usb_otg. The reason why I ask this question is the most structures at usb_otg (only enum usb_otg_state)are useless for otg_fsm, this structure may only for hardware otg fsm driver, so your OTG framework should only for software FSM drivers, right? right. we only need it for enum usb_otg_state. Do you see how we can improve it? Felipe Roger, if we need to go on supporting legacies otg driver, we need to keep struct usb_otg unchanged, and teach new otg driver using Roger's framework. Peter -- 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: [RFC][PATCH 2/9] usb: gadget: add usb_gadget_start/stop()
On Fri, Mar 20, 2015 at 01:08:25PM +0200, Roger Quadros wrote: On 20/03/15 11:46, Roger Quadros wrote: On 20/03/15 09:18, Peter Chen wrote: On Thu, Mar 19, 2015 at 04:50:31PM +0200, Roger Quadros wrote: On 19/03/15 16:09, Li Jun wrote: On Thu, Mar 19, 2015 at 12:14:39PM +0200, Roger Quadros wrote: On 19/03/15 05:30, Peter Chen wrote: On Wed, Mar 18, 2015 at 03:55:56PM +0200, Roger Quadros wrote: The OTG state machine needs a mechanism to start and stop the gadget controller. Add usb_gadget_start() and usb_gadget_stop(). Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/gadget/udc/udc-core.c | 166 +++--- include/linux/usb/gadget.h| 3 + 2 files changed, 158 insertions(+), 11 deletions(-) diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 5a81cb0..69b4123 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -35,6 +35,8 @@ * @dev - the child device to the actual controller * @gadget - the gadget. For use by the class code * @list - for use by the udc class driver + * @running - udc is running Doesn't OTG FSM should know it? Not really, as the gadget driver might not have been loaded yet or userspace might have disabled softconnect when the OTG FSM wants UDC to start. So only UDC knows if it has really started or not based on this flag. why this can not be known by check the otg fsm state? i.e. if the device in b_peripheral or a_peripheral state, udc should had started, isn't it? If gadget function driver (which is different from UDC driver) is not yet loaded then we can't start UDC even if otg fsm is in b_peripheral. Also, if userspace has disabled softconnect we can't start UDC. So, b_peripheral != UDC_started. I've tried to address this issue by adding the checks in usb_gadget_start(). Ok, maybe we have different understanding for 'B-Device' at software, In spec, it says the Micro-AB receptacle with nothing connected can be 'B-Device', but in fact, we may not enable device mode before loading gadget driver, in chipidea fsm design, if the gadget driver is not loaded, the FSM will not start, and it is at OTG_STATE_UNDEFINED. Right. I mixed up into thinking that we should respect the softconnect while in OTG mode. It seems that we should ignore it. One more thing is we may need to find a place to issue SRP when we load gadget driver, since we may at b_idle at that time due to host closes the vbus (timeout for a_wait_bcon). Issuing SRP should be done by the otg-fsm and not udc-core. The udc-core can at the least call usb_otg_kick_fsm() after setting the gadget driver so that otg-fsm knows that we now have a valid gadget and can take necessary action. i.e. change from b_idle to b_srp_init and then to b_peripheral. To clarify further. Is it right to assume that OTG FSM will not be started till both gadget UDC driver _AND_ gadget function driver are loaded? And it will be stopped when either of them unloads. This simplifies things a lot. Yes, you are right. cheers, -roger What is the softconnect used for? In otg fsm, we use b_bus_req for FSM. I understand now that we shouldn't bother with softconnect when we are in OTG fsm mode. That solves our problem with the running flags. So now, b_peripheral == UDC_started. I will address this in v2. cheers, -roger -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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: [RFC][PATCH 1/9] usb: hcd: Introduce usb_start/stop_hcd()
(hcd); - hcd-state = HC_STATE_HALT; - clear_bit(HCD_FLAG_POLL_RH, hcd-flags); - del_timer_sync(hcd-rh_timer); err_hcd_driver_start: if (usb_hcd_is_primary_hcd(hcd) hcd-irq 0) free_irq(irqnum, hcd); @@ -2830,18 +2861,20 @@ err_phy: EXPORT_SYMBOL_GPL(usb_add_hcd); /** - * usb_remove_hcd - shutdown processing for generic HCDs - * @hcd: the usb_hcd structure to remove - * Context: !in_interrupt() + * usb_stop_hcd - stop the HCD + * @hcd: the usb_hcd structure to initialize * - * Disconnects the root hub, then reverses the effects of usb_add_hcd(), - * invoking the HCD's stop() method. + * removes the USB sysfs attributes, disconnects the root hub + * and calls the driver's stop() routine. */ -void usb_remove_hcd(struct usb_hcd *hcd) +void usb_stop_hcd(struct usb_hcd *hcd) { struct usb_device *rhdev = hcd-self.root_hub; - dev_info(hcd-self.controller, remove, state %x\n, hcd-state); + if (hcd-state == HC_STATE_HALT) { + dev_err(hcd-self.controller, not stopping a halted HCD\n); + return; + } usb_get_dev(rhdev); sysfs_remove_group(rhdev-dev.kobj, usb_bus_attr_group); @@ -2888,6 +2921,22 @@ void usb_remove_hcd(struct usb_hcd *hcd) /* In case the HCD restarted the timer, stop it again. */ clear_bit(HCD_FLAG_POLL_RH, hcd-flags); del_timer_sync(hcd-rh_timer); +} +EXPORT_SYMBOL_GPL(usb_stop_hcd); + +/** + * usb_remove_hcd - shutdown processing for generic HCDs + * @hcd: the usb_hcd structure to remove + * Context: !in_interrupt() + * + * Disconnects the root hub, then reverses the effects of usb_add_hcd(), + * invoking the HCD's stop() method. + */ +void usb_remove_hcd(struct usb_hcd *hcd) +{ + dev_info(hcd-self.controller, remove, state %x\n, hcd-state); + + usb_stop_hcd(hcd); if (usb_hcd_is_primary_hcd(hcd)) { if (hcd-irq 0) diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 68b1e83..12aaf4c 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -433,6 +433,8 @@ extern void usb_put_hcd(struct usb_hcd *hcd); extern int usb_hcd_is_primary_hcd(struct usb_hcd *hcd); extern int usb_add_hcd(struct usb_hcd *hcd, unsigned int irqnum, unsigned long irqflags); +extern int usb_start_hcd(struct usb_hcd *hcd); +extern void usb_stop_hcd(struct usb_hcd *hcd); extern void usb_remove_hcd(struct usb_hcd *hcd); extern int usb_hcd_find_raw_port_number(struct usb_hcd *hcd, int port1); -- 2.1.0 -- 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: [RFC][PATCH 2/9] usb: gadget: add usb_gadget_start/stop()
; + + dev_dbg(gadget-dev, %s\n, __func__); + mutex_lock(udc_lock); + list_for_each_entry(udc, udc_list, list) + if (udc-gadget == gadget) + goto found; + + dev_err(gadget-dev.parent, %s: gadget not registered.\n, + __func__); + mutex_unlock(udc_lock); + return -EINVAL; + +found: + ret = usb_udc_stop(udc); + mutex_unlock(udc_lock); + + return 0; +} +EXPORT_SYMBOL_GPL(usb_gadget_stop); + +/** * usb_udc_release - release the usb_udc struct * @dev: the dev member within usb_udc * @@ -278,6 +419,7 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget, goto err3; udc-gadget = gadget; + udc-softconnect = 1; /* connect by default */ mutex_lock(udc_lock); list_add_tail(udc-list, udc_list); @@ -329,10 +471,8 @@ static void usb_gadget_remove_driver(struct usb_udc *udc) kobject_uevent(udc-dev.kobj, KOBJ_CHANGE); - usb_gadget_disconnect(udc-gadget); - udc-driver-disconnect(udc-gadget); + usb_udc_stop(udc); udc-driver-unbind(udc-gadget); - usb_gadget_udc_stop(udc); udc-driver = NULL; udc-dev.driver = NULL; @@ -378,6 +518,7 @@ EXPORT_SYMBOL_GPL(usb_del_gadget_udc); /* - */ +/* udc_lock must be held */ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *driver) { int ret; @@ -392,12 +533,12 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri ret = driver-bind(udc-gadget, driver); if (ret) goto err1; - ret = usb_gadget_udc_start(udc); + + ret = usb_udc_start(udc); if (ret) { driver-unbind(udc-gadget); goto err1; } - usb_gadget_connect(udc-gadget); kobject_uevent(udc-dev.kobj, KOBJ_CHANGE); return 0; @@ -510,12 +651,15 @@ static ssize_t usb_udc_softconn_store(struct device *dev, } if (sysfs_streq(buf, connect)) { - usb_gadget_udc_start(udc); - usb_gadget_connect(udc-gadget); + mutex_lock(udc_lock); + udc-softconnect = 1; + usb_udc_start(udc); + mutex_unlock(udc_lock); } else if (sysfs_streq(buf, disconnect)) { - usb_gadget_disconnect(udc-gadget); - udc-driver-disconnect(udc-gadget); - usb_gadget_udc_stop(udc); + mutex_lock(udc_lock); + udc-softconnect = 0; + usb_udc_stop(udc); + mutex_unlock(udc_lock); } else { dev_err(dev, unsupported command '%s'\n, buf); return -EINVAL; diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index e2f00fd..7ada7e6 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -922,6 +922,9 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver); */ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver); +int usb_gadget_start(struct usb_gadget *gadget); +int usb_gadget_stop(struct usb_gadget *gadget); + extern int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget, void (*release)(struct device *dev)); extern int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget); -- 2.1.0 -- 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: [RFC][PATCH 8/9] usb: otg-fsm: Remove unused members in struct otg_fsm
On Wed, Mar 18, 2015 at 03:56:02PM +0200, Roger Quadros wrote: These members are not used anywhere so remove them. Signed-off-by: Roger Quadros rog...@ti.com --- include/linux/usb/otg-fsm.h | 5 - 1 file changed, 5 deletions(-) diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index b6ba1bf..176c4fc 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -95,11 +95,6 @@ struct otg_fsm { int b_hnp_enable; int a_clr_err; - /* Informative variables */ - int a_bus_drop_inf; - int a_bus_req_inf; - int a_clr_err_inf; - int b_bus_req_inf; /* Auxilary informative variables */ int a_suspend_req_inf; But the above are defined at: ch 7.4.4, On-The-Go and Embedded Host Supplement to the USB Revision 2.0 Specification -- 2.1.0 -- 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: [RFC][PATCH 3/9] usb: otg: add OTG core
*gadget); +void usb_otg_sync_inputs(struct otg_fsm *fsm); +int usb_otg_kick_fsm(struct device *hcd_gcd_device); +bool usb_otg_gadget_can_start(struct usb_gadget *gadget); + +#else /* CONFIG_USB_OTG_CORE */ + +static inline struct otg_fsm *usb_otg_register(struct device *parent_dev, +struct otg_fsm_ops *fsm_ops) +{ + return ERR_PTR(-ENOSYS); +} + +static inline int usb_otg_unregister(struct device *parent_dev) +{ + return -ENOSYS; +} + +static inline int usb_otg_register_hcd(struct usb_hcd *hcd) +{ + return -ENOSYS; +} + +int usb_otg_unregister_hcd(struct usb_hcd *hcd) +{ + return -ENOSYS; +} + +static inline int usb_otg_register_gadget(struct usb_gadget *gadget) +{ + return -ENOSYS; +} + +static inline int usb_otg_unregister_gadget(struct usb_gadget *gadget) +{ + return -ENOSYS; +} + +static inline void usb_otg_sync_inputs(struct otg_fsm *fsm) +{ +} + +int usb_otg_kick_fsm(struct device *hcd_gcd_device) +{ + return -ENOSYS; +} + +bool usb_otg_gadget_can_start(struct usb_gadget *gadget) +{ + return true; +} +#endif /* CONFIG_USB_OTG_CORE */ + +#endif /* __LINUX_USB_OTG_CORE */ -- 2.1.0 -- 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 01/19] usb: common: drd-lib: Add DRD lib for USB.
= ~DRD_DEVICE_ACTIVE; + drd_gadget-g_driver = NULL; + spin_unlock(drd_lock); + + return 0; +} +EXPORT_SYMBOL_GPL(usb_drd_unregister_udc_driver); + +int usb_drd_start_udc(struct device *parent) +{ + struct usb_drd *drd; + struct usb_drd_gadget *drd_gadget; + struct usb_drd_setup *setup; + + drd = usb_drd_get_dev(parent); + if (!drd) + return -ENODEV; + + if (WARN_ON(!(drd-state DRD_DEVICE_REGISTERED))) + return -EINVAL; + + drd_gadget = drd-gadget; + setup = drd_gadget-gadget_setup; + + if (setup setup-ll_start) + setup-ll_start(setup-data); + + usb_add_gadget_udc_release(parent, drd_gadget-gadget, +setup-ll_release); + spin_lock(drd_lock); + drd-state |= DRD_DEVICE_ACTIVE; + spin_unlock(drd_lock); + + return 0; +} +EXPORT_SYMBOL_GPL(usb_drd_start_udc); + +int usb_drd_stop_udc(struct device *parent) +{ + struct usb_drd *drd; + struct usb_drd_gadget *drd_gadget; + struct usb_drd_setup *setup; + + drd = usb_drd_get_dev(parent); + if (!drd) + return -ENODEV; + + if (WARN_ON(!(drd-state DRD_DEVICE_REGISTERED))) + return -EINVAL; + + drd_gadget = drd-gadget; + setup = drd_gadget-gadget_setup; + if (setup setup-ll_stop) + setup-ll_stop(setup-data); + + usb_del_gadget_udc(drd_gadget-gadget); + + spin_lock(drd_lock); + drd-state = drd-state ~DRD_DEVICE_ACTIVE; + spin_unlock(drd_lock); + + return 0; +} +EXPORT_SYMBOL_GPL(usb_drd_stop_udc); + +MODULE_DESCRIPTION(USB-DRD Library); +MODULE_AUTHOR(George Cherian george.cher...@ti.com); +MODULE_LICENSE(GPL v2); diff --git a/include/linux/usb/drd.h b/include/linux/usb/drd.h new file mode 100644 index 000..71c64dc --- /dev/null +++ b/include/linux/usb/drd.h @@ -0,0 +1,77 @@ +#include linux/usb/gadget.h +#include linux/usb/otg.h +#include linux/usb/hcd.h + +struct usb_drd_setup { + int (*ll_start)(void *); + int (*ll_stop)(void *); + void(*ll_release)(struct device *); + void*data; +}; + +struct usb_drd_host { + struct usb_hcd *main_hcd; + struct usb_hcd *shared_hcd; + int hcd_irq; + struct usb_drd_setup *host_setup; +}; + +struct usb_drd_gadget { + struct usb_gadget_driver *g_driver; + struct usb_gadget *gadget; + struct usb_drd_setup *gadget_setup; +}; + +#define DRD_UNREGISTERED 0x0 +#define DRD_DEVICE_REGISTERED0x1 +#define DRD_HOST_REGISTERED 0x2 +#define DRD_HOST_ACTIVE 0x4 +#define DRD_DEVICE_ACTIVE0x8 + +#if IS_ENABLED(CONFIG_DRD_LIB) +int usb_drd_release(struct device *parent); +int usb_drd_add(struct device *parent); +int usb_drd_register_udc(struct device *parent, + struct usb_drd_gadget *gadget); +int usb_drd_register_udc_driver(struct device *parent, + struct usb_gadget_driver *driver); +int usb_drd_unregister_udc(struct device *parent); +int usb_drd_unregister_udc_driver(struct device *parent); +int usb_drd_register_hcd(struct device *parent, + struct usb_drd_host *host); +int usb_drd_unregister_hcd(struct device *parent); +int usb_drd_start_hcd(struct device *parent); +int usb_drd_stop_hcd(struct device *parent); +int usb_drd_start_udc(struct device *parent); +int usb_drd_stop_udc(struct device *parent); +int usb_drd_get_state(struct device *parent); +#else +static inline int usb_drd_release(struct device *parent) +{ return 0; } +static inline int usb_drd_add(struct device *parent) +{ return 0; } +static inline int usb_drd_register_udc(struct device *parent, +struct usb_drd_gadget *gadget) +{ return 0; } +static inline int usb_drd_register_udc_driver(struct device *parent, + struct usb_gadget_driver *driver) +{ return 0; } +static inline int usb_drd_unregister_udc(struct device *parent, + struct usb_drd_gadget *gadget) +{ return 0; } +static inline int usb_drd_unregister_udc_driver(struct device *parent) +{ return 0; } +static inline int usb_drd_register_hcd(struct device *parent, +struct usb_drd_host *host) +{ return 0; } +static inline int usb_drd_unregister_hcd(struct device *parent) +{ return 0; } +static inline int usb_drd_stop_hcd(struct device *parent) +{ return 0; } +static inline int usb_drd_start_udc(struct device *parent) +{ return 0; } +static inline int usb_drd_stop_udc(struct device *parent) +{ return 0; } +static inline int usb_drd_get_state(struct device *parent) +{ return 0; } +#endif -- 1.8.3.1 -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord
Re: RCU bug with v3.17-rc3 ?
On Fri, Oct 10, 2014 at 08:44:33PM -0500, Nathan Lynch wrote: On 10/10/2014 11:25 AM, Russell King - ARM Linux wrote: Right, so GCC 4.8.{1,2} are totally unsuitable for kernel building (and it seems that this has been known about for some time.) Looking at http://gcc.gnu.org/PR58854 it seems that all 4.8.x for x 3 are affected, as well as 4.9.0. We can blacklist these GCC versions quite easily. We already have GCC 3.3 blacklisted, and it's trivial to add others. I would want to include some proper details about the bug, just like the other existing entries we already have in asm-offsets.c, where we name the functions that the compiler is known to break where appropriate. Before blacklisting anything, it's worth considering that simple version checks would break existing pre-4.8.3 compilers that have been patched for PR58854. It looks like Yocto and Buildroot issued releases with patched 4.8.2 compilers well before the (fixed) 4.8.3 release. I think the most we can reasonably do without breaking some correctly-behaving toolchains is to emit a warning. Yocto has PR58854 problem patch. http://git.yoctoproject.org/cgit/cgit.cgi/poky/tree/meta/recipes-devtools/gcc/gcc-4.8/0048-PR58854_fix_arm_apcs_epilogue.patch?h=daisy Hopefully nobody's still using gcc 4.8 from the Linaro 2013.11 toolchain release -- since it's a 4.8.3 prerelease from before the fix was committed you'll get GCC_VERSION == 40803 but still generate bad code. However, I'm rather annoyed that there are people here who have known for some time that GCC 4.8.1 and GCC 4.8.2 _can_ lead to filesystem corruption, and have sat on their backsides doing nothing about getting it blacklisted for something like a year. Mea culpa, although I hadn't drawn the connection to FS corruption reports until now. I have known about the issue for some time, but figured the prevalence of the fix in downstream projects largely mitigated the issue. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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] USB: set device dma_mask without reference to global data
On Wed, May 8, 2013 at 6:53 AM, Stephen Warren swar...@wwwdotorg.org wrote: From: Stephen Warren swar...@nvidia.com Many USB host drivers contain code such as: if (!pdev-dev.dma_mask) pdev-dev.dma_mask = tegra_ehci_dma_mask; ... where tegra_ehci_dma_mask is a global. I suspect this code originated in commit 4a53f4e USB: ehci-tegra: add probing through device tree and was simply copied everywhere else. One question: why device tree can't do this when create device? -- 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] USB: set device dma_mask without reference to global data
This probably could be initialized from some DT property. However, there's no such property defined right now, and considering that DT is supposed to be an ABI, we'd always need the code in this patch as a fallback for DTs that were created before any such property was defined. Equally, since the data is SoC-specific rather than board-specific, and is even fairly unlikely to vary between SoC versions since these values are all 0x anyway, I don't really see much point in putting it into DT, rather than just putting the static data into the driver. I mean there is already dev-dev.coherent_dma_mask = DMA_BIT_MASK(32); at function of_platform_device_create, why can't add dev-dev.dma_mask = dev-dev.coherent_dma_mask after that? If DT core can do above things, can we delete dma_mask assignment at every driver? -- BR, 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 0/6] twl4030: Various fixes for charing-from-USB
On Wed, Apr 25, 2012 at 05:33:10PM +1000, NeilBrown wrote: Following are a collection of patches that I've need using for a while to make sure the charge-from-usb on my GTA04 works. Hopefully I've included the right people in the recipient list :-) The issues are: - charge the backup battery as well as the main battery - charge from a charger which ties ID to ground via a resistor - charge while device is suspended, or when no gadget module is loaded (i.e. when the USB side thinks the phy should be powered down). According to USB Spec, when the USB bus goes to suspend, the device should not draw more than 2.5mA. So when the device is suspended, it should notify the battery driver do NOT charge any more. Questions and comments more welcome. Thanks, NeilBrown --- NeilBrown (6): twl4030-usb: Don't report EVENT_ID when there is VBUS. twl4030-usb: Don't power down phy when it is in-use by charger. twl4030_charger: Allow charger to control the regulator that feeds it. twl4030_charger: allow charging whenever VBUS is present. twl4030_charger: add backup-battery charging. twl4030_charger: Fix some typos drivers/mfd/twl-core.c |9 ++-- drivers/power/twl4030_charger.c | 86 +++ drivers/usb/otg/twl4030-usb.c | 27 include/linux/i2c/twl.h |2 + 4 files changed, 102 insertions(+), 22 deletions(-) -- Signature -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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