| Message ID | 20220503225104.12108-25-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | dmaengine: dw-edma: Add RP/EP local DMA controllers support | expand |
On Wed, May 04, 2022 at 01:51:02AM +0300, Serge Semin wrote: > DW eDMA driver private data is preserved in the passed DW eDMA chip info > structure. If either probe procedure failed or for some reason the passed > info object doesn't have private data pointer initialized we need to halt > the DMA device cleanup procedure in order to prevent possible system > crashes. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> As I said in the previous iteration, I'm not sure we will hit this issue. But I don't object the patch. So, Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Thanks, Mani > --- > drivers/dma/dw-edma/dw-edma-core.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c > index 908607785401..561686b51915 100644 > --- a/drivers/dma/dw-edma/dw-edma-core.c > +++ b/drivers/dma/dw-edma/dw-edma-core.c > @@ -1035,6 +1035,10 @@ int dw_edma_remove(struct dw_edma_chip *chip) > struct dw_edma *dw = chip->dw; > int i; > > + /* Skip removal if no private data found */ > + if (!dw) > + return -ENODEV; > + > /* Disable eDMA */ > dw_edma_v0_core_off(dw); > > -- > 2.35.1 >
On Thu, May 12, 2022 at 08:53:54PM +0530, Manivannan Sadhasivam wrote: > On Wed, May 04, 2022 at 01:51:02AM +0300, Serge Semin wrote: > > DW eDMA driver private data is preserved in the passed DW eDMA chip info > > structure. If either probe procedure failed or for some reason the passed > > info object doesn't have private data pointer initialized we need to halt > > the DMA device cleanup procedure in order to prevent possible system > > crashes. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > As I said in the previous iteration, I'm not sure we will hit this issue. It's unlikely but still not impossible. Consider the case when your DW PCIe controller has eDMA capability, but the platform code isn't ready to have it properly utilized. In that case dw_edma_probe() will fail at some point while the rest of the DW PCIe code will go on executing ignoring that error. In that case calling the dw_edma_remove() will cause the system crash since the dw_edma_chip.dw pointer will be left uninitialized. For instance possible circumstances can be as follows: dw_pcie_host_init() +-> dw_pcie_edma_detect() FAIL ignored. +-> pci_host_probe() FAIL goto err_stop_link | +-> dw_pcie_edma_remove() CRASH !!! So to be on a safe side we need to make sure the DW eDMA descriptor cleanup is skipped in case if it hasn't been properly initialized. The proper initialization is indicated by having the dw_edma_chip.dw set with the valid pointer to the private data. > But I don't object the patch. So, > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Thanks. -Sergey > > Thanks, > Mani > > > --- > > drivers/dma/dw-edma/dw-edma-core.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c > > index 908607785401..561686b51915 100644 > > --- a/drivers/dma/dw-edma/dw-edma-core.c > > +++ b/drivers/dma/dw-edma/dw-edma-core.c > > @@ -1035,6 +1035,10 @@ int dw_edma_remove(struct dw_edma_chip *chip) > > struct dw_edma *dw = chip->dw; > > int i; > > > > + /* Skip removal if no private data found */ > > + if (!dw) > > + return -ENODEV; > > + > > /* Disable eDMA */ > > dw_edma_v0_core_off(dw); > > > > -- > > 2.35.1 > > > > -- > மணிவண்ணன் சதாசிவம்
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c index 908607785401..561686b51915 100644 --- a/drivers/dma/dw-edma/dw-edma-core.c +++ b/drivers/dma/dw-edma/dw-edma-core.c @@ -1035,6 +1035,10 @@ int dw_edma_remove(struct dw_edma_chip *chip) struct dw_edma *dw = chip->dw; int i; + /* Skip removal if no private data found */ + if (!dw) + return -ENODEV; + /* Disable eDMA */ dw_edma_v0_core_off(dw);
DW eDMA driver private data is preserved in the passed DW eDMA chip info structure. If either probe procedure failed or for some reason the passed info object doesn't have private data pointer initialized we need to halt the DMA device cleanup procedure in order to prevent possible system crashes. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> --- drivers/dma/dw-edma/dw-edma-core.c | 4 ++++ 1 file changed, 4 insertions(+)