[RFC PATCH 3/7] regulator: IRQ based event/error notification helpers

[RFC PATCH 3/7] regulator: IRQ based event/error notification helpers

[Matti Vaittinen]

Provide helper function for IC's implementing regulator notifications
when an IRQ fires. The helper also works for IRQs which can not be acked.
Helper can be set to disable the IRQ at handler and then re-enabling it
on delayed work later. The helper also adds regulator_get_error_flags()
errors in cache for the duration of IRQ disabling.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---

This patch has gone through only a very limited amount of testing. All
reviews / suggestions / testing is highly appreciated.

 drivers/regulator/Makefile       |   2 +-
 drivers/regulator/core.c         |  24 +-
 drivers/regulator/irq_helpers.c  | 396 +++++++++++++++++++++++++++++++
 include/linux/regulator/driver.h | 129 ++++++++++
 4 files changed, 547 insertions(+), 4 deletions(-)
 create mode 100644 drivers/regulator/irq_helpers.c

diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 44d2f8bf4b74..e25f1c2d6c9b 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -4,7 +4,7 @@
 #
 
 
-obj-$(CONFIG_REGULATOR) += core.o dummy.o fixed-helper.o helpers.o devres.o
+obj-$(CONFIG_REGULATOR) += core.o dummy.o fixed-helper.o helpers.o devres.o irq_helpers.o
 obj-$(CONFIG_OF) += of_regulator.o
 obj-$(CONFIG_REGULATOR_FIXED_VOLTAGE) += fixed.o
 obj-$(CONFIG_REGULATOR_VIRTUAL_CONSUMER) += virtual.o
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 8f35a3dd4c30..9f06b33ff1e2 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4366,20 +4366,37 @@ unsigned int regulator_get_mode(struct regulator *regulator)
 }
 EXPORT_SYMBOL_GPL(regulator_get_mode);
 
