[PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Andrey Smirnov]

Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
produces a system where built-in PCIE bridge (16c3:abcd) isn't bound
to pcieport driver. This, in turn, results in a PCIE bus that is
capable of enumerating attached PCIE device, but lacks functional
interrupt support.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Assuming this is a reasonable dependency, shold this be done to more
than just i.MX6 driver?

 drivers/pci/controller/dwc/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 2b139acccf32..44ededbeab85 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -92,6 +92,7 @@ config PCI_IMX6
 	bool "Freescale i.MX6 PCIe controller"
 	depends on SOC_IMX8MQ || SOC_IMX6Q || (ARM && COMPILE_TEST)
 	depends on PCI_MSI_IRQ_DOMAIN
+	depends on PCIEPORTBUS
 	select PCIE_DW_HOST
 
 config PCIE_SPEAR13XX
-- 
2.19.1

Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Baruch Siach]

Hi Andrey,

Adding Robert Hancock who reported[1] on a PCIe MSI issue with i.MX6.

Andrey Smirnov writes:

> Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
> produces a system where built-in PCIE bridge (16c3:abcd) isn't bound
> to pcieport driver. This, in turn, results in a PCIE bus that is
> capable of enumerating attached PCIE device, but lacks functional
> interrupt support.

Robert, does that fix your issue?

> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>
> Assuming this is a reasonable dependency, shold this be done to more
> than just i.MX6 driver?
>
>  drivers/pci/controller/dwc/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 2b139acccf32..44ededbeab85 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -92,6 +92,7 @@ config PCI_IMX6
>  	bool "Freescale i.MX6 PCIe controller"
>  	depends on SOC_IMX8MQ || SOC_IMX6Q || (ARM && COMPILE_TEST)
>  	depends on PCI_MSI_IRQ_DOMAIN
> +	depends on PCIEPORTBUS

This effectively disables PCIe in imx_v6_v7_defconfig, since
CONFIG_PCIEPORTBUS is not enabled there. Maybe do 'select' instead?

>  	Select PCIE_DW_HOST
>
>  config PCIE_SPEAR13XX

baruch

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-November/614800.html

--
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Lucas Stach]

Am Mittwoch, den 05.12.2018, 23:45 -0800 schrieb Andrey Smirnov:
> Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
> produces a system where built-in PCIE bridge (16c3:abcd) isn't bound
> to pcieport driver. This, in turn, results in a PCIE bus that is
> capable of enumerating attached PCIE device, but lacks functional
> interrupt support.

This is odd. AFAIK PCI port services are a totally optional thing and
them being absent should not lead to a non-functional PCI bus. So I
would really like to see some deeper analysis what is going on here.

Regards,
Lucas

> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
> 
> Assuming this is a reasonable dependency, shold this be done to more
> than just i.MX6 driver?
> 
>  drivers/pci/controller/dwc/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 2b139acccf32..44ededbeab85 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -92,6 +92,7 @@ config PCI_IMX6
> >  	bool "Freescale i.MX6 PCIe controller"
> >  	depends on SOC_IMX8MQ || SOC_IMX6Q || (ARM && COMPILE_TEST)
> >  	depends on PCI_MSI_IRQ_DOMAIN
> > +	depends on PCIEPORTBUS
> >  	select PCIE_DW_HOST
>  
>  config PCIE_SPEAR13XX

Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Robert Hancock]

On 2018-12-06 2:10 a.m., Baruch Siach wrote:
> Hi Andrey,
> 
> Adding Robert Hancock who reported[1] on a PCIe MSI issue with i.MX6.
> 
> Andrey Smirnov writes:
> 
>> Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
>> produces a system where built-in PCIE bridge (16c3:abcd) isn't bound
>> to pcieport driver. This, in turn, results in a PCIE bus that is
>> capable of enumerating attached PCIE device, but lacks functional
>> interrupt support.
> 
> Robert, does that fix your issue?

Unfortunately, no.. in fact the situation on my setup is even worse with
CONFIG_PCIEPORTBUS enabled: Not only does MSI still not function, but
now INTx interrupts are somehow broken as well - no interrupts are
received. The IRQ information shown in /proc/interrupts is correct, but
the count remains stubbornly at 0.

So given that outcome, I don't think we should add this as a hard
dependency until we can figure out what is going on, as it seems to
regress working setups.

