[PATCH] usb: dwc3: gadget: fix writing NYET threshold

[PATCH] usb: dwc3: gadget: fix writing NYET threshold

[André Draszik]

Before writing a new value to the register, the old value needs to be
masked out for the new value to be programmed as intended.

At the moment, the dwc3 core initialises the threshold to the maximum
value (0xf), with the option to override it via a DT. No upstream DTs
seem to override it, therefore this commit doesn't change behaviour for
any upstream platform. Nevertheless, the code should be fixed to have
the desired outcome.

Do so.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/usb/dwc3/core.h   | 1 +
 drivers/usb/dwc3/gadget.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index ee73789326bc..9335fd095968 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -465,6 +465,7 @@
 
 /* These apply for core versions 1.94a and later */
 #define DWC3_DCTL_NYET_THRES(n)		(((n) & 0xf) << 20)
+#define DWC3_DCTL_NYET_THRES_MASK	DWC3_DCTL_NYET_THRES(0xf)
 
 #define DWC3_DCTL_KEEP_CONNECT		BIT(19)
 #define DWC3_DCTL_L1_HIBER_EN		BIT(18)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 83dc7304d701..31a654c6f15b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -4195,8 +4195,10 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 		WARN_ONCE(DWC3_VER_IS_PRIOR(DWC3, 240A) && dwc->has_lpm_erratum,
 				"LPM Erratum not available on dwc3 revisions < 2.40a\n");
 
-		if (dwc->has_lpm_erratum && !DWC3_VER_IS_PRIOR(DWC3, 240A))
+		if (dwc->has_lpm_erratum && !DWC3_VER_IS_PRIOR(DWC3, 240A)) {
+			reg &= ~DWC3_DCTL_NYET_THRES_MASK;
 			reg |= DWC3_DCTL_NYET_THRES(dwc->lpm_nyet_threshold);
+		}
 
 		dwc3_gadget_dctl_write_safe(dwc, reg);
 	} else {

---
base-commit: c245a7a79602ccbee780c004c1e4abcda66aec32
change-id: 20241206-dwc3-nyet-fix-7085f6d71d04

Best regards,
-- 
André Draszik <andre.draszik@linaro.org>

Re: [PATCH] usb: dwc3: gadget: fix writing NYET threshold

[Thinh Nguyen]

On Fri, Dec 06, 2024, André Draszik wrote:
> Before writing a new value to the register, the old value needs to be
> masked out for the new value to be programmed as intended.
> 
> At the moment, the dwc3 core initialises the threshold to the maximum
> value (0xf), with the option to override it via a DT. No upstream DTs
> seem to override it, therefore this commit doesn't change behaviour for
> any upstream platform. Nevertheless, the code should be fixed to have
> the desired outcome.
> 
> Do so.
> 
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
>  drivers/usb/dwc3/core.h   | 1 +
>  drivers/usb/dwc3/gadget.c | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index ee73789326bc..9335fd095968 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -465,6 +465,7 @@
>  
>  /* These apply for core versions 1.94a and later */
>  #define DWC3_DCTL_NYET_THRES(n)		(((n) & 0xf) << 20)
> +#define DWC3_DCTL_NYET_THRES_MASK	DWC3_DCTL_NYET_THRES(0xf)

If you're going to define a mask macro this way, do this:
DWC3_DCTL_NYET_THRES(~0)

>  
>  #define DWC3_DCTL_KEEP_CONNECT		BIT(19)
>  #define DWC3_DCTL_L1_HIBER_EN		BIT(18)
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 83dc7304d701..31a654c6f15b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -4195,8 +4195,10 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
>  		WARN_ONCE(DWC3_VER_IS_PRIOR(DWC3, 240A) && dwc->has_lpm_erratum,
>  				"LPM Erratum not available on dwc3 revisions < 2.40a\n");
>  
> -		if (dwc->has_lpm_erratum && !DWC3_VER_IS_PRIOR(DWC3, 240A))
> +		if (dwc->has_lpm_erratum && !DWC3_VER_IS_PRIOR(DWC3, 240A)) {
> +			reg &= ~DWC3_DCTL_NYET_THRES_MASK;
>  			reg |= DWC3_DCTL_NYET_THRES(dwc->lpm_nyet_threshold);
> +		}
>  
>  		dwc3_gadget_dctl_write_safe(dwc, reg);
>  	} else {
> 
> ---
> base-commit: c245a7a79602ccbee780c004c1e4abcda66aec32
> change-id: 20241206-dwc3-nyet-fix-7085f6d71d04
> 
> Best regards,
> -- 
> André Draszik <andre.draszik@linaro.org>
> 

Can you also include a fixes tag and Cc stable?

Thanks,
Thinh

Re: [PATCH] usb: dwc3: gadget: fix writing NYET threshold

[Thinh Nguyen]

On Fri, Dec 06, 2024, Thinh Nguyen wrote:
> On Fri, Dec 06, 2024, André Draszik wrote:
> > Before writing a new value to the register, the old value needs to be
> > masked out for the new value to be programmed as intended.
> > 
> > At the moment, the dwc3 core initialises the threshold to the maximum
> > value (0xf), with the option to override it via a DT. No upstream DTs
> > seem to override it, therefore this commit doesn't change behaviour for
> > any upstream platform. Nevertheless, the code should be fixed to have
> > the desired outcome.
> > 
> > Do so.
> > 
> > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > ---
> >  drivers/usb/dwc3/core.h   | 1 +
> >  drivers/usb/dwc3/gadget.c | 4 +++-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> > index ee73789326bc..9335fd095968 100644
> > --- a/drivers/usb/dwc3/core.h
> > +++ b/drivers/usb/dwc3/core.h
> > @@ -465,6 +465,7 @@
> >  
> >  /* These apply for core versions 1.94a and later */
> >  #define DWC3_DCTL_NYET_THRES(n)		(((n) & 0xf) << 20)
> > +#define DWC3_DCTL_NYET_THRES_MASK	DWC3_DCTL_NYET_THRES(0xf)
> 
> If you're going to define a mask macro this way, do this:
> DWC3_DCTL_NYET_THRES(~0)
> 

Actually, let's keep it consistent as how the other masks are defined,
don't use DWC3_DCTL_NYET_THRES(n) to define the mask.

Use GENMASK or do something like this:
#define DWC3_DCTL_NYET_THRES_MASK	(0xf << 20)

BR,
Thinh