Re: [PATCH v5 5/5] power: supply: ltc3350-charger: Add driver

[Mike Looijmans <mike.looijmans@topic.nl>]

On 27-05-2024 22:40, Sebastian Reichel wrote:
>> ...
>> +static int ltc3350_get_property(struct power_supply *psy, enum power_supply_property psp,
>> +				union power_supply_propval *val)
>> +{
>> +	struct ltc3350_info *info = power_supply_get_drvdata(psy);
>> +
>> +	switch (psp) {
>> +	case POWER_SUPPLY_PROP_STATUS:
>> +		return ltc3350_get_status(info, val);
>> +	case POWER_SUPPLY_PROP_CHARGE_TYPE:
>> +		return ltc3350_get_charge_type(info, val);
>> +	case POWER_SUPPLY_PROP_ONLINE:
>> +		return ltc3350_get_online(info, val);
>> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>> +		return ltc3350_get_scaled(info, LTC3350_REG_MEAS_VOUT, 22100, &val->intval);
>> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
>> +		return ltc3350_get_scaled(info, LTC3350_REG_VOUT_UV_LVL, 22100, &val->intval);
>> +	case POWER_SUPPLY_PROP_VOLTAGE_MAX:
>> +		return ltc3350_get_scaled(info, LTC3350_REG_VOUT_OV_LVL, 22100, &val->intval);
> For USB chargers VOLTAGE_NOW/MIN/MAX refers to VBUS, which is the
> voltage on the USB lines and thus the input voltage. From my
> understanding of the LTC3350 this would be VIN and not VOUT. With
> VOUT being supplied by either VIN or the Capacitors.
>
> So I think your custom vin/vin_uv/vin_ov should be exposed with the
> above properties.

Yeah, power supplies report their input voltage. So this should report VIN.


> My understanding for VOUT is, that this is basically the system
> supply - I would expect more regulators to follow, which convert
> it to typical voltages like 3.3V or 1.8V. In that case it would
> make sense to expose VOUT as regulator, so that it can be referenced
> as vin-supply. You can find a few examples for charger drivers doing
> that for USB-OTG functionality.

Looked at other drivers for that. Significant difference is that those 
have something useful to supply to other drivers, e.g. being able to 
enable/disable the output for one thing.

For the LTC3350, the output (voltage) is just a measurement and there's 
nothing for the driver to configure. Any regulator_ops would be 
completely empty.

Given that, I think it should get the same treatment as GPI, so either a 
custom property or some other device (IIO). Or maybe add a VOLTAGE_OUT 
property? It's not uncommon for power management devices to report their 
output voltage.


>

-- 
Mike Looijmans
System Expert

TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl