[PATCH 00/11] dmaengine: cppi41: Add dma support to da8xx

[PATCH 00/11] dmaengine: cppi41: Add dma support to da8xx

[Alexandre Bailon]

This series adds support of da8xx to cppi41 dma controller driver.
This update the cppi41 driver to make it more generic (was only supporting
the am335x), and implement the support for the da8xx.
Some other changes are required in platform and musb drivers to make the dma
work on da8xx (though, it should build correctly and should not cause
any issues).

The changes have been tested on the beaglebone black and omapl138-lcd.
I haven't noticed any regression on beaglebone black
(though I have noticed some performence issues since 4.9).

On on da8xx, I have sometime some warnings happening during a teardown.
I only got them while I was running some corner cases and I'm still trying
to fix them. 
Anyway, this warnings doesn't seem to cause any issues as usb keep working
after they happen.

I also got some issues related to pm runtime.
I tried to fix them with the series "dmaengine: cppi41: PM runtime fixes",
but I still get few warnings sometime (again, doesn't seem to cause any issue).

Alexandre Bailon (11):
  dmaengine: cppi41: rename platform variables
  dmaengine: cppi41: Split out the interrupt handler
  dmaengine: cppi41: Move some constants to glue layer
  dmaengine: cppi41: init_sched(): Get number of channels from DT
  dmaengine: cppi41: Add a way to test if the driver is running on
    am335x
  dmaengine: cppi41: Only configure am335x's registers on amm335x
    platform
  dt/bindings: da8xx-usb: Add binding for the cppi41 dma controller
  dmaengine: cppi41: Implement the glue for da8xx
  dmaengine: cppi41: Fix a race between PM runtime and channel abort
  dmaengine: cppi41: Fix da8xx interrupt issue
  dmaengine: cppi41: Fix teardown warnings

 .../devicetree/bindings/usb/da8xx-usb.txt          |  39 ++++
 drivers/dma/cppi41.c                               | 252 +++++++++++++++++----
 2 files changed, 247 insertions(+), 44 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 01/11] dmaengine: cppi41: rename platform variables

[Alexandre Bailon]

Currently, only the am335x is supported by the driver.
Though the driver has a glue layer to support different platforms,
some platform variable names are not prefixed with the platform name.
To facilitate the addition of a new platform,
rename some variables owned by the am335x glue.

Currently, only the am335x is supported by the driver.
Though the driver provide a glue layer that should be used to support
another platforms. Rename variable specifics to the am335x glue.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/cppi41.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index d5ba43a..25ee71d 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -156,7 +156,7 @@ struct cppi41_dd {
 };
 
 #define FIST_COMPLETION_QUEUE	93