+static int rdev_get_cached_err_flags(struct regulator_dev *rdev)
+{
+	int ret = 0;
+
+	if (rdev->use_cached_err) {
+		spin_lock(&rdev->err_lock);
+		ret = rdev->cached_err;
+		spin_unlock(&rdev->err_lock);
+	}
+	return ret;
+}
+
 static int _regulator_get_error_flags(struct regulator_dev *rdev,
 					unsigned int *flags)
 {
-	int ret;
+	int ret, tmpret;
 
 	regulator_lock(rdev);
 
+	ret = rdev_get_cached_err_flags(rdev);
+
 	/* sanity check */
-	if (!rdev->desc->ops->get_error_flags) {
+	if (rdev->desc->ops->get_error_flags) {
+		tmpret = rdev->desc->ops->get_error_flags(rdev, flags);
+		if (tmpret > 0)
+			ret |= tmpret;
+	} else if (!rdev->use_cached_err) {
 		ret = -EINVAL;
 		goto out;
 	}
 
-	ret = rdev->desc->ops->get_error_flags(rdev, flags);
 out:
 	regulator_unlock(rdev);
 	return ret;
@@ -5214,6 +5231,7 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		goto rinse;
 	}
 	device_initialize(&rdev->dev);
+	spin_lock_init(&rdev->err_lock);
 
 	/*
 	 * Duplicate the config so the driver could override it after
diff --git a/drivers/regulator/irq_helpers.c b/drivers/regulator/irq_helpers.c
new file mode 100644
index 000000000000..57121554de8e
--- /dev/null
+++ b/drivers/regulator/irq_helpers.c
@@ -0,0 +1,396 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 ROHM Semiconductors
+ * regulator IRQ based event notification helpers
+ *
+ * Logic has been partially adapted from qcom-labibb driver.
+ *
+ * Author: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/of_irq.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/regulator/driver.h>
+
+struct regulator_irq {
+	struct regulator_irq_data rdata;
+	struct regulator_irq_desc desc;
+	int irq;
+	int retry_cnt;
+	struct delayed_work isr_work;
+};
+
+/*
+ * Should only be called from threaded handler to prevent potential deadlock
+ */
+static void rdev_flag_err(struct regulator_dev *rdev, int err)
+{
+	spin_lock(&rdev->err_lock);
+	rdev->cached_err |= err;
+	spin_unlock(&rdev->err_lock);
+}
+
+static void rdev_clear_err(struct regulator_dev *rdev, int err)
+{
+	spin_lock(&rdev->err_lock);
+	rdev->cached_err &= ~err;
+	spin_unlock(&rdev->err_lock);
+}
+
+static void regulator_notifier_isr_work(struct work_struct *work)
+{
+	struct regulator_irq *h;
+	struct regulator_irq_desc *d;
+	struct regulator_irq_data *rid;
+	int ret = 0;
+	int tmo, i;
+	int num_rdevs;
+
+	h = container_of(work, struct regulator_irq,
+			    isr_work.work);
+
+
+	d = &h->desc;
+	rid = &h->rdata;
+	num_rdevs = rid->num_states;
+
+reread:
+	if (d->fatal_cnt && h->retry_cnt > d->fatal_cnt) {
+		if (d->die)
+			ret = d->die(rid);
+		else
+			BUG();
+
+		/*
+		 * If the 'last resort' IC recovery failed we will have
+		 * nothing else left to do...
+		 */
+		BUG_ON(ret);
+
+		/*
+		 * If h->die() was implemented we assume recovery has been
+		 * attempted (probably regulator was shut down)
+		 * and we just enable IRQ and bail-out.
+		 */
+		goto enable_out;
+	}
+	if (d->renable) {
+		ret = d->renable(rid);
+
+		if (ret == REGULATOR_FAILED_RETRY) {
+			h->retry_cnt++;
+			if (!d->reread_ms)
+				goto reread;
+			/*
+			 * driver indicated problem is still on - let's not enable IRQ
+			 * but just wait a little more
+			 */
+			tmo = d->reread_ms;
+			goto reschedule;
+		}
+
+		if (ret) {
+			/*
+			 * IC status reading succeeded. update error info
+			 * just in case the renable changed it.
+			 */
+			for (i = 0; i < num_rdevs; i++) {
+				struct regulator_err_state *stat;
+				struct regulator_dev *rdev;
+
+				stat = &rid->states[i];
+				rdev = stat->rdev;
+				rdev_clear_err(rdev, (~stat->errors) &
+						      stat->possible_errs);
+			}
+			h->retry_cnt++;
+			/*
+			 * The IC indicated problem is still ON - no point in
+			 * re-enabling the IRQ. Retry later.
+			 */
+			tmo = d->irq_off_ms;
+			goto reschedule;
+		}
+	}
+
+	/*
+	 * Either IC reported problem cleared or no status checker was provided.
+	 * If problems are gone - good. If not - then the IRQ will fire again
+	 * and we'll have new nice loop. In any case we should clear error flags
+	 * here and re-enable IRQs.
+	 */
+	for (i = 0; i < num_rdevs; i++) {
+		struct regulator_err_state *stat;
+		struct regulator_dev *rdev;
+
+		stat = &rid->states[i];
+		rdev = stat->rdev;
+		rdev_clear_err(rdev, stat->possible_errs);
+	}
+
+	/*
+	 * Things have been seemingly successful => zero retry-counter.
+	 */
+	h->retry_cnt = 0;
+
+enable_out:
+	enable_irq(h->irq);
+
+	return;
+
+reschedule:
+	if (!d->high_prio)
+		mod_delayed_work(system_wq, &h->isr_work,
+				 msecs_to_jiffies(tmo));
+	else
+		mod_delayed_work(system_highpri_wq, &h->isr_work,
+				 msecs_to_jiffies(tmo));
+}
+
+static irqreturn_t regulator_notifier_isr(int irq, void *data)
+{
+	struct regulator_irq *h = data;
+	struct regulator_irq_desc *d;
+	struct regulator_irq_data *rid;
+	unsigned long rdev_map = 0;
+	int num_rdevs;
+	int ret, i, j;
+
+	d = &h->desc;
+	rid = &h->rdata;
+	num_rdevs = rid->num_states;
+
+	if (d->fatal_cnt)
+		h->retry_cnt++;
+
+	/*
+	 * we spare few cycles by not clearing statuses prior this call.
+	 * The IC driver must initialize the status buffers for rdevs
+	 * which it indicates having active events via rdev_map.
+	 *
+	 * Maybe we should just to be on a safer side(?)
+	 */
+	if (d->map_event)
+		ret = d->map_event(irq, rid, &rdev_map);
+
+	/*
+	 * If status reading fails (which is unlikely) we don't ack/disable
+	 * IRQ but just increase fail count and retry when IRQ fires again.
+	 * If retry_count exceeds given safety limit we call IC specific die
+	 * handler which can try disabling regulator(s).
+	 *
+	 * If no die handler is given we will just bug() as a last resort.
+	 *
+	 * We could try disabling all associated rdevs - but we might shoot
+	 * ourself in the head and leave problematic regulator enabled. So
+	 * if IC has no die-handler populated we just assume the regulator
+	 * can't be disabled.
+	 */
+	if (unlikely(ret == REGULATOR_FAILED_RETRY))
+		goto fail_out;
+
+	h->retry_cnt = 0;
+	/*
+	 * Let's not disable IRQ if there was no status bits for us. We'd
+	 * better leave spurious IRQ handling to genirq
+	 */
+	if (ret || !rdev_map)
+		return IRQ_NONE;
+
+	/*
+	 * Some events are bogus if regulator is disabled. Skip such events
+	 * if all relevant regulators are disabled
+	 */
+	if (d->skip_off) {
+		int skip = 1;
+
+		for_each_set_bit(i, &rdev_map, num_rdevs) {
+			struct regulator_dev *rdev;
+			const struct regulator_ops *ops;
+
+			rdev = rid->states[i].rdev;
+			ops = rdev->desc->ops;
+
+			/*
+			 * If any of the flagged regulators is enabled we do
+			 * handle this
+			 */
+			if (ops->is_enabled(rdev)) {
+				skip = 0;
+				break;
+			}
+		}
+		if (skip)
+			return IRQ_NONE;
+	}
+
+	/* Disable IRQ if HW keeps line asserted */
+	if (d->irq_off_ms)
+		disable_irq_nosync(irq);
+
+	/*
+	 * IRQ seems to be for us. Let's fire correct notifiers / store error
+	 * flags
+	 */
+	for_each_set_bit(i, &rdev_map, num_rdevs) {
+		struct regulator_err_state *stat;
+		int len;
+		struct regulator_dev *rdev;
+
+		stat = &rid->states[i];
+		len = sizeof(stat->notifs);
+
+		rdev = stat->rdev;
+		for_each_set_bit(j, &stat->notifs, 8 * len)
+			regulator_notifier_call_chain(rdev, 1 << (j - 1), NULL);
+
+		rdev_flag_err(rdev, stat->errors);
+	}
+
+	if (d->irq_off_ms) {
+		if (!d->high_prio)
+			schedule_delayed_work(&h->isr_work,
+					      msecs_to_jiffies(d->irq_off_ms));
+		else
+			mod_delayed_work(system_highpri_wq,
+					 &h->isr_work,
+					 msecs_to_jiffies(d->irq_off_ms));
+	}
+
+	return IRQ_HANDLED;
+
+fail_out:
+	if (d->fatal_cnt && h->retry_cnt > d->fatal_cnt) {
+		if (d->die)
+			ret = d->die(rid);
+
+		/*
+		 * If die() failed or was not implemented just BUG() as last
+		 * attemt to save HW.
+		 */
+		BUG_ON(ret);
+	}
+	return IRQ_NONE;
+}
+
+static void dev_delayed_work_drop(struct device *dev, void *res)
+{
+	cancel_delayed_work_sync(*(struct delayed_work **)res);
+}
+
+int dev_delayed_work_autocancel(struct device *dev, struct delayed_work *w,
+				void (*worker)(struct work_struct *work))
+{
+	struct delayed_work **ptr;
+
+	ptr = devres_alloc(dev_delayed_work_drop, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	INIT_DELAYED_WORK(w, worker);
+	*ptr = w;
+	devres_add(dev, ptr);
+
+	return 0;
+}
+
+static int init_rdev_state(struct device *dev, struct regulator_irq *h,
+			   struct regulator_dev **rdev, int common_err,
+			   int *rdev_err, int rdev_amount)
+{
+	int i;
+
+	h->rdata.states = devm_kzalloc(dev, sizeof(*h->rdata.states) *
+				       rdev_amount, GFP_KERNEL);
+	if (!h->rdata.states)
+		return -ENOMEM;
+
+	h->rdata.num_states = rdev_amount;
+	h->rdata.data = h->desc.data;
+
+	for (i = 0; i < rdev_amount; i++) {
+		h->rdata.states[i].possible_errs = common_err;
+		if (rdev_err)
+			h->rdata.states[i].possible_errs |= *rdev_err++;
+		h->rdata.states[i].rdev = *rdev++;
+	}
+
+	return 0;
+}
+
+static void init_rdev_errors(struct regulator_irq *h)
+{
+	int i;
+
+	for (i = 0; i < h->rdata.num_states; i++) {
+		if (h->rdata.states[i].possible_errs)
+			/* Can we trust writing this boolean is atomic? */
+			h->rdata.states[i].rdev->use_cached_err = true;
+	}
+}
+
+/**
+ * regulator_irq_helper - register IRQ based regulator event/error notifier
+ *
+ * @dev:		device to which lifetime the helper's lifetime is
+ *			bound.
+ * @d:			IRQ helper descriptor.
+ * @irq:		IRQ used to inform events/errors to be notified.
+ * @irq_flags:		Extra IRQ flags to be OR's with the default IRQF_ONESHOT
+ *			when requesting the (threaded) irq.
+ * @common_errs:	Errors which can be flagged by this IRQ for all rdevs.
+ *			When IRQ is re-enabled these errors will be cleared
+ *			from all associated regulators
+ * @per_rdev_errs:	Optional error flag array describing errors specific
+ *			for only some of the regulators. These errors will be
+ *			or'ed with common erros. If this is given the array
+ *			should contain rdev_amount flags. Can be set to NULL
+ *			if there is no regulator specific error flags for this
+ *			IRQ.
+ * @rdev:		Array of regulators associated with this IRQ.
+ * @rdev_amount:	Amount of regulators associated wit this IRQ.
+ */
+int regulator_irq_helper(struct device *dev, const struct regulator_irq_desc *d,
+			 int irq, int irq_flags, int common_errs,
+			 int *per_rdev_errs, struct regulator_dev **rdev,
+			 int rdev_amount)
+{
+	struct regulator_irq *h;
+	int ret;
+
+	if (!rdev_amount || !d)
+		return -EINVAL;
+
+	h = devm_kzalloc(dev, sizeof(*h), GFP_KERNEL);
+	if (!h)
+		return -ENOMEM;
+
+	if (irq <= 0) {
+		dev_err(dev, "No IRQ\n");
+		return -EINVAL;
+	}
+
+	h->irq = irq;
+	h->desc = *d;
+
+	ret = init_rdev_state(dev, h, rdev, common_errs, per_rdev_errs,
+			      rdev_amount);
+	if (ret)
+		return ret;
+
+	init_rdev_errors(h);
+
+	if (h->desc.irq_off_ms)
+		dev_delayed_work_autocancel(dev, &h->isr_work,
+					    regulator_notifier_isr_work);
+
+	return devm_request_threaded_irq(dev, h->irq, NULL,
+					 regulator_notifier_isr, IRQF_ONESHOT |
+					 irq_flags, h->desc.name, h);
+}
+EXPORT_SYMBOL_GPL(regulator_irq_helper);
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index d7c77ee370f3..9a9bc0f5dcea 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -409,6 +409,128 @@ struct regulator_config {
 	struct gpio_desc *ena_gpiod;
 };
 
+/**
+ * struct regulator_err_state - regulator error/notification status
+ *
+ * @rdev:		Regulator which status the struct indicates.
+ * @notifs:		Events which have occurred on regulator.
+ * @errors:		Errors which are active on regulator.
+ * @possible_errs:	Errors which can be signaled (by given IRQ).
+ */
+struct regulator_err_state {
+	struct regulator_dev *rdev;
+	unsigned long notifs;
+	unsigned long errors;
+	int possible_errs;
+};
+
+/**
+ * struct regulator_irq_data - regulator error/notification status date
+ *
+ * @states:	Status structs for each of the associated regulators.
+ * @num_states:	Amount of associated regulators.
+ * @data:	Driver data pointer given at regulator_irq_desc.
+ * @opaque:	Value storage for IC driver. Core does not update this. ICs
+ *		may want to store status register value here at map_event and
+ *		compare contents at renable to see if new problems have been
+ *		added to status. If that is the case it may be desirable to
+ *		return REGULATOR_ERROR_CLEARED and not REGULATOR_ERROR_ON to
+ *		allow IRQ fire again and to generate notifications also for
+ *		the new issues.
+ *
+ * This structure is passed to map_event and renable for reporting reulator
+ * status to core.
+ */
+struct regulator_irq_data {
+	struct regulator_err_state *states;
+	int num_states;
+	void *data;
+	long opaque;
+};
+
+/**
+ * struct regulator_irq_desc - notification sender for IRQ based events.
+ *
+ * @name:	The visible name for the IRQ
+ * @fatal_cnt:	If this IRQ is used to signal HW damaging condition it may be
+ *		best to shut-down regulator(s) or reboot the SOC if error
+ *		handling is repeteadly failing. If fatal_cnt is given the IRQ
+ *		handling is aborted if it fails for fatal_cnt times and die()
+ *		callback (if populated) or BUG() is called to try to prevent
+ *		further damage.
+ * @reread_ms:	The time which is waited before attempting to re-read status
+ *		at the worker if IC reading fails. Immediate re-read is done
+ *		if time is not specified.
+ * @irq_off_ms:	The time which IRQ is kept disabled before re-evaluating the
+ *		status for devices which keep IRQ disabled for duration of the
+ *		error. If this is not given the IRQ is left enabled and renable
+ *		is not called.
+ * @skip_off:	If set to true the IRQ handler will attempt to check if any of
+ *		the associated regulators are enabled prior to taking other
+ *		actions. If no regulators are enabled and this is set to true
+ *		a spurious IRQ is assumed and IRQ_NONE is returned.
+ * @high_prio:	Boolean to indicate that high priority WQ should be used.
+ * @data:	Driver private data pointer which will be passed as such to
+ *		the renable, map_event and die callbacks in regulator_irq_data.
+ * @die:	Protection callback. If IC status reading or recovery actions
+ *		fail fatal_cnt times this callback or BUG() is called. This
+ *		callback should implement final protection attempt like
+ *		disabling the regulator. If protection succeeded this may
+ *		return 0. If anything else is returned the core assumes final
+ *		protection failed and calls BUG() as a last resort.
+ * @map_event:	Driver callback to map IRQ status into regulator devices with
+ *		events / errors. NOTE: callback MUST initialize both the
+ *		errors and notifs for all rdevs which it signals having
+ *		active events as core does not clean the map data.
+ *		REGULATOR_FAILED_RETRY can be returned to indicate that the
+ *		status reading from IC failed. If this is repeated for
+ *		fatal_cnt times the core will call die() callback or BUG()
+ *		as a last resort to protect the HW.
+ * @renable:	Optional callback to check status (if HW supports that) before
+ *		re-enabling IRQ. If implemented this should clear the error
+ *		flags so that errors fetched by regulator_get_error_flags()
+ *		are updated. If callback is not impelemted then errors are
+ *		assumed to be cleared and IRQ is re-enabled.
+ *		REGULATOR_FAILED_RETRY can be returned to
+ *		indicate that the status reading from IC failed. If this is
+ *		repeated for 'fatal_cnt' times the core will call die()
+ *		callback or BUG() as a last resort to protect the HW.
+ *		Returning zero indicates that the problem in HW has been solved
+ *		and IRQ will be re-enabled. Returning REGULATOR_ERROR_ON
+ *		indicates the error condition is still active and keeps IRQ
+ *		disabled. Please note that returning REGULATOR_ERROR_ON does
+ *		not retrigger evaluating what events are active or resending
+ *		notifications. If this is needed you probably want to return
+ *		zero and allow IRQ to retrigger causing events to be
+ *		re-evaluated and re-sent.
+ *
+ * This structure is used for registering regulator IRQ notification helper.
+ */
+struct regulator_irq_desc {
+	const char *name;
+	int irq_flags;
+	int fatal_cnt;
+	int reread_ms;
+	int irq_off_ms;
+	bool skip_off;
+	bool high_prio;
+	void *data;
+
+	int (*die)(struct regulator_irq_data *rid);
+	int (*map_event)(int irq, struct regulator_irq_data *rid,
+			  unsigned long *dev_mask);
+	int (*renable)(struct regulator_irq_data *rid);
+};
+
+/*
+ * Return values for regulator IRQ helpers.
+ */
+enum {
+	REGULATOR_ERROR_CLEARED,
+	REGULATOR_FAILED_RETRY,
+	REGULATOR_ERROR_ON,
+};
+
 /*
  * struct coupling_desc
  *
@@ -473,6 +595,9 @@ struct regulator_dev {
 
 	/* time when this regulator was disabled last time */
 	unsigned long last_off_jiffy;
+	int cached_err;
+	bool use_cached_err;
+	spinlock_t err_lock;
 };
 
 struct regulator_dev *
@@ -487,6 +612,10 @@ void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev);
 
 int regulator_notifier_call_chain(struct regulator_dev *rdev,
 				  unsigned long event, void *data);
