On Thu, Jan 24, 2019 at 03:44:26PM +0200, Mika Westerberg wrote:
> On Thu, Jan 24, 2019 at 02:25:42PM +0100, Lukas Wunner wrote:
> > On Thu, Jan 24, 2019 at 04:12:27PM +0300, Mika Westerberg wrote:
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -2501,6 +2501,23 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev)
> > > pm_runtime_put_sync(parent);
> > > }
> > >
> > > +static const struct dmi_system_id bridge_d3_blacklist[] = {
> > > + {
> > > + /*
> > > + * Gigabyte X299 root port is not marked as hotplug
> > > + * capable which allows Linux to power manage it.
> > > + * However, this confuses the BIOS SMI handler so don't
> > > + * power manage root ports on that system.
> > > + */
> > > + .ident = "X299 DESIGNARE EX-CF",
> > > + .matches = {
> > > + DMI_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."),
> > > + DMI_MATCH(DMI_BOARD_NAME, "X299 DESIGNARE EX-CF"),
> > > + },
> > > + },
> > > + { }
> > > +};
> >
> > Unfortunately this doesn't take my comment voiced Jan 8 into account:
> >
> > "Please constrain either the blacklist entry for the Gigabyte mainboard
> > or the blacklist check itself to x86 using IS_ENABLED() or #ifdef to
> > avoid code bloat on other arches."
> > https://patchwork.kernel.org/patch/10750549/#22408911
>
> I tried to take it into account based on what you said:
>
> Please constrain either the blacklist entry for the Gigabyte mainboard
>
> so I constrained it to Gigabyte motherboard using DMI entry. Maybe I
> misunderstood you. Sorry about that.
What I meant is that you could encapsulate the struct dmi_system_id for
the Gigabyte mainboard in "#ifdef CONFIG_X86" / "#endif".
> > Note that CONFIG_DMI may be enabled on arm64 and ia64 which have no use
> > for this quirk.
The rationale for this sentence was that the compiler might be smart
enough to optimize bridge_d3_blacklist[] away if CONFIG_DMI is not
defined, but it might be defined on arm64 and ia64 so on those arches
you'd still be including an unnecessary DMI entry.
Granted those few bytes won't hurt, but people might come after you and
amend the blacklist, if the #ifdef is already in place they'll know to
add their entry within the #ifdef or add a new #ifdef if the quirk isn't
for x86, thus neatly keeping generic code free of arch-specific quirks.
Thanks,
Lukas
