All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] imx6: fix pcie enumeration
@ 2018-01-04 15:48 Koen Vandeputte
  2018-01-04 20:24   ` Bjorn Helgaas
  0 siblings, 1 reply; 25+ messages in thread
From: Koen Vandeputte @ 2018-01-04 15:48 UTC (permalink / raw)
  To: linux-pci; +Cc: bhelgaas, Koen Vandeputte

By default, when the imx6 PCIe RC boots up, the subordinate is set
equally to the secondary bus (1), and does not alter afterwards.

This means that theoretically, the highest bus reachable downstream is
bus 1.

Before commit a20c7f36bd3d ("PCI: Do not allocate more buses than
available in parent"), the driver ignored the subord value and just
allowed up to 0xff on each device downstream.

This caused a lot of errors to be printed, as this is not logical
according to spec. (but it worked ..)

After this commit, the driver stopped scanning deeper when the last
allocated busnr equals the subordinate of it's master, causing devices
to be undiscovered (especially behind bridges), uncovering the impact of
this bug.

Before:

00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 ...
	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0

00:00.0 0604: 16c3:abcd (rev 01)
01:00.0 0604: 10b5:8604 (rev ba)
... stops after bus 1 ...

After:

00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
...
	Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0

00:00.0 0604: 16c3:abcd (rev 01)
01:00.0 0604: 10b5:8604 (rev ba)
02:01.0 0604: 10b5:8604 (rev ba)
02:04.0 0604: 10b5:8604 (rev ba)
02:05.0 0604: 10b5:8604 (rev ba)
03:00.0 0280: 168c:0033 (rev 01)
05:00.0 0280: 168c:0033 (rev 01)

Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
---

Needs backports to 4.14 & 4.9 stables


 drivers/pci/dwc/pci-imx6.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
index b73483534a5b..3d13fa8c2eb1 100644
--- a/drivers/pci/dwc/pci-imx6.c
+++ b/drivers/pci/dwc/pci-imx6.c
@@ -76,6 +76,9 @@ struct imx6_pcie {
 
 #define PCIE_RC_LCSR				0x80
 
+#define PCIE_RC_BNR				0x18
+#define PCIE_RC_BNR_MAX_SUBORDINATE		(0xff << 16)
+
 /* PCIe Port Logic registers (memory-mapped) */
 #define PL_OFFSET 0x700
 #define PCIE_PL_PFLR (PL_OFFSET + 0x08)
@@ -562,6 +565,17 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
 	int ret;
 
 	/*
+	 * By default, the subordinate is set equally to the secondary
+	 * bus (0x01) when the RC boots.
+	 * This means that theoretically, only bus 1 is reachable from the RC.
+	 * Force the PCIe RC subordinate to 0xff, otherwise no downstream
+	 * devices will be detected behind bus 1.
+	*/
+	tmp = dw_pcie_readl_rc(pp, PCIE_RC_BNR);
+	tmp |= PCIE_RC_BNR_MAX_SUBORDINATE;
+	dw_pcie_writel_rc(pp, PCIE_RC_BNR, tmp);
+
+	/*
 	 * Force Gen1 operation when starting the link.  In case the link is
 	 * started in Gen2 mode, there is a possibility the devices on the
 	 * bus will not be detected at all.  This happens with PCIe switches.
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] imx6: fix pcie enumeration
  2018-01-04 15:48 [PATCH] imx6: fix pcie enumeration Koen Vandeputte
@ 2018-01-04 20:24   ` Bjorn Helgaas
  0 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2018-01-04 20:24 UTC (permalink / raw)
  To: Koen Vandeputte
  Cc: Lorenzo Pieralisi, Richard Zhu, linux-pci, bhelgaas,
	linux-arm-kernel, Lucas Stach

[+cc Richard, Lucas, Lorenzo, linux-arm-kernel]

Hi Koen,

Thanks a lot for the fix!

Please run "git log --oneline drivers/pci/dwc/pci-imx6.c" and follow
the style convention, including "PCIe" capitalization.

"fix pcie enumeration" is not very descriptive.  It should say something
about the specific problem, e.g., setting the root port's subordinate
bus number.

On Thu, Jan 04, 2018 at 04:48:03PM +0100, Koen Vandeputte wrote:
> By default, when the imx6 PCIe RC boots up, the subordinate is set

Not sure what "RC boots up" means.  Maybe you're talking about a
hardware-defined default value at power-up, or maybe a value
programmed by a boot-loader?  Please clarify and update the similar
comment in the code.

> equally to the secondary bus (1), and does not alter afterwards.
> 
> This means that theoretically, the highest bus reachable downstream is
> bus 1.

Not just theoretically.  If the bridge is operating correctly, it
should not forward any config transaction for a bus number greater
than the subordinate bus number.

