linux-kernel - Re: Enabling pmbus power control

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5105ada1-643a-8e58-a52d-d3c8dbef86b9@roeck-us.net>
Date:   Tue, 30 Mar 2021 03:34:16 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Zev Weiss <zev@...ilderbeest.net>,
        Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org
Cc:     Andrew Jeffery <andrew@...id.au>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org,
        openbmc@...ts.ozlabs.org
Subject: Re: Enabling pmbus power control

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@roeck-us.net/),
> 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
>