All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Koen Vandeputte <koen.vandeputte@ncentric.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	linux-pci@vger.kernel.org, bhelgaas@google.com,
	Richard Zhu <hongxing.zhu@nxp.com>,
	Lucas Stach <l.stach@pengutronix.de>,
	linux-arm-kernel@lists.infradead.org,
	Joao Pinto <Joao.Pinto@synopsys.com>,
	Jingoo Han <jingoohan1@gmail.com>
Subject: Re: [PATCH] imx6: fix pcie enumeration
Date: Mon, 8 Jan 2018 15:46:48 +0000	[thread overview]
Message-ID: <20180108154648.GA2530@red-moon> (raw)
In-Reply-To: <1684b8c6-1006-948b-f4f9-c9aaf9cf26a8@ncentric.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] imx6: fix pcie enumeration
Date: Mon, 8 Jan 2018 15:46:48 +0000	[thread overview]
Message-ID: <20180108154648.GA2530@red-moon> (raw)
In-Reply-To: <1684b8c6-1006-948b-f4f9-c9aaf9cf26a8@ncentric.com>

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

  reply	other threads:[~2018-01-08 15:46 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180108154648.GA2530@red-moon \
    --to=lorenzo.pieralisi@arm.com \
    --cc=Joao.Pinto@synopsys.com \
    --cc=bhelgaas@google.com \
    --cc=helgaas@kernel.org \
    --cc=hongxing.zhu@nxp.com \
    --cc=jingoohan1@gmail.com \
    --cc=koen.vandeputte@ncentric.com \
    --cc=l.stach@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.