| Message ID | c493e890dc8f02500fa1356353b5ab6c830b9cf1.1571770732.git.thinhn@synopsys.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: Link state changes | expand |
Hi Thinh, Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes: > DCTL.ULSTCHNGREQ is a write-only field. When doing a read-modify-write > to DCTL, the driver must make sure that there's no unintended link state > change request from whatever is read from DCTL.ULSTCHNGREQ. Set link > state change to no-action when the driver writes to DCTL. > > Signed-off-by: Thinh Nguyen <thinhn@synopsys.com> WHAT A GREAT CATCH!!! However, let's do one small change here: > --- > drivers/usb/dwc3/gadget.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 86dc1db788a9..24178b4b9d46 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -57,6 +57,8 @@ int dwc3_gadget_set_test_mode(struct dwc3 *dwc, int mode) > return -EINVAL; > } > > + /* Don't send link state change request */ > + reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK; > dwc3_writel(dwc->regs, DWC3_DCTL, reg); Let's a small macro or a little function to wrap this. Something dwc3_dctl_write_safe() or something long those line. Then that macro/function will make sure to clear those bits.
Hi, Felipe Balbi wrote: > Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes: >> DCTL.ULSTCHNGREQ is a write-only field. When doing a read-modify-write >> to DCTL, the driver must make sure that there's no unintended link state >> change request from whatever is read from DCTL.ULSTCHNGREQ. Set link >> state change to no-action when the driver writes to DCTL. >> >> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com> > WHAT A GREAT CATCH!!! However, let's do one small change here: > >> --- >> drivers/usb/dwc3/gadget.c | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index 86dc1db788a9..24178b4b9d46 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -57,6 +57,8 @@ int dwc3_gadget_set_test_mode(struct dwc3 *dwc, int mode) >> return -EINVAL; >> } >> >> + /* Don't send link state change request */ >> + reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK; >> dwc3_writel(dwc->regs, DWC3_DCTL, reg); > Let's a small macro or a little function to wrap this. Something > dwc3_dctl_write_safe() or something long those line. > > Then that macro/function will make sure to clear those bits. Sure. I'll resend with the change. Thanks, Thinh
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 86dc1db788a9..24178b4b9d46 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -57,6 +57,8 @@ int dwc3_gadget_set_test_mode(struct dwc3 *dwc, int mode) return -EINVAL; } + /* Don't send link state change request */ + reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK; dwc3_writel(dwc->regs, DWC3_DCTL, reg); return 0; @@ -1822,6 +1824,9 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend) dwc->pullups_connected = false; } + /* Don't send link state change request */ + reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK; + dwc3_writel(dwc->regs, DWC3_DCTL, reg); do { @@ -2744,6 +2749,10 @@ static void dwc3_gadget_disconnect_interrupt(struct dwc3 *dwc) int reg; reg = dwc3_readl(dwc->regs, DWC3_DCTL); + + /* Don't send link state change request */ + reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK; + reg &= ~DWC3_DCTL_INITU1ENA; dwc3_writel(dwc->regs, DWC3_DCTL, reg); @@ -2799,6 +2808,10 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc) dwc3_reset_gadget(dwc); reg = dwc3_readl(dwc->regs, DWC3_DCTL); + + /* Don't send link state change request */ + reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK; + reg &= ~DWC3_DCTL_TSTCTRL_MASK; dwc3_writel(dwc->regs, DWC3_DCTL, reg); dwc->test_mode = false; @@ -2904,10 +2917,15 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc) if (dwc->has_lpm_erratum && dwc->revision >= DWC3_REVISION_240A) reg |= DWC3_DCTL_NYET_THRES(dwc->lpm_nyet_threshold); + /* Don't send link state change request */ + reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK; dwc3_writel(dwc->regs, DWC3_DCTL, reg); } else { reg = dwc3_readl(dwc->regs, DWC3_DCTL); reg &= ~DWC3_DCTL_HIRD_THRES_MASK; + + /* Don't send link state change request */ + reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK; dwc3_writel(dwc->regs, DWC3_DCTL, reg); } @@ -3017,6 +3035,9 @@ static void dwc3_gadget_linksts_change_interrupt(struct dwc3 *dwc, reg &= ~u1u2; + /* Don't send link state change request */ + reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK; + dwc3_writel(dwc->regs, DWC3_DCTL, reg); break; default:
DCTL.ULSTCHNGREQ is a write-only field. When doing a read-modify-write to DCTL, the driver must make sure that there's no unintended link state change request from whatever is read from DCTL.ULSTCHNGREQ. Set link state change to no-action when the driver writes to DCTL. Signed-off-by: Thinh Nguyen <thinhn@synopsys.com> --- drivers/usb/dwc3/gadget.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)