linux-remoteproc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/2] Extend SSR notifications framework
@ 2020-06-24  2:23 Rishabh Bhatnagar
  2020-06-24  2:23 ` [PATCH v6 1/2] remoteproc: qcom: Add per subsystem SSR notification Rishabh Bhatnagar
  2020-06-24  2:23 ` [PATCH v6 2/2] remoteproc: qcom: Add notification types to SSR Rishabh Bhatnagar
  0 siblings, 2 replies; 7+ messages in thread
From: Rishabh Bhatnagar @ 2020-06-24  2:23 UTC (permalink / raw)
  To: linux-remoteproc, linux-kernel
  Cc: bjorn.andersson, tsoni, psodagud, sidgup, elder, Rishabh Bhatnagar

This set of patches gives kernel client drivers the ability to register
for a particular remoteproc's SSR notifications. Also the notifications
are extended to before/after-powerup/shutdown stages.
It also fixes the bug where clients need to register for notifications
again if the platform driver is removed. This is done by creating a
global list of per-remoteproc notification info data structures which
remain static. An API is exported to register for a remoteproc's SSR
notifications and uses remoteproc's ssr_name and notifier block as
arguments.

Changelog:

v6 -> v5:
- Fix mutec locking and naming convention

v5 -> v4:
- Make qcom_ssr_get_subsys static function
- Fix mutex locking
- Fix function comments

v4 -> v3:
- Fix naming convention 

v3 -> v2:
- Create a global list of per remoteproc notification data structure
- Pass ssr_name and crashed information as part of notification data
- Move notification type enum to qcom_rproc.h from remoteproc.h

v2 -> v1:
- Fix commit text

Rishabh Bhatnagar (1):
  remoteproc: qcom: Add per subsystem SSR notification

Siddharth Gupta (1):
  remoteproc: qcom: Add notification types to SSR

 drivers/remoteproc/qcom_common.c      | 128 ++++++++++++++++++++++++++++++----
 drivers/remoteproc/qcom_common.h      |   5 +-
 include/linux/remoteproc/qcom_rproc.h |  36 ++++++++--
 3 files changed, 149 insertions(+), 20 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v6 1/2] remoteproc: qcom: Add per subsystem SSR notification
  2020-06-24  2:23 [PATCH v6 0/2] Extend SSR notifications framework Rishabh Bhatnagar
@ 2020-06-24  2:23 ` Rishabh Bhatnagar
  2020-07-08 23:56   ` Alex Elder
  2020-07-13 16:26   ` Jon Hunter
  2020-06-24  2:23 ` [PATCH v6 2/2] remoteproc: qcom: Add notification types to SSR Rishabh Bhatnagar
  1 sibling, 2 replies; 7+ messages in thread
From: Rishabh Bhatnagar @ 2020-06-24  2:23 UTC (permalink / raw)
  To: linux-remoteproc, linux-kernel
  Cc: bjorn.andersson, tsoni, psodagud, sidgup, elder, Rishabh Bhatnagar

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 <sidgup@codeaurora.org>
Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
---
 drivers/remoteproc/qcom_common.c      | 90 ++++++++++++++++++++++++++++++-----
 drivers/remoteproc/qcom_common.h      |  5 +-
 include/linux/remoteproc/qcom_rproc.h | 20 ++++++--
 3 files changed, 95 insertions(+), 20 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 9028cea..7a7384c 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/notifier.h>
 #include <linux/remoteproc.h>
+#include <linux/remoteproc/qcom_rproc.h>
 #include <linux/rpmsg/qcom_glink.h>
 #include <linux/rpmsg/qcom_smd.h>
 #include <linux/soc/qcom/mdt_loader.h>
@@ -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);
+static DEFINE_MUTEX(qcom_ssr_subsys_lock);
 
 static int glink_subdev_start(struct rproc_subdev *subdev)
 {
@@ -189,37 +197,83 @@ void qcom_remove_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
 }
 EXPORT_SYMBOL_GPL(qcom_remove_smd_subdev);
 
