| Message ID | 20220324014836.19149-23-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dmaengine: dw-edma: Add RP/EP local DMA controllers support | expand |
On Fri, Mar 25, 2022 at 03:32:04PM +0530, Manivannan Sadhasivam wrote: > On Thu, Mar 24, 2022 at 04:48:33AM +0300, Serge Semin wrote: > > Using some abstract number as the DW eDMA chip identifier isn't really > > practical. First of all there can be more than one DW eDMA controller on > > the platform some of them can be detected as the PCIe end-points, some of > > them can be embedded into the DW PCIe Root Port/End-point controllers. > > Seeing some abstract number in for instance IRQ handlers list doesn't give > > a notion regarding their reference to the particular DMA controller. > > Secondly current DW eDMA chip id implementation doesn't provide the > > multi-eDMA platforms support for same reason of possibly having eDMA > > detected on different system buses. At the same time re-implementing > > something ida-based won't give much benefits especially seeing the DW eDMA > > chip ID is only used in the IRQ request procedure. So to speak in order to > > preserve the code simplicity and get to have the multi-eDMA platforms > > support let's just use the parental device name to create the DW eDMA > > controller name. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > drivers/dma/dw-edma/dw-edma-core.c | 3 ++- > > drivers/dma/dw-edma/dw-edma-core.h | 2 +- > > drivers/dma/dw-edma/dw-edma-pcie.c | 1 - > > include/linux/dma/edma.h | 1 - > > 4 files changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c > > index dbe1119fd1d2..72a51970bfba 100644 > > --- a/drivers/dma/dw-edma/dw-edma-core.c > > +++ b/drivers/dma/dw-edma/dw-edma-core.c > > @@ -970,7 +970,8 @@ int dw_edma_probe(struct dw_edma_chip *chip) > > if (!dw->chan) > > return -ENOMEM; > > > > - snprintf(dw->name, sizeof(dw->name), "dw-edma-core:%d", chip->id); > > + snprintf(dw->name, sizeof(dw->name), "dw-edma-core:%s", > > + dev_name(chip->dev)); > > > > /* Disable eDMA, only to establish the ideal initial conditions */ > > dw_edma_v0_core_off(dw); > > diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h > > index 980adb079182..dc25798d4ba9 100644 > > --- a/drivers/dma/dw-edma/dw-edma-core.h > > +++ b/drivers/dma/dw-edma/dw-edma-core.h > > @@ -96,7 +96,7 @@ struct dw_edma_irq { > > }; > > > > struct dw_edma { > > - char name[20]; > > + char name[30]; > > I'm not sure if this length is sufficient. Other than this, > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> My calculations were based on the prefix+pci-device-name length. For instance, for the case of the remote eDMA the name length would be strlen("dw-edma-core:0000:00:00.0") = 25 + 1 (for '\0'). There's even some room left. Seeing the prefix is always used in the string there will be at most 16 chars for the unique part of the name. If you predict it to be greater than that I'll extend the length as you say. -Sergey > > Thanks, > Mani > > > > > struct dma_device dma; > > > > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c > > index f530bacfd716..3f9dadc73854 100644 > > --- a/drivers/dma/dw-edma/dw-edma-pcie.c > > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c > > @@ -222,7 +222,6 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev, > > > > /* Data structure initialization */ > > chip->dev = dev; > > - chip->id = pdev->devfn; > > > > chip->mf = vsec_data.mf; > > chip->nr_irqs = nr_irqs; > > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h > > index 5cc87cfdd685..241c5a97ddf4 100644 > > --- a/include/linux/dma/edma.h > > +++ b/include/linux/dma/edma.h > > @@ -73,7 +73,6 @@ enum dw_edma_map_format { > > */ > > struct dw_edma_chip { > > struct device *dev; > > - int id; > > int nr_irqs; > > const struct dw_edma_core_ops *ops; > > u32 flags; > > -- > > 2.35.1 > >
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c index dbe1119fd1d2..72a51970bfba 100644 --- a/drivers/dma/dw-edma/dw-edma-core.c +++ b/drivers/dma/dw-edma/dw-edma-core.c @@ -970,7 +970,8 @@ int dw_edma_probe(struct dw_edma_chip *chip) if (!dw->chan) return -ENOMEM; - snprintf(dw->name, sizeof(dw->name), "dw-edma-core:%d", chip->id); + snprintf(dw->name, sizeof(dw->name), "dw-edma-core:%s", + dev_name(chip->dev)); /* Disable eDMA, only to establish the ideal initial conditions */ dw_edma_v0_core_off(dw); diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h index 980adb079182..dc25798d4ba9 100644 --- a/drivers/dma/dw-edma/dw-edma-core.h +++ b/drivers/dma/dw-edma/dw-edma-core.h @@ -96,7 +96,7 @@ struct dw_edma_irq { }; struct dw_edma { - char name[20]; + char name[30]; struct dma_device dma; diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c index f530bacfd716..3f9dadc73854 100644 --- a/drivers/dma/dw-edma/dw-edma-pcie.c +++ b/drivers/dma/dw-edma/dw-edma-pcie.c @@ -222,7 +222,6 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev, /* Data structure initialization */ chip->dev = dev; - chip->id = pdev->devfn; chip->mf = vsec_data.mf; chip->nr_irqs = nr_irqs; diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h index 5cc87cfdd685..241c5a97ddf4 100644 --- a/include/linux/dma/edma.h +++ b/include/linux/dma/edma.h @@ -73,7 +73,6 @@ enum dw_edma_map_format { */ struct dw_edma_chip { struct device *dev; - int id; int nr_irqs; const struct dw_edma_core_ops *ops; u32 flags;
Using some abstract number as the DW eDMA chip identifier isn't really practical. First of all there can be more than one DW eDMA controller on the platform some of them can be detected as the PCIe end-points, some of them can be embedded into the DW PCIe Root Port/End-point controllers. Seeing some abstract number in for instance IRQ handlers list doesn't give a notion regarding their reference to the particular DMA controller. Secondly current DW eDMA chip id implementation doesn't provide the multi-eDMA platforms support for same reason of possibly having eDMA detected on different system buses. At the same time re-implementing something ida-based won't give much benefits especially seeing the DW eDMA chip ID is only used in the IRQ request procedure. So to speak in order to preserve the code simplicity and get to have the multi-eDMA platforms support let's just use the parental device name to create the DW eDMA controller name. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> --- drivers/dma/dw-edma/dw-edma-core.c | 3 ++- drivers/dma/dw-edma/dw-edma-core.h | 2 +- drivers/dma/dw-edma/dw-edma-pcie.c | 1 - include/linux/dma/edma.h | 1 - 4 files changed, 3 insertions(+), 4 deletions(-)