Re: [PATCH v4 04/13] otg-fsm: move usb_bus_start_enum into otg-fsm->ops

2015-09-06 Thread Peter Chen
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 
> Acked-by: Peter Chen 
> ---
>  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 07/13] usb: otg: add OTG core

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

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

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

Peter

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

Re: [PATCH] ARM: multi_v7_defconfig: Enable PBIAS regulator

2015-09-06 Thread Guenter Roeck
On Thu, Sep 03, 2015 at 03:25:11PM +0530, Kishon Vijay Abraham I wrote:
> PBIAS regulator is required for MMC module in OMAP2, OMAP3, OMAP4,
> OMAP5 and DRA7 SoCs. Enable it here.
> 
> Signed-off-by: Kishon Vijay Abraham I 

Tested-by: Guenter Roeck 

> ---
>  arch/arm/configs/multi_v7_defconfig |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/configs/multi_v7_defconfig 
> b/arch/arm/configs/multi_v7_defconfig
> index 5fd8df6..f497c96 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -403,6 +403,7 @@ CONFIG_REGULATOR_MAX8973=y
>  CONFIG_REGULATOR_MAX77686=y
>  CONFIG_REGULATOR_MAX77693=m
>  CONFIG_REGULATOR_PALMAS=y
> +CONFIG_REGULATOR_PBIAS=y
>  CONFIG_REGULATOR_S2MPS11=y
>  CONFIG_REGULATOR_S5M8767=y
>  CONFIG_REGULATOR_TPS51632=y
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 
--
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

2015-09-06 Thread Peter Chen
On Mon, Aug 24, 2015 at 04:21:11PM +0300, Roger Quadros wrote:
> Hi,
> 
> This series centralizes OTG/Dual-role functionality in the kernel.
> As of now I've got Dual-role functionality working pretty reliably on
> dra7-evm and am437x-gp-evm.
> 
> DWC3 controller and platform related patches will be sent separately.
> 
> Series is based on Greg's usb-next tree.
> 
> Changelog:
> -
> v4:
> - Added DT support for tying otg-controller to host and gadget
>  controllers. For DT we no longer have the constraint that
>  OTG controller needs to be parent of host and gadget. They can be
>  tied together using the "otg-controller" property.
> - Relax the requirement for DT case that otg controller must register before
>  host/gadget. We maintain a wait list of host/gadget devices
>  waiting on the otg controller.
> - Use a single struct usb_otg for otg data.
> - Don't override host/gadget start/stop APIs. Let the controller
>  drivers do what they want as they know best. Helper API is provided
>  for controller start/stop that controller driver can use.
> - Introduce struct usb_otg_config to pass the otg capabilities,
>  otg ops and otg timer timeouts during otg controller registration.
> - rebased on Greg's usb.git/usb-next

Roger, thanks for your hard work. Since it is complicated, and can't
know its correctness and scalable well just reading code. I will run
it for chipidea driver, wait some time please.

Peter
> 
> v3:
> - all otg related definations now in otg.h
> - single kernel config USB_OTG to enable OTG core and FSM.
> - resolved symbol dependency issues.
> - use dev_vdbg instead of VDBG() in usb-otg-fsm.c
> - rebased on v4.2-rc1
> 
> v2:
> - Use add/remove_hcd() instead of start/stop_hcd() to enable/disable
>  the host controller
> - added dual-role-device (DRD) state machine which is a much simpler
>  mode of operation when compared to OTG. Here we don't support fancy
>  OTG features like HNP, SRP, on the fly role-swap. The mode of operation
>  is determined based on ID pin (cable type) and the role doesn't change
>  till the cable type changes.
> 
> Why?:
> 
> 
> Most of the OTG drivers have been dealing with the OTG state machine
> themselves and there is a scope for code re-use. This has been
> partly addressed by the usb/common/usb-otg-fsm.c but it still
> leaves the instantiation of the state machine and OTG timers
> to the controller drivers. We re-use usb-otg-fsm.c but
> go one step further by instantiating the state machine and timers
> thus making it easier for drivers to implement OTG functionality.
> 
> Newer OTG cores support standard host interface (e.g. xHCI) so
> host and gadget functionality are no longer closely knit like older
> cores. There needs to be a way to co-ordinate the operation of the
> host and gadget in OTG mode. i.e. to stop and start them from a
> central location. This central location should be the USB OTG core.
> 
> Host and gadget controllers might be sharing resources and can't
> be always running. One has to be stopped for the other to run.
> This can't be done as of now and can be done from the OTG core.
> 
> What?:
> -
> 
> The OTG core instantiates the OTG/DRD 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
> 
> - Registering an OTG capable controller
> struct otg_fsm *usb_otg_register(struct device *dev,
>  struct usb_otg_config *config);
> 
> int usb_otg_unregister(struct device *dev);
> 
> - Registering Host controllers to OTG core (used by hcd-core)
> 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);
> 
> 
> - Registering Gadget controllers to OTG core (used by udc-core)
> int usb_otg_register_gadget(struct usb_gadget *gadget,
> struct otg_gadget_ops *ops);
> int usb_otg_unregister_gadget(struct usb_gadget *gadget);
> 
> 
> - Providing inputs to and kicking the OTG state machine
> void usb_otg_sync_inputs(struct otg_fsm *fsm);
> int usb_otg_kick_fsm(struct device *hcd_gcd_device);
> 
> - Getting controller device structure from OTG state machine instance
> struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm);
> 
> 'struct otg_fsm' is the interface to the OTG state machine.
> It contains inputs to the fsm, status of the fsm and operations
> for the OTG controller driver.
> 
> - Helper APIs for starting/stopping host/gadget controllers
> int usb_otg_start_host(struct otg_fsm *fsm, int on);
> 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 

Re: [PATCH v4 02/13] usb: otg-fsm: support multiple instances

2015-09-06 Thread Peter Chen
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 
> ---
>  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(&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 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 

-- 

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