[PATCH] x86/PCI: adjust section annotations

[PATCH] x86/PCI: adjust section annotations

[Jan Beulich]

DMI tables referenced from __init code only can be __initconst, and as
a result the functions referenced from there can become __init.

pcibios_setup() can be __init as being a command line parsing function
only.

A few other variables can then also have their attributes adjusted.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

---
 arch/x86/pci/common.c |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

--- 3.5-rc3/arch/x86/pci/common.c
+++ 3.5-rc3-x86-pci-common-sections/arch/x86/pci/common.c
@@ -21,10 +21,10 @@
 unsigned int pci_probe = PCI_PROBE_BIOS | PCI_PROBE_CONF1 | PCI_PROBE_CONF2 |
 				PCI_PROBE_MMCONF;
 
-unsigned int pci_early_dump_regs;
-static int pci_bf_sort;
-static int smbios_type_b1_flag;
-int pci_routeirq;
+unsigned int __initdata pci_early_dump_regs;
+static int __devinitdata pci_bf_sort;
+static int __devinitdata smbios_type_b1_flag;
+int __initdata pci_routeirq;
 int noioapicquirk;
 #ifdef CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS
 int noioapicreroute = 0;
@@ -32,7 +32,7 @@ int noioapicreroute = 0;
 int noioapicreroute = 1;
 #endif
 int pcibios_last_bus = -1;
-unsigned long pirq_table_addr;
+unsigned long __initdata pirq_table_addr;
 struct pci_bus *pci_root_bus;
 const struct pci_raw_ops *__read_mostly raw_pci_ops;
 const struct pci_raw_ops *__read_mostly raw_pci_ext_ops;
@@ -80,14 +80,14 @@ struct pci_ops pci_root_ops = {
  */
 DEFINE_RAW_SPINLOCK(pci_config_lock);
 
-static int __devinit can_skip_ioresource_align(const struct dmi_system_id *d)
+static int __init can_skip_ioresource_align(const struct dmi_system_id *d)
 {
 	pci_probe |= PCI_CAN_SKIP_ISA_ALIGN;
 	printk(KERN_INFO "PCI: %s detected, can skip ISA alignment\n", d->ident);
 	return 0;
 }
 
-static const struct dmi_system_id can_skip_pciprobe_dmi_table[] __devinitconst = {
+static const struct dmi_system_id can_skip_pciprobe_dmi_table[] __initconst = {
 /*
  * Systems where PCI IO resource ISA alignment can be skipped
  * when the ISA enable bit in the bridge control is not set
@@ -221,7 +221,7 @@ static int __devinit find_sort_method(co
  * Enable renumbering of PCI bus# ranges to reach all PCI busses (Cardbus)
  */
 #ifdef __i386__
-static int __devinit assign_all_busses(const struct dmi_system_id *d)
+static int __init assign_all_busses(const struct dmi_system_id *d)
 {
 	pci_probe |= PCI_ASSIGN_ALL_BUSSES;
 	printk(KERN_INFO "%s detected: enabling PCI bus# renumbering"
@@ -230,7 +230,7 @@ static int __devinit assign_all_busses(c
 }
 #endif
 
-static int __devinit set_scan_all(const struct dmi_system_id *d)
+static int __init set_scan_all(const struct dmi_system_id *d)
 {
 	printk(KERN_INFO "PCI: %s detected, enabling pci=pcie_scan_all\n",
 	       d->ident);
@@ -238,7 +238,7 @@ static int __devinit set_scan_all(const
 	return 0;
 }
 
-static const struct dmi_system_id __devinitconst pciprobe_dmi_table[] = {
+static const struct dmi_system_id __initconst pciprobe_dmi_table[] = {
 #ifdef __i386__
 /*
  * Laptops which need pci=assign-busses to see Cardbus cards
@@ -494,7 +494,7 @@ int __init pcibios_init(void)
 	return 0;
 }
 
-char * __devinit  pcibios_setup(char *str)
+char *__init pcibios_setup(char *str)
 {
 	if (!strcmp(str, "off")) {
 		pci_probe = 0;

Re: [PATCH] x86/PCI: adjust section annotations

[Bjorn Helgaas]

On Mon, Jun 18, 2012 at 4:34 AM, Jan Beulich <JBeulich@suse.com> wrote:
> DMI tables referenced from __init code only can be __initconst, and as
> a result the functions referenced from there can become __init.
>
> pcibios_setup() can be __init as being a command line parsing function
> only.
>
> A few other variables can then also have their attributes adjusted.

This seems OK as far as it goes.

However, if you're going to make pcibios_setup() __init for x86, I'd
really encourage you to make it consistent across all the other
architectures.  And if you do *that*, I think it would be cool if you
supplied a generic do-nothing "weak" version in the PCI core.  That
would allow you to remove it altogether from alpha, ia64, microblaze,
mips pmc-sierra, parisc, powerpc, sh, sparc, tile, and xtensa.

CRIS-folk: It would also fix what looks like a bug in cris, which
implements pcibios_setup() such that pci_setup() doesn't even look for
all the supposedly generic options.

> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> ---
>  arch/x86/pci/common.c |   22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> --- 3.5-rc3/arch/x86/pci/common.c
> +++ 3.5-rc3-x86-pci-common-sections/arch/x86/pci/common.c
> @@ -21,10 +21,10 @@
>  unsigned int pci_probe = PCI_PROBE_BIOS | PCI_PROBE_CONF1 | PCI_PROBE_CONF2 |
>                                PCI_PROBE_MMCONF;
>
> -unsigned int pci_early_dump_regs;
> -static int pci_bf_sort;
> -static int smbios_type_b1_flag;
> -int pci_routeirq;
> +unsigned int __initdata pci_early_dump_regs;
> +static int __devinitdata pci_bf_sort;
> +static int __devinitdata smbios_type_b1_flag;
> +int __initdata pci_routeirq;
>  int noioapicquirk;
>  #ifdef CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS
>  int noioapicreroute = 0;
> @@ -32,7 +32,7 @@ int noioapicreroute = 0;
>  int noioapicreroute = 1;
>  #endif
>  int pcibios_last_bus = -1;
> -unsigned long pirq_table_addr;
> +unsigned long __initdata pirq_table_addr;
>  struct pci_bus *pci_root_bus;
>  const struct pci_raw_ops *__read_mostly raw_pci_ops;
>  const struct pci_raw_ops *__read_mostly raw_pci_ext_ops;
> @@ -80,14 +80,14 @@ struct pci_ops pci_root_ops = {
>  */
>  DEFINE_RAW_SPINLOCK(pci_config_lock);
>
> -static int __devinit can_skip_ioresource_align(const struct dmi_system_id *d)
> +static int __init can_skip_ioresource_align(const struct dmi_system_id *d)
>  {
>        pci_probe |= PCI_CAN_SKIP_ISA_ALIGN;
>        printk(KERN_INFO "PCI: %s detected, can skip ISA alignment\n", d->ident);
>        return 0;
>  }
>
> -static const struct dmi_system_id can_skip_pciprobe_dmi_table[] __devinitconst = {
> +static const struct dmi_system_id can_skip_pciprobe_dmi_table[] __initconst = {
>  /*
>  * Systems where PCI IO resource ISA alignment can be skipped
>  * when the ISA enable bit in the bridge control is not set
> @@ -221,7 +221,7 @@ static int __devinit find_sort_method(co
>  * Enable renumbering of PCI bus# ranges to reach all PCI busses (Cardbus)
>  */
>  #ifdef __i386__
> -static int __devinit assign_all_busses(const struct dmi_system_id *d)
> +static int __init assign_all_busses(const struct dmi_system_id *d)
>  {
>        pci_probe |= PCI_ASSIGN_ALL_BUSSES;
>        printk(KERN_INFO "%s detected: enabling PCI bus# renumbering"
> @@ -230,7 +230,7 @@ static int __devinit assign_all_busses(c
>  }
>  #endif
>
> -static int __devinit set_scan_all(const struct dmi_system_id *d)
> +static int __init set_scan_all(const struct dmi_system_id *d)
>  {
>        printk(KERN_INFO "PCI: %s detected, enabling pci=pcie_scan_all\n",
>               d->ident);
> @@ -238,7 +238,7 @@ static int __devinit set_scan_all(const
>        return 0;
>  }
>
> -static const struct dmi_system_id __devinitconst pciprobe_dmi_table[] = {
> +static const struct dmi_system_id __initconst pciprobe_dmi_table[] = {
>  #ifdef __i386__
>  /*
>  * Laptops which need pci=assign-busses to see Cardbus cards
> @@ -494,7 +494,7 @@ int __init pcibios_init(void)
>        return 0;
>  }
>
> -char * __devinit  pcibios_setup(char *str)
> +char *__init pcibios_setup(char *str)
>  {
>        if (!strcmp(str, "off")) {
>                pci_probe = 0;
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] x86/PCI: adjust section annotations

[Jan Beulich]

>>> On 21.06.12 at 02:06, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Mon, Jun 18, 2012 at 4:34 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> DMI tables referenced from __init code only can be __initconst, and as
>> a result the functions referenced from there can become __init.
>>
>> pcibios_setup() can be __init as being a command line parsing function
>> only.
>>
>> A few other variables can then also have their attributes adjusted.
> 
> This seems OK as far as it goes.
> 
> However, if you're going to make pcibios_setup() __init for x86, I'd
> really encourage you to make it consistent across all the other
> architectures.  And if you do *that*, I think it would be cool if you
> supplied a generic do-nothing "weak" version in the PCI core.  That
> would allow you to remove it altogether from alpha, ia64, microblaze,
> mips pmc-sierra, parisc, powerpc, sh, sparc, tile, and xtensa.

I'd prefer to leave this to the respective arch maintainers, the
patch here really was meant to be x86 specific (as its title says).

Jan

Re: [PATCH] x86/PCI: adjust section annotations

[Bjorn Helgaas]

On Thu, Jun 21, 2012 at 3:22 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 21.06.12 at 02:06, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Mon, Jun 18, 2012 at 4:34 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>> DMI tables referenced from __init code only can be __initconst, and as
>>> a result the functions referenced from there can become __init.
>>>
>>> pcibios_setup() can be __init as being a command line parsing function
>>> only.
>>>
>>> A few other variables can then also have their attributes adjusted.
>>
>> This seems OK as far as it goes.
>>
>> However, if you're going to make pcibios_setup() __init for x86, I'd
>> really encourage you to make it consistent across all the other
>> architectures.  And if you do *that*, I think it would be cool if you
>> supplied a generic do-nothing "weak" version in the PCI core.  That
>> would allow you to remove it altogether from alpha, ia64, microblaze,
>> mips pmc-sierra, parisc, powerpc, sh, sparc, tile, and xtensa.
>
> I'd prefer to leave this to the respective arch maintainers, the
> patch here really was meant to be x86 specific (as its title says).

OK.  Ingo, do you want to take this after all?

If I push stuff through my PCI tree, I try really hard to make PCI
overall more consistent, not less consistent, so I'm not very
interested in taking minor improvements to just one arch.

Bjorn

Re: [PATCH] x86/PCI: adjust section annotations

[H. Peter Anvin]

On 06/21/2012 07:19 AM, Bjorn Helgaas wrote:
> 
> OK.  Ingo, do you want to take this after all?
> 
> If I push stuff through my PCI tree, I try really hard to make PCI
> overall more consistent, not less consistent, so I'm not very
> interested in taking minor improvements to just one arch.
> 

I can take this, but the above sounds very close to a NAK to me...

	-hpa

Re: [PATCH] x86/PCI: adjust section annotations

[Jan Beulich]

>>> On 21.06.12 at 16:24, "H. Peter Anvin" <hpa@zytor.com> wrote:
> On 06/21/2012 07:19 AM, Bjorn Helgaas wrote:
>> 
>> OK.  Ingo, do you want to take this after all?
>> 
>> If I push stuff through my PCI tree, I try really hard to make PCI
>> overall more consistent, not less consistent, so I'm not very
>> interested in taking minor improvements to just one arch.
>> 
> 
> I can take this, but the above sounds very close to a NAK to me...

If that one hunk is causing so much grief, how about I resend
the whole patch with that one change dropped?

Jan

Re: [PATCH] x86/PCI: adjust section annotations

[Bjorn Helgaas]

On Thu, Jun 21, 2012 at 9:03 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 21.06.12 at 16:24, "H. Peter Anvin" <hpa@zytor.com> wrote:
>> On 06/21/2012 07:19 AM, Bjorn Helgaas wrote:
>>>
>>> OK.  Ingo, do you want to take this after all?
>>>
>>> If I push stuff through my PCI tree, I try really hard to make PCI
>>> overall more consistent, not less consistent, so I'm not very
>>> interested in taking minor improvements to just one arch.
>>>
>>
>> I can take this, but the above sounds very close to a NAK to me...
>
> If that one hunk is causing so much grief, how about I resend
> the whole patch with that one change dropped?

I'm sorry, I didn't handle this very well.  I'm afraid it sounded as
if I were faulting you for not doing more, but that's not it.
Suggesting the pcibios_setup() change was valuable because reviewing
it uncovered a bug and a nice cleanup opportunity, so thank you for
that!

My *intent*, in this as in other cases, is just to encourage folks to
step back from "solving my immediate problem" and take a broader view
that includes "does this same problem occur other places?" and "how
can I leverage this point solution to make things a bit cleaner for
everybody?"

In the time I've spent on this email thread, I could have done the
whole cleanup myself, but in the long term, I think it's important to
encourage an attitude of preserving and improving the commons.  Maybe
the fact that I'd rather deal with more general solutions will
encourage employers to support that kind of work, or at least give
people more ammunition when they management to support it.  That's my
hope, anyway :)

Bjorn

Re: [PATCH] x86/PCI: adjust section annotations

[Jesper Nilsson]

On Thu, Jun 21, 2012 at 02:06:55AM +0200, Bjorn Helgaas wrote:
> On Mon, Jun 18, 2012 at 4:34 AM, Jan Beulich <JBeulich@suse.com> wrote:
> > DMI tables referenced from __init code only can be __initconst, and as
> > a result the functions referenced from there can become __init.
> >
> > pcibios_setup() can be __init as being a command line parsing function
> > only.
> >
> > A few other variables can then also have their attributes adjusted.
> 
> This seems OK as far as it goes.
> 
> However, if you're going to make pcibios_setup() __init for x86, I'd
> really encourage you to make it consistent across all the other
> architectures.  And if you do *that*, I think it would be cool if you
> supplied a generic do-nothing "weak" version in the PCI core.  That
> would allow you to remove it altogether from alpha, ia64, microblaze,
> mips pmc-sierra, parisc, powerpc, sh, sparc, tile, and xtensa.
> 
> CRIS-folk: It would also fix what looks like a bug in cris, which
> implements pcibios_setup() such that pci_setup() doesn't even look for
> all the supposedly generic options.

Ok, thanks for the heads-up!

I'm going to take a look at the PCI code in the CRIS-port.

My current feeling is that the PCI support for CRIS probably
should be dropped, both due to rotted code, but also that the
hardware is hard to come by. I need to do some more digging around...

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com