Re: Enabling pmbus power control
From: Guenter Roeck
Date: Tue Mar 30 2021 - 06:35:24 EST
On 3/30/21 1:17 AM, Zev Weiss wrote:
>
> Hello,
>
> I'm working on a board that has a handful of LM25066 PMICs controlling
> the power supply to various devices, and I'd like to have both the
> existing hwmon sensor functionality as well as userspace power on/off
> control, which does not currently seem to be available (other than via
> 'i2cset -f', which I'd of course prefer to avoid). I've drafted up a
> couple possible versions of this, and was hoping to get some opinions
> on the appropriate overall approach.
>
> One option is to add a read-write sysfs attribute to the existing
> hwmon directory (current incarnation of the patch:
> https://thorn.bewilderbeest.net/~zev/patches/pmbus-statectl.patch).
> This bears a vague resemblance to a patch that was rejected a couple
> years ago
> (https://lore.kernel.org/linux-hwmon/20190417161817.GA13109@xxxxxxxxxxxx/),
> but is different enough that I wonder if it might potentially be
> tolerable? (It exposes significantly less, for one thing.)
>
This is a no-go. We are not going to replicate regulator functionality
in the hwmon subsystem, no matter by what means.
> The other approach involves layering a regulator device over the pmbus
> device as is done in the LTC2978 driver, and then putting a
> reg-userspace-consumer on top of that (current patch:
> https://thorn.bewilderbeest.net/~zev/patches/pmbus-ureg.patch). My
This is the way to go, but the regulator descriptor (what is currently
CONFIG_PMBUS_USERSPACE_REGULATOR_CONSUMER) should be in the lm25066
driver. I don't want to pollute the pmbus core with that at this point
(and I don't know if the userspace consumer code is appropriate - you
might want to check with the regulator maintainer on that).
> first attempt at this ran into problems with all the
> reg-userspace-consumer instances getting attached to the first
> regulator device, I think due to all of the regulators ending up under
> the same name in the global namespace of regulator_map_list. I worked
> around that by adding an ID counter to produce a unique name for each,
> though that changes device names in userspace-visible ways that I'm
> not sure would be considered OK for backwards compatibility. (I'm not
> familiar enough with the regulator code to know if there's a better
> way of fixing that problem.) The #if-ing to keep it behind a Kconfig
Maybe ask that question on the regulator mailing list.
Guenter
> option is also kind of ugly as it stands.
>
> The first version seems simpler to me (and avoids the rather more
> cumbersome sysfs paths the second one produces, for what that's
> worth). I think the second is (at least structurally) perhaps more
> aligned with what Guenter was saying in the previous discussion linked
> above, though. Does anyone have any advice on the best way to proceed
> with this? If the reg-userspace-consumer approach is the preferred
> route, suggestions on a better fix for the name collision problem
> would be welcome.
>
>
> Thanks,
> Zev
>