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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e823a43e-20a8-4142-875f-a3575cbb0d0b@gmail.com>
Date: Fri, 9 May 2025 17:25:06 +0300
From: Péter Ujfalusi <peter.ujfalusi@...il.com>
To: Sai Sree Kartheek Adivi <s-adivi@...com>, vkoul@...nel.org,
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, nm@...com,
 ssantosh@...nel.org, dmaengine@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 praneeth@...com, vigneshr@...com, u-kumar1@...com, a-chavda@...com
Subject: Re: [PATCH 3/8] drivers: dma: ti: Refactor TI K3 UDMA driver

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