Received-SPF: pass (google.com: domain of linux-kernel+bounces-198462-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Mon, 3 Jun 2024 00:40:20 +0200
From: Sebastian Reichel <sebastian.reichel@collabora.com>
To: Mike Looijmans <mike.looijmans@topic.nl>
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 5/5] power: supply: ltc3350-charger: Add driver
Message-ID: <wqvc3uasq4bzkwuhdpaeu34hyhag2h2trff4ryhecenjvx6nei@bx4sdp66oxew>
References: <20240416121818.543896-1-mike.looijmans@topic.nl>
 <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.949ef384-8293-46b8-903f-40a477c056ae.4a7cccf3-e7cb-4980-be8e-de8bd2795354@emailsignatures365.codetwo.com>
 <20240416121818.543896-5-mike.looijmans@topic.nl>
 <t4avn7fmzw355a73hu3yu4wzdxyc5gkupi5s4yfio5iyiygkx6@5eubzdbqthqx>
 <fb93cdd9-0c9b-4cb1-91c1-ccf832fcba75@topic.nl>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="hnayuqdj26rfnhzn"
Content-Disposition: inline
In-Reply-To: <fb93cdd9-0c9b-4cb1-91c1-ccf832fcba75@topic.nl>


--hnayuqdj26rfnhzn
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi,

On Tue, May 28, 2024 at 11:41:36AM +0200, Mike Looijmans wrote:
> 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 =3D 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, &v=
al->intval);
> > > +	case POWER_SUPPLY_PROP_VOLTAGE_MAX:
> > > +		return ltc3350_get_scaled(info, LTC3350_REG_VOUT_OV_LVL, 22100, &v=
al->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.
> >=20
> > So I think your custom vin/vin_uv/vin_ov should be exposed with the
> > above properties.
>=20
> Yeah, power supplies report their input voltage. So this should
> report VIN.
>=20
> > 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.
>=20
> Looked at other drivers for that. Significant difference is that those ha=
ve
> something useful to supply to other drivers, e.g. being able to
> enable/disable the output for one thing.
>=20
> 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.

Which is also true for fixed regulators, see fixed_voltage_ops in
drivers/regulator/fixed.c. Not having any control options is not a
problem per se.

Note, that regulators have pre-defined events for the functionality
you are trying to provide for VOUT, e.g.
 - REGULATOR_EVENT_OVER_VOLTAGE_WARN
 - REGULATOR_EVENT_UNDER_VOLTAGE
 - REGULATOR_EVENT_UNDER_VOLTAGE_WARN

> 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.

power-supply handles batteries and battery chargers. For both cases
the kernel already has properties for the output voltage (which
would be the charging voltage in case of a charger). Other PM voltage
regulation elements are so far handled via the regulator subsystem
and hwmon is used for monitoring.

Greetings,

-- Sebastian

--hnayuqdj26rfnhzn
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEE72YNB0Y/i3JqeVQT2O7X88g7+poFAmZc9NEACgkQ2O7X88g7
+prK8g/+KrIg6jyostU7AAlJ6XQjuFjR/XRqyoLCaZ9VZvfEudd4volkjse8Oxrp
T95sQd0cJXRP6rzXNAhmKZjY+XmNMW3zJIO99rLArWjsTIc0ryru2K/dc3kyL3OZ
Z1hVyvi9qUqHjXg5Ci1A34etVNOAqbo4n4taAg3Z8PC/crL34zFClUlcshhIgrZ6
qDyGw/ZzFWW3FXlvMxuxpeYQkIxnhnvMzhk08fcDmbOmLxGjvK9INBIDIzt67Xci
TiE+uZemOcvOZA65CiqXIxw1XtHv4IOjA3bZ96YmqP7mdERsXkbYkjlWZKVRR+9K
n9QLdLdexWwK7xr8IxETj894B9JCM3Of9pUkuq6tJzZWu4k4sWL40un0RAZTMDpl
78mIJ3USM0kFrFmLRgJNQWmb5V8v6NCtEKM+yARL3G/x9OPp4yc0c1iCnNgHCoMT
sEVVc1PQy/Jt/TUz0AB7MeNJvS7CcK57/iygHP/Gny5DqmTCbC+rNvuvbp+3CoGs
48oFoazcvsclCNNfUBo+cqr0uEnPt9sCZlb2+aoEkKr4FXDFBIA/j8fCoX6pVeh0
f0VsHj2b/LALV8TLORWjBz9SDptMEFxAigYbhKaAzPF7h/zc+44euj93OkI9vG2E
/Z4idtNcFxvhTMRkedkU3sefOcvIbcAL2Lqg3Hbhp7UmHSE8/PU=
=P2wV
-----END PGP SIGNATURE-----

--hnayuqdj26rfnhzn--