Re: [PATCH v3] PCI: Blacklist power management of Gigabyte X299 DESIGNARE EX PCIe ports

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> Note that CONFIG_DMI may be enabled on arm64 and ia64 which have no use
> for this quirk.  It's generally undesirable to have arch-specific quirks
> in generic code and folks running Linux on memory-constrained mips routers
> have complained before that quirks.c contains too much code unrelated to
> their arch.

I understand that but you end up with code looking as ugly as this:

  if (IS_ENABLED(CONFIG_X86) && dmi_check_system(bridge_d3_blacklist))
  	return false;

just to save a function call and and couple of bytes from kernel data. I
would rather prefer readability in this case. But no problem doing the
above if that's the way Bjorn likes to have it.



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux