[RFC PATCH 00/14] usb: dwc3: Introduce DWC_usb32

[RFC PATCH 00/14] usb: dwc3: Introduce DWC_usb32

[Thinh Nguyen]

This patch series adds support to Synopsys DWC_usb32 controller which is
capable of dual-lane and USB speed up to 40 Gbps. In order to support this new
controller, we need to make a few updates the USB stack and dwc3 driver:

1) dwc3 driver needs to update its IP and revision check. The current scheme
does not support more than 2 controllers.

2) Introduce Lane Speed Mantissa and lane count on the gadget side. Devices
operating in SuperSpeed Plus can refer to gen2x1, gen1x2, or gen2x2.

3) Add a new gadget opts to set the sublink speed for drivers that are
constrained to certain lane count or lane speed mantissa.

4) Add miscellaneous initialization checks for DWC_usb32.


Any review comment is highly appreciated.

Thank you,
Thinh




This patch series depends on the following patches

usb: dwc3: Fix GTXFIFOSIZ.TXFDEP macro name
usb: dwc3: gadget: Properly set maxpacket limit

https://patchwork.kernel.org/cover/11283761/


Thinh Nguyen (14):
  usb: gadget: Add lane count and lsm
  usb: gadget: Add callback to set lane and transfer rate
  usb: composite: Properly report lsm
  usb: dwc3: Implement new id check for DWC_usb32
  usb: dwc3: Update IP checks to support DWC_usb32
  usb: devicetree: dwc3: Add max lane and lsm
  usb: dwc3: gadget: Set lane count and lsm
  usb: dwc3: gadget: Track connected lane count and speed
  usb: dwc3: gadget: Limit the setting of speed
  usb: dwc3: Update HWPARAMS0.MDWIDTH for DWC_usb32
  usb: devicetree: dwc3: Add TRB prefetch count
  usb: dwc3: gadget: Set number of TRB prefetch
  usb: devicetree: dwc3: Add property to disable mult TRB fetch
  usb: dwc3: gadget: Implement disabling of mult TRB fetch

 Documentation/devicetree/bindings/usb/dwc3.txt |   9 ++
 drivers/usb/dwc3/core.c                        |  88 ++++++++----
 drivers/usb/dwc3/core.h                        |  65 ++++++---
 drivers/usb/dwc3/debugfs.c                     |  14 +-
 drivers/usb/dwc3/gadget.c                      | 181 +++++++++++++++++++------
 drivers/usb/dwc3/host.c                        |   2 +-
 drivers/usb/gadget/composite.c                 |  16 ++-
 drivers/usb/gadget/legacy/mass_storage.c       |   2 +
 drivers/usb/gadget/udc/core.c                  |  38 +++++-
 include/linux/usb/composite.h                  |   4 +
 include/linux/usb/gadget.h                     |  15 ++
 11 files changed, 344 insertions(+), 90 deletions(-)

-- 
2.11.0

[RFC PATCH 01/14] usb: gadget: Add lane count and lsm

[Thinh Nguyen]

The USB 3.2 specification supports dual-lane and different transfer
rates. The speed field is not sufficient to describe the device's
sublink. Devices operating in SuperSpeed Plus can refer to gen2x1,
gen1x2, or gen2x2.

The driver may be contrained by a specific sublink attribute. Add max
lane count and lane speed mantissa in Gbps in usb_gadget_driver to
provide more contrains for function drivers that support SuperSpeed
Plus.

