2023-01-13 17:36:45

by Serge Semin

[permalink] [raw]
Subject: [PATCH v9 09/27] dmaengine: dw-edma: Drop chancnt initialization

DMA device drivers aren't supposed to initialize the dma_device.chancnt
field. It will be done by the DMA-engine core in accordance with number of
added virtual DMA-channels. Pre-initializing it with some value causes
having a wrong number of channels printed in the device summary.

Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Tested-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Vinod Koul <[email protected]>
---
drivers/dma/dw-edma/dw-edma-core.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 6c9f95a8e397..ecd3e8f7ac5d 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -817,7 +817,6 @@ static int dw_edma_channel_setup(struct dw_edma *dw, bool write,
dma->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
dma->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
dma->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
- dma->chancnt = cnt;

/* Set DMA channel callbacks */
dma->dev = chip->dev;
--
2.39.0



2023-01-21 00:06:03

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v9 09/27] dmaengine: dw-edma: Drop chancnt initialization

On Fri, Jan 13, 2023 at 08:13:51PM +0300, Serge Semin wrote:
> DMA device drivers aren't supposed to initialize the dma_device.chancnt
> field. It will be done by the DMA-engine core in accordance with number of
> added virtual DMA-channels. Pre-initializing it with some value causes
> having a wrong number of channels printed in the device summary.
>
> Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> Signed-off-by: Serge Semin <[email protected]>
> Reviewed-by: Manivannan Sadhasivam <[email protected]>
> Tested-by: Manivannan Sadhasivam <[email protected]>
> Acked-by: Vinod Koul <[email protected]>
> ---
> drivers/dma/dw-edma/dw-edma-core.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 6c9f95a8e397..ecd3e8f7ac5d 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -817,7 +817,6 @@ static int dw_edma_channel_setup(struct dw_edma *dw, bool write,
> dma->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> dma->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> dma->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
> - dma->chancnt = cnt;

Did you look for other instances of this bug? There are several other
places that *look* like they might have the same issue:

axi_dmac_probe
dw_probe (dw-axi-dmac-platform.c)
ioat_pci_probe
plx_dma_create
...

I hate to fix just one if there are other similar issues.

> /* Set DMA channel callbacks */
> dma->dev = chip->dev;
> --
> 2.39.0
>
>

2023-01-21 21:36:13

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v9 09/27] dmaengine: dw-edma: Drop chancnt initialization

On Fri, Jan 20, 2023 at 05:54:49PM -0600, Bjorn Helgaas wrote:
> On Fri, Jan 13, 2023 at 08:13:51PM +0300, Serge Semin wrote:
> > DMA device drivers aren't supposed to initialize the dma_device.chancnt
> > field. It will be done by the DMA-engine core in accordance with number of
> > added virtual DMA-channels. Pre-initializing it with some value causes
> > having a wrong number of channels printed in the device summary.
> >
> > Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> > Signed-off-by: Serge Semin <[email protected]>
> > Reviewed-by: Manivannan Sadhasivam <[email protected]>
> > Tested-by: Manivannan Sadhasivam <[email protected]>
> > Acked-by: Vinod Koul <[email protected]>
> > ---
> > drivers/dma/dw-edma/dw-edma-core.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index 6c9f95a8e397..ecd3e8f7ac5d 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -817,7 +817,6 @@ static int dw_edma_channel_setup(struct dw_edma *dw, bool write,
> > dma->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> > dma->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> > dma->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
> > - dma->chancnt = cnt;
>

> Did you look for other instances of this bug? There are several other
> places that *look* like they might have the same issue:
>
> axi_dmac_probe
> dw_probe (dw-axi-dmac-platform.c)
> ioat_pci_probe
> plx_dma_create
> ...
>
> I hate to fix just one if there are other similar issues.

Indeed. The same problem can be spotted in:
drivers/dma/dma-axi-dmac.c
drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
drivers/dma/ioat/init.c
drivers/dma/plx_dma.c
drivers/dma/qcom/hidma.c
drivers/dma/sprd-dma.c
drivers/rapidio/devices/tsi721_dma.c

Moreover besides causing a wrong number of channels printed in the
device summary the problem may also lead to the dma_ida IDs leak. So
it's a bit more complex than I described in the patch log.

@Vinod you didn't say anything about that during the patch review. Do
you want me to provide a similar fix for the denoted drivers?

-Serge(y)

>
> > /* Set DMA channel callbacks */
> > dma->dev = chip->dev;
> > --
> > 2.39.0
> >
> >