Re: [PATCH v6 5/5] dmaengine: dw-edma: Add trace support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dmaengine: dw-edma: tracepoints use edma_ prefix instead of dw_edma_
--

commit 8d4bbe2d0d3de12559142ae2213cc91782526216
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Add trace support

This commit adds tracepoints for linked-list publication, descriptor append
and completion, and doorbells in the dw-edma driver to help diagnose stalls.

> diff --git a/drivers/dma/dw-edma/dw-edma-trace.h b/drivers/dma/dw-edma/dw-edma-trace.h
> new file mode 100644
> index 0000000000000..653b36acff158
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-edma-trace.h
> @@ -0,0 +1,117 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright 2023 NXP.
> + */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM dw_edma
> +
> +#if !defined(__LINUX_DW_EDMA_TRACE) || defined(TRACE_HEADER_MULTI_READ)
> +#define __LINUX_DW_EDMA_TRACE
> +
> +#include <linux/types.h>
> +#include <linux/tracepoint.h>
> +
> +DECLARE_EVENT_CLASS(edma_desc_info,
> +	TP_PROTO(struct dw_edma_desc *desc),
> +	TP_ARGS(desc),
> +	TP_STRUCT__entry(
> +		__field(size_t, nburst)
> +		__field(size_t, start_burst)
> +		__field(dma_cookie_t, cookie)
> +		__field(u32, id)
> +		__field(u8, dir)
> +	),
> +	TP_fast_assign(
> +		__entry->nburst = desc->nburst;
> +		__entry->start_burst = desc->start_burst;
> +		__entry->id = desc->chan->id;
> +		__entry->dir = desc->chan->dir;
> +		__entry->cookie = desc->vd.tx.cookie;
> +	),
> +	TP_printk("chan %u%c desc %d nburst %zu start_burst %zu",
> +		__entry->id,
> +		__entry->dir ? 'R' : 'W',
> +		__entry->cookie,
> +		__entry->nburst,
> +		__entry->start_burst)
> +);
> +
> +DEFINE_EVENT(edma_desc_info, edma_append_desc,
> +	TP_PROTO(struct dw_edma_desc *desc),
> +	TP_ARGS(desc)
> +);
> +
> +DEFINE_EVENT(edma_desc_info, edma_complete_desc,
> +	TP_PROTO(struct dw_edma_desc *desc),
> +	TP_ARGS(desc)
> +);
> +
> +TRACE_EVENT(edma_doorbell,
> +	TP_PROTO(struct dw_edma_chan *chan),
> +	TP_ARGS(chan),
> +	TP_STRUCT__entry(
> +		__field(u32, id)
> +		__field(u8, dir)
> +		__field(u32, done)
> +		__field(u32, head)
> +	),
> +	TP_fast_assign(
> +		__entry->id = chan->id;
> +		__entry->dir = chan->dir;
> +		__entry->done = chan->ll_done;
> +		__entry->head = chan->ll_head;
> +	),
> +	TP_printk("chan %u%c done %u head %u",
> +		__entry->id,
> +		__entry->dir ? 'R' : 'W',
> +		__entry->done,
> +		__entry->head)
> +);
> +
> +TRACE_EVENT(edma_fill_ll,
> +	TP_PROTO(struct dw_edma_chan *chan, u32 idx, dma_cookie_t cookie, u64 src,
> +		 u64 dest, u32 sz, bool flag),
> +	TP_ARGS(chan, idx, cookie, src, dest, sz, flag),

[Severity: Low]
This isn't a bug, but since TRACE_SYSTEM in drivers/dma/dw-edma/dw-edma-trace.h
is defined as dw_edma, should the event class and tracepoints follow the
subsystem_event naming convention and use the dw_edma_ prefix instead of
edma_?

Specifically, edma_desc_info, edma_append_desc, edma_complete_desc,
edma_doorbell, and edma_fill_ll export global tracepoint symbols under the
generic edma_ prefix (which is also used by drivers/dma/fsl-edma-trace.h)
and are invoked in drivers/dma/dw-edma/dw-edma-core.c from
dw_edma_ll_consume(), dw_edma_core_ll_start(), dw_edma_start_transfer(),
and dw_edma_core_ch_doorbell().

Renaming them to dw_edma_desc_info, dw_edma_append_desc,
dw_edma_complete_desc, dw_edma_doorbell, and dw_edma_fill_ll (along with
their callers in dw-edma-core.c) would match the TRACE_SYSTEM name and avoid
generic edma_ symbol names.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030150742.2586776-1-den@valinux.co.jp?part=5