On Wed, Jan 18, 2017 at 06:29:59PM -0800, Tony Lindgren wrote:
> At least with the cppi41 dma, size 1 in dma transfers will just wait

In which case do you see the size 1 transfer? using testusb?

> until the device is disconnected. This causes timeouts in cppi41 dma
> runtime PM.
> 
> Also the initial size 8 transfers take about 200ms to complete when
> plugging a USB mass storage device to a hub. But we probably want to
> keep those to avoid using PIO.
> 
> Fix the issue by adding a quirk for cppi41 and skip size 1 in dma if
> set.

It is fine to bypass dma for size 1 transfers, due to the dma setup
overhead. But I'd like to know the test case to understand why it hangs.

> 
> Note that additional cpp41 patches are needed to avoid error -115
> messages, but that can be applied separately.
> 
> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> Signed-off-by: Tony Lindgren <t...@atomide.com>
> ---
>  drivers/usb/musb/musb_cppi41.c |  1 +
>  drivers/usb/musb/musb_dma.h    |  3 +++
>  drivers/usb/musb/musb_host.c   | 16 +++++++++++++++-
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
> --- a/drivers/usb/musb/musb_cppi41.c
> +++ b/drivers/usb/musb/musb_cppi41.c
> @@ -695,6 +695,7 @@ cppi41_dma_controller_create(struct musb *musb, void 
> __iomem *base)
>       controller->controller.channel_program = cppi41_dma_channel_program;
>       controller->controller.channel_abort = cppi41_dma_channel_abort;
>       controller->controller.is_compatible = cppi41_is_compatible;
> +     controller->controller.quirks = MUSB_DMA_QUIRK_CPPI41_IN;
>  
>       ret = cppi41_dma_controller_start(controller);
>       if (ret)
> diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
> --- a/drivers/usb/musb/musb_dma.h
> +++ b/drivers/usb/musb/musb_dma.h
> @@ -171,6 +171,8 @@ dma_channel_status(struct dma_channel *c)
>       return (is_dma_capable() && c) ? c->status : MUSB_DMA_STATUS_UNKNOWN;
>  }
>  
> +#define MUSB_DMA_QUIRK_CPPI41_IN     BIT(0)
> +
>  /**
>   * struct dma_controller - A DMA Controller.
>   * @start: call this to start a DMA controller;
> @@ -196,6 +198,7 @@ struct dma_controller {
>       int                     (*is_compatible)(struct dma_channel *channel,
>                                                       u16 maxpacket,
>                                                       void *buf, u32 length);
> +     unsigned int quirks;
>  };
>  
>  /* called after channel_program(), may indicate a fault */
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -743,6 +743,8 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
>  
>       musb_ep_select(mbase, epnum);
>  
> +     dma_controller = musb->dma_controller;
> +
>       if (is_out && !len) {
>               use_dma = 0;
>               csr = musb_readw(epio, MUSB_TXCSR);
> @@ -751,8 +753,20 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
>               hw_ep->tx_channel = NULL;
>       }
>  
> +     /*
> +      * At least cppi41 in dma will just hang with size of 1 until the
> +      * device is disconnected.
> +      */
> +     if (!is_out && dma_controller &&
> +         (dma_controller->quirks & MUSB_DMA_QUIRK_CPPI41_IN)) {

Forget to add size constraint here?

Regards,
-Bin.

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

Reply via email to