-static struct chan_queues usb_queues_tx[] = {
+static struct chan_queues am335x_usb_queues_tx[] = {
 	/* USB0 ENDP 1 */
 	[ 0] = { .submit = 32, .complete =  93},
 	[ 1] = { .submit = 34, .complete =  94},
@@ -192,7 +192,7 @@ static struct chan_queues usb_queues_tx[] = {
 	[29] = { .submit = 90, .complete = 139},
 };
 
-static const struct chan_queues usb_queues_rx[] = {
+static const struct chan_queues am335x_usb_queues_rx[] = {
 	/* USB0 ENDP 1 */
 	[ 0] = { .submit =  1, .complete = 109},
 	[ 1] = { .submit =  2, .complete = 110},
@@ -918,8 +918,9 @@ static bool cpp41_dma_filter_fn(struct dma_chan *chan, void *param)
 	else
 		queues = cdd->queues_rx;
 
-	BUILD_BUG_ON(ARRAY_SIZE(usb_queues_rx) != ARRAY_SIZE(usb_queues_tx));
-	if (WARN_ON(cchan->port_num > ARRAY_SIZE(usb_queues_rx)))
+	BUILD_BUG_ON(ARRAY_SIZE(am335x_usb_queues_rx) !=
+		     ARRAY_SIZE(am335x_usb_queues_tx));
+	if (WARN_ON(cchan->port_num > ARRAY_SIZE(am335x_usb_queues_rx)))
 		return false;
 
 	cchan->q_num = queues[cchan->port_num].submit;
@@ -947,15 +948,15 @@ static struct dma_chan *cppi41_dma_xlate(struct of_phandle_args *dma_spec,
 			&dma_spec->args[0]);
 }
 
-static const struct cppi_glue_infos usb_infos = {
+static const struct cppi_glue_infos am335x_usb_infos = {
 	.isr = cppi41_irq,
-	.queues_rx = usb_queues_rx,
-	.queues_tx = usb_queues_tx,
+	.queues_rx = am335x_usb_queues_rx,
+	.queues_tx = am335x_usb_queues_tx,
 	.td_queue = { .submit = 31, .complete = 0 },
 };
 
 static const struct of_device_id cppi41_dma_ids[] = {
-	{ .compatible = "ti,am3359-cppi41", .data = &usb_infos},
+	{ .compatible = "ti,am3359-cppi41", .data = &am335x_usb_infos},
 	{},
 };
 MODULE_DEVICE_TABLE(of, cppi41_dma_ids);
-- 
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 01/11] dmaengine: cppi41: rename platform variables

[Sergei Shtylyov]

Hello!

On 01/09/2017 07:06 PM, Alexandre Bailon wrote:

> Currently, only the am335x is supported by the driver.
> Though the driver has a glue layer to support different platforms,
> some platform variable names are not prefixed with the platform name.
> To facilitate the addition of a new platform,
> rename some variables owned by the am335x glue.
>
> Currently, only the am335x is supported by the driver.
> Though the driver provide a glue layer that should be used to support
> another platforms. Rename variable specifics to the am335x glue.

    You are repeating yourself. :-)

> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
[...]

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

[PATCH 02/11] dmaengine: cppi41: Split out the interrupt handler

[Alexandre Bailon]

The current interrupt handler do some actions specifics to am335x.
These actions should be made by the platform interrupt handler.
Split out the interrupt handler in two:
one for the am335x platform and a generic one.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/cppi41.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 25ee71d..42744ed 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -284,18 +284,11 @@ static u32 cppi41_pop_desc(struct cppi41_dd *cdd, unsigned queue_num)
 	return desc;
 }
 
-static irqreturn_t cppi41_irq(int irq, void *data)
+static irqreturn_t cppi41_irq(struct cppi41_dd *cdd)
 {
-	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;
@@ -351,6 +344,19 @@ static irqreturn_t cppi41_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t am335x_cppi41_irq(int irq, void *data)
+{
+	struct cppi41_dd *cdd = data;
+	u32 status;
+
+	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);
+
+	return cppi41_irq(cdd);
+}
+
 static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
 {
 	dma_cookie_t cookie;
@@ -949,7 +955,7 @@ static struct dma_chan *cppi41_dma_xlate(struct of_phandle_args *dma_spec,
 }
 
 static const struct cppi_glue_infos am335x_usb_infos = {
-	.isr = cppi41_irq,
+	.isr = am335x_cppi41_irq,
 	.queues_rx = am335x_usb_queues_rx,
 	.queues_tx = am335x_usb_queues_tx,
 	.td_queue = { .submit = 31, .complete = 0 },
-- 
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 02/11] dmaengine: cppi41: Split out the interrupt handler

[Sergei Shtylyov]

Hello.

On 01/09/2017 07:06 PM, Alexandre Bailon wrote:

> The current interrupt handler do some actions specifics to am335x.
> These actions should be made by the platform interrupt handler.
> Split out the interrupt handler in two:
> one for the am335x platform and a generic one.
>
> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/dma/cppi41.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> index 25ee71d..42744ed 100644
> --- a/drivers/dma/cppi41.c
> +++ b/drivers/dma/cppi41.c
> @@ -284,18 +284,11 @@ static u32 cppi41_pop_desc(struct cppi41_dd *cdd, unsigned queue_num)
>  	return desc;
>  }
>
> -static irqreturn_t cppi41_irq(int irq, void *data)
> +static irqreturn_t cppi41_irq(struct cppi41_dd *cdd)
>  {
> -	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);
> -

    I fail to understand why non-CPPI 4.1 regs are used here (and that 
positioning itself as an "abstract" CPPI 4.1 driver).

[...]
> @@ -351,6 +344,19 @@ static irqreturn_t cppi41_irq(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>
> +static irqreturn_t am335x_cppi41_irq(int irq, void *data)
> +{
> +	struct cppi41_dd *cdd = data;
> +	u32 status;
> +
> +	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);
> +
> +	return cppi41_irq(cdd);
> +}
> +

    IMHO this stuff just needs to move to the MUSB glue.

[...]

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

[PATCH 03/11] dmaengine: cppi41: Move some constants to glue layer

[Alexandre Bailon]

Some constants are defined and use by the driver whereas they are
specifics to am335x.
Add new variables to the glue layer, initialize them with the constants,
and use them in the driver.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/cppi41.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 42744ed..d0d3bdd 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -68,7 +68,6 @@
 #define QMGR_MEMCTRL_IDX_SH	16
 #define QMGR_MEMCTRL_DESC_SH	8
 
-#define QMGR_NUM_PEND	5
 #define QMGR_PEND(x)	(0x90 + (x) * 4)
 
 #define QMGR_PENDING_SLOT_Q(x)	(x / 32)
@@ -147,6 +146,8 @@ struct cppi41_dd {
 	const struct chan_queues *queues_rx;
 	const struct chan_queues *queues_tx;
 	struct chan_queues td_queue;
+	u16 first_completion_queue;
+	u16 qmgr_num_pend;
 
 	struct list_head pending;	/* Pending queued transfers */
 	spinlock_t lock;		/* Lock for pending list */
@@ -155,7 +156,6 @@ struct cppi41_dd {
 	unsigned int dma_tdfdq;
 };
 
-#define FIST_COMPLETION_QUEUE	93
 static struct chan_queues am335x_usb_queues_tx[] = {
 	/* USB0 ENDP 1 */
 	[ 0] = { .submit = 32, .complete =  93},
@@ -233,6 +233,8 @@ struct cppi_glue_infos {
 	const struct chan_queues *queues_rx;
 	const struct chan_queues *queues_tx;
 	struct chan_queues td_queue;
+	u16 first_completion_queue;
+	u16 qmgr_num_pend;
 };
 
 static struct cppi41_channel *to_cpp41_chan(struct dma_chan *c)
@@ -286,19 +288,21 @@ static u32 cppi41_pop_desc(struct cppi41_dd *cdd, unsigned queue_num)
 
 static irqreturn_t cppi41_irq(struct cppi41_dd *cdd)
 {
+	u16 first_completion_queue = cdd->first_completion_queue;
+	u16 qmgr_num_pend = cdd->qmgr_num_pend;
 	struct cppi41_channel *c;
 	int i;
 
-	for (i = QMGR_PENDING_SLOT_Q(FIST_COMPLETION_QUEUE); i < QMGR_NUM_PEND;
+	for (i = QMGR_PENDING_SLOT_Q(first_completion_queue); i < qmgr_num_pend;
 			i++) {
 		u32 val;
 		u32 q_num;
 
 		val = cppi_readl(cdd->qmgr_mem + QMGR_PEND(i));
-		if (i == QMGR_PENDING_SLOT_Q(FIST_COMPLETION_QUEUE) && val) {
+		if (i == QMGR_PENDING_SLOT_Q(first_completion_queue) && val) {
 			u32 mask;
 			/* set corresponding bit for completetion Q 93 */
-			mask = 1 << QMGR_PENDING_BIT_Q(FIST_COMPLETION_QUEUE);
+			mask = 1 << QMGR_PENDING_BIT_Q(first_completion_queue);
 			/* not set all bits for queues less than Q 93 */
 			mask--;
 			/* now invert and keep only Q 93+ set */
@@ -876,7 +880,7 @@ static int init_cppi41(struct device *dev, struct cppi41_dd *cdd)
 		return -ENOMEM;
 
 	cppi_writel(cdd->scratch_phys, cdd->qmgr_mem + QMGR_LRAM0_BASE);
-	cppi_writel(QMGR_SCRATCH_SIZE, cdd->qmgr_mem + QMGR_LRAM_SIZE);
+	cppi_writel(TOTAL_DESCS_NUM, cdd->qmgr_mem + QMGR_LRAM_SIZE);
 	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
 
 	ret = init_descs(dev, cdd);
@@ -959,6 +963,8 @@ static const struct cppi_glue_infos am335x_usb_infos = {
 	.queues_rx = am335x_usb_queues_rx,
 	.queues_tx = am335x_usb_queues_tx,
 	.td_queue = { .submit = 31, .complete = 0 },
+	.first_completion_queue = 93,
+	.qmgr_num_pend = 5,
 };
 
 static const struct of_device_id cppi41_dma_ids[] = {
@@ -1036,6 +1042,8 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 	cdd->queues_rx = glue_info->queues_rx;
 	cdd->queues_tx = glue_info->queues_tx;
 	cdd->td_queue = glue_info->td_queue;
+	cdd->qmgr_num_pend = glue_info->qmgr_num_pend;
+	cdd->first_completion_queue = glue_info->first_completion_queue;
 
 	ret = init_cppi41(dev, cdd);
 	if (ret)
-- 
2.10.2

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

[PATCH 04/11] dmaengine: cppi41: init_sched(): Get number of channels from DT

[Alexandre Bailon]

Despite the driver is already using DT to get the number of channels,
init_sched() is using an hardcoded value to get it.
Use DT to get the number of channels.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/cppi41.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index d0d3bdd..8d7965d 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -148,6 +148,8 @@ struct cppi41_dd {
 	struct chan_queues td_queue;
 	u16 first_completion_queue;
 	u16 qmgr_num_pend;
+	u32 n_chans;
+	u8 platform;
 
 	struct list_head pending;	/* Pending queued transfers */
 	spinlock_t lock;		/* Lock for pending list */
@@ -734,13 +736,8 @@ static int cppi41_add_chans(struct device *dev, struct cppi41_dd *cdd)
 {
 	struct cppi41_channel *cchan;
 	int i;
-	int ret;
-	u32 n_chans;
+	u32 n_chans = cdd->n_chans;
 
-	ret = of_property_read_u32(dev->of_node, "#dma-channels",
-			&n_chans);
-	if (ret)
-		return ret;
 	/*
 	 * The channels can only be used as TX or as RX. So we add twice
 	 * that much dma channels because USB can only do RX or TX.
@@ -846,7 +843,7 @@ static int init_descs(struct device *dev, struct cppi41_dd *cdd)
 	return 0;
 }
 
-static void init_sched(struct cppi41_dd *cdd)
+static int init_sched(struct device *dev, struct cppi41_dd *cdd)
 {
 	unsigned ch;
 	unsigned word;
@@ -854,7 +851,7 @@ static void init_sched(struct cppi41_dd *cdd)
 
 	word = 0;
 	cppi_writel(0, cdd->sched_mem + DMA_SCHED_CTRL);
-	for (ch = 0; ch < 15 * 2; ch += 2) {
+	for (ch = 0; ch < cdd->n_chans; ch += 2) {
 
 		reg = SCHED_ENTRY0_CHAN(ch);
 		reg |= SCHED_ENTRY1_CHAN(ch) | SCHED_ENTRY1_IS_RX;
@@ -864,9 +861,11 @@ static void init_sched(struct cppi41_dd *cdd)
 		cppi_writel(reg, cdd->sched_mem + DMA_SCHED_WORD(word));
 		word++;
 	}
-	reg = 15 * 2 * 2 - 1;
+	reg = cdd->n_chans * 2 - 1;
 	reg |= DMA_SCHED_CTRL_EN;
 	cppi_writel(reg, cdd->sched_mem + DMA_SCHED_CTRL);
+
+	return 0;
 }
 
 static int init_cppi41(struct device *dev, struct cppi41_dd *cdd)
@@ -885,12 +884,14 @@ static int init_cppi41(struct device *dev, struct cppi41_dd *cdd)
 
 	ret = init_descs(dev, cdd);
 	if (ret)
-		goto err_td;
+		goto deinit;
 
 	cppi_writel(cdd->td_queue.submit, cdd->ctrl_mem + DMA_TDFDQ);
-	init_sched(cdd);
+	ret = init_sched(dev, cdd);
+	if (ret)
+		goto deinit;
 	return 0;
-err_td:
+deinit:
 	deinit_cppi41(dev, cdd);
 	return ret;
 }
@@ -1045,6 +1046,11 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 	cdd->qmgr_num_pend = glue_info->qmgr_num_pend;
 	cdd->first_completion_queue = glue_info->first_completion_queue;
 
+	ret = of_property_read_u32(dev->of_node,
+				   "#dma-channels", &cdd->n_chans);
+	if (ret)
+		goto err_get_n_chans;
+
 	ret = init_cppi41(dev, cdd);
 	if (ret)
 		goto err_init_cppi;
@@ -1090,6 +1096,7 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 	deinit_cppi41(dev, cdd);
 err_init_cppi:
 	pm_runtime_dont_use_autosuspend(dev);
+err_get_n_chans:
 err_get_sync:
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
@@ -1150,7 +1157,7 @@ static int __maybe_unused cppi41_resume(struct device *dev)
 		if (!c->is_tx)
 			cppi_writel(c->q_num, c->gcr_reg + RXHPCRA0);
 
-	init_sched(cdd);
+	init_sched(dev, cdd);
 
 	cppi_writel(cdd->dma_tdfdq, cdd->ctrl_mem + DMA_TDFDQ);
 	cppi_writel(cdd->scratch_phys, cdd->qmgr_mem + QMGR_LRAM0_BASE);
-- 
2.10.2

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

[PATCH 05/11] dmaengine: cppi41: Add a way to test if the driver is running on am335x

[Alexandre Bailon]

As the cppi41 is present in different platform, the driver needs
to determine on which platform it is running to execute instructions
specific to this platform (such as configure IRQ on am335x).
Add a way to test if we are running on am335x.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/cppi41.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 8d7965d..58b27ef 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -89,6 +89,8 @@
 /* Packet Descriptor */
 #define PD2_ZERO_LENGTH		(1 << 19)
 
+#define AM335X_CPPI41		0
+
 struct cppi41_channel {
 	struct dma_chan chan;
 	struct dma_async_tx_descriptor txd;
@@ -237,6 +239,7 @@ struct cppi_glue_infos {
 	struct chan_queues td_queue;
 	u16 first_completion_queue;
 	u16 qmgr_num_pend;
+	u8 platform;
 };
 
 static struct cppi41_channel *to_cpp41_chan(struct dma_chan *c)
@@ -966,6 +969,7 @@ static const struct cppi_glue_infos am335x_usb_infos = {
 	.td_queue = { .submit = 31, .complete = 0 },
 	.first_completion_queue = 93,
 	.qmgr_num_pend = 5,
+	.platform = AM335X_CPPI41,
 };
 
 static const struct of_device_id cppi41_dma_ids[] = {
@@ -984,6 +988,13 @@ static const struct cppi_glue_infos *get_glue_info(struct device *dev)
 	return of_id->data;
 }
 
+static int is_am335x_cppi41(struct device *dev)
+{
+	struct cppi41_dd *cdd = dev_get_drvdata(dev);
+
+	return cdd->platform == AM335X_CPPI41;
+}
+
 #define CPPI41_DMA_BUSWIDTHS	(BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
 				BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
 				BIT(DMA_SLAVE_BUSWIDTH_3_BYTES) | \
@@ -1045,6 +1056,7 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 	cdd->td_queue = glue_info->td_queue;
 	cdd->qmgr_num_pend = glue_info->qmgr_num_pend;
 	cdd->first_completion_queue = glue_info->first_completion_queue;
+	cdd->platform = glue_info->platform;
 
 	ret = of_property_read_u32(dev->of_node,
 				   "#dma-channels", &cdd->n_chans);
-- 
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 06/11] dmaengine: cppi41: Only configure am335x's registers on amm335x platform

[Alexandre Bailon]

Currently, the driver configure some am335x's usb registers.
Test if the driver is running on am335x before to configure
these registers.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/cppi41.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 58b27ef..939398e 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -1077,7 +1077,9 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 		goto err_irq;
 	}
 
-	cppi_writel(USBSS_IRQ_PD_COMP, cdd->usbss_mem + USBSS_IRQ_ENABLER);
+	if (is_am335x_cppi41(dev))
+		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);
@@ -1102,7 +1104,8 @@ 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);
+	if (is_am335x_cppi41(dev))
+		cppi_writel(0, cdd->usbss_mem + USBSS_IRQ_CLEARR);
 	cleanup_chans(cdd);
 err_chans:
 	deinit_cppi41(dev, cdd);
@@ -1131,7 +1134,8 @@ 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);
+	if (is_am335x_cppi41(&pdev->dev))
+		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);
@@ -1150,7 +1154,8 @@ 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);
+	if (is_am335x_cppi41(dev))
+		cppi_writel(0, cdd->usbss_mem + USBSS_IRQ_CLEARR);
 	disable_sched(cdd);
 
 	return 0;
@@ -1176,7 +1181,9 @@ 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);
+	if (is_am335x_cppi41(dev))
+		cppi_writel(USBSS_IRQ_PD_COMP,
+			    cdd->usbss_mem + USBSS_IRQ_ENABLER);
 
 	return 0;
 }
-- 
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 07/11] dt/bindings: da8xx-usb: Add binding for the cppi41 dma controller

[Alexandre Bailon]

DT binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx cppi41 dma controller.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 .../devicetree/bindings/usb/da8xx-usb.txt          | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
index ccb844a..2a4d737 100644
--- a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
+++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
@@ -18,10 +18,26 @@ Required properties:
 
  - phy-names: Should be "usb-phy"
 
+ - dmas: specifies the dma channels
+
+ - dma-names: specifies the names of the channels. Use "rxN" for receive
+   and "txN" for transmit endpoints. N specifies the endpoint number.
+
 Optional properties:
 ~~~~~~~~~~~~~~~~~~~~
  - vbus-supply: Phandle to a regulator providing the USB bus power.
 
+DMA
+~~~
+- compatible: ti,da8xx-cppi41
+- reg: offset and length of the following register spaces: USBSS, USB
+  CPPI DMA Controller, USB CPPI DMA Scheduler, USB Queue Manager
+- reg-names: glue, controller, scheduler, queuemgr
+- #dma-cells: should be set to 2. The first number represents the
+  endpoint number (0 … 3 for endpoints 1 … 4).
+  The second number is 0 for RX and 1 for TX transfers.
+- #dma-channels: should be set to 4 representing the 4 endpoints.
+
 Example:
 	usb_phy: usb-phy {
 		compatible = "ti,da830-usb-phy";
@@ -39,5 +55,28 @@ Example:
 		phys = <&usb_phy 0>;
 		phy-names = "usb-phy";
 
+		dmas = <&cppi41dma 0 0 &cppi41dma 1 0
+			&cppi41dma 2 0 &cppi41dma 3 0
+			&cppi41dma 0 1 &cppi41dma 1 1
+			&cppi41dma 2 1 &cppi41dma 3 1>;
+		dma-names =
+			"rx1", "rx2", "rx3", "rx4",
+			"tx1", "tx2", "tx3", "tx4";
+
 		status = "okay";
 	};
+	cppi41dma: dma-controller@201000 {
+		compatible = "ti,da8xx-cppi41";
+		reg =  <0x200000 0x1000
+			0x201000 0x1000
+			0x202000 0x1000
+			0x204000 0x4000>;
+		reg-names = "glue", "controller", "scheduler", "queuemgr";
+		interrupts = <58>;
+		interrupt-names = "glue";
+		#dma-cells = <2>;
+		#dma-channels = <4>;
+		#dma-requests = <256>;
+		status = "disabled";
+
+	};
-- 
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 07/11] dt/bindings: da8xx-usb: Add binding for the cppi41 dma controller

[Sergei Shtylyov]

Hello!

On 01/09/2017 07:06 PM, Alexandre Bailon wrote:

> DT binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx cppi41 dma controller.

    It's called CPPI 4.1, not cppi41.
    Let me introduce myself: I was the author of the 1st CPPI 4.1 drivers 
(never merged).

> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
>  .../devicetree/bindings/usb/da8xx-usb.txt          | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
> index ccb844a..2a4d737 100644
> --- a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
> @@ -18,10 +18,26 @@ Required properties:
[...]
>  Optional properties:
>  ~~~~~~~~~~~~~~~~~~~~
>   - vbus-supply: Phandle to a regulator providing the USB bus power.
>
> +DMA
> +~~~
> +- compatible: ti,da8xx-cppi41
> +- reg: offset and length of the following register spaces: USBSS, USB

    I don't understand what you mean by USBSS and how it's related to CPPI 4.1.

> +  CPPI DMA Controller, USB CPPI DMA Scheduler, USB Queue Manager

    I'd drop "USB" everywhere, the DMAC scheduler and queue manager are not a 
part of any USB logic.

> +- reg-names: glue, controller, scheduler, queuemgr

    Quoted, maybe?

> +- #dma-cells: should be set to 2. The first number represents the
> +  endpoint number (0 … 3 for endpoints 1 … 4).

    Not sure why you need EPs here. Is that required by the DMA driver?
The DMA controller operates with channels...

[...]

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 07/11] dt/bindings: da8xx-usb: Add binding for the cppi41 dma controller