> 
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>
>> Assuming this is a reasonable dependency, shold this be done to more
>> than just i.MX6 driver?
>>
>>  drivers/pci/controller/dwc/Kconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>> index 2b139acccf32..44ededbeab85 100644
>> --- a/drivers/pci/controller/dwc/Kconfig
>> +++ b/drivers/pci/controller/dwc/Kconfig
>> @@ -92,6 +92,7 @@ config PCI_IMX6
>>  	bool "Freescale i.MX6 PCIe controller"
>>  	depends on SOC_IMX8MQ || SOC_IMX6Q || (ARM && COMPILE_TEST)
>>  	depends on PCI_MSI_IRQ_DOMAIN
>> +	depends on PCIEPORTBUS
> 
> This effectively disables PCIe in imx_v6_v7_defconfig, since
> CONFIG_PCIEPORTBUS is not enabled there. Maybe do 'select' instead?
> 
>>  	Select PCIE_DW_HOST
>>
>>  config PCIE_SPEAR13XX
> 
> baruch
> 
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-November/614800.html
> 
> --
>      http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
>    - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
> 

-- 
Robert Hancock
Senior Software Developer
SED Systems
Email: hancock@sedsystems.ca

Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Lucas Stach]

Am Donnerstag, den 06.12.2018, 09:45 -0600 schrieb Robert Hancock:
> On 2018-12-06 2:10 a.m., Baruch Siach wrote:
> > Hi Andrey,
> > 
> > Adding Robert Hancock who reported[1] on a PCIe MSI issue with i.MX6.
> > 
> > Andrey Smirnov writes:
> > 
> > > Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
> > > produces a system where built-in PCIE bridge (16c3:abcd) isn't bound
> > > to pcieport driver. This, in turn, results in a PCIE bus that is
> > > capable of enumerating attached PCIE device, but lacks functional
> > > interrupt support.
> > 
> > Robert, does that fix your issue?
> 
> Unfortunately, no.. in fact the situation on my setup is even worse with
> CONFIG_PCIEPORTBUS enabled: Not only does MSI still not function, but
> now INTx interrupts are somehow broken as well - no interrupts are
> received. The IRQ information shown in /proc/interrupts is correct, but
> the count remains stubbornly at 0.

That's expected. The port services will use an MSI IRQ when available
and due to a design issue with the DWC PCIe it will not forward any
legacy IRQs if any MSI is in use. If any of the PCIe devices in your
system are unable to work with MSI IRQs, you must boot with "nomsi" on
the kernel command line set.

Regards,
Lucas

Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Robert Hancock]

On 2018-12-06 9:50 a.m., Lucas Stach wrote:
> Am Donnerstag, den 06.12.2018, 09:45 -0600 schrieb Robert Hancock:
>> On 2018-12-06 2:10 a.m., Baruch Siach wrote:
>>> Hi Andrey,
>>>
>>> Adding Robert Hancock who reported[1] on a PCIe MSI issue with i.MX6.
>>>
>>> Andrey Smirnov writes:
>>>
>>>> Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
>>>> produces a system where built-in PCIE bridge (16c3:abcd) isn't bound
>>>> to pcieport driver. This, in turn, results in a PCIE bus that is
>>>> capable of enumerating attached PCIE device, but lacks functional
>>>> interrupt support.
>>>
>>> Robert, does that fix your issue?
>>
>> Unfortunately, no.. in fact the situation on my setup is even worse with
>> CONFIG_PCIEPORTBUS enabled: Not only does MSI still not function, but
>> now INTx interrupts are somehow broken as well - no interrupts are
>> received. The IRQ information shown in /proc/interrupts is correct, but
>> the count remains stubbornly at 0.
> 
> That's expected. The port services will use an MSI IRQ when available
> and due to a design issue with the DWC PCIe it will not forward any
> legacy IRQs if any MSI is in use. If any of the PCIe devices in your
> system are unable to work with MSI IRQs, you must boot with "nomsi" on
> the kernel command line set.

That seems like an unfortunate design choice on their part.. well that
would probably argue against adding this as a hard dependency then, if
non-MSI-supporting PCIe devices can't work with default boot options
with that set.

I'm looking into testing with an NXP Smart Devices board and some PCIe
cards to see if I can verify whether MSI works on those or not, since we
currently don't have a way to independently verify that the MSI
implementation in our FPGA is working or whether another PCIe device
works with MSI (the FPGA is integrated on the system board).

-- 
Robert Hancock
Senior Software Developer
SED Systems
Email: hancock@sedsystems.ca

Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Trent Piepho]

On Thu, 2018-12-06 at 11:28 +0100, Lucas Stach wrote:
> Am Mittwoch, den 05.12.2018, 23:45 -0800 schrieb Andrey Smirnov:
> > Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
> > produces a system where built-in PCIE bridge (16c3:abcd) isn't
> > bound
> > to pcieport driver. This, in turn, results in a PCIE bus that is
> > capable of enumerating attached PCIE device, but lacks functional
> > interrupt support.
> 
> This is odd. AFAIK PCI port services are a totally optional thing and
> them being absent should not lead to a non-functional PCI bus. So I
> would really like to see some deeper analysis what is going on here.

I noticed this previously, 
https://www.spinics.net/lists/linux-pci/msg77840.html

What happened was that PCIEPORTBUS *was* selected by all the platform
dwc drivers and those selections were removed in:

commit f3fdfc4ac3a26865e84627a7fe08a906081e5abc
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Fri May 18 15:08:36 2018 -0500

    PCI: Remove host driver Kconfig selection of CONFIG_PCIEPORTBUS
    
    Host bridge drivers do not use the portdrv interfaces (struct pcie_device,
    struct pcie_port_service_driver, pcie_port_service_register(), etc), and
    they should not select CONFIG_PCIEPORTBUS.

I encountered this when a kernel defconfig that previously turned on
PCIEPORTBUS stopped turning in on, resulting in the situation that
Andrey describes:  no errors, device present, but no MSI interrupts.

The in kernel defconfigs (mine not being one of those) for all the dwc
pcie users should probably be refreshed to enable pcieportbus.

It seems appropriate that the port services are optional and it's
unfortunate that the kernel defconfig system resolves in the change in
this manner, vs keeping pcieportbus on by default.  I do know why it is
this way and I don't think there is any great solution there.

It has never been clear to me why pcieportbus was necessary for MSI.

Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Robert Hancock]

On 2018-12-06 10:10 a.m., Robert Hancock wrote:
> On 2018-12-06 9:50 a.m., Lucas Stach wrote:
>> Am Donnerstag, den 06.12.2018, 09:45 -0600 schrieb Robert Hancock:
>>> On 2018-12-06 2:10 a.m., Baruch Siach wrote:
>>>> Hi Andrey,
>>>>
>>>> Adding Robert Hancock who reported[1] on a PCIe MSI issue with i.MX6.
>>>>
>>>> Andrey Smirnov writes:
>>>>
>>>>> Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
>>>>> produces a system where built-in PCIE bridge (16c3:abcd) isn't bound
>>>>> to pcieport driver. This, in turn, results in a PCIE bus that is
>>>>> capable of enumerating attached PCIE device, but lacks functional
>>>>> interrupt support.
>>>>
>>>> Robert, does that fix your issue?
>>>
>>> Unfortunately, no.. in fact the situation on my setup is even worse with
>>> CONFIG_PCIEPORTBUS enabled: Not only does MSI still not function, but
>>> now INTx interrupts are somehow broken as well - no interrupts are
>>> received. The IRQ information shown in /proc/interrupts is correct, but
>>> the count remains stubbornly at 0.
>>
>> That's expected. The port services will use an MSI IRQ when available
>> and due to a design issue with the DWC PCIe it will not forward any
>> legacy IRQs if any MSI is in use. If any of the PCIe devices in your
>> system are unable to work with MSI IRQs, you must boot with "nomsi" on
>> the kernel command line set.
> 
> That seems like an unfortunate design choice on their part.. well that
> would probably argue against adding this as a hard dependency then, if
> non-MSI-supporting PCIe devices can't work with default boot options
> with that set.
> 
> I'm looking into testing with an NXP Smart Devices board and some PCIe
> cards to see if I can verify whether MSI works on those or not, since we
> currently don't have a way to independently verify that the MSI
> implementation in our FPGA is working or whether another PCIe device
> works with MSI (the FPGA is integrated on the system board).

I've now done some tests with a NXP SabreSD reference board and an Intel
wireless card:

-With the standard imx_v6_v7 defconfig, MSI does not work, INTx works

-With CONFIG_PCIEPORTBUS=y, MSI does work

So it seems like enabling PCIEPORTBUS should fix our MSI issue on the
CPU side, and our remaining problem is likely on the FPGA device side.
However, there's still the issue that enabling that option breaks INTx
support - I don't have a PCIe card handy that the kernel doesn't enable
MSI for in order to test that on the Sabre board, but based on Lucas's
comment and my results on our board, it definitely seems to be an issue.
I would hope there must be a way to handle that..