+static struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
+{
+	struct qcom_ssr_subsystem *info;
+
+	mutex_lock(&qcom_ssr_subsys_lock);
+	/* Match in the global qcom_ssr_subsystem_list with name */
+	list_for_each_entry(info, &qcom_ssr_subsystem_list, list)
+		if (!strcmp(info->name, name))
+			goto out;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info) {
+		info = ERR_PTR(-ENOMEM);
+		goto out;
+	}
+	info->name = kstrdup_const(name, GFP_KERNEL);
+	srcu_init_notifier_head(&info->notifier_list);
+
+	/* Add to global notification list */
+	list_add_tail(&info->list, &qcom_ssr_subsystem_list);
+
+out:
+	mutex_unlock(&qcom_ssr_subsys_lock);
+	return info;
+}
+
 /**
  * qcom_register_ssr_notifier() - register SSR notification handler
- * @nb:		notifier_block to notify for restart notifications
+ * @name:	Subsystem's SSR name
+ * @nb:		notifier_block to be invoked upon subsystem's state change
  *
- * Returns 0 on success, negative errno on failure.
+ * 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 remote processor's SSR events occur
+ * (pre/post startup and pre/post shutdown).
  *
- * 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.
+ * Return: a subsystem cookie on success, ERR_PTR on failure.
  */
-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(&ssr_notifiers, nb);
+	struct qcom_ssr_subsystem *info;
+
+	info = qcom_ssr_get_subsys(name);
+	if (IS_ERR(info))
+		return info;
+
+	srcu_notifier_chain_register(&info->notifier_list, nb);
+
+	return &info->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
+ *
+ * This function will unregister the notifier from the particular notifier
+ * chain.
+ *
+ * Return: 0 on success, %ENOENT otherwise.
  */
-void qcom_unregister_ssr_notifier(struct notifier_block *nb)
+int qcom_unregister_ssr_notifier(void *notify, struct notifier_block *nb)
 {
-	blocking_notifier_chain_unregister(&ssr_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_notify_data data = {
+		.name = ssr->info->name,
+		.crashed = false,
+	};
 
-	blocking_notifier_call_chain(&ssr_notifiers, 0, (void *)ssr->name);
+	srcu_notifier_call_chain(&ssr->info->notifier_list, 0, &data);
 }
 
 /**
@@ -229,12 +283,21 @@ static void ssr_notify_unprepare(struct rproc_subdev *subdev)
  * @ssr_name:	identifier to use for notifications originating from @rproc
  *
  * As the @ssr is registered with the @rproc SSR events will be sent to all
- * registered listeners in the system as the remoteproc is shut down.
+ * registered listeners for the remoteproc when it's SSR events occur
+ * (pre/post startup and pre/post shutdown).
  */
 void qcom_add_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr,
 			 const char *ssr_name)
 {
-	ssr->name = ssr_name;
+	struct qcom_ssr_subsystem *info;
+
+	info = qcom_ssr_get_subsys(ssr_name);
+	if (IS_ERR(info)) {
+		dev_err(&rproc->dev, "Failed to add ssr subdevice\n");
+		return;
+	}
+
+	ssr->info = info;
 	ssr->subdev.unprepare = ssr_notify_unprepare;
 
 	rproc_add_subdev(rproc, &ssr->subdev);
@@ -249,6 +312,7 @@ EXPORT_SYMBOL_GPL(qcom_add_ssr_subdev);
 void qcom_remove_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr)
 {
 	rproc_remove_subdev(rproc, &ssr->subdev);
+	ssr->info = NULL;
 }
 EXPORT_SYMBOL_GPL(qcom_remove_ssr_subdev);
 
diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
index 34e5188..dfc641c 100644
--- a/drivers/remoteproc/qcom_common.h
+++ b/drivers/remoteproc/qcom_common.h
@@ -26,10 +26,11 @@ struct qcom_rproc_subdev {
 	struct qcom_smd_edge *edge;
 };
 
+struct qcom_ssr_subsystem;
+
 struct qcom_rproc_ssr {
 	struct rproc_subdev subdev;
-
-	const char *name;
+	struct qcom_ssr_subsystem *info;
 };
 
 void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink,
diff --git a/include/linux/remoteproc/qcom_rproc.h b/include/linux/remoteproc/qcom_rproc.h
index fa8e386..2a1d6d0 100644
--- a/include/linux/remoteproc/qcom_rproc.h
+++ b/include/linux/remoteproc/qcom_rproc.h
@@ -5,17 +5,27 @@ struct notifier_block;
 
 #if IS_ENABLED(CONFIG_QCOM_RPROC_COMMON)
 
