Re: [RFC/PATCH v4] usb: dwc3: Introduce OTG driver for dwc3

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mon, Aug 06, 2012 at 03:31:17PM +0300, Ido Shayevitz wrote:
> This is first release of otg driver for the dwc3 Synopsys USB3 core.
> The otg driver implements the otg final state machine and control the
> activation of the device controller or host controller.
> 
> In this first implementation, only simple DRD mode is implemented,
> determine if A or B device according to the ID pin as reflected in the
> OSTS.ConIDSts field.
> 
> Signed-off-by: Ido Shayevitz <idos@xxxxxxxxxxxxxx>
> 
> ---
>  drivers/usb/dwc3/Makefile    |    2 +
>  drivers/usb/dwc3/core.c      |   15 +-
>  drivers/usb/dwc3/core.h      |   55 ++++-
>  drivers/usb/dwc3/dwc3_otg.c  |  537 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/dwc3/dwc3_otg.h  |   59 +++++
>  drivers/usb/dwc3/gadget.c    |   63 +++++
>  drivers/usb/host/xhci-plat.c |   21 ++
>  drivers/usb/host/xhci.c      |   13 +-
>  8 files changed, 755 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/usb/dwc3/dwc3_otg.c
>  create mode 100644 drivers/usb/dwc3/dwc3_otg.h
> 
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index d441fe4..ffb3f55 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -1,11 +1,13 @@
>  ccflags-$(CONFIG_USB_DWC3_DEBUG)	:= -DDEBUG
>  ccflags-$(CONFIG_USB_DWC3_VERBOSE)	+= -DVERBOSE_DEBUG
> +ccflags-y += -Idrivers/usb/host

this is wrong.

>  obj-$(CONFIG_USB_DWC3)			+= dwc3.o
>  
>  dwc3-y					:= core.o
>  dwc3-y					+= host.o
>  dwc3-y					+= gadget.o ep0.o
> +dwc3-y					+= dwc3_otg.o
>  
>  ifneq ($(CONFIG_DEBUG_FS),)
>  	dwc3-y				+= debugfs.o
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index c34452a..5343e39 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -517,15 +517,24 @@ static int __devinit dwc3_probe(struct platform_device *pdev)
>  		break;
>  	case DWC3_MODE_DRD:
>  		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> +		ret = dwc3_otg_init(dwc);
> +		if (ret) {
> +			dev_err(dev, "failed to initialize otg\n");
> +			goto err1;
> +		}
> +
>  		ret = dwc3_host_init(dwc);
>  		if (ret) {
>  			dev_err(dev, "failed to initialize host\n");
> +			dwc3_otg_exit(dwc);
>  			goto err1;
>  		}
>  
>  		ret = dwc3_gadget_init(dwc);
>  		if (ret) {
>  			dev_err(dev, "failed to initialize gadget\n");
> +			dwc3_host_exit(dwc);
> +			dwc3_otg_exit(dwc);
>  			goto err1;
>  		}
>  		break;
> @@ -554,8 +563,9 @@ err2:
>  		dwc3_host_exit(dwc);
>  		break;
>  	case DWC3_MODE_DRD:
> -		dwc3_host_exit(dwc);
>  		dwc3_gadget_exit(dwc);
> +		dwc3_host_exit(dwc);

inverting gadget and host is fixing something ? If it is, it should be a
separate patch with a proper reasoning.

> +		dwc3_otg_exit(dwc);
>  		break;
>  	default:
>  		/* do nothing */
> @@ -588,8 +598,9 @@ static int __devexit dwc3_remove(struct platform_device *pdev)
>  		dwc3_host_exit(dwc);
>  		break;
>  	case DWC3_MODE_DRD:
> -		dwc3_host_exit(dwc);
>  		dwc3_gadget_exit(dwc);
> +		dwc3_host_exit(dwc);

likewise.

> +		dwc3_otg_exit(dwc);
>  		break;
>  	default:
>  		/* do nothing */
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index c611d80..c2521ba 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -50,6 +50,8 @@
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
>  
> +#include "dwc3_otg.h"
> +
>  /* Global constants */
>  #define DWC3_EP0_BOUNCE_SIZE	512
>  #define DWC3_ENDPOINTS_NUM	32
> @@ -152,8 +154,9 @@
>  /* OTG Registers */
>  #define DWC3_OCFG		0xcc00
>  #define DWC3_OCTL		0xcc04
> -#define DWC3_OEVTEN		0xcc08
> -#define DWC3_OSTS		0xcc0C
> +#define DWC3_OEVT		0xcc08
> +#define DWC3_OEVTEN		0xcc0c
> +#define DWC3_OSTS		0xcc10

this should be a separate patch fixing the macro definitions.

