[PATCH 0/3] RZ/G2L DMAC enhancements

[PATCH 0/3] RZ/G2L DMAC enhancements

[Biju Das]

This patch series aims to add enhancement to RZ/G2L DMAC driver.
It is basically for supporting SCIF DMA.

It is based on a patch in the BSP by Long Luu which is similar to
rcar-dmac implementation.
<long.luu.ur@renesas.com>

Tested audio, RSPI and scif with these changes.
 15:      65880          0     GICv3 157 Edge      11820000.dma-controller:0
 16:      65880          0     GICv3 158 Edge      11820000.dma-controller:1
 17:      28498          0     GICv3 159 Edge      11820000.dma-controller:2
 18:      28498          0     GICv3 160 Edge      11820000.dma-controller:3
 19:       7934          0     GICv3 161 Edge      11820000.dma-controller:4
 20:       5766          0     GICv3 162 Edge      11820000.dma-controller:5
 21:          0          0     GICv3 163 Edge      11820000.dma-controller:6
 22:          0          0     GICv3 164 Edge      11820000.dma-controller:7
 23:          0          0     GICv3 165 Edge      11820000.dma-controller:8
 24:          0          0     GICv3 166 Edge      11820000.dma-controller:9
 25:          0          0     GICv3 167 Edge      11820000.dma-controller:10
 26:          0          0     GICv3 168 Edge      11820000.dma-controller:11
 27:          0          0     GICv3 169 Edge      11820000.dma-controller:12
 28:          0          0     GICv3 170 Edge      11820000.dma-controller:13
 29:          0          0     GICv3 171 Edge      11820000.dma-controller:14
 30:          0          0     GICv3 172 Edge      11820000.dma-controller:15

Biju Das (3):
  dmaengine: sh: rz-dmac: Reinitialize lmdescriptor head
  dmaengine: sh: rz-dmac: Add device_tx_status() callback
  dmaengine: sh: rz-dmac: Add device_pause() callback

 drivers/dma/sh/rz-dmac.c | 196 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 195 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH 1/3] dmaengine: sh: rz-dmac: Reinitialize lmdescriptor head

[Biju Das]

Reinitialize link mode descriptor head during terminate_all().
It fixes the incorrect serial messages during serial transfer when
DMA is enabled.

Based on a patch in the BSP by Long Luu
<long.luu.ur@renesas.com>

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/dma/sh/rz-dmac.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 6b62e01ba658..a04a37ce03fd 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -534,11 +534,18 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 static int rz_dmac_terminate_all(struct dma_chan *chan)
 {
 	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
+	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
 	unsigned long flags;
+	unsigned int i;
+
 	LIST_HEAD(head);
 
 	rz_dmac_disable_hw(channel);
 	spin_lock_irqsave(&channel->vc.lock, flags);
+
+	for (i = 0; i < DMAC_NR_LMDESC; i++)
+		lmdesc[i].header = 0;
+
 	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
 	list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
 	spin_unlock_irqrestore(&channel->vc.lock, flags);
-- 
2.25.1

[PATCH 2/3] dmaengine: sh: rz-dmac: Add device_tx_status() callback

[Biju Das]

The device_tx_status() callback is needed for serial DMA (RZ/G2L
SCIFA). Add support for device_tx_status() callback.

Based on a patch in the BSP by Long Luu similar to rcar-dmac
<long.luu.ur@renesas.com>

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/dma/sh/rz-dmac.c | 169 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 168 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index a04a37ce03fd..3625925d9f9f 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -110,10 +110,12 @@ struct rz_dmac {
  * Registers
  */
 
+#define CRTB				0x0020
 #define CHSTAT				0x0024
 #define CHCTRL				0x0028
 #define CHCFG				0x002c
 #define NXLA				0x0038
+#define CRLA				0x003c
 
 #define DCTRL				0x0000
 
@@ -655,6 +657,171 @@ static void rz_dmac_device_synchronize(struct dma_chan *chan)
 	rz_dmac_set_dmars_register(dmac, channel->index, 0);
 }
 
+static unsigned int calculate_total_bytes_in_vd(struct rz_dmac_desc *desc)
+{
+	struct scatterlist *sg, *sgl = desc->sg;
+	unsigned int i, size, sg_len = desc->sgcount;
+
+	for (i = 0, size = 0, sg = sgl; i < sg_len; i++, sg = sg_next(sg))
+		size += sg_dma_len(sg);
+
+	return size;
+}
+
+static unsigned int calculate_residue_bytes_in_vd(struct rz_dmac_chan *channel)
+{
+	struct rz_lmdesc *lmdesc = channel->lmdesc.head;
+	struct dma_chan *chan = &channel->vc.chan;
+	struct rz_dmac *dmac = to_rz_dmac(chan->device);
+	unsigned int residue = 0, i = 0;
+	unsigned int crla;
+
+	/* get current lmdesc */
+	crla = rz_dmac_ch_readl(channel, CRLA, 1);
+	while (!(lmdesc->nxla == crla)) {
+		lmdesc++;
+		if (lmdesc >= (channel->lmdesc.base + DMAC_NR_LMDESC))
+			lmdesc = channel->lmdesc.base;
+		i++;
+		/* Not found current lmdesc */
+		if (i > DMAC_NR_LMDESC)
+			return 0;
+	}
+
+	/* Point to current processing lmdesc in hardware */
+	lmdesc++;
+	if (lmdesc >= (channel->lmdesc.base + DMAC_NR_LMDESC))
+		lmdesc = channel->lmdesc.base;
+
+	/* Calculate residue from next lmdesc to end of virtual desc*/
+	while (lmdesc->chcfg & CHCFG_DEM) {
+		lmdesc++;
+		if (lmdesc >= (channel->lmdesc.base + DMAC_NR_LMDESC))
+			lmdesc = channel->lmdesc.base;
+		residue += lmdesc->tb;
+	}
+
+	dev_dbg(dmac->dev, "%s: Getting residue is %d\n", __func__, residue);
+
+	return residue;
+}
+
+static unsigned int rz_dmac_chan_get_residue(struct rz_dmac_chan *channel,
+					     dma_cookie_t cookie)
+{
+	struct rz_dmac_desc *current_desc, *desc;
+	enum dma_status status;
+	unsigned int residue;
+	unsigned int crla;
+	unsigned int crtb;
+	unsigned int i;
+
+	/* Get current processing virtual descriptor */
+	current_desc = list_first_entry(&channel->ld_active,
+					struct rz_dmac_desc, node);
+	if (!current_desc)
+		return 0;
+
+	/*
+	 * If the cookie corresponds to a descriptor that has been completed
+	 * there is no residue. The same check has already been performed by the
+	 * caller but without holding the channel lock, so the descriptor could
+	 * now be complete.
+	 */
+	status = dma_cookie_status(&channel->vc.chan, cookie, NULL);
+	if (status == DMA_COMPLETE)
+		return 0;
+
+	/*
+	 * If the cookie doesn't correspond to the currently processing virtual
+	 * descriptor then the descriptor hasn't been processed yet, and the
+	 * residue is equal to the full descriptor size.
+	 * Also, a client driver is possible to call this function before
+	 * rz_dmac_irq_handler_thread() runs. In this case, the running
+	 * descriptor will be the next descriptor, and the done list will
+	 * appear. So, if the argument cookie matches the done list's cookie,
+	 * we can assume the residue is zero.
+	 */
+	if (cookie != current_desc->vd.tx.cookie) {
+		list_for_each_entry(desc, &channel->ld_free, node) {
+			if (cookie == desc->vd.tx.cookie)
+				return 0;
+		}
+
+		list_for_each_entry(desc, &channel->ld_queue, node) {
+			if (cookie == desc->vd.tx.cookie)
+				return calculate_total_bytes_in_vd(desc);
+		}
+
+		list_for_each_entry(desc, &channel->ld_active, node) {
+			if (cookie == desc->vd.tx.cookie)
+				return calculate_total_bytes_in_vd(desc);
+		}
+
+		/*
+		 * No descriptor found for the cookie, there's thus no residue.
+		 * This shouldn't happen if the calling driver passes a correct
+		 * cookie value.
+		 */
+		WARN(1, "No descriptor for cookie!");
+		return 0;
+	}
+
+	/*
+	 * Correspond to the currently processing virtual descriptor
+	 *
+	 * Make sure the hardware does not move to next lmdesc
+	 * while reading the counter.
+	 * Trying it 3 times should be enough: Initial read, retry, retry
+	 * for the paranoid.
+	 * The current lmdesc running in hardware is channel.lmdesc.head
+	 */
+	for (i = 0; i < 3; i++) {
+		crla = rz_dmac_ch_readl(channel, CRLA, 1);
+		crtb = rz_dmac_ch_readl(channel, CRTB, 1);
+		/* Still the same? */
+		if (crla == rz_dmac_ch_readl(channel, CRLA, 1))
+			break;
+	}
+
+	WARN_ONCE(i >= 3, "residue might be not continuous!");
+
+	/*
+	 * Calculate number of byte transferred in processing virtual descriptor
+	 * One virtual descriptor can have many lmdesc
+	 */
+	residue = crtb;
+	residue += calculate_residue_bytes_in_vd(channel);
+
+	return residue;
+}
+
+static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
+					 dma_cookie_t cookie,
+					 struct dma_tx_state *txstate)
+{
+	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
+	enum dma_status status;
+	unsigned int residue;
+	unsigned long flags;
+
+	status = dma_cookie_status(chan, cookie, txstate);
+	if (status == DMA_COMPLETE || !txstate)
+		return status;
+
+	spin_lock_irqsave(&channel->vc.lock, flags);
+	residue = rz_dmac_chan_get_residue(channel, cookie);
+	spin_unlock_irqrestore(&channel->vc.lock, flags);
+
+	/* if there's no residue, the cookie is complete */
+	if (!residue)
+		return DMA_COMPLETE;
+
+	dma_set_residue(txstate, residue);
+
+	return status;
+}
+
 /*
  * -----------------------------------------------------------------------------
  * IRQ handling
@@ -937,7 +1104,7 @@ static int rz_dmac_probe(struct platform_device *pdev)
 
 	engine->device_alloc_chan_resources = rz_dmac_alloc_chan_resources;
 	engine->device_free_chan_resources = rz_dmac_free_chan_resources;
-	engine->device_tx_status = dma_cookie_status;
+	engine->device_tx_status = rz_dmac_tx_status;
 	engine->device_prep_slave_sg = rz_dmac_prep_slave_sg;
 	engine->device_prep_dma_memcpy = rz_dmac_prep_dma_memcpy;
 	engine->device_config = rz_dmac_config;
-- 
2.25.1

[PATCH 3/3] dmaengine: sh: rz-dmac: Add device_pause() callback

[Biju Das]

The device_pause() callback is needed for serial DMA (RZ/G2L
SCIFA). Add support for device_pause() callback.

Based on a patch in the BSP by Long Luu
<long.luu.ur@renesas.com>

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/dma/sh/rz-dmac.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 3625925d9f9f..a0cfb8f75534 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -822,6 +822,25 @@ static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
 	return status;
 }
 
+static int rz_dmac_device_pause(struct dma_chan *chan)
+{
+	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
+	struct rz_dmac *dmac = to_rz_dmac(chan->device);
+	unsigned int i;
+	u32 chstat;
+
+	for (i = 0; i < 1024; i++) {
+		chstat = rz_dmac_ch_readl(channel, CHSTAT, 1);
+		if (!(chstat & CHSTAT_EN))
+			break;
+		udelay(1);
+	}
+
+	rz_dmac_set_dmars_register(dmac, channel->index, 0);
+
+	return 0;
+}
+
 /*
  * -----------------------------------------------------------------------------
  * IRQ handling
@@ -1111,6 +1130,7 @@ static int rz_dmac_probe(struct platform_device *pdev)
 	engine->device_terminate_all = rz_dmac_terminate_all;
 	engine->device_issue_pending = rz_dmac_issue_pending;
 	engine->device_synchronize = rz_dmac_device_synchronize;
+	engine->device_pause = rz_dmac_device_pause;
 
 	engine->copy_align = DMAENGINE_ALIGN_1_BYTE;
 	dma_set_max_seg_size(engine->dev, U32_MAX);
-- 
2.25.1

Re: [PATCH 1/3] dmaengine: sh: rz-dmac: Reinitialize lmdescriptor head

[Vinod Koul]

On 24-03-23, 09:49, Biju Das wrote:
> Reinitialize link mode descriptor head during terminate_all().
> It fixes the incorrect serial messages during serial transfer when
> DMA is enabled.
> 
> Based on a patch in the BSP by Long Luu
> <long.luu.ur@renesas.com>
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/dma/sh/rz-dmac.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 6b62e01ba658..a04a37ce03fd 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -534,11 +534,18 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  static int rz_dmac_terminate_all(struct dma_chan *chan)
>  {
>  	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> +	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
>  	unsigned long flags;
> +	unsigned int i;
> +
>  	LIST_HEAD(head);
>  
>  	rz_dmac_disable_hw(channel);
>  	spin_lock_irqsave(&channel->vc.lock, flags);
> +
> +	for (i = 0; i < DMAC_NR_LMDESC; i++)
> +		lmdesc[i].header = 0;

Any reason not to use memset for this?

> +
>  	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
>  	list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
>  	spin_unlock_irqrestore(&channel->vc.lock, flags);
> -- 
> 2.25.1

-- 
~Vinod

Re: [PATCH 2/3] dmaengine: sh: rz-dmac: Add device_tx_status() callback

[Vinod Koul]

On 24-03-23, 09:49, Biju Das wrote:
> The device_tx_status() callback is needed for serial DMA (RZ/G2L
> SCIFA). Add support for device_tx_status() callback.
> 
> Based on a patch in the BSP by Long Luu similar to rcar-dmac
> <long.luu.ur@renesas.com>
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/dma/sh/rz-dmac.c | 169 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 168 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index a04a37ce03fd..3625925d9f9f 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -110,10 +110,12 @@ struct rz_dmac {
>   * Registers
>   */
>  
> +#define CRTB				0x0020
>  #define CHSTAT				0x0024
>  #define CHCTRL				0x0028
>  #define CHCFG				0x002c
>  #define NXLA				0x0038
> +#define CRLA				0x003c
>  
>  #define DCTRL				0x0000
>  
> @@ -655,6 +657,171 @@ static void rz_dmac_device_synchronize(struct dma_chan *chan)
>  	rz_dmac_set_dmars_register(dmac, channel->index, 0);
>  }
>  
> +static unsigned int calculate_total_bytes_in_vd(struct rz_dmac_desc *desc)
> +{
> +	struct scatterlist *sg, *sgl = desc->sg;
> +	unsigned int i, size, sg_len = desc->sgcount;
> +
> +	for (i = 0, size = 0, sg = sgl; i < sg_len; i++, sg = sg_next(sg))
> +		size += sg_dma_len(sg);

for_each_sg() ?

> +
> +	return size;
> +}
> +
> +static unsigned int calculate_residue_bytes_in_vd(struct rz_dmac_chan *channel)
> +{
> +	struct rz_lmdesc *lmdesc = channel->lmdesc.head;
> +	struct dma_chan *chan = &channel->vc.chan;
> +	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> +	unsigned int residue = 0, i = 0;
> +	unsigned int crla;
> +
> +	/* get current lmdesc */
> +	crla = rz_dmac_ch_readl(channel, CRLA, 1);
> +	while (!(lmdesc->nxla == crla)) {
> +		lmdesc++;
> +		if (lmdesc >= (channel->lmdesc.base + DMAC_NR_LMDESC))
> +			lmdesc = channel->lmdesc.base;
> +		i++;
> +		/* Not found current lmdesc */
> +		if (i > DMAC_NR_LMDESC)
> +			return 0;
> +	}
> +
> +	/* Point to current processing lmdesc in hardware */
> +	lmdesc++;
> +	if (lmdesc >= (channel->lmdesc.base + DMAC_NR_LMDESC))
> +		lmdesc = channel->lmdesc.base;
> +
> +	/* Calculate residue from next lmdesc to end of virtual desc*/
> +	while (lmdesc->chcfg & CHCFG_DEM) {
> +		lmdesc++;
> +		if (lmdesc >= (channel->lmdesc.base + DMAC_NR_LMDESC))
> +			lmdesc = channel->lmdesc.base;
> +		residue += lmdesc->tb;
> +	}
> +
> +	dev_dbg(dmac->dev, "%s: Getting residue is %d\n", __func__, residue);
> +
> +	return residue;
> +}
> +
> +static unsigned int rz_dmac_chan_get_residue(struct rz_dmac_chan *channel,
> +					     dma_cookie_t cookie)
> +{
> +	struct rz_dmac_desc *current_desc, *desc;
> +	enum dma_status status;
> +	unsigned int residue;
> +	unsigned int crla;
> +	unsigned int crtb;
> +	unsigned int i;
> +
> +	/* Get current processing virtual descriptor */
> +	current_desc = list_first_entry(&channel->ld_active,
> +					struct rz_dmac_desc, node);
> +	if (!current_desc)
> +		return 0;
> +
> +	/*
> +	 * If the cookie corresponds to a descriptor that has been completed
> +	 * there is no residue. The same check has already been performed by the
> +	 * caller but without holding the channel lock, so the descriptor could
> +	 * now be complete.
> +	 */
> +	status = dma_cookie_status(&channel->vc.chan, cookie, NULL);
> +	if (status == DMA_COMPLETE)
> +		return 0;
> +
> +	/*
> +	 * If the cookie doesn't correspond to the currently processing virtual
> +	 * descriptor then the descriptor hasn't been processed yet, and the
> +	 * residue is equal to the full descriptor size.
> +	 * Also, a client driver is possible to call this function before
> +	 * rz_dmac_irq_handler_thread() runs. In this case, the running
> +	 * descriptor will be the next descriptor, and the done list will
> +	 * appear. So, if the argument cookie matches the done list's cookie,
> +	 * we can assume the residue is zero.
> +	 */
> +	if (cookie != current_desc->vd.tx.cookie) {
> +		list_for_each_entry(desc, &channel->ld_free, node) {
> +			if (cookie == desc->vd.tx.cookie)
> +				return 0;
> +		}
> +
> +		list_for_each_entry(desc, &channel->ld_queue, node) {
> +			if (cookie == desc->vd.tx.cookie)
> +				return calculate_total_bytes_in_vd(desc);
> +		}
> +
> +		list_for_each_entry(desc, &channel->ld_active, node) {
> +			if (cookie == desc->vd.tx.cookie)
> +				return calculate_total_bytes_in_vd(desc);
> +		}
> +
> +		/*
> +		 * No descriptor found for the cookie, there's thus no residue.
> +		 * This shouldn't happen if the calling driver passes a correct
> +		 * cookie value.
> +		 */
> +		WARN(1, "No descriptor for cookie!");
> +		return 0;
> +	}
> +
> +	/*
> +	 * Correspond to the currently processing virtual descriptor
> +	 *
> +	 * Make sure the hardware does not move to next lmdesc
> +	 * while reading the counter.
> +	 * Trying it 3 times should be enough: Initial read, retry, retry
> +	 * for the paranoid.
> +	 * The current lmdesc running in hardware is channel.lmdesc.head
> +	 */
> +	for (i = 0; i < 3; i++) {
> +		crla = rz_dmac_ch_readl(channel, CRLA, 1);
> +		crtb = rz_dmac_ch_readl(channel, CRTB, 1);
> +		/* Still the same? */
> +		if (crla == rz_dmac_ch_readl(channel, CRLA, 1))
> +			break;
> +	}
> +
> +	WARN_ONCE(i >= 3, "residue might be not continuous!");
> +
> +	/*
> +	 * Calculate number of byte transferred in processing virtual descriptor
> +	 * One virtual descriptor can have many lmdesc
> +	 */
> +	residue = crtb;
> +	residue += calculate_residue_bytes_in_vd(channel);
> +
> +	return residue;
> +}
> +
> +static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
> +					 dma_cookie_t cookie,
> +					 struct dma_tx_state *txstate)
> +{
> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> +	enum dma_status status;
> +	unsigned int residue;
> +	unsigned long flags;
> +
> +	status = dma_cookie_status(chan, cookie, txstate);
> +	if (status == DMA_COMPLETE || !txstate)
> +		return status;
> +
> +	spin_lock_irqsave(&channel->vc.lock, flags);
> +	residue = rz_dmac_chan_get_residue(channel, cookie);
> +	spin_unlock_irqrestore(&channel->vc.lock, flags);
> +
> +	/* if there's no residue, the cookie is complete */
> +	if (!residue)
> +		return DMA_COMPLETE;
> +
> +	dma_set_residue(txstate, residue);
> +
> +	return status;
> +}
> +
>  /*
>   * -----------------------------------------------------------------------------
>   * IRQ handling
> @@ -937,7 +1104,7 @@ static int rz_dmac_probe(struct platform_device *pdev)
>  
>  	engine->device_alloc_chan_resources = rz_dmac_alloc_chan_resources;
>  	engine->device_free_chan_resources = rz_dmac_free_chan_resources;
> -	engine->device_tx_status = dma_cookie_status;
> +	engine->device_tx_status = rz_dmac_tx_status;
>  	engine->device_prep_slave_sg = rz_dmac_prep_slave_sg;
>  	engine->device_prep_dma_memcpy = rz_dmac_prep_dma_memcpy;
>  	engine->device_config = rz_dmac_config;
> -- 
> 2.25.1

-- 
~Vinod

Re: [PATCH 3/3] dmaengine: sh: rz-dmac: Add device_pause() callback

[Vinod Koul]

On 24-03-23, 09:49, Biju Das wrote:
> The device_pause() callback is needed for serial DMA (RZ/G2L
> SCIFA). Add support for device_pause() callback.
> 
> Based on a patch in the BSP by Long Luu
> <long.luu.ur@renesas.com>
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/dma/sh/rz-dmac.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 3625925d9f9f..a0cfb8f75534 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -822,6 +822,25 @@ static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
>  	return status;
>  }
>  
> +static int rz_dmac_device_pause(struct dma_chan *chan)
> +{
> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> +	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> +	unsigned int i;
> +	u32 chstat;
> +
> +	for (i = 0; i < 1024; i++) {
> +		chstat = rz_dmac_ch_readl(channel, CHSTAT, 1);
> +		if (!(chstat & CHSTAT_EN))
> +			break;
> +		udelay(1);
> +	}
> +
> +	rz_dmac_set_dmars_register(dmac, channel->index, 0);
> +
> +	return 0;
> +}
> +
>  /*
>   * -----------------------------------------------------------------------------
>   * IRQ handling
> @@ -1111,6 +1130,7 @@ static int rz_dmac_probe(struct platform_device *pdev)
>  	engine->device_terminate_all = rz_dmac_terminate_all;
>  	engine->device_issue_pending = rz_dmac_issue_pending;
>  	engine->device_synchronize = rz_dmac_device_synchronize;
> +	engine->device_pause = rz_dmac_device_pause;

No resume?

>  
>  	engine->copy_align = DMAENGINE_ALIGN_1_BYTE;
>  	dma_set_max_seg_size(engine->dev, U32_MAX);
> -- 
> 2.25.1

-- 
~Vinod

RE: [PATCH 1/3] dmaengine: sh: rz-dmac: Reinitialize lmdescriptor head

[Biju Das]

Hi Vinod,

Thanks for the feedback.

> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: Friday, March 31, 2023 1:15 PM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; Geert
> Uytterhoeven <geert+renesas@glider.be>; dmaengine@vger.kernel.org; linux-
> renesas-soc@vger.kernel.org
> Subject: Re: [PATCH 1/3] dmaengine: sh: rz-dmac: Reinitialize lmdescriptor
> head
> 
> On 24-03-23, 09:49, Biju Das wrote:
> > Reinitialize link mode descriptor head during terminate_all().
> > It fixes the incorrect serial messages during serial transfer when DMA
> > is enabled.
> >
> > Based on a patch in the BSP by Long Luu <long.luu.ur@renesas.com>
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  drivers/dma/sh/rz-dmac.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index
> > 6b62e01ba658..a04a37ce03fd 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> > @@ -534,11 +534,18 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan,
> > struct scatterlist *sgl,  static int rz_dmac_terminate_all(struct
> > dma_chan *chan)  {
> >  	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> > +	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
> >  	unsigned long flags;
> > +	unsigned int i;
> > +
> >  	LIST_HEAD(head);
> >
> >  	rz_dmac_disable_hw(channel);
> >  	spin_lock_irqsave(&channel->vc.lock, flags);
> > +
> > +	for (i = 0; i < DMAC_NR_LMDESC; i++)
> > +		lmdesc[i].header = 0;
> 
> Any reason not to use memset for this?

1) If I use memset, then it need to set 64 * 8 * 4 = 2048 bytes compared to 
64 * 4 =256 bytes here and consistently we use the above logic in the driver.
https://elixir.bootlin.com/linux/v6.0-rc4/source/drivers/dma/sh/rz-dmac.c#L239

2) Another reason is if we do memset, eventhough there will not be any new irq,
irqthread may be still running which can cause DMA bus Error due to nxla
being null with memset. Currently nxla being assigned during probe.

https://elixir.bootlin.com/linux/v6.0-rc4/source/drivers/dma/sh/rz-dmac.c#L214

Cheers,
Biju


> 
> > +
> >  	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
> >  	list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
> >  	spin_unlock_irqrestore(&channel->vc.lock, flags);
> > --
> > 2.25.1
> 
> --
> ~Vinod

RE: [PATCH 2/3] dmaengine: sh: rz-dmac: Add device_tx_status() callback

[Biju Das]

Hi Vinod,

Thanks for the feedback.

> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: Friday, March 31, 2023 1:17 PM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; Geert
> Uytterhoeven <geert+renesas@glider.be>; dmaengine@vger.kernel.org; linux-
> renesas-soc@vger.kernel.org
> Subject: Re: [PATCH 2/3] dmaengine: sh: rz-dmac: Add device_tx_status()
> callback
> 
> On 24-03-23, 09:49, Biju Das wrote:
> > The device_tx_status() callback is needed for serial DMA (RZ/G2L
> > SCIFA). Add support for device_tx_status() callback.
> >
> > Based on a patch in the BSP by Long Luu similar to rcar-dmac
> > <long.luu.ur@renesas.com>
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  drivers/dma/sh/rz-dmac.c | 169
> > ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 168 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index
> > a04a37ce03fd..3625925d9f9f 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> > @@ -110,10 +110,12 @@ struct rz_dmac {
> >   * Registers
> >   */
> >
> > +#define CRTB				0x0020
> >  #define CHSTAT				0x0024
> >  #define CHCTRL				0x0028
> >  #define CHCFG				0x002c
> >  #define NXLA				0x0038
> > +#define CRLA				0x003c
> >
> >  #define DCTRL				0x0000
> >
> > @@ -655,6 +657,171 @@ static void rz_dmac_device_synchronize(struct
> dma_chan *chan)
> >  	rz_dmac_set_dmars_register(dmac, channel->index, 0);  }
> >
> > +static unsigned int calculate_total_bytes_in_vd(struct rz_dmac_desc
> > +*desc) {
> > +	struct scatterlist *sg, *sgl = desc->sg;
> > +	unsigned int i, size, sg_len = desc->sgcount;
> > +
> > +	for (i = 0, size = 0, sg = sgl; i < sg_len; i++, sg = sg_next(sg))
> > +		size += sg_dma_len(sg);
> 
> for_each_sg() ?

Agreed. Much simpler now.

Cheers,
Biju

> 
> > +
> > +	return size;
> > +}
> > +
> > +static unsigned int calculate_residue_bytes_in_vd(struct rz_dmac_chan
> > +*channel) {
> > +	struct rz_lmdesc *lmdesc = channel->lmdesc.head;
> > +	struct dma_chan *chan = &channel->vc.chan;
> > +	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> > +	unsigned int residue = 0, i = 0;
> > +	unsigned int crla;
> > +
> > +	/* get current lmdesc */
> > +	crla = rz_dmac_ch_readl(channel, CRLA, 1);
> > +	while (!(lmdesc->nxla == crla)) {
> > +		lmdesc++;
> > +		if (lmdesc >= (channel->lmdesc.base + DMAC_NR_LMDESC))
> > +			lmdesc = channel->lmdesc.base;
> > +		i++;
> > +		/* Not found current lmdesc */
> > +		if (i > DMAC_NR_LMDESC)
> > +			return 0;
> > +	}
> > +
> > +	/* Point to current processing lmdesc in hardware */
> > +	lmdesc++;
> > +	if (lmdesc >= (channel->lmdesc.base + DMAC_NR_LMDESC))
> > +		lmdesc = channel->lmdesc.base;
> > +
> > +	/* Calculate residue from next lmdesc to end of virtual desc*/
> > +	while (lmdesc->chcfg & CHCFG_DEM) {
> > +		lmdesc++;
> > +		if (lmdesc >= (channel->lmdesc.base + DMAC_NR_LMDESC))
> > +			lmdesc = channel->lmdesc.base;
> > +		residue += lmdesc->tb;
> > +	}
> > +
> > +	dev_dbg(dmac->dev, "%s: Getting residue is %d\n", __func__,
> > +residue);
> > +
> > +	return residue;
> > +}
> > +
> > +static unsigned int rz_dmac_chan_get_residue(struct rz_dmac_chan
> *channel,
> > +					     dma_cookie_t cookie)
> > +{
> > +	struct rz_dmac_desc *current_desc, *desc;
> > +	enum dma_status status;
> > +	unsigned int residue;
> > +	unsigned int crla;
> > +	unsigned int crtb;
> > +	unsigned int i;
> > +
> > +	/* Get current processing virtual descriptor */
> > +	current_desc = list_first_entry(&channel->ld_active,
> > +					struct rz_dmac_desc, node);
> > +	if (!current_desc)
> > +		return 0;
> > +
> > +	/*
> > +	 * If the cookie corresponds to a descriptor that has been completed
> > +	 * there is no residue. The same check has already been performed by
> the
> > +	 * caller but without holding the channel lock, so the descriptor
> could
> > +	 * now be complete.
> > +	 */
> > +	status = dma_cookie_status(&channel->vc.chan, cookie, NULL);
> > +	if (status == DMA_COMPLETE)
> > +		return 0;
> > +
> > +	/*
> > +	 * If the cookie doesn't correspond to the currently processing
> virtual
> > +	 * descriptor then the descriptor hasn't been processed yet, and the
> > +	 * residue is equal to the full descriptor size.
> > +	 * Also, a client driver is possible to call this function before
> > +	 * rz_dmac_irq_handler_thread() runs. In this case, the running
> > +	 * descriptor will be the next descriptor, and the done list will
> > +	 * appear. So, if the argument cookie matches the done list's cookie,
> > +	 * we can assume the residue is zero.
> > +	 */
> > +	if (cookie != current_desc->vd.tx.cookie) {
> > +		list_for_each_entry(desc, &channel->ld_free, node) {
> > +			if (cookie == desc->vd.tx.cookie)
> > +				return 0;
> > +		}
> > +
> > +		list_for_each_entry(desc, &channel->ld_queue, node) {
> > +			if (cookie == desc->vd.tx.cookie)
> > +				return calculate_total_bytes_in_vd(desc);
> > +		}
> > +
> > +		list_for_each_entry(desc, &channel->ld_active, node) {
> > +			if (cookie == desc->vd.tx.cookie)
> > +				return calculate_total_bytes_in_vd(desc);
> > +		}
> > +
> > +		/*
> > +		 * No descriptor found for the cookie, there's thus no residue.
> > +		 * This shouldn't happen if the calling driver passes a correct
> > +		 * cookie value.
> > +		 */
> > +		WARN(1, "No descriptor for cookie!");
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * Correspond to the currently processing virtual descriptor
> > +	 *
> > +	 * Make sure the hardware does not move to next lmdesc
> > +	 * while reading the counter.
> > +	 * Trying it 3 times should be enough: Initial read, retry, retry
> > +	 * for the paranoid.
> > +	 * The current lmdesc running in hardware is channel.lmdesc.head
> > +	 */
> > +	for (i = 0; i < 3; i++) {
> > +		crla = rz_dmac_ch_readl(channel, CRLA, 1);
> > +		crtb = rz_dmac_ch_readl(channel, CRTB, 1);
> > +		/* Still the same? */
> > +		if (crla == rz_dmac_ch_readl(channel, CRLA, 1))
> > +			break;
> > +	}
> > +
> > +	WARN_ONCE(i >= 3, "residue might be not continuous!");
> > +
> > +	/*
> > +	 * Calculate number of byte transferred in processing virtual
> descriptor
> > +	 * One virtual descriptor can have many lmdesc
> > +	 */
> > +	residue = crtb;
> > +	residue += calculate_residue_bytes_in_vd(channel);
> > +
> > +	return residue;
> > +}
> > +
> > +static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
> > +					 dma_cookie_t cookie,
> > +					 struct dma_tx_state *txstate)
> > +{
> > +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> > +	enum dma_status status;
> > +	unsigned int residue;
> > +	unsigned long flags;
> > +
> > +	status = dma_cookie_status(chan, cookie, txstate);
> > +	if (status == DMA_COMPLETE || !txstate)
> > +		return status;
> > +
> > +	spin_lock_irqsave(&channel->vc.lock, flags);
> > +	residue = rz_dmac_chan_get_residue(channel, cookie);
> > +	spin_unlock_irqrestore(&channel->vc.lock, flags);
> > +
> > +	/* if there's no residue, the cookie is complete */
> > +	if (!residue)
> > +		return DMA_COMPLETE;
> > +
> > +	dma_set_residue(txstate, residue);
> > +
> > +	return status;
> > +}
> > +
> >  /*
> >   * ----------------------------------------------------------------------
> -------
> >   * IRQ handling
> > @@ -937,7 +1104,7 @@ static int rz_dmac_probe(struct platform_device
> > *pdev)
> >
> >  	engine->device_alloc_chan_resources = rz_dmac_alloc_chan_resources;
> >  	engine->device_free_chan_resources = rz_dmac_free_chan_resources;
> > -	engine->device_tx_status = dma_cookie_status;
> > +	engine->device_tx_status = rz_dmac_tx_status;
> >  	engine->device_prep_slave_sg = rz_dmac_prep_slave_sg;
> >  	engine->device_prep_dma_memcpy = rz_dmac_prep_dma_memcpy;
> >  	engine->device_config = rz_dmac_config;
> > --
> > 2.25.1
> 
> --
> ~Vinod

RE: [PATCH 3/3] dmaengine: sh: rz-dmac: Add device_pause() callback

[Biju Das]

Hi Vinod,

Thanks for the feedback.

> Subject: Re: [PATCH 3/3] dmaengine: sh: rz-dmac: Add device_pause() callback
> 
> On 24-03-23, 09:49, Biju Das wrote:
> > The device_pause() callback is needed for serial DMA (RZ/G2L SCIFA).
> > Add support for device_pause() callback.
> >
> > Based on a patch in the BSP by Long Luu <long.luu.ur@renesas.com>
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  drivers/dma/sh/rz-dmac.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index
> > 3625925d9f9f..a0cfb8f75534 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> > @@ -822,6 +822,25 @@ static enum dma_status rz_dmac_tx_status(struct
> dma_chan *chan,
> >  	return status;
> >  }
> >
> > +static int rz_dmac_device_pause(struct dma_chan *chan) {
> > +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> > +	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> > +	unsigned int i;
> > +	u32 chstat;
> > +
> > +	for (i = 0; i < 1024; i++) {
> > +		chstat = rz_dmac_ch_readl(channel, CHSTAT, 1);
> > +		if (!(chstat & CHSTAT_EN))
> > +			break;
> > +		udelay(1);
> > +	}
> > +
> > +	rz_dmac_set_dmars_register(dmac, channel->index, 0);
> > +
> > +	return 0;
> > +}
> > +
> >  /*
> >   * ----------------------------------------------------------------------
> -------
> >   * IRQ handling
> > @@ -1111,6 +1130,7 @@ static int rz_dmac_probe(struct platform_device
> *pdev)
> >  	engine->device_terminate_all = rz_dmac_terminate_all;
> >  	engine->device_issue_pending = rz_dmac_issue_pending;
> >  	engine->device_synchronize = rz_dmac_device_synchronize;
> > +	engine->device_pause = rz_dmac_device_pause;
> 
> No resume?

OK, will add resume.

+static int rz_dmac_device_resume(struct dma_chan *chan)
+{
+	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
+	struct rz_dmac *dmac = to_rz_dmac(chan->device);
+
+	rz_dmac_set_dmars_register(dmac, channel->index, channel->mid_rid);
+
+	return 0;
+}

Cheers,
Biju

> 
> >
> >  	engine->copy_align = DMAENGINE_ALIGN_1_BYTE;
> >  	dma_set_max_seg_size(engine->dev, U32_MAX);
> > --
> > 2.25.1
> 
> --
> ~Vinod

RE: [PATCH 1/3] dmaengine: sh: rz-dmac: Reinitialize lmdescriptor head

[Biju Das]

Hi Vinod,

> Subject: RE: [PATCH 1/3] dmaengine: sh: rz-dmac: Reinitialize lmdescriptor
> head
> 
> Hi Vinod,
> 
> Thanks for the feedback.
> 
> > -----Original Message-----
> > From: Vinod Koul <vkoul@kernel.org>
> > Sent: Friday, March 31, 2023 1:15 PM
> > To: Biju Das <biju.das.jz@bp.renesas.com>
> > Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>;
> > Geert Uytterhoeven <geert+renesas@glider.be>;
> > dmaengine@vger.kernel.org; linux- renesas-soc@vger.kernel.org
> > Subject: Re: [PATCH 1/3] dmaengine: sh: rz-dmac: Reinitialize
> > lmdescriptor head
> >
> > On 24-03-23, 09:49, Biju Das wrote:
> > > Reinitialize link mode descriptor head during terminate_all().
> > > It fixes the incorrect serial messages during serial transfer when
> > > DMA is enabled.
> > >
> > > Based on a patch in the BSP by Long Luu <long.luu.ur@renesas.com>
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > >  drivers/dma/sh/rz-dmac.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > > index 6b62e01ba658..a04a37ce03fd 100644
> > > --- a/drivers/dma/sh/rz-dmac.c
> > > +++ b/drivers/dma/sh/rz-dmac.c
> > > @@ -534,11 +534,18 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan,
> > > struct scatterlist *sgl,  static int rz_dmac_terminate_all(struct
> > > dma_chan *chan)  {
> > >  	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> > > +	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
> > >  	unsigned long flags;
> > > +	unsigned int i;
> > > +
> > >  	LIST_HEAD(head);
> > >
> > >  	rz_dmac_disable_hw(channel);
> > >  	spin_lock_irqsave(&channel->vc.lock, flags);
> > > +
> > > +	for (i = 0; i < DMAC_NR_LMDESC; i++)
> > > +		lmdesc[i].header = 0;
> >

I will send an improved version for invalidating lmdesc. 

Add helper function rz_dmac_invalidate_lmdesc() and share the code between
rz_dmac_free_chan_resources and rz_dmac_terminate_all(),so that hardware
descriptors can be reused in rz_dmac_lmdesc_recycle().

+static void rz_dmac_invalidate_lmdesc(struct rz_dmac_chan *channel)
+{
+	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
+
+	for (; lmdesc < channel->lmdesc.base + DMAC_NR_LMDESC; lmdesc++) {
+		if (lmdesc->header)
+			lmdesc->header = 0;
+	}
+}
+

static void rz_dmac_lmdesc_recycle(struct rz_dmac_chan *channel)
 {
 	struct rz_lmdesc *lmdesc = channel->lmdesc.head;
@@ -437,16 +447,11 @@ static void rz_dmac_free_chan_resources(struct dma_chan *chan)
 {
 	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
 	struct rz_dmac *dmac = to_rz_dmac(chan->device);
-	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
 	struct rz_dmac_desc *desc, *_desc;
 	unsigned long flags;
-	unsigned int i;
 
 	spin_lock_irqsave(&channel->vc.lock, flags);
-
-	for (i = 0; i < DMAC_NR_LMDESC; i++)
-		lmdesc[i].header = 0;
-
+	rz_dmac_invalidate_lmdesc(channel);
 	rz_dmac_disable_hw(channel);
 	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
 	list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
@@ -537,6 +542,7 @@ static int rz_dmac_terminate_all(struct dma_chan *chan)
 
 	rz_dmac_disable_hw(channel);
 	spin_lock_irqsave(&channel->vc.lock, flags);
+	rz_dmac_invalidate_lmdesc(channel);