Re: [PATCH 2/2] power: supply: qcom_battmgr: Add support batteryless boards as MAINS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The new MAINS power supply exposes BATTERY properties (STATUS, PRESENT) instead of ONLINE, breaking userspace expectations.
- [Medium] The `qcom,batteryless` property is silently ignored on `SC8280XP` and `X1E80100` variants.
--

commit fd3fd36bafd15a4f0b8a90d2f6b2f13ff0fc3a3d
Author: Rakesh Kota <rakesh.kota@oss.qualcomm.com>

power: supply: qcom_battmgr: Add support batteryless boards as MAINS

This commit adds support for the qcom,batteryless device tree property to
register the power supply as a MAINS type instead of a BATTERY. This is
intended to prevent userspace from triggering battery power-saving sequences
on boards powered directly by adapters.

> diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
> index 490137a23d00e..35d266375d8c4 100644
> --- a/drivers/power/supply/qcom_battmgr.c
> +++ b/drivers/power/supply/qcom_battmgr.c
[ ... ]
> @@ -930,6 +931,47 @@ static const struct power_supply_desc sm8550_bat_psy_desc = {
>  	.property_is_writeable = qcom_battmgr_bat_is_writeable,
>  };
>  
> +static int qcom_battmgr_dcin_get_property(struct power_supply *psy,
> +					  enum power_supply_property psp,
> +					  union power_supply_propval *val)
> +{
> +	struct qcom_battmgr *battmgr = power_supply_get_drvdata(psy);
> +	int ret;
> +
> +	if (!battmgr->service_up)
> +		return -EAGAIN;
> +
> +	ret = qcom_battmgr_bat_sm8350_update(battmgr, psp);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_STATUS:
> +		val->intval = battmgr->status.status;
> +		break;
> +	case POWER_SUPPLY_PROP_PRESENT:
> +		val->intval = battmgr->info.present;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const enum power_supply_property dcin_props[] = {
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_PRESENT,
> +};
> +
> +static const struct power_supply_desc dcin_psy_desc = {
> +	.name = "qcom-battmgr-dcin",
> +	.type = POWER_SUPPLY_TYPE_MAINS,
> +	.properties = dcin_props,
> +	.num_properties = ARRAY_SIZE(dcin_props),
> +	.get_property = qcom_battmgr_dcin_get_property,
> +};

Since dcin_psy_desc is registered as POWER_SUPPLY_TYPE_MAINS, should it
expose POWER_SUPPLY_PROP_ONLINE instead of POWER_SUPPLY_PROP_STATUS and
POWER_SUPPLY_PROP_PRESENT?

Userspace power managers typically expect MAINS power supplies to report their
active state via the ONLINE property.

If the firmware returns PRESENT=0 on a batteryless board, won't userspace
fail to recognize the AC adapter as an active power source and still trigger
power-saving or shutdown sequences?

[ ... ]
> @@ -1690,7 +1733,9 @@ static int qcom_battmgr_probe(struct auxiliary_device *adev,
>  			return dev_err_probe(dev, PTR_ERR(battmgr->wls_psy),
>  					     "failed to register wireless charing power supply\n");
>  	} else {
> -		if (battmgr->variant == QCOM_BATTMGR_SM8550)
> +		if (battmgr->batteryless)
> +			psy_desc = &dcin_psy_desc;
> +		else if (battmgr->variant == QCOM_BATTMGR_SM8550)
>  			psy_desc = &sm8550_bat_psy_desc;
>  		else
>  			psy_desc = &sm8350_bat_psy_desc;

Does this bypass the qcom,batteryless property on SC8280XP and X1E80100
variants?

Looking at qcom_battmgr_probe(), the check for battmgr->batteryless is only
placed inside the else block which executes for SM8350 and SM8550:

drivers/power/supply/qcom_battmgr.c:qcom_battmgr_probe() {
...
	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
	    battmgr->variant == QCOM_BATTMGR_X1E80100) {
		if (battmgr->variant == QCOM_BATTMGR_X1E80100)
			psy_desc = &x1e80100_bat_psy_desc;
		else
			psy_desc = &sc8280xp_bat_psy_desc;

		battmgr->bat_psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
...
	} else {
		if (battmgr->batteryless)
			psy_desc = &dcin_psy_desc;
...
}

If a board uses the SC8280XP or X1E80100 variant, won't it bypass this check
and unconditionally register the standard battery power supply?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-add_dc_in_support-v1-0-31fbaa329879@oss.qualcomm.com?part=2