-int qcom_register_ssr_notifier(struct notifier_block *nb);
-void qcom_unregister_ssr_notifier(struct notifier_block *nb);
+struct qcom_ssr_notify_data {
+	const char *name;
+	bool crashed;
+};
+
+void *qcom_register_ssr_notifier(const char *name, struct notifier_block *nb);
+int qcom_unregister_ssr_notifier(void *notify, struct notifier_block *nb);
 
 #else
 
-static inline int qcom_register_ssr_notifier(struct notifier_block *nb)
+static inline void *qcom_register_ssr_notifier(const char *name,
+					       struct notifier_block *nb)
 {
-	return 0;
+	return NULL;
 }
 
-static inline void qcom_unregister_ssr_notifier(struct notifier_block *nb) {}
+static inline int qcom_unregister_ssr_notifier(void *notify,
+					       struct notifier_block *nb)
+{
+	return 0;
+}
 
 #endif
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v6 2/2] remoteproc: qcom: Add notification types to SSR
  2020-06-24  2:23 [PATCH v6 0/2] Extend SSR notifications framework Rishabh Bhatnagar
  2020-06-24  2:23 ` [PATCH v6 1/2] remoteproc: qcom: Add per subsystem SSR notification Rishabh Bhatnagar
@ 2020-06-24  2:23 ` Rishabh Bhatnagar
  2020-07-08 23:56   ` Alex Elder
  1 sibling, 1 reply; 7+ messages in thread
From: Rishabh Bhatnagar @ 2020-06-24  2:23 UTC (permalink / raw)
  To: linux-remoteproc, linux-kernel
  Cc: bjorn.andersson, tsoni, psodagud, sidgup, elder, Rishabh Bhatnagar

The SSR subdevice only adds callback for the unprepare event. Add callbacks
for prepare, start and prepare events. The client driver for a particular
remoteproc might be interested in knowing the status of the remoteproc
while undergoing SSR, not just when the remoteproc has finished shutting
down.

Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
---
 drivers/remoteproc/qcom_common.c      | 44 ++++++++++++++++++++++++++++++++++-
 include/linux/remoteproc/qcom_rproc.h | 16 +++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 7a7384c..7ec4597 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -265,6 +265,44 @@ int qcom_unregister_ssr_notifier(void *notify, struct notifier_block *nb)
 }
 EXPORT_SYMBOL_GPL(qcom_unregister_ssr_notifier);
 
+static int ssr_notify_prepare(struct rproc_subdev *subdev)
+{
+	struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
+	struct qcom_ssr_notify_data data = {
+		.name = ssr->info->name,
+		.crashed = false,
+	};
+
+	srcu_notifier_call_chain(&ssr->info->notifier_list,
+				 QCOM_SSR_BEFORE_POWERUP, &data);
+	return 0;
+}
+
+static int ssr_notify_start(struct rproc_subdev *subdev)
+{
+	struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
+	struct qcom_ssr_notify_data data = {
+		.name = ssr->info->name,
+		.crashed = false,
+	};
+
+	srcu_notifier_call_chain(&ssr->info->notifier_list,
+				 QCOM_SSR_AFTER_POWERUP, &data);
+	return 0;
+}
+
+static void ssr_notify_stop(struct rproc_subdev *subdev, bool crashed)
+{
+	struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
+	struct qcom_ssr_notify_data data = {
+		.name = ssr->info->name,
+		.crashed = crashed,
+	};
+
+	srcu_notifier_call_chain(&ssr->info->notifier_list,
+				 QCOM_SSR_BEFORE_SHUTDOWN, &data);
+}
+
 static void ssr_notify_unprepare(struct rproc_subdev *subdev)
 {
 	struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
@@ -273,7 +311,8 @@ static void ssr_notify_unprepare(struct rproc_subdev *subdev)
 		.crashed = false,
 	};
 
-	srcu_notifier_call_chain(&ssr->info->notifier_list, 0, &data);
+	srcu_notifier_call_chain(&ssr->info->notifier_list,
+				 QCOM_SSR_AFTER_SHUTDOWN, &data);
 }
 
 /**
@@ -298,6 +337,9 @@ void qcom_add_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr,
 	}
 
 	ssr->info = info;
+	ssr->subdev.prepare = ssr_notify_prepare;
+	ssr->subdev.start = ssr_notify_start;
+	ssr->subdev.stop = ssr_notify_stop;
 	ssr->subdev.unprepare = ssr_notify_unprepare;
 
 	rproc_add_subdev(rproc, &ssr->subdev);
diff --git a/include/linux/remoteproc/qcom_rproc.h b/include/linux/remoteproc/qcom_rproc.h
index 2a1d6d0..6470516 100644
--- a/include/linux/remoteproc/qcom_rproc.h
+++ b/include/linux/remoteproc/qcom_rproc.h
@@ -5,6 +5,22 @@ struct notifier_block;
 
 #if IS_ENABLED(CONFIG_QCOM_RPROC_COMMON)
 
+/**
+ * enum qcom_ssr_notify_type - Startup/Shutdown events related to a remoteproc
+ * processor.
+ *
+ * @QCOM_SSR_BEFORE_POWERUP:	Remoteproc about to start (prepare stage)
+ * @QCOM_SSR_AFTER_POWERUP:	Remoteproc is running (start stage)
+ * @QCOM_SSR_BEFORE_SHUTDOWN:	Remoteproc crashed or shutting down (stop stage)
+ * @QCOM_SSR_AFTER_SHUTDOWN:	Remoteproc is down (unprepare stage)
+ */
+enum qcom_ssr_notify_type {
+	QCOM_SSR_BEFORE_POWERUP,
+	QCOM_SSR_AFTER_POWERUP,
+	QCOM_SSR_BEFORE_SHUTDOWN,
+	QCOM_SSR_AFTER_SHUTDOWN,
+};
+
 struct qcom_ssr_notify_data {
 	const char *name;
 	bool crashed;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 1/2] remoteproc: qcom: Add per subsystem SSR notification
  2020-06-24  2:23 ` [PATCH v6 1/2] remoteproc: qcom: Add per subsystem SSR notification Rishabh Bhatnagar
