Re: [PATCH v5 1/3] PM: wakeup: Add routine to help fetch wakeup source object.
On Monday, August 19, 2019 10:33:25 AM CEST Ran Wang wrote:
> Hi Rafael,
>
> On Monday, August 19, 2019 16:20, Rafael J. Wysocki wrote:
> >
> > On Mon, Aug 19, 2019 at 10:15 AM Ran Wang wrote:
> > >
> > > Hi Rafael,
> > >
> > > On Monday, August 05, 2019 17:59, Rafael J. Wysocki wrote:
> > > >
> > > > On Wednesday, July 24, 2019 9:47:20 AM CEST Ran Wang 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
> > > > > ---
> > > > > 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 | 24
> > > > > include/linux/pm_wakeup.h | 3 +++
> > > > > 2 files changed, 27 insertions(+)
> > > > >
> > > > > diff --git a/drivers/base/power/wakeup.c
> > > > > b/drivers/base/power/wakeup.c index ee31d4f..2fba891 100644
> > > > > --- a/drivers/base/power/wakeup.c
> > > > > +++ b/drivers/base/power/wakeup.c
> > > > > @@ -14,6 +14,7 @@
> > > > > #include
> > > > > #include
> > > > > #include
> > > > > +#include
> > > > > #include
> > > > > #include
> > > > >
> > > > > @@ -226,6 +227,28 @@ void wakeup_source_unregister(struct
> > > > wakeup_source *ws)
> > > > > }
> > > > > }
> > > > > EXPORT_SYMBOL_GPL(wakeup_source_unregister);
> > > > > +/**
> > > > > + * wakeup_source_get_next - Get next wakeup source from the list
> > > > > + * @ws: Previous wakeup source object, null means caller want first
> > > > > one.
> > > > > + */
> > > > > +struct wakeup_source *wakeup_source_get_next(struct wakeup_source
> > > > > +*ws) {
> > > > > + struct list_head *ws_head = _sources;
> > > > > + struct wakeup_source *next_ws = NULL;
> > > > > + int idx;
> > > > > +
> > > > > + idx = srcu_read_lock(_srcu);
> > > > > + if (ws)
> > > > > + next_ws = list_next_or_null_rcu(ws_head, >entry,
> > > > > + struct wakeup_source, entry);
> > > > > + else
> > > > > + next_ws = list_entry_rcu(ws_head->next,
> > > > > + struct wakeup_source, entry);
> > > > > + srcu_read_unlock(_srcu, idx);
> > > > > +
> > > >
> > > > This is incorrect.
> > > >
> > > > The SRCU cannot be unlocked until the caller of this is done with
> > > > the object returned by it, or that object can be freed while it is
> > > > still being
> > accessed.
> > >
> > > Thanks for the comment. Looks like I was not fully understanding your
> > > point on
> > > v4 discussion. So I will implement 3 APIs by referring
> > > wakeup_sources_stats_seq_start/next/stop()
> > >
> > > > Besides, this patch conflicts with some general wakeup sources
> > > > changes in the works, so it needs to be deferred and rebased on top of
> > > > those
> > changes.
> > >
> > > Could you please tell me which is the right code base I should developing
> > > on?
> > > I just tried applying v5 patch on latest
> > > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git branch master
> > (d1abaeb Linux 5.3-rc5) and no conflict encountered.
> >
> > It is better to use the most recent -rc from Linus (5.3-rc5 as of
> > today) as the base unless your patches depend on some changes that are not
> > in
> > there.
>
> OK, So I need to implement on latest
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git branch
> master, am I right?
>
> However, I just checked v5.3-rc5 code and found it has the same HEAD (d1abaeb
> Linux 5.3-rc5
> on which I did not observe v5 patch apply conflict, did I miss something?
> Thanks.
The conflict I mentioned earlier was with another patch series in the works
which is not in 5.3-rc5. However, there are problems with that series and it
is not linux-next now even, so please just base your series on top of -rc5.
RE: [PATCH v5 1/3] PM: wakeup: Add routine to help fetch wakeup source object.
Hi Rafael,
On Monday, August 19, 2019 16:20, Rafael J. Wysocki wrote:
>
> On Mon, Aug 19, 2019 at 10:15 AM Ran Wang wrote:
> >
> > Hi Rafael,
> >
> > On Monday, August 05, 2019 17:59, Rafael J. Wysocki wrote:
> > >
> > > On Wednesday, July 24, 2019 9:47:20 AM CEST Ran Wang 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
> > > > ---
> > > > 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 | 24
> > > > include/linux/pm_wakeup.h | 3 +++
> > > > 2 files changed, 27 insertions(+)
> > > >
> > > > diff --git a/drivers/base/power/wakeup.c
> > > > b/drivers/base/power/wakeup.c index ee31d4f..2fba891 100644
> > > > --- a/drivers/base/power/wakeup.c
> > > > +++ b/drivers/base/power/wakeup.c
> > > > @@ -14,6 +14,7 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > #include
> > > > #include
> > > >
> > > > @@ -226,6 +227,28 @@ void wakeup_source_unregister(struct
> > > wakeup_source *ws)
> > > > }
> > > > }
> > > > EXPORT_SYMBOL_GPL(wakeup_source_unregister);
> > > > +/**
> > > > + * wakeup_source_get_next - Get next wakeup source from the list
> > > > + * @ws: Previous wakeup source object, null means caller want first
> > > > one.
> > > > + */
> > > > +struct wakeup_source *wakeup_source_get_next(struct wakeup_source
> > > > +*ws) {
> > > > + struct list_head *ws_head = _sources;
> > > > + struct wakeup_source *next_ws = NULL;
> > > > + int idx;
> > > > +
> > > > + idx = srcu_read_lock(_srcu);
> > > > + if (ws)
> > > > + next_ws = list_next_or_null_rcu(ws_head, >entry,
> > > > + struct wakeup_source, entry);
> > > > + else
> > > > + next_ws = list_entry_rcu(ws_head->next,
> > > > + struct wakeup_source, entry);
> > > > + srcu_read_unlock(_srcu, idx);
> > > > +
> > >
> > > This is incorrect.
> > >
> > > The SRCU cannot be unlocked until the caller of this is done with
> > > the object returned by it, or that object can be freed while it is still
> > > being
> accessed.
> >
> > Thanks for the comment. Looks like I was not fully understanding your
> > point on
> > v4 discussion. So I will implement 3 APIs by referring
> > wakeup_sources_stats_seq_start/next/stop()
> >
> > > Besides, this patch conflicts with some general wakeup sources
> > > changes in the works, so it needs to be deferred and rebased on top of
> > > those
> changes.
> >
> > Could you please tell me which is the right code base I should developing
> > on?
> > I just tried applying v5 patch on latest
> > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git branch master
> (d1abaeb Linux 5.3-rc5) and no conflict encountered.
>
> It is better to use the most recent -rc from Linus (5.3-rc5 as of
> today) as the base unless your patches depend on some changes that are not in
> there.
OK, So I need to implement on latest
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git branch master,
am I right?
However, I just checked v5.3-rc5 code and found it has the same HEAD (d1abaeb
Linux 5.3-rc5
on which I did not observe v5 patch apply conflict, did I miss something?
Thanks.
Regards,
Ran
Re: [PATCH v5 1/3] PM: wakeup: Add routine to help fetch wakeup source object.
On Mon, Aug 19, 2019 at 10:15 AM Ran Wang wrote:
>
> Hi Rafael,
>
> On Monday, August 05, 2019 17:59, Rafael J. Wysocki wrote:
> >
> > On Wednesday, July 24, 2019 9:47:20 AM CEST Ran Wang 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
> > > ---
> > > 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 | 24
> > > include/linux/pm_wakeup.h | 3 +++
> > > 2 files changed, 27 insertions(+)
> > >
> > > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> > > index ee31d4f..2fba891 100644
> > > --- a/drivers/base/power/wakeup.c
> > > +++ b/drivers/base/power/wakeup.c
> > > @@ -14,6 +14,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > >
> > > @@ -226,6 +227,28 @@ void wakeup_source_unregister(struct
> > wakeup_source *ws)
> > > }
> > > }
> > > EXPORT_SYMBOL_GPL(wakeup_source_unregister);
> > > +/**
> > > + * wakeup_source_get_next - Get next wakeup source from the list
> > > + * @ws: Previous wakeup source object, null means caller want first one.
> > > + */
> > > +struct wakeup_source *wakeup_source_get_next(struct wakeup_source
> > > +*ws) {
> > > + struct list_head *ws_head = _sources;
> > > + struct wakeup_source *next_ws = NULL;
> > > + int idx;
> > > +
> > > + idx = srcu_read_lock(_srcu);
> > > + if (ws)
> > > + next_ws = list_next_or_null_rcu(ws_head, >entry,
> > > + struct wakeup_source, entry);
> > > + else
> > > + next_ws = list_entry_rcu(ws_head->next,
> > > + struct wakeup_source, entry);
> > > + srcu_read_unlock(_srcu, idx);
> > > +
> >
> > This is incorrect.
> >
> > The SRCU cannot be unlocked until the caller of this is done with the object
> > returned by it, or that object can be freed while it is still being
> > accessed.
>
> Thanks for the comment. Looks like I was not fully understanding your point on
> v4 discussion. So I will implement 3 APIs by referring
> wakeup_sources_stats_seq_start/next/stop()
>
> > Besides, this patch conflicts with some general wakeup sources changes in
> > the
> > works, so it needs to be deferred and rebased on top of those changes.
>
> Could you please tell me which is the right code base I should developing on?
> I just tried applying v5 patch on latest
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git branch master
> (d1abaeb Linux 5.3-rc5)
> and no conflict encountered.
It is better to use the most recent -rc from Linus (5.3-rc5 as of
today) as the base unless your patches depend on some changes that are
not in there.
RE: [PATCH v5 1/3] PM: wakeup: Add routine to help fetch wakeup source object.
Hi Rafael,
On Monday, August 05, 2019 17:59, Rafael J. Wysocki wrote:
>
> On Wednesday, July 24, 2019 9:47:20 AM CEST Ran Wang 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
> > ---
> > 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 | 24
> > include/linux/pm_wakeup.h | 3 +++
> > 2 files changed, 27 insertions(+)
> >
> > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> > index ee31d4f..2fba891 100644
> > --- a/drivers/base/power/wakeup.c
> > +++ b/drivers/base/power/wakeup.c
> > @@ -14,6 +14,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -226,6 +227,28 @@ void wakeup_source_unregister(struct
> wakeup_source *ws)
> > }
> > }
> > EXPORT_SYMBOL_GPL(wakeup_source_unregister);
> > +/**
> > + * wakeup_source_get_next - Get next wakeup source from the list
> > + * @ws: Previous wakeup source object, null means caller want first one.
> > + */
> > +struct wakeup_source *wakeup_source_get_next(struct wakeup_source
> > +*ws) {
> > + struct list_head *ws_head = _sources;
> > + struct wakeup_source *next_ws = NULL;
> > + int idx;
> > +
> > + idx = srcu_read_lock(_srcu);
> > + if (ws)
> > + next_ws = list_next_or_null_rcu(ws_head, >entry,
> > + struct wakeup_source, entry);
> > + else
> > + next_ws = list_entry_rcu(ws_head->next,
> > + struct wakeup_source, entry);
> > + srcu_read_unlock(_srcu, idx);
> > +
>
> This is incorrect.
>
> The SRCU cannot be unlocked until the caller of this is done with the object
> returned by it, or that object can be freed while it is still being accessed.
Thanks for the comment. Looks like I was not fully understanding your point on
v4 discussion. So I will implement 3 APIs by referring
wakeup_sources_stats_seq_start/next/stop()
> Besides, this patch conflicts with some general wakeup sources changes in the
> works, so it needs to be deferred and rebased on top of those changes.
Could you please tell me which is the right code base I should developing on?
I just tried applying v5 patch on latest
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git branch master
(d1abaeb Linux 5.3-rc5)
and no conflict encountered.
Thanks & Regards,
Ran
Re: [PATCH v5 1/3] PM: wakeup: Add routine to help fetch wakeup source object.
On Wednesday, July 24, 2019 9:47:20 AM CEST Ran Wang 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
> ---
> 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 | 24
> include/linux/pm_wakeup.h | 3 +++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index ee31d4f..2fba891 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -14,6 +14,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -226,6 +227,28 @@ void wakeup_source_unregister(struct wakeup_source *ws)
> }
> }
> EXPORT_SYMBOL_GPL(wakeup_source_unregister);
> +/**
> + * wakeup_source_get_next - Get next wakeup source from the list
> + * @ws: Previous wakeup source object, null means caller want first one.
> + */
> +struct wakeup_source *wakeup_source_get_next(struct wakeup_source *ws)
> +{
> + struct list_head *ws_head = _sources;
> + struct wakeup_source *next_ws = NULL;
> + int idx;
> +
> + idx = srcu_read_lock(_srcu);
> + if (ws)
> + next_ws = list_next_or_null_rcu(ws_head, >entry,
> + struct wakeup_source, entry);
> + else
> + next_ws = list_entry_rcu(ws_head->next,
> + struct wakeup_source, entry);
> + srcu_read_unlock(_srcu, idx);
> +
This is incorrect.
The SRCU cannot be unlocked until the caller of this is done
with the object returned by it, or that object can be freed
while it is still being accessed.
Besides, this patch conflicts with some general wakeup sources
changes in the works, so it needs to be deferred and rebased on
top of those changes.
> + return next_ws;
> +}
> +EXPORT_SYMBOL_GPL(wakeup_source_get_next);
>
> /**
> * device_wakeup_attach - Attach a wakeup source object to a device object.
> @@ -242,6 +265,7 @@ static int device_wakeup_attach(struct device *dev,
> struct wakeup_source *ws)
> return -EEXIST;
> }
> dev->power.wakeup = ws;
> + ws->dev = dev;
> if (dev->power.wakeirq)
> device_wakeup_attach_irq(dev, dev->power.wakeirq);
> spin_unlock_irq(>power.lock);
> diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
> index 9102760..fc23c1a 100644
> --- a/include/linux/pm_wakeup.h
> +++ b/include/linux/pm_wakeup.h
> @@ -23,6 +23,7 @@ struct wake_irq;
> * @name: Name of the wakeup source
> * @entry: Wakeup source list entry
> * @lock: Wakeup source lock
> + * @dev: The device it attached to
> * @wakeirq: Optional device specific wakeirq
> * @timer: Wakeup timer list
> * @timer_expires: Wakeup timer expiration
> @@ -42,6 +43,7 @@ struct wakeup_source {
> const char *name;
> struct list_headentry;
> spinlock_t lock;
> + struct device *dev;
> struct wake_irq *wakeirq;
> struct timer_list timer;
> unsigned long timer_expires;
> @@ -88,6 +90,7 @@ extern void wakeup_source_add(struct wakeup_source *ws);
> extern void wakeup_source_remove(struct wakeup_source *ws);
> extern struct wakeup_source *wakeup_source_register(const char *name);
> extern void wakeup_source_unregister(struct wakeup_source *ws);
> +extern struct wakeup_source *wakeup_source_get_next(struct wakeup_source
> *ws);
> extern int device_wakeup_enable(struct device *dev);
> extern int device_wakeup_disable(struct device *dev);
> extern void device_set_wakeup_capable(struct device *dev, bool capable);
>
[PATCH v5 1/3] PM: wakeup: Add routine to help fetch wakeup source object.
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
---
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 | 24
include/linux/pm_wakeup.h | 3 +++
2 files changed, 27 insertions(+)
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index ee31d4f..2fba891 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
@@ -226,6 +227,28 @@ void wakeup_source_unregister(struct wakeup_source *ws)
}
}
EXPORT_SYMBOL_GPL(wakeup_source_unregister);
+/**
+ * wakeup_source_get_next - Get next wakeup source from the list
+ * @ws: Previous wakeup source object, null means caller want first one.
+ */
+struct wakeup_source *wakeup_source_get_next(struct wakeup_source *ws)
+{
+ struct list_head *ws_head = _sources;
+ struct wakeup_source *next_ws = NULL;
+ int idx;
+
+ idx = srcu_read_lock(_srcu);
+ if (ws)
+ next_ws = list_next_or_null_rcu(ws_head, >entry,
+ struct wakeup_source, entry);
+ else
+ next_ws = list_entry_rcu(ws_head->next,
+ struct wakeup_source, entry);
+ srcu_read_unlock(_srcu, idx);
+
+ return next_ws;
+}
+EXPORT_SYMBOL_GPL(wakeup_source_get_next);
/**
* device_wakeup_attach - Attach a wakeup source object to a device object.
@@ -242,6 +265,7 @@ static int device_wakeup_attach(struct device *dev, struct
wakeup_source *ws)
return -EEXIST;
}
dev->power.wakeup = ws;
+ ws->dev = dev;
if (dev->power.wakeirq)
device_wakeup_attach_irq(dev, dev->power.wakeirq);
spin_unlock_irq(>power.lock);
diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
index 9102760..fc23c1a 100644
--- a/include/linux/pm_wakeup.h
+++ b/include/linux/pm_wakeup.h
@@ -23,6 +23,7 @@ struct wake_irq;
* @name: Name of the wakeup source
* @entry: Wakeup source list entry
* @lock: Wakeup source lock
+ * @dev: The device it attached to
* @wakeirq: Optional device specific wakeirq
* @timer: Wakeup timer list
* @timer_expires: Wakeup timer expiration
@@ -42,6 +43,7 @@ struct wakeup_source {
const char *name;
struct list_headentry;
spinlock_t lock;
+ struct device *dev;
struct wake_irq *wakeirq;
struct timer_list timer;
unsigned long timer_expires;
@@ -88,6 +90,7 @@ extern void wakeup_source_add(struct wakeup_source *ws);
extern void wakeup_source_remove(struct wakeup_source *ws);
extern struct wakeup_source *wakeup_source_register(const char *name);
extern void wakeup_source_unregister(struct wakeup_source *ws);
+extern struct wakeup_source *wakeup_source_get_next(struct wakeup_source *ws);
extern int device_wakeup_enable(struct device *dev);
extern int device_wakeup_disable(struct device *dev);
extern void device_set_wakeup_capable(struct device *dev, bool capable);
--
2.7.4
