Re: [PATCH v3 2/5] efi: Export arch_tables variable
On Wed, 26 Mar, at 03:08:00PM, Daniel Kiper wrote:
>
> Hmmm... I am not sure which approach is better. I saw that
> in many places declarations have annotations. Could you
> point me some docs which states (and explains) that this
> is wrong idea.
Note I've had the following patch from Joe queued up for weeks. It's in
the tip tree and will be sent to Linus during the merge window.
commit 9b7d204982cf
Author: Joe Perches
Date: Fri Jan 3 16:08:48 2014 -0800
x86/efi: Style neatening
Coalesce formats and remove spaces before tabs.
Move __initdata after the variable declaration.
Signed-off-by: Joe Perches
Signed-off-by: Matt Fleming
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index dc1dd6d09dec..876dc5b9e4a3 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -67,7 +67,7 @@ struct efi_memory_map memmap;
static struct efi efi_phys __initdata;
static efi_system_table_t efi_systab __initdata;
-static __initdata efi_config_table_type_t arch_tables[] = {
+static efi_config_table_type_t arch_tables[] __initdata = {
#ifdef CONFIG_X86_UV
{UV_SYSTEM_TABLE_GUID, "UVsystab", &efi.uv_systab},
#endif
@@ -76,7 +76,7 @@ static __initdata efi_config_table_type_t arch_tables[] = {
u64 efi_setup; /* efi setup_data physical address */
-static bool __initdata disable_runtime = false;
+static bool disable_runtime __initdata = false;
static int __init setup_noefi(char *arg)
{
disable_runtime = true;
@@ -263,9 +263,9 @@ static efi_status_t __init phys_efi_get_time(efi_time_t *tm,
int efi_set_rtc_mmss(const struct timespec *now)
{
unsigned long nowtime = now->tv_sec;
- efi_status_tstatus;
- efi_time_t eft;
- efi_time_cap_t cap;
+ efi_status_tstatus;
+ efi_time_t eft;
+ efi_time_cap_t cap;
struct rtc_time tm;
status = efi.get_time(&eft, &cap);
@@ -283,9 +283,8 @@ int efi_set_rtc_mmss(const struct timespec *now)
eft.second = tm.tm_sec;
eft.nanosecond = 0;
} else {
- printk(KERN_ERR
- "%s: Invalid EFI RTC value: write of %lx to EFI RTC
failed\n",
- __FUNCTION__, nowtime);
+ pr_err("%s: Invalid EFI RTC value: write of %lx to EFI RTC
failed\n",
+ __func__, nowtime);
return -1;
}
@@ -401,8 +400,7 @@ static void __init print_efi_memmap(void)
p < memmap.map_end;
p += memmap.desc_size, i++) {
md = p;
- pr_info("mem%02u: type=%u, attr=0x%llx, "
- "range=[0x%016llx-0x%016llx) (%lluMB)\n",
+ pr_info("mem%02u: type=%u, attr=0x%llx,
range=[0x%016llx-0x%016llx) (%lluMB)\n",
i, md->type, md->attribute, md->phys_addr,
md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT),
(md->num_pages >> (20 - EFI_PAGE_SHIFT)));
@@ -434,9 +432,8 @@ void __init efi_reserve_boot_services(void)
memblock_is_region_reserved(start, size)) {
/* Could not reserve, skip it */
md->num_pages = 0;
- memblock_dbg("Could not reserve boot range "
- "[0x%010llx-0x%010llx]\n",
- start, start+size-1);
+ memblock_dbg("Could not reserve boot range
[0x%010llx-0x%010llx]\n",
+start, start+size-1);
} else
memblock_reserve(start, size);
}
@@ -572,8 +569,7 @@ static int __init efi_systab_init(void *phys)
return -EINVAL;
}
if ((efi.systab->hdr.revision >> 16) == 0)
- pr_err("Warning: System table version "
- "%d.%02d, expected 1.00 or greater!\n",
+ pr_err("Warning: System table version %d.%02d, expected 1.00 or
greater!\n",
efi.systab->hdr.revision >> 16,
efi.systab->hdr.revision & 0x);
--
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/5] efi: Export arch_tables variable
>>> On 26.03.14 at 15:08, wrote:
> On Wed, Mar 26, 2014 at 01:21:19PM +, Jan Beulich wrote:
>> >>> On 25.03.14 at 21:57, wrote:
>> > Export arch_tables variable. Xen init function calls efi_config_init()
>> > which takes it as an argument.
>> >
>> > Additionally, put __initdata in place suggested by include/linux/init.h.
>>
>> Which isn't necessarily the most appropriate place.
>
> Why? If comments in include/linux/init.h are not valid they should be
> changed.
Because they can't go there uniformly: While on function declarations
you can put them there, on function definitions they need to come
before the function name. And placing attributes between type and
name does - iirc - work consistently for everything.
>> > --- a/include/linux/efi.h
>> > +++ b/include/linux/efi.h
>> > @@ -583,6 +583,8 @@ extern struct efi {
>> >struct efi_memory_map *memmap;
>> > } efi;
>> >
>> > +extern efi_config_table_type_t arch_tables[] __initdata;
>>
>> And section placement annotations are bogus on declarations.
>
> Hmmm... I am not sure which approach is better. I saw that
> in many places declarations have annotations. Could you
> point me some docs which states (and explains) that this
> is wrong idea.
Just use common sense: Attributes that are of concern to the
caller should go on the declaration. Attributes that only affect
code generation for the function/object in question should go on
the definition.
Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/5] efi: Export arch_tables variable
On Wed, Mar 26, 2014 at 01:21:19PM +, Jan Beulich wrote:
> >>> On 25.03.14 at 21:57, wrote:
> > Export arch_tables variable. Xen init function calls efi_config_init()
> > which takes it as an argument.
> >
> > Additionally, put __initdata in place suggested by include/linux/init.h.
>
> Which isn't necessarily the most appropriate place.
Why? If comments in include/linux/init.h are not valid they should be changed.
> > --- a/arch/x86/platform/efi/efi.c
> > +++ b/arch/x86/platform/efi/efi.c
> > @@ -70,7 +70,7 @@ static efi_system_table_t efi_systab __initdata;
> >
> > unsigned long x86_efi_facility;
> >
> > -static __initdata efi_config_table_type_t arch_tables[] = {
> > +efi_config_table_type_t arch_tables[] __initdata = {
>
> efi_config_table_type_t __initdata arch_tables[] = {
>
> would be what I'd recommend.
>
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -583,6 +583,8 @@ extern struct efi {
> > struct efi_memory_map *memmap;
> > } efi;
> >
> > +extern efi_config_table_type_t arch_tables[] __initdata;
>
> And section placement annotations are bogus on declarations.
Hmmm... I am not sure which approach is better. I saw that
in many places declarations have annotations. Could you
point me some docs which states (and explains) that this
is wrong idea.
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/5] efi: Export arch_tables variable
>>> On 25.03.14 at 21:57, wrote:
> Export arch_tables variable. Xen init function calls efi_config_init()
> which takes it as an argument.
>
> Additionally, put __initdata in place suggested by include/linux/init.h.
Which isn't necessarily the most appropriate place.
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -70,7 +70,7 @@ static efi_system_table_t efi_systab __initdata;
>
> unsigned long x86_efi_facility;
>
> -static __initdata efi_config_table_type_t arch_tables[] = {
> +efi_config_table_type_t arch_tables[] __initdata = {
efi_config_table_type_t __initdata arch_tables[] = {
would be what I'd recommend.
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -583,6 +583,8 @@ extern struct efi {
> struct efi_memory_map *memmap;
> } efi;
>
> +extern efi_config_table_type_t arch_tables[] __initdata;
And section placement annotations are bogus on declarations.
Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/5] efi: Export arch_tables variable
Export arch_tables variable. Xen init function calls efi_config_init()
which takes it as an argument.
Additionally, put __initdata in place suggested by include/linux/init.h.
Signed-off-by: Daniel Kiper
---
arch/x86/platform/efi/efi.c |2 +-
include/linux/efi.h |2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e4af217..26651c6 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -70,7 +70,7 @@ static efi_system_table_t efi_systab __initdata;
unsigned long x86_efi_facility;
-static __initdata efi_config_table_type_t arch_tables[] = {
+efi_config_table_type_t arch_tables[] __initdata = {
#ifdef CONFIG_X86_UV
{UV_SYSTEM_TABLE_GUID, "UVsystab", &efi.uv_systab},
#endif
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 67ba1a0..f00ef14 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -583,6 +583,8 @@ extern struct efi {
struct efi_memory_map *memmap;
} efi;
+extern efi_config_table_type_t arch_tables[] __initdata;
+
static inline int
efi_guidcmp (efi_guid_t left, efi_guid_t right)
{
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
