diff mbox series

[v2,2/3] sam460ex: Remove FDT_PPC dependency from KConfig

Message ID a55cd539835084bd322695c483273dc4b144ccbc.1609413115.git.balaton@eik.bme.hu (mailing list archive)
State New, archived
Headers show
Series Clean up sam460ex irq mapping | expand

Commit Message

BALATON Zoltan Dec. 25, 2020, 8:12 p.m. UTC
Dependency on FDT_PPC was added in commit b0048f76095
("hw/ppc/Kconfig: Only select FDT helper for machines using it") but
it does not seem to be really necessary so remove it again.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
v2: Do not remove PPC405, reworded commit message

 hw/ppc/Kconfig | 1 -
 1 file changed, 1 deletion(-)

Comments

Philippe Mathieu-Daudé Dec. 31, 2020, 4:08 p.m. UTC | #1
On 12/31/20 12:11 PM, BALATON Zoltan via wrote:
> Dependency on FDT_PPC was added in commit b0048f76095
> ("hw/ppc/Kconfig: Only select FDT helper for machines using it") but
> it does not seem to be really necessary so remove it again.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> v2: Do not remove PPC405, reworded commit message
> 
>  hw/ppc/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 8548f42b0d..f1e1be208e 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -64,7 +64,6 @@ config SAM460EX
>      select SMBUS_EEPROM
>      select USB_EHCI_SYSBUS
>      select USB_OHCI
> -    select FDT_PPC
>  
>  config PREP
>      bool
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
BALATON Zoltan Jan. 4, 2021, 1:51 a.m. UTC | #2
On Thu, 31 Dec 2020, BALATON Zoltan via wrote:
> Dependency on FDT_PPC was added in commit b0048f76095
> ("hw/ppc/Kconfig: Only select FDT helper for machines using it") but
> it does not seem to be really necessary so remove it again.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> v2: Do not remove PPC405, reworded commit message
>
> hw/ppc/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 8548f42b0d..f1e1be208e 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -64,7 +64,6 @@ config SAM460EX
>     select SMBUS_EEPROM
>     select USB_EHCI_SYSBUS
>     select USB_OHCI
> -    select FDT_PPC
>
> config PREP
>     bool

Something is still not right with this, I've noticed that a few other 
boards also have this option selected but they don't need fdt.o that's 
gated by this option in meson.build. That fdt.o is only needed by PSERIES 
and POWERNV but removing FDT_PPC from other boards I get:

../hw/ppc/sam460ex.c:43:10: fatal error: libfdt.h: No such file or directory
  #include <libfdt.h>

so apparently this switch also pulls in the necessary CPPFLAGS or libfdt 
dependency. Is there a separate switch for that or we can only get it with 
fdt.o. Not a big deal just not trivial why we need an option that at first 
sight select a source file which we don't need. I think I'll drop this 
patch for now when resending the series.

Regards,
BALATON Zoltan
Peter Maydell Jan. 4, 2021, 11:48 a.m. UTC | #3
On Mon, 4 Jan 2021 at 01:51, BALATON Zoltan <balaton@eik.bme.hu> wrote:
> Something is still not right with this, I've noticed that a few other
> boards also have this option selected but they don't need fdt.o that's
> gated by this option in meson.build. That fdt.o is only needed by PSERIES
> and POWERNV but removing FDT_PPC from other boards I get:
>
> ../hw/ppc/sam460ex.c:43:10: fatal error: libfdt.h: No such file or directory
>   #include <libfdt.h>
>
> so apparently this switch also pulls in the necessary CPPFLAGS or libfdt
> dependency. Is there a separate switch for that or we can only get it with
> fdt.o. Not a big deal just not trivial why we need an option that at first
> sight select a source file which we don't need. I think I'll drop this
> patch for now when resending the series.

This happens because hw/ppc/meson.build does:

ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: [files(
  'fdt.c',
), fdt])

ie if CONFIG_FDT_PPC is set then both
 * add fdt.c to the build
 * add the 'fdt' dependency (which brings in the CFLAGS and
   LDFLAGS necessary for libfdt).

So yes, at the moment for PPC there are only two options:
 * board doesn't use FDT at all
 * board uses FDT and gets hw/ppc/fdt.c linked in even if it
   doesn't use the functions there

Since fdt.c currently has just one not very large function (which
is only even present if TARGET_PPC64) this doesn't seem like a big
deal, but in theory the "need libfdt" and "need fdt.c" parts could
be decoupled.

thanks
-- PMM
Paolo Bonzini Jan. 5, 2021, 12:50 p.m. UTC | #4
On 04/01/21 02:51, BALATON Zoltan wrote:
> On Thu, 31 Dec 2020, BALATON Zoltan via wrote:
>> Dependency on FDT_PPC was added in commit b0048f76095
>> ("hw/ppc/Kconfig: Only select FDT helper for machines using it") but
>> it does not seem to be really necessary so remove it again.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> v2: Do not remove PPC405, reworded commit message
>>
>> hw/ppc/Kconfig | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
>> index 8548f42b0d..f1e1be208e 100644
>> --- a/hw/ppc/Kconfig
>> +++ b/hw/ppc/Kconfig
>> @@ -64,7 +64,6 @@ config SAM460EX
>>     select SMBUS_EEPROM
>>     select USB_EHCI_SYSBUS
>>     select USB_OHCI
>> -    select FDT_PPC
>>
>> config PREP
>>     bool
> 
> Something is still not right with this, I've noticed that a few other 
> boards also have this option selected but they don't need fdt.o that's 
> gated by this option in meson.build. That fdt.o is only needed by 
> PSERIES and POWERNV but removing FDT_PPC from other boards I get:
> 
> ../hw/ppc/sam460ex.c:43:10: fatal error: libfdt.h: No such file or 
> directory
>   #include <libfdt.h>
> 
> so apparently this switch also pulls in the necessary CPPFLAGS or libfdt 
> dependency. Is there a separate switch for that or we can only get it 
> with fdt.o. Not a big deal just not trivial why we need an option that 
> at first sight select a source file which we don't need. I think I'll 
> drop this patch for now when resending the series.

You can always do

ppc_ss.add(when: 'CONFIG_SAM460EX', if_true: [files('sam460ex.c'), fdt])

if you want to get rid of the flag.

Paolo
diff mbox series

Patch

diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 8548f42b0d..f1e1be208e 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -64,7 +64,6 @@  config SAM460EX
     select SMBUS_EEPROM
     select USB_EHCI_SYSBUS
     select USB_OHCI
-    select FDT_PPC
 
 config PREP
     bool