* [discussion] pci: imx: enable pcie msi support
@ 2013-12-09 9:25 Richard Zhu
2013-12-09 9:25 ` [PATCH] " Richard Zhu
0 siblings, 1 reply; 6+ messages in thread
From: Richard Zhu @ 2013-12-09 9:25 UTC (permalink / raw)
To: marex; +Cc: jbe, hrhaan, shawn.guo, jgarzik, tj, linux-ide, Richard Zhu
eanble pcie msi support on imx6 platforms
* add check_device api in the msi chip.
* add the quirks into pcie_port struct for the deviation
from standard routines.
Signed-off-by: Richard Zhu <richard.zhuhongxing@gmail.com>
---
arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 6 +++
arch/arm/mach-imx/Kconfig | 1 +
drivers/pci/host/pci-imx6.c | 34 +++++++++++++++++-
drivers/pci/host/pcie-designware.c | 60 +++++++++++++++++++++++---------
drivers/pci/host/pcie-designware.h | 5 +++
5 files changed, 88 insertions(+), 18 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] pci: imx: enable pcie msi support
2013-12-09 9:25 [discussion] pci: imx: enable pcie msi support Richard Zhu
@ 2013-12-09 9:25 ` Richard Zhu
2013-12-09 11:00 ` Jürgen Beisert
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Richard Zhu @ 2013-12-09 9:25 UTC (permalink / raw)
To: marex
Cc: jbe, hrhaan, shawn.guo, jgarzik, tj, linux-ide, Richard Zhu, Richard Zhu
From: Richard Zhu <r65037@freescale.com>
eanble pcie msi support on imx6 platforms
* add check_device api in the msi chip.
* add the quirks into pcie_port struct for the deviation
from standard routines.
Signed-off-by: Richard Zhu <richard.zhuhongxing@gmail.com>
---
arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 6 +++
arch/arm/mach-imx/Kconfig | 1 +
drivers/pci/host/pci-imx6.c | 34 +++++++++++++++++-
drivers/pci/host/pcie-designware.c | 60 +++++++++++++++++++++++---------
drivers/pci/host/pcie-designware.h | 5 +++
5 files changed, 88 insertions(+), 18 deletions(-)
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index e75e11b..b821f87 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -212,6 +212,12 @@
};
};
+&pcie {
+ power-on-gpio = <&gpio3 19 0>;
+ reset-gpio = <&gpio7 12 0>;
+ status = "okay";
+};
+
&pwm1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm0_1>;
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 7a6e6f7..3083c5b 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -796,6 +796,7 @@ config SOC_IMX6Q
select MFD_SYSCON
select MIGHT_HAVE_PCI
select PCI_DOMAINS if PCI
+ select ARCH_SUPPORTS_MSI
select PINCTRL
select PINCTRL_IMX6Q
select PL310_ERRATA_588369 if CACHE_PL310
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index bd70af8..fbd75bf 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -14,6 +14,7 @@
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/gpio.h>
+#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/mfd/syscon.h>
#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
@@ -299,6 +300,15 @@ static void imx6_pcie_init_phy(struct pcie_port *pp)
IMX6Q_GPR8_TX_SWING_LOW, 127 << 25);
}
+static irqreturn_t imx_pcie_msi_irq_handler(int irq, void *arg)
+{
+ struct pcie_port *pp = arg;
+
+ dw_handle_msi_irq(pp);
+
+ return IRQ_HANDLED;
+}
+
static void imx6_pcie_host_init(struct pcie_port *pp)
{
int count = 0;
@@ -324,10 +334,16 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
"DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",
readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
- break;
+ return;
}
}
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ pp->quirks |= DW_PCIE_QUIRK_NO_MSI_VEC;
+ pp->quirks |= DW_PCIE_QUIRK_MSI_SELF_EN;
+ dw_pcie_msi_init(pp);
+ }
+
return;
}
@@ -393,6 +409,22 @@ static int imx6_add_pcie_port(struct pcie_port *pp,
return -ENODEV;
}
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ pp->msi_irq = pp->irq - 3;
+ if (!pp->msi_irq) {
+ dev_err(&pdev->dev, "failed to get msi irq\n");
+ return -ENODEV;
+ }
+
+ ret = devm_request_irq(&pdev->dev, pp->msi_irq,
+ imx_pcie_msi_irq_handler,
+ IRQF_SHARED, "imx6q-pcie", pp);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to request msi irq\n");
+ return ret;
+ }
+ }
+
pp->root_bus_nr = -1;
pp->ops = &imx6_pcie_host_ops;
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index e33b68b..96d2b78 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -308,23 +308,28 @@ static int dw_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev,
return -EINVAL;
}
- pci_read_config_word(pdev, desc->msi_attrib.pos+PCI_MSI_FLAGS,
- &msg_ctr);
- msgvec = (msg_ctr&PCI_MSI_FLAGS_QSIZE) >> 4;
- if (msgvec == 0)
- msgvec = (msg_ctr & PCI_MSI_FLAGS_QMASK) >> 1;
- if (msgvec > 5)
- msgvec = 0;
-
- irq = assign_irq((1 << msgvec), desc, &pos);
- if (irq < 0)
- return irq;
-
- msg_ctr &= ~PCI_MSI_FLAGS_QSIZE;
- msg_ctr |= msgvec << 4;
- pci_write_config_word(pdev, desc->msi_attrib.pos + PCI_MSI_FLAGS,
- msg_ctr);
- desc->msi_attrib.multiple = msgvec;
+ if (pp->quirks & DW_PCIE_QUIRK_NO_MSI_VEC) {
+ irq = assign_irq(1, desc, &pos);
+ set_irq_flags(irq, IRQF_VALID);
+ } else {
+ pci_read_config_word(pdev, desc->msi_attrib.pos+PCI_MSI_FLAGS,
+ &msg_ctr);
+ msgvec = (msg_ctr&PCI_MSI_FLAGS_QSIZE) >> 4;
+ if (msgvec == 0)
+ msgvec = (msg_ctr & PCI_MSI_FLAGS_QMASK) >> 1;
+ if (msgvec > 5)
+ msgvec = 0;
+
+ irq = assign_irq((1 << msgvec), desc, &pos);
+ if (irq < 0)
+ return irq;
+
+ msg_ctr &= ~PCI_MSI_FLAGS_QSIZE;
+ msg_ctr |= msgvec << 4;
+ pci_write_config_word(pdev, desc->msi_attrib.pos + PCI_MSI_FLAGS,
+ msg_ctr);
+ desc->msi_attrib.multiple = msgvec;
+ }
msg.address_lo = virt_to_phys((void *)pp->msi_data);
msg.address_hi = 0x0;
@@ -339,9 +344,30 @@ static void dw_msi_teardown_irq(struct msi_chip *chip, unsigned int irq)
clear_irq(irq);
}
+static int dw_msi_check_device(struct msi_chip *chip, struct pci_dev *pdev,
+ int nvec, int type)
+{
+ struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
+ u32 val;
+
+ if (pp->quirks & DW_PCIE_QUIRK_MSI_SELF_EN) {
+ if ((type == PCI_CAP_ID_MSI) || (type == PCI_CAP_ID_MSIX)) {
+ /* Set MSI enable of RC here */
+ val = readl(pp->dbi_base + 0x50);
+ if ((val & (PCI_MSI_FLAGS_ENABLE << 16)) == 0) {
+ val |= PCI_MSI_FLAGS_ENABLE << 16;
+ writel(val, pp->dbi_base + 0x50);
+ }
+ }
+ }
+
+ return 0;
+}
+
static struct msi_chip dw_pcie_msi_chip = {
.setup_irq = dw_msi_setup_irq,
.teardown_irq = dw_msi_teardown_irq,
+ .check_device = dw_msi_check_device,
};
int dw_pcie_link_up(struct pcie_port *pp)
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index c15379b..79111fe 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -49,6 +49,11 @@ struct pcie_port {
int irq;
u32 lanes;
struct pcie_host_ops *ops;
+ u32 quirks; /* Deviations from spec. */
+/* Controller doesn't support MSI VEC */
+#define DW_PCIE_QUIRK_NO_MSI_VEC (1<<0)
+/* MSI EN of Controller should be configured when MSI is enabled */
+#define DW_PCIE_QUIRK_MSI_SELF_EN (1<<1)
int msi_irq;
struct irq_domain *irq_domain;
unsigned long msi_data;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: imx: enable pcie msi support
2013-12-09 9:25 ` [PATCH] " Richard Zhu
@ 2013-12-09 11:00 ` Jürgen Beisert
2013-12-16 5:44 ` Hong-Xing.Zhu
2013-12-09 11:16 ` Marek Vasut
2013-12-12 18:57 ` Harro Haan
2 siblings, 1 reply; 6+ messages in thread
From: Jürgen Beisert @ 2013-12-09 11:00 UTC (permalink / raw)
To: Richard Zhu; +Cc: marex, hrhaan, shawn.guo, jgarzik, tj, linux-ide, Richard Zhu
Hi Richard,
On Monday 09 December 2013 10:25:19 Richard Zhu wrote:
> [...]
> +static int dw_msi_check_device(struct msi_chip *chip, struct pci_dev *pdev,
> + int nvec, int type)
> +{
> + struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
> + u32 val;
> +
> + if (pp->quirks & DW_PCIE_QUIRK_MSI_SELF_EN) {
> + if ((type == PCI_CAP_ID_MSI) || (type == PCI_CAP_ID_MSIX)) {
> + /* Set MSI enable of RC here */
> + val = readl(pp->dbi_base + 0x50);
> + if ((val & (PCI_MSI_FLAGS_ENABLE << 16)) == 0) {
> + val |= PCI_MSI_FLAGS_ENABLE << 16;
> + writel(val, pp->dbi_base + 0x50);
> + }
> + }
> + }
> +
> + return 0;
> +}
Maybe I'm wrong: but does setting this bit not already happen in function
msi_set_enable() in 'drivers/pci/msi.c' (just to a 16 bit register, so there's
no bit shift required)?
Regards,
Juergen
--
Pengutronix e.K. | Juergen Beisert |
Linux Solutions for Science and Industry | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: imx: enable pcie msi support
2013-12-09 9:25 ` [PATCH] " Richard Zhu
2013-12-09 11:00 ` Jürgen Beisert
@ 2013-12-09 11:16 ` Marek Vasut
2013-12-12 18:57 ` Harro Haan
2 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2013-12-09 11:16 UTC (permalink / raw)
To: Richard Zhu; +Cc: jbe, hrhaan, shawn.guo, jgarzik, tj, linux-ide, Richard Zhu
On Monday, December 09, 2013 at 10:25:19 AM, Richard Zhu wrote:
> From: Richard Zhu <r65037@freescale.com>
>
> eanble pcie msi support on imx6 platforms
> * add check_device api in the msi chip.
> * add the quirks into pcie_port struct for the deviation
> from standard routines.
>
> Signed-off-by: Richard Zhu <richard.zhuhongxing@gmail.com>
[...]
> diff --git a/drivers/pci/host/pcie-designware.c
> b/drivers/pci/host/pcie-designware.c index e33b68b..96d2b78 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -308,23 +308,28 @@ static int dw_msi_setup_irq(struct msi_chip *chip,
> struct pci_dev *pdev, return -EINVAL;
> }
>
> - pci_read_config_word(pdev, desc->msi_attrib.pos+PCI_MSI_FLAGS,
> - &msg_ctr);
> - msgvec = (msg_ctr&PCI_MSI_FLAGS_QSIZE) >> 4;
> - if (msgvec == 0)
> - msgvec = (msg_ctr & PCI_MSI_FLAGS_QMASK) >> 1;
> - if (msgvec > 5)
> - msgvec = 0;
> -
> - irq = assign_irq((1 << msgvec), desc, &pos);
> - if (irq < 0)
> - return irq;
> -
> - msg_ctr &= ~PCI_MSI_FLAGS_QSIZE;
> - msg_ctr |= msgvec << 4;
> - pci_write_config_word(pdev, desc->msi_attrib.pos + PCI_MSI_FLAGS,
> - msg_ctr);
> - desc->msi_attrib.multiple = msgvec;
> + if (pp->quirks & DW_PCIE_QUIRK_NO_MSI_VEC) {
> + irq = assign_irq(1, desc, &pos);
> + set_irq_flags(irq, IRQF_VALID);
What does this do exactly please ? I don't quite understand how this code works.
A beefy comment how this works and why it's needed would really help.
> + } else {
> + pci_read_config_word(pdev, desc->msi_attrib.pos+PCI_MSI_FLAGS,
> + &msg_ctr);
> + msgvec = (msg_ctr&PCI_MSI_FLAGS_QSIZE) >> 4;
> + if (msgvec == 0)
> + msgvec = (msg_ctr & PCI_MSI_FLAGS_QMASK) >> 1;
> + if (msgvec > 5)
> + msgvec = 0;
> +
> + irq = assign_irq((1 << msgvec), desc, &pos);
> + if (irq < 0)
> + return irq;
> +
> + msg_ctr &= ~PCI_MSI_FLAGS_QSIZE;
> + msg_ctr |= msgvec << 4;
> + pci_write_config_word(pdev, desc->msi_attrib.pos +
PCI_MSI_FLAGS,
> + msg_ctr);
> + desc->msi_attrib.multiple = msgvec;
> + }
>
> msg.address_lo = virt_to_phys((void *)pp->msi_data);
> msg.address_hi = 0x0;
> @@ -339,9 +344,30 @@ static void dw_msi_teardown_irq(struct msi_chip *chip,
> unsigned int irq) clear_irq(irq);
> }
>
> +static int dw_msi_check_device(struct msi_chip *chip, struct pci_dev
> *pdev, + int nvec, int type)
> +{
> + struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
> + u32 val;
Can we not have a callback here into the MX6 PCIe driver instead of having this
code here? Then we would likely not need these quirk flags at all.
> + if (pp->quirks & DW_PCIE_QUIRK_MSI_SELF_EN) {
> + if ((type == PCI_CAP_ID_MSI) || (type == PCI_CAP_ID_MSIX)) {
> + /* Set MSI enable of RC here */
> + val = readl(pp->dbi_base + 0x50);
> + if ((val & (PCI_MSI_FLAGS_ENABLE << 16)) == 0) {
> + val |= PCI_MSI_FLAGS_ENABLE << 16;
> + writel(val, pp->dbi_base + 0x50);
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> static struct msi_chip dw_pcie_msi_chip = {
> .setup_irq = dw_msi_setup_irq,
> .teardown_irq = dw_msi_teardown_irq,
> + .check_device = dw_msi_check_device,
> };
>
> int dw_pcie_link_up(struct pcie_port *pp)
[...]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: imx: enable pcie msi support
2013-12-09 9:25 ` [PATCH] " Richard Zhu
2013-12-09 11:00 ` Jürgen Beisert
2013-12-09 11:16 ` Marek Vasut
@ 2013-12-12 18:57 ` Harro Haan
2 siblings, 0 replies; 6+ messages in thread
From: Harro Haan @ 2013-12-12 18:57 UTC (permalink / raw)
To: Richard Zhu
Cc: Marek Vašut, Jürgen Beisert, Shawn Guo, jgarzik, tj,
linux-ide, Richard Zhu
Richard,
On 9 December 2013 10:25, Richard Zhu <richard.zhuhongxing@gmail.com> wrote:
> [...]
> + if (pp->quirks & DW_PCIE_QUIRK_NO_MSI_VEC) {
> + irq = assign_irq(1, desc, &pos);
> + set_irq_flags(irq, IRQF_VALID);
> + } else {
> [...]
Thanks, the above quirk fixes the problem with the MSIX interrupts
> [...]
> + if (pp->quirks & DW_PCIE_QUIRK_MSI_SELF_EN) {
> + if ((type == PCI_CAP_ID_MSI) || (type == PCI_CAP_ID_MSIX)) {
> + /* Set MSI enable of RC here */
> + val = readl(pp->dbi_base + 0x50);
> + if ((val & (PCI_MSI_FLAGS_ENABLE << 16)) == 0) {
> + val |= PCI_MSI_FLAGS_ENABLE << 16;
> + writel(val, pp->dbi_base + 0x50);
> + }
> + }
> + }
> [...]
Why is it actually needed to change the value from 0x1807005 to 0x1817005?
On the SabreSD the MSI interrupts also work without this change.
Best regards,
Harro
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] pci: imx: enable pcie msi support
2013-12-09 11:00 ` Jürgen Beisert
@ 2013-12-16 5:44 ` Hong-Xing.Zhu
0 siblings, 0 replies; 6+ messages in thread
From: Hong-Xing.Zhu @ 2013-12-16 5:44 UTC (permalink / raw)
To: Jürgen Beisert, Richard Zhu
Cc: marex, hrhaan, shawn.guo, jgarzik, tj, linux-ide
Hi Juergen:
Thanks for your review.
> -----Original Message-----
> From: Jürgen Beisert [mailto:jbe@pengutronix.de]
> Sent: Monday, December 09, 2013 7:01 PM
> To: Richard Zhu
> Cc: marex@denx.de; hrhaan@gmail.com; shawn.guo@linaro.org; jgarzik@pobox.com;
> tj@kernel.org; linux-ide@vger.kernel.org; Zhu Richard-R65037
> Subject: Re: [PATCH] pci: imx: enable pcie msi support
>
> Hi Richard,
>
> On Monday 09 December 2013 10:25:19 Richard Zhu wrote:
> > [...]
> > +static int dw_msi_check_device(struct msi_chip *chip, struct pci_dev *pdev,
> > + int nvec, int type)
> > +{
> > + struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
> > + u32 val;
> > +
> > + if (pp->quirks & DW_PCIE_QUIRK_MSI_SELF_EN) {
> > + if ((type == PCI_CAP_ID_MSI) || (type == PCI_CAP_ID_MSIX)) {
> > + /* Set MSI enable of RC here */
> > + val = readl(pp->dbi_base + 0x50);
> > + if ((val & (PCI_MSI_FLAGS_ENABLE << 16)) == 0) {
> > + val |= PCI_MSI_FLAGS_ENABLE << 16;
> > + writel(val, pp->dbi_base + 0x50);
> > + }
> > + }
> > + }
> > +
> > + return 0;
> > +}
>
> Maybe I'm wrong: but does setting this bit not already happen in function
> msi_set_enable() in 'drivers/pci/msi.c' (just to a 16 bit register, so there's
> no bit shift required)?
>
[Richard] yes, this bit would be set in msi_set_enable invoked by msi_capability_init.
But I can't monitor that this bit is set when e1000e is probed. :(.
So I had to set this bit explicitly here.
> Regards,
> Juergen
>
> --
> Pengutronix e.K. | Juergen Beisert |
> Linux Solutions for Science and Industry | http://www.pengutronix.de/ |
>
Best Regards
Richard Zhu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-16 6:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-09 9:25 [discussion] pci: imx: enable pcie msi support Richard Zhu
2013-12-09 9:25 ` [PATCH] " Richard Zhu
2013-12-09 11:00 ` Jürgen Beisert
2013-12-16 5:44 ` Hong-Xing.Zhu
2013-12-09 11:16 ` Marek Vasut
2013-12-12 18:57 ` Harro Haan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.