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

Adivi, Sai Sree Kartheek
2025-05-28T10:29:31Z

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..
>
>> +};