-- 
Robert Hancock
Senior Software Developer
SED Systems
Email: hancock@sedsystems.ca

Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Andrey Smirnov]

On Thu, Dec 6, 2018 at 2:28 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Am Mittwoch, den 05.12.2018, 23:45 -0800 schrieb Andrey Smirnov:
> > Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
> > produces a system where built-in PCIE bridge (16c3:abcd) isn't bound
> > to pcieport driver. This, in turn, results in a PCIE bus that is
> > capable of enumerating attached PCIE device, but lacks functional
> > interrupt support.
>
> This is odd. AFAIK PCI port services are a totally optional thing and
> them being absent should not lead to a non-functional PCI bus. So I
> would really like to see some deeper analysis what is going on here.
>

AFAICT, this is due to pcieport driver enabling MSI of the bridge
device (16c3:abcd) via pcie_port_device_register() ->
pcie_init_service_irqs() -> pcie_port_enable_irq_vec() -> etc.

I did an experiment on a i.MX8MQ/PCIE -> i210 setup I have: I disabled
CONFIG_PCIEPORTBUS and hacked igb_main.c enough to make the i210
driver believe it should fall back onto legacy interrupts. Even
without pcieport present in the system, i210 worked as expected via
legacy interrupts, which seems to collaborate my conjecture above.

Thanks,
Andrey Smirnov

Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Niklas Cassel]

On Thu, Dec 06, 2018 at 08:55:13PM -0800, Andrey Smirnov wrote:
> On Thu, Dec 6, 2018 at 2:28 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> >
> > Am Mittwoch, den 05.12.2018, 23:45 -0800 schrieb Andrey Smirnov:
> > > Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
> > > produces a system where built-in PCIE bridge (16c3:abcd) isn't bound
> > > to pcieport driver. This, in turn, results in a PCIE bus that is
> > > capable of enumerating attached PCIE device, but lacks functional
> > > interrupt support.
> >
> > This is odd. AFAIK PCI port services are a totally optional thing and
> > them being absent should not lead to a non-functional PCI bus. So I
> > would really like to see some deeper analysis what is going on here.
> >
> 
> AFAICT, this is due to pcieport driver enabling MSI of the bridge
> device (16c3:abcd) via pcie_port_device_register() ->
> pcie_init_service_irqs() -> pcie_port_enable_irq_vec() -> etc.
> 
> I did an experiment on a i.MX8MQ/PCIE -> i210 setup I have: I disabled
> CONFIG_PCIEPORTBUS and hacked igb_main.c enough to make the i210
> driver believe it should fall back onto legacy interrupts. Even
> without pcieport present in the system, i210 worked as expected via
> legacy interrupts, which seems to collaborate my conjecture above.
> 
> Thanks,
> Andrey Smirnov

IIUC PCIEPORTBUS should not be needed for MSIs to work,
it is only needed if you want e.g. PME or AER.

The difference is that if PCIEPORTBUS is enabled, a MSI irq vector will be
allocated for the Root Complex itself, so that it can send an irq when
e.g. AER has detected an error.


If we disregard that MSI handling is currently broken on DWC PCIe:
https://marc.info/?l=linux-pci&m=154214986924244&w=2
It is very possible to have MSIs on dragonboard 820c, which also
uses the DWC PCIe controller, without having PCIEPORTBUS selected:

# zcat /proc/config.gz | grep -E "PCIE_QCOM|PCIEPORTBUS"
# CONFIG_PCIEPORTBUS is not set
CONFIG_PCIE_QCOM=y


# lspci -v -s 0000:00:00.0
0000:00:00.0 PCI bridge: Qualcomm Device 0104 (prog-if 00 [Normal decode])
	...
        Capabilities: [50] MSI: Enable- Count=1/32 Maskable+ 64bit+

# lspci -v -s 0000:01:00.0
0000:01:00.0 Network controller: Qualcomm Atheros QCA6174 802.11ac Wireless Network Adapter (rev 32)
	...
        Capabilities: [50] MSI: Enable+ Count=1/8 Maskable+ 64bit-


# cat /proc/interrupts | grep MSI
 70:       5620          0          0          0   PCI-MSI 524288 Edge      ath10k_pci

So perhaps this is a bug specific to imx6?


