[v4,2/2] usb/gadget: Add driver for Aspeed SoC virtual hub

[Felipe Balbi <balbi@kernel.org>]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> The Aspeed BMC SoCs support a "virtual hub" function. It provides some
> HW support for a top-level USB2 hub behind which sit 5 gadget "ports".
>
> This driver adds support for the full functionality, emulating the
> hub standard requests and exposing 5 UDC gadget drivers corresponding
> to the ports.
>
> The hub itself has HW provided dedicated EP0 and EP1 (the latter for
> hub interrupts). It also has dedicated EP0s for each function. For
> other endpoints, there's a pool of 15 "generic" endpoints that are
> shared among the ports.
>
> The driver relies on my previous patch adding a "dispose" EP op to
> handle EP allocation between ports. EPs are allocated from the shared
> pool in the UDC "match_ep" callback and assigned to the UDC instance
> (added to the gadget ep_list).
>
> When the composite driver gets unbound, the new hook will allow the UDC
> to clean things up and return those EPs to the shared pool.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> v4. - Fix missing unlock ast_vhub_udc_wakeup() error path
>     - Make "irq" signed to deal with error from platform_get_irq
>     - Fix Makefile for module builds
>     - Fix a missing endian conversion
>
> v3. - Rebased
>     - Add clk stuff
>
> v2. - Cosmetic fixes
>     - Properly "allocate" device addresses instead of using a never
>       reset counter
>     - Move .dtsi updates to a different patch
>
> foo
>
> goo
> ---
>  drivers/usb/gadget/udc/Kconfig              |   2 +
>  drivers/usb/gadget/udc/Makefile             |   1 +
>  drivers/usb/gadget/udc/aspeed-vhub/Kconfig  |   6 +
>  drivers/usb/gadget/udc/aspeed-vhub/Makefile |   3 +
>  drivers/usb/gadget/udc/aspeed-vhub/core.c   | 429 ++++++++++++++
>  drivers/usb/gadget/udc/aspeed-vhub/dev.c    | 580 +++++++++++++++++++
>  drivers/usb/gadget/udc/aspeed-vhub/ep0.c    | 474 ++++++++++++++++
>  drivers/usb/gadget/udc/aspeed-vhub/epn.c    | 840 ++++++++++++++++++++++++++++
>  drivers/usb/gadget/udc/aspeed-vhub/hub.c    | 822 +++++++++++++++++++++++++++
>  drivers/usb/gadget/udc/aspeed-vhub/vhub.h   | 496 ++++++++++++++++
>  10 files changed, 3653 insertions(+)
>  create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/Kconfig
>  create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/Makefile
>  create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/core.c
>  create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/dev.c
>  create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/ep0.c
>  create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/epn.c
>  create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/hub.c
>  create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/vhub.h
>
> diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> index 1e9567091d86..14cf31a2703a 100644
> --- a/drivers/usb/gadget/udc/Kconfig
> +++ b/drivers/usb/gadget/udc/Kconfig
> @@ -439,6 +439,8 @@ config USB_GADGET_XILINX
>  	  dynamically linked module called "udc-xilinx" and force all
>  	  gadget drivers to also be dynamically linked.
>  
> +source "drivers/usb/gadget/udc/aspeed-vhub/Kconfig"
> +
>  #
>  # LAST -- dummy/emulated controller
>  #
> diff --git a/drivers/usb/gadget/udc/Makefile b/drivers/usb/gadget/udc/Makefile
> index ce865b129fd6..897f648f3cf1 100644
> --- a/drivers/usb/gadget/udc/Makefile
> +++ b/drivers/usb/gadget/udc/Makefile
> @@ -39,4 +39,5 @@ obj-$(CONFIG_USB_MV_U3D)	+= mv_u3d_core.o
>  obj-$(CONFIG_USB_GR_UDC)	+= gr_udc.o
>  obj-$(CONFIG_USB_GADGET_XILINX)	+= udc-xilinx.o
>  obj-$(CONFIG_USB_SNP_UDC_PLAT) += snps_udc_plat.o
> +obj-$(CONFIG_USB_ASPEED_VHUB)	+= aspeed-vhub/
>  obj-$(CONFIG_USB_BDC_UDC)	+= bdc/
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/Kconfig b/drivers/usb/gadget/udc/aspeed-vhub/Kconfig
> new file mode 100644
> index 000000000000..cb022c885425
> --- /dev/null
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/Kconfig
> @@ -0,0 +1,6 @@
> +config USB_ASPEED_VHUB
> +	tristate "Aspeed vHub UDC driver"
> +	depends on ARCH_ASPEED || COMPILE_TEST
> +	help
> +	  USB peripheral controller for the Aspeed AST2500 family
> +	  SoCs supporting the "vHub" functionality and USB2.0
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/Makefile b/drivers/usb/gadget/udc/aspeed-vhub/Makefile
> new file mode 100644
> index 000000000000..4256266c0a8a
> --- /dev/null
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/Makefile
> @@ -0,0 +1,3 @@
> +obj-$(CONFIG_USB_ASPEED_VHUB)	+= aspeed-vhub.o
> +aspeed-vhub-y	:= core.o ep0.o epn.o dev.o hub.o
> +
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/core.c b/drivers/usb/gadget/udc/aspeed-vhub/core.c
> new file mode 100644
> index 000000000000..31ed2b6e241b
> --- /dev/null
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/core.c
> @@ -0,0 +1,429 @@

