[PATCH v2 1/2] usb: musb: Fix DMA for host mode

[PATCH v2 1/2] usb: musb: Fix DMA for host mode

[Joshua Henderson]

From: Cristian Birsan <cristian.birsan@microchip.com>

Commit ac33cdb16681 ("usb: musb: Remove ifdefs for musb_host_rx in
musb_host.c part5") introduces a problem setting DMA host mode.

This commit 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>
---
Changes since v1:
	- Don't add unecessary new variable. Just correct done.
---
 drivers/usb/musb/musb_host.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 795a45b..6bef86f 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -2006,9 +2006,15 @@ void musb_host_rx(struct musb *musb, u8 epnum)
 			done = musb_rx_dma_in_inventra_cppi41(c, hw_ep, qh,
 							      urb, xfer_len,
 							      iso_err);
-			if (done)
+			if (done) {
+				/* Don't call msub_advance_schedule()
+				 * immediately after receiving an endpoint RX
+				 * interrupt without waiting for the DMA
+				 * transfer to complete.
+				 */
+				done = false;
 				goto finish;
-			else
+			} else
 				dev_err(musb->controller, "error: rx_dma failed\n");
 		}
 
-- 
1.7.9.5

[PATCH v2 2/2] usb: musb: Fix DMA desired mode for Mentor DMA engine

[Joshua Henderson]

From: Cristian Birsan <cristian.birsan@microchip.com>

Commit 754fe4a92c07 ("usb: musb: Remove ifdefs for TX DMA for musb_host.c")
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 an address, 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>
---
Changes since v1:
	- Fix commit comment citing the cause of the regression.
---
 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 6bef86f..b9e24862 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

Re: [PATCH v2 2/2] usb: musb: Fix DMA desired mode for Mentor DMA engine

[Sergei Shtylyov]

Hello.

On 2/9/2016 1:15 AM, Joshua Henderson wrote:

> From: Cristian Birsan <cristian.birsan@microchip.com>
>
> Commit 754fe4a92c07 ("usb: musb: Remove ifdefs for TX DMA for musb_host.c")
> 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 an address,

    Pointer assigned to address? I don't see how that matches the code.

> 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>
> ---
> Changes since v1:
> 	- Fix commit comment citing the cause of the regression.
> ---
>   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 6bef86f..b9e24862 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;

MBR, Sergei

Re: [PATCH v2 1/2] usb: musb: Fix DMA for host mode

[Sergei Shtylyov]

On 2/9/2016 1:15 AM, Joshua Henderson wrote:

> From: Cristian Birsan <cristian.birsan@microchip.com>
>
> Commit ac33cdb16681 ("usb: musb: Remove ifdefs for musb_host_rx in
> musb_host.c part5") introduces a problem setting DMA host mode.
>
> This commit 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>
> ---
> Changes since v1:
> 	- Don't add unecessary new variable. Just correct done.
> ---
>   drivers/usb/musb/musb_host.c |   10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index 795a45b..6bef86f 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -2006,9 +2006,15 @@ void musb_host_rx(struct musb *musb, u8 epnum)
>   			done = musb_rx_dma_in_inventra_cppi41(c, hw_ep, qh,
>   							      urb, xfer_len,
>   							      iso_err);
> -			if (done)
> +			if (done) {
> +				/* Don't call msub_advance_schedule()
> +				 * immediately after receiving an endpoint RX
> +				 * interrupt without waiting for the DMA
> +				 * transfer to complete.
> +				 */
> +				done = false;
>   				goto finish;

    By not needing the 'ret' variable I meant that you should just call 
musb_rx_dma_in_inventra_cppi41() directly from *if*.

> -			else
> +			} else
>   				dev_err(musb->controller, "error: rx_dma failed\n");

    Need to enclose all branches in {} if at least one is enclosed.

MBR, Sergei