[PATCH 1/6] usb: mtu3: restore HS function when set SS/SSP

[PATCH 1/6] usb: mtu3: restore HS function when set SS/SSP

[Chunfeng Yun]

Due to HS function is disabled when set as FS, need restore
it when set as SS/SSP.

Fixes: dc4c1aa7eae9 ("usb: mtu3: add ->udc_set_speed()")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/mtu3/mtu3_core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c
index 562f4357831e..6403f01947b2 100644
--- a/drivers/usb/mtu3/mtu3_core.c
+++ b/drivers/usb/mtu3/mtu3_core.c
@@ -227,11 +227,13 @@ static void mtu3_set_speed(struct mtu3 *mtu, enum usb_device_speed speed)
 		mtu3_setbits(mbase, U3D_POWER_MANAGEMENT, HS_ENABLE);
 		break;
 	case USB_SPEED_SUPER:
+		mtu3_setbits(mbase, U3D_POWER_MANAGEMENT, HS_ENABLE);
 		mtu3_clrbits(mtu->ippc_base, SSUSB_U3_CTRL(0),
 			     SSUSB_U3_PORT_SSP_SPEED);
 		break;
 	case USB_SPEED_SUPER_PLUS:
-			mtu3_setbits(mtu->ippc_base, SSUSB_U3_CTRL(0),
+		mtu3_setbits(mbase, U3D_POWER_MANAGEMENT, HS_ENABLE);
+		mtu3_setbits(mtu->ippc_base, SSUSB_U3_CTRL(0),
 			     SSUSB_U3_PORT_SSP_SPEED);
 		break;
 	default:
-- 
2.25.1

[PATCH 2/6] usb: mtu3: fix the wrong HS mult value

[Chunfeng Yun]

Use usb_endpoint_maxp() and usb_endpoint_maxp_mult() seperately
to get maxpacket and mult.
Meanwhile fix the bug that should use @mult but not @burst
to save mult value.

Fixes: 4d79e042ed8b ("usb: mtu3: add support for usb3.1 IP")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/mtu3/mtu3_gadget.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3_gadget.c b/drivers/usb/mtu3/mtu3_gadget.c
index 5e21ba05ebf0..a399fd84c71f 100644
--- a/drivers/usb/mtu3/mtu3_gadget.c
+++ b/drivers/usb/mtu3/mtu3_gadget.c
@@ -64,14 +64,12 @@ static int mtu3_ep_enable(struct mtu3_ep *mep)
 	u32 interval = 0;
 	u32 mult = 0;
 	u32 burst = 0;
-	int max_packet;
 	int ret;
 
 	desc = mep->desc;
 	comp_desc = mep->comp_desc;
 	mep->type = usb_endpoint_type(desc);
-	max_packet = usb_endpoint_maxp(desc);
-	mep->maxp = max_packet & GENMASK(10, 0);
+	mep->maxp = usb_endpoint_maxp(desc);
 
 	switch (mtu->g.speed) {
 	case USB_SPEED_SUPER:
@@ -92,7 +90,7 @@ static int mtu3_ep_enable(struct mtu3_ep *mep)
 				usb_endpoint_xfer_int(desc)) {
 			interval = desc->bInterval;
 			interval = clamp_val(interval, 1, 16) - 1;
-			burst = (max_packet & GENMASK(12, 11)) >> 11;
+			mult = usb_endpoint_maxp_mult(desc) - 1;
 		}
 		break;
 	default:
-- 
2.25.1

Re: [PATCH 2/6] usb: mtu3: fix the wrong HS mult value

[Felipe Balbi]

Chunfeng Yun <chunfeng.yun@mediatek.com> writes:

> Use usb_endpoint_maxp() and usb_endpoint_maxp_mult() seperately
> to get maxpacket and mult.
> Meanwhile fix the bug that should use @mult but not @burst
> to save mult value.

I really think you should split this into two patches. One which *only*
fixes the bug and another (patch 2) which *only* corrects the use
usb_endpoint_maxp()

-- 
balbi

Re: [PATCH 2/6] usb: mtu3: fix the wrong HS mult value

[Chunfeng Yun (云春峰)]

On Thu, 2021-08-12 at 09:49 +0300, Felipe Balbi wrote:
> Chunfeng Yun <chunfeng.yun@mediatek.com> writes:
> 
> > Use usb_endpoint_maxp() and usb_endpoint_maxp_mult() seperately
> > to get maxpacket and mult.
> > Meanwhile fix the bug that should use @mult but not @burst
> > to save mult value.
> 
> I really think you should split this into two patches. One which
> *only*
> fixes the bug and another (patch 2) which *only* corrects the use
> usb_endpoint_maxp()
Ok, thanks
> 

[PATCH 3/6] usb: cdnsp: fix the wrong mult value for HS isoc or intr

[Chunfeng Yun]

usb_endpoint_maxp() only returns the bit[10:0] of wMaxPacketSize
of endpoint descriptor, not include bit[12:11] anymore, so use
usb_endpoint_maxp_mult() instead.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/cdns3/cdnsp-mem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/cdns3/cdnsp-mem.c b/drivers/usb/cdns3/cdnsp-mem.c
index a47948a1623f..ad9aee3f1e39 100644
--- a/drivers/usb/cdns3/cdnsp-mem.c
+++ b/drivers/usb/cdns3/cdnsp-mem.c
@@ -882,7 +882,7 @@ static u32 cdnsp_get_endpoint_max_burst(struct usb_gadget *g,
 	if (g->speed == USB_SPEED_HIGH &&
 	    (usb_endpoint_xfer_isoc(pep->endpoint.desc) ||
 	     usb_endpoint_xfer_int(pep->endpoint.desc)))
-		return (usb_endpoint_maxp(pep->endpoint.desc) & 0x1800) >> 11;
+		return usb_endpoint_maxp_mult(pep->endpoint.desc) - 1;
 
 	return 0;
 }
-- 
2.25.1

Re: [PATCH 3/6] usb: cdnsp: fix the wrong mult value for HS isoc or intr

[Felipe Balbi]

Chunfeng Yun <chunfeng.yun@mediatek.com> writes:

> usb_endpoint_maxp() only returns the bit[10:0] of wMaxPacketSize
> of endpoint descriptor, not include bit[12:11] anymore, so use
> usb_endpoint_maxp_mult() instead.
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/cdns3/cdnsp-mem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/cdns3/cdnsp-mem.c b/drivers/usb/cdns3/cdnsp-mem.c
> index a47948a1623f..ad9aee3f1e39 100644
> --- a/drivers/usb/cdns3/cdnsp-mem.c
> +++ b/drivers/usb/cdns3/cdnsp-mem.c
> @@ -882,7 +882,7 @@ static u32 cdnsp_get_endpoint_max_burst(struct usb_gadget *g,
>  	if (g->speed == USB_SPEED_HIGH &&
>  	    (usb_endpoint_xfer_isoc(pep->endpoint.desc) ||
>  	     usb_endpoint_xfer_int(pep->endpoint.desc)))
> -		return (usb_endpoint_maxp(pep->endpoint.desc) & 0x1800) >> 11;
> +		return usb_endpoint_maxp_mult(pep->endpoint.desc) - 1;

this looks like a bugfix. Do we need to Cc stable here?

In any case:

Acked-by: Felipe Balbi <balbi@kernel.org>

-- 
balbi

Re: [PATCH 3/6] usb: cdnsp: fix the wrong mult value for HS isoc or intr

[Chunfeng Yun (云春峰)]

On Thu, 2021-08-12 at 09:51 +0300, Felipe Balbi wrote:
> Chunfeng Yun <chunfeng.yun@mediatek.com> writes:
> 
> > usb_endpoint_maxp() only returns the bit[10:0] of wMaxPacketSize
> > of endpoint descriptor, not include bit[12:11] anymore, so use
> > usb_endpoint_maxp_mult() instead.
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/usb/cdns3/cdnsp-mem.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/cdns3/cdnsp-mem.c
> > b/drivers/usb/cdns3/cdnsp-mem.c
> > index a47948a1623f..ad9aee3f1e39 100644
> > --- a/drivers/usb/cdns3/cdnsp-mem.c
> > +++ b/drivers/usb/cdns3/cdnsp-mem.c
> > @@ -882,7 +882,7 @@ static u32 cdnsp_get_endpoint_max_burst(struct
> > usb_gadget *g,
> >  	if (g->speed == USB_SPEED_HIGH &&
> >  	    (usb_endpoint_xfer_isoc(pep->endpoint.desc) ||
> >  	     usb_endpoint_xfer_int(pep->endpoint.desc)))
> > -		return (usb_endpoint_maxp(pep->endpoint.desc) & 0x1800)
> > >> 11;
> > +		return usb_endpoint_maxp_mult(pep->endpoint.desc) - 1;
> 
> this looks like a bugfix. Do we need to Cc stable here?
It's better to Cc stable, will do it, thanks

> 
> In any case:
> 
> Acked-by: Felipe Balbi <balbi@kernel.org>
> 

RE: [PATCH 3/6] usb: cdnsp: fix the wrong mult value for HS isoc or intr

[Pawel Laszczak]

>
>Chunfeng Yun <chunfeng.yun@mediatek.com> writes:
>
>> usb_endpoint_maxp() only returns the bit[10:0] of wMaxPacketSize
>> of endpoint descriptor, not include bit[12:11] anymore, so use
>> usb_endpoint_maxp_mult() instead.
>>
>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>> ---
>>  drivers/usb/cdns3/cdnsp-mem.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/cdns3/cdnsp-mem.c b/drivers/usb/cdns3/cdnsp-mem.c
>> index a47948a1623f..ad9aee3f1e39 100644
>> --- a/drivers/usb/cdns3/cdnsp-mem.c
>> +++ b/drivers/usb/cdns3/cdnsp-mem.c
>> @@ -882,7 +882,7 @@ static u32 cdnsp_get_endpoint_max_burst(struct usb_gadget *g,
>>  	if (g->speed == USB_SPEED_HIGH &&
>>  	    (usb_endpoint_xfer_isoc(pep->endpoint.desc) ||
>>  	     usb_endpoint_xfer_int(pep->endpoint.desc)))
>> -		return (usb_endpoint_maxp(pep->endpoint.desc) & 0x1800) >> 11;
>> +		return usb_endpoint_maxp_mult(pep->endpoint.desc) - 1;
>
>this looks like a bugfix. Do we need to Cc stable here?
>
>In any case:
>
>Acked-by: Felipe Balbi <balbi@kernel.org>
>

It's not a bugfix. The result is the same.

Acked-by: Pawel Laszczak <pawell@cadence.com>

--

Thanks 
Pawel Laszczak

Re: [PATCH 3/6] usb: cdnsp: fix the wrong mult value for HS isoc or intr

[Chunfeng Yun (云春峰)]

On Tue, 2021-08-17 at 05:05 +0000, Pawel Laszczak wrote:
> > 
> > Chunfeng Yun <chunfeng.yun@mediatek.com> writes:
> > 
> > > usb_endpoint_maxp() only returns the bit[10:0] of wMaxPacketSize
> > > of endpoint descriptor, not include bit[12:11] anymore, so use
> > > usb_endpoint_maxp_mult() instead.
> > > 
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > >  drivers/usb/cdns3/cdnsp-mem.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/usb/cdns3/cdnsp-mem.c
> > > b/drivers/usb/cdns3/cdnsp-mem.c
> > > index a47948a1623f..ad9aee3f1e39 100644
> > > --- a/drivers/usb/cdns3/cdnsp-mem.c
> > > +++ b/drivers/usb/cdns3/cdnsp-mem.c
> > > @@ -882,7 +882,7 @@ static u32
> > > cdnsp_get_endpoint_max_burst(struct usb_gadget *g,
> > >  	if (g->speed == USB_SPEED_HIGH &&
> > >  	    (usb_endpoint_xfer_isoc(pep->endpoint.desc) ||
> > >  	     usb_endpoint_xfer_int(pep->endpoint.desc)))
> > > -		return (usb_endpoint_maxp(pep->endpoint.desc) & 0x1800)
> > > >> 11;
> > > +		return usb_endpoint_maxp_mult(pep->endpoint.desc) - 1;
> > 
> > this looks like a bugfix. Do we need to Cc stable here?
> > 
> > In any case:
> > 
> > Acked-by: Felipe Balbi <balbi@kernel.org>
> > 
> 
> It's not a bugfix. The result is the same.

"(usb_endpoint_maxp(pep->endpoint.desc) & 0x1800) >> 11" is always zero
now!
but not for "usb_endpoint_maxp_mult(pep->endpoint.desc) - 1".

> 
> Acked-by: Pawel Laszczak <pawell@cadence.com>
> 
> --
> 
> Thanks 
> Pawel Laszczak

[PATCH 4/6] usb: gadget: tegra-xudc: fix the wrong mult value for HS isoc or intr

[Chunfeng Yun]

usb_endpoint_maxp() only returns the bit[10:0] of wMaxPacketSize
of endpoint descriptor, not includes bit[12:11] anymore, so use
usb_endpoint_maxp_mult() instead.
Meanwhile no need AND 0x7ff when get maxp, remove it.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/gadget/udc/tegra-xudc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
index a54d1cef17db..40a7417e7ae4 100644
--- a/drivers/usb/gadget/udc/tegra-xudc.c
+++ b/drivers/usb/gadget/udc/tegra-xudc.c
@@ -1610,7 +1610,7 @@ static void tegra_xudc_ep_context_setup(struct tegra_xudc_ep *ep)
 	u16 maxpacket, maxburst = 0, esit = 0;
 	u32 val;
 
-	maxpacket = usb_endpoint_maxp(desc) & 0x7ff;
+	maxpacket = usb_endpoint_maxp(desc);
 	if (xudc->gadget.speed == USB_SPEED_SUPER) {
 		if (!usb_endpoint_xfer_control(desc))
 			maxburst = comp_desc->bMaxBurst;
@@ -1621,7 +1621,7 @@ static void tegra_xudc_ep_context_setup(struct tegra_xudc_ep *ep)
 		   (usb_endpoint_xfer_int(desc) ||
 		    usb_endpoint_xfer_isoc(desc))) {
 		if (xudc->gadget.speed == USB_SPEED_HIGH) {
-			maxburst = (usb_endpoint_maxp(desc) >> 11) & 0x3;
+			maxburst = usb_endpoint_maxp_mult(desc) - 1;
 			if (maxburst == 0x3) {
 				dev_warn(xudc->dev,
 					 "invalid endpoint maxburst\n");
-- 
2.25.1

Re: [PATCH 4/6] usb: gadget: tegra-xudc: fix the wrong mult value for HS isoc or intr

[Felipe Balbi]

Chunfeng Yun <chunfeng.yun@mediatek.com> writes:

> usb_endpoint_maxp() only returns the bit[10:0] of wMaxPacketSize
> of endpoint descriptor, not includes bit[12:11] anymore, so use
> usb_endpoint_maxp_mult() instead.
> Meanwhile no need AND 0x7ff when get maxp, remove it.
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/gadget/udc/tegra-xudc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
> index a54d1cef17db..40a7417e7ae4 100644
> --- a/drivers/usb/gadget/udc/tegra-xudc.c
> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
> @@ -1610,7 +1610,7 @@ static void tegra_xudc_ep_context_setup(struct tegra_xudc_ep *ep)
>  	u16 maxpacket, maxburst = 0, esit = 0;
>  	u32 val;
>  
> -	maxpacket = usb_endpoint_maxp(desc) & 0x7ff;
> +	maxpacket = usb_endpoint_maxp(desc);
>  	if (xudc->gadget.speed == USB_SPEED_SUPER) {
>  		if (!usb_endpoint_xfer_control(desc))
>  			maxburst = comp_desc->bMaxBurst;
> @@ -1621,7 +1621,7 @@ static void tegra_xudc_ep_context_setup(struct tegra_xudc_ep *ep)
>  		   (usb_endpoint_xfer_int(desc) ||
>  		    usb_endpoint_xfer_isoc(desc))) {
>  		if (xudc->gadget.speed == USB_SPEED_HIGH) {
> -			maxburst = (usb_endpoint_maxp(desc) >> 11) & 0x3;
> +			maxburst = usb_endpoint_maxp_mult(desc) - 1;

this looks like a bug fix. Before this change, maxburst was always 0,
right?

In any case:

Acked-by: Felipe Balbi <balbi@kernel.org>

-- 
balbi

Re: [PATCH 4/6] usb: gadget: tegra-xudc: fix the wrong mult value for HS isoc or intr

[Chunfeng Yun (云春峰)]

On Thu, 2021-08-12 at 09:51 +0300, Felipe Balbi wrote:
> Chunfeng Yun <chunfeng.yun@mediatek.com> writes:
> 
> > usb_endpoint_maxp() only returns the bit[10:0] of wMaxPacketSize
> > of endpoint descriptor, not includes bit[12:11] anymore, so use
> > usb_endpoint_maxp_mult() instead.
> > Meanwhile no need AND 0x7ff when get maxp, remove it.
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/usb/gadget/udc/tegra-xudc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/udc/tegra-xudc.c
> > b/drivers/usb/gadget/udc/tegra-xudc.c
> > index a54d1cef17db..40a7417e7ae4 100644
> > --- a/drivers/usb/gadget/udc/tegra-xudc.c
> > +++ b/drivers/usb/gadget/udc/tegra-xudc.c
> > @@ -1610,7 +1610,7 @@ static void
> > tegra_xudc_ep_context_setup(struct tegra_xudc_ep *ep)
> >  	u16 maxpacket, maxburst = 0, esit = 0;
> >  	u32 val;
> >  
> > -	maxpacket = usb_endpoint_maxp(desc) & 0x7ff;
> > +	maxpacket = usb_endpoint_maxp(desc);
> >  	if (xudc->gadget.speed == USB_SPEED_SUPER) {
> >  		if (!usb_endpoint_xfer_control(desc))
> >  			maxburst = comp_desc->bMaxBurst;
> > @@ -1621,7 +1621,7 @@ static void
> > tegra_xudc_ep_context_setup(struct tegra_xudc_ep *ep)
> >  		   (usb_endpoint_xfer_int(desc) ||
> >  		    usb_endpoint_xfer_isoc(desc))) {
> >  		if (xudc->gadget.speed == USB_SPEED_HIGH) {
> > -			maxburst = (usb_endpoint_maxp(desc) >> 11) &
> > 0x3;
> > +			maxburst = usb_endpoint_maxp_mult(desc) - 1;
> 
> this looks like a bug fix. Before this change, maxburst was always 0,
> right?
Yes
> 
> In any case:
> 
> Acked-by: Felipe Balbi <balbi@kernel.org>
> 

[PATCH 5/6] usb: gadget: bdc: remove unnecessary AND operation when get ep maxp

[Chunfeng Yun]

usb_endpoint_maxp() already returns actual max packet size, no need
AND 0x7ff.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/gadget/udc/bdc/bdc_cmd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/bdc/bdc_cmd.c b/drivers/usb/gadget/udc/bdc/bdc_cmd.c
index 995f79c79f96..67887316a1a6 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_cmd.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_cmd.c
@@ -153,7 +153,6 @@ int bdc_config_ep(struct bdc *bdc, struct bdc_ep *ep)
 	si = clamp_val(si, 1, 16) - 1;
 
 	mps = usb_endpoint_maxp(desc);
-	mps &= 0x7ff;
 	param2 |= mps << MP_SHIFT;
 	param2 |= usb_endpoint_type(desc) << EPT_SHIFT;
 
-- 
2.25.1

Re: [PATCH 5/6] usb: gadget: bdc: remove unnecessary AND operation when get ep maxp

[Felipe Balbi]

Chunfeng Yun <chunfeng.yun@mediatek.com> writes:

> usb_endpoint_maxp() already returns actual max packet size, no need
> AND 0x7ff.
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

Acked-by: Felipe Balbi <balbi@kernel.org>

-- 
balbi

[PATCH 6/6] usb: gadget: remove unnecessary AND operation when get ep maxp

[Chunfeng Yun]

usb_endpoint_maxp() already returns actual max packet size, no need
AND 0x7ff.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 include/linux/usb/gadget.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 75c7538e350a..f181c84310b6 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -491,7 +491,7 @@ extern char *usb_get_gadget_udc_name(void);
  */
 static inline size_t usb_ep_align(struct usb_ep *ep, size_t len)
 {
-	int max_packet_size = (size_t)usb_endpoint_maxp(ep->desc) & 0x7ff;
+	int max_packet_size = (size_t)usb_endpoint_maxp(ep->desc);
 
 	return round_up(len, max_packet_size);
 }
-- 
2.25.1

Re: [PATCH 6/6] usb: gadget: remove unnecessary AND operation when get ep maxp

[Felipe Balbi]

Chunfeng Yun <chunfeng.yun@mediatek.com> writes:

> usb_endpoint_maxp() already returns actual max packet size, no need
> AND 0x7ff.
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

Acked-by: Felipe Balbi <balbi@kernel.org>

-- 
balbi