Re: Devices without power management support
On Fri, Aug 19, 2022 at 01:52:47AM +, Taylor R Campbell wrote:
> All that said: What is the failure mode you're seeing for ihidev that
> blocks suspend?
It fails to read HID descriptor. The ihidev device remains unfunctionnal
and breaks suspend.
I thought about this workaround:
--- sys/dev/i2c/ihidev.c.orig
+++ sys/dev/i2c/ihidev.c
@@ -161,8 +163,17 @@
sc->sc_tag = ia->ia_tag;
sc->sc_addr = ia->ia_addr;
mutex_init(>sc_lock, MUTEX_DEFAULT, IPL_NONE);
+ /*
+* We register null handlers so that a failed attachement
+* does not result in a device that breaks suspend.
+* The real handler are registered at the end of this
+* function on success.
+*/
+ if (!pmf_device_register(self, NULL, NULL))
+ aprint_error_dev(self, "couldn't establish power handler\n");
+
sc->sc_phandle = ia->ia_cookie;
if (ia->ia_cookietype != I2C_COOKIE_ACPI) {
aprint_error(": unsupported device tree type\n");
return;
--
Emmanuel Dreyfus
m...@netbsd.org
Re: Devices without power management support
p...@whooppee.com (Paul Goyette) writes: >Don't forget to deregister the device if the xxx_attach() later exits... I think the point was to not do this, so that a failed attach doesn't prevent the system from entering sleep mode. Here, calling pmf_register first on attachment is then required, but then you have to think on how to handle failures of pmf_register. You wouldn't want the device to fail attachment in that case, but rather accept that you cannot suspend in that situation. You also have to think about drivers that are left only partially configured after a failed attachment. That is already unsafe with drivers that only do a dummy pmf registration, but what about hardware that actually needs support routines ? Registering these for a partially configured driver may not work either. Unregister again and register dummy routines ? An easy solution would be to make attach actually fail, autoconf could then detached the driver immediately and it won't interfere with pmf. N.B. if autoconf API is changed, you could also integrate pmf into cfattach.
Re: Devices without power management support
> Date: Fri, 19 Aug 2022 01:22:39 + > From: Emmanuel Dreyfus > > I have patches to move the pmf_device_register() earlier in xxx_attach() > to workaround the problem in iwm(4) and ihidev(4) where I encounter the > problem, but perhaps the problem could be worked around globaly, by not > failing sleep mode if a driver did not call pmf_device_register()? I think we should just make the device suspend/resume functions be autoconf routines like match, attach, detach, and dispense with pmf_device_register altogether. FYI, moving pmf_device_register earlier can turn a working driver into a broken driver, because right now there's nothing that prevents suspend from happening before attach completes, and the suspend routine may assume that attach has completed. All that said: What is the failure mode you're seeing for ihidev that blocks suspend? (iwm(4) attach is flaky and needs to be rototilled, but I'd rather not touch that on HEAD while martin's wifi branch is in progress.)
Re: Devices without power management support
On Fri, 19 Aug 2022, Emmanuel Dreyfus wrote: Hello When a devices fails initialization, the xxx_attach() function may exit early, before calling pmf_device_register(). The consequence is that any attempt to put the system to sleep fail with a "Devices without power management support: ..." message. Weird things happens, and I experienced situation where some devices randomly or always fail initialization. This breaks sleeping mode. I have patches to move the pmf_device_register() earlier in xxx_attach() to workaround the problem in iwm(4) and ihidev(4) where I encounter the problem, but perhaps the problem could be worked around globaly, by not failing sleep mode if a driver did not call pmf_device_register()? Don't forget to deregister the device if the xxx_attach() later exits... Of course the lack of a registered suspend handler may miss the expectation of a system sleeping with reduced power, but on the other hand, being unable to sleep at all means even less power reduction. -- Emmanuel Dreyfus m...@netbsd.org !DSPAM:62fee614256621427699200! ++--+--+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses:| | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com| | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | | & Network Engineer | | pgoyett...@gmail.com | ++--+--+
Devices without power management support
Hello When a devices fails initialization, the xxx_attach() function may exit early, before calling pmf_device_register(). The consequence is that any attempt to put the system to sleep fail with a "Devices without power management support: ..." message. Weird things happens, and I experienced situation where some devices randomly or always fail initialization. This breaks sleeping mode. I have patches to move the pmf_device_register() earlier in xxx_attach() to workaround the problem in iwm(4) and ihidev(4) where I encounter the problem, but perhaps the problem could be worked around globaly, by not failing sleep mode if a driver did not call pmf_device_register()? Of course the lack of a registered suspend handler may miss the expectation of a system sleeping with reduced power, but on the other hand, being unable to sleep at all means even less power reduction. -- Emmanuel Dreyfus m...@netbsd.org
