commit 9715f3c7c6433f0c1a9a5eea9d2acb338c517b99
author Teik Heng Chong <teik.heng.chong@intel.com> Wed Jun 21 11:13:58 2023 +0800
committer Marek Vasut <marex@denx.de> Wed Jun 21 13:15:58 2023 +0200
tree cf8e2042b7ff8b7d9f3b80dc901a1dbed8f03642
parent 914b1196c01c984abfa1bc2d79c695734e2c1cbf

usb: dwc2: Fix the write to W1C fields in HPRT register

Fix the write to the HPRT register which treat W1C fields
as if they were mere RW. This leads to unintended clearing of such fields

This bug was found during the testing on Simics model. Referring to
specification DesignWare Cores USB 2.0 Hi-Speed On-The-Go (OTG)
Databook (3.30a)"5.3.4.8 Host Port Control and Status Register (HPRT)", the
HPRT.PrtPwr is cleared by this mistake. In the Linux driver (contrary to
U-Boot), HPRT is always read using dwc2_read_hprt0 helper function which
clears W1C bits. So after write back those bits are zeroes.

Signed-off-by: Teik Heng Chong <teik.heng.chong@intel.com>

drivers/usb/host/dwc2.c

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
index 23060fc..9818f9b 100644
--- a/drivers/usb/host/dwc2.c
+++ b/drivers/usb/host/dwc2.c
@@ -315,9 +315,7 @@
 
 	/* Turn on the vbus power. */
 	if (readl(&regs->gintsts) & DWC2_GINTSTS_CURMODE_HOST) {
-		hprt0 = readl(&regs->hprt0);
-		hprt0 &= ~(DWC2_HPRT0_PRTENA | DWC2_HPRT0_PRTCONNDET);
-		hprt0 &= ~(DWC2_HPRT0_PRTENCHNG | DWC2_HPRT0_PRTOVRCURRCHNG);
+		hprt0 = readl(&regs->hprt0) & ~DWC2_HPRT0_W1C_MASK;
 		if (!(hprt0 & DWC2_HPRT0_PRTPWR)) {
 			hprt0 |= DWC2_HPRT0_PRTPWR;
 			writel(hprt0, &regs->hprt0);
@@ -748,7 +746,7 @@
 	case (USB_REQ_CLEAR_FEATURE << 8) | USB_RECIP_OTHER | USB_TYPE_CLASS:
 		switch (wValue) {
 		case USB_PORT_FEAT_C_CONNECTION:
-			setbits_le32(&regs->hprt0, DWC2_HPRT0_PRTCONNDET);
+			clrsetbits_le32(&regs->hprt0, DWC2_HPRT0_W1C_MASK, DWC2_HPRT0_PRTCONNDET);
 			break;
 		}
 		break;
@@ -759,21 +757,13 @@
 			break;
 
 		case USB_PORT_FEAT_RESET:
-			clrsetbits_le32(&regs->hprt0, DWC2_HPRT0_PRTENA |
-					DWC2_HPRT0_PRTCONNDET |
-					DWC2_HPRT0_PRTENCHNG |
-					DWC2_HPRT0_PRTOVRCURRCHNG,
-					DWC2_HPRT0_PRTRST);
+			clrsetbits_le32(&regs->hprt0, DWC2_HPRT0_W1C_MASK, DWC2_HPRT0_PRTRST);
 			mdelay(50);
-			clrbits_le32(&regs->hprt0, DWC2_HPRT0_PRTRST);
+			clrbits_le32(&regs->hprt0, DWC2_HPRT0_W1C_MASK | DWC2_HPRT0_PRTRST);
 			break;
 
 		case USB_PORT_FEAT_POWER:
-			clrsetbits_le32(&regs->hprt0, DWC2_HPRT0_PRTENA |
-					DWC2_HPRT0_PRTCONNDET |
-					DWC2_HPRT0_PRTENCHNG |
-					DWC2_HPRT0_PRTOVRCURRCHNG,
-					DWC2_HPRT0_PRTRST);
+			clrsetbits_le32(&regs->hprt0, DWC2_HPRT0_W1C_MASK, DWC2_HPRT0_PRTRST);
 			break;
 
 		case USB_PORT_FEAT_ENABLE:
@@ -1213,14 +1203,9 @@
 		dwc_otg_core_host_init(dev, regs);
 	}
 
-	clrsetbits_le32(&regs->hprt0, DWC2_HPRT0_PRTENA |
-			DWC2_HPRT0_PRTCONNDET | DWC2_HPRT0_PRTENCHNG |
-			DWC2_HPRT0_PRTOVRCURRCHNG,
-			DWC2_HPRT0_PRTRST);
+	clrsetbits_le32(&regs->hprt0, DWC2_HPRT0_W1C_MASK, DWC2_HPRT0_PRTRST);
 	mdelay(50);
-	clrbits_le32(&regs->hprt0, DWC2_HPRT0_PRTENA | DWC2_HPRT0_PRTCONNDET |
-		     DWC2_HPRT0_PRTENCHNG | DWC2_HPRT0_PRTOVRCURRCHNG |
-		     DWC2_HPRT0_PRTRST);
+	clrbits_le32(&regs->hprt0, DWC2_HPRT0_W1C_MASK | DWC2_HPRT0_PRTRST);
 
 	for (i = 0; i < MAX_DEVICE; i++) {
 		for (j = 0; j < MAX_ENDPOINT; j++) {
@@ -1246,10 +1231,7 @@
 static void dwc2_uninit_common(struct dwc2_core_regs *regs)
 {
 	/* Put everything in reset. */
-	clrsetbits_le32(&regs->hprt0, DWC2_HPRT0_PRTENA |
-			DWC2_HPRT0_PRTCONNDET | DWC2_HPRT0_PRTENCHNG |
-			DWC2_HPRT0_PRTOVRCURRCHNG,
-			DWC2_HPRT0_PRTRST);
+	clrsetbits_le32(&regs->hprt0, DWC2_HPRT0_W1C_MASK, DWC2_HPRT0_PRTRST);
 }
 
 #if !CONFIG_IS_ENABLED(DM_USB)

drivers/usb/host/dwc2.h

diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h
index a6f562f..6f022e3 100644
--- a/drivers/usb/host/dwc2.h
+++ b/drivers/usb/host/dwc2.h
@@ -543,6 +543,10 @@
 #define DWC2_HPRT0_PRTSPD_LOW				(2 << 17)
 #define DWC2_HPRT0_PRTSPD_MASK				(0x3 << 17)
 #define DWC2_HPRT0_PRTSPD_OFFSET			17
+#define DWC2_HPRT0_W1C_MASK				(DWC2_HPRT0_PRTCONNDET | \
+							DWC2_HPRT0_PRTENA | \
+							DWC2_HPRT0_PRTENCHNG | \
+							DWC2_HPRT0_PRTOVRCURRCHNG)
 #define DWC2_HAINT_CH0					(1 << 0)
 #define DWC2_HAINT_CH0_OFFSET				0
 #define DWC2_HAINT_CH1					(1 << 1)