[PATCH 0/1] power: supply: axp288_charger: Use one notifier_block per extcon cable

[PATCH 0/1] power: supply: axp288_charger: Use one notifier_block per extcon cable

[Hans de Goede]

Hi All,

Here is one more axp288_charger fix, note this applies on top of v2 of my
previous axp288_charger fixes series.

Regards,

Hans

[PATCH] power: supply: axp288_charger: Use one notifier_block per extcon cable

[Hans de Goede]

Prior to this commit the code was using 1 notifier_block for all
types of charger cable, this is incorrect as the notifier_block
becomes part of a linked-list and now the same notifier_block
is part of 3 linked lists.

This commit fixes this by using a separate nb per extcon cable.

Note this happened to work fine sofar because axp288_charger was the only
listener, so when added to each of the 3 notifier chains, the next pointer
in the nb would be set to 0, so we've 3 heads pointing to the same nb,
with its next pointing to NULL. But as soon as we mix in a second extcon
consumer things will go boom.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/axp288_charger.c | 41 +++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
index ca4fa51..6be2fe2 100644
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -115,6 +115,9 @@
 #define AXP288_EXTCON_DEV_NAME		"axp288_extcon"
 #define USB_HOST_EXTCON_DEV_NAME	"INT3496:00"
 
+static const unsigned int cable_ids[] =
+	{ EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP };
+
 enum {
 	VBUS_OV_IRQ = 0,
 	CHARGE_DONE_IRQ,
@@ -149,7 +152,7 @@ struct axp288_chrg_info {
 		struct extcon_dev *edev;
 		bool connected;
 		enum power_supply_type chg_type;
-		struct notifier_block nb;
+		struct notifier_block nb[ARRAY_SIZE(cable_ids)];
 		struct work_struct work;
 	} cable;
 
@@ -625,14 +628,35 @@ static void axp288_charger_extcon_evt_worker(struct work_struct *work)
 	power_supply_changed(info->psy_usb);
 }
 
-static int axp288_charger_handle_cable_evt(struct notifier_block *nb,
-					  unsigned long event, void *param)
+/*
+ * 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 axp288_charger_handle_cable0_evt(struct notifier_block *nb,
+					    unsigned long event, void *param)
 {
 	struct axp288_chrg_info *info =
-	    container_of(nb, struct axp288_chrg_info, cable.nb);
+		container_of(nb, struct axp288_chrg_info, cable.nb[0]);
+	schedule_work(&info->cable.work);
+	return NOTIFY_OK;
+}
 
+static int axp288_charger_handle_cable1_evt(struct notifier_block *nb,
+					    unsigned long event, void *param)
+{
+	struct axp288_chrg_info *info =
+		container_of(nb, struct axp288_chrg_info, cable.nb[1]);
 	schedule_work(&info->cable.work);
+	return NOTIFY_OK;
+}
 
+static int axp288_charger_handle_cable2_evt(struct notifier_block *nb,
+					    unsigned long event, void *param)
+{
+	struct axp288_chrg_info *info =
+		container_of(nb, struct axp288_chrg_info, cable.nb[2]);
+	schedule_work(&info->cable.work);
 	return NOTIFY_OK;
 }
 
@@ -766,9 +790,6 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
 	struct power_supply_config charger_cfg = {};
-	unsigned int cable_ids[] = { EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP,
-				     EXTCON_CHG_USB_DCP };
-
 	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
@@ -811,10 +832,12 @@ static int axp288_charger_probe(struct platform_device *pdev)
 
 	/* Register for extcon notification */
 	INIT_WORK(&info->cable.work, axp288_charger_extcon_evt_worker);
-	info->cable.nb.notifier_call = axp288_charger_handle_cable_evt;
+	info->cable.nb[0].notifier_call = axp288_charger_handle_cable0_evt;
+	info->cable.nb[1].notifier_call = axp288_charger_handle_cable1_evt;
+	info->cable.nb[2].notifier_call = axp288_charger_handle_cable2_evt;
 	for (i = 0; i < ARRAY_SIZE(cable_ids); i++) {
 		ret = devm_extcon_register_notifier(dev, info->cable.edev,
-						cable_ids[i], &info->cable.nb);
+					  cable_ids[i], &info->cable.nb[i]);
 		if (ret) {
 			dev_err(dev, "failed to register extcon notifier for %u: %d\n",
 				cable_ids[i], ret);
-- 
2.9.3

Re: [PATCH 0/1] power: supply: axp288_charger: Use one notifier_block per extcon cable

[Sebastian Reichel]

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

Hi,

On Wed, Dec 21, 2016 at 06:28:22PM +0100, Hans de Goede wrote:
> Here is one more axp288_charger fix, note this applies on top of
> v2 of my previous axp288_charger fixes series.

Thanks, queued.

-- Sebastian

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