> -----Original Message-----
> From: Linux-aspeed
> <linux-aspeed-bounces+ryan_chen=aspeedtech.com@xxxxxxxxxxxxxxxx> On
> Behalf Of Jeremy Kerr
> Sent: Wednesday, June 12, 2024 10:08 AM
> To: Neal Liu <neal_liu@xxxxxxxxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>; Joel Stanley <joel@xxxxxxxxx>; Andrew Jeffery
> <andrew@xxxxxxxxxxxxxxxxxxxx>
> Cc: linux-usb@xxxxxxxxxxxxxxx; linux-aspeed@xxxxxxxxxxxxxxxx
> Subject: [PATCH] usb: gadget: aspeed_udc: fix device address configuration
>
> In the aspeed UDC setup, we configure the UDC hardware with the assigned
> USB device address.
>
> However, we have an off-by-one in the bitmask, so we're only setting the
> lower 6 bits of the address (USB addresses being 7 bits, and the hardware
> bitmask being bits 0:6).
>
> This means that device enumeration fails if the assigned address is greater
> than 64:
>
> [ 344.607255] usb 1-1: new high-speed USB device number 63 using
> ehci-platform [ 344.808459] usb 1-1: New USB device found, idVendor=cc00,
> idProduct=cc00, bcdDevice= 6.10 [ 344.817684] usb 1-1: New USB device
> strings: Mfr=1, Product=2, SerialNumber=3 [ 344.825671] usb 1-1: Product:
> Test device [ 344.831075] usb 1-1: Manufacturer: Test vendor
> [ 344.836335] usb 1-1: SerialNumber: 00 [ 349.917181] usb 1-1: USB
> disconnect, device number 63 [ 352.036775] usb 1-1: new high-speed USB
> device number 64 using ehci-platform [ 352.249432] usb 1-1: device
> descriptor read/all, error -71 [ 352.696740] usb 1-1: new high-speed USB
> device number 65 using ehci-platform [ 352.909431] usb 1-1: device
> descriptor read/all, error -71
>
> Use the correct mask of 0x7f (rather than 0x3f), and generate this through the
> GENMASK macro, so we have numbers that correspond exactly to the
> hardware register definition.
>
> Fixes: 055276c13205 ("usb: gadget: add Aspeed ast2600 udc driver")
> Signed-off-by: Jeremy Kerr <jk@xxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/usb/gadget/udc/aspeed_udc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/aspeed_udc.c
> b/drivers/usb/gadget/udc/aspeed_udc.c
> index 3916c8e2ba01..821a6ab5da56 100644
> --- a/drivers/usb/gadget/udc/aspeed_udc.c
> +++ b/drivers/usb/gadget/udc/aspeed_udc.c
> @@ -66,8 +66,8 @@
> #define USB_UPSTREAM_EN BIT(0)
>
> /* Main config reg */
> -#define UDC_CFG_SET_ADDR(x) ((x) & 0x3f)
> -#define UDC_CFG_ADDR_MASK (0x3f)
> +#define UDC_CFG_SET_ADDR(x) ((x) & UDC_CFG_ADDR_MASK)
> +#define UDC_CFG_ADDR_MASK GENMASK(6, 0)
>
It should be GENMASK(5,0), not GENMASK(6, 0), am I right?
Ryan
> /* Interrupt ctrl & status reg */
> #define UDC_IRQ_EP_POOL_NAK BIT(17)
>
> ---
> base-commit: 32f88d65f01bf6f45476d7edbe675e44fb9e1d58
> change-id: 20240611-aspeed-udc-07dcde062ccf
>
> Best regards,
> --
> Jeremy Kerr <jk@xxxxxxxxxxxxxxxxxxxx>
