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

["Péter Ujfalusi" <peter.ujfalusi@gmail.com>]

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 ....

>  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..

> +};
-- 
Péter