Re: [PATCH 2/2] PCI: pciehp: Convert pciehp to be builtin only, not modular

[Bjorn Helgaas <bhelgaas@google.com>]

[updated Rafael's email addr; not sure if sisk.pl still works or not]

On Wed, May 27, 2015 at 11:31:21AM -0700, Yinghai Lu wrote:
> On Fri, Jul 26, 2013 at 5:43 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Thu, Jul 25, 2013 at 10:57 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> >> Convert pciehp to be builtin only, with no module option.
> >>
> >> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> >> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >> ---
> >>  drivers/pci/pcie/Kconfig |    5 +----
> >>  1 file changed, 1 insertion(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
> >> index 569f82f..3b94cfc 100644
> >> --- a/drivers/pci/pcie/Kconfig
> >> +++ b/drivers/pci/pcie/Kconfig
> >> @@ -14,15 +14,12 @@ config PCIEPORTBUS
> >>  # Include service Kconfig here
> >>  #
> >>  config HOTPLUG_PCI_PCIE
> >> -       tristate "PCI Express Hotplug driver"
> >> +       bool "PCI Express Hotplug driver"
> >>         depends on HOTPLUG_PCI && PCIEPORTBUS
> >>         help
> >>           Say Y here if you have a motherboard that supports PCI Express Native
> >>           Hotplug
> >>
> >> -         To compile this driver as a module, choose M here: the
> >> -         module will be called pciehp.
> >> -
> >>           When in doubt, say N.
> >>
> >>  source "drivers/pci/pcie/aer/Kconfig"
> >>
> >
> > Acked-by: Yinghai Lu <yinghai@kernel.org>
> 
> Hi Bjorn,
> 
> Looks like we lose the option to disable pciehp after we make it as built-in.
> 
> Before acpiphp and pciehp could be compiled as modules, and user could
> blacklist to disable them.
> 
> Now they are all built-in, but only acpiphp has acpiphp.disable to
> disable acpiphp.
> we don't have pciehp.disable yet.
> 
> Do you think if we should add pciehp.disable ?

Did you find a situation that would require pciehp.disable?  I hesitate to
add it because if there's a problem and pciehp.disable fixes it, people
tend to think the solution is "boot with pciehp.disable."  But the *real*
solution is to fix whatever is broken in the kernel, so no parameter is
needed at all.

> BTW we don't have any description for acpiphp.disable anywhere.

True.  I'll give you my opinion; Rafael may have a different one.

I don't know whether it's a good idea to add a description or not, for the
same reason as above.  I think we should actively discourage people from
using kernel parameters, except for debugging purposes and for some legacy
issues where there's no way for the kernel to figure things out by itself.
But in my opinion, acpiphp isn't in any of those categories, so I'm content
to have the parameter present but undocumented.  It seems more likely that
we'll hear about issues then, and we might be able to do something about
them.

Bjorn