From: Alan Mikhak <[email protected]>
Decouple dw-edma-core.c from struct pci_dev as a step toward integration
of dw-edma with pci-epf-test so the latter can initiate dma operations
locally from the endpoint side. A barrier to such integration is the
dependency of dw_edma_probe() and other functions in dw-edma-core.c on
struct pci_dev.
The Synopsys DesignWare dw-edma driver was designed to run on host side
of PCIe link to initiate DMA operations remotely using eDMA channels of
PCIe controller on the endpoint side. This can be inferred from seeing
that dw-edma uses struct pci_dev and accesses hardware registers of dma
channels across the bus using BAR0 and BAR2.
The ops field of struct dw_edma in dw-edma-core.h is currenty undefined:
const struct dw_edma_core_ops *ops;
However, the kernel builds without failure even when dw-edma driver is
enabled. Instead of removing the currently undefined and usued ops field,
define struct dw_edma_core_ops and use the ops field to decouple
dw-edma-core.c from struct pci_dev.
Signed-off-by: Alan Mikhak <[email protected]>
---
drivers/dma/dw-edma/dw-edma-core.c | 29 ++++++++++++++++++++---------
drivers/dma/dw-edma/dw-edma-core.h | 4 ++++
drivers/dma/dw-edma/dw-edma-pcie.c | 10 ++++++++++
3 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index ff392c01bad1..db401eb11322 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -14,7 +14,7 @@
#include <linux/err.h>
#include <linux/interrupt.h>
#include <linux/dma/edma.h>
-#include <linux/pci.h>
+#include <linux/dma-mapping.h>
#include "dw-edma-core.h"
#include "dw-edma-v0-core.h"
@@ -781,7 +781,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
if (dw->nr_irqs == 1) {
/* Common IRQ shared among all channels */
- err = request_irq(pci_irq_vector(to_pci_dev(dev), 0),
+ err = request_irq(dw->ops->irq_vector(dev, 0),
dw_edma_interrupt_common,
IRQF_SHARED, dw->name, &dw->irq[0]);
if (err) {
@@ -789,7 +789,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
return err;
}
- get_cached_msi_msg(pci_irq_vector(to_pci_dev(dev), 0),
+ get_cached_msi_msg(dw->ops->irq_vector(dev, 0),
&dw->irq[0].msi);
} else {
/* Distribute IRQs equally among all channels */
@@ -804,7 +804,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
dw_edma_add_irq_mask(&rd_mask, *rd_alloc, dw->rd_ch_cnt);
for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
- err = request_irq(pci_irq_vector(to_pci_dev(dev), i),
+ err = request_irq(dw->ops->irq_vector(dev, i),
i < *wr_alloc ?
dw_edma_interrupt_write :
dw_edma_interrupt_read,
@@ -815,7 +815,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
return err;
}
- get_cached_msi_msg(pci_irq_vector(to_pci_dev(dev), i),
+ get_cached_msi_msg(dw->ops->irq_vector(dev, i),
&dw->irq[i].msi);
}
@@ -827,12 +827,23 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
int dw_edma_probe(struct dw_edma_chip *chip)
{
- struct device *dev = chip->dev;
- struct dw_edma *dw = chip->dw;
+ struct device *dev;
+ struct dw_edma *dw;
u32 wr_alloc = 0;
u32 rd_alloc = 0;
int i, err;
+ if (!chip)
+ return -EINVAL;
+
+ dev = chip->dev;
+ if (!dev)
+ return -EINVAL;
+
+ dw = chip->dw;
+ if (!dw || !dw->irq || !dw->ops || !dw->ops->irq_vector)
+ return -EINVAL;
+
raw_spin_lock_init(&dw->lock);
/* Find out how many write channels are supported by hardware */
@@ -884,7 +895,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
err_irq_free:
for (i = (dw->nr_irqs - 1); i >= 0; i--)
- free_irq(pci_irq_vector(to_pci_dev(dev), i), &dw->irq[i]);
+ free_irq(dw->ops->irq_vector(dev, i), &dw->irq[i]);
dw->nr_irqs = 0;
@@ -904,7 +915,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
/* Free irqs */
for (i = (dw->nr_irqs - 1); i >= 0; i--)
- free_irq(pci_irq_vector(to_pci_dev(dev), i), &dw->irq[i]);
+ free_irq(dw->ops->irq_vector(dev, i), &dw->irq[i]);
/* Power management */
pm_runtime_disable(dev);
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 4e5f9f6e901b..31fc50d31792 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -103,6 +103,10 @@ struct dw_edma_irq {
struct dw_edma *dw;
};
+struct dw_edma_core_ops {
+ int (*irq_vector)(struct device *dev, unsigned int nr);
+};
+
struct dw_edma {
char name[20];
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index dc85f55e1bb8..1eafc602e17e 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -54,6 +54,15 @@ static const struct dw_edma_pcie_data snps_edda_data = {
.irqs = 1,
};
+static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr)
+{
+ return pci_irq_vector(to_pci_dev(dev), nr);
+}
+
+static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
+ .irq_vector = dw_edma_pcie_irq_vector,
+};
+
static int dw_edma_pcie_probe(struct pci_dev *pdev,
const struct pci_device_id *pid)
{
@@ -151,6 +160,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
dw->version = pdata->version;
dw->mode = pdata->mode;
dw->nr_irqs = nr_irqs;
+ dw->ops = &dw_edma_pcie_core_ops;
/* Debug info */
pci_dbg(pdev, "Version:\t%u\n", dw->version);
--
2.7.4
On Wed, Apr 15, 2020 at 18:27:9, Alan Mikhak <[email protected]>
wrote:
> From: Alan Mikhak <[email protected]>
>
> Decouple dw-edma-core.c from struct pci_dev as a step toward integration
> of dw-edma with pci-epf-test so the latter can initiate dma operations
> locally from the endpoint side. A barrier to such integration is the
> dependency of dw_edma_probe() and other functions in dw-edma-core.c on
> struct pci_dev.
>
> The Synopsys DesignWare dw-edma driver was designed to run on host side
> of PCIe link to initiate DMA operations remotely using eDMA channels of
> PCIe controller on the endpoint side. This can be inferred from seeing
> that dw-edma uses struct pci_dev and accesses hardware registers of dma
> channels across the bus using BAR0 and BAR2.
>
> The ops field of struct dw_edma in dw-edma-core.h is currenty undefined:
>
> const struct dw_edma_core_ops *ops;
>
> However, the kernel builds without failure even when dw-edma driver is
> enabled. Instead of removing the currently undefined and usued ops field,
> define struct dw_edma_core_ops and use the ops field to decouple
> dw-edma-core.c from struct pci_dev.
>
> Signed-off-by: Alan Mikhak <[email protected]>
> ---
> drivers/dma/dw-edma/dw-edma-core.c | 29 ++++++++++++++++++++---------
> drivers/dma/dw-edma/dw-edma-core.h | 4 ++++
> drivers/dma/dw-edma/dw-edma-pcie.c | 10 ++++++++++
> 3 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index ff392c01bad1..db401eb11322 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -14,7 +14,7 @@
> #include <linux/err.h>
> #include <linux/interrupt.h>
> #include <linux/dma/edma.h>
> -#include <linux/pci.h>
> +#include <linux/dma-mapping.h>
>
> #include "dw-edma-core.h"
> #include "dw-edma-v0-core.h"
> @@ -781,7 +781,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
>
> if (dw->nr_irqs == 1) {
> /* Common IRQ shared among all channels */
> - err = request_irq(pci_irq_vector(to_pci_dev(dev), 0),
> + err = request_irq(dw->ops->irq_vector(dev, 0),
> dw_edma_interrupt_common,
> IRQF_SHARED, dw->name, &dw->irq[0]);
> if (err) {
> @@ -789,7 +789,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> return err;
> }
>
> - get_cached_msi_msg(pci_irq_vector(to_pci_dev(dev), 0),
> + get_cached_msi_msg(dw->ops->irq_vector(dev, 0),
> &dw->irq[0].msi);
> } else {
> /* Distribute IRQs equally among all channels */
> @@ -804,7 +804,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> dw_edma_add_irq_mask(&rd_mask, *rd_alloc, dw->rd_ch_cnt);
>
> for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
> - err = request_irq(pci_irq_vector(to_pci_dev(dev), i),
> + err = request_irq(dw->ops->irq_vector(dev, i),
> i < *wr_alloc ?
> dw_edma_interrupt_write :
> dw_edma_interrupt_read,
> @@ -815,7 +815,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> return err;
> }
>
> - get_cached_msi_msg(pci_irq_vector(to_pci_dev(dev), i),
> + get_cached_msi_msg(dw->ops->irq_vector(dev, i),
> &dw->irq[i].msi);
> }
>
> @@ -827,12 +827,23 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
>
> int dw_edma_probe(struct dw_edma_chip *chip)
> {
> - struct device *dev = chip->dev;
> - struct dw_edma *dw = chip->dw;
> + struct device *dev;
> + struct dw_edma *dw;
> u32 wr_alloc = 0;
> u32 rd_alloc = 0;
> int i, err;
>
> + if (!chip)
> + return -EINVAL;
> +
> + dev = chip->dev;
> + if (!dev)
> + return -EINVAL;
> +
> + dw = chip->dw;
> + if (!dw || !dw->irq || !dw->ops || !dw->ops->irq_vector)
> + return -EINVAL;
> +
> raw_spin_lock_init(&dw->lock);
>
> /* Find out how many write channels are supported by hardware */
> @@ -884,7 +895,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>
> err_irq_free:
> for (i = (dw->nr_irqs - 1); i >= 0; i--)
> - free_irq(pci_irq_vector(to_pci_dev(dev), i), &dw->irq[i]);
> + free_irq(dw->ops->irq_vector(dev, i), &dw->irq[i]);
>
> dw->nr_irqs = 0;
>
> @@ -904,7 +915,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
>
> /* Free irqs */
> for (i = (dw->nr_irqs - 1); i >= 0; i--)
> - free_irq(pci_irq_vector(to_pci_dev(dev), i), &dw->irq[i]);
> + free_irq(dw->ops->irq_vector(dev, i), &dw->irq[i]);
>
> /* Power management */
> pm_runtime_disable(dev);
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 4e5f9f6e901b..31fc50d31792 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -103,6 +103,10 @@ struct dw_edma_irq {
> struct dw_edma *dw;
> };
>
> +struct dw_edma_core_ops {
> + int (*irq_vector)(struct device *dev, unsigned int nr);
> +};
> +
> struct dw_edma {
> char name[20];
>
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index dc85f55e1bb8..1eafc602e17e 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -54,6 +54,15 @@ static const struct dw_edma_pcie_data snps_edda_data = {
> .irqs = 1,
> };
>
> +static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr)
> +{
> + return pci_irq_vector(to_pci_dev(dev), nr);
> +}
> +
> +static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
> + .irq_vector = dw_edma_pcie_irq_vector,
> +};
> +
> static int dw_edma_pcie_probe(struct pci_dev *pdev,
> const struct pci_device_id *pid)
> {
> @@ -151,6 +160,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> dw->version = pdata->version;
> dw->mode = pdata->mode;
> dw->nr_irqs = nr_irqs;
> + dw->ops = &dw_edma_pcie_core_ops;
>
> /* Debug info */
> pci_dbg(pdev, "Version:\t%u\n", dw->version);
> --
> 2.7.4
Acked-by: Gustavo Pimentel <[email protected]>
On Wed, Apr 15, 2020 at 10:27 AM Alan Mikhak <[email protected]> wrote:
>
> From: Alan Mikhak <[email protected]>
>
> Decouple dw-edma-core.c from struct pci_dev as a step toward integration
> of dw-edma with pci-epf-test so the latter can initiate dma operations
> locally from the endpoint side. A barrier to such integration is the
> dependency of dw_edma_probe() and other functions in dw-edma-core.c on
> struct pci_dev.
>
> The Synopsys DesignWare dw-edma driver was designed to run on host side
> of PCIe link to initiate DMA operations remotely using eDMA channels of
> PCIe controller on the endpoint side. This can be inferred from seeing
> that dw-edma uses struct pci_dev and accesses hardware registers of dma
> channels across the bus using BAR0 and BAR2.
>
> The ops field of struct dw_edma in dw-edma-core.h is currenty undefined:
>
> const struct dw_edma_core_ops *ops;
>
> However, the kernel builds without failure even when dw-edma driver is
> enabled. Instead of removing the currently undefined and usued ops field,
> define struct dw_edma_core_ops and use the ops field to decouple
> dw-edma-core.c from struct pci_dev.
>
> Signed-off-by: Alan Mikhak <[email protected]>
Adding the ACK from RFC patch
Acked-by: Gustavo Pimentel <[email protected]>
> ---
> drivers/dma/dw-edma/dw-edma-core.c | 29 ++++++++++++++++++++---------
> drivers/dma/dw-edma/dw-edma-core.h | 4 ++++
> drivers/dma/dw-edma/dw-edma-pcie.c | 10 ++++++++++
> 3 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index ff392c01bad1..db401eb11322 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -14,7 +14,7 @@
> #include <linux/err.h>
> #include <linux/interrupt.h>
> #include <linux/dma/edma.h>
> -#include <linux/pci.h>
> +#include <linux/dma-mapping.h>
>
> #include "dw-edma-core.h"
> #include "dw-edma-v0-core.h"
> @@ -781,7 +781,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
>
> if (dw->nr_irqs == 1) {
> /* Common IRQ shared among all channels */
> - err = request_irq(pci_irq_vector(to_pci_dev(dev), 0),
> + err = request_irq(dw->ops->irq_vector(dev, 0),
> dw_edma_interrupt_common,
> IRQF_SHARED, dw->name, &dw->irq[0]);
> if (err) {
> @@ -789,7 +789,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> return err;
> }
>
> - get_cached_msi_msg(pci_irq_vector(to_pci_dev(dev), 0),
> + get_cached_msi_msg(dw->ops->irq_vector(dev, 0),
> &dw->irq[0].msi);
> } else {
> /* Distribute IRQs equally among all channels */
> @@ -804,7 +804,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> dw_edma_add_irq_mask(&rd_mask, *rd_alloc, dw->rd_ch_cnt);
>
> for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
> - err = request_irq(pci_irq_vector(to_pci_dev(dev), i),
> + err = request_irq(dw->ops->irq_vector(dev, i),
> i < *wr_alloc ?
> dw_edma_interrupt_write :
> dw_edma_interrupt_read,
> @@ -815,7 +815,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> return err;
> }
>
> - get_cached_msi_msg(pci_irq_vector(to_pci_dev(dev), i),
> + get_cached_msi_msg(dw->ops->irq_vector(dev, i),
> &dw->irq[i].msi);
> }
>
> @@ -827,12 +827,23 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
>
> int dw_edma_probe(struct dw_edma_chip *chip)
> {
> - struct device *dev = chip->dev;
> - struct dw_edma *dw = chip->dw;
> + struct device *dev;
> + struct dw_edma *dw;
> u32 wr_alloc = 0;
> u32 rd_alloc = 0;
> int i, err;
>
> + if (!chip)
> + return -EINVAL;
> +
> + dev = chip->dev;
> + if (!dev)
> + return -EINVAL;
> +
> + dw = chip->dw;
> + if (!dw || !dw->irq || !dw->ops || !dw->ops->irq_vector)
> + return -EINVAL;
> +
> raw_spin_lock_init(&dw->lock);
>
> /* Find out how many write channels are supported by hardware */
> @@ -884,7 +895,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>
> err_irq_free:
> for (i = (dw->nr_irqs - 1); i >= 0; i--)
> - free_irq(pci_irq_vector(to_pci_dev(dev), i), &dw->irq[i]);
> + free_irq(dw->ops->irq_vector(dev, i), &dw->irq[i]);
>
> dw->nr_irqs = 0;
>
> @@ -904,7 +915,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
>
> /* Free irqs */
> for (i = (dw->nr_irqs - 1); i >= 0; i--)
> - free_irq(pci_irq_vector(to_pci_dev(dev), i), &dw->irq[i]);
> + free_irq(dw->ops->irq_vector(dev, i), &dw->irq[i]);
>
> /* Power management */
> pm_runtime_disable(dev);
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 4e5f9f6e901b..31fc50d31792 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -103,6 +103,10 @@ struct dw_edma_irq {
> struct dw_edma *dw;
> };
>
> +struct dw_edma_core_ops {
> + int (*irq_vector)(struct device *dev, unsigned int nr);
> +};
> +
> struct dw_edma {
> char name[20];
>
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index dc85f55e1bb8..1eafc602e17e 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -54,6 +54,15 @@ static const struct dw_edma_pcie_data snps_edda_data = {
> .irqs = 1,
> };
>
> +static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr)
> +{
> + return pci_irq_vector(to_pci_dev(dev), nr);
> +}
> +
> +static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
> + .irq_vector = dw_edma_pcie_irq_vector,
> +};
> +
> static int dw_edma_pcie_probe(struct pci_dev *pdev,
> const struct pci_device_id *pid)
> {
> @@ -151,6 +160,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> dw->version = pdata->version;
> dw->mode = pdata->mode;
> dw->nr_irqs = nr_irqs;
> + dw->ops = &dw_edma_pcie_core_ops;
>
> /* Debug info */
> pci_dbg(pdev, "Version:\t%u\n", dw->version);
> --
> 2.7.4
>
On 15-04-20, 10:27, Alan Mikhak wrote:
> From: Alan Mikhak <[email protected]>
>
> Decouple dw-edma-core.c from struct pci_dev as a step toward integration
> of dw-edma with pci-epf-test so the latter can initiate dma operations
> locally from the endpoint side. A barrier to such integration is the
> dependency of dw_edma_probe() and other functions in dw-edma-core.c on
> struct pci_dev.
>
> The Synopsys DesignWare dw-edma driver was designed to run on host side
> of PCIe link to initiate DMA operations remotely using eDMA channels of
> PCIe controller on the endpoint side. This can be inferred from seeing
> that dw-edma uses struct pci_dev and accesses hardware registers of dma
> channels across the bus using BAR0 and BAR2.
>
> The ops field of struct dw_edma in dw-edma-core.h is currenty undefined:
>
> const struct dw_edma_core_ops *ops;
>
> However, the kernel builds without failure even when dw-edma driver is
> enabled. Instead of removing the currently undefined and usued ops field,
> define struct dw_edma_core_ops and use the ops field to decouple
> dw-edma-core.c from struct pci_dev.
Applied, thanks
--
~Vinod