* [PATCH 1/2] usb: musb: Fix DMA for host mode
@ 2016-02-02 0:37 Joshua Henderson
2016-02-02 0:37 ` [PATCH 2/2] usb: musb: Fix DMA desired mode for Mentor DMA engine Joshua Henderson
2016-02-02 13:24 ` [PATCH 1/2] usb: musb: Fix DMA for host mode Sergei Shtylyov
0 siblings, 2 replies; 7+ messages in thread
From: Joshua Henderson @ 2016-02-02 0:37 UTC (permalink / raw)
To: linux-kernel
Cc: Cristian Birsan, Joshua Henderson, Felipe Balbi,
Greg Kroah-Hartman, linux-usb
From: Cristian Birsan <cristian.birsan@microchip.com>
Commit ac33cdb166811223cc (usb: musb: Remove ifdefs for musb_host_rx in
musb_host.c part5) introduces a problem setting DMA host mode.
This fixes the done condition that advances the musb schedule. Without this
patch the the msub_advance_schedule() is called immediately after receiving an
endpoint rx interrupt without waiting for the dma transfer to complete. As a
consequence when the dma complete interrupt arrives the in_qh member of hw_ep
is already null an the musb_host_rx() exits on !urb error case.
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com>
---
drivers/usb/musb/musb_host.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 795a45b..3fdc99b 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1825,6 +1825,7 @@ void musb_host_rx(struct musb *musb, u8 epnum)
u16 rx_csr, val;
bool iso_err = false;
bool done = false;
+ int ret;
u32 status;
struct dma_channel *dma;
unsigned int sg_flags = SG_MITER_ATOMIC | SG_MITER_TO_SG;
@@ -2003,10 +2004,10 @@ void musb_host_rx(struct musb *musb, u8 epnum)
qh->offset,
urb->transfer_buffer_length);
- done = musb_rx_dma_in_inventra_cppi41(c, hw_ep, qh,
+ ret = musb_rx_dma_in_inventra_cppi41(c, hw_ep, qh,
urb, xfer_len,
iso_err);
- if (done)
+ if (ret)
goto finish;
else
dev_err(musb->controller, "error: rx_dma failed\n");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] usb: musb: Fix DMA desired mode for Mentor DMA engine
2016-02-02 0:37 [PATCH 1/2] usb: musb: Fix DMA for host mode Joshua Henderson
@ 2016-02-02 0:37 ` Joshua Henderson
2016-02-02 13:32 ` Sergei Shtylyov
2016-02-02 13:24 ` [PATCH 1/2] usb: musb: Fix DMA for host mode Sergei Shtylyov
1 sibling, 1 reply; 7+ messages in thread
From: Joshua Henderson @ 2016-02-02 0:37 UTC (permalink / raw)
To: linux-kernel
Cc: Cristian Birsan, Joshua Henderson, Felipe Balbi,
Greg Kroah-Hartman, linux-usb
From: Cristian Birsan <cristian.birsan@microchip.com>
Commit 6b6e97107f12f3a9f7 (USB: musb: fix isochronous TXDMA (take 2)) introduces
a problem setting the desired channel mode for the Mentor DMA engine.
There is a case where the pointer of the channel DMA mode is incorrectly
assigned to a pointer value, when it should be assigned the actual mode value.
This results in the value of channel->desired_mode not being correct.
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com>
---
drivers/usb/musb/musb_host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 3fdc99b..748379e 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -662,7 +662,7 @@ static int musb_tx_dma_set_mode_mentor(struct dma_controller *dma,
csr &= ~(MUSB_TXCSR_AUTOSET | MUSB_TXCSR_DMAMODE);
csr |= MUSB_TXCSR_DMAENAB; /* against programmer's guide */
}
- channel->desired_mode = mode;
+ channel->desired_mode = *mode;
musb_writew(epio, MUSB_TXCSR, csr);
return 0;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] usb: musb: Fix DMA for host mode
2016-02-02 0:37 [PATCH 1/2] usb: musb: Fix DMA for host mode Joshua Henderson
2016-02-02 0:37 ` [PATCH 2/2] usb: musb: Fix DMA desired mode for Mentor DMA engine Joshua Henderson
@ 2016-02-02 13:24 ` Sergei Shtylyov
2016-02-04 16:44 ` Sergei Shtylyov
1 sibling, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2016-02-02 13:24 UTC (permalink / raw)
To: Joshua Henderson, linux-kernel
Cc: Cristian Birsan, Felipe Balbi, Greg Kroah-Hartman, linux-usb
Hello.
On 2/2/2016 3:37 AM, Joshua Henderson wrote:
> From: Cristian Birsan <cristian.birsan@microchip.com>
>
> Commit ac33cdb166811223cc (usb: musb: Remove ifdefs for musb_host_rx in
> musb_host.c part5) introduces a problem setting DMA host mode.
scripts/checkpatch.pl now enforces certain commit citing style, yours
doesn't quite match it.
> This fixes the done condition that advances the musb schedule. Without this
> patch the the msub_advance_schedule() is called immediately after receiving an
> endpoint rx interrupt without waiting for the dma transfer to complete. As a
> consequence when the dma complete interrupt arrives the in_qh member of hw_ep
> is already null an the musb_host_rx() exits on !urb error case.
>
> Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
> Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com>
> ---
> drivers/usb/musb/musb_host.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index 795a45b..3fdc99b 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -1825,6 +1825,7 @@ void musb_host_rx(struct musb *musb, u8 epnum)
> u16 rx_csr, val;
> bool iso_err = false;
> bool done = false;
> + int ret;
> u32 status;
> struct dma_channel *dma;
> unsigned int sg_flags = SG_MITER_ATOMIC | SG_MITER_TO_SG;
> @@ -2003,10 +2004,10 @@ void musb_host_rx(struct musb *musb, u8 epnum)
> qh->offset,
> urb->transfer_buffer_length);
>
> - done = musb_rx_dma_in_inventra_cppi41(c, hw_ep, qh,
> + ret = musb_rx_dma_in_inventra_cppi41(c, hw_ep, qh,
> urb, xfer_len,
> iso_err);
> - if (done)
> + if (ret)
Hm, does that really fix anything?
> goto finish;
> else
> dev_err(musb->controller, "error: rx_dma failed\n");
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] usb: musb: Fix DMA desired mode for Mentor DMA engine
2016-02-02 0:37 ` [PATCH 2/2] usb: musb: Fix DMA desired mode for Mentor DMA engine Joshua Henderson
@ 2016-02-02 13:32 ` Sergei Shtylyov
2016-02-02 16:36 ` Joshua Henderson
0 siblings, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2016-02-02 13:32 UTC (permalink / raw)
To: Joshua Henderson, linux-kernel
Cc: Cristian Birsan, Felipe Balbi, Greg Kroah-Hartman, linux-usb
On 2/2/2016 3:37 AM, Joshua Henderson wrote:
> From: Cristian Birsan <cristian.birsan@microchip.com>
>
> Commit 6b6e97107f12f3a9f7 (USB: musb: fix isochronous TXDMA (take 2)) introduces
Again, wrong commit style. And I really doubt that blaming my commit was
correct. :-)
> a problem setting the desired channel mode for the Mentor DMA engine.
>
> There is a case where the pointer of the channel DMA mode is incorrectly
> assigned to a pointer value, when it should be assigned the actual mode value.
> This results in the value of channel->desired_mode not being correct.
>
> Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
> Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com>
> ---
> drivers/usb/musb/musb_host.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index 3fdc99b..748379e 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -662,7 +662,7 @@ static int musb_tx_dma_set_mode_mentor(struct dma_controller *dma,
Hm, there was no such function at the time of my commit...
> csr &= ~(MUSB_TXCSR_AUTOSET | MUSB_TXCSR_DMAMODE);
> csr |= MUSB_TXCSR_DMAENAB; /* against programmer's guide */
> }
> - channel->desired_mode = mode;
> + channel->desired_mode = *mode;
'mode' was 'u8' at the time of my commit, see:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6b6e97107f12f3a9f7b5b43a6c3b94409240bcff
I think that the recent commit below should be blamed instead:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=754fe4a92c072a6e36d89fa328ed789c9ebc1af5
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] usb: musb: Fix DMA desired mode for Mentor DMA engine
2016-02-02 13:32 ` Sergei Shtylyov
@ 2016-02-02 16:36 ` Joshua Henderson
2016-02-02 16:51 ` Sergei Shtylyov
0 siblings, 1 reply; 7+ messages in thread
From: Joshua Henderson @ 2016-02-02 16:36 UTC (permalink / raw)
To: Sergei Shtylyov, linux-kernel
Cc: Cristian Birsan, Felipe Balbi, Greg Kroah-Hartman, linux-usb
Sergei,
On 02/02/2016 06:32 AM, Sergei Shtylyov wrote:
> On 2/2/2016 3:37 AM, Joshua Henderson wrote:
>
>> From: Cristian Birsan <cristian.birsan@microchip.com>
>>
>> Commit 6b6e97107f12f3a9f7 (USB: musb: fix isochronous TXDMA (take 2)) introduces
>
> Again, wrong commit style. And I really doubt that blaming my commit was correct. :-)
I apologize. I noticed you added the line in question, but did not notice the variable type of "mode" changed *afterward*. Completely my fault. I shall also fix the commit message style.
>
>> a problem setting the desired channel mode for the Mentor DMA engine.
>>
>> There is a case where the pointer of the channel DMA mode is incorrectly
>> assigned to a pointer value, when it should be assigned the actual mode value.
>> This results in the value of channel->desired_mode not being correct.
>>
>> Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
>> Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com>
>> ---
>> drivers/usb/musb/musb_host.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
>> index 3fdc99b..748379e 100644
>> --- a/drivers/usb/musb/musb_host.c
>> +++ b/drivers/usb/musb/musb_host.c
>> @@ -662,7 +662,7 @@ static int musb_tx_dma_set_mode_mentor(struct dma_controller *dma,
>
> Hm, there was no such function at the time of my commit...
>
>> csr &= ~(MUSB_TXCSR_AUTOSET | MUSB_TXCSR_DMAMODE);
>> csr |= MUSB_TXCSR_DMAENAB; /* against programmer's guide */
>> }
>> - channel->desired_mode = mode;
>> + channel->desired_mode = *mode;
>
> 'mode' was 'u8' at the time of my commit, see:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6b6e97107f12f3a9f7b5b43a6c3b94409240bcff
>
> I think that the recent commit below should be blamed instead:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=754fe4a92c072a6e36d89fa328ed789c9ebc1af5
I'll take a look at this and make sure.
Thanks,
Josh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] usb: musb: Fix DMA desired mode for Mentor DMA engine
2016-02-02 16:36 ` Joshua Henderson
@ 2016-02-02 16:51 ` Sergei Shtylyov
0 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2016-02-02 16:51 UTC (permalink / raw)
To: Joshua Henderson, linux-kernel
Cc: Cristian Birsan, Felipe Balbi, Greg Kroah-Hartman, linux-usb
On 02/02/2016 07:36 PM, Joshua Henderson wrote:
>>> From: Cristian Birsan <cristian.birsan@microchip.com>
>>>
>>> Commit 6b6e97107f12f3a9f7 (USB: musb: fix isochronous TXDMA (take 2)) introduces
>>
>> Again, wrong commit style.
Commit citing style, I meant.
>> And I really doubt that blaming my commit was correct. :-)
>
> I apologize. I noticed you added the line in question, but did not notice the variable type of "mode" changed *afterward*.
So I thought. :-)
> Completely my fault. I shall also fix the commit message style.
TIA.
[...]
> Thanks,
> Josh
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] usb: musb: Fix DMA for host mode
2016-02-02 13:24 ` [PATCH 1/2] usb: musb: Fix DMA for host mode Sergei Shtylyov
@ 2016-02-04 16:44 ` Sergei Shtylyov
0 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2016-02-04 16:44 UTC (permalink / raw)
To: Joshua Henderson, linux-kernel
Cc: Cristian Birsan, Felipe Balbi, Greg Kroah-Hartman, linux-usb
On 02/02/2016 04:24 PM, Sergei Shtylyov wrote:
>> From: Cristian Birsan <cristian.birsan@microchip.com>
>>
>> Commit ac33cdb166811223cc (usb: musb: Remove ifdefs for musb_host_rx in
>> musb_host.c part5) introduces a problem setting DMA host mode.
>
> scripts/checkpatch.pl now enforces certain commit citing style, yours
> doesn't quite match it.
>
>> This fixes the done condition that advances the musb schedule. Without this
>> patch the the msub_advance_schedule() is called immediately after receiving an
>> endpoint rx interrupt without waiting for the dma transfer to complete. As a
>> consequence when the dma complete interrupt arrives the in_qh member of hw_ep
>> is already null an the musb_host_rx() exits on !urb error case.
>>
>> Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
>> Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com>
>> ---
>> drivers/usb/musb/musb_host.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
>> index 795a45b..3fdc99b 100644
>> --- a/drivers/usb/musb/musb_host.c
>> +++ b/drivers/usb/musb/musb_host.c
>> @@ -1825,6 +1825,7 @@ void musb_host_rx(struct musb *musb, u8 epnum)
>> u16 rx_csr, val;
>> bool iso_err = false;
>> bool done = false;
>> + int ret;
>> u32 status;
>> struct dma_channel *dma;
>> unsigned int sg_flags = SG_MITER_ATOMIC | SG_MITER_TO_SG;
>> @@ -2003,10 +2004,10 @@ void musb_host_rx(struct musb *musb, u8 epnum)
>> qh->offset,
>> urb->transfer_buffer_length);
>>
>> - done = musb_rx_dma_in_inventra_cppi41(c, hw_ep, qh,
>> + ret = musb_rx_dma_in_inventra_cppi41(c, hw_ep, qh,
>> urb, xfer_len,
>> iso_err);
>> - if (done)
>> + if (ret)
>
> Hm, does that really fix anything?
Looking again, I got the idea. However, there's no dire need for the new
variable...
(Handling Inventra and CPPI 4.1 DMA in the same code looks fishy. Too bad
I wasn't able to push the CPPI 4.1 drivers myself.)
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-02-04 16:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-02 0:37 [PATCH 1/2] usb: musb: Fix DMA for host mode Joshua Henderson
2016-02-02 0:37 ` [PATCH 2/2] usb: musb: Fix DMA desired mode for Mentor DMA engine Joshua Henderson
2016-02-02 13:32 ` Sergei Shtylyov
2016-02-02 16:36 ` Joshua Henderson
2016-02-02 16:51 ` Sergei Shtylyov
2016-02-02 13:24 ` [PATCH 1/2] usb: musb: Fix DMA for host mode Sergei Shtylyov
2016-02-04 16:44 ` Sergei Shtylyov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.