linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Jingoo Han <jingoohan1@gmail.com>,
	Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Fawad Lateef <fawadlateef@gmail.com>,
	Yue Wang <yue.wang@Amlogic.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Andy Gross <agross@kernel.org>,
	Stanimir Varbanov <svarbanov@mm-sol.com>,
	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: *_pcie_establish_link() usage
Date: Wed, 28 Aug 2019 08:00:56 -0500	[thread overview]
Message-ID: <20190828130056.GA4550@google.com> (raw)
In-Reply-To: <20190801212529.GE151852@google.com>

On Thu, Aug 01, 2019 at 04:25:29PM -0500, Bjorn Helgaas wrote:
> Hi,
> 
> I got the following dmesg log from Fawad [1]:
> 
>   imx6q-pcie 1ffc000.pcie: host bridge /soc/pcie@1ffc000 ranges:
>   imx6q-pcie 1ffc000.pcie:    IO 0x01f80000..0x01f8ffff -> 0x00000000
>   imx6q-pcie 1ffc000.pcie:   MEM 0x01000000..0x01efffff -> 0x01000000
>   imx6q-pcie 1ffc000.pcie: Link up
>   imx6q-pcie 1ffc000.pcie: Link: Gen2 disabled
>   imx6q-pcie 1ffc000.pcie: Link up, Gen1
>   imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
>   pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
>   pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> 
> This is unrelated to the problem Fawad is working on, but the above
> looks wrong to me because it associates the "Link up" and link speed
> info with the host bridge (imx6q-pcie 1ffc000.pcie), not the Root Port
> (pci 0000:00:00.0).
> 
> I see that *_pcie_establish_link() is generally called from
> *_pcie_host_init(), typically via the struct
> dw_pcie_host_ops.host_init pointer, e.g.,
> 
>   dra7xx_pcie_probe
>     dra7xx_add_pcie_port(dra7xx)
>       struct dw_pcie *pci = dra7xx->pci
>       struct pcie_port *pp = &pci->pp
>       dw_pcie_host_init(struct pcie_port *pp)
> 	bridge = devm_pci_alloc_host_bridge
> 	devm_of_pci_get_host_bridge_resources
> 	pp->ops->host_init(pp)
> 	  dra7xx_pcie_host_init               # .host_init
> 	    dra7xx_pcie_establish_link        # <--- bring up link
> 	    dw_pcie_wait_for_link
> 	pci_scan_root_bus_bridge(bridge)      # <--- enumerate
> 	pp->root_bus = bridge->bus
> 	pci_bus_add_devices(pp->root_bus)
> 
> AFAICT, the *_pcie_establish_link() functions all operate on a single
> PCIe link, i.e., they are bringing up the link going downstream from a
> single Root Port.
> 
> It looks like this only allows a single Root Port per struct dw_pcie
> device.  Is that true?  *Should* that be true?
> 
> It looks like we bring up the link before enumerating.  In some cases,
> (meson_pcie_host_init(), qcom_pcie_host_init(),
> uniphier_pcie_host_init()) if the link doesn't come up, we return
> failure, which means dw_pcie_host_init() will not enumerate devices at
> all.
> 
> That seems wrong -- can't we have Root Complex Integrated Endpoints
> and even other Root Ports on that root bus?  Those should be
> accessible and possibly useful even if we can't bring up a link on
> *one* Root Port.
> 
> I would *expect* that we would enumerate all the devices on the root
> bus.  Then if we find one or more Root Ports, we might try to bring up
> the link for each one, and if successful, enumerate the downstream
> devices.
> 
> I'm confused.  Is there some restriction that means there can only be
> a single Root Port in this design, and no RCiEPs?  Even if there is,
> can we change the code so it enumerates the root bus first and brings
> up links as necessary so it matches the generic PCIe topology better?

Anyone?  I'll add this to a list of projects for interested people to
work on unless somebody objects.

> [1] https://lore.kernel.org/linux-pci/CAGgoGu7rot61LSgu2syOMq9Onx26_u3PEtS7pf_QNQRxOaifhg@mail.gmail.com/

  reply	other threads:[~2019-08-28 13:01 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-01 21:25 *_pcie_establish_link() usage Bjorn Helgaas
2019-08-28 13:00 ` Bjorn Helgaas [this message]
2019-08-28 20:42   ` Lorenzo Pieralisi

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=20190828130056.GA4550@google.com \
    --to=helgaas@kernel.org \
    --cc=agross@kernel.org \
    --cc=fawadlateef@gmail.com \
    --cc=gustavo.pimentel@synopsys.com \
    --cc=hayashi.kunihiko@socionext.com \
    --cc=jingoohan1@gmail.com \
    --cc=khilman@baylibre.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=svarbanov@mm-sol.com \
    --cc=yue.wang@Amlogic.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).