Re: Enabling pmbus power control

[Zev Weiss]

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