Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1638658 > unrolled thread

[PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size

Started byPeter Ujfalusi <peter.ujfalusi@ti.com>
First post2017-05-10 10:50 +0200
Last post2017-05-11 18:10 +0200
Articles 6 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size Peter Ujfalusi <peter.ujfalusi@ti.com> - 2017-05-10 10:50 +0200
    Re: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the  other direction's packet size Tony Lindgren <tony@atomide.com> - 2017-05-10 18:30 +0200
    Re: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the  other direction's packet size Bin Liu <b-liu@ti.com> - 2017-05-10 19:10 +0200
      Re: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the  other direction's packet size Joe Perches <joe@perches.com> - 2017-05-11 01:20 +0200
        Re: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other  direction's packet size Peter Ujfalusi <peter.ujfalusi@ti.com> - 2017-05-11 08:20 +0200
          Re: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the  other direction's packet size Bin Liu <b-liu@ti.com> - 2017-05-11 18:10 +0200

#1638658 — [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size

FromPeter Ujfalusi <peter.ujfalusi@ti.com>
Date2017-05-10 10:50 +0200
Subject[PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size
Message-ID<tFvtL-6TH-3@gated-at.bofh.it>
We have one register for each EP to set the maximum packet size for both
TX and RX.
If for example an RX programming would happen before the previous TX
transfer finishes we would reset the TX packet side.

To fix this issue, only modify the TX or RX part of the register.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/usb/musb/tusb6010_omap.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c
index db2e4c379ccf..4e1a6e4a61b8 100644
--- a/drivers/usb/musb/tusb6010_omap.c
+++ b/drivers/usb/musb/tusb6010_omap.c
@@ -389,15 +389,19 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz,
 
 	if (chdat->tx) {
 		/* Send transfer_packet_sz packets at a time */
-		musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET,
-			chdat->transfer_packet_sz);
+		u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET);
+		psize &= ~0x7ff;
+		psize |= chdat->transfer_packet_sz;
+		musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, psize);
 
 		musb_writel(ep_conf, TUSB_EP_TX_OFFSET,
 			TUSB_EP_CONFIG_XFR_SIZE(chdat->transfer_len));
 	} else {
 		/* Receive transfer_packet_sz packets at a time */
-		musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET,
-			chdat->transfer_packet_sz << 16);
+		u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET);
+		psize &= ~(0x7ff << 16);
+		psize |= (chdat->transfer_packet_sz << 16);
+		musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, psize);
 
 		musb_writel(ep_conf, TUSB_EP_RX_OFFSET,
 			TUSB_EP_CONFIG_XFR_SIZE(chdat->transfer_len));
-- 
2.12.2

[toc] | [next] | [standalone]

#1638944 — Re: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size

FromTony Lindgren <tony@atomide.com>
Date2017-05-10 18:30 +0200
SubjectRe: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size
Message-ID<tFCEV-2YC-3@gated-at.bofh.it>
In reply to#1638658
* Peter Ujfalusi <peter.ujfalusi@ti.com> [170510 01:45]:
> We have one register for each EP to set the maximum packet size for both
> TX and RX.
> If for example an RX programming would happen before the previous TX
> transfer finishes we would reset the TX packet side.
> 
> To fix this issue, only modify the TX or RX part of the register.

Oh this is a nice one. I think we've always had this.

Bin, care to merge this one as a fix? This should have:

Fixes: 550a7375fe72 ("USB: Add MUSB and TUSB support")
Tested-by: Tony Lindgren <tony@atomide.com>

[toc] | [prev] | [next] | [standalone]

#1638971 — Re: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size

FromBin Liu <b-liu@ti.com>
Date2017-05-10 19:10 +0200
SubjectRe: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size
Message-ID<tFDhD-3pT-15@gated-at.bofh.it>
In reply to#1638658
Hi,

On Wed, May 10, 2017 at 11:42:27AM +0300, Peter Ujfalusi wrote:
> We have one register for each EP to set the maximum packet size for both
> TX and RX.
> If for example an RX programming would happen before the previous TX
> transfer finishes we would reset the TX packet side.
> 
> To fix this issue, only modify the TX or RX part of the register.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/usb/musb/tusb6010_omap.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c
> index db2e4c379ccf..4e1a6e4a61b8 100644
> --- a/drivers/usb/musb/tusb6010_omap.c
> +++ b/drivers/usb/musb/tusb6010_omap.c
> @@ -389,15 +389,19 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz,
>  
>  	if (chdat->tx) {
>  		/* Send transfer_packet_sz packets at a time */
> -		musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET,
> -			chdat->transfer_packet_sz);
> +		u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET);

checkpatch.pl complains about declaration and assignment together.