> Before commit a20c7f36bd3d ("PCI: Do not allocate more buses than
> available in parent"), the driver ignored the subord value and just
> allowed up to 0xff on each device downstream.
> 
> This caused a lot of errors to be printed, as this is not logical
> according to spec. (but it worked ..)

Including a sample error in the changelog might help somebody
suffering from the problem find this solution.

> After this commit, the driver stopped scanning deeper when the last
> allocated busnr equals the subordinate of it's master, causing devices
> to be undiscovered (especially behind bridges), uncovering the impact of
> this bug.
> 
> Before:
> 
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 ...
> 	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> 
> 00:00.0 0604: 16c3:abcd (rev 01)
> 01:00.0 0604: 10b5:8604 (rev ba)
> ... stops after bus 1 ...
> 
> After:
> 
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> ...
> 	Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> 
> 00:00.0 0604: 16c3:abcd (rev 01)
> 01:00.0 0604: 10b5:8604 (rev ba)
> 02:01.0 0604: 10b5:8604 (rev ba)
> 02:04.0 0604: 10b5:8604 (rev ba)
> 02:05.0 0604: 10b5:8604 (rev ba)
> 03:00.0 0280: 168c:0033 (rev 01)
> 05:00.0 0280: 168c:0033 (rev 01)
> 

Should have a "Fixes: a20c7f36bd3d ..." tag if that's really the
correct commit.

> Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
> ---
> 
> Needs backports to 4.14 & 4.9 stables

Add a "CC: stable@vger.kernel.org" after your signed-off-by to handle
this.  But something's wrong because a20c7f36bd3d only appeared in
v4.15-rc1, so either that's the wrong commit or stable kernels don't
need the fix.

>  drivers/pci/dwc/pci-imx6.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> index b73483534a5b..3d13fa8c2eb1 100644
> --- a/drivers/pci/dwc/pci-imx6.c
> +++ b/drivers/pci/dwc/pci-imx6.c
> @@ -76,6 +76,9 @@ struct imx6_pcie {
>  
>  #define PCIE_RC_LCSR				0x80
>  
> +#define PCIE_RC_BNR				0x18
> +#define PCIE_RC_BNR_MAX_SUBORDINATE		(0xff << 16)
> +
>  /* PCIe Port Logic registers (memory-mapped) */
>  #define PL_OFFSET 0x700
>  #define PCIE_PL_PFLR (PL_OFFSET + 0x08)
> @@ -562,6 +565,17 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>  	int ret;
>  
>  	/*
> +	 * By default, the subordinate is set equally to the secondary
> +	 * bus (0x01) when the RC boots.
> +	 * This means that theoretically, only bus 1 is reachable from the RC.
> +	 * Force the PCIe RC subordinate to 0xff, otherwise no downstream
> +	 * devices will be detected behind bus 1.
> +	*/
> +	tmp = dw_pcie_readl_rc(pp, PCIE_RC_BNR);
> +	tmp |= PCIE_RC_BNR_MAX_SUBORDINATE;
> +	dw_pcie_writel_rc(pp, PCIE_RC_BNR, tmp);

This functionality is not related to establishing the link, so I think
it should be put elsewhere, maybe either directly in imx6_pcie_probe()
or in imx6_pcie_host_init().

The DT really should contain a "bus-range" property and
dw_pcie_host_init() will already pay attention to that if it's
present, and I think it defaults to 0-0xff if it's not present.

So I think you should be using pp->busn instead of hard-coding
PCIE_RC_BNR_MAX_SUBORDINATE.

> +	/*
>  	 * Force Gen1 operation when starting the link.  In case the link is
>  	 * started in Gen2 mode, there is a possibility the devices on the
>  	 * bus will not be detected at all.  This happens with PCIe switches.
> -- 
> 2.7.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-04 20:24   ` Bjorn Helgaas
  0 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2018-01-04 20:24 UTC (permalink / raw)
  To: linux-arm-kernel

[+cc Richard, Lucas, Lorenzo, linux-arm-kernel]

Hi Koen,

Thanks a lot for the fix!

Please run "git log --oneline drivers/pci/dwc/pci-imx6.c" and follow
the style convention, including "PCIe" capitalization.

"fix pcie enumeration" is not very descriptive.  It should say something
about the specific problem, e.g., setting the root port's subordinate
bus number.

On Thu, Jan 04, 2018 at 04:48:03PM +0100, Koen Vandeputte wrote:
> By default, when the imx6 PCIe RC boots up, the subordinate is set

Not sure what "RC boots up" means.  Maybe you're talking about a
hardware-defined default value at power-up, or maybe a value
programmed by a boot-loader?  Please clarify and update the similar
comment in the code.

> equally to the secondary bus (1), and does not alter afterwards.
> 
> This means that theoretically, the highest bus reachable downstream is
> bus 1.

Not just theoretically.  If the bridge is operating correctly, it
should not forward any config transaction for a bus number greater
than the subordinate bus number.

> Before commit a20c7f36bd3d ("PCI: Do not allocate more buses than
> available in parent"), the driver ignored the subord value and just
> allowed up to 0xff on each device downstream.
> 
> This caused a lot of errors to be printed, as this is not logical
> according to spec. (but it worked ..)

Including a sample error in the changelog might help somebody
suffering from the problem find this solution.

> After this commit, the driver stopped scanning deeper when the last
> allocated busnr equals the subordinate of it's master, causing devices
> to be undiscovered (especially behind bridges), uncovering the impact of
> this bug.
> 
> Before:
> 
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 ...
> 	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> 
> 00:00.0 0604: 16c3:abcd (rev 01)
> 01:00.0 0604: 10b5:8604 (rev ba)
> ... stops after bus 1 ...
> 
> After:
> 
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> ...
> 	Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> 
> 00:00.0 0604: 16c3:abcd (rev 01)
> 01:00.0 0604: 10b5:8604 (rev ba)
> 02:01.0 0604: 10b5:8604 (rev ba)
> 02:04.0 0604: 10b5:8604 (rev ba)
> 02:05.0 0604: 10b5:8604 (rev ba)
> 03:00.0 0280: 168c:0033 (rev 01)
> 05:00.0 0280: 168c:0033 (rev 01)
> 

Should have a "Fixes: a20c7f36bd3d ..." tag if that's really the
correct commit.

> Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
> ---
> 
> Needs backports to 4.14 & 4.9 stables

Add a "CC: stable at vger.kernel.org" after your signed-off-by to handle
this.  But something's wrong because a20c7f36bd3d only appeared in
v4.15-rc1, so either that's the wrong commit or stable kernels don't
need the fix.

>  drivers/pci/dwc/pci-imx6.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> index b73483534a5b..3d13fa8c2eb1 100644
> --- a/drivers/pci/dwc/pci-imx6.c
> +++ b/drivers/pci/dwc/pci-imx6.c
> @@ -76,6 +76,9 @@ struct imx6_pcie {
>  
>  #define PCIE_RC_LCSR				0x80
>  
> +#define PCIE_RC_BNR				0x18
> +#define PCIE_RC_BNR_MAX_SUBORDINATE		(0xff << 16)
> +
>  /* PCIe Port Logic registers (memory-mapped) */
>  #define PL_OFFSET 0x700
>  #define PCIE_PL_PFLR (PL_OFFSET + 0x08)
> @@ -562,6 +565,17 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>  	int ret;
>  
>  	/*
> +	 * By default, the subordinate is set equally to the secondary
> +	 * bus (0x01) when the RC boots.
> +	 * This means that theoretically, only bus 1 is reachable from the RC.
> +	 * Force the PCIe RC subordinate to 0xff, otherwise no downstream
> +	 * devices will be detected behind bus 1.
> +	*/
> +	tmp = dw_pcie_readl_rc(pp, PCIE_RC_BNR);
> +	tmp |= PCIE_RC_BNR_MAX_SUBORDINATE;
> +	dw_pcie_writel_rc(pp, PCIE_RC_BNR, tmp);

This functionality is not related to establishing the link, so I think
it should be put elsewhere, maybe either directly in imx6_pcie_probe()
or in imx6_pcie_host_init().

The DT really should contain a "bus-range" property and
dw_pcie_host_init() will already pay attention to that if it's
present, and I think it defaults to 0-0xff if it's not present.

So I think you should be using pp->busn instead of hard-coding
PCIE_RC_BNR_MAX_SUBORDINATE.

> +	/*
>  	 * Force Gen1 operation when starting the link.  In case the link is
>  	 * started in Gen2 mode, there is a possibility the devices on the
>  	 * bus will not be detected at all.  This happens with PCIe switches.
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] imx6: fix pcie enumeration
  2018-01-04 20:24   ` Bjorn Helgaas
@ 2018-01-05  9:56     ` Koen Vandeputte
  -1 siblings, 0 replies; 25+ messages in thread
From: Koen Vandeputte @ 2018-01-05  9:56 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, bhelgaas, Richard Zhu, Lucas Stach, Lorenzo Pieralisi,
	linux-arm-kernel



On 2018-01-04 21:24, Bjorn Helgaas wrote:
> [+cc Richard, Lucas, Lorenzo, linux-arm-kernel]
>
> Hi Koen,
>
> Thanks a lot for the fix!
>
> Please run "git log --oneline drivers/pci/dwc/pci-imx6.c" and follow
> the style convention, including "PCIe" capitalization.
>
> "fix pcie enumeration" is not very descriptive.  It should say something
> about the specific problem, e.g., setting the root port's subordinate
> bus number.
>
> On Thu, Jan 04, 2018 at 04:48:03PM +0100, Koen Vandeputte wrote:
>> By default, when the imx6 PCIe RC boots up, the subordinate is set
> Not sure what "RC boots up" means.  Maybe you're talking about a
> hardware-defined default value at power-up, or maybe a value
> programmed by a boot-loader?  Please clarify and update the similar
> comment in the code.
>
>> equally to the secondary bus (1), and does not alter afterwards.
>>
>> This means that theoretically, the highest bus reachable downstream is
>> bus 1.
> Not just theoretically.  If the bridge is operating correctly, it
> should not forward any config transaction for a bus number greater
> than the subordinate bus number.
>
>> Before commit a20c7f36bd3d ("PCI: Do not allocate more buses than
>> available in parent"), the driver ignored the subord value and just
>> allowed up to 0xff on each device downstream.
>>
>> This caused a lot of errors to be printed, as this is not logical
>> according to spec. (but it worked ..)
> Including a sample error in the changelog might help somebody
> suffering from the problem find this solution.
>
>> After this commit, the driver stopped scanning deeper when the last
>> allocated busnr equals the subordinate of it's master, causing devices
>> to be undiscovered (especially behind bridges), uncovering the impact of
>> this bug.
>>
>> Before:
>>
>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 ...
>> 	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>>
>> 00:00.0 0604: 16c3:abcd (rev 01)
>> 01:00.0 0604: 10b5:8604 (rev ba)
>> ... stops after bus 1 ...
>>
>> After:
>>
>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
>> ...
>> 	Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
>>
>> 00:00.0 0604: 16c3:abcd (rev 01)
>> 01:00.0 0604: 10b5:8604 (rev ba)
>> 02:01.0 0604: 10b5:8604 (rev ba)
>> 02:04.0 0604: 10b5:8604 (rev ba)
>> 02:05.0 0604: 10b5:8604 (rev ba)
>> 03:00.0 0280: 168c:0033 (rev 01)
>> 05:00.0 0280: 168c:0033 (rev 01)
>>
> Should have a "Fixes: a20c7f36bd3d ..." tag if that's really the
> correct commit.
>
>> Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
>> ---
>>
>> Needs backports to 4.14 & 4.9 stables
> Add a "CC: stable@vger.kernel.org" after your signed-off-by to handle
> this.  But something's wrong because a20c7f36bd3d only appeared in
> v4.15-rc1, so either that's the wrong commit or stable kernels don't
> need the fix.
>
>>   drivers/pci/dwc/pci-imx6.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
>> index b73483534a5b..3d13fa8c2eb1 100644
>> --- a/drivers/pci/dwc/pci-imx6.c
>> +++ b/drivers/pci/dwc/pci-imx6.c
>> @@ -76,6 +76,9 @@ struct imx6_pcie {
>>   
>>   #define PCIE_RC_LCSR				0x80
>>   
>> +#define PCIE_RC_BNR				0x18
>> +#define PCIE_RC_BNR_MAX_SUBORDINATE		(0xff << 16)
>> +
>>   /* PCIe Port Logic registers (memory-mapped) */
>>   #define PL_OFFSET 0x700
>>   #define PCIE_PL_PFLR (PL_OFFSET + 0x08)
>> @@ -562,6 +565,17 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>>   	int ret;
>>   
>>   	/*
>> +	 * By default, the subordinate is set equally to the secondary
>> +	 * bus (0x01) when the RC boots.
>> +	 * This means that theoretically, only bus 1 is reachable from the RC.
>> +	 * Force the PCIe RC subordinate to 0xff, otherwise no downstream
>> +	 * devices will be detected behind bus 1.
>> +	*/
>> +	tmp = dw_pcie_readl_rc(pp, PCIE_RC_BNR);
>> +	tmp |= PCIE_RC_BNR_MAX_SUBORDINATE;
>> +	dw_pcie_writel_rc(pp, PCIE_RC_BNR, tmp);
> This functionality is not related to establishing the link, so I think
> it should be put elsewhere, maybe either directly in imx6_pcie_probe()
> or in imx6_pcie_host_init().
>
> The DT really should contain a "bus-range" property and
> dw_pcie_host_init() will already pay attention to that if it's
> present, and I think it defaults to 0-0xff if it's not present.
>
> So I think you should be using pp->busn instead of hard-coding
> PCIE_RC_BNR_MAX_SUBORDINATE.
>
>> +	/*
>>   	 * Force Gen1 operation when starting the link.  In case the link is
>>   	 * started in Gen2 mode, there is a possibility the devices on the
>>   	 * bus will not be detected at all.  This happens with PCIe switches.
>> -- 
>> 2.7.4
>>




Hi Bjorn,

Thanks for your time and patience writing extended comments on all points,
especially since this is my first commit to this list.

Highly appreciated!


Secondly,

Based on your suggestions, I've dug around deeper in the code and found 
the actual line that initially causes it:  [1]


*drivers/pci/dwc/pcie-designware-host.c* 
<http://elixir.free-electrons.com/linux/v4.15-rc6/source/drivers/pci/dwc/pcie-designware-host.c#L588> 
void  dw_pcie_setup_rc 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_setup_rc>(struct  pcie_port <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pcie_port>  *pp <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pp>)
{
...

	/* setup bus numbers */
	val  =  dw_pcie_readl_dbi 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_readl_dbi>(pci 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pci>,  PCI_PRIMARY_BUS 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/PCI_PRIMARY_BUS>);
	val  &=  0xff000000;
	val  |=  0x00010100; <---
	dw_pcie_writel_dbi 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_writel_dbi>(pci 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pci>,  PCI_PRIMARY_BUS 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/PCI_PRIMARY_BUS>,  val); ... The i.MX6 reference manual (page 417 - section 48.8.7 "Bus 
Number Registers (PCIE_RC_BNR)") shows the following layout [2]: 31 23 
15 7 0 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
| Sec lat timer | Subord num | Sec busnum | Prim busnum | 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
Shouldn't this:"val |= 0x00010100;" change to: "val |= 0x00ff0100;"
?



It seems other platforms also use this function (and thus register layout) to setup the PCIe RC [3],
so instead of doing a single fix for i.MX6 maybe it's best to fix it here so other platforms also benefit?

As you know the code a lot better than me, what's your opinion on this? (or from anyone in CC)


[1] http://elixir.free-electrons.com/linux/v4.15-rc6/source/drivers/pci/dwc/pcie-designware-host.c#L588
[2] https://community.nxp.com/servlet/JiveServlet/downloadBody/101783-102-6-17831/IMX6DQRMr2_part2.pdf
[3] http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_setup_rc


Koen

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-05  9:56     ` Koen Vandeputte
  0 siblings, 0 replies; 25+ messages in thread
From: Koen Vandeputte @ 2018-01-05  9:56 UTC (permalink / raw)
  To: linux-arm-kernel



On 2018-01-04 21:24, Bjorn Helgaas wrote:
> [+cc Richard, Lucas, Lorenzo, linux-arm-kernel]
>
> Hi Koen,
>
> Thanks a lot for the fix!
>
> Please run "git log --oneline drivers/pci/dwc/pci-imx6.c" and follow
> the style convention, including "PCIe" capitalization.
>
> "fix pcie enumeration" is not very descriptive.  It should say something
> about the specific problem, e.g., setting the root port's subordinate
> bus number.
>
> On Thu, Jan 04, 2018 at 04:48:03PM +0100, Koen Vandeputte wrote:
>> By default, when the imx6 PCIe RC boots up, the subordinate is set
> Not sure what "RC boots up" means.  Maybe you're talking about a
> hardware-defined default value at power-up, or maybe a value
> programmed by a boot-loader?  Please clarify and update the similar
> comment in the code.
>
>> equally to the secondary bus (1), and does not alter afterwards.
>>
>> This means that theoretically, the highest bus reachable downstream is
>> bus 1.
> Not just theoretically.  If the bridge is operating correctly, it
> should not forward any config transaction for a bus number greater
> than the subordinate bus number.
>
>> Before commit a20c7f36bd3d ("PCI: Do not allocate more buses than
>> available in parent"), the driver ignored the subord value and just
>> allowed up to 0xff on each device downstream.
>>
>> This caused a lot of errors to be printed, as this is not logical
>> according to spec. (but it worked ..)
> Including a sample error in the changelog might help somebody
> suffering from the problem find this solution.
>
>> After this commit, the driver stopped scanning deeper when the last
>> allocated busnr equals the subordinate of it's master, causing devices
>> to be undiscovered (especially behind bridges), uncovering the impact of
>> this bug.
>>
>> Before:
>>
>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 ...
>> 	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>>
>> 00:00.0 0604: 16c3:abcd (rev 01)
>> 01:00.0 0604: 10b5:8604 (rev ba)
>> ... stops after bus 1 ...
>>
>> After:
>>
>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
>> ...
>> 	Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
>>
>> 00:00.0 0604: 16c3:abcd (rev 01)
>> 01:00.0 0604: 10b5:8604 (rev ba)
>> 02:01.0 0604: 10b5:8604 (rev ba)
>> 02:04.0 0604: 10b5:8604 (rev ba)
>> 02:05.0 0604: 10b5:8604 (rev ba)
>> 03:00.0 0280: 168c:0033 (rev 01)
>> 05:00.0 0280: 168c:0033 (rev 01)
>>
> Should have a "Fixes: a20c7f36bd3d ..." tag if that's really the
> correct commit.
>
>> Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
>> ---
>>
>> Needs backports to 4.14 & 4.9 stables
> Add a "CC: stable at vger.kernel.org" after your signed-off-by to handle
> this.  But something's wrong because a20c7f36bd3d only appeared in
> v4.15-rc1, so either that's the wrong commit or stable kernels don't
> need the fix.
>
>>   drivers/pci/dwc/pci-imx6.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
>> index b73483534a5b..3d13fa8c2eb1 100644
>> --- a/drivers/pci/dwc/pci-imx6.c
>> +++ b/drivers/pci/dwc/pci-imx6.c
>> @@ -76,6 +76,9 @@ struct imx6_pcie {
>>   
>>   #define PCIE_RC_LCSR				0x80
>>   
>> +#define PCIE_RC_BNR				0x18
>> +#define PCIE_RC_BNR_MAX_SUBORDINATE		(0xff << 16)
>> +
>>   /* PCIe Port Logic registers (memory-mapped) */
>>   #define PL_OFFSET 0x700
>>   #define PCIE_PL_PFLR (PL_OFFSET + 0x08)
>> @@ -562,6 +565,17 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>>   	int ret;
>>   
>>   	/*
>> +	 * By default, the subordinate is set equally to the secondary
>> +	 * bus (0x01) when the RC boots.
>> +	 * This means that theoretically, only bus 1 is reachable from the RC.
>> +	 * Force the PCIe RC subordinate to 0xff, otherwise no downstream
>> +	 * devices will be detected behind bus 1.
>> +	*/
>> +	tmp = dw_pcie_readl_rc(pp, PCIE_RC_BNR);
>> +	tmp |= PCIE_RC_BNR_MAX_SUBORDINATE;
>> +	dw_pcie_writel_rc(pp, PCIE_RC_BNR, tmp);
> This functionality is not related to establishing the link, so I think
> it should be put elsewhere, maybe either directly in imx6_pcie_probe()
> or in imx6_pcie_host_init().
>
> The DT really should contain a "bus-range" property and
> dw_pcie_host_init() will already pay attention to that if it's
> present, and I think it defaults to 0-0xff if it's not present.
>
> So I think you should be using pp->busn instead of hard-coding
> PCIE_RC_BNR_MAX_SUBORDINATE.
>
>> +	/*
>>   	 * Force Gen1 operation when starting the link.  In case the link is
>>   	 * started in Gen2 mode, there is a possibility the devices on the
>>   	 * bus will not be detected at all.  This happens with PCIe switches.
>> -- 
>> 2.7.4
>>




Hi Bjorn,

Thanks for your time and patience writing extended comments on all points,
especially since this is my first commit to this list.

Highly appreciated!


Secondly,

Based on your suggestions, I've dug around deeper in the code and found 
the actual line that initially causes it:? [1]


*drivers/pci/dwc/pcie-designware-host.c* 
<http://elixir.free-electrons.com/linux/v4.15-rc6/source/drivers/pci/dwc/pcie-designware-host.c#L588> 
void  dw_pcie_setup_rc 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_setup_rc>(struct  pcie_port <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pcie_port>  *pp <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pp>)
{
...

	/* setup bus numbers */
	val  =  dw_pcie_readl_dbi 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_readl_dbi>(pci 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pci>,  PCI_PRIMARY_BUS 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/PCI_PRIMARY_BUS>);
	val  &=  0xff000000;
	val  |=  0x00010100; <---
	dw_pcie_writel_dbi 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_writel_dbi>(pci 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pci>,  PCI_PRIMARY_BUS 
<http://elixir.free-electrons.com/linux/v4.15-rc6/ident/PCI_PRIMARY_BUS>,  val); ... The i.MX6 reference manual (page 417 - section 48.8.7 "Bus 
Number Registers (PCIE_RC_BNR)") shows the following layout [2]: 31 23 
15 7 0 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
| Sec lat timer | Subord num | Sec busnum | Prim busnum | 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
Shouldn't this:"val |= 0x00010100;" change to: "val |= 0x00ff0100;"
?



It seems other platforms also use this function (and thus register layout) to setup the PCIe RC [3],
so instead of doing a single fix for i.MX6 maybe it's best to fix it here so other platforms also benefit?

As you know the code a lot better than me, what's your opinion on this? (or from anyone in CC)


[1] http://elixir.free-electrons.com/linux/v4.15-rc6/source/drivers/pci/dwc/pcie-designware-host.c#L588
[2] https://community.nxp.com/servlet/JiveServlet/downloadBody/101783-102-6-17831/IMX6DQRMr2_part2.pdf
[3] http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_setup_rc


Koen

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] imx6: fix pcie enumeration
  2018-01-05  9:56     ` Koen Vandeputte
@ 2018-01-05 12:32       ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 25+ messages in thread
From: Lorenzo Pieralisi @ 2018-01-05 12:32 UTC (permalink / raw)
  To: Koen Vandeputte
  Cc: Bjorn Helgaas, linux-pci, bhelgaas, Richard Zhu, Lucas Stach,
	linux-arm-kernel

On Fri, Jan 05, 2018 at 10:56:31AM +0100, Koen Vandeputte wrote:
> 
> 
> On 2018-01-04 21:24, Bjorn Helgaas wrote:
> >[+cc Richard, Lucas, Lorenzo, linux-arm-kernel]
> >
> >Hi Koen,
> >
> >Thanks a lot for the fix!
> >
> >Please run "git log --oneline drivers/pci/dwc/pci-imx6.c" and follow
> >the style convention, including "PCIe" capitalization.
> >
> >"fix pcie enumeration" is not very descriptive.  It should say something
> >about the specific problem, e.g., setting the root port's subordinate
> >bus number.
> >
> >On Thu, Jan 04, 2018 at 04:48:03PM +0100, Koen Vandeputte wrote:
> >>By default, when the imx6 PCIe RC boots up, the subordinate is set
> >Not sure what "RC boots up" means.  Maybe you're talking about a
> >hardware-defined default value at power-up, or maybe a value
> >programmed by a boot-loader?  Please clarify and update the similar
> >comment in the code.
> >
> >>equally to the secondary bus (1), and does not alter afterwards.
> >>
> >>This means that theoretically, the highest bus reachable downstream is
> >>bus 1.
> >Not just theoretically.  If the bridge is operating correctly, it
> >should not forward any config transaction for a bus number greater
> >than the subordinate bus number.
> >
> >>Before commit a20c7f36bd3d ("PCI: Do not allocate more buses than
> >>available in parent"), the driver ignored the subord value and just
> >>allowed up to 0xff on each device downstream.
> >>
> >>This caused a lot of errors to be printed, as this is not logical
> >>according to spec. (but it worked ..)
> >Including a sample error in the changelog might help somebody
> >suffering from the problem find this solution.
> >
> >>After this commit, the driver stopped scanning deeper when the last
> >>allocated busnr equals the subordinate of it's master, causing devices
> >>to be undiscovered (especially behind bridges), uncovering the impact of
> >>this bug.
> >>
> >>Before:
> >>
> >>00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 ...
> >>	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> >>
> >>00:00.0 0604: 16c3:abcd (rev 01)
> >>01:00.0 0604: 10b5:8604 (rev ba)
> >>... stops after bus 1 ...
> >>
> >>After:
> >>
> >>00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> >>...
> >>	Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> >>
> >>00:00.0 0604: 16c3:abcd (rev 01)
> >>01:00.0 0604: 10b5:8604 (rev ba)
> >>02:01.0 0604: 10b5:8604 (rev ba)
> >>02:04.0 0604: 10b5:8604 (rev ba)
> >>02:05.0 0604: 10b5:8604 (rev ba)
> >>03:00.0 0280: 168c:0033 (rev 01)
> >>05:00.0 0280: 168c:0033 (rev 01)
> >>
> >Should have a "Fixes: a20c7f36bd3d ..." tag if that's really the
> >correct commit.
> >
> >>Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
> >>---
> >>
> >>Needs backports to 4.14 & 4.9 stables
> >Add a "CC: stable@vger.kernel.org" after your signed-off-by to handle
> >this.  But something's wrong because a20c7f36bd3d only appeared in
> >v4.15-rc1, so either that's the wrong commit or stable kernels don't
> >need the fix.
> >
> >>  drivers/pci/dwc/pci-imx6.c | 14 ++++++++++++++
> >>  1 file changed, 14 insertions(+)
> >>
> >>diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> >>index b73483534a5b..3d13fa8c2eb1 100644
> >>--- a/drivers/pci/dwc/pci-imx6.c
> >>+++ b/drivers/pci/dwc/pci-imx6.c
> >>@@ -76,6 +76,9 @@ struct imx6_pcie {
> >>  #define PCIE_RC_LCSR				0x80
> >>+#define PCIE_RC_BNR				0x18
> >>+#define PCIE_RC_BNR_MAX_SUBORDINATE		(0xff << 16)
> >>+
> >>  /* PCIe Port Logic registers (memory-mapped) */
> >>  #define PL_OFFSET 0x700
> >>  #define PCIE_PL_PFLR (PL_OFFSET + 0x08)
> >>@@ -562,6 +565,17 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
> >>  	int ret;
> >>  	/*
> >>+	 * By default, the subordinate is set equally to the secondary
> >>+	 * bus (0x01) when the RC boots.
> >>+	 * This means that theoretically, only bus 1 is reachable from the RC.
> >>+	 * Force the PCIe RC subordinate to 0xff, otherwise no downstream
> >>+	 * devices will be detected behind bus 1.
> >>+	*/
> >>+	tmp = dw_pcie_readl_rc(pp, PCIE_RC_BNR);
> >>+	tmp |= PCIE_RC_BNR_MAX_SUBORDINATE;
> >>+	dw_pcie_writel_rc(pp, PCIE_RC_BNR, tmp);
> >This functionality is not related to establishing the link, so I think
> >it should be put elsewhere, maybe either directly in imx6_pcie_probe()
> >or in imx6_pcie_host_init().
> >
> >The DT really should contain a "bus-range" property and
> >dw_pcie_host_init() will already pay attention to that if it's
> >present, and I think it defaults to 0-0xff if it's not present.
> >
> >So I think you should be using pp->busn instead of hard-coding
> >PCIE_RC_BNR_MAX_SUBORDINATE.
> >
> >>+	/*
> >>  	 * Force Gen1 operation when starting the link.  In case the link is
> >>  	 * started in Gen2 mode, there is a possibility the devices on the
> >>  	 * bus will not be detected at all.  This happens with PCIe switches.
> >>-- 
> >>2.7.4
> >>
> 
> 
> 
> 
> Hi Bjorn,
> 
> Thanks for your time and patience writing extended comments on all points,
> especially since this is my first commit to this list.
> 
> Highly appreciated!
> 
> 
> Secondly,
> 
> Based on your suggestions, I've dug around deeper in the code and found the
> actual line that initially causes it:?? [1]
> 
> 
> *drivers/pci/dwc/pcie-designware-host.c* <http://elixir.free-electrons.com/linux/v4.15-rc6/source/drivers/pci/dwc/pcie-designware-host.c#L588>
> void  dw_pcie_setup_rc <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_setup_rc>(struct
> pcie_port <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pcie_port>
> *pp <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pp>)
> {
> ...
> 
> 	/* setup bus numbers */
> 	val  =  dw_pcie_readl_dbi <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_readl_dbi>(pci
> <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pci>,
> PCI_PRIMARY_BUS
> <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/PCI_PRIMARY_BUS>);
> 	val  &=  0xff000000;
> 	val  |=  0x00010100; <---
> 	dw_pcie_writel_dbi <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_writel_dbi>(pci
> <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pci>,
> PCI_PRIMARY_BUS
> <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/PCI_PRIMARY_BUS>,
> val); ... The i.MX6 reference manual (page 417 - section 48.8.7 "Bus Number
> Registers (PCIE_RC_BNR)") shows the following layout [2]: 31 23 15 7 0
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Sec lat
> timer | Subord num | Sec busnum | Prim busnum |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ Shouldn't
> this:"val |= 0x00010100;" change to: "val |= 0x00ff0100;"
> ?
> 
> 
> 
> It seems other platforms also use this function (and thus register layout) to setup the PCIe RC [3],
> so instead of doing a single fix for i.MX6 maybe it's best to fix it here so other platforms also benefit?
> 
> As you know the code a lot better than me, what's your opinion on this? (or from anyone in CC)

I *think* I understand what's going on - the kernel takes the primary,
secondary and subordinate values in the host bridge as valid in:

pci_scan_bridge_extend()

and given that pcibios_assign_all_busses() returns false (guess) it sets-up
the bus hierarchy with a bus resource with subordinate number as read from
PCI host bridge config space - which, given that it is 1 according to your
explanation - this triggers the issue you reported.

After commit a20c7f36bd3d the root bus resource is propagated down the
hierarchy, hence the problem.

So, in order to fix the issue I think the best way is to programme the
root bridge in:

drivers/pci/dwc/pci-designware-host.c

but with the value coming from the root bus IORESOURCE_BUS resource,
not hardcoding 0xff.

I would kindly ask you to send logs with debug turned on in:

drivers/pci/probe.c

since I would like to check my understanding is correct.

Please CC all dwc host maintainers since this has potential widespread
impact.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-05 12:32       ` Lorenzo Pieralisi
  0 siblings, 0 replies; 25+ messages in thread
From: Lorenzo Pieralisi @ 2018-01-05 12:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 05, 2018 at 10:56:31AM +0100, Koen Vandeputte wrote:
> 
> 
> On 2018-01-04 21:24, Bjorn Helgaas wrote:
> >[+cc Richard, Lucas, Lorenzo, linux-arm-kernel]
> >
> >Hi Koen,
> >
> >Thanks a lot for the fix!
> >
> >Please run "git log --oneline drivers/pci/dwc/pci-imx6.c" and follow
> >the style convention, including "PCIe" capitalization.
> >
> >"fix pcie enumeration" is not very descriptive.  It should say something
> >about the specific problem, e.g., setting the root port's subordinate
> >bus number.
> >
> >On Thu, Jan 04, 2018 at 04:48:03PM +0100, Koen Vandeputte wrote:
> >>By default, when the imx6 PCIe RC boots up, the subordinate is set
> >Not sure what "RC boots up" means.  Maybe you're talking about a
> >hardware-defined default value at power-up, or maybe a value
> >programmed by a boot-loader?  Please clarify and update the similar
> >comment in the code.
> >
> >>equally to the secondary bus (1), and does not alter afterwards.
> >>
> >>This means that theoretically, the highest bus reachable downstream is
> >>bus 1.
> >Not just theoretically.  If the bridge is operating correctly, it
> >should not forward any config transaction for a bus number greater
> >than the subordinate bus number.
> >
> >>Before commit a20c7f36bd3d ("PCI: Do not allocate more buses than
> >>available in parent"), the driver ignored the subord value and just
> >>allowed up to 0xff on each device downstream.
> >>
> >>This caused a lot of errors to be printed, as this is not logical
> >>according to spec. (but it worked ..)
> >Including a sample error in the changelog might help somebody
> >suffering from the problem find this solution.
> >
> >>After this commit, the driver stopped scanning deeper when the last
> >>allocated busnr equals the subordinate of it's master, causing devices
> >>to be undiscovered (especially behind bridges), uncovering the impact of
> >>this bug.
> >>
> >>Before:
> >>
> >>00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 ...
> >>	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> >>
> >>00:00.0 0604: 16c3:abcd (rev 01)
> >>01:00.0 0604: 10b5:8604 (rev ba)
> >>... stops after bus 1 ...
> >>
> >>After:
> >>
> >>00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> >>...
> >>	Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> >>
> >>00:00.0 0604: 16c3:abcd (rev 01)
> >>01:00.0 0604: 10b5:8604 (rev ba)
> >>02:01.0 0604: 10b5:8604 (rev ba)
> >>02:04.0 0604: 10b5:8604 (rev ba)
> >>02:05.0 0604: 10b5:8604 (rev ba)
> >>03:00.0 0280: 168c:0033 (rev 01)
> >>05:00.0 0280: 168c:0033 (rev 01)
> >>
> >Should have a "Fixes: a20c7f36bd3d ..." tag if that's really the
> >correct commit.
> >
> >>Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
> >>---
> >>
> >>Needs backports to 4.14 & 4.9 stables
> >Add a "CC: stable at vger.kernel.org" after your signed-off-by to handle
> >this.  But something's wrong because a20c7f36bd3d only appeared in
> >v4.15-rc1, so either that's the wrong commit or stable kernels don't
> >need the fix.
> >
> >>  drivers/pci/dwc/pci-imx6.c | 14 ++++++++++++++
> >>  1 file changed, 14 insertions(+)
> >>
> >>diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> >>index b73483534a5b..3d13fa8c2eb1 100644
> >>--- a/drivers/pci/dwc/pci-imx6.c
> >>+++ b/drivers/pci/dwc/pci-imx6.c
> >>@@ -76,6 +76,9 @@ struct imx6_pcie {
> >>  #define PCIE_RC_LCSR				0x80
> >>+#define PCIE_RC_BNR				0x18
> >>+#define PCIE_RC_BNR_MAX_SUBORDINATE		(0xff << 16)
> >>+
> >>  /* PCIe Port Logic registers (memory-mapped) */
> >>  #define PL_OFFSET 0x700
> >>  #define PCIE_PL_PFLR (PL_OFFSET + 0x08)
> >>@@ -562,6 +565,17 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
> >>  	int ret;
> >>  	/*
> >>+	 * By default, the subordinate is set equally to the secondary
> >>+	 * bus (0x01) when the RC boots.
> >>+	 * This means that theoretically, only bus 1 is reachable from the RC.
> >>+	 * Force the PCIe RC subordinate to 0xff, otherwise no downstream
> >>+	 * devices will be detected behind bus 1.
> >>+	*/
> >>+	tmp = dw_pcie_readl_rc(pp, PCIE_RC_BNR);
> >>+	tmp |= PCIE_RC_BNR_MAX_SUBORDINATE;
> >>+	dw_pcie_writel_rc(pp, PCIE_RC_BNR, tmp);
> >This functionality is not related to establishing the link, so I think
> >it should be put elsewhere, maybe either directly in imx6_pcie_probe()
> >or in imx6_pcie_host_init().
> >
> >The DT really should contain a "bus-range" property and
> >dw_pcie_host_init() will already pay attention to that if it's
> >present, and I think it defaults to 0-0xff if it's not present.
> >
> >So I think you should be using pp->busn instead of hard-coding
> >PCIE_RC_BNR_MAX_SUBORDINATE.
> >
> >>+	/*
> >>  	 * Force Gen1 operation when starting the link.  In case the link is
> >>  	 * started in Gen2 mode, there is a possibility the devices on the
> >>  	 * bus will not be detected at all.  This happens with PCIe switches.
> >>-- 
> >>2.7.4
> >>
> 
> 
> 
> 
> Hi Bjorn,
> 
> Thanks for your time and patience writing extended comments on all points,
> especially since this is my first commit to this list.
> 
> Highly appreciated!
> 
> 
> Secondly,
> 
> Based on your suggestions, I've dug around deeper in the code and found the
> actual line that initially causes it:?? [1]
> 
> 
> *drivers/pci/dwc/pcie-designware-host.c* <http://elixir.free-electrons.com/linux/v4.15-rc6/source/drivers/pci/dwc/pcie-designware-host.c#L588>
> void  dw_pcie_setup_rc <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_setup_rc>(struct
> pcie_port <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pcie_port>
> *pp <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pp>)
> {
> ...
> 
> 	/* setup bus numbers */
> 	val  =  dw_pcie_readl_dbi <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_readl_dbi>(pci
> <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pci>,
> PCI_PRIMARY_BUS
> <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/PCI_PRIMARY_BUS>);
> 	val  &=  0xff000000;
> 	val  |=  0x00010100; <---
> 	dw_pcie_writel_dbi <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/dw_pcie_writel_dbi>(pci
> <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/pci>,
> PCI_PRIMARY_BUS
> <http://elixir.free-electrons.com/linux/v4.15-rc6/ident/PCI_PRIMARY_BUS>,
> val); ... The i.MX6 reference manual (page 417 - section 48.8.7 "Bus Number
> Registers (PCIE_RC_BNR)") shows the following layout [2]: 31 23 15 7 0
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Sec lat
> timer | Subord num | Sec busnum | Prim busnum |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ Shouldn't
> this:"val |= 0x00010100;" change to: "val |= 0x00ff0100;"
> ?
> 
> 
> 
> It seems other platforms also use this function (and thus register layout) to setup the PCIe RC [3],
> so instead of doing a single fix for i.MX6 maybe it's best to fix it here so other platforms also benefit?
> 
> As you know the code a lot better than me, what's your opinion on this? (or from anyone in CC)

I *think* I understand what's going on - the kernel takes the primary,
secondary and subordinate values in the host bridge as valid in:

pci_scan_bridge_extend()

and given that pcibios_assign_all_busses() returns false (guess) it sets-up
the bus hierarchy with a bus resource with subordinate number as read from
PCI host bridge config space - which, given that it is 1 according to your
explanation - this triggers the issue you reported.

After commit a20c7f36bd3d the root bus resource is propagated down the
hierarchy, hence the problem.

So, in order to fix the issue I think the best way is to programme the
root bridge in:

drivers/pci/dwc/pci-designware-host.c

but with the value coming from the root bus IORESOURCE_BUS resource,
not hardcoding 0xff.

I would kindly ask you to send logs with debug turned on in:

drivers/pci/probe.c

since I would like to check my understanding is correct.

Please CC all dwc host maintainers since this has potential widespread
impact.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] imx6: fix pcie enumeration
  2018-01-05 12:32       ` Lorenzo Pieralisi
@ 2018-01-05 13:39         ` Koen Vandeputte
  -1 siblings, 0 replies; 25+ messages in thread
From: Koen Vandeputte @ 2018-01-05 13:39 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Bjorn Helgaas, linux-pci, bhelgaas, Richard Zhu, Lucas Stach,
	linux-arm-kernel



On 2018-01-05 13:32, Lorenzo Pieralisi wrote:
>
>> 	/* setup bus numbers */
>> 	val  =  dw_pcie_readl_dbi
>> 	val  &=  0xff000000;
>> 	val  |=  0x00010100; <---  hardcoded today
>> 	dw_pcie_writel_dbi

>> I *think* I understand what's going on - the kernel takes the primary,
>> secondary and subordinate values in the host bridge as valid in:
>>
>> pci_scan_bridge_extend()
>>
>> and given that pcibios_assign_all_busses() returns false (guess) it sets-up
>> the bus hierarchy with a bus resource with subordinate number as read from
>> PCI host bridge config space - which, given that it is 1 according to your
>> explanation - this triggers the issue you reported.
>>
>> After commit a20c7f36bd3d the root bus resource is propagated down the
>> hierarchy, hence the problem.
>>
>> So, in order to fix the issue I think the best way is to programme the
>> root bridge in:
>>
>> drivers/pci/dwc/pci-designware-host.c
>>
>> but with the value coming from the root bus IORESOURCE_BUS resource,
>> not hardcoding 0xff.
>>
>> I would kindly ask you to send logs with debug turned on in:
>>
>> drivers/pci/probe.c
>>
>> since I would like to check my understanding is correct.
>>
>> Please CC all dwc host maintainers since this has potential widespread
>> impact.
>>
>> Thanks,
>> Lorenzo
Hi Lorenzo,

This is exactly what I'm trying to explain:

The host starts of with a (hardcoded today) subord of 1. [bits 16:23]

Since commit a20c7f36bd3d, downstream devices cannot assign bus nr's 
higher than the subord of the upstream device.
So in this case, scanning stops after the bridge as soon as bus 1 is 
assigned .. :)


As other targets besides i.MX6 (layerscape, armada8k, ...) also use the 
same function to init PCIe, I believe those targets are also affected.

I've tested here setting the PCI_PRIMARY_BUS register to 0x 00 ff 01 00  
(ignored-subord-secbus-primbus), and the whole scanning works again.
I fully agree that hardcoding is not the final fix, as this param can be 
defined in a DT.


Fixing this, combined with the upstream commit exposing the error, fixes 
all following pci boot errors:

..
[    0.466405] pci_bus 0000:05: [bus 05] partially hidden behind bridge 
0000:01 [bus 01]
..
[    0.466435] pci_bus 0000:02: busn_res: can not insert [bus 02-05] 
under [bus 01] (conflicts with (null) [bus 01])
[    0.466454] pci_bus 0000:02: [bus 02-05] partially hidden behind 
bridge 0000:01 [bus 01]
..


Watching the tree using lspci also shows that all primaries, secondaries 
and subords are perfectly logical as expected.


Thanks,

Koen


Log showing the initial issue without any fixup:


[    0.116673] OF: PCI: host bridge /soc/pcie@0x01000000 ranges:
[    0.116692] OF: PCI:   No bus range found for /soc/pcie@0x01000000, 
using [bus 00-ff]
[    0.116719] OF: PCI:    IO 0x01f80000..0x01f8ffff -> 0x00000000
[    0.116739] OF: PCI:   MEM 0x01000000..0x01efffff -> 0x01000000
[    0.337752] imx6q-pcie 1ffc000.pcie: link up
[    0.337771] imx6q-pcie 1ffc000.pcie: Link: Gen2 disabled
[    0.337785] imx6q-pcie 1ffc000.pcie: link up
[    0.337796] imx6q-pcie 1ffc000.pcie: Link up, Gen1
[    0.338039] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
[    0.338055] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.338069] pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
[    0.338082] pci_bus 0000:00: root bus resource [mem 
0x01000000-0x01efffff]
[    0.338094] pci_bus 0000:00: scanning bus
[    0.338127] pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
[    0.338151] pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x000fffff]
[    0.338168] pci 0000:00:00.0: reg 0x38: [mem 0x00000000-0x0000ffff pref]
[    0.338204] pci 0000:00:00.0: calling pci_fixup_ide_bases+0x0/0x4c
[    0.338259] pci 0000:00:00.0: supports D1
[    0.338267] pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
[    0.338276] pci 0000:00:00.0: PME# disabled
[    0.338512] pci_bus 0000:00: fixups for bus
[    0.338525] PCI: bus0: Fast back to back transfers disabled
[    0.338541] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 0
[    0.338673] pci_bus 0000:01: scanning bus
[    0.338773] pci 0000:01:00.0: [10b5:8604] type 01 class 0x060400
[    0.338816] pci 0000:01:00.0: calling ventana_pciesw_early_fixup+0x0/0xa4
[    0.467817] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0001ffff]
[    0.467999] pci 0000:01:00.0: calling pci_fixup_ide_bases+0x0/0x4c
[    0.468467] pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
[    0.468491] pci 0000:01:00.0: PME# disabled
[    0.468795] pci_bus 0000:01: fixups for bus
[    0.468854] PCI: bus1: Fast back to back transfers disabled
[    0.468877] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 0
[    0.468886] pci 0000:01:00.0: bridge configuration invalid ([bus 
00-00]), reconfiguring
[    0.468939] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 1
[    0.469265] pci_bus 0000:02: busn_res: can not insert [bus 02-01] 
under [bus 01] (conflicts with (null) [bus 01])
[    0.469282] pci_bus 0000:02: scanning bus
[    0.469554] pci_bus 0000:02: fixups for bus
[    0.469559] PCI: bus2: Fast back to back transfers enabled
[    0.469572] pci_bus 0000:02: bus scan returning with max=02
[    0.469582] pci_bus 0000:02: busn_res: [bus 02-01] end is updated to 02
[    0.469593] pci_bus 0000:02: busn_res: can not insert [bus 02] under 
[bus 01] (conflicts with (null) [bus 01])
[    0.469615] pci_bus 0000:02: [bus 02] partially hidden behind bridge 
0000:01 [bus 01]
[    0.469636] pci_bus 0000:01: bus scan returning with max=02
[    0.469643] pci 0000:00:00.0: bridge has subordinate 01 but max busn 02
[    0.469661] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 1
[    0.469671] pci_bus 0000:00: bus scan returning with max=01
[    0.469791] pci 0000:00:00.0: fixup irq: got 298
[    0.469800] pci 0000:00:00.0: assigning IRQ 298
[    0.469849] pci 0000:01:00.0: fixup irq: got 298
[    0.469856] pci 0000:01:00.0: assigning IRQ 298
[    0.469946] pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
[    0.469965] pci 0000:00:00.0: BAR 8: assigned [mem 0x01100000-0x011fffff]
[    0.469980] pci 0000:00:00.0: BAR 6: assigned [mem 
0x01200000-0x0120ffff pref]
[    0.469997] pci 0000:01:00.0: BAR 0: assigned [mem 0x01100000-0x0111ffff]
[    0.470026] pci 0000:01:00.0: PCI bridge to [bus 02]
[    0.470108] pci 0000:00:00.0: PCI bridge to [bus 01]
[    0.470121] pci 0000:00:00.0:   bridge window [mem 0x01100000-0x011fffff]
[    0.470381] pcieport 0000:00:00.0: Signaling PME through PCIe PME 
interrupt
[    0.470397] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
[    0.470412] pcie_pme 0000:00:00.0:pcie001: service driver pcie_pme loaded
[    0.470660] pcieport 0000:01:00.0: enabling device (0140 -> 0142)
[    0.470788] pcieport 0000:01:00.0: enabling bus mastering



[ Node 4 | node-4 ] lspci -tv
-[0000:00]---00.0-[01]----00.0-[02]--
[ Node 4 | node-4 ]



[ Node 4 | node-4 ] lspci -v
00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 
[Normal decode])
     Flags: bus master, fast devsel, latency 0, IRQ 298
     Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
     Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
     I/O behind bridge: None
     Memory behind bridge: 01100000-011fffff [size=1M]
     Prefetchable memory behind bridge: None
     [virtual] Expansion ROM at 01200000 [disabled] [size=64K]
     Capabilities: [40] Power Management version 3
     Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
     Capabilities: [70] Express Root Port (Slot-), MSI 00
     Capabilities: [100] Advanced Error Reporting
     Capabilities: [140] Virtual Channel
     Kernel driver in use: pcieport
lspci: Unable to load libkmod resources: error -12

01:00.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI 
Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal decode])
     Flags: bus master, fast devsel, latency 0, IRQ 298
     Memory at 01100000 (32-bit, non-prefetchable) [size=128K]
     Bus: primary=01, secondary=02, subordinate=02, sec-latency=0
     I/O behind bridge: None
     Memory behind bridge: None
     Prefetchable memory behind bridge: None
     Capabilities: [40] Power Management version 3
     Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
     Capabilities: [68] Express Upstream Port, MSI 00
     Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604 4-lane, 
4-Port PCI Express Gen 2 (5.0 GT/s) Switch
     Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
     Capabilities: [fb4] Advanced Error Reporting
     Capabilities: [138] Power Budgeting <?>
     Capabilities: [148] Virtual Channel
     Capabilities: [448] Vendor Specific Information: ID=0000 Rev=0 
Len=0cc <?>
     Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0 
Len=010 <?>
     Kernel driver in use: pcieport
[ Node 4 | node-4 ]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-05 13:39         ` Koen Vandeputte
  0 siblings, 0 replies; 25+ messages in thread
From: Koen Vandeputte @ 2018-01-05 13:39 UTC (permalink / raw)
  To: linux-arm-kernel



On 2018-01-05 13:32, Lorenzo Pieralisi wrote:
>
>> 	/* setup bus numbers */
>> 	val  =  dw_pcie_readl_dbi
>> 	val  &=  0xff000000;
>> 	val  |=  0x00010100; <---  hardcoded today
>> 	dw_pcie_writel_dbi

>> I *think* I understand what's going on - the kernel takes the primary,
>> secondary and subordinate values in the host bridge as valid in:
>>
>> pci_scan_bridge_extend()
>>
>> and given that pcibios_assign_all_busses() returns false (guess) it sets-up
>> the bus hierarchy with a bus resource with subordinate number as read from
>> PCI host bridge config space - which, given that it is 1 according to your
>> explanation - this triggers the issue you reported.
>>
>> After commit a20c7f36bd3d the root bus resource is propagated down the
>> hierarchy, hence the problem.
>>
>> So, in order to fix the issue I think the best way is to programme the
>> root bridge in:
>>
>> drivers/pci/dwc/pci-designware-host.c
>>
>> but with the value coming from the root bus IORESOURCE_BUS resource,
>> not hardcoding 0xff.
>>
>> I would kindly ask you to send logs with debug turned on in:
>>
>> drivers/pci/probe.c
>>
>> since I would like to check my understanding is correct.
>>
>> Please CC all dwc host maintainers since this has potential widespread
>> impact.
>>
>> Thanks,
>> Lorenzo
Hi Lorenzo,

This is exactly what I'm trying to explain:

The host starts of with a (hardcoded today) subord of 1. [bits 16:23]

Since commit a20c7f36bd3d, downstream devices cannot assign bus nr's 
higher than the subord of the upstream device.
So in this case, scanning stops after the bridge as soon as bus 1 is 
assigned .. :)


As other targets besides i.MX6 (layerscape, armada8k, ...) also use the 
same function to init PCIe, I believe those targets are also affected.

I've tested here setting the PCI_PRIMARY_BUS register to 0x 00 ff 01 00? 
(ignored-subord-secbus-primbus), and the whole scanning works again.
I fully agree that hardcoding is not the final fix, as this param can be 
defined in a DT.


Fixing this, combined with the upstream commit exposing the error, fixes 
all following pci boot errors:

..
[??? 0.466405] pci_bus 0000:05: [bus 05] partially hidden behind bridge 
0000:01 [bus 01]
..
[??? 0.466435] pci_bus 0000:02: busn_res: can not insert [bus 02-05] 
under [bus 01] (conflicts with (null) [bus 01])
[??? 0.466454] pci_bus 0000:02: [bus 02-05] partially hidden behind 
bridge 0000:01 [bus 01]
..


Watching the tree using lspci also shows that all primaries, secondaries 
and subords are perfectly logical as expected.


Thanks,

Koen


Log showing the initial issue without any fixup:


[??? 0.116673] OF: PCI: host bridge /soc/pcie at 0x01000000 ranges:
[??? 0.116692] OF: PCI:?? No bus range found for /soc/pcie at 0x01000000, 
using [bus 00-ff]
[??? 0.116719] OF: PCI:??? IO 0x01f80000..0x01f8ffff -> 0x00000000
[??? 0.116739] OF: PCI:?? MEM 0x01000000..0x01efffff -> 0x01000000
[??? 0.337752] imx6q-pcie 1ffc000.pcie: link up
[??? 0.337771] imx6q-pcie 1ffc000.pcie: Link: Gen2 disabled
[??? 0.337785] imx6q-pcie 1ffc000.pcie: link up
[??? 0.337796] imx6q-pcie 1ffc000.pcie: Link up, Gen1
[??? 0.338039] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
[??? 0.338055] pci_bus 0000:00: root bus resource [bus 00-ff]
[??? 0.338069] pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
[??? 0.338082] pci_bus 0000:00: root bus resource [mem 
0x01000000-0x01efffff]
[??? 0.338094] pci_bus 0000:00: scanning bus
[??? 0.338127] pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
[??? 0.338151] pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x000fffff]
[??? 0.338168] pci 0000:00:00.0: reg 0x38: [mem 0x00000000-0x0000ffff pref]
[??? 0.338204] pci 0000:00:00.0: calling pci_fixup_ide_bases+0x0/0x4c
[??? 0.338259] pci 0000:00:00.0: supports D1
[??? 0.338267] pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
[??? 0.338276] pci 0000:00:00.0: PME# disabled
[??? 0.338512] pci_bus 0000:00: fixups for bus
[??? 0.338525] PCI: bus0: Fast back to back transfers disabled
[??? 0.338541] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 0
[??? 0.338673] pci_bus 0000:01: scanning bus
[??? 0.338773] pci 0000:01:00.0: [10b5:8604] type 01 class 0x060400
[??? 0.338816] pci 0000:01:00.0: calling ventana_pciesw_early_fixup+0x0/0xa4
[??? 0.467817] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0001ffff]
[??? 0.467999] pci 0000:01:00.0: calling pci_fixup_ide_bases+0x0/0x4c
[??? 0.468467] pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
[??? 0.468491] pci 0000:01:00.0: PME# disabled
[??? 0.468795] pci_bus 0000:01: fixups for bus
[??? 0.468854] PCI: bus1: Fast back to back transfers disabled
[??? 0.468877] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 0
[??? 0.468886] pci 0000:01:00.0: bridge configuration invalid ([bus 
00-00]), reconfiguring
[??? 0.468939] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 1
[??? 0.469265] pci_bus 0000:02: busn_res: can not insert [bus 02-01] 
under [bus 01] (conflicts with (null) [bus 01])
[??? 0.469282] pci_bus 0000:02: scanning bus
[??? 0.469554] pci_bus 0000:02: fixups for bus
[??? 0.469559] PCI: bus2: Fast back to back transfers enabled
[??? 0.469572] pci_bus 0000:02: bus scan returning with max=02
[??? 0.469582] pci_bus 0000:02: busn_res: [bus 02-01] end is updated to 02
[??? 0.469593] pci_bus 0000:02: busn_res: can not insert [bus 02] under 
[bus 01] (conflicts with (null) [bus 01])
[??? 0.469615] pci_bus 0000:02: [bus 02] partially hidden behind bridge 
0000:01 [bus 01]
[??? 0.469636] pci_bus 0000:01: bus scan returning with max=02
[??? 0.469643] pci 0000:00:00.0: bridge has subordinate 01 but max busn 02
[??? 0.469661] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 1
[??? 0.469671] pci_bus 0000:00: bus scan returning with max=01
[??? 0.469791] pci 0000:00:00.0: fixup irq: got 298
[??? 0.469800] pci 0000:00:00.0: assigning IRQ 298
[??? 0.469849] pci 0000:01:00.0: fixup irq: got 298
[??? 0.469856] pci 0000:01:00.0: assigning IRQ 298
[??? 0.469946] pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
[??? 0.469965] pci 0000:00:00.0: BAR 8: assigned [mem 0x01100000-0x011fffff]
[??? 0.469980] pci 0000:00:00.0: BAR 6: assigned [mem 
0x01200000-0x0120ffff pref]
[??? 0.469997] pci 0000:01:00.0: BAR 0: assigned [mem 0x01100000-0x0111ffff]
[??? 0.470026] pci 0000:01:00.0: PCI bridge to [bus 02]
[??? 0.470108] pci 0000:00:00.0: PCI bridge to [bus 01]
[??? 0.470121] pci 0000:00:00.0:?? bridge window [mem 0x01100000-0x011fffff]
[??? 0.470381] pcieport 0000:00:00.0: Signaling PME through PCIe PME 
interrupt
[??? 0.470397] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
[??? 0.470412] pcie_pme 0000:00:00.0:pcie001: service driver pcie_pme loaded
[??? 0.470660] pcieport 0000:01:00.0: enabling device (0140 -> 0142)
[??? 0.470788] pcieport 0000:01:00.0: enabling bus mastering



[ Node 4 | node-4 ] lspci -tv
-[0000:00]---00.0-[01]----00.0-[02]--
[ Node 4 | node-4 ]



[ Node 4 | node-4 ] lspci -v
00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 
[Normal decode])
 ??? Flags: bus master, fast devsel, latency 0, IRQ 298
 ??? Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
 ??? Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
 ??? I/O behind bridge: None
 ??? Memory behind bridge: 01100000-011fffff [size=1M]
 ??? Prefetchable memory behind bridge: None
 ??? [virtual] Expansion ROM at 01200000 [disabled] [size=64K]
 ??? Capabilities: [40] Power Management version 3
 ??? Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
 ??? Capabilities: [70] Express Root Port (Slot-), MSI 00
 ??? Capabilities: [100] Advanced Error Reporting
 ??? Capabilities: [140] Virtual Channel
 ??? Kernel driver in use: pcieport
lspci: Unable to load libkmod resources: error -12

01:00.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI 
Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal decode])
 ??? Flags: bus master, fast devsel, latency 0, IRQ 298
 ??? Memory at 01100000 (32-bit, non-prefetchable) [size=128K]
 ??? Bus: primary=01, secondary=02, subordinate=02, sec-latency=0
 ??? I/O behind bridge: None
 ??? Memory behind bridge: None
 ??? Prefetchable memory behind bridge: None
 ??? Capabilities: [40] Power Management version 3
 ??? Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
 ??? Capabilities: [68] Express Upstream Port, MSI 00
 ??? Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604 4-lane, 
4-Port PCI Express Gen 2 (5.0 GT/s) Switch
 ??? Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
 ??? Capabilities: [fb4] Advanced Error Reporting
 ??? Capabilities: [138] Power Budgeting <?>
 ??? Capabilities: [148] Virtual Channel
 ??? Capabilities: [448] Vendor Specific Information: ID=0000 Rev=0 
Len=0cc <?>
 ??? Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0 
Len=010 <?>
 ??? Kernel driver in use: pcieport
[ Node 4 | node-4 ]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] imx6: fix pcie enumeration
  2018-01-05  9:56     ` Koen Vandeputte
@ 2018-01-05 13:46       ` Bjorn Helgaas
  -1 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2018-01-05 13:46 UTC (permalink / raw)
  To: Koen Vandeputte
  Cc: Lorenzo Pieralisi, Richard Zhu, linux-pci, bhelgaas,
	linux-arm-kernel, Lucas Stach

On Fri, Jan 05, 2018 at 10:56:31AM +0100, Koen Vandeputte wrote:
> ...
> Hi Bjorn,
> 
> Thanks for your time and patience writing extended comments on all points,
> especially since this is my first commit to this list.

Welcome, hope we see more from you!  Lorenzo is giving you more and
better tips for digging into this, so I'm sure you'll get it all
sorted out soon.

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-05 13:46       ` Bjorn Helgaas
  0 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2018-01-05 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 05, 2018 at 10:56:31AM +0100, Koen Vandeputte wrote:
> ...
> Hi Bjorn,
> 
> Thanks for your time and patience writing extended comments on all points,
> especially since this is my first commit to this list.

Welcome, hope we see more from you!  Lorenzo is giving you more and
better tips for digging into this, so I'm sure you'll get it all
sorted out soon.

Bjorn

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] imx6: fix pcie enumeration
  2018-01-05 13:39         ` Koen Vandeputte
@ 2018-01-05 17:18           ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 25+ messages in thread
From: Lorenzo Pieralisi @ 2018-01-05 17:18 UTC (permalink / raw)
  To: Koen Vandeputte
  Cc: Bjorn Helgaas, linux-pci, bhelgaas, Richard Zhu, Lucas Stach,
	linux-arm-kernel

On Fri, Jan 05, 2018 at 02:39:57PM +0100, Koen Vandeputte wrote:
> 
> 
> On 2018-01-05 13:32, Lorenzo Pieralisi wrote:
> >
> >>	/* setup bus numbers */
> >>	val  =  dw_pcie_readl_dbi
> >>	val  &=  0xff000000;
> >>	val  |=  0x00010100; <---  hardcoded today
> >>	dw_pcie_writel_dbi
> 
> >>I *think* I understand what's going on - the kernel takes the primary,
> >>secondary and subordinate values in the host bridge as valid in:
> >>
> >>pci_scan_bridge_extend()
> >>
> >>and given that pcibios_assign_all_busses() returns false (guess) it sets-up
> >>the bus hierarchy with a bus resource with subordinate number as read from
> >>PCI host bridge config space - which, given that it is 1 according to your
> >>explanation - this triggers the issue you reported.
> >>
> >>After commit a20c7f36bd3d the root bus resource is propagated down the
> >>hierarchy, hence the problem.
> >>
> >>So, in order to fix the issue I think the best way is to programme the
> >>root bridge in:
> >>
> >>drivers/pci/dwc/pci-designware-host.c
> >>
> >>but with the value coming from the root bus IORESOURCE_BUS resource,
> >>not hardcoding 0xff.
> >>
> >>I would kindly ask you to send logs with debug turned on in:
> >>
> >>drivers/pci/probe.c
> >>
> >>since I would like to check my understanding is correct.
> >>
> >>Please CC all dwc host maintainers since this has potential widespread
> >>impact.
> >>
> >>Thanks,
> >>Lorenzo
> Hi Lorenzo,
> 
> This is exactly what I'm trying to explain:
> 
> The host starts of with a (hardcoded today) subord of 1. [bits 16:23]
> 
> Since commit a20c7f36bd3d, downstream devices cannot assign bus nr's
> higher than the subord of the upstream device.
> So in this case, scanning stops after the bridge as soon as bus 1 is
> assigned .. :)

There is one thing that I need to understand though. Before the commit
above, how would enumeration works given that the subordinate bus number
was set to 1 and that the kernel, AFAICS, does not overwrite it ?

Are you able to send me a log (enumeration with debugging enabled and
lspci) with the commit above reverted please ?

Thanks,
Lorenzo

> As other targets besides i.MX6 (layerscape, armada8k, ...) also use
> the same function to init PCIe, I believe those targets are also
> affected.
> 
> I've tested here setting the PCI_PRIMARY_BUS register to 0x 00 ff 01
> 00  (ignored-subord-secbus-primbus), and the whole scanning works
> again.
> I fully agree that hardcoding is not the final fix, as this param
> can be defined in a DT.
> 
> 
> Fixing this, combined with the upstream commit exposing the error,
> fixes all following pci boot errors:
> 
> ..
> [    0.466405] pci_bus 0000:05: [bus 05] partially hidden behind
> bridge 0000:01 [bus 01]
> ..
> [    0.466435] pci_bus 0000:02: busn_res: can not insert [bus 02-05]
> under [bus 01] (conflicts with (null) [bus 01])
> [    0.466454] pci_bus 0000:02: [bus 02-05] partially hidden behind
> bridge 0000:01 [bus 01]
> ..
> 
> 
> Watching the tree using lspci also shows that all primaries,
> secondaries and subords are perfectly logical as expected.
> 
> 
> Thanks,
> 
> Koen
> 
> 
> Log showing the initial issue without any fixup:
> 
> 
> [    0.116673] OF: PCI: host bridge /soc/pcie@0x01000000 ranges:
> [    0.116692] OF: PCI:   No bus range found for
> /soc/pcie@0x01000000, using [bus 00-ff]
> [    0.116719] OF: PCI:    IO 0x01f80000..0x01f8ffff -> 0x00000000
> [    0.116739] OF: PCI:   MEM 0x01000000..0x01efffff -> 0x01000000
> [    0.337752] imx6q-pcie 1ffc000.pcie: link up
> [    0.337771] imx6q-pcie 1ffc000.pcie: Link: Gen2 disabled
> [    0.337785] imx6q-pcie 1ffc000.pcie: link up
> [    0.337796] imx6q-pcie 1ffc000.pcie: Link up, Gen1
> [    0.338039] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
> [    0.338055] pci_bus 0000:00: root bus resource [bus 00-ff]
> [    0.338069] pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
> [    0.338082] pci_bus 0000:00: root bus resource [mem
> 0x01000000-0x01efffff]
> [    0.338094] pci_bus 0000:00: scanning bus
> [    0.338127] pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
> [    0.338151] pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x000fffff]
> [    0.338168] pci 0000:00:00.0: reg 0x38: [mem 0x00000000-0x0000ffff pref]
> [    0.338204] pci 0000:00:00.0: calling pci_fixup_ide_bases+0x0/0x4c
> [    0.338259] pci 0000:00:00.0: supports D1
> [    0.338267] pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
> [    0.338276] pci 0000:00:00.0: PME# disabled
> [    0.338512] pci_bus 0000:00: fixups for bus
> [    0.338525] PCI: bus0: Fast back to back transfers disabled
> [    0.338541] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 0
> [    0.338673] pci_bus 0000:01: scanning bus
> [    0.338773] pci 0000:01:00.0: [10b5:8604] type 01 class 0x060400
> [    0.338816] pci 0000:01:00.0: calling ventana_pciesw_early_fixup+0x0/0xa4
> [    0.467817] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0001ffff]
> [    0.467999] pci 0000:01:00.0: calling pci_fixup_ide_bases+0x0/0x4c
> [    0.468467] pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
> [    0.468491] pci 0000:01:00.0: PME# disabled
> [    0.468795] pci_bus 0000:01: fixups for bus
> [    0.468854] PCI: bus1: Fast back to back transfers disabled
> [    0.468877] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 0
> [    0.468886] pci 0000:01:00.0: bridge configuration invalid ([bus
> 00-00]), reconfiguring
> [    0.468939] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 1
> [    0.469265] pci_bus 0000:02: busn_res: can not insert [bus 02-01]
> under [bus 01] (conflicts with (null) [bus 01])
> [    0.469282] pci_bus 0000:02: scanning bus
> [    0.469554] pci_bus 0000:02: fixups for bus
> [    0.469559] PCI: bus2: Fast back to back transfers enabled
> [    0.469572] pci_bus 0000:02: bus scan returning with max=02
> [    0.469582] pci_bus 0000:02: busn_res: [bus 02-01] end is updated to 02
> [    0.469593] pci_bus 0000:02: busn_res: can not insert [bus 02]
> under [bus 01] (conflicts with (null) [bus 01])
> [    0.469615] pci_bus 0000:02: [bus 02] partially hidden behind
> bridge 0000:01 [bus 01]
> [    0.469636] pci_bus 0000:01: bus scan returning with max=02
> [    0.469643] pci 0000:00:00.0: bridge has subordinate 01 but max busn 02
> [    0.469661] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 1
> [    0.469671] pci_bus 0000:00: bus scan returning with max=01
> [    0.469791] pci 0000:00:00.0: fixup irq: got 298
> [    0.469800] pci 0000:00:00.0: assigning IRQ 298
> [    0.469849] pci 0000:01:00.0: fixup irq: got 298
> [    0.469856] pci 0000:01:00.0: assigning IRQ 298
> [    0.469946] pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
> [    0.469965] pci 0000:00:00.0: BAR 8: assigned [mem 0x01100000-0x011fffff]
> [    0.469980] pci 0000:00:00.0: BAR 6: assigned [mem
> 0x01200000-0x0120ffff pref]
> [    0.469997] pci 0000:01:00.0: BAR 0: assigned [mem 0x01100000-0x0111ffff]
> [    0.470026] pci 0000:01:00.0: PCI bridge to [bus 02]
> [    0.470108] pci 0000:00:00.0: PCI bridge to [bus 01]
> [    0.470121] pci 0000:00:00.0:   bridge window [mem 0x01100000-0x011fffff]
> [    0.470381] pcieport 0000:00:00.0: Signaling PME through PCIe PME
> interrupt
> [    0.470397] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
> [    0.470412] pcie_pme 0000:00:00.0:pcie001: service driver pcie_pme loaded
> [    0.470660] pcieport 0000:01:00.0: enabling device (0140 -> 0142)
> [    0.470788] pcieport 0000:01:00.0: enabling bus mastering
> 
> 
> 
> [ Node 4 | node-4 ] lspci -tv
> -[0000:00]---00.0-[01]----00.0-[02]--
> [ Node 4 | node-4 ]
> 
> 
> 
> [ Node 4 | node-4 ] lspci -v
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> [Normal decode])
>     Flags: bus master, fast devsel, latency 0, IRQ 298
>     Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
>     Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>     I/O behind bridge: None
>     Memory behind bridge: 01100000-011fffff [size=1M]
>     Prefetchable memory behind bridge: None
>     [virtual] Expansion ROM at 01200000 [disabled] [size=64K]
>     Capabilities: [40] Power Management version 3
>     Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
>     Capabilities: [70] Express Root Port (Slot-), MSI 00
>     Capabilities: [100] Advanced Error Reporting
>     Capabilities: [140] Virtual Channel
>     Kernel driver in use: pcieport
> lspci: Unable to load libkmod resources: error -12
> 
> 01:00.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI
> Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal
> decode])
>     Flags: bus master, fast devsel, latency 0, IRQ 298
>     Memory at 01100000 (32-bit, non-prefetchable) [size=128K]
>     Bus: primary=01, secondary=02, subordinate=02, sec-latency=0
>     I/O behind bridge: None
>     Memory behind bridge: None
>     Prefetchable memory behind bridge: None
>     Capabilities: [40] Power Management version 3
>     Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
>     Capabilities: [68] Express Upstream Port, MSI 00
>     Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604
> 4-lane, 4-Port PCI Express Gen 2 (5.0 GT/s) Switch
>     Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
>     Capabilities: [fb4] Advanced Error Reporting
>     Capabilities: [138] Power Budgeting <?>
>     Capabilities: [148] Virtual Channel
>     Capabilities: [448] Vendor Specific Information: ID=0000 Rev=0
> Len=0cc <?>
>     Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0
> Len=010 <?>
>     Kernel driver in use: pcieport
> [ Node 4 | node-4 ]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-05 17:18           ` Lorenzo Pieralisi
  0 siblings, 0 replies; 25+ messages in thread
From: Lorenzo Pieralisi @ 2018-01-05 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 05, 2018 at 02:39:57PM +0100, Koen Vandeputte wrote:
> 
> 
> On 2018-01-05 13:32, Lorenzo Pieralisi wrote:
> >
> >>	/* setup bus numbers */
> >>	val  =  dw_pcie_readl_dbi
> >>	val  &=  0xff000000;
> >>	val  |=  0x00010100; <---  hardcoded today
> >>	dw_pcie_writel_dbi
> 
> >>I *think* I understand what's going on - the kernel takes the primary,
> >>secondary and subordinate values in the host bridge as valid in:
> >>
> >>pci_scan_bridge_extend()
> >>
> >>and given that pcibios_assign_all_busses() returns false (guess) it sets-up
> >>the bus hierarchy with a bus resource with subordinate number as read from
> >>PCI host bridge config space - which, given that it is 1 according to your
> >>explanation - this triggers the issue you reported.
> >>
> >>After commit a20c7f36bd3d the root bus resource is propagated down the
> >>hierarchy, hence the problem.
> >>
> >>So, in order to fix the issue I think the best way is to programme the
> >>root bridge in:
> >>
> >>drivers/pci/dwc/pci-designware-host.c
> >>
> >>but with the value coming from the root bus IORESOURCE_BUS resource,
> >>not hardcoding 0xff.
> >>
> >>I would kindly ask you to send logs with debug turned on in:
> >>
> >>drivers/pci/probe.c
> >>
> >>since I would like to check my understanding is correct.
> >>
> >>Please CC all dwc host maintainers since this has potential widespread
> >>impact.
> >>
> >>Thanks,
> >>Lorenzo
> Hi Lorenzo,
> 
> This is exactly what I'm trying to explain:
> 
> The host starts of with a (hardcoded today) subord of 1. [bits 16:23]
> 
> Since commit a20c7f36bd3d, downstream devices cannot assign bus nr's
> higher than the subord of the upstream device.
> So in this case, scanning stops after the bridge as soon as bus 1 is
> assigned .. :)

There is one thing that I need to understand though. Before the commit
above, how would enumeration works given that the subordinate bus number
was set to 1 and that the kernel, AFAICS, does not overwrite it ?

Are you able to send me a log (enumeration with debugging enabled and
lspci) with the commit above reverted please ?

Thanks,
Lorenzo

> As other targets besides i.MX6 (layerscape, armada8k, ...) also use
> the same function to init PCIe, I believe those targets are also
> affected.
> 
> I've tested here setting the PCI_PRIMARY_BUS register to 0x 00 ff 01
> 00? (ignored-subord-secbus-primbus), and the whole scanning works
> again.
> I fully agree that hardcoding is not the final fix, as this param
> can be defined in a DT.
> 
> 
> Fixing this, combined with the upstream commit exposing the error,
> fixes all following pci boot errors:
> 
> ..
> [??? 0.466405] pci_bus 0000:05: [bus 05] partially hidden behind
> bridge 0000:01 [bus 01]
> ..
> [??? 0.466435] pci_bus 0000:02: busn_res: can not insert [bus 02-05]
> under [bus 01] (conflicts with (null) [bus 01])
> [??? 0.466454] pci_bus 0000:02: [bus 02-05] partially hidden behind
> bridge 0000:01 [bus 01]
> ..
> 
> 
> Watching the tree using lspci also shows that all primaries,
> secondaries and subords are perfectly logical as expected.
> 
> 
> Thanks,
> 
> Koen
> 
> 
> Log showing the initial issue without any fixup:
> 
> 
> [??? 0.116673] OF: PCI: host bridge /soc/pcie at 0x01000000 ranges:
> [??? 0.116692] OF: PCI:?? No bus range found for
> /soc/pcie at 0x01000000, using [bus 00-ff]
> [??? 0.116719] OF: PCI:??? IO 0x01f80000..0x01f8ffff -> 0x00000000
> [??? 0.116739] OF: PCI:?? MEM 0x01000000..0x01efffff -> 0x01000000
> [??? 0.337752] imx6q-pcie 1ffc000.pcie: link up
> [??? 0.337771] imx6q-pcie 1ffc000.pcie: Link: Gen2 disabled
> [??? 0.337785] imx6q-pcie 1ffc000.pcie: link up
> [??? 0.337796] imx6q-pcie 1ffc000.pcie: Link up, Gen1
> [??? 0.338039] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
> [??? 0.338055] pci_bus 0000:00: root bus resource [bus 00-ff]
> [??? 0.338069] pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
> [??? 0.338082] pci_bus 0000:00: root bus resource [mem
> 0x01000000-0x01efffff]
> [??? 0.338094] pci_bus 0000:00: scanning bus
> [??? 0.338127] pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
> [??? 0.338151] pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x000fffff]
> [??? 0.338168] pci 0000:00:00.0: reg 0x38: [mem 0x00000000-0x0000ffff pref]
> [??? 0.338204] pci 0000:00:00.0: calling pci_fixup_ide_bases+0x0/0x4c
> [??? 0.338259] pci 0000:00:00.0: supports D1
> [??? 0.338267] pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
> [??? 0.338276] pci 0000:00:00.0: PME# disabled
> [??? 0.338512] pci_bus 0000:00: fixups for bus
> [??? 0.338525] PCI: bus0: Fast back to back transfers disabled
> [??? 0.338541] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 0
> [??? 0.338673] pci_bus 0000:01: scanning bus
> [??? 0.338773] pci 0000:01:00.0: [10b5:8604] type 01 class 0x060400
> [??? 0.338816] pci 0000:01:00.0: calling ventana_pciesw_early_fixup+0x0/0xa4
> [??? 0.467817] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0001ffff]
> [??? 0.467999] pci 0000:01:00.0: calling pci_fixup_ide_bases+0x0/0x4c
> [??? 0.468467] pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
> [??? 0.468491] pci 0000:01:00.0: PME# disabled
> [??? 0.468795] pci_bus 0000:01: fixups for bus
> [??? 0.468854] PCI: bus1: Fast back to back transfers disabled
> [??? 0.468877] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 0
> [??? 0.468886] pci 0000:01:00.0: bridge configuration invalid ([bus
> 00-00]), reconfiguring
> [??? 0.468939] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 1
> [??? 0.469265] pci_bus 0000:02: busn_res: can not insert [bus 02-01]
> under [bus 01] (conflicts with (null) [bus 01])
> [??? 0.469282] pci_bus 0000:02: scanning bus
> [??? 0.469554] pci_bus 0000:02: fixups for bus
> [??? 0.469559] PCI: bus2: Fast back to back transfers enabled
> [??? 0.469572] pci_bus 0000:02: bus scan returning with max=02
> [??? 0.469582] pci_bus 0000:02: busn_res: [bus 02-01] end is updated to 02
> [??? 0.469593] pci_bus 0000:02: busn_res: can not insert [bus 02]
> under [bus 01] (conflicts with (null) [bus 01])
> [??? 0.469615] pci_bus 0000:02: [bus 02] partially hidden behind
> bridge 0000:01 [bus 01]
> [??? 0.469636] pci_bus 0000:01: bus scan returning with max=02
> [??? 0.469643] pci 0000:00:00.0: bridge has subordinate 01 but max busn 02
> [??? 0.469661] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 1
> [??? 0.469671] pci_bus 0000:00: bus scan returning with max=01
> [??? 0.469791] pci 0000:00:00.0: fixup irq: got 298
> [??? 0.469800] pci 0000:00:00.0: assigning IRQ 298
> [??? 0.469849] pci 0000:01:00.0: fixup irq: got 298
> [??? 0.469856] pci 0000:01:00.0: assigning IRQ 298
> [??? 0.469946] pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
> [??? 0.469965] pci 0000:00:00.0: BAR 8: assigned [mem 0x01100000-0x011fffff]
> [??? 0.469980] pci 0000:00:00.0: BAR 6: assigned [mem
> 0x01200000-0x0120ffff pref]
> [??? 0.469997] pci 0000:01:00.0: BAR 0: assigned [mem 0x01100000-0x0111ffff]
> [??? 0.470026] pci 0000:01:00.0: PCI bridge to [bus 02]
> [??? 0.470108] pci 0000:00:00.0: PCI bridge to [bus 01]
> [??? 0.470121] pci 0000:00:00.0:?? bridge window [mem 0x01100000-0x011fffff]
> [??? 0.470381] pcieport 0000:00:00.0: Signaling PME through PCIe PME
> interrupt
> [??? 0.470397] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
> [??? 0.470412] pcie_pme 0000:00:00.0:pcie001: service driver pcie_pme loaded
> [??? 0.470660] pcieport 0000:01:00.0: enabling device (0140 -> 0142)
> [??? 0.470788] pcieport 0000:01:00.0: enabling bus mastering
> 
> 
> 
> [ Node 4 | node-4 ] lspci -tv
> -[0000:00]---00.0-[01]----00.0-[02]--
> [ Node 4 | node-4 ]
> 
> 
> 
> [ Node 4 | node-4 ] lspci -v
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> [Normal decode])
> ??? Flags: bus master, fast devsel, latency 0, IRQ 298
> ??? Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
> ??? Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> ??? I/O behind bridge: None
> ??? Memory behind bridge: 01100000-011fffff [size=1M]
> ??? Prefetchable memory behind bridge: None
> ??? [virtual] Expansion ROM at 01200000 [disabled] [size=64K]
> ??? Capabilities: [40] Power Management version 3
> ??? Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
> ??? Capabilities: [70] Express Root Port (Slot-), MSI 00
> ??? Capabilities: [100] Advanced Error Reporting
> ??? Capabilities: [140] Virtual Channel
> ??? Kernel driver in use: pcieport
> lspci: Unable to load libkmod resources: error -12
> 
> 01:00.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI
> Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal
> decode])
> ??? Flags: bus master, fast devsel, latency 0, IRQ 298
> ??? Memory at 01100000 (32-bit, non-prefetchable) [size=128K]
> ??? Bus: primary=01, secondary=02, subordinate=02, sec-latency=0
> ??? I/O behind bridge: None
> ??? Memory behind bridge: None
> ??? Prefetchable memory behind bridge: None
> ??? Capabilities: [40] Power Management version 3
> ??? Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
> ??? Capabilities: [68] Express Upstream Port, MSI 00
> ??? Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604
> 4-lane, 4-Port PCI Express Gen 2 (5.0 GT/s) Switch
> ??? Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
> ??? Capabilities: [fb4] Advanced Error Reporting
> ??? Capabilities: [138] Power Budgeting <?>
> ??? Capabilities: [148] Virtual Channel
> ??? Capabilities: [448] Vendor Specific Information: ID=0000 Rev=0
> Len=0cc <?>
> ??? Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0
> Len=010 <?>
> ??? Kernel driver in use: pcieport
> [ Node 4 | node-4 ]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] imx6: fix pcie enumeration
  2018-01-05 17:18           ` Lorenzo Pieralisi
@ 2018-01-08  8:51             ` Koen Vandeputte
  -1 siblings, 0 replies; 25+ messages in thread
From: Koen Vandeputte @ 2018-01-08  8:51 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Bjorn Helgaas, linux-pci, bhelgaas, Richard Zhu, Lucas Stach,
	linux-arm-kernel



On 2018-01-05 18:18, Lorenzo Pieralisi wrote:
>>
>> Hi Lorenzo,
>>
>> This is exactly what I'm trying to explain:
>>
>> The host starts of with a (hardcoded today) subord of 1. [bits 16:23]
>>
>> Since commit a20c7f36bd3d, downstream devices cannot assign bus nr's
>> higher than the subord of the upstream device.
>> So in this case, scanning stops after the bridge as soon as bus 1 is
>> assigned .. :)
> There is one thing that I need to understand though. Before the commit
> above, how would enumeration works given that the subordinate bus number
> was set to 1 and that the kernel, AFAICS, does not overwrite it ?
>
> Are you able to send me a log (enumeration with debugging enabled and
> lspci) with the commit above reverted please ?
>
> Thanks,
> Lorenzo
>


Info below as requested:



[    0.116729] OF: PCI: host bridge /soc/pcie@0x01000000 ranges:
[    0.116748] OF: PCI:   No bus range found for /soc/pcie@0x01000000, 
using [bus 00-ff]
[    0.116777] OF: PCI:    IO 0x01f80000..0x01f8ffff -> 0x00000000
[    0.116796] OF: PCI:   MEM 0x01000000..0x01efffff -> 0x01000000
[    0.337917] imx6q-pcie 1ffc000.pcie: link up
[    0.337934] imx6q-pcie 1ffc000.pcie: Link: Gen2 disabled
[    0.337947] imx6q-pcie 1ffc000.pcie: link up
[    0.337958] imx6q-pcie 1ffc000.pcie: Link up, Gen1
[    0.338197] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
[    0.338215] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.338230] pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
[    0.338243] pci_bus 0000:00: root bus resource [mem 
0x01000000-0x01efffff]
[    0.338255] pci_bus 0000:00: scanning bus
[    0.338286] pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
[    0.338311] pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x000fffff]
[    0.338328] pci 0000:00:00.0: reg 0x38: [mem 0x00000000-0x0000ffff pref]
[    0.338362] pci 0000:00:00.0: calling pci_fixup_ide_bases+0x0/0x4c
[    0.338416] pci 0000:00:00.0: supports D1
[    0.338425] pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
[    0.338436] pci 0000:00:00.0: PME# disabled
[    0.338664] pci_bus 0000:00: fixups for bus
[    0.338676] PCI: bus0: Fast back to back transfers disabled
[    0.338692] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 0
[    0.338822] pci_bus 0000:01: scanning bus
[    0.338926] pci 0000:01:00.0: [10b5:8604] type 01 class 0x060400
[    0.338969] pci 0000:01:00.0: calling ventana_pciesw_early_fixup+0x0/0xa4
[    0.457984] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0001ffff]
[    0.458167] pci 0000:01:00.0: calling pci_fixup_ide_bases+0x0/0x4c
[    0.458635] pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
[    0.458660] pci 0000:01:00.0: PME# disabled
[    0.458970] pci_bus 0000:01: fixups for bus
[    0.459027] PCI: bus1: Fast back to back transfers disabled
[    0.459052] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 0
[    0.459060] pci 0000:01:00.0: bridge configuration invalid ([bus 
00-00]), reconfiguring
[    0.459115] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 1
[    0.459443] pci_bus 0000:02: busn_res: can not insert [bus 02-ff] 
under [bus 01] (conflicts with (null) [bus 01])
[    0.459461] pci_bus 0000:02: scanning bus
[    0.459573] pci 0000:02:01.0: [10b5:8604] type 01 class 0x060400
[    0.459617] pci 0000:02:01.0: calling ventana_pciesw_early_fixup+0x0/0xa4
[    0.459865] pci 0000:02:01.0: calling pci_fixup_ide_bases+0x0/0x4c
[    0.460298] pci 0000:02:01.0: PME# supported from D0 D3hot D3cold
[    0.460321] pci 0000:02:01.0: PME# disabled
[    0.460719] pci 0000:02:04.0: [10b5:8604] type 01 class 0x060400
[    0.460760] pci 0000:02:04.0: calling ventana_pciesw_early_fixup+0x0/0xa4
[    0.461009] pci 0000:02:04.0: calling pci_fixup_ide_bases+0x0/0x4c
[    0.461436] pci 0000:02:04.0: PME# supported from D0 D3hot D3cold
[    0.461460] pci 0000:02:04.0: PME# disabled
[    0.461841] pci 0000:02:05.0: [10b5:8604] type 01 class 0x060400
[    0.461883] pci 0000:02:05.0: calling ventana_pciesw_early_fixup+0x0/0xa4
[    0.462128] pci 0000:02:05.0: calling pci_fixup_ide_bases+0x0/0x4c
[    0.462553] pci 0000:02:05.0: PME# supported from D0 D3hot D3cold
[    0.462578] pci 0000:02:05.0: PME# disabled
[    0.463084] pci_bus 0000:02: fixups for bus
[    0.463231] PCI: bus2: Fast back to back transfers disabled
[    0.463255] pci 0000:02:01.0: scanning [bus 00-00] behind bridge, pass 0
[    0.463264] pci 0000:02:01.0: bridge configuration invalid ([bus 
00-00]), reconfiguring
[    0.463319] pci 0000:02:04.0: scanning [bus 00-00] behind bridge, pass 0
[    0.463328] pci 0000:02:04.0: bridge configuration invalid ([bus 
00-00]), reconfiguring
[    0.463378] pci 0000:02:05.0: scanning [bus 00-00] behind bridge, pass 0
[    0.463385] pci 0000:02:05.0: bridge configuration invalid ([bus 
00-00]), reconfiguring
[    0.463435] pci 0000:02:01.0: scanning [bus 00-00] behind bridge, pass 1
[    0.463764] pci_bus 0000:03: scanning bus
[    0.463785] pci_bus 0000:03: fixups for bus
[    0.463791] PCI: bus3: Fast back to back transfers enabled
[    0.463803] pci_bus 0000:03: bus scan returning with max=03
[    0.463814] pci_bus 0000:03: busn_res: [bus 03-ff] end is updated to 03
[    0.463833] pci_bus 0000:03: [bus 03] partially hidden behind bridge 
0000:01 [bus 01]
[    0.463862] pci 0000:02:04.0: scanning [bus 00-00] behind bridge, pass 1
[    0.464178] pci_bus 0000:04: scanning bus
[    0.464197] pci_bus 0000:04: fixups for bus
[    0.464202] PCI: bus4: Fast back to back transfers enabled
[    0.464214] pci_bus 0000:04: bus scan returning with max=04
[    0.464223] pci_bus 0000:04: busn_res: [bus 04-ff] end is updated to 04
[    0.464242] pci_bus 0000:04: [bus 04] partially hidden behind bridge 
0000:01 [bus 01]
[    0.464271] pci 0000:02:05.0: scanning [bus 00-00] behind bridge, pass 1
[    0.464586] pci_bus 0000:05: scanning bus
[    0.464691] pci 0000:05:00.0: [168c:0033] type 00 class 0x028000
[    0.464825] pci 0000:05:00.0: reg 0x10: [mem 0x00000000-0x0001ffff 64bit]
[    0.465036] pci 0000:05:00.0: reg 0x30: [mem 0x00000000-0x0000ffff pref]
[    0.465095] pci 0000:05:00.0: calling pci_fixup_ide_bases+0x0/0x4c
[    0.465117] pci 0000:05:00.0: calling quirk_no_bus_reset+0x0/0x20
[    0.465489] pci 0000:05:00.0: supports D1
[    0.465498] pci 0000:05:00.0: PME# supported from D0 D1 D3hot
[    0.465524] pci 0000:05:00.0: PME# disabled
[    0.465859] pci_bus 0000:05: fixups for bus
[    0.465903] PCI: bus5: Fast back to back transfers disabled
[    0.465916] pci_bus 0000:05: bus scan returning with max=05
[    0.465926] pci_bus 0000:05: busn_res: [bus 05-ff] end is updated to 05
[    0.465946] pci_bus 0000:05: [bus 05] partially hidden behind bridge 
0000:01 [bus 01]
[    0.465965] pci_bus 0000:02: bus scan returning with max=05
[    0.465974] pci_bus 0000:02: busn_res: [bus 02-ff] end is updated to 05
[    0.465984] pci_bus 0000:02: busn_res: can not insert [bus 02-05] 
under [bus 01] (conflicts with (null) [bus 01])
[    0.466005] pci_bus 0000:02: [bus 02-05] partially hidden behind 
bridge 0000:01 [bus 01]
[    0.466026] pci_bus 0000:01: bus scan returning with max=05
[    0.466033] pci 0000:00:00.0: bridge has subordinate 01 but max busn 05
[    0.466049] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 1
[    0.466059] pci_bus 0000:00: bus scan returning with max=01
[    0.466186] pci 0000:00:00.0: fixup irq: got 298
[    0.466196] pci 0000:00:00.0: assigning IRQ 298
[    0.466247] pci 0000:01:00.0: fixup irq: got 298
[    0.466254] pci 0000:01:00.0: assigning IRQ 298
[    0.466374] pci 0000:02:01.0: fixup irq: got 299
[    0.466382] pci 0000:02:01.0: assigning IRQ 299
[    0.466436] pci 0000:02:04.0: fixup irq: got 298
[    0.466442] pci 0000:02:04.0: assigning IRQ 298
[    0.466501] pci 0000:02:05.0: fixup irq: got 299
[    0.466509] pci 0000:02:05.0: assigning IRQ 299
[    0.466562] pci 0000:05:00.0: fixup irq: got 299
[    0.466569] pci 0000:05:00.0: assigning IRQ 299
[    0.466807] pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
[    0.466825] pci 0000:00:00.0: BAR 8: assigned [mem 0x01100000-0x012fffff]
[    0.466843] pci 0000:00:00.0: BAR 6: assigned [mem 
0x01300000-0x0130ffff pref]
[    0.466862] pci 0000:01:00.0: BAR 8: assigned [mem 0x01100000-0x011fffff]
[    0.466875] pci 0000:01:00.0: BAR 0: assigned [mem 0x01200000-0x0121ffff]
[    0.466908] pci 0000:02:05.0: BAR 8: assigned [mem 0x01100000-0x011fffff]
[    0.466919] pci 0000:02:01.0: PCI bridge to [bus 03]
[    0.467001] pci 0000:02:04.0: PCI bridge to [bus 04]
[    0.467086] pci 0000:05:00.0: BAR 0: assigned [mem 
0x01100000-0x0111ffff 64bit]
[    0.467160] pci 0000:05:00.0: BAR 6: assigned [mem 
0x01120000-0x0112ffff pref]
[    0.467171] pci 0000:02:05.0: PCI bridge to [bus 05]
[    0.467206] pci 0000:02:05.0:   bridge window [mem 0x01100000-0x011fffff]
[    0.467262] pci 0000:01:00.0: PCI bridge to [bus 02-05]
[    0.467297] pci 0000:01:00.0:   bridge window [mem 0x01100000-0x011fffff]
[    0.467352] pci 0000:00:00.0: PCI bridge to [bus 01]
[    0.467364] pci 0000:00:00.0:   bridge window [mem 0x01100000-0x012fffff]
[    0.467627] pcieport 0000:00:00.0: Signaling PME through PCIe PME 
interrupt
[    0.467643] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
[    0.467653] pci 0000:02:01.0: Signaling PME through PCIe PME interrupt
[    0.467662] pci 0000:02:04.0: Signaling PME through PCIe PME interrupt
[    0.467671] pci 0000:02:05.0: Signaling PME through PCIe PME interrupt
[    0.467680] pci 0000:05:00.0: Signaling PME through PCIe PME interrupt
[    0.467694] pcie_pme 0000:00:00.0:pcie001: service driver pcie_pme loaded
[    0.468019] pcieport 0000:01:00.0: enabling device (0140 -> 0142)
[    0.468147] pcieport 0000:01:00.0: enabling bus mastering
[    0.468886] pcieport 0000:02:01.0: enabling bus mastering
[    0.469576] pcieport 0000:02:04.0: enabling bus mastering
[    0.470165] pcieport 0000:02:05.0: enabling device (0140 -> 0142)
[    0.470275] pcieport 0000:02:05.0: enabling bus mastering


[ Node 4 | node-4 ] lspci -v
00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 
[Normal decode])
     Flags: bus master, fast devsel, latency 0, IRQ 298
     Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
     Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
     I/O behind bridge: None
     Memory behind bridge: 01100000-012fffff [size=2M]
     Prefetchable memory behind bridge: None
     [virtual] Expansion ROM at 01300000 [disabled] [size=64K]
     Capabilities: [40] Power Management version 3
     Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
     Capabilities: [70] Express Root Port (Slot-), MSI 00
     Capabilities: [100] Advanced Error Reporting
     Capabilities: [140] Virtual Channel
     Kernel driver in use: pcieport
lspci: Unable to load libkmod resources: error -12

01:00.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI 
Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal decode])
     Flags: bus master, fast devsel, latency 0, IRQ 298
     Memory at 01200000 (32-bit, non-prefetchable) [size=128K]
     Bus: primary=01, secondary=02, subordinate=05, sec-latency=0
     I/O behind bridge: None
     Memory behind bridge: 01100000-011fffff [size=1M]
     Prefetchable memory behind bridge: None
     Capabilities: [40] Power Management version 3
     Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
     Capabilities: [68] Express Upstream Port, MSI 00
     Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604 4-lane, 
4-Port PCI Express Gen 2 (5.0 GT/s) Switch
     Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
     Capabilities: [fb4] Advanced Error Reporting
     Capabilities: [138] Power Budgeting <?>
     Capabilities: [148] Virtual Channel
     Capabilities: [448] Vendor Specific Information: ID=0000 Rev=0 
Len=0cc <?>
     Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0 
Len=010 <?>
     Kernel driver in use: pcieport

02:01.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI 
Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal decode])
     Flags: bus master, fast devsel, latency 0, IRQ 299
     Bus: primary=02, secondary=03, subordinate=03, sec-latency=0
     I/O behind bridge: None
     Memory behind bridge: None
     Prefetchable memory behind bridge: None
     Capabilities: [40] Power Management version 3
     Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
     Capabilities: [68] Express Downstream Port (Slot+), MSI 00
     Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604 4-lane, 
4-Port PCI Express Gen 2 (5.0 GT/s) Switch
     Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
     Capabilities: [fb4] Advanced Error Reporting
     Capabilities: [148] Virtual Channel
     Capabilities: [520] Access Control Services
     Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0 
Len=010 <?>
     Kernel driver in use: pcieport

02:04.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI 
Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal decode])
     Flags: bus master, fast devsel, latency 0, IRQ 298
     Bus: primary=02, secondary=04, subordinate=04, sec-latency=0
     I/O behind bridge: None
     Memory behind bridge: None
     Prefetchable memory behind bridge: None
     Capabilities: [40] Power Management version 3
     Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
     Capabilities: [68] Express Downstream Port (Slot+), MSI 00
     Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604 4-lane, 
4-Port PCI Express Gen 2 (5.0 GT/s) Switch
     Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
     Capabilities: [fb4] Advanced Error Reporting
     Capabilities: [148] Virtual Channel
     Capabilities: [520] Access Control Services
     Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0 
Len=010 <?>
     Kernel driver in use: pcieport

02:05.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI 
Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal decode])
     Flags: bus master, fast devsel, latency 0, IRQ 299
     Bus: primary=02, secondary=05, subordinate=05, sec-latency=0
     I/O behind bridge: None
     Memory behind bridge: 01100000-011fffff [size=1M]
     Prefetchable memory behind bridge: None
     Capabilities: [40] Power Management version 3
     Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
     Capabilities: [68] Express Downstream Port (Slot+), MSI 00
     Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604 4-lane, 
4-Port PCI Express Gen 2 (5.0 GT/s) Switch
     Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
     Capabilities: [fb4] Advanced Error Reporting
     Capabilities: [148] Virtual Channel
     Capabilities: [520] Access Control Services
     Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0 
Len=010 <?>
     Kernel driver in use: pcieport

05:00.0 Network controller: Qualcomm Atheros AR958x 802.11abgn Wireless 
Network Adapter (rev 01)
     Subsystem: Device 19b6:d016
     Flags: bus master, fast devsel, latency 0, IRQ 299
     Memory at 01100000 (64-bit, non-prefetchable) [size=128K]
     [virtual] Expansion ROM at 01120000 [disabled] [size=64K]
     Capabilities: [40] Power Management version 3
     Capabilities: [50] MSI: Enable- Count=1/4 Maskable+ 64bit+
     Capabilities: [70] Express Endpoint, MSI 00
     Capabilities: [100] Advanced Error Reporting
     Capabilities: [140] Virtual Channel
     Capabilities: [300] Device Serial Number 00-00-00-00-00-00-00-00
     Kernel driver in use: ath9k

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-08  8:51             ` Koen Vandeputte
  0 siblings, 0 replies; 25+ messages in thread