>  
>  /* Bit fields */
>  
> @@ -203,6 +206,9 @@
>  #define DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(n)	(((n) & (0x0f << 13)) >> 13)
>  #define DWC3_MAX_HIBER_SCRATCHBUFS		15
>  
> +/* Global HWPARAMS6 Register */
> +#define DWC3_GHWPARAMS6_SRP_SUPPORT	(1 << 10)
> +
>  /* Device Configuration Register */
>  #define DWC3_DCFG_LPM_CAP	(1 << 22)
>  #define DWC3_DCFG_DEVADDR(addr)	((addr) << 3)
> @@ -358,6 +364,38 @@
>  #define DWC3_DEPCMD_TYPE_BULK		2
>  #define DWC3_DEPCMD_TYPE_INTR		3
>  
> +/* OTG Events Register */
> +#define DWC3_OEVT_DEVICEMODE			(1 << 31)
> +#define DWC3_OEVT_CLEAR_ALL			(~DWC3_OEVT_DEVICEMODE)
> +#define DWC3_OEVTEN_OTGCONIDSTSCHNGEVNT		(1 << 24)
> +#define DWC3_OEVTEN_OTGADEVBHOSTENDEVNT		(1 << 20)
> +#define DWC3_OEVTEN_OTGADEVHOSTEVNT		(1 << 19)
> +#define DWC3_OEVTEN_OTGADEVHNPCHNGEVNT		(1 << 18)
> +#define DWC3_OEVTEN_OTGADEVSRPDETEVNT		(1 << 17)
> +#define DWC3_OEVTEN_OTGADEVSESSENDDETEVNT	(1 << 16)
> +#define DWC3_OEVTEN_OTGBDEVBHOSTENDEVNT		(1 << 11)
> +#define DWC3_OEVTEN_OTGBDEVHNPCHNGEVNT		(1 << 10)
> +#define DWC3_OEVTEN_OTGBDEVSESSVLDDETEVNT	(1 << 9)
> +#define DWC3_OEVTEN_OTGBDEVVBUSCHNGEVNT		(1 << 8)
> +
> +/* OTG OSTS register */
> +#define DWC3_OTG_OSTS_OTGSTATE_SHIFT	(8)
> +#define DWC3_OTG_OSTS_OTGSTATE		(0xF << DWC3_OTG_OSTS_OTGSTATE_SHIFT)
> +#define DWC3_OTG_OSTS_PERIPHERALSTATE	(1 << 4)
> +#define DWC3_OTG_OSTS_XHCIPRTPOWER	(1 << 3)
> +#define DWC3_OTG_OSTS_BSESVALID		(1 << 2)
> +#define DWC3_OTG_OSTS_VBUSVALID		(1 << 1)
> +#define DWC3_OTG_OSTS_CONIDSTS		(1 << 0)
> +
> +/* OTG OCTL register */
> +#define DWC3_OTG_OCTL_PERIMODE		(1 << 6)
> +#define DWC3_OTG_OCTL_PRTPWRCTL		(1 << 5)
> +#define DWC3_OTG_OCTL_HNPREQ		(1 << 4)
> +#define DWC3_OTG_OCTL_SESREQ		(1 << 3)
> +#define DWC3_OTG_OCTL_TERMSELDLPULSE	(1 << 2)
> +#define DWC3_OTG_OCTL_DEVSETHNPEN	(1 << 1)
> +#define DWC3_OTG_OCTL_HSTSETHNPEN	(1 << 0)
> +
>  /* Structures */
>  
>  struct dwc3_trb;
> @@ -611,6 +649,7 @@ struct dwc3_scratchpad_array {
>   * @ep0_bounce_addr: dma address of ep0_bounce
>   * @lock: for synchronizing
>   * @dev: pointer to our struct device
> + * @dotg: pointer to the dwc3 otg instance
>   * @xhci: pointer to our xHCI child
>   * @event_buffer_list: a list of event buffers
>   * @gadget: device side representation of the peripheral controller
> @@ -643,6 +682,8 @@ struct dwc3_scratchpad_array {
>   * @mem: points to start of memory which is used for this struct.
>   * @hwparams: copy of hwparams registers
>   * @root: debugfs root folder pointer
> + * @vbus_active: Indicates if the gadget was powered by the otg driver
> + * @softconnect: Indicates if pullup was issued by the usb_gadget_driver
>   */
>  struct dwc3 {
>  	struct usb_ctrlrequest	*ctrl_req;
> @@ -657,6 +698,7 @@ struct dwc3 {
>  	spinlock_t		lock;
>  	struct device		*dev;
>  
> +	struct dwc3_otg		*dotg;
>  	struct platform_device	*xhci;
>  	struct resource		xhci_resources[DWC3_XHCI_RESOURCES_NUM];
>  
> @@ -719,6 +761,12 @@ struct dwc3 {
>  
>  	u8			test_mode;
>  	u8			test_mode_nr;
> +
> +	/* Indicate if the gadget was powered by the otg driver */
> +	bool			vbus_active;
> +
> +	/* Indicate if pullup was issued by the usb_gadget_driver */
> +	bool			softconnect;
>  };
>  
>  /* -------------------------------------------------------------------------- */
> @@ -857,6 +905,9 @@ union dwc3_event {
>  void dwc3_set_mode(struct dwc3 *dwc, u32 mode);
>  int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
>  
> +int dwc3_otg_init(struct dwc3 *dwc);
> +void dwc3_otg_exit(struct dwc3 *dwc);
> +
>  int dwc3_host_init(struct dwc3 *dwc);
>  void dwc3_host_exit(struct dwc3 *dwc);
>  
> diff --git a/drivers/usb/dwc3/dwc3_otg.c b/drivers/usb/dwc3/dwc3_otg.c
> new file mode 100644
> index 0000000..5061559
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3_otg.c

let's name the file otg.c only (and otg.h for its counterpart). The only
files with prefixes are the glue layers.

> @@ -0,0 +1,537 @@
> +/**
> + * dwc3_otg.c - DesignWare USB3 DRD Controller OTG
> + *
> + * Copyright (c) 2012, Code Aurora Forum. All rights reserved.
> +
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, and the entire permission notice in its entirety,
> + *    including the disclaimer of warranties.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote
> + *    products derived from this software without specific prior
> + *    written permission.
> +
> + * ALTERNATIVELY, this product may be distributed under the terms of
> + * the GNU General Public License, version 2, in which case the provisions
> + * of the GPL version 2 are required INSTEAD OF the BSD license.
> +
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF
> + * WHICH ARE HEREBY DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
> + * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> + * USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH
> + * DAMAGE.
> + */
> +
> +#include <linux/usb.h>
> +#include <linux/usb/hcd.h>
> +#include <linux/platform_device.h>
> +
> +#include "core.h"
> +#include "dwc3_otg.h"
> +#include "io.h"
> +#include "xhci.h"
> +
> +
> +/**
> + * dwc3_otg_set_host_regs - reset dwc3 otg registers to host operation.
> + *
> + * This function sets the OTG registers to work in A-Device host mode.
> + * This function should be called just before entering to A-Device mode.
> + *
> + * @dotg: Pointer to the dwc3 otg structure.
> + */
> +static void dwc3_otg_set_host_regs(struct dwc3_otg *dotg)
> +{
> +	u32 octl;
> +
> +	/* Set OCTL[6](PeriMode) to 0 (host) */
> +	octl = dwc3_readl(dotg->regs, DWC3_OCTL);
> +	octl &= ~DWC3_OTG_OCTL_PERIMODE;
> +	dwc3_writel(dotg->regs, DWC3_OCTL, octl);
> +
> +	/*
> +	 * TODO: add more OTG registers writes for HOST mode here,
> +	 * see figure 12-10 A-device flow in dwc3 Synopsis spec
> +	 */
> +}
> +
> +/**
> + * dwc3_otg_set_peripheral_regs - reset dwc3 otg registers to peripheral operation.
> + *
> + * This function sets the OTG registers to work in B-Device peripheral mode.
> + * This function should be called just before entering to B-Device mode.
> + *
> + * @dotg: Pointer to the dwc3 otg structure.
> + */
> +static void dwc3_otg_set_peripheral_regs(struct dwc3_otg *dotg)
> +{
> +	u32 octl;
> +
> +	/* Set OCTL[6](PeriMode) to 1 (peripheral) */
> +	octl = dwc3_readl(dotg->regs, DWC3_OCTL);
> +	octl |= DWC3_OTG_OCTL_PERIMODE;
> +	dwc3_writel(dotg->regs, DWC3_OCTL, octl);
> +
> +	/*
> +	 * TODO: add more OTG registers writes for PERIPHERAL mode here,
> +	 * see figure 12-19 B-device flow in dwc3 Synopsis spec
> +	 */
> +}
> +
> +/**
> + * dwc3_otg_start_host -  helper function for starting/stoping the host controller driver.
> + *
> + * @otg: Pointer to the usb_otg structure.
> + * @on: start / stop the host controller driver.
> + *
> + * Returns 0 on success otherwise negative errno.
> + */
> +static int dwc3_otg_start_host(struct usb_otg *otg, int on)
> +{
> +	struct dwc3_otg *dotg = container_of(otg, struct dwc3_otg, otg);
> +	struct usb_hcd *hcd;
> +	struct xhci_hcd *xhci;
> +	int ret = 0;
> +
> +	if (!otg->host)
> +		return -EINVAL;
> +
> +	hcd = bus_to_hcd(otg->host);
> +	xhci = hcd_to_xhci(hcd);
> +	if (on) {
> +		dev_dbg(otg->phy->dev, "%s: turn on host %s\n",
> +					__func__, otg->host->bus_name);
> +		dwc3_otg_set_host_regs(dotg);
> +
> +		/*
> +		 * This should be revisited for more testing post-silicon.
> +		 * In worst case we may need to disconnect the root hub
> +		 * before stopping the controller so that it does not
> +		 * interfere with runtime pm/system pm.
> +		 * We can also consider registering and unregistering xhci
> +		 * platform device. It is almost similar to add_hcd and
> +		 * remove_hcd, But we may not use standard set_host method
> +		 * anymore.
> +		 */
> +		ret = hcd->driver->start(hcd);
> +		if (ret) {
> +			dev_err(otg->phy->dev,
> +				"%s: failed to start primary hcd, ret=%d\n",
> +				__func__, ret);
> +			return ret;
> +		}
> +
> +		ret = xhci->shared_hcd->driver->start(xhci->shared_hcd);
> +		if (ret) {
> +			dev_err(otg->phy->dev,
> +				"%s: failed to start secondary hcd, ret=%d\n",
> +				__func__, ret);
> +			return ret;
> +		}
> +	} else {
> +		dev_dbg(otg->phy->dev, "%s: turn off host %s\n",
> +					__func__, otg->host->bus_name);
> +		hcd->driver->stop(hcd);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * dwc3_otg_set_host -  bind/unbind the host controller driver.
> + *
> + * @otg: Pointer to the usb_otg structure.
> + * @host: Pointer to the usb_bus structure.
> + *
> + * Returns 0 on success otherwise negative errno.
> + */
> +static int dwc3_otg_set_host(struct usb_otg *otg, struct usb_bus *host)
> +{
> +	struct dwc3_otg *dotg = container_of(otg, struct dwc3_otg, otg);
> +
> +	if (host) {
> +		dev_dbg(otg->phy->dev, "%s: set host %s\n",
> +					__func__, host->bus_name);
> +		otg->host = host;
> +
> +		/*
> +		 * Prevents unnecessary activation of the work function.
> +		 * If both peripheral and host are set or if ID pin is low
> +		 * then we ensure that work function will enter to valid state.
> +		 */
> +		if (otg->gadget || !(dotg->osts & DWC3_OTG_OSTS_CONIDSTS))
> +			schedule_work(&dotg->sm_work);
> +	} else {
> +		if (otg->phy->state == OTG_STATE_A_HOST) {
> +			dwc3_otg_start_host(otg, 0);
> +			otg->host = NULL;
> +			otg->phy->state = OTG_STATE_UNDEFINED;
> +			schedule_work(&dotg->sm_work);
> +		} else {
> +			otg->host = NULL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * dwc3_otg_start_peripheral -  bind/unbind the peripheral controller.
> + *
> + * @otg: Pointer to the usb_otg structure.
> + * @on: start / stop the gadget controller driver.
> + *
> + * Returns 0 on success otherwise negative errno.
> + */
> +static int dwc3_otg_start_peripheral(struct usb_otg *otg, int on)
> +{
> +	struct dwc3_otg *dotg = container_of(otg, struct dwc3_otg, otg);
> +
> +	if (!otg->gadget)
> +		return -EINVAL;
> +
> +	if (on) {
> +		dev_dbg(otg->phy->dev, "%s: turn on gadget %s\n",
> +					__func__, otg->gadget->name);
> +		dwc3_otg_set_peripheral_regs(dotg);
> +		usb_gadget_vbus_connect(otg->gadget);
> +	} else {
> +		dev_dbg(otg->phy->dev, "%s: turn off gadget %s\n",
> +					__func__, otg->gadget->name);
> +		usb_gadget_vbus_disconnect(otg->gadget);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * dwc3_otg_set_peripheral -  bind/unbind the peripheral controller driver.
> + *
> + * @otg: Pointer to the usb_otg structure.
> + * @gadget: pointer to the usb_gadget structure.
> + *
> + * Returns 0 on success otherwise negative errno.
> + */
> +static int dwc3_otg_set_peripheral(struct usb_otg *otg,
> +				struct usb_gadget *gadget)
> +{
> +	struct dwc3_otg *dotg = container_of(otg, struct dwc3_otg, otg);
> +
> +	if (gadget) {
> +		dev_dbg(otg->phy->dev, "%s: set gadget %s\n",
> +					__func__, gadget->name);
> +		otg->gadget = gadget;
> +
> +		/*
> +		 * Prevents unnecessary activation of the work function.
> +		 * If both peripheral and host are set or if ID pin is high
> +		 * then we ensure that work function will enter to valid state.
> +		 */
> +		if (otg->host || dotg->osts & DWC3_OTG_OSTS_CONIDSTS)
> +			schedule_work(&dotg->sm_work);

why do you need a workqueue ? It shouldn't be necessary at all.

> +	} else {
> +		if (otg->phy->state == OTG_STATE_B_PERIPHERAL) {
> +			dwc3_otg_start_peripheral(otg, 0);
> +			otg->gadget = NULL;
> +			otg->phy->state = OTG_STATE_UNDEFINED;
> +			schedule_work(&dotg->sm_work);
> +		} else {
> +			otg->gadget = NULL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * dwc3_otg_interrupt - interrupt handler for dwc3 otg events.
> + *
> + * @irq: irq number.
> + * @_dotg: Pointer to our controller context structure.
> + *
> + * Returns IRQ_HANDLED on success otherwise IRQ_NONE.
> + */
> +static irqreturn_t dwc3_otg_interrupt(int irq, void *_dotg)
> +{
> +	struct dwc3_otg *dotg = (struct dwc3_otg *)_dotg;
> +	u32 oevt_reg;
> +	int ret = IRQ_NONE;
> +	u32 handled_irqs = 0;
> +
> +	oevt_reg = dwc3_readl(dotg->regs, DWC3_OEVT);
> +
> +	if (oevt_reg & DWC3_OEVTEN_OTGCONIDSTSCHNGEVNT) {
> +		/*
> +		 * ID sts has changed, read it and later, in the workqueue
> +		 * function, switch from A to B or from B to A.
> +		 */
> +		dotg->osts = dwc3_readl(dotg->regs, DWC3_OSTS);
> +		if ((dotg->otg.phy->state == OTG_STATE_B_IDLE) ||
> +		    (dotg->otg.phy->state == OTG_STATE_A_IDLE)) {
> +
> +			/*
> +			 * OTG state is ABOUT to change to A or B device, but
> +			 * since ID sts was changed, then we return the state
> +			 * machine to the start point.
> +			 */
> +			 dotg->otg.phy->state = OTG_STATE_UNDEFINED;
> +		}
> +		schedule_work(&dotg->sm_work);

NAK. Use a proper threaded_irq here.

> +
> +		handled_irqs |= DWC3_OEVTEN_OTGCONIDSTSCHNGEVNT;
> +		ret = IRQ_HANDLED;
> +	}
> +
> +	/*
> +	 * Clear the interrupts we handled.
> +	 */
> +	if (ret == IRQ_HANDLED)
> +		dwc3_writel(dotg->regs, DWC3_OEVT, handled_irqs);
> +
> +	return ret;
> +}
> +
> +/**
> + * dwc3_otg_sm_work - work function.
> + *
> + * @w: Pointer to the dwc3 otg work structure.
> + *
> + * NOTE: After any change in phy->state,
> + * we must reschdule the state machine.
> + */
> +static void dwc3_otg_sm_work(struct work_struct *w)

this can be handled on the IRQ thread.

> +{
> +	struct dwc3_otg *dotg = container_of(w, struct dwc3_otg, sm_work);
> +	struct usb_phy *phy = dotg->otg.phy;
> +
> +	dev_dbg(phy->dev, "%s state\n", otg_state_string(phy->state));
> +
> +	/* Check OTG state */
> +	switch (phy->state) {
> +	case OTG_STATE_UNDEFINED:
> +		/* Switch to A or B-Device according to IDSTS */
> +		if (dotg->osts & DWC3_OTG_OSTS_CONIDSTS)
> +			phy->state = OTG_STATE_B_IDLE;
> +		else
> +			phy->state = OTG_STATE_A_IDLE;
> +
> +		schedule_work(&dotg->sm_work);
> +		break;
> +	case OTG_STATE_B_IDLE:
> +		if (dwc3_otg_start_peripheral(&dotg->otg, 1)) {
> +			/*
> +			 * Probably set_peripheral was not called yet.
> +			 * We will re-try as soon as it will be called
> +			 */
> +			dev_err(phy->dev,
> +				"unable to start B-device\n");
> +			phy->state = OTG_STATE_UNDEFINED;
> +		} else {
> +			phy->state = OTG_STATE_B_PERIPHERAL;
> +			schedule_work(&dotg->sm_work);
> +		}
> +
> +		break;
> +	case OTG_STATE_B_PERIPHERAL:
> +		if (!(dotg->osts & DWC3_OTG_OSTS_CONIDSTS)) {
> +			dwc3_otg_start_peripheral(&dotg->otg, 0);
> +			phy->state = OTG_STATE_A_IDLE;
> +			schedule_work(&dotg->sm_work);
> +		}
> +		break;
> +	case OTG_STATE_A_IDLE:
> +		/* Switch to A-Device*/
> +		if (dwc3_otg_start_host(&dotg->otg, 1)) {
> +			/*
> +			 * Probably set_host was not called yet.
> +			 * We will re-try as soon as it will be called
> +			 */
> +			dev_err(phy->dev,
> +				"unable to start A-device\n");
> +			phy->state = OTG_STATE_UNDEFINED;
> +		} else {
> +			phy->state = OTG_STATE_A_HOST;
> +			schedule_work(&dotg->sm_work);
> +		}
> +
> +		break;
> +	case OTG_STATE_A_HOST:
> +		if (dotg->osts & DWC3_OTG_OSTS_CONIDSTS) {
> +			dwc3_otg_start_host(&dotg->otg, 0);
> +			phy->state = OTG_STATE_B_IDLE;
> +			schedule_work(&dotg->sm_work);
> +		}
> +		break;
> +	default:
> +		dev_err(phy->dev, "%s: invalid otg-state\n", __func__);
> +
> +	}
> +}
> +
> +
> +/**
> + * dwc3_otg_reset - reset dwc3 otg registers.
> + *
> + * @dotg: Pointer to the dwc3 otg structure.
> + */
> +static void dwc3_otg_reset(struct dwc3_otg *dotg)
> +{
> +	/*
> +	 * OCFG[2] - OTG-Version = 0
> +	 * OCFG[1] - HNPCap = 0
> +	 * OCFG[0] - SRPCap = 0
> +	 */
> +	dwc3_writel(dotg->regs, DWC3_OCFG, 0x0);
> +
> +	/*
> +	 * OCTL[6] - PeriMode = 1
> +	 * OCTL[5] - PrtPwrCtl = 0
> +	 * OCTL[4] - HNPReq = 0
> +	 * OCTL[3] - SesReq = 0
> +	 * OCTL[2] - TermSelDLPulse = 0
> +	 * OCTL[1] - DevSetHNPEn = 0
> +	 * OCTL[0] - HstSetHNPEn = 0
> +	 */
> +	dwc3_writel(dotg->regs, DWC3_OCTL, DWC3_OTG_OCTL_PERIMODE);
> +
> +	/* Clear all otg events (interrupts) indications  */
> +	dwc3_writel(dotg->regs, DWC3_OEVT, DWC3_OEVT_CLEAR_ALL);
> +
> +	/* Enable only the ConIDStsChngEn event*/
> +	dwc3_writel(dotg->regs, DWC3_OEVTEN,
> +				DWC3_OEVTEN_OTGCONIDSTSCHNGEVNT);
> +
> +	/* Read OSTS  */
> +	dotg->osts = dwc3_readl(dotg->regs, DWC3_OSTS);
> +
> +}
> +
> +/**
> + * dwc3_otg_init - Initializes otg related registers
> + * @dwc: Pointer to out controller context structure
> + *
> + * Returns 0 on success otherwise negative errno.
> + */
> +int dwc3_otg_init(struct dwc3 *dwc)
> +{
> +	u32 reg;
> +	int ret = 0;
> +	struct dwc3_otg *dotg;
> +
> +	dev_dbg(dwc->dev, "%s\n", __func__);
> +
> +	/*
> +	 * GHWPARAMS6[10] bit is SRPSupport.
> +	 * This bit also reflects DWC_USB3_EN_OTG
> +	 */
> +	reg = dwc3_readl(dwc->regs, DWC3_GHWPARAMS6);
> +	if (!(reg & DWC3_GHWPARAMS6_SRP_SUPPORT)) {
> +		/*
> +		 * No OTG support in the HW core.
> +		 * We return 0 to indicate no error, since this is acceptable
> +		 * situation, just continue probe the dwc3 driver without otg.
> +		 */
> +		dev_dbg(dwc->dev, "dwc3_otg address space is not supported\n");
> +		return 0;
> +	}
> +
> +	/* Allocate and init otg instance */
> +	dotg = kzalloc(sizeof(struct dwc3_otg), GFP_KERNEL);

use devm_* apis like the rest of the driver.

> +	if (!dotg) {
> +		dev_err(dwc->dev, "unable to allocate dwc3_otg\n");
> +		return -ENOMEM;
> +	}
> +
> +	dotg->irq = platform_get_irq(to_platform_device(dwc->dev), 0);
> +	if (dotg->irq < 0) {
> +		dev_err(dwc->dev, "%s: missing IRQ\n", __func__);
> +		ret = -ENODEV;
> +		goto err1;
> +	}
> +
> +	dotg->regs = dwc->regs;
> +
> +	dotg->otg.set_peripheral = dwc3_otg_set_peripheral;
> +	dotg->otg.set_host = dwc3_otg_set_host;
> +
> +	/* This reference is used by dwc3 modules for checking otg existance */
> +	dwc->dotg = dotg;
> +
> +	dotg->otg.phy = kzalloc(sizeof(struct usb_phy), GFP_KERNEL);
> +	if (!dotg->otg.phy) {
> +		dev_err(dwc->dev, "unable to allocate dwc3_otg.phy\n");
> +		ret = -ENOMEM;
> +		goto err1;
> +	}
> +
> +	dotg->otg.phy->otg = &dotg->otg;
> +	dotg->otg.phy->dev = dwc->dev;
> +
> +	ret = usb_add_phy(dotg->otg.phy, USB_PHY_TYPE_USB3);

this looks wrong, actually. Is this small driver really a PHY driver ?
And if it is, you need a USB2 and USB3 phy otherwise this won't work
properly.

> +	if (ret) {
> +		dev_err(dotg->otg.phy->dev,
> +			"%s: failed to set transceiver, already exists\n",
> +			__func__);
> +		goto err2;
> +	}
> +
> +	dwc3_otg_reset(dotg);
> +
> +	dotg->otg.phy->state = OTG_STATE_UNDEFINED;
> +
> +	INIT_WORK(&dotg->sm_work, dwc3_otg_sm_work);
> +
> +	ret = request_irq(dotg->irq, dwc3_otg_interrupt, IRQF_SHARED,
> +				"dwc3_otg", dotg);
> +	if (ret) {
> +		dev_err(dotg->otg.phy->dev, "failed to request irq #%d --> %d\n",
> +				dotg->irq, ret);
> +		goto err3;
> +	}
> +
> +	return 0;
> +
> +err3:
> +	cancel_work_sync(&dotg->sm_work);
> +	usb_remove_phy(dotg->otg.phy);
> +err2:
> +	kfree(dotg->otg.phy);
> +err1:
> +	dwc->dotg = NULL;
> +	kfree(dotg);
> +
> +	return ret;
> +}
> +
> +/**
> + * dwc3_otg_exit
> + * @dwc: Pointer to out controller context structure
> + *
> + * Returns 0 on success otherwise negative errno.
> + */
> +void dwc3_otg_exit(struct dwc3 *dwc)
> +{
> +	struct dwc3_otg *dotg = dwc->dotg;
> +
> +	/* dotg is null when GHWPARAMS6[10]=SRPSupport=0, see dwc3_otg_init */
> +	if (dotg) {

this is wrong. Just check if you have SRP support on probe and return
early from probe(). Likewise here. If SRPSupport is zero, just return 0.

> +		cancel_work_sync(&dotg->sm_work);
> +		usb_remove_phy(dotg->otg.phy);
> +		free_irq(dotg->irq, dotg);
> +		kfree(dotg->otg.phy);
> +		kfree(dotg);
> +		dwc->dotg = NULL;
> +	}
> +}
> diff --git a/drivers/usb/dwc3/dwc3_otg.h b/drivers/usb/dwc3/dwc3_otg.h
> new file mode 100644
> index 0000000..e462b99
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3_otg.h
> @@ -0,0 +1,59 @@
> +/**
> + * dwc3_otg.h - DesignWare USB3 DRD Controller OTG
> + *
> + * Copyright (c) 2012, Code Aurora Forum. All rights reserved.
> +
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, and the entire permission notice in its entirety,
> + *    including the disclaimer of warranties.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote
> + *    products derived from this software without specific prior
> + *    written permission.
> +
> + * ALTERNATIVELY, this product may be distributed under the terms of
> + * the GNU General Public License, version 2, in which case the provisions
> + * of the GPL version 2 are required INSTEAD OF the BSD license.
> +
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF
> + * WHICH ARE HEREBY DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
> + * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> + * USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH
> + * DAMAGE.
> + */
> +
> +#ifndef __LINUX_USB_DWC3_OTG_H
> +#define __LINUX_USB_DWC3_OTG_H
> +
> +#include <linux/workqueue.h>
> +
> +#include <linux/usb/otg.h>
> +
> +/**
> + * struct dwc3_otg: OTG driver data. Shared by HCD and DCD.
> + * @otg: USB OTG Transceiver structure.
> + * @irq: IRQ number assigned for HSUSB controller.
> + * @regs: ioremapped register base address.
> + * @sm_work: OTG state machine work.
> + * @osts: last value of the OSTS register, as read from HW.
> + */
> +struct dwc3_otg {
> +	struct usb_otg otg;
> +	int irq;
> +	void __iomem *regs;
> +	struct work_struct sm_work;
> +	u32 osts;
> +};
> +#endif /* __LINUX_USB_DWC3_OTG_H */
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 283c0cb..3c6ec7c 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -49,6 +49,7 @@
>  
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
> +#include <linux/usb/otg.h>
>  
>  #include "core.h"
>  #include "gadget.h"
> @@ -1467,12 +1468,64 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on)
>  	is_on = !!is_on;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
> +
> +	dwc->softconnect = is_on;
> +
> +	if ((dwc->dotg && !dwc->vbus_active) ||
> +		!dwc->gadget_driver) {

pullup will never be called without a gadget_driver loaded. If it is,
it's a bug on udc-core which needs to be solved.

> +		spin_unlock_irqrestore(&dwc->lock, flags);
> +
> +		/*
> +		 * Need to wait for vbus_session(on) from otg driver or to
> +		 * the udc_start.
> +		 */
> +		return 0;
> +	}
> +
>  	ret = dwc3_gadget_run_stop(dwc, is_on);
> +
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
>  	return ret;
>  }
>  
> +static int dwc3_gadget_vbus_session(struct usb_gadget *_gadget, int is_active)
> +{
> +	struct dwc3 *dwc = gadget_to_dwc(_gadget);
> +	unsigned long flags;
> +
> +	if (!dwc->dotg)
> +		return -EPERM;
> +
> +	is_active = !!is_active;
> +
> +	spin_lock_irqsave(&dwc->lock, flags);
> +
> +	/* Mark that the vbus was powered */
> +	dwc->vbus_active = is_active;

dwc->vbus_session would be better as it would clearly refer to the
implementation/user.

> +	/*
> +	 * Check if upper level usb_gadget_driver was already registerd with
> +	 * this udc controller driver (if dwc3_gadget_start was called)
> +	 */
> +	if (dwc->gadget_driver && dwc->softconnect) {

you should make sure this only gets called when both conditions are true
to avoid this trickery.

> +		if (dwc->vbus_active) {
> +			/*
> +			 * Both vbus was activated by otg and pullup was
> +			 * signaled by the gadget driver.
> +			 */
> +			dwc3_gadget_run_stop(dwc, 1);
> +		} else {
> +			dwc3_gadget_run_stop(dwc, 0);
> +		}
> +	}
> +
> +	spin_unlock_irqrestore(&dwc->lock, flags);
> +
> +	return 0;
> +}
> +
>  static int dwc3_gadget_start(struct usb_gadget *g,
>  		struct usb_gadget_driver *driver)
>  {
> @@ -1576,6 +1629,7 @@ static const struct usb_gadget_ops dwc3_gadget_ops = {
>  	.get_frame		= dwc3_gadget_get_frame,
>  	.wakeup			= dwc3_gadget_wakeup,
>  	.set_selfpowered	= dwc3_gadget_set_selfpowered,
> +	.vbus_session		= dwc3_gadget_vbus_session,
>  	.pullup			= dwc3_gadget_pullup,
>  	.udc_start		= dwc3_gadget_start,
>  	.udc_stop		= dwc3_gadget_stop,
> @@ -2476,6 +2530,15 @@ int __devinit dwc3_gadget_init(struct dwc3 *dwc)
>  		goto err7;
>  	}
>  
> +	if (dwc->dotg) {
> +		/* dwc3 otg driver is active (DRD mode + SRPSupport=1) */
> +		ret = otg_set_peripheral(&dwc->dotg->otg, &dwc->gadget);

this is wrong. You should add proper devm_usb_get_phy() and use that
indirection to set the default mode as peripheral.

This part of the code shouldn't make assumptions about the underlying
PHY (whether it's built into the same IP, or provided by a separate
entity).

That way, we make sure that the same code is used for platforms which
don't use Synopsys' OTG layer and provide their own implementation.

> +		if (ret) {
> +			dev_err(dwc->dev, "failed to set peripheral to otg\n");
> +			goto err7;
> +		}
> +	}
> +
>  	return 0;
>  
>  err7:
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 689bc18..ef3f095 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -14,9 +14,12 @@
>  #include <linux/platform_device.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/usb/otg.h>
>  
>  #include "xhci.h"
>  
> +static struct usb_phy *phy;

wrong. You can have multiple xhci controllers on a system, so we
shouldn't prevent those systems from working. Even if they're not around
today, it's likely that will happen at some point.

>  static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
>  {
>  	/*
> @@ -149,6 +152,19 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto put_usb3_hcd;
>  
> +	phy = usb_get_phy(USB_PHY_TYPE_USB3);

devm_usb_get_phy()

you also need to get both USB2 and USB3 phys.

> +	if (phy && phy->otg) {
> +		dev_dbg(&pdev->dev, "%s otg support available\n", __func__);
> +		hcd->driver->stop(hcd);
> +		ret = otg_set_host(phy->otg, &hcd->self);
> +		if (ret) {
> +			dev_err(&pdev->dev, "%s otg_set_host failed\n",
> +				__func__);
> +			usb_put_phy(phy);
> +			goto put_usb3_hcd;
> +		}
> +	}
> +
>  	return 0;
>  
>  put_usb3_hcd:
> @@ -182,6 +198,11 @@ static int xhci_plat_remove(struct platform_device *dev)
>  	usb_put_hcd(hcd);
>  	kfree(xhci);
>  
> +	if (phy && phy->otg) {
> +		otg_set_host(phy->otg, NULL);
> +		usb_put_phy(phy);
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index a979cd0..287cecd 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -521,6 +521,13 @@ int xhci_run(struct usb_hcd *hcd)
>  
>  	xhci_dbg(xhci, "xhci_run\n");
>  
> +	xhci_dbg(xhci, "Calling HCD init\n");
> +	/* Initialize HCD and host controller data structures. */
> +	ret = xhci_init(hcd);
> +	if (ret)
> +		return ret;
> +	xhci_dbg(xhci, "Called HCD init\n");
> +
>  	ret = xhci_try_enable_msi(hcd);
>  	if (ret)
>  		return ret;
> @@ -4519,12 +4526,6 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
>  		dma_set_mask(hcd->self.controller, DMA_BIT_MASK(32));
>  	}
>  
> -	xhci_dbg(xhci, "Calling HCD init\n");
> -	/* Initialize HCD and host controller data structures. */
> -	retval = xhci_init(hcd);
> -	if (retval)
> -		goto error;
> -	xhci_dbg(xhci, "Called HCD init\n");

these last two hunks have nothing to do with $SUBJECT. You should split
them to their own patch and Cc Sarah Sharp. What's the reasoning behind
this, btw ?

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux