cdd->cd and cdd->descs_phys are allocated DESCS_AREAS times from
init_descs() and freed as often from purge_descs(). This doesn't cause
any trouble as DESCS_AREAS is ensured to be 1, but it looks wrong to
do the (de)allocation inside the loop.
While at it, remove the intermediate variable mem_decs (I guess it was
only there to make the code comply to the 80-chars CodingSytle rule).
Signed-off-by: Daniel Mack
---
Hi Vinod,
Hi Sebastian,
I know we discussed that patch before, but AFAIR, there was no
conclusion. Could we have another look please?
As mentioned, I still think the current code looks wrong, even though it
doesn't harm because DESCS_AREAS is always 1, as ensured by
BUILD_BUG_ON(DESCS_AREAS != 1).
Still, calling dma_{alloc,free}_coherent inside the loop without the
loop variable anywhere in the assigned pointer really smells like
a memory leak or double-free, respectively, doesn't it? :)
Many thanks,
Daniel
drivers/dma/cppi41.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index c29dacf..896252f 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -719,13 +719,12 @@ static void purge_descs(struct device *dev, struct
cppi41_dd *cdd)
mem_decs = ALLOC_DECS_NUM * sizeof(struct cppi41_desc);
for (i = 0; i < DESCS_AREAS; i++) {
-
cppi_writel(0, cdd->qmgr_mem + QMGR_MEMBASE(i));
cppi_writel(0, cdd->qmgr_mem + QMGR_MEMCTRL(i));
-
- dma_free_coherent(dev, mem_decs, cdd->cd,
- cdd->descs_phys);
}
+
+ dma_free_coherent(dev, ALLOC_DECS_NUM * sizeof(struct cppi41_desc),
+ cdd->cd, cdd->descs_phys);
}
static void disable_sched(struct cppi41_dd *cdd)
@@ -747,8 +746,7 @@ static void deinit_cppi41(struct device *dev, struct
cppi41_dd *cdd)
static int init_descs(struct device *dev, struct cppi41_dd *cdd)
{
- unsigned int desc_size;
- unsigned int mem_decs;
+ unsigned int desc_size = sizeof(struct cppi41_desc);
int i;
u32 reg;
u32 idx;
@@ -757,28 +755,25 @@ static int init_descs(struct device *dev, struct
cppi41_dd *cdd)
(sizeof(struct cppi41_desc) - 1));
BUILD_BUG_ON(sizeof(struct cppi41_desc) < 32);
BUILD_BUG_ON(ALLOC_DECS_NUM < 32);
+ BUILD_BUG_ON(DESCS_AREAS != 1);
- desc_size = sizeof(struct cppi41_desc);
- mem_decs = ALLOC_DECS_NUM * desc_size;
+ cdd->cd = dma_alloc_coherent(dev, ALLOC_DECS_NUM * desc_size,
+&cdd->descs_phys, GFP_KERNEL);
+ if (!cdd->cd)
+ return -ENOMEM;
idx = 0;
for (i = 0; i < DESCS_AREAS; i++) {
-
reg = idx << QMGR_MEMCTRL_IDX_SH;
reg |= (ilog2(desc_size) - 5) << QMGR_MEMCTRL_DESC_SH;
reg |= ilog2(ALLOC_DECS_NUM) - 5;
- BUILD_BUG_ON(DESCS_AREAS != 1);
- cdd->cd = dma_alloc_coherent(dev, mem_decs,
- &cdd->descs_phys, GFP_KERNEL);
- if (!cdd->cd)
- return -ENOMEM;
-
cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
cppi_writel(reg, cdd->qmgr_mem + QMGR_MEMCTRL(i));
idx += ALLOC_DECS_NUM;
}
+
return 0;
}
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