From: Koen Vandeputte @ 2018-01-08  8:51 UTC (permalink / raw)
  To: linux-arm-kernel



On 2018-01-05 18:18, Lorenzo Pieralisi wrote:
>>
>> Hi Lorenzo,
>>
>> This is exactly what I'm trying to explain:
>>
>> The host starts of with a (hardcoded today) subord of 1. [bits 16:23]
>>
>> Since commit a20c7f36bd3d, downstream devices cannot assign bus nr's
>> higher than the subord of the upstream device.
>> So in this case, scanning stops after the bridge as soon as bus 1 is
>> assigned .. :)
> There is one thing that I need to understand though. Before the commit
> above, how would enumeration works given that the subordinate bus number
> was set to 1 and that the kernel, AFAICS, does not overwrite it ?
>
> Are you able to send me a log (enumeration with debugging enabled and
> lspci) with the commit above reverted please ?
>
> Thanks,
> Lorenzo
>


Info below as requested:



[??? 0.116729] OF: PCI: host bridge /soc/pcie at 0x01000000 ranges:
[??? 0.116748] OF: PCI:?? No bus range found for /soc/pcie at 0x01000000, 
using [bus 00-ff]
[??? 0.116777] OF: PCI:??? IO 0x01f80000..0x01f8ffff -> 0x00000000
[??? 0.116796] OF: PCI:?? MEM 0x01000000..0x01efffff -> 0x01000000
[??? 0.337917] imx6q-pcie 1ffc000.pcie: link up
[??? 0.337934] imx6q-pcie 1ffc000.pcie: Link: Gen2 disabled
[??? 0.337947] imx6q-pcie 1ffc000.pcie: link up
[??? 0.337958] imx6q-pcie 1ffc000.pcie: Link up, Gen1
[??? 0.338197] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
[??? 0.338215] pci_bus 0000:00: root bus resource [bus 00-ff]
[??? 0.338230] pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
[??? 0.338243] pci_bus 0000:00: root bus resource [mem 
0x01000000-0x01efffff]
[??? 0.338255] pci_bus 0000:00: scanning bus
[??? 0.338286] pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
[??? 0.338311] pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x000fffff]
[??? 0.338328] pci 0000:00:00.0: reg 0x38: [mem 0x00000000-0x0000ffff pref]
[??? 0.338362] pci 0000:00:00.0: calling pci_fixup_ide_bases+0x0/0x4c
[??? 0.338416] pci 0000:00:00.0: supports D1
[??? 0.338425] pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
[??? 0.338436] pci 0000:00:00.0: PME# disabled
[??? 0.338664] pci_bus 0000:00: fixups for bus
[??? 0.338676] PCI: bus0: Fast back to back transfers disabled
[??? 0.338692] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 0
[??? 0.338822] pci_bus 0000:01: scanning bus
[??? 0.338926] pci 0000:01:00.0: [10b5:8604] type 01 class 0x060400
[??? 0.338969] pci 0000:01:00.0: calling ventana_pciesw_early_fixup+0x0/0xa4
[??? 0.457984] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0001ffff]
[??? 0.458167] pci 0000:01:00.0: calling pci_fixup_ide_bases+0x0/0x4c
[??? 0.458635] pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
[??? 0.458660] pci 0000:01:00.0: PME# disabled
[??? 0.458970] pci_bus 0000:01: fixups for bus
[??? 0.459027] PCI: bus1: Fast back to back transfers disabled
[??? 0.459052] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 0
[??? 0.459060] pci 0000:01:00.0: bridge configuration invalid ([bus 
00-00]), reconfiguring
[??? 0.459115] pci 0000:01:00.0: scanning [bus 00-00] behind bridge, pass 1
[??? 0.459443] pci_bus 0000:02: busn_res: can not insert [bus 02-ff] 
under [bus 01] (conflicts with (null) [bus 01])
[??? 0.459461] pci_bus 0000:02: scanning bus
[??? 0.459573] pci 0000:02:01.0: [10b5:8604] type 01 class 0x060400
[??? 0.459617] pci 0000:02:01.0: calling ventana_pciesw_early_fixup+0x0/0xa4
[??? 0.459865] pci 0000:02:01.0: calling pci_fixup_ide_bases+0x0/0x4c
[??? 0.460298] pci 0000:02:01.0: PME# supported from D0 D3hot D3cold
[??? 0.460321] pci 0000:02:01.0: PME# disabled
[??? 0.460719] pci 0000:02:04.0: [10b5:8604] type 01 class 0x060400
[??? 0.460760] pci 0000:02:04.0: calling ventana_pciesw_early_fixup+0x0/0xa4
[??? 0.461009] pci 0000:02:04.0: calling pci_fixup_ide_bases+0x0/0x4c
[??? 0.461436] pci 0000:02:04.0: PME# supported from D0 D3hot D3cold
[??? 0.461460] pci 0000:02:04.0: PME# disabled
[??? 0.461841] pci 0000:02:05.0: [10b5:8604] type 01 class 0x060400
[??? 0.461883] pci 0000:02:05.0: calling ventana_pciesw_early_fixup+0x0/0xa4
[??? 0.462128] pci 0000:02:05.0: calling pci_fixup_ide_bases+0x0/0x4c
[??? 0.462553] pci 0000:02:05.0: PME# supported from D0 D3hot D3cold
[??? 0.462578] pci 0000:02:05.0: PME# disabled
[??? 0.463084] pci_bus 0000:02: fixups for bus
[??? 0.463231] PCI: bus2: Fast back to back transfers disabled
[??? 0.463255] pci 0000:02:01.0: scanning [bus 00-00] behind bridge, pass 0
[??? 0.463264] pci 0000:02:01.0: bridge configuration invalid ([bus 
00-00]), reconfiguring
[??? 0.463319] pci 0000:02:04.0: scanning [bus 00-00] behind bridge, pass 0
[??? 0.463328] pci 0000:02:04.0: bridge configuration invalid ([bus 
00-00]), reconfiguring
[??? 0.463378] pci 0000:02:05.0: scanning [bus 00-00] behind bridge, pass 0
[??? 0.463385] pci 0000:02:05.0: bridge configuration invalid ([bus 
00-00]), reconfiguring
[??? 0.463435] pci 0000:02:01.0: scanning [bus 00-00] behind bridge, pass 1
[??? 0.463764] pci_bus 0000:03: scanning bus
[??? 0.463785] pci_bus 0000:03: fixups for bus
[??? 0.463791] PCI: bus3: Fast back to back transfers enabled
[??? 0.463803] pci_bus 0000:03: bus scan returning with max=03
[??? 0.463814] pci_bus 0000:03: busn_res: [bus 03-ff] end is updated to 03
[??? 0.463833] pci_bus 0000:03: [bus 03] partially hidden behind bridge 
0000:01 [bus 01]
[??? 0.463862] pci 0000:02:04.0: scanning [bus 00-00] behind bridge, pass 1
[??? 0.464178] pci_bus 0000:04: scanning bus
[??? 0.464197] pci_bus 0000:04: fixups for bus
[??? 0.464202] PCI: bus4: Fast back to back transfers enabled
[??? 0.464214] pci_bus 0000:04: bus scan returning with max=04
[??? 0.464223] pci_bus 0000:04: busn_res: [bus 04-ff] end is updated to 04
[??? 0.464242] pci_bus 0000:04: [bus 04] partially hidden behind bridge 
0000:01 [bus 01]
[??? 0.464271] pci 0000:02:05.0: scanning [bus 00-00] behind bridge, pass 1
[??? 0.464586] pci_bus 0000:05: scanning bus
[??? 0.464691] pci 0000:05:00.0: [168c:0033] type 00 class 0x028000
[??? 0.464825] pci 0000:05:00.0: reg 0x10: [mem 0x00000000-0x0001ffff 64bit]
[??? 0.465036] pci 0000:05:00.0: reg 0x30: [mem 0x00000000-0x0000ffff pref]
[??? 0.465095] pci 0000:05:00.0: calling pci_fixup_ide_bases+0x0/0x4c
[??? 0.465117] pci 0000:05:00.0: calling quirk_no_bus_reset+0x0/0x20
[??? 0.465489] pci 0000:05:00.0: supports D1
[??? 0.465498] pci 0000:05:00.0: PME# supported from D0 D1 D3hot
[??? 0.465524] pci 0000:05:00.0: PME# disabled
[??? 0.465859] pci_bus 0000:05: fixups for bus
[??? 0.465903] PCI: bus5: Fast back to back transfers disabled
[??? 0.465916] pci_bus 0000:05: bus scan returning with max=05
[??? 0.465926] pci_bus 0000:05: busn_res: [bus 05-ff] end is updated to 05
[??? 0.465946] pci_bus 0000:05: [bus 05] partially hidden behind bridge 
0000:01 [bus 01]
[??? 0.465965] pci_bus 0000:02: bus scan returning with max=05
[??? 0.465974] pci_bus 0000:02: busn_res: [bus 02-ff] end is updated to 05
[??? 0.465984] pci_bus 0000:02: busn_res: can not insert [bus 02-05] 
under [bus 01] (conflicts with (null) [bus 01])
[??? 0.466005] pci_bus 0000:02: [bus 02-05] partially hidden behind 
bridge 0000:01 [bus 01]
[??? 0.466026] pci_bus 0000:01: bus scan returning with max=05
[??? 0.466033] pci 0000:00:00.0: bridge has subordinate 01 but max busn 05
[??? 0.466049] pci 0000:00:00.0: scanning [bus 01-01] behind bridge, pass 1
[??? 0.466059] pci_bus 0000:00: bus scan returning with max=01
[??? 0.466186] pci 0000:00:00.0: fixup irq: got 298
[??? 0.466196] pci 0000:00:00.0: assigning IRQ 298
[??? 0.466247] pci 0000:01:00.0: fixup irq: got 298
[??? 0.466254] pci 0000:01:00.0: assigning IRQ 298
[??? 0.466374] pci 0000:02:01.0: fixup irq: got 299
[??? 0.466382] pci 0000:02:01.0: assigning IRQ 299
[??? 0.466436] pci 0000:02:04.0: fixup irq: got 298
[??? 0.466442] pci 0000:02:04.0: assigning IRQ 298
[??? 0.466501] pci 0000:02:05.0: fixup irq: got 299
[??? 0.466509] pci 0000:02:05.0: assigning IRQ 299
[??? 0.466562] pci 0000:05:00.0: fixup irq: got 299
[??? 0.466569] pci 0000:05:00.0: assigning IRQ 299
[??? 0.466807] pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
[??? 0.466825] pci 0000:00:00.0: BAR 8: assigned [mem 0x01100000-0x012fffff]
[??? 0.466843] pci 0000:00:00.0: BAR 6: assigned [mem 
0x01300000-0x0130ffff pref]
[??? 0.466862] pci 0000:01:00.0: BAR 8: assigned [mem 0x01100000-0x011fffff]
[??? 0.466875] pci 0000:01:00.0: BAR 0: assigned [mem 0x01200000-0x0121ffff]
[??? 0.466908] pci 0000:02:05.0: BAR 8: assigned [mem 0x01100000-0x011fffff]
[??? 0.466919] pci 0000:02:01.0: PCI bridge to [bus 03]
[??? 0.467001] pci 0000:02:04.0: PCI bridge to [bus 04]
[??? 0.467086] pci 0000:05:00.0: BAR 0: assigned [mem 
0x01100000-0x0111ffff 64bit]
[??? 0.467160] pci 0000:05:00.0: BAR 6: assigned [mem 
0x01120000-0x0112ffff pref]
[??? 0.467171] pci 0000:02:05.0: PCI bridge to [bus 05]
[??? 0.467206] pci 0000:02:05.0:?? bridge window [mem 0x01100000-0x011fffff]
[??? 0.467262] pci 0000:01:00.0: PCI bridge to [bus 02-05]
[??? 0.467297] pci 0000:01:00.0:?? bridge window [mem 0x01100000-0x011fffff]
[??? 0.467352] pci 0000:00:00.0: PCI bridge to [bus 01]
[??? 0.467364] pci 0000:00:00.0:?? bridge window [mem 0x01100000-0x012fffff]
[??? 0.467627] pcieport 0000:00:00.0: Signaling PME through PCIe PME 
interrupt
[??? 0.467643] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
[??? 0.467653] pci 0000:02:01.0: Signaling PME through PCIe PME interrupt
[??? 0.467662] pci 0000:02:04.0: Signaling PME through PCIe PME interrupt
[??? 0.467671] pci 0000:02:05.0: Signaling PME through PCIe PME interrupt
[??? 0.467680] pci 0000:05:00.0: Signaling PME through PCIe PME interrupt
[??? 0.467694] pcie_pme 0000:00:00.0:pcie001: service driver pcie_pme loaded
[??? 0.468019] pcieport 0000:01:00.0: enabling device (0140 -> 0142)
[??? 0.468147] pcieport 0000:01:00.0: enabling bus mastering
[??? 0.468886] pcieport 0000:02:01.0: enabling bus mastering
[??? 0.469576] pcieport 0000:02:04.0: enabling bus mastering
[??? 0.470165] pcieport 0000:02:05.0: enabling device (0140 -> 0142)
[??? 0.470275] pcieport 0000:02:05.0: enabling bus mastering


[ Node 4 | node-4 ] lspci -v
00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 
[Normal decode])
 ??? Flags: bus master, fast devsel, latency 0, IRQ 298
 ??? Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
 ??? Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
 ??? I/O behind bridge: None
 ??? Memory behind bridge: 01100000-012fffff [size=2M]
 ??? Prefetchable memory behind bridge: None
 ??? [virtual] Expansion ROM at 01300000 [disabled] [size=64K]
 ??? Capabilities: [40] Power Management version 3
 ??? Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
 ??? Capabilities: [70] Express Root Port (Slot-), MSI 00
 ??? Capabilities: [100] Advanced Error Reporting
 ??? Capabilities: [140] Virtual Channel
 ??? Kernel driver in use: pcieport
lspci: Unable to load libkmod resources: error -12

01:00.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI 
Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal decode])
 ??? Flags: bus master, fast devsel, latency 0, IRQ 298
 ??? Memory at 01200000 (32-bit, non-prefetchable) [size=128K]
 ??? Bus: primary=01, secondary=02, subordinate=05, sec-latency=0
 ??? I/O behind bridge: None
 ??? Memory behind bridge: 01100000-011fffff [size=1M]
 ??? Prefetchable memory behind bridge: None
 ??? Capabilities: [40] Power Management version 3
 ??? Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
 ??? Capabilities: [68] Express Upstream Port, MSI 00
 ??? Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604 4-lane, 
4-Port PCI Express Gen 2 (5.0 GT/s) Switch
 ??? Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
 ??? Capabilities: [fb4] Advanced Error Reporting
 ??? Capabilities: [138] Power Budgeting <?>
 ??? Capabilities: [148] Virtual Channel
 ??? Capabilities: [448] Vendor Specific Information: ID=0000 Rev=0 
Len=0cc <?>
 ??? Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0 
Len=010 <?>
 ??? Kernel driver in use: pcieport

