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

Péter Ujfalusi
2025-05-09T14:23:55Z

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