+int regulator_irq_helper(struct device *dev, const struct regulator_irq_desc *d,
+			 int irq, int irq_flags, int common_errs,
+			 int *per_rdev_errs, struct regulator_dev **rdev,
+			 int rdev_amount);
 
 void *rdev_get_drvdata(struct regulator_dev *rdev);
 struct device *rdev_get_dev(struct regulator_dev *rdev);
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

Re: [RFC PATCH 3/7] regulator: IRQ based event/error notification helpers

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2570 bytes --]

Hi Matti,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on regulator/for-next]
[also build test WARNING on v5.11-rc7 next-20210211]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Matti-Vaittinen/Extend-regulator-notification-support/20210211-204336
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
config: i386-randconfig-s001-20210211 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-215-g0fb77bb6-dirty
        # https://github.com/0day-ci/linux/commit/1844ad67f3ebe118184da7e83b30df9f4eb0caf0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Matti-Vaittinen/Extend-regulator-notification-support/20210211-204336
        git checkout 1844ad67f3ebe118184da7e83b30df9f4eb0caf0
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/regulator/irq_helpers.c:286:5: warning: no previous prototype for 'dev_delayed_work_autocancel' [-Wmissing-prototypes]
     286 | int dev_delayed_work_autocancel(struct device *dev, struct delayed_work *w,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~


"sparse warnings: (new ones prefixed by >>)"
>> drivers/regulator/irq_helpers.c:286:5: sparse: sparse: symbol 'dev_delayed_work_autocancel' was not declared. Should it be static?

Please review and possibly fold the followup patch.

vim +/dev_delayed_work_autocancel +286 drivers/regulator/irq_helpers.c

   285	
 > 286	int dev_delayed_work_autocancel(struct device *dev, struct delayed_work *w,
   287					void (*worker)(struct work_struct *work))
   288	{
   289		struct delayed_work **ptr;
   290	
   291		ptr = devres_alloc(dev_delayed_work_drop, sizeof(*ptr), GFP_KERNEL);
   292		if (!ptr)
   293			return -ENOMEM;
   294	
   295		INIT_DELAYED_WORK(w, worker);
   296		*ptr = w;
   297		devres_add(dev, ptr);
   298	
   299		return 0;
   300	}
   301	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33850 bytes --]

Re: [RFC PATCH 3/7] regulator: IRQ based event/error notification helpers

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 10774 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <3daf0531910c25d8b0da3964778ae2a6c9049d43.1613042245.git.matti.vaittinen@fi.rohmeurope.com>
References: <3daf0531910c25d8b0da3964778ae2a6c9049d43.1613042245.git.matti.vaittinen@fi.rohmeurope.com>
TO: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>, Matti Vaittinen <mazziesaccount@gmail.com>

Hi Matti,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on regulator/for-next]
[also build test WARNING on v5.11-rc7 next-20210211]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Matti-Vaittinen/Extend-regulator-notification-support/20210211-204336
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
:::::: branch date: 6 hours ago
:::::: commit date: 6 hours ago
config: x86_64-randconfig-m001-20210209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/regulator/irq_helpers.c:194 regulator_notifier_isr() error: uninitialized symbol 'ret'.

vim +/ret +194 drivers/regulator/irq_helpers.c

1844ad67f3ebe1 Matti Vaittinen 2021-02-11  154  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  155  static irqreturn_t regulator_notifier_isr(int irq, void *data)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  156  {
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  157  	struct regulator_irq *h = data;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  158  	struct regulator_irq_desc *d;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  159  	struct regulator_irq_data *rid;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  160  	unsigned long rdev_map = 0;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  161  	int num_rdevs;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  162  	int ret, i, j;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  163  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  164  	d = &h->desc;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  165  	rid = &h->rdata;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  166  	num_rdevs = rid->num_states;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  167  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  168  	if (d->fatal_cnt)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  169  		h->retry_cnt++;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  170  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  171  	/*
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  172  	 * we spare few cycles by not clearing statuses prior this call.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  173  	 * The IC driver must initialize the status buffers for rdevs
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  174  	 * which it indicates having active events via rdev_map.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  175  	 *
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  176  	 * Maybe we should just to be on a safer side(?)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  177  	 */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  178  	if (d->map_event)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  179  		ret = d->map_event(irq, rid, &rdev_map);
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  180  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  181  	/*
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  182  	 * If status reading fails (which is unlikely) we don't ack/disable
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  183  	 * IRQ but just increase fail count and retry when IRQ fires again.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  184  	 * If retry_count exceeds given safety limit we call IC specific die
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  185  	 * handler which can try disabling regulator(s).
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  186  	 *
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  187  	 * If no die handler is given we will just bug() as a last resort.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  188  	 *
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  189  	 * We could try disabling all associated rdevs - but we might shoot
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  190  	 * ourself in the head and leave problematic regulator enabled. So
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  191  	 * if IC has no die-handler populated we just assume the regulator
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  192  	 * can't be disabled.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  193  	 */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11 @194  	if (unlikely(ret == REGULATOR_FAILED_RETRY))
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  195  		goto fail_out;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  196  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  197  	h->retry_cnt = 0;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  198  	/*
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  199  	 * Let's not disable IRQ if there was no status bits for us. We'd
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  200  	 * better leave spurious IRQ handling to genirq
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  201  	 */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  202  	if (ret || !rdev_map)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  203  		return IRQ_NONE;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  204  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  205  	/*
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  206  	 * Some events are bogus if regulator is disabled. Skip such events
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  207  	 * if all relevant regulators are disabled
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  208  	 */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  209  	if (d->skip_off) {
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  210  		int skip = 1;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  211  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  212  		for_each_set_bit(i, &rdev_map, num_rdevs) {
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  213  			struct regulator_dev *rdev;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  214  			const struct regulator_ops *ops;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  215  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  216  			rdev = rid->states[i].rdev;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  217  			ops = rdev->desc->ops;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  218  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  219  			/*
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  220  			 * If any of the flagged regulators is enabled we do
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  221  			 * handle this
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  222  			 */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  223  			if (ops->is_enabled(rdev)) {
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  224  				skip = 0;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  225  				break;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  226  			}
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  227  		}
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  228  		if (skip)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  229  			return IRQ_NONE;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  230  	}
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  231  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  232  	/* Disable IRQ if HW keeps line asserted */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  233  	if (d->irq_off_ms)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  234  		disable_irq_nosync(irq);
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  235  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  236  	/*
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  237  	 * IRQ seems to be for us. Let's fire correct notifiers / store error
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  238  	 * flags
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  239  	 */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  240  	for_each_set_bit(i, &rdev_map, num_rdevs) {
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  241  		struct regulator_err_state *stat;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  242  		int len;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  243  		struct regulator_dev *rdev;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  244  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  245  		stat = &rid->states[i];
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  246  		len = sizeof(stat->notifs);
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  247  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  248  		rdev = stat->rdev;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  249  		for_each_set_bit(j, &stat->notifs, 8 * len)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  250  			regulator_notifier_call_chain(rdev, 1 << (j - 1), NULL);
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  251  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  252  		rdev_flag_err(rdev, stat->errors);
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  253  	}
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  254  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  255  	if (d->irq_off_ms) {
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  256  		if (!d->high_prio)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  257  			schedule_delayed_work(&h->isr_work,
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  258  					      msecs_to_jiffies(d->irq_off_ms));
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  259  		else
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  260  			mod_delayed_work(system_highpri_wq,
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  261  					 &h->isr_work,
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  262  					 msecs_to_jiffies(d->irq_off_ms));
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  263  	}
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  264  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  265  	return IRQ_HANDLED;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  266  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  267  fail_out:
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  268  	if (d->fatal_cnt && h->retry_cnt > d->fatal_cnt) {
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  269  		if (d->die)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  270  			ret = d->die(rid);
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  271  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  272  		/*
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  273  		 * If die() failed or was not implemented just BUG() as last
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  274  		 * attemt to save HW.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  275  		 */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  276  		BUG_ON(ret);
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  277  	}
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  278  	return IRQ_NONE;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  279  }
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  280  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29633 bytes --]

