[linux,dev-5.4,v2,1/3] usb: gadget: aspeed: read vhub config from of_device_id
diff mbox series

Message ID 20200123074956.21482-2-rentao.bupt@gmail.com
State Superseded, archived
Headers show
Series
  • aspeed-g6: enable usb support
Related show

Commit Message

Tao Ren Jan. 23, 2020, 7:49 a.m. UTC
From: Tao Ren <rentao.bupt@gmail.com>

The patch moves hardcoded vhub attributes (maximum downstream ports and
generic endpoints) to "ast_vhub_config" structure which is attached to
struct of_device_id. By doing this, it will be very straightforward to
enable support of AST2600 vhub which has different number of downstream
ports and endpoints.

Signed-off-by: Tao Ren <rentao.bupt@gmail.com>
---
 Changes in v2:
  - this is the first version. It's given v2 to align with the version
    of patch series.


 drivers/usb/gadget/udc/aspeed-vhub/core.c | 100 ++++++++++++++--------
 drivers/usb/gadget/udc/aspeed-vhub/dev.c  |  30 +++++--
 drivers/usb/gadget/udc/aspeed-vhub/epn.c  |   4 +-
 drivers/usb/gadget/udc/aspeed-vhub/hub.c  |  22 +++--
 drivers/usb/gadget/udc/aspeed-vhub/vhub.h |  21 ++---
 5 files changed, 107 insertions(+), 70 deletions(-)

Comments