@ 2020-07-08 23:56   ` Alex Elder
  2020-07-13 16:26   ` Jon Hunter
  1 sibling, 0 replies; 7+ messages in thread
From: Alex Elder @ 2020-07-08 23:56 UTC (permalink / raw)
  To: Rishabh Bhatnagar, linux-remoteproc, linux-kernel
  Cc: bjorn.andersson, tsoni, psodagud, sidgup

On 6/23/20 9:23 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.

You misspelled "cookie" (coookie) but please fix that later.

I did not look as closely at this as I did last time.  However
I *did* do a quick implementation in the IPA driver of using
this instead of the IPA notification code, and it should be
a straightforward conversion.

I'm going to call this good...

Reviewed-by: Alex Elder <elder@linaro.org>



> 
> Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
> Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
> ---
>  drivers/remoteproc/qcom_common.c      | 90 ++++++++++++++++++++++++++++++-----
>  drivers/remoteproc/qcom_common.h      |  5 +-
>  include/linux/remoteproc/qcom_rproc.h | 20 ++++++--
>  3 files changed, 95 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 9028cea..7a7384c 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -12,6 +12,7 @@
>  #include <linux/module.h>
>  #include <linux/notifier.h>
>  #include <linux/remoteproc.h>
> +#include <linux/remoteproc/qcom_rproc.h>
>  #include <linux/rpmsg/qcom_glink.h>
>  #include <linux/rpmsg/qcom_smd.h>
>  #include <linux/soc/qcom/mdt_loader.h>
> @@ -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);
> +static DEFINE_MUTEX(qcom_ssr_subsys_lock);
>  
>  static int glink_subdev_start(struct rproc_subdev *subdev)
>  {
> @@ -189,37 +197,83 @@ void qcom_remove_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
>  }
>  EXPORT_SYMBOL_GPL(qcom_remove_smd_subdev);
>  
> +static struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
> +{
> +	struct qcom_ssr_subsystem *info;
> +
> +	mutex_lock(&qcom_ssr_subsys_lock);
> +	/* Match in the global qcom_ssr_subsystem_list with name */
> +	list_for_each_entry(info, &qcom_ssr_subsystem_list, list)
> +		if (!strcmp(info->name, name))
> +			goto out;
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info) {
> +		info = ERR_PTR(-ENOMEM);
> +		goto out;
> +	}
> +	info->name = kstrdup_const(name, GFP_KERNEL);
> +	srcu_init_notifier_head(&info->notifier_list);
> +
> +	/* Add to global notification list */
> +	list_add_tail(&info->list, &qcom_ssr_subsystem_list);
> +
> +out:
> +	mutex_unlock(&qcom_ssr_subsys_lock);
> +	return info;
> +}
> +
>  /**
>   * qcom_register_ssr_notifier() - register SSR notification handler
> - * @nb:		notifier_block to notify for restart notifications
> + * @name:	Subsystem's SSR name
> + * @nb:		notifier_block to be invoked upon subsystem's state change
>   *
> - * Returns 0 on success, negative errno on failure.
> + * 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 remote processor's SSR events occur
> + * (pre/post startup and pre/post shutdown).
>   *
> - * 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.
> + * Return: a subsystem cookie on success, ERR_PTR on failure.
>   */
> -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(&ssr_notifiers, nb);
> +	struct qcom_ssr_subsystem *info;
> +
> +	info = qcom_ssr_get_subsys(name);
> +	if (IS_ERR(info))
> +		return info;
> +
> +	srcu_notifier_chain_register(&info->notifier_list, nb);
> +
> +	return &info->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
> + *
> + * This function will unregister the notifier from the particular notifier
> + * chain.
> + *
> + * Return: 0 on success, %ENOENT otherwise.
>   */
> -void qcom_unregister_ssr_notifier(struct notifier_block *nb)
> +int qcom_unregister_ssr_notifier(void *notify, struct notifier_block *nb)
>  {
> -	blocking_notifier_chain_unregister(&ssr_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_notify_data data = {
> +		.name = ssr->info->name,
> +		.crashed = false,
> +	};
>  
> -	blocking_notifier_call_chain(&ssr_notifiers, 0, (void *)ssr->name);
> +	srcu_notifier_call_chain(&ssr->info->notifier_list, 0, &data);
>  }
>  
>  /**
> @@ -229,12 +283,21 @@ static void ssr_notify_unprepare(struct rproc_subdev *subdev)
>   * @ssr_name:	identifier to use for notifications originating from @rproc
>   *
>   * As the @ssr is registered with the @rproc SSR events will be sent to all
> - * registered listeners in the system as the remoteproc is shut down.
> + * registered listeners for the remoteproc when it's SSR events occur
> + * (pre/post startup and pre/post shutdown).
>   */
>  void qcom_add_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr,
>  			 const char *ssr_name)
>  {
> -	ssr->name = ssr_name;
> +	struct qcom_ssr_subsystem *info;
> +
> +	info = qcom_ssr_get_subsys(ssr_name);
> +	if (IS_ERR(info)) {
> +		dev_err(&rproc->dev, "Failed to add ssr subdevice\n");
> +		return;
> +	}
> +
> +	ssr->info = info;
>  	ssr->subdev.unprepare = ssr_notify_unprepare;
>  
>  	rproc_add_subdev(rproc, &ssr->subdev);
> @@ -249,6 +312,7 @@ EXPORT_SYMBOL_GPL(qcom_add_ssr_subdev);
>  void qcom_remove_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr)
>  {
>  	rproc_remove_subdev(rproc, &ssr->subdev);
> +	ssr->info = NULL;
>  }
>  EXPORT_SYMBOL_GPL(qcom_remove_ssr_subdev);
>  
> diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
> index 34e5188..dfc641c 100644
> --- a/drivers/remoteproc/qcom_common.h
> +++ b/drivers/remoteproc/qcom_common.h
> @@ -26,10 +26,11 @@ struct qcom_rproc_subdev {
>  	struct qcom_smd_edge *edge;
>  };
>  
> +struct qcom_ssr_subsystem;
> +
>  struct qcom_rproc_ssr {
>  	struct rproc_subdev subdev;
> -
> -	const char *name;
> +	struct qcom_ssr_subsystem *info;
>  };
>  
>  void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink,
> diff --git a/include/linux/remoteproc/qcom_rproc.h b/include/linux/remoteproc/qcom_rproc.h
> index fa8e386..2a1d6d0 100644
> --- a/include/linux/remoteproc/qcom_rproc.h
> +++ b/include/linux/remoteproc/qcom_rproc.h
> @@ -5,17 +5,27 @@ struct notifier_block;
>  
>  #if IS_ENABLED(CONFIG_QCOM_RPROC_COMMON)
>  
> -int qcom_register_ssr_notifier(struct notifier_block *nb);
> -void qcom_unregister_ssr_notifier(struct notifier_block *nb);
> +struct qcom_ssr_notify_data {
> +	const char *name;
> +	bool crashed;
> +};
> +
> +void *qcom_register_ssr_notifier(const char *name, struct notifier_block *nb);
> +int qcom_unregister_ssr_notifier(void *notify, struct notifier_block *nb);
>  
>  #else
>  
> -static inline int qcom_register_ssr_notifier(struct notifier_block *nb)
> +static inline void *qcom_register_ssr_notifier(const char *name,
> +					       struct notifier_block *nb)
>  {
> -	return 0;
> +	return NULL;
>  }
>  
> -static inline void qcom_unregister_ssr_notifier(struct notifier_block *nb) {}
> +static inline int qcom_unregister_ssr_notifier(void *notify,
> +					       struct notifier_block *nb)
> +{
> +	return 0;
> +}
>  
>  #endif
>  
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 2/2] remoteproc: qcom: Add notification types to SSR
  2020-06-24  2:23 ` [PATCH v6 2/2] remoteproc: qcom: Add notification types to SSR Rishabh Bhatnagar
@ 2020-07-08 23:56   ` Alex Elder
  0 siblings, 0 replies; 7+ messages in thread
From: Alex Elder @ 2020-07-08 23:56 UTC (permalink / raw)
  To: Rishabh Bhatnagar, linux-remoteproc, linux-kernel
  Cc: bjorn.andersson, tsoni, psodagud, sidgup

On 6/23/20 9:23 PM, Rishabh Bhatnagar wrote:
> The SSR subdevice only adds callback for the unprepare event. Add callbacks
> for prepare, start and prepare events. The client driver for a particular
> remoteproc might be interested in knowing the status of the remoteproc
> while undergoing SSR, not just when the remoteproc has finished shutting
> down.

Looks good.

Reviewed-by: Alex Elder <elder@linaro.org>

> Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
> Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
> ---
>  drivers/remoteproc/qcom_common.c      | 44 ++++++++++++++++++++++++++++++++++-
>  include/linux/remoteproc/qcom_rproc.h | 16 +++++++++++++
>  2 files changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 7a7384c..7ec4597 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -265,6 +265,44 @@ int qcom_unregister_ssr_notifier(void *notify, struct notifier_block *nb)
>  }
>  EXPORT_SYMBOL_GPL(qcom_unregister_ssr_notifier);
>  
> +static int ssr_notify_prepare(struct rproc_subdev *subdev)
> +{
> +	struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
> +	struct qcom_ssr_notify_data data = {
> +		.name = ssr->info->name,
> +		.crashed = false,
> +	};
> +
> +	srcu_notifier_call_chain(&ssr->info->notifier_list,
> +				 QCOM_SSR_BEFORE_POWERUP, &data);
> +	return 0;
> +}
> +
> +static int ssr_notify_start(struct rproc_subdev *subdev)
> +{
> +	struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
> +	struct qcom_ssr_notify_data data = {
> +		.name = ssr->info->name,
> +		.crashed = false,
> +	};
> +
> +	srcu_notifier_call_chain(&ssr->info->notifier_list,
> +				 QCOM_SSR_AFTER_POWERUP, &data);
> +	return 0;
> +}
> +
> +static void ssr_notify_stop(struct rproc_subdev *subdev, bool crashed)
> +{
> +	struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
> +	struct qcom_ssr_notify_data data = {
> +		.name = ssr->info->name,
> +		.crashed = crashed,
> +	};
> +
> +	srcu_notifier_call_chain(&ssr->info->notifier_list,
> +				 QCOM_SSR_BEFORE_SHUTDOWN, &data);
> +}
> +
>  static void ssr_notify_unprepare(struct rproc_subdev *subdev)
>  {
>  	struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
> @@ -273,7 +311,8 @@ static void ssr_notify_unprepare(struct rproc_subdev *subdev)
>  		.crashed = false,
>  	};
>  
> -	srcu_notifier_call_chain(&ssr->info->notifier_list, 0, &data);
> +	srcu_notifier_call_chain(&ssr->info->notifier_list,
> +				 QCOM_SSR_AFTER_SHUTDOWN, &data);
>  }
>  
>  /**
> @@ -298,6 +337,9 @@ void qcom_add_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr,
>  	}
>  
>  	ssr->info = info;
> +	ssr->subdev.prepare = ssr_notify_prepare;
> +	ssr->subdev.start = ssr_notify_start;
> +	ssr->subdev.stop = ssr_notify_stop;
>  	ssr->subdev.unprepare = ssr_notify_unprepare;
>  
>  	rproc_add_subdev(rproc, &ssr->subdev);
> diff --git a/include/linux/remoteproc/qcom_rproc.h b/include/linux/remoteproc/qcom_rproc.h
> index 2a1d6d0..6470516 100644
> --- a/include/linux/remoteproc/qcom_rproc.h
> +++ b/include/linux/remoteproc/qcom_rproc.h
> @@ -5,6 +5,22 @@ struct notifier_block;
>  
>  #if IS_ENABLED(CONFIG_QCOM_RPROC_COMMON)
>  
> +/**
> + * enum qcom_ssr_notify_type - Startup/Shutdown events related to a remoteproc
> + * processor.
> + *
> + * @QCOM_SSR_BEFORE_POWERUP:	Remoteproc about to start (prepare stage)
> + * @QCOM_SSR_AFTER_POWERUP:	Remoteproc is running (start stage)
> + * @QCOM_SSR_BEFORE_SHUTDOWN:	Remoteproc crashed or shutting down (stop stage)
> + * @QCOM_SSR_AFTER_SHUTDOWN:	Remoteproc is down (unprepare stage)
> + */
> +enum qcom_ssr_notify_type {
> +	QCOM_SSR_BEFORE_POWERUP,
> +	QCOM_SSR_AFTER_POWERUP,
> +	QCOM_SSR_BEFORE_SHUTDOWN,
> +	QCOM_SSR_AFTER_SHUTDOWN,
> +};
> +
>  struct qcom_ssr_notify_data {
>  	const char *name;
>  	bool crashed;
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 1/2] remoteproc: qcom: Add per subsystem SSR notification
  2020-06-24  2:23 ` [PATCH v6 1/2] remoteproc: qcom: Add per subsystem SSR notification Rishabh Bhatnagar
  2020-07-08 23:56   ` Alex Elder
