On Thu, Jan 19, 2017 at 11:06:59AM +0100, Alexandre Bailon wrote:
> Despite the CPPI 4.1 is a generic DMA, it is tied to USB.
> On the dsps, CPPI 4.1 interrupt's registers are in USBSS (the MUSB glue).
> Currently, to enable / disable and clear interrupts, the CPPI 4.1 driver
> maps and accesses to USBSS's register, which making CPPI 4.1 driver not
> really generic.
> Move the interrupt management to dsps driver.
> 
> Signed-off-by: Alexandre Bailon <[email protected]>
> ---
>  drivers/dma/cppi41.c         | 28 ++++------------
>  drivers/usb/musb/musb_dsps.c | 77 
> ++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 82 insertions(+), 23 deletions(-)

This patch touches both dma and musb modules, I know it makes review
easier, but how we get it merged? One maintainer ACK it and the other
pick it up? Sorry for the dumb question, I am new as a maintainer...

> 
> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> index d5ba43a..4999e7d 100644
> --- a/drivers/dma/cppi41.c
> +++ b/drivers/dma/cppi41.c

[...]

> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index 9f125e1..9dad3a6 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -121,6 +121,7 @@ struct dsps_glue {
>       struct timer_list timer;        /* otg_workaround timer */
>       unsigned long last_timer;    /* last timer data for each instance */
>       bool sw_babble_enabled;
> +     void __iomem *usbss_base;
>  
>       struct dsps_context context;
>       struct debugfs_regset32 regset;
> @@ -145,6 +146,13 @@ static const struct debugfs_reg32 dsps_musb_regs[] = {
>       { "mode",               0xe8 },
>  };
>  
> +/* USBSS  / USB AM335x */
> +#define USBSS_IRQ_STATUS     0x28
> +#define USBSS_IRQ_ENABLER    0x2c
> +#define USBSS_IRQ_CLEARR     0x30
> +
> +#define USBSS_IRQ_PD_COMP    (1 <<  2)

Please fix the double white spaces bwteen '<' and '2' this time.

> +
>  /**
>   * dsps_musb_enable - enable interrupts
>   */
> @@ -619,14 +627,72 @@ static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, 
> u16 len, u8 *dst)
>       }
>  }
>  
> +#ifdef CONFIG_USB_TI_CPPI41_DMA
> +static void dsps_dma_controller_callback(struct dma_controller *c)
> +{
> +     struct musb *musb = c->musb;
> +     struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
> +     void __iomem *usbss_base = glue->usbss_base;
> +     u32 status;
> +
> +     status = musb_readl(usbss_base, USBSS_IRQ_STATUS);
> +     if (status & USBSS_IRQ_PD_COMP)
> +             musb_writel(usbss_base, USBSS_IRQ_STATUS, USBSS_IRQ_PD_COMP);
> +}
> +
> +static struct dma_controller *
> +dsps_dma_controller_create(struct musb *musb, void __iomem *base)
> +{
> +     struct dma_controller *controller;
> +     struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
> +     void __iomem *usbss_base = glue->usbss_base;
> +
> +     controller = cppi41_dma_controller_create(musb, base);
> +     if (IS_ERR_OR_NULL(controller))
> +             return controller;
> +
> +     musb_writel(usbss_base, USBSS_IRQ_ENABLER, USBSS_IRQ_PD_COMP);
> +     controller->dma_callback = dsps_dma_controller_callback;
> +
> +     return controller;
> +}
> +
> +static void dsps_dma_controller_destroy(struct dma_controller *c)
> +{
> +     struct musb *musb = c->musb;
> +     struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
> +     void __iomem *usbss_base = glue->usbss_base;
> +
> +     musb_writel(usbss_base, USBSS_IRQ_CLEARR, USBSS_IRQ_PD_COMP);
> +     cppi41_dma_controller_destroy(c);
> +}
> +
> +static void dsps_dma_controller_suspend(struct dsps_glue *glue)
> +{
> +     void __iomem *usbss_base = glue->usbss_base;
> +
> +     musb_writel(usbss_base, USBSS_IRQ_CLEARR, USBSS_IRQ_PD_COMP);
> +}
> +
> +static void dsps_dma_controller_resume(struct dsps_glue *glue)
> +{
> +     void __iomem *usbss_base = glue->usbss_base;
> +
> +     musb_writel(usbss_base, USBSS_IRQ_ENABLER, USBSS_IRQ_PD_COMP);
> +}

The two functions above need to be wrapped in CONFIG_PM_SLEEP.

> +#else
> +static void dsps_dma_controller_suspend(struct dsps_glue *glue) {}
> +static void dsps_dma_controller_resume(struct dsps_glue *glue) {}
> +#endif
> +
>  static struct musb_platform_ops dsps_ops = {
>       .quirks         = MUSB_DMA_CPPI41 | MUSB_INDEXED_EP,
>       .init           = dsps_musb_init,
>       .exit           = dsps_musb_exit,
>  
>  #ifdef CONFIG_USB_TI_CPPI41_DMA
> -     .dma_init       = cppi41_dma_controller_create,
> -     .dma_exit       = cppi41_dma_controller_destroy,
> +     .dma_init       = dsps_dma_controller_create,
> +     .dma_exit       = dsps_dma_controller_destroy,
>  #endif
>       .enable         = dsps_musb_enable,
>       .disable        = dsps_musb_disable,
> @@ -792,6 +858,9 @@ static int dsps_probe(struct platform_device *pdev)
>  
>       glue->dev = &pdev->dev;
>       glue->wrp = wrp;
> +     glue->usbss_base = of_iomap(pdev->dev.parent->of_node, 0);
> +     if (!glue->usbss_base)

use IS_ERR()?

> +             return -ENXIO;

and return PTR_ERR()?

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to