Re: [PATCH 0/2] musb_host: move DMA engine check from musb_tx_dma_set_mode_cppi_tusb() to its caller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

On 05/04/2016 11:46 PM, Bin Liu wrote:

Commit 754fe4a92c07 ("usb: musb: Remove ifdefs for TX DMA for musb_host.c")
looks incomplete: the DMA engine checks are  done outside the Mentor/UX500
handler  but inside the CPPI/TUSB handler. Move the checks out of the CPPI/
TUSB handler into its caller, musb_tx_dma_program().

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>

---
  drivers/usb/musb/musb_host.c |    7 +++----
  1 file changed, 3 insertions(+), 4 deletions(-)

Index: usb/drivers/usb/musb/musb_host.c
===================================================================
--- usb.orig/drivers/usb/musb/musb_host.c
+++ usb/drivers/usb/musb/musb_host.c
@@ -678,9 +678,6 @@ static int musb_tx_dma_set_mode_cppi_tus
  {
  	struct dma_channel *channel = hw_ep->tx_channel;

-	if (!is_cppi_enabled(hw_ep->musb) && !tusb_dma_omap(hw_ep->musb))
-		return -ENODEV;
-
  	channel->actual_len = 0;

Since this function has only two lines now, does it make sense to get rid
of it completely?

    That would be the reverse to what Tony's patches did. I think gcc
will inline this function anyway.

I believe the intention of Tony's patch is to get rid of #ifdefs. Any

   Right. But while doing that, he tried to avoid the code motion.

further patch could do whatever is right to improve the code. I
personally don't rely on compiler's optimization. I don't have strong
opinion here, you make the call.

   I'll leave the things as they are then.

Regards,
-Bin.


  	/*
@@ -704,9 +701,11 @@ static bool musb_tx_dma_program(struct d
  	if (musb_dma_inventra(hw_ep->musb) || musb_dma_ux500(hw_ep->musb))
  		res = musb_tx_dma_set_mode_mentor(dma, hw_ep, qh, urb,
  						 offset, &length, &mode);
-	else
+	else if (is_cppi_enabled(hw_ep->musb) || tusb_dma_omap(hw_ep->musb))
  		res = musb_tx_dma_set_mode_cppi_tusb(dma, hw_ep, qh, urb,
  						     offset, &length, &mode);
+	else
+		return false;

    Well, using ret = -EINVAL; would have been more appropriate, I'd

   s/ret/res/.

respin if you won't mind?

I don't mind respin at all. But the function return type is bool, so
flase is better, right?

   It'll just bail out on the *if* below:

  	if (res)
  		return false;


MBR, Seregi

Regards,
-Bin.

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux