On Mon, Jul 03, 2017 at 02:33:39PM +0800, Ethan Zhao wrote:
> On Mon, Jul 3, 2017 at 12:26 PM, Chen Yu <yu.c.chen@xxxxxxxxx> wrote:
> > On Sun, Jul 02, 2017 at 03:59:48PM -0500, Bjorn Helgaas wrote:
> >> --- a/arch/x86/pci/fixup.c
> >> +++ b/arch/x86/pci/fixup.c
[snip]
> >> +static void quirk_apple_mbp_poweroff(struct pci_dev *pdev)
> >> +{
> >> + struct device *dev = &pdev->dev;
> >> + struct resource *res;
> >> +
> >> + if ((!dmi_match(DMI_PRODUCT_NAME, "MacBookPro11,4") &&
> >> + !dmi_match(DMI_PRODUCT_NAME, "MacBookPro11,5")) ||
> >> + pdev->bus->number != 0 || pdev->devfn != PCI_DEVFN(0x1c, 0))
> >> + return;
> >> +
> > Not sure why we need to also the bus number of devfn, as there
> > is only one PCI bridge that would match 0x8c10? according to
> > https://bugzilla.kernel.org/attachment.cgi?id=210611
> > am I missing something?
>
> To make sure it runs only once only when 00:1c.0 is 0x8c10 ?
Each root port has a different PCI device ID and is only present once
on the affected machines, so I think Chen Yu is right.
Actually, since the quirk is now related to a memory region and no longer
to a specific PCI device, it need not be a PCI fixup. It could go into
arch/x86/kernel/setup.c:trim_platform_memory_ranges() as a DMI quirk.
This would also allow declaration of the code as __init.
Thanks,
Lukas
> >> + res = request_mem_region(0x7fa00000, 0x200000,
> >> + "MacBook Pro poweroff workaround");
> >> + if (res)
> >> + dev_info(dev, "claimed %s %pR\n", res->name, res);
> >> + else
> >> + dev_info(dev, "can't work around MacBook Pro poweroff issue\n");
> >> +}
> >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x8c10, quirk_apple_mbp_poweroff);
