Re: [PATCH linux dev-5.4 1/2] usb: gadget: aspeed: add ast2600 vhub support

[Andrew Jeffery]

On Fri, 17 Jan 2020, at 09:55, rentao.bupt at gmail.com wrote:
> From: Tao Ren <rentao.bupt at gmail.com>
> 
> Add AST2600 support in aspeed-vhub driver.
> 
> There are 3 major differences between AST2500 and AST2600 vhub:
>   - AST2600 supports 7 downstream devices while AST2500 supports 5.
>   - AST2600 supports 21 programmable endpoints while AST2500 supports 15.
>   - EP0 data buffer's 8-byte DMA alignment restriction is removed from
>     AST2600.
> 
> Signed-off-by: Tao Ren <rentao.bupt at gmail.com>
> ---
>  drivers/usb/gadget/udc/aspeed-vhub/Kconfig |  4 +--
>  drivers/usb/gadget/udc/aspeed-vhub/core.c  | 25 ++++++-----------
>  drivers/usb/gadget/udc/aspeed-vhub/vhub.h  | 32 ++++++++++++++++------
>  3 files changed, 35 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/Kconfig 
> b/drivers/usb/gadget/udc/aspeed-vhub/Kconfig
> index 83ba8a2eb6af..605500b19cf3 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/Kconfig
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/Kconfig
> @@ -4,5 +4,5 @@ config USB_ASPEED_VHUB
>  	depends on ARCH_ASPEED || COMPILE_TEST
>  	depends on USB_LIBCOMPOSITE
>  	help
> -	  USB peripheral controller for the Aspeed AST2500 family
> -	  SoCs supporting the "vHub" functionality and USB2.0
> +	  USB peripheral controller for the Aspeed AST2400, AST2500 and
> +	  AST2600 family SoCs supporting the "vHub" functionality and USB2.0
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/core.c 
> b/drivers/usb/gadget/udc/aspeed-vhub/core.c
> index 90b134d5dca9..5fafe91d3619 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/core.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/core.c
> @@ -97,6 +97,7 @@ void ast_vhub_free_request(struct usb_ep *u_ep, 
> struct usb_request *u_req)
>  
>  static irqreturn_t ast_vhub_irq(int irq, void *data)
>  {
> +	u32 i;
>  	struct ast_vhub *vhub = data;
>  	irqreturn_t iret = IRQ_NONE;
>  	u32 istat;
> @@ -121,7 +122,7 @@ 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++) {
> @@ -134,21 +135,10 @@ 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 < AST_VHUB_NUM_PORTS; i++) {
> +		u32 dev_irq = VHUB_IRQ_DEVICE1 << i;
> +		if (istat & dev_irq)
> +			ast_vhub_dev_irq(&vhub->ports[i].dev);
>  	}
>  
>  	/* Handle top-level vHub EP0 interrupts */
> @@ -407,6 +397,9 @@ static const struct of_device_id ast_vhub_dt_ids[] 
> = {
>  	{
>  		.compatible = "aspeed,ast2500-usb-vhub",
>  	},
> +	{
> +		.compatible = "aspeed,ast2600-usb-vhub",
> +	},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, ast_vhub_dt_ids);
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h 
> b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
> index 761919e220d3..76935d02decf 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
> @@ -2,6 +2,23 @@
>  #ifndef __ASPEED_VHUB_H
>  #define __ASPEED_VHUB_H
>  
> +/*****************************
> + *                           *
> + * Maximum devices/endpoints *
> + *                           *
> + *****************************/
> +
> +#ifdef CONFIG_MACH_ASPEED_G6

No, this prevents building a kernel supporting multiple AST generations. Please
describe the differences in a platform data struct attached to struct of_device_id.

Also, what's the plan for upstreaming these changes? It's okay to send them for
inclusion in the openbmc tree if you're wanting them to bake there for some
more widespread testing, but it's not clear what the intent is.

Cheers,

Andrew