diff mbox series

[15/25] dmaengine: dw-edma: Convert DebugFS descs to being kz-allocated

Message ID 20220324014836.19149-16-Sergey.Semin@baikalelectronics.ru (mailing list archive)
State Superseded
Headers show
Series dmaengine: dw-edma: Add RP/EP local DMA controllers support | expand

Commit Message

Serge Semin March 24, 2022, 1:48 a.m. UTC
Currently all the DW eDMA DebugFS nodes descriptors are allocated on
stack, while the DW eDMA driver private data and CSR limits are statically
preserved. Such design won't work for the multi-eDMA platforms. As a
preparation to adding the multi-eDMA system setups support we need to have
each DebugFS node separately allocated and described. Afterwards we'll put
an addition info there like Read/Write channel flag, channel ID, DW eDMA
private data reference.

Note this conversion is mainly required due to having the legacy DW eDMA
controllers with indirect Read/Write channels context CSRs access. If we
didn't need to have a synchronized access to these registers the DebugFS
code of the driver would have been much simpler.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Serge Semin April 18, 2022, 7:17 a.m. UTC | #1
On Fri, Mar 25, 2022 at 11:33:49AM +0530, Manivannan Sadhasivam wrote:
> On Thu, Mar 24, 2022 at 04:48:26AM +0300, Serge Semin wrote:
> > Currently all the DW eDMA DebugFS nodes descriptors are allocated on
> > stack, while the DW eDMA driver private data and CSR limits are statically
> > preserved. Such design won't work for the multi-eDMA platforms.
> 
> Can you please explain why?
> 
> > As a
> > preparation to adding the multi-eDMA system setups support we need to have
> > each DebugFS node separately allocated and described. Afterwards we'll put
> > an addition info there like Read/Write channel flag, channel ID, DW eDMA
> > private data reference.
> > 
> > Note this conversion is mainly required due to having the legacy DW eDMA
> > controllers with indirect Read/Write channels context CSRs access. If we
> > didn't need to have a synchronized access to these registers the DebugFS
> > code of the driver would have been much simpler.
> > 
> 

> I fail to understand how this change is beneficial or the exact issue.

Just to be clear. It has only one benefit - an ability to preserve a
device-specific data in the dw_edma_debugfs_entry structure. That data
will be the dw_edma private data instance, which in it turn will be
used to access the register spin lock. All of that in general is
required to support more than one DW eDMA controllers in the system.

> 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > ---
> >  drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
> > index afd519d9568b..7eb0147912fa 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
> > @@ -53,7 +53,8 @@ struct dw_edma_debugfs_entry {
> >  
> >  static int dw_edma_debugfs_u32_get(void *data, u64 *val)
> >  {
> > -	void __iomem *reg = data;
> > +	struct dw_edma_debugfs_entry __iomem *entry = data;
> 

> Why the entry has to be of __iomem?

Good question. It has just slipped through my fingers in from the
previous code. You are right. I should drop it.

-Sergey

> 
> Thanks,
> Mani
> 
> > +	void __iomem *reg = entry->reg;
> >  
> >  	if (dw->chip->mf == EDMA_MF_EDMA_LEGACY &&
> >  	    reg >= (void __iomem *)&regs->type.legacy.ch) {
> > @@ -94,14 +95,22 @@ static int dw_edma_debugfs_u32_get(void *data, u64 *val)
> >  }
> >  DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_edma_debugfs_u32_get, NULL, "0x%08llx\n");
> >  
> > -static void dw_edma_debugfs_create_x32(const struct dw_edma_debugfs_entry entries[],
> > +static void dw_edma_debugfs_create_x32(const struct dw_edma_debugfs_entry ini[],
> >  				       int nr_entries, struct dentry *dir)
> >  {
> > +	struct dw_edma_debugfs_entry *entries;
> >  	int i;
> >  
> > +	entries = devm_kcalloc(dw->chip->dev, nr_entries, sizeof(*entries),
> > +			       GFP_KERNEL);
> > +	if (!entries)
> > +		return;
> > +
> >  	for (i = 0; i < nr_entries; i++) {
> > +		entries[i] = ini[i];
> > +
> >  		debugfs_create_file_unsafe(entries[i].name, 0444, dir,
> > -					   entries[i].reg, &fops_x32);
> > +					   &entries[i], &fops_x32);
> >  	}
> >  }
> >  
> > -- 
> > 2.35.1
> >
diff mbox series

Patch

diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
index afd519d9568b..7eb0147912fa 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
@@ -53,7 +53,8 @@  struct dw_edma_debugfs_entry {
 
 static int dw_edma_debugfs_u32_get(void *data, u64 *val)
 {
-	void __iomem *reg = data;
+	struct dw_edma_debugfs_entry __iomem *entry = data;
+	void __iomem *reg = entry->reg;
 
 	if (dw->chip->mf == EDMA_MF_EDMA_LEGACY &&
 	    reg >= (void __iomem *)&regs->type.legacy.ch) {
@@ -94,14 +95,22 @@  static int dw_edma_debugfs_u32_get(void *data, u64 *val)
 }
 DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_edma_debugfs_u32_get, NULL, "0x%08llx\n");
 
-static void dw_edma_debugfs_create_x32(const struct dw_edma_debugfs_entry entries[],
+static void dw_edma_debugfs_create_x32(const struct dw_edma_debugfs_entry ini[],
 				       int nr_entries, struct dentry *dir)
 {
+	struct dw_edma_debugfs_entry *entries;
 	int i;
 
+	entries = devm_kcalloc(dw->chip->dev, nr_entries, sizeof(*entries),
+			       GFP_KERNEL);
+	if (!entries)
+		return;
+
 	for (i = 0; i < nr_entries; i++) {
+		entries[i] = ini[i];
+
 		debugfs_create_file_unsafe(entries[i].name, 0444, dir,
-					   entries[i].reg, &fops_x32);
+					   &entries[i], &fops_x32);
 	}
 }