Hello, [dropped Bartlomiej Zolnierkiewicz from Cc:] On Tue, May 17, 2022 at 09:38:56AM -0700, Guenter Roeck wrote: > On 5/17/22 07:26, Alexander Stein wrote: > > This adds the enable attribute which is used to differentiate if PWM duty > > means to switch off regulator and PWM or to keep them enabled but > > at inactive PWM output level. > > > > Signed-off-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx> > > --- > > Documentation/hwmon/pwm-fan.rst | 10 ++++ > > drivers/hwmon/pwm-fan.c | 95 +++++++++++++++++++++++++++++---- > > 2 files changed, 95 insertions(+), 10 deletions(-) > > > > diff --git a/Documentation/hwmon/pwm-fan.rst b/Documentation/hwmon/pwm-fan.rst > > index 82fe96742fee..0083480068d1 100644 > > --- a/Documentation/hwmon/pwm-fan.rst > > +++ b/Documentation/hwmon/pwm-fan.rst > > @@ -18,3 +18,13 @@ the hwmon's sysfs interface. > > The fan rotation speed returned via the optional 'fan1_input' is extrapolated > > from the sampled interrupts from the tachometer signal within 1 second. > > + > > +The driver provides the following sensor accesses in sysfs: > > + > > +=============== ======= ======================================================= > > +fan1_input ro fan tachometer speed in RPM > > +pwm1_enable rw keep enable mode, defines behaviour when pwm1=0 > > + 0=switch off regulator and disable PWM > > + 1=keep regulator enabled and set PWM to inactive level > > Unless I am missing something, I think we have (at least) three > conditions to handle: > > - regulator disabled (independent of pwm value) > - regulator enabled, pwm output disabled if pwm=0 > - regulator enabled, pwm output enabled and set to 0 (or, if inverted, > to maximum) if pwm=0 What is your expectation for a disabled PWM? https://lore.kernel.org/linux-pwm/20220517150555.404363-1-u.kleine-koenig@xxxxxxxxxxxxxx might be relevant. If you assume that a pwm might output the active level after disabling, the case "regulator enabled, pwm output disabled if pwm=0" sounds wrong. Would "pwm1_disable_on_zero" be a better name than "pwm1_enable"? The latter is completely unintuitive to me. Maybe go for 0 -> keep pwm and regulator on 1 -> disable pwm, keep regulator on 2 -> keep pwm on, disable regulator 3 -> disable pwm and regulator (so one bit for pwm and one for regulator), even if 1 is wrong/unusual/dangerous? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
