From: Bjorn Helgaas <bjorn.helgaas@gmail.com>
To: Ray Jui <ray.jui@broadcom.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>, Wei Liu <wei.liu@kernel.org>,
linux-pci@vger.kernel.org, rjui@broadcom.com,
Andrew Murray <andrew.murray@arm.com>,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Subject: Re: [PATCH] PCI: iproc: move quirks to driver
Date: Wed, 11 Dec 2019 18:02:21 -0600 [thread overview]
Message-ID: <CABhMZUU82iRD67yQhpUG3MUx3s9WaZ=tAXA=QriEEjUkNbu22w@mail.gmail.com> (raw)
In-Reply-To: <afbb14fb-e114-d6de-0bfe-d39be354842e@broadcom.com>
On Wed, Dec 11, 2019 at 5:40 PM Ray Jui <ray.jui@broadcom.com> wrote:
>
>
>
> On 12/11/19 2:34 PM, Bjorn Helgaas wrote:
> > On Wed, Dec 11, 2019 at 05:45:11PM +0000, Wei Liu wrote:
> >> The quirks were originally enclosed by ifdef. That made the quirks not
> >> to be applied when respective drivers were compiled as modules.
> >>
> >> Move the quirks to driver code to fix the issue.
> >>
> >> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> >
> > This straddles the core and native driver boundary, so I applied it to
> > pci/misc for v5.6. Thanks, I think this is a great solution! It's
> > always nice when we can encapsulate device-specific things in a
> > driver.
> >
>
> Opps! I was going to review and comment and you are quick, :)
>
> I was going to say, I think it's better to keep this quirk in
> "pcie-iproc.c" instead of "pcie-iproc-platform.c".
>
> The quirk is specific to certain PCIe devices under iProc (activated
> based on device ID), but should not be tied to a specific bus
> architecture (i.e., platform vs BCMA).
I'm happy to move it; that's no problem.
> >> ---
> >> drivers/pci/controller/pcie-iproc-platform.c | 24 ++++++++++++++++++
> >> drivers/pci/quirks.c | 26 --------------------
> >> 2 files changed, 24 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/drivers/pci/controller/pcie-iproc-platform.c b/drivers/pci/controller/pcie-iproc-platform.c
> >> index ff0a81a632a1..4e6f7cdd9a25 100644
> >> --- a/drivers/pci/controller/pcie-iproc-platform.c
> >> +++ b/drivers/pci/controller/pcie-iproc-platform.c
> >> @@ -19,6 +19,30 @@
> >> #include "../pci.h"
> >> #include "pcie-iproc.h"
> >>
> >> +static void quirk_paxc_bridge(struct pci_dev *pdev)
> >> +{
> >> + /*
> >> + * The PCI config space is shared with the PAXC root port and the first
> >> + * Ethernet device. So, we need to workaround this by telling the PCI
> >> + * code that the bridge is not an Ethernet device.
> >> + */
> >> + if (pdev->hdr_type == PCI_HEADER_TYPE_BRIDGE)
> >> + pdev->class = PCI_CLASS_BRIDGE_PCI << 8;
> >> +
> >> + /*
> >> + * MPSS is not being set properly (as it is currently 0). This is
> >> + * because that area of the PCI config space is hard coded to zero, and
> >> + * is not modifiable by firmware. Set this to 2 (e.g., 512 byte MPS)
> >> + * so that the MPS can be set to the real max value.
> >> + */
> >> + pdev->pcie_mpss = 2;
> >> +}
> >> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0x16cd, quirk_paxc_bridge);
> >> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0x16f0, quirk_paxc_bridge);
> >> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd750, quirk_paxc_bridge);
> >> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd802, quirk_paxc_bridge);
> >> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd804, quirk_paxc_bridge);
> >> +
> >> static const struct of_device_id iproc_pcie_of_match_table[] = {
> >> {
> >> .compatible = "brcm,iproc-pcie",
> >> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> >> index 4937a088d7d8..202837ed68c9 100644
> >> --- a/drivers/pci/quirks.c
> >> +++ b/drivers/pci/quirks.c
> >> @@ -2381,32 +2381,6 @@ DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_BROADCOM,
> >> PCI_DEVICE_ID_TIGON3_5719,
> >> quirk_brcm_5719_limit_mrrs);
> >>
> >> -#ifdef CONFIG_PCIE_IPROC_PLATFORM
> >> -static void quirk_paxc_bridge(struct pci_dev *pdev)
> >> -{
> >> - /*
> >> - * The PCI config space is shared with the PAXC root port and the first
> >> - * Ethernet device. So, we need to workaround this by telling the PCI
> >> - * code that the bridge is not an Ethernet device.
> >> - */
> >> - if (pdev->hdr_type == PCI_HEADER_TYPE_BRIDGE)
> >> - pdev->class = PCI_CLASS_BRIDGE_PCI << 8;
> >> -
> >> - /*
> >> - * MPSS is not being set properly (as it is currently 0). This is
> >> - * because that area of the PCI config space is hard coded to zero, and
> >> - * is not modifiable by firmware. Set this to 2 (e.g., 512 byte MPS)
> >> - * so that the MPS can be set to the real max value.
> >> - */
> >> - pdev->pcie_mpss = 2;
> >> -}
> >> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0x16cd, quirk_paxc_bridge);
> >> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0x16f0, quirk_paxc_bridge);
> >> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd750, quirk_paxc_bridge);
> >> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd802, quirk_paxc_bridge);
> >> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd804, quirk_paxc_bridge);
> >> -#endif
> >> -
> >> /*
> >> * Originally in EDAC sources for i82875P: Intel tells BIOS developers to
> >> * hide device 6 which configures the overflow device access containing the
> >> --
> >> 2.20.1
> >>
next prev parent reply other threads:[~2019-12-12 0:02 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-11 17:45 [PATCH] PCI: iproc: move quirks to driver Wei Liu
2019-12-11 22:34 ` Bjorn Helgaas
2019-12-11 23:40 ` Ray Jui
2019-12-12 0:02 ` Bjorn Helgaas [this message]
2019-12-12 0:05 ` Ray Jui
2019-12-12 10:33 ` Wei Liu
2019-12-12 23:23 ` Bjorn Helgaas
2019-12-13 0:08 ` Ray Jui
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='CABhMZUU82iRD67yQhpUG3MUx3s9WaZ=tAXA=QriEEjUkNbu22w@mail.gmail.com' \
--to=bjorn.helgaas@gmail.com \
--cc=andrew.murray@arm.com \
--cc=bjorn@helgaas.com \
--cc=helgaas@kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=ray.jui@broadcom.com \
--cc=rjui@broadcom.com \
--cc=wei.liu@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).