Re: [RFC PATCH 3/7] regulator: IRQ based event/error notification helpers

[Vaittinen, Matti]

On Thu, 2021-02-11 at 14:35 +0200, Matti Vaittinen wrote:
> Provide helper function for IC's implementing regulator notifications
> when an IRQ fires. The helper also works for IRQs which can not be
> acked.
> Helper can be set to disable the IRQ at handler and then re-enabling
> it
> on delayed work later. The helper also adds
> regulator_get_error_flags()
> errors in cache for the duration of IRQ disabling.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> 
> This patch has gone through only a very limited amount of testing.
> All
> reviews / suggestions / testing is highly appreciated.
> 

/* SNIP */

> +
> +static void dev_delayed_work_drop(struct device *dev, void *res)
> +{
> +	cancel_delayed_work_sync(*(struct delayed_work **)res);
> +}
> +
> +int dev_delayed_work_autocancel(struct device *dev, struct
> delayed_work *w,
> +				void (*worker)(struct work_struct
> *work))
> +{
> +	struct delayed_work **ptr;
> +
> +	ptr = devres_alloc(dev_delayed_work_drop, sizeof(*ptr),
> GFP_KERNEL);
> +	if (!ptr)
> +		return -ENOMEM;
> +
> +	INIT_DELAYED_WORK(w, worker);
> +	*ptr = w;
> +	devres_add(dev, ptr);
> +
> +	return 0;
> +}
> +

I got mail from build-bot. Sparse warning. Bot suggested staticizing
the dev_delayed_work_autocancel(). I should've been more careful.

It how ever made me wonder if this would actually be worth exporting?

There seems to be few drivers which need delayed wq and which implement
.remove() just to call the cancel_delayed_work_sync().

I think this might help cleaning up those(?) Or am I completely off
here?

I just did:
git grep -A15 remove |grep -B10 -A10 cancel_delayed_work_sync

in drivers directory and spotted couple of candidates like
watchdog/retu_wdt.c (should also use devm_watchdog_register_device)
regulator/qcom_spmi-regulator.c
power/supply/sbs-charger.c
power/supply/sbs-battery.c
power/supply/ltc2941-battery-gauge.c
...

And no. I am not offering to go through _all_ drivers, but I guess I
could go through at least few of them :)

And sorry for noise if this has been suggested and rejected before - I
didn't spot something like this from mail lists. (Maybe I am missing
something?)

Best Regards
   Matti Vaittinen


--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland
SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~

Simon says - in Latin please.
"non cogito me" dixit Rene Descarte, deinde evanescavit

(Thanks for the translation Simon)

Re: [RFC PATCH 3/7] regulator: IRQ based event/error notification helpers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 310 bytes --]

On Fri, Feb 12, 2021 at 09:33:44AM +0000, Vaittinen, Matti wrote:

> There seems to be few drivers which need delayed wq and which implement
> .remove() just to call the cancel_delayed_work_sync().

> I think this might help cleaning up those(?) Or am I completely off
> here?

I can see it being useful, yes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC PATCH 3/7] regulator: IRQ based event/error notification helpers

[Vaittinen, Matti]

On Thu, 2021-02-11 at 14:35 +0200, Matti Vaittinen wrote:
> Provide helper function for IC's implementing regulator notifications
> when an IRQ fires. The helper also works for IRQs which can not be
> acked.
> Helper can be set to disable the IRQ at handler and then re-enabling
> it
> on delayed work later. The helper also adds
> regulator_get_error_flags()
> errors in cache for the duration of IRQ disabling.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> 
> This patch has gone through only a very limited amount of testing.
> All
> reviews / suggestions / testing is highly appreciated.
> 

// Snip

> +
> +static void dev_delayed_work_drop(struct device *dev, void *res)
> +{
> +	cancel_delayed_work_sync(*(struct delayed_work **)res);
> +}
> +
> +int dev_delayed_work_autocancel(struct device *dev, struct
> delayed_work *w,
> +				void (*worker)(struct work_struct
> *work))
> +{
> +	struct delayed_work **ptr;
> +
> +	ptr = devres_alloc(dev_delayed_work_drop, sizeof(*ptr),
> GFP_KERNEL);
> +	if (!ptr)
> +		return -ENOMEM;
> +
> +	INIT_DELAYED_WORK(w, worker);
> +	*ptr = w;
> +	devres_add(dev, ptr);
> +
> +	return 0;
> +}

I sent this dev_delayed_work_autocancel() + few cleanup patches as own
series. Discussion that series created made me realize that we don't
want to force use of devm by hiding the WQ init here. We should
introduce also non devm variant + manual cancellation routine for those
who don't use devm to register rdevs.

And as I see that Greg was strongly opposing the devm based delayed
work cancellation - I guess that if we want to proceed with this one
we'd better first implement the 'non devm' variant which uses manual wq
cancellation + manual IRQ deregistering and use that cancellation to
build a devm one...

I'll try to cook v2 still this week.

Best Regards
	Matti Vaittinen

Re: [RFC PATCH 3/7] regulator: IRQ based event/error notification helpers

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 4325 bytes --]

Hi Matti,

url:    https://github.com/0day-ci/linux/commits/Matti-Vaittinen/Extend-regulator-notification-support/20210211-204336
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
config: x86_64-randconfig-m001-20210209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/regulator/irq_helpers.c:194 regulator_notifier_isr() error: uninitialized symbol 'ret'.

vim +/ret +194 drivers/regulator/irq_helpers.c

1844ad67f3ebe1 Matti Vaittinen 2021-02-11  155  static irqreturn_t regulator_notifier_isr(int irq, void *data)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  156  {
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  157  	struct regulator_irq *h = data;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  158  	struct regulator_irq_desc *d;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  159  	struct regulator_irq_data *rid;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  160  	unsigned long rdev_map = 0;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  161  	int num_rdevs;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  162  	int ret, i, j;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  163  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  164  	d = &h->desc;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  165  	rid = &h->rdata;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  166  	num_rdevs = rid->num_states;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  167  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  168  	if (d->fatal_cnt)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  169  		h->retry_cnt++;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  170  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  171  	/*
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  172  	 * we spare few cycles by not clearing statuses prior this call.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  173  	 * The IC driver must initialize the status buffers for rdevs
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  174  	 * which it indicates having active events via rdev_map.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  175  	 *
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  176  	 * Maybe we should just to be on a safer side(?)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  177  	 */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  178  	if (d->map_event)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  179  		ret = d->map_event(irq, rid, &rdev_map);

"ret" not initialized on else path.

1844ad67f3ebe1 Matti Vaittinen 2021-02-11  180  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  181  	/*
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  182  	 * If status reading fails (which is unlikely) we don't ack/disable
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  183  	 * IRQ but just increase fail count and retry when IRQ fires again.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  184  	 * If retry_count exceeds given safety limit we call IC specific die
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  185  	 * handler which can try disabling regulator(s).
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  186  	 *
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  187  	 * If no die handler is given we will just bug() as a last resort.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  188  	 *
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  189  	 * We could try disabling all associated rdevs - but we might shoot
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  190  	 * ourself in the head and leave problematic regulator enabled. So
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  191  	 * if IC has no die-handler populated we just assume the regulator
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  192  	 * can't be disabled.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  193  	 */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11 @194  	if (unlikely(ret == REGULATOR_FAILED_RETRY))
                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1844ad67f3ebe1 Matti Vaittinen 2021-02-11  195  		goto fail_out;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  196  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  197  	h->retry_cnt = 0;

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29633 bytes --]

Re: [RFC PATCH 3/7] regulator: IRQ based event/error notification helpers

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 4325 bytes --]

Hi Matti,

url:    https://github.com/0day-ci/linux/commits/Matti-Vaittinen/Extend-regulator-notification-support/20210211-204336
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
config: x86_64-randconfig-m001-20210209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/regulator/irq_helpers.c:194 regulator_notifier_isr() error: uninitialized symbol 'ret'.

vim +/ret +194 drivers/regulator/irq_helpers.c

1844ad67f3ebe1 Matti Vaittinen 2021-02-11  155  static irqreturn_t regulator_notifier_isr(int irq, void *data)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  156  {
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  157  	struct regulator_irq *h = data;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  158  	struct regulator_irq_desc *d;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  159  	struct regulator_irq_data *rid;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  160  	unsigned long rdev_map = 0;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  161  	int num_rdevs;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  162  	int ret, i, j;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  163  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  164  	d = &h->desc;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  165  	rid = &h->rdata;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  166  	num_rdevs = rid->num_states;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  167  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  168  	if (d->fatal_cnt)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  169  		h->retry_cnt++;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  170  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  171  	/*
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  172  	 * we spare few cycles by not clearing statuses prior this call.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  173  	 * The IC driver must initialize the status buffers for rdevs
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  174  	 * which it indicates having active events via rdev_map.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  175  	 *
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  176  	 * Maybe we should just to be on a safer side(?)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  177  	 */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  178  	if (d->map_event)
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  179  		ret = d->map_event(irq, rid, &rdev_map);

"ret" not initialized on else path.

1844ad67f3ebe1 Matti Vaittinen 2021-02-11  180  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  181  	/*
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  182  	 * If status reading fails (which is unlikely) we don't ack/disable
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  183  	 * IRQ but just increase fail count and retry when IRQ fires again.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  184  	 * If retry_count exceeds given safety limit we call IC specific die
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  185  	 * handler which can try disabling regulator(s).
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  186  	 *
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  187  	 * If no die handler is given we will just bug() as a last resort.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  188  	 *
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  189  	 * We could try disabling all associated rdevs - but we might shoot
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  190  	 * ourself in the head and leave problematic regulator enabled. So
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  191  	 * if IC has no die-handler populated we just assume the regulator
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  192  	 * can't be disabled.
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  193  	 */
1844ad67f3ebe1 Matti Vaittinen 2021-02-11 @194  	if (unlikely(ret == REGULATOR_FAILED_RETRY))
                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1844ad67f3ebe1 Matti Vaittinen 2021-02-11  195  		goto fail_out;
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  196  
1844ad67f3ebe1 Matti Vaittinen 2021-02-11  197  	h->retry_cnt = 0;

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29633 bytes --]