On Mon, Oct 21, 2019 at 5:49 AM Ran Wang <ran.wang_1@xxxxxxx> wrote:
>
> Some user might want to go through all registered wakeup sources
> and doing things accordingly. For example, SoC PM driver might need to
> do HW programming to prevent powering down specific IP which wakeup
> source depending on. So add this API to help walk through all registered
> wakeup source objects on that list and return them one by one.
>
> Signed-off-by: Ran Wang <ran.wang_1@xxxxxxx>
> Tested-by: Leonard Crestez <leonard.crestez@xxxxxxx>
> ---
> Change in v7:
> - Remove define of member *dev in wake_irq to fix conflict with commit
> c8377adfa781 ("PM / wakeup: Show wakeup sources stats in sysfs"), user
> will use ws->dev->parent instead.
> - Remove '#include <linux/of_device.h>' because it is not used.
>
> Change in v6:
> - Add wakeup_source_get_star() and wakeup_source_get_stop() to aligned
> with wakeup_sources_stats_seq_start/nex/stop.
>
> Change in v5:
> - Update commit message, add decription of walk through all wakeup
> source objects.
> - Add SCU protection in function wakeup_source_get_next().
> - Rename wakeup_source member 'attached_dev' to 'dev' and move it up
> (before wakeirq).
>
> Change in v4:
> - None.
>
> Change in v3:
> - Adjust indentation of *attached_dev;.
>
> Change in v2:
> - None.
>
> drivers/base/power/wakeup.c | 37 +++++++++++++++++++++++++++++++++++++
> include/linux/pm_wakeup.h | 3 +++
> 2 files changed, 40 insertions(+)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 5817b51..dee1b09 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -248,6 +248,43 @@ void wakeup_source_unregister(struct wakeup_source *ws)
> EXPORT_SYMBOL_GPL(wakeup_source_unregister);
>
> /**
> + * wakeup_source_get_star - Begin a walk on wakeup source list
The "get" in the name suggests acquiring a reference of some kind
which doesn't happen here.
What about renaming it to wakeup_sources_walk_start()?
> + * @srcuidx: Lock index allocated for this caller.
> + */
> +struct wakeup_source *wakeup_source_get_start(int *srcuidx)
I don't quite like the calling convention here with passing an int
pointer to get the SRCU index back.
What about splitting this into, say, wakeup_sources_read_lock() (that
will return the SRCU index) and wakeup_sources_walk_start() (that will
return the first list entry)?
Then, you could do something like
idx = wakeup_sources_read_lock();
ws = wakeup_sources_walk_start();
while (ws) {
stuff
ws = wakeup_sources_walk_next();
}
wakeup_sources_read_unlock(idx);
Or even define for_each_wakeup_source(ws) as
for (ws = wakeup_sources_walk_start(); ws; ws = wakeup_sources_walk_next())
and use that under a _read_lock()/_read_unlock() pair?