Andrew Jeffery Jan. 24, 2020, 12:09 a.m. UTC | #1
On Thu, 23 Jan 2020, at 18:19, rentao.bupt@gmail.com wrote:
> From: Tao Ren <rentao.bupt@gmail.com>
> 
> The patch moves hardcoded vhub attributes (maximum downstream ports and
> generic endpoints) to "ast_vhub_config" structure which is attached to
> struct of_device_id. By doing this, it will be very straightforward to
> enable support of AST2600 vhub which has different number of downstream
> ports and endpoints.
> 
> Signed-off-by: Tao Ren <rentao.bupt@gmail.com>
> ---
>  Changes in v2:
>   - this is the first version. It's given v2 to align with the version
>     of patch series.
> 
> 
>  drivers/usb/gadget/udc/aspeed-vhub/core.c | 100 ++++++++++++++--------
>  drivers/usb/gadget/udc/aspeed-vhub/dev.c  |  30 +++++--
>  drivers/usb/gadget/udc/aspeed-vhub/epn.c  |   4 +-
>  drivers/usb/gadget/udc/aspeed-vhub/hub.c  |  22 +++--
>  drivers/usb/gadget/udc/aspeed-vhub/vhub.h |  21 ++---
>  5 files changed, 107 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/core.c 
> b/drivers/usb/gadget/udc/aspeed-vhub/core.c
> index 90b134d5dca9..9efbfdffad30 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/core.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/core.c
> @@ -32,6 +32,29 @@
>  
>  #include "vhub.h"
>  
> +struct ast_vhub_config {
> +	u32 max_ports;	/* max number of downstream ports */
> +	u32 max_epns;	/* max number of generic endpoints */
> +};
> +
> +static const struct ast_vhub_config ast2400_config = {
> +	.max_ports = 5,
> +	.max_epns = 15,
> +};
> +
> +static const struct of_device_id ast_vhub_dt_ids[] = {
> +	{
> +		.compatible = "aspeed,ast2400-usb-vhub",
> +		.data = &ast2400_config,
> +	},
> +	{
> +		.compatible = "aspeed,ast2500-usb-vhub",
> +		.data = &ast2400_config,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ast_vhub_dt_ids);
> +

Is there a reason to move the table from where it was? It's not a problem,
just makes the diff a bit noisier.

>  void ast_vhub_done(struct ast_vhub_ep *ep, struct ast_vhub_req *req,
>  		   int status)
>  {
> @@ -99,7 +122,7 @@ static irqreturn_t ast_vhub_irq(int irq, void *data)
>  {
>  	struct ast_vhub *vhub = data;
>  	irqreturn_t iret = IRQ_NONE;
> -	u32 istat;
> +	u32 i, istat;
>  
>  	/* Stale interrupt while tearing down */
>  	if (!vhub->ep0_bufs)
> @@ -121,10 +144,10 @@ static irqreturn_t ast_vhub_irq(int irq, void *data)
>  
>  	/* Handle generic EPs first */
>  	if (istat & VHUB_IRQ_EP_POOL_ACK_STALL) {
> -		u32 i, ep_acks = readl(vhub->regs + AST_VHUB_EP_ACK_ISR);
> +		u32 ep_acks = readl(vhub->regs + AST_VHUB_EP_ACK_ISR);
>  		writel(ep_acks, vhub->regs + AST_VHUB_EP_ACK_ISR);
>  
> -		for (i = 0; ep_acks && i < AST_VHUB_NUM_GEN_EPs; i++) {
> +		for (i = 0; ep_acks && i < vhub->max_epns; i++) {
>  			u32 mask = VHUB_EP_IRQ(i);
>  			if (ep_acks & mask) {
>  				ast_vhub_epn_ack_irq(&vhub->epns[i]);
> @@ -134,21 +157,11 @@ static irqreturn_t ast_vhub_irq(int irq, void *data)
>  	}
>  
>  	/* 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);
> +	for (i = 0; i < vhub->max_ports; i++) {
> +		u32 dev_mask = VHUB_IRQ_DEVICE1 << i;
> +
> +		if (istat & dev_mask)
> +			ast_vhub_dev_irq(&vhub->ports[i].dev);
>  	}
>  
>  	/* Handle top-level vHub EP0 interrupts */
> @@ -182,7 +195,7 @@ static irqreturn_t ast_vhub_irq(int irq, void *data)
>  
>  void ast_vhub_init_hw(struct ast_vhub *vhub)
>  {
> -	u32 ctrl;
> +	u32 ctrl, port_mask, epn_mask;
>  
>  	UDCDBG(vhub,"(Re)Starting HW ...\n");
>  
> @@ -222,15 +235,20 @@ void ast_vhub_init_hw(struct ast_vhub *vhub)
>  	}
>  
>  	/* Reset all devices */
> -	writel(VHUB_SW_RESET_ALL, vhub->regs + AST_VHUB_SW_RESET);
> +	port_mask = GENMASK(vhub->max_ports, 1);
> +	writel(VHUB_SW_RESET_ROOT_HUB |
> +	       VHUB_SW_RESET_DMA_CONTROLLER |
> +	       VHUB_SW_RESET_EP_POOL |
> +	       port_mask, vhub->regs + AST_VHUB_SW_RESET);
>  	udelay(1);
>  	writel(0, vhub->regs + AST_VHUB_SW_RESET);
>  
>  	/* Disable and cleanup EP ACK/NACK interrupts */
> +	epn_mask = GENMASK(vhub->max_epns - 1, 0);
>  	writel(0, vhub->regs + AST_VHUB_EP_ACK_IER);
>  	writel(0, vhub->regs + AST_VHUB_EP_NACK_IER);
> -	writel(VHUB_EP_IRQ_ALL, vhub->regs + AST_VHUB_EP_ACK_ISR);
> -	writel(VHUB_EP_IRQ_ALL, vhub->regs + AST_VHUB_EP_NACK_ISR);
> +	writel(epn_mask, vhub->regs + AST_VHUB_EP_ACK_ISR);
> +	writel(epn_mask, vhub->regs + AST_VHUB_EP_NACK_ISR);
>  
>  	/* Default settings for EP0, enable HW hub EP1 */
>  	writel(0, vhub->regs + AST_VHUB_EP0_CTRL);
> @@ -273,7 +291,7 @@ static int ast_vhub_remove(struct platform_device *pdev)
>  		return 0;
>  
>  	/* Remove devices */
> -	for (i = 0; i < AST_VHUB_NUM_PORTS; i++)
> +	for (i = 0; i < vhub->max_ports; i++)
>  		ast_vhub_del_dev(&vhub->ports[i].dev);
>  
>  	spin_lock_irqsave(&vhub->lock, flags);
> @@ -295,7 +313,7 @@ static int ast_vhub_remove(struct platform_device *pdev)
>  	if (vhub->ep0_bufs)
>  		dma_free_coherent(&pdev->dev,
>  				  AST_VHUB_EP0_MAX_PACKET *
> -				  (AST_VHUB_NUM_PORTS + 1),
> +				  (vhub->max_ports + 1),
>  				  vhub->ep0_bufs,
>  				  vhub->ep0_bufs_dma);
>  	vhub->ep0_bufs = NULL;
> @@ -309,11 +327,30 @@ static int ast_vhub_probe(struct platform_device *pdev)
>  	struct ast_vhub *vhub;
>  	struct resource *res;
>  	int i, rc = 0;
> +	const struct of_device_id *ofdid;
> +	struct ast_vhub_config *config;
>  
>  	vhub = devm_kzalloc(&pdev->dev, sizeof(*vhub), GFP_KERNEL);
>  	if (!vhub)
>  		return -ENOMEM;
>  
> +	ofdid = of_match_node(ast_vhub_dt_ids, pdev->dev.of_node);
> +	if (!ofdid)
> +		return -EINVAL;
> +	config = ofdid->data;
> +
> +	vhub->max_ports = config->max_ports;
> +	vhub->ports = devm_kcalloc(&pdev->dev, vhub->max_ports,
> +				   sizeof(*vhub->ports), GFP_KERNEL);
> +	if (!vhub->ports)
> +		return -ENOMEM;
> +
> +	vhub->max_epns = config->max_epns;
> +	vhub->epns = devm_kcalloc(&pdev->dev, vhub->max_epns,
> +				  sizeof(*vhub->epns), GFP_KERNEL);
> +	if (!vhub->epns)
> +		return -ENOMEM;
> +
>  	spin_lock_init(&vhub->lock);
>  	vhub->pdev = pdev;
>  
> @@ -366,7 +403,7 @@ static int ast_vhub_probe(struct platform_device *pdev)
>  	 */
>  	vhub->ep0_bufs = dma_alloc_coherent(&pdev->dev,
>  					    AST_VHUB_EP0_MAX_PACKET *
> -					    (AST_VHUB_NUM_PORTS + 1),
> +					    (vhub->max_ports + 1),
>  					    &vhub->ep0_bufs_dma, GFP_KERNEL);
>  	if (!vhub->ep0_bufs) {
>  		dev_err(&pdev->dev, "Failed to allocate EP0 DMA buffers\n");
> @@ -380,7 +417,7 @@ static int ast_vhub_probe(struct platform_device *pdev)
>  	ast_vhub_init_ep0(vhub, &vhub->ep0, NULL);
>  
>  	/* Init devices */
> -	for (i = 0; i < AST_VHUB_NUM_PORTS && rc == 0; i++)
> +	for (i = 0; i < vhub->max_ports && rc == 0; i++)
>  		rc = ast_vhub_init_dev(vhub, i);
>  	if (rc)
>  		goto err;
> @@ -400,17 +437,6 @@ static int ast_vhub_probe(struct platform_device *pdev)
>  	return rc;
>  }
>  
> -static const struct of_device_id ast_vhub_dt_ids[] = {
> -	{
> -		.compatible = "aspeed,ast2400-usb-vhub",
> -	},
> -	{
> -		.compatible = "aspeed,ast2500-usb-vhub",
> -	},
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(of, ast_vhub_dt_ids);
> -
>  static struct platform_driver ast_vhub_driver = {
>  	.probe		= ast_vhub_probe,
>  	.remove		= ast_vhub_remove,
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/dev.c 
> b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> index 4008e7a51188..f9b762951121 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> @@ -77,7 +77,7 @@ static void ast_vhub_dev_enable(struct ast_vhub_dev 
> *d)
>  	writel(d->ep0.buf_dma, d->regs + AST_VHUB_DEV_EP0_DATA);
>  
>  	/* Clear stall on all EPs */
> -	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++) {
> +	for (i = 0; i < d->max_epns; i++) {
>  		struct ast_vhub_ep *ep = d->epns[i];
>  
>  		if (ep && (ep->epn.stalled || ep->epn.wedged)) {
> @@ -137,7 +137,7 @@ static int ast_vhub_ep_feature(struct ast_vhub_dev *d,
>  	     is_set ? "SET" : "CLEAR", ep_num, wValue);
>  	if (ep_num == 0)
>  		return std_req_complete;
> -	if (ep_num >= AST_VHUB_NUM_GEN_EPs || !d->epns[ep_num - 1])
> +	if (ep_num >= d->max_epns || !d->epns[ep_num - 1])
>  		return std_req_stall;
>  	if (wValue != USB_ENDPOINT_HALT)
>  		return std_req_driver;
> @@ -181,7 +181,7 @@ static int ast_vhub_ep_status(struct ast_vhub_dev *d,
>  
>  	DDBG(d, "GET_STATUS(ep%d)\n", ep_num);
>  
> -	if (ep_num >= AST_VHUB_NUM_GEN_EPs)
> +	if (ep_num >= d->max_epns)
>  		return std_req_stall;
>  	if (ep_num != 0) {
>  		ep = d->epns[ep_num - 1];
> @@ -299,7 +299,7 @@ static void ast_vhub_dev_nuke(struct ast_vhub_dev *d)
>  {
>  	unsigned int i;
>  
> -	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++) {
> +	for (i = 0; i < d->max_epns; i++) {
>  		if (!d->epns[i])
>  			continue;
>  		ast_vhub_nuke(d->epns[i], -ESHUTDOWN);
> @@ -416,10 +416,10 @@ static struct usb_ep 
> *ast_vhub_udc_match_ep(struct usb_gadget *gadget,
>  	 * that will allow the generic code to use our
>  	 * assigned address.
>  	 */
> -	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++)
> +	for (i = 0; i < d->max_epns; i++)
>  		if (d->epns[i] == NULL)
>  			break;
> -	if (i >= AST_VHUB_NUM_GEN_EPs)
> +	if (i >= d->max_epns)
>  		return NULL;
>  	addr = i + 1;
>  
> @@ -526,6 +526,7 @@ void ast_vhub_del_dev(struct ast_vhub_dev *d)
>  
>  	usb_del_gadget_udc(&d->gadget);
>  	device_unregister(d->port_dev);
> +	kfree(d->epns);
>  }
>  
>  static void ast_vhub_dev_release(struct device *dev)
> @@ -546,14 +547,25 @@ int ast_vhub_init_dev(struct ast_vhub *vhub, 
> unsigned int idx)
>  
>  	ast_vhub_init_ep0(vhub, &d->ep0, d);
>  
> +	/*
> +	 * A USB device can have up to 30 endpoints besides control
> +	 * endpoint 0.
> +	 */
> +	d->max_epns = min(vhub->max_epns, 30);
> +	d->epns = kcalloc(d->max_epns, sizeof(*d->epns), GFP_KERNEL);
> +	if (!d->epns)
> +		return -ENOMEM;
> +
>  	/*
>  	 * The UDC core really needs us to have separate and uniquely
>  	 * named "parent" devices for each port so we create a sub device
>  	 * here for that purpose
>  	 */
>  	d->port_dev = kzalloc(sizeof(struct device), GFP_KERNEL);
> -	if (!d->port_dev)
> -		return -ENOMEM;
> +	if (!d->port_dev) {
> +		rc = -ENOMEM;
> +		goto fail_alloc;
> +	}
>  	device_initialize(d->port_dev);
>  	d->port_dev->release = ast_vhub_dev_release;
>  	d->port_dev->parent = parent;
> @@ -584,6 +596,8 @@ int ast_vhub_init_dev(struct ast_vhub *vhub, 
> unsigned int idx)
>  	device_del(d->port_dev);
>   fail_add:
>  	put_device(d->port_dev);
> + fail_alloc:
> +	kfree(d->epns);
>  
>  	return rc;
>  }
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c 
> b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> index 7475c74aa5c5..0bd6b20435b8 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> @@ -800,10 +800,10 @@ struct ast_vhub_ep *ast_vhub_alloc_epn(struct 
> ast_vhub_dev *d, u8 addr)
>  
>  	/* Find a free one (no device) */
>  	spin_lock_irqsave(&vhub->lock, flags);
> -	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++)
> +	for (i = 0; i < vhub->max_epns; i++)
>  		if (vhub->epns[i].dev == NULL)
>  			break;
> -	if (i >= AST_VHUB_NUM_GEN_EPs) {
> +	if (i >= vhub->max_epns) {
>  		spin_unlock_irqrestore(&vhub->lock, flags);
>  		return NULL;
>  	}
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c 
> b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> index 19b3517e04c0..aa1c127e9f2f 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> @@ -133,10 +133,9 @@ static const struct ast_vhub_full_cdesc {
>  
>  #define AST_VHUB_HUB_DESC_SIZE	(USB_DT_HUB_NONVAR_SIZE + 2)
>  
> -static const struct usb_hub_descriptor ast_vhub_hub_desc = {
> +static struct usb_hub_descriptor ast_vhub_hub_desc = {

This seems unfortunate, though we only have one on the SoC... is
it worth dynamically allocating it? Or adding a comment?

Andrew
Tao Ren Jan. 24, 2020, 1:23 a.m. UTC | #2
On Fri, Jan 24, 2020 at 10:39:45AM +1030, Andrew Jeffery wrote:
> 
> 
> On Thu, 23 Jan 2020, at 18:19, rentao.bupt@gmail.com wrote:
> > From: Tao Ren <rentao.bupt@gmail.com>
> > 
> > The patch moves hardcoded vhub attributes (maximum downstream ports and
> > generic endpoints) to "ast_vhub_config" structure which is attached to
> > struct of_device_id. By doing this, it will be very straightforward to
> > enable support of AST2600 vhub which has different number of downstream
> > ports and endpoints.
> > 
> > Signed-off-by: Tao Ren <rentao.bupt@gmail.com>
> > ---
> >  Changes in v2:
> >   - this is the first version. It's given v2 to align with the version
> >     of patch series.
> > 
> > 
> >  drivers/usb/gadget/udc/aspeed-vhub/core.c | 100 ++++++++++++++--------
> >  drivers/usb/gadget/udc/aspeed-vhub/dev.c  |  30 +++++--
> >  drivers/usb/gadget/udc/aspeed-vhub/epn.c  |   4 +-
> >  drivers/usb/gadget/udc/aspeed-vhub/hub.c  |  22 +++--
> >  drivers/usb/gadget/udc/aspeed-vhub/vhub.h |  21 ++---
> >  5 files changed, 107 insertions(+), 70 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/udc/aspeed-vhub/core.c 
> > b/drivers/usb/gadget/udc/aspeed-vhub/core.c
> > index 90b134d5dca9..9efbfdffad30 100644
> > --- a/drivers/usb/gadget/udc/aspeed-vhub/core.c
> > +++ b/drivers/usb/gadget/udc/aspeed-vhub/core.c
> > @@ -32,6 +32,29 @@
> >  
> >  #include "vhub.h"
> >  
> > +struct ast_vhub_config {
> > +	u32 max_ports;	/* max number of downstream ports */
> > +	u32 max_epns;	/* max number of generic endpoints */
> > +};
> > +
> > +static const struct ast_vhub_config ast2400_config = {
> > +	.max_ports = 5,
> > +	.max_epns = 15,
> > +};
> > +
> > +static const struct of_device_id ast_vhub_dt_ids[] = {
> > +	{
> > +		.compatible = "aspeed,ast2400-usb-vhub",
> > +		.data = &ast2400_config,
> > +	},
> > +	{
> > +		.compatible = "aspeed,ast2500-usb-vhub",
> > +		.data = &ast2400_config,
> > +	},
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, ast_vhub_dt_ids);
> > +
> 
> Is there a reason to move the table from where it was? It's not a problem,
> just makes the diff a bit noisier.

I move the table before ast_vhub_probe() because it is referenced in the
function.

> 
> >  void ast_vhub_done(struct ast_vhub_ep *ep, struct ast_vhub_req *req,
> >  		   int status)
> >  {
> > @@ -99,7 +122,7 @@ static irqreturn_t ast_vhub_irq(int irq, void *data)
> >  {
> >  	struct ast_vhub *vhub = data;
> >  	irqreturn_t iret = IRQ_NONE;
> > -	u32 istat;
> > +	u32 i, istat;
> >  
> >  	/* Stale interrupt while tearing down */
> >  	if (!vhub->ep0_bufs)
> > @@ -121,10 +144,10 @@ static irqreturn_t ast_vhub_irq(int irq, void *data)
> >  
> >  	/* Handle generic EPs first */
> >  	if (istat & VHUB_IRQ_EP_POOL_ACK_STALL) {
> > -		u32 i, ep_acks = readl(vhub->regs + AST_VHUB_EP_ACK_ISR);
> > +		u32 ep_acks = readl(vhub->regs + AST_VHUB_EP_ACK_ISR);
> >  		writel(ep_acks, vhub->regs + AST_VHUB_EP_ACK_ISR);
> >  
> > -		for (i = 0; ep_acks && i < AST_VHUB_NUM_GEN_EPs; i++) {
> > +		for (i = 0; ep_acks && i < vhub->max_epns; i++) {
> >  			u32 mask = VHUB_EP_IRQ(i);
> >  			if (ep_acks & mask) {
> >  				ast_vhub_epn_ack_irq(&vhub->epns[i]);
> > @@ -134,21 +157,11 @@ static irqreturn_t ast_vhub_irq(int irq, void *data)
> >  	}
> >  
> >  	/* 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);
> > +	for (i = 0; i < vhub->max_ports; i++) {
> > +		u32 dev_mask = VHUB_IRQ_DEVICE1 << i;
> > +
> > +		if (istat & dev_mask)
> > +			ast_vhub_dev_irq(&vhub->ports[i].dev);
> >  	}
> >  
> >  	/* Handle top-level vHub EP0 interrupts */
> > @@ -182,7 +195,7 @@ static irqreturn_t ast_vhub_irq(int irq, void *data)
> >  
> >  void ast_vhub_init_hw(struct ast_vhub *vhub)
> >  {
> > -	u32 ctrl;
> > +	u32 ctrl, port_mask, epn_mask;
> >  
> >  	UDCDBG(vhub,"(Re)Starting HW ...\n");
> >  
> > @@ -222,15 +235,20 @@ void ast_vhub_init_hw(struct ast_vhub *vhub)
> >  	}
> >  
> >  	/* Reset all devices */
> > -	writel(VHUB_SW_RESET_ALL, vhub->regs + AST_VHUB_SW_RESET);
> > +	port_mask = GENMASK(vhub->max_ports, 1);
> > +	writel(VHUB_SW_RESET_ROOT_HUB |
> > +	       VHUB_SW_RESET_DMA_CONTROLLER |
> > +	       VHUB_SW_RESET_EP_POOL |
> > +	       port_mask, vhub->regs + AST_VHUB_SW_RESET);
> >  	udelay(1);
> >  	writel(0, vhub->regs + AST_VHUB_SW_RESET);
> >  
> >  	/* Disable and cleanup EP ACK/NACK interrupts */
> > +	epn_mask = GENMASK(vhub->max_epns - 1, 0);
> >  	writel(0, vhub->regs + AST_VHUB_EP_ACK_IER);
> >  	writel(0, vhub->regs + AST_VHUB_EP_NACK_IER);
> > -	writel(VHUB_EP_IRQ_ALL, vhub->regs + AST_VHUB_EP_ACK_ISR);
> > -	writel(VHUB_EP_IRQ_ALL, vhub->regs + AST_VHUB_EP_NACK_ISR);
> > +	writel(epn_mask, vhub->regs + AST_VHUB_EP_ACK_ISR);
> > +	writel(epn_mask, vhub->regs + AST_VHUB_EP_NACK_ISR);
> >  
> >  	/* Default settings for EP0, enable HW hub EP1 */
> >  	writel(0, vhub->regs + AST_VHUB_EP0_CTRL);
> > @@ -273,7 +291,7 @@ static int ast_vhub_remove(struct platform_device *pdev)
> >  		return 0;
> >  
> >  	/* Remove devices */
> > -	for (i = 0; i < AST_VHUB_NUM_PORTS; i++)
> > +	for (i = 0; i < vhub->max_ports; i++)
> >  		ast_vhub_del_dev(&vhub->ports[i].dev);
> >  
> >  	spin_lock_irqsave(&vhub->lock, flags);
> > @@ -295,7 +313,7 @@ static int ast_vhub_remove(struct platform_device *pdev)
> >  	if (vhub->ep0_bufs)
> >  		dma_free_coherent(&pdev->dev,
> >  				  AST_VHUB_EP0_MAX_PACKET *
> > -				  (AST_VHUB_NUM_PORTS + 1),
> > +				  (vhub->max_ports + 1),
> >  				  vhub->ep0_bufs,
> >  				  vhub->ep0_bufs_dma);
> >  	vhub->ep0_bufs = NULL;
> > @@ -309,11 +327,30 @@ static int ast_vhub_probe(struct platform_device *pdev)
> >  	struct ast_vhub *vhub;
> >  	struct resource *res;
> >  	int i, rc = 0;
> > +	const struct of_device_id *ofdid;
> > +	struct ast_vhub_config *config;
> >  
> >  	vhub = devm_kzalloc(&pdev->dev, sizeof(*vhub), GFP_KERNEL);
> >  	if (!vhub)
> >  		return -ENOMEM;
> >  
> > +	ofdid = of_match_node(ast_vhub_dt_ids, pdev->dev.of_node);
> > +	if (!ofdid)
> > +		return -EINVAL;
> > +	config = ofdid->data;
> > +
> > +	vhub->max_ports = config->max_ports;
> > +	vhub->ports = devm_kcalloc(&pdev->dev, vhub->max_ports,
> > +				   sizeof(*vhub->ports), GFP_KERNEL);
> > +	if (!vhub->ports)
> > +		return -ENOMEM;
> > +
> > +	vhub->max_epns = config->max_epns;
> > +	vhub->epns = devm_kcalloc(&pdev->dev, vhub->max_epns,
> > +				  sizeof(*vhub->epns), GFP_KERNEL);
> > +	if (!vhub->epns)
> > +		return -ENOMEM;
> > +
> >  	spin_lock_init(&vhub->lock);
> >  	vhub->pdev = pdev;
> >  
> > @@ -366,7 +403,7 @@ static int ast_vhub_probe(struct platform_device *pdev)
> >  	 */
> >  	vhub->ep0_bufs = dma_alloc_coherent(&pdev->dev,
> >  					    AST_VHUB_EP0_MAX_PACKET *
> > -					    (AST_VHUB_NUM_PORTS + 1),
> > +					    (vhub->max_ports + 1),
> >  					    &vhub->ep0_bufs_dma, GFP_KERNEL);
> >  	if (!vhub->ep0_bufs) {
> >  		dev_err(&pdev->dev, "Failed to allocate EP0 DMA buffers\n");
> > @@ -380,7 +417,7 @@ static int ast_vhub_probe(struct platform_device *pdev)
> >  	ast_vhub_init_ep0(vhub, &vhub->ep0, NULL);
> >  
> >  	/* Init devices */
> > -	for (i = 0; i < AST_VHUB_NUM_PORTS && rc == 0; i++)
> > +	for (i = 0; i < vhub->max_ports && rc == 0; i++)
> >  		rc = ast_vhub_init_dev(vhub, i);
> >  	if (rc)
> >  		goto err;
> > @@ -400,17 +437,6 @@ static int ast_vhub_probe(struct platform_device *pdev)
> >  	return rc;
> >  }
> >  
> > -static const struct of_device_id ast_vhub_dt_ids[] = {
> > -	{
> > -		.compatible = "aspeed,ast2400-usb-vhub",
> > -	},
> > -	{
> > -		.compatible = "aspeed,ast2500-usb-vhub",
> > -	},
> > -	{ }
> > -};
> > -MODULE_DEVICE_TABLE(of, ast_vhub_dt_ids);
> > -
> >  static struct platform_driver ast_vhub_driver = {
> >  	.probe		= ast_vhub_probe,
> >  	.remove		= ast_vhub_remove,
> > diff --git a/drivers/usb/gadget/udc/aspeed-vhub/dev.c 
> > b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> > index 4008e7a51188..f9b762951121 100644
> > --- a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> > +++ b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> > @@ -77,7 +77,7 @@ static void ast_vhub_dev_enable(struct ast_vhub_dev 
> > *d)
> >  	writel(d->ep0.buf_dma, d->regs + AST_VHUB_DEV_EP0_DATA);
> >  
> >  	/* Clear stall on all EPs */
> > -	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++) {
> > +	for (i = 0; i < d->max_epns; i++) {
> >  		struct ast_vhub_ep *ep = d->epns[i];
> >  
> >  		if (ep && (ep->epn.stalled || ep->epn.wedged)) {
> > @@ -137,7 +137,7 @@ static int ast_vhub_ep_feature(struct ast_vhub_dev *d,
> >  	     is_set ? "SET" : "CLEAR", ep_num, wValue);
> >  	if (ep_num == 0)
> >  		return std_req_complete;
> > -	if (ep_num >= AST_VHUB_NUM_GEN_EPs || !d->epns[ep_num - 1])
> > +	if (ep_num >= d->max_epns || !d->epns[ep_num - 1])
> >  		return std_req_stall;
> >  	if (wValue != USB_ENDPOINT_HALT)
> >  		return std_req_driver;
> > @@ -181,7 +181,7 @@ static int ast_vhub_ep_status(struct ast_vhub_dev *d,
> >  
> >  	DDBG(d, "GET_STATUS(ep%d)\n", ep_num);
> >  
> > -	if (ep_num >= AST_VHUB_NUM_GEN_EPs)
> > +	if (ep_num >= d->max_epns)
> >  		return std_req_stall;
> >  	if (ep_num != 0) {
> >  		ep = d->epns[ep_num - 1];
> > @@ -299,7 +299,7 @@ static void ast_vhub_dev_nuke(struct ast_vhub_dev *d)
> >  {
> >  	unsigned int i;
> >  
> > -	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++) {
> > +	for (i = 0; i < d->max_epns; i++) {
> >  		if (!d->epns[i])
> >  			continue;
> >  		ast_vhub_nuke(d->epns[i], -ESHUTDOWN);
> > @@ -416,10 +416,10 @@ static struct usb_ep 
> > *ast_vhub_udc_match_ep(struct usb_gadget *gadget,
> >  	 * that will allow the generic code to use our
> >  	 * assigned address.
> >  	 */
> > -	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++)
> > +	for (i = 0; i < d->max_epns; i++)
> >  		if (d->epns[i] == NULL)
> >  			break;
> > -	if (i >= AST_VHUB_NUM_GEN_EPs)
> > +	if (i >= d->max_epns)
> >  		return NULL;
> >  	addr = i + 1;
> >  
> > @@ -526,6 +526,7 @@ void ast_vhub_del_dev(struct ast_vhub_dev *d)
> >  
> >  	usb_del_gadget_udc(&d->gadget);
> >  	device_unregister(d->port_dev);
> > +	kfree(d->epns);
> >  }
> >  
> >  static void ast_vhub_dev_release(struct device *dev)
> > @@ -546,14 +547,25 @@ int ast_vhub_init_dev(struct ast_vhub *vhub, 
> > unsigned int idx)
> >  
> >  	ast_vhub_init_ep0(vhub, &d->ep0, d);
> >  
> > +	/*
> > +	 * A USB device can have up to 30 endpoints besides control
> > +	 * endpoint 0.
> > +	 */
> > +	d->max_epns = min(vhub->max_epns, 30);
> > +	d->epns = kcalloc(d->max_epns, sizeof(*d->epns), GFP_KERNEL);
> > +	if (!d->epns)
> > +		return -ENOMEM;
> > +
> >  	/*
> >  	 * The UDC core really needs us to have separate and uniquely
> >  	 * named "parent" devices for each port so we create a sub device
> >  	 * here for that purpose
> >  	 */
> >  	d->port_dev = kzalloc(sizeof(struct device), GFP_KERNEL);
> > -	if (!d->port_dev)
> > -		return -ENOMEM;
> > +	if (!d->port_dev) {
> > +		rc = -ENOMEM;
> > +		goto fail_alloc;
> > +	}
> >  	device_initialize(d->port_dev);
> >  	d->port_dev->release = ast_vhub_dev_release;
> >  	d->port_dev->parent = parent;
> > @@ -584,6 +596,8 @@ int ast_vhub_init_dev(struct ast_vhub *vhub, 
> > unsigned int idx)
> >  	device_del(d->port_dev);
> >   fail_add:
> >  	put_device(d->port_dev);
> > + fail_alloc:
> > +	kfree(d->epns);
> >  
> >  	return rc;
> >  }
> > diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c 
> > b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> > index 7475c74aa5c5..0bd6b20435b8 100644
> > --- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> > +++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> > @@ -800,10 +800,10 @@ struct ast_vhub_ep *ast_vhub_alloc_epn(struct 
> > ast_vhub_dev *d, u8 addr)
> >  
> >  	/* Find a free one (no device) */
> >  	spin_lock_irqsave(&vhub->lock, flags);
> > -	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++)
> > +	for (i = 0; i < vhub->max_epns; i++)
> >  		if (vhub->epns[i].dev == NULL)
> >  			break;
> > -	if (i >= AST_VHUB_NUM_GEN_EPs) {
> > +	if (i >= vhub->max_epns) {
> >  		spin_unlock_irqrestore(&vhub->lock, flags);
> >  		return NULL;
> >  	}
> > diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c 
> > b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > index 19b3517e04c0..aa1c127e9f2f 100644
> > --- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > +++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > @@ -133,10 +133,9 @@ static const struct ast_vhub_full_cdesc {
> >  
> >  #define AST_VHUB_HUB_DESC_SIZE	(USB_DT_HUB_NONVAR_SIZE + 2)
> >  
> > -static const struct usb_hub_descriptor ast_vhub_hub_desc = {
> > +static struct usb_hub_descriptor ast_vhub_hub_desc = {
> 
> This seems unfortunate, though we only have one on the SoC... is
> it worth dynamically allocating it? Or adding a comment?
> 
> Andrew

According to the comment at the beginning of the file (line 39-47), we
may customize more descriptors in the future. Adding comments involves
little change in this case, because by allocating the descriptors, we
will also need a function to free the descriptors when ast_vhub_remove
is called. Anyways I'm fine with either way.

There is another option which is to fixup descriptors in request_desc
callback, like ast_vhub_patch_dev_desc_usb1() in the file. But I don't
like the approach personally.

Which way do you prefer?


Cheers,

Tao
Andrew Jeffery Jan. 28, 2020, 12:57 a.m. UTC | #3
On Fri, 24 Jan 2020, at 11:53, Tao Ren wrote:
> On Fri, Jan 24, 2020 at 10:39:45AM +1030, Andrew Jeffery wrote:
> > > diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c 
> > > b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > > index 19b3517e04c0..aa1c127e9f2f 100644
> > > --- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > > +++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > > @@ -133,10 +133,9 @@ static const struct ast_vhub_full_cdesc {
> > >  
> > >  #define AST_VHUB_HUB_DESC_SIZE	(USB_DT_HUB_NONVAR_SIZE + 2)
> > >  
> > > -static const struct usb_hub_descriptor ast_vhub_hub_desc = {
> > > +static struct usb_hub_descriptor ast_vhub_hub_desc = {
> > 
> > This seems unfortunate, though we only have one on the SoC... is
> > it worth dynamically allocating it? Or adding a comment?
> > 
> > Andrew
> 
> According to the comment at the beginning of the file (line 39-47), we
> may customize more descriptors in the future. Adding comments involves
> little change in this case, because by allocating the descriptors, we
> will also need a function to free the descriptors when ast_vhub_remove
> is called. Anyways I'm fine with either way.
> 
> There is another option which is to fixup descriptors in request_desc
> callback, like ast_vhub_patch_dev_desc_usb1() in the file. But I don't
> like the approach personally.
> 
> Which way do you prefer?

I'm not wedded to doing anything different from what you've already got
in the patch - we don't have hardware that requires a different solution at
this point. We can always fix the driver if that changes. The approach just
felt a bit icky, but I can live with that :)

Andrew
Tao Ren Jan. 28, 2020, 3:13 a.m. UTC | #4
On Tue, Jan 28, 2020 at 11:27:57AM +1030, Andrew Jeffery wrote:
> 
> 
> On Fri, 24 Jan 2020, at 11:53, Tao Ren wrote:
> > On Fri, Jan 24, 2020 at 10:39:45AM +1030, Andrew Jeffery wrote:
> > > > diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c 
> > > > b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > > > index 19b3517e04c0..aa1c127e9f2f 100644
> > > > --- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > > > +++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > > > @@ -133,10 +133,9 @@ static const struct ast_vhub_full_cdesc {
> > > >  
> > > >  #define AST_VHUB_HUB_DESC_SIZE	(USB_DT_HUB_NONVAR_SIZE + 2)
> > > >  
> > > > -static const struct usb_hub_descriptor ast_vhub_hub_desc = {
> > > > +static struct usb_hub_descriptor ast_vhub_hub_desc = {
> > > 
> > > This seems unfortunate, though we only have one on the SoC... is
> > > it worth dynamically allocating it? Or adding a comment?
> > > 
> > > Andrew
> > 
> > According to the comment at the beginning of the file (line 39-47), we
> > may customize more descriptors in the future. Adding comments involves
> > little change in this case, because by allocating the descriptors, we
> > will also need a function to free the descriptors when ast_vhub_remove
> > is called. Anyways I'm fine with either way.
> > 
> > There is another option which is to fixup descriptors in request_desc
> > callback, like ast_vhub_patch_dev_desc_usb1() in the file. But I don't
> > like the approach personally.
> > 
> > Which way do you prefer?
> 
> I'm not wedded to doing anything different from what you've already got
> in the patch - we don't have hardware that requires a different solution at
> this point. We can always fix the driver if that changes. The approach just
> felt a bit icky, but I can live with that :)
> 
> Andrew

Thanks Andrew. I just sent out patch v3 which adds some comments for the
hub descriptor; 2 compile warnings are also fixed in v3. Please kindly
review when you have bandwidth.

Cheers,

Tao

Patch
diff mbox series

diff --git a/drivers/usb/gadget/udc/aspeed-vhub/core.c b/drivers/usb/gadget/udc/aspeed-vhub/core.c
index 90b134d5dca9..9efbfdffad30 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/core.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/core.c
@@ -32,6 +32,29 @@ 
 
 #include "vhub.h"
 
+struct ast_vhub_config {
+	u32 max_ports;	/* max number of downstream ports */
+	u32 max_epns;	/* max number of generic endpoints */
+};
+
+static const struct ast_vhub_config ast2400_config = {
+	.max_ports = 5,
+	.max_epns = 15,
+};
+
+static const struct of_device_id ast_vhub_dt_ids[] = {
+	{
+		.compatible = "aspeed,ast2400-usb-vhub",
+		.data = &ast2400_config,
+	},
+	{
+		.compatible = "aspeed,ast2500-usb-vhub",
+		.data = &ast2400_config,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ast_vhub_dt_ids);
+
 void ast_vhub_done(struct ast_vhub_ep *ep, struct ast_vhub_req *req,
 		   int status)
 {
@@ -99,7 +122,7 @@  static irqreturn_t ast_vhub_irq(int irq, void *data)
 {
 	struct ast_vhub *vhub = data;
 	irqreturn_t iret = IRQ_NONE;
-	u32 istat;
+	u32 i, istat;
 
 	/* Stale interrupt while tearing down */
 	if (!vhub->ep0_bufs)
@@ -121,10 +144,10 @@  static irqreturn_t ast_vhub_irq(int irq, void *data)
 
 	/* Handle generic EPs first */
 	if (istat & VHUB_IRQ_EP_POOL_ACK_STALL) {
-		u32 i, ep_acks = readl(vhub->regs + AST_VHUB_EP_ACK_ISR);
+		u32 ep_acks = readl(vhub->regs + AST_VHUB_EP_ACK_ISR);
 		writel(ep_acks, vhub->regs + AST_VHUB_EP_ACK_ISR);
 
-		for (i = 0; ep_acks && i < AST_VHUB_NUM_GEN_EPs; i++) {
+		for (i = 0; ep_acks && i < vhub->max_epns; i++) {
 			u32 mask = VHUB_EP_IRQ(i);
 			if (ep_acks & mask) {
 				ast_vhub_epn_ack_irq(&vhub->epns[i]);
@@ -134,21 +157,11 @@  static irqreturn_t ast_vhub_irq(int irq, void *data)
 	}
 
 	/* 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);
+	for (i = 0; i < vhub->max_ports; i++) {
+		u32 dev_mask = VHUB_IRQ_DEVICE1 << i;
+
+		if (istat & dev_mask)
+			ast_vhub_dev_irq(&vhub->ports[i].dev);
 	}
 
 	/* Handle top-level vHub EP0 interrupts */
@@ -182,7 +195,7 @@  static irqreturn_t ast_vhub_irq(int irq, void *data)
 
 void ast_vhub_init_hw(struct ast_vhub *vhub)
 {
-	u32 ctrl;
+	u32 ctrl, port_mask, epn_mask;
 
 	UDCDBG(vhub,"(Re)Starting HW ...\n");
 
@@ -222,15 +235,20 @@  void ast_vhub_init_hw(struct ast_vhub *vhub)
 	}
 
 	/* Reset all devices */
-	writel(VHUB_SW_RESET_ALL, vhub->regs + AST_VHUB_SW_RESET);
+	port_mask = GENMASK(vhub->max_ports, 1);
+	writel(VHUB_SW_RESET_ROOT_HUB |
+	       VHUB_SW_RESET_DMA_CONTROLLER |
+	       VHUB_SW_RESET_EP_POOL |
+	       port_mask, vhub->regs + AST_VHUB_SW_RESET);
 	udelay(1);
 	writel(0, vhub->regs + AST_VHUB_SW_RESET);
 
 	/* Disable and cleanup EP ACK/NACK interrupts */
+	epn_mask = GENMASK(vhub->max_epns - 1, 0);
 	writel(0, vhub->regs + AST_VHUB_EP_ACK_IER);
 	writel(0, vhub->regs + AST_VHUB_EP_NACK_IER);
-	writel(VHUB_EP_IRQ_ALL, vhub->regs + AST_VHUB_EP_ACK_ISR);
-	writel(VHUB_EP_IRQ_ALL, vhub->regs + AST_VHUB_EP_NACK_ISR);
+	writel(epn_mask, vhub->regs + AST_VHUB_EP_ACK_ISR);
+	writel(epn_mask, vhub->regs + AST_VHUB_EP_NACK_ISR);
 
 	/* Default settings for EP0, enable HW hub EP1 */
 	writel(0, vhub->regs + AST_VHUB_EP0_CTRL);
@@ -273,7 +291,7 @@  static int ast_vhub_remove(struct platform_device *pdev)
 		return 0;
 
 	/* Remove devices */
-	for (i = 0; i < AST_VHUB_NUM_PORTS; i++)
+	for (i = 0; i < vhub->max_ports; i++)
 		ast_vhub_del_dev(&vhub->ports[i].dev);
 
 	spin_lock_irqsave(&vhub->lock, flags);
@@ -295,7 +313,7 @@  static int ast_vhub_remove(struct platform_device *pdev)
 	if (vhub->ep0_bufs)
 		dma_free_coherent(&pdev->dev,
 				  AST_VHUB_EP0_MAX_PACKET *
-				  (AST_VHUB_NUM_PORTS + 1),
+				  (vhub->max_ports + 1),
 				  vhub->ep0_bufs,
 				  vhub->ep0_bufs_dma);
 	vhub->ep0_bufs = NULL;
@@ -309,11 +327,30 @@  static int ast_vhub_probe(struct platform_device *pdev)
 	struct ast_vhub *vhub;
 	struct resource *res;
 	int i, rc = 0;
+	const struct of_device_id *ofdid;
+	struct ast_vhub_config *config;
 
 	vhub = devm_kzalloc(&pdev->dev, sizeof(*vhub), GFP_KERNEL);
 	if (!vhub)
 		return -ENOMEM;
 
+	ofdid = of_match_node(ast_vhub_dt_ids, pdev->dev.of_node);
+	if (!ofdid)
+		return -EINVAL;
+	config = ofdid->data;
+
+	vhub->max_ports = config->max_ports;
+	vhub->ports = devm_kcalloc(&pdev->dev, vhub->max_ports,
+				   sizeof(*vhub->ports), GFP_KERNEL);
+	if (!vhub->ports)
+		return -ENOMEM;
+
+	vhub->max_epns = config->max_epns;
+	vhub->epns = devm_kcalloc(&pdev->dev, vhub->max_epns,
+				  sizeof(*vhub->epns), GFP_KERNEL);
+	if (!vhub->epns)
+		return -ENOMEM;
+
 	spin_lock_init(&vhub->lock);
 	vhub->pdev = pdev;
 
@@ -366,7 +403,7 @@  static int ast_vhub_probe(struct platform_device *pdev)
 	 */
 	vhub->ep0_bufs = dma_alloc_coherent(&pdev->dev,
 					    AST_VHUB_EP0_MAX_PACKET *
-					    (AST_VHUB_NUM_PORTS + 1),
+					    (vhub->max_ports + 1),
 					    &vhub->ep0_bufs_dma, GFP_KERNEL);
 	if (!vhub->ep0_bufs) {
 		dev_err(&pdev->dev, "Failed to allocate EP0 DMA buffers\n");
@@ -380,7 +417,7 @@  static int ast_vhub_probe(struct platform_device *pdev)
 	ast_vhub_init_ep0(vhub, &vhub->ep0, NULL);
 
 	/* Init devices */
-	for (i = 0; i < AST_VHUB_NUM_PORTS && rc == 0; i++)
+	for (i = 0; i < vhub->max_ports && rc == 0; i++)
 		rc = ast_vhub_init_dev(vhub, i);
 	if (rc)
 		goto err;
@@ -400,17 +437,6 @@  static int ast_vhub_probe(struct platform_device *pdev)
 	return rc;
 }
 
-static const struct of_device_id ast_vhub_dt_ids[] = {
-	{
-		.compatible = "aspeed,ast2400-usb-vhub",
-	},
-	{
-		.compatible = "aspeed,ast2500-usb-vhub",
-	},
-	{ }
-};
-MODULE_DEVICE_TABLE(of, ast_vhub_dt_ids);
-
 static struct platform_driver ast_vhub_driver = {
 	.probe		= ast_vhub_probe,
 	.remove		= ast_vhub_remove,
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/dev.c b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
index 4008e7a51188..f9b762951121 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
@@ -77,7 +77,7 @@  static void ast_vhub_dev_enable(struct ast_vhub_dev *d)
 	writel(d->ep0.buf_dma, d->regs + AST_VHUB_DEV_EP0_DATA);
 
 	/* Clear stall on all EPs */
-	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++) {
+	for (i = 0; i < d->max_epns; i++) {
 		struct ast_vhub_ep *ep = d->epns[i];
 
 		if (ep && (ep->epn.stalled || ep->epn.wedged)) {
@@ -137,7 +137,7 @@  static int ast_vhub_ep_feature(struct ast_vhub_dev *d,
 	     is_set ? "SET" : "CLEAR", ep_num, wValue);
 	if (ep_num == 0)
 		return std_req_complete;
-	if (ep_num >= AST_VHUB_NUM_GEN_EPs || !d->epns[ep_num - 1])
+	if (ep_num >= d->max_epns || !d->epns[ep_num - 1])
 		return std_req_stall;
 	if (wValue != USB_ENDPOINT_HALT)
 		return std_req_driver;
@@ -181,7 +181,7 @@  static int ast_vhub_ep_status(struct ast_vhub_dev *d,
 
 	DDBG(d, "GET_STATUS(ep%d)\n", ep_num);
 
-	if (ep_num >= AST_VHUB_NUM_GEN_EPs)
+	if (ep_num >= d->max_epns)
 		return std_req_stall;
 	if (ep_num != 0) {
 		ep = d->epns[ep_num - 1];
@@ -299,7 +299,7 @@  static void ast_vhub_dev_nuke(struct ast_vhub_dev *d)
 {
 	unsigned int i;
 
-	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++) {
+	for (i = 0; i < d->max_epns; i++) {
 		if (!d->epns[i])
 			continue;
 		ast_vhub_nuke(d->epns[i], -ESHUTDOWN);
@@ -416,10 +416,10 @@  static struct usb_ep *ast_vhub_udc_match_ep(struct usb_gadget *gadget,
 	 * that will allow the generic code to use our
 	 * assigned address.
 	 */
-	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++)
+	for (i = 0; i < d->max_epns; i++)
 		if (d->epns[i] == NULL)
 			break;
-	if (i >= AST_VHUB_NUM_GEN_EPs)
+	if (i >= d->max_epns)
 		return NULL;
 	addr = i + 1;
 
@@ -526,6 +526,7 @@  void ast_vhub_del_dev(struct ast_vhub_dev *d)
 
 	usb_del_gadget_udc(&d->gadget);
 	device_unregister(d->port_dev);
+	kfree(d->epns);
 }
 
 static void ast_vhub_dev_release(struct device *dev)
@@ -546,14 +547,25 @@  int ast_vhub_init_dev(struct ast_vhub *vhub, unsigned int idx)
 
 	ast_vhub_init_ep0(vhub, &d->ep0, d);
 
+	/*
+	 * A USB device can have up to 30 endpoints besides control
+	 * endpoint 0.
+	 */
+	d->max_epns = min(vhub->max_epns, 30);
+	d->epns = kcalloc(d->max_epns, sizeof(*d->epns), GFP_KERNEL);
+	if (!d->epns)
+		return -ENOMEM;
+
 	/*
 	 * The UDC core really needs us to have separate and uniquely
 	 * named "parent" devices for each port so we create a sub device
 	 * here for that purpose
 	 */
 	d->port_dev = kzalloc(sizeof(struct device), GFP_KERNEL);
-	if (!d->port_dev)
-		return -ENOMEM;
+	if (!d->port_dev) {
+		rc = -ENOMEM;
+		goto fail_alloc;
+	}
 	device_initialize(d->port_dev);
 	d->port_dev->release = ast_vhub_dev_release;
 	d->port_dev->parent = parent;
@@ -584,6 +596,8 @@  int ast_vhub_init_dev(struct ast_vhub *vhub, unsigned int idx)
 	device_del(d->port_dev);
  fail_add:
 	put_device(d->port_dev);
+ fail_alloc:
+	kfree(d->epns);
 
 	return rc;
 }
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
index 7475c74aa5c5..0bd6b20435b8 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
@@ -800,10 +800,10 @@  struct ast_vhub_ep *ast_vhub_alloc_epn(struct ast_vhub_dev *d, u8 addr)
 
 	/* Find a free one (no device) */
 	spin_lock_irqsave(&vhub->lock, flags);
-	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++)
+	for (i = 0; i < vhub->max_epns; i++)
 		if (vhub->epns[i].dev == NULL)
 			break;
-	if (i >= AST_VHUB_NUM_GEN_EPs) {
+	if (i >= vhub->max_epns) {
 		spin_unlock_irqrestore(&vhub->lock, flags);
 		return NULL;
 	}
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
index 19b3517e04c0..aa1c127e9f2f 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
@@ -133,10 +133,9 @@  static const struct ast_vhub_full_cdesc {
 
 #define AST_VHUB_HUB_DESC_SIZE	(USB_DT_HUB_NONVAR_SIZE + 2)
 
-static const struct usb_hub_descriptor ast_vhub_hub_desc = {
+static struct usb_hub_descriptor ast_vhub_hub_desc = {
 	.bDescLength			= AST_VHUB_HUB_DESC_SIZE,
 	.bDescriptorType		= USB_DT_HUB,
-	.bNbrPorts			= AST_VHUB_NUM_PORTS,
 	.wHubCharacteristics		= cpu_to_le16(HUB_CHAR_NO_LPSM),
 	.bPwrOn2PwrGood			= 10,
 	.bHubContrCurrent		= 0,
@@ -504,7 +503,7 @@  static void ast_vhub_wake_work(struct work_struct *work)
 	 * we let the normal host wake path deal with it later.
 	 */
 	spin_lock_irqsave(&vhub->lock, flags);
-	for (i = 0; i < AST_VHUB_NUM_PORTS; i++) {
+	for (i = 0; i < vhub->max_ports; i++) {
 		struct ast_vhub_port *p = &vhub->ports[i];
 
 		if (!(p->status & USB_PORT_STAT_SUSPEND))
@@ -587,7 +586,7 @@  static enum std_req_rc ast_vhub_set_port_feature(struct ast_vhub_ep *ep,
 	struct ast_vhub *vhub = ep->vhub;
 	struct ast_vhub_port *p;
 
-	if (port == 0 || port > AST_VHUB_NUM_PORTS)
+	if (port == 0 || port > vhub->max_ports)
 		return std_req_stall;
 	port--;
 	p = &vhub->ports[port];
@@ -630,7 +629,7 @@  static enum std_req_rc ast_vhub_clr_port_feature(struct ast_vhub_ep *ep,
 	struct ast_vhub *vhub = ep->vhub;
 	struct ast_vhub_port *p;
 
-	if (port == 0 || port > AST_VHUB_NUM_PORTS)
+	if (port == 0 || port > vhub->max_ports)
 		return std_req_stall;
 	port--;
 	p = &vhub->ports[port];
@@ -676,7 +675,7 @@  static enum std_req_rc ast_vhub_get_port_stat(struct ast_vhub_ep *ep,
 	struct ast_vhub *vhub = ep->vhub;
 	u16 stat, chg;
 
-	if (port == 0 || port > AST_VHUB_NUM_PORTS)
+	if (port == 0 || port > vhub->max_ports)
 		return std_req_stall;
 	port--;
 
@@ -757,7 +756,7 @@  void ast_vhub_hub_suspend(struct ast_vhub *vhub)
 	 * Forward to unsuspended ports without changing
 	 * their connection status.
 	 */
-	for (i = 0; i < AST_VHUB_NUM_PORTS; i++) {
+	for (i = 0; i < vhub->max_ports; i++) {
 		struct ast_vhub_port *p = &vhub->ports[i];
 
 		if (!(p->status & USB_PORT_STAT_SUSPEND))
@@ -780,7 +779,7 @@  void ast_vhub_hub_resume(struct ast_vhub *vhub)
 	 * Forward to unsuspended ports without changing
 	 * their connection status.
 	 */
-	for (i = 0; i < AST_VHUB_NUM_PORTS; i++) {
+	for (i = 0; i < vhub->max_ports; i++) {
 		struct ast_vhub_port *p = &vhub->ports[i];
 
 		if (!(p->status & USB_PORT_STAT_SUSPEND))
@@ -814,7 +813,7 @@  void ast_vhub_hub_reset(struct ast_vhub *vhub)
 	 * Clear all port status, disable gadgets and "suspend"
 	 * them. They will be woken up by a port reset.
 	 */
-	for (i = 0; i < AST_VHUB_NUM_PORTS; i++) {
+	for (i = 0; i < vhub->max_ports; i++) {
 		struct ast_vhub_port *p = &vhub->ports[i];
 
 		/* Only keep the connected flag */
@@ -838,5 +837,10 @@  void ast_vhub_init_hub(struct ast_vhub *vhub)
 {
 	vhub->speed = USB_SPEED_UNKNOWN;
 	INIT_WORK(&vhub->wake_work, ast_vhub_wake_work);
+
+	/*
+	 * Fixup number of ports in hub descriptor.
+	 */
+	ast_vhub_hub_desc.bNbrPorts = vhub->max_ports;
 }
 
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
index 761919e220d3..11414548cd32 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
+++ b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
@@ -76,17 +76,9 @@ 
 #define VHUB_SW_RESET_DEVICE2			(1 << 2)
 #define VHUB_SW_RESET_DEVICE1			(1 << 1)
 #define VHUB_SW_RESET_ROOT_HUB			(1 << 0)
-#define VHUB_SW_RESET_ALL			(VHUB_SW_RESET_EP_POOL | \
-						 VHUB_SW_RESET_DMA_CONTROLLER | \
-						 VHUB_SW_RESET_DEVICE5 | \
-						 VHUB_SW_RESET_DEVICE4 | \
-						 VHUB_SW_RESET_DEVICE3 | \
-						 VHUB_SW_RESET_DEVICE2 | \
-						 VHUB_SW_RESET_DEVICE1 | \
-						 VHUB_SW_RESET_ROOT_HUB)
+
 /* EP ACK/NACK IRQ masks */
 #define VHUB_EP_IRQ(n)				(1 << (n))
-#define VHUB_EP_IRQ_ALL				0x7fff	/* 15 EPs */
 
 /* USB status reg */
 #define VHUB_USBSTS_HISPEED			(1 << 27)
@@ -210,8 +202,6 @@ 
  *                                      *
  ****************************************/
 
-#define AST_VHUB_NUM_GEN_EPs	15	/* Generic non-0 EPs */
-#define AST_VHUB_NUM_PORTS	5	/* vHub ports */
 #define AST_VHUB_EP0_MAX_PACKET	64	/* EP0's max packet size */
 #define AST_VHUB_EPn_MAX_PACKET	1024	/* Generic EPs max packet size */
 #define AST_VHUB_DESCS_COUNT	256	/* Use 256 descriptor mode (valid
@@ -358,7 +348,8 @@  struct ast_vhub_dev {
 
 	/* Endpoint structures */
 	struct ast_vhub_ep		ep0;
-	struct ast_vhub_ep		*epns[AST_VHUB_NUM_GEN_EPs];
+	struct ast_vhub_ep		**epns;
+	u32				max_epns;
 
 };
 #define to_ast_dev(__g) container_of(__g, struct ast_vhub_dev, gadget)
@@ -393,10 +384,12 @@  struct ast_vhub {
 	bool				ep1_stalled : 1;
 
 	/* Per-port info */
-	struct ast_vhub_port		ports[AST_VHUB_NUM_PORTS];
+	struct ast_vhub_port		*ports;
+	u32				max_ports;
 
 	/* Generic EP data structures */
-	struct ast_vhub_ep		epns[AST_VHUB_NUM_GEN_EPs];
+	struct ast_vhub_ep		*epns;
+	u32				max_epns;
 
 	/* Upstream bus is suspended ? */
 	bool				suspended : 1;