Update usb_gadget struct to report the max number of lanes
support and connected lane count. Also, add lane speed mantissa in
Gigabits per second to properly describe the device transfer rate.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 include/linux/usb/composite.h |  4 ++++
 include/linux/usb/gadget.h    | 12 ++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 8675e145ea8b..ed3fb9a53c4a 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -356,6 +356,8 @@ enum {
  *	are predefined. The first entry that may be used is
  *	USB_GADGET_FIRST_AVAIL_IDX
  * @max_speed: Highest speed the driver supports.
+ * @max_lane_count: maximum number of lanes the driver supports (SSP only).
+ * @max_lsm: maximum lane speed mantissa in Gbps the driver supports (SSP only).
  * @needs_serial: set to 1 if the gadget needs userspace to provide
  * 	a serial number.  If one is not provided, warning will be printed.
  * @bind: (REQUIRED) Used to allocate resources that are shared across the
@@ -387,6 +389,8 @@ struct usb_composite_driver {
 	const struct usb_device_descriptor	*dev;
 	struct usb_gadget_strings		**strings;
 	enum usb_device_speed			max_speed;
+	u8					max_lane_count;
+	u8					max_lsm;
 	unsigned		needs_serial:1;
 
 	int			(*bind)(struct usb_composite_dev *cdev);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 124462d65eac..cb7531a6f784 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -335,6 +335,10 @@ struct usb_gadget_ops {
  * @speed: Speed of current connection to USB host.
  * @max_speed: Maximal speed the UDC can handle.  UDC must support this
  *      and all slower speeds.
+ * @lane_count: number of lanes in use.
+ * @max_lane_count: maximum number of lanes the UDC can handle.
+ * @lsm: current connection lane speed mantissa in Gbps
+ * @max_lsm: maximum lane speed mantissa in Gbps
  * @state: the state we are now (attached, suspended, configured, etc)
  * @name: Identifies the controller hardware type.  Used in diagnostics
  *	and sometimes configuration.
@@ -401,6 +405,10 @@ struct usb_gadget {
 	struct list_head		ep_list;	/* of usb_ep */
 	enum usb_device_speed		speed;
 	enum usb_device_speed		max_speed;
+	unsigned			lane_count;
+	unsigned			max_lane_count;
+	unsigned			lsm;
+	unsigned			max_lsm;
 	enum usb_device_state		state;
 	const char			*name;
 	struct device			dev;
@@ -600,6 +608,8 @@ static inline int usb_gadget_activate(struct usb_gadget *gadget)
  * struct usb_gadget_driver - driver for usb 'slave' devices
  * @function: String describing the gadget's function
  * @max_speed: Highest speed the driver handles.
+ * @max_lane_count: maximum lane count the driver handles (SSP only).
+ * @max_lsm: maximum lane speed mantissa in Gbps the driver handles (SSP only).
  * @setup: Invoked for ep0 control requests that aren't handled by
  *	the hardware level driver. Most calls must be handled by
  *	the gadget driver, including descriptor and configuration
@@ -672,6 +682,8 @@ static inline int usb_gadget_activate(struct usb_gadget *gadget)
 struct usb_gadget_driver {
 	char			*function;
 	enum usb_device_speed	max_speed;
+	u8			max_lane_count;
+	u8			max_lsm;
 	int			(*bind)(struct usb_gadget *gadget,
 					struct usb_gadget_driver *driver);
 	void			(*unbind)(struct usb_gadget *);
-- 
2.11.0

[RFC PATCH 02/14] usb: gadget: Add callback to set lane and transfer rate

[Thinh Nguyen]

Introduce gadget opts udc_set_sublink_speed callback to set the lane
count and transfer rate (in lane speed mantissa of Gbps) for SuperSpeed
Plus capable gadgets. In the same way udc_set_speed, this function can
control the gadget's sublink attributes for SuperSpeed Plus.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/gadget/composite.c           |  2 ++
 drivers/usb/gadget/legacy/mass_storage.c |  2 ++
 drivers/usb/gadget/udc/core.c            | 38 +++++++++++++++++++++++++++++++-
 include/linux/usb/gadget.h               |  3 +++
 4 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 3b4f67000315..a4de5a8c0f19 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2353,6 +2353,8 @@ int usb_composite_probe(struct usb_composite_driver *driver)
 	gadget_driver->function =  (char *) driver->name;
 	gadget_driver->driver.name = driver->name;
 	gadget_driver->max_speed = driver->max_speed;
+	gadget_driver->max_lane_count = driver->max_lane_count;
+	gadget_driver->max_lsm = driver->max_lsm;
 
 	return usb_gadget_probe_driver(gadget_driver);
 }
diff --git a/drivers/usb/gadget/legacy/mass_storage.c b/drivers/usb/gadget/legacy/mass_storage.c
index f18f77584fc2..a0912c5afffc 100644
--- a/drivers/usb/gadget/legacy/mass_storage.c
+++ b/drivers/usb/gadget/legacy/mass_storage.c
@@ -223,6 +223,8 @@ static struct usb_composite_driver msg_driver = {
 	.name		= "g_mass_storage",
 	.dev		= &msg_device_desc,
 	.max_speed	= USB_SPEED_SUPER_PLUS,
+	.max_lane_count	= 2,
+	.max_lsm	= 10,
 	.needs_serial	= 1,
 	.strings	= dev_strings,
 	.bind		= msg_bind,
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 51fa614b4079..a3b106a22a6e 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1120,6 +1120,35 @@ static inline void usb_gadget_udc_set_speed(struct usb_udc *udc,
 	}
 }
 
+/**
+ * usb_gadget_udc_set_sublink_attr - tells usb device controller the sublink
+ *    attributes supported by the current driver
+ * @udc: The device we want to set maximum speed
+ * @lane_count: The maximum number of lanes to connect
+ * @lsm: The maximum lane speed mantissa in Gbps to run
+ *
+ * In the same way as usb_gadget_udc_set_speed(), this function can set the
+ * gadget's sublink attributes for SuperSpeed Plus.
+ *
+ * This call is issued by the UDC Class driver before calling
+ * usb_gadget_udc_start() in order to make sure that we don't try to
+ * connect on speeds the gadget driver doesn't support.
+ */
+static inline void usb_gadget_udc_set_sublink_attr(struct usb_udc *udc,
+						   unsigned int lane_count,
+						   unsigned int lsm)
+{
+	if (udc->gadget->ops->udc_set_sublink_attr) {
+		unsigned int rate;
+		unsigned int lanes;
+
+		rate = min(lsm, udc->gadget->max_lsm);
+		lanes = min(lane_count, udc->gadget->max_lane_count);
+		udc->gadget->ops->udc_set_sublink_attr(udc->gadget,
+						       lanes, rate);
+	}
+}
+
 /**
  * usb_udc_release - release the usb_udc struct
  * @dev: the dev member within usb_udc
@@ -1353,7 +1382,14 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri
 	udc->dev.driver = &driver->driver;
 	udc->gadget->dev.driver = &driver->driver;
 
-	usb_gadget_udc_set_speed(udc, driver->max_speed);
+	if (udc->gadget->ops->udc_set_sublink_attr &&
+	    udc->gadget->max_speed == USB_SPEED_SUPER_PLUS &&
+	    driver->max_lsm && driver->max_lane_count &&
+	    driver->max_speed == USB_SPEED_SUPER_PLUS)
+		usb_gadget_udc_set_sublink_attr(udc, driver->max_lane_count,
+						driver->max_lsm);
+	else
+		usb_gadget_udc_set_speed(udc, driver->max_speed);
 
 	ret = driver->bind(udc->gadget, driver);
 	if (ret)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index cb7531a6f784..a8ee2480b408 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -319,6 +319,9 @@ struct usb_gadget_ops {
 			struct usb_gadget_driver *);
 	int	(*udc_stop)(struct usb_gadget *);
 	void	(*udc_set_speed)(struct usb_gadget *, enum usb_device_speed);
+	void	(*udc_set_sublink_attr)(struct usb_gadget *,
+					unsigned int lane_count,
+					unsigned int lsm);
 	struct usb_ep *(*match_ep)(struct usb_gadget *,
 			struct usb_endpoint_descriptor *,
 			struct usb_ss_ep_comp_descriptor *);
-- 
2.11.0

[RFC PATCH 03/14] usb: composite: Properly report lsm

[Thinh Nguyen]

Use the max lane speed mantisa value from the gadget and report to the
device descriptor if available.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/gadget/composite.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index a4de5a8c0f19..cd38078d6a42 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -682,6 +682,11 @@ static int bos_desc(struct usb_composite_dev *cdev)
 	/* The SuperSpeedPlus USB Device Capability descriptor */
 	if (gadget_is_superspeed_plus(cdev->gadget)) {
 		struct usb_ssp_cap_descriptor *ssp_cap;
+		int lsm = 10;
+
+		if (cdev->gadget->ops->udc_set_sublink_attr &&
+		    cdev->gadget->max_lsm)
+			lsm = cdev->gadget->max_lsm;
 
 		ssp_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
 		bos->bNumDeviceCaps++;
@@ -709,20 +714,21 @@ static int bos_desc(struct usb_composite_dev *cdev)
 		 *   ST  = Symmetric, RX
 		 *   LSE =  3 (Gbps)
 		 *   LP  =  1 (SuperSpeedPlus)
-		 *   LSM = 10 (10 Gbps)
+		 *   LSM =  5 or 10
 		 */
 		ssp_cap->bmSublinkSpeedAttr[0] =
-			cpu_to_le32((3 << 4) | (1 << 14) | (0xa << 16));
+			cpu_to_le32((3 << 4) | (1 << 14) | (lsm << 16));
+
 		/*
 		 * bmSublinkSpeedAttr[1] =
 		 *   ST  = Symmetric, TX
 		 *   LSE =  3 (Gbps)
 		 *   LP  =  1 (SuperSpeedPlus)
-		 *   LSM = 10 (10 Gbps)
+		 *   LSM =  5 or 10
 		 */
 		ssp_cap->bmSublinkSpeedAttr[1] =
 			cpu_to_le32((3 << 4) | (1 << 14) |
-				    (0xa << 16) | (1 << 7));
+				    (lsm << 16) | (1 << 7));
 	}
 
 	return le16_to_cpu(bos->wTotalLength);
-- 
2.11.0

[RFC PATCH 04/14] usb: dwc3: Implement new id check for DWC_usb32

[Thinh Nguyen]

Synopsys introduces a new controller DWC_usb32 that supports dual-lane
and speed up to 40 Gbps, and DWC3 driver will drive this controller.
Currently DWC3 uses a single field dwc->revision to ID both the
controllers and their version number. This was sufficient for two
controllers DWC_usb3 and DWC_usb31. However, this check no longer works
to support additional controllers. As a result, let's separate the
dwc->revision field to 2 separate fields: ip and revision. The ip field
now stores the ID of the controller's IP while the revision field stores
the controller's version number.

This new scheme enforces DWC3 to compare the revision within the same
controller's IP only. This patch updates all the revision check of
DWC_usb3 and DWC_usb31 controller to also check the controller's IP.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/core.c   | 31 +++++++++++++++----------------
 drivers/usb/dwc3/core.h   | 44 +++++++++++++++++++++++---------------------
 drivers/usb/dwc3/gadget.c | 44 ++++++++++++++++++++++++--------------------
 drivers/usb/dwc3/host.c   |  2 +-
 4 files changed, 63 insertions(+), 58 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index f561c6c9e8a9..694984a30c5f 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -85,7 +85,7 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
 		 * specified or set to OTG, then set the mode to peripheral.
 		 */
 		if (mode == USB_DR_MODE_OTG &&
-		    dwc->revision >= DWC3_REVISION_330A)
+		    !(dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_330A))
 			mode = USB_DR_MODE_PERIPHERAL;
 	}
 
@@ -304,7 +304,7 @@ static void dwc3_frame_length_adjustment(struct dwc3 *dwc)
 	u32 reg;
 	u32 dft;
 
-	if (dwc->revision < DWC3_REVISION_250A)
+	if (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_250A)
 		return;
 
 	if (dwc->fladj == 0)
@@ -585,7 +585,7 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 	 * will be '0' when the core is reset. Application needs to set it
 	 * to '1' after the core initialization is completed.
 	 */
-	if (dwc->revision > DWC3_REVISION_194A)
+	if (!(dwc3_is_usb3(dwc) && dwc->revision <= DWC3_REVISION_194A))
 		reg |= DWC3_GUSB3PIPECTL_SUSPHY;
 
 	/*
@@ -676,7 +676,7 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 	 * be '0' when the core is reset. Application needs to set it to
 	 * '1' after the core initialization is completed.
 	 */
-	if (dwc->revision > DWC3_REVISION_194A)
+	if (!(dwc3_is_usb3(dwc) && dwc->revision <= DWC3_REVISION_194A))
 		reg |= DWC3_GUSB2PHYCFG_SUSPHY;
 
 	/*
@@ -725,15 +725,13 @@ static bool dwc3_core_is_valid(struct dwc3 *dwc)
 	u32 reg;
 
 	reg = dwc3_readl(dwc->regs, DWC3_GSNPSID);
+	dwc->ip = DWC3_GSNPS_ID(reg);
 
 	/* This should read as U3 followed by revision number */
-	if ((reg & DWC3_GSNPSID_MASK) == 0x55330000) {
-		/* Detected DWC_usb3 IP */
+	if (dwc->ip == DWC3_IP_USB3) {
 		dwc->revision = reg;
-	} else if ((reg & DWC3_GSNPSID_MASK) == 0x33310000) {
-		/* Detected DWC_usb31 IP */
+	} else if (dwc->ip == DWC3_IP_USB31 || dwc->ip == DWC3_IP_USB32) {
 		dwc->revision = dwc3_readl(dwc->regs, DWC3_VER_NUMBER);
-		dwc->revision |= DWC3_REVISION_IS_DWC31;
 		dwc->version_type = dwc3_readl(dwc->regs, DWC3_VER_TYPE);
 	} else {
 		return false;
@@ -766,8 +764,9 @@ static void dwc3_core_setup_global_control(struct dwc3 *dwc)
 		 */
 		if ((dwc->dr_mode == USB_DR_MODE_HOST ||
 				dwc->dr_mode == USB_DR_MODE_OTG) &&
-				(dwc->revision >= DWC3_REVISION_210A &&
-				dwc->revision <= DWC3_REVISION_250A))
+				(dwc3_is_usb3(dwc) &&
+				 dwc->revision >= DWC3_REVISION_210A &&
+				 dwc->revision <= DWC3_REVISION_250A))
 			reg |= DWC3_GCTL_DSBLCLKGTNG | DWC3_GCTL_SOFITPSYNC;
 		else
 			reg &= ~DWC3_GCTL_DSBLCLKGTNG;
@@ -810,7 +809,7 @@ static void dwc3_core_setup_global_control(struct dwc3 *dwc)
 	 * and falls back to high-speed mode which causes
 	 * the device to enter a Connect/Disconnect loop
 	 */
-	if (dwc->revision < DWC3_REVISION_190A)
+	if (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_190A)
 		reg |= DWC3_GCTL_U2RSTECN;
 
 	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
@@ -963,7 +962,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
 		goto err0a;
 
 	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
-	    dwc->revision > DWC3_REVISION_194A) {
+	    !(dwc3_is_usb3(dwc) && dwc->revision <= DWC3_REVISION_194A)) {
 		if (!dwc->dis_u3_susphy_quirk) {
 			reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
 			reg |= DWC3_GUSB3PIPECTL_SUSPHY;
@@ -1016,14 +1015,14 @@ static int dwc3_core_init(struct dwc3 *dwc)
 		dwc3_writel(dwc->regs, DWC3_GUCTL2, reg);
 	}
 
-	if (dwc->revision >= DWC3_REVISION_250A) {
+	if (!(dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_250A)) {
 		reg = dwc3_readl(dwc->regs, DWC3_GUCTL1);
 
 		/*
 		 * Enable hardware control of sending remote wakeup
 		 * in HS when the device is in the L1 state.
 		 */
-		if (dwc->revision >= DWC3_REVISION_290A)
+		if (!(dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_290A))
 			reg |= DWC3_GUCTL1_DEV_L1_EXIT_BY_HW;
 
 		if (dwc->dis_tx_ipgap_linecheck_quirk)
@@ -1393,7 +1392,7 @@ static void dwc3_check_params(struct dwc3 *dwc)
 	 * affected version.
 	 */
 	if (!dwc->imod_interval &&
-	    (dwc->revision == DWC3_REVISION_300A))
+	    (dwc3_is_usb3(dwc) && dwc->revision == DWC3_REVISION_300A))
 		dwc->imod_interval = 1;
 
 	/* Check the maximum_speed parameter */
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 0f019db5e125..7fde3c7da543 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -68,6 +68,7 @@
 #define DWC3_GEVNTCOUNT_EHB	BIT(31)
 #define DWC3_GSNPSID_MASK	0xffff0000
 #define DWC3_GSNPSREV_MASK	0xffff
+#define DWC3_GSNPS_ID(p)	(((p) & DWC3_GSNPSID_MASK) >> 16)
 
 /* DWC3 registers memory space boundries */
 #define DWC3_XHCI_REGS_START		0x0
@@ -945,7 +946,8 @@ struct dwc3_scratchpad_array {
  * @nr_scratch: number of scratch buffers
  * @u1u2: only used on revisions <1.83a for workaround
  * @maximum_speed: maximum speed requested (mainly for testing purposes)
- * @revision: revision register contents
+ * @ip: controller's ID
+ * @revision: controller's version of an IP
  * @version_type: VERSIONTYPE register contents, a sub release of a revision
  * @dr_mode: requested mode of operation
  * @current_dr_role: current role of operation when in dual-role mode
@@ -1099,13 +1101,12 @@ struct dwc3 {
 	u32			u1u2;
 	u32			maximum_speed;
 
-	/*
-	 * All 3.1 IP version constants are greater than the 3.0 IP
-	 * version constants. This works for most version checks in
-	 * dwc3. However, in the future, this may not apply as
-	 * features may be developed on newer versions of the 3.0 IP
-	 * that are not in the 3.1 IP.
-	 */
+	u32			ip;
+
+#define DWC3_IP_USB3		0x5533
+#define DWC3_IP_USB31		0x3331
+#define DWC3_IP_USB32		0x3332
+
 	u32			revision;
 
 #define DWC3_REVISION_173A	0x5533173a
@@ -1132,17 +1133,12 @@ struct dwc3 {
 #define DWC3_REVISION_310A	0x5533310a
 #define DWC3_REVISION_330A	0x5533330a
 
-/*
- * NOTICE: we're using bit 31 as a "is usb 3.1" flag. This is really
- * just so dwc31 revisions are always larger than dwc3.
- */
-#define DWC3_REVISION_IS_DWC31		0x80000000
-#define DWC3_USB31_REVISION_110A	(0x3131302a | DWC3_REVISION_IS_DWC31)
-#define DWC3_USB31_REVISION_120A	(0x3132302a | DWC3_REVISION_IS_DWC31)
-#define DWC3_USB31_REVISION_160A	(0x3136302a | DWC3_REVISION_IS_DWC31)
-#define DWC3_USB31_REVISION_170A	(0x3137302a | DWC3_REVISION_IS_DWC31)
-#define DWC3_USB31_REVISION_180A	(0x3138302a | DWC3_REVISION_IS_DWC31)
-#define DWC3_USB31_REVISION_190A	(0x3139302a | DWC3_REVISION_IS_DWC31)
+#define DWC3_USB31_REVISION_110A	0x3131302a
+#define DWC3_USB31_REVISION_120A	0x3132302a
+#define DWC3_USB31_REVISION_160A	0x3136302a
+#define DWC3_USB31_REVISION_170A	0x3137302a
+#define DWC3_USB31_REVISION_180A	0x3138302a
+#define DWC3_USB31_REVISION_190A	0x3139302a
 
 	u32			version_type;
 
@@ -1391,13 +1387,19 @@ u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type);
 /* check whether we are on the DWC_usb3 core */
 static inline bool dwc3_is_usb3(struct dwc3 *dwc)
 {
-	return !(dwc->revision & DWC3_REVISION_IS_DWC31);
+	return dwc->ip == DWC3_IP_USB3;
 }
 
 /* check whether we are on the DWC_usb31 core */
 static inline bool dwc3_is_usb31(struct dwc3 *dwc)
 {
-	return !!(dwc->revision & DWC3_REVISION_IS_DWC31);
+	return dwc->ip == DWC3_IP_USB31;
+}
+
+/* check whether we are on the DWC_usb32 core */
+static inline bool dwc3_is_usb32(struct dwc3 *dwc)
+{
+	return dwc->ip == DWC3_IP_USB32;
 }
 
 bool dwc3_has_imod(struct dwc3 *dwc);
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 8657c744e826..e0652253ad27 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -95,7 +95,7 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum dwc3_link_state state)
 	 * Wait until device controller is ready. Only applies to 1.94a and
 	 * later RTL.
 	 */
-	if (dwc->revision >= DWC3_REVISION_194A) {
+	if (!(dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_194A)) {
 		while (--retries) {
 			reg = dwc3_readl(dwc->regs, DWC3_DSTS);
 			if (reg & DWC3_DSTS_DCNRD)
@@ -122,7 +122,7 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum dwc3_link_state state)
 	 * The following code is racy when called from dwc3_gadget_wakeup,
 	 * and is not needed, at least on newer versions
 	 */
-	if (dwc->revision >= DWC3_REVISION_194A)
+	if (!(dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_194A))
 		return 0;
 
 	/* wait for a change in DSTS */
@@ -415,7 +415,8 @@ static int dwc3_send_clear_stall_ep_cmd(struct dwc3_ep *dep)
 	 * IN transfers due to a mishandled error condition. Synopsys
 	 * STAR 9000614252.
 	 */
-	if (dep->direction && (dwc->revision >= DWC3_REVISION_260A) &&
+	if (dep->direction &&
+	    !(dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_260A) &&
 	    (dwc->gadget.speed >= USB_SPEED_SUPER))
 		cmd |= DWC3_DEPCMD_CLEARPENDIN;
 
@@ -1747,7 +1748,7 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc)
 	}
 
 	/* Recent versions do this automatically */
-	if (dwc->revision < DWC3_REVISION_194A) {
+	if (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_194A) {
 		/* write zeroes to Link Change Request */
 		reg = dwc3_readl(dwc->regs, DWC3_DCTL);
 		reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK;
@@ -1809,12 +1810,12 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
 
 	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
 	if (is_on) {
-		if (dwc->revision <= DWC3_REVISION_187A) {
+		if (dwc3_is_usb3(dwc) && dwc->revision <= DWC3_REVISION_187A) {
 			reg &= ~DWC3_DCTL_TRGTULST_MASK;
 			reg |= DWC3_DCTL_TRGTULST_RX_DET;
 		}
 
-		if (dwc->revision >= DWC3_REVISION_194A)
+		if (!(dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_194A))
 			reg &= ~DWC3_DCTL_KEEP_CONNECT;
 		reg |= DWC3_DCTL_RUN_STOP;
 
@@ -1888,7 +1889,7 @@ static void dwc3_gadget_enable_irq(struct dwc3 *dwc)
 			DWC3_DEVTEN_USBRSTEN |
 			DWC3_DEVTEN_DISCONNEVTEN);
 
-	if (dwc->revision < DWC3_REVISION_250A)
+	if (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_250A)
 		reg |= DWC3_DEVTEN_ULSTCNGEN;
 
 	dwc3_writel(dwc->regs, DWC3_DEVTEN, reg);
@@ -2145,7 +2146,7 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
 	 * STAR#9000525659: Clock Domain Crossing on DCTL in
 	 * USB 2.0 Mode
 	 */
-	if (dwc->revision < DWC3_REVISION_220A &&
+	if (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_220A &&
 	    !dwc->dis_metastability_quirk) {
 		reg |= DWC3_DCFG_SUPERSPEED;
 	} else {
@@ -2171,7 +2172,7 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
 		default:
 			dev_err(dwc->dev, "invalid speed (%d)\n", speed);
 
-			if (dwc->revision & DWC3_REVISION_IS_DWC31)
+			if (dwc3_is_usb31(dwc))
 				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
 			else
 				reg |= DWC3_DCFG_SUPERSPEED;
@@ -2588,7 +2589,7 @@ static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
 	 * WORKAROUND: This is the 2nd half of U1/U2 -> U0 workaround.
 	 * See dwc3_gadget_linksts_change_interrupt() for 1st half.
 	 */
-	if (dwc->revision < DWC3_REVISION_183A) {
+	if (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_183A) {
 		u32		reg;
 		int		i;
 
@@ -2755,7 +2756,8 @@ static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,
 	if (!interrupt)
 		dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
 
-	if (dwc3_is_usb31(dwc) || dwc->revision < DWC3_REVISION_310A)
+	if (dwc3_is_usb31(dwc) ||
+	    (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_310A))
 		udelay(100);
 }
 
@@ -2833,7 +2835,7 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc)
 	 * STAR#9000466709: RTL: Device : Disconnect event not
 	 * generated if setup packet pending in FIFO
 	 */
-	if (dwc->revision < DWC3_REVISION_188A) {
+	if (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_188A) {
 		if (dwc->setup_packet_pending)
 			dwc3_gadget_disconnect_interrupt(dwc);
 	}
@@ -2892,7 +2894,7 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 		 * STAR#9000483510: RTL: SS : USB3 reset event may
 		 * not be generated always when the link enters poll
 		 */
-		if (dwc->revision < DWC3_REVISION_190A)
+		if (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_190A)
 			dwc3_gadget_reset_interrupt(dwc);
 
 		dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
@@ -2920,7 +2922,7 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 
 	/* Enable USB2 LPM Capability */
 
-	if ((dwc->revision > DWC3_REVISION_194A) &&
+	if (!(dwc3_is_usb3(dwc) && dwc->revision <= DWC3_REVISION_194A) &&
 	    (speed != DWC3_DSTS_SUPERSPEED) &&
 	    (speed != DWC3_DSTS_SUPERSPEED_PLUS)) {
 		reg = dwc3_readl(dwc->regs, DWC3_DCFG);
@@ -2939,11 +2941,13 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 		 * BESL value in the LPM token is less than or equal to LPM
 		 * NYET threshold.
 		 */
-		WARN_ONCE(dwc->revision < DWC3_REVISION_240A
+		WARN_ONCE(dwc3_is_usb3(dwc) &&
+			  dwc->revision < DWC3_REVISION_240A
 				&& dwc->has_lpm_erratum,
 				"LPM Erratum not available on dwc3 revisions < 2.40a\n");
 
-		if (dwc->has_lpm_erratum && dwc->revision >= DWC3_REVISION_240A)
+		if (dwc->has_lpm_erratum &&
+		    !(dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_240A))
 			reg |= DWC3_DCTL_NYET_THRES(dwc->lpm_nyet_threshold);
 
 		dwc3_gadget_dctl_write_safe(dwc, reg);
@@ -3014,7 +3018,7 @@ static void dwc3_gadget_linksts_change_interrupt(struct dwc3 *dwc,
 	 * operational mode
 	 */
 	pwropt = DWC3_GHWPARAMS1_EN_PWROPT(dwc->hwparams.hwparams1);
-	if ((dwc->revision < DWC3_REVISION_250A) &&
+	if (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_250A &&
 			(pwropt != DWC3_GHWPARAMS1_EN_PWROPT_HIB)) {
 		if ((dwc->link_state == DWC3_LINK_STATE_U3) &&
 				(next == DWC3_LINK_STATE_RESUME)) {
@@ -3040,7 +3044,7 @@ static void dwc3_gadget_linksts_change_interrupt(struct dwc3 *dwc,
 	 * STAR#9000446952: RTL: Device SS : if U1/U2 ->U0 takes >128us
 	 * core send LGO_Ux entering U0
 	 */
-	if (dwc->revision < DWC3_REVISION_183A) {
+	if (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_183A) {
 		if (next == DWC3_LINK_STATE_U0) {
 			u32	u1u2;
 			u32	reg;
@@ -3151,7 +3155,7 @@ static void dwc3_gadget_interrupt(struct dwc3 *dwc,
 		break;
 	case DWC3_DEVICE_EVENT_EOPF:
 		/* It changed to be suspend event for version 2.30a and above */
-		if (dwc->revision >= DWC3_REVISION_230A) {
+		if (!(dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_230A)) {
 			/*
 			 * Ignore suspend event until the gadget enters into
 			 * USB_STATE_CONFIGURED state.
@@ -3396,7 +3400,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 	 * is less than super speed because we don't have means, yet, to tell
 	 * composite.c that we are USB 2.0 + LPM ECN.
 	 */
-	if (dwc->revision < DWC3_REVISION_220A &&
+	if (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_220A &&
 	    !dwc->dis_metastability_quirk)
 		dev_info(dwc->dev, "changing max_speed on rev %08x\n",
 				dwc->revision);
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 5567ed2cddbe..e8fc3800e3c0 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -102,7 +102,7 @@ int dwc3_host_init(struct dwc3 *dwc)
 	 *
 	 * This following flag tells XHCI to do just that.
 	 */
-	if (dwc->revision <= DWC3_REVISION_300A)
+	if (dwc3_is_usb3(dwc) && dwc->revision <= DWC3_REVISION_300A)
 		props[prop_idx++].name = "quirk-broken-port-ped";
 
 	if (prop_idx) {
-- 
2.11.0

[RFC PATCH 05/14] usb: dwc3: Update IP checks to support DWC_usb32

[Thinh Nguyen]

Add IP checks within DWC3 driver to support DWC_usb32. Note that these
conditions match the current checks for DWC_usb31 version 1.90a. Any new
and different behavior of DWC_usb32 are added in separate patches.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/core.c   | 17 ++++++++++-------
 drivers/usb/dwc3/gadget.c | 32 ++++++++++++++++----------------
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 694984a30c5f..0bae1beea8a6 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -257,7 +257,8 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
 	 * take a little more than 50ms. Set the polling rate at 20ms
 	 * for 10 times instead.
 	 */
-	if (dwc3_is_usb31(dwc) && dwc->revision >= DWC3_USB31_REVISION_190A)
+	if ((dwc3_is_usb31(dwc) && dwc->revision >= DWC3_USB31_REVISION_190A) ||
+	    dwc3_is_usb32(dwc))
 		retries = 10;
 
 	do {
@@ -265,8 +266,9 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
 		if (!(reg & DWC3_DCTL_CSFTRST))
 			goto done;
 
-		if (dwc3_is_usb31(dwc) &&
-		    dwc->revision >= DWC3_USB31_REVISION_190A)
+		if ((dwc3_is_usb31(dwc) &&
+		     dwc->revision >= DWC3_USB31_REVISION_190A) ||
+		    dwc3_is_usb32(dwc))
 			msleep(20);
 		else
 			udelay(1);
@@ -1009,7 +1011,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
 	 * the DWC_usb3 controller. It is NOT available in the
 	 * DWC_usb31 controller.
 	 */
-	if (!dwc3_is_usb31(dwc) && dwc->revision >= DWC3_REVISION_310A) {
+	if (dwc3_is_usb3(dwc) && dwc->revision >= DWC3_REVISION_310A) {
 		reg = dwc3_readl(dwc->regs, DWC3_GUCTL2);
 		reg |= DWC3_GUCTL2_RST_ACTBITLATER;
 		dwc3_writel(dwc->regs, DWC3_GUCTL2, reg);
@@ -1051,7 +1053,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
 	 * Must config both number of packets and max burst settings to enable
 	 * RX and/or TX threshold.
 	 */
-	if (dwc3_is_usb31(dwc) && dwc->dr_mode == USB_DR_MODE_HOST) {
+	if (!dwc3_is_usb3(dwc) && dwc->dr_mode == USB_DR_MODE_HOST) {
 		u8 rx_thr_num = dwc->rx_thr_num_pkt_prd;
 		u8 rx_maxburst = dwc->rx_max_burst_prd;
 		u8 tx_thr_num = dwc->tx_thr_num_pkt_prd;
@@ -1371,7 +1373,8 @@ bool dwc3_has_imod(struct dwc3 *dwc)
 	return ((dwc3_is_usb3(dwc) &&
 		 dwc->revision >= DWC3_REVISION_300A) ||
 		(dwc3_is_usb31(dwc) &&
-		 dwc->revision >= DWC3_USB31_REVISION_120A));
+		 dwc->revision >= DWC3_USB31_REVISION_120A) ||
+		dwc3_is_usb32(dwc));
 }
 
 static void dwc3_check_params(struct dwc3 *dwc)
@@ -1414,7 +1417,7 @@ static void dwc3_check_params(struct dwc3 *dwc)
 		/*
 		 * default to superspeed plus if we are capable.
 		 */
-		if (dwc3_is_usb31(dwc) &&
+		if ((dwc3_is_usb31(dwc) || dwc3_is_usb32(dwc)) &&
 		    (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) ==
 		     DWC3_GHWPARAMS3_SSPHY_IFC_GEN2))
 			dwc->maximum_speed = USB_SPEED_SUPER_PLUS;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index e0652253ad27..a6d562e208a9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1970,10 +1970,10 @@ static int __dwc3_gadget_start(struct dwc3 *dwc)
 	 * bursts of data without going through any sort of endpoint throttling.
 	 */
 	reg = dwc3_readl(dwc->regs, DWC3_GRXTHRCFG);
-	if (dwc3_is_usb31(dwc))
-		reg &= ~DWC31_GRXTHRCFG_PKTCNTSEL;
-	else
+	if (dwc3_is_usb3(dwc))
 		reg &= ~DWC3_GRXTHRCFG_PKTCNTSEL;
+	else
+		reg &= ~DWC31_GRXTHRCFG_PKTCNTSEL;
 
 	dwc3_writel(dwc->regs, DWC3_GRXTHRCFG, reg);
 
@@ -2164,18 +2164,18 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
 			reg |= DWC3_DCFG_SUPERSPEED;
 			break;
 		case USB_SPEED_SUPER_PLUS:
-			if (dwc3_is_usb31(dwc))
-				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
-			else
+			if (dwc3_is_usb3(dwc))
 				reg |= DWC3_DCFG_SUPERSPEED;
+			else
+				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
 			break;
 		default:
 			dev_err(dwc->dev, "invalid speed (%d)\n", speed);
 
-			if (dwc3_is_usb31(dwc))
-				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
-			else
+			if (dwc3_is_usb3(dwc))
 				reg |= DWC3_DCFG_SUPERSPEED;
+			else
+				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
 		}
 	}
 	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
@@ -2222,10 +2222,10 @@ static int dwc3_gadget_init_in_endpoint(struct dwc3_ep *dep)
 	mdwidth /= 8;
 
 	size = dwc3_readl(dwc->regs, DWC3_GTXFIFOSIZ(dep->number >> 1));
-	if (dwc3_is_usb31(dwc))
-		size = DWC31_GTXFIFOSIZ_TXFDEP(size);
-	else
+	if (dwc3_is_usb3(dwc))
 		size = DWC3_GTXFIFOSIZ_TXFDEP(size);
+	else
+		size = DWC31_GTXFIFOSIZ_TXFDEP(size);
 
 	/* FIFO Depth is in MDWDITH bytes. Multiply */
 	size *= mdwidth;
@@ -2268,10 +2268,10 @@ static int dwc3_gadget_init_out_endpoint(struct dwc3_ep *dep)
 
 	/* All OUT endpoints share a single RxFIFO space */
 	size = dwc3_readl(dwc->regs, DWC3_GRXFIFOSIZ(0));
-	if (dwc3_is_usb31(dwc))
-		size = DWC31_GRXFIFOSIZ_RXFDEP(size);
-	else
+	if (dwc3_is_usb3(dwc))
 		size = DWC3_GRXFIFOSIZ_RXFDEP(size);
+	else
+		size = DWC31_GRXFIFOSIZ_RXFDEP(size);
 
 	/* FIFO depth is in MDWDITH bytes */
 	size *= mdwidth;
@@ -2756,7 +2756,7 @@ static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,
 	if (!interrupt)
 		dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
 
-	if (dwc3_is_usb31(dwc) ||
+	if (dwc3_is_usb31(dwc) || dwc3_is_usb32(dwc) ||
 	    (dwc3_is_usb3(dwc) && dwc->revision < DWC3_REVISION_310A))
 		udelay(100);
 }
-- 
2.11.0

[RFC PATCH 06/14] usb: devicetree: dwc3: Add max lane and lsm

[Thinh Nguyen]

Add a new property to set maximum number of lanes and transfer rated
supported for DWC_usb32. By default, the driver will configure the
controller to use dual-lane at 10Gbps.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 Documentation/devicetree/bindings/usb/dwc3.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
index 66780a47ad85..7da1c4e7d380 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -85,6 +85,10 @@ Optional properties:
  - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ
 	register for post-silicon frame length adjustment when the
 	fladj_30mhz_sdbnd signal is invalid or incorrect.
+ - snps,maximum-lane-count: set to specify the number of lanes to use for
+			DWC_usb32 and later. Default is dual-lanes.
+ - snps,maximum-lsm: set to specify the lane speed mantissa to use in Gbps.
+ 			Default is 10Gbps for SuperSpeed Plus.
  - snps,rx-thr-num-pkt-prd: periodic ESS RX packet threshold count - host mode
 			only. Set this and rx-max-burst-prd to a valid,
 			non-zero value 1-16 (DWC_usb31 programming guide
-- 
2.11.0

[RFC PATCH 07/14] usb: dwc3: gadget: Set lane count and lsm

[Thinh Nguyen]

DWC_usb32 supports dual-lane at different transfer rate. This patch
initializes the controller to use the maximum support transfer rate
describes by the dwc3 device property of lane count and lane speed
mantissa in Gbps.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/core.c   | 36 +++++++++++++++++++++++++++++
 drivers/usb/dwc3/core.h   | 10 ++++++++
 drivers/usb/dwc3/gadget.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 0bae1beea8a6..d09e968644c1 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1297,6 +1297,10 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 				"snps,usb3_lpm_capable");
 	dwc->usb2_lpm_disable = device_property_read_bool(dev,
 				"snps,usb2-lpm-disable");
+	device_property_read_u8(dev, "snps,maximum-lane-count",
+				&dwc->maximum_lanes);
+	device_property_read_u8(dev, "snps,maximum-lsm",
+				&dwc->maximum_lsm);
 	device_property_read_u8(dev, "snps,rx-thr-num-pkt-prd",
 				&rx_thr_num_pkt_prd);
 	device_property_read_u8(dev, "snps,rx-max-burst-prd",
@@ -1424,6 +1428,38 @@ static void dwc3_check_params(struct dwc3 *dwc)
 
 		break;
 	}
+
+	switch (dwc->maximum_lsm) {
+	case 5:
+		break;
+	case 10:
+		if (dwc->maximum_speed == USB_SPEED_SUPER)
+			dev_err(dev, "invalid maximum_lsm parameter %d\n",
+				dwc->maximum_lsm);
+		/* Fall Through */
+	default:
+		if (dwc->maximum_speed == USB_SPEED_SUPER)
+			dwc->maximum_lsm = 5;
+		else if (dwc->maximum_speed > USB_SPEED_SUPER)
+			dwc->maximum_lsm = 10;
+		break;
+	}
+
+	switch (dwc->maximum_lanes) {
+	case 1:
+	case 2:
+		break;
+	default:
+		if (dwc->maximum_lanes > 2)
+			dev_err(dev, "invalid number of lanes %d\n",
+				dwc->maximum_lanes);
+
+		if (dwc3_is_usb32(dwc) &&
+		    dwc->maximum_speed == USB_SPEED_SUPER_PLUS)
+			dwc->maximum_lanes = 2;
+		else
+			dwc->maximum_lanes = 1;
+	}
 }
 
 static int dwc3_probe(struct platform_device *pdev)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 7fde3c7da543..8e729d4cd5bd 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -376,6 +376,8 @@
 #define DWC3_GUCTL2_RST_ACTBITLATER		BIT(14)
 
 /* Device Configuration Register */
+#define DWC3_DCFG_NUMLANES(n)	(((n) & 0x3) << 30) /* DWC_usb32 only */
+
 #define DWC3_DCFG_DEVADDR(addr)	((addr) << 3)
 #define DWC3_DCFG_DEVADDR_MASK	DWC3_DCFG_DEVADDR(0x7f)
 
@@ -449,6 +451,8 @@
 #define DWC3_DEVTEN_USBRSTEN		BIT(1)
 #define DWC3_DEVTEN_DISCONNEVTEN	BIT(0)
 
+#define DWC3_DSTS_CONNLANES(n)		(((n) >> 30) & 0x3) /* DWC_usb32 only */
+
 /* Device Status Register */
 #define DWC3_DSTS_DCNRD			BIT(29)
 
@@ -946,6 +950,8 @@ struct dwc3_scratchpad_array {
  * @nr_scratch: number of scratch buffers
  * @u1u2: only used on revisions <1.83a for workaround
  * @maximum_speed: maximum speed requested (mainly for testing purposes)
+ * @maximum_lsm: maximum lane speed mantissa in Gbps
+ * @maximum_lanes: maximum lane count
  * @ip: controller's ID
  * @revision: controller's version of an IP
  * @version_type: VERSIONTYPE register contents, a sub release of a revision
@@ -973,6 +979,7 @@ struct dwc3_scratchpad_array {
  * @ep0state: state of endpoint zero
  * @link_state: link state
  * @speed: device speed (super, high, full, low)
+ * @lane_count: number of connected lanes
  * @hwparams: copy of hwparams registers
  * @root: debugfs root folder pointer
  * @regset: debugfs pointer to regdump file
@@ -1100,6 +1107,8 @@ struct dwc3 {
 	u32			nr_scratch;
 	u32			u1u2;
 	u32			maximum_speed;
+	u8			maximum_lsm;
+	u8			maximum_lanes;
 
 	u32			ip;
 
@@ -1159,6 +1168,7 @@ struct dwc3 {
 	u8			u1pel;
 
 	u8			speed;
+	u8			lane_count;
 
 	u8			num_eps;
 
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a6d562e208a9..c31144af3261 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2183,6 +2183,53 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
 	spin_unlock_irqrestore(&dwc->lock, flags);
 }
 
+static void dwc3_gadget_set_sublink_attr(struct usb_gadget *g,
+					 unsigned int lane_count,
+					 unsigned int lsm)
+{
+	struct dwc3	*dwc = gadget_to_dwc(g);
+	unsigned int	lanes;
+	unsigned long	flags;
+	u32		reg;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	if (dwc->maximum_speed <= USB_SPEED_SUPER) {
+		/* Fall back to maximum speed supported by HW */
+		spin_unlock_irqrestore(&dwc->lock, flags);
+		dwc3_gadget_set_speed(g, dwc->maximum_speed);
+		spin_lock_irqsave(&dwc->lock, flags);
+
+		goto done;
+	}
+
+	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
+	reg &= ~DWC3_DCFG_SPEED_MASK;
+
+	switch (lsm) {
+	case 5:
+		reg |= DWC3_DCFG_SUPERSPEED;
+		break;
+	case 10:
+		reg |= DWC3_DCFG_SUPERSPEED_PLUS;
+		break;
+	default:
+		dev_err(dwc->dev, "invalid lane speed mantissa (%d)\n", lsm);
+		goto done;
+	}
+
+	/* Lane configuration is only available to dwc_usb32 and higher */
+	if (dwc3_is_usb32(dwc)) {
+		lanes = clamp_t(unsigned int, lane_count,
+				1, dwc->maximum_lanes);
+		reg &= ~DWC3_DCFG_NUMLANES(3);
+		reg |= DWC3_DCFG_NUMLANES(lanes - 1);
+	}
+
+	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
+done:
+	spin_unlock_irqrestore(&dwc->lock, flags);
+}
+
 static const struct usb_gadget_ops dwc3_gadget_ops = {
 	.get_frame		= dwc3_gadget_get_frame,
 	.wakeup			= dwc3_gadget_wakeup,
@@ -2191,6 +2238,7 @@ static const struct usb_gadget_ops dwc3_gadget_ops = {
 	.udc_start		= dwc3_gadget_start,
 	.udc_stop		= dwc3_gadget_stop,
 	.udc_set_speed		= dwc3_gadget_set_speed,
+	.udc_set_sublink_attr	= dwc3_gadget_set_sublink_attr,
 	.get_config_params	= dwc3_gadget_config_params,
 };
 
@@ -3383,6 +3431,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 	dwc->gadget.sg_supported	= true;
 	dwc->gadget.name		= "dwc3-gadget";
 	dwc->gadget.lpm_capable		= true;
+	dwc->gadget.lane_count		= 1;
 
 	/*
 	 * FIXME We might be setting max_speed to <SUPER, however versions
@@ -3406,6 +3455,8 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 				dwc->revision);
 
 	dwc->gadget.max_speed		= dwc->maximum_speed;
+	dwc->gadget.max_lsm		= dwc->maximum_lsm;
+	dwc->gadget.max_lane_count	= dwc->maximum_lanes;
 
 	/*
 	 * REVISIT: Here we should clear all pending IRQs to be
@@ -3422,7 +3473,12 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 		goto err4;
 	}
 
-	dwc3_gadget_set_speed(&dwc->gadget, dwc->maximum_speed);
+	if (dwc3_is_usb32(dwc) && dwc->maximum_lsm)
+		dwc3_gadget_set_sublink_attr(&dwc->gadget,
+					     dwc->maximum_lanes,
+					     dwc->maximum_lsm);
+	else
+		dwc3_gadget_set_speed(&dwc->gadget, dwc->maximum_speed);
 
 	return 0;
 
-- 
2.11.0

[RFC PATCH 08/14] usb: dwc3: gadget: Track connected lane count and speed

[Thinh Nguyen]

Track the number of lanes connected in gadget->lanes and track the lane
speed mantissa for SuperSpeed Plus devices. Also, if the gadget is
running in gen1x2, set the gadget->speed to USB_SPEED_SUPER_PLUS.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c31144af3261..06325e269234 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2907,12 +2907,18 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 	struct dwc3_ep		*dep;
 	int			ret;
 	u32			reg;
+	u8			lanes = 1;
 	u8			speed;
 
 	reg = dwc3_readl(dwc->regs, DWC3_DSTS);
 	speed = reg & DWC3_DSTS_CONNECTSPD;
 	dwc->speed = speed;
 
+	if (dwc3_is_usb32(dwc))
+		lanes = DWC3_DSTS_CONNLANES(reg) + 1;
+
+	dwc->gadget.lane_count = lanes;
+
 	/*
 	 * RAMClkSel is reset to 0 after USB reset, so it must be reprogrammed
 	 * each time on Connect Done.
@@ -2927,6 +2933,7 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 		dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
 		dwc->gadget.ep0->maxpacket = 512;
 		dwc->gadget.speed = USB_SPEED_SUPER_PLUS;
+		dwc->gadget.lsm	= 10;
 		break;
 	case DWC3_DSTS_SUPERSPEED:
 		/*
@@ -2947,7 +2954,12 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 
 		dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
 		dwc->gadget.ep0->maxpacket = 512;
-		dwc->gadget.speed = USB_SPEED_SUPER;
+		dwc->gadget.lsm	= 5;
+
+		if (lanes > 1)
+			dwc->gadget.speed = USB_SPEED_SUPER_PLUS;
+		else
+			dwc->gadget.speed = USB_SPEED_SUPER;
 		break;
 	case DWC3_DSTS_HIGHSPEED:
 		dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(64);
-- 
2.11.0

[RFC PATCH 09/14] usb: dwc3: gadget: Limit the setting of speed

[Thinh Nguyen]

The setting of device speed should be limited by the device property
maximum_speed. This patch adds a check and limit that setting.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 06325e269234..d56feea01066 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2126,6 +2126,7 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
 				  enum usb_device_speed speed)
 {
 	struct dwc3		*dwc = gadget_to_dwc(g);
+	enum usb_device_speed	allowable_speed = speed;
 	unsigned long		flags;
 	u32			reg;
 
@@ -2150,7 +2151,10 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
 	    !dwc->dis_metastability_quirk) {
 		reg |= DWC3_DCFG_SUPERSPEED;
 	} else {
-		switch (speed) {
+		if (speed > dwc->maximum_speed)
+			allowable_speed = dwc->maximum_speed;
+
+		switch (allowable_speed) {
 		case USB_SPEED_LOW:
 			reg |= DWC3_DCFG_LOWSPEED;
 			break;
@@ -2170,7 +2174,8 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
 				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
 			break;
 		default:
-			dev_err(dwc->dev, "invalid speed (%d)\n", speed);
+			dev_err(dwc->dev, "invalid speed (%d)\n",
+				allowable_speed);
 
 			if (dwc3_is_usb3(dwc))
 				reg |= DWC3_DCFG_SUPERSPEED;
-- 
2.11.0

[RFC PATCH 10/14] usb: dwc3: Update HWPARAMS0.MDWIDTH for DWC_usb32

[Thinh Nguyen]

DWC_usb32 supports MDWIDTH larger than 255 and up to 1023 (10 bit
value). The field HWPARAMS6[9:8] field is used to store the upper 2-bit
values of the MDWIDTH. This patch checks that param and properly get the
MDWIDTH for DWC_usb32.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/core.h    |  1 +
 drivers/usb/dwc3/debugfs.c | 14 ++++++++++++--
 drivers/usb/dwc3/gadget.c  |  7 +++++++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 8e729d4cd5bd..bd446dca0869 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -363,6 +363,7 @@
 #define DWC3_GHWPARAMS6_HNPSUPPORT		BIT(11)
 #define DWC3_GHWPARAMS6_SRPSUPPORT		BIT(10)
 #define DWC3_GHWPARAMS6_EN_FPGA			BIT(7)
+#define DWC3_GHWPARAMS6_MDWIDTH(n)		((n) & (0x3 << 8)) /* DWC_usb32 only */
 
 /* Global HWPARAMS7 Register */
 #define DWC3_GHWPARAMS7_RAM1_DEPTH(n)	((n) & 0xffff)
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 4fe8b1e1485c..7610f51e1f82 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -635,13 +635,18 @@ static int dwc3_tx_fifo_size_show(struct seq_file *s, void *unused)
 	struct dwc3_ep		*dep = s->private;
 	struct dwc3		*dwc = dep->dwc;
 	unsigned long		flags;
+	int			mdwidth;
 	u32			val;
 
 	spin_lock_irqsave(&dwc->lock, flags);
 	val = dwc3_core_fifo_space(dep, DWC3_TXFIFO);
 
 	/* Convert to bytes */
-	val *= DWC3_MDWIDTH(dwc->hwparams.hwparams0);
+	mdwidth = DWC3_MDWIDTH(dwc->hwparams.hwparams0);
+	if (dwc3_is_usb32(dwc))
+		mdwidth += DWC3_GHWPARAMS6_MDWIDTH(dwc->hwparams.hwparams6);
+
+	val *= mdwidth;
 	val >>= 3;
 	seq_printf(s, "%u\n", val);
 	spin_unlock_irqrestore(&dwc->lock, flags);
@@ -654,13 +659,18 @@ static int dwc3_rx_fifo_size_show(struct seq_file *s, void *unused)
 	struct dwc3_ep		*dep = s->private;
 	struct dwc3		*dwc = dep->dwc;
 	unsigned long		flags;
+	int			mdwidth;
 	u32			val;
 
 	spin_lock_irqsave(&dwc->lock, flags);
 	val = dwc3_core_fifo_space(dep, DWC3_RXFIFO);
 
 	/* Convert to bytes */
-	val *= DWC3_MDWIDTH(dwc->hwparams.hwparams0);
+	mdwidth = DWC3_MDWIDTH(dwc->hwparams.hwparams0);
+	if (dwc3_is_usb32(dwc))
+		mdwidth += DWC3_GHWPARAMS6_MDWIDTH(dwc->hwparams.hwparams6);
+
+	val *= mdwidth;
 	val >>= 3;
 	seq_printf(s, "%u\n", val);
 	spin_unlock_irqrestore(&dwc->lock, flags);
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index d56feea01066..569be19c84d3 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1934,6 +1934,8 @@ static void dwc3_gadget_setup_nump(struct dwc3 *dwc)
 
 	ram2_depth = DWC3_GHWPARAMS7_RAM2_DEPTH(dwc->hwparams.hwparams7);
 	mdwidth = DWC3_GHWPARAMS0_MDWIDTH(dwc->hwparams.hwparams0);
+	if (dwc3_is_usb32(dwc))
+		mdwidth += DWC3_GHWPARAMS6_MDWIDTH(dwc->hwparams.hwparams6);
 
 	nump = ((ram2_depth * mdwidth / 8) - 24 - 16) / 1024;
 	nump = min_t(u32, nump, 16);
@@ -2271,6 +2273,9 @@ static int dwc3_gadget_init_in_endpoint(struct dwc3_ep *dep)
 	int size;
 
 	mdwidth = DWC3_MDWIDTH(dwc->hwparams.hwparams0);
+	if (dwc3_is_usb32(dwc))
+		mdwidth += DWC3_GHWPARAMS6_MDWIDTH(dwc->hwparams.hwparams6);
+
 	/* MDWIDTH is represented in bits, we need it in bytes */
 	mdwidth /= 8;
 
@@ -2315,6 +2320,8 @@ static int dwc3_gadget_init_out_endpoint(struct dwc3_ep *dep)
 	int size;
 
 	mdwidth = DWC3_MDWIDTH(dwc->hwparams.hwparams0);
+	if (dwc3_is_usb32(dwc))
+		mdwidth += DWC3_GHWPARAMS6_MDWIDTH(dwc->hwparams.hwparams6);
 
 	/* MDWIDTH is represented in bits, convert to bytes */
 	mdwidth /= 8;
-- 
2.11.0

[RFC PATCH 11/14] usb: devicetree: dwc3: Add TRB prefetch count

[Thinh Nguyen]

DWC_usb32 has an enhancement that allows the controller to cache
multiple TRBs for non-control endpoints. Introduce a new property to
DWC3 to set the maximum number of TRBs to cache in advance. The property
can be set from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER (CoreConsultant
value). By default, the number of cache TRB is
DWC_USB32_CACHE_TRBS_PER_TRANSFER.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
index 7da1c4e7d380..ff35fa6de2eb 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -105,6 +105,9 @@ Optional properties:
 			this and tx-thr-num-pkt-prd to a valid, non-zero value
 			1-16 (DWC_usb31 programming guide section 1.2.3) to
 			enable periodic ESS TX threshold.
+ - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
+			can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
+			Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
 
  - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated.
  - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0
-- 
2.11.0

[RFC PATCH 12/14] usb: dwc3: gadget: Set number of TRB prefetch

[Thinh Nguyen]

DWC_usb32 has new feature that allows the controller to cache multiple
TRBs for non-control endpoints. The number of TRB cache can be from 1 to
DWC_USB32_CACHE_TRBS_PER_TRANSFER. By default, if the property is not
set, then the controller will cache up to
DWC_USB32_CACHE_TRBS_PER_TRANSFER number of TRBs.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/core.c   | 2 ++
 drivers/usb/dwc3/core.h   | 5 +++++
 drivers/usb/dwc3/gadget.c | 9 +++++++++
 3 files changed, 16 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index d09e968644c1..8de2928f47ac 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1309,6 +1309,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 				&tx_thr_num_pkt_prd);
 	device_property_read_u8(dev, "snps,tx-max-burst-prd",
 				&tx_max_burst_prd);
+	device_property_read_u8(dev, "snps,num-trb-prefetch",
+				&dwc->num_trb_prefetch);
 
 	dwc->disable_scramble_quirk = device_property_read_bool(dev,
 				"snps,disable_scramble_quirk");
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index bd446dca0869..6315c0fa28b5 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -154,6 +154,8 @@
 #define DWC3_DEPCMDPAR0		0x08
 #define DWC3_DEPCMD		0x0c
 
+#define DWC32_DEPCMDPAR2_TRB_PREFETCH(n)	(((n) & 0xff) << 24)
+
 #define DWC3_DEV_IMOD(n)	(0xca00 + ((n) * 0x4))
 
 /* OTG Registers */
@@ -353,6 +355,7 @@
 #define DWC3_GHWPARAMS3_FSPHY_IFC_ENA		1
 
 /* Global HWPARAMS4 Register */
+#define DWC32_GHWPARAMS4_CACHE_TRBS(n)		((n) & 0x3f) /* DWC_usb32 only */
 #define DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(n)	(((n) & (0x0f << 13)) >> 13)
 #define DWC3_MAX_HIBER_SCRATCHBUFS		15
 
@@ -993,6 +996,7 @@ struct dwc3_scratchpad_array {
  * @rx_max_burst_prd: max periodic ESS receive burst size
  * @tx_thr_num_pkt_prd: periodic ESS transmit packet count
  * @tx_max_burst_prd: max periodic ESS transmit burst size
+ * @num_trb_prefetch: number of TRBs to cache
  * @hsphy_interface: "utmi" or "ulpi"
  * @connected: true when we're connected to a host, false otherwise
  * @delayed_status: true when gadget driver asks for delayed status
@@ -1187,6 +1191,7 @@ struct dwc3 {
 	u8			rx_max_burst_prd;
 	u8			tx_thr_num_pkt_prd;
 	u8			tx_max_burst_prd;
+	u8			num_trb_prefetch;
 
 	const char		*hsphy_interface;
 
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 569be19c84d3..cd478de6c008 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -566,6 +566,15 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep *dep, unsigned int action)
 	if (action == DWC3_DEPCFG_ACTION_RESTORE)
 		params.param2 |= dep->saved_state;
 
+	if (dwc3_is_usb32(dwc) && dep->number > 1 && dwc->num_trb_prefetch) {
+		u32 hwparams4 = dwc->hwparams.hwparams4;
+		int trb_count_max = DWC32_GHWPARAMS4_CACHE_TRBS(hwparams4);
+		int trb_count;
+
+		trb_count = min_t(int, dwc->num_trb_prefetch, trb_count_max);
+		params.param2 |= DWC32_DEPCMDPAR2_TRB_PREFETCH(trb_count);
+	}
+
 	if (usb_endpoint_xfer_control(desc))
 		params.param1 = DWC3_DEPCFG_XFER_COMPLETE_EN;
 
-- 
2.11.0

[RFC PATCH 13/14] usb: devicetree: dwc3: Add property to disable mult TRB fetch

[Thinh Nguyen]

DWC_usb32 has a feature where it can issue multiple TRB fetch requests.
Add a new property to limit and only do only single TRB fetch request.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
index ff35fa6de2eb..29d6f9b1fc70 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -108,6 +108,8 @@ Optional properties:
  - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
 			can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
 			Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
+ - snps,dis-mult-trb-fetch: set to issue only single TRB fetch request in
+			DWC_usb32.
 
  - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated.
  - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0
-- 
2.11.0

[RFC PATCH 14/14] usb: dwc3: gadget: Implement disabling of mult TRB fetch

[Thinh Nguyen]

DWC_usb32 controller allows the HW to send multiple TRB fetch requests.
If the second fetch to the same TRB complete before the previous
request, then the result of the second fetch is taken and the first to
drop. This feature improve the performance when the number of cache TRB
is greater than 2. DWC_usb32 also has the option to issue only single
TRB fetch, which this patch implements the "snps,dis-mult-trb-fetch"
property.

For optimal performance, the programming guide recommend to do only
single TRB fetch if num_trb_prefetch is 2 or lower.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/core.c   |  2 ++
 drivers/usb/dwc3/core.h   |  5 +++++
 drivers/usb/dwc3/gadget.c | 12 ++++++++++++
 3 files changed, 19 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 8de2928f47ac..713b5fdce38b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1346,6 +1346,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 				"snps,dis-del-phy-power-chg-quirk");
 	dwc->dis_tx_ipgap_linecheck_quirk = device_property_read_bool(dev,
 				"snps,dis-tx-ipgap-linecheck-quirk");
+	dwc->dis_mult_trb_fetch = device_property_read_bool(dev,
+				"snps,dis-mult-trb-fetch");
 
 	dwc->tx_de_emphasis_quirk = device_property_read_bool(dev,
 				"snps,tx_de_emphasis_quirk");
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 6315c0fa28b5..49692d88f15b 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -146,6 +146,7 @@
 #define DWC3_DSTS		0xc70c
 #define DWC3_DGCMDPAR		0xc710
 #define DWC3_DGCMD		0xc714
+#define DWC32_DCTL1		0xc718 /* DWC_usb32 only */
 #define DWC3_DALEPENA		0xc720
 
 #define DWC3_DEP_BASE(n)	(0xc800 + ((n) * 0x10))
@@ -441,6 +442,8 @@
 #define DWC3_DCTL_ULSTCHNG_COMPLIANCE	(DWC3_DCTL_ULSTCHNGREQ(10))
 #define DWC3_DCTL_ULSTCHNG_LOOPBACK	(DWC3_DCTL_ULSTCHNGREQ(11))
 
+#define DWC32_DCTL1_SINGLE_TRB_FETCH	BIT(0)
+
 /* Device Event Enable Register */
 #define DWC3_DEVTEN_VNDRDEVTSTRCVEDEN	BIT(12)
 #define DWC3_DEVTEN_EVNTOVERFLOWEN	BIT(11)
@@ -1040,6 +1043,7 @@ struct dwc3_scratchpad_array {
  *			change quirk.
  * @dis_tx_ipgap_linecheck_quirk: set if we disable u2mac linestate
  *			check during HS transmit.
+ * @dis_mult_trb_fetch: set to disable multiple TRB cache
  * @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk
  * @tx_de_emphasis: Tx de-emphasis value
  * 	0	- -6dB de-emphasis
@@ -1229,6 +1233,7 @@ struct dwc3 {
 	unsigned		dis_u2_freeclk_exists_quirk:1;
 	unsigned		dis_del_phy_power_chg_quirk:1;
 	unsigned		dis_tx_ipgap_linecheck_quirk:1;
+	unsigned		dis_mult_trb_fetch:1;
 
 	unsigned		tx_de_emphasis_quirk:1;
 	unsigned		tx_de_emphasis:2;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index cd478de6c008..60bed79f0681 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3506,6 +3506,18 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 		goto err4;
 	}
 
+	if (dwc3_is_usb32(dwc) && dwc->dis_mult_trb_fetch) {
+		u32 reg;
+
+		/*
+		 * For optimal performance, set DCTL1.SINGLE_TRB_FETCH_EN
+		 * if the number of trb prefetch is 2 or less.
+		 */
+		reg = dwc3_readl(dwc->regs, DWC32_DCTL1);
+		reg |= DWC32_DCTL1_SINGLE_TRB_FETCH;
+		dwc3_writel(dwc->regs, DWC32_DCTL1, reg);
+	}
+
 	if (dwc3_is_usb32(dwc) && dwc->maximum_lsm)
 		dwc3_gadget_set_sublink_attr(&dwc->gadget,
 					     dwc->maximum_lanes,
-- 
2.11.0

Re: [RFC PATCH 02/14] usb: gadget: Add callback to set lane and transfer rate

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 4979 bytes --]


Hi,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> Introduce gadget opts udc_set_sublink_speed callback to set the lane
> count and transfer rate (in lane speed mantissa of Gbps) for SuperSpeed
> Plus capable gadgets. In the same way udc_set_speed, this function can
> control the gadget's sublink attributes for SuperSpeed Plus.
>
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
>  drivers/usb/gadget/composite.c           |  2 ++
>  drivers/usb/gadget/legacy/mass_storage.c |  2 ++

I would rather not add new features to the legacy gadgets and focus on
our configfs interface for anything new. Moreover, using the feature
you introduced could, arguably, be done as a separate patch.

> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 3b4f67000315..a4de5a8c0f19 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -2353,6 +2353,8 @@ int usb_composite_probe(struct usb_composite_driver *driver)
>  	gadget_driver->function =  (char *) driver->name;
>  	gadget_driver->driver.name = driver->name;
>  	gadget_driver->max_speed = driver->max_speed;
> +	gadget_driver->max_lane_count = driver->max_lane_count;
> +	gadget_driver->max_lsm = driver->max_lsm;
>  
>  	return usb_gadget_probe_driver(gadget_driver);
>  }
> diff --git a/drivers/usb/gadget/legacy/mass_storage.c b/drivers/usb/gadget/legacy/mass_storage.c
> index f18f77584fc2..a0912c5afffc 100644
> --- a/drivers/usb/gadget/legacy/mass_storage.c
> +++ b/drivers/usb/gadget/legacy/mass_storage.c
> @@ -223,6 +223,8 @@ static struct usb_composite_driver msg_driver = {
>  	.name		= "g_mass_storage",
>  	.dev		= &msg_device_desc,
>  	.max_speed	= USB_SPEED_SUPER_PLUS,
> +	.max_lane_count	= 2,
> +	.max_lsm	= 10,

Right, as mentioned, I'd prefer not touch the legacy gadgets. But in any
case, why is it so that the gadget is telling you about max lane count
and lsm? That should be abstracted away from the gadget driver. Gadget
driver shouldn't have knowledge of number of lanes because, at the end
of the day, that doesn't really change anything in practice. Unlike HS
vs SS which changes a bunch of things.

>  	.needs_serial	= 1,
>  	.strings	= dev_strings,
>  	.bind		= msg_bind,
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 51fa614b4079..a3b106a22a6e 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -1120,6 +1120,35 @@ static inline void usb_gadget_udc_set_speed(struct usb_udc *udc,
>  	}
>  }
>  
> +/**
> + * usb_gadget_udc_set_sublink_attr - tells usb device controller the sublink
> + *    attributes supported by the current driver
> + * @udc: The device we want to set maximum speed
> + * @lane_count: The maximum number of lanes to connect
> + * @lsm: The maximum lane speed mantissa in Gbps to run
> + *
> + * In the same way as usb_gadget_udc_set_speed(), this function can set the
> + * gadget's sublink attributes for SuperSpeed Plus.
> + *
> + * This call is issued by the UDC Class driver before calling
> + * usb_gadget_udc_start() in order to make sure that we don't try to
> + * connect on speeds the gadget driver doesn't support.
> + */
> +static inline void usb_gadget_udc_set_sublink_attr(struct usb_udc *udc,
> +						   unsigned int lane_count,
> +						   unsigned int lsm)

do we envision a possibility of future USB spec releases adding more
data here? How about introducing a struct usb_sublink_attr to be passed
around? Could be used by both host and gadget stacks.

> +{
> +	if (udc->gadget->ops->udc_set_sublink_attr) {
> +		unsigned int rate;
> +		unsigned int lanes;
> +
> +		rate = min(lsm, udc->gadget->max_lsm);
> +		lanes = min(lane_count, udc->gadget->max_lane_count);

considering that lsm and lane_count are from 0 to their respective max
values, do you need a min() here? Might be better to WARN() when either
in over their max values.

> +		udc->gadget->ops->udc_set_sublink_attr(udc->gadget,
> +						       lanes, rate);

indentation using spaces? (same above, please fix)

> @@ -1353,7 +1382,14 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri
>  	udc->dev.driver = &driver->driver;
>  	udc->gadget->dev.driver = &driver->driver;
>  
> -	usb_gadget_udc_set_speed(udc, driver->max_speed);
> +	if (udc->gadget->ops->udc_set_sublink_attr &&
> +	    udc->gadget->max_speed == USB_SPEED_SUPER_PLUS &&
> +	    driver->max_lsm && driver->max_lane_count &&
> +	    driver->max_speed == USB_SPEED_SUPER_PLUS)

So if driver doesn't provide max_lsm and max_speed you don't set sublink
attr? Won't this cause problems? Also, the sublink_attr is still,
conceptually, setting the max speed for the bus, right? So you may want
to call usb_gadget_udc_set_sublink_attr() from inside
usb_gadget_udc_set_speed(), then we don't need to modify all the callers.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [RFC PATCH 03/14] usb: composite: Properly report lsm

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]


Hi,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> Use the max lane speed mantisa value from the gadget and report to the
> device descriptor if available.
>
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
>  drivers/usb/gadget/composite.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index a4de5a8c0f19..cd38078d6a42 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -682,6 +682,11 @@ static int bos_desc(struct usb_composite_dev *cdev)
>  	/* The SuperSpeedPlus USB Device Capability descriptor */
>  	if (gadget_is_superspeed_plus(cdev->gadget)) {
>  		struct usb_ssp_cap_descriptor *ssp_cap;
> +		int lsm = 10;
> +
> +		if (cdev->gadget->ops->udc_set_sublink_attr &&
> +		    cdev->gadget->max_lsm)
> +			lsm = cdev->gadget->max_lsm;
>  
>  		ssp_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
>  		bos->bNumDeviceCaps++;
> @@ -709,20 +714,21 @@ static int bos_desc(struct usb_composite_dev *cdev)
>  		 *   ST  = Symmetric, RX
>  		 *   LSE =  3 (Gbps)
>  		 *   LP  =  1 (SuperSpeedPlus)
> -		 *   LSM = 10 (10 Gbps)
> +		 *   LSM =  5 or 10
>  		 */
>  		ssp_cap->bmSublinkSpeedAttr[0] =
> -			cpu_to_le32((3 << 4) | (1 << 14) | (0xa << 16));
> +			cpu_to_le32((3 << 4) | (1 << 14) | (lsm << 16));

while at that, can we get rid of the magic constants?

>  		ssp_cap->bmSublinkSpeedAttr[1] =
>  			cpu_to_le32((3 << 4) | (1 << 14) |
> -				    (0xa << 16) | (1 << 7));
> +				    (lsm << 16) | (1 << 7));

ditto.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [RFC PATCH 05/14] usb: dwc3: Update IP checks to support DWC_usb32

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 293 bytes --]


Hi,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> @@ -1009,7 +1011,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  	 * the DWC_usb3 controller. It is NOT available in the

perhaps update the comment here? "It is available ONLY in the DWC_usb30
controller".


-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [RFC PATCH 06/14] usb: devicetree: dwc3: Add max lane and lsm

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:

> Add a new property to set maximum number of lanes and transfer rated
                                                                 ^^^^^
                                                                 rate?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [RFC PATCH 07/14] usb: dwc3: gadget: Set lane count and lsm

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 6026 bytes --]

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:

> DWC_usb32 supports dual-lane at different transfer rate. This patch
> initializes the controller to use the maximum support transfer rate
> describes by the dwc3 device property of lane count and lane speed
> mantissa in Gbps.

Perhaps this should read as:

	"DWC_usb32 supports dual-lane at different transfer rate. This
	patch initializes the controller to use the maximum supported
	transfer rate described by the dwc3 device property of lane
	count and lane speed mantissa in Gbps."

> @@ -1424,6 +1428,38 @@ static void dwc3_check_params(struct dwc3 *dwc)
>  
>  		break;
>  	}
> +
> +	switch (dwc->maximum_lsm) {
> +	case 5:
> +		break;
> +	case 10:
> +		if (dwc->maximum_speed == USB_SPEED_SUPER)
> +			dev_err(dev, "invalid maximum_lsm parameter %d\n",
> +				dwc->maximum_lsm);
> +		/* Fall Through */
> +	default:
> +		if (dwc->maximum_speed == USB_SPEED_SUPER)
> +			dwc->maximum_lsm = 5;
> +		else if (dwc->maximum_speed > USB_SPEED_SUPER)
> +			dwc->maximum_lsm = 10;
> +		break;
> +	}
> +
> +	switch (dwc->maximum_lanes) {
> +	case 1:
> +	case 2:
> +		break;
> +	default:
> +		if (dwc->maximum_lanes > 2)
> +			dev_err(dev, "invalid number of lanes %d\n",
> +				dwc->maximum_lanes);
> +
> +		if (dwc3_is_usb32(dwc) &&
> +		    dwc->maximum_speed == USB_SPEED_SUPER_PLUS)
> +			dwc->maximum_lanes = 2;
> +		else
> +			dwc->maximum_lanes = 1;
> +	}
>  }
>  
>  static int dwc3_probe(struct platform_device *pdev)
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 7fde3c7da543..8e729d4cd5bd 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -376,6 +376,8 @@
>  #define DWC3_GUCTL2_RST_ACTBITLATER		BIT(14)
>  
>  /* Device Configuration Register */
> +#define DWC3_DCFG_NUMLANES(n)	(((n) & 0x3) << 30) /* DWC_usb32 only */
> +
>  #define DWC3_DCFG_DEVADDR(addr)	((addr) << 3)
>  #define DWC3_DCFG_DEVADDR_MASK	DWC3_DCFG_DEVADDR(0x7f)
>  
> @@ -449,6 +451,8 @@
>  #define DWC3_DEVTEN_USBRSTEN		BIT(1)
>  #define DWC3_DEVTEN_DISCONNEVTEN	BIT(0)
>  
> +#define DWC3_DSTS_CONNLANES(n)		(((n) >> 30) & 0x3) /* DWC_usb32 only */
> +
>  /* Device Status Register */
>  #define DWC3_DSTS_DCNRD			BIT(29)
>  
> @@ -946,6 +950,8 @@ struct dwc3_scratchpad_array {
>   * @nr_scratch: number of scratch buffers
>   * @u1u2: only used on revisions <1.83a for workaround
>   * @maximum_speed: maximum speed requested (mainly for testing purposes)
> + * @maximum_lsm: maximum lane speed mantissa in Gbps
> + * @maximum_lanes: maximum lane count
>   * @ip: controller's ID
>   * @revision: controller's version of an IP
>   * @version_type: VERSIONTYPE register contents, a sub release of a revision
> @@ -973,6 +979,7 @@ struct dwc3_scratchpad_array {
>   * @ep0state: state of endpoint zero
>   * @link_state: link state
>   * @speed: device speed (super, high, full, low)
> + * @lane_count: number of connected lanes
>   * @hwparams: copy of hwparams registers
>   * @root: debugfs root folder pointer
>   * @regset: debugfs pointer to regdump file
> @@ -1100,6 +1107,8 @@ struct dwc3 {
>  	u32			nr_scratch;
>  	u32			u1u2;
>  	u32			maximum_speed;
> +	u8			maximum_lsm;
> +	u8			maximum_lanes;
>  
>  	u32			ip;
>  
> @@ -1159,6 +1168,7 @@ struct dwc3 {
>  	u8			u1pel;
>  
>  	u8			speed;
> +	u8			lane_count;
>  
>  	u8			num_eps;
>  
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index a6d562e208a9..c31144af3261 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2183,6 +2183,53 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  }
>  
> +static void dwc3_gadget_set_sublink_attr(struct usb_gadget *g,
> +					 unsigned int lane_count,
> +					 unsigned int lsm)
> +{
> +	struct dwc3	*dwc = gadget_to_dwc(g);
> +	unsigned int	lanes;
> +	unsigned long	flags;
> +	u32		reg;
> +
> +	spin_lock_irqsave(&dwc->lock, flags);
> +	if (dwc->maximum_speed <= USB_SPEED_SUPER) {
> +		/* Fall back to maximum speed supported by HW */
> +		spin_unlock_irqrestore(&dwc->lock, flags);
> +		dwc3_gadget_set_speed(g, dwc->maximum_speed);
> +		spin_lock_irqsave(&dwc->lock, flags);

it looks like we should extract a __dwc3_gadget_set_speed() to avoid the
possible race here:

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a9aba716bf80..e317b696029e 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2118,14 +2118,11 @@ static void dwc3_gadget_config_params(struct usb_gadget *g,
 				cpu_to_le16(DWC3_DEFAULT_U2_DEV_EXIT_LAT);
 }
 
-static void dwc3_gadget_set_speed(struct usb_gadget *g,
+static void __dwc3_gadget_set_speed(struct dwc3 *dwc,
 				  enum usb_device_speed speed)
 {
-	struct dwc3		*dwc = gadget_to_dwc(g);
-	unsigned long		flags;
 	u32			reg;
 
-	spin_lock_irqsave(&dwc->lock, flags);
 	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
 	reg &= ~(DWC3_DCFG_SPEED_MASK);
 
@@ -2175,7 +2172,16 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
 		}
 	}
 	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
+}
+
+static void dwc3_gadget_set_speed(struct usb_gadget *g,
+				  enum usb_device_speed speed)
+{
+	struct dwc3		*dwc = gadget_to_dwc(g);
+	unsigned long		flags;
 
+	spin_lock_irqsave(&dwc->lock, flags);
+	__dwc3_gadget_set_speed(dwc, speed);
 	spin_unlock_irqrestore(&dwc->lock, flags);
 }
 
Then your patch would look like:

static void dwc3_gadget_set_sublink_attr(struct usb_gadget *g,
					 unsigned int lane_count,
					 unsigned int lsm)
{
	struct dwc3	*dwc = gadget_to_dwc(g);
	unsigned int	lanes;
	unsigned long	flags;
	u32		reg;

	spin_lock_irqsave(&dwc->lock, flags);
	if (dwc->maximum_speed <= USB_SPEED_SUPER) {
		/* Fall back to maximum speed supported by HW */
		__dwc3_gadget_set_speed(dwc, dwc->maximum_speed);
		goto done;
	}

	[...]
}

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [RFC PATCH 11/14] usb: devicetree: dwc3: Add TRB prefetch count

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]


Hi,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> DWC_usb32 has an enhancement that allows the controller to cache
> multiple TRBs for non-control endpoints. Introduce a new property to
> DWC3 to set the maximum number of TRBs to cache in advance. The property
> can be set from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER (CoreConsultant
> value). By default, the number of cache TRB is
> DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
>  Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> index 7da1c4e7d380..ff35fa6de2eb 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -105,6 +105,9 @@ Optional properties:
>  			this and tx-thr-num-pkt-prd to a valid, non-zero value
>  			1-16 (DWC_usb31 programming guide section 1.2.3) to
>  			enable periodic ESS TX threshold.
> + - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
> +			can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
> +			Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.

how about we just leave it at the maximum and in case someone notices
problems, then we consider adding more DT bindings?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [RFC PATCH 13/14] usb: devicetree: dwc3: Add property to disable mult TRB fetch

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]


Hi,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> DWC_usb32 has a feature where it can issue multiple TRB fetch requests.
> Add a new property to limit and only do only single TRB fetch request.
>
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
>  Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> index ff35fa6de2eb..29d6f9b1fc70 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -108,6 +108,8 @@ Optional properties:
>   - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
>  			can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>  			Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
> + - snps,dis-mult-trb-fetch: set to issue only single TRB fetch request in
> +			DWC_usb32.

two questions:

- how is this different from passing 1 to the previous DT binding
- do we know of anybody having issues with multi-trb prefetch?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [RFC PATCH 02/14] usb: gadget: Add callback to set lane and transfer rate

[Alan Stern]

On Thu, 12 Dec 2019, Felipe Balbi wrote:

> Hi,
> 
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> > Introduce gadget opts udc_set_sublink_speed callback to set the lane
> > count and transfer rate (in lane speed mantissa of Gbps) for SuperSpeed
> > Plus capable gadgets. In the same way udc_set_speed, this function can
> > control the gadget's sublink attributes for SuperSpeed Plus.
> >
> > Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> > ---
> >  drivers/usb/gadget/composite.c           |  2 ++
> >  drivers/usb/gadget/legacy/mass_storage.c |  2 ++
> 
> I would rather not add new features to the legacy gadgets and focus on
> our configfs interface for anything new. Moreover, using the feature
> you introduced could, arguably, be done as a separate patch.
> 
> > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> > index 3b4f67000315..a4de5a8c0f19 100644
> > --- a/drivers/usb/gadget/composite.c
> > +++ b/drivers/usb/gadget/composite.c
> > @@ -2353,6 +2353,8 @@ int usb_composite_probe(struct usb_composite_driver *driver)
> >  	gadget_driver->function =  (char *) driver->name;
> >  	gadget_driver->driver.name = driver->name;
> >  	gadget_driver->max_speed = driver->max_speed;
> > +	gadget_driver->max_lane_count = driver->max_lane_count;
> > +	gadget_driver->max_lsm = driver->max_lsm;
> >  
> >  	return usb_gadget_probe_driver(gadget_driver);
> >  }
> > diff --git a/drivers/usb/gadget/legacy/mass_storage.c b/drivers/usb/gadget/legacy/mass_storage.c
> > index f18f77584fc2..a0912c5afffc 100644
> > --- a/drivers/usb/gadget/legacy/mass_storage.c
> > +++ b/drivers/usb/gadget/legacy/mass_storage.c
> > @@ -223,6 +223,8 @@ static struct usb_composite_driver msg_driver = {
> >  	.name		= "g_mass_storage",
> >  	.dev		= &msg_device_desc,
> >  	.max_speed	= USB_SPEED_SUPER_PLUS,
> > +	.max_lane_count	= 2,
> > +	.max_lsm	= 10,
> 
> Right, as mentioned, I'd prefer not touch the legacy gadgets. But in any
> case, why is it so that the gadget is telling you about max lane count
> and lsm? That should be abstracted away from the gadget driver. Gadget
> driver shouldn't have knowledge of number of lanes because, at the end
> of the day, that doesn't really change anything in practice. Unlike HS
> vs SS which changes a bunch of things.

I agree completely.  Furthermore, it isn't at all clear where those two 
numbers came from.  Why would g-mass-storage care that lane_count <= 2 
and lsm <= 10?

Alan Stern

Re: [RFC PATCH 02/14] usb: gadget: Add callback to set lane and transfer rate

[Thinh Nguyen]

Hi,

Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>> Introduce gadget opts udc_set_sublink_speed callback to set the lane
>> count and transfer rate (in lane speed mantissa of Gbps) for SuperSpeed
>> Plus capable gadgets. In the same way udc_set_speed, this function can
>> control the gadget's sublink attributes for SuperSpeed Plus.
>>
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>>   drivers/usb/gadget/composite.c           |  2 ++
>>   drivers/usb/gadget/legacy/mass_storage.c |  2 ++
> I would rather not add new features to the legacy gadgets and focus on
> our configfs interface for anything new. Moreover, using the feature
> you introduced could, arguably, be done as a separate patch.

Sure. I'll revise this.

>
>> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
>> index 3b4f67000315..a4de5a8c0f19 100644
>> --- a/drivers/usb/gadget/composite.c
>> +++ b/drivers/usb/gadget/composite.c
>> @@ -2353,6 +2353,8 @@ int usb_composite_probe(struct usb_composite_driver *driver)
>>   	gadget_driver->function =  (char *) driver->name;
>>   	gadget_driver->driver.name = driver->name;
>>   	gadget_driver->max_speed = driver->max_speed;
>> +	gadget_driver->max_lane_count = driver->max_lane_count;
>> +	gadget_driver->max_lsm = driver->max_lsm;
>>   
>>   	return usb_gadget_probe_driver(gadget_driver);
>>   }
>> diff --git a/drivers/usb/gadget/legacy/mass_storage.c b/drivers/usb/gadget/legacy/mass_storage.c
>> index f18f77584fc2..a0912c5afffc 100644
>> --- a/drivers/usb/gadget/legacy/mass_storage.c
>> +++ b/drivers/usb/gadget/legacy/mass_storage.c
>> @@ -223,6 +223,8 @@ static struct usb_composite_driver msg_driver = {
>>   	.name		= "g_mass_storage",
>>   	.dev		= &msg_device_desc,
>>   	.max_speed	= USB_SPEED_SUPER_PLUS,
>> +	.max_lane_count	= 2,
>> +	.max_lsm	= 10,
> Right, as mentioned, I'd prefer not touch the legacy gadgets. But in any
> case, why is it so that the gadget is telling you about max lane count
> and lsm? That should be abstracted away from the gadget driver. Gadget
> driver shouldn't have knowledge of number of lanes because, at the end
> of the day, that doesn't really change anything in practice. Unlike HS
> vs SS which changes a bunch of things.

Ok, that makes sense. I'll remove this.

>
>>   	.needs_serial	= 1,
>>   	.strings	= dev_strings,
>>   	.bind		= msg_bind,
>> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
>> index 51fa614b4079..a3b106a22a6e 100644
>> --- a/drivers/usb/gadget/udc/core.c
>> +++ b/drivers/usb/gadget/udc/core.c
>> @@ -1120,6 +1120,35 @@ static inline void usb_gadget_udc_set_speed(struct usb_udc *udc,
>>   	}
>>   }
>>   
>> +/**
>> + * usb_gadget_udc_set_sublink_attr - tells usb device controller the sublink
>> + *    attributes supported by the current driver
>> + * @udc: The device we want to set maximum speed
>> + * @lane_count: The maximum number of lanes to connect
>> + * @lsm: The maximum lane speed mantissa in Gbps to run
>> + *
>> + * In the same way as usb_gadget_udc_set_speed(), this function can set the
>> + * gadget's sublink attributes for SuperSpeed Plus.
>> + *
>> + * This call is issued by the UDC Class driver before calling
>> + * usb_gadget_udc_start() in order to make sure that we don't try to
>> + * connect on speeds the gadget driver doesn't support.
>> + */
>> +static inline void usb_gadget_udc_set_sublink_attr(struct usb_udc *udc,
>> +						   unsigned int lane_count,
>> +						   unsigned int lsm)
> do we envision a possibility of future USB spec releases adding more
> data here? How about introducing a struct usb_sublink_attr to be passed
> around? Could be used by both host and gadget stacks.

Good idea. That'd be much better. Thanks.

>
>> +{
>> +	if (udc->gadget->ops->udc_set_sublink_attr) {
>> +		unsigned int rate;
>> +		unsigned int lanes;
>> +
>> +		rate = min(lsm, udc->gadget->max_lsm);
>> +		lanes = min(lane_count, udc->gadget->max_lane_count);
> considering that lsm and lane_count are from 0 to their respective max
> values, do you need a min() here? Might be better to WARN() when either
> in over their max values.

Sure. That'd be better.

>
>> +		udc->gadget->ops->udc_set_sublink_attr(udc->gadget,
>> +						       lanes, rate);
> indentation using spaces? (same above, please fix)

It's both tab and spaces to align to next to the above open parentheses. 
It's based on checkpatch.

>
>> @@ -1353,7 +1382,14 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri
>>   	udc->dev.driver = &driver->driver;
>>   	udc->gadget->dev.driver = &driver->driver;
>>   
>> -	usb_gadget_udc_set_speed(udc, driver->max_speed);
>> +	if (udc->gadget->ops->udc_set_sublink_attr &&
>> +	    udc->gadget->max_speed == USB_SPEED_SUPER_PLUS &&
>> +	    driver->max_lsm && driver->max_lane_count &&
>> +	    driver->max_speed == USB_SPEED_SUPER_PLUS)
> So if driver doesn't provide max_lsm and max_speed you don't set sublink
> attr? Won't this cause problems? Also, the sublink_attr is still,
> conceptually, setting the max speed for the bus, right? So you may want
> to call usb_gadget_udc_set_sublink_attr() from inside
> usb_gadget_udc_set_speed(), then we don't need to modify all the callers.
>

The idea was that if the driver doesn't provide max_lsm and max_speed, 
then it's not constrained by the number of lanes or lsm. It will 
fallback to the usb_gadget_udc_set_speed().

I didn't think about creating a new usb_sublink_attr structure, so I 
couldn't use usb_gadget_udc_set_sublink_attr() inside 
usb_gadget_udc_set_speed() before.

Thanks for the review!

Thinh

Re: [RFC PATCH 03/14] usb: composite: Properly report lsm

[Thinh Nguyen]

Hi,

Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>> Use the max lane speed mantisa value from the gadget and report to the
>> device descriptor if available.
>>
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>>   drivers/usb/gadget/composite.c | 14 ++++++++++----
>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
>> index a4de5a8c0f19..cd38078d6a42 100644
>> --- a/drivers/usb/gadget/composite.c
>> +++ b/drivers/usb/gadget/composite.c
>> @@ -682,6 +682,11 @@ static int bos_desc(struct usb_composite_dev *cdev)
>>   	/* The SuperSpeedPlus USB Device Capability descriptor */
>>   	if (gadget_is_superspeed_plus(cdev->gadget)) {
>>   		struct usb_ssp_cap_descriptor *ssp_cap;
>> +		int lsm = 10;
>> +
>> +		if (cdev->gadget->ops->udc_set_sublink_attr &&
>> +		    cdev->gadget->max_lsm)
>> +			lsm = cdev->gadget->max_lsm;
>>   
>>   		ssp_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
>>   		bos->bNumDeviceCaps++;
>> @@ -709,20 +714,21 @@ static int bos_desc(struct usb_composite_dev *cdev)
>>   		 *   ST  = Symmetric, RX
>>   		 *   LSE =  3 (Gbps)
>>   		 *   LP  =  1 (SuperSpeedPlus)
>> -		 *   LSM = 10 (10 Gbps)
>> +		 *   LSM =  5 or 10
>>   		 */
>>   		ssp_cap->bmSublinkSpeedAttr[0] =
>> -			cpu_to_le32((3 << 4) | (1 << 14) | (0xa << 16));
>> +			cpu_to_le32((3 << 4) | (1 << 14) | (lsm << 16));
> while at that, can we get rid of the magic constants?

Ok.

>
>>   		ssp_cap->bmSublinkSpeedAttr[1] =
>>   			cpu_to_le32((3 << 4) | (1 << 14) |
>> -				    (0xa << 16) | (1 << 7));
>> +				    (lsm << 16) | (1 << 7));
> ditto.
>

Thanks,
Thinh

Re: [RFC PATCH 05/14] usb: dwc3: Update IP checks to support DWC_usb32

[Thinh Nguyen]

Hi,

Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>> @@ -1009,7 +1011,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>   	 * the DWC_usb3 controller. It is NOT available in the
> perhaps update the comment here? "It is available ONLY in the DWC_usb30
> controller".

Ah.. I missed that.

Thanks,
Thinh

Re: [RFC PATCH 07/14] usb: dwc3: gadget: Set lane count and lsm

[Thinh Nguyen]

Hi,

Felipe Balbi wrote:
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>
>> DWC_usb32 supports dual-lane at different transfer rate. This patch
>> initializes the controller to use the maximum support transfer rate
>> describes by the dwc3 device property of lane count and lane speed
>> mantissa in Gbps.
> Perhaps this should read as:
>
> 	"DWC_usb32 supports dual-lane at different transfer rate. This
> 	patch initializes the controller to use the maximum supported
> 	transfer rate described by the dwc3 device property of lane
> 	count and lane speed mantissa in Gbps."

Ok.

>> @@ -1424,6 +1428,38 @@ static void dwc3_check_params(struct dwc3 *dwc)
>>   
>>   		break;
>>   	}
>> +
>> +	switch (dwc->maximum_lsm) {
>> +	case 5:
>> +		break;
>> +	case 10:
>> +		if (dwc->maximum_speed == USB_SPEED_SUPER)
>> +			dev_err(dev, "invalid maximum_lsm parameter %d\n",
>> +				dwc->maximum_lsm);
>> +		/* Fall Through */
>> +	default:
>> +		if (dwc->maximum_speed == USB_SPEED_SUPER)
>> +			dwc->maximum_lsm = 5;
>> +		else if (dwc->maximum_speed > USB_SPEED_SUPER)
>> +			dwc->maximum_lsm = 10;
>> +		break;
>> +	}
>> +
>> +	switch (dwc->maximum_lanes) {
>> +	case 1:
>> +	case 2:
>> +		break;
>> +	default:
>> +		if (dwc->maximum_lanes > 2)
>> +			dev_err(dev, "invalid number of lanes %d\n",
>> +				dwc->maximum_lanes);
>> +
>> +		if (dwc3_is_usb32(dwc) &&
>> +		    dwc->maximum_speed == USB_SPEED_SUPER_PLUS)
>> +			dwc->maximum_lanes = 2;
>> +		else
>> +			dwc->maximum_lanes = 1;
>> +	}
>>   }
>>   
>>   static int dwc3_probe(struct platform_device *pdev)
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 7fde3c7da543..8e729d4cd5bd 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -376,6 +376,8 @@
>>   #define DWC3_GUCTL2_RST_ACTBITLATER		BIT(14)
>>   
>>   /* Device Configuration Register */
>> +#define DWC3_DCFG_NUMLANES(n)	(((n) & 0x3) << 30) /* DWC_usb32 only */
>> +
>>   #define DWC3_DCFG_DEVADDR(addr)	((addr) << 3)
>>   #define DWC3_DCFG_DEVADDR_MASK	DWC3_DCFG_DEVADDR(0x7f)
>>   
>> @@ -449,6 +451,8 @@
>>   #define DWC3_DEVTEN_USBRSTEN		BIT(1)
>>   #define DWC3_DEVTEN_DISCONNEVTEN	BIT(0)
>>   
>> +#define DWC3_DSTS_CONNLANES(n)		(((n) >> 30) & 0x3) /* DWC_usb32 only */
>> +
>>   /* Device Status Register */
>>   #define DWC3_DSTS_DCNRD			BIT(29)
>>   
>> @@ -946,6 +950,8 @@ struct dwc3_scratchpad_array {
>>    * @nr_scratch: number of scratch buffers
>>    * @u1u2: only used on revisions <1.83a for workaround
>>    * @maximum_speed: maximum speed requested (mainly for testing purposes)
>> + * @maximum_lsm: maximum lane speed mantissa in Gbps
>> + * @maximum_lanes: maximum lane count
>>    * @ip: controller's ID
>>    * @revision: controller's version of an IP
>>    * @version_type: VERSIONTYPE register contents, a sub release of a revision
>> @@ -973,6 +979,7 @@ struct dwc3_scratchpad_array {
>>    * @ep0state: state of endpoint zero
>>    * @link_state: link state
>>    * @speed: device speed (super, high, full, low)
>> + * @lane_count: number of connected lanes
>>    * @hwparams: copy of hwparams registers
>>    * @root: debugfs root folder pointer
>>    * @regset: debugfs pointer to regdump file
>> @@ -1100,6 +1107,8 @@ struct dwc3 {
>>   	u32			nr_scratch;
>>   	u32			u1u2;
>>   	u32			maximum_speed;
>> +	u8			maximum_lsm;
>> +	u8			maximum_lanes;
>>   
>>   	u32			ip;
>>   
>> @@ -1159,6 +1168,7 @@ struct dwc3 {
>>   	u8			u1pel;
>>   
>>   	u8			speed;
>> +	u8			lane_count;
>>   
>>   	u8			num_eps;
>>   
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index a6d562e208a9..c31144af3261 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2183,6 +2183,53 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
>>   	spin_unlock_irqrestore(&dwc->lock, flags);
>>   }
>>   
>> +static void dwc3_gadget_set_sublink_attr(struct usb_gadget *g,
>> +					 unsigned int lane_count,
>> +					 unsigned int lsm)
>> +{
>> +	struct dwc3	*dwc = gadget_to_dwc(g);
>> +	unsigned int	lanes;
>> +	unsigned long	flags;
>> +	u32		reg;
>> +
>> +	spin_lock_irqsave(&dwc->lock, flags);
>> +	if (dwc->maximum_speed <= USB_SPEED_SUPER) {
>> +		/* Fall back to maximum speed supported by HW */
>> +		spin_unlock_irqrestore(&dwc->lock, flags);
>> +		dwc3_gadget_set_speed(g, dwc->maximum_speed);
>> +		spin_lock_irqsave(&dwc->lock, flags);
> it looks like we should extract a __dwc3_gadget_set_speed() to avoid the
> possible race here:
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index a9aba716bf80..e317b696029e 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2118,14 +2118,11 @@ static void dwc3_gadget_config_params(struct usb_gadget *g,
>   				cpu_to_le16(DWC3_DEFAULT_U2_DEV_EXIT_LAT);
>   }
>   
> -static void dwc3_gadget_set_speed(struct usb_gadget *g,
> +static void __dwc3_gadget_set_speed(struct dwc3 *dwc,
>   				  enum usb_device_speed speed)
>   {
> -	struct dwc3		*dwc = gadget_to_dwc(g);
> -	unsigned long		flags;
>   	u32			reg;
>   
> -	spin_lock_irqsave(&dwc->lock, flags);
>   	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
>   	reg &= ~(DWC3_DCFG_SPEED_MASK);
>   
> @@ -2175,7 +2172,16 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
>   		}
>   	}
>   	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
> +}
> +
> +static void dwc3_gadget_set_speed(struct usb_gadget *g,
> +				  enum usb_device_speed speed)
> +{
> +	struct dwc3		*dwc = gadget_to_dwc(g);
> +	unsigned long		flags;
>   
> +	spin_lock_irqsave(&dwc->lock, flags);
> +	__dwc3_gadget_set_speed(dwc, speed);
>   	spin_unlock_irqrestore(&dwc->lock, flags);
>   }
>   
> Then your patch would look like:
>
> static void dwc3_gadget_set_sublink_attr(struct usb_gadget *g,
> 					 unsigned int lane_count,
> 					 unsigned int lsm)
> {
> 	struct dwc3	*dwc = gadget_to_dwc(g);
> 	unsigned int	lanes;
> 	unsigned long	flags;
> 	u32		reg;
>
> 	spin_lock_irqsave(&dwc->lock, flags);
> 	if (dwc->maximum_speed <= USB_SPEED_SUPER) {
> 		/* Fall back to maximum speed supported by HW */
> 		__dwc3_gadget_set_speed(dwc, dwc->maximum_speed);
> 		goto done;
> 	}
>
> 	[...]
> }

Ok. I'll revise this.

Thanks!
Thinh

Re: [RFC PATCH 11/14] usb: devicetree: dwc3: Add TRB prefetch count

[Thinh Nguyen]

Hi,

Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>> DWC_usb32 has an enhancement that allows the controller to cache
>> multiple TRBs for non-control endpoints. Introduce a new property to
>> DWC3 to set the maximum number of TRBs to cache in advance. The property
>> can be set from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER (CoreConsultant
>> value). By default, the number of cache TRB is
>> DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>>
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>>   Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>> index 7da1c4e7d380..ff35fa6de2eb 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>> @@ -105,6 +105,9 @@ Optional properties:
>>   			this and tx-thr-num-pkt-prd to a valid, non-zero value
>>   			1-16 (DWC_usb31 programming guide section 1.2.3) to
>>   			enable periodic ESS TX threshold.
>> + - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
>> +			can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>> +			Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
> how about we just leave it at the maximum and in case someone notices
> problems, then we consider adding more DT bindings?
>

Sure. We can do that. I'll remove this.

Thinh

Re: [RFC PATCH 13/14] usb: devicetree: dwc3: Add property to disable mult TRB fetch

[Thinh Nguyen]

Hi,

Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>> DWC_usb32 has a feature where it can issue multiple TRB fetch requests.
>> Add a new property to limit and only do only single TRB fetch request.
>>
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>>   Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>> index ff35fa6de2eb..29d6f9b1fc70 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>> @@ -108,6 +108,8 @@ Optional properties:
>>    - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
>>   			can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>>   			Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>> + - snps,dis-mult-trb-fetch: set to issue only single TRB fetch request in
>> +			DWC_usb32.
> two questions:
>
> - how is this different from passing 1 to the previous DT binding

The previous DT binding is related to the number TRBs to cache while 
this one is related to whether the controller will send multiple 
(internal) fetch commands to fetch the TRBs.

> - do we know of anybody having issues with multi-trb prefetch?

No, we added this for various internal tests.

BR,
Thinh

Re: [RFC PATCH 02/14] usb: gadget: Add callback to set lane and transfer rate

[Thinh Nguyen]

Hi,

Alan Stern wrote:
> On Thu, 12 Dec 2019, Felipe Balbi wrote:
>
>> Hi,
>>
>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>>> Introduce gadget opts udc_set_sublink_speed callback to set the lane
>>> count and transfer rate (in lane speed mantissa of Gbps) for SuperSpeed
>>> Plus capable gadgets. In the same way udc_set_speed, this function can
>>> control the gadget's sublink attributes for SuperSpeed Plus.
>>>
>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>> ---
>>>   drivers/usb/gadget/composite.c           |  2 ++
>>>   drivers/usb/gadget/legacy/mass_storage.c |  2 ++
>> I would rather not add new features to the legacy gadgets and focus on
>> our configfs interface for anything new. Moreover, using the feature
>> you introduced could, arguably, be done as a separate patch.
>>
>>> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
>>> index 3b4f67000315..a4de5a8c0f19 100644
>>> --- a/drivers/usb/gadget/composite.c
>>> +++ b/drivers/usb/gadget/composite.c
>>> @@ -2353,6 +2353,8 @@ int usb_composite_probe(struct usb_composite_driver *driver)
>>>   	gadget_driver->function =  (char *) driver->name;
>>>   	gadget_driver->driver.name = driver->name;
>>>   	gadget_driver->max_speed = driver->max_speed;
>>> +	gadget_driver->max_lane_count = driver->max_lane_count;
>>> +	gadget_driver->max_lsm = driver->max_lsm;
>>>   
>>>   	return usb_gadget_probe_driver(gadget_driver);
>>>   }
>>> diff --git a/drivers/usb/gadget/legacy/mass_storage.c b/drivers/usb/gadget/legacy/mass_storage.c
>>> index f18f77584fc2..a0912c5afffc 100644
>>> --- a/drivers/usb/gadget/legacy/mass_storage.c
>>> +++ b/drivers/usb/gadget/legacy/mass_storage.c
>>> @@ -223,6 +223,8 @@ static struct usb_composite_driver msg_driver = {
>>>   	.name		= "g_mass_storage",
>>>   	.dev		= &msg_device_desc,
>>>   	.max_speed	= USB_SPEED_SUPER_PLUS,
>>> +	.max_lane_count	= 2,
>>> +	.max_lsm	= 10,
>> Right, as mentioned, I'd prefer not touch the legacy gadgets. But in any
>> case, why is it so that the gadget is telling you about max lane count
>> and lsm? That should be abstracted away from the gadget driver. Gadget
>> driver shouldn't have knowledge of number of lanes because, at the end
>> of the day, that doesn't really change anything in practice. Unlike HS
>> vs SS which changes a bunch of things.
> I agree completely.  Furthermore, it isn't at all clear where those two
> numbers came from.  Why would g-mass-storage care that lane_count <= 2
> and lsm <= 10?
>
> Alan Stern
>

Right, I'll remove this and update according to Felipe's suggestions.

Thanks,
Thinh

Re: [RFC PATCH 13/14] usb: devicetree: dwc3: Add property to disable mult TRB fetch

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1754 bytes --]


Hi,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>>> DWC_usb32 has a feature where it can issue multiple TRB fetch requests.
>>> Add a new property to limit and only do only single TRB fetch request.
>>>
>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>> ---
>>>   Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> index ff35fa6de2eb..29d6f9b1fc70 100644
>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> @@ -108,6 +108,8 @@ Optional properties:
>>>    - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
>>>   			can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>>>   			Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>>> + - snps,dis-mult-trb-fetch: set to issue only single TRB fetch request in
>>> +			DWC_usb32.
>> two questions:
>>
>> - how is this different from passing 1 to the previous DT binding
>
> The previous DT binding is related to the number TRBs to cache while 
> this one is related to whether the controller will send multiple 
> (internal) fetch commands to fetch the TRBs.
>
>> - do we know of anybody having issues with multi-trb prefetch?
>
> No, we added this for various internal tests.

We really a better way for you guys to have your test coverage enabled
with upstream kernel. I wonder if DT guys would accept a set of bindings
marked as "for testing purposes". In any case, we really need to enable
Silicon Validation with upstream kernel.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [RFC PATCH 13/14] usb: devicetree: dwc3: Add property to disable mult TRB fetch

[Thinh Nguyen]

Hi,

Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>>>> DWC_usb32 has a feature where it can issue multiple TRB fetch requests.
>>>> Add a new property to limit and only do only single TRB fetch request.
>>>>
>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>> index ff35fa6de2eb..29d6f9b1fc70 100644
>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>> @@ -108,6 +108,8 @@ Optional properties:
>>>>     - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
>>>>    			can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>>>>    			Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>>>> + - snps,dis-mult-trb-fetch: set to issue only single TRB fetch request in
>>>> +			DWC_usb32.
>>> two questions:
>>>
>>> - how is this different from passing 1 to the previous DT binding
>> The previous DT binding is related to the number TRBs to cache while
>> this one is related to whether the controller will send multiple
>> (internal) fetch commands to fetch the TRBs.
>>
>>> - do we know of anybody having issues with multi-trb prefetch?
>> No, we added this for various internal tests.
> We really a better way for you guys to have your test coverage enabled
> with upstream kernel. I wonder if DT guys would accept a set of bindings
> marked as "for testing purposes". In any case, we really need to enable
> Silicon Validation with upstream kernel.
>

That would be great! If there's a sensible way to do so, we're open to 
suggestions.

Thanks,
Thinh

Re: [RFC PATCH 06/14] usb: devicetree: dwc3: Add max lane and lsm

[Rob Herring]

On Wed, Dec 11, 2019 at 06:49:37PM -0800, Thinh Nguyen wrote:
> Add a new property to set maximum number of lanes and transfer rated
> supported for DWC_usb32. By default, the driver will configure the
> controller to use dual-lane at 10Gbps.
> 
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
>  Documentation/devicetree/bindings/usb/dwc3.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> index 66780a47ad85..7da1c4e7d380 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -85,6 +85,10 @@ Optional properties:
>   - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ
>  	register for post-silicon frame length adjustment when the
>  	fladj_30mhz_sdbnd signal is invalid or incorrect.
> + - snps,maximum-lane-count: set to specify the number of lanes to use for
> +			DWC_usb32 and later. Default is dual-lanes.

Why do you need this? When is it not the number of lanes the phy has?

Reuse 'num-lanes' from PCI binding?

> + - snps,maximum-lsm: set to specify the lane speed mantissa to use in Gbps.
> + 			Default is 10Gbps for SuperSpeed Plus.

So the value is '10' or '10Gbps'. Other valid values?

>   - snps,rx-thr-num-pkt-prd: periodic ESS RX packet threshold count - host mode
>  			only. Set this and rx-max-burst-prd to a valid,
>  			non-zero value 1-16 (DWC_usb31 programming guide
> -- 
> 2.11.0
> 

Re: [RFC PATCH 13/14] usb: devicetree: dwc3: Add property to disable mult TRB fetch

[Rob Herring]

On Fri, Dec 13, 2019 at 09:04:54AM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> >> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> >>> DWC_usb32 has a feature where it can issue multiple TRB fetch requests.
> >>> Add a new property to limit and only do only single TRB fetch request.
> >>>
> >>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> >>> ---
> >>>   Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
> >>>   1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> >>> index ff35fa6de2eb..29d6f9b1fc70 100644
> >>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> >>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> >>> @@ -108,6 +108,8 @@ Optional properties:
> >>>    - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
> >>>   			can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
> >>>   			Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
> >>> + - snps,dis-mult-trb-fetch: set to issue only single TRB fetch request in
> >>> +			DWC_usb32.
> >> two questions:
> >>
> >> - how is this different from passing 1 to the previous DT binding
> >
> > The previous DT binding is related to the number TRBs to cache while 
> > this one is related to whether the controller will send multiple 
> > (internal) fetch commands to fetch the TRBs.
> >
> >> - do we know of anybody having issues with multi-trb prefetch?
> >
> > No, we added this for various internal tests.
> 
> We really a better way for you guys to have your test coverage enabled
> with upstream kernel. I wonder if DT guys would accept a set of bindings
> marked as "for testing purposes". In any case, we really need to enable
> Silicon Validation with upstream kernel.

Well, anything would be better than the endless stream of new 
properties. Include 'test-mode' in the property names would be fine I 
guess.

However, why do they need to be in DT rather than module params or 
debugfs settings? Changing at runtime or reloading the module is a 
better experience than editting a DT and rebooting.

Rob

Re: [RFC PATCH 06/14] usb: devicetree: dwc3: Add max lane and lsm

[Thinh Nguyen]

Hi,

Rob Herring wrote:
> On Wed, Dec 11, 2019 at 06:49:37PM -0800, Thinh Nguyen wrote:
>> Add a new property to set maximum number of lanes and transfer rated
>> supported for DWC_usb32. By default, the driver will configure the
>> controller to use dual-lane at 10Gbps.
>>
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>>   Documentation/devicetree/bindings/usb/dwc3.txt | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>> index 66780a47ad85..7da1c4e7d380 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>> @@ -85,6 +85,10 @@ Optional properties:
>>    - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ
>>   	register for post-silicon frame length adjustment when the
>>   	fladj_30mhz_sdbnd signal is invalid or incorrect.
>> + - snps,maximum-lane-count: set to specify the number of lanes to use for
>> +			DWC_usb32 and later. Default is dual-lanes.
> Why do you need this? When is it not the number of lanes the phy has?

The issue is the controller doesn't know the number of lanes the phy 
supports. There's no HW parameter for this. The user needs to inform 
this to the controller.

>
> Reuse 'num-lanes' from PCI binding?

Ok.

>
>> + - snps,maximum-lsm: set to specify the lane speed mantissa to use in Gbps.
>> + 			Default is 10Gbps for SuperSpeed Plus.
> So the value is '10' or '10Gbps'. Other valid values?

I'm missing this in the description. It can be either 10 or 5. If not 
set, then default to 10Gbps for SSP. I'll update this.

>
>>    - snps,rx-thr-num-pkt-prd: periodic ESS RX packet threshold count - host mode
>>   			only. Set this and rx-max-burst-prd to a valid,
>>   			non-zero value 1-16 (DWC_usb31 programming guide
>> -- 
>> 2.11.0
>>

Thanks,
Thinh

Re: [RFC PATCH 13/14] usb: devicetree: dwc3: Add property to disable mult TRB fetch

[Thinh Nguyen]

Hi,

Rob Herring wrote:
> On Fri, Dec 13, 2019 at 09:04:54AM +0200, Felipe Balbi wrote:
>> Hi,
>>
>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>>>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>>>>> DWC_usb32 has a feature where it can issue multiple TRB fetch requests.
>>>>> Add a new property to limit and only do only single TRB fetch request.
>>>>>
>>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>>>> ---
>>>>>    Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>> index ff35fa6de2eb..29d6f9b1fc70 100644
>>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>> @@ -108,6 +108,8 @@ Optional properties:
>>>>>     - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
>>>>>    			can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>>>>>    			Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>>>>> + - snps,dis-mult-trb-fetch: set to issue only single TRB fetch request in
>>>>> +			DWC_usb32.
>>>> two questions:
>>>>
>>>> - how is this different from passing 1 to the previous DT binding
>>> The previous DT binding is related to the number TRBs to cache while
>>> this one is related to whether the controller will send multiple
>>> (internal) fetch commands to fetch the TRBs.
>>>
>>>> - do we know of anybody having issues with multi-trb prefetch?
>>> No, we added this for various internal tests.
>> We really a better way for you guys to have your test coverage enabled
>> with upstream kernel. I wonder if DT guys would accept a set of bindings
>> marked as "for testing purposes". In any case, we really need to enable
>> Silicon Validation with upstream kernel.
> Well, anything would be better than the endless stream of new
> properties. Include 'test-mode' in the property names would be fine I
> guess.
>

Generally the properties are for some quirks or configurations that the 
controller must use to work properly and not for debugging purposes. 
Unfortunately, this list can be long..

> However, why do they need to be in DT rather than module params or
> debugfs settings? Changing at runtime or reloading the module is a
> better experience than editting a DT and rebooting.
>
> Rob

Internally, our tool can debug different properties as if they are 
module parameters. They can be changed at runtime. Setting properties is 
one of the options which we can configure without tampering much of the 
existing code.

BR,
Thinh

Re: [RFC PATCH 13/14] usb: devicetree: dwc3: Add property to disable mult TRB fetch

[Rob Herring]

On Thu, Dec 19, 2019 at 3:52 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
>
> Hi,
>
> Rob Herring wrote:
> > On Fri, Dec 13, 2019 at 09:04:54AM +0200, Felipe Balbi wrote:
> >> Hi,
> >>
> >> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> >>>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> >>>>> DWC_usb32 has a feature where it can issue multiple TRB fetch requests.
> >>>>> Add a new property to limit and only do only single TRB fetch request.
> >>>>>
> >>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> >>>>> ---
> >>>>>    Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
> >>>>>    1 file changed, 2 insertions(+)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>> index ff35fa6de2eb..29d6f9b1fc70 100644
> >>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>> @@ -108,6 +108,8 @@ Optional properties:
> >>>>>     - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
> >>>>>                           can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
> >>>>>                           Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
> >>>>> + - snps,dis-mult-trb-fetch: set to issue only single TRB fetch request in
> >>>>> +                 DWC_usb32.
> >>>> two questions:
> >>>>
> >>>> - how is this different from passing 1 to the previous DT binding
> >>> The previous DT binding is related to the number TRBs to cache while
> >>> this one is related to whether the controller will send multiple
> >>> (internal) fetch commands to fetch the TRBs.
> >>>
> >>>> - do we know of anybody having issues with multi-trb prefetch?
> >>> No, we added this for various internal tests.
> >> We really a better way for you guys to have your test coverage enabled
> >> with upstream kernel. I wonder if DT guys would accept a set of bindings
> >> marked as "for testing purposes". In any case, we really need to enable
> >> Silicon Validation with upstream kernel.
> > Well, anything would be better than the endless stream of new
> > properties. Include 'test-mode' in the property names would be fine I
> > guess.
> >
>
> Generally the properties are for some quirks or configurations that the
> controller must use to work properly and not for debugging purposes.
> Unfortunately, this list can be long..

quirks or testing? Now I'm confused, which is it?

I'm sure I've said this before (for DWC3), but it is better to have a
compatible string for each implementation (usually an SoC) so you can
address new quirks without a dtb change and continually adding new
properties.

Rob

Re: [RFC PATCH 13/14] usb: devicetree: dwc3: Add property to disable mult TRB fetch

[Thinh Nguyen]

Hi,

Rob Herring wrote:
> On Thu, Dec 19, 2019 at 3:52 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
>> Hi,
>>
>> Rob Herring wrote:
>>> On Fri, Dec 13, 2019 at 09:04:54AM +0200, Felipe Balbi wrote:
>>>> Hi,
>>>>
>>>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>>>>>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>>>>>>> DWC_usb32 has a feature where it can issue multiple TRB fetch requests.
>>>>>>> Add a new property to limit and only do only single TRB fetch request.
>>>>>>>
>>>>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>>>>>> ---
>>>>>>>     Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>>>>>>>     1 file changed, 2 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>>>> index ff35fa6de2eb..29d6f9b1fc70 100644
>>>>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>>>> @@ -108,6 +108,8 @@ Optional properties:
>>>>>>>      - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
>>>>>>>                            can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>>>>>>>                            Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
>>>>>>> + - snps,dis-mult-trb-fetch: set to issue only single TRB fetch request in
>>>>>>> +                 DWC_usb32.
>>>>>> two questions:
>>>>>>
>>>>>> - how is this different from passing 1 to the previous DT binding
>>>>> The previous DT binding is related to the number TRBs to cache while
>>>>> this one is related to whether the controller will send multiple
>>>>> (internal) fetch commands to fetch the TRBs.
>>>>>
>>>>>> - do we know of anybody having issues with multi-trb prefetch?
>>>>> No, we added this for various internal tests.
>>>> We really a better way for you guys to have your test coverage enabled
>>>> with upstream kernel. I wonder if DT guys would accept a set of bindings
>>>> marked as "for testing purposes". In any case, we really need to enable
>>>> Silicon Validation with upstream kernel.
>>> Well, anything would be better than the endless stream of new
>>> properties. Include 'test-mode' in the property names would be fine I
>>> guess.
>>>
>> Generally the properties are for some quirks or configurations that the
>> controller must use to work properly and not for debugging purposes.
>> Unfortunately, this list can be long..
> quirks or testing? Now I'm confused, which is it?

I was referring to the existing properties, they are necessary for a 
working device. Nothing "extra" solely for debugging purposes.

With the exception for these couple properties related to TRB fetching 
in this RFC series, they are for testing only. Regardless, I agreed to 
Felipe previously that we can remove them.


> I'm sure I've said this before (for DWC3), but it is better to have a
> compatible string for each implementation (usually an SoC) so you can
> address new quirks without a dtb change and continually adding new
> properties.
>
> Rob

Yes, you mentioned it before. It may work on SoC, but it won't work for 
PCI devices. We can't use VID:PID either, because the HAPS devices only 
have a set of VID:PID.

Please refer back to a couple discussions back:
1) https://www.spinics.net/lists/linux-usb/msg164494.html

This one is related to cadence's, but I think it's relevant:
2) https://www.spinics.net/lists/linux-usb/msg175199.html

There maybe more discussions previously, but I don't have the history of 
all the emails.

BR,
Thinh