Re: [PATCH v4 1/2] remoteproc: qcom: Add per subsystem SSR notification
On Sat 20 Jun 12:48 PDT 2020, risha...@codeaurora.org wrote:
> On 2020-06-18 16:35, Bjorn Andersson wrote:
> > On Thu 18 Jun 16:00 PDT 2020, Alex Elder wrote:
> >
> > > On 5/27/20 10:34 PM, Rishabh Bhatnagar wrote:
> > > > Currently there is a single notification chain which is called whenever
> > > > any
> > > > remoteproc shuts down. This leads to all the listeners being notified,
> > > > and
> > > > is not an optimal design as kernel drivers might only be interested in
> > > > listening to notifications from a particular remoteproc. Create a global
> > > > list of remoteproc notification info data structures. This will hold the
> > > > name and notifier_list information for a particular remoteproc. The API
> > > > to register for notifications will use name argument to retrieve the
> > > > notification info data structure and the notifier block will be added to
> > > > that data structure's notification chain. Also move from blocking
> > > > notifier
> > > > to srcu notifer based implementation to support dynamic notifier head
> > > > creation.
> > >
> > > I'm looking at these patches now, without having reviewed the
> > > previous versions. Forgive me if I contradict or duplicate
> > > previous feedback.
> > >
> > > I have a number of suggestions, below.
> > >
> > > -Alex
> > >
> >
> > Thanks for your review Alex, some feedback on the patch and your
> > response below.
> >
> > > > Signed-off-by: Siddharth Gupta
> > > > Signed-off-by: Rishabh Bhatnagar
> > > > ---
> > > > drivers/remoteproc/qcom_common.c | 84
> > > > ++-
> > > > drivers/remoteproc/qcom_common.h | 5 ++-
> > > > include/linux/remoteproc/qcom_rproc.h | 20 ++---
> > > > 3 files changed, 90 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/drivers/remoteproc/qcom_common.c
> > > > b/drivers/remoteproc/qcom_common.c
> > > > index 9028cea..61ff2dd 100644
> > > > --- a/drivers/remoteproc/qcom_common.c
> > > > +++ b/drivers/remoteproc/qcom_common.c
> > > > @@ -12,6 +12,7 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > @@ -23,7 +24,14 @@
> > > > #define to_smd_subdev(d) container_of(d, struct qcom_rproc_subdev,
> > > > subdev)
> > > > #define to_ssr_subdev(d) container_of(d, struct qcom_rproc_ssr,
> > > > subdev)
> > > > -static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
> > > > +struct qcom_ssr_subsystem {
> > > > + const char *name;
> > > > + struct srcu_notifier_head notifier_list;
> > > > + struct list_head list;
> > > > +};
> > > > +
> > > > +static LIST_HEAD(qcom_ssr_subsystem_list);
> > > > +DEFINE_MUTEX(qcom_ssr_subsys_lock);
> > >
> > > There is no need for this mutex to be global.
> > >
> > > > static int glink_subdev_start(struct rproc_subdev *subdev)
> > > > {
> > > > @@ -189,39 +197,79 @@ void qcom_remove_smd_subdev(struct rproc *rproc,
> > > > struct qcom_rproc_subdev *smd)
> > > > }
> > > > EXPORT_SYMBOL_GPL(qcom_remove_smd_subdev);
> > > > +struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
> > >
> > > This function should be made private (static).
> > >
> >
> > Yes.
> >
> > > I think the mutex should be taken in this function rather than
> > > the caller (more on this below). But if you leave it this way,
> > > please mention something in a comment that indicates the caller
> > > must hold the qcom_ssr_subsys_lock mutex.
> > >
> >
> > I agree, that would simplify reasoning about the lock.
> >
> > > > +{
> > > > + struct qcom_ssr_subsystem *info;
> > > > +
> > > > + /* Match in the global qcom_ssr_subsystem_list with name */
> > > > + list_for_each_entry(info, _ssr_subsystem_list, list) {
> > > > + if (!strcmp(info->name, name))
> > > > + return info;
> > >
> > > This probably isn't strictly necessary, because you are
> > > returning a void pointer, but you could do this here:
> > > return ERR_CAST(info);
> >
> > Info is a struct qcom_ssr_subsystem * and that's the function's return
> > type as well, so Rishabh's approach looks correct to me.
> >
> > >
> > > > + }
> > >
> > > This is purely a style thing, but the curly braces around
> > > the loop body aren't necessary.
> > >
> > > > + info = kzalloc(sizeof(*info), GFP_KERNEL);
> > > > + if (!info)
> > > > + return ERR_PTR(-ENOMEM);
> > > > + info->name = kstrdup_const(name, GFP_KERNEL);
> > > > + srcu_init_notifier_head(>notifier_list);
> > > > +
> > > > + /* Add to global notif list */
> > >
> > > s/notif/notification/
> > >
> > > > + INIT_LIST_HEAD(>list);
> > >
> > > No need to initialize the list element when adding it
> > > to a list. Both uts fields will be overwritten anyway.
> > >
> > > > + list_add_tail(>list, _ssr_subsystem_list);
> > > > +
> > > > + return
Re: [PATCH v4 1/2] remoteproc: qcom: Add per subsystem SSR notification
On 2020-06-18 16:35, Bjorn Andersson wrote:
On Thu 18 Jun 16:00 PDT 2020, Alex Elder wrote:
On 5/27/20 10:34 PM, Rishabh Bhatnagar wrote:
> Currently there is a single notification chain which is called whenever any
> remoteproc shuts down. This leads to all the listeners being notified, and
> is not an optimal design as kernel drivers might only be interested in
> listening to notifications from a particular remoteproc. Create a global
> list of remoteproc notification info data structures. This will hold the
> name and notifier_list information for a particular remoteproc. The API
> to register for notifications will use name argument to retrieve the
> notification info data structure and the notifier block will be added to
> that data structure's notification chain. Also move from blocking notifier
> to srcu notifer based implementation to support dynamic notifier head
> creation.
I'm looking at these patches now, without having reviewed the
previous versions. Forgive me if I contradict or duplicate
previous feedback.
I have a number of suggestions, below.
-Alex
Thanks for your review Alex, some feedback on the patch and your
response below.
> Signed-off-by: Siddharth Gupta
> Signed-off-by: Rishabh Bhatnagar
> ---
> drivers/remoteproc/qcom_common.c | 84
++-
> drivers/remoteproc/qcom_common.h | 5 ++-
> include/linux/remoteproc/qcom_rproc.h | 20 ++---
> 3 files changed, 90 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_common.c
b/drivers/remoteproc/qcom_common.c
> index 9028cea..61ff2dd 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -23,7 +24,14 @@
> #define to_smd_subdev(d) container_of(d, struct qcom_rproc_subdev, subdev)
> #define to_ssr_subdev(d) container_of(d, struct qcom_rproc_ssr, subdev)
> -static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
> +struct qcom_ssr_subsystem {
> + const char *name;
> + struct srcu_notifier_head notifier_list;
> + struct list_head list;
> +};
> +
> +static LIST_HEAD(qcom_ssr_subsystem_list);
> +DEFINE_MUTEX(qcom_ssr_subsys_lock);
There is no need for this mutex to be global.
> static int glink_subdev_start(struct rproc_subdev *subdev)
> {
> @@ -189,39 +197,79 @@ void qcom_remove_smd_subdev(struct rproc *rproc, struct
qcom_rproc_subdev *smd)
> }
> EXPORT_SYMBOL_GPL(qcom_remove_smd_subdev);
> +struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
This function should be made private (static).
Yes.
I think the mutex should be taken in this function rather than
the caller (more on this below). But if you leave it this way,
please mention something in a comment that indicates the caller
must hold the qcom_ssr_subsys_lock mutex.
I agree, that would simplify reasoning about the lock.
> +{
> + struct qcom_ssr_subsystem *info;
> +
> + /* Match in the global qcom_ssr_subsystem_list with name */
> + list_for_each_entry(info, _ssr_subsystem_list, list) {
> + if (!strcmp(info->name, name))
> + return info;
This probably isn't strictly necessary, because you are
returning a void pointer, but you could do this here:
return ERR_CAST(info);
Info is a struct qcom_ssr_subsystem * and that's the function's return
type as well, so Rishabh's approach looks correct to me.
> + }
This is purely a style thing, but the curly braces around
the loop body aren't necessary.
> + info = kzalloc(sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return ERR_PTR(-ENOMEM);
> + info->name = kstrdup_const(name, GFP_KERNEL);
> + srcu_init_notifier_head(>notifier_list);
> +
> + /* Add to global notif list */
s/notif/notification/
> + INIT_LIST_HEAD(>list);
No need to initialize the list element when adding it
to a list. Both uts fields will be overwritten anyway.
> + list_add_tail(>list, _ssr_subsystem_list);
> +
> + return info;
> +}
> +
> /**
>* qcom_register_ssr_notifier() - register SSR notification handler
> + * @name: name that will be searched in global ssr subsystem list
Maybe just "SSR subsystem name".
>* @nb: notifier_block to notify for restart notifications
Drop or modify "restart" in the comment line above.
>*
> - * Returns 0 on success, negative errno on failure.
> + * Returns a subsystem cookie on success, ERR_PTR on failure.
Maybe make the above a @Return: comment.
No @ in that, but "Return: foo" is the appropriate format.
>*
> - * This register the @notify function as handler for restart notifications.
As
> - * remote processors are stopped this function will be called, with the SSR
> - * name passed as a parameter.
> + * This registers the @nb notifier block as part the notifier chain for a
> + * remoteproc associated with @name. The notifier
Re: [PATCH v4 1/2] remoteproc: qcom: Add per subsystem SSR notification
On Thu 18 Jun 16:00 PDT 2020, Alex Elder wrote:
> On 5/27/20 10:34 PM, Rishabh Bhatnagar wrote:
> > Currently there is a single notification chain which is called whenever any
> > remoteproc shuts down. This leads to all the listeners being notified, and
> > is not an optimal design as kernel drivers might only be interested in
> > listening to notifications from a particular remoteproc. Create a global
> > list of remoteproc notification info data structures. This will hold the
> > name and notifier_list information for a particular remoteproc. The API
> > to register for notifications will use name argument to retrieve the
> > notification info data structure and the notifier block will be added to
> > that data structure's notification chain. Also move from blocking notifier
> > to srcu notifer based implementation to support dynamic notifier head
> > creation.
>
> I'm looking at these patches now, without having reviewed the
> previous versions. Forgive me if I contradict or duplicate
> previous feedback.
>
> I have a number of suggestions, below.
>
> -Alex
>
Thanks for your review Alex, some feedback on the patch and your
response below.
> > Signed-off-by: Siddharth Gupta
> > Signed-off-by: Rishabh Bhatnagar
> > ---
> > drivers/remoteproc/qcom_common.c | 84
> > ++-
> > drivers/remoteproc/qcom_common.h | 5 ++-
> > include/linux/remoteproc/qcom_rproc.h | 20 ++---
> > 3 files changed, 90 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/remoteproc/qcom_common.c
> > b/drivers/remoteproc/qcom_common.c
> > index 9028cea..61ff2dd 100644
> > --- a/drivers/remoteproc/qcom_common.c
> > +++ b/drivers/remoteproc/qcom_common.c
> > @@ -12,6 +12,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -23,7 +24,14 @@
> > #define to_smd_subdev(d) container_of(d, struct qcom_rproc_subdev, subdev)
> > #define to_ssr_subdev(d) container_of(d, struct qcom_rproc_ssr, subdev)
> > -static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
> > +struct qcom_ssr_subsystem {
> > + const char *name;
> > + struct srcu_notifier_head notifier_list;
> > + struct list_head list;
> > +};
> > +
> > +static LIST_HEAD(qcom_ssr_subsystem_list);
> > +DEFINE_MUTEX(qcom_ssr_subsys_lock);
>
> There is no need for this mutex to be global.
>
> > static int glink_subdev_start(struct rproc_subdev *subdev)
> > {
> > @@ -189,39 +197,79 @@ void qcom_remove_smd_subdev(struct rproc *rproc,
> > struct qcom_rproc_subdev *smd)
> > }
> > EXPORT_SYMBOL_GPL(qcom_remove_smd_subdev);
> > +struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
>
> This function should be made private (static).
>
Yes.
> I think the mutex should be taken in this function rather than
> the caller (more on this below). But if you leave it this way,
> please mention something in a comment that indicates the caller
> must hold the qcom_ssr_subsys_lock mutex.
>
I agree, that would simplify reasoning about the lock.
> > +{
> > + struct qcom_ssr_subsystem *info;
> > +
> > + /* Match in the global qcom_ssr_subsystem_list with name */
> > + list_for_each_entry(info, _ssr_subsystem_list, list) {
> > + if (!strcmp(info->name, name))
> > + return info;
>
> This probably isn't strictly necessary, because you are
> returning a void pointer, but you could do this here:
> return ERR_CAST(info);
Info is a struct qcom_ssr_subsystem * and that's the function's return
type as well, so Rishabh's approach looks correct to me.
>
> > + }
>
> This is purely a style thing, but the curly braces around
> the loop body aren't necessary.
>
> > + info = kzalloc(sizeof(*info), GFP_KERNEL);
> > + if (!info)
> > + return ERR_PTR(-ENOMEM);
> > + info->name = kstrdup_const(name, GFP_KERNEL);
> > + srcu_init_notifier_head(>notifier_list);
> > +
> > + /* Add to global notif list */
>
> s/notif/notification/
>
> > + INIT_LIST_HEAD(>list);
>
> No need to initialize the list element when adding it
> to a list. Both uts fields will be overwritten anyway.
>
> > + list_add_tail(>list, _ssr_subsystem_list);
> > +
> > + return info;
> > +}
> > +
> > /**
> >* qcom_register_ssr_notifier() - register SSR notification handler
> > + * @name: name that will be searched in global ssr subsystem list
>
> Maybe just "SSR subsystem name".
>
> >* @nb: notifier_block to notify for restart notifications
>
> Drop or modify "restart" in the comment line above.
>
> >*
> > - * Returns 0 on success, negative errno on failure.
> > + * Returns a subsystem cookie on success, ERR_PTR on failure.
>
> Maybe make the above a @Return: comment.
>
No @ in that, but "Return: foo" is the appropriate format.
> >*
> > - * This register the @notify function as handler for restart
> > notifications. As
> > - * remote
Re: [PATCH v4 1/2] remoteproc: qcom: Add per subsystem SSR notification
On 5/27/20 10:34 PM, Rishabh Bhatnagar wrote:
Currently there is a single notification chain which is called whenever any
remoteproc shuts down. This leads to all the listeners being notified, and
is not an optimal design as kernel drivers might only be interested in
listening to notifications from a particular remoteproc. Create a global
list of remoteproc notification info data structures. This will hold the
name and notifier_list information for a particular remoteproc. The API
to register for notifications will use name argument to retrieve the
notification info data structure and the notifier block will be added to
that data structure's notification chain. Also move from blocking notifier
to srcu notifer based implementation to support dynamic notifier head
creation.
I'm looking at these patches now, without having reviewed the
previous versions. Forgive me if I contradict or duplicate
previous feedback.
I have a number of suggestions, below.
-Alex
Signed-off-by: Siddharth Gupta
Signed-off-by: Rishabh Bhatnagar
---
drivers/remoteproc/qcom_common.c | 84 ++-
drivers/remoteproc/qcom_common.h | 5 ++-
include/linux/remoteproc/qcom_rproc.h | 20 ++---
3 files changed, 90 insertions(+), 19 deletions(-)
diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 9028cea..61ff2dd 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -23,7 +24,14 @@
#define to_smd_subdev(d) container_of(d, struct qcom_rproc_subdev, subdev)
#define to_ssr_subdev(d) container_of(d, struct qcom_rproc_ssr, subdev)
-static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
+struct qcom_ssr_subsystem {
+ const char *name;
+ struct srcu_notifier_head notifier_list;
+ struct list_head list;
+};
+
+static LIST_HEAD(qcom_ssr_subsystem_list);
+DEFINE_MUTEX(qcom_ssr_subsys_lock);
There is no need for this mutex to be global.
static int glink_subdev_start(struct rproc_subdev *subdev)
{
@@ -189,39 +197,79 @@ void qcom_remove_smd_subdev(struct rproc *rproc, struct
qcom_rproc_subdev *smd)
}
EXPORT_SYMBOL_GPL(qcom_remove_smd_subdev);
+struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
This function should be made private (static).
I think the mutex should be taken in this function rather than
the caller (more on this below). But if you leave it this way,
please mention something in a comment that indicates the caller
must hold the qcom_ssr_subsys_lock mutex.
+{
+ struct qcom_ssr_subsystem *info;
+
+ /* Match in the global qcom_ssr_subsystem_list with name */
+ list_for_each_entry(info, _ssr_subsystem_list, list) {
+ if (!strcmp(info->name, name))
+ return info;
This probably isn't strictly necessary, because you are
returning a void pointer, but you could do this here:
return ERR_CAST(info);
+ }
This is purely a style thing, but the curly braces around
the loop body aren't necessary.
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return ERR_PTR(-ENOMEM);
+ info->name = kstrdup_const(name, GFP_KERNEL);
+ srcu_init_notifier_head(>notifier_list);
+
+ /* Add to global notif list */
s/notif/notification/
+ INIT_LIST_HEAD(>list);
No need to initialize the list element when adding it
to a list. Both uts fields will be overwritten anyway.
+ list_add_tail(>list, _ssr_subsystem_list);
+
+ return info;
+}
+
/**
* qcom_register_ssr_notifier() - register SSR notification handler
+ * @name: name that will be searched in global ssr subsystem list
Maybe just "SSR subsystem name".
* @nb: notifier_block to notify for restart notifications
Drop or modify "restart" in the comment line above.
*
- * Returns 0 on success, negative errno on failure.
+ * Returns a subsystem cookie on success, ERR_PTR on failure.
Maybe make the above a @Return: comment.
*
- * This register the @notify function as handler for restart notifications. As
- * remote processors are stopped this function will be called, with the SSR
- * name passed as a parameter.
+ * This registers the @nb notifier block as part the notifier chain for a
+ * remoteproc associated with @name. The notifier block's callback
+ * will be invoked when the particular remote processor is stopped.
It's not just for stopping, right? Maybe something
more like:
Register to receive notification callbacks when
remoteproc SSR events occur (pre- and post-startup
and pre- and post-shutdown).
*/
-int qcom_register_ssr_notifier(struct notifier_block *nb)
+void *qcom_register_ssr_notifier(const char *name, struct notifier_block *nb)
{
- return
Re: [PATCH v4 1/2] remoteproc: qcom: Add per subsystem SSR notification
Hi Rishabh,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on next-20200526]
[also build test WARNING on v5.7-rc7]
[cannot apply to linus/master linux/master v5.7-rc7 v5.7-rc6 v5.7-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Rishabh-Bhatnagar/Extend-SSR-notifications-framework/20200528-115948
base:b0523c7b1c9d0edcd6c0fe6d2cb558a9ad5c60a8
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> drivers/remoteproc/qcom_common.c:200:28: warning: no previous prototype for
>> 'qcom_ssr_get_subsys' [-Wmissing-prototypes]
200 | struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
|^~~
vim +/qcom_ssr_get_subsys +200 drivers/remoteproc/qcom_common.c
199
> 200 struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
201 {
202 struct qcom_ssr_subsystem *info;
203
204 /* Match in the global qcom_ssr_subsystem_list with name */
205 list_for_each_entry(info, _ssr_subsystem_list, list) {
206 if (!strcmp(info->name, name))
207 return info;
208 }
209 info = kzalloc(sizeof(*info), GFP_KERNEL);
210 if (!info)
211 return ERR_PTR(-ENOMEM);
212 info->name = kstrdup_const(name, GFP_KERNEL);
213 srcu_init_notifier_head(>notifier_list);
214
215 /* Add to global notif list */
216 INIT_LIST_HEAD(>list);
217 list_add_tail(>list, _ssr_subsystem_list);
218
219 return info;
220 }
221
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[PATCH v4 1/2] remoteproc: qcom: Add per subsystem SSR notification
Currently there is a single notification chain which is called whenever any
remoteproc shuts down. This leads to all the listeners being notified, and
is not an optimal design as kernel drivers might only be interested in
listening to notifications from a particular remoteproc. Create a global
list of remoteproc notification info data structures. This will hold the
name and notifier_list information for a particular remoteproc. The API
to register for notifications will use name argument to retrieve the
notification info data structure and the notifier block will be added to
that data structure's notification chain. Also move from blocking notifier
to srcu notifer based implementation to support dynamic notifier head
creation.
Signed-off-by: Siddharth Gupta
Signed-off-by: Rishabh Bhatnagar
---
drivers/remoteproc/qcom_common.c | 84 ++-
drivers/remoteproc/qcom_common.h | 5 ++-
include/linux/remoteproc/qcom_rproc.h | 20 ++---
3 files changed, 90 insertions(+), 19 deletions(-)
diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 9028cea..61ff2dd 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -23,7 +24,14 @@
#define to_smd_subdev(d) container_of(d, struct qcom_rproc_subdev, subdev)
#define to_ssr_subdev(d) container_of(d, struct qcom_rproc_ssr, subdev)
-static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
+struct qcom_ssr_subsystem {
+ const char *name;
+ struct srcu_notifier_head notifier_list;
+ struct list_head list;
+};
+
+static LIST_HEAD(qcom_ssr_subsystem_list);
+DEFINE_MUTEX(qcom_ssr_subsys_lock);
static int glink_subdev_start(struct rproc_subdev *subdev)
{
@@ -189,39 +197,79 @@ void qcom_remove_smd_subdev(struct rproc *rproc, struct
qcom_rproc_subdev *smd)
}
EXPORT_SYMBOL_GPL(qcom_remove_smd_subdev);
+struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
+{
+ struct qcom_ssr_subsystem *info;
+
+ /* Match in the global qcom_ssr_subsystem_list with name */
+ list_for_each_entry(info, _ssr_subsystem_list, list) {
+ if (!strcmp(info->name, name))
+ return info;
+ }
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return ERR_PTR(-ENOMEM);
+ info->name = kstrdup_const(name, GFP_KERNEL);
+ srcu_init_notifier_head(>notifier_list);
+
+ /* Add to global notif list */
+ INIT_LIST_HEAD(>list);
+ list_add_tail(>list, _ssr_subsystem_list);
+
+ return info;
+}
+
/**
* qcom_register_ssr_notifier() - register SSR notification handler
+ * @name: name that will be searched in global ssr subsystem list
* @nb:notifier_block to notify for restart notifications
*
- * Returns 0 on success, negative errno on failure.
+ * Returns a subsystem cookie on success, ERR_PTR on failure.
*
- * This register the @notify function as handler for restart notifications. As
- * remote processors are stopped this function will be called, with the SSR
- * name passed as a parameter.
+ * This registers the @nb notifier block as part the notifier chain for a
+ * remoteproc associated with @name. The notifier block's callback
+ * will be invoked when the particular remote processor is stopped.
*/
-int qcom_register_ssr_notifier(struct notifier_block *nb)
+void *qcom_register_ssr_notifier(const char *name, struct notifier_block *nb)
{
- return blocking_notifier_chain_register(_notifiers, nb);
+ struct qcom_ssr_subsystem *info;
+
+ mutex_lock(_ssr_subsys_lock);
+ info = qcom_ssr_get_subsys(name);
+ if (IS_ERR(info)) {
+ mutex_unlock(_ssr_subsys_lock);
+ return info;
+ }
+
+ srcu_notifier_chain_register(>notifier_list, nb);
+ mutex_unlock(_ssr_subsys_lock);
+ return >notifier_list;
}
EXPORT_SYMBOL_GPL(qcom_register_ssr_notifier);
/**
* qcom_unregister_ssr_notifier() - unregister SSR notification handler
+ * @notify:subsystem coookie returned from qcom_register_ssr_notifier
* @nb:notifier_block to unregister
*/
-void qcom_unregister_ssr_notifier(struct notifier_block *nb)
+int qcom_unregister_ssr_notifier(void *notify, struct notifier_block *nb)
{
- blocking_notifier_chain_unregister(_notifiers, nb);
+ return srcu_notifier_chain_unregister(notify, nb);
}
EXPORT_SYMBOL_GPL(qcom_unregister_ssr_notifier);
static void ssr_notify_unprepare(struct rproc_subdev *subdev)
{
struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
+ struct qcom_ssr_notif_data data = {
+ .name = ssr->info->name,
+ .crashed = false,
+ };
- blocking_notifier_call_chain(_notifiers, 0, (void *)ssr->name);
+ srcu_notifier_call_chain(>info->notifier_list, 0, );
}
+
/**
*
