| Message ID | 20240701164233.2563221-7-Basavaraj.Natikar@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support of AMD AE4DMA DMA Engine | expand |
Hey Basavaraj, thanks for the rework. From what I can see most looks quite good now. Just found one more thing; sorry for not providing it sooner: On Mon, 2024-07-01 at 22:12 +0530, Basavaraj Natikar wrote: > To support multi-channel functionality with AE4DMA engine, extend the > ptdma-debugfs with reusable components. > > Reviewed-by: Raju Rangoju <Raju.Rangoju@amd.com> > Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> > --- > drivers/dma/amd/ptdma/ptdma-debugfs.c | 76 +++++++++++++++++++------ > -- > 1 file changed, 55 insertions(+), 21 deletions(-) > > diff --git a/drivers/dma/amd/ptdma/ptdma-debugfs.c > b/drivers/dma/amd/ptdma/ptdma-debugfs.c > index c8307d3044a3..9aa7a49ae5be 100644 > --- a/drivers/dma/amd/ptdma/ptdma-debugfs.c > +++ b/drivers/dma/amd/ptdma/ptdma-debugfs.c > @@ -12,7 +12,7 @@ > #include <linux/debugfs.h> > #include <linux/seq_file.h> > > -#include "ptdma.h" > +#include "../common/amd_dma.h" > > /* DebugFS helpers */ > #define RI_VERSION_NUM 0x0000003F > @@ -23,11 +23,19 @@ > static int pt_debugfs_info_show(struct seq_file *s, void *p) > { > struct pt_device *pt = s->private; > + struct ae4_device *ae4; > unsigned int regval; > > seq_printf(s, "Device name: %s\n", dev_name(pt->dev)); > - seq_printf(s, " # Queues: %d\n", 1); > - seq_printf(s, " # Cmds: %d\n", pt->cmd_count); > + > + if (pt->ver == AE4_DMA_VERSION) { > + ae4 = container_of(pt, struct ae4_device, pt); > + seq_printf(s, " # Queues: %d\n", ae4->cmd_q_count); > + seq_printf(s, " # Cmds per queue: %d\n", > CMD_Q_LEN); > + } else { > + seq_printf(s, " # Queues: %d\n", 1); > + seq_printf(s, " # Cmds: %d\n", pt->cmd_count); > + } > > regval = ioread32(pt->io_regs + CMD_PT_VERSION); > > @@ -55,6 +63,7 @@ static int pt_debugfs_stats_show(struct seq_file > *s, void *p) > static int pt_debugfs_queue_show(struct seq_file *s, void *p) > { > struct pt_cmd_queue *cmd_q = s->private; > + struct pt_device *pt; > unsigned int regval; > > if (!cmd_q) > @@ -62,18 +71,24 @@ static int pt_debugfs_queue_show(struct seq_file > *s, void *p) > > seq_printf(s, " Pass-Thru: %ld\n", cmd_q- > >total_pt_ops); > > - regval = ioread32(cmd_q->reg_control + 0x000C); > - > - seq_puts(s, " Enabled Interrupts:"); > - if (regval & INT_EMPTY_QUEUE) > - seq_puts(s, " EMPTY"); > - if (regval & INT_QUEUE_STOPPED) > - seq_puts(s, " STOPPED"); > - if (regval & INT_ERROR) > - seq_puts(s, " ERROR"); > - if (regval & INT_COMPLETION) > - seq_puts(s, " COMPLETION"); > - seq_puts(s, "\n"); > + pt = cmd_q->pt; > + if (pt->ver == AE4_DMA_VERSION) { > + regval = readl(cmd_q->reg_control + 0x4); > + seq_printf(s, " Enabled Interrupts:: status > 0x%x\n", regval); > + } else { > + regval = ioread32(cmd_q->reg_control + 0x000C); > + > + seq_puts(s, " Enabled Interrupts:"); > + if (regval & INT_EMPTY_QUEUE) > + seq_puts(s, " EMPTY"); > + if (regval & INT_QUEUE_STOPPED) > + seq_puts(s, " STOPPED"); > + if (regval & INT_ERROR) > + seq_puts(s, " ERROR"); > + if (regval & INT_COMPLETION) > + seq_puts(s, " COMPLETION"); > + seq_puts(s, "\n"); > + } > > return 0; > } > @@ -84,8 +99,12 @@ DEFINE_SHOW_ATTRIBUTE(pt_debugfs_stats); > > void ptdma_debugfs_setup(struct pt_device *pt) > { > - struct pt_cmd_queue *cmd_q; > struct dentry *debugfs_q_instance; > + struct ae4_cmd_queue *ae4cmd_q; > + struct pt_cmd_queue *cmd_q; > + struct ae4_device *ae4; > + char name[30]; Constant _might_ be better unless you're sure that the function will seldomly be changed, and: > + int i; > > if (!debugfs_initialized()) > return; > @@ -96,11 +115,26 @@ void ptdma_debugfs_setup(struct pt_device *pt) > debugfs_create_file("stats", 0400, pt->dma_dev.dbg_dev_root, > pt, > &pt_debugfs_stats_fops); > > - cmd_q = &pt->cmd_q; > > - debugfs_q_instance = > - debugfs_create_dir("q", pt->dma_dev.dbg_dev_root); > + if (pt->ver == AE4_DMA_VERSION) { > + ae4 = container_of(pt, struct ae4_device, pt); > + for (i = 0; i < ae4->cmd_q_count; i++) { > + ae4cmd_q = &ae4->ae4cmd_q[i]; > + cmd_q = &ae4cmd_q->cmd_q; > + > + snprintf(name, 29, "q%d", ae4cmd_q->id); That seems wrong to me. The trailing '\0' should be included: /** * snprintf - Format a string and place it in a buffer * @buf: The buffer to place the result into * @size: The size of the buffer, including the trailing null space * @fmt: The format string to use * @...: Arguments for the format string * * The return value is the number of characters which would be * generated for the given input, excluding the trailing null, * as per ISO C99. If the return is greater than or equal to * @size, the resulting string is truncated. * * See the vsnprintf() documentation for format string extensions over C99. */ int snprintf(char *buf, size_t size, const char *fmt, ...) Greetings, P. > + > + debugfs_q_instance = > + debugfs_create_dir(name, pt- > >dma_dev.dbg_dev_root); > > - debugfs_create_file("stats", 0400, debugfs_q_instance, cmd_q, > - &pt_debugfs_queue_fops); > + debugfs_create_file("stats", 0400, > debugfs_q_instance, cmd_q, > + &pt_debugfs_queue_fops); > + } > + } else { > + debugfs_q_instance = > + debugfs_create_dir("q", pt- > >dma_dev.dbg_dev_root); > + cmd_q = &pt->cmd_q; > + debugfs_create_file("stats", 0400, > debugfs_q_instance, cmd_q, > + &pt_debugfs_queue_fops); > + } > }
diff --git a/drivers/dma/amd/ptdma/ptdma-debugfs.c b/drivers/dma/amd/ptdma/ptdma-debugfs.c index c8307d3044a3..9aa7a49ae5be 100644 --- a/drivers/dma/amd/ptdma/ptdma-debugfs.c +++ b/drivers/dma/amd/ptdma/ptdma-debugfs.c @@ -12,7 +12,7 @@ #include <linux/debugfs.h> #include <linux/seq_file.h> -#include "ptdma.h" +#include "../common/amd_dma.h" /* DebugFS helpers */ #define RI_VERSION_NUM 0x0000003F @@ -23,11 +23,19 @@ static int pt_debugfs_info_show(struct seq_file *s, void *p) { struct pt_device *pt = s->private; + struct ae4_device *ae4; unsigned int regval; seq_printf(s, "Device name: %s\n", dev_name(pt->dev)); - seq_printf(s, " # Queues: %d\n", 1); - seq_printf(s, " # Cmds: %d\n", pt->cmd_count); + + if (pt->ver == AE4_DMA_VERSION) { + ae4 = container_of(pt, struct ae4_device, pt); + seq_printf(s, " # Queues: %d\n", ae4->cmd_q_count); + seq_printf(s, " # Cmds per queue: %d\n", CMD_Q_LEN); + } else { + seq_printf(s, " # Queues: %d\n", 1); + seq_printf(s, " # Cmds: %d\n", pt->cmd_count); + } regval = ioread32(pt->io_regs + CMD_PT_VERSION); @@ -55,6 +63,7 @@ static int pt_debugfs_stats_show(struct seq_file *s, void *p) static int pt_debugfs_queue_show(struct seq_file *s, void *p) { struct pt_cmd_queue *cmd_q = s->private; + struct pt_device *pt; unsigned int regval; if (!cmd_q) @@ -62,18 +71,24 @@ static int pt_debugfs_queue_show(struct seq_file *s, void *p) seq_printf(s, " Pass-Thru: %ld\n", cmd_q->total_pt_ops); - regval = ioread32(cmd_q->reg_control + 0x000C); - - seq_puts(s, " Enabled Interrupts:"); - if (regval & INT_EMPTY_QUEUE) - seq_puts(s, " EMPTY"); - if (regval & INT_QUEUE_STOPPED) - seq_puts(s, " STOPPED"); - if (regval & INT_ERROR) - seq_puts(s, " ERROR"); - if (regval & INT_COMPLETION) - seq_puts(s, " COMPLETION"); - seq_puts(s, "\n"); + pt = cmd_q->pt; + if (pt->ver == AE4_DMA_VERSION) { + regval = readl(cmd_q->reg_control + 0x4); + seq_printf(s, " Enabled Interrupts:: status 0x%x\n", regval); + } else { + regval = ioread32(cmd_q->reg_control + 0x000C); + + seq_puts(s, " Enabled Interrupts:"); + if (regval & INT_EMPTY_QUEUE) + seq_puts(s, " EMPTY"); + if (regval & INT_QUEUE_STOPPED) + seq_puts(s, " STOPPED"); + if (regval & INT_ERROR) + seq_puts(s, " ERROR"); + if (regval & INT_COMPLETION) + seq_puts(s, " COMPLETION"); + seq_puts(s, "\n"); + } return 0; } @@ -84,8 +99,12 @@ DEFINE_SHOW_ATTRIBUTE(pt_debugfs_stats); void ptdma_debugfs_setup(struct pt_device *pt) { - struct pt_cmd_queue *cmd_q; struct dentry *debugfs_q_instance; + struct ae4_cmd_queue *ae4cmd_q; + struct pt_cmd_queue *cmd_q; + struct ae4_device *ae4; + char name[30]; + int i; if (!debugfs_initialized()) return; @@ -96,11 +115,26 @@ void ptdma_debugfs_setup(struct pt_device *pt) debugfs_create_file("stats", 0400, pt->dma_dev.dbg_dev_root, pt, &pt_debugfs_stats_fops); - cmd_q = &pt->cmd_q; - debugfs_q_instance = - debugfs_create_dir("q", pt->dma_dev.dbg_dev_root); + if (pt->ver == AE4_DMA_VERSION) { + ae4 = container_of(pt, struct ae4_device, pt); + for (i = 0; i < ae4->cmd_q_count; i++) { + ae4cmd_q = &ae4->ae4cmd_q[i]; + cmd_q = &ae4cmd_q->cmd_q; + + snprintf(name, 29, "q%d", ae4cmd_q->id); + + debugfs_q_instance = + debugfs_create_dir(name, pt->dma_dev.dbg_dev_root); - debugfs_create_file("stats", 0400, debugfs_q_instance, cmd_q, - &pt_debugfs_queue_fops); + debugfs_create_file("stats", 0400, debugfs_q_instance, cmd_q, + &pt_debugfs_queue_fops); + } + } else { + debugfs_q_instance = + debugfs_create_dir("q", pt->dma_dev.dbg_dev_root); + cmd_q = &pt->cmd_q; + debugfs_create_file("stats", 0400, debugfs_q_instance, cmd_q, + &pt_debugfs_queue_fops); + } }