[Alexandre Bailon]

Hello,

On 01/09/2017 07:41 PM, Sergei Shtylyov wrote:
> Hello!
> 
> On 01/09/2017 07:06 PM, Alexandre Bailon wrote:
> 
>> DT binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx cppi41 dma controller.
> 
>    It's called CPPI 4.1, not cppi41.
>    Let me introduce myself: I was the author of the 1st CPPI 4.1 drivers
> (never merged).
> 
>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>>  .../devicetree/bindings/usb/da8xx-usb.txt          | 39
>> ++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> index ccb844a..2a4d737 100644
>> --- a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> +++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> @@ -18,10 +18,26 @@ Required properties:
> [...]
>>  Optional properties:
>>  ~~~~~~~~~~~~~~~~~~~~
>>   - vbus-supply: Phandle to a regulator providing the USB bus power.
>>
>> +DMA
>> +~~~
>> +- compatible: ti,da8xx-cppi41
>> +- reg: offset and length of the following register spaces: USBSS, USB
> 
>    I don't understand what you mean by USBSS and how it's related to
> CPPI 4.1.
I have kept the terminology used in the CPPI 4.1 driver.
USBSS is how the MUSB glue is named in AM335x datasheet.
The CPPI 4.1 driver need to access to few of this registers.
> 
>> +  CPPI DMA Controller, USB CPPI DMA Scheduler, USB Queue Manager
> 
>    I'd drop "USB" everywhere, the DMAC scheduler and queue manager are
> not a part of any USB logic.
Will do it.
> 
>> +- reg-names: glue, controller, scheduler, queuemgr
> 
>    Quoted, maybe?
> 
>> +- #dma-cells: should be set to 2. The first number represents the
>> +  endpoint number (0 … 3 for endpoints 1 … 4).
> 
>    Not sure why you need EPs here. Is that required by the DMA driver?
> The DMA controller operates with channels...
I have taken this from am335x-usb bindings and I kept it as is because
I don't think that's wrong.
I agree we should use DMA terminology but actually the CPPI 4.1 is only
used by USB in DA8xx (share the same clock, the same irq, etc).
In the case of DA8xx, I'm not sure that would make things more clear.

Best Regards,
Alexandre
> 
> [...]
> 
> MBR, Sergei
> 

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

[PATCH 08/11] dmaengine: cppi41: Implement the glue for da8xx

[Alexandre Bailon]

The da8xx has a cppi41 dma controller.
This is add the glue layer required to make it work on da8xx,
as well some changes in driver (e.g to manage clock).

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/cppi41.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 939398e..4318e53 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -1,3 +1,4 @@
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/dmaengine.h>
 #include <linux/dma-mapping.h>
@@ -86,10 +87,19 @@
 
 #define USBSS_IRQ_PD_COMP	(1 <<  2)
 
+/* USB DA8XX */
+#define DA8XX_INTR_SRC_MASKED	0x38
+#define DA8XX_END_OF_INTR	0x3c
+
+#define DA8XX_QMGR_PENDING_MASK	(0xf << 24)
+
+
+
 /* Packet Descriptor */
 #define PD2_ZERO_LENGTH		(1 << 19)
 
 #define AM335X_CPPI41		0
+#define DA8XX_CPPI41		1
 
 struct cppi41_channel {
 	struct dma_chan chan;
@@ -158,6 +168,9 @@ struct cppi41_dd {
 
 	/* context for suspend/resume */
 	unsigned int dma_tdfdq;
+
+	/* da8xx clock */
+	struct clk *clk;
 };
 
 static struct chan_queues am335x_usb_queues_tx[] = {
@@ -232,6 +245,20 @@ static const struct chan_queues am335x_usb_queues_rx[] = {
 	[29] = { .submit = 30, .complete = 155},
 };
 
+static const struct chan_queues da8xx_usb_queues_tx[] = {
+	[0] = { .submit =  16, .complete = 24},
+	[1] = { .submit =  18, .complete = 24},
+	[2] = { .submit =  20, .complete = 24},
+	[3] = { .submit =  22, .complete = 24},
+};
+
+static const struct chan_queues da8xx_usb_queues_rx[] = {
+	[0] = { .submit =  1, .complete = 26},
+	[1] = { .submit =  3, .complete = 26},
+	[2] = { .submit =  5, .complete = 26},
+	[3] = { .submit =  7, .complete = 26},
+};
+
 struct cppi_glue_infos {
 	irqreturn_t (*isr)(int irq, void *data);
 	const struct chan_queues *queues_rx;
@@ -366,6 +393,26 @@ static irqreturn_t am335x_cppi41_irq(int irq, void *data)
 	return cppi41_irq(cdd);
 }
 
+static irqreturn_t da8xx_cppi41_irq(int irq, void *data)
+{
+	struct cppi41_dd *cdd = data;
+	u32 status;
+	u32 usbss_status;
+
+	status = cppi_readl(cdd->qmgr_mem + QMGR_PEND(0));
+	if (status & DA8XX_QMGR_PENDING_MASK)
+		cppi41_irq(cdd);
+	else
+		return IRQ_NONE;
+
+	/* Re-assert IRQ if there no usb core interrupts pending */
+	usbss_status = cppi_readl(cdd->usbss_mem + DA8XX_INTR_SRC_MASKED);
+	if (!usbss_status)
+		cppi_writel(0, cdd->usbss_mem + DA8XX_END_OF_INTR);
+
+	return IRQ_HANDLED;
+}
+
 static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
 {
 	dma_cookie_t cookie;
@@ -972,8 +1019,19 @@ static const struct cppi_glue_infos am335x_usb_infos = {
 	.platform = AM335X_CPPI41,
 };
 
+static const struct cppi_glue_infos da8xx_usb_infos = {
+	.isr = da8xx_cppi41_irq,
+	.queues_rx = da8xx_usb_queues_rx,
+	.queues_tx = da8xx_usb_queues_tx,
+	.td_queue = { .submit = 31, .complete = 0 },
+	.first_completion_queue = 24,
+	.qmgr_num_pend = 2,
+	.platform = DA8XX_CPPI41,
+};
+
 static const struct of_device_id cppi41_dma_ids[] = {
 	{ .compatible = "ti,am3359-cppi41", .data = &am335x_usb_infos},
+	{ .compatible = "ti,da8xx-cppi41", .data = &da8xx_usb_infos},
 	{},
 };
 MODULE_DEVICE_TABLE(of, cppi41_dma_ids);
@@ -995,6 +1053,13 @@ static int is_am335x_cppi41(struct device *dev)
 	return cdd->platform == AM335X_CPPI41;
 }
 
+static int is_da8xx_cppi41(struct device *dev)
+{
+	struct cppi41_dd *cdd = dev_get_drvdata(dev);
+
+	return cdd->platform == DA8XX_CPPI41;
+}
+
 #define CPPI41_DMA_BUSWIDTHS	(BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
 				BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
 				BIT(DMA_SLAVE_BUSWIDTH_3_BYTES) | \
@@ -1058,6 +1123,21 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 	cdd->first_completion_queue = glue_info->first_completion_queue;
 	cdd->platform = glue_info->platform;
 
+	if (is_da8xx_cppi41(dev)) {
+		cdd->clk = devm_clk_get(&pdev->dev, "usb20");
+		ret = PTR_ERR_OR_ZERO(cdd->clk);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to get clock\n");
+			goto err_clk_en;
+		}
+
+		ret = clk_prepare_enable(cdd->clk);
+		if (ret) {
+			dev_err(dev, "failed to enable clock\n");
+			goto err_clk_en;
+		}
+	}
+
 	ret = of_property_read_u32(dev->of_node,
 				   "#dma-channels", &cdd->n_chans);
 	if (ret)
@@ -1112,6 +1192,9 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 err_init_cppi:
 	pm_runtime_dont_use_autosuspend(dev);
 err_get_n_chans:
+	if (is_da8xx_cppi41(dev))
+		clk_disable_unprepare(cdd->clk);
+err_clk_en:
 err_get_sync:
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
@@ -1146,6 +1229,8 @@ static int cppi41_dma_remove(struct platform_device *pdev)
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+	if (is_da8xx_cppi41(&pdev->dev))
+		clk_disable_unprepare(cdd->clk);
 	return 0;
 }
 
@@ -1158,6 +1243,9 @@ static int __maybe_unused cppi41_suspend(struct device *dev)
 		cppi_writel(0, cdd->usbss_mem + USBSS_IRQ_CLEARR);
 	disable_sched(cdd);
 
+	if (is_da8xx_cppi41(dev))
+		clk_disable_unprepare(cdd->clk);
+
 	return 0;
 }
 
@@ -1165,8 +1253,15 @@ static int __maybe_unused cppi41_resume(struct device *dev)
 {
 	struct cppi41_dd *cdd = dev_get_drvdata(dev);
 	struct cppi41_channel *c;
+	int ret;
 	int i;
 
+	if (is_da8xx_cppi41(dev)) {
+		ret = clk_prepare_enable(cdd->clk);
+		if (ret)
+			return ret;
+	}
+
 	for (i = 0; i < DESCS_AREAS; i++)
 		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
 
-- 
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 08/11] dmaengine: cppi41: Implement the glue for da8xx

[Grygorii Strashko]

On 01/09/2017 10:06 AM, Alexandre Bailon wrote:
> The da8xx has a cppi41 dma controller.
> This is add the glue layer required to make it work on da8xx,
> as well some changes in driver (e.g to manage clock).
> 
> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/dma/cppi41.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
> 
> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> index 939398e..4318e53 100644
> --- a/drivers/dma/cppi41.c
> +++ b/drivers/dma/cppi41.c
> @@ -1,3 +1,4 @@
> +#include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/dmaengine.h>
>  #include <linux/dma-mapping.h>
> @@ -86,10 +87,19 @@
>  
>  #define USBSS_IRQ_PD_COMP	(1 <<  2)
>  
> +/* USB DA8XX */
> +#define DA8XX_INTR_SRC_MASKED	0x38
> +#define DA8XX_END_OF_INTR	0x3c
> +
> +#define DA8XX_QMGR_PENDING_MASK	(0xf << 24)
> +
> +
> +
>  /* Packet Descriptor */
>  #define PD2_ZERO_LENGTH		(1 << 19)
>  
>  #define AM335X_CPPI41		0
> +#define DA8XX_CPPI41		1
>  
>  struct cppi41_channel {
>  	struct dma_chan chan;
> @@ -158,6 +168,9 @@ struct cppi41_dd {
>  
>  	/* context for suspend/resume */
>  	unsigned int dma_tdfdq;
> +
> +	/* da8xx clock */
> +	struct clk *clk;
>  };
>  
>  static struct chan_queues am335x_usb_queues_tx[] = {
> @@ -232,6 +245,20 @@ static const struct chan_queues am335x_usb_queues_rx[] = {
>  	[29] = { .submit = 30, .complete = 155},
>  };
>  
> +static const struct chan_queues da8xx_usb_queues_tx[] = {
> +	[0] = { .submit =  16, .complete = 24},
> +	[1] = { .submit =  18, .complete = 24},
> +	[2] = { .submit =  20, .complete = 24},
> +	[3] = { .submit =  22, .complete = 24},
> +};
> +
> +static const struct chan_queues da8xx_usb_queues_rx[] = {
> +	[0] = { .submit =  1, .complete = 26},
> +	[1] = { .submit =  3, .complete = 26},
> +	[2] = { .submit =  5, .complete = 26},
> +	[3] = { .submit =  7, .complete = 26},
> +};
> +
>  struct cppi_glue_infos {
>  	irqreturn_t (*isr)(int irq, void *data);
>  	const struct chan_queues *queues_rx;
> @@ -366,6 +393,26 @@ static irqreturn_t am335x_cppi41_irq(int irq, void *data)
>  	return cppi41_irq(cdd);
>  }
>  
> +static irqreturn_t da8xx_cppi41_irq(int irq, void *data)
> +{
> +	struct cppi41_dd *cdd = data;
> +	u32 status;
> +	u32 usbss_status;
> +
> +	status = cppi_readl(cdd->qmgr_mem + QMGR_PEND(0));
> +	if (status & DA8XX_QMGR_PENDING_MASK)
> +		cppi41_irq(cdd);
> +	else
> +		return IRQ_NONE;
> +
> +	/* Re-assert IRQ if there no usb core interrupts pending */
> +	usbss_status = cppi_readl(cdd->usbss_mem + DA8XX_INTR_SRC_MASKED);
> +	if (!usbss_status)
> +		cppi_writel(0, cdd->usbss_mem + DA8XX_END_OF_INTR);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
>  {
>  	dma_cookie_t cookie;
> @@ -972,8 +1019,19 @@ static const struct cppi_glue_infos am335x_usb_infos = {
>  	.platform = AM335X_CPPI41,
>  };
>  
> +static const struct cppi_glue_infos da8xx_usb_infos = {
> +	.isr = da8xx_cppi41_irq,
> +	.queues_rx = da8xx_usb_queues_rx,
> +	.queues_tx = da8xx_usb_queues_tx,
> +	.td_queue = { .submit = 31, .complete = 0 },
> +	.first_completion_queue = 24,
> +	.qmgr_num_pend = 2,
> +	.platform = DA8XX_CPPI41,
> +};
> +
>  static const struct of_device_id cppi41_dma_ids[] = {
>  	{ .compatible = "ti,am3359-cppi41", .data = &am335x_usb_infos},
> +	{ .compatible = "ti,da8xx-cppi41", .data = &da8xx_usb_infos},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, cppi41_dma_ids);
> @@ -995,6 +1053,13 @@ static int is_am335x_cppi41(struct device *dev)
>  	return cdd->platform == AM335X_CPPI41;
>  }
>  
> +static int is_da8xx_cppi41(struct device *dev)
> +{
> +	struct cppi41_dd *cdd = dev_get_drvdata(dev);
> +
> +	return cdd->platform == DA8XX_CPPI41;
> +}
> +
>  #define CPPI41_DMA_BUSWIDTHS	(BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
>  				BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
>  				BIT(DMA_SLAVE_BUSWIDTH_3_BYTES) | \
> @@ -1058,6 +1123,21 @@ static int cppi41_dma_probe(struct platform_device *pdev)
>  	cdd->first_completion_queue = glue_info->first_completion_queue;
>  	cdd->platform = glue_info->platform;
>  
> +	if (is_da8xx_cppi41(dev)) {
> +		cdd->clk = devm_clk_get(&pdev->dev, "usb20");
> +		ret = PTR_ERR_OR_ZERO(cdd->clk);
> +		if (ret) {
> +			dev_err(&pdev->dev, "failed to get clock\n");
> +			goto err_clk_en;
> +		}
> +
> +		ret = clk_prepare_enable(cdd->clk);
> +		if (ret) {
> +			dev_err(dev, "failed to enable clock\n");
> +			goto err_clk_en;
> +		}
> +	}

if this is functional clock then why not to use ./arch/arm/mach-davinci/pm_domain.c ?
wouldn't it work for use if you will just rename "usb20" -> "fck" -
so PM runtime should manage this clock for you?

> +
>  	ret = of_property_read_u32(dev->of_node,
>  				   "#dma-channels", &cdd->n_chans);
>  	if (ret)
> @@ -1112,6 +1192,9 @@ static int cppi41_dma_probe(struct platform_device *pdev)
>  err_init_cppi:
>  	pm_runtime_dont_use_autosuspend(dev);
>  err_get_n_chans:
> +	if (is_da8xx_cppi41(dev))
> +		clk_disable_unprepare(cdd->clk);
> +err_clk_en:
>  err_get_sync:
>  	pm_runtime_put_sync(dev);
>  	pm_runtime_disable(dev);
> @@ -1146,6 +1229,8 @@ static int cppi41_dma_remove(struct platform_device *pdev)
>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
> +	if (is_da8xx_cppi41(&pdev->dev))
> +		clk_disable_unprepare(cdd->clk);
>  	return 0;
>  }
>  
> @@ -1158,6 +1243,9 @@ static int __maybe_unused cppi41_suspend(struct device *dev)
>  		cppi_writel(0, cdd->usbss_mem + USBSS_IRQ_CLEARR);
>  	disable_sched(cdd);
>  
> +	if (is_da8xx_cppi41(dev))
> +		clk_disable_unprepare(cdd->clk);
> +
>  	return 0;
>  }
>  
> @@ -1165,8 +1253,15 @@ static int __maybe_unused cppi41_resume(struct device *dev)
>  {
>  	struct cppi41_dd *cdd = dev_get_drvdata(dev);
>  	struct cppi41_channel *c;
> +	int ret;
>  	int i;
>  
> +	if (is_da8xx_cppi41(dev)) {
> +		ret = clk_prepare_enable(cdd->clk);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	for (i = 0; i < DESCS_AREAS; i++)
>  		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
>  
> 

-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 08/11] dmaengine: cppi41: Implement the glue for da8xx

[Alexandre Bailon]

On 01/09/2017 07:08 PM, Grygorii Strashko wrote:
> 
> 
> On 01/09/2017 10:06 AM, Alexandre Bailon wrote:
>> The da8xx has a cppi41 dma controller.
>> This is add the glue layer required to make it work on da8xx,
>> as well some changes in driver (e.g to manage clock).
>>
>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>>  drivers/dma/cppi41.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 95 insertions(+)
>>
>> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
>> index 939398e..4318e53 100644
>> --- a/drivers/dma/cppi41.c
>> +++ b/drivers/dma/cppi41.c
>> @@ -1,3 +1,4 @@
>> +#include <linux/clk.h>
>>  #include <linux/delay.h>
>>  #include <linux/dmaengine.h>
>>  #include <linux/dma-mapping.h>
>> @@ -86,10 +87,19 @@
>>  
>>  #define USBSS_IRQ_PD_COMP	(1 <<  2)
>>  
>> +/* USB DA8XX */
>> +#define DA8XX_INTR_SRC_MASKED	0x38
>> +#define DA8XX_END_OF_INTR	0x3c
>> +
>> +#define DA8XX_QMGR_PENDING_MASK	(0xf << 24)
>> +
>> +
>> +
>>  /* Packet Descriptor */
>>  #define PD2_ZERO_LENGTH		(1 << 19)
>>  
>>  #define AM335X_CPPI41		0
>> +#define DA8XX_CPPI41		1
>>  
>>  struct cppi41_channel {
>>  	struct dma_chan chan;
>> @@ -158,6 +168,9 @@ struct cppi41_dd {
>>  
>>  	/* context for suspend/resume */
>>  	unsigned int dma_tdfdq;
>> +
>> +	/* da8xx clock */
>> +	struct clk *clk;
>>  };
>>  
>>  static struct chan_queues am335x_usb_queues_tx[] = {
>> @@ -232,6 +245,20 @@ static const struct chan_queues am335x_usb_queues_rx[] = {
>>  	[29] = { .submit = 30, .complete = 155},
>>  };
>>  
>> +static const struct chan_queues da8xx_usb_queues_tx[] = {
>> +	[0] = { .submit =  16, .complete = 24},
>> +	[1] = { .submit =  18, .complete = 24},
>> +	[2] = { .submit =  20, .complete = 24},
>> +	[3] = { .submit =  22, .complete = 24},
>> +};
>> +
>> +static const struct chan_queues da8xx_usb_queues_rx[] = {
>> +	[0] = { .submit =  1, .complete = 26},
>> +	[1] = { .submit =  3, .complete = 26},
>> +	[2] = { .submit =  5, .complete = 26},
>> +	[3] = { .submit =  7, .complete = 26},
>> +};
>> +
>>  struct cppi_glue_infos {
>>  	irqreturn_t (*isr)(int irq, void *data);
>>  	const struct chan_queues *queues_rx;
>> @@ -366,6 +393,26 @@ static irqreturn_t am335x_cppi41_irq(int irq, void *data)
>>  	return cppi41_irq(cdd);
>>  }
>>  
>> +static irqreturn_t da8xx_cppi41_irq(int irq, void *data)
>> +{
>> +	struct cppi41_dd *cdd = data;
>> +	u32 status;
>> +	u32 usbss_status;
>> +
>> +	status = cppi_readl(cdd->qmgr_mem + QMGR_PEND(0));
>> +	if (status & DA8XX_QMGR_PENDING_MASK)
>> +		cppi41_irq(cdd);
>> +	else
>> +		return IRQ_NONE;
>> +
>> +	/* Re-assert IRQ if there no usb core interrupts pending */
>> +	usbss_status = cppi_readl(cdd->usbss_mem + DA8XX_INTR_SRC_MASKED);
>> +	if (!usbss_status)
>> +		cppi_writel(0, cdd->usbss_mem + DA8XX_END_OF_INTR);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>>  static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
>>  {
>>  	dma_cookie_t cookie;
>> @@ -972,8 +1019,19 @@ static const struct cppi_glue_infos am335x_usb_infos = {
>>  	.platform = AM335X_CPPI41,
>>  };
>>  
>> +static const struct cppi_glue_infos da8xx_usb_infos = {
>> +	.isr = da8xx_cppi41_irq,
>> +	.queues_rx = da8xx_usb_queues_rx,
>> +	.queues_tx = da8xx_usb_queues_tx,
>> +	.td_queue = { .submit = 31, .complete = 0 },
>> +	.first_completion_queue = 24,
>> +	.qmgr_num_pend = 2,
>> +	.platform = DA8XX_CPPI41,
>> +};
>> +
>>  static const struct of_device_id cppi41_dma_ids[] = {
>>  	{ .compatible = "ti,am3359-cppi41", .data = &am335x_usb_infos},
>> +	{ .compatible = "ti,da8xx-cppi41", .data = &da8xx_usb_infos},
>>  	{},
>>  };
>>  MODULE_DEVICE_TABLE(of, cppi41_dma_ids);
>> @@ -995,6 +1053,13 @@ static int is_am335x_cppi41(struct device *dev)
>>  	return cdd->platform == AM335X_CPPI41;
>>  }
>>  
>> +static int is_da8xx_cppi41(struct device *dev)
>> +{
>> +	struct cppi41_dd *cdd = dev_get_drvdata(dev);
>> +
>> +	return cdd->platform == DA8XX_CPPI41;
>> +}
>> +
>>  #define CPPI41_DMA_BUSWIDTHS	(BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
>>  				BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
>>  				BIT(DMA_SLAVE_BUSWIDTH_3_BYTES) | \
>> @@ -1058,6 +1123,21 @@ static int cppi41_dma_probe(struct platform_device *pdev)
>>  	cdd->first_completion_queue = glue_info->first_completion_queue;
>>  	cdd->platform = glue_info->platform;
>>  
>> +	if (is_da8xx_cppi41(dev)) {
>> +		cdd->clk = devm_clk_get(&pdev->dev, "usb20");
>> +		ret = PTR_ERR_OR_ZERO(cdd->clk);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "failed to get clock\n");
>> +			goto err_clk_en;
>> +		}
>> +
>> +		ret = clk_prepare_enable(cdd->clk);
>> +		if (ret) {
>> +			dev_err(dev, "failed to enable clock\n");
>> +			goto err_clk_en;
>> +		}
>> +	}
> 
> if this is functional clock then why not to use ./arch/arm/mach-davinci/pm_domain.c ?
> wouldn't it work for use if you will just rename "usb20" -> "fck" -
> so PM runtime should manage this clock for you?
As is, I don't think it will work.
The usb20 is shared by the cppi41 and the usb otg.
So, if we rename "usb20" to "fck", clk_get() won't be able to find the
clock.
But may be adding "usb20" to "con_ids" in
arch/arm/mach-davinci/pm_domain.c could work.
But I think it will require some changes in da8xx musb driver.
I will take look.

Thanks,
Alexandre
> 
>> +
>>  	ret = of_property_read_u32(dev->of_node,
>>  				   "#dma-channels", &cdd->n_chans);
>>  	if (ret)
>> @@ -1112,6 +1192,9 @@ static int cppi41_dma_probe(struct platform_device *pdev)
>>  err_init_cppi:
>>  	pm_runtime_dont_use_autosuspend(dev);
>>  err_get_n_chans:
>> +	if (is_da8xx_cppi41(dev))
>> +		clk_disable_unprepare(cdd->clk);
>> +err_clk_en:
>>  err_get_sync:
>>  	pm_runtime_put_sync(dev);
>>  	pm_runtime_disable(dev);
>> @@ -1146,6 +1229,8 @@ static int cppi41_dma_remove(struct platform_device *pdev)
>>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
>>  	pm_runtime_put_sync(&pdev->dev);
>>  	pm_runtime_disable(&pdev->dev);
>> +	if (is_da8xx_cppi41(&pdev->dev))
>> +		clk_disable_unprepare(cdd->clk);
>>  	return 0;
>>  }
>>  
>> @@ -1158,6 +1243,9 @@ static int __maybe_unused cppi41_suspend(struct device *dev)
>>  		cppi_writel(0, cdd->usbss_mem + USBSS_IRQ_CLEARR);
>>  	disable_sched(cdd);
>>  
>> +	if (is_da8xx_cppi41(dev))
>> +		clk_disable_unprepare(cdd->clk);
>> +
>>  	return 0;
>>  }
>>  
>> @@ -1165,8 +1253,15 @@ static int __maybe_unused cppi41_resume(struct device *dev)
>>  {
>>  	struct cppi41_dd *cdd = dev_get_drvdata(dev);
>>  	struct cppi41_channel *c;
>> +	int ret;
>>  	int i;
>>  
>> +	if (is_da8xx_cppi41(dev)) {
>> +		ret = clk_prepare_enable(cdd->clk);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>>  	for (i = 0; i < DESCS_AREAS; i++)
>>  		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
>>  
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 08/11] dmaengine: cppi41: Implement the glue for da8xx

[Sekhar Nori]

On Tuesday 10 January 2017 03:08 PM, Alexandre Bailon wrote:
> On 01/09/2017 07:08 PM, Grygorii Strashko wrote:
>>
>>
>> On 01/09/2017 10:06 AM, Alexandre Bailon wrote:
>>> The da8xx has a cppi41 dma controller.
>>> This is add the glue layer required to make it work on da8xx,
>>> as well some changes in driver (e.g to manage clock).
>>>
>>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>> ---
>>>  drivers/dma/cppi41.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 95 insertions(+)
>>>
>>> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
>>> index 939398e..4318e53 100644
>>> --- a/drivers/dma/cppi41.c
>>> +++ b/drivers/dma/cppi41.c
>>> @@ -1,3 +1,4 @@
>>> +#include <linux/clk.h>
>>>  #include <linux/delay.h>
>>>  #include <linux/dmaengine.h>
>>>  #include <linux/dma-mapping.h>
>>> @@ -86,10 +87,19 @@
>>>  
>>>  #define USBSS_IRQ_PD_COMP	(1 <<  2)
>>>  
>>> +/* USB DA8XX */
>>> +#define DA8XX_INTR_SRC_MASKED	0x38
>>> +#define DA8XX_END_OF_INTR	0x3c
>>> +
>>> +#define DA8XX_QMGR_PENDING_MASK	(0xf << 24)
>>> +
>>> +
>>> +
>>>  /* Packet Descriptor */
>>>  #define PD2_ZERO_LENGTH		(1 << 19)
>>>  
>>>  #define AM335X_CPPI41		0
>>> +#define DA8XX_CPPI41		1
>>>  
>>>  struct cppi41_channel {
>>>  	struct dma_chan chan;
>>> @@ -158,6 +168,9 @@ struct cppi41_dd {
>>>  
>>>  	/* context for suspend/resume */
>>>  	unsigned int dma_tdfdq;
>>> +
>>> +	/* da8xx clock */
>>> +	struct clk *clk;
>>>  };
>>>  
>>>  static struct chan_queues am335x_usb_queues_tx[] = {
>>> @@ -232,6 +245,20 @@ static const struct chan_queues am335x_usb_queues_rx[] = {
>>>  	[29] = { .submit = 30, .complete = 155},
>>>  };
>>>  
>>> +static const struct chan_queues da8xx_usb_queues_tx[] = {
>>> +	[0] = { .submit =  16, .complete = 24},
>>> +	[1] = { .submit =  18, .complete = 24},
>>> +	[2] = { .submit =  20, .complete = 24},
>>> +	[3] = { .submit =  22, .complete = 24},
>>> +};
>>> +
>>> +static const struct chan_queues da8xx_usb_queues_rx[] = {
>>> +	[0] = { .submit =  1, .complete = 26},
>>> +	[1] = { .submit =  3, .complete = 26},
>>> +	[2] = { .submit =  5, .complete = 26},
>>> +	[3] = { .submit =  7, .complete = 26},
>>> +};
>>> +
>>>  struct cppi_glue_infos {
>>>  	irqreturn_t (*isr)(int irq, void *data);
>>>  	const struct chan_queues *queues_rx;
>>> @@ -366,6 +393,26 @@ static irqreturn_t am335x_cppi41_irq(int irq, void *data)
>>>  	return cppi41_irq(cdd);
>>>  }
>>>  
>>> +static irqreturn_t da8xx_cppi41_irq(int irq, void *data)
>>> +{
>>> +	struct cppi41_dd *cdd = data;
>>> +	u32 status;
>>> +	u32 usbss_status;
>>> +
>>> +	status = cppi_readl(cdd->qmgr_mem + QMGR_PEND(0));
>>> +	if (status & DA8XX_QMGR_PENDING_MASK)
>>> +		cppi41_irq(cdd);
>>> +	else
>>> +		return IRQ_NONE;
>>> +
>>> +	/* Re-assert IRQ if there no usb core interrupts pending */
>>> +	usbss_status = cppi_readl(cdd->usbss_mem + DA8XX_INTR_SRC_MASKED);
>>> +	if (!usbss_status)
>>> +		cppi_writel(0, cdd->usbss_mem + DA8XX_END_OF_INTR);
>>> +
>>> +	return IRQ_HANDLED;
>>> +}
>>> +
>>>  static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
>>>  {
>>>  	dma_cookie_t cookie;
>>> @@ -972,8 +1019,19 @@ static const struct cppi_glue_infos am335x_usb_infos = {
>>>  	.platform = AM335X_CPPI41,
>>>  };
>>>  
>>> +static const struct cppi_glue_infos da8xx_usb_infos = {
>>> +	.isr = da8xx_cppi41_irq,
>>> +	.queues_rx = da8xx_usb_queues_rx,
>>> +	.queues_tx = da8xx_usb_queues_tx,
>>> +	.td_queue = { .submit = 31, .complete = 0 },
>>> +	.first_completion_queue = 24,
>>> +	.qmgr_num_pend = 2,
>>> +	.platform = DA8XX_CPPI41,
>>> +};
>>> +
>>>  static const struct of_device_id cppi41_dma_ids[] = {
>>>  	{ .compatible = "ti,am3359-cppi41", .data = &am335x_usb_infos},
>>> +	{ .compatible = "ti,da8xx-cppi41", .data = &da8xx_usb_infos},
>>>  	{},
>>>  };
>>>  MODULE_DEVICE_TABLE(of, cppi41_dma_ids);
>>> @@ -995,6 +1053,13 @@ static int is_am335x_cppi41(struct device *dev)
>>>  	return cdd->platform == AM335X_CPPI41;
>>>  }
>>>  
>>> +static int is_da8xx_cppi41(struct device *dev)
>>> +{
>>> +	struct cppi41_dd *cdd = dev_get_drvdata(dev);
>>> +
>>> +	return cdd->platform == DA8XX_CPPI41;
>>> +}
>>> +
>>>  #define CPPI41_DMA_BUSWIDTHS	(BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
>>>  				BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
>>>  				BIT(DMA_SLAVE_BUSWIDTH_3_BYTES) | \
>>> @@ -1058,6 +1123,21 @@ static int cppi41_dma_probe(struct platform_device *pdev)
>>>  	cdd->first_completion_queue = glue_info->first_completion_queue;
>>>  	cdd->platform = glue_info->platform;
>>>  
>>> +	if (is_da8xx_cppi41(dev)) {
>>> +		cdd->clk = devm_clk_get(&pdev->dev, "usb20");
>>> +		ret = PTR_ERR_OR_ZERO(cdd->clk);
>>> +		if (ret) {
>>> +			dev_err(&pdev->dev, "failed to get clock\n");
>>> +			goto err_clk_en;
>>> +		}
>>> +
>>> +		ret = clk_prepare_enable(cdd->clk);
>>> +		if (ret) {
>>> +			dev_err(dev, "failed to enable clock\n");
>>> +			goto err_clk_en;
>>> +		}
>>> +	}
>>
>> if this is functional clock then why not to use ./arch/arm/mach-davinci/pm_domain.c ?
>> wouldn't it work for use if you will just rename "usb20" -> "fck" -
>> so PM runtime should manage this clock for you?
> As is, I don't think it will work.
> The usb20 is shared by the cppi41 and the usb otg.
> So, if we rename "usb20" to "fck", clk_get() won't be able to find the
> clock.
> But may be adding "usb20" to "con_ids" in
> arch/arm/mach-davinci/pm_domain.c could work.
> But I think it will require some changes in da8xx musb driver.
> I will take look.

On DA8xx, CPPI 4.1 DMAengine is not an independent system resource, but
embedded within the USB 2.0 controller. So, I think all that is needed
is for MUSB DA8xx glue to trigger probe of CPPI 4.1 dmaengine driver
when it is ready. I am not sure all this DA850-specific clock handling
is really necessary.

Even in DT, the CPPI 4.1 node should be a child node of USB 2.0 node.

Thanks,
sekhar
--
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 08/11] dmaengine: cppi41: Implement the glue for da8xx

[Alexandre Bailon]

On 01/10/2017 11:05 AM, Sekhar Nori wrote:
> On Tuesday 10 January 2017 03:08 PM, Alexandre Bailon wrote:
>> On 01/09/2017 07:08 PM, Grygorii Strashko wrote:
>>>
>>>
>>> On 01/09/2017 10:06 AM, Alexandre Bailon wrote:
>>>> The da8xx has a cppi41 dma controller.
>>>> This is add the glue layer required to make it work on da8xx,
>>>> as well some changes in driver (e.g to manage clock).
>>>>
>>>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>>  drivers/dma/cppi41.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 95 insertions(+)
>>>>
>>>> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
>>>> index 939398e..4318e53 100644
>>>> --- a/drivers/dma/cppi41.c
>>>> +++ b/drivers/dma/cppi41.c
>>>> @@ -1,3 +1,4 @@
>>>> +#include <linux/clk.h>
>>>>  #include <linux/delay.h>
>>>>  #include <linux/dmaengine.h>
>>>>  #include <linux/dma-mapping.h>
>>>> @@ -86,10 +87,19 @@
>>>>  
>>>>  #define USBSS_IRQ_PD_COMP	(1 <<  2)
>>>>  
>>>> +/* USB DA8XX */
>>>> +#define DA8XX_INTR_SRC_MASKED	0x38
>>>> +#define DA8XX_END_OF_INTR	0x3c
>>>> +
>>>> +#define DA8XX_QMGR_PENDING_MASK	(0xf << 24)
>>>> +
>>>> +
>>>> +
>>>>  /* Packet Descriptor */
>>>>  #define PD2_ZERO_LENGTH		(1 << 19)
>>>>  
>>>>  #define AM335X_CPPI41		0
>>>> +#define DA8XX_CPPI41		1
>>>>  
>>>>  struct cppi41_channel {
>>>>  	struct dma_chan chan;
>>>> @@ -158,6 +168,9 @@ struct cppi41_dd {
>>>>  
>>>>  	/* context for suspend/resume */
>>>>  	unsigned int dma_tdfdq;
>>>> +
>>>> +	/* da8xx clock */
>>>> +	struct clk *clk;
>>>>  };
>>>>  
>>>>  static struct chan_queues am335x_usb_queues_tx[] = {
>>>> @@ -232,6 +245,20 @@ static const struct chan_queues am335x_usb_queues_rx[] = {
>>>>  	[29] = { .submit = 30, .complete = 155},
>>>>  };
>>>>  
>>>> +static const struct chan_queues da8xx_usb_queues_tx[] = {
>>>> +	[0] = { .submit =  16, .complete = 24},
>>>> +	[1] = { .submit =  18, .complete = 24},
>>>> +	[2] = { .submit =  20, .complete = 24},
>>>> +	[3] = { .submit =  22, .complete = 24},
>>>> +};
>>>> +
>>>> +static const struct chan_queues da8xx_usb_queues_rx[] = {
>>>> +	[0] = { .submit =  1, .complete = 26},
>>>> +	[1] = { .submit =  3, .complete = 26},
>>>> +	[2] = { .submit =  5, .complete = 26},
>>>> +	[3] = { .submit =  7, .complete = 26},
>>>> +};
>>>> +
>>>>  struct cppi_glue_infos {
>>>>  	irqreturn_t (*isr)(int irq, void *data);
>>>>  	const struct chan_queues *queues_rx;
>>>> @@ -366,6 +393,26 @@ static irqreturn_t am335x_cppi41_irq(int irq, void *data)
>>>>  	return cppi41_irq(cdd);
>>>>  }
>>>>  
>>>> +static irqreturn_t da8xx_cppi41_irq(int irq, void *data)
>>>> +{
>>>> +	struct cppi41_dd *cdd = data;
>>>> +	u32 status;
>>>> +	u32 usbss_status;
>>>> +
>>>> +	status = cppi_readl(cdd->qmgr_mem + QMGR_PEND(0));
>>>> +	if (status & DA8XX_QMGR_PENDING_MASK)
>>>> +		cppi41_irq(cdd);
>>>> +	else
>>>> +		return IRQ_NONE;
>>>> +
>>>> +	/* Re-assert IRQ if there no usb core interrupts pending */
>>>> +	usbss_status = cppi_readl(cdd->usbss_mem + DA8XX_INTR_SRC_MASKED);
>>>> +	if (!usbss_status)
>>>> +		cppi_writel(0, cdd->usbss_mem + DA8XX_END_OF_INTR);
>>>> +
>>>> +	return IRQ_HANDLED;
>>>> +}
>>>> +
>>>>  static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
>>>>  {
>>>>  	dma_cookie_t cookie;
>>>> @@ -972,8 +1019,19 @@ static const struct cppi_glue_infos am335x_usb_infos = {
>>>>  	.platform = AM335X_CPPI41,
>>>>  };
>>>>  
>>>> +static const struct cppi_glue_infos da8xx_usb_infos = {
>>>> +	.isr = da8xx_cppi41_irq,
>>>> +	.queues_rx = da8xx_usb_queues_rx,
>>>> +	.queues_tx = da8xx_usb_queues_tx,
>>>> +	.td_queue = { .submit = 31, .complete = 0 },
>>>> +	.first_completion_queue = 24,
>>>> +	.qmgr_num_pend = 2,
>>>> +	.platform = DA8XX_CPPI41,
>>>> +};
>>>> +
>>>>  static const struct of_device_id cppi41_dma_ids[] = {
>>>>  	{ .compatible = "ti,am3359-cppi41", .data = &am335x_usb_infos},
>>>> +	{ .compatible = "ti,da8xx-cppi41", .data = &da8xx_usb_infos},
>>>>  	{},
>>>>  };
>>>>  MODULE_DEVICE_TABLE(of, cppi41_dma_ids);
>>>> @@ -995,6 +1053,13 @@ static int is_am335x_cppi41(struct device *dev)
>>>>  	return cdd->platform == AM335X_CPPI41;
>>>>  }
>>>>  
>>>> +static int is_da8xx_cppi41(struct device *dev)
>>>> +{
>>>> +	struct cppi41_dd *cdd = dev_get_drvdata(dev);
>>>> +
>>>> +	return cdd->platform == DA8XX_CPPI41;
>>>> +}
>>>> +
>>>>  #define CPPI41_DMA_BUSWIDTHS	(BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
>>>>  				BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
>>>>  				BIT(DMA_SLAVE_BUSWIDTH_3_BYTES) | \
>>>> @@ -1058,6 +1123,21 @@ static int cppi41_dma_probe(struct platform_device *pdev)
>>>>  	cdd->first_completion_queue = glue_info->first_completion_queue;
>>>>  	cdd->platform = glue_info->platform;
>>>>  
>>>> +	if (is_da8xx_cppi41(dev)) {
>>>> +		cdd->clk = devm_clk_get(&pdev->dev, "usb20");
>>>> +		ret = PTR_ERR_OR_ZERO(cdd->clk);
>>>> +		if (ret) {
>>>> +			dev_err(&pdev->dev, "failed to get clock\n");
>>>> +			goto err_clk_en;
>>>> +		}
>>>> +
>>>> +		ret = clk_prepare_enable(cdd->clk);
>>>> +		if (ret) {
>>>> +			dev_err(dev, "failed to enable clock\n");
>>>> +			goto err_clk_en;
>>>> +		}
>>>> +	}
>>>
>>> if this is functional clock then why not to use ./arch/arm/mach-davinci/pm_domain.c ?
>>> wouldn't it work for use if you will just rename "usb20" -> "fck" -
>>> so PM runtime should manage this clock for you?
>> As is, I don't think it will work.
>> The usb20 is shared by the cppi41 and the usb otg.
>> So, if we rename "usb20" to "fck", clk_get() won't be able to find the
>> clock.
>> But may be adding "usb20" to "con_ids" in
>> arch/arm/mach-davinci/pm_domain.c could work.
>> But I think it will require some changes in da8xx musb driver.
>> I will take look.
> 
> On DA8xx, CPPI 4.1 DMAengine is not an independent system resource, but
> embedded within the USB 2.0 controller. So, I think all that is needed
> is for MUSB DA8xx glue to trigger probe of CPPI 4.1 dmaengine driver
> when it is ready. I am not sure all this DA850-specific clock handling
> is really necessary.
Actually, we have a circular dependency.
USB core tries to get DMA channels during the probe, which fails because
CPPI 4.1 driver is not ready.
But it will never be ready because the USB clock must be enabled before
DMA driver probe, what will not happen because USB driver have disabled
the clock when probe failed.

Someone in the office suggested me to use the component API,
that could help me to probe the DMA from the USB probe.

Another way to workaround the dependency would be to do defer the
function calls that access to hardware to avoid to control clock from
DMA driver.
> 
> Even in DT, the CPPI 4.1 node should be a child node of USB 2.0 node.
I agree, it should a child but it would require some changes in CPPI 4.1
driver. But except to have a better hardware description, I don't see
any benefit to do it.
> 
> Thanks,
> sekhar
> 
Thanks,
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 08/11] dmaengine: cppi41: Implement the glue for da8xx

[Tony Lindgren]

* Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> [170110 07:23]:
> On 01/10/2017 11:05 AM, Sekhar Nori wrote:
> > On DA8xx, CPPI 4.1 DMAengine is not an independent system resource, but
> > embedded within the USB 2.0 controller. So, I think all that is needed
> > is for MUSB DA8xx glue to trigger probe of CPPI 4.1 dmaengine driver
> > when it is ready. I am not sure all this DA850-specific clock handling
> > is really necessary.
> Actually, we have a circular dependency.
> USB core tries to get DMA channels during the probe, which fails because
> CPPI 4.1 driver is not ready.
> But it will never be ready because the USB clock must be enabled before
> DMA driver probe, what will not happen because USB driver have disabled
> the clock when probe failed.
> 
> Someone in the office suggested me to use the component API,
> that could help me to probe the DMA from the USB probe.
> 
> Another way to workaround the dependency would be to do defer the
> function calls that access to hardware to avoid to control clock from
> DMA driver.

Or you really have some wrapper IP block around musb and cppi41 just
like am335x has.

See drivers/usb/musb/musb_am335x.c and compatible properties for
"ti,am33xx-usb" and it's children in am33xx.dtsi.

Regards,

Tony
--
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 08/11] dmaengine: cppi41: Implement the glue for da8xx

[Sekhar Nori]

On Tuesday 10 January 2017 09:19 PM, Tony Lindgren wrote:
> * Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> [170110 07:23]:
>> On 01/10/2017 11:05 AM, Sekhar Nori wrote:
>>> On DA8xx, CPPI 4.1 DMAengine is not an independent system resource, but
>>> embedded within the USB 2.0 controller. So, I think all that is needed
>>> is for MUSB DA8xx glue to trigger probe of CPPI 4.1 dmaengine driver
>>> when it is ready. I am not sure all this DA850-specific clock handling
>>> is really necessary.
>> Actually, we have a circular dependency.
>> USB core tries to get DMA channels during the probe, which fails because
>> CPPI 4.1 driver is not ready.
>> But it will never be ready because the USB clock must be enabled before
>> DMA driver probe, what will not happen because USB driver have disabled
>> the clock when probe failed.
>>
>> Someone in the office suggested me to use the component API,
>> that could help me to probe the DMA from the USB probe.
>>
>> Another way to workaround the dependency would be to do defer the
>> function calls that access to hardware to avoid to control clock from
>> DMA driver.
> 
> Or you really have some wrapper IP block around musb and cppi41 just
> like am335x has.
> 
> See drivers/usb/musb/musb_am335x.c and compatible properties for
> "ti,am33xx-usb" and it's children in am33xx.dtsi.

This looks like what will will need to da850 too.

Alexandre,

If the wrapper is going to work, can you see if it will end up breaking
DT compatibility once introduced? If yes, I suggest reverting the usb1
DT node in da850-lcdk.dts so we don't get into a DT compatibility
problem once v4.10 releases.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 08/11] dmaengine: cppi41: Implement the glue for da8xx

[Alexandre Bailon]

On 01/10/2017 04:22 PM, Alexandre Bailon wrote:
> On 01/10/2017 11:05 AM, Sekhar Nori wrote:
>> On Tuesday 10 January 2017 03:08 PM, Alexandre Bailon wrote:
>>> On 01/09/2017 07:08 PM, Grygorii Strashko wrote:
>>>>
>>>>
>>>> On 01/09/2017 10:06 AM, Alexandre Bailon wrote:
>>>>> The da8xx has a cppi41 dma controller.
>>>>> This is add the glue layer required to make it work on da8xx,
>>>>> as well some changes in driver (e.g to manage clock).
>>>>>
>>>>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>>>> ---
>>>>>  drivers/dma/cppi41.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 95 insertions(+)
>>>>>
>>>>> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
>>>>> index 939398e..4318e53 100644
>>>>> --- a/drivers/dma/cppi41.c
>>>>> +++ b/drivers/dma/cppi41.c
>>>>> @@ -1,3 +1,4 @@
>>>>> +#include <linux/clk.h>
>>>>>  #include <linux/delay.h>
>>>>>  #include <linux/dmaengine.h>
>>>>>  #include <linux/dma-mapping.h>
>>>>> @@ -86,10 +87,19 @@
>>>>>  
>>>>>  #define USBSS_IRQ_PD_COMP	(1 <<  2)
>>>>>  
>>>>> +/* USB DA8XX */
>>>>> +#define DA8XX_INTR_SRC_MASKED	0x38
>>>>> +#define DA8XX_END_OF_INTR	0x3c
>>>>> +
>>>>> +#define DA8XX_QMGR_PENDING_MASK	(0xf << 24)
>>>>> +
>>>>> +
>>>>> +
>>>>>  /* Packet Descriptor */
>>>>>  #define PD2_ZERO_LENGTH		(1 << 19)
>>>>>  
>>>>>  #define AM335X_CPPI41		0
>>>>> +#define DA8XX_CPPI41		1
>>>>>  
>>>>>  struct cppi41_channel {
>>>>>  	struct dma_chan chan;
>>>>> @@ -158,6 +168,9 @@ struct cppi41_dd {
>>>>>  
>>>>>  	/* context for suspend/resume */
>>>>>  	unsigned int dma_tdfdq;
>>>>> +
>>>>> +	/* da8xx clock */
>>>>> +	struct clk *clk;
>>>>>  };
>>>>>  
>>>>>  static struct chan_queues am335x_usb_queues_tx[] = {
>>>>> @@ -232,6 +245,20 @@ static const struct chan_queues am335x_usb_queues_rx[] = {
>>>>>  	[29] = { .submit = 30, .complete = 155},
>>>>>  };
>>>>>  
>>>>> +static const struct chan_queues da8xx_usb_queues_tx[] = {
>>>>> +	[0] = { .submit =  16, .complete = 24},
>>>>> +	[1] = { .submit =  18, .complete = 24},
>>>>> +	[2] = { .submit =  20, .complete = 24},
>>>>> +	[3] = { .submit =  22, .complete = 24},
>>>>> +};
>>>>> +
>>>>> +static const struct chan_queues da8xx_usb_queues_rx[] = {
>>>>> +	[0] = { .submit =  1, .complete = 26},
>>>>> +	[1] = { .submit =  3, .complete = 26},
>>>>> +	[2] = { .submit =  5, .complete = 26},
>>>>> +	[3] = { .submit =  7, .complete = 26},
>>>>> +};
>>>>> +
>>>>>  struct cppi_glue_infos {
>>>>>  	irqreturn_t (*isr)(int irq, void *data);
>>>>>  	const struct chan_queues *queues_rx;
>>>>> @@ -366,6 +393,26 @@ static irqreturn_t am335x_cppi41_irq(int irq, void *data)
>>>>>  	return cppi41_irq(cdd);
>>>>>  }
>>>>>  
>>>>> +static irqreturn_t da8xx_cppi41_irq(int irq, void *data)
>>>>> +{
>>>>> +	struct cppi41_dd *cdd = data;
>>>>> +	u32 status;
>>>>> +	u32 usbss_status;
>>>>> +
>>>>> +	status = cppi_readl(cdd->qmgr_mem + QMGR_PEND(0));
>>>>> +	if (status & DA8XX_QMGR_PENDING_MASK)
>>>>> +		cppi41_irq(cdd);
>>>>> +	else
>>>>> +		return IRQ_NONE;
>>>>> +
>>>>> +	/* Re-assert IRQ if there no usb core interrupts pending */
>>>>> +	usbss_status = cppi_readl(cdd->usbss_mem + DA8XX_INTR_SRC_MASKED);
>>>>> +	if (!usbss_status)
>>>>> +		cppi_writel(0, cdd->usbss_mem + DA8XX_END_OF_INTR);
>>>>> +
>>>>> +	return IRQ_HANDLED;
>>>>> +}
>>>>> +
>>>>>  static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
>>>>>  {
>>>>>  	dma_cookie_t cookie;
>>>>> @@ -972,8 +1019,19 @@ static const struct cppi_glue_infos am335x_usb_infos = {
>>>>>  	.platform = AM335X_CPPI41,
>>>>>  };
>>>>>  
>>>>> +static const struct cppi_glue_infos da8xx_usb_infos = {
>>>>> +	.isr = da8xx_cppi41_irq,
>>>>> +	.queues_rx = da8xx_usb_queues_rx,
>>>>> +	.queues_tx = da8xx_usb_queues_tx,
>>>>> +	.td_queue = { .submit = 31, .complete = 0 },
>>>>> +	.first_completion_queue = 24,
>>>>> +	.qmgr_num_pend = 2,
>>>>> +	.platform = DA8XX_CPPI41,
>>>>> +};
>>>>> +
>>>>>  static const struct of_device_id cppi41_dma_ids[] = {
>>>>>  	{ .compatible = "ti,am3359-cppi41", .data = &am335x_usb_infos},
>>>>> +	{ .compatible = "ti,da8xx-cppi41", .data = &da8xx_usb_infos},
>>>>>  	{},
>>>>>  };
>>>>>  MODULE_DEVICE_TABLE(of, cppi41_dma_ids);
>>>>> @@ -995,6 +1053,13 @@ static int is_am335x_cppi41(struct device *dev)
>>>>>  	return cdd->platform == AM335X_CPPI41;
>>>>>  }
>>>>>  
>>>>> +static int is_da8xx_cppi41(struct device *dev)
>>>>> +{
>>>>> +	struct cppi41_dd *cdd = dev_get_drvdata(dev);
>>>>> +
>>>>> +	return cdd->platform == DA8XX_CPPI41;
>>>>> +}
>>>>> +
>>>>>  #define CPPI41_DMA_BUSWIDTHS	(BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
>>>>>  				BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
>>>>>  				BIT(DMA_SLAVE_BUSWIDTH_3_BYTES) | \
>>>>> @@ -1058,6 +1123,21 @@ static int cppi41_dma_probe(struct platform_device *pdev)
>>>>>  	cdd->first_completion_queue = glue_info->first_completion_queue;
>>>>>  	cdd->platform = glue_info->platform;
>>>>>  
>>>>> +	if (is_da8xx_cppi41(dev)) {
>>>>> +		cdd->clk = devm_clk_get(&pdev->dev, "usb20");
>>>>> +		ret = PTR_ERR_OR_ZERO(cdd->clk);
>>>>> +		if (ret) {
>>>>> +			dev_err(&pdev->dev, "failed to get clock\n");
>>>>> +			goto err_clk_en;
>>>>> +		}
>>>>> +
>>>>> +		ret = clk_prepare_enable(cdd->clk);
>>>>> +		if (ret) {
>>>>> +			dev_err(dev, "failed to enable clock\n");
>>>>> +			goto err_clk_en;
>>>>> +		}
>>>>> +	}
>>>>
>>>> if this is functional clock then why not to use ./arch/arm/mach-davinci/pm_domain.c ?
>>>> wouldn't it work for use if you will just rename "usb20" -> "fck" -
>>>> so PM runtime should manage this clock for you?
>>> As is, I don't think it will work.
>>> The usb20 is shared by the cppi41 and the usb otg.
>>> So, if we rename "usb20" to "fck", clk_get() won't be able to find the
>>> clock.
>>> But may be adding "usb20" to "con_ids" in
>>> arch/arm/mach-davinci/pm_domain.c could work.
>>> But I think it will require some changes in da8xx musb driver.
>>> I will take look.
>>
>> On DA8xx, CPPI 4.1 DMAengine is not an independent system resource, but
>> embedded within the USB 2.0 controller. So, I think all that is needed
>> is for MUSB DA8xx glue to trigger probe of CPPI 4.1 dmaengine driver
>> when it is ready. I am not sure all this DA850-specific clock handling
>> is really necessary.
> Actually, we have a circular dependency.
> USB core tries to get DMA channels during the probe, which fails because
> CPPI 4.1 driver is not ready.
> But it will never be ready because the USB clock must be enabled before
> DMA driver probe, what will not happen because USB driver have disabled
> the clock when probe failed.
> 
> Someone in the office suggested me to use the component API,
> that could help me to probe the DMA from the USB probe.
> 
> Another way to workaround the dependency would be to do defer the
> function calls that access to hardware to avoid to control clock from
> DMA driver.
>>
>> Even in DT, the CPPI 4.1 node should be a child node of USB 2.0 node.
> I agree, it should a child but it would require some changes in CPPI 4.1
> driver. But except to have a better hardware description, I don't see
> any benefit to do it.
Finally, I have been able to do it and there is real benefit to do it
(unlike I thought).
Now that the CPPI 4.1 is a child of USB 2.0, I only have to update
the DA8xx glue driver to support PM runtime and let PM runtime do
everything in CPPI 4.1 driver.
>>
>> Thanks,
>> sekhar
>>
> Thanks,
> 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 08/11] dmaengine: cppi41: Implement the glue for da8xx

[Sergei Shtylyov]

On 01/09/2017 07:06 PM, Alexandre Bailon wrote:

> The da8xx has a cppi41 dma controller.

    It's called CPPI 4.1. :-)

> This is add the glue layer required to make it work on da8xx,
> as well some changes in driver (e.g to manage clock).
>
> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/dma/cppi41.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
>
> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> index 939398e..4318e53 100644
> --- a/drivers/dma/cppi41.c
> +++ b/drivers/dma/cppi41.c
[...]
> @@ -86,10 +87,19 @@
>
>  #define USBSS_IRQ_PD_COMP	(1 <<  2)
>
> +/* USB DA8XX */
> +#define DA8XX_INTR_SRC_MASKED	0x38
> +#define DA8XX_END_OF_INTR	0x3c
> +
> +#define DA8XX_QMGR_PENDING_MASK	(0xf << 24)
> +
> +
> +

    One empty line is enough.

>  /* Packet Descriptor */
>  #define PD2_ZERO_LENGTH		(1 << 19)
>
>  #define AM335X_CPPI41		0
> +#define DA8XX_CPPI41		1
>
>  struct cppi41_channel {
>  	struct dma_chan chan;
[...]
> @@ -366,6 +393,26 @@ static irqreturn_t am335x_cppi41_irq(int irq, void *data)
>  	return cppi41_irq(cdd);
>  }
>
> +static irqreturn_t da8xx_cppi41_irq(int irq, void *data)
> +{
> +	struct cppi41_dd *cdd = data;
> +	u32 status;
> +	u32 usbss_status;
> +
> +	status = cppi_readl(cdd->qmgr_mem + QMGR_PEND(0));
> +	if (status & DA8XX_QMGR_PENDING_MASK)
> +		cppi41_irq(cdd);
> +	else
> +		return IRQ_NONE;

    Seems correct...

> +
> +	/* Re-assert IRQ if there no usb core interrupts pending */
> +	usbss_status = cppi_readl(cdd->usbss_mem + DA8XX_INTR_SRC_MASKED);
> +	if (!usbss_status)
> +		cppi_writel(0, cdd->usbss_mem + DA8XX_END_OF_INTR);

    I don't understand this...

> +
> +	return IRQ_HANDLED;
> +}
> +
>  static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
>  {
>  	dma_cookie_t cookie;
[...]

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 08/11] dmaengine: cppi41: Implement the glue for da8xx

[Alexandre Bailon]

On 01/10/2017 06:53 PM, Sergei Shtylyov wrote:
> On 01/09/2017 07:06 PM, Alexandre Bailon wrote:
> 
>> The da8xx has a cppi41 dma controller.
> 
>    It's called CPPI 4.1. :-)
> 
>> This is add the glue layer required to make it work on da8xx,
>> as well some changes in driver (e.g to manage clock).
>>
>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>>  drivers/dma/cppi41.c | 95
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 95 insertions(+)
>>
>> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
>> index 939398e..4318e53 100644
>> --- a/drivers/dma/cppi41.c
>> +++ b/drivers/dma/cppi41.c
> [...]
>> @@ -86,10 +87,19 @@
>>
>>  #define USBSS_IRQ_PD_COMP    (1 <<  2)
>>
>> +/* USB DA8XX */
>> +#define DA8XX_INTR_SRC_MASKED    0x38
>> +#define DA8XX_END_OF_INTR    0x3c
>> +
>> +#define DA8XX_QMGR_PENDING_MASK    (0xf << 24)
>> +
>> +
>> +
> 
>    One empty line is enough.
> 
>>  /* Packet Descriptor */
>>  #define PD2_ZERO_LENGTH        (1 << 19)
>>
>>  #define AM335X_CPPI41        0
>> +#define DA8XX_CPPI41        1
>>
>>  struct cppi41_channel {
>>      struct dma_chan chan;
> [...]
>> @@ -366,6 +393,26 @@ static irqreturn_t am335x_cppi41_irq(int irq,
>> void *data)
>>      return cppi41_irq(cdd);
>>  }
>>
>> +static irqreturn_t da8xx_cppi41_irq(int irq, void *data)
>> +{
>> +    struct cppi41_dd *cdd = data;
>> +    u32 status;
>> +    u32 usbss_status;
>> +
>> +    status = cppi_readl(cdd->qmgr_mem + QMGR_PEND(0));
>> +    if (status & DA8XX_QMGR_PENDING_MASK)
>> +        cppi41_irq(cdd);
>> +    else
>> +        return IRQ_NONE;
> 
>    Seems correct...
> 
>> +
>> +    /* Re-assert IRQ if there no usb core interrupts pending */
>> +    usbss_status = cppi_readl(cdd->usbss_mem + DA8XX_INTR_SRC_MASKED);
>> +    if (!usbss_status)
>> +        cppi_writel(0, cdd->usbss_mem + DA8XX_END_OF_INTR);
> 
>    I don't understand this...
Well, it might not be necessary anymore.
I had an issue with teardown. After a teardown, USB were not working
anymore.
It was because an interrupt was fired on teardown completion  but
because the completion queue was empty, interrupt was ignored by the
interrupt handler. And in USB driver, because the interrupt was not
fired by USB core, then interrupt was ignored and never re-asserted.

But I guess the change I made in patch 11 should prevent this issue.
> 
>> +
>> +    return IRQ_HANDLED;
>> +}
>> +
>>  static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
>>  {
>>      dma_cookie_t cookie;
> [...]
> 
> MBR, Sergei
> 

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

[PATCH 09/11] dmaengine: cppi41: Fix a race between PM runtime and channel abort

[Alexandre Bailon]

cppi41_dma_issue_pending() may be called while the device is runtime
suspended. In that case, the descritpor will be push to the pending
list and then be queued to hardware queue.
But if cppi41_stop_chan() is called before the device got time to
resume, then the descriptor will remain in the pending list and be
queued to hardware queue after the teardown.
During the channel stop, check if there is a pendding descriptor
and if so, remove it.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/cppi41.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 4318e53..e8470b1 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -751,10 +751,17 @@ static int cppi41_stop_chan(struct dma_chan *chan)
 {
 	struct cppi41_channel *c = to_cpp41_chan(chan);
 	struct cppi41_dd *cdd = c->cdd;
+	unsigned long flags;
 	u32 desc_num;
 	u32 desc_phys;
 	int ret;
 
+	/* Remove pending descriptor that haven't been pushed to queue */
+	spin_lock_irqsave(&cdd->lock, flags);
+	if (!list_empty(&c->node))
+		list_del_init(&c->node);
+	spin_unlock_irqrestore(&cdd->lock, flags);
+
 	desc_phys = lower_32_bits(c->desc_phys);
 	desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
 	if (!cdd->chan_busy[desc_num])
@@ -812,6 +819,7 @@ static int cppi41_add_chans(struct device *dev, struct cppi41_dd *cdd)
 		cchan->desc_phys = cdd->descs_phys;
 		cchan->desc_phys += i * sizeof(struct cppi41_desc);
 		cchan->chan.device = &cdd->ddev;
+		INIT_LIST_HEAD(&cchan->node);
 		list_add_tail(&cchan->chan.device_node, &cdd->ddev.channels);
 	}
 	cdd->first_td_desc = n_chans;
@@ -1301,7 +1309,7 @@ static int __maybe_unused cppi41_runtime_resume(struct device *dev)
 	spin_lock_irqsave(&cdd->lock, flags);
 	list_for_each_entry_safe(c, _c, &cdd->pending, node) {
 		push_desc_queue(c);
-		list_del(&c->node);
+		list_del_init(&c->node);
 	}
 	spin_unlock_irqrestore(&cdd->lock, flags);
 
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 09/11] dmaengine: cppi41: Fix a race between PM runtime and channel abort

[Sergei Shtylyov]

On 01/09/2017 07:06 PM, Alexandre Bailon wrote:

> cppi41_dma_issue_pending() may be called while the device is runtime
> suspended. In that case, the descritpor will be push to the pending

   "Descriptor" and "pushed".

> list and then be queued to hardware queue.
> But if cppi41_stop_chan() is called before the device got time to
> resume, then the descriptor will remain in the pending list and be
> queued to hardware queue after the teardown.
> During the channel stop, check if there is a pendding descriptor

    Pending.

> and if so, remove it.
>
> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
[...]

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

[PATCH 10/11] dmaengine: cppi41: Fix da8xx interrupt issue

[Alexandre Bailon]

Sometime, after a teardown, interrupts are not fired anymore.
This happen because the interrupt handler doesn't re-assert IRQ.
Once the teardown complete, the teardown descriptor is moved
to completion queue, which is causing an interrupt.
But cppi41_tear_down_chan() is called from atomic section,
and it polls the queue until it got the teardown descriptor.
Then, the interrupt handler is called but it is not able
detect the cause of the interrupt and assume the interrupt
has been fired by USB core. In that situation, the IRQ won't
be re-asserted and interrupts won't work anymore.
Add the td_complete variable to detect an interrupt fired by
DMA during a teardown, and then re-assert IRQ.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/cppi41.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index e8470b1..0060391 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -171,6 +171,8 @@ struct cppi41_dd {
 
 	/* da8xx clock */
 	struct clk *clk;
+
+	bool td_complete;
 };
 
 static struct chan_queues am335x_usb_queues_tx[] = {
@@ -398,19 +400,29 @@ static irqreturn_t da8xx_cppi41_irq(int irq, void *data)
 	struct cppi41_dd *cdd = data;
 	u32 status;
 	u32 usbss_status;
+	irqreturn_t ret = IRQ_NONE;
 
 	status = cppi_readl(cdd->qmgr_mem + QMGR_PEND(0));
 	if (status & DA8XX_QMGR_PENDING_MASK)
-		cppi41_irq(cdd);
-	else
-		return IRQ_NONE;
+		ret = cppi41_irq(cdd);
+
+	if (cdd->td_complete) {
+		/*
+		 * Spurious IRQ caused by teardown.
+		 * DMA interrupts are not maskable, so there is now way
+		 * to prevent it.
+		 * Just ensure that the IRQ will be re-asserted.
+		 */
+		cdd->td_complete = false;
+		ret = IRQ_HANDLED;
+	}
 
 	/* Re-assert IRQ if there no usb core interrupts pending */
 	usbss_status = cppi_readl(cdd->usbss_mem + DA8XX_INTR_SRC_MASKED);
-	if (!usbss_status)
+	if (ret == IRQ_HANDLED && !usbss_status)
 		cppi_writel(0, cdd->usbss_mem + DA8XX_END_OF_INTR);
 
-	return IRQ_HANDLED;
+	return ret;
 }
 
 static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
@@ -740,6 +752,14 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c)
 		WARN_ON(!desc_phys);
 	}
 
+	/* On DA8xx, we are using the PEND0 register to determine if
+	 * the interrupt is generated by DMA. But because the teardown has
+	 * already been popped from completion queue, PEND0 is clear and
+	 * the interrupt handler will assume the interrupt has been fired
+	 * by the USB core.
+	 */
+	cdd->td_complete = true;
+
 	c->td_queued = 0;
 	c->td_seen = 0;
 	c->td_desc_seen = 0;
-- 
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 11/11] dmaengine: cppi41: Fix teardown warnings

[Alexandre Bailon]

During the teardown of a RX channel, because there is only one
completion queue available for RX channel, descriptor of another
channel may be popped which will cause 2 warnings:
- the first one because we popped a wrong descriptor
  (neither the channel's descriptor, neither the teardown descriptor).
- the second one happen during the teardown of another channel,
  because we can't find the channel descriptor
  (that is, the one that caused the first warning).
Use the teardown queue as completion queue during the teardown.

Note that fix doesn't fix all the teardown warnings:
I still get some when I run some corner case.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/cppi41.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 0060391..eeab29d 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -698,7 +698,7 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c)
 		if (!c->is_tx) {
 			reg |= GCR_STARV_RETRY;
 			reg |= GCR_DESC_TYPE_HOST;
-			reg |= c->q_comp_num;
+			reg |= cdd->td_queue.complete;
 		}
 		reg |= GCR_TEARDOWN;
 		cppi_writel(reg, c->gcr_reg);
@@ -709,7 +709,7 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c)
 	if (!c->td_seen || !c->td_desc_seen) {
 
 		desc_phys = cppi41_pop_desc(cdd, cdd->td_queue.complete);
-		if (!desc_phys)
+		if (!desc_phys && c->is_tx)
 			desc_phys = cppi41_pop_desc(cdd, c->q_comp_num);
 
 		if (desc_phys == c->desc_phys) {
-- 
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 11/11] dmaengine: cppi41: Fix teardown warnings

[Sergei Shtylyov]

On 01/09/2017 07:06 PM, Alexandre Bailon wrote:

> During the teardown of a RX channel, because there is only one
> completion queue available for RX channel, descriptor of another
> channel may be popped which will cause 2 warnings:
> - the first one because we popped a wrong descriptor
>   (neither the channel's descriptor, neither the teardown descriptor).

    Neither ... nor.

> - the second one happen during the teardown of another channel,
>   because we can't find the channel descriptor
>   (that is, the one that caused the first warning).
> Use the teardown queue as completion queue during the teardown.

    Hm, what's a teardown queue? I don't remember this documented...
memory fade is always possible tho... :-)

> Note that fix doesn't fix all the teardown warnings:
> I still get some when I run some corner case.
>
> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
[...]

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 11/11] dmaengine: cppi41: Fix teardown warnings

[Alexandre Bailon]

On 01/09/2017 07:46 PM, Sergei Shtylyov wrote:
> On 01/09/2017 07:06 PM, Alexandre Bailon wrote:
> 
>> During the teardown of a RX channel, because there is only one
>> completion queue available for RX channel, descriptor of another
>> channel may be popped which will cause 2 warnings:
>> - the first one because we popped a wrong descriptor
>>   (neither the channel's descriptor, neither the teardown descriptor).
> 
>    Neither ... nor.
> 
>> - the second one happen during the teardown of another channel,
>>   because we can't find the channel descriptor
>>   (that is, the one that caused the first warning).
>> Use the teardown queue as completion queue during the teardown.
> 
>    Hm, what's a teardown queue? I don't remember this documented...
> memory fade is always possible tho... :-)
I guess your memory is right :-)
There is no teardown queue. Though, we have to "allocate" an unassigned
queue and populate it with free teardown descriptors.
And when we are doing the teardown, we can select the queue to use for
completion. In the driver, we have to configure both of them.

Best regards,
Alexandre
> 
>> Note that fix doesn't fix all the teardown warnings:
>> I still get some when I run some corner case.
>>
>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> [...]
> 
> MBR, Sergei
> 

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