02:01.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI 
Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal decode])
 ??? Flags: bus master, fast devsel, latency 0, IRQ 299
 ??? Bus: primary=02, secondary=03, subordinate=03, sec-latency=0
 ??? I/O behind bridge: None
 ??? Memory behind bridge: None
 ??? Prefetchable memory behind bridge: None
 ??? Capabilities: [40] Power Management version 3
 ??? Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
 ??? Capabilities: [68] Express Downstream Port (Slot+), MSI 00
 ??? Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604 4-lane, 
4-Port PCI Express Gen 2 (5.0 GT/s) Switch
 ??? Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
 ??? Capabilities: [fb4] Advanced Error Reporting
 ??? Capabilities: [148] Virtual Channel
 ??? Capabilities: [520] Access Control Services
 ??? Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0 
Len=010 <?>
 ??? Kernel driver in use: pcieport

02:04.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI 
Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal decode])
 ??? Flags: bus master, fast devsel, latency 0, IRQ 298
 ??? Bus: primary=02, secondary=04, subordinate=04, sec-latency=0
 ??? I/O behind bridge: None
 ??? Memory behind bridge: None
 ??? Prefetchable memory behind bridge: None
 ??? Capabilities: [40] Power Management version 3
 ??? Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
 ??? Capabilities: [68] Express Downstream Port (Slot+), MSI 00
 ??? Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604 4-lane, 
4-Port PCI Express Gen 2 (5.0 GT/s) Switch
 ??? Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
 ??? Capabilities: [fb4] Advanced Error Reporting
 ??? Capabilities: [148] Virtual Channel
 ??? Capabilities: [520] Access Control Services
 ??? Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0 
Len=010 <?>
 ??? Kernel driver in use: pcieport

02:05.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI 
Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal decode])
 ??? Flags: bus master, fast devsel, latency 0, IRQ 299
 ??? Bus: primary=02, secondary=05, subordinate=05, sec-latency=0
 ??? I/O behind bridge: None
 ??? Memory behind bridge: 01100000-011fffff [size=1M]
 ??? Prefetchable memory behind bridge: None
 ??? Capabilities: [40] Power Management version 3
 ??? Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
 ??? Capabilities: [68] Express Downstream Port (Slot+), MSI 00
 ??? Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604 4-lane, 
4-Port PCI Express Gen 2 (5.0 GT/s) Switch
 ??? Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
 ??? Capabilities: [fb4] Advanced Error Reporting
 ??? Capabilities: [148] Virtual Channel
 ??? Capabilities: [520] Access Control Services
 ??? Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0 
Len=010 <?>
 ??? Kernel driver in use: pcieport

05:00.0 Network controller: Qualcomm Atheros AR958x 802.11abgn Wireless 
Network Adapter (rev 01)
 ??? Subsystem: Device 19b6:d016
 ??? Flags: bus master, fast devsel, latency 0, IRQ 299
 ??? Memory at 01100000 (64-bit, non-prefetchable) [size=128K]
 ??? [virtual] Expansion ROM at 01120000 [disabled] [size=64K]
 ??? Capabilities: [40] Power Management version 3
 ??? Capabilities: [50] MSI: Enable- Count=1/4 Maskable+ 64bit+
 ??? Capabilities: [70] Express Endpoint, MSI 00
 ??? Capabilities: [100] Advanced Error Reporting
 ??? Capabilities: [140] Virtual Channel
 ??? Capabilities: [300] Device Serial Number 00-00-00-00-00-00-00-00
 ??? Kernel driver in use: ath9k

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] imx6: fix pcie enumeration
  2018-01-08  8:51             ` Koen Vandeputte
@ 2018-01-08 11:00               ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 25+ messages in thread
From: Lorenzo Pieralisi @ 2018-01-08 11:00 UTC (permalink / raw)
  To: Koen Vandeputte
  Cc: Bjorn Helgaas, linux-pci, bhelgaas, Richard Zhu, Lucas Stach,
	linux-arm-kernel, Joao Pinto, Jingoo Han

[+cc Joao, Jingoo]

On Mon, Jan 08, 2018 at 09:51:37AM +0100, Koen Vandeputte wrote:

[...]

> [ Node 4 | node-4 ] lspci -v
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> [Normal decode])
>     Flags: bus master, fast devsel, latency 0, IRQ 298
>     Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
>     Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
                                     ^^^^^^^^^^^^^^

So basically, the subordinate number in the root port does not
affect config space forwarding from what I see and it has always
been like that for dwc.

You are forced to update it to 0xff because otherwise the kernel
stops enumerating bus numbers > 1 but that's a software issue
not HW - the subordinate bus number does not seem to affect anything
here.

Sigh.

Another option would consist in forcing the kernel to reassign
all bus numbers by setting the PCI_REASSIGN_ALL_BUS flag but
that's not a good idea given how inconsistent that flag usage is.

I think that updating the subordinate bus numbers in the DWC
config register is the correct solution to make sure the kernel
won't get confused anymore by what seems to be a fake root port,
I need input from DWC maintainers to confirm my understanding.

Thanks,
Lorenzo

>     I/O behind bridge: None
>     Memory behind bridge: 01100000-012fffff [size=2M]
>     Prefetchable memory behind bridge: None
>     [virtual] Expansion ROM at 01300000 [disabled] [size=64K]
>     Capabilities: [40] Power Management version 3
>     Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
>     Capabilities: [70] Express Root Port (Slot-), MSI 00
>     Capabilities: [100] Advanced Error Reporting
>     Capabilities: [140] Virtual Channel
>     Kernel driver in use: pcieport
> lspci: Unable to load libkmod resources: error -12
> 
> 01:00.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI
> Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal
> decode])
>     Flags: bus master, fast devsel, latency 0, IRQ 298
>     Memory at 01200000 (32-bit, non-prefetchable) [size=128K]
>     Bus: primary=01, secondary=02, subordinate=05, sec-latency=0
>     I/O behind bridge: None
>     Memory behind bridge: 01100000-011fffff [size=1M]
>     Prefetchable memory behind bridge: None
>     Capabilities: [40] Power Management version 3
>     Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
>     Capabilities: [68] Express Upstream Port, MSI 00
>     Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604
> 4-lane, 4-Port PCI Express Gen 2 (5.0 GT/s) Switch
>     Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
>     Capabilities: [fb4] Advanced Error Reporting
>     Capabilities: [138] Power Budgeting <?>
>     Capabilities: [148] Virtual Channel
>     Capabilities: [448] Vendor Specific Information: ID=0000 Rev=0
> Len=0cc <?>
>     Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0
> Len=010 <?>
>     Kernel driver in use: pcieport
> 
> 02:01.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI
> Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal
> decode])
>     Flags: bus master, fast devsel, latency 0, IRQ 299
>     Bus: primary=02, secondary=03, subordinate=03, sec-latency=0
>     I/O behind bridge: None
>     Memory behind bridge: None
>     Prefetchable memory behind bridge: None
>     Capabilities: [40] Power Management version 3
>     Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
>     Capabilities: [68] Express Downstream Port (Slot+), MSI 00
>     Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604
> 4-lane, 4-Port PCI Express Gen 2 (5.0 GT/s) Switch
>     Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
>     Capabilities: [fb4] Advanced Error Reporting
>     Capabilities: [148] Virtual Channel
>     Capabilities: [520] Access Control Services
>     Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0
> Len=010 <?>
>     Kernel driver in use: pcieport
> 
> 02:04.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI
> Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal
> decode])
>     Flags: bus master, fast devsel, latency 0, IRQ 298
>     Bus: primary=02, secondary=04, subordinate=04, sec-latency=0
>     I/O behind bridge: None
>     Memory behind bridge: None
>     Prefetchable memory behind bridge: None
>     Capabilities: [40] Power Management version 3
>     Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
>     Capabilities: [68] Express Downstream Port (Slot+), MSI 00
>     Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604
> 4-lane, 4-Port PCI Express Gen 2 (5.0 GT/s) Switch
>     Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
>     Capabilities: [fb4] Advanced Error Reporting
>     Capabilities: [148] Virtual Channel
>     Capabilities: [520] Access Control Services
>     Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0
> Len=010 <?>
>     Kernel driver in use: pcieport
> 
> 02:05.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI
> Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal
> decode])
>     Flags: bus master, fast devsel, latency 0, IRQ 299
>     Bus: primary=02, secondary=05, subordinate=05, sec-latency=0
>     I/O behind bridge: None
>     Memory behind bridge: 01100000-011fffff [size=1M]
>     Prefetchable memory behind bridge: None
>     Capabilities: [40] Power Management version 3
>     Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
>     Capabilities: [68] Express Downstream Port (Slot+), MSI 00
>     Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604
> 4-lane, 4-Port PCI Express Gen 2 (5.0 GT/s) Switch
>     Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
>     Capabilities: [fb4] Advanced Error Reporting
>     Capabilities: [148] Virtual Channel
>     Capabilities: [520] Access Control Services
>     Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0
> Len=010 <?>
>     Kernel driver in use: pcieport
> 
> 05:00.0 Network controller: Qualcomm Atheros AR958x 802.11abgn
> Wireless Network Adapter (rev 01)
>     Subsystem: Device 19b6:d016
>     Flags: bus master, fast devsel, latency 0, IRQ 299
>     Memory at 01100000 (64-bit, non-prefetchable) [size=128K]
>     [virtual] Expansion ROM at 01120000 [disabled] [size=64K]
>     Capabilities: [40] Power Management version 3
>     Capabilities: [50] MSI: Enable- Count=1/4 Maskable+ 64bit+
>     Capabilities: [70] Express Endpoint, MSI 00
>     Capabilities: [100] Advanced Error Reporting
>     Capabilities: [140] Virtual Channel
>     Capabilities: [300] Device Serial Number 00-00-00-00-00-00-00-00
>     Kernel driver in use: ath9k
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-08 11:00               ` Lorenzo Pieralisi
  0 siblings, 0 replies; 25+ messages in thread
From: Lorenzo Pieralisi @ 2018-01-08 11:00 UTC (permalink / raw)
  To: linux-arm-kernel

[+cc Joao, Jingoo]

On Mon, Jan 08, 2018 at 09:51:37AM +0100, Koen Vandeputte wrote:

[...]

> [ Node 4 | node-4 ] lspci -v
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> [Normal decode])
> ??? Flags: bus master, fast devsel, latency 0, IRQ 298
> ??? Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
> ??? Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
                                     ^^^^^^^^^^^^^^

So basically, the subordinate number in the root port does not
affect config space forwarding from what I see and it has always
been like that for dwc.

You are forced to update it to 0xff because otherwise the kernel
stops enumerating bus numbers > 1 but that's a software issue
not HW - the subordinate bus number does not seem to affect anything
here.

Sigh.

Another option would consist in forcing the kernel to reassign
all bus numbers by setting the PCI_REASSIGN_ALL_BUS flag but
that's not a good idea given how inconsistent that flag usage is.

I think that updating the subordinate bus numbers in the DWC
config register is the correct solution to make sure the kernel
won't get confused anymore by what seems to be a fake root port,
I need input from DWC maintainers to confirm my understanding.

Thanks,
Lorenzo

> ??? I/O behind bridge: None
> ??? Memory behind bridge: 01100000-012fffff [size=2M]
> ??? Prefetchable memory behind bridge: None
> ??? [virtual] Expansion ROM at 01300000 [disabled] [size=64K]
> ??? Capabilities: [40] Power Management version 3
> ??? Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
> ??? Capabilities: [70] Express Root Port (Slot-), MSI 00
> ??? Capabilities: [100] Advanced Error Reporting
> ??? Capabilities: [140] Virtual Channel
> ??? Kernel driver in use: pcieport
> lspci: Unable to load libkmod resources: error -12
> 
> 01:00.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI
> Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal
> decode])
> ??? Flags: bus master, fast devsel, latency 0, IRQ 298
> ??? Memory at 01200000 (32-bit, non-prefetchable) [size=128K]
> ??? Bus: primary=01, secondary=02, subordinate=05, sec-latency=0
> ??? I/O behind bridge: None
> ??? Memory behind bridge: 01100000-011fffff [size=1M]
> ??? Prefetchable memory behind bridge: None
> ??? Capabilities: [40] Power Management version 3
> ??? Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
> ??? Capabilities: [68] Express Upstream Port, MSI 00
> ??? Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604
> 4-lane, 4-Port PCI Express Gen 2 (5.0 GT/s) Switch
> ??? Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
> ??? Capabilities: [fb4] Advanced Error Reporting
> ??? Capabilities: [138] Power Budgeting <?>
> ??? Capabilities: [148] Virtual Channel
> ??? Capabilities: [448] Vendor Specific Information: ID=0000 Rev=0
> Len=0cc <?>
> ??? Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0
> Len=010 <?>
> ??? Kernel driver in use: pcieport
> 
> 02:01.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI
> Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal
> decode])
> ??? Flags: bus master, fast devsel, latency 0, IRQ 299
> ??? Bus: primary=02, secondary=03, subordinate=03, sec-latency=0
> ??? I/O behind bridge: None
> ??? Memory behind bridge: None
> ??? Prefetchable memory behind bridge: None
> ??? Capabilities: [40] Power Management version 3
> ??? Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
> ??? Capabilities: [68] Express Downstream Port (Slot+), MSI 00
> ??? Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604
> 4-lane, 4-Port PCI Express Gen 2 (5.0 GT/s) Switch
> ??? Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
> ??? Capabilities: [fb4] Advanced Error Reporting
> ??? Capabilities: [148] Virtual Channel
> ??? Capabilities: [520] Access Control Services
> ??? Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0
> Len=010 <?>
> ??? Kernel driver in use: pcieport
> 
> 02:04.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI
> Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal
> decode])
> ??? Flags: bus master, fast devsel, latency 0, IRQ 298
> ??? Bus: primary=02, secondary=04, subordinate=04, sec-latency=0
> ??? I/O behind bridge: None
> ??? Memory behind bridge: None
> ??? Prefetchable memory behind bridge: None
> ??? Capabilities: [40] Power Management version 3
> ??? Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
> ??? Capabilities: [68] Express Downstream Port (Slot+), MSI 00
> ??? Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604
> 4-lane, 4-Port PCI Express Gen 2 (5.0 GT/s) Switch
> ??? Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
> ??? Capabilities: [fb4] Advanced Error Reporting
> ??? Capabilities: [148] Virtual Channel
> ??? Capabilities: [520] Access Control Services
> ??? Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0
> Len=010 <?>
> ??? Kernel driver in use: pcieport
> 
> 02:05.0 PCI bridge: PLX Technology, Inc. PEX 8604 4-lane, 4-Port PCI
> Express Gen 2 (5.0 GT/s) Switch (rev ba) (prog-if 00 [Normal
> decode])
> ??? Flags: bus master, fast devsel, latency 0, IRQ 299
> ??? Bus: primary=02, secondary=05, subordinate=05, sec-latency=0
> ??? I/O behind bridge: None
> ??? Memory behind bridge: 01100000-011fffff [size=1M]
> ??? Prefetchable memory behind bridge: None
> ??? Capabilities: [40] Power Management version 3
> ??? Capabilities: [48] MSI: Enable- Count=1/4 Maskable+ 64bit+
> ??? Capabilities: [68] Express Downstream Port (Slot+), MSI 00
> ??? Capabilities: [a4] Subsystem: PLX Technology, Inc. PEX 8604
> 4-lane, 4-Port PCI Express Gen 2 (5.0 GT/s) Switch
> ??? Capabilities: [100] Device Serial Number ba-86-01-10-b5-df-0e-00
> ??? Capabilities: [fb4] Advanced Error Reporting
> ??? Capabilities: [148] Virtual Channel
> ??? Capabilities: [520] Access Control Services
> ??? Capabilities: [950] Vendor Specific Information: ID=0001 Rev=0
> Len=010 <?>
> ??? Kernel driver in use: pcieport
> 
> 05:00.0 Network controller: Qualcomm Atheros AR958x 802.11abgn
> Wireless Network Adapter (rev 01)
> ??? Subsystem: Device 19b6:d016
> ??? Flags: bus master, fast devsel, latency 0, IRQ 299
> ??? Memory at 01100000 (64-bit, non-prefetchable) [size=128K]
> ??? [virtual] Expansion ROM at 01120000 [disabled] [size=64K]
> ??? Capabilities: [40] Power Management version 3
> ??? Capabilities: [50] MSI: Enable- Count=1/4 Maskable+ 64bit+
> ??? Capabilities: [70] Express Endpoint, MSI 00
> ??? Capabilities: [100] Advanced Error Reporting
> ??? Capabilities: [140] Virtual Channel
> ??? Capabilities: [300] Device Serial Number 00-00-00-00-00-00-00-00
> ??? Kernel driver in use: ath9k
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] imx6: fix pcie enumeration
  2018-01-08 11:00               ` Lorenzo Pieralisi
@ 2018-01-08 11:13                 ` Koen Vandeputte
  -1 siblings, 0 replies; 25+ messages in thread
From: Koen Vandeputte @ 2018-01-08 11:13 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Bjorn Helgaas, linux-pci, bhelgaas, Richard Zhu, Lucas Stach,
	linux-arm-kernel, Joao Pinto, Jingoo Han



On 2018-01-08 12:00, Lorenzo Pieralisi wrote:
> [+cc Joao, Jingoo]
>
> On Mon, Jan 08, 2018 at 09:51:37AM +0100, Koen Vandeputte wrote:
>
> [...]
>
>> [ Node 4 | node-4 ] lspci -v
>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
>> [Normal decode])
>>      Flags: bus master, fast devsel, latency 0, IRQ 298
>>      Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
>>      Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>                                       ^^^^^^^^^^^^^^
>
> So basically, the subordinate number in the root port does not
> affect config space forwarding from what I see and it has always
> been like that for dwc.
>
> You are forced to update it to 0xff because otherwise the kernel
> stops enumerating bus numbers > 1
Indeed, which affects all devices using Designware PCIe init + a PCIe 
bridge downstream
> but that's a software issue
> not HW - the subordinate bus number does not seem to affect anything
> here.

> Sigh.
>
> Another option would consist in forcing the kernel to reassign
> all bus numbers by setting the PCI_REASSIGN_ALL_BUS flag but
> that's not a good idea given how inconsistent that flag usage is.
>
> I think that updating the subordinate bus numbers in the DWC
> config register is the correct solution to make sure the kernel
> won't get confused anymore by what seems to be a fake root port,
> I need input from DWC maintainers to confirm my understanding.
>
> Thanks,
> Lorenzo
>

The patch I'm currently using internally:


