| Message ID | 20220303184635.2603-4-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally | expand |
On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote: > Add support for the DMA controller in the DesignWare PCIe core > > The DMA can transfer data to any remote address location > regardless PCI address space size. > > Prepare struct dw_edma_chip() and call dw_edma_probe() > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > > This patch depended on some ep enable patch for imx. This makes it really hard to apply because we don't know *which* patch this depends on. Patches that depend on each other should be posted in the same series, or at the very least, include lore URL to what they depend on. > Change from v1 to v2 > - rework commit message > - align dw_edma_chip change > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index efa8b81711090..7dc55986c947d 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -38,6 +38,7 @@ > #include "../../pci.h" > > #include "pcie-designware.h" > +#include "linux/dma/edma.h" > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17) > @@ -164,6 +165,8 @@ struct imx6_pcie { > const struct imx6_pcie_drvdata *drvdata; > struct regulator *epdev_on; > struct phy *phy; > + > + struct dw_edma_chip dma_chip; > }; > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */ > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = { > .get_features = imx_pcie_ep_get_features, > }; > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + > + return platform_get_irq_byname(pdev, "dma"); > +} > + > +static struct dw_edma_core_ops dma_ops = { > + .irq_vector = imx_dma_irq_vector, > +}; > + > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie) Rename these functions to use "imx6" prefix, as the other functions in the file do. Mentioned this last time, too. Also applies to imx_add_pcie_ep() below; I guess that must be in the other prerequisite patch? > +{ > + unsigned int pcie_dma_offset; > + struct dw_pcie *pci = imx6_pcie->pci; > + struct device *dev = pci->dev; > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip; > + int i = 0; No need to make a variable for this. Just use 0 below, which will be easier to read. > + int sz = PAGE_SIZE; > + > + pcie_dma_offset = 0x970; This would be better as a #define. No point in making a local variable on the stack. > + dma->dev = dev; > + > + dma->reg_base = pci->dbi_base + pcie_dma_offset; > + > + dma->ops = &dma_ops; > + dma->nr_irqs = 1; > + > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP; > + > + dma->ll_wr_cnt = dma->ll_rd_cnt=1; > + dma->ll_region_wr[0].sz = sz; > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz, > + &dma->ll_region_wr[i].paddr, > + GFP_KERNEL); > + > + dma->ll_region_rd[0].sz = sz; > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz, > + &dma->ll_region_rd[i].paddr, > + GFP_KERNEL); Wrap all the above to fit in 80 columns. I would consider something like this to reduce some of the repetition: struct dw_edma_region *region; dma->ll_wr_cnt = 1; region = &dma->ll_region_wr[0]; region->sz = sz; region->vaddr = dmam_alloc_coherent(dev, sz, ®ion->paddr, GFP_KERNEL); dma->ll_rd_cnt = 1; region = &dma->ll_region_rd[0]; region->sz = sz; region->vaddr = dmam_alloc_coherent(dev, sz, ®ion->paddr, GFP_KERNEL); > + > + return dw_edma_probe(dma); > +} > + > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie, > struct platform_device *pdev) > { > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev) > goto err_ret; > } > > + if (imx_add_pcie_dma(imx6_pcie)) > + dev_info(dev, "pci edma probe failure\n"); > + > return 0; > > err_ret: > -- > 2.24.0.rc1 >
On Fri, Mar 4, 2022 at 3:33 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote: > > Add support for the DMA controller in the DesignWare PCIe core > > > > The DMA can transfer data to any remote address location > > regardless PCI address space size. > > > > Prepare struct dw_edma_chip() and call dw_edma_probe() > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > > > This patch depended on some ep enable patch for imx. > > This makes it really hard to apply because we don't know *which* patch > this depends on. Patches that depend on each other should be posted > in the same series, or at the very least, include lore URL to what > they depend on. I did not realize It was missing some EP support patches in pci-imx6.c upstream at v1. Richard is working on this upstream work. This patch don't impact previous patch 1-3 and 5 I will skip this patch next time and wait for richard finish imx6 EP function upstream. Other patches are independent and can be merged separately. With patch 1-3 and 5, other platforms rather than imx that using designware pci ip can be benefited. > > > Change from v1 to v2 > > - rework commit message > > - align dw_edma_chip change > > > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > index efa8b81711090..7dc55986c947d 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -38,6 +38,7 @@ > > #include "../../pci.h" > > > > #include "pcie-designware.h" > > +#include "linux/dma/edma.h" > > > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c > > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17) > > @@ -164,6 +165,8 @@ struct imx6_pcie { > > const struct imx6_pcie_drvdata *drvdata; > > struct regulator *epdev_on; > > struct phy *phy; > > + > > + struct dw_edma_chip dma_chip; > > }; > > > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */ > > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = { > > .get_features = imx_pcie_ep_get_features, > > }; > > > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr) > > +{ > > + struct platform_device *pdev = to_platform_device(dev); > > + > > + return platform_get_irq_byname(pdev, "dma"); > > +} > > + > > +static struct dw_edma_core_ops dma_ops = { > > + .irq_vector = imx_dma_irq_vector, > > +}; > > + > > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie) > > Rename these functions to use "imx6" prefix, as the other functions in > the file do. Mentioned this last time, too. Also applies to > imx_add_pcie_ep() below; I guess that must be in the other > prerequisite patch? > > > +{ > > + unsigned int pcie_dma_offset; > > + struct dw_pcie *pci = imx6_pcie->pci; > > + struct device *dev = pci->dev; > > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip; > > + int i = 0; > > No need to make a variable for this. Just use 0 below, which will be > easier to read. > > > + int sz = PAGE_SIZE; > > + > > + pcie_dma_offset = 0x970; > > This would be better as a #define. No point in making a local > variable on the stack. > > > + dma->dev = dev; > > + > > + dma->reg_base = pci->dbi_base + pcie_dma_offset; > > + > > + dma->ops = &dma_ops; > > + dma->nr_irqs = 1; > > + > > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP; > > + > > + dma->ll_wr_cnt = dma->ll_rd_cnt=1; > > + dma->ll_region_wr[0].sz = sz; > > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz, > > + &dma->ll_region_wr[i].paddr, > > + GFP_KERNEL); > > + > > + dma->ll_region_rd[0].sz = sz; > > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz, > > + &dma->ll_region_rd[i].paddr, > > + GFP_KERNEL); > > Wrap all the above to fit in 80 columns. > > I would consider something like this to reduce some of the repetition: > > struct dw_edma_region *region; > > dma->ll_wr_cnt = 1; > region = &dma->ll_region_wr[0]; > region->sz = sz; > region->vaddr = dmam_alloc_coherent(dev, sz, ®ion->paddr, GFP_KERNEL); > > dma->ll_rd_cnt = 1; > region = &dma->ll_region_rd[0]; > region->sz = sz; > region->vaddr = dmam_alloc_coherent(dev, sz, ®ion->paddr, GFP_KERNEL); > > > + > > + return dw_edma_probe(dma); > > +} > > + > > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie, > > struct platform_device *pdev) > > { > > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev) > > goto err_ret; > > } > > > > + if (imx_add_pcie_dma(imx6_pcie)) > > + dev_info(dev, "pci edma probe failure\n"); > > + > > return 0; > > > > err_ret: > > -- > > 2.24.0.rc1 > >
On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote: > Add support for the DMA controller in the DesignWare PCIe core > > The DMA can transfer data to any remote address location > regardless PCI address space size. > > Prepare struct dw_edma_chip() and call dw_edma_probe() > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > > This patch depended on some ep enable patch for imx. > > Change from v1 to v2 > - rework commit message > - align dw_edma_chip change > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index efa8b81711090..7dc55986c947d 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -38,6 +38,7 @@ > #include "../../pci.h" > > #include "pcie-designware.h" > +#include "linux/dma/edma.h" > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17) > @@ -164,6 +165,8 @@ struct imx6_pcie { > const struct imx6_pcie_drvdata *drvdata; > struct regulator *epdev_on; > struct phy *phy; > + > + struct dw_edma_chip dma_chip; > }; > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */ > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = { > .get_features = imx_pcie_ep_get_features, > }; > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + > + return platform_get_irq_byname(pdev, "dma"); > +} > + > +static struct dw_edma_core_ops dma_ops = { > + .irq_vector = imx_dma_irq_vector, > +}; > + > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie) > +{ > + unsigned int pcie_dma_offset; > + struct dw_pcie *pci = imx6_pcie->pci; > + struct device *dev = pci->dev; > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip; > + int i = 0; Unused? > + int sz = PAGE_SIZE; > + > + pcie_dma_offset = 0x970; Can you get this offset from the devicetree node of ep? > + > + dma->dev = dev; > + > + dma->reg_base = pci->dbi_base + pcie_dma_offset; > + > + dma->ops = &dma_ops; > + dma->nr_irqs = 1; > + > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP; > + > + dma->ll_wr_cnt = dma->ll_rd_cnt=1; Is this a hard limitation of the eDMA implementation or because of difficulties in requesting the correct channel from client driver? If it's the latter, you could use my patch: https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810 > + dma->ll_region_wr[0].sz = sz; > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz, > + &dma->ll_region_wr[i].paddr, > + GFP_KERNEL); Allocation could fail. Please add error checking here and below. Thanks, Mani > + > + dma->ll_region_rd[0].sz = sz; > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz, > + &dma->ll_region_rd[i].paddr, > + GFP_KERNEL); > + > + return dw_edma_probe(dma); > +} > + > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie, > struct platform_device *pdev) > { > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev) > goto err_ret; > } > > + if (imx_add_pcie_dma(imx6_pcie)) > + dev_info(dev, "pci edma probe failure\n"); > + > return 0; > > err_ret: > -- > 2.24.0.rc1 >
On Wed, Mar 9, 2022 at 6:02 AM Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > > On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote: > > Add support for the DMA controller in the DesignWare PCIe core > > > > The DMA can transfer data to any remote address location > > regardless PCI address space size. > > > > Prepare struct dw_edma_chip() and call dw_edma_probe() > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > > > This patch depended on some ep enable patch for imx. > > > > Change from v1 to v2 > > - rework commit message > > - align dw_edma_chip change > > > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > index efa8b81711090..7dc55986c947d 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -38,6 +38,7 @@ > > #include "../../pci.h" > > > > #include "pcie-designware.h" > > +#include "linux/dma/edma.h" > > > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c > > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17) > > @@ -164,6 +165,8 @@ struct imx6_pcie { > > const struct imx6_pcie_drvdata *drvdata; > > struct regulator *epdev_on; > > struct phy *phy; > > + > > + struct dw_edma_chip dma_chip; > > }; > > > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */ > > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = { > > .get_features = imx_pcie_ep_get_features, > > }; > > > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr) > > +{ > > + struct platform_device *pdev = to_platform_device(dev); > > + > > + return platform_get_irq_byname(pdev, "dma"); > > +} > > + > > +static struct dw_edma_core_ops dma_ops = { > > + .irq_vector = imx_dma_irq_vector, > > +}; > > + > > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie) > > +{ > > + unsigned int pcie_dma_offset; > > + struct dw_pcie *pci = imx6_pcie->pci; > > + struct device *dev = pci->dev; > > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip; > > + int i = 0; > > Unused? > > > + int sz = PAGE_SIZE; > > + > > + pcie_dma_offset = 0x970; > > Can you get this offset from the devicetree node of ep? > > > + > > + dma->dev = dev; > > + > > + dma->reg_base = pci->dbi_base + pcie_dma_offset; > > + > > + dma->ops = &dma_ops; > > + dma->nr_irqs = 1; > > + > > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP; > > + > > + dma->ll_wr_cnt = dma->ll_rd_cnt=1; > > Is this a hard limitation of the eDMA implementation or because of difficulties > in requesting the correct channel from client driver? > > If it's the latter, you could use my patch: It is because our hardware only has 1 channel. > > https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810 My problem is in dw_edma_channel_setup() dma->directions = BIT(write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV); Already set direction. why need overwrite default device_caps? > > > + dma->ll_region_wr[0].sz = sz; > > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz, > > + &dma->ll_region_wr[i].paddr, > > + GFP_KERNEL); > > Allocation could fail. Please add error checking here and below. > > Thanks, > Mani > > > + > > + dma->ll_region_rd[0].sz = sz; > > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz, > > + &dma->ll_region_rd[i].paddr, > > + GFP_KERNEL); > > + > > + return dw_edma_probe(dma); > > +} > > + > > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie, > > struct platform_device *pdev) > > { > > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev) > > goto err_ret; > > } > > > > + if (imx_add_pcie_dma(imx6_pcie)) > > + dev_info(dev, "pci edma probe failure\n"); > > + > > return 0; > > > > err_ret: > > -- > > 2.24.0.rc1 > >
On Wed, Mar 09, 2022 at 12:31:57PM -0600, Zhi Li wrote: > On Wed, Mar 9, 2022 at 6:02 AM Manivannan Sadhasivam > <manivannan.sadhasivam@linaro.org> wrote: > > > > On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote: > > > Add support for the DMA controller in the DesignWare PCIe core > > > > > > The DMA can transfer data to any remote address location > > > regardless PCI address space size. > > > > > > Prepare struct dw_edma_chip() and call dw_edma_probe() > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > --- > > > > > > This patch depended on some ep enable patch for imx. > > > > > > Change from v1 to v2 > > > - rework commit message > > > - align dw_edma_chip change > > > > > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++ > > > 1 file changed, 51 insertions(+) > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > > index efa8b81711090..7dc55986c947d 100644 > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > @@ -38,6 +38,7 @@ > > > #include "../../pci.h" > > > > > > #include "pcie-designware.h" > > > +#include "linux/dma/edma.h" > > > > > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c > > > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17) > > > @@ -164,6 +165,8 @@ struct imx6_pcie { > > > const struct imx6_pcie_drvdata *drvdata; > > > struct regulator *epdev_on; > > > struct phy *phy; > > > + > > > + struct dw_edma_chip dma_chip; > > > }; > > > > > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */ > > > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = { > > > .get_features = imx_pcie_ep_get_features, > > > }; > > > > > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr) > > > +{ > > > + struct platform_device *pdev = to_platform_device(dev); > > > + > > > + return platform_get_irq_byname(pdev, "dma"); > > > +} > > > + > > > +static struct dw_edma_core_ops dma_ops = { > > > + .irq_vector = imx_dma_irq_vector, > > > +}; > > > + > > > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie) > > > +{ > > > + unsigned int pcie_dma_offset; > > > + struct dw_pcie *pci = imx6_pcie->pci; > > > + struct device *dev = pci->dev; > > > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip; > > > + int i = 0; > > > > Unused? > > > > > + int sz = PAGE_SIZE; > > > + > > > + pcie_dma_offset = 0x970; > > > > Can you get this offset from the devicetree node of ep? > > > > > + > > > + dma->dev = dev; > > > + > > > + dma->reg_base = pci->dbi_base + pcie_dma_offset; > > > + > > > + dma->ops = &dma_ops; > > > + dma->nr_irqs = 1; > > > + > > > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP; > > > + > > > + dma->ll_wr_cnt = dma->ll_rd_cnt=1; > > > > Is this a hard limitation of the eDMA implementation or because of difficulties > > in requesting the correct channel from client driver? > > > > If it's the latter, you could use my patch: > > It is because our hardware only has 1 channel. Ah okay. It is fine if that's the case. > > > > > https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810 > > My problem is > in dw_edma_channel_setup() > dma->directions = BIT(write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV); > > Already set direction. why need overwrite default device_caps? > The above sets both direction in the caps. That's fine if you want to verify whether the channel is valid or not. But that won't help you want to choose the correct channel. In your case it will work because, read and write channel count is 1. But what if the channel count is 8? write channel - 0 to 7 read channel - 8 to 15 Now without identifying the channel direction, the client would be getting the wrong one. For instance, if client requests read channel after write, the dmaengine would pass 1 because the requested direction matches the caps and that's wrong. Thanks, Mani > > > > > + dma->ll_region_wr[0].sz = sz; > > > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz, > > > + &dma->ll_region_wr[i].paddr, > > > + GFP_KERNEL); > > > > Allocation could fail. Please add error checking here and below. > > > > Thanks, > > Mani > > > > > + > > > + dma->ll_region_rd[0].sz = sz; > > > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz, > > > + &dma->ll_region_rd[i].paddr, > > > + GFP_KERNEL); > > > + > > > + return dw_edma_probe(dma); > > > +} > > > + > > > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie, > > > struct platform_device *pdev) > > > { > > > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev) > > > goto err_ret; > > > } > > > > > > + if (imx_add_pcie_dma(imx6_pcie)) > > > + dev_info(dev, "pci edma probe failure\n"); > > > + > > > return 0; > > > > > > err_ret: > > > -- > > > 2.24.0.rc1 > > >
On Wed, Mar 9, 2022 at 12:42 PM Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > > On Wed, Mar 09, 2022 at 12:31:57PM -0600, Zhi Li wrote: > > On Wed, Mar 9, 2022 at 6:02 AM Manivannan Sadhasivam > > <manivannan.sadhasivam@linaro.org> wrote: > > > > > > On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote: > > > > Add support for the DMA controller in the DesignWare PCIe core > > > > > > > > The DMA can transfer data to any remote address location > > > > regardless PCI address space size. > > > > > > > > Prepare struct dw_edma_chip() and call dw_edma_probe() > > > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > --- > > > > > > > > This patch depended on some ep enable patch for imx. > > > > > > > > Change from v1 to v2 > > > > - rework commit message > > > > - align dw_edma_chip change > > > > > > > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++ > > > > 1 file changed, 51 insertions(+) > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > > > index efa8b81711090..7dc55986c947d 100644 > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > > @@ -38,6 +38,7 @@ > > > > #include "../../pci.h" > > > > > > > > #include "pcie-designware.h" > > > > +#include "linux/dma/edma.h" > > > > > > > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c > > > > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17) > > > > @@ -164,6 +165,8 @@ struct imx6_pcie { > > > > const struct imx6_pcie_drvdata *drvdata; > > > > struct regulator *epdev_on; > > > > struct phy *phy; > > > > + > > > > + struct dw_edma_chip dma_chip; > > > > }; > > > > > > > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */ > > > > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = { > > > > .get_features = imx_pcie_ep_get_features, > > > > }; > > > > > > > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr) > > > > +{ > > > > + struct platform_device *pdev = to_platform_device(dev); > > > > + > > > > + return platform_get_irq_byname(pdev, "dma"); > > > > +} > > > > + > > > > +static struct dw_edma_core_ops dma_ops = { > > > > + .irq_vector = imx_dma_irq_vector, > > > > +}; > > > > + > > > > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie) > > > > +{ > > > > + unsigned int pcie_dma_offset; > > > > + struct dw_pcie *pci = imx6_pcie->pci; > > > > + struct device *dev = pci->dev; > > > > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip; > > > > + int i = 0; > > > > > > Unused? > > > > > > > + int sz = PAGE_SIZE; > > > > + > > > > + pcie_dma_offset = 0x970; > > > > > > Can you get this offset from the devicetree node of ep? > > > > > > > + > > > > + dma->dev = dev; > > > > + > > > > + dma->reg_base = pci->dbi_base + pcie_dma_offset; > > > > + > > > > + dma->ops = &dma_ops; > > > > + dma->nr_irqs = 1; > > > > + > > > > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP; > > > > + > > > > + dma->ll_wr_cnt = dma->ll_rd_cnt=1; > > > > > > Is this a hard limitation of the eDMA implementation or because of difficulties > > > in requesting the correct channel from client driver? > > > > > > If it's the latter, you could use my patch: > > > > It is because our hardware only has 1 channel. > > Ah okay. It is fine if that's the case. > > > > > > > > > https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810 > > > > My problem is > > in dw_edma_channel_setup() > > dma->directions = BIT(write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV); > > > > Already set direction. why need overwrite default device_caps? > > > > The above sets both direction in the caps. Why set both direction? write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV Only one bit set > That's fine if you want to verify > whether the channel is valid or not. But that won't help you want to choose the > correct channel. > > In your case it will work because, read and write channel count is 1. But what > if the channel count is 8? I know my case is special one. I just feel strange. dma_async_device_register(dma); register two devices, one read, one write. The default int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps) { device = chan->device; caps->directions = device->directions; } It should return read/write devices's directions. > > write channel - 0 to 7 > read channel - 8 to 15 > > Now without identifying the channel direction, the client would be getting the > wrong one. For instance, if client requests read channel after write, the > dmaengine would pass 1 because the requested direction matches the caps and > that's wrong. Do you mean if I request a read after write can reproduce the problem? > > Thanks, > Mani > > > > > > > > + dma->ll_region_wr[0].sz = sz; > > > > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz, > > > > + &dma->ll_region_wr[i].paddr, > > > > + GFP_KERNEL); > > > > > > Allocation could fail. Please add error checking here and below. > > > > > > Thanks, > > > Mani > > > > > > > + > > > > + dma->ll_region_rd[0].sz = sz; > > > > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz, > > > > + &dma->ll_region_rd[i].paddr, > > > > + GFP_KERNEL); > > > > + > > > > + return dw_edma_probe(dma); > > > > +} > > > > + > > > > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie, > > > > struct platform_device *pdev) > > > > { > > > > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev) > > > > goto err_ret; > > > > } > > > > > > > > + if (imx_add_pcie_dma(imx6_pcie)) > > > > + dev_info(dev, "pci edma probe failure\n"); > > > > + > > > > return 0; > > > > > > > > err_ret: > > > > -- > > > > 2.24.0.rc1 > > > >
On Wed, Mar 9, 2022 at 1:14 PM Zhi Li <lznuaa@gmail.com> wrote: > > On Wed, Mar 9, 2022 at 12:42 PM Manivannan Sadhasivam > <manivannan.sadhasivam@linaro.org> wrote: > > > > On Wed, Mar 09, 2022 at 12:31:57PM -0600, Zhi Li wrote: > > > On Wed, Mar 9, 2022 at 6:02 AM Manivannan Sadhasivam > > > <manivannan.sadhasivam@linaro.org> wrote: > > > > > > > > On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote: > > > > > Add support for the DMA controller in the DesignWare PCIe core > > > > > > > > > > The DMA can transfer data to any remote address location > > > > > regardless PCI address space size. > > > > > > > > > > Prepare struct dw_edma_chip() and call dw_edma_probe() > > > > > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > > --- > > > > > > > > > > This patch depended on some ep enable patch for imx. > > > > > > > > > > Change from v1 to v2 > > > > > - rework commit message > > > > > - align dw_edma_chip change > > > > > > > > > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++ > > > > > 1 file changed, 51 insertions(+) > > > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > > > > index efa8b81711090..7dc55986c947d 100644 > > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > > > @@ -38,6 +38,7 @@ > > > > > #include "../../pci.h" > > > > > > > > > > #include "pcie-designware.h" > > > > > +#include "linux/dma/edma.h" > > > > > > > > > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c > > > > > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17) > > > > > @@ -164,6 +165,8 @@ struct imx6_pcie { > > > > > const struct imx6_pcie_drvdata *drvdata; > > > > > struct regulator *epdev_on; > > > > > struct phy *phy; > > > > > + > > > > > + struct dw_edma_chip dma_chip; > > > > > }; > > > > > > > > > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */ > > > > > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = { > > > > > .get_features = imx_pcie_ep_get_features, > > > > > }; > > > > > > > > > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr) > > > > > +{ > > > > > + struct platform_device *pdev = to_platform_device(dev); > > > > > + > > > > > + return platform_get_irq_byname(pdev, "dma"); > > > > > +} > > > > > + > > > > > +static struct dw_edma_core_ops dma_ops = { > > > > > + .irq_vector = imx_dma_irq_vector, > > > > > +}; > > > > > + > > > > > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie) > > > > > +{ > > > > > + unsigned int pcie_dma_offset; > > > > > + struct dw_pcie *pci = imx6_pcie->pci; > > > > > + struct device *dev = pci->dev; > > > > > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip; > > > > > + int i = 0; > > > > > > > > Unused? > > > > > > > > > + int sz = PAGE_SIZE; > > > > > + > > > > > + pcie_dma_offset = 0x970; > > > > > > > > Can you get this offset from the devicetree node of ep? > > > > > > > > > + > > > > > + dma->dev = dev; > > > > > + > > > > > + dma->reg_base = pci->dbi_base + pcie_dma_offset; > > > > > + > > > > > + dma->ops = &dma_ops; > > > > > + dma->nr_irqs = 1; > > > > > + > > > > > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP; > > > > > + > > > > > + dma->ll_wr_cnt = dma->ll_rd_cnt=1; > > > > > > > > Is this a hard limitation of the eDMA implementation or because of difficulties > > > > in requesting the correct channel from client driver? > > > > > > > > If it's the latter, you could use my patch: > > > > > > It is because our hardware only has 1 channel. > > > > Ah okay. It is fine if that's the case. > > > > > > > > > > > > > https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810 > > > > > > My problem is > > > in dw_edma_channel_setup() > > > dma->directions = BIT(write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV); > > > > > > Already set direction. why need overwrite default device_caps? > > > > > > > The above sets both direction in the caps. > > Why set both direction? > write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV > Only one bit set > > > That's fine if you want to verify > > whether the channel is valid or not. But that won't help you want to choose the > > correct channel. > > > > In your case it will work because, read and write channel count is 1. But what > > if the channel count is 8? > > I know my case is special one. I just feel strange. > > dma_async_device_register(dma); register two devices, one read, one write. > > The default int dma_get_slave_caps(struct dma_chan *chan, struct > dma_slave_caps *caps) > { > device = chan->device; > caps->directions = device->directions; > } > > It should return read/write devices's directions. > > > > > write channel - 0 to 7 > > read channel - 8 to 15 > > > > Now without identifying the channel direction, the client would be getting the > > wrong one. For instance, if client requests read channel after write, the > > dmaengine would pass 1 because the requested direction matches the caps and > > that's wrong. > > Do you mean if I request a read after write can reproduce the problem? After I force channel number to 8, diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index f26afd02f3a86..04ec644ecde5a 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -180,7 +180,7 @@ static bool epf_dma_filter_fn(struct dma_chan *chan, void *node) memset(&caps, 0, sizeof(caps)); dma_get_slave_caps(chan, &caps); - +pr_err("dma cap: 0x%llx\n", caps.directions); return chan->device->dev == filter->dev && (filter->dma_mask & caps.directions); } [ 17.715167] dma cap: 0x6 ( other DMA engine) [ 17.717707] dma cap: 0x6 ( other DMA engine) [ 17.720299] dma cap: 0x6 ( other DMA engine) [ 17.722893] dma cap: 0x6 ( other DMA engine) [ 17.725437] dma cap: 0x6 ( other DMA engine) [ 17.728040] dma cap: 0x6 ( other DMA engine) [ 17.730652] dma cap: 0x6 ( other DMA engine) [ 17.733197] dma cap: 0x4 (EDMA W) [ 17.735762] dma cap: 0x4 (EDMA W) [ 17.738390] dma cap: 0x4 ..... [ 17.740937] dma cap: 0x4 [ 17.743518] dma cap: 0x4 [ 17.746108] dma cap: 0x4 [ 17.748651] dma cap: 0x4 [ 17.751219] dma cap: 0x2 (EDMA WR) I get the correct DMA channel without your patch. > > Mani > > > > > > > > > > > + dma->ll_region_wr[0].sz = sz; > > > > > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz, > > > > > + &dma->ll_region_wr[i].paddr, > > > > > + GFP_KERNEL); > > > > > > > > Allocation could fail. Please add error checking here and below. > > > > > > > > Thanks, > > > > Mani > > > > > > > > > + > > > > > + dma->ll_region_rd[0].sz = sz; > > > > > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz, > > > > > + &dma->ll_region_rd[i].paddr, > > > > > + GFP_KERNEL); > > > > > + > > > > > + return dw_edma_probe(dma); > > > > > +} > > > > > + > > > > > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie, > > > > > struct platform_device *pdev) > > > > > { > > > > > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev) > > > > > goto err_ret; > > > > > } > > > > > > > > > > + if (imx_add_pcie_dma(imx6_pcie)) > > > > > + dev_info(dev, "pci edma probe failure\n"); > > > > > + > > > > > return 0; > > > > > > > > > > err_ret: > > > > > -- > > > > > 2.24.0.rc1 > > > > >
On Wed, Mar 09, 2022 at 01:14:33PM -0600, Zhi Li wrote: > On Wed, Mar 9, 2022 at 12:42 PM Manivannan Sadhasivam > <manivannan.sadhasivam@linaro.org> wrote: > > > > On Wed, Mar 09, 2022 at 12:31:57PM -0600, Zhi Li wrote: > > > On Wed, Mar 9, 2022 at 6:02 AM Manivannan Sadhasivam > > > <manivannan.sadhasivam@linaro.org> wrote: > > > > > > > > On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote: > > > > > Add support for the DMA controller in the DesignWare PCIe core > > > > > > > > > > The DMA can transfer data to any remote address location > > > > > regardless PCI address space size. > > > > > > > > > > Prepare struct dw_edma_chip() and call dw_edma_probe() > > > > > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > > --- > > > > > > > > > > This patch depended on some ep enable patch for imx. > > > > > > > > > > Change from v1 to v2 > > > > > - rework commit message > > > > > - align dw_edma_chip change > > > > > > > > > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++ > > > > > 1 file changed, 51 insertions(+) > > > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > > > > index efa8b81711090..7dc55986c947d 100644 > > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > > > @@ -38,6 +38,7 @@ > > > > > #include "../../pci.h" > > > > > > > > > > #include "pcie-designware.h" > > > > > +#include "linux/dma/edma.h" > > > > > > > > > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c > > > > > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17) > > > > > @@ -164,6 +165,8 @@ struct imx6_pcie { > > > > > const struct imx6_pcie_drvdata *drvdata; > > > > > struct regulator *epdev_on; > > > > > struct phy *phy; > > > > > + > > > > > + struct dw_edma_chip dma_chip; > > > > > }; > > > > > > > > > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */ > > > > > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = { > > > > > .get_features = imx_pcie_ep_get_features, > > > > > }; > > > > > > > > > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr) > > > > > +{ > > > > > + struct platform_device *pdev = to_platform_device(dev); > > > > > + > > > > > + return platform_get_irq_byname(pdev, "dma"); > > > > > +} > > > > > + > > > > > +static struct dw_edma_core_ops dma_ops = { > > > > > + .irq_vector = imx_dma_irq_vector, > > > > > +}; > > > > > + > > > > > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie) > > > > > +{ > > > > > + unsigned int pcie_dma_offset; > > > > > + struct dw_pcie *pci = imx6_pcie->pci; > > > > > + struct device *dev = pci->dev; > > > > > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip; > > > > > + int i = 0; > > > > > > > > Unused? > > > > > > > > > + int sz = PAGE_SIZE; > > > > > + > > > > > + pcie_dma_offset = 0x970; > > > > > > > > Can you get this offset from the devicetree node of ep? > > > > > > > > > + > > > > > + dma->dev = dev; > > > > > + > > > > > + dma->reg_base = pci->dbi_base + pcie_dma_offset; > > > > > + > > > > > + dma->ops = &dma_ops; > > > > > + dma->nr_irqs = 1; > > > > > + > > > > > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP; > > > > > + > > > > > + dma->ll_wr_cnt = dma->ll_rd_cnt=1; > > > > > > > > Is this a hard limitation of the eDMA implementation or because of difficulties > > > > in requesting the correct channel from client driver? > > > > > > > > If it's the latter, you could use my patch: > > > > > > It is because our hardware only has 1 channel. > > > > Ah okay. It is fine if that's the case. > > > > > > > > > > > > > https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810 > > > > > > My problem is > > > in dw_edma_channel_setup() > > > dma->directions = BIT(write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV); > > > > > > Already set direction. why need overwrite default device_caps? > > > > > > > The above sets both direction in the caps. > > Why set both direction? > write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV > Only one bit set > > > That's fine if you want to verify > > whether the channel is valid or not. But that won't help you want to choose the > > correct channel. > > > > In your case it will work because, read and write channel count is 1. But what > > if the channel count is 8? > > I know my case is special one. I just feel strange. > > dma_async_device_register(dma); register two devices, one read, one write. > > The default int dma_get_slave_caps(struct dma_chan *chan, struct > dma_slave_caps *caps) > { > device = chan->device; > caps->directions = device->directions; > } > > It should return read/write devices's directions. > Urgh... In our kernel tree, my colleague merged two dma devices into one and I failed to notice that :/ So yeah, it should work by default. Sorry for the noise. Thanks, Mani > > > > write channel - 0 to 7 > > read channel - 8 to 15 > > > > Now without identifying the channel direction, the client would be getting the > > wrong one. For instance, if client requests read channel after write, the > > dmaengine would pass 1 because the requested direction matches the caps and > > that's wrong. > > Do you mean if I request a read after write can reproduce the problem? > > > > > > Thanks, > > Mani > > > > > > > > > > > + dma->ll_region_wr[0].sz = sz; > > > > > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz, > > > > > + &dma->ll_region_wr[i].paddr, > > > > > + GFP_KERNEL); > > > > > > > > Allocation could fail. Please add error checking here and below. > > > > > > > > Thanks, > > > > Mani > > > > > > > > > + > > > > > + dma->ll_region_rd[0].sz = sz; > > > > > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz, > > > > > + &dma->ll_region_rd[i].paddr, > > > > > + GFP_KERNEL); > > > > > + > > > > > + return dw_edma_probe(dma); > > > > > +} > > > > > + > > > > > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie, > > > > > struct platform_device *pdev) > > > > > { > > > > > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev) > > > > > goto err_ret; > > > > > } > > > > > > > > > > + if (imx_add_pcie_dma(imx6_pcie)) > > > > > + dev_info(dev, "pci edma probe failure\n"); > > > > > + > > > > > return 0; > > > > > > > > > > err_ret: > > > > > -- > > > > > 2.24.0.rc1 > > > > >
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index efa8b81711090..7dc55986c947d 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -38,6 +38,7 @@ #include "../../pci.h" #include "pcie-designware.h" +#include "linux/dma/edma.h" #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17) @@ -164,6 +165,8 @@ struct imx6_pcie { const struct imx6_pcie_drvdata *drvdata; struct regulator *epdev_on; struct phy *phy; + + struct dw_edma_chip dma_chip; }; /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */ @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = { .get_features = imx_pcie_ep_get_features, }; +static int imx_dma_irq_vector(struct device *dev, unsigned int nr) +{ + struct platform_device *pdev = to_platform_device(dev); + + return platform_get_irq_byname(pdev, "dma"); +} + +static struct dw_edma_core_ops dma_ops = { + .irq_vector = imx_dma_irq_vector, +}; + +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie) +{ + unsigned int pcie_dma_offset; + struct dw_pcie *pci = imx6_pcie->pci; + struct device *dev = pci->dev; + struct dw_edma_chip *dma = &imx6_pcie->dma_chip; + int i = 0; + int sz = PAGE_SIZE; + + pcie_dma_offset = 0x970; + + dma->dev = dev; + + dma->reg_base = pci->dbi_base + pcie_dma_offset; + + dma->ops = &dma_ops; + dma->nr_irqs = 1; + + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP; + + dma->ll_wr_cnt = dma->ll_rd_cnt=1; + dma->ll_region_wr[0].sz = sz; + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz, + &dma->ll_region_wr[i].paddr, + GFP_KERNEL); + + dma->ll_region_rd[0].sz = sz; + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz, + &dma->ll_region_rd[i].paddr, + GFP_KERNEL); + + return dw_edma_probe(dma); +} + static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie, struct platform_device *pdev) { @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev) goto err_ret; } + if (imx_add_pcie_dma(imx6_pcie)) + dev_info(dev, "pci edma probe failure\n"); + return 0; err_ret:
Add support for the DMA controller in the DesignWare PCIe core The DMA can transfer data to any remote address location regardless PCI address space size. Prepare struct dw_edma_chip() and call dw_edma_probe() Signed-off-by: Frank Li <Frank.Li@nxp.com> --- This patch depended on some ep enable patch for imx. Change from v1 to v2 - rework commit message - align dw_edma_chip change drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)