Re: [Qemu-devel] [PATCH 1/4] Confine use of global rtc_state to PC CMOS functions

[Blue Swirl]

On 5/19/09, Markus Armbruster <address@hidden> wrote:
> Sorry for the sloooow respons, I was on vacation.
>
>
>  Blue Swirl <address@hidden> writes:
>
>  > On 5/4/09, Markus Armbruster <address@hidden> wrote:
>  >> Blue Swirl <address@hidden> writes:
>  >>
>  >>  > On 4/30/09, Markus Armbruster <address@hidden> wrote:
>  >>  >> Pass the state as argument to cmos_init() and cmos_init_hd().
>  >>  >>  cmos_init() still needs to save it in rtc_state for use by
>  >>  >>  cmos_set_s3_resume().
>  >>  >
>  >>  > pc.c could pass acpi an opaque handle (former rtc_state) at init or
>  >>  > acpi could export a function to set the handle, called by pc.c. Then
>  >>  > cmos_set_s3_resume could take a state parameter.
>  >>
>  >>
>  >> We'd just move a global variable from pc.c to acpi.c, wouldn't we?
>  >>  Could you explain why that's a better place?
>  >
>  > No, acpi would only have an opaque pointer to the variable stored in
>  > PIIX4PMState, the "owner" would still be pc.c.
>
>
> Okay, so we'd just move a global variable from pc.c into only instance
>  of struct PIIX4PMState, referenced from a global variable in acpi.c.
>  The fact that it becomes opaque in the process isn't much of an
>  improvement, isn't it?

Yes it is, acpi.c does not need to know about rtc_state internals.

>  >>  Passing rtc_state to piix4_pm_init() doesn't work well for pcdt.c,
>  >>  because we'd have to pass it from RTC device to PIIX3 ACPI device
>  >>  somehow, creating one of those ugly "non-tree" device dependencies,
>  >>  i.e. one that doesn't follow device tree or interrupt tree edges.  Your
>  >>  other idea (a function to set the handle) allows me to keep the two
>  >>  devices decoupled, provided I can set the handle even before
>  >>  piix4_pm_init().
>  >
>  > Still one idea: a signal (qemu_irq) could be used to convey the s3
>  > resume condition. That may be another tree.
>
>
> Hmm.
>
>  How does the hardware do it?  Does it update CMOS all by itself?

Just guessing: there could be a signal wire from the power management
chip to CMOS GPIO pin and this would be visible in a CMOS register.

>  > Setting the handle before piix4_pm_init will fail if the handle is
>  > going to be stored in PIIX4PMState,
>
>
> Correct.
>
>
>  >                                     but the setup can be reversed:
>  > piix4_pm_init can call a function to get the handle.
>
>
> Where from?
>
>  A global variable in pc.c?  But then we create global state in acpi.c
>  without reducing it in pc.c.

pc.c is the "owner" of most devices, so that's OK. acpi.c only has a handle.

>  In the context of pcdt.c: creates the same non-tree device dependency,
>  only in the opposite direction.

I don't see how you can avoid it. Maybe power management wants a tree
of its own.