| Message ID | 20230113171409.30470-10-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | dmaengine: dw-edma: Add RP/EP local DMA controllers support | expand |
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 <Sergey.Semin@baikalelectronics.ru> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Tested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Acked-by: Vinod Koul <vkoul@kernel.org> > --- > 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 > >
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 <Sergey.Semin@baikalelectronics.ru> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Tested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Acked-by: Vinod Koul <vkoul@kernel.org> > > --- > > 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 > > > >
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;
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 <Sergey.Semin@baikalelectronics.ru> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Tested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Acked-by: Vinod Koul <vkoul@kernel.org> --- drivers/dma/dw-edma/dw-edma-core.c | 1 - 1 file changed, 1 deletion(-)