Am 5. Oktober 2023 11:26:56 UTC schrieb BALATON Zoltan <bala...@eik.bme.hu>:
>On Thu, 5 Oct 2023, Bernhard Beschow wrote:
>> Am 4. Oktober 2023 12:28:58 UTC schrieb BALATON Zoltan <bala...@eik.bme.hu>:
>>> On Wed, 4 Oct 2023, Bernhard Beschow wrote:
>>>> According to the datasheet, SCI interrupts of the power management function
>>>> aren't routed through the PCI pins but rather directly to the integrated
>>>> PIC.
>>>> The routing is configurable through the ACPI interrupt select register at
>>>> offset
>>>> 0x42 in the PCI configuration space of the ISA function.
>>>>
>>>> Signed-off-by: Bernhard Beschow <shen...@gmail.com>
>>>>
>>>> ---
>>>>
>>>> v2:
>>>> * Introduce named constants for the ACPI interrupt select register at
>>>> offset
>>>> 0x42 (Phil)
>>>> ---
>>>> hw/isa/vt82c686.c | 47 +++++++++++++++++++++++++++++++++++------------
>>>> 1 file changed, 35 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>>> index 57bdfb4e78..93ffaaf706 100644
>>>> --- a/hw/isa/vt82c686.c
>>>> +++ b/hw/isa/vt82c686.c
>>>> @@ -46,6 +46,8 @@ struct ViaPMState {
>>>> ACPIREGS ar;
>>>> APMState apm;
>>>> PMSMBus smb;
>>>> +
>>>> + qemu_irq irq;
>>>
>>> Is it better to name this sci_irq because there seems to be an smi_irq too?
>>> Also there seems to be no SCI pin but there's an SMI pin so does this
>>> sci_irq need to be forwaeded to the ISA bridge and exposed as a qemu_gpio
>>> or could it just set the ISA IRQ within its own handler in the via_pm
>>> object?
>>
>> Triggering the PIC in the PM function seems more complicated since ISA
>> function embeds PM function and also PM function is implemented before ISA
>> function, so this would create nesting problems in the code. Either
>
>Where PM function is implemented is just because it was there before or that's
>how I went through the functions when cleaning it up and ended up there but it
>could be moved, it's not bolted down...
>
>However even if it comes before, we had the pattern before for via-ide and usb
>that they can look up function 0 of their own devfn to find the ISA bridge and
>sinve we're in vt82b686.c here you can consider these to be friend classes so
>pm func could access the ISA irq's directly (or bring back the via_isa_set_irq
>func I had before for this). That way it's simpler and does not need QOM
>wizardry in ISA function that does not even model what real chip does so I
>think this should be implemented in an irq handler func within the PM object
>that matches what happens in the real chip.
This would require container_of() to be used which violates QOM best practices
where a device should only know about its (transitive) children. Given the
current nesting of devices I prefer to keep the PIC triggering in the ISA
function where the PIC resides.
Best regards,
Bernhard
>
>> way, both approaches need to sneak into the other PCI function's data, so
>> I'd keep via_isa_set_pm_irq() and just update the constants.
>>
>>> There are also other registers that select between SMI and SCI, do those
>>> need to be taken into account?
>>
>> Maybe later. The current code already made the decision to use SCI by
>> triggering the PIC, so I'd declare this to be out of scope for now.
>
>Yes, if you're now just handling sci with one wpecific mapping then those can
>wait until adding smi, just found them now and asked if they need to be
>considered for sci now.
>
>Regards,
>BALATON Zoltan
