Skip to content

Commit

Permalink
phy: ti: tusb1210: Resolve charger-det crash if charger psy is unregi…
Browse files Browse the repository at this point in the history
…stered

[ Upstream commit bf6e4ee ]

The power_supply frame-work is not really designed for there to be
long living in kernel references to power_supply devices.

Specifically unregistering a power_supply while some other code has
a reference to it triggers a WARN in power_supply_unregister():

	WARN_ON(atomic_dec_return(&psy->use_cnt));

Folllowed by the power_supply still getting removed and the
backing data freed anyway, leaving the tusb1210 charger-detect code
with a dangling reference, resulting in a crash the next time
tusb1210_get_online() is called.

Fix this by only holding the reference in tusb1210_get_online()
freeing it at the end of the function. Note this still leaves
a theoretical race window, but it avoids the issue when manually
rmmod-ing the charger chip driver during development.

Fixes: 48969a5 ("phy: ti: tusb1210: Add charger detection")
Signed-off-by: Hans de Goede <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
  • Loading branch information
jwrdegoede authored and gregkh committed May 2, 2024
1 parent 4201b8c commit 25b3498
Showing 1 changed file with 12 additions and 11 deletions.
23 changes: 12 additions & 11 deletions drivers/phy/ti/phy-tusb1210.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ struct tusb1210 {
struct delayed_work chg_det_work;
struct notifier_block psy_nb;
struct power_supply *psy;
struct power_supply *charger;
#endif
};

Expand Down Expand Up @@ -230,19 +229,24 @@ static const char * const tusb1210_chargers[] = {

static bool tusb1210_get_online(struct tusb1210 *tusb)
{
struct power_supply *charger = NULL;
union power_supply_propval val;
int i;
bool online = false;
int i, ret;

for (i = 0; i < ARRAY_SIZE(tusb1210_chargers) && !tusb->charger; i++)
tusb->charger = power_supply_get_by_name(tusb1210_chargers[i]);
for (i = 0; i < ARRAY_SIZE(tusb1210_chargers) && !charger; i++)
charger = power_supply_get_by_name(tusb1210_chargers[i]);

if (!tusb->charger)
if (!charger)
return false;

if (power_supply_get_property(tusb->charger, POWER_SUPPLY_PROP_ONLINE, &val))
return false;
ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_ONLINE, &val);
if (ret == 0)
online = val.intval;

power_supply_put(charger);

return val.intval;
return online;
}

static void tusb1210_chg_det_work(struct work_struct *work)
Expand Down Expand Up @@ -466,9 +470,6 @@ static void tusb1210_remove_charger_detect(struct tusb1210 *tusb)
cancel_delayed_work_sync(&tusb->chg_det_work);
power_supply_unregister(tusb->psy);
}

if (tusb->charger)
power_supply_put(tusb->charger);
}
#else
static void tusb1210_probe_charger_detect(struct tusb1210 *tusb) { }
Expand Down

0 comments on commit 25b3498

Please sign in to comment.