diff mbox

PCI: pciehp: Convert pciehp to be builtin only, not modular

Message ID 20130723180821.19616.52936.stgit@bhelgaas-glaptop (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Bjorn Helgaas July 23, 2013, 6:08 p.m. UTC
Convert pciehp to be builtin only, with no module option.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pcie/Kconfig |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Rafael Wysocki July 23, 2013, 9:26 p.m. UTC | #1
On Tuesday, July 23, 2013 12:08:21 PM Bjorn Helgaas 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"
>
Yinghai Lu July 23, 2013, 11:10 p.m. UTC | #2
On Tue, Jul 23, 2013 at 11:08 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>
> ---
>  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

HOTPLUG_PCI is still tristate,

 menuconfig HOTPLUG_PCI
        tristate "Support for PCI Hotplug"


Can you make HOTPLUG_PCI to be built-in ?

or to be more aggressive, just kill HOTPLUG_PCI as with HOTPLUG.

Thanks

Yinghai

>         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"
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas July 24, 2013, 5:59 p.m. UTC | #3
On Tue, Jul 23, 2013 at 5:10 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Tue, Jul 23, 2013 at 11:08 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>
>> ---
>>  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
>
> HOTPLUG_PCI is still tristate,
>
>  menuconfig HOTPLUG_PCI
>         tristate "Support for PCI Hotplug"
>
>
> Can you make HOTPLUG_PCI to be built-in ?

Is that necessary for this CONFIG_HOTPLUG_PCI_PCIE change?  The config
tools are smart enough to only offer CONFIG_HOTPLUG_PCI_PCIE as an
option when CONFIG_HOTPLUG_PCI=y, so I'm not sure it's actually a
problem yet.

> or to be more aggressive, just kill HOTPLUG_PCI as with HOTPLUG.

Getting rid of CONFIG_HOTPLUG let us get rid of all the __devinit*
markings, which helps avoid a whole class of bugs.  That doesn't apply
to CONFIG_HOTPLUG_PCI, of course.

But if somebody wants to do the work and see if it's useful to remove
CONFIG_HOTPLUG_PCI, I'm certainly willing to consider it.  I'm sure
most general-purpose distros will set CONFIG_HOTPLUG_PCI=y and
CONFIG_HOTPLUG_PCI_PCIE=y, but I wouldn't be surprised if embedded
folks still leave them both disabled.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Donald Dutile July 24, 2013, 9:09 p.m. UTC | #4
On 07/23/2013 02:08 PM, Bjorn Helgaas wrote:
> Convert pciehp to be builtin only, with no module option.
>
> Signed-off-by: Bjorn Helgaas<bhelgaas@google.com>

Acked-by: Donald Dutile <ddutile@redhat.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"
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yinghai Lu July 25, 2013, 1:14 p.m. UTC | #5
On Wed, Jul 24, 2013 at 10:59 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Tue, Jul 23, 2013 at 5:10 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Tue, Jul 23, 2013 at 11:08 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>
>>> ---
>>>  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
>>
>> HOTPLUG_PCI is still tristate,
>>
>>  menuconfig HOTPLUG_PCI
>>         tristate "Support for PCI Hotplug"
>>
>>
>> Can you make HOTPLUG_PCI to be built-in ?
>
> Is that necessary for this CONFIG_HOTPLUG_PCI_PCIE change?  The config
> tools are smart enough to only offer CONFIG_HOTPLUG_PCI_PCIE as an
> option when CONFIG_HOTPLUG_PCI=y, so I'm not sure it's actually a
> problem yet.

a little weird.
before this change normally
CONFIG_HOTPLUG_PCI=m, and CONFIG_HOTPLUG_PCI_PCIE=m.

now if user may select
CONFIG_HOTPLUG_PCI=m, and CONFIG_HOTPLUG_PCI_PCIE=y
never test that if it is really work.

>
>> or to be more aggressive, just kill HOTPLUG_PCI as with HOTPLUG.
>
> Getting rid of CONFIG_HOTPLUG let us get rid of all the __devinit*
> markings, which helps avoid a whole class of bugs.  That doesn't apply
> to CONFIG_HOTPLUG_PCI, of course.
>
> But if somebody wants to do the work and see if it's useful to remove
> CONFIG_HOTPLUG_PCI, I'm certainly willing to consider it.  I'm sure
> most general-purpose distros will set CONFIG_HOTPLUG_PCI=y and
> CONFIG_HOTPLUG_PCI_PCIE=y, but I wouldn't be surprised if embedded
> folks still leave them both disabled.

ok, let's leave HOTPLUG_PCI alone.

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas July 25, 2013, 4:47 p.m. UTC | #6
On Thu, Jul 25, 2013 at 7:14 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Wed, Jul 24, 2013 at 10:59 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Tue, Jul 23, 2013 at 5:10 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Tue, Jul 23, 2013 at 11:08 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>
>>>> ---
>>>>  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
>>>
>>> HOTPLUG_PCI is still tristate,
>>>
>>>  menuconfig HOTPLUG_PCI
>>>         tristate "Support for PCI Hotplug"
>>>
>>>
>>> Can you make HOTPLUG_PCI to be built-in ?
>>
>> Is that necessary for this CONFIG_HOTPLUG_PCI_PCIE change?  The config
>> tools are smart enough to only offer CONFIG_HOTPLUG_PCI_PCIE as an
>> option when CONFIG_HOTPLUG_PCI=y, so I'm not sure it's actually a
>> problem yet.
>
> a little weird.
> before this change normally
> CONFIG_HOTPLUG_PCI=m, and CONFIG_HOTPLUG_PCI_PCIE=m.
>
> now if user may select
> CONFIG_HOTPLUG_PCI=m, and CONFIG_HOTPLUG_PCI_PCIE=y
> never test that if it is really work.

Ooh, you're right!  I tried CONFIG_HOTPLUG_PCI=m and
CONFIG_HOTPLUG_PCI_PCIE=y, and config *allows* that and builds a
working kernel (though pciehp doesn't actually work), even though I
don't think menuconfig will actually generate that combination.  I'll
convert CONFIG_HOTPLUG_PCI to a bool as well.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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"