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

[Bernhard Beschow]

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.

Why? This function works well as is for other device models.

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