--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -861,7 +861,7 @@ void dw_pcie_setup_rc(struct pcie_port *
      /* setup bus numbers */
      val = dw_pcie_readl_rc(pp, PCI_PRIMARY_BUS);
      val &= 0xff000000;
-    val |= 0x00010100;
+    val |= 0x00ff0100;
      dw_pcie_writel_rc(pp, PCI_PRIMARY_BUS, val);

      /* setup command register */


Above version logically fixes it for all dwc devices using a bridge 
after the RC, not only imx6.
If this is fine, I would submit the patch above and drop the current one.

Backporting this to stable kernels (4.9 .. 4.4 .. etc) will fix all 
nasty warnings on these setups during boot without any change in 
functionality.
These kernels will require a separate patch as this source file got 
moved & renamed.


Thanks for your time and analysis so far,

Koen

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-08 11:13                 ` Koen Vandeputte
  0 siblings, 0 replies; 25+ messages in thread
From: Koen Vandeputte @ 2018-01-08 11:13 UTC (permalink / raw)
  To: linux-arm-kernel



On 2018-01-08 12:00, Lorenzo Pieralisi wrote:
> [+cc Joao, Jingoo]
>
> On Mon, Jan 08, 2018 at 09:51:37AM +0100, Koen Vandeputte wrote:
>
> [...]
>
>> [ Node 4 | node-4 ] lspci -v
>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
>> [Normal decode])
>>  ??? Flags: bus master, fast devsel, latency 0, IRQ 298
>>  ??? Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
>>  ??? Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>                                       ^^^^^^^^^^^^^^
>
> So basically, the subordinate number in the root port does not
> affect config space forwarding from what I see and it has always
> been like that for dwc.
>
> You are forced to update it to 0xff because otherwise the kernel
> stops enumerating bus numbers > 1
Indeed, which affects all devices using Designware PCIe init + a PCIe 
bridge downstream
> but that's a software issue
> not HW - the subordinate bus number does not seem to affect anything
> here.

> Sigh.
>
> Another option would consist in forcing the kernel to reassign
> all bus numbers by setting the PCI_REASSIGN_ALL_BUS flag but
> that's not a good idea given how inconsistent that flag usage is.
>
> I think that updating the subordinate bus numbers in the DWC
> config register is the correct solution to make sure the kernel
> won't get confused anymore by what seems to be a fake root port,
> I need input from DWC maintainers to confirm my understanding.
>
> Thanks,
> Lorenzo
>

The patch I'm currently using internally:


--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -861,7 +861,7 @@ void dw_pcie_setup_rc(struct pcie_port *
 ???? /* setup bus numbers */
 ???? val = dw_pcie_readl_rc(pp, PCI_PRIMARY_BUS);
 ???? val &= 0xff000000;
-??? val |= 0x00010100;
+??? val |= 0x00ff0100;
 ???? dw_pcie_writel_rc(pp, PCI_PRIMARY_BUS, val);

 ???? /* setup command register */


Above version logically fixes it for all dwc devices using a bridge 
after the RC, not only imx6.
If this is fine, I would submit the patch above and drop the current one.

Backporting this to stable kernels (4.9 .. 4.4 .. etc) will fix all 
nasty warnings on these setups during boot without any change in 
functionality.
These kernels will require a separate patch as this source file got 
moved & renamed.


Thanks for your time and analysis so far,

Koen

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] imx6: fix pcie enumeration
  2018-01-08 11:13                 ` Koen Vandeputte
@ 2018-01-08 15:46                   ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 25+ messages in thread
From: Lorenzo Pieralisi @ 2018-01-08 15:46 UTC (permalink / raw)
  To: Koen Vandeputte
  Cc: Bjorn Helgaas, linux-pci, bhelgaas, Richard Zhu, Lucas Stach,
	linux-arm-kernel, Joao Pinto, Jingoo Han

On Mon, Jan 08, 2018 at 12:13:34PM +0100, Koen Vandeputte wrote:
> 
> 
> On 2018-01-08 12:00, Lorenzo Pieralisi wrote:
> >[+cc Joao, Jingoo]
> >
> >On Mon, Jan 08, 2018 at 09:51:37AM +0100, Koen Vandeputte wrote:
> >
> >[...]
> >
> >>[ Node 4 | node-4 ] lspci -v
> >>00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> >>[Normal decode])
> >>     Flags: bus master, fast devsel, latency 0, IRQ 298
> >>     Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
> >>     Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> >                                      ^^^^^^^^^^^^^^
> >
> >So basically, the subordinate number in the root port does not
> >affect config space forwarding from what I see and it has always
> >been like that for dwc.
> >
> >You are forced to update it to 0xff because otherwise the kernel
> >stops enumerating bus numbers > 1
> Indeed, which affects all devices using Designware PCIe init + a
> PCIe bridge downstream
> >but that's a software issue
> >not HW - the subordinate bus number does not seem to affect anything
> >here.
> 
> >Sigh.
> >
> >Another option would consist in forcing the kernel to reassign
> >all bus numbers by setting the PCI_REASSIGN_ALL_BUS flag but
> >that's not a good idea given how inconsistent that flag usage is.
> >
> >I think that updating the subordinate bus numbers in the DWC
> >config register is the correct solution to make sure the kernel
> >won't get confused anymore by what seems to be a fake root port,
> >I need input from DWC maintainers to confirm my understanding.
> >
> >Thanks,
> >Lorenzo
> >
> 
> The patch I'm currently using internally:
> 
> 
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -861,7 +861,7 @@ void dw_pcie_setup_rc(struct pcie_port *
>      /* setup bus numbers */
>      val = dw_pcie_readl_rc(pp, PCI_PRIMARY_BUS);
>      val &= 0xff000000;
> -    val |= 0x00010100;
> +    val |= 0x00ff0100;
>      dw_pcie_writel_rc(pp, PCI_PRIMARY_BUS, val);
> 
>      /* setup command register */
> 
> 
> Above version logically fixes it for all dwc devices using a bridge
> after the RC, not only imx6.
> If this is fine, I would submit the patch above and drop the current one.
It is fine by me but I won't merge it till I get ACKs and tested-by
from the respective maintainers - it can have potential widespread
impact.

> Backporting this to stable kernels (4.9 .. 4.4 .. etc) will fix all
> nasty warnings on these setups during boot without any change in
> functionality.
> These kernels will require a separate patch as this source file got
> moved & renamed.
> Thanks for your time and analysis so far,

Thank you for reporting it and fixing it.

Lorenzo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-08 15:46                   ` Lorenzo Pieralisi
  0 siblings, 0 replies; 25+ messages in thread
From: Lorenzo Pieralisi @ 2018-01-08 15:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 08, 2018 at 12:13:34PM +0100, Koen Vandeputte wrote:
> 
> 
> On 2018-01-08 12:00, Lorenzo Pieralisi wrote:
> >[+cc Joao, Jingoo]
> >
> >On Mon, Jan 08, 2018 at 09:51:37AM +0100, Koen Vandeputte wrote:
> >
> >[...]
> >
> >>[ Node 4 | node-4 ] lspci -v
> >>00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> >>[Normal decode])
> >> ??? Flags: bus master, fast devsel, latency 0, IRQ 298
> >> ??? Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
> >> ??? Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> >                                      ^^^^^^^^^^^^^^
> >
> >So basically, the subordinate number in the root port does not
> >affect config space forwarding from what I see and it has always
> >been like that for dwc.
> >
> >You are forced to update it to 0xff because otherwise the kernel
> >stops enumerating bus numbers > 1
> Indeed, which affects all devices using Designware PCIe init + a
> PCIe bridge downstream
> >but that's a software issue
> >not HW - the subordinate bus number does not seem to affect anything
> >here.
> 
> >Sigh.
> >
> >Another option would consist in forcing the kernel to reassign
> >all bus numbers by setting the PCI_REASSIGN_ALL_BUS flag but
> >that's not a good idea given how inconsistent that flag usage is.
> >
> >I think that updating the subordinate bus numbers in the DWC
> >config register is the correct solution to make sure the kernel
> >won't get confused anymore by what seems to be a fake root port,
> >I need input from DWC maintainers to confirm my understanding.
> >
> >Thanks,
> >Lorenzo
> >
> 
> The patch I'm currently using internally:
> 
> 
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -861,7 +861,7 @@ void dw_pcie_setup_rc(struct pcie_port *
> ???? /* setup bus numbers */
> ???? val = dw_pcie_readl_rc(pp, PCI_PRIMARY_BUS);
> ???? val &= 0xff000000;
> -??? val |= 0x00010100;
> +??? val |= 0x00ff0100;
> ???? dw_pcie_writel_rc(pp, PCI_PRIMARY_BUS, val);
> 
> ???? /* setup command register */
> 
> 
> Above version logically fixes it for all dwc devices using a bridge
> after the RC, not only imx6.
> If this is fine, I would submit the patch above and drop the current one.
It is fine by me but I won't merge it till I get ACKs and tested-by
from the respective maintainers - it can have potential widespread
impact.

> Backporting this to stable kernels (4.9 .. 4.4 .. etc) will fix all
> nasty warnings on these setups during boot without any change in
> functionality.
> These kernels will require a separate patch as this source file got
> moved & renamed.
> Thanks for your time and analysis so far,

Thank you for reporting it and fixing it.

Lorenzo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Re: [PATCH] imx6: fix pcie enumeration
  2018-01-08 11:13                 ` Koen Vandeputte
@ 2018-01-09 13:48                   ` Niklas Cassel
  -1 siblings, 0 replies; 25+ messages in thread
From: Niklas Cassel @ 2018-01-09 13:48 UTC (permalink / raw)
  To: Koen Vandeputte, Lorenzo Pieralisi
  Cc: Bjorn Helgaas, linux-pci, bhelgaas, Richard Zhu, Lucas Stach,
	linux-arm-kernel, Joao Pinto, Jingoo Han

On 08/01/18 12:13, Koen Vandeputte wrote:
> 
> 
> On 2018-01-08 12:00, Lorenzo Pieralisi wrote:
>> [+cc Joao, Jingoo]
>>
>> On Mon, Jan 08, 2018 at 09:51:37AM +0100, Koen Vandeputte wrote:
>>
>> [...]
>>
>>> [ Node 4 | node-4 ] lspci -v
>>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
>>> [Normal decode])
>>>      Flags: bus master, fast devsel, latency 0, IRQ 298
>>>      Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
>>>      Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>>                                       ^^^^^^^^^^^^^^
>>
>> So basically, the subordinate number in the root port does not
>> affect config space forwarding from what I see and it has always
>> been like that for dwc.
>>
>> You are forced to update it to 0xff because otherwise the kernel
>> stops enumerating bus numbers > 1
> Indeed, which affects all devices using Designware PCIe init + a PCIe bridge downstream
>> but that's a software issue
>> not HW - the subordinate bus number does not seem to affect anything
>> here.
> 
>> Sigh.
>>
>> Another option would consist in forcing the kernel to reassign
>> all bus numbers by setting the PCI_REASSIGN_ALL_BUS flag but
>> that's not a good idea given how inconsistent that flag usage is.
>>
>> I think that updating the subordinate bus numbers in the DWC
>> config register is the correct solution to make sure the kernel
>> won't get confused anymore by what seems to be a fake root port,
>> I need input from DWC maintainers to confirm my understanding.
>>
>> Thanks,
>> Lorenzo
>>
> 
> The patch I'm currently using internally:
> 
> 
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -861,7 +861,7 @@ void dw_pcie_setup_rc(struct pcie_port *
>      /* setup bus numbers */
>      val = dw_pcie_readl_rc(pp, PCI_PRIMARY_BUS);
>      val &= 0xff000000;
> -    val |= 0x00010100;
> +    val |= 0x00ff0100;
>      dw_pcie_writel_rc(pp, PCI_PRIMARY_BUS, val);
> 
>      /* setup command register */
> 
> 
> Above version logically fixes it for all dwc devices using a bridge after the RC, not only imx6.
> If this is fine, I would submit the patch above and drop the current one.

I can confirm that commit a20c7f36bd3d ("PCI: Do not allocate more buses than
available in parent") broke enumerating PCIe devices behind a PCIe switch
on ARTPEC-6 (which uses the DWC IP).
(FTR, arch/arm/boot/dts/artpec6.dtsi specifies bus-range = <0x00 0xff>).

This patch resolves the problem.
(I verified on linux-next: next-20180109).

However, note that I had to patch the file
drivers/pci/dwc/pcie-designware-host.c
rather than
drivers/pci/host/pcie-designware.c,
which this patch suggests.

Please feel free to submit a new patch with:

Tested-by: Niklas Cassel <niklas.cassel@axis.com>

> 
> Backporting this to stable kernels (4.9 .. 4.4 .. etc) will fix all nasty warnings on these setups during boot without any change in functionality.
> These kernels will require a separate patch as this source file got moved & renamed.
> 
> 
> Thanks for your time and analysis so far,
> 
> Koen

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-09 13:48                   ` Niklas Cassel
  0 siblings, 0 replies; 25+ messages in thread
From: Niklas Cassel @ 2018-01-09 13:48 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/01/18 12:13, Koen Vandeputte wrote:
> 
> 
> On 2018-01-08 12:00, Lorenzo Pieralisi wrote:
>> [+cc Joao, Jingoo]
>>
>> On Mon, Jan 08, 2018 at 09:51:37AM +0100, Koen Vandeputte wrote:
>>
>> [...]
>>
>>> [ Node 4 | node-4 ] lspci -v
>>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
>>> [Normal decode])
>>> ???? Flags: bus master, fast devsel, latency 0, IRQ 298
>>> ???? Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
>>> ???? Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>> ????????????????????????????????????? ^^^^^^^^^^^^^^
>>
>> So basically, the subordinate number in the root port does not
>> affect config space forwarding from what I see and it has always
>> been like that for dwc.
>>
>> You are forced to update it to 0xff because otherwise the kernel
>> stops enumerating bus numbers > 1
> Indeed, which affects all devices using Designware PCIe init + a PCIe bridge downstream
>> but that's a software issue
>> not HW - the subordinate bus number does not seem to affect anything
>> here.
> 
>> Sigh.
>>
>> Another option would consist in forcing the kernel to reassign
>> all bus numbers by setting the PCI_REASSIGN_ALL_BUS flag but
>> that's not a good idea given how inconsistent that flag usage is.
>>
>> I think that updating the subordinate bus numbers in the DWC
>> config register is the correct solution to make sure the kernel
>> won't get confused anymore by what seems to be a fake root port,
>> I need input from DWC maintainers to confirm my understanding.
>>
>> Thanks,
>> Lorenzo
>>
> 
> The patch I'm currently using internally:
> 
> 
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -861,7 +861,7 @@ void dw_pcie_setup_rc(struct pcie_port *
> ???? /* setup bus numbers */
> ???? val = dw_pcie_readl_rc(pp, PCI_PRIMARY_BUS);
> ???? val &= 0xff000000;
> -??? val |= 0x00010100;
> +??? val |= 0x00ff0100;
> ???? dw_pcie_writel_rc(pp, PCI_PRIMARY_BUS, val);
> 
> ???? /* setup command register */
> 
> 
> Above version logically fixes it for all dwc devices using a bridge after the RC, not only imx6.
> If this is fine, I would submit the patch above and drop the current one.

I can confirm that commit a20c7f36bd3d ("PCI: Do not allocate more buses than
available in parent") broke enumerating PCIe devices behind a PCIe switch
on ARTPEC-6 (which uses the DWC IP).
(FTR, arch/arm/boot/dts/artpec6.dtsi specifies bus-range = <0x00 0xff>).

This patch resolves the problem.
(I verified on linux-next: next-20180109).

However, note that I had to patch the file
drivers/pci/dwc/pcie-designware-host.c
rather than
drivers/pci/host/pcie-designware.c,
which this patch suggests.

Please feel free to submit a new patch with:

Tested-by: Niklas Cassel <niklas.cassel@axis.com>

> 
> Backporting this to stable kernels (4.9 .. 4.4 .. etc) will fix all nasty warnings on these setups during boot without any change in functionality.
> These kernels will require a separate patch as this source file got moved & renamed.
> 
> 
> Thanks for your time and analysis so far,
> 
> Koen

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] imx6: fix pcie enumeration
  2018-01-09 13:48                   ` Niklas Cassel
@ 2018-01-09 13:58                     ` Koen Vandeputte
  -1 siblings, 0 replies; 25+ messages in thread
From: Koen Vandeputte @ 2018-01-09 13:58 UTC (permalink / raw)
  To: Niklas Cassel, Lorenzo Pieralisi
  Cc: Bjorn Helgaas, linux-pci, bhelgaas, Richard Zhu, Lucas Stach,
	linux-arm-kernel, Joao Pinto, Jingoo Han



On 2018-01-09 14:48, Niklas Cassel wrote:
> <snip>

>>       /* setup bus numbers */
>>       val = dw_pcie_readl_rc(pp, PCI_PRIMARY_BUS);
>>       val &= 0xff000000;
>> -    val |= 0x00010100;
>> +    val |= 0x00ff0100;
>>       dw_pcie_writel_rc(pp, PCI_PRIMARY_BUS, val);
>>
>>       /* setup command register */
>>
>>
>> Above version logically fixes it for all dwc devices using a bridge after the RC, not only imx6.
>> If this is fine, I would submit the patch above and drop the current one.
> I can confirm that commit a20c7f36bd3d ("PCI: Do not allocate more buses than
> available in parent") broke enumerating PCIe devices behind a PCIe switch
> on ARTPEC-6 (which uses the DWC IP).
> (FTR, arch/arm/boot/dts/artpec6.dtsi specifies bus-range = <0x00 0xff>).
>
> This patch resolves the problem.
> (I verified on linux-next: next-20180109).
>
> However, note that I had to patch the file
> drivers/pci/dwc/pcie-designware-host.c
> rather than
> drivers/pci/host/pcie-designware.c,
> which this patch suggests.
Above internally used example patch is based on kernel 4.9.74, where 
this file hasn't moved/renamed yet to dwc subfolder
I'll post a proper one shortly for the upstream tree.

Thanks for testing,

Koen
> Please feel free to submit a new patch with:
>
> Tested-by: Niklas Cassel <niklas.cassel@axis.com>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] imx6: fix pcie enumeration
@ 2018-01-09 13:58                     ` Koen Vandeputte
  0 siblings, 0 replies; 25+ messages in thread
From: Koen Vandeputte @ 2018-01-09 13:58 UTC (permalink / raw)
  To: linux-arm-kernel



On 2018-01-09 14:48, Niklas Cassel wrote:
> <snip>

>>  ???? /* setup bus numbers */
>>  ???? val = dw_pcie_readl_rc(pp, PCI_PRIMARY_BUS);
>>  ???? val &= 0xff000000;
>> -??? val |= 0x00010100;
>> +??? val |= 0x00ff0100;
>>  ???? dw_pcie_writel_rc(pp, PCI_PRIMARY_BUS, val);
>>
>>  ???? /* setup command register */
>>
>>
>> Above version logically fixes it for all dwc devices using a bridge after the RC, not only imx6.
>> If this is fine, I would submit the patch above and drop the current one.
> I can confirm that commit a20c7f36bd3d ("PCI: Do not allocate more buses than
> available in parent") broke enumerating PCIe devices behind a PCIe switch
> on ARTPEC-6 (which uses the DWC IP).
> (FTR, arch/arm/boot/dts/artpec6.dtsi specifies bus-range = <0x00 0xff>).
>
> This patch resolves the problem.
> (I verified on linux-next: next-20180109).
>
> However, note that I had to patch the file
> drivers/pci/dwc/pcie-designware-host.c
> rather than
> drivers/pci/host/pcie-designware.c,
> which this patch suggests.
Above internally used example patch is based on kernel 4.9.74, where 
this file hasn't moved/renamed yet to dwc subfolder
I'll post a proper one shortly for the upstream tree.

Thanks for testing,

Koen
> Please feel free to submit a new patch with:
>
> Tested-by: Niklas Cassel <niklas.cassel@axis.com>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2018-01-09 13:58 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-04 15:48 [PATCH] imx6: fix pcie enumeration Koen Vandeputte
2018-01-04 20:24 ` Bjorn Helgaas
2018-01-04 20:24   ` Bjorn Helgaas
2018-01-05  9:56   ` Koen Vandeputte
2018-01-05  9:56     ` Koen Vandeputte
2018-01-05 12:32     ` Lorenzo Pieralisi
2018-01-05 12:32       ` Lorenzo Pieralisi
2018-01-05 13:39       ` Koen Vandeputte
2018-01-05 13:39         ` Koen Vandeputte
2018-01-05 17:18         ` Lorenzo Pieralisi
2018-01-05 17:18           ` Lorenzo Pieralisi
2018-01-08  8:51           ` Koen Vandeputte
2018-01-08  8:51             ` Koen Vandeputte
2018-01-08 11:00             ` Lorenzo Pieralisi
2018-01-08 11:00               ` Lorenzo Pieralisi
2018-01-08 11:13               ` Koen Vandeputte
2018-01-08 11:13                 ` Koen Vandeputte
2018-01-08 15:46                 ` Lorenzo Pieralisi
2018-01-08 15:46                   ` Lorenzo Pieralisi
2018-01-09 13:48                 ` Niklas Cassel
2018-01-09 13:48                   ` Niklas Cassel
2018-01-09 13:58                   ` Koen Vandeputte
2018-01-09 13:58                     ` Koen Vandeputte
2018-01-05 13:46     ` Bjorn Helgaas
2018-01-05 13:46       ` Bjorn Helgaas

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.