| Message ID | ba56a34f11c4bb699079946ca51bb11244ac713e.1556043127.git.gustavo.pimentel@synopsys.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dmaengine: Add Synopsys eDMA IP driver (version 0) | expand |
On 23-04-19, 20:30, Gustavo Pimentel wrote: > int dw_edma_v0_core_debugfs_on(struct dw_edma_chip *chip) > { > - return 0; > + return dw_edma_v0_debugfs_on(chip); who calls this? > +#define RD_PERM 0444 lets not use a macro for these, 444 is more readable :) > +static int dw_edma_debugfs_u32_get(void *data, u64 *val) > +{ > + if (dw->mode == EDMA_MODE_LEGACY && > + data >= (void *)®s->type.legacy.ch) { > + void *ptr = (void *)®s->type.legacy.ch; > + u32 viewport_sel = 0; > + unsigned long flags; > + u16 ch; > + > + for (ch = 0; ch < dw->wr_ch_cnt; ch++) > + if (lim[0][ch].start >= data && data < lim[0][ch].end) { > + ptr += (data - lim[0][ch].start); > + goto legacy_sel_wr; > + } > + > + for (ch = 0; ch < dw->rd_ch_cnt; ch++) > + if (lim[1][ch].start >= data && data < lim[1][ch].end) { > + ptr += (data - lim[1][ch].start); > + goto legacy_sel_rd; > + } > + > + return 0; > +legacy_sel_rd: > + viewport_sel = BIT(31); > +legacy_sel_wr: > + viewport_sel |= FIELD_PREP(EDMA_V0_VIEWPORT_MASK, ch); > + > + raw_spin_lock_irqsave(&dw->lock, flags); > + > + writel(viewport_sel, ®s->type.legacy.viewport_sel); > + *val = readl((u32 *)ptr); why do you need the cast? > +static int dw_edma_debugfs_regs(void) > +{ > + const struct debugfs_entries debugfs_regs[] = { > + REGISTER(ctrl_data_arb_prior), > + REGISTER(ctrl), > + }; > + struct dentry *regs_dir; > + int nr_entries, err; > + > + regs_dir = debugfs_create_dir(REGISTERS_STR, base_dir); > + if (!regs_dir) > + return -EPERM; > + > + nr_entries = ARRAY_SIZE(debugfs_regs); > + err = dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir); > + if (err) > + return err; > + > + err = dw_edma_debugfs_regs_wr(regs_dir); > + if (err) > + return err; > + > + err = dw_edma_debugfs_regs_rd(regs_dir); > + if (err) > + return err; > + > + return 0; single return err would suffice right? > +int dw_edma_v0_debugfs_on(struct dw_edma_chip *chip) > +{ > + struct dentry *entry; > + int err; > + > + dw = chip->dw; > + if (!dw) > + return -EPERM; > + > + regs = (struct dw_edma_v0_regs *)dw->rg_region.vaddr; > + if (!regs) > + return -EPERM; > + > + base_dir = debugfs_create_dir(dw->name, 0); > + if (!base_dir) > + return -EPERM; > + > + entry = debugfs_create_u32("version", RD_PERM, base_dir, &dw->version); > + if (!entry) > + return -EPERM; > + > + entry = debugfs_create_u32("mode", RD_PERM, base_dir, &dw->mode); > + if (!entry) > + return -EPERM; > + > + entry = debugfs_create_u16("wr_ch_cnt", RD_PERM, base_dir, > + &dw->wr_ch_cnt); > + if (!entry) > + return -EPERM; > + > + entry = debugfs_create_u16("rd_ch_cnt", RD_PERM, base_dir, > + &dw->rd_ch_cnt); > + if (!entry) > + return -EPERM; > + > + err = dw_edma_debugfs_regs(); > + return err; return dw_edma_debugfs_regs() perhpaps..?
Hi Vinod, On Mon, May 6, 2019 at 13:7:10, Vinod Koul <vkoul@kernel.org> wrote: > On 23-04-19, 20:30, Gustavo Pimentel wrote: > > > int dw_edma_v0_core_debugfs_on(struct dw_edma_chip *chip) > > { > > - return 0; > > + return dw_edma_v0_debugfs_on(chip); > > who calls this? The main driver. This was done like this for 2 reasons: 1) To not break the patch #1 compilation 2) Since the code it's to extensive, I decided to break it in another patch. > > > +#define RD_PERM 0444 > > lets not use a macro for these, 444 is more readable :) Ok. > > > +static int dw_edma_debugfs_u32_get(void *data, u64 *val) > > +{ > > + if (dw->mode == EDMA_MODE_LEGACY && > > + data >= (void *)®s->type.legacy.ch) { > > + void *ptr = (void *)®s->type.legacy.ch; > > + u32 viewport_sel = 0; > > + unsigned long flags; > > + u16 ch; > > + > > + for (ch = 0; ch < dw->wr_ch_cnt; ch++) > > + if (lim[0][ch].start >= data && data < lim[0][ch].end) { > > + ptr += (data - lim[0][ch].start); > > + goto legacy_sel_wr; > > + } > > + > > + for (ch = 0; ch < dw->rd_ch_cnt; ch++) > > + if (lim[1][ch].start >= data && data < lim[1][ch].end) { > > + ptr += (data - lim[1][ch].start); > > + goto legacy_sel_rd; > > + } > > + > > + return 0; > > +legacy_sel_rd: > > + viewport_sel = BIT(31); > > +legacy_sel_wr: > > + viewport_sel |= FIELD_PREP(EDMA_V0_VIEWPORT_MASK, ch); > > + > > + raw_spin_lock_irqsave(&dw->lock, flags); > > + > > + writel(viewport_sel, ®s->type.legacy.viewport_sel); > > + *val = readl((u32 *)ptr); > > why do you need the cast? I can't tell from my head, but I think checkpatch or some code analysis tool was complaining about not having that. > > > +static int dw_edma_debugfs_regs(void) > > +{ > > + const struct debugfs_entries debugfs_regs[] = { > > + REGISTER(ctrl_data_arb_prior), > > + REGISTER(ctrl), > > + }; > > + struct dentry *regs_dir; > > + int nr_entries, err; > > + > > + regs_dir = debugfs_create_dir(REGISTERS_STR, base_dir); > > + if (!regs_dir) > > + return -EPERM; > > + > > + nr_entries = ARRAY_SIZE(debugfs_regs); > > + err = dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir); > > + if (err) > > + return err; > > + > > + err = dw_edma_debugfs_regs_wr(regs_dir); > > + if (err) > > + return err; > > + > > + err = dw_edma_debugfs_regs_rd(regs_dir); > > + if (err) > > + return err; > > + > > + return 0; > > single return err would suffice right? By looking now to this code, I decided to remove the err variable and perform the if immediately on the function, if the result is different from 0 it goes directly to a return -EPERM. > > > +int dw_edma_v0_debugfs_on(struct dw_edma_chip *chip) > > +{ > > + struct dentry *entry; > > + int err; > > + > > + dw = chip->dw; > > + if (!dw) > > + return -EPERM; > > + > > + regs = (struct dw_edma_v0_regs *)dw->rg_region.vaddr; > > + if (!regs) > > + return -EPERM; > > + > > + base_dir = debugfs_create_dir(dw->name, 0); > > + if (!base_dir) > > + return -EPERM; > > + > > + entry = debugfs_create_u32("version", RD_PERM, base_dir, &dw->version); > > + if (!entry) > > + return -EPERM; > > + > > + entry = debugfs_create_u32("mode", RD_PERM, base_dir, &dw->mode); > > + if (!entry) > > + return -EPERM; > > + > > + entry = debugfs_create_u16("wr_ch_cnt", RD_PERM, base_dir, > > + &dw->wr_ch_cnt); > > + if (!entry) > > + return -EPERM; > > + > > + entry = debugfs_create_u16("rd_ch_cnt", RD_PERM, base_dir, > > + &dw->rd_ch_cnt); > > + if (!entry) > > + return -EPERM; > > + > > + err = dw_edma_debugfs_regs(); > > + return err; > > return dw_edma_debugfs_regs() perhpaps..? Sure, small traces of old code. > -- > ~Vinod Regards, Gustavo
On 06-05-19, 17:09, Gustavo Pimentel wrote: > Hi Vinod, > > On Mon, May 6, 2019 at 13:7:10, Vinod Koul <vkoul@kernel.org> wrote: > > > On 23-04-19, 20:30, Gustavo Pimentel wrote: > > > > > int dw_edma_v0_core_debugfs_on(struct dw_edma_chip *chip) > > > { > > > - return 0; > > > + return dw_edma_v0_debugfs_on(chip); > > > > who calls this? > > The main driver. This was done like this for 2 reasons: > 1) To not break the patch #1 compilation > 2) Since the code it's to extensive, I decided to break it in another > patch. Hmm I guess I missed that one. I was actually looking at usage and not questioning split :) > > > +static int dw_edma_debugfs_u32_get(void *data, u64 *val) > > > +{ > > > + if (dw->mode == EDMA_MODE_LEGACY && > > > + data >= (void *)®s->type.legacy.ch) { > > > + void *ptr = (void *)®s->type.legacy.ch; > > > + u32 viewport_sel = 0; > > > + unsigned long flags; > > > + u16 ch; > > > + > > > + for (ch = 0; ch < dw->wr_ch_cnt; ch++) > > > + if (lim[0][ch].start >= data && data < lim[0][ch].end) { > > > + ptr += (data - lim[0][ch].start); > > > + goto legacy_sel_wr; > > > + } > > > + > > > + for (ch = 0; ch < dw->rd_ch_cnt; ch++) > > > + if (lim[1][ch].start >= data && data < lim[1][ch].end) { > > > + ptr += (data - lim[1][ch].start); > > > + goto legacy_sel_rd; > > > + } > > > + > > > + return 0; > > > +legacy_sel_rd: > > > + viewport_sel = BIT(31); > > > +legacy_sel_wr: > > > + viewport_sel |= FIELD_PREP(EDMA_V0_VIEWPORT_MASK, ch); > > > + > > > + raw_spin_lock_irqsave(&dw->lock, flags); > > > + > > > + writel(viewport_sel, ®s->type.legacy.viewport_sel); > > > + *val = readl((u32 *)ptr); > > > > why do you need the cast? > > I can't tell from my head, but I think checkpatch or some code analysis > tool was complaining about not having that. ptr is void, so there is no need for casts to or away from void. > > > +static int dw_edma_debugfs_regs(void) > > > +{ > > > + const struct debugfs_entries debugfs_regs[] = { > > > + REGISTER(ctrl_data_arb_prior), > > > + REGISTER(ctrl), > > > + }; > > > + struct dentry *regs_dir; > > > + int nr_entries, err; > > > + > > > + regs_dir = debugfs_create_dir(REGISTERS_STR, base_dir); > > > + if (!regs_dir) > > > + return -EPERM; > > > + > > > + nr_entries = ARRAY_SIZE(debugfs_regs); > > > + err = dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir); > > > + if (err) > > > + return err; > > > + > > > + err = dw_edma_debugfs_regs_wr(regs_dir); > > > + if (err) > > > + return err; > > > + > > > + err = dw_edma_debugfs_regs_rd(regs_dir); > > > + if (err) > > > + return err; > > > + > > > + return 0; > > > > single return err would suffice right? > > By looking now to this code, I decided to remove the err variable and > perform the if immediately on the function, if the result is different > from 0 it goes directly to a return -EPERM. And one more things, we do not need to check errors on debugfs calls, and if it fails it fails...
On Tue, May 7, 2019 at 6:11:36, Vinod Koul <vkoul@kernel.org> wrote: > On 06-05-19, 17:09, Gustavo Pimentel wrote: > > Hi Vinod, > > > > On Mon, May 6, 2019 at 13:7:10, Vinod Koul <vkoul@kernel.org> wrote: > > > > > On 23-04-19, 20:30, Gustavo Pimentel wrote: > > > > > > > int dw_edma_v0_core_debugfs_on(struct dw_edma_chip *chip) > > > > { > > > > - return 0; > > > > + return dw_edma_v0_debugfs_on(chip); > > > > > > who calls this? > > > > The main driver. This was done like this for 2 reasons: > > 1) To not break the patch #1 compilation > > 2) Since the code it's to extensive, I decided to break it in another > > patch. > > Hmm I guess I missed that one. I was actually looking at usage and not > questioning split :) > > > > > +static int dw_edma_debugfs_u32_get(void *data, u64 *val) > > > > +{ > > > > + if (dw->mode == EDMA_MODE_LEGACY && > > > > + data >= (void *)®s->type.legacy.ch) { > > > > + void *ptr = (void *)®s->type.legacy.ch; > > > > + u32 viewport_sel = 0; > > > > + unsigned long flags; > > > > + u16 ch; > > > > + > > > > + for (ch = 0; ch < dw->wr_ch_cnt; ch++) > > > > + if (lim[0][ch].start >= data && data < lim[0][ch].end) { > > > > + ptr += (data - lim[0][ch].start); > > > > + goto legacy_sel_wr; > > > > + } > > > > + > > > > + for (ch = 0; ch < dw->rd_ch_cnt; ch++) > > > > + if (lim[1][ch].start >= data && data < lim[1][ch].end) { > > > > + ptr += (data - lim[1][ch].start); > > > > + goto legacy_sel_rd; > > > > + } > > > > + > > > > + return 0; > > > > +legacy_sel_rd: > > > > + viewport_sel = BIT(31); > > > > +legacy_sel_wr: > > > > + viewport_sel |= FIELD_PREP(EDMA_V0_VIEWPORT_MASK, ch); > > > > + > > > > + raw_spin_lock_irqsave(&dw->lock, flags); > > > > + > > > > + writel(viewport_sel, ®s->type.legacy.viewport_sel); > > > > + *val = readl((u32 *)ptr); > > > > > > why do you need the cast? > > > > I can't tell from my head, but I think checkpatch or some code analysis > > tool was complaining about not having that. > > ptr is void, so there is no need for casts to or away from void. > > > > > +static int dw_edma_debugfs_regs(void) > > > > +{ > > > > + const struct debugfs_entries debugfs_regs[] = { > > > > + REGISTER(ctrl_data_arb_prior), > > > > + REGISTER(ctrl), > > > > + }; > > > > + struct dentry *regs_dir; > > > > + int nr_entries, err; > > > > + > > > > + regs_dir = debugfs_create_dir(REGISTERS_STR, base_dir); > > > > + if (!regs_dir) > > > > + return -EPERM; > > > > + > > > > + nr_entries = ARRAY_SIZE(debugfs_regs); > > > > + err = dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir); > > > > + if (err) > > > > + return err; > > > > + > > > > + err = dw_edma_debugfs_regs_wr(regs_dir); > > > > + if (err) > > > > + return err; > > > > + > > > > + err = dw_edma_debugfs_regs_rd(regs_dir); > > > > + if (err) > > > > + return err; > > > > + > > > > + return 0; > > > > > > single return err would suffice right? > > > > By looking now to this code, I decided to remove the err variable and > > perform the if immediately on the function, if the result is different > > from 0 it goes directly to a return -EPERM. > > And one more things, we do not need to check errors on debugfs calls, > and if it fails it fails... Ok, makes sense. I'll remove all return validation relatively to debugfs calls. > > -- > ~Vinod
diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile index 01c7c63..0c53033 100644 --- a/drivers/dma/dw-edma/Makefile +++ b/drivers/dma/dw-edma/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_DW_EDMA) += dw-edma.o +dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o dw-edma-objs := dw-edma-core.o \ - dw-edma-v0-core.o + dw-edma-v0-core.o $(dw-edma-y) diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c index aa5d3f7..2fde20c 100644 --- a/drivers/dma/dw-edma/dw-edma-v0-core.c +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c @@ -339,9 +339,10 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan) /* eDMA debugfs callbacks */ int dw_edma_v0_core_debugfs_on(struct dw_edma_chip *chip) { - return 0; + return dw_edma_v0_debugfs_on(chip); } void dw_edma_v0_core_debugfs_off(void) { + dw_edma_v0_debugfs_off(); } diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c new file mode 100644 index 0000000..d25497f --- /dev/null +++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c @@ -0,0 +1,361 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018-2019 Synopsys, Inc. and/or its affiliates. + * Synopsys DesignWare eDMA v0 core + * + * Author: Gustavo Pimentel <gustavo.pimentel@synopsys.com> + */ + +#include <linux/debugfs.h> +#include <linux/bitfield.h> + +#include "dw-edma-v0-debugfs.h" +#include "dw-edma-v0-regs.h" +#include "dw-edma-core.h" + +#define RD_PERM 0444 + +#define REGS_ADDR(name) \ + ((dma_addr_t *)®s->name) +#define REGISTER(name) \ + { #name, REGS_ADDR(name) } + +#define WR_REGISTER(name) \ + { #name, REGS_ADDR(wr_##name) } +#define RD_REGISTER(name) \ + { #name, REGS_ADDR(rd_##name) } + +#define WR_REGISTER_LEGACY(name) \ + { #name, REGS_ADDR(type.legacy.wr_##name) } +#define RD_REGISTER_LEGACY(name) \ + { #name, REGS_ADDR(type.legacy.rd_##name) } + +#define WR_REGISTER_UNROLL(name) \ + { #name, REGS_ADDR(type.unroll.wr_##name) } +#define RD_REGISTER_UNROLL(name) \ + { #name, REGS_ADDR(type.unroll.rd_##name) } + +#define WRITE_STR "write" +#define READ_STR "read" +#define CHANNEL_STR "channel" +#define REGISTERS_STR "registers" + +static struct dentry *base_dir; +static struct dw_edma *dw; +static struct dw_edma_v0_regs *regs; + +static struct { + void *start; + void *end; +} lim[2][EDMA_V0_MAX_NR_CH]; + +struct debugfs_entries { + char name[24]; + dma_addr_t *reg; +}; + +static int dw_edma_debugfs_u32_get(void *data, u64 *val) +{ + if (dw->mode == EDMA_MODE_LEGACY && + data >= (void *)®s->type.legacy.ch) { + void *ptr = (void *)®s->type.legacy.ch; + u32 viewport_sel = 0; + unsigned long flags; + u16 ch; + + for (ch = 0; ch < dw->wr_ch_cnt; ch++) + if (lim[0][ch].start >= data && data < lim[0][ch].end) { + ptr += (data - lim[0][ch].start); + goto legacy_sel_wr; + } + + for (ch = 0; ch < dw->rd_ch_cnt; ch++) + if (lim[1][ch].start >= data && data < lim[1][ch].end) { + ptr += (data - lim[1][ch].start); + goto legacy_sel_rd; + } + + return 0; +legacy_sel_rd: + viewport_sel = BIT(31); +legacy_sel_wr: + viewport_sel |= FIELD_PREP(EDMA_V0_VIEWPORT_MASK, ch); + + raw_spin_lock_irqsave(&dw->lock, flags); + + writel(viewport_sel, ®s->type.legacy.viewport_sel); + *val = readl((u32 *)ptr); + + raw_spin_unlock_irqrestore(&dw->lock, flags); + } else { + *val = readl((u32 *)data); + } + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_edma_debugfs_u32_get, NULL, "0x%08llx\n"); + +static int dw_edma_debugfs_create_x32(const struct debugfs_entries entries[], + int nr_entries, struct dentry *dir) +{ + struct dentry *entry; + int i; + + for (i = 0; i < nr_entries; i++) { + entry = debugfs_create_file_unsafe(entries[i].name, RD_PERM, + dir, entries[i].reg, + &fops_x32); + if (!entry) + return -EPERM; + } + + return 0; +} + +static int dw_edma_debugfs_regs_ch(struct dw_edma_v0_ch_regs *regs, + struct dentry *dir) +{ + int nr_entries; + const struct debugfs_entries debugfs_regs[] = { + REGISTER(ch_control1), + REGISTER(ch_control2), + REGISTER(transfer_size), + REGISTER(sar_low), + REGISTER(sar_high), + REGISTER(dar_low), + REGISTER(dar_high), + REGISTER(llp_low), + REGISTER(llp_high), + }; + + nr_entries = ARRAY_SIZE(debugfs_regs); + return dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, dir); +} + +static int dw_edma_debugfs_regs_wr(struct dentry *dir) +{ + const struct debugfs_entries debugfs_regs[] = { + /* eDMA global registers */ + WR_REGISTER(engine_en), + WR_REGISTER(doorbell), + WR_REGISTER(ch_arb_weight_low), + WR_REGISTER(ch_arb_weight_high), + /* eDMA interrupts registers */ + WR_REGISTER(int_status), + WR_REGISTER(int_mask), + WR_REGISTER(int_clear), + WR_REGISTER(err_status), + WR_REGISTER(done_imwr_low), + WR_REGISTER(done_imwr_high), + WR_REGISTER(abort_imwr_low), + WR_REGISTER(abort_imwr_high), + WR_REGISTER(ch01_imwr_data), + WR_REGISTER(ch23_imwr_data), + WR_REGISTER(ch45_imwr_data), + WR_REGISTER(ch67_imwr_data), + WR_REGISTER(linked_list_err_en), + }; + const struct debugfs_entries debugfs_unroll_regs[] = { + /* eDMA channel context grouping */ + WR_REGISTER_UNROLL(engine_chgroup), + WR_REGISTER_UNROLL(engine_hshake_cnt_low), + WR_REGISTER_UNROLL(engine_hshake_cnt_high), + WR_REGISTER_UNROLL(ch0_pwr_en), + WR_REGISTER_UNROLL(ch1_pwr_en), + WR_REGISTER_UNROLL(ch2_pwr_en), + WR_REGISTER_UNROLL(ch3_pwr_en), + WR_REGISTER_UNROLL(ch4_pwr_en), + WR_REGISTER_UNROLL(ch5_pwr_en), + WR_REGISTER_UNROLL(ch6_pwr_en), + WR_REGISTER_UNROLL(ch7_pwr_en), + }; + struct dentry *regs_dir, *ch_dir; + int nr_entries, i, err; + char name[16]; + + regs_dir = debugfs_create_dir(WRITE_STR, dir); + if (!regs_dir) + return -EPERM; + + nr_entries = ARRAY_SIZE(debugfs_regs); + err = dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir); + if (err) + return err; + + if (dw->mode == EDMA_MODE_UNROLL) { + nr_entries = ARRAY_SIZE(debugfs_unroll_regs); + err = dw_edma_debugfs_create_x32(debugfs_unroll_regs, + nr_entries, regs_dir); + if (err) + return err; + } + + for (i = 0; i < dw->wr_ch_cnt; i++) { + snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i); + + ch_dir = debugfs_create_dir(name, regs_dir); + if (!ch_dir) + return -EPERM; + + err = dw_edma_debugfs_regs_ch(®s->type.unroll.ch[i].wr, + ch_dir); + if (err) + return err; + + lim[0][i].start = ®s->type.unroll.ch[i].wr; + lim[0][i].end = ®s->type.unroll.ch[i].padding_1[0]; + } + + return 0; +} + +static int dw_edma_debugfs_regs_rd(struct dentry *dir) +{ + const struct debugfs_entries debugfs_regs[] = { + /* eDMA global registers */ + RD_REGISTER(engine_en), + RD_REGISTER(doorbell), + RD_REGISTER(ch_arb_weight_low), + RD_REGISTER(ch_arb_weight_high), + /* eDMA interrupts registers */ + RD_REGISTER(int_status), + RD_REGISTER(int_mask), + RD_REGISTER(int_clear), + RD_REGISTER(err_status_low), + RD_REGISTER(err_status_high), + RD_REGISTER(linked_list_err_en), + RD_REGISTER(done_imwr_low), + RD_REGISTER(done_imwr_high), + RD_REGISTER(abort_imwr_low), + RD_REGISTER(abort_imwr_high), + RD_REGISTER(ch01_imwr_data), + RD_REGISTER(ch23_imwr_data), + RD_REGISTER(ch45_imwr_data), + RD_REGISTER(ch67_imwr_data), + }; + const struct debugfs_entries debugfs_unroll_regs[] = { + /* eDMA channel context grouping */ + RD_REGISTER_UNROLL(engine_chgroup), + RD_REGISTER_UNROLL(engine_hshake_cnt_low), + RD_REGISTER_UNROLL(engine_hshake_cnt_high), + RD_REGISTER_UNROLL(ch0_pwr_en), + RD_REGISTER_UNROLL(ch1_pwr_en), + RD_REGISTER_UNROLL(ch2_pwr_en), + RD_REGISTER_UNROLL(ch3_pwr_en), + RD_REGISTER_UNROLL(ch4_pwr_en), + RD_REGISTER_UNROLL(ch5_pwr_en), + RD_REGISTER_UNROLL(ch6_pwr_en), + RD_REGISTER_UNROLL(ch7_pwr_en), + }; + struct dentry *regs_dir, *ch_dir; + int nr_entries, i, err; + char name[16]; + + regs_dir = debugfs_create_dir(READ_STR, dir); + if (!regs_dir) + return -EPERM; + + nr_entries = ARRAY_SIZE(debugfs_regs); + err = dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir); + if (err) + return err; + + if (dw->mode == EDMA_MODE_UNROLL) { + nr_entries = ARRAY_SIZE(debugfs_unroll_regs); + err = dw_edma_debugfs_create_x32(debugfs_unroll_regs, + nr_entries, regs_dir); + if (err) + return err; + } + + for (i = 0; i < dw->rd_ch_cnt; i++) { + snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i); + + ch_dir = debugfs_create_dir(name, regs_dir); + if (!ch_dir) + return -EPERM; + + err = dw_edma_debugfs_regs_ch(®s->type.unroll.ch[i].rd, + ch_dir); + if (err) + return err; + + lim[1][i].start = ®s->type.unroll.ch[i].rd; + lim[1][i].end = ®s->type.unroll.ch[i].padding_2[0]; + } + + return 0; +} + +static int dw_edma_debugfs_regs(void) +{ + const struct debugfs_entries debugfs_regs[] = { + REGISTER(ctrl_data_arb_prior), + REGISTER(ctrl), + }; + struct dentry *regs_dir; + int nr_entries, err; + + regs_dir = debugfs_create_dir(REGISTERS_STR, base_dir); + if (!regs_dir) + return -EPERM; + + nr_entries = ARRAY_SIZE(debugfs_regs); + err = dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir); + if (err) + return err; + + err = dw_edma_debugfs_regs_wr(regs_dir); + if (err) + return err; + + err = dw_edma_debugfs_regs_rd(regs_dir); + if (err) + return err; + + return 0; +} + +int dw_edma_v0_debugfs_on(struct dw_edma_chip *chip) +{ + struct dentry *entry; + int err; + + dw = chip->dw; + if (!dw) + return -EPERM; + + regs = (struct dw_edma_v0_regs *)dw->rg_region.vaddr; + if (!regs) + return -EPERM; + + base_dir = debugfs_create_dir(dw->name, 0); + if (!base_dir) + return -EPERM; + + entry = debugfs_create_u32("version", RD_PERM, base_dir, &dw->version); + if (!entry) + return -EPERM; + + entry = debugfs_create_u32("mode", RD_PERM, base_dir, &dw->mode); + if (!entry) + return -EPERM; + + entry = debugfs_create_u16("wr_ch_cnt", RD_PERM, base_dir, + &dw->wr_ch_cnt); + if (!entry) + return -EPERM; + + entry = debugfs_create_u16("rd_ch_cnt", RD_PERM, base_dir, + &dw->rd_ch_cnt); + if (!entry) + return -EPERM; + + err = dw_edma_debugfs_regs(); + return err; +} + +void dw_edma_v0_debugfs_off(void) +{ + debugfs_remove_recursive(base_dir); +} diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.h b/drivers/dma/dw-edma/dw-edma-v0-debugfs.h new file mode 100644 index 0000000..cf17820 --- /dev/null +++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2018-2019 Synopsys, Inc. and/or its affiliates. + * Synopsys DesignWare eDMA v0 core + * + * Author: Gustavo Pimentel <gustavo.pimentel@synopsys.com> + */ + +#ifndef _DW_EDMA_V0_DEBUG_FS_H +#define _DW_EDMA_V0_DEBUG_FS_H + +#include <linux/dma/edma.h> + +#ifdef CONFIG_DEBUG_FS +int dw_edma_v0_debugfs_on(struct dw_edma_chip *chip); +void dw_edma_v0_debugfs_off(void); +#else +static inline int dw_edma_v0_debugfs_on(struct dw_edma_chip *chip) +{ + return 0; +} + +static inline void dw_edma_v0_debugfs_off(void) +{ +} +#endif /* CONFIG_DEBUG_FS */ + +#endif /* _DW_EDMA_V0_DEBUG_FS_H */
Add Synopsys eDMA IP version 0 debugfs support to assist any debug in the future. Creates a file system structure composed by folders and files that mimic the IP register map (this files are read only) to ease any debug. To enable this feature is necessary to select DEBUG_FS option on kernel configuration. Small output example: (eDMA IP version 0, unroll, 1 write + 1 read channels) % mount -t debugfs none /sys/kernel/debug/ % tree /sys/kernel/debug/dw-edma-core:0/ dw-edma/ ├── version ├── mode ├── wr_ch_cnt ├── rd_ch_cnt └── registers ├── ctrl_data_arb_prior ├── ctrl ├── write │ ├── engine_en │ ├── doorbell │ ├── ch_arb_weight_low │ ├── ch_arb_weight_high │ ├── int_status │ ├── int_mask │ ├── int_clear │ ├── err_status │ ├── done_imwr_low │ ├── done_imwr_high │ ├── abort_imwr_low │ ├── abort_imwr_high │ ├── ch01_imwr_data │ ├── ch23_imwr_data │ ├── ch45_imwr_data │ ├── ch67_imwr_data │ ├── linked_list_err_en │ ├── engine_chgroup │ ├── engine_hshake_cnt_low │ ├── engine_hshake_cnt_high │ ├── ch0_pwr_en │ ├── ch1_pwr_en │ ├── ch2_pwr_en │ ├── ch3_pwr_en │ ├── ch4_pwr_en │ ├── ch5_pwr_en │ ├── ch6_pwr_en │ ├── ch7_pwr_en │ └── channel:0 │ ├── ch_control1 │ ├── ch_control2 │ ├── transfer_size │ ├── sar_low │ ├── sar_high │ ├── dar_high │ ├── llp_low │ └── llp_high └── read ├── engine_en ├── doorbell ├── ch_arb_weight_low ├── ch_arb_weight_high ├── int_status ├── int_mask ├── int_clear ├── err_status_low ├── err_status_high ├── done_imwr_low ├── done_imwr_high ├── abort_imwr_low ├── abort_imwr_high ├── ch01_imwr_data ├── ch23_imwr_data ├── ch45_imwr_data ├── ch67_imwr_data ├── linked_list_err_en ├── engine_chgroup ├── engine_hshake_cnt_low ├── engine_hshake_cnt_high ├── ch0_pwr_en ├── ch1_pwr_en ├── ch2_pwr_en ├── ch3_pwr_en ├── ch4_pwr_en ├── ch5_pwr_en ├── ch6_pwr_en ├── ch7_pwr_en └── channel:0 ├── ch_control1 ├── ch_control2 ├── transfer_size ├── sar_low ├── sar_high ├── dar_high ├── llp_low └── llp_high Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> Cc: Vinod Koul <vkoul@kernel.org> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Russell King <rmk+kernel@armlinux.org.uk> Cc: Joao Pinto <jpinto@synopsys.com> --- Changes: RFC v1->RFC v2: - Replace comments // (C99 style) by /**/ - Fix the headers of the .c and .h files according to the most recent convention - Fix errors and checks pointed out by checkpatch with --strict option - Replace patch small description tag from dma by dmaengine RFC v2->RFC v3: - Code rewrite to use FIELD_PREP() and FIELD_GET() - Add define to magic numbers - Rebase to v5.0-rc1 RFC v3->RFC v4: - Replaced number of entries of a struct calculation by a helper macro - Reorder variables declaration in reverse tree order on several functions - Fix inline function declarations when CONFIG_DEBUG_FS is not defined - Fix license header - Add missing cast RFC v4->RFC v5: - Patch resended, forgot to replace of '___' by '---' and to remove duplicate signed-off RFC v5->RFC v6: - Add author on file header drivers/dma/dw-edma/Makefile | 3 +- drivers/dma/dw-edma/dw-edma-v0-core.c | 3 +- drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 361 +++++++++++++++++++++++++++++++ drivers/dma/dw-edma/dw-edma-v0-debugfs.h | 28 +++ 4 files changed, 393 insertions(+), 2 deletions(-) create mode 100644 drivers/dma/dw-edma/dw-edma-v0-debugfs.c create mode 100644 drivers/dma/dw-edma/dw-edma-v0-debugfs.h