@ 2020-07-13 16:26   ` Jon Hunter
  2020-07-13 19:09     ` Bjorn Andersson
  1 sibling, 1 reply; 7+ messages in thread
From: Jon Hunter @ 2020-07-13 16:26 UTC (permalink / raw)
  To: Rishabh Bhatnagar, linux-remoteproc, linux-kernel
  Cc: bjorn.andersson, tsoni, psodagud, sidgup, elder, linux-tegra


On 24/06/2020 03:23, 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.
> 
> Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
> Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
> ---
>  drivers/remoteproc/qcom_common.c      | 90 ++++++++++++++++++++++++++++++-----
>  drivers/remoteproc/qcom_common.h      |  5 +-
>  include/linux/remoteproc/qcom_rproc.h | 20 ++++++--
>  3 files changed, 95 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 9028cea..7a7384c 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -12,6 +12,7 @@
>  #include <linux/module.h>
>  #include <linux/notifier.h>
>  #include <linux/remoteproc.h>
> +#include <linux/remoteproc/qcom_rproc.h>
>  #include <linux/rpmsg/qcom_glink.h>
>  #include <linux/rpmsg/qcom_smd.h>
>  #include <linux/soc/qcom/mdt_loader.h>
> @@ -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);
> +static DEFINE_MUTEX(qcom_ssr_subsys_lock);
>  
>  static int glink_subdev_start(struct rproc_subdev *subdev)
>  {
> @@ -189,37 +197,83 @@ void qcom_remove_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
>  }
>  EXPORT_SYMBOL_GPL(qcom_remove_smd_subdev);
>  
> +static struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
> +{
> +	struct qcom_ssr_subsystem *info;
> +
> +	mutex_lock(&qcom_ssr_subsys_lock);
> +	/* Match in the global qcom_ssr_subsystem_list with name */
> +	list_for_each_entry(info, &qcom_ssr_subsystem_list, list)
> +		if (!strcmp(info->name, name))
> +			goto out;
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info) {
> +		info = ERR_PTR(-ENOMEM);
> +		goto out;
> +	}


