From: Bjorn Helgaas <helgaas@kernel.org>
To: Krzysztof Wilczynski <kw@linux.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
Dominik Brodowski <linux@dominikbrodowski.net>,
Chuhong Yuan <hslester96@gmail.com>,
Thomas Gleixner <tglx@linutronix.de>,
Takashi Iwai <tiwai@suse.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-pci@vger.kernel.org
Subject: Re: [PATCH v2 1/3] PCI: Correct the PCI quirk for the ALI M7101 chipset
Date: Wed, 20 May 2020 12:06:09 -0500 [thread overview]
Message-ID: <20200520170609.GA1102503@bjorn-Precision-5520> (raw)
In-Reply-To: <20200520103147.985326-2-kw@linux.com>
On Wed, May 20, 2020 at 10:31:45AM +0000, Krzysztof Wilczynski wrote:
> As per the specifications from the vendor:
>
> - "M1543 Preliminary Data Sheet", "M1543: Desktop South Bridge",
> Acer Laboratories Inc., January 1998, Version 1.25, p. 78
>
> - "M1543C Preliminary Datasheet", "M1543C Desktop Southbridge",
> Acer Laboratories Inc., September 1998, Version 1.10, p. 126
>
> Both the ACPI I/O and SMB I/O registers should be mapped into I/O space,
> but the second quirk used to claimed an I/O resource from the memory
> window which is not correct.
>
> Signed-off-by: Krzysztof Wilczynski <kw@linux.com>
> ---
> drivers/pci/quirks.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index ca9ed5774eb1..c71fdd8bd6f8 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -654,7 +654,7 @@ DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID,
> static void quirk_ali7101_acpi(struct pci_dev *dev)
> {
> quirk_io_region(dev, 0xE0, 64, PCI_BRIDGE_RESOURCES, "ali7101 ACPI");
> - quirk_io_region(dev, 0xE2, 32, PCI_BRIDGE_RESOURCES+1, "ali7101 SMB");
> + quirk_io_region(dev, 0xE2, 32, PCI_BRIDGE_RESOURCES, "ali7101 SMB");
Wow, this is way too subtle. I proposed this fix, but I was wrong.
I thought PCI_BRIDGE_RESOURCES was identifying a bridge window that
the region should be claimed from: dev->resource[PCI_BRIDGE_RESOURCES]
would be the I/O port window of a bridge.
But that's not what's happening at all. quirk_io_region() is using
that index to fill in a dev->resource[] entry with an IORESOURCE_IO
resource, and then it calls pci_claim_resource(), which looks for an
upstream bridge window that contains the resource (in
pci_find_parent_resource()).
So the use of PCI_BRIDGE_RESOURCES here is just a convenient way to
find an empty slot in dev->resource[]. We know it's empty because
the ali7101 device is not a bridge, so it has no windows.
And PCI_BRIDGE_RESOURCES+1 *looks* like a reference to a bridge's
memory window, but it's not; it's just a way to find another empty
dev->resource[] slot.
Sigh. This is ugly and misleading. But in the absence of good ideas,
I guess we should just leave it alone.
> }
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AL, PCI_DEVICE_ID_AL_M7101, quirk_ali7101_acpi);
>
> --
> 2.26.2
>
next prev parent reply other threads:[~2020-05-20 17:06 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-19 21:49 [PATCH] PCI: Reference bridge window resources explicitly Krzysztof Wilczynski
2020-05-20 10:31 ` [PATCH v2 0/3] " Krzysztof Wilczynski
2020-05-20 10:31 ` [PATCH v2 1/3] PCI: Correct the PCI quirk for the ALI M7101 chipset Krzysztof Wilczynski
2020-05-20 17:06 ` Bjorn Helgaas [this message]
2020-05-20 18:34 ` [PATCH v3 0/2] PCI: Reference bridge window resources explicitly Krzysztof Wilczynski
2020-05-20 18:34 ` [PATCH v3 1/2] PCI: Move from using PCI_BRIDGE_RESOURCES to bridge resource definitions Krzysztof Wilczynski
2020-05-20 18:34 ` [PATCH v3 2/2] pcmcia: Use resources definitions when freeing CardBus resources Krzysztof Wilczynski
2020-05-20 20:30 ` [PATCH v3 0/2] PCI: Reference bridge window resources explicitly Bjorn Helgaas
2020-05-21 8:16 ` Krzysztof Wilczynski
2020-05-21 8:24 ` Dominik Brodowski
2020-05-21 11:20 ` [PATCH v4 " Krzysztof Wilczynski
2020-05-21 11:20 ` [PATCH v4 1/2] PCI: Move from using PCI_BRIDGE_RESOURCES to bridge resource definitions Krzysztof Wilczynski
2020-05-21 11:20 ` [PATCH v4 2/2] pcmcia: Use resources definitions when freeing CardBus resources Krzysztof Wilczynski
2020-05-20 10:31 ` [PATCH v2 2/3] PCI: Move from using PCI_BRIDGE_RESOURCES to bridge resource definitions Krzysztof Wilczynski
2020-05-20 10:31 ` [PATCH v2 3/3] pcmcia: Use resources definitions when freeing CardBus resources Krzysztof Wilczynski
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=20200520170609.GA1102503@bjorn-Precision-5520 \
--to=helgaas@kernel.org \
--cc=bhelgaas@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=hslester96@gmail.com \
--cc=kw@linux.com \
--cc=linux-pci@vger.kernel.org \
--cc=linux@dominikbrodowski.net \
--cc=tglx@linutronix.de \
--cc=tiwai@suse.de \
/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