From: Bjorn Helgaas <bhelgaas@google.com>
To: Yinghai Lu <yinghai@kernel.org>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] PCI: Apply _HPX link control for all supported pcie device
Date: Mon, 10 Nov 2014 21:23:24 -0700 [thread overview]
Message-ID: <20141111042324.GF28161@google.com> (raw)
In-Reply-To: <1415325580-5974-1-git-send-email-yinghai@kernel.org>
On Thu, Nov 06, 2014 at 05:59:40PM -0800, Yinghai Lu wrote:
> With commit 6cd33649fa83 ("PCI: Add pci_configure_device() during enumeration")
> and following commits, we apply _HPX for pci devices.
>
> that is called via pci_scan_slot
> ==>pci_scan_single_device
> ==>pci_device_add
> ==>pci_configure_device
> ==>program_hpp_type2
> and dev->subordinate is not allocated yet.
>
> So that link control setup get skipped.
>
> Acoording to ACPI spec, there is no requirement that need to be set to pcie
> root port or downstream port.
>
> We could just change to pcie_cap_has_lnkctl to cover all pcie devices with
> link control cap.
>
> Fixes: 6cd33649fa83 ("PCI: Add pci_configure_device() during enumeration")
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>
> ---
> drivers/pci/access.c | 13 -------------
> drivers/pci/pci.h | 13 +++++++++++++
> drivers/pci/probe.c | 2 +-
> 3 files changed, 14 insertions(+), 14 deletions(-)
>
> Index: linux-2.6/drivers/pci/probe.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/probe.c
> +++ linux-2.6/drivers/pci/probe.c
> @@ -1323,7 +1323,7 @@ static void program_hpp_type2(struct pci
> ~hpp->pci_exp_devctl_and, hpp->pci_exp_devctl_or);
>
> /* Initialize Link Control Register */
> - if (dev->subordinate)
> + if (pcie_cap_has_lnkctl(dev))
> pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL,
> ~hpp->pci_exp_lnkctl_and, hpp->pci_exp_lnkctl_or);
Nice catch! It's much cleaner to do this based on pcie_cap_has_lnkctl().
> Index: linux-2.6/drivers/pci/access.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/access.c
> +++ linux-2.6/drivers/pci/access.c
> @@ -444,19 +444,6 @@ static inline int pcie_cap_version(const
> return pcie_caps_reg(dev) & PCI_EXP_FLAGS_VERS;
> }
>
> -static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)
> -{
> - int type = pci_pcie_type(dev);
> -
> - return type == PCI_EXP_TYPE_ENDPOINT ||
> - type == PCI_EXP_TYPE_LEG_END ||
> - type == PCI_EXP_TYPE_ROOT_PORT ||
> - type == PCI_EXP_TYPE_UPSTREAM ||
> - type == PCI_EXP_TYPE_DOWNSTREAM ||
> - type == PCI_EXP_TYPE_PCI_BRIDGE ||
> - type == PCI_EXP_TYPE_PCIE_BRIDGE;
> -}
> -
> static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)
> {
> int type = pci_pcie_type(dev);
> Index: linux-2.6/drivers/pci/pci.h
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci.h
> +++ linux-2.6/drivers/pci/pci.h
> @@ -6,6 +6,19 @@
>
> extern const unsigned char pcie_link_speed[];
>
> +static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)
> +{
> + int type = pci_pcie_type(dev);
> +
> + return type == PCI_EXP_TYPE_ENDPOINT ||
> + type == PCI_EXP_TYPE_LEG_END ||
> + type == PCI_EXP_TYPE_ROOT_PORT ||
> + type == PCI_EXP_TYPE_UPSTREAM ||
> + type == PCI_EXP_TYPE_DOWNSTREAM ||
> + type == PCI_EXP_TYPE_PCI_BRIDGE ||
> + type == PCI_EXP_TYPE_PCIE_BRIDGE;
> +}
But I don't want to move pcie_cap_has_lnkctl() to pci.h and leave
pcie_cap_has_sltctl() and pcie_cap_has_rtctl() in access.c. They're all
related, so I think they should stay together.
What if we left them in access.c, made them non-static, non-inline, and
just added prototypes to pci.h? Or I guess we could add a prototype just
for pcie_cap_has_lnkctl(), since that's the only one we need so far.
Bjorn
> +
> /* Functions internal to the PCI core code */
>
> int pci_create_sysfs_dev_files(struct pci_dev *pdev);
next prev parent reply other threads:[~2014-11-11 4:23 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-07 1:59 [PATCH] PCI: Apply _HPX link control for all supported pcie device Yinghai Lu
2014-11-11 4:23 ` Bjorn Helgaas [this message]
2014-11-11 18:50 ` Yinghai Lu
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=20141111042324.GF28161@google.com \
--to=bhelgaas@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=yinghai@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 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).