Skip to content

Commit d0266d7

Browse files
jhovoldsre
jhovold
authored and
sre
committed
Revert "power: supply: qcom_battmgr: Register the power supplies after PDR is up"
This reverts commit b43f7dd. The offending commit deferred power-supply class device registration until the service-started notification is received. This triggers a NULL pointer dereference during boot of the Lenovo ThinkPad X13s and SC8280XP CRD as battery status notifications can be received before the service-start notification: Unable to handle kernel NULL pointer dereference at virtual address 00000000000005c0 ... Call trace: _acquire+0x338/0x2064 acquire+0x1e8/0x318 spin_lock_irqsave+0x60/0x88 _supply_changed+0x2c/0xa4 battmgr_callback+0x1d4/0x60c [qcom_battmgr] pmic_glink_rpmsg_callback+0x5c/0xa4 [pmic_glink] qcom_glink_native_rx+0x58c/0x7e8 qcom_glink_smem_intr+0x14/0x24 [qcom_glink_smem] __handle_irq_event_percpu+0xb0/0x2d4 handle_irq_event+0x4c/0xb8 As trying to serialise this is non-trivial and risks missing notifications, let's revert to registration during probe so that the driver data is all set up once the service goes live. The warning message during resume in case the aDSP firmware is not running that motivated the change can be considered a feature and should not be suppressed. Fixes: b43f7dd ("power: supply: qcom_battmgr: Register the power supplies after PDR is up") Signed-off-by: Johan Hovold <johan+linaro@kernel.org> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Link: https://lore.kernel.org/r/20240123160053.18331-1-johan+linaro@kernel.org Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
1 parent 6613476 commit d0266d7

File tree

1 file changed

+49
-60
lines changed

1 file changed

+49
-60
lines changed

‎drivers/power/supply/qcom_battmgr.c

Lines changed: 49 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,6 @@ struct qcom_battmgr_wireless {
282282

283283
struct qcom_battmgr {
284284
struct device *dev;
285-
struct auxiliary_device *adev;
286285
struct pmic_glink_client *client;
287286

288287
enum qcom_battmgr_variant variant;
@@ -1294,69 +1293,11 @@ static void qcom_battmgr_enable_worker(struct work_struct *work)
12941293
dev_err(battmgr->dev, "failed to request power notifications\n");
12951294
}
12961295

1297-
static char *qcom_battmgr_battery[] = { "battery" };
1298-
1299-
static void qcom_battmgr_register_psy(struct qcom_battmgr *battmgr)
1300-
{
1301-
struct power_supply_config psy_cfg_supply = {};
1302-
struct auxiliary_device *adev = battmgr->adev;
1303-
struct power_supply_config psy_cfg = {};
1304-
struct device *dev = &adev->dev;
1305-
1306-
psy_cfg.drv_data = battmgr;
1307-
psy_cfg.of_node = adev->dev.of_node;
1308-
1309-
psy_cfg_supply.drv_data = battmgr;
1310-
psy_cfg_supply.of_node = adev->dev.of_node;
1311-
psy_cfg_supply.supplied_to = qcom_battmgr_battery;
1312-
psy_cfg_supply.num_supplicants = 1;
1313-
1314-
if (battmgr->variant == QCOM_BATTMGR_SC8280XP) {
1315-
battmgr->bat_psy = devm_power_supply_register(dev, &sc8280xp_bat_psy_desc, &psy_cfg);
1316-
if (IS_ERR(battmgr->bat_psy))
1317-
dev_err(dev, "failed to register battery power supply (%ld)\n",
1318-
PTR_ERR(battmgr->bat_psy));
1319-
1320-
battmgr->ac_psy = devm_power_supply_register(dev, &sc8280xp_ac_psy_desc, &psy_cfg_supply);
1321-
if (IS_ERR(battmgr->ac_psy))
1322-
dev_err(dev, "failed to register AC power supply (%ld)\n",
1323-
PTR_ERR(battmgr->ac_psy));
1324-
1325-
battmgr->usb_psy = devm_power_supply_register(dev, &sc8280xp_usb_psy_desc, &psy_cfg_supply);
1326-
if (IS_ERR(battmgr->usb_psy))
1327-
dev_err(dev, "failed to register USB power supply (%ld)\n",
1328-
PTR_ERR(battmgr->usb_psy));
1329-
1330-
battmgr->wls_psy = devm_power_supply_register(dev, &sc8280xp_wls_psy_desc, &psy_cfg_supply);
1331-
if (IS_ERR(battmgr->wls_psy))
1332-
dev_err(dev, "failed to register wireless charing power supply (%ld)\n",
1333-
PTR_ERR(battmgr->wls_psy));
1334-
} else {
1335-
battmgr->bat_psy = devm_power_supply_register(dev, &sm8350_bat_psy_desc, &psy_cfg);
1336-
if (IS_ERR(battmgr->bat_psy))
1337-
dev_err(dev, "failed to register battery power supply (%ld)\n",
1338-
PTR_ERR(battmgr->bat_psy));
1339-
1340-
battmgr->usb_psy = devm_power_supply_register(dev, &sm8350_usb_psy_desc, &psy_cfg_supply);
1341-
if (IS_ERR(battmgr->usb_psy))
1342-
dev_err(dev, "failed to register USB power supply (%ld)\n",
1343-
PTR_ERR(battmgr->usb_psy));
1344-
1345-
battmgr->wls_psy = devm_power_supply_register(dev, &sm8350_wls_psy_desc, &psy_cfg_supply);
1346-
if (IS_ERR(battmgr->wls_psy))
1347-
dev_err(dev, "failed to register wireless charing power supply (%ld)\n",
1348-
PTR_ERR(battmgr->wls_psy));
1349-
}
1350-
}
1351-
13521296
static void qcom_battmgr_pdr_notify(void *priv, int state)
13531297
{
13541298
struct qcom_battmgr *battmgr = priv;
13551299

13561300
if (state == SERVREG_SERVICE_STATE_UP) {
1357-
if (!battmgr->bat_psy)
1358-
qcom_battmgr_register_psy(battmgr);
1359-
13601301
battmgr->service_up = true;
13611302
schedule_work(&battmgr->enable_work);
13621303
} else {
@@ -1371,9 +1312,13 @@ static const struct of_device_id qcom_battmgr_of_variants[] = {
13711312
{}
13721313
};
13731314

1315+
static char *qcom_battmgr_battery[] = { "battery" };
1316+
13741317
static int qcom_battmgr_probe(struct auxiliary_device *adev,
13751318
const struct auxiliary_device_id *id)
13761319
{
1320+
struct power_supply_config psy_cfg_supply = {};
1321+
struct power_supply_config psy_cfg = {};
13771322
const struct of_device_id *match;
13781323
struct qcom_battmgr *battmgr;
13791324
struct device *dev = &adev->dev;
@@ -1383,7 +1328,14 @@ static int qcom_battmgr_probe(struct auxiliary_device *adev,
13831328
return -ENOMEM;
13841329

13851330
battmgr->dev = dev;
1386-
battmgr->adev = adev;
1331+
1332+
psy_cfg.drv_data = battmgr;
1333+
psy_cfg.of_node = adev->dev.of_node;
1334+
1335+
psy_cfg_supply.drv_data = battmgr;
1336+
psy_cfg_supply.of_node = adev->dev.of_node;
1337+
psy_cfg_supply.supplied_to = qcom_battmgr_battery;
1338+
psy_cfg_supply.num_supplicants = 1;
13871339

13881340
INIT_WORK(&battmgr->enable_work, qcom_battmgr_enable_worker);
13891341
mutex_init(&battmgr->lock);
@@ -1395,6 +1347,43 @@ static int qcom_battmgr_probe(struct auxiliary_device *adev,
13951347
else
13961348
battmgr->variant = QCOM_BATTMGR_SM8350;
13971349

1350+
if (battmgr->variant == QCOM_BATTMGR_SC8280XP) {
1351+
battmgr->bat_psy = devm_power_supply_register(dev, &sc8280xp_bat_psy_desc, &psy_cfg);
1352+
if (IS_ERR(battmgr->bat_psy))
1353+
return dev_err_probe(dev, PTR_ERR(battmgr->bat_psy),
1354+
"failed to register battery power supply\n");
1355+
1356+
battmgr->ac_psy = devm_power_supply_register(dev, &sc8280xp_ac_psy_desc, &psy_cfg_supply);
1357+
if (IS_ERR(battmgr->ac_psy))
1358+
return dev_err_probe(dev, PTR_ERR(battmgr->ac_psy),
1359+
"failed to register AC power supply\n");
1360+
1361+
battmgr->usb_psy = devm_power_supply_register(dev, &sc8280xp_usb_psy_desc, &psy_cfg_supply);
1362+
if (IS_ERR(battmgr->usb_psy))
1363+
return dev_err_probe(dev, PTR_ERR(battmgr->usb_psy),
1364+
"failed to register USB power supply\n");
1365+
1366+
battmgr->wls_psy = devm_power_supply_register(dev, &sc8280xp_wls_psy_desc, &psy_cfg_supply);
1367+
if (IS_ERR(battmgr->wls_psy))
1368+
return dev_err_probe(dev, PTR_ERR(battmgr->wls_psy),
1369+
"failed to register wireless charing power supply\n");
1370+
} else {
1371+
battmgr->bat_psy = devm_power_supply_register(dev, &sm8350_bat_psy_desc, &psy_cfg);
1372+
if (IS_ERR(battmgr->bat_psy))
1373+
return dev_err_probe(dev, PTR_ERR(battmgr->bat_psy),
1374+
"failed to register battery power supply\n");
1375+
1376+
battmgr->usb_psy = devm_power_supply_register(dev, &sm8350_usb_psy_desc, &psy_cfg_supply);
1377+
if (IS_ERR(battmgr->usb_psy))
1378+
return dev_err_probe(dev, PTR_ERR(battmgr->usb_psy),
1379+
"failed to register USB power supply\n");
1380+
1381+
battmgr->wls_psy = devm_power_supply_register(dev, &sm8350_wls_psy_desc, &psy_cfg_supply);
1382+
if (IS_ERR(battmgr->wls_psy))
1383+
return dev_err_probe(dev, PTR_ERR(battmgr->wls_psy),
1384+
"failed to register wireless charing power supply\n");
1385+
}
1386+
13981387
battmgr->client = devm_pmic_glink_register_client(dev,
13991388
PMIC_GLINK_OWNER_BATTMGR,
14001389
qcom_battmgr_callback,

0 commit comments

Comments
 (0)