The above appears to be breaking the ARM64 build on the latest -next
when building the modules  ...
 
  CC [M]  drivers/remoteproc/qcom_common.o
drivers/remoteproc/qcom_common.c: In function 'qcom_ssr_get_subsys':
remoteproc/qcom_common.c:210:9: error: implicit declaration of function 'kzalloc' [-Werror=implicit-function-declaration]
  info = kzalloc(sizeof(*info), GFP_KERNEL);
         ^~~~~~~
drivers/remoteproc/qcom_common.c:210:7: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
  info = kzalloc(sizeof(*info), GFP_KERNEL);

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 1/2] remoteproc: qcom: Add per subsystem SSR notification
  2020-07-13 16:26   ` Jon Hunter
@ 2020-07-13 19:09     ` Bjorn Andersson
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Andersson @ 2020-07-13 19:09 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Rishabh Bhatnagar, linux-remoteproc, linux-kernel, tsoni,
	psodagud, sidgup, elder, linux-tegra

On Mon 13 Jul 09:26 PDT 2020, Jon Hunter wrote:

> 
> On 24/06/2020 03:23, 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.
> > 
> > Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
> > Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
> > ---
> >  drivers/remoteproc/qcom_common.c      | 90 ++++++++++++++++++++++++++++++-----
> >  drivers/remoteproc/qcom_common.h      |  5 +-
> >  include/linux/remoteproc/qcom_rproc.h | 20 ++++++--
> >  3 files changed, 95 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> > index 9028cea..7a7384c 100644
> > --- a/drivers/remoteproc/qcom_common.c
> > +++ b/drivers/remoteproc/qcom_common.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/module.h>
> >  #include <linux/notifier.h>
> >  #include <linux/remoteproc.h>
> > +#include <linux/remoteproc/qcom_rproc.h>
> >  #include <linux/rpmsg/qcom_glink.h>
> >  #include <linux/rpmsg/qcom_smd.h>
> >  #include <linux/soc/qcom/mdt_loader.h>
> > @@ -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);
> > +static DEFINE_MUTEX(qcom_ssr_subsys_lock);
> >  
> >  static int glink_subdev_start(struct rproc_subdev *subdev)
> >  {
> > @@ -189,37 +197,83 @@ void qcom_remove_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
> >  }
> >  EXPORT_SYMBOL_GPL(qcom_remove_smd_subdev);
> >  
> > +static struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
> > +{
> > +	struct qcom_ssr_subsystem *info;
> > +
> > +	mutex_lock(&qcom_ssr_subsys_lock);
> > +	/* Match in the global qcom_ssr_subsystem_list with name */
> > +	list_for_each_entry(info, &qcom_ssr_subsystem_list, list)
> > +		if (!strcmp(info->name, name))
> > +			goto out;
> > +
> > +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> > +	if (!info) {
> > +		info = ERR_PTR(-ENOMEM);
> > +		goto out;
> > +	}
> 
> 
> The above appears to be breaking the ARM64 build on the latest -next
> when building the modules  ...
>  
>   CC [M]  drivers/remoteproc/qcom_common.o
> drivers/remoteproc/qcom_common.c: In function 'qcom_ssr_get_subsys':
> remoteproc/qcom_common.c:210:9: error: implicit declaration of function 'kzalloc' [-Werror=implicit-function-declaration]
>   info = kzalloc(sizeof(*info), GFP_KERNEL);
>          ^~~~~~~
> drivers/remoteproc/qcom_common.c:210:7: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
>   info = kzalloc(sizeof(*info), GFP_KERNEL);
> 

You're right, not sure why the test build completes successfully for
me...

I've applied a fix from Kefeng Wang for this, sorry for the disruption.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-07-13 19:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-24  2:23 [PATCH v6 0/2] Extend SSR notifications framework Rishabh Bhatnagar
2020-06-24  2:23 ` [PATCH v6 1/2] remoteproc: qcom: Add per subsystem SSR notification Rishabh Bhatnagar
2020-07-08 23:56   ` Alex Elder
2020-07-13 16:26   ` Jon Hunter
2020-07-13 19:09     ` Bjorn Andersson
2020-06-24  2:23 ` [PATCH v6 2/2] remoteproc: qcom: Add notification types to SSR Rishabh Bhatnagar
2020-07-08 23:56   ` Alex Elder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).