Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1305622 > unrolled thread

[PATCH -next] efi: Do not include asm/efi.h if not needed

Started byGuenter Roeck <linux@roeck-us.net>
First post2016-01-10 16:40 +0100
Last post2016-01-10 20:50 +0100
Articles 5 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH -next] efi: Do not include asm/efi.h if not needed Guenter Roeck <linux@roeck-us.net> - 2016-01-10 16:40 +0100
    Re: [PATCH -next] efi: Do not include asm/efi.h if not needed Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-01-10 18:10 +0100
      Re: [PATCH -next] efi: Do not include asm/efi.h if not needed Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-01-10 18:40 +0100
        Re: [PATCH -next] efi: Do not include asm/efi.h if not needed Guenter Roeck <linux@roeck-us.net> - 2016-01-10 20:50 +0100
          Re: [PATCH -next] efi: Do not include asm/efi.h if not needed Guenter Roeck <linux@roeck-us.net> - 2016-01-10 20:50 +0100

#1305622 — [PATCH -next] efi: Do not include asm/efi.h if not needed

FromGuenter Roeck <linux@roeck-us.net>
Date2016-01-10 16:40 +0100
Subject[PATCH -next] efi: Do not include asm/efi.h if not needed
Message-ID<qPqg2-62t-3@gated-at.bofh.it>
Commit f7d924894265 ("arm64/efi: refactor EFI init and runtime code
for reuse by 32-bit ARM") adds an include of asm/efi.h to efi.c.
This causes a build failure for ia64, where asm/efi.h does not exist.

drivers/firmware/efi/efi.c:28:21: fatal error:
	asm/efi.h: No such file or directory

asm/efi.h does not define or declare anything used by efi.c, thus
including it should not be needed.

Fixes: f7d924894265 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM")
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/firmware/efi/efi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index cffa89b3317b..027ca212179f 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -25,8 +25,6 @@
 #include <linux/io.h>
 #include <linux/platform_device.h>
 
-#include <asm/efi.h>
-
 struct efi __read_mostly efi = {
 	.mps			= EFI_INVALID_TABLE_ADDR,
 	.acpi			= EFI_INVALID_TABLE_ADDR,
-- 
2.1.4

[toc] | [next] | [standalone]


#1305636

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2016-01-10 18:10 +0100
Message-ID<qPrF7-75A-1@gated-at.bofh.it>
In reply to#1305622
On 10 January 2016 at 16:35, Guenter Roeck <linux@roeck-us.net> wrote:
> Commit f7d924894265 ("arm64/efi: refactor EFI init and runtime code
> for reuse by 32-bit ARM") adds an include of asm/efi.h to efi.c.
> This causes a build failure for ia64, where asm/efi.h does not exist.
>
> drivers/firmware/efi/efi.c:28:21: fatal error:
>         asm/efi.h: No such file or directory
>
> asm/efi.h does not define or declare anything used by efi.c, thus
> including it should not be needed.
>
> Fixes: f7d924894265 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM")
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

This fixes ia64 but breaks ARM

drivers/firmware/efi/efi.c: In function ‘efi_mem_desc_lookup’:
drivers/firmware/efi/efi.c:291:3: error: implicit declaration of
function ‘early_memremap’ [-Werror=implicit-function-declaration]
drivers/firmware/efi/efi.c:291:6: warning: assignment makes pointer
from integer without a cast [enabled by default]
drivers/firmware/efi/efi.c:301:4: error: implicit declaration of
function ‘early_memunmap’ [-Werror=implicit-function-declaration]
drivers/firmware/efi/efi.c: In function ‘efi_config_parse_tables’:
drivers/firmware/efi/efi.c:404:7: warning: assignment makes pointer
from integer without a cast [enabled by default]
drivers/firmware/efi/efi.c: In function ‘efi_config_init’:
drivers/firmware/efi/efi.c:433:16: warning: assignment makes pointer
from integer without a cast [enabled by default]

> ---
>  drivers/firmware/efi/efi.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index cffa89b3317b..027ca212179f 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -25,8 +25,6 @@
>  #include <linux/io.h>
>  #include <linux/platform_device.h>
>
> -#include <asm/efi.h>
> -
>  struct efi __read_mostly efi = {
>         .mps                    = EFI_INVALID_TABLE_ADDR,
>         .acpi                   = EFI_INVALID_TABLE_ADDR,
> --
> 2.1.4
>

[toc] | [prev] | [next] | [standalone]


#1305642

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2016-01-10 18:40 +0100
Message-ID<qPs8a-7fC-29@gated-at.bofh.it>
In reply to#1305636
On 10 January 2016 at 18:02, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 10 January 2016 at 16:35, Guenter Roeck <linux@roeck-us.net> wrote:
>> Commit f7d924894265 ("arm64/efi: refactor EFI init and runtime code
>> for reuse by 32-bit ARM") adds an include of asm/efi.h to efi.c.
>> This causes a build failure for ia64, where asm/efi.h does not exist.
>>
>> drivers/firmware/efi/efi.c:28:21: fatal error:
>>         asm/efi.h: No such file or directory
>>
>> asm/efi.h does not define or declare anything used by efi.c, thus
>> including it should not be needed.
>>
>> Fixes: f7d924894265 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM")
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
> This fixes ia64 but breaks ARM
>
> drivers/firmware/efi/efi.c: In function ‘efi_mem_desc_lookup’:
> drivers/firmware/efi/efi.c:291:3: error: implicit declaration of
> function ‘early_memremap’ [-Werror=implicit-function-declaration]
> drivers/firmware/efi/efi.c:291:6: warning: assignment makes pointer
> from integer without a cast [enabled by default]
> drivers/firmware/efi/efi.c:301:4: error: implicit declaration of
> function ‘early_memunmap’ [-Werror=implicit-function-declaration]
> drivers/firmware/efi/efi.c: In function ‘efi_config_parse_tables’:
> drivers/firmware/efi/efi.c:404:7: warning: assignment makes pointer
> from integer without a cast [enabled by default]
> drivers/firmware/efi/efi.c: In function ‘efi_config_init’:
> drivers/firmware/efi/efi.c:433:16: warning: assignment makes pointer
> from integer without a cast [enabled by default]
>

This means efi.c should include <asm/early_ioremap.h> but that does
not exist on ia64 either.
Perhaps add a #ifdef CONFIG_IA64?

>> ---
>>  drivers/firmware/efi/efi.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
>> index cffa89b3317b..027ca212179f 100644
>> --- a/drivers/firmware/efi/efi.c
>> +++ b/drivers/firmware/efi/efi.c
>> @@ -25,8 +25,6 @@
>>  #include <linux/io.h>
>>  #include <linux/platform_device.h>
>>
>> -#include <asm/efi.h>
>> -
>>  struct efi __read_mostly efi = {
>>         .mps                    = EFI_INVALID_TABLE_ADDR,
>>         .acpi                   = EFI_INVALID_TABLE_ADDR,
>> --
>> 2.1.4
>>

[toc] | [prev] | [next] | [standalone]


#1305664

FromGuenter Roeck <linux@roeck-us.net>
Date2016-01-10 20:50 +0100
Message-ID<qPu9Y-5i-9@gated-at.bofh.it>
In reply to#1305642
On 01/10/2016 09:31 AM, Ard Biesheuvel wrote:
> On 10 January 2016 at 18:02, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 10 January 2016 at 16:35, Guenter Roeck <linux@roeck-us.net> wrote:
>>> Commit f7d924894265 ("arm64/efi: refactor EFI init and runtime code
>>> for reuse by 32-bit ARM") adds an include of asm/efi.h to efi.c.
>>> This causes a build failure for ia64, where asm/efi.h does not exist.
>>>
>>> drivers/firmware/efi/efi.c:28:21: fatal error:
>>>          asm/efi.h: No such file or directory
>>>
>>> asm/efi.h does not define or declare anything used by efi.c, thus
>>> including it should not be needed.
>>>
>>> Fixes: f7d924894265 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM")
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Cc: Will Deacon <will.deacon@arm.com>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>
>> This fixes ia64 but breaks ARM
>>
>> drivers/firmware/efi/efi.c: In function ‘efi_mem_desc_lookup’:
>> drivers/firmware/efi/efi.c:291:3: error: implicit declaration of
>> function ‘early_memremap’ [-Werror=implicit-function-declaration]
>> drivers/firmware/efi/efi.c:291:6: warning: assignment makes pointer
>> from integer without a cast [enabled by default]
>> drivers/firmware/efi/efi.c:301:4: error: implicit declaration of
>> function ‘early_memunmap’ [-Werror=implicit-function-declaration]
>> drivers/firmware/efi/efi.c: In function ‘efi_config_parse_tables’:
>> drivers/firmware/efi/efi.c:404:7: warning: assignment makes pointer
>> from integer without a cast [enabled by default]
>> drivers/firmware/efi/efi.c: In function ‘efi_config_init’:
>> drivers/firmware/efi/efi.c:433:16: warning: assignment makes pointer
>> from integer without a cast [enabled by default]
>>
>
> This means efi.c should include <asm/early_ioremap.h> but that does
> not exist on ia64 either.
> Perhaps add a #ifdef CONFIG_IA64?
>
#include <asm/io.h>

I'll check if that works for all architectures.

Guenter

>>> ---
>>>   drivers/firmware/efi/efi.c | 2 --
>>>   1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
>>> index cffa89b3317b..027ca212179f 100644
>>> --- a/drivers/firmware/efi/efi.c
>>> +++ b/drivers/firmware/efi/efi.c
>>> @@ -25,8 +25,6 @@
>>>   #include <linux/io.h>
>>>   #include <linux/platform_device.h>
>>>
>>> -#include <asm/efi.h>
>>> -
>>>   struct efi __read_mostly efi = {
>>>          .mps                    = EFI_INVALID_TABLE_ADDR,
>>>          .acpi                   = EFI_INVALID_TABLE_ADDR,
>>> --
>>> 2.1.4
>>>
>

[toc] | [prev] | [next] | [standalone]


#1305665

FromGuenter Roeck <linux@roeck-us.net>
Date2016-01-10 20:50 +0100
Message-ID<qPu9Y-5i-7@gated-at.bofh.it>
In reply to#1305664
On 01/10/2016 11:39 AM, Guenter Roeck wrote:
> On 01/10/2016 09:31 AM, Ard Biesheuvel wrote:
>> On 10 January 2016 at 18:02, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>> On 10 January 2016 at 16:35, Guenter Roeck <linux@roeck-us.net> wrote:
>>>> Commit f7d924894265 ("arm64/efi: refactor EFI init and runtime code
>>>> for reuse by 32-bit ARM") adds an include of asm/efi.h to efi.c.
>>>> This causes a build failure for ia64, where asm/efi.h does not exist.
>>>>
>>>> drivers/firmware/efi/efi.c:28:21: fatal error:
>>>>          asm/efi.h: No such file or directory
>>>>
>>>> asm/efi.h does not define or declare anything used by efi.c, thus
>>>> including it should not be needed.
>>>>
>>>> Fixes: f7d924894265 ("arm64/efi: refactor EFI init and runtime code for reuse by 32-bit ARM")
>>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> Cc: Will Deacon <will.deacon@arm.com>
>>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>
>>> This fixes ia64 but breaks ARM
>>>
>>> drivers/firmware/efi/efi.c: In function ‘efi_mem_desc_lookup’:
>>> drivers/firmware/efi/efi.c:291:3: error: implicit declaration of
>>> function ‘early_memremap’ [-Werror=implicit-function-declaration]
>>> drivers/firmware/efi/efi.c:291:6: warning: assignment makes pointer
>>> from integer without a cast [enabled by default]
>>> drivers/firmware/efi/efi.c:301:4: error: implicit declaration of
>>> function ‘early_memunmap’ [-Werror=implicit-function-declaration]
>>> drivers/firmware/efi/efi.c: In function ‘efi_config_parse_tables’:
>>> drivers/firmware/efi/efi.c:404:7: warning: assignment makes pointer
>>> from integer without a cast [enabled by default]
>>> drivers/firmware/efi/efi.c: In function ‘efi_config_init’:
>>> drivers/firmware/efi/efi.c:433:16: warning: assignment makes pointer
>>> from integer without a cast [enabled by default]
>>>
>>
>> This means efi.c should include <asm/early_ioremap.h> but that does
>> not exist on ia64 either.
>> Perhaps add a #ifdef CONFIG_IA64?
>>
> #include <asm/io.h>
>
> I'll check if that works for all architectures.
>

... and it doesn't.

Ok, I give up on that one. I don't know what a proper fix would be.
My argument would be that the arm changes broke the build for ia64,
are therefore the culprit and should either be reverted or dropped.
But I'll leave that to the ia64 and efi maintainers to sort out.

Thanks,
Guenter

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web