Re: [PATCH 1/2] Add CanWakeUp property to NMDevice
Hi,
Thank you for your feedback. Answers inside.
Le mercredi 12 octobre 2011 à 23:44 -0500, Dan Williams a écrit :
> On Wed, 2011-10-12 at 13:22 +0200, Jean Parpaillon wrote:
> > From: Jean Parpaillon
>
> So what's the overall architecture here? This is the implementation,
> clearly, but what goes into the device's virtual functions like
> can_wake_up() and prepare_wake_up()?
>
I must admit that I sent this hoping to (re?)start a discussion on the
subject.
My understanding is the following:
- for ethernet (or ethernet-like) devices, there is a common interface
for wakeup capable devices. It mainly consist in enable/disable wakeup
and, for some devices, setting a 'secret'
- this interface have been (partly) generalized with the ACPI syfs
interface. Each (capable) device has a wakeup file in his sysfs
structure in where you can write enable/disable and this is suppose to
deal with power during S3 ACPI state and maybe more (rfkill ?, network
not disconnecting when calling kernel halt() ?) ?
My developments were stopped when I found that my laptop BIOS does not
support powering up mini PCIe device when in S3 state, as it was
supposed to be the case (thanks lenovo ;) )
So, in a first time, I needed to have this property to ensure
NetworkManager is not disconnecting the device when sleeping.
prepare_wakeup(), in my thoughts, must prepare the device for waking up
(!), which means, for instance, setting the secret for waking up. But
this is based on the device I use, which is ericsson F5521gw.
Nevertheless, I think that this architecture is pretty generic, as it is
also used by ethtool.
Of course, CanWakeUp should be consistent with ACPI wakeup property in
sysfs and prepare_wakeup() should just call a generic interface in the
kernel... which exists only in the (old ?) ethtool interface.
Does it make sense for you ? For what you can see, I have still a lot of
questions but I am really interested in discussing this subject to
provide a NM "standardized" way of dealing with wow.
> Also would be good to pull Matthew Garrett in here; we've had
> discussions about WOL/WOWLAN before.
>
Sure.
> Dan
>
Best regards,
Jean
> > ---
> > introspection/nm-device.xml |5
> > libnm-glib/nm-device.c | 48
> > +++
> > libnm-glib/nm-device.h |2 +
> > src/nm-device-interface.c |7 ++
> > src/nm-device-interface.h |2 +
> > src/nm-device.c | 23
> > src/nm-device.h |4 +++
> > 7 files changed, 91 insertions(+), 0 deletions(-)
> >
> > diff --git a/introspection/nm-device.xml b/introspection/nm-device.xml
> > index 5fdda96..2e29bb4 100644
> > --- a/introspection/nm-device.xml
> > +++ b/introspection/nm-device.xml
> > @@ -97,6 +97,11 @@
> > The general type of the network device; ie Ethernet, WiFi, etc.
> >
> >
> > +
> > +
> > +If TRUE, the device can wake up the host when sleeping.
> > +
> > +
> >
> >
> > > value="impl_device_disconnect"/>
> > diff --git a/libnm-glib/nm-device.c b/libnm-glib/nm-device.c
> > index f06c1e4..9b832d3 100644
> > --- a/libnm-glib/nm-device.c
> > +++ b/libnm-glib/nm-device.c
> > @@ -55,6 +55,7 @@ typedef struct {
> > NMDeviceCapabilities capabilities;
> > gboolean managed;
> > gboolean firmware_missing;
> > + gboolean can_wake_up;
> > NMIP4Config *ip4_config;
> > gboolean got_ip4_config;
> > NMDHCP4Config *dhcp4_config;
> > @@ -81,6 +82,7 @@ enum {
> > PROP_CAPABILITIES,
> > PROP_MANAGED,
> > PROP_FIRMWARE_MISSING,
> > + PROP_CAN_WAKE_UP,
> > PROP_IP4_CONFIG,
> > PROP_DHCP4_CONFIG,
> > PROP_IP6_CONFIG,
> > @@ -304,6 +306,7 @@ register_for_property_changed (NMDevice *device)
> > { NM_DEVICE_CAPABILITIES, _nm_object_demarshal_generic,
> > &priv->capabilities },
> > { NM_DEVICE_MANAGED, _nm_object_demarshal_generic,
> > &priv->managed },
> > { NM_DEVICE_FIRMWARE_MISSING, _nm_object_demarshal_generic,
> > &priv->firmware_missing },
> > + { NM_DEVICE_CAN_WAKE_UP, _nm_object_demarshal_generic,
> > &priv->can_wake_up },
> > { NM_DEVICE_IP4_CONFIG, demarshal_ip4_config,
> > &priv->ip4_config },
> > { NM_DEVICE_DHCP4_CONFIG, demarshal_dhcp4_config,
> > &priv->dhcp4_config },
> > { NM_DEVICE_IP6_CONFIG, demarshal_ip6_config,
> > &priv->ip6_config },
> > @@ -452,6 +455,9 @@ get_property (GObject *object,
> > case PROP_FIRMWARE_MISSING:
> > g_value_set_boolean (value, nm_device_get_firmware_missing
> > (device));
> > break;
> > + case PROP_CAN_WAKE_UP:
> > + g_value_set_boolean (value, nm_device_get_can_wake_up (device));
> > + break;
> > case PROP_IP4_CONFIG:
> > g_value_set_object (value, nm_device_get_ip4_config (device));
> > break
Re: [PATCH 1/2] Add CanWakeUp property to NMDevice
On Wed, 2011-10-12 at 13:22 +0200, Jean Parpaillon wrote:
> From: Jean Parpaillon
So what's the overall architecture here? This is the implementation,
clearly, but what goes into the device's virtual functions like
can_wake_up() and prepare_wake_up()?
Also would be good to pull Matthew Garrett in here; we've had
discussions about WOL/WOWLAN before.
Dan
> ---
> introspection/nm-device.xml |5
> libnm-glib/nm-device.c | 48
> +++
> libnm-glib/nm-device.h |2 +
> src/nm-device-interface.c |7 ++
> src/nm-device-interface.h |2 +
> src/nm-device.c | 23
> src/nm-device.h |4 +++
> 7 files changed, 91 insertions(+), 0 deletions(-)
>
> diff --git a/introspection/nm-device.xml b/introspection/nm-device.xml
> index 5fdda96..2e29bb4 100644
> --- a/introspection/nm-device.xml
> +++ b/introspection/nm-device.xml
> @@ -97,6 +97,11 @@
> The general type of the network device; ie Ethernet, WiFi, etc.
>
>
> +
> +
> +If TRUE, the device can wake up the host when sleeping.
> +
> +
>
>
> value="impl_device_disconnect"/>
> diff --git a/libnm-glib/nm-device.c b/libnm-glib/nm-device.c
> index f06c1e4..9b832d3 100644
> --- a/libnm-glib/nm-device.c
> +++ b/libnm-glib/nm-device.c
> @@ -55,6 +55,7 @@ typedef struct {
> NMDeviceCapabilities capabilities;
> gboolean managed;
> gboolean firmware_missing;
> + gboolean can_wake_up;
> NMIP4Config *ip4_config;
> gboolean got_ip4_config;
> NMDHCP4Config *dhcp4_config;
> @@ -81,6 +82,7 @@ enum {
> PROP_CAPABILITIES,
> PROP_MANAGED,
> PROP_FIRMWARE_MISSING,
> + PROP_CAN_WAKE_UP,
> PROP_IP4_CONFIG,
> PROP_DHCP4_CONFIG,
> PROP_IP6_CONFIG,
> @@ -304,6 +306,7 @@ register_for_property_changed (NMDevice *device)
> { NM_DEVICE_CAPABILITIES, _nm_object_demarshal_generic,
> &priv->capabilities },
> { NM_DEVICE_MANAGED, _nm_object_demarshal_generic,
> &priv->managed },
> { NM_DEVICE_FIRMWARE_MISSING, _nm_object_demarshal_generic,
> &priv->firmware_missing },
> + { NM_DEVICE_CAN_WAKE_UP, _nm_object_demarshal_generic,
> &priv->can_wake_up },
> { NM_DEVICE_IP4_CONFIG, demarshal_ip4_config,
> &priv->ip4_config },
> { NM_DEVICE_DHCP4_CONFIG, demarshal_dhcp4_config,
> &priv->dhcp4_config },
> { NM_DEVICE_IP6_CONFIG, demarshal_ip6_config,
> &priv->ip6_config },
> @@ -452,6 +455,9 @@ get_property (GObject *object,
> case PROP_FIRMWARE_MISSING:
> g_value_set_boolean (value, nm_device_get_firmware_missing
> (device));
> break;
> + case PROP_CAN_WAKE_UP:
> + g_value_set_boolean (value, nm_device_get_can_wake_up (device));
> + break;
> case PROP_IP4_CONFIG:
> g_value_set_object (value, nm_device_get_ip4_config (device));
> break;
> @@ -496,6 +502,9 @@ set_property (GObject *object,
> /* Construct only */
> priv->device_type = g_value_get_uint (value);
> break;
> + case PROP_CAN_WAKE_UP:
> + priv->can_wake_up = g_value_get_boolean (value);
> + break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> break;
> @@ -629,6 +638,19 @@ nm_device_class_init (NMDeviceClass *device_class)
> G_PARAM_READABLE));
>
> /**
> + * NMDevice:can-wake-up:
> + *
> + * When %TRUE indicates the device can be used to wake up the machine.
> + **/
> + g_object_class_install_property
> + (object_class, PROP_CAN_WAKE_UP,
> + g_param_spec_boolean (NM_DEVICE_CAN_WAKE_UP,
> +"CanWakeUp",
> +"Device can WakeUp",
> +FALSE,
> +G_PARAM_READWRITE));
> +
> + /**
>* NMDevice:ip4-config:
>*
>* The #NMIP4Config of the device.
> @@ -1036,6 +1058,32 @@ nm_device_get_firmware_missing (NMDevice *device)
> }
>
> /**
> + * nm_device_get_can_wake_up:
> + * @device: a #NMDevice
> + *
> + * Indicates that the device can be used to wake up the machine
> + *
> + * Returns: %TRUE if the device can be used to wake up the machine.
> + **/
> +gboolean
> +nm_device_get_can_wake_up (NMDevice *device)
> +{
> + NMDevicePrivate *priv;
> +
> + g_return_val_if_fail (NM_IS_DEVICE (device), 0);
> +
> + priv = NM_DEVICE_GET_PRIVATE (device);
> + if (!priv->can_wake_up) {
> + priv->can_wake_up = _nm_object_get_boolean_property (NM_OBJEC
[PATCH 1/2] Add CanWakeUp property to NMDevice
From: Jean Parpaillon
---
introspection/nm-device.xml |5
libnm-glib/nm-device.c | 48 +++
libnm-glib/nm-device.h |2 +
src/nm-device-interface.c |7 ++
src/nm-device-interface.h |2 +
src/nm-device.c | 23
src/nm-device.h |4 +++
7 files changed, 91 insertions(+), 0 deletions(-)
diff --git a/introspection/nm-device.xml b/introspection/nm-device.xml
index 5fdda96..2e29bb4 100644
--- a/introspection/nm-device.xml
+++ b/introspection/nm-device.xml
@@ -97,6 +97,11 @@
The general type of the network device; ie Ethernet, WiFi, etc.
+
+
+If TRUE, the device can wake up the host when sleeping.
+
+
diff --git a/libnm-glib/nm-device.c b/libnm-glib/nm-device.c
index f06c1e4..9b832d3 100644
--- a/libnm-glib/nm-device.c
+++ b/libnm-glib/nm-device.c
@@ -55,6 +55,7 @@ typedef struct {
NMDeviceCapabilities capabilities;
gboolean managed;
gboolean firmware_missing;
+ gboolean can_wake_up;
NMIP4Config *ip4_config;
gboolean got_ip4_config;
NMDHCP4Config *dhcp4_config;
@@ -81,6 +82,7 @@ enum {
PROP_CAPABILITIES,
PROP_MANAGED,
PROP_FIRMWARE_MISSING,
+ PROP_CAN_WAKE_UP,
PROP_IP4_CONFIG,
PROP_DHCP4_CONFIG,
PROP_IP6_CONFIG,
@@ -304,6 +306,7 @@ register_for_property_changed (NMDevice *device)
{ NM_DEVICE_CAPABILITIES, _nm_object_demarshal_generic,
&priv->capabilities },
{ NM_DEVICE_MANAGED, _nm_object_demarshal_generic,
&priv->managed },
{ NM_DEVICE_FIRMWARE_MISSING, _nm_object_demarshal_generic,
&priv->firmware_missing },
+ { NM_DEVICE_CAN_WAKE_UP, _nm_object_demarshal_generic,
&priv->can_wake_up },
{ NM_DEVICE_IP4_CONFIG, demarshal_ip4_config,
&priv->ip4_config },
{ NM_DEVICE_DHCP4_CONFIG, demarshal_dhcp4_config,
&priv->dhcp4_config },
{ NM_DEVICE_IP6_CONFIG, demarshal_ip6_config,
&priv->ip6_config },
@@ -452,6 +455,9 @@ get_property (GObject *object,
case PROP_FIRMWARE_MISSING:
g_value_set_boolean (value, nm_device_get_firmware_missing
(device));
break;
+ case PROP_CAN_WAKE_UP:
+ g_value_set_boolean (value, nm_device_get_can_wake_up (device));
+ break;
case PROP_IP4_CONFIG:
g_value_set_object (value, nm_device_get_ip4_config (device));
break;
@@ -496,6 +502,9 @@ set_property (GObject *object,
/* Construct only */
priv->device_type = g_value_get_uint (value);
break;
+ case PROP_CAN_WAKE_UP:
+ priv->can_wake_up = g_value_get_boolean (value);
+ break;
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
break;
@@ -629,6 +638,19 @@ nm_device_class_init (NMDeviceClass *device_class)
G_PARAM_READABLE));
/**
+* NMDevice:can-wake-up:
+*
+* When %TRUE indicates the device can be used to wake up the machine.
+**/
+ g_object_class_install_property
+ (object_class, PROP_CAN_WAKE_UP,
+g_param_spec_boolean (NM_DEVICE_CAN_WAKE_UP,
+ "CanWakeUp",
+ "Device can WakeUp",
+ FALSE,
+ G_PARAM_READWRITE));
+
+ /**
* NMDevice:ip4-config:
*
* The #NMIP4Config of the device.
@@ -1036,6 +1058,32 @@ nm_device_get_firmware_missing (NMDevice *device)
}
/**
+ * nm_device_get_can_wake_up:
+ * @device: a #NMDevice
+ *
+ * Indicates that the device can be used to wake up the machine
+ *
+ * Returns: %TRUE if the device can be used to wake up the machine.
+ **/
+gboolean
+nm_device_get_can_wake_up (NMDevice *device)
+{
+ NMDevicePrivate *priv;
+
+ g_return_val_if_fail (NM_IS_DEVICE (device), 0);
+
+ priv = NM_DEVICE_GET_PRIVATE (device);
+ if (!priv->can_wake_up) {
+ priv->can_wake_up = _nm_object_get_boolean_property (NM_OBJECT
(device),
+
NM_DBUS_INTERFACE_DEVICE,
+
"CanWakeUp",
+
NULL);
+ }
+
+ return priv->can_wake_up;
+}
+
+/**
* nm_device_ge
