LinuxLists.cc - [PATCH v2 0/3] PCI: mediatek: Add MSI support for MT2712 and MT7622

2017-08-14 13:05:11

Subject: [PATCH v2 0/3] PCI: mediatek: Add MSI support for MT2712 and MT7622

2017-08-14 13:04:47

Subject: [PATCH v2 2/3] PCI: mediatek: take use of bus->sysdata to get host private data

From: Honghui Zhang <[email protected]>

Commit ae02a6dda285 ("PCI: mediatek: Add controller support for MT2712 and
MT7622") has put the mtk_pcie * into bus->sysdata, take advantage of that
to get the private data and simplify the code.

Signed-off-by: Honghui Zhang <[email protected]>
---
drivers/pci/host/pcie-mediatek.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index 946c056..acc2e28 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -511,8 +511,7 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,
static void __iomem *mtk_pcie_map_bus(struct pci_bus *bus,
unsigned int devfn, int where)
{
- struct pci_host_bridge *host = pci_find_host_bridge(bus);
- struct mtk_pcie *pcie = pci_host_bridge_priv(host);
+ struct mtk_pcie *pcie = bus->sysdata;

writel(PCIE_CONF_ADDR(where, PCI_FUNC(devfn), PCI_SLOT(devfn),
bus->number), pcie->base + PCIE_CFG_ADDR);
--
2.6.4

2017-08-14 13:04:45

by Honghui Zhang

[permalink] [raw]

Subject: [PATCH v2 1/3] PCI: mediatek: Fix return value in case of error

From: Honghui Zhang <[email protected]>

In commit ae02a6dda285 ("PCI: mediatek: Add controller support for MT2712
and MT7622"), the function 'mtk_pcie_init_irq_domain', the pattern used to
check and return error is:

if (!var) {
dev_err(...);
return PTR_ERR(var);
}

The return value in such case is always 0, change it to return -ENODEV
instead. And if error was returned from this function, the error should
be returned to upper layer instead of -ENODEV.

Signed-off-by: Honghui Zhang <[email protected]>
---
drivers/pci/host/pcie-mediatek.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index 44e7885..946c056 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -450,14 +450,14 @@ static int mtk_pcie_init_irq_domain(struct mtk_pcie_port *port,
pcie_intc_node = of_get_next_child(node, NULL);
if (!pcie_intc_node) {
dev_err(dev, "no PCIe Intc node found\n");
- return PTR_ERR(pcie_intc_node);
+ return -ENODEV;
}

port->irq_domain = irq_domain_add_linear(pcie_intc_node, INTX_NUM,
&intx_domain_ops, port);
if (!port->irq_domain) {
dev_err(dev, "failed to get INTx IRQ domain\n");
- return PTR_ERR(port->irq_domain);
+ return -ENODEV;
}

return 0;
@@ -502,7 +502,7 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,
err = mtk_pcie_init_irq_domain(port, node);
if (err) {
dev_err(dev, "failed to init PCIe legacy IRQ domain\n");
- return -ENODEV;
+ return err;
}

return 0;
--
2.6.4

2017-08-14 13:05:12

by Honghui Zhang

[permalink] [raw]

Subject: [PATCH v2 3/3] PCI: mediatek: add msi support for MT2712 and MT7622

From: Honghui Zhang <[email protected]>

MT2712 and MT7622's PCIe host controller support MSI, but only 32bit MSI
address are supportted. It connect to GIC with the same IRQ number of INTx
IRQ, so it shares the same IRQ with INTx IRQ.

This patch add MSI support for MT2712 and MT7622.

Signed-off-by: Honghui Zhang <[email protected]>
---
drivers/pci/host/pcie-mediatek.c | 148 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 147 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index acc2e28..35ab232 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -73,11 +73,17 @@
#define PCIE_CSR_ASPM_L1_EN(x) BIT(1 + (x) * 8)

/* PCIe V2 per-port registers */
+#define PCIE_MSI_VECTOR 0x0c0
#define PCIE_INT_MASK 0x420
#define INTX_MASK GENMASK(19, 16)
#define INTX_SHIFT 16
#define INTX_NUM 4
#define PCIE_INT_STATUS 0x424
+#define MSI_STATUS BIT(23)
+#define PCIE_IMSI_STATUS 0x42c
+#define PCIE_IMSI_ADDR 0x430
+#define MSI_MASK BIT(23)
+#define MTK_MSI_IRQS_NUM 32

#define PCIE_AHB_TRANS_BASE0_L 0x438
#define PCIE_AHB_TRANS_BASE0_H 0x43c
@@ -128,11 +134,13 @@ struct mtk_pcie_port;

/**
* struct mtk_pcie_soc - differentiate between host generations
+ * @has_msi: whether this host support MSI interrupt or not
* @ops: pointer to configuration access functions
* @startup: pointer to controller setting functions
* @setup_irq: pointer to initialize IRQ functions
*/
struct mtk_pcie_soc {
+ bool has_msi;
struct pci_ops *ops;
int (*startup)(struct mtk_pcie_port *port);
int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
@@ -156,6 +164,8 @@ struct mtk_pcie_soc {
* @lane: lane count
* @slot: port slot
* @irq_domain: legacy INTx IRQ domain
+ * @msi_domain: MSI IRQ domain
+ * @msi_irq_in_use: bit map for assigned MSI IRQ
*/
struct mtk_pcie_port {
void __iomem *base;
@@ -172,6 +182,8 @@ struct mtk_pcie_port {
u32 lane;
u32 slot;
struct irq_domain *irq_domain;
+ struct irq_domain *msi_domain;
+ DECLARE_BITMAP(msi_irq_in_use, MTK_MSI_IRQS_NUM);
};

/**
@@ -427,6 +439,108 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
return 0;
}

+static int mtk_pcie_assign_msi(struct mtk_pcie_port *port)
+{
+ int pos;
+
+ pos = find_first_zero_bit(port->msi_irq_in_use, MTK_MSI_IRQS_NUM);
+ if (pos < MTK_MSI_IRQS_NUM)
+ set_bit(pos, port->msi_irq_in_use);
+ else
+ return -ENOSPC;
+
+ return pos;
+}
+
+static int mtk_pcie_msi_setup_irq(struct msi_controller *chip,
+ struct pci_dev *pdev,
+ struct msi_desc *desc)
+{
+ struct mtk_pcie_port *port;
+ struct msi_msg msg;
+ int hwirq;
+ u32 irq;
+
+ port = mtk_pcie_find_port(pdev->bus, pdev->devfn);
+ if (!port)
+ return -EINVAL;
+
+ chip->dev = &pdev->dev;
+ hwirq = mtk_pcie_assign_msi(port);
+ if (hwirq < 0)
+ return hwirq;
+
+ irq = irq_create_mapping(port->msi_domain, hwirq);
+ if (!irq)
+ return -EINVAL;
+
+ irq_set_msi_desc(irq, desc);
+
+ /* MT2712/MT7622 only support 32 bit MSI address */
+ msg.address_hi = 0;
+ msg.address_lo = lower_32_bits((u64)(port->base + PCIE_MSI_VECTOR));
+ msg.data = hwirq;
+
+ pci_write_msi_msg(irq, &msg);
+
+ return 0;
+}
+
+static void mtk_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
+{
+ struct pci_dev *pdev = to_pci_dev(chip->dev);
+ struct irq_data *d = irq_get_irq_data(irq);
+ irq_hw_number_t hwirq = irqd_to_hwirq(d);
+ struct mtk_pcie_port *port;
+
+ port = mtk_pcie_find_port(pdev->bus, pdev->devfn);
+ if (!port)
+ return;
+
+ if (!test_bit(hwirq, port->msi_irq_in_use))
+ dev_err(&pdev->dev, "trying to free unused MSI#%d\n", irq);
+ else
+ clear_bit(hwirq, port->msi_irq_in_use);
+}
+
+static struct msi_controller mtk_pcie_msi_chip = {
+ .setup_irq = mtk_pcie_msi_setup_irq,
+ .teardown_irq = mtk_msi_teardown_irq,
+};
+
+static struct irq_chip mtk_msi_irq_chip = {
+ .name = "MTK PCIe MSI",
+ .irq_enable = pci_msi_unmask_irq,
+ .irq_disable = pci_msi_mask_irq,
+ .irq_mask = pci_msi_mask_irq,
+ .irq_unmask = pci_msi_unmask_irq,
+};
+
+static int mtk_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_chip_and_handler(irq, &mtk_msi_irq_chip, handle_simple_irq);
+ irq_set_chip_data(irq, domain->host_data);
+
+ return 0;
+}
+
+static const struct irq_domain_ops msi_domain_ops = {
+ .map = mtk_pcie_msi_map,
+};
+
+static void mtk_pcie_enable_msi(struct mtk_pcie_port *port)
+{
+ u32 val;
+
+ val = lower_32_bits((u64)(port->base + PCIE_MSI_VECTOR));
+ writel(val, port->base + PCIE_IMSI_ADDR);
+
+ val = readl(port->base + PCIE_INT_MASK);
+ val &= ~MSI_MASK;
+ writel(val, port->base + PCIE_INT_MASK);
+}
+
static int mtk_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
irq_hw_number_t hwirq)
{
@@ -460,6 +574,18 @@ static int mtk_pcie_init_irq_domain(struct mtk_pcie_port *port,
return -ENODEV;
}

+ /* Setup MSI */
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ port->msi_domain = irq_domain_add_linear(node, MTK_MSI_IRQS_NUM,
+ &msi_domain_ops,
+ &mtk_pcie_msi_chip);
+ if (!port->msi_domain) {
+ dev_err(dev, "failed to get MSI IRQ domain\n");
+ return -ENODEV;
+ }
+ mtk_pcie_enable_msi(port);
+ }
+
return 0;
}

@@ -480,6 +606,23 @@ static irqreturn_t mtk_pcie_intr_handler(int irq, void *data)
}
}

+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ while ((status = readl(port->base + PCIE_INT_STATUS)) & MSI_STATUS) {
+ unsigned long imsi_status;
+
+ while ((imsi_status = readl(port->base + PCIE_IMSI_STATUS))) {
+ for_each_set_bit(bit, &imsi_status, MTK_MSI_IRQS_NUM) {
+ /* Clear the MSI */
+ writel(1 << bit, port->base + PCIE_IMSI_STATUS);
+ virq = irq_find_mapping(port->msi_domain, bit);
+ generic_handle_irq(virq);
+ }
+ }
+ /* Clear MSI interrupt status */
+ writel(MSI_STATUS, port->base + PCIE_INT_STATUS);
+ }
+ }
+
return IRQ_HANDLED;
}

@@ -501,7 +644,7 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,

err = mtk_pcie_init_irq_domain(port, node);
if (err) {
- dev_err(dev, "failed to init PCIe legacy IRQ domain\n");
+ dev_err(dev, "failed to init PCIe IRQ domain\n");
return err;
}

@@ -938,6 +1081,8 @@ static int mtk_pcie_register_host(struct pci_host_bridge *host)
host->map_irq = of_irq_parse_and_map_pci;
host->swizzle_irq = pci_common_swizzle;
host->sysdata = pcie;
+ if (IS_ENABLED(CONFIG_PCI_MSI) && pcie->soc->has_msi)
+ host->msi = &mtk_pcie_msi_chip;

err = pci_scan_root_bus_bridge(host);
if (err < 0)
@@ -999,6 +1144,7 @@ static const struct mtk_pcie_soc mtk_pcie_soc_v1 = {
};

static const struct mtk_pcie_soc mtk_pcie_soc_v2 = {
+ .has_msi = true,
.ops = &mtk_pcie_ops_v2,
.startup = mtk_pcie_startup_port_v2,
.setup_irq = mtk_pcie_setup_irq,
--
2.6.4

2017-08-22 21:36:08

by Bjorn Helgaas

[permalink] [raw]

Subject: Re: [PATCH v2 0/3] PCI: mediatek: Add MSI support for MT2712 and MT7622

2017-08-23 01:04:31

by Ryder Lee

[permalink] [raw]

Subject: Re: [PATCH v2 0/3] PCI: mediatek: Add MSI support for MT2712 and MT7622

On Tue, 2017-08-22 at 16:36 -0500, Bjorn Helgaas wrote:
> On Mon, Aug 14, 2017 at 09:04:25PM +0800, [email protected] wrote:
> > From: Honghui Zhang <[email protected]>
> >
> > MT2712 and MT7622's PCIe host controller support MSI, but only 32bit MSI
> > address are supportted. It connect to GIC with the same IRQ number of INTx
> > IRQ, so it shares the same IRQ with INTx IRQ.
> >
> > This patchset add MSI support for MT2712 and MT7622.
> > Also do some code fixup and cleanups.
> >
> > Change Since V1:
> > - Add the first two patches into this patchset.
> > - Using -ENODEV instead of PTR_ERR if msi_domain == NULL
> > - Change the error logs with consistency of lower-case
> > - Using has_msi instead of support_msi to make the parameter name a bit shorter
> >
> > Honghui Zhang (3):
> > PCI: mediatek: Fix return value in case of error
> > PCI: mediatek: take use of bus->sysdata to get host private data
> > PCI: mediatek: add msi support for MT2712 and MT7622
> >
> > drivers/pci/host/pcie-mediatek.c | 157 +++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 151 insertions(+), 6 deletions(-)
>
> Just waiting for an ack from Ryder...

For whole series:

Acked-by: Ryder Lee <[email protected]>

2017-08-23 18:58:34

by Bjorn Helgaas

[permalink] [raw]

Subject: Re: [PATCH v2 0/3] PCI: mediatek: Add MSI support for MT2712 and MT7622

On Mon, Aug 14, 2017 at 09:04:25PM +0800, [email protected] wrote:
> From: Honghui Zhang <[email protected]>
>
> MT2712 and MT7622's PCIe host controller support MSI, but only 32bit MSI
> address are supportted. It connect to GIC with the same IRQ number of INTx
> IRQ, so it shares the same IRQ with INTx IRQ.
>
> This patchset add MSI support for MT2712 and MT7622.
> Also do some code fixup and cleanups.
>
> Change Since V1:
> - Add the first two patches into this patchset.
> - Using -ENODEV instead of PTR_ERR if msi_domain == NULL
> - Change the error logs with consistency of lower-case
> - Using has_msi instead of support_msi to make the parameter name a bit shorter
>
> Honghui Zhang (3):
> PCI: mediatek: Fix return value in case of error
> PCI: mediatek: take use of bus->sysdata to get host private data
> PCI: mediatek: add msi support for MT2712 and MT7622
>
> drivers/pci/host/pcie-mediatek.c | 157 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 151 insertions(+), 6 deletions(-)

Since "PCI: mediatek: Add controller support for MT2712 and MT7622" patch
hasn't appeared upstream yet, I folded the return value fix directly into
it.

I applied the others to pci/host-mediatek. I modified the third one to
remove what seemed to be needless differences from the rcar and tegra
drivers. The incremental diff is attached. Please check to make sure I
didn't break anything.

diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index 35ab2321ed16..8891c00bf13c 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -134,7 +134,7 @@ struct mtk_pcie_port;

/**
* struct mtk_pcie_soc - differentiate between host generations
- * @has_msi: whether this host support MSI interrupt or not
+ * @has_msi: whether this host supports MSI interrupts or not
* @ops: pointer to configuration access functions
* @startup: pointer to controller setting functions
* @setup_irq: pointer to initialize IRQ functions
@@ -439,44 +439,51 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
return 0;
}

-static int mtk_pcie_assign_msi(struct mtk_pcie_port *port)
+static int mtk_pcie_msi_alloc(struct mtk_pcie_port *port)
{
- int pos;
+ int msi;

- pos = find_first_zero_bit(port->msi_irq_in_use, MTK_MSI_IRQS_NUM);
- if (pos < MTK_MSI_IRQS_NUM)
- set_bit(pos, port->msi_irq_in_use);
+ msi = find_first_zero_bit(port->msi_irq_in_use, MTK_MSI_IRQS_NUM);
+ if (msi < MTK_MSI_IRQS_NUM)
+ set_bit(msi, port->msi_irq_in_use);
else
return -ENOSPC;

- return pos;
+ return msi;
+}
+
+static void mtk_pcie_msi_free(struct mtk_pcie_port *port, unsigned long hwirq)
+{
+ clear_bit(hwirq, port->msi_irq_in_use);
}

static int mtk_pcie_msi_setup_irq(struct msi_controller *chip,
- struct pci_dev *pdev,
- struct msi_desc *desc)
+ struct pci_dev *pdev, struct msi_desc *desc)
{
struct mtk_pcie_port *port;
struct msi_msg msg;
+ unsigned int irq;
int hwirq;
- u32 irq;

port = mtk_pcie_find_port(pdev->bus, pdev->devfn);
if (!port)
return -EINVAL;

- chip->dev = &pdev->dev;
- hwirq = mtk_pcie_assign_msi(port);
+ hwirq = mtk_pcie_msi_alloc(port);
if (hwirq < 0)
return hwirq;

irq = irq_create_mapping(port->msi_domain, hwirq);
- if (!irq)
+ if (!irq) {
+ mtk_pcie_msi_free(port, hwirq);
return -EINVAL;
+ }
+
+ chip->dev = &pdev->dev;

irq_set_msi_desc(irq, desc);

- /* MT2712/MT7622 only support 32 bit MSI address */
+ /* MT2712/MT7622 only support 32-bit MSI addresses */
msg.address_hi = 0;
msg.address_lo = lower_32_bits((u64)(port->base + PCIE_MSI_VECTOR));
msg.data = hwirq;
@@ -497,10 +504,8 @@ static void mtk_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
if (!port)
return;

- if (!test_bit(hwirq, port->msi_irq_in_use))
- dev_err(&pdev->dev, "trying to free unused MSI#%d\n", irq);
- else
- clear_bit(hwirq, port->msi_irq_in_use);
+ irq_dispose_mapping(irq);
+ mtk_pcie_msi_free(port, hwirq);
}

static struct msi_controller mtk_pcie_msi_chip = {
@@ -529,16 +534,26 @@ static const struct irq_domain_ops msi_domain_ops = {
.map = mtk_pcie_msi_map,
};

-static void mtk_pcie_enable_msi(struct mtk_pcie_port *port)
+static int mtk_pcie_enable_msi(struct mtk_pcie_port *port)
{
u32 val;

+ port->msi_domain = irq_domain_add_linear(node, MTK_MSI_IRQS_NUM,
+ &msi_domain_ops,
+ &mtk_pcie_msi_chip);
+ if (!port->msi_domain) {
+ dev_err(dev, "failed to create MSI IRQ domain\n");
+ return -ENOMEM;
+ }
+
val = lower_32_bits((u64)(port->base + PCIE_MSI_VECTOR));
writel(val, port->base + PCIE_IMSI_ADDR);

val = readl(port->base + PCIE_INT_MASK);
val &= ~MSI_MASK;
writel(val, port->base + PCIE_INT_MASK);
+
+ return 0;
}

static int mtk_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
@@ -559,6 +574,7 @@ static int mtk_pcie_init_irq_domain(struct mtk_pcie_port *port,
{
struct device *dev = port->pcie->dev;
struct device_node *pcie_intc_node;
+ int err;

/* Setup INTx */
pcie_intc_node = of_get_next_child(node, NULL);
@@ -574,16 +590,10 @@ static int mtk_pcie_init_irq_domain(struct mtk_pcie_port *port,
return -ENODEV;
}

- /* Setup MSI */
if (IS_ENABLED(CONFIG_PCI_MSI)) {
- port->msi_domain = irq_domain_add_linear(node, MTK_MSI_IRQS_NUM,
- &msi_domain_ops,
- &mtk_pcie_msi_chip);
- if (!port->msi_domain) {
- dev_err(dev, "failed to get MSI IRQ domain\n");
- return -ENODEV;
- }
- mtk_pcie_enable_msi(port);
+ err = mtk_pcie_enable_msi(port);
+ if (err < 0)
+ return err;
}

return 0;