On Sat, 6 Aug 2022 at 19:18, Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > On Tue, 2 Aug 2022 17:32:22 +0200 > Crt Mori <cmo@xxxxxxxxxxx> wrote: > > > Hi Andy, > > Thanks for the explanation and help - now the PM runtime suspend and > > resume are triggered. It was indeed the combination of bad > > initialization and that I did not have "enter wakeup" on the read > > data. > > > > Currently, I view pm_runtime mode as a power saving mode (device > > should be in low power) and normal running as running mode. However, > > the sensor is able to report data in both modes - just reading is a > > bit different. I also do not have a problem with autosuspend after > > some time, but I do wonder how would user change the power mode from > > pm_runtime to normal mode (reading, in this case, should not change > > the mode). What is the recommended ABI for that? > > This comes up from time to time. Normally it's very hard for a user > to know the right answer to power mode questions because they rarely > know enough of the impacts, so we try to use things that do have obvious > meaning. > > * If we are doing buffered capture, can assume the user wants good perf > so switch to a high power mode. If doing sysfs reads they probably > care less about perf. We could apply a heuristic along the lines of > they read it twice in the last second, keep it in high power mode. > I do not think the buffered mode is kinda something that would be applicable here since refresh rate is not really that fast. (64 Hz). Or you think I should implement this simply for continuous mode? > * Often power modes are reflected in sampling rate. So just wire it up > to that (with possible side effects on other parameters). > I took a quick look at the mlx90632 data sheet and it seems the modes > we have are. > > Sleep step mode: Goes to sleep between individual measurement sets. So > each one has higher latency after request. Various subtle complexities > around control int his mode, but it's the latency at start and finish > which differs from other modes This is the new part of the driver I want to add. We call it Burst measurement mode, and it allows sensor to be in sleep step mode, while performing triggered measurements. This is the mode I would go after autosuspend. > > Step mode: Stays powered up, but you need to poll to get a measurement. This mode I would skip. There is no bigger added value at least in Linux case. > > Continuous: Autonomous sampling mode. > > So fun device. I think you kind of have to implement a heuristic for this. > Probably switch between step mode and sleep step mode based on time between > last two accesses or similar. If there haven't been 2 accesses yet > stay in sleep step mode. This mode I would have in repeated measurements. However, how do I define that? Is there a PM function that would tell me to switch from Sleep step mode to this? Because runtime_resume in this case would prevent sensor to work in sleep step mode, so like you suggested I will check for the time between last two accesses. Does this time need to be configurable, or can I hardcode it to some "reasonable" value? > > Side note. As you presumably have hardware for this part, could you run > a rest with UNIVERSAL_DEV_PM_OPS (which is deprecated) switched to > DEFINE_RUNTIME_PM_OPS() The RUNTIME version checks if we are already > suspended on entry into a full suspend and only powers down the device > if not already powered down. I plan to spin a patch soon to do this > anyway, but it is technically different behaviour so want it tested > if at all possible! (feel free to not wait for my patch, or to send > your own :) > I see you already sent your patch, although what I am preparing (with runtime_pm) will probably overwrite it. I will test your patch tomorrow as my patches will probably take some time. Thanks for the answers. Crt > Jonathan > > > > > > On Mon, 25 Jul 2022 at 23:45, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > > > > > > On Mon, Jul 25, 2022 at 11:36 PM Crt Mori <cmo@xxxxxxxxxxx> wrote: > > > > On Mon, 25 Jul 2022 at 23:27, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > > > > > On Mon, Jul 25, 2022 at 10:48 AM Crt Mori <cmo@xxxxxxxxxxx> wrote: > > > > > > > > > > > > Hi, > > > > > > I am implementing the power saving modes for mlx90632 device driver > > > > > > and while I have implemented routines for SET_RUNTIME_PM_OPS > > > > > > (runtime_pm_suspend and runtime_pm_resume) I am not able to find out > > > > > > how to trigger them from the terminal. > > > > > > > > > > > > It could be that my driver code for power management implementation is > > > > > > incomplete and I need to initialize something more. > > > > > > > > > > > > Maybe it is helpful, but the power submodule of the device contains below files: > > > > > > > > > > > > $ ls -al /sys/bus/iio/devices/iio\:device0/power > > > > > > total 0 > > > > > > drwxrwxr-x 2 root gpio 0 Apr 6 14:17 . > > > > > > drwxrwxr-x 3 root gpio 0 Apr 6 14:17 .. > > > > > > -rw-rw-r-- 1 root gpio 4096 Apr 6 14:17 async > > > > > > -rw-rw-r-- 1 root gpio 4096 Apr 6 14:17 autosuspend_delay_ms > > > > > > -rw-rw-r-- 1 root gpio 4096 Apr 6 14:18 control > > > > > > -r--r--r-- 1 root gpio 4096 Apr 6 14:17 runtime_active_kids > > > > > > -r--r--r-- 1 root gpio 4096 Apr 6 14:17 runtime_active_time > > > > > > -r--r--r-- 1 root gpio 4096 Apr 6 14:17 runtime_enabled > > > > > > -r--r--r-- 1 root gpio 4096 Apr 6 14:17 runtime_status > > > > > > -r--r--r-- 1 root gpio 4096 Apr 6 14:17 runtime_suspended_time > > > > > > -r--r--r-- 1 root gpio 4096 Apr 6 14:17 runtime_usage > > > > > > > > > > > > And control is already set to "auto" which according to documentation > > > > > > should allow the PM. > > > > > > > > > > 'auto' should enable it. So, whenever the driver thinks it's a time to > > > > > power off/on the device it will call the methods. > > > > > > > > > > You may hack a bit to enable autosuspend (which often is not a good > > > > > idea for IIO sensors) and see it done automatically after some time. > > > > > > > > So the idea is to wait? > > > > > > Yes. > > > > > > > How would I enable autosuspend - by lowering > > > > the autosusped_delay_ms? > > > > > > Yep, if you wish. The driver should enable it though. > > > > Is there a way for driver to attach a callback to > > power/autosuspend_delay_ms request or to power/control? > > > > > > > > > How does the driver decide that it is time to > > > > power off/on? > > > > > > I'm not a driver author, it seems you , who should answer this > > > question (as you are about to add PM there, am I right?). > > > > > > > Do I need something else enabled to have this done automatically? > > > > Autosuspend is 5000 in my case which would mean 5 seconds, so I am > > > > quite sure I waited that long and I did not see printk's from the > > > > driver. > > > > > > Something prevents it from doing (reference counting) or simply some > > > initialization / enablement is forgotten. For different buses > > > different PM runtime rules are applied (for example, IIRC, PCI core > > > does it for you, while platform bus is on what driver wants basis). > > > > > > -- > > > With Best Regards, > > > Andy Shevchenko >
