Re: pmf(9) vs sysmon for power events (especially sleep when powerd(8) is not running)

[Jean-Yves Migeon <jeanyves.migeon%free.fr@localhost>]

On 07.05.2011 21:45, Jukka Ruohonen wrote:
> On Sat, May 07, 2011 at 09:03:42PM +0200, Jean-Yves Migeon wrote:
>> - sysmon_pswitch(9) can still be used to register power switch events,
>> these events being modeled following a "switch" functionality e.g. when
>> a threshold is passed.
> 
> Yes. Although I don't know what you mean by thresholds.

By threshold I just meant that the button gets pressed or released.

>> - pmf(9) is focused on device states, so it's lower level than
>> sysmon_pswitch(9) events. pmf(9) event injection is not supposed to be
>> called directly, but rather through sysmon (for switch-like
>> functionality), or within pmf(9) itself for inter-device signaling.
> 
> No. Device drivers are calling pmf(9) event injections directly. I think
> Jared or Jörg should clarify this, but I think the pmf(9) calls you cited
> earlier were added to the sysmon routines for compatibility-like reasons.
> To be effective, there needs to be also a listener for the injected events.

okay.

>> So, in the current form, power switches/buttons are not supposed to
>> register as devices and implement their own hooks for registration with
>> pmf(9)?
> 
> I am not sure what you mean by this. For instance, a platform/laptop-specific
> driver registers naturally with pmf(9), but it may also use the sysmon
> routines for various tasks (e.g. also some hotkeys are handled by the sysmon
> routines). There is no grand scheme of things. It is just duplicity.

Let's take xenbus(4). During attach, it registers a handler
(xenbus_shutdown_handler) that will handle powerof/halt/reboot events.
This is done via the sysmon_pswitch(9) API [1].

It could use pmf_event_register(), and use one of the
pmf_generic_event_t (or roll out its own) to inject events.

The duplicity is rather flagrant for my case with Xen: shutdown/reboot
events can be handled by sysmon_pswitch_event(), but sleep is not. And I
am wondering whether I should:

1 - use sysmon_pswitch_register() to manage the sleep (hence adding
callback to fix the /* XXX */ comment in sysmon_pswitch_event) (see
attached sysmon.diff)

2 - or use pmf_event_register(), and add bind sysmon(9) and pmf(9) (see
pmf_inject.diff).

And this all boils down to which API will become the de facto "standard"
for managing system power events. Note that 2) is less invasive than 1)
for the current KPI.

[1]
http://nxr.netbsd.org/source/xref/src/sys/arch/xen/xen/shutdown_xenbus.c#xenbus_shutdown_handler

-- 
Jean-Yves Migeon
jeanyves.migeon%free.fr@localhost

Index: dev/sysmon/sysmon_power.c
===================================================================
RCS file: /cvsroot/src/sys/dev/sysmon/sysmon_power.c,v
retrieving revision 1.44
diff -u -p -r1.44 sysmon_power.c
--- dev/sysmon/sysmon_power.c   11 Mar 2010 13:51:01 -0000      1.44
+++ dev/sysmon/sysmon_power.c   7 May 2011 18:12:15 -0000
@@ -989,6 +989,7 @@ sysmon_pswitch_event(struct sysmon_pswit
                 */
                /* XXX */
                printf("%s: sleep button pressed.\n", smpsw->smpsw_name);
+               pmf_event_inject(NULL, PMFE_SYSTEM_SLEEP);
                break;
 
        case PSWITCH_TYPE_HOTKEY:
Index: sys/pmf.h
===================================================================
RCS file: /cvsroot/src/sys/sys/pmf.h,v
retrieving revision 1.18
diff -u -p -r1.18 pmf.h
--- sys/pmf.h   24 Feb 2010 22:38:10 -0000      1.18
+++ sys/pmf.h   7 May 2011 21:39:28 -0000
@@ -46,7 +46,10 @@ typedef enum {
        PMFE_AUDIO_VOLUME_DOWN,
        PMFE_AUDIO_VOLUME_TOGGLE,
        PMFE_CHASSIS_LID_CLOSE,
-       PMFE_CHASSIS_LID_OPEN
+       PMFE_CHASSIS_LID_OPEN,
+       PMFE_TYPE_POWER,
+       PMFE_TYPE_RESET,
+       PMFE_TYPE_SLEEP
 } pmf_generic_event_t;
 
 struct pmf_qual {

Index: dev/sysmon/sysmon_power.c
===================================================================
RCS file: /cvsroot/src/sys/dev/sysmon/sysmon_power.c,v
retrieving revision 1.44
diff -u -p -r1.44 sysmon_power.c
--- dev/sysmon/sysmon_power.c   11 Mar 2010 13:51:01 -0000      1.44
+++ dev/sysmon/sysmon_power.c   7 May 2011 18:22:01 -0000
@@ -889,7 +889,7 @@ sysmon_penvsys_event(struct penvsys_stat
 int
 sysmon_pswitch_register(struct sysmon_pswitch *smpsw)
 {
-       /* nada */
+       /* register hook called from MD code for the PSWITCH_TYPE_SLEEP event */
        return 0;
 }
 
Index: dev/sysmon/sysmonvar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/sysmon/sysmonvar.h,v
retrieving revision 1.40
diff -u -p -r1.40 sysmonvar.h
--- dev/sysmon/sysmonvar.h      4 Jan 2011 01:51:06 -0000       1.40
+++ dev/sysmon/sysmonvar.h      7 May 2011 18:22:01 -0000
@@ -252,6 +252,7 @@ void        sysmon_wdog_init(void);
 struct sysmon_pswitch {
        const char *smpsw_name;         /* power switch name */
        int smpsw_type;                 /* power switch type */
+       void  (*smpsw_func)(void *, int); /* handler associated with pswitch */
 
        LIST_ENTRY(sysmon_pswitch) smpsw_list;
 };
Index: arch/xen/x86/mainbus.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/x86/mainbus.c,v
retrieving revision 1.13
diff -u -p -r1.13 mainbus.c
--- arch/xen/x86/mainbus.c      12 Nov 2010 02:07:27 -0000      1.13
+++ arch/xen/x86/mainbus.c      7 May 2011 18:27:46 -0000
@@ -210,6 +210,8 @@ mainbus_attach(device_t parent, device_t
        mba.mba_haa.haa_busname = "hypervisor";
        config_found_ia(self, "hypervisorbus", &mba.mba_haa, mainbus_print);
 #endif
+
+       /*
+        * Set the power type, so that powerd(8) can adapt its behavior
+        * depending on the hardware/power context.
+        * The value should be unique.
+        */
+       sysmon_power_settype(/* ACPI, Xen-domU, PPC, ... */);
 }
 
 int
Index: dev/sysmon/sysmon_power.c
===================================================================
RCS file: /cvsroot/src/sys/dev/sysmon/sysmon_power.c,v
retrieving revision 1.44
diff -u -p -r1.44 sysmon_power.c
--- dev/sysmon/sysmon_power.c   11 Mar 2010 13:51:01 -0000      1.44
+++ dev/sysmon/sysmon_power.c   7 May 2011 21:42:58 -0000
@@ -889,7 +889,7 @@ sysmon_penvsys_event(struct penvsys_stat
 int
 sysmon_pswitch_register(struct sysmon_pswitch *smpsw)
 {
-       /* nada */
+       /* Register a hook for the PSWITCH_TYPE_SLEEP event */
        return 0;
 }
 
@@ -987,8 +987,8 @@ sysmon_pswitch_event(struct sysmon_pswit
                /*
                 * Try to enter a "sleep" state.
                 */
-               /* XXX */
                printf("%s: sleep button pressed.\n", smpsw->smpsw_name);
+               /* call the registered callback */
                break;
 
        case PSWITCH_TYPE_HOTKEY: