[U-Boot] [PATCH v4 1/4] USB: gadget: Add the cadence USB3 gadget driver

[Sherry Sun]

Hi Marek,

> 
> On 8/19/19 8:10 AM, Sherry Sun wrote:
> > This driver is ported from NXP i.MX U-Boot version imx_v2019.04 and
> > some changes have also been made to adapt to U-Boot.
> >
> > Add the Cadence USB3 IP(CDNS3) driver for the gadget (device mode).
> > The CDNS3 gadget driver support DM mode. CONFIG_DM_USB_GADGET
> should
> > be enabled when use this driver.
> >
> > Signed-off-by: Sherry Sun <sherry.sun at nxp.com>
> 
> [...]
> 
> > +static int cdns_req_ep0_set_configuration(struct usb_ss_dev *usb_ss,
> > +					  struct usb_ctrlrequest *ctrl_req) {
> > +	enum usb_device_state device_state = usb_ss->gadget.state;
> > +	u32 config = le16_to_cpu(ctrl_req->wValue);
> > +	struct usb_ep *ep;
> > +	struct usb_ss_endpoint *usb_ss_ep, *temp_ss_ep;
> > +	int i, result = 0;
> > +
> > +	switch (device_state) {
> > +	case USB_STATE_ADDRESS:
> > +		/* Configure non-control EPs */
> > +		list_for_each_entry_safe(usb_ss_ep, temp_ss_ep,
> > +					 &usb_ss->ep_match_list,
> > +					 ep_match_pending_list) {
> > +			cdns_ep_config(usb_ss_ep);
> > +			list_del(&usb_ss_ep->ep_match_pending_list);
> > +		}
> > +
> > +		list_for_each_entry(ep, &usb_ss->gadget.ep_list, ep_list) {
> > +			usb_ss_ep = to_usb_ss_ep(ep);
> > +			if (usb_ss_ep->used)
> > +				cdns_ep_config(usb_ss_ep);
> > +		}
> > +
> > +#ifdef CDNS_THREADED_IRQ_HANDLING
> > +		usb_ss->ep_ien = cdns_readl(&usb_ss->regs->ep_ien)
> > +			| EP_IEN__EOUTEN0__MASK | EP_IEN__EINEN0__MASK; #endif
> 
> This is probably not needed ?
> 

Yes, you are right, I have removed those unneeded code.

> [...]
> 
> > +/* Common TRB fields */
> > +#define TRB_SET_CYCLE_BIT		1uL
> > +#define TRB_SET_CHAIN_BIT		0x10
> > +
> > +/* offset 0 */
> > +#define TRB_DATA_BUFFER_POINTER_MASK	0xFFFFFFFF
> > +#define TRB_SET_DATA_BUFFER_POINTER(p)	(p &
> TRB_DATA_BUFFER_POINTER_MASK)
> > +
> > +/* offset 4 */
> > +#define TRB_TRANSFER_LENGTH_MASK	0x1FFFF
> > +#define TRB_SET_TRANSFER_LENGTH(l)	(l &
> TRB_TRANSFER_LENGTH_MASK)
> > +
> > +#define TRB_BURST_LENGTH_MASK		0xFF
> > +#define TRB_SET_BURST_LENGTH(l)		((l & TRB_BURST_LENGTH_MASK)
> << 24)
> > +
> > +/* offset 8 */
> > +#define TRB_SET_INT_ON_SHORT_PACKET	0x04
> > +#define TRB_SET_FIFO_MODE		0x08
> > +#define TRB_SET_INT_ON_COMPLETION	0x20
> > +
> > +#define TRB_TYPE_NORMAL			0x400
> > +
> > +#define TRB_STREAM_ID_MASK		0xFFFF
> > +#define TRB_SET_STREAM_ID(sid)		((sid & TRB_STREAM_ID_MASK) <<
> 16)
> > +
> 
> $sid needs parenthesis, that is (((sid) & TRB_STREAM_ID_MASK) << 16)
> 
> there are a few more such issues above, fix them too.

Okay, thanks for reminding.

> 
> [...]
> 
> > +#endif /* __DRIVERS_CDNS3_GADGET */
> > diff --git a/drivers/usb/cdns3/io.h b/drivers/usb/cdns3/io.h new file
> > mode 100644 index 0000000000..22b1b03950
> > --- /dev/null
> > +++ b/drivers/usb/cdns3/io.h
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2016 Cadence Design Systems -
> >
> +https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww
> >
> +.cadence.com%2F&data=02%7C01%7Csherry.sun%40nxp.com%7C8c397
> 93120e
> >
> +840bf693608d7249879e6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%
> 7C0%7C63
> >
> +7018109609981006&sdata=aCuyDMx%2Fv8Bp3hSAG%2FrFenr3duDX5
> UG2GaByBf
> > +hDuk8%3D&reserved=0
> > + * Copyright 2019 NXP
> > + */
> > +
> > +#ifndef __DRIVERS_USB_CDNS_IO_H
> > +#define __DRIVERS_USB_CDNS_IO_H
> > +
> > +#include <linux/io.h>
> > +
> > +static inline u32 cdns_readl(u32 __iomem *reg) {
> > +	u32 value = 0;
> 
> return readl() ? The value is assigned twice here, for no reason.

I have fixed this issue.

> 
> > +	value = readl(reg);
> > +	return value;
> > +}
> > +
> > +static inline void cdns_writel(u32 __iomem *reg, u32 value) {
> > +	writel(value, reg);
> > +}
> > +
> > +static inline void cdns_flush_cache(uintptr_t addr, int length) {
> > +	flush_dcache_range(addr, addr + ROUND(length,
> ARCH_DMA_MINALIGN));
> 
> Drop the ROUND() thing here, just use addr + length. If the start or end address
> of the cache flush is wrong, the arch code will warn you about it. Here, it will
> just hide possible bugs.
> 

Okay, thanks for reminding.

> > +}
> > +
> > +#endif /* __DRIVERS_USB_CDNS_IO_H */
> > diff --git a/drivers/usb/gadget/epautoconf.c
> > b/drivers/usb/gadget/epautoconf.c index 179b94cdd0..360f2b75ff 100644
> > --- a/drivers/usb/gadget/epautoconf.c
> > +++ b/drivers/usb/gadget/epautoconf.c
> > @@ -167,6 +167,10 @@ static int ep_matches(
> >  			size = 64;
> >  		put_unaligned(cpu_to_le16(size), &desc->wMaxPacketSize);
> >  	}
> > +
> > +	if (gadget->ops->match_ep)
> > +		return gadget->ops->match_ep(gadget, ep, desc);
> > +
> 
> Separate patch for core framework changes please.
Okay.

Best regards
Sherry sun