Re: (EXT) Re: [PATCH v2 1/1] hwmon: pwm-fan: dynamically switch regulator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello,

On Fri, May 06, 2022 at 09:15:55AM +0200, Alexander Stein wrote:
> Am Freitag, 6. Mai 2022, 00:00:37 CEST schrieb Guenter Roeck:
> > On Wed, May 04, 2022 at 02:45:51PM +0200, Alexander Stein wrote:
> > > From: Markus Niebel <Markus.Niebel@xxxxxxxxxxxxxxx>
> > > 
> > > A pwm value equal to zero is meant to switch off the pwm
> > > hence also switching off the fan. Currently the optional
> > > regulator is always on. When using this driver on boards
> > > with an inverted pwm signal polarity this can cause running
> > > the fan at maximum speed when setting pwm to zero.
> > 
> > The appropriate solution in this case would be to tell the
> > software that the pwm is inverted. Turning off the regulator
> > in that situation is a bad idea since setting the pwm value to
> > 1 would set it to almost full speed. That does not really make
> > sense.
> 
> The pwm-fan driver is already configured for inverted PWM (ommited some 
> properties for shortness):
> fan0: pwm-fan {
> 	compatible = "pwm-fan";
> 	fan-supply = <&reg_pwm_fan>;
> 	pwms = <&pwm3 0 40000 PWM_POLARITY_INVERTED>;
> 	cooling-levels = <0 32 64 128 196 240>;
> [...]
> };
> 
> The problem here is that the pwm-fan driver currently enables the regulator 
> unconditionally, but the PWM only when the fan is enabled, refer to 
> __set_pwm(). This results in a fan at full speed when pwm-fan is idle, as pwm 
> state is not enabled.

Which PWM driver are you using?

There is an implicit assumption in some PWM consumers that a disabled
PWM emits the inactive level. However not all PWMs do this. Is this such
a case? 

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux