Re: [PATCH v8 7/8] dmaengine: sh: rz-dmac: Add device_tx_status() callback

[Claudiu Beznea <claudiu.beznea@tuxon.dev>]

Hi, Frank,

On 2/25/26 18:43, Frank Li wrote:
> On Tue, Jan 20, 2026 at 03:33:29PM +0200, Claudiu wrote:
>> From: Biju Das <biju.das.jz@bp.renesas.com>
>>
>> Add support for device_tx_status() callback as it is needed for
>> RZ/G2L SCIFA driver.
>>
>> Based on a patch in the BSP similar to rcar-dmac by
>> Long Luu <long.luu.ur@renesas.com>.
> 
> If you want to give credit to Long Luu, any public link?

No public link as far as I'm aware. Anyway, I'll add his SoB + Co-developed-by.

> 
>>
>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>> [claudiu.beznea:
>>   - post-increment lmdesc in rz_dmac_get_next_lmdesc() to allow the next
>>     pointer to advance
>>   - use 'lmdesc->nxla != crla' comparison instead of
>>     '!(lmdesc->nxla == crla)' in rz_dmac_calculate_residue_bytes_in_vd()
>>   - in rz_dmac_calculate_residue_bytes_in_vd() use '++i >= DMAC_NR_LMDESC'
>>     to verify if the full lmdesc list was checked
>>   - drop rz_dmac_calculate_total_bytes_in_vd() and use desc->len instead
>>   - re-arranged comments so they span fewer lines and are wrapped to ~80
>>     characters
>>   - use u32 for the residue value and the functions returning it
>>   - use u32 for the variables storing register values
>>   - fixed typos]
> 
> Suppose needn't this section

Just followed the process. I can drop it.

> 
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>
>> Changes in v8:
>> - populated engine->residue_granularity
>>
>> Changes in v7:
>> - none
>>
>> Changes in v6:
>> - s/byte/bytes in comment from rz_dmac_chan_get_residue()
>>
>> Changes in v5:
>> - post-increment lmdesc in rz_dmac_get_next_lmdesc() to allow the next
>>    pointer to advance
>> - use 'lmdesc->nxla != crla' comparison instead of
>>    '!(lmdesc->nxla == crla)' in rz_dmac_calculate_residue_bytes_in_vd()
>> - in rz_dmac_calculate_residue_bytes_in_vd() use '++i >= DMAC_NR_LMDESC'
>>    to verify if the full lmdesc list was checked
>> - drop rz_dmac_calculate_total_bytes_in_vd() and use desc->len instead
>> - re-arranged comments so they span fewer lines and are wrapped to ~80
>>    characters
>> - use u32 for the residue value and the functions returning it
>> - use u32 for the variables storing register values
>> - fixed typos
>>
>>   drivers/dma/sh/rz-dmac.c | 145 ++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 144 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
>> index 4602f8b7408a..27c963083e29 100644
>> --- a/drivers/dma/sh/rz-dmac.c
>> +++ b/drivers/dma/sh/rz-dmac.c
>> @@ -125,10 +125,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
>>
>> @@ -684,6 +686,146 @@ static void rz_dmac_device_synchronize(struct dma_chan *chan)
>>   	rz_dmac_set_dma_req_no(dmac, channel->index, dmac->info->default_dma_req_no);
>>   }
>>
>> +static struct rz_lmdesc *
>> +rz_dmac_get_next_lmdesc(struct rz_lmdesc *base, struct rz_lmdesc *lmdesc)
>> +{
>> +	struct rz_lmdesc *next = ++lmdesc;
>> +
>> +	if (next >= base + DMAC_NR_LMDESC)
>> +		next = base;
>> +
>> +	return next;
>> +}
>> +
>> +static u32 rz_dmac_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);
>> +	u32 residue = 0, crla, i = 0;
>> +
>> +	crla = rz_dmac_ch_readl(channel, CRLA, 1);
>> +	while (lmdesc->nxla != crla) {
>> +		lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
>> +		if (++i >= DMAC_NR_LMDESC)
>> +			return 0;
>> +	}
>> +
>> +	/* Calculate residue from next lmdesc to end of virtual desc */
>> +	while (lmdesc->chcfg & CHCFG_DEM) {
>> +		residue += lmdesc->tb;
>> +		lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
>> +	}
> 
> can use one loop
> 
> for (int i=0; i<DMAC_NR_LMDESC; i++) {
> 	if (lmdesc->nxla == crla)
> 		residue = 0; 	//reset to 0;
> 
> 	if (lmdesc->chcfg & CHCFG_DEM)
> 		residue += lmdesc->tb;
> 
> 	lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);

I'm not sure this will work as the descriptors list is cyclic and resetting the 
residue to zero when lmdesc->nxla == crla and then start acumulating from there 
will not work if there are descriptors enqueued for the current transfers at the 
end and the beginning of the list, e.g:

descriptors list:

| d3 | d5 | d6 | ... | d0 | d1 | d2 |

^				    ^
start			 	   end
(index 0)			(index DMAC_NR_LMDESC-1)

> }
> 
> return residue;
> 
>> +
>> +	dev_dbg(dmac->dev, "%s: VD residue is %u\n", __func__, residue);
>> +
>> +	return residue;
>> +}
>> +
>> +static u32 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;
>> +	u32 crla, crtb, 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 will appear in the done list. 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 desc->len;
>> +		}
>> +
>> +		list_for_each_entry(desc, &channel->ld_active, node) {
>> +			if (cookie == desc->vd.tx.cookie)
>> +				return desc->len;
>> +		}
>> +
>> +		/*
>> +		 * 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;
>> +	}
>> +
>> +	/*
>> +	 * We need to read two registers. Make sure the hardware does not move
>> +	 * to next lmdesc while reading the current lmdesc. Trying it 3 times
>> +	 * should be enough: initial read, retry, retry for the paranoid.
>> +	 */
>> +	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 not be continuous!");
>> +
>> +	/*
>> +	 * Calculate number of bytes transferred in processing virtual descriptor.
>> +	 * One virtual descriptor can have many lmdesc.
>> +	 */
>> +	return crtb + rz_dmac_calculate_residue_bytes_in_vd(channel);
> 
> you don't use varible 'ctra' here, so retry 3 become useless. suppose
> rz_dmac_calculate_residue_bytes_in_vd(channel, ctra)
> 
> and avoid rz_dmac_ch_readl(channel, CRLA, 1) in
> rz_dmac_calculate_residue_bytes_in_vd() to keep ctra and ctrb reflect the
> correct hardware state.

Good point, I'll update it.

Thank you for your review,
Claudiu