missing SPDX license identifier (all files)

> +static bool force_usb1 = false;
> +static bool no_dma_desc = false;
> +
> +module_param_named(force_usb1, force_usb1, bool, 0644);
> +MODULE_PARM_DESC(force_usb1, "Set to true to force to USB1 speed");
> +module_param_named(no_dma_desc, no_dma_desc, bool, 0644);
> +MODULE_PARM_DESC(no_dma_desc, "Set to true to disable use of DMA descriptors");

module parameters? Sorry, but no.

> +void ast_vhub_done(struct ast_vhub_ep *ep, struct ast_vhub_req *req,
> +		   int status)
> +{
> +	bool internal = req->internal;
> +
> +	EPVDBG(ep, "completing request @%p, status %d\n", req, status);
> +
> +	list_del_init(&req->queue);
> +
> +	if (req->req.status == -EINPROGRESS)
> +		req->req.status = status;
> +
> +	if (req->req.dma) {
> +		if (!WARN_ON(!ep->dev))
> +			usb_gadget_unmap_request(&ep->dev->gadget,
> +						 &req->req, ep->epn.is_in);
> +		req->req.dma = 0;
> +	}
> +
> +	/*
> +	 * If this isn't an internal EP0 request, call the core
> +	 * to call the gadget completion.
> +	 */
> +	if (!internal) {
> +		spin_unlock(&ep->vhub->lock);
> +		usb_gadget_giveback_request(&ep->ep, &req->req);
> +		spin_lock(&ep->vhub->lock);
> +	}
> +}
> +
> +void ast_vhub_nuke(struct ast_vhub_ep *ep, int status)
> +{
> +	struct ast_vhub_req *req;
> +
> +	EPDBG(ep, "Nuking\n");
> +
> +	/* terminate any request in the queue */
> +	if (list_empty(&ep->queue))
> +		return;

unnecessary test.

> +
> +	/* Beware, lock will be dropped & req-acquired by done() */
> +	while (!list_empty(&ep->queue)) {
> +		req = list_first_entry(&ep->queue, struct ast_vhub_req, queue);

list_for_each_entry_safe() ??

> +		ast_vhub_done(ep, req, status);
> +	}
> +}
> +
> +struct usb_request *ast_vhub_alloc_request(struct usb_ep *u_ep,
> +					   gfp_t gfp_flags)
> +{
> +	struct ast_vhub_req *req;
> +
> +	req = kzalloc(sizeof(*req), gfp_flags);
> +	if (!req)
> +		return NULL;

Don't you wanna keep a reference to the endpoint that owns $this
request?

> +	INIT_LIST_HEAD(&req->queue);

unnecessary list initialization

> +	return &req->req;
> +}
> +
> +void ast_vhub_free_request(struct usb_ep *u_ep, struct usb_request *u_req)
> +{
> +	struct ast_vhub_req *req = to_ast_req(u_req);
> +
> +	kfree(req);
> +}
> +
> +static irqreturn_t ast_vhub_irq (int irq, void *data)
                                  ^
                                  unnecessary space

