| Message ID | 1358525267-14268-5-git-send-email-jiang.liu@huawei.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Saturday, January 19, 2013 12:07:42 AM Jiang Liu wrote: > As discussed in thread at https://patchwork.kernel.org/patch/1946851/, > there's no value in supporting CONFIG_ACPI_PCI_SLOT=m any more. > So change Kconfig and code to only support building pci_slot as > built-in driver. > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/acpi/Kconfig | 5 +---- > drivers/acpi/internal.h | 5 +++++ > drivers/acpi/pci_slot.c | 13 +------------ > drivers/acpi/scan.c | 1 + > 4 files changed, 8 insertions(+), 16 deletions(-) > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig > index 0300bf6..7efd0d0 100644 > --- a/drivers/acpi/Kconfig > +++ b/drivers/acpi/Kconfig > @@ -299,7 +299,7 @@ config ACPI_DEBUG_FUNC_TRACE > is about half of the penalty and is rarely useful. > > config ACPI_PCI_SLOT > - tristate "PCI slot detection driver" > + bool "PCI slot detection driver" > depends on SYSFS > default n > help > @@ -308,9 +308,6 @@ config ACPI_PCI_SLOT > i.e., segment/bus/device/function tuples, with physical slots in > the system. If you are unsure, say N. > > - To compile this driver as a module, choose M here: > - the module will be called pci_slot. > - > config X86_PM_TIMER > bool "Power Management Timer Support" if EXPERT > depends on X86 > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > index e050254..7374cfc 100644 > --- a/drivers/acpi/internal.h > +++ b/drivers/acpi/internal.h > @@ -67,6 +67,11 @@ struct acpi_ec { > > extern struct acpi_ec *first_ec; > > +#ifdef CONFIG_ACPI_PCI_SLOT > +void acpi_pci_slot_init(void); > +#else > +static inline void acpi_pci_slot_init(void) { } > +#endif > int acpi_pci_root_init(void); > int acpi_ec_init(void); > int acpi_ec_ecdt_probe(void); > diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c > index d22585f..a7d7e77 100644 > --- a/drivers/acpi/pci_slot.c > +++ b/drivers/acpi/pci_slot.c > @@ -330,19 +330,8 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = { > {} > }; > > -static int __init > -acpi_pci_slot_init(void) > +void __init acpi_pci_slot_init(void) > { > dmi_check_system(acpi_pci_slot_dmi_table); > acpi_pci_register_driver(&acpi_pci_slot_driver); > - return 0; > } > - > -static void __exit > -acpi_pci_slot_exit(void) > -{ > - acpi_pci_unregister_driver(&acpi_pci_slot_driver); > -} > - > -module_init(acpi_pci_slot_init); > -module_exit(acpi_pci_slot_exit); > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index f8a0d0f..cb964ac 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -1732,6 +1732,7 @@ int __init acpi_scan_init(void) > > acpi_power_init(); > acpi_pci_root_init(); > + acpi_pci_slot_init(); > > /* > * Enumerate devices in the ACPI namespace. >
On Sun, Jan 20, 2013 at 5:01 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Saturday, January 19, 2013 12:07:42 AM Jiang Liu wrote: >> As discussed in thread at https://patchwork.kernel.org/patch/1946851/, >> there's no value in supporting CONFIG_ACPI_PCI_SLOT=m any more. >> So change Kconfig and code to only support building pci_slot as >> built-in driver. >> >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> I think we should eventually get rid of acpi_pci_register_driver() and do this initialization directly from acpi_pci_root_add(). But removing the module option here is a good first step. Rafael, do you want to apply this (and [6/8]) via your tree? If not, I can take it. >> --- >> drivers/acpi/Kconfig | 5 +---- >> drivers/acpi/internal.h | 5 +++++ >> drivers/acpi/pci_slot.c | 13 +------------ >> drivers/acpi/scan.c | 1 + >> 4 files changed, 8 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig >> index 0300bf6..7efd0d0 100644 >> --- a/drivers/acpi/Kconfig >> +++ b/drivers/acpi/Kconfig >> @@ -299,7 +299,7 @@ config ACPI_DEBUG_FUNC_TRACE >> is about half of the penalty and is rarely useful. >> >> config ACPI_PCI_SLOT >> - tristate "PCI slot detection driver" >> + bool "PCI slot detection driver" >> depends on SYSFS >> default n >> help >> @@ -308,9 +308,6 @@ config ACPI_PCI_SLOT >> i.e., segment/bus/device/function tuples, with physical slots in >> the system. If you are unsure, say N. >> >> - To compile this driver as a module, choose M here: >> - the module will be called pci_slot. >> - >> config X86_PM_TIMER >> bool "Power Management Timer Support" if EXPERT >> depends on X86 >> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h >> index e050254..7374cfc 100644 >> --- a/drivers/acpi/internal.h >> +++ b/drivers/acpi/internal.h >> @@ -67,6 +67,11 @@ struct acpi_ec { >> >> extern struct acpi_ec *first_ec; >> >> +#ifdef CONFIG_ACPI_PCI_SLOT >> +void acpi_pci_slot_init(void); >> +#else >> +static inline void acpi_pci_slot_init(void) { } >> +#endif >> int acpi_pci_root_init(void); >> int acpi_ec_init(void); >> int acpi_ec_ecdt_probe(void); >> diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c >> index d22585f..a7d7e77 100644 >> --- a/drivers/acpi/pci_slot.c >> +++ b/drivers/acpi/pci_slot.c >> @@ -330,19 +330,8 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = { >> {} >> }; >> >> -static int __init >> -acpi_pci_slot_init(void) >> +void __init acpi_pci_slot_init(void) >> { >> dmi_check_system(acpi_pci_slot_dmi_table); >> acpi_pci_register_driver(&acpi_pci_slot_driver); >> - return 0; >> } >> - >> -static void __exit >> -acpi_pci_slot_exit(void) >> -{ >> - acpi_pci_unregister_driver(&acpi_pci_slot_driver); >> -} >> - >> -module_init(acpi_pci_slot_init); >> -module_exit(acpi_pci_slot_exit); >> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c >> index f8a0d0f..cb964ac 100644 >> --- a/drivers/acpi/scan.c >> +++ b/drivers/acpi/scan.c >> @@ -1732,6 +1732,7 @@ int __init acpi_scan_init(void) >> >> acpi_power_init(); >> acpi_pci_root_init(); >> + acpi_pci_slot_init(); >> >> /* >> * Enumerate devices in the ACPI namespace. >> > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center. -- 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
On Mon, Jan 28, 2013 at 1:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Sun, Jan 20, 2013 at 5:01 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> On Saturday, January 19, 2013 12:07:42 AM Jiang Liu wrote: >>> As discussed in thread at https://patchwork.kernel.org/patch/1946851/, >>> there's no value in supporting CONFIG_ACPI_PCI_SLOT=m any more. >>> So change Kconfig and code to only support building pci_slot as >>> built-in driver. >>> >>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> >> >> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > I think we should eventually get rid of acpi_pci_register_driver() and > do this initialization directly from acpi_pci_root_add(). But > removing the module option here is a good first step. > > Rafael, do you want to apply this (and [6/8]) via your tree? If not, > I can take it. If bios have messed up slot name or idx, we will get strange 1-1.... other crazy name. if you really need to put it as built-in, may need to some command line that user could switch it off. 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
On Mon, Jan 28, 2013 at 2:29 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Mon, Jan 28, 2013 at 1:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> On Sun, Jan 20, 2013 at 5:01 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >>> On Saturday, January 19, 2013 12:07:42 AM Jiang Liu wrote: >>>> As discussed in thread at https://patchwork.kernel.org/patch/1946851/, >>>> there's no value in supporting CONFIG_ACPI_PCI_SLOT=m any more. >>>> So change Kconfig and code to only support building pci_slot as >>>> built-in driver. >>>> >>>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> >>> >>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> >> Acked-by: Bjorn Helgaas <bhelgaas@google.com> >> >> I think we should eventually get rid of acpi_pci_register_driver() and >> do this initialization directly from acpi_pci_root_add(). But >> removing the module option here is a good first step. >> >> Rafael, do you want to apply this (and [6/8]) via your tree? If not, >> I can take it. > > If bios have messed up slot name or idx, we will get strange 1-1.... > other crazy name. > > if you really need to put it as built-in, may need to some command > line that user could switch it off. It would save us all a lot of time if you gave an example and worked through the scenario where this is a problem. We already have the choice of having pci_slot built-in, so if there's a bug in that config, we already have the bug. This patch merely removes a config where the bug might be covered up. I don't know why "adding a command line switch" appeals to you as the solution to every problem. As far as I'm concerned that's not a solution to ANY problem. It might be a band-aid to enable users to limp along while we figure out a correct solution, but it's certainly not a resolution. 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
On Mon, Jan 28, 2013 at 1:52 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Mon, Jan 28, 2013 at 2:29 PM, Yinghai Lu <yinghai@kernel.org> wrote: ... >> If bios have messed up slot name or idx, we will get strange 1-1.... >> other crazy name. >> >> if you really need to put it as built-in, may need to some command >> line that user could switch it off. > > It would save us all a lot of time if you gave an example and worked > through the scenario where this is a problem. > > We already have the choice of having pci_slot built-in, so if there's > a bug in that config, we already have the bug. This patch merely > removes a config where the bug might be covered up. for distribution, current it is with module, so user could blacklist in module.conf Now with built-in or not, distribution will have it built-in, and user have no chance to disable it. > I don't know why "adding a command line switch" appeals to you as the > solution to every problem. As far as I'm concerned that's not a > solution to ANY problem. It might be a band-aid to enable users to > limp along while we figure out a correct solution, but it's certainly > not a resolution. Looks like you want to remove command line support, right ? 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
On Mon, Jan 28, 2013 at 3:00 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Mon, Jan 28, 2013 at 1:52 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> On Mon, Jan 28, 2013 at 2:29 PM, Yinghai Lu <yinghai@kernel.org> wrote: > ... >>> If bios have messed up slot name or idx, we will get strange 1-1.... >>> other crazy name. >>> >>> if you really need to put it as built-in, may need to some command >>> line that user could switch it off. >> >> It would save us all a lot of time if you gave an example and worked >> through the scenario where this is a problem. >> >> We already have the choice of having pci_slot built-in, so if there's >> a bug in that config, we already have the bug. This patch merely >> removes a config where the bug might be covered up. > > > for distribution, current it is with module, so user could blacklist > in module.conf > > Now with built-in or not, distribution will have it built-in, and user > have no chance to > disable it. CONFIG_ACPI_PCI_SLOT=y in RHEL6, so evidently they have this problem. Asking users to edit module.conf by hand is not a solution, just like asking users to boot with a command line option is not a solution. That sort of stuff is fine for a hobbyist OS intended only for techie geeks. It's not fine for Linux. If you would give a concrete example of the ACPI namespace info and device config, hotplug sequence, etc., required to show the problem, we could have a useful discussion about ways to fix it. But if all you have is FUD about "this might break and users won't have the ability to edit modules.conf," that doesn't help me see why this patch is a bad idea. >> I don't know why "adding a command line switch" appeals to you as the >> solution to every problem. As far as I'm concerned that's not a >> solution to ANY problem. It might be a band-aid to enable users to >> limp along while we figure out a correct solution, but it's certainly >> not a resolution. > > Looks like you want to remove command line support, right ? > > 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
On Mon, Jan 28, 2013 at 2:14 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > > CONFIG_ACPI_PCI_SLOT=y in RHEL6, so evidently they have this problem. oh, I only checked opensuse that has that set to m. > > Asking users to edit module.conf by hand is not a solution, just like > asking users to boot with a command line option is not a solution. > That sort of stuff is fine for a hobbyist OS intended only for techie > geeks. It's not fine for Linux. not sure. add something in command line or conf files. but recompile kernel is another story. > > If you would give a concrete example of the ACPI namespace info and > device config, hotplug sequence, etc., required to show the problem, > we could have a useful discussion about ways to fix it. But if all > you have is FUD about "this might break and users won't have the > ability to edit modules.conf," that doesn't help me see why this patch > is a bad idea. Never mind, We should save your bandwidth to more patches. 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
On Monday, January 28, 2013 02:09:22 PM Bjorn Helgaas wrote: > On Sun, Jan 20, 2013 at 5:01 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > On Saturday, January 19, 2013 12:07:42 AM Jiang Liu wrote: > >> As discussed in thread at https://patchwork.kernel.org/patch/1946851/, > >> there's no value in supporting CONFIG_ACPI_PCI_SLOT=m any more. > >> So change Kconfig and code to only support building pci_slot as > >> built-in driver. > >> > >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > > > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > I think we should eventually get rid of acpi_pci_register_driver() and > do this initialization directly from acpi_pci_root_add(). But > removing the module option here is a good first step. > > Rafael, do you want to apply this (and [6/8]) via your tree? If not, > I can take it. I will, thanks. Rafael > >> --- > >> drivers/acpi/Kconfig | 5 +---- > >> drivers/acpi/internal.h | 5 +++++ > >> drivers/acpi/pci_slot.c | 13 +------------ > >> drivers/acpi/scan.c | 1 + > >> 4 files changed, 8 insertions(+), 16 deletions(-) > >> > >> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig > >> index 0300bf6..7efd0d0 100644 > >> --- a/drivers/acpi/Kconfig > >> +++ b/drivers/acpi/Kconfig > >> @@ -299,7 +299,7 @@ config ACPI_DEBUG_FUNC_TRACE > >> is about half of the penalty and is rarely useful. > >> > >> config ACPI_PCI_SLOT > >> - tristate "PCI slot detection driver" > >> + bool "PCI slot detection driver" > >> depends on SYSFS > >> default n > >> help > >> @@ -308,9 +308,6 @@ config ACPI_PCI_SLOT > >> i.e., segment/bus/device/function tuples, with physical slots in > >> the system. If you are unsure, say N. > >> > >> - To compile this driver as a module, choose M here: > >> - the module will be called pci_slot. > >> - > >> config X86_PM_TIMER > >> bool "Power Management Timer Support" if EXPERT > >> depends on X86 > >> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > >> index e050254..7374cfc 100644 > >> --- a/drivers/acpi/internal.h > >> +++ b/drivers/acpi/internal.h > >> @@ -67,6 +67,11 @@ struct acpi_ec { > >> > >> extern struct acpi_ec *first_ec; > >> > >> +#ifdef CONFIG_ACPI_PCI_SLOT > >> +void acpi_pci_slot_init(void); > >> +#else > >> +static inline void acpi_pci_slot_init(void) { } > >> +#endif > >> int acpi_pci_root_init(void); > >> int acpi_ec_init(void); > >> int acpi_ec_ecdt_probe(void); > >> diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c > >> index d22585f..a7d7e77 100644 > >> --- a/drivers/acpi/pci_slot.c > >> +++ b/drivers/acpi/pci_slot.c > >> @@ -330,19 +330,8 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = { > >> {} > >> }; > >> > >> -static int __init > >> -acpi_pci_slot_init(void) > >> +void __init acpi_pci_slot_init(void) > >> { > >> dmi_check_system(acpi_pci_slot_dmi_table); > >> acpi_pci_register_driver(&acpi_pci_slot_driver); > >> - return 0; > >> } > >> - > >> -static void __exit > >> -acpi_pci_slot_exit(void) > >> -{ > >> - acpi_pci_unregister_driver(&acpi_pci_slot_driver); > >> -} > >> - > >> -module_init(acpi_pci_slot_init); > >> -module_exit(acpi_pci_slot_exit); > >> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > >> index f8a0d0f..cb964ac 100644 > >> --- a/drivers/acpi/scan.c > >> +++ b/drivers/acpi/scan.c > >> @@ -1732,6 +1732,7 @@ int __init acpi_scan_init(void) > >> > >> acpi_power_init(); > >> acpi_pci_root_init(); > >> + acpi_pci_slot_init(); > >> > >> /* > >> * Enumerate devices in the ACPI namespace. > >> > > -- > > I speak only for myself. > > Rafael J. Wysocki, Intel Open Source Technology Center. > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2013-1-29 6:58, Yinghai Lu wrote: > On Mon, Jan 28, 2013 at 2:14 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> >> CONFIG_ACPI_PCI_SLOT=y in RHEL6, so evidently they have this problem. > > oh, I only checked opensuse that has that set to m. > >> >> Asking users to edit module.conf by hand is not a solution, just like >> asking users to boot with a command line option is not a solution. >> That sort of stuff is fine for a hobbyist OS intended only for techie >> geeks. It's not fine for Linux. > > not sure. add something in command line or conf files. > but recompile kernel is another story. > >> >> If you would give a concrete example of the ACPI namespace info and >> device config, hotplug sequence, etc., required to show the problem, >> we could have a useful discussion about ways to fix it. But if all >> you have is FUD about "this might break and users won't have the >> ability to edit modules.conf," that doesn't help me see why this patch >> is a bad idea. > > Never mind, We should save your bandwidth to more patches. Hi Yinghai, Could we use quirk to auto-disable PCIe native hotplug for problematic platforms? Regards! Gerry > > 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
On Mon, Jan 28, 2013 at 6:07 PM, Jiang Liu <jiang.liu@huawei.com> wrote: > Could we use quirk to auto-disable PCIe native hotplug for > problematic platforms? that is some kind of boot command line way? 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
On 2013-1-29 10:21, Yinghai Lu wrote: > On Mon, Jan 28, 2013 at 6:07 PM, Jiang Liu <jiang.liu@huawei.com> wrote: >> Could we use quirk to auto-disable PCIe native hotplug for >> problematic platforms? > > that is some kind of boot command line way? We could negotiate between acpiphp and pciehp if those problematic platforms/chipsets could be identified by DMI info or PCI device ID. > > 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
On Mon, Jan 28, 2013 at 7:45 PM, Jiang Liu <jiang.liu@huawei.com> wrote: > On 2013-1-29 10:21, Yinghai Lu wrote: >> On Mon, Jan 28, 2013 at 6:07 PM, Jiang Liu <jiang.liu@huawei.com> wrote: >>> Could we use quirk to auto-disable PCIe native hotplug for >>> problematic platforms? >> >> that is some kind of boot command line way? > We could negotiate between acpiphp and pciehp if those problematic > platforms/chipsets could be identified by DMI info or PCI device ID. That's a sub-optimal solution because it requires us to identify all the affected systems and also may require ongoing maintenance in the future. If we could get specifics on the issue, we might be able to design a better solution that works for everybody. (BTW, this thread is specifically about pci_slot, which I think is a slightly different issue than the acpiphp/pciehp conflicts.) -- 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
On Mon, Jan 28, 2013 at 03:14:11PM -0700, Bjorn Helgaas wrote:
> CONFIG_ACPI_PCI_SLOT=y in RHEL6, so evidently they have this problem.
I'm not aware of anyone ever filing any bugs against it, either, though
Myron would have a better idea. What's the worst case outcome of someone
ending up with an incorrect slot number?
On Tue, Jan 29, 2013 at 10:07:22AM +0800, Jiang Liu wrote: > Could we use quirk to auto-disable PCIe native hotplug for > problematic platforms? Do these problematic platforms successfully boot Windows 7?
On Monday, January 28, 2013 02:09:22 PM Bjorn Helgaas wrote: > On Sun, Jan 20, 2013 at 5:01 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > On Saturday, January 19, 2013 12:07:42 AM Jiang Liu wrote: > >> As discussed in thread at https://patchwork.kernel.org/patch/1946851/, > >> there's no value in supporting CONFIG_ACPI_PCI_SLOT=m any more. > >> So change Kconfig and code to only support building pci_slot as > >> built-in driver. > >> > >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > > > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > I think we should eventually get rid of acpi_pci_register_driver() and > do this initialization directly from acpi_pci_root_add(). But > removing the module option here is a good first step. > > Rafael, do you want to apply this (and [6/8]) via your tree? If not, > I can take it. Both applied to bleeding-edge. I've added your ACK to the [6/8] too, hope that's OK. Thanks, Rafael
On Sun, Feb 3, 2013 at 1:18 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Monday, January 28, 2013 02:09:22 PM Bjorn Helgaas wrote: >> On Sun, Jan 20, 2013 at 5:01 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> > On Saturday, January 19, 2013 12:07:42 AM Jiang Liu wrote: >> >> As discussed in thread at https://patchwork.kernel.org/patch/1946851/, >> >> there's no value in supporting CONFIG_ACPI_PCI_SLOT=m any more. >> >> So change Kconfig and code to only support building pci_slot as >> >> built-in driver. >> >> >> >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> >> > >> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> >> Acked-by: Bjorn Helgaas <bhelgaas@google.com> >> >> I think we should eventually get rid of acpi_pci_register_driver() and >> do this initialization directly from acpi_pci_root_add(). But >> removing the module option here is a good first step. >> >> Rafael, do you want to apply this (and [6/8]) via your tree? If not, >> I can take it. > > Both applied to bleeding-edge. I've added your ACK to the [6/8] too, hope > that's OK. Yep, that's fine, thanks! 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
On Mon, 2013-01-28 at 14:09 -0700, Bjorn Helgaas wrote: > On Sun, Jan 20, 2013 at 5:01 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > On Saturday, January 19, 2013 12:07:42 AM Jiang Liu wrote: > >> As discussed in thread at https://patchwork.kernel.org/patch/1946851/, > >> there's no value in supporting CONFIG_ACPI_PCI_SLOT=m any more. > >> So change Kconfig and code to only support building pci_slot as > >> built-in driver. > >> > >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > > > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > I think we should eventually get rid of acpi_pci_register_driver() and > do this initialization directly from acpi_pci_root_add(). But > removing the module option here is a good first step. Bjorn, Rafael: If either of you are interested in that still let me know and I can re-work any or all of the "PCI/ACPI: Remove "pci_root" sub-driver support" series - https://lkml.org/lkml/2012/12/7/11 Note [PATCH 13/15] was effectively the same as below and continued on with initializing directly from acpi_pci_root_add() in [PATCH 14/15]. There may have been worthwhile fixes in some of the earlier content such as [PATCH 11/15] worth re-considering also. Thanks, Myron > > Rafael, do you want to apply this (and [6/8]) via your tree? If not, > I can take it. > > >> --- > >> drivers/acpi/Kconfig | 5 +---- > >> drivers/acpi/internal.h | 5 +++++ > >> drivers/acpi/pci_slot.c | 13 +------------ > >> drivers/acpi/scan.c | 1 + > >> 4 files changed, 8 insertions(+), 16 deletions(-) > >> > >> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig > >> index 0300bf6..7efd0d0 100644 > >> --- a/drivers/acpi/Kconfig > >> +++ b/drivers/acpi/Kconfig > >> @@ -299,7 +299,7 @@ config ACPI_DEBUG_FUNC_TRACE > >> is about half of the penalty and is rarely useful. > >> > >> config ACPI_PCI_SLOT > >> - tristate "PCI slot detection driver" > >> + bool "PCI slot detection driver" > >> depends on SYSFS > >> default n > >> help > >> @@ -308,9 +308,6 @@ config ACPI_PCI_SLOT > >> i.e., segment/bus/device/function tuples, with physical slots in > >> the system. If you are unsure, say N. > >> > >> - To compile this driver as a module, choose M here: > >> - the module will be called pci_slot. > >> - > >> config X86_PM_TIMER > >> bool "Power Management Timer Support" if EXPERT > >> depends on X86 > >> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > >> index e050254..7374cfc 100644 > >> --- a/drivers/acpi/internal.h > >> +++ b/drivers/acpi/internal.h > >> @@ -67,6 +67,11 @@ struct acpi_ec { > >> > >> extern struct acpi_ec *first_ec; > >> > >> +#ifdef CONFIG_ACPI_PCI_SLOT > >> +void acpi_pci_slot_init(void); > >> +#else > >> +static inline void acpi_pci_slot_init(void) { } > >> +#endif > >> int acpi_pci_root_init(void); > >> int acpi_ec_init(void); > >> int acpi_ec_ecdt_probe(void); > >> diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c > >> index d22585f..a7d7e77 100644 > >> --- a/drivers/acpi/pci_slot.c > >> +++ b/drivers/acpi/pci_slot.c > >> @@ -330,19 +330,8 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = { > >> {} > >> }; > >> > >> -static int __init > >> -acpi_pci_slot_init(void) > >> +void __init acpi_pci_slot_init(void) > >> { > >> dmi_check_system(acpi_pci_slot_dmi_table); > >> acpi_pci_register_driver(&acpi_pci_slot_driver); > >> - return 0; > >> } > >> - > >> -static void __exit > >> -acpi_pci_slot_exit(void) > >> -{ > >> - acpi_pci_unregister_driver(&acpi_pci_slot_driver); > >> -} > >> - > >> -module_init(acpi_pci_slot_init); > >> -module_exit(acpi_pci_slot_exit); > >> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > >> index f8a0d0f..cb964ac 100644 > >> --- a/drivers/acpi/scan.c > >> +++ b/drivers/acpi/scan.c > >> @@ -1732,6 +1732,7 @@ int __init acpi_scan_init(void) > >> > >> acpi_power_init(); > >> acpi_pci_root_init(); > >> + acpi_pci_slot_init(); > >> > >> /* > >> * Enumerate devices in the ACPI namespace. > >> > > -- > > I speak only for myself. > > Rafael J. Wysocki, Intel Open Source Technology Center. -- 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
On Sunday, February 03, 2013 03:47:15 PM Myron Stowe wrote: > On Mon, 2013-01-28 at 14:09 -0700, Bjorn Helgaas wrote: > > On Sun, Jan 20, 2013 at 5:01 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > > On Saturday, January 19, 2013 12:07:42 AM Jiang Liu wrote: > > >> As discussed in thread at https://patchwork.kernel.org/patch/1946851/, > > >> there's no value in supporting CONFIG_ACPI_PCI_SLOT=m any more. > > >> So change Kconfig and code to only support building pci_slot as > > >> built-in driver. > > >> > > >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > > > > > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > > > I think we should eventually get rid of acpi_pci_register_driver() and > > do this initialization directly from acpi_pci_root_add(). But > > removing the module option here is a good first step. > > Bjorn, Rafael: > > If either of you are interested in that still let me know and I can > re-work any or all of the "PCI/ACPI: Remove "pci_root" sub-driver > support" series - https://lkml.org/lkml/2012/12/7/11 I am. However, I think it's better to wait with the rework until our trees are merged into the mainline, so that you can base the patchset on a common tree. > Note [PATCH 13/15] was effectively the same as below and continued on > with initializing directly from acpi_pci_root_add() in [PATCH 14/15]. > There may have been worthwhile fixes in some of the earlier content such > as [PATCH 11/15] worth re-considering also. Well, as I said. All of the patches in the series making sense after v3.9-rc1 will be interesting to me certainly. Thanks, Rafael > > Rafael, do you want to apply this (and [6/8]) via your tree? If not, > > I can take it. > > > > >> --- > > >> drivers/acpi/Kconfig | 5 +---- > > >> drivers/acpi/internal.h | 5 +++++ > > >> drivers/acpi/pci_slot.c | 13 +------------ > > >> drivers/acpi/scan.c | 1 + > > >> 4 files changed, 8 insertions(+), 16 deletions(-) > > >> > > >> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig > > >> index 0300bf6..7efd0d0 100644 > > >> --- a/drivers/acpi/Kconfig > > >> +++ b/drivers/acpi/Kconfig > > >> @@ -299,7 +299,7 @@ config ACPI_DEBUG_FUNC_TRACE > > >> is about half of the penalty and is rarely useful. > > >> > > >> config ACPI_PCI_SLOT > > >> - tristate "PCI slot detection driver" > > >> + bool "PCI slot detection driver" > > >> depends on SYSFS > > >> default n > > >> help > > >> @@ -308,9 +308,6 @@ config ACPI_PCI_SLOT > > >> i.e., segment/bus/device/function tuples, with physical slots in > > >> the system. If you are unsure, say N. > > >> > > >> - To compile this driver as a module, choose M here: > > >> - the module will be called pci_slot. > > >> - > > >> config X86_PM_TIMER > > >> bool "Power Management Timer Support" if EXPERT > > >> depends on X86 > > >> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > > >> index e050254..7374cfc 100644 > > >> --- a/drivers/acpi/internal.h > > >> +++ b/drivers/acpi/internal.h > > >> @@ -67,6 +67,11 @@ struct acpi_ec { > > >> > > >> extern struct acpi_ec *first_ec; > > >> > > >> +#ifdef CONFIG_ACPI_PCI_SLOT > > >> +void acpi_pci_slot_init(void); > > >> +#else > > >> +static inline void acpi_pci_slot_init(void) { } > > >> +#endif > > >> int acpi_pci_root_init(void); > > >> int acpi_ec_init(void); > > >> int acpi_ec_ecdt_probe(void); > > >> diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c > > >> index d22585f..a7d7e77 100644 > > >> --- a/drivers/acpi/pci_slot.c > > >> +++ b/drivers/acpi/pci_slot.c > > >> @@ -330,19 +330,8 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = { > > >> {} > > >> }; > > >> > > >> -static int __init > > >> -acpi_pci_slot_init(void) > > >> +void __init acpi_pci_slot_init(void) > > >> { > > >> dmi_check_system(acpi_pci_slot_dmi_table); > > >> acpi_pci_register_driver(&acpi_pci_slot_driver); > > >> - return 0; > > >> } > > >> - > > >> -static void __exit > > >> -acpi_pci_slot_exit(void) > > >> -{ > > >> - acpi_pci_unregister_driver(&acpi_pci_slot_driver); > > >> -} > > >> - > > >> -module_init(acpi_pci_slot_init); > > >> -module_exit(acpi_pci_slot_exit); > > >> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > > >> index f8a0d0f..cb964ac 100644 > > >> --- a/drivers/acpi/scan.c > > >> +++ b/drivers/acpi/scan.c > > >> @@ -1732,6 +1732,7 @@ int __init acpi_scan_init(void) > > >> > > >> acpi_power_init(); > > >> acpi_pci_root_init(); > > >> + acpi_pci_slot_init(); > > >> > > >> /* > > >> * Enumerate devices in the ACPI namespace. > > >> > > > -- > > > I speak only for myself. > > > Rafael J. Wysocki, Intel Open Source Technology Center. > >
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 0300bf6..7efd0d0 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -299,7 +299,7 @@ config ACPI_DEBUG_FUNC_TRACE is about half of the penalty and is rarely useful. config ACPI_PCI_SLOT - tristate "PCI slot detection driver" + bool "PCI slot detection driver" depends on SYSFS default n help @@ -308,9 +308,6 @@ config ACPI_PCI_SLOT i.e., segment/bus/device/function tuples, with physical slots in the system. If you are unsure, say N. - To compile this driver as a module, choose M here: - the module will be called pci_slot. - config X86_PM_TIMER bool "Power Management Timer Support" if EXPERT depends on X86 diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index e050254..7374cfc 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -67,6 +67,11 @@ struct acpi_ec { extern struct acpi_ec *first_ec; +#ifdef CONFIG_ACPI_PCI_SLOT +void acpi_pci_slot_init(void); +#else +static inline void acpi_pci_slot_init(void) { } +#endif int acpi_pci_root_init(void); int acpi_ec_init(void); int acpi_ec_ecdt_probe(void); diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c index d22585f..a7d7e77 100644 --- a/drivers/acpi/pci_slot.c +++ b/drivers/acpi/pci_slot.c @@ -330,19 +330,8 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = { {} }; -static int __init -acpi_pci_slot_init(void) +void __init acpi_pci_slot_init(void) { dmi_check_system(acpi_pci_slot_dmi_table); acpi_pci_register_driver(&acpi_pci_slot_driver); - return 0; } - -static void __exit -acpi_pci_slot_exit(void) -{ - acpi_pci_unregister_driver(&acpi_pci_slot_driver); -} - -module_init(acpi_pci_slot_init); -module_exit(acpi_pci_slot_exit); diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index f8a0d0f..cb964ac 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1732,6 +1732,7 @@ int __init acpi_scan_init(void) acpi_power_init(); acpi_pci_root_init(); + acpi_pci_slot_init(); /* * Enumerate devices in the ACPI namespace.
As discussed in thread at https://patchwork.kernel.org/patch/1946851/, there's no value in supporting CONFIG_ACPI_PCI_SLOT=m any more. So change Kconfig and code to only support building pci_slot as built-in driver. Signed-off-by: Jiang Liu <jiang.liu@huawei.com> --- drivers/acpi/Kconfig | 5 +---- drivers/acpi/internal.h | 5 +++++ drivers/acpi/pci_slot.c | 13 +------------ drivers/acpi/scan.c | 1 + 4 files changed, 8 insertions(+), 16 deletions(-)