[PATCH v3 0/3] usb: musb: cppi41: Add a way to manage DMA irq

[PATCH v3 0/3] usb: musb: cppi41: Add a way to manage DMA irq

[Alexandre Bailon]

This series was "dmaengine: cppi41: Make the driver more generic".
I have tried to separate as munch I could CPPI 4.1 MUSB driver changes.

Currently, the DMA interrupt is managed by the CPPI 4.1 driver.
The issue here is the CPPI 4.1 driver must access to MUSB glue registers
to manage its interrupt.
In order to move the interrupts management from CPPI 4.1 driver to MUSB
(and then make it more generic), update the MUSB CPPI 4.1 driver with
changes that will help to manage DMA interrupt from MUSB driver.

Changes in v3:
- Move a patch from another series to this one to avoid build error report
  from kbuild test robot
- Instead of adding and exporting function, add one callback and a pointer
  to musb in struct dma_controller
- Surround the DMA function introduced in musb_dsps with #ifdef / #endif.

Changes in v2:
- Fix some typo in commit messages
- Add more explanation about some changes made by patch 2 in commit message

Alexandre Bailon (3):
  usb: musb: dma: Add a DMA completion platform callback
  usb: musb: cppi41: Detect aborted transfers in cppi41_dma_callback()
  usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in dsps

 drivers/dma/cppi41.c           | 28 ++++------------
 drivers/usb/musb/musb_cppi41.c | 20 ++++++++----
 drivers/usb/musb/musb_dma.h    |  4 +++
 drivers/usb/musb/musb_dsps.c   | 72 ++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 95 insertions(+), 29 deletions(-)

-- 
2.10.2

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

[PATCH v3 1/3] usb: musb: dma: Add a DMA completion platform callback

[Alexandre Bailon]

Currently, the CPPI 4.1 driver is not completely generic and
only work on dsps. This is because of IRQ management.
Add a callback to dma_controller that could be invoked on DMA completion
to acknodlege the IRQ.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/usb/musb/musb_cppi41.c | 7 +++++--
 drivers/usb/musb/musb_dma.h    | 4 ++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index 1636385..f7d3d27 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -217,6 +217,10 @@ static void cppi41_dma_callback(void *private_data)
 	int is_hs = 0;
 	bool empty;
 
+	controller = cppi41_channel->controller;
+	if (controller->controller.dma_callback)
+		controller->controller.dma_callback(&controller->controller);
+
 	spin_lock_irqsave(&musb->lock, flags);
 
 	dmaengine_tx_status(cppi41_channel->dc, cppi41_channel->cookie,
@@ -249,8 +253,6 @@ static void cppi41_dma_callback(void *private_data)
 	 * We spin on HS (no longer than than 25us and setup a timer on
 	 * FS to check for the bit and complete the transfer.
 	 */
-	controller = cppi41_channel->controller;
-
 	if (is_host_active(musb)) {
 		if (musb->port1_status & USB_PORT_STAT_HIGH_SPEED)
 			is_hs = 1;
@@ -695,6 +697,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.musb = musb;
 
 	ret = cppi41_dma_controller_start(controller);
 	if (ret)
diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
index 46357e1..8bea0cd 100644
--- a/drivers/usb/musb/musb_dma.h
+++ b/drivers/usb/musb/musb_dma.h
@@ -181,10 +181,13 @@ dma_channel_status(struct dma_channel *c)
  * @channel_release: call this to release a DMA channel
  * @channel_abort: call this to abort a pending DMA transaction,
  *	returning it to FREE (but allocated) state
+ * @platform_dma_callback: invoked on DMA completion, useful to run platform
+ *	code such IRQ acknowledgment.
  *
  * Controllers manage dma channels.
  */
 struct dma_controller {
+	struct musb *musb;
 	struct dma_channel	*(*channel_alloc)(struct dma_controller *,
 					struct musb_hw_ep *, u8 is_tx);
 	void			(*channel_release)(struct dma_channel *);
@@ -196,6 +199,7 @@ struct dma_controller {
 	int			(*is_compatible)(struct dma_channel *channel,
 							u16 maxpacket,
 							void *buf, u32 length);
+	void			(*dma_callback)(struct dma_controller *);
 };
 
 /* called after channel_program(), may indicate a fault */
-- 
2.10.2

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

Re: [PATCH v3 1/3] usb: musb: dma: Add a DMA completion platform callback

[Sergei Shtylyov]

Hello!

On 1/19/2017 1:06 PM, Alexandre Bailon wrote:

> Currently, the CPPI 4.1 driver is not completely generic and
> only work on dsps. This is because of IRQ management.

    Works. DSPS.

> Add a callback to dma_controller that could be invoked on DMA completion
> to acknodlege the IRQ.

     Acknowledge.

> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
[...]
> diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
> index 46357e1..8bea0cd 100644
> --- a/drivers/usb/musb/musb_dma.h
> +++ b/drivers/usb/musb/musb_dma.h
> @@ -181,10 +181,13 @@ dma_channel_status(struct dma_channel *c)
>   * @channel_release: call this to release a DMA channel
>   * @channel_abort: call this to abort a pending DMA transaction,
>   *	returning it to FREE (but allocated) state
> + * @platform_dma_callback: invoked on DMA completion, useful to run platform

    It's called just dma_callback below.

> + *	code such IRQ acknowledgment.
>   *
>   * Controllers manage dma channels.
>   */
>  struct dma_controller {
> +	struct musb *musb;

    You forgot to document this above.

>  	struct dma_channel	*(*channel_alloc)(struct dma_controller *,
>  					struct musb_hw_ep *, u8 is_tx);
>  	void			(*channel_release)(struct dma_channel *);
> @@ -196,6 +199,7 @@ struct dma_controller {
>  	int			(*is_compatible)(struct dma_channel *channel,
>  							u16 maxpacket,
>  							void *buf, u32 length);
> +	void			(*dma_callback)(struct dma_controller *);
>  };
>
>  /* called after channel_program(), may indicate a fault */

MBR, Sergei

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

Re: [PATCH v3 1/3] usb: musb: dma: Add a DMA completion platform callback

[Bin Liu]

On Thu, Jan 19, 2017 at 11:06:57AM +0100, Alexandre Bailon wrote:
> Currently, the CPPI 4.1 driver is not completely generic and
> only work on dsps. This is because of IRQ management.
> Add a callback to dma_controller that could be invoked on DMA completion
> to acknodlege the IRQ.
> 
> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/usb/musb/musb_cppi41.c | 7 +++++--
>  drivers/usb/musb/musb_dma.h    | 4 ++++
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
> index 1636385..f7d3d27 100644
> --- a/drivers/usb/musb/musb_cppi41.c
> +++ b/drivers/usb/musb/musb_cppi41.c
> @@ -217,6 +217,10 @@ static void cppi41_dma_callback(void *private_data)
>  	int is_hs = 0;
>  	bool empty;
>  
> +	controller = cppi41_channel->controller;
> +	if (controller->controller.dma_callback)
> +		controller->controller.dma_callback(&controller->controller);
> +
>  	spin_lock_irqsave(&musb->lock, flags);
>  
>  	dmaengine_tx_status(cppi41_channel->dc, cppi41_channel->cookie,
> @@ -249,8 +253,6 @@ static void cppi41_dma_callback(void *private_data)
>  	 * We spin on HS (no longer than than 25us and setup a timer on
>  	 * FS to check for the bit and complete the transfer.
>  	 */
> -	controller = cppi41_channel->controller;
> -
>  	if (is_host_active(musb)) {
>  		if (musb->port1_status & USB_PORT_STAT_HIGH_SPEED)
>  			is_hs = 1;
> @@ -695,6 +697,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.musb = musb;
>  
>  	ret = cppi41_dma_controller_start(controller);
>  	if (ret)
> diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
> index 46357e1..8bea0cd 100644
> --- a/drivers/usb/musb/musb_dma.h
> +++ b/drivers/usb/musb/musb_dma.h
> @@ -181,10 +181,13 @@ dma_channel_status(struct dma_channel *c)
>   * @channel_release: call this to release a DMA channel
>   * @channel_abort: call this to abort a pending DMA transaction,
>   *	returning it to FREE (but allocated) state
> + * @platform_dma_callback: invoked on DMA completion, useful to run platform
> + *	code such IRQ acknowledgment.
>   *
>   * Controllers manage dma channels.
>   */
>  struct dma_controller {
> +	struct musb *musb;

Since musb is added here, can we clean up the corresponding one in
struct cppi41_dma_controller and struct cppi?

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

Re: [PATCH v3 1/3] usb: musb: dma: Add a DMA completion platform callback

[Alexandre Bailon]

On 01/20/2017 09:00 PM, Bin Liu wrote:
> On Thu, Jan 19, 2017 at 11:06:57AM +0100, Alexandre Bailon wrote:
>> Currently, the CPPI 4.1 driver is not completely generic and
>> only work on dsps. This is because of IRQ management.
>> Add a callback to dma_controller that could be invoked on DMA completion
>> to acknodlege the IRQ.
>>
>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>>  drivers/usb/musb/musb_cppi41.c | 7 +++++--
>>  drivers/usb/musb/musb_dma.h    | 4 ++++
>>  2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
>> index 1636385..f7d3d27 100644
>> --- a/drivers/usb/musb/musb_cppi41.c
>> +++ b/drivers/usb/musb/musb_cppi41.c
>> @@ -217,6 +217,10 @@ static void cppi41_dma_callback(void *private_data)
>>  	int is_hs = 0;
>>  	bool empty;
>>  
>> +	controller = cppi41_channel->controller;
>> +	if (controller->controller.dma_callback)
>> +		controller->controller.dma_callback(&controller->controller);
>> +
>>  	spin_lock_irqsave(&musb->lock, flags);
>>  
>>  	dmaengine_tx_status(cppi41_channel->dc, cppi41_channel->cookie,
>> @@ -249,8 +253,6 @@ static void cppi41_dma_callback(void *private_data)
>>  	 * We spin on HS (no longer than than 25us and setup a timer on
>>  	 * FS to check for the bit and complete the transfer.
>>  	 */
>> -	controller = cppi41_channel->controller;
>> -
>>  	if (is_host_active(musb)) {
>>  		if (musb->port1_status & USB_PORT_STAT_HIGH_SPEED)
>>  			is_hs = 1;
>> @@ -695,6 +697,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.musb = musb;
>>  
>>  	ret = cppi41_dma_controller_start(controller);
>>  	if (ret)
>> diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
>> index 46357e1..8bea0cd 100644
>> --- a/drivers/usb/musb/musb_dma.h
>> +++ b/drivers/usb/musb/musb_dma.h
>> @@ -181,10 +181,13 @@ dma_channel_status(struct dma_channel *c)
>>   * @channel_release: call this to release a DMA channel
>>   * @channel_abort: call this to abort a pending DMA transaction,
>>   *	returning it to FREE (but allocated) state
>> + * @platform_dma_callback: invoked on DMA completion, useful to run platform
>> + *	code such IRQ acknowledgment.
>>   *
>>   * Controllers manage dma channels.
>>   */
>>  struct dma_controller {
>> +	struct musb *musb;
> 
> Since musb is added here, can we clean up the corresponding one in
> struct cppi41_dma_controller and struct cppi?
Yes, and also for the one in tusb_omap_dma.
> 
> Regards,
> -Bin.
> 
Regards,
Alexandre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/3] usb: musb: cppi41: Detect aborted transfers in cppi41_dma_callback()

[Alexandre Bailon]

Update cppi41_dma_callback() to detect an aborted transfer.
This was not required before because cppi41_dma_callback() was only
invoked on transfer completion.
In order to make CPPI 4.1 driver more generic, cppi41_dma_callback()
will be invoked after a transfer abort in order to let the MUSB driver
perform some action such as acknowledge the interrupt that may be fired
during a teardown.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/usb/musb/musb_cppi41.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index f7d3d27..1fe7eae 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -99,7 +99,8 @@ static bool musb_is_tx_fifo_empty(struct musb_hw_ep *hw_ep)
 	return true;
 }
 
-static void cppi41_dma_callback(void *private_data);
+static void cppi41_dma_callback(void *private_data,
+				const struct dmaengine_result *result);
 
 static void cppi41_trans_done(struct cppi41_dma_channel *cppi41_channel)
 {
@@ -154,7 +155,7 @@ static void cppi41_trans_done(struct cppi41_dma_channel *cppi41_channel)
 		if (WARN_ON(!dma_desc))
 			return;
 
-		dma_desc->callback = cppi41_dma_callback;
+		dma_desc->callback_result = cppi41_dma_callback;
 		dma_desc->callback_param = &cppi41_channel->channel;
 		cppi41_channel->cookie = dma_desc->tx_submit(dma_desc);
 		trace_musb_cppi41_cont(cppi41_channel);
@@ -204,7 +205,8 @@ static enum hrtimer_restart cppi41_recheck_tx_req(struct hrtimer *timer)
 	return ret;
 }
 
-static void cppi41_dma_callback(void *private_data)
+static void cppi41_dma_callback(void *private_data,
+				const struct dmaengine_result *result)
 {
 	struct dma_channel *channel = private_data;
 	struct cppi41_dma_channel *cppi41_channel = channel->private_data;
@@ -221,6 +223,9 @@ static void cppi41_dma_callback(void *private_data)
 	if (controller->controller.dma_callback)
 		controller->controller.dma_callback(&controller->controller);
 
+	if (result->result == DMA_TRANS_ABORTED)
+		return;
+
 	spin_lock_irqsave(&musb->lock, flags);
 
 	dmaengine_tx_status(cppi41_channel->dc, cppi41_channel->cookie,
@@ -403,7 +408,7 @@ static bool cppi41_configure_channel(struct dma_channel *channel,
 	if (!dma_desc)
 		return false;
 
-	dma_desc->callback = cppi41_dma_callback;
+	dma_desc->callback_result = cppi41_dma_callback;
 	dma_desc->callback_param = channel;
 	cppi41_channel->cookie = dma_desc->tx_submit(dma_desc);
 	cppi41_channel->channel.rx_packet_done = false;
-- 
2.10.2

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

[PATCH v3 3/3] usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in dsps

[Alexandre Bailon]

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-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/cppi41.c         | 28 ++++------------
 drivers/usb/musb/musb_dsps.c | 77 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 82 insertions(+), 23 deletions(-)

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
@@ -79,14 +79,6 @@
 #define QMGR_QUEUE_C(n)	(0x2008 + (n) * 0x10)
 #define QMGR_QUEUE_D(n)	(0x200c + (n) * 0x10)
 
-/* Glue layer specific */
-/* 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)
-
 /* Packet Descriptor */
 #define PD2_ZERO_LENGTH		(1 << 19)
 
@@ -288,14 +280,8 @@ static irqreturn_t cppi41_irq(int irq, void *data)
 {
 	struct cppi41_dd *cdd = data;
 	struct cppi41_channel *c;
-	u32 status;
 	int i;
 
-	status = cppi_readl(cdd->usbss_mem + USBSS_IRQ_STATUS);
-	if (!(status & USBSS_IRQ_PD_COMP))
-		return IRQ_NONE;
-	cppi_writel(status, cdd->usbss_mem + USBSS_IRQ_STATUS);
-
 	for (i = QMGR_PENDING_SLOT_Q(FIST_COMPLETION_QUEUE); i < QMGR_NUM_PEND;
 			i++) {
 		u32 val;
@@ -599,6 +585,7 @@ static void cppi41_compute_td_desc(struct cppi41_desc *d)
 
 static int cppi41_tear_down_chan(struct cppi41_channel *c)
 {
+	struct dmaengine_result abort_result;
 	struct cppi41_dd *cdd = c->cdd;
 	struct cppi41_desc *td;
 	u32 reg;
@@ -682,6 +669,12 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c)
 	c->td_seen = 0;
 	c->td_desc_seen = 0;
 	cppi_writel(0, c->gcr_reg);
+
+	/* Invoke the callback to do the necessary clean-up */
+	abort_result.result = DMA_TRANS_ABORTED;
+	dma_cookie_complete(&c->txd);
+	dmaengine_desc_get_callback_invoke(&c->txd, &abort_result);
+
 	return 0;
 }
 
@@ -1044,8 +1037,6 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 		goto err_irq;
 	}
 
-	cppi_writel(USBSS_IRQ_PD_COMP, cdd->usbss_mem + USBSS_IRQ_ENABLER);
-
 	ret = devm_request_irq(&pdev->dev, irq, glue_info->isr, IRQF_SHARED,
 			dev_name(dev), cdd);
 	if (ret)
@@ -1069,7 +1060,6 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 	dma_async_device_unregister(&cdd->ddev);
 err_dma_reg:
 err_irq:
-	cppi_writel(0, cdd->usbss_mem + USBSS_IRQ_CLEARR);
 	cleanup_chans(cdd);
 err_chans:
 	deinit_cppi41(dev, cdd);
@@ -1097,7 +1087,6 @@ static int cppi41_dma_remove(struct platform_device *pdev)
 	of_dma_controller_free(pdev->dev.of_node);
 	dma_async_device_unregister(&cdd->ddev);
 
-	cppi_writel(0, cdd->usbss_mem + USBSS_IRQ_CLEARR);
 	devm_free_irq(&pdev->dev, cdd->irq, cdd);
 	cleanup_chans(cdd);
 	deinit_cppi41(&pdev->dev, cdd);
@@ -1116,7 +1105,6 @@ static int __maybe_unused cppi41_suspend(struct device *dev)
 	struct cppi41_dd *cdd = dev_get_drvdata(dev);
 
 	cdd->dma_tdfdq = cppi_readl(cdd->ctrl_mem + DMA_TDFDQ);
-	cppi_writel(0, cdd->usbss_mem + USBSS_IRQ_CLEARR);
 	disable_sched(cdd);
 
 	return 0;
@@ -1142,8 +1130,6 @@ static int __maybe_unused cppi41_resume(struct device *dev)
 	cppi_writel(QMGR_SCRATCH_SIZE, cdd->qmgr_mem + QMGR_LRAM_SIZE);
 	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
 
-	cppi_writel(USBSS_IRQ_PD_COMP, cdd->usbss_mem + USBSS_IRQ_ENABLER);
-
 	return 0;
 }
 
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)
+
 /**
  * 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);
+}
+#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)
+		return -ENXIO;
 
 	platform_set_drvdata(pdev, glue);
 	pm_runtime_enable(&pdev->dev);
@@ -880,6 +949,8 @@ static int dsps_suspend(struct device *dev)
 	glue->context.tx_mode = musb_readl(mbase, wrp->tx_mode);
 	glue->context.rx_mode = musb_readl(mbase, wrp->rx_mode);
 
+	dsps_dma_controller_suspend(glue);
+
 	return 0;
 }
 
@@ -893,6 +964,8 @@ static int dsps_resume(struct device *dev)
 	if (!musb)
 		return 0;
 
+	dsps_dma_controller_resume(glue);
+
 	mbase = musb->ctrl_base;
 	musb_writel(mbase, wrp->control, glue->context.control);
 	musb_writel(mbase, wrp->epintr_set, glue->context.epintr);
-- 
2.10.2

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

Re: [PATCH v3 3/3] usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in dsps

[Bin Liu]

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-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
>  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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/3] usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in dsps

[Alexandre Bailon]

On 01/20/2017 09:17 PM, Bin Liu wrote:
> 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-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>>  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()?
I don't we can use it here. As far I know, of_iomap() only return NULL
in the case of error.
> 
>> +		return -ENXIO;
> 
> and return PTR_ERR()?
Again, I don't think we can use it here. THough, may be -ENXIO is not
the best error code to return. I used it because it was the used in DMA
driver.
> 
> Regards,
> -Bin.
> 
Regards,
Alexandre

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

Re: [PATCH v3 3/3] usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in dsps

[Kevin Hilman]

Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> writes:

> 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-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>>  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...

For patches where the change needs to go together, then one maintainer
can ack, and the other can merge.  Alternately, if the patch can be done
in a way that the parts can go independently, that is sometimes
cleaner.  I don't know this code well enough to know which is which.

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

Re: [PATCH v3 3/3] usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in dsps

[Bin Liu]

On Mon, Jan 23, 2017 at 09:44:42AM -0800, Kevin Hilman wrote:
> Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> writes:
> 
> > 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-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> >> ---
> >>  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...
> 
> For patches where the change needs to go together, then one maintainer
> can ack, and the other can merge.  Alternately, if the patch can be done

Ok, this is what I thought :)

> in a way that the parts can go independently, that is sometimes
> cleaner.  I don't know this code well enough to know which is which.

In this case it is better to keep all together, since it moves code from
one place to another.

I think it makes sense it goes to my tree, since the reset patches are
for musb.

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