Re: [PATCH 3/8] drivers: dma: ti: Refactor TI K3 UDMA driver

["Adivi, Sai Sree Kartheek" <s-adivi@ti.com>]

On 5/9/2025 7:55 PM, Péter Ujfalusi wrote:
> Hi,
> 
> On 28/04/2025 10:20, Sai Sree Kartheek Adivi wrote:
>> Refactors and split the driver into common and device
>> specific parts. There are no functional changes.
>>
>> Signed-off-by: Sai Sree Kartheek Adivi <s-adivi@ti.com>
>> ---
>>   drivers/dma/ti/Makefile         |    2 +-
>>   drivers/dma/ti/k3-udma-common.c | 2909 ++++++++++++++++++++++++
>>   drivers/dma/ti/k3-udma.c        | 3751 ++-----------------------------
> 
> I'm affraid you do need to break this one up a bit. It might be doing it
> correctly, but it is impossible to check with the churn, like ....
noted. I'm working on splitting this. Will post a v2.

> 
>>   drivers/dma/ti/k3-udma.h        |  548 ++++-
>>   4 files changed, 3700 insertions(+), 3510 deletions(-)
>>   create mode 100644 drivers/dma/ti/k3-udma-common.c
> 
> ...
> 
>> -static bool udma_is_chan_running(struct udma_chan *uc)
>> -{
>> -	u32 trt_ctl = 0;
>> -	u32 rrt_ctl = 0;
>> -
>> -	if (uc->tchan)
>> -		trt_ctl = udma_tchanrt_read(uc, UDMA_CHAN_RT_CTL_REG);
>> -	if (uc->rchan)
>> -		rrt_ctl = udma_rchanrt_read(uc, UDMA_CHAN_RT_CTL_REG);
>> -
>> -	if (trt_ctl & UDMA_CHAN_RT_CTL_EN || rrt_ctl & UDMA_CHAN_RT_CTL_EN)
>> -		return true;
>> -
>> -	return false;
>> -}
>> -
>>   static bool udma_is_chan_paused(struct udma_chan *uc)
>>   {
>>   	u32 val, pause_mask;
>> @@ -643,189 +88,73 @@ static bool udma_is_chan_paused(struct udma_chan *uc)
>>   	return false;
>>   }
>>   
>> -static inline dma_addr_t udma_get_rx_flush_hwdesc_paddr(struct udma_chan *uc)
>> +static void udma_decrement_byte_counters(struct udma_chan *uc, u32 val)
> 
> 
> These sort of diffs.
> 
>>   {
>> -	return uc->ud->rx_flush.hwdescs[uc->config.pkt_mode].cppi5_desc_paddr;
>> +	if (uc->desc->dir == DMA_DEV_TO_MEM) {
>> +		udma_rchanrt_write(uc, UDMA_CHAN_RT_BCNT_REG, val);
>> +		udma_rchanrt_write(uc, UDMA_CHAN_RT_SBCNT_REG, val);
>> +		if (uc->config.ep_type != PSIL_EP_NATIVE)
>> +			udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
>> +	} else {
>> +		udma_tchanrt_write(uc, UDMA_CHAN_RT_BCNT_REG, val);
>> +		udma_tchanrt_write(uc, UDMA_CHAN_RT_SBCNT_REG, val);
>> +		if (!uc->bchan && uc->config.ep_type != PSIL_EP_NATIVE)
>> +			udma_tchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
>> +	}
>>   }
>>   
>> -static int udma_push_to_ring(struct udma_chan *uc, int idx)
>> +static void udma_reset_counters(struct udma_chan *uc)
>>   {
> 
> ...
> 
>> +struct udma_dev {
>> +	struct dma_device ddev;
>> +	struct device *dev;
>> +	void __iomem *mmrs[MMR_LAST];
>> +	const struct udma_match_data *match_data;
>> +	const struct udma_soc_data *soc_data;
>> +
>> +	struct udma_tpl bchan_tpl;
>> +	struct udma_tpl tchan_tpl;
>> +	struct udma_tpl rchan_tpl;
>> +
>> +	size_t desc_align; /* alignment to use for descriptors */
>> +
>> +	struct udma_tisci_rm tisci_rm;
>> +
>> +	struct k3_ringacc *ringacc;
>> +
>> +	struct work_struct purge_work;
>> +	struct list_head desc_to_purge;
>> +	spinlock_t lock;
>> +
>> +	struct udma_rx_flush rx_flush;
>> +
>> +	int bchan_cnt;
>> +	int tchan_cnt;
>> +	int echan_cnt;
>> +	int rchan_cnt;
>> +	int rflow_cnt;
>> +	int tflow_cnt;
>> +	unsigned long *bchan_map;
>> +	unsigned long *tchan_map;
>> +	unsigned long *rchan_map;
>> +	unsigned long *rflow_gp_map;
>> +	unsigned long *rflow_gp_map_allocated;
>> +	unsigned long *rflow_in_use;
>> +	unsigned long *tflow_map;
>> +
>> +	struct udma_bchan *bchans;
>> +	struct udma_tchan *tchans;
>> +	struct udma_rchan *rchans;
>> +	struct udma_rflow *rflows;
>> +
>> +	struct udma_chan *channels;
>> +	u32 psil_base;
>> +	u32 atype;
>> +	u32 asel;
>> +
>> +	int (*udma_start)(struct udma_chan *uc);
>> +	int (*udma_stop)(struct udma_chan *uc);
>> +	int (*udma_reset_chan)(struct udma_chan *uc, bool hard);
>> +	bool (*udma_is_desc_really_done)(struct udma_chan *uc, struct udma_desc *d);
>> +	void (*udma_decrement_byte_counters)(struct udma_chan *uc, u32 val);
> 
> You can drop the udma_ prefix, it is clear that they are for udma..
> 
>> +};