Re: [PATCH 01/12] vt82c686: Add APM and ACPI dependencies for VT82C686

[BALATON Zoltan]

Hello,

On Mon, 28 Dec 2020, Huacai Chen wrote:
> Hi, BALATON
>
> On Sun, Dec 27, 2020 at 9:21 AM BALATON Zoltan <balaton@eik.bme.hu> wrote:
> > Compiling vt82c686.c fails without APM and ACPI_PM functions. Add
> > dependency on these in Kconfig to fix this.
> >
> > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > ---
> >  hw/isa/Kconfig | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
> > index c7f07854f7..2ca2593ee6 100644
> > --- a/hw/isa/Kconfig
> > +++ b/hw/isa/Kconfig
> > @@ -47,6 +47,8 @@ config VT82C686
> >      select ACPI_SMBUS
> >      select SERIAL_ISA
> >      select FDC
> > +    select APM
> > +    select ACPI_X86
> I feel a bit uncomfortable with ACPI_X86 in the MIPS code, can we just
> select ACPI? And if that is not enough, can we select more options?

This patch is not new, I've tried submitting it before but got rejeceted 
for similar reason:

https://lists.nongnu.org/archive/html/qemu-devel/2019-03/msg03428.html

Then Philippe said he had a better alternative but it's still not fixed in 
master so this patch is needed and you likely already depend on X86 
without knowing as something is pulling these in for MIPS. This can be 
reproduced e,g, by adding this device to PPC as:

diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index d235a096c6..90b53d40c2 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -64,6 +64,7 @@ config SAM460EX
     select SMBUS_EEPROM
     select USB_EHCI_SYSBUS
     select USB_OHCI
+    select VT82C686

 config PREP
     bool

then compiling --target-list=ppc-softmmu
Even after:

diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index c7f07854f7..75986671b9 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -47,6 +47,8 @@ config VT82C686
     select ACPI_SMBUS
     select SERIAL_ISA
     select FDC
+    select APM
+    select ACPI

 config SMC37C669
     bool

I get:

[] Linking target qemu-system-ppc
FAILED: qemu-system-ppc
ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `vt82c686b_pm_realize':
hw/isa/vt82c686.c:378: undefined reference to `acpi_pm_tmr_init'
ld: hw/isa/vt82c686.c:379: undefined reference to `acpi_pm1_evt_init'
ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `pm_update_sci':
hw/isa/vt82c686.c:192: undefined reference to `acpi_pm1_evt_get_sts'
ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `vt82c686b_pm_realize':
hw/isa/vt82c686.c:380: undefined reference to `acpi_pm1_cnt_init'
ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `pm_update_sci':
hw/isa/vt82c686.c:200: undefined reference to `acpi_pm_tmr_update'
collect2: error: ld returned 1 exit status

So my patch just makes existing dependencies explicit and allows this to 
build but I'm OK with any other fix you propose that fixes the above case 
as that's how I'll try to use this in the future. (I did look at this when 
first found it and concluded that I could not make a better fix than 
depending on ACPI_X86 here. I forgot the details but it was way more work 
than I want to take up for this so please propose a better fix if you 
can't accept this patch.)

Maybe Philippe remembers some more.

Regards,
BALATON Zoltan