[PATCH] remoteproc: qcom: Add notification timeouts

From: Rishabh Bhatnagar
Date: Thu Oct 29 2020 - 20:11:16 EST


Clients can register their callbacks for power-on/off notifications
of remote processors. While executing the notifier chain sometimes
these callbacks may get stuck or take a long time which is not desired.
To detect such cases this patch introduces a timeout and prints out a
warning indicating that notifier chain is taking a long time.
The timeout is set to 20secs by default and can be adjusted.

Signed-off-by: Rishabh Bhatnagar <rishabhb@xxxxxxxxxxxxxx>
---
drivers/remoteproc/Kconfig | 16 ++++++++++++++++
drivers/remoteproc/qcom_common.c | 41 ++++++++++++++++++++++++++++++++--------
2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index d99548f..e1e623e 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -234,6 +234,22 @@ config QCOM_WCNSS_PIL
Say y here to support the Peripheral Image Loader for the Qualcomm
Wireless Connectivity Subsystem.

+config QCOM_NOTIFY_TIMEOUT
+ int "Default timeout for ssr notifications to complete (in milliseconds)"
+ depends on QCOM_RPROC_COMMON
+ default 20000
+ help
+ As part of ssr notification clients can register their callbacks
+ to a notifier chain which is invoked whenever the remoteproc
+ powers-on/off. This option controls the timeout for ssr notifications
+ to complete.
+ This is a good to have debug feature as sometimes callbacks
+ can get stuck or take a long time. This feature helps in identifying
+ such scenarios.
+
+ The default value is kept as 20 secs and should be left as it is
+ in most cases.
+
config ST_REMOTEPROC
tristate "ST remoteproc support"
depends on ARCH_STI
diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 085fd73..d72c4f5 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -29,6 +29,8 @@ struct qcom_ssr_subsystem {
const char *name;
struct srcu_notifier_head notifier_list;
struct list_head list;
+ struct timer_list notify_timer;
+ const char *notify_type;
};

static LIST_HEAD(qcom_ssr_subsystem_list);
@@ -198,6 +200,14 @@ void qcom_remove_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
}
EXPORT_SYMBOL_GPL(qcom_remove_smd_subdev);

+static void notify_timeout_handler(struct timer_list *t)
+{
+ struct qcom_ssr_subsystem *info = from_timer(info, t, notify_timer);
+
+ WARN(1, "srcu notifier chain for %s:%s taking too long", info->name,
+ info->notify_type);
+}
+
static struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
{
struct qcom_ssr_subsystem *info;
@@ -216,6 +226,9 @@ static struct qcom_ssr_subsystem *qcom_ssr_get_subsys(const char *name)
info->name = kstrdup_const(name, GFP_KERNEL);
srcu_init_notifier_head(&info->notifier_list);

+ /* Setup the notification timer */
+ timer_setup(&info->notify_timer, notify_timeout_handler, 0);
+
/* Add to global notification list */
list_add_tail(&info->list, &qcom_ssr_subsystem_list);

@@ -266,6 +279,18 @@ int qcom_unregister_ssr_notifier(void *notify, struct notifier_block *nb)
}
EXPORT_SYMBOL_GPL(qcom_unregister_ssr_notifier);

+static inline void notify_ssr_clients(struct qcom_rproc_ssr *ssr,
+ enum qcom_ssr_notify_type type,
+ struct qcom_ssr_notify_data *data)
+{
+ unsigned long timeout;
+
+ timeout = jiffies + msecs_to_jiffies(CONFIG_QCOM_NOTIFY_TIMEOUT);
+ mod_timer(&ssr->info->notify_timer, timeout);
+ srcu_notifier_call_chain(&ssr->info->notifier_list, type, data);
+ del_timer_sync(&ssr->info->notify_timer);
+}
+
static int ssr_notify_prepare(struct rproc_subdev *subdev)
{
struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
@@ -274,8 +299,8 @@ static int ssr_notify_prepare(struct rproc_subdev *subdev)
.crashed = false,
};

- srcu_notifier_call_chain(&ssr->info->notifier_list,
- QCOM_SSR_BEFORE_POWERUP, &data);
+ ssr->info->notify_type = "BEFORE_POWERUP";
+ notify_ssr_clients(ssr, QCOM_SSR_BEFORE_POWERUP, &data);
return 0;
}

@@ -287,8 +312,8 @@ static int ssr_notify_start(struct rproc_subdev *subdev)
.crashed = false,
};

- srcu_notifier_call_chain(&ssr->info->notifier_list,
- QCOM_SSR_AFTER_POWERUP, &data);
+ ssr->info->notify_type = "AFTER_POWERUP";
+ notify_ssr_clients(ssr, QCOM_SSR_AFTER_POWERUP, &data);
return 0;
}

@@ -300,8 +325,8 @@ static void ssr_notify_stop(struct rproc_subdev *subdev, bool crashed)
.crashed = crashed,
};

- srcu_notifier_call_chain(&ssr->info->notifier_list,
- QCOM_SSR_BEFORE_SHUTDOWN, &data);
+ ssr->info->notify_type = "BEFORE_SHUTDOWN";
+ notify_ssr_clients(ssr, QCOM_SSR_BEFORE_SHUTDOWN, &data);
}

static void ssr_notify_unprepare(struct rproc_subdev *subdev)
@@ -312,8 +337,8 @@ static void ssr_notify_unprepare(struct rproc_subdev *subdev)
.crashed = false,
};

- srcu_notifier_call_chain(&ssr->info->notifier_list,
- QCOM_SSR_AFTER_SHUTDOWN, &data);
+ ssr->info->notify_type = "AFTER_SHUTDOWN";
+ notify_ssr_clients(ssr, QCOM_SSR_AFTER_SHUTDOWN, &data);
}

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