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

Subsystems: mediatek usb3 drd ip driver, the rest, usb subsystem

STALE1811d

16 messages, 3 authors, 2021-08-17 · open the first message on its own page

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

From: Chunfeng Yun <chunfeng.yun@mediatek.com>
Date: 2021-08-12 03:33:56

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 <redacted>
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

From: Chunfeng Yun <chunfeng.yun@mediatek.com>
Date: 2021-08-12 03:33:51

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 <redacted>
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

From: Felipe Balbi <balbi@kernel.org>
Date: 2021-08-12 06:50:29

Chunfeng Yun [off-list ref] 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

From: Chunfeng Yun (云春峰) <Chunfeng.Yun@mediatek.com>
Date: 2021-08-12 11:52:04

On Thu, 2021-08-12 at 09:49 +0300, Felipe Balbi wrote:

Chunfeng Yun [off-list ref] writes:

quoted

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

From: Chunfeng Yun <chunfeng.yun@mediatek.com>
Date: 2021-08-12 03:34:05

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

From: Felipe Balbi <balbi@kernel.org>
Date: 2021-08-12 06:51:39

Chunfeng Yun [off-list ref] writes:

quoted hunk

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

From: Chunfeng Yun (云春峰) <Chunfeng.Yun@mediatek.com>
Date: 2021-08-12 11:53:41

On Thu, 2021-08-12 at 09:51 +0300, Felipe Balbi wrote:

Chunfeng Yun [off-list ref] writes:

quoted

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)

quoted

quoted
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

From: Pawel Laszczak <pawell@cadence.com>
Date: 2021-08-17 05:05:54

Chunfeng Yun [off-list ref] writes:

quoted

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

From: Chunfeng Yun (云春峰) <Chunfeng.Yun@mediatek.com>
Date: 2021-08-17 07:10:53

On Tue, 2021-08-17 at 05:05 +0000, Pawel Laszczak wrote:

quoted

Chunfeng Yun [off-list ref] writes:

quoted

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)

quoted

quoted
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 6/6] usb: gadget: remove unnecessary AND operation when get ep maxp

From: Chunfeng Yun <chunfeng.yun@mediatek.com>
Date: 2021-08-12 03:34:05

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

From: Felipe Balbi <balbi@kernel.org>
Date: 2021-08-12 06:53:21

Chunfeng Yun [off-list ref] 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 4/6] usb: gadget: tegra-xudc: fix the wrong mult value for HS isoc or intr

From: Chunfeng Yun <chunfeng.yun@mediatek.com>
Date: 2021-08-12 03:34:09

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

From: Felipe Balbi <balbi@kernel.org>
Date: 2021-08-12 06:52:34

Chunfeng Yun [off-list ref] writes:

quoted hunk

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

From: Chunfeng Yun (云春峰) <Chunfeng.Yun@mediatek.com>
Date: 2021-08-12 11:54:32

On Thu, 2021-08-12 at 09:51 +0300, Felipe Balbi wrote:

Chunfeng Yun [off-list ref] writes:

quoted

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

From: Chunfeng Yun <chunfeng.yun@mediatek.com>
Date: 2021-08-12 03:34:15

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

From: Felipe Balbi <balbi@kernel.org>
Date: 2021-08-12 06:52:55

Chunfeng Yun [off-list ref] 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

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help