Re: [PATCH v2 1/1] usb: dwc2: Fix the write to W1C fields in HPRT register

Tue, 20 Jun 2023 14:38:23 -0700 

On 6/20/23 15:52, [email protected] wrote:

From: Teik Heng Chong <[email protected]>

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

Signed-off-by: Teik Heng Chong <[email protected]>

---

V1->V2
- update subject tags to usb:dwc2
---
  drivers/usb/host/dwc2.c | 34 ++++++++--------------------------
  drivers/usb/host/dwc2.h |  4 ++++
  2 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
index 23060fc369..9818f9be94 100644
--- a/drivers/usb/host/dwc2.c
+++ b/drivers/usb/host/dwc2.c
@@ -315,9 +315,7 @@ static void dwc_otg_core_host_init(struct udevice *dev,
/* 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 @@ static int dwc_otg_submit_rh_msg_out(struct dwc2_priv *priv,
        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 @@ static int dwc_otg_submit_rh_msg_out(struct dwc2_priv 
*priv,
                        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 @@ static int dwc2_init_common(struct udevice *dev, struct 
dwc2_priv *priv)
                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 int dwc2_init_common(struct udevice *dev, struct 
dwc2_priv *priv)
  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) 
diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h
index a6f562fe60..6f022e33a1 100644
--- a/drivers/usb/host/dwc2.h
+++ b/drivers/usb/host/dwc2.h
@@ -543,6 +543,10 @@ struct dwc2_core_regs {
  #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)


+CC ST

Is there an actual bug this is solving ? Details please ?

Reply via email to