Date:   Sun, 19 Mar 2017 19:23:34 +0100
From:   Hans de Goede <hdegoede@...hat.com>
To:     MyungJoo Ham <myungjoo.ham@...sung.com>,
        Chanwoo Choi <cw00.choi@...sung.com>
Cc:     Hans de Goede <hdegoede@...hat.com>, linux-kernel@...r.kernel.org
Subject: [PATCH v2] extcon: Allow registering a single notifier for all cables on an extcon_dev

In some cases a driver may want to monitor multiple cables on a single
extcon. For example a charger driver will typically want to monitor all
of EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP to configure
the max. current it can sink while charging.

Due to the signature of the notifier_call function + how extcon passes
state and the extcon_dev as parameters this requires using one
notifier_block + one notifier_call function per cable, otherwise the
notifier_call function cannot get to its driver's data (using container_of
requires it to know which notifier block its first parameter is).

For a driver wanting to monitor the above 3 cables that would result
in something like this:

static const unsigned int vbus_cable_ids[] = {
	EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP };

struct driver_data {
	struct notifier_block vbus_nb[ARRAY_SIZE(vbus_cable_ids)];
}

/*
 * We need 3 copies of this, because there is no way to find out for which
 * cable id we are being called from the passed in arguments; and we must
 * have a separate nb for each extcon_register_notifier call.
 */
static int vbus_cable0_evt(struct notifier_block *nb, unsigned long e, void *p)
{
	struct driver_data *data =
		container_of(nb, struct driver_data, vbus_nb[0]);
	...
}

static int vbus_cable1_evt(struct notifier_block *nb, unsigned long e, void *p)
{
	struct driver_data *data =
		container_of(nb, struct driver_data, vbus_nb[1]);
	...
}

static int vbus_cable2_evt(struct notifier_block *nb, unsigned long e, void *p)
{
	struct driver_data *data =
		container_of(nb, struct driver_data, vbus_nb[2]);
	...
}

int probe(...)
{
	/* Register for vbus notification */
	data->vbus_nb[0].notifier_call = vbus_cable0_evt;
	data->vbus_nb[1].notifier_call = vbus_cable1_evt;
	data->vbus_nb[2].notifier_call = vbus_cable2_evt;
	for (i = 0; i < ARRAY_SIZE(vbus_cable_ids); i++) {
		ret = devm_extcon_register_notifier(dev, data->vbus_extcon,
					vbus_cable_ids[i], &data->vbus_nb[i]);
		if (ret)
			...
	}
}

And then in the event handling the driver often checks the state of
all cables explicitly using extcon_get_state, rather then using the
event argument to the notifier_call.

This commit makes extcon_[un]register_notifier accept -1 as cable-id,
which will cause the notifier to get called for changes on any cable
on the extcon_dev. Compared to the above example code this allows much
simpler code in drivers which want to monitor multiple cable types.

Signed-off-by: Hans de Goede <hdegoede@...hat.com>
---
Changes in v2:
-Fix false-positive "warning: 'idx' may be used uninitialized" warning
 seen with some compiler versions
---
 drivers/extcon/extcon.c | 35 ++++++++++++++++++++++++++---------
 drivers/extcon/extcon.h |  1 +
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 09ac5e7..2a042ee 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -449,6 +449,7 @@ int extcon_sync(struct extcon_dev *edev, unsigned int id)
 
 	state = !!(edev->state & BIT(index));
 	raw_notifier_call_chain(&edev->nh[index], state, edev);
+	raw_notifier_call_chain(&edev->nh_all, 0, edev);
 
 	/* This could be in interrupt handler */
 	prop_buf = (char *)get_zeroed_page(GFP_ATOMIC);
@@ -900,6 +901,8 @@ EXPORT_SYMBOL_GPL(extcon_get_extcon_dev);
  *				any attach status changes from the extcon.
  * @edev:	the extcon device that has the external connecotr.
  * @id:		the unique id of each external connector in extcon enumeration.
+ *		or -1 to get notififications for all cables on edev, in this
+ *		case no state info will get passed to the notifier_call.
  * @nb:		a notifier block to be registered.
  *
  * Note that the second parameter given to the callback of nb (val) is
@@ -915,12 +918,18 @@ int extcon_register_notifier(struct extcon_dev *edev, unsigned int id,
 	if (!edev || !nb)
 		return -EINVAL;
 
-	idx = find_cable_index_by_id(edev, id);
-	if (idx < 0)
-		return idx;
+	if ((int)id != -1) {
+		idx = find_cable_index_by_id(edev, id);
+		if (idx < 0)
+			return idx;
+	}
 
 	spin_lock_irqsave(&edev->lock, flags);
-	ret = raw_notifier_chain_register(&edev->nh[idx], nb);
+	if ((int)id != -1)
+		ret = raw_notifier_chain_register(&edev->nh[idx], nb);
+	else
+		ret = raw_notifier_chain_register(&edev->nh_all, nb);
+
 	spin_unlock_irqrestore(&edev->lock, flags);
 
 	return ret;
@@ -937,17 +946,23 @@ int extcon_unregister_notifier(struct extcon_dev *edev, unsigned int id,
 				struct notifier_block *nb)
 {
 	unsigned long flags;
-	int ret, idx;
+	int ret, idx = 0;
 
 	if (!edev || !nb)
 		return -EINVAL;
 
-	idx = find_cable_index_by_id(edev, id);
-	if (idx < 0)
-		return idx;
+	if ((int)id != -1) {
+		idx = find_cable_index_by_id(edev, id);
+		if (idx < 0)
+			return idx;
+	}
 
 	spin_lock_irqsave(&edev->lock, flags);
-	ret = raw_notifier_chain_unregister(&edev->nh[idx], nb);
+	if ((int)id != -1)
+		ret = raw_notifier_chain_unregister(&edev->nh[idx], nb);
+	else
+		ret = raw_notifier_chain_unregister(&edev->nh_all, nb);
+
 	spin_unlock_irqrestore(&edev->lock, flags);
 
 	return ret;
@@ -1212,6 +1227,8 @@ int extcon_dev_register(struct extcon_dev *edev)
 	for (index = 0; index < edev->max_supported; index++)
 		RAW_INIT_NOTIFIER_HEAD(&edev->nh[index]);
 
+	RAW_INIT_NOTIFIER_HEAD(&edev->nh_all);
+
 	dev_set_drvdata(&edev->dev, edev);
 	edev->state = 0;
 
diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
index 993ddcc..2e6c09d 100644
--- a/drivers/extcon/extcon.h
+++ b/drivers/extcon/extcon.h
@@ -44,6 +44,7 @@ struct extcon_dev {
 	/* Internal data. Please do not set. */
 	struct device dev;
 	struct raw_notifier_head *nh;
+	struct raw_notifier_head nh_all;
 	struct list_head entry;
 	int max_supported;
 	spinlock_t lock;	/* could be called by irq handler */
-- 
2.9.3

Powered by blists - more mailing lists