Kind regards,
Niklas

Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Andrey Smirnov]

On Fri, Dec 7, 2018 at 5:11 AM Niklas Cassel <niklas.cassel@linaro.org> wrote:
>
> On Thu, Dec 06, 2018 at 08:55:13PM -0800, Andrey Smirnov wrote:
> > On Thu, Dec 6, 2018 at 2:28 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > >
> > > Am Mittwoch, den 05.12.2018, 23:45 -0800 schrieb Andrey Smirnov:
> > > > Building a kernel with CONFIG_PCI_IMX6=y, but CONFIG_PCIEPORTBUS=n
> > > > produces a system where built-in PCIE bridge (16c3:abcd) isn't bound
> > > > to pcieport driver. This, in turn, results in a PCIE bus that is
> > > > capable of enumerating attached PCIE device, but lacks functional
> > > > interrupt support.
> > >
> > > This is odd. AFAIK PCI port services are a totally optional thing and
> > > them being absent should not lead to a non-functional PCI bus. So I
> > > would really like to see some deeper analysis what is going on here.
> > >
> >
> > AFAICT, this is due to pcieport driver enabling MSI of the bridge
> > device (16c3:abcd) via pcie_port_device_register() ->
> > pcie_init_service_irqs() -> pcie_port_enable_irq_vec() -> etc.
> >
> > I did an experiment on a i.MX8MQ/PCIE -> i210 setup I have: I disabled
> > CONFIG_PCIEPORTBUS and hacked igb_main.c enough to make the i210
> > driver believe it should fall back onto legacy interrupts. Even
> > without pcieport present in the system, i210 worked as expected via
> > legacy interrupts, which seems to collaborate my conjecture above.
> >
> > Thanks,
> > Andrey Smirnov
>
> IIUC PCIEPORTBUS should not be needed for MSIs to work,
> it is only needed if you want e.g. PME or AER.
>
> The difference is that if PCIEPORTBUS is enabled, a MSI irq vector will be
> allocated for the Root Complex itself, so that it can send an irq when
> e.g. AER has detected an error.
>
>
> If we disregard that MSI handling is currently broken on DWC PCIe:
> https://marc.info/?l=linux-pci&m=154214986924244&w=2
> It is very possible to have MSIs on dragonboard 820c, which also
> uses the DWC PCIe controller, without having PCIEPORTBUS selected:
>
> # zcat /proc/config.gz | grep -E "PCIE_QCOM|PCIEPORTBUS"
> # CONFIG_PCIEPORTBUS is not set
> CONFIG_PCIE_QCOM=y
>
>
> # lspci -v -s 0000:00:00.0
> 0000:00:00.0 PCI bridge: Qualcomm Device 0104 (prog-if 00 [Normal decode])
>         ...
>         Capabilities: [50] MSI: Enable- Count=1/32 Maskable+ 64bit+
>
> # lspci -v -s 0000:01:00.0
> 0000:01:00.0 Network controller: Qualcomm Atheros QCA6174 802.11ac Wireless Network Adapter (rev 32)
>         ...
>         Capabilities: [50] MSI: Enable+ Count=1/8 Maskable+ 64bit-
>
>
> # cat /proc/interrupts | grep MSI
>  70:       5620          0          0          0   PCI-MSI 524288 Edge      ath10k_pci
>
> So perhaps this is a bug specific to imx6?
>

Yeah, that seems entirely plausible. I reached out to NXP via one of
the support channels to clarify. I'll report if I hear back from them.

Thanks,
Andrey Smirnov

RE: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on PCIEPORTBUS

[Richard Zhu]

> -----Original Message-----
> From: Andrey Smirnov [mailto:andrew.smirnov@gmail.com]
> Sent: 2018年12月8日 7:57
> To: niklas.cassel@linaro.org
> Cc: Lucas Stach <l.stach@pengutronix.de>; linux-pci@vger.kernel.org; Bjorn
> Helgaas <bhelgaas@google.com>; Chris Healy <cphealy@gmail.com>;
> Leonard Crestez <leonard.crestez@nxp.com>; Aisheng Dong
> <aisheng.dong@nxp.com>; Richard Zhu <hongxing.zhu@nxp.com>;
> dl-linux-imx <linux-imx@nxp.com>; linux-arm-kernel
> <linux-arm-kernel@lists.infradead.org>; linux-kernel
> <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH] PCI: controller: dwc: Make PCI_IMX6 depend on
> PCIEPORTBUS
> 
> On Fri, Dec 7, 2018 at 5:11 AM Niklas Cassel <niklas.cassel@linaro.org>
> wrote:
> >
> > On Thu, Dec 06, 2018 at 08:55:13PM -0800, Andrey Smirnov wrote:
> > > On Thu, Dec 6, 2018 at 2:28 AM Lucas Stach <l.stach@pengutronix.de>
> wrote:
> > > >
> > > > Am Mittwoch, den 05.12.2018, 23:45 -0800 schrieb Andrey Smirnov:
> > > > > Building a kernel with CONFIG_PCI_IMX6=y, but
> > > > > CONFIG_PCIEPORTBUS=n produces a system where built-in PCIE
> > > > > bridge (16c3:abcd) isn't bound to pcieport driver. This, in
> > > > > turn, results in a PCIE bus that is capable of enumerating
> > > > > attached PCIE device, but lacks functional interrupt support.
> > > >
> > > > This is odd. AFAIK PCI port services are a totally optional thing
> > > > and them being absent should not lead to a non-functional PCI bus.
> > > > So I would really like to see some deeper analysis what is going on here.
> > > >
> > >
> > > AFAICT, this is due to pcieport driver enabling MSI of the bridge
> > > device (16c3:abcd) via pcie_port_device_register() ->
> > > pcie_init_service_irqs() -> pcie_port_enable_irq_vec() -> etc.
> > >
> > > I did an experiment on a i.MX8MQ/PCIE -> i210 setup I have: I
> > > disabled CONFIG_PCIEPORTBUS and hacked igb_main.c enough to make
> the
> > > i210 driver believe it should fall back onto legacy interrupts. Even
> > > without pcieport present in the system, i210 worked as expected via
> > > legacy interrupts, which seems to collaborate my conjecture above.
> > >
> > > Thanks,
> > > Andrey Smirnov
> >
> > IIUC PCIEPORTBUS should not be needed for MSIs to work, it is only
> > needed if you want e.g. PME or AER.
> >
> > The difference is that if PCIEPORTBUS is enabled, a MSI irq vector
> > will be allocated for the Root Complex itself, so that it can send an
> > irq when e.g. AER has detected an error.
> >
> >
> > If we disregard that MSI handling is currently broken on DWC PCIe:
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmar
> >
> c.info%2F%3Fl%3Dlinux-pci%26m%3D154214986924244%26w%3D2&amp;d
> ata=02%7C
> >
> 01%7Chongxing.zhu%40nxp.com%7Cd0e75c24b7504c3f395508d65c9fc1a3%
> 7C686ea
> >
> 1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C636798238552985486&amp;
> sdata=rwd
> > edF0SijZqkFrpSHFq7uLmXeftGag0pJyT9XC4NOc%3D&amp;reserved=0
> > It is very possible to have MSIs on dragonboard 820c, which also uses
> > the DWC PCIe controller, without having PCIEPORTBUS selected:
> >
> > # zcat /proc/config.gz | grep -E "PCIE_QCOM|PCIEPORTBUS"
> > # CONFIG_PCIEPORTBUS is not set
> > CONFIG_PCIE_QCOM=y
> >
> >
> > # lspci -v -s 0000:00:00.0
> > 0000:00:00.0 PCI bridge: Qualcomm Device 0104 (prog-if 00 [Normal
> decode])
> >         ...
> >         Capabilities: [50] MSI: Enable- Count=1/32 Maskable+ 64bit+
> >
> > # lspci -v -s 0000:01:00.0
> > 0000:01:00.0 Network controller: Qualcomm Atheros QCA6174 802.11ac
> Wireless Network Adapter (rev 32)
> >         ...
> >         Capabilities: [50] MSI: Enable+ Count=1/8 Maskable+ 64bit-
> >
> >
> > # cat /proc/interrupts | grep MSI
> >  70:       5620          0          0          0   PCI-MSI
> 524288 Edge      ath10k_pci
> >
> > So perhaps this is a bug specific to imx6?
> >
> 
> Yeah, that seems entirely plausible. I reached out to NXP via one of the
> support channels to clarify. I'll report if I hear back from them.
> 
[Richard Zhu] Did some tests.
The MSI_EN of the MSI CAP of RC should be set to 1 on iMX6 PCIe.
Otherwise, the MSI wouldn't be triggered although the device is present and the MSIs are assigned.

> Thanks,
> Andrey Smirnov