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

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

[Guenter Roeck]

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

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

[Ard Biesheuvel]

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
>

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

[Ard Biesheuvel]

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
>>

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

[Guenter Roeck]

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
>>>
>

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

[Guenter Roeck]

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