Re: Enabling pmbus power control

[Zev Weiss <zev@bewilderbeest.net>]

On Tue, Apr 20, 2021 at 05:52:16AM CDT, Guenter Roeck wrote:
>On 4/20/21 12:06 AM, Zev Weiss wrote:
>> On Tue, Apr 20, 2021 at 01:00:25AM CDT, Guenter Roeck wrote:
>>> On 4/19/21 10:50 PM, Zev Weiss wrote:
>>> [ ... ]
>>>
>>>> I had a glance at the enclosure driver; it looks pretty geared toward SES-like things (drivers/scsi/ses.c being its only usage I can see in the kernel at the moment) and while it could perhaps be pressed into working for this it seems like it would probably drag in a fair amount of boilerplate and result in a somewhat gratuitously confusing driver arrangement (calling the things involved in the cases we're looking at "enclosures" seems like a bit of a stretch).
>>>>
>>>> As an alternative, would something like the patch below be more along the lines of what you're suggesting?  And if so, would it make sense to generalize it into something like 'pmbus-switch.c' and add a PMBUS_HAVE_POWERSWITCH functionality bit or similar in the pmbus code instead of hardcoding it for only LM25066 support?
>>>>
>>>>
>>>
>>> No. Don't access pmbus functions from outside drivers/hwmon/pmbus.
>>>
>>> I used to be opposed to function export restrictions (aka export namespaces),
>>> but you are making a good case that we need to introduce them for pmbus
>>> functions.
>>>
>>> Guenter
>>
>> Okay -- I figured that was likely to be frowned upon, but the alternative seemed to be effectively duplicating non-trivial chunks of the pmbus code.  However, upon realizing that the LM25066 doesn't actually require any of the paging work the generic pmbus code provides, I suppose it can actually be done with a simple smbus read & write.  Does this version look better?
>>
>
>It is just getting worse and worse. You are re-implementing regulator
>support for the lm25066. The correct solution would be to implement
>regulator support in the lm25066 and use it from the secondary driver
>(which should be chip independent).
>
>Guenter
>

Implementing it by way of regulator support seems like it would make 
sense, but that in turn sounds like it would then end up just being a 
reimplementation of REGULATOR_USERSPACE_CONSUMER, which (unless there 
was some more subtle distinction that I missed) Mark already told me in 
no uncertain terms is not the way to go.  So I hope I could be forgiven 
for feeling a bit stuck here.

Mark, do you have any further input on what a viable approach might look 
like?


Thanks,
Zev