LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Jeremy Linton <jeremy.linton@arm.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	PCI <linux-pci@vger.kernel.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Nicolas Saenz Julienne <nsaenz@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>, Krzysztof Wilczynski <kw@linux.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" 
	<bcm-kernel-feedback-list@broadcom.com>,
	"open list:ACPI FOR ARM64 (ACPI/arm64)"
	<linux-acpi@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	"moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" 
	<linux-rpi-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
Date: Mon, 9 Aug 2021 09:27:01 -0600	[thread overview]
Message-ID: <CAL_JsqL=ipGBOsMUaCDvAETX5DQ2tCunNSAtFs6VZybOW7Xrdg@mail.gmail.com> (raw)
In-Reply-To: <5f4f484b-9eef-2722-405d-a7ff6259aa0f@arm.com>

On Fri, Aug 6, 2021 at 6:35 PM Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Hi,
>
> Thanks for looking at this.
>
> On 8/6/21 5:12 PM, Bjorn Helgaas wrote:
> > In subject, this or similar would match history:
> >
> >    PCI/ACPI: Add Broadcom bcm2711 MCFG quirk
> >
> > On Thu, Aug 05, 2021 at 04:12:00PM -0500, Jeremy Linton wrote:
> >> Now that we have a bcm2711 quirk, we need to be able to
> >> detect it when the MCFG is missing. Use a namespace
> >> property as an alternative to the MCFG OEM.
> >
> > Rewrap to use ~75 columns.
> >
> > Mention the DT namespace property here.
> >
> >> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> >> ---
> >>   drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
> >>   1 file changed, 14 insertions(+)
> >>
> >> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> >> index 53cab975f612..7d77fc72c2a4 100644
> >> --- a/drivers/acpi/pci_mcfg.c
> >> +++ b/drivers/acpi/pci_mcfg.c
> >> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
> >>      ALTRA_ECAM_QUIRK(1, 13),
> >>      ALTRA_ECAM_QUIRK(1, 14),
> >>      ALTRA_ECAM_QUIRK(1, 15),
> >> +
> >> +    { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
> >> +      DEFINE_RES_MEM(0xFD500000, 0xA000) },
> >>   };
> >>
> >>   static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> >> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
> >>      u16 segment = root->segment;
> >>      struct resource *bus_range = &root->secondary;
> >>      struct mcfg_fixup *f;
> >> +    const char *soc;
> >>      int i;
> >>
> >> +    /*
> >> +     * This could be a machine with a PCI/SMC conduit,
> >> +     * which means it doens't have MCFG. Get the machineid from
> >> +     * the namespace definition instead.
> >
> > s/SMC/SMCCC/ ?  Cover letter uses SMCCC (not sure it's relevant anyway)
> > s/doens't/doesn't/
> >
> > Rewrap comment to use ~80 columns.
> >
> > Seems pretty reasonable that a platform without standard ECAM might
> > not have MCFG, since MCFG basically implies ECAM.
>
>
> Sure, on all the above comments.
>
> >
> > Is "linux,pcie-quirk" the right property to look for?  It doesn't
> > sound very generic, and it doesn't sound like anything related to
> > ECAM.  Is it new?  I don't see it in the tree yet.  Should it be in
> > Documentation/devicetree/bindings/pci/pci.txt so we don't get a
> > different property name for every new platform?

No, it should not be in pci.txt. Nothing to do with DT and I don't
review ACPI bindings (someone should) if I can help it.

> Yes, I made it up. Someone else commented about the "linux," partially
> because it should be "linux-" to conform with
> https://github.com/UEFI/DSD-Guide. But also in the same context of it
> being linux specific.  I think that guide is where it should end up,
> rather than the devicetree bindings.
>
> I guess we can request addition to the uefi- but that seems like a
> mistake this is really (hopefully?) a Linux specific properly as other
> OS's will simply use the SMC. I think we could request another prefix if
> we come up with a good one and think it belongs in that guide.

It's only Linux specific until it's not.

What happens when there's a second PCIe quirk here? Say what the quirk
is (and not in terms of Linux policy).

Don't you know the device here and can imply all this (like implying
off of 'compatible' in DT)? Adding properties to address issues
creates compatibility issues. Maybe not an issue in this case if the
firmware is not stable, but not a good practice to be in.

Rob

  reply	other threads:[~2021-08-09 15:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-05 21:11 [PATCH 0/3] CM4 ACPI PCIe quirk Jeremy Linton
2021-08-05 21:11 ` [PATCH 1/3] PCI: brcmstb: Break register definitions into separate header Jeremy Linton
2021-08-10 10:07   ` Florian Fainelli
2021-08-10 15:10     ` Jeremy Linton
2021-08-11  8:39       ` Florian Fainelli
2021-08-05 21:11 ` [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk Jeremy Linton
2021-08-06 22:21   ` Bjorn Helgaas
2021-08-07  2:55     ` Jeremy Linton
2021-08-09 17:42       ` Bjorn Helgaas
2021-08-09 19:48         ` Jeremy Linton
2021-08-09 20:33           ` Bjorn Helgaas
2021-08-09 21:21             ` Jeremy Linton
2021-08-05 21:12 ` [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711 Jeremy Linton
2021-08-06 22:12   ` Bjorn Helgaas
2021-08-07  0:34     ` Jeremy Linton
2021-08-09 15:27       ` Rob Herring [this message]
2021-08-09 16:24         ` Jeremy Linton
2021-08-10 14:31   ` Shanker R Donthineni
2021-08-10 14:47     ` Jeremy Linton
2021-08-10 15:09       ` Shanker R Donthineni
2021-08-06 11:40 ` [PATCH 0/3] CM4 ACPI PCIe quirk Stefan Wahren

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='CAL_JsqL=ipGBOsMUaCDvAETX5DQ2tCunNSAtFs6VZybOW7Xrdg@mail.gmail.com' \
    --to=robh@kernel.org \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=bhelgaas@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=helgaas@kernel.org \
    --cc=jeremy.linton@arm.com \
    --cc=kw@linux.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=nsaenz@kernel.org \
    --cc=rjw@rjwysocki.net \
    --subject='Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711' \
    /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

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).