[PATCH] PCI: dwc: fix enumeration end when reaching root subordinate

[PATCH] PCI: dwc: fix enumeration end when reaching root subordinate

[Koen Vandeputte]

The subordinate value indicates the highest bus number which can be
reached downstream though a certain device.

Commit a20c7f36bd3d ("PCI: Do not allocate more buses than available in
parent")
ensures that downstream devices cannot assign busnumbers higher than the
upstream device subordinate number, which was indeed illogical.

By default, dw_pcie_setup_rc() inits the Root Complex subordinate to a
value of 0x01.

Due to this combined with above commit, enumeration stops digging deeper
downstream as soon as bus num 0x01 has been assigned, which is always
the case for a bridge device.

This results in all devices behind a bridge bus to remain undetected, as
these would be connected to bus 0x02 or higher.

Fix this by initializing the RC to a subordinate value of 0xff, meaning
that all busses [0x00-0xff] are reachable through this RC.

Fixes: a20c7f36bd3d ("PCI: Do not allocate more buses than available in
parent")
Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
Tested-by: Niklas Cassel <niklas.cassel@axis.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
---

Will send separate patches to stable as this file got moved/renamed


 drivers/pci/dwc/pcie-designware-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index bf558df5b7b3..2b5470173196 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -616,7 +616,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 	/* setup bus numbers */
 	val = dw_pcie_readl_dbi(pci, PCI_PRIMARY_BUS);
 	val &= 0xff000000;
-	val |= 0x00010100;
+	val |= 0x00ff0100;
 	dw_pcie_writel_dbi(pci, PCI_PRIMARY_BUS, val);
 
 	/* setup command register */
-- 
2.7.4

Re: [PATCH] PCI: dwc: fix enumeration end when reaching root subordinate

[Lorenzo Pieralisi]

On Tue, Jan 09, 2018 at 03:42:21PM +0100, Koen Vandeputte wrote:
> The subordinate value indicates the highest bus number which can be
> reached downstream though a certain device.
> 
> Commit a20c7f36bd3d ("PCI: Do not allocate more buses than available in
> parent")
> ensures that downstream devices cannot assign busnumbers higher than the
> upstream device subordinate number, which was indeed illogical.
> 
> By default, dw_pcie_setup_rc() inits the Root Complex subordinate to a
> value of 0x01.
> 
> Due to this combined with above commit, enumeration stops digging deeper
> downstream as soon as bus num 0x01 has been assigned, which is always
> the case for a bridge device.
> 
> This results in all devices behind a bridge bus to remain undetected, as
> these would be connected to bus 0x02 or higher.
> 
> Fix this by initializing the RC to a subordinate value of 0xff, meaning
> that all busses [0x00-0xff] are reachable through this RC.

This is not a correct description of the problem. AFAICS all busses
are reachable through this RC _regardless_ of whatever subordinate
bus number value you programme into it.

You should extend the CC list to all dwc host submaintainers so
that you can actually get it tested.

> Fixes: a20c7f36bd3d ("PCI: Do not allocate more buses than available in
> parent")
> Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
> Tested-by: Niklas Cassel <niklas.cassel@axis.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> 
> Will send separate patches to stable as this file got moved/renamed

Fixes: commit appeared at v4.15-rc1 (and v4.15 has not been released
yet) - there is no separate patch to be sent.

Thanks,
Lorenzo

>  drivers/pci/dwc/pcie-designware-host.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
> index bf558df5b7b3..2b5470173196 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -616,7 +616,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>  	/* setup bus numbers */
>  	val = dw_pcie_readl_dbi(pci, PCI_PRIMARY_BUS);
>  	val &= 0xff000000;
> -	val |= 0x00010100;
> +	val |= 0x00ff0100;
>  	dw_pcie_writel_dbi(pci, PCI_PRIMARY_BUS, val);
>  
>  	/* setup command register */
> -- 
> 2.7.4
> 

Re: [PATCH] PCI: dwc: fix enumeration end when reaching root subordinate

[Mika Westerberg]

On Tue, Jan 09, 2018 at 03:25:58PM +0000, Lorenzo Pieralisi wrote:
> On Tue, Jan 09, 2018 at 03:42:21PM +0100, Koen Vandeputte wrote:
> > The subordinate value indicates the highest bus number which can be
> > reached downstream though a certain device.
> > 
> > Commit a20c7f36bd3d ("PCI: Do not allocate more buses than available in
> > parent")
> > ensures that downstream devices cannot assign busnumbers higher than the
> > upstream device subordinate number, which was indeed illogical.
> > 
> > By default, dw_pcie_setup_rc() inits the Root Complex subordinate to a
> > value of 0x01.
> > 
> > Due to this combined with above commit, enumeration stops digging deeper
> > downstream as soon as bus num 0x01 has been assigned, which is always
> > the case for a bridge device.
> > 
> > This results in all devices behind a bridge bus to remain undetected, as
> > these would be connected to bus 0x02 or higher.
> > 
> > Fix this by initializing the RC to a subordinate value of 0xff, meaning
> > that all busses [0x00-0xff] are reachable through this RC.
> 
> This is not a correct description of the problem. AFAICS all busses
> are reachable through this RC _regardless_ of whatever subordinate
> bus number value you programme into it.

Type 1 (the ones directed to the other side of the bridge) configuration
transactions are not forwarded if the bus number in the transaction is
not included in secondary and subordinate numbers of the root bridge. I
think the description here is pretty accurate as the bridges (anything
higher than bus number 1) below are effectively hidden.

Re: [PATCH] PCI: dwc: fix enumeration end when reaching root subordinate

[Lorenzo Pieralisi]

On Tue, Jan 09, 2018 at 10:00:25PM +0200, Mika Westerberg wrote:
> On Tue, Jan 09, 2018 at 03:25:58PM +0000, Lorenzo Pieralisi wrote:
> > On Tue, Jan 09, 2018 at 03:42:21PM +0100, Koen Vandeputte wrote:
> > > The subordinate value indicates the highest bus number which can be
> > > reached downstream though a certain device.
> > > 
> > > Commit a20c7f36bd3d ("PCI: Do not allocate more buses than available in
> > > parent")
> > > ensures that downstream devices cannot assign busnumbers higher than the
> > > upstream device subordinate number, which was indeed illogical.
> > > 
> > > By default, dw_pcie_setup_rc() inits the Root Complex subordinate to a
> > > value of 0x01.
> > > 
> > > Due to this combined with above commit, enumeration stops digging deeper
> > > downstream as soon as bus num 0x01 has been assigned, which is always
> > > the case for a bridge device.
> > > 
> > > This results in all devices behind a bridge bus to remain undetected, as
> > > these would be connected to bus 0x02 or higher.
> > > 
> > > Fix this by initializing the RC to a subordinate value of 0xff, meaning
> > > that all busses [0x00-0xff] are reachable through this RC.
> > 
> > This is not a correct description of the problem. AFAICS all busses
> > are reachable through this RC _regardless_ of whatever subordinate
> > bus number value you programme into it.
> 
> Type 1 (the ones directed to the other side of the bridge) configuration
> transactions are not forwarded if the bus number in the transaction is
> not included in secondary and subordinate numbers of the root bridge. I
> think the description here is pretty accurate as the bridges (anything
> higher than bus number 1) below are effectively hidden.

That's correct - but that's not what this patch is fixing which is
what the commit log should describe.

See:

https://marc.info/?l=linux-arm-kernel&m=151540153522730&w=2

Re: [PATCH] PCI: dwc: fix enumeration end when reaching root subordinate

[Koen Vandeputte]

Hi Lorenzo,

Apologies for the late reply, It's been really busy over here.


On 2018-01-09 16:25, Lorenzo Pieralisi wrote:
> <snip>
>> Fix this by initializing the RC to a subordinate value of 0xff, meaning
>> that all busses [0x00-0xff] are reachable through this RC.
> This is not a correct description of the problem. AFAICS all busses
> are reachable through this RC _regardless_ of whatever subordinate
> bus number value you programme into it.
Noted.

This was written meaning: "as seen be the probing functions below"
I'll try harder in V2 to actually include the message that it's not a HW 
related problem or influencing HW in any way, but really "tricking" the 
probing functions below
>
> You should extend the CC list to all dwc host submaintainers so
> that you can actually get it tested.
How to figure out who to include? (& please provide who to include)
Reading online manuals on "how to send patches" only demo's 
"get_maintainer.pl" script

>> <snip>
>> ---
>>
>> Will send separate patches to stable as this file got moved/renamed
> Fixes: commit appeared at v4.15-rc1 (and v4.15 has not been released
> yet) - there is no separate patch to be sent.
This is something typical which is hard to learn/understand by just 
reading "how to send patches" docs available everywhere. (like I did 
before sending this one over ;-) )

2 questions basically:
- As the commit causing it was included in 4.15-*RC1*,  do I need to add 
a "Fixes; bla bla" at all?
- As the commit causing it was backported to 4.9, (how) should I send a 
separate patch in order to get it fixed there? [0]

> Thanks,
> Lorenzo

Probably annoying questions triggering a 'sigh' .. but I'm lacking 
experience here ..

Thanks for your time & patience so far,
Highly appreciated,

Koen


[0] 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?h=v4.9.76&id=9a4bf05126f42c2632729ab0da503021d74ed454

Re: [PATCH] PCI: dwc: fix enumeration end when reaching root subordinate

[Lorenzo Pieralisi]

On Fri, Jan 12, 2018 at 04:56:33PM +0100, Koen Vandeputte wrote:
> Hi Lorenzo,
> 
> Apologies for the late reply, It's been really busy over here.
> 
> 
> On 2018-01-09 16:25, Lorenzo Pieralisi wrote:
> ><snip>
> >>Fix this by initializing the RC to a subordinate value of 0xff, meaning
> >>that all busses [0x00-0xff] are reachable through this RC.
> >This is not a correct description of the problem. AFAICS all busses
> >are reachable through this RC _regardless_ of whatever subordinate
> >bus number value you programme into it.
> Noted.
> 
> This was written meaning: "as seen be the probing functions below"
> I'll try harder in V2 to actually include the message that it's not
> a HW related problem or influencing HW in any way, but really
> "tricking" the probing functions below

Ok, add it to the log please.

> >You should extend the CC list to all dwc host submaintainers so
> >that you can actually get it tested.
> How to figure out who to include? (& please provide who to include)
> Reading online manuals on "how to send patches" only demo's
> "get_maintainer.pl" script

Well, you can do it manually, by checking all pci/dwc submaintainers
in MAINTAINERS.

There are other ways of course but it is simple enough.

> >><snip>
> >>---
> >>
> >>Will send separate patches to stable as this file got moved/renamed
> >Fixes: commit appeared at v4.15-rc1 (and v4.15 has not been released
> >yet) - there is no separate patch to be sent.
> This is something typical which is hard to learn/understand by just
> reading "how to send patches" docs available everywhere. (like I did
> before sending this one over ;-) )
> 
> 2 questions basically:
> - As the commit causing it was included in 4.15-*RC1*,  do I need to
> add a "Fixes; bla bla" at all?

Yes.

> - As the commit causing it was backported to 4.9, (how) should I
> send a separate patch in order to get it fixed there? [0]

So submitting a stable tag does make sense, sorry I missed that.

Or we can send to stable kernel specific backports.

> Probably annoying questions triggering a 'sigh' .. but I'm lacking
> experience here ..

No way, thank you for fixing it !

> Thanks for your time & patience so far,
> Highly appreciated,

Same here, thank you.

Lorenzo