Re: [RFC PATCH 2/4] pmbus: Add fan configuration support

Guenter Roeck
2017-07-12T15:10:48Z

On Wed, Jul 12, 2017 at 04:31:09PM +0930, Andrew Jeffery wrote:
>
> Indeed. Hence RFC in case I had overlooked something :) Clearly I have.
>
Not surprising. It isn't exceptionally well documented :-)

> >
> > > However, the addition of the callbacks was driven by the behaviour of
> > > the MAX31785, where some values written to PMBUS_FAN_COMMAND_1 trigger
> > > automated control, while others retain manual control. Patch 4/4 should
> > > provide a bit more context, though I've also outlined the behaviour in
> > > the commit message for this patch. I don't have a lot of experience
> > > with PMBus devices so I don't have a good idea if there's a better way
> > > to capture the behaviour that isn't so unconstrained in its approach.
> > >
> >
> > Many pmbus commands have side effects. I don't see how an explicit callback
> > would be different to overloading a standard register or to providing a virtual
> > register/command, whichever is more convenient.
>
> I'm going to experiment with the virtual registers. From your
> description above and looking at the comments in pmbus.h I think I can
> make something work (and drop the callbacks).
>
Excellent.

>
> Sure. FWIW I plan on sending a follow-up RFC based on the feedback
> you've given here, and I'll look to chop out pmbus_fan_ctrl. I was
> suspicious of needing it as well, but was after your input on the
> general approach and figured sending the patches was better than
> guessing at your potential feedback.
>
> If a follow-up isn't of interest and you'd definitely rather take on
> the work up yourself, let me know.
>

By all means, please go ahead. I got way too much on my plate already.

Thanks,
Guenter