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 <abailon@xxxxxxxxxxxx>
> ---
> 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 dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