> +{
> +	struct ast_vhub *vhub = data;
> +	irqreturn_t iret = IRQ_NONE;
> +	u32 istat;
> +
> +	/* Stale interrupt while tearing down */
> +	if (!vhub->ep0_bufs)
> +		return IRQ_NONE;
> +
> +	spin_lock(&vhub->lock);

this is your hardirq handler right? Do you really need this lock here?

> +
> +	/* Read and ACK interrupts */
> +	istat = readl(vhub->regs + AST_VHUB_ISR);
> +	if (!istat)
> +		goto bail;
> +	writel(istat, vhub->regs + AST_VHUB_ISR);
> +	iret = IRQ_HANDLED;
> +
> +	UDCVDBG(vhub, "irq status=%08x, ep_acks=%08x ep_nacks=%08x\n",
> +	       istat,
> +	       readl(vhub->regs + AST_VHUB_EP_ACK_ISR),
> +	       readl(vhub->regs + AST_VHUB_EP_NACK_ISR));
> +
> +	/* Handle generic EPs first */
> +	if (istat & VHUB_IRQ_EP_POOL_ACK_STALL) {
> +		u32 i, ep_acks = readl(vhub->regs + AST_VHUB_EP_ACK_ISR);
> +		writel(ep_acks, vhub->regs + AST_VHUB_EP_ACK_ISR);
> +
> +		/* Check if we have a useful ffs on arm ... */
> +		for (i = 0; ep_acks && i < AST_VHUB_NUM_GEN_EPs; i++) {
> +			u32 mask = 1 << i;
> +			if (ep_acks & mask) {
> +				ast_vhub_epn_ack_irq(&vhub->epns[i]);
> +				ep_acks &= ~mask;
> +			}
> +		}
> +	}
> +
> +	/* Handle device interrupts */
> +	if (istat & (VHUB_IRQ_DEVICE1 |
> +		     VHUB_IRQ_DEVICE2 |
> +		     VHUB_IRQ_DEVICE3 |
> +		     VHUB_IRQ_DEVICE4 |
> +		     VHUB_IRQ_DEVICE5)) {
> +		if (istat & VHUB_IRQ_DEVICE1)
> +			ast_vhub_dev_irq(&vhub->ports[0].dev);
> +		if (istat & VHUB_IRQ_DEVICE2)
> +			ast_vhub_dev_irq(&vhub->ports[1].dev);
> +		if (istat & VHUB_IRQ_DEVICE3)
> +			ast_vhub_dev_irq(&vhub->ports[2].dev);
> +		if (istat & VHUB_IRQ_DEVICE4)
> +			ast_vhub_dev_irq(&vhub->ports[3].dev);
> +		if (istat & VHUB_IRQ_DEVICE5)
> +			ast_vhub_dev_irq(&vhub->ports[4].dev);
> +	}
> +
> +	/* Handle top-level vHub EP0 interrupts */
> +	if (istat & (VHUB_IRQ_HUB_EP0_OUT_ACK_STALL |
> +		     VHUB_IRQ_HUB_EP0_IN_ACK_STALL |
> +		     VHUB_IRQ_HUB_EP0_SETUP)) {
> +		if (istat & VHUB_IRQ_HUB_EP0_IN_ACK_STALL)
> +			ast_vhub_ep0_handle_ack(&vhub->ep0, true);
> +		if (istat & VHUB_IRQ_HUB_EP0_OUT_ACK_STALL)
> +			ast_vhub_ep0_handle_ack(&vhub->ep0, false);
> +		if (istat & VHUB_IRQ_HUB_EP0_SETUP)
> +			ast_vhub_ep0_handle_setup(&vhub->ep0);
> +	}
> +
> +	/* Various top level bus events */
> +	if (istat & (VHUB_IRQ_BUS_RESUME |
> +		     VHUB_IRQ_BUS_SUSPEND |
> +		     VHUB_IRQ_BUS_RESET)) {
> +		if (istat & VHUB_IRQ_BUS_RESUME)
> +			ast_vhub_hub_resume(vhub);
> +		if (istat & VHUB_IRQ_BUS_SUSPEND)
> +			ast_vhub_hub_suspend(vhub);
> +		if (istat & VHUB_IRQ_BUS_RESET)
> +			ast_vhub_hub_reset(vhub);
> +	}
> +
> + bail:
> +	spin_unlock(&vhub->lock);
> +	return iret;
> +}
> +
> +void ast_vhub_init_hw(struct ast_vhub *vhub)
> +{
> +	u32 ctrl;
> +
> +	UDCDBG(vhub,"(Re)Starting HW ...\n");
> +
> +	/* Enable PHY */
> +	ctrl = VHUB_CTRL_PHY_CLK |
> +		VHUB_CTRL_PHY_RESET_DIS;
> +
> +#if 0 /*
> +       * This causes registers to become inaccessible during
> +       * suspend. Need to figure out how to bring the controller
> +       * back into life to issue a wakeup.
> +       */
> +	ctrl |= VHUB_CTRL_CLK_STOP_SUSPEND;
> +#endif

no commented code.

> +	/*
> +	 * Set some ISO & split control bits according to Aspeed
> +	 * recommendation
> +	 *
> +	 * VHUB_CTRL_ISO_RSP_CTRL: When set tells the HW to respond
> +	 * with 0 bytes data packet to ISO IN endpoints when no data
> +	 * is available.
> +	 *
> +	 * VHUB_CTRL_SPLIT_IN: This makes a SOF complete a split IN
> +	 * transaction.
> +	 */
> +	ctrl |= VHUB_CTRL_ISO_RSP_CTRL | VHUB_CTRL_SPLIT_IN;
> +	writel(ctrl, vhub->regs + AST_VHUB_CTRL);
> +	udelay(1);
> +
> +	/* Set descriptor ring size */
> +#if AST_VHUB_DESCS_COUNT == 256
> +	ctrl |= VHUB_CTRL_LONG_DESC;
> +	writel(ctrl, vhub->regs + AST_VHUB_CTRL);
> +#elif AST_VHUB_DESCS_COUNT != 32
> +	#error Invalid AST_VHUB_DESCS_COUNT
> +#endif

find a better way for this. No ifdefs

> +
> +	/* Reset all devices */
> +	writel(0x33f, vhub->regs + AST_VHUB_SW_REST);

no magic constants...

> +	udelay(1);
> +	writel(0, vhub->regs + AST_VHUB_SW_REST);
> +
> +	/* Disable and cleanup EP ACK/NACK interrupts */
> +	writel(0, vhub->regs + AST_VHUB_EP_ACK_IER);
> +	writel(0, vhub->regs + AST_VHUB_EP_NACK_IER);
> +	writel(0xffffffff, vhub->regs + AST_VHUB_EP_ACK_ISR);
> +	writel(0xffffffff, vhub->regs + AST_VHUB_EP_NACK_ISR);

... of any kind

> +	/* Default settings for EP0, enable HW hub EP1 */
> +	writel(0, vhub->regs + AST_VHUB_EP0_CTRL);
> +	writel(VHUB_EP1_CTRL_RESET_TOGGLE |
> +	       VHUB_EP1_CTRL_ENABLE,
> +	       vhub->regs + AST_VHUB_EP1_CTRL);
> +	writel(0, vhub->regs + AST_VHUB_EP1_STS_CHG);
> +
> +	/* Configure EP0 DMA buffer */
> +	writel(vhub->ep0.buf_dma, vhub->regs + AST_VHUB_EP0_DATA);
> +
> +	/* Clear address */
> +	writel(0, vhub->regs + AST_VHUB_CONF);
> +
> +	/* Pullup hub (activate on host) */
> +	if (vhub->force_usb1)
> +		ctrl |= VHUB_CTRL_FULL_SPEED_ONLY;
> +	ctrl |= VHUB_CTRL_UPSTREAM_CONNECT;
> +	writel(ctrl, vhub->regs + AST_VHUB_CTRL);
> +
> +	/* Enable some interrupts */
> +	writel(VHUB_IRQ_HUB_EP0_IN_ACK_STALL |
> +	       VHUB_IRQ_HUB_EP0_OUT_ACK_STALL |
> +	       VHUB_IRQ_HUB_EP0_SETUP |
> +	       VHUB_IRQ_EP_POOL_ACK_STALL |
> +	       VHUB_IRQ_BUS_RESUME |
> +	       VHUB_IRQ_BUS_SUSPEND |
> +	       VHUB_IRQ_BUS_RESET,
> +	       vhub->regs + AST_VHUB_IER);
> +}
> +
> +static int ast_vhub_remove(struct platform_device *pdev)
> +{
> +	struct ast_vhub *vhub = platform_get_drvdata(pdev);
> +	unsigned long flags;
> +	int i;
> +
> +	if (!vhub || !vhub->regs)
> +		return 0;
> +
> +	/* Remove devices */
> +	for (i = 0; i < AST_VHUB_NUM_PORTS; i++)
> +		ast_vhub_del_dev(&vhub->ports[i].dev);
> +
> +	spin_lock_irqsave(&vhub->lock, flags);
> +
> +	/* Mask & ack all interrupts  */
> +	writel(0, vhub->regs + AST_VHUB_IER);
> +	writel(VHUB_IRQ_ACK_ALL, vhub->regs + AST_VHUB_ISR);
> +
> +	/* Pull device, leave PHY enabled */
> +	writel(VHUB_CTRL_PHY_CLK |
> +	       VHUB_CTRL_PHY_RESET_DIS,
> +	       vhub->regs + AST_VHUB_CTRL);
> +
> +	if (vhub->clk)
> +		clk_disable_unprepare(vhub->clk);
> +
> +	spin_unlock_irqrestore(&vhub->lock, flags);
> +
> +	if (vhub->ep0_bufs)
> +		dma_free_coherent(&pdev->dev,
> +				  AST_VHUB_EP0_MAX_PACKET *
> +				  (AST_VHUB_NUM_PORTS + 1),
> +				  vhub->ep0_bufs,
> +				  vhub->ep0_bufs_dma);
> +	vhub->ep0_bufs = NULL;
> +
> +	return 0;
> +}
> +
> +static int ast_vhub_probe(struct platform_device *pdev)
> +{
> +	struct ast_vhub *vhub;
> +	struct resource *res;
> +	int i, rc = 0;
> +
> +	vhub = devm_kzalloc(&pdev->dev, sizeof(*vhub), GFP_KERNEL);
> +	if (!vhub)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&vhub->lock);
> +	vhub->pdev = pdev;
> +	vhub->force_usb1 = force_usb1;
> +	vhub->no_dma_desc = no_dma_desc;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	vhub->regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(vhub->regs)) {
> +		dev_err(&pdev->dev, "Failed to map resources\n");
> +		return PTR_ERR(vhub->regs);
> +	}
> +	UDCDBG(vhub, "vHub@%pR mapped @%p\n", res, vhub->regs);
> +
> +	platform_set_drvdata(pdev, vhub);
> +
> +	vhub->clk = devm_clk_get(&pdev->dev, NULL);
> +        if (IS_ERR(vhub->clk)) {
> +                rc = PTR_ERR(vhub->clk);
> +                goto err;
> +        }
> +	rc = clk_prepare_enable(vhub->clk);
> +	if (rc) {
> +		dev_err(&pdev->dev, "Error couldn't enable clock (%d)\n", rc);
> +		goto err;
> +        }
> +
> +	/* Mask & ack all interrupts before installing the handler */
> +	writel(0, vhub->regs + AST_VHUB_IER);
> +	writel(VHUB_IRQ_ACK_ALL, vhub->regs + AST_VHUB_ISR);
> +
> +	/* Find interrupt and install handler */
> +	vhub->irq = platform_get_irq(pdev, 0);
> +	if (vhub->irq < 0) {
> +		dev_err(&pdev->dev, "Failed to get interrupt\n");
> +		rc = vhub->irq;
> +		goto err;
> +	}
> +	rc = devm_request_irq(&pdev->dev, vhub->irq, ast_vhub_irq, 0,
> +			      KBUILD_MODNAME, vhub);

we usually rely on the device name, but okay.