Re: [PATCH v5 3/5] hw/isa/vt82c686: Reuse acpi_update_sci()

[Bernhard Beschow <shentey@gmail.com>]

Am 29. Oktober 2023 00:07:00 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Sat, 28 Oct 2023, Bernhard Beschow wrote:
>> acpi_update_sci() covers everything pm_update_sci() does. It implements common
>> ACPI funtionality in a generic fashion. Note that it agnostic to any
>> Frankenstein usage of the general purpose event registers in other device
>> models. It just implements a generic mechanism which can be wired to arbitrary
>> functionality.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/isa/vt82c686.c | 20 ++------------------
>> 1 file changed, 2 insertions(+), 18 deletions(-)
>> 
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index 60ca781e03..7b44ad9485 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -145,26 +145,10 @@ static const MemoryRegionOps pm_io_ops = {
>>     },
>> };
>> 
>> -static void pm_update_sci(ViaPMState *s)
>> -{
>> -    int sci_level, pmsts;
>> -
>> -    pmsts = acpi_pm1_evt_get_sts(&s->ar);
>> -    sci_level = (((pmsts & s->ar.pm1.evt.en) &
>> -                  (ACPI_BITMASK_RT_CLOCK_ENABLE |
>> -                   ACPI_BITMASK_POWER_BUTTON_ENABLE |
>> -                   ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
>> -                   ACPI_BITMASK_TIMER_ENABLE)) != 0);
>> -    qemu_set_irq(s->sci_irq, sci_level);
>> -    /* schedule a timer interruption if needed */
>> -    acpi_pm_tmr_update(&s->ar, (s->ar.pm1.evt.en & ACPI_BITMASK_TIMER_ENABLE) &&
>> -                       !(pmsts & ACPI_BITMASK_TIMER_STATUS));
>> -}
>> -
>> static void pm_tmr_timer(ACPIREGS *ar)
>> {
>>     ViaPMState *s = container_of(ar, ViaPMState, ar);
>> -    pm_update_sci(s);
>> +    acpi_update_sci(&s->ar, s->sci_irq);
>
>To avoid needing an interrupt here maybe you could modify acpi_update_sci() to allow NULL irq then call via_isa_set_irq here so the interrupt routing can be done within the ISA bridge.

Given the interesting behavior of the amigaone boot loader I'd respin this series with the last two patches only.

Best regards,
Bernhard

>
>Regards,
>BALATON Zoltan
>
>> }
>> 
>> static void via_pm_reset(DeviceState *d)
>> @@ -182,7 +166,7 @@ static void via_pm_reset(DeviceState *d)
>>     acpi_pm1_cnt_reset(&s->ar);
>>     acpi_pm_tmr_reset(&s->ar);
>>     acpi_gpe_reset(&s->ar);
>> -    pm_update_sci(s);
>> +    acpi_update_sci(&s->ar, s->sci_irq);
>> 
>>     pm_io_space_update(s);
>>     smb_io_space_update(s);
>>