Re: [PATCH v10 5/6] power: supply: max77759: add charger driver

[Sebastian Reichel <sre@kernel.org>]

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

Hi,

On Tue, Mar 31, 2026 at 11:22:20PM +0000, Amit Sunil Dhamne via B4 Relay wrote:
> +/* Charge Termination Voltage Limits (in mV) */
> +static const struct linear_range chg_cv_prm_ranges[] = {
> +	LINEAR_RANGE(3800, 0x38, 0x39, 100),
> +	LINEAR_RANGE(4000, 0x0, 0x32, 10),
> +};

Let me quote from include/linux/power_supply.h:

 * All voltages, currents, charges, energies, time and temperatures in uV,
 * µA, µAh, µWh, seconds and tenths of degree Celsius unless otherwise
 * stated. It's driver's job to convert its raw values to units in which
 * this class operates.

What makes you think that CONSTANT_CHARGE_VOLTAGE_MAX is
special?

[...]

> +static int max77759_charger_get_property(struct power_supply *psy,
> +					 enum power_supply_property psp,
> +					 union power_supply_propval *pval)
> +{
> +	struct max77759_charger *chg = power_supply_get_drvdata(psy);
> +	int ret;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		ret = get_online(chg);
> +		break;
> +	case POWER_SUPPLY_PROP_PRESENT:
> +		ret = charger_input_valid(chg);
> +		break;
> +	case POWER_SUPPLY_PROP_STATUS:
> +		ret = get_status(chg);
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_TYPE:
> +		ret = get_charge_type(chg);
> +		break;
> +	case POWER_SUPPLY_PROP_HEALTH:
> +		ret = get_health(chg);
> +		break;
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
> +		ret = get_fast_charge_current(chg);
> +		break;
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
> +		ret = get_float_voltage(chg);
> +		break;
> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +		ret = get_input_current_limit(chg);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	pval->intval = ret;
> +	return ret < 0 ? ret : 0;

As people like to use existing drivers as reference this definitely
needs a comment, that none of the properties used by this driver
support negative values. This is not a general thing as e.g. the
CHARGE current may be negative depending on the battery being
charged or discharged (OTG mode).

Greetings,

-- Sebastian

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