Hi,
On Wed, May 04, 2016 at 11:06:58PM +0300, Sergei Shtylyov wrote:
> On 05/04/2016 11:01 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
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.
>
> >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
> 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?
>
> >> if (res)
> >> return false;
> >>
>
> MBR, Seregi
Regards,
-Bin.
--
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