> +		psize &= ~0x7ff;
> +		psize |= chdat->transfer_packet_sz;
> +		musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, psize);
>  
>  		musb_writel(ep_conf, TUSB_EP_TX_OFFSET,
>  			TUSB_EP_CONFIG_XFR_SIZE(chdat->transfer_len));
>  	} else {
>  		/* Receive transfer_packet_sz packets at a time */
> -		musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET,
> -			chdat->transfer_packet_sz << 16);
> +		u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET);

and at here too.

> +		psize &= ~(0x7ff << 16);
> +		psize |= (chdat->transfer_packet_sz << 16);
> +		musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, psize);
>  
>  		musb_writel(ep_conf, TUSB_EP_RX_OFFSET,
>  			TUSB_EP_CONFIG_XFR_SIZE(chdat->transfer_len));

Regards,
-Bin.

[toc] | [prev] | [next] | [standalone]

#1639105 — Re: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size

FromJoe Perches <joe@perches.com>
Date2017-05-11 01:20 +0200
SubjectRe: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size
Message-ID<tFJ3H-6Xc-3@gated-at.bofh.it>
In reply to#1638971
On Wed, 2017-05-10 at 12:07 -0500, Bin Liu wrote:
> On Wed, May 10, 2017 at 11:42:27AM +0300, Peter Ujfalusi wrote:
> > We have one register for each EP to set the maximum packet size for both
> > TX and RX.
> > If for example an RX programming would happen before the previous TX
> > transfer finishes we would reset the TX packet side.
> > 
> > To fix this issue, only modify the TX or RX part of the register.
[]
> > diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c
[]
> > @@ -389,15 +389,19 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz,
> >  
> >  	if (chdat->tx) {
> >  		/* Send transfer_packet_sz packets at a time */
> > -		musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET,
> > -			chdat->transfer_packet_sz);
> > +		u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET);
> 
> checkpatch.pl complains about declaration and assignment together.

No it doesn't.

[toc] | [prev] | [next] | [standalone]

#1639202 — Re: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size

FromPeter Ujfalusi <peter.ujfalusi@ti.com>
Date2017-05-11 08:20 +0200
SubjectRe: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size
Message-ID<tFPC9-2Ew-3@gated-at.bofh.it>
In reply to#1639105

On 2017-05-11 02:16, Joe Perches wrote:
> On Wed, 2017-05-10 at 12:07 -0500, Bin Liu wrote:
>> On Wed, May 10, 2017 at 11:42:27AM +0300, Peter Ujfalusi wrote:
>>> We have one register for each EP to set the maximum packet size for both
>>> TX and RX.
>>> If for example an RX programming would happen before the previous TX
>>> transfer finishes we would reset the TX packet side.
>>>
>>> To fix this issue, only modify the TX or RX part of the register.
> []
>>> diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c
> []
>>> @@ -389,15 +389,19 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz,
>>>   
>>>   	if (chdat->tx) {
>>>   		/* Send transfer_packet_sz packets at a time */
>>> -		musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET,
>>> -			chdat->transfer_packet_sz);
>>> +		u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET);
>>
>> checkpatch.pl complains about declaration and assignment together.
> 
> No it doesn't.

It 'only' complains about:
WARNING: Missing a blank line after declarations

which is valid.

- Péter

[toc] | [prev] | [next] | [standalone]

#1639828 — Re: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size

FromBin Liu <b-liu@ti.com>
Date2017-05-11 18:10 +0200
SubjectRe: [PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size
Message-ID<tFYP8-7g-19@gated-at.bofh.it>
In reply to#1639202
On Thu, May 11, 2017 at 09:19:17AM +0300, Peter Ujfalusi wrote:
> 
> 
> On 2017-05-11 02:16, Joe Perches wrote:
> >On Wed, 2017-05-10 at 12:07 -0500, Bin Liu wrote:
> >>On Wed, May 10, 2017 at 11:42:27AM +0300, Peter Ujfalusi wrote:
> >>>We have one register for each EP to set the maximum packet size for both
> >>>TX and RX.
> >>>If for example an RX programming would happen before the previous TX
> >>>transfer finishes we would reset the TX packet side.
> >>>
> >>>To fix this issue, only modify the TX or RX part of the register.
> >[]
> >>>diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c
> >[]
> >>>@@ -389,15 +389,19 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz,
> >>>  	if (chdat->tx) {
> >>>  		/* Send transfer_packet_sz packets at a time */
> >>>-		musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET,
> >>>-			chdat->transfer_packet_sz);
> >>>+		u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET);
> >>
> >>checkpatch.pl complains about declaration and assignment together.
> >
> >No it doesn't.
> 
> It 'only' complains about:
> WARNING: Missing a blank line after declarations

It was it. My bad, I was multi-tasking and didn't read the log
carefully.

> 
> which is valid.

So will you update the patch to move the declaration to the beginning of
the function to avoid this WARNING. I would just fix it locally if you
prefer.

Regards,
-Bin.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web