[PATCH 0/6] xen/arm: Move in/out code to/from init section

[PATCH 0/6] xen/arm: Move in/out code to/from init section

[Julien Grall]

Hello,

This small series add/remove __init on different functions. This allow Xen to
free around 8Kb more of memory after it has finished to boot.

Xen size in memory before to free init section: 1029Kb
Freed memory init: 288Kb

Regards,

Julien Grall (6):
  arm/setup: Add missing __init to add_boot_module
  xen/arm: domain_build: Move all DOM0 building code in init section
  xen/arm: kernel: Move kernel loading code in init section
  xen/arm: device: Move device_type in init section
  xen/arm: platforms: Move init_time and specific_mapping in init
    section
  xen/arm: SMP: Move out of the init section the code to bring up a CPU

 xen/arch/arm/arm32/smpboot.c         |  4 +--
 xen/arch/arm/device.c                |  2 +-
 xen/arch/arm/domain_build.c          | 66 ++++++++++++++++++------------------
 xen/arch/arm/kernel.c                | 30 ++++++++--------
 xen/arch/arm/kernel.h                |  4 +--
 xen/arch/arm/platform.c              |  2 +-
 xen/arch/arm/platforms/exynos5.c     |  4 +--
 xen/arch/arm/platforms/omap5.c       |  4 +--
 xen/arch/arm/platforms/xgene-storm.c | 16 ++++-----
 xen/arch/arm/setup.c                 |  6 ++--
 xen/arch/arm/smpboot.c               |  2 +-
 xen/include/asm-arm/device.h         |  2 +-
 xen/include/asm-arm/setup.h          | 10 +++---
 xen/include/asm-arm/smp.h            |  2 +-
 14 files changed, 77 insertions(+), 77 deletions(-)

-- 
2.1.4

[PATCH 1/6] arm/setup: Add missing __init to add_boot_module

[Julien Grall]

add_boot_module is calling a function which lies in the init section.
Furthermore, it's only used during Xen boot.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/setup.c        | 6 +++---
 xen/include/asm-arm/setup.h | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index f49569d..5fc27ce0 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -185,9 +185,9 @@ static void dt_unreserved_regions(paddr_t s, paddr_t e,
     cb(s, e);
 }
 
-struct bootmodule *add_boot_module(bootmodule_kind kind,
-                                   paddr_t start, paddr_t size,
-                                   const char *cmdline)
+struct bootmodule __init *add_boot_module(bootmodule_kind kind,
+                                          paddr_t start, paddr_t size,
+                                          const char *cmdline)
 {
     struct bootmodules *mods = &bootinfo.modules;
     struct bootmodule *mod;
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index ba5a67d..ed2ba16 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -60,10 +60,10 @@ void discard_initial_modules(void);
 size_t __init boot_fdt_info(const void *fdt, paddr_t paddr);
 const char __init *boot_fdt_cmdline(const void *fdt);
 
-struct bootmodule *add_boot_module(bootmodule_kind kind,
-                                   paddr_t start, paddr_t size,
-                                   const char *cmdline);
-struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
+struct bootmodule __init *add_boot_module(bootmodule_kind kind,
+                                          paddr_t start, paddr_t size,
+                                          const char *cmdline);
+struct bootmodule __init *boot_module_find_by_kind(bootmodule_kind kind);
 const char * __init boot_module_kind_as_string(bootmodule_kind kind);
 
 #endif
-- 
2.1.4

Re: [PATCH 1/6] arm/setup: Add missing __init to add_boot_module

[Andrew Cooper]

On 16/01/15 16:20, Julien Grall wrote:
> add_boot_module is calling a function which lies in the init section.
> Furthermore, it's only used during Xen boot.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/setup.c        | 6 +++---
>  xen/include/asm-arm/setup.h | 8 ++++----
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index f49569d..5fc27ce0 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -185,9 +185,9 @@ static void dt_unreserved_regions(paddr_t s, paddr_t e,
>      cb(s, e);
>  }
>  
> -struct bootmodule *add_boot_module(bootmodule_kind kind,
> -                                   paddr_t start, paddr_t size,
> -                                   const char *cmdline)
> +struct bootmodule __init *add_boot_module(bootmodule_kind kind,

__init should be after the type, rather than spliced into the middle of it.

"struct bootmodule * __init add_boot_module(...)" should work.

However, only static functions should be annotated at the definition. 
non-static functions can get their annotation from the declaration alone.

> +                                          paddr_t start, paddr_t size,
> +                                          const char *cmdline)
>  {
>      struct bootmodules *mods = &bootinfo.modules;
>      struct bootmodule *mod;
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index ba5a67d..ed2ba16 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -60,10 +60,10 @@ void discard_initial_modules(void);
>  size_t __init boot_fdt_info(const void *fdt, paddr_t paddr);
>  const char __init *boot_fdt_cmdline(const void *fdt);
>  
> -struct bootmodule *add_boot_module(bootmodule_kind kind,
> -                                   paddr_t start, paddr_t size,
> -                                   const char *cmdline);
> -struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
> +struct bootmodule __init *add_boot_module(bootmodule_kind kind,
> +                                          paddr_t start, paddr_t size,
> +                                          const char *cmdline);
> +struct bootmodule __init *boot_module_find_by_kind(bootmodule_kind kind);
>  const char * __init boot_module_kind_as_string(bootmodule_kind kind);
>  
>  #endif

Re: [PATCH 1/6] arm/setup: Add missing __init to add_boot_module

[Julien Grall]

On 16/01/15 16:28, Andrew Cooper wrote:
> On 16/01/15 16:20, Julien Grall wrote:
>> add_boot_module is calling a function which lies in the init section.
>> Furthermore, it's only used during Xen boot.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>>  xen/arch/arm/setup.c        | 6 +++---
>>  xen/include/asm-arm/setup.h | 8 ++++----
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index f49569d..5fc27ce0 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -185,9 +185,9 @@ static void dt_unreserved_regions(paddr_t s, paddr_t e,
>>      cb(s, e);
>>  }
>>  
>> -struct bootmodule *add_boot_module(bootmodule_kind kind,
>> -                                   paddr_t start, paddr_t size,
>> -                                   const char *cmdline)
>> +struct bootmodule __init *add_boot_module(bootmodule_kind kind,
> 
> __init should be after the type, rather than spliced into the middle of it.

Ok.

> "struct bootmodule * __init add_boot_module(...)" should work.
> 
> However, only static functions should be annotated at the definition. 
> non-static functions can get their annotation from the declaration alone.

It's useful for documentation purpose... You directly know this function
should be only used during Xen boot time.

Regards,

-- 
Julien Grall

[PATCH 2/6] xen/arm: domain_build: Move all DOM0 building code in init section

[Julien Grall]

The code to build DOM0 is only used during Xen boot.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/domain_build.c | 66 ++++++++++++++++++++++-----------------------
 xen/include/asm-arm/setup.h |  2 +-
 2 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c2dcb49..d30df08 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -76,7 +76,7 @@ struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
     return alloc_vcpu(dom0, 0, 0);
 }
 
-static unsigned int get_11_allocation_size(paddr_t size)
+static unsigned int __init get_11_allocation_size(paddr_t size)
 {
     /*
      * get_order_from_bytes returns the order greater than or equal to
@@ -94,10 +94,10 @@ static unsigned int get_11_allocation_size(paddr_t size)
  * Returns false if the memory would be below bank 0 or we have run
  * out of banks. In this case it will free the pages.
  */
-static bool_t insert_11_bank(struct domain *d,
-                             struct kernel_info *kinfo,
-                             struct page_info *pg,
-                             unsigned int order)
+static bool_t __init insert_11_bank(struct domain *d,
+                                    struct kernel_info *kinfo,
+                                    struct page_info *pg,
+                                    unsigned int order)
 {
     int res, i;
     paddr_t spfn;
@@ -240,7 +240,8 @@ fail:
  * (as described above) we allow higher allocations and continue until
  * that runs out (or we have allocated sufficient dom0 memory).
  */
-static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo)
+static void __init allocate_memory_11(struct domain *d,
+                                      struct kernel_info *kinfo)
 {
     const unsigned int min_low_order =
         get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128)));
@@ -348,7 +349,7 @@ static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo)
     }
 }
 
-static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
+static void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
 {
 
     struct dt_device_node *memory = NULL;
@@ -398,8 +399,8 @@ static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
     }
 }
 
-static int write_properties(struct domain *d, struct kernel_info *kinfo,
-                            const struct dt_device_node *node)
+static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
+                                   const struct dt_device_node *node)
 {
     const char *bootargs = NULL;
     const struct dt_property *prop;
@@ -504,8 +505,8 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
 
 typedef __be32 gic_interrupt_t[3];
 
-static void set_interrupt_ppi(gic_interrupt_t interrupt, unsigned int irq,
-                              unsigned int cpumask, unsigned int level)
+static void __init set_interrupt_ppi(gic_interrupt_t interrupt, unsigned int irq,
+                                     unsigned int cpumask, unsigned int level)
 {
     __be32 *cells = interrupt;
 
@@ -523,8 +524,8 @@ static void set_interrupt_ppi(gic_interrupt_t interrupt, unsigned int irq,
  *  "interrupts": contains the list of interrupts
  *  "interrupt-parent": link to the GIC
  */
-static int fdt_property_interrupts(void *fdt, gic_interrupt_t *intr,
-                                   unsigned num_irq)
+static int __init fdt_property_interrupts(void *fdt, gic_interrupt_t *intr,
+                                          unsigned num_irq)
 {
     int res;
 
@@ -538,10 +539,9 @@ static int fdt_property_interrupts(void *fdt, gic_interrupt_t *intr,
     return res;
 }
 
-static int make_memory_node(const struct domain *d,
-                            void *fdt,
-                            const struct dt_device_node *parent,
-                            const struct kernel_info *kinfo)
+static int __init make_memory_node(const struct domain *d, void *fdt,
+                                   const struct dt_device_node *parent,
+                                   const struct kernel_info *kinfo)
 {
     int res, i;
     int reg_size = dt_n_addr_cells(parent) + dt_n_size_cells(parent);
@@ -581,8 +581,8 @@ static int make_memory_node(const struct domain *d,
     return res;
 }
 
-static int make_hypervisor_node(struct domain *d,
-                                void *fdt, const struct dt_device_node *parent)
+static int __init make_hypervisor_node(struct domain *d, void *fdt,
+                                       const struct dt_device_node *parent)
 {
     const char compat[] =
         "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
@@ -646,7 +646,7 @@ static int make_hypervisor_node(struct domain *d,
     return res;
 }
 
-static int make_psci_node(void *fdt, const struct dt_device_node *parent)
+static int __init make_psci_node(void *fdt, const struct dt_device_node *parent)
 {
     int res;
     const char compat[] =
@@ -681,8 +681,8 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent)
     return res;
 }
 
-static int make_cpus_node(const struct domain *d, void *fdt,
-                          const struct dt_device_node *parent)
+static int __init make_cpus_node(const struct domain *d, void *fdt,
+                                 const struct dt_device_node *parent)
 {
     int res;
     const struct dt_device_node *cpus = dt_find_node_by_path("/cpus");
@@ -785,8 +785,8 @@ static int make_cpus_node(const struct domain *d, void *fdt,
     return res;
 }
 
-static int make_gic_node(const struct domain *d, void *fdt,
-                         const struct dt_device_node *node)
+static int __init make_gic_node(const struct domain *d, void *fdt,
+                                const struct dt_device_node *node)
 {
     const struct dt_device_node *gic = dt_interrupt_controller;
     int res = 0;
@@ -834,8 +834,8 @@ static int make_gic_node(const struct domain *d, void *fdt,
     return res;
 }
 
-static int make_timer_node(const struct domain *d, void *fdt,
-                           const struct dt_device_node *node)
+static int __init make_timer_node(const struct domain *d, void *fdt,
+                                  const struct dt_device_node *node)
 {
     static const struct dt_device_match timer_ids[] __initconst =
     {
@@ -910,7 +910,7 @@ static int make_timer_node(const struct domain *d, void *fdt,
 }
 
 /* Map the device in the domain */
-static int map_device(struct domain *d, struct dt_device_node *dev)
+static int __init map_device(struct domain *d, struct dt_device_node *dev)
 {
     unsigned int nirq;
     unsigned int naddr;
@@ -1020,8 +1020,8 @@ static int map_device(struct domain *d, struct dt_device_node *dev)
     return 0;
 }
 
-static int handle_node(struct domain *d, struct kernel_info *kinfo,
-                       struct dt_device_node *node)
+static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
+                              struct dt_device_node *node)
 {
     static const struct dt_device_match skip_matches[] __initconst =
     {
@@ -1155,7 +1155,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
     return res;
 }
 
-static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
+static int __init prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 {
     const void *fdt;
     int new_size;
@@ -1192,7 +1192,7 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
     return -EINVAL;
 }
 
-static void dtb_load(struct kernel_info *kinfo)
+static void __init dtb_load(struct kernel_info *kinfo)
 {
     void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;
     unsigned long left;
@@ -1207,7 +1207,7 @@ static void dtb_load(struct kernel_info *kinfo)
     xfree(kinfo->fdt);
 }
 
-static void initrd_load(struct kernel_info *kinfo)
+static void __init initrd_load(struct kernel_info *kinfo)
 {
     const struct bootmodule *mod = kinfo->initrd_bootmodule;
     paddr_t load_addr = kinfo->initrd_paddr;
@@ -1271,7 +1271,7 @@ static void initrd_load(struct kernel_info *kinfo)
     }
 }
 
-int construct_dom0(struct domain *d)
+int __init construct_dom0(struct domain *d)
 {
     struct kernel_info kinfo = {};
     struct vcpu *saved_current;
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index ed2ba16..113655d 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -53,7 +53,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
 
 void arch_get_xen_caps(xen_capabilities_info_t *info);
 
-int construct_dom0(struct domain *d);
+int __init construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);
 
-- 
2.1.4

[PATCH 3/6] xen/arm: kernel: Move kernel loading code in init section

[Julien Grall]

The code to load the kernel is only used when Xen builds dom0. It
happens during the boot.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/kernel.c | 30 +++++++++++++++---------------
 xen/arch/arm/kernel.h |  4 ++--
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 209c3dd..5b72c77 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -67,8 +67,8 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
     clear_fixmap(FIXMAP_MISC);
 }
 
-static void place_modules(struct kernel_info *info,
-                          paddr_t kernbase, paddr_t kernend)
+static void __init place_modules(struct kernel_info *info,
+                                 paddr_t kernbase, paddr_t kernend)
 {
     /* Align DTB and initrd size to 2Mb. Linux only requires 4 byte alignment */
     const struct bootmodule *mod = info->initrd_bootmodule;
@@ -121,7 +121,7 @@ static void place_modules(struct kernel_info *info,
     info->initrd_paddr = info->dtb_paddr + dtb_len;
 }
 
-static paddr_t kernel_zimage_place(struct kernel_info *info)
+static paddr_t __init kernel_zimage_place(struct kernel_info *info)
 {
     paddr_t load_addr;
 
@@ -153,7 +153,7 @@ static paddr_t kernel_zimage_place(struct kernel_info *info)
     return load_addr;
 }
 
-static void kernel_zimage_load(struct kernel_info *info)
+static void __init kernel_zimage_load(struct kernel_info *info)
 {
     paddr_t load_addr = kernel_zimage_place(info);
     paddr_t paddr = info->zimage.kernel_addr;
@@ -200,8 +200,8 @@ static void kernel_zimage_load(struct kernel_info *info)
 /*
  * Check if the image is a uImage and setup kernel_info
  */
-static int kernel_uimage_probe(struct kernel_info *info,
-                                 paddr_t addr, paddr_t size)
+static int __init kernel_uimage_probe(struct kernel_info *info,
+                                      paddr_t addr, paddr_t size)
 {
     struct {
         __be32 magic;   /* Image Header Magic Number */
@@ -261,8 +261,8 @@ static int kernel_uimage_probe(struct kernel_info *info,
 /*
  * Check if the image is a 64-bit Image.
  */
-static int kernel_zimage64_probe(struct kernel_info *info,
-                                 paddr_t addr, paddr_t size)
+static int __init kernel_zimage64_probe(struct kernel_info *info,
+                                        paddr_t addr, paddr_t size)
 {
     /* linux/Documentation/arm64/booting.txt */
     struct {
@@ -315,8 +315,8 @@ static int kernel_zimage64_probe(struct kernel_info *info,
 /*
  * Check if the image is a 32-bit zImage and setup kernel_info
  */
-static int kernel_zimage32_probe(struct kernel_info *info,
-                                 paddr_t addr, paddr_t size)
+static int __init kernel_zimage32_probe(struct kernel_info *info,
+                                        paddr_t addr, paddr_t size)
 {
     uint32_t zimage[ZIMAGE32_HEADER_LEN/4];
     uint32_t start, end;
@@ -364,7 +364,7 @@ static int kernel_zimage32_probe(struct kernel_info *info,
     return 0;
 }
 
-static void kernel_elf_load(struct kernel_info *info)
+static void __init kernel_elf_load(struct kernel_info *info)
 {
     /*
      * TODO: can the ELF header be used to find the physical address
@@ -387,8 +387,8 @@ static void kernel_elf_load(struct kernel_info *info)
     free_xenheap_pages(info->elf.kernel_img, info->elf.kernel_order);
 }
 
-static int kernel_elf_probe(struct kernel_info *info,
-                            paddr_t addr, paddr_t size)
+static int __init kernel_elf_probe(struct kernel_info *info,
+                                   paddr_t addr, paddr_t size)
 {
     int rc;
 
@@ -439,7 +439,7 @@ err:
     return rc;
 }
 
-int kernel_probe(struct kernel_info *info)
+int __init kernel_probe(struct kernel_info *info)
 {
     struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
     int rc;
@@ -476,7 +476,7 @@ int kernel_probe(struct kernel_info *info)
     return rc;
 }
 
-void kernel_load(struct kernel_info *info)
+void __init kernel_load(struct kernel_info *info)
 {
     info->load(info);
 }
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index 0050dfb..512b573 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -56,7 +56,7 @@ struct kernel_info {
  *  ->type
  *  ->load hook, and sets loader specific variables ->{zimage,elf}
  */
-int kernel_probe(struct kernel_info *info);
+int __init kernel_probe(struct kernel_info *info);
 
 /*
  * Loads the kernel into guest RAM.
@@ -70,7 +70,7 @@ int kernel_probe(struct kernel_info *info);
  *  ->dtb_paddr
  *  ->initrd_paddr
  */
-void kernel_load(struct kernel_info *info);
+void __init kernel_load(struct kernel_info *info);
 
 #endif /* #ifdef __ARCH_ARM_KERNEL_H__ */
 
-- 
2.1.4

Re: [PATCH 3/6] xen/arm: kernel: Move kernel loading code in init section

[Vitaly Kuznetsov]

Julien Grall <julien.grall@linaro.org> writes:

> The code to load the kernel is only used when Xen builds dom0. It
> happens during the boot.

I suppose we don't have dom0 kexec for ARM now or this code is not
(won't be) required?

-- 
  Vitaly

Re: [PATCH 3/6] xen/arm: kernel: Move kernel loading code in init section

[Julien Grall]

Hello,

On 16/01/15 17:33, Vitaly Kuznetsov wrote:
> Julien Grall <julien.grall@linaro.org> writes:
> 
>> The code to load the kernel is only used when Xen builds dom0. It
>> happens during the boot.
> 
> I suppose we don't have dom0 kexec for ARM now or this code is not
> (won't be) required?

We don't have support for DOM0 Kexec and this code is only able to load
kernel from the physical RAM (not from a guest memory).

Regards,

-- 
Julien Grall

Re: [PATCH 3/6] xen/arm: kernel: Move kernel loading code in init section

[Ian Campbell]

On Fri, 2015-01-16 at 17:49 +0000, Julien Grall wrote:
> Hello,
> 
> On 16/01/15 17:33, Vitaly Kuznetsov wrote:
> > Julien Grall <julien.grall@linaro.org> writes:
> > 
> >> The code to load the kernel is only used when Xen builds dom0. It
> >> happens during the boot.
> > 
> > I suppose we don't have dom0 kexec for ARM now or this code is not
> > (won't be) required?
> 
> We don't have support for DOM0 Kexec and this code is only able to load
> kernel from the physical RAM (not from a guest memory).

I don't think you would want to rerun any of this sort of thing again
for kexec anyway, it is somewhat like a bootloader, which kexec doesn't
rely on AFAIK.

Ian.

[PATCH 4/6] xen/arm: device: Move device_type in init section

[Julien Grall]

The function is calling device_is_compatible which lies in init section
and is only called during Xen boot.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/device.c        | 2 +-
 xen/include/asm-arm/device.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
index 59e94c0..3e5c458 100644
--- a/xen/arch/arm/device.c
+++ b/xen/arch/arm/device.c
@@ -67,7 +67,7 @@ int __init device_init(struct dt_device_node *dev, enum device_type type,
     return -EBADF;
 }
 
-enum device_type device_get_type(const struct dt_device_node *dev)
+enum device_type __init device_get_type(const struct dt_device_node *dev)
 {
     const struct device_desc *desc;
 
diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
index 74a80c6..e23673c 100644
--- a/xen/include/asm-arm/device.h
+++ b/xen/include/asm-arm/device.h
@@ -41,7 +41,7 @@ int __init device_init(struct dt_device_node *dev, enum device_type type,
  *
  * Return the device type on success or DEVICE_ANY on failure
  */
-enum device_type device_get_type(const struct dt_device_node *dev);
+enum device_type __init device_get_type(const struct dt_device_node *dev);
 
 #define DT_DEVICE_START(_name, _namestr, _type)                     \
 static const struct device_desc __dev_desc_##_name __used           \
-- 
2.1.4

[PATCH 5/6] xen/arm: platforms: Move init_time and specific_mapping in init section

[Julien Grall]

The callbacks init_time and specific_mapping are respectively used
during timer initialization and DOM0 building. These 2 taks only
happen during Xen boot.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/platforms/exynos5.c     |  4 ++--
 xen/arch/arm/platforms/omap5.c       |  4 ++--
 xen/arch/arm/platforms/xgene-storm.c | 16 ++++++++--------
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
index 79e3a5f..e90b659 100644
--- a/xen/arch/arm/platforms/exynos5.c
+++ b/xen/arch/arm/platforms/exynos5.c
@@ -37,7 +37,7 @@ static bool_t secure_firmware;
 
 #define SMC_CMD_CPU1BOOT            (-4)
 
-static int exynos5_init_time(void)
+static int __init exynos5_init_time(void)
 {
     uint32_t reg;
     void __iomem *mct;
@@ -80,7 +80,7 @@ static int exynos5_init_time(void)
 }
 
 /* Additional mappings for dom0 (Not in the DTS) */
-static int exynos5250_specific_mapping(struct domain *d)
+static int __init exynos5250_specific_mapping(struct domain *d)
 {
     /* Map the chip ID */
     map_mmio_regions(d, paddr_to_pfn(EXYNOS5_PA_CHIPID), 1,
diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
index 9d6e504..7086c0b 100644
--- a/xen/arch/arm/platforms/omap5.c
+++ b/xen/arch/arm/platforms/omap5.c
@@ -45,7 +45,7 @@ static uint16_t num_den[8][2] = {
  * clocks, adjusting the increment per clock in hardware accordingly to
  * maintain a constant count rate.
  */
-static int omap5_init_time(void)
+static int __init omap5_init_time(void)
 {
     void __iomem *ckgen_prm_base;
     void __iomem *rt_ct_base;
@@ -99,7 +99,7 @@ static int omap5_init_time(void)
 }
 
 /* Additional mappings for dom0 (not in the DTS) */
-static int omap5_specific_mapping(struct domain *d)
+static int __init omap5_specific_mapping(struct domain *d)
 {
     /* Map the PRM module */
     map_mmio_regions(d, paddr_to_pfn(OMAP5_PRM_BASE), 2,
diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
index 0b3492d..8c650c3 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -40,7 +40,7 @@ static uint32_t xgene_storm_quirks(void)
     return PLATFORM_QUIRK_GIC_64K_STRIDE|PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI;
 }
 
-static int map_one_mmio(struct domain *d, const char *what,
+static int __init map_one_mmio(struct domain *d, const char *what,
                          unsigned long start, unsigned long end)
 {
     int ret;
@@ -54,8 +54,8 @@ static int map_one_mmio(struct domain *d, const char *what,
     return ret;
 }
 
-static int map_one_spi(struct domain *d, const char *what,
-                       unsigned int spi, unsigned int type)
+static int __init map_one_spi(struct domain *d, const char *what,
+                              unsigned int spi, unsigned int type)
 {
     unsigned int irq;
     int ret;
@@ -79,10 +79,10 @@ static int map_one_spi(struct domain *d, const char *what,
 }
 
 /* Creates MMIO mappings base..end as well as 4 SPIs from the given base. */
-static int xgene_storm_pcie_specific_mapping(struct domain *d,
-                                             const struct dt_device_node *node,
-                                             paddr_t base, paddr_t end,
-                                             int base_spi)
+static int __init xgene_storm_pcie_specific_mapping(struct domain *d,
+                                                    const struct dt_device_node *node,
+                                                    paddr_t base, paddr_t end,
+                                                    int base_spi)
 {
     int ret;
 
@@ -121,7 +121,7 @@ err:
  * "interrupt-map" properties to domain 0). Instead for now map the
  * necessary resources manually.
  */
-static int xgene_storm_specific_mapping(struct domain *d)
+static int __init xgene_storm_specific_mapping(struct domain *d)
 {
     struct dt_device_node *node = NULL;
     int ret;
-- 
2.1.4

[PATCH 6/6] xen/arm: SMP: Move out of the init section the code to bring up a CPU

[Julien Grall]

When CPU hotplug will be supported, it will require to bring up a CPU.

Signed-off-by: Julien Grall <julien.grall@linaro.org>

---

I'm not sure if we should flag these functions with __cpuinit. The
define doesn't do anything.
---
 xen/arch/arm/arm32/smpboot.c | 4 ++--
 xen/arch/arm/platform.c      | 2 +-
 xen/arch/arm/smpboot.c       | 2 +-
 xen/include/asm-arm/smp.h    | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/arm32/smpboot.c b/xen/arch/arm/arm32/smpboot.c
index 2a77f29..226d6fd 100644
--- a/xen/arch/arm/arm32/smpboot.c
+++ b/xen/arch/arm/arm32/smpboot.c
@@ -8,7 +8,7 @@ int __init arch_smp_init(void)
     return platform_smp_init();
 }
 
-int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
+int arch_cpu_init(int cpu, struct dt_device_node *dn)
 {
     /* Not needed on ARM32, as there is no relevant information in
      * the CPU device tree node for ARMv7 CPUs.
@@ -16,7 +16,7 @@ int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
     return 0;
 }
 
-int __init arch_cpu_up(int cpu)
+int arch_cpu_up(int cpu)
 {
     return platform_cpu_up(cpu);
 }
diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
index cb4cda8..0368679 100644
--- a/xen/arch/arm/platform.c
+++ b/xen/arch/arm/platform.c
@@ -108,7 +108,7 @@ int __init platform_specific_mapping(struct domain *d)
 }
 
 #ifdef CONFIG_ARM_32
-int __init platform_cpu_up(int cpu)
+int platform_cpu_up(int cpu)
 {
     if ( psci_ver )
         return call_psci_cpu_on(cpu);
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index a96cda2..a228623 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -343,7 +343,7 @@ void stop_cpu(void)
         wfi();
 }
 
-int __init cpu_up_send_sgi(int cpu)
+int cpu_up_send_sgi(int cpu)
 {
     /* We don't know the GIC ID of the CPU until it has woken up, so just
      * signal everyone and rely on our own smp_up_cpu gate to ensure only
diff --git a/xen/include/asm-arm/smp.h b/xen/include/asm-arm/smp.h
index 91b1e52..181aead 100644
--- a/xen/include/asm-arm/smp.h
+++ b/xen/include/asm-arm/smp.h
@@ -17,7 +17,7 @@ DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask);
 
 extern void noreturn stop_cpu(void);
 
-extern int arch_smp_init(void);
+extern int __init arch_smp_init(void);
 extern int arch_cpu_init(int cpu, struct dt_device_node *dn);
 extern int arch_cpu_up(int cpu);
 
-- 
2.1.4

Re: [PATCH 0/6] xen/arm: Move in/out code to/from init section

[Julien Grall]

Hello,

Ping? Any more review for this version of this series?

Regards,

On 16/01/15 16:20, Julien Grall wrote:
> Hello,
> 
> This small series add/remove __init on different functions. This allow Xen to
> free around 8Kb more of memory after it has finished to boot.
> 
> Xen size in memory before to free init section: 1029Kb
> Freed memory init: 288Kb
> 
> Regards,
> 
> Julien Grall (6):
>   arm/setup: Add missing __init to add_boot_module
>   xen/arm: domain_build: Move all DOM0 building code in init section
>   xen/arm: kernel: Move kernel loading code in init section
>   xen/arm: device: Move device_type in init section
>   xen/arm: platforms: Move init_time and specific_mapping in init
>     section
>   xen/arm: SMP: Move out of the init section the code to bring up a CPU
> 
>  xen/arch/arm/arm32/smpboot.c         |  4 +--
>  xen/arch/arm/device.c                |  2 +-
>  xen/arch/arm/domain_build.c          | 66 ++++++++++++++++++------------------
>  xen/arch/arm/kernel.c                | 30 ++++++++--------
>  xen/arch/arm/kernel.h                |  4 +--
>  xen/arch/arm/platform.c              |  2 +-
>  xen/arch/arm/platforms/exynos5.c     |  4 +--
>  xen/arch/arm/platforms/omap5.c       |  4 +--
>  xen/arch/arm/platforms/xgene-storm.c | 16 ++++-----
>  xen/arch/arm/setup.c                 |  6 ++--
>  xen/arch/arm/smpboot.c               |  2 +-
>  xen/include/asm-arm/device.h         |  2 +-
>  xen/include/asm-arm/setup.h          | 10 +++---
>  xen/include/asm-arm/smp.h            |  2 +-
>  14 files changed, 77 insertions(+), 77 deletions(-)
> 


-- 
Julien Grall

Re: [PATCH 0/6] xen/arm: Move in/out code to/from init section

[Ian Campbell]

On Thu, 2015-01-29 at 18:32 +0000, Julien Grall wrote:
> Hello,
> 
> Ping? Any more review for this version of this series?

I was awaiting a version with the declarations in the right places as
pointed out by Andy (including his point about definition vs. prototype
I'm afraid, which is the style we use).

> 
> Regards,
> 
> On 16/01/15 16:20, Julien Grall wrote:
> > Hello,
> > 
> > This small series add/remove __init on different functions. This allow Xen to
> > free around 8Kb more of memory after it has finished to boot.
> > 
> > Xen size in memory before to free init section: 1029Kb
> > Freed memory init: 288Kb
> > 
> > Regards,
> > 
> > Julien Grall (6):
> >   arm/setup: Add missing __init to add_boot_module
> >   xen/arm: domain_build: Move all DOM0 building code in init section
> >   xen/arm: kernel: Move kernel loading code in init section
> >   xen/arm: device: Move device_type in init section
> >   xen/arm: platforms: Move init_time and specific_mapping in init
> >     section
> >   xen/arm: SMP: Move out of the init section the code to bring up a CPU
> > 
> >  xen/arch/arm/arm32/smpboot.c         |  4 +--
> >  xen/arch/arm/device.c                |  2 +-
> >  xen/arch/arm/domain_build.c          | 66 ++++++++++++++++++------------------
> >  xen/arch/arm/kernel.c                | 30 ++++++++--------
> >  xen/arch/arm/kernel.h                |  4 +--
> >  xen/arch/arm/platform.c              |  2 +-
> >  xen/arch/arm/platforms/exynos5.c     |  4 +--
> >  xen/arch/arm/platforms/omap5.c       |  4 +--
> >  xen/arch/arm/platforms/xgene-storm.c | 16 ++++-----
> >  xen/arch/arm/setup.c                 |  6 ++--
> >  xen/arch/arm/smpboot.c               |  2 +-
> >  xen/include/asm-arm/device.h         |  2 +-
> >  xen/include/asm-arm/setup.h          | 10 +++---
> >  xen/include/asm-arm/smp.h            |  2 +-
> >  14 files changed, 77 insertions(+), 77 deletions(-)
> > 
> 
> 

Re: [PATCH 0/6] xen/arm: Move in/out code to/from init section

[Julien Grall]

Hi Ian,

On 30/01/15 11:30, Ian Campbell wrote:
> On Thu, 2015-01-29 at 18:32 +0000, Julien Grall wrote:
>> Hello,
>>
>> Ping? Any more review for this version of this series?
> 
> I was awaiting a version with the declarations in the right places as
> pointed out by Andy (including his point about definition vs. prototype
> I'm afraid, which is the style we use).

I'm not convince about your last point. We have many places (if not all
on ARM) where __init is used in the header.

Some of them was even added by you ;). So I would prefer if we keep
them, it's more readable. FWIW Linux does it too.

Regards,

-- 
Julien Grall

Re: [PATCH 0/6] xen/arm: Move in/out code to/from init section

[Ian Campbell]

On Fri, 2015-01-30 at 11:33 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 30/01/15 11:30, Ian Campbell wrote:
> > On Thu, 2015-01-29 at 18:32 +0000, Julien Grall wrote:
> >> Hello,
> >>
> >> Ping? Any more review for this version of this series?
> > 
> > I was awaiting a version with the declarations in the right places as
> > pointed out by Andy (including his point about definition vs. prototype
> > I'm afraid, which is the style we use).
> 
> I'm not convince about your last point. We have many places (if not all
> on ARM) where __init is used in the header.

Those are mistakes, it seems.

> Some of them was even added by you ;). So I would prefer if we keep
> them, it's more readable. FWIW Linux does it too.

I personally don't particularly care where these things go, but other
hypervisor maintainers seem to and we should aim for the code base to be
consistent where possible.

If we want to change this coding style then we should do that directly
and explicitly, not through the backdoor by just ignoring the policy.

Ian.

Re: [PATCH 0/6] xen/arm: Move in/out code to/from init section

[Jan Beulich]

>>> On 02.02.15 at 11:58, <Ian.Campbell@citrix.com> wrote:
> On Fri, 2015-01-30 at 11:33 +0000, Julien Grall wrote:
>> On 30/01/15 11:30, Ian Campbell wrote:
>> > On Thu, 2015-01-29 at 18:32 +0000, Julien Grall wrote:
>> >> Hello,
>> >>
>> >> Ping? Any more review for this version of this series?
>> > 
>> > I was awaiting a version with the declarations in the right places as
>> > pointed out by Andy (including his point about definition vs. prototype
>> > I'm afraid, which is the style we use).
>> 
>> I'm not convince about your last point. We have many places (if not all
>> on ARM) where __init is used in the header.
> 
> Those are mistakes, it seems.
> 
>> Some of them was even added by you ;). So I would prefer if we keep
>> them, it's more readable. FWIW Linux does it too.
> 
> I personally don't particularly care where these things go, but other
> hypervisor maintainers seem to and we should aim for the code base to be
> consistent where possible.
> 
> If we want to change this coding style then we should do that directly
> and explicitly, not through the backdoor by just ignoring the policy.

And I think we should strive to keep pointless redundancy down:
There's no point in repeating attributes - either they belong on the
declaration (when producer and consumers care), or they belong
on the definition (when only the producer cares). Repeating them
on the definition when the declaration already has them only results
in needless churn when such an attribute later gets changed/
removed, and having producer-relevant attributes on declarations
means needless extra work by the compiler (however little that may
be - it adds up).

Jan

Re: [PATCH 0/6] xen/arm: Move in/out code to/from init section

[Julien Grall]

On 02/02/15 11:15, Jan Beulich wrote:
>>>> On 02.02.15 at 11:58, <Ian.Campbell@citrix.com> wrote:
>> On Fri, 2015-01-30 at 11:33 +0000, Julien Grall wrote:
>>> On 30/01/15 11:30, Ian Campbell wrote:
>>>> On Thu, 2015-01-29 at 18:32 +0000, Julien Grall wrote:
>>>>> Hello,
>>>>>
>>>>> Ping? Any more review for this version of this series?
>>>>
>>>> I was awaiting a version with the declarations in the right places as
>>>> pointed out by Andy (including his point about definition vs. prototype
>>>> I'm afraid, which is the style we use).
>>>
>>> I'm not convince about your last point. We have many places (if not all
>>> on ARM) where __init is used in the header.
>>
>> Those are mistakes, it seems.
>>
>>> Some of them was even added by you ;). So I would prefer if we keep
>>> them, it's more readable. FWIW Linux does it too.
>>
>> I personally don't particularly care where these things go, but other
>> hypervisor maintainers seem to and we should aim for the code base to be
>> consistent where possible.
>>
>> If we want to change this coding style then we should do that directly
>> and explicitly, not through the backdoor by just ignoring the policy.
> 
> And I think we should strive to keep pointless redundancy down:
> There's no point in repeating attributes - either they belong on the
> declaration (when producer and consumers care), or they belong
> on the definition (when only the producer cares). Repeating them
> on the definition when the declaration already has them only results
> in needless churn when such an attribute later gets changed/
> removed, and having producer-relevant attributes on declarations
> means needless extra work by the compiler (however little that may
> be - it adds up).

I think we should allow the annotation in both place. When a developer
is looking for the prototype of the function, it will likely look at the
headers and not the implementation itself.

This may lead to call an init function in code running after the boot. I
don't think we can catch it easily during review.

Regards,

-- 
Julien Grall

Re: [PATCH 0/6] xen/arm: Move in/out code to/from init section

[Jan Beulich]

>>> On 02.02.15 at 13:52, <julien.grall@linaro.org> wrote:
> On 02/02/15 11:15, Jan Beulich wrote:
>>>>> On 02.02.15 at 11:58, <Ian.Campbell@citrix.com> wrote:
>>> On Fri, 2015-01-30 at 11:33 +0000, Julien Grall wrote:
>>>> On 30/01/15 11:30, Ian Campbell wrote:
>>>>> On Thu, 2015-01-29 at 18:32 +0000, Julien Grall wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Ping? Any more review for this version of this series?
>>>>>
>>>>> I was awaiting a version with the declarations in the right places as
>>>>> pointed out by Andy (including his point about definition vs. prototype
>>>>> I'm afraid, which is the style we use).
>>>>
>>>> I'm not convince about your last point. We have many places (if not all
>>>> on ARM) where __init is used in the header.
>>>
>>> Those are mistakes, it seems.
>>>
>>>> Some of them was even added by you ;). So I would prefer if we keep
>>>> them, it's more readable. FWIW Linux does it too.
>>>
>>> I personally don't particularly care where these things go, but other
>>> hypervisor maintainers seem to and we should aim for the code base to be
>>> consistent where possible.
>>>
>>> If we want to change this coding style then we should do that directly
>>> and explicitly, not through the backdoor by just ignoring the policy.
>> 
>> And I think we should strive to keep pointless redundancy down:
>> There's no point in repeating attributes - either they belong on the
>> declaration (when producer and consumers care), or they belong
>> on the definition (when only the producer cares). Repeating them
>> on the definition when the declaration already has them only results
>> in needless churn when such an attribute later gets changed/
>> removed, and having producer-relevant attributes on declarations
>> means needless extra work by the compiler (however little that may
>> be - it adds up).
> 
> I think we should allow the annotation in both place. When a developer
> is looking for the prototype of the function, it will likely look at the
> headers and not the implementation itself.
> 
> This may lead to call an init function in code running after the boot. I
> don't think we can catch it easily during review.

This example in particular makes clear that looking at just the
declaration is insufficient in certain cases. Whoever is looking
for specific properties of a functions needs to know which ones
(s)he cares about, and then look in the appropriate place. Unless
a rule gets put in place overriding my personal opinion on this, I'm
not going to ack or otherwise accept needless code duplication.
If the ARM maintainers feel differently, so be it.

Jan

Re: [PATCH 0/6] xen/arm: Move in/out code to/from init section

[Julien Grall]

On 02/02/15 13:12, Jan Beulich wrote:
> This example in particular makes clear that looking at just the
> declaration is insufficient in certain cases.Whoever is looking
> for specific properties of a functions needs to know which ones
> (s)he cares about, and then look in the appropriate place.

__init is common attribute that should be looking by anyone calling a
function. You can't guess about it.

> Unless a rule gets put in place overriding my personal opinion on
this, I'm
> not going to ack or otherwise accept needless code duplication.

It may be needless for the compiler but not for some of the developers.

> If the ARM maintainers feel differently, so be it.

If you look a the ARM headers (asm-arm) we use __init on the prototype.
But x86 and common code is not using it.

The point of this discussion is there is no written rules... It happened
because an x86 maintainer shout about inconsistency on the ARM headers
on my first patch.

If this is going to be the standard on every headers, this should be
written somewhere and not letting the developer guess how the maintainer
will feel about it.

Regards,

-- 
Julien Grall

Re: [PATCH 0/6] xen/arm: Move in/out code to/from init section

[Julien Grall]

On 02/02/15 10:58, Ian Campbell wrote:
> On Fri, 2015-01-30 at 11:33 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 30/01/15 11:30, Ian Campbell wrote:
>>> On Thu, 2015-01-29 at 18:32 +0000, Julien Grall wrote:
>>>> Hello,
>>>>
>>>> Ping? Any more review for this version of this series?
>>>
>>> I was awaiting a version with the declarations in the right places as
>>> pointed out by Andy (including his point about definition vs. prototype
>>> I'm afraid, which is the style we use).
>>
>> I'm not convince about your last point. We have many places (if not all
>> on ARM) where __init is used in the header.
> 
> Those are mistakes, it seems.
> 
>> Some of them was even added by you ;). So I would prefer if we keep
>> them, it's more readable. FWIW Linux does it too.
> 
> I personally don't particularly care where these things go, but other
> hypervisor maintainers seem to and we should aim for the code base to be
> consistent where possible.
> 
> If we want to change this coding style then we should do that directly
> and explicitly, not through the backdoor by just ignoring the policy.

Which policy are you talking about? AFAICT, this seems to be an
unwritten rule. It is let to the appreciation of the person who review
the patches...

Actually, it doesn't seem to have a consensus between maintainers. ARM
people seems to allow attributes on prototype and not x86 people.

Regards,

-- 
Julien Grall

Re: [PATCH 0/6] xen/arm: Move in/out code to/from init section

[Ian Campbell]

On Mon, 2015-02-02 at 12:48 +0000, Julien Grall wrote:
> On 02/02/15 10:58, Ian Campbell wrote:
> > On Fri, 2015-01-30 at 11:33 +0000, Julien Grall wrote:
> >> Hi Ian,
> >>
> >> On 30/01/15 11:30, Ian Campbell wrote:
> >>> On Thu, 2015-01-29 at 18:32 +0000, Julien Grall wrote:
> >>>> Hello,
> >>>>
> >>>> Ping? Any more review for this version of this series?
> >>>
> >>> I was awaiting a version with the declarations in the right places as
> >>> pointed out by Andy (including his point about definition vs. prototype
> >>> I'm afraid, which is the style we use).
> >>
> >> I'm not convince about your last point. We have many places (if not all
> >> on ARM) where __init is used in the header.
> > 
> > Those are mistakes, it seems.
> > 
> >> Some of them was even added by you ;). So I would prefer if we keep
> >> them, it's more readable. FWIW Linux does it too.
> > 
> > I personally don't particularly care where these things go, but other
> > hypervisor maintainers seem to and we should aim for the code base to be
> > consistent where possible.
> > 
> > If we want to change this coding style then we should do that directly
> > and explicitly, not through the backdoor by just ignoring the policy.
> 
> Which policy are you talking about? AFAICT, this seems to be an
> unwritten rule. It is let to the appreciation of the person who review
> the patches...

Regardless of whether it is currently written down it is something which
Jan has been consistent about requesting for ages.

We should probably add it to some sort of coding style document though.

> Actually, it doesn't seem to have a consensus between maintainers. ARM
> people seems to allow attributes on prototype and not x86 people.

This ARM person cares more about consistency with common code and other
arches than his own personal feelings on the matter, which are "meh".

Ian.