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

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2559 bytes --]

[Dropped Bartlomiej Zolnierkiewicz from Cc:; my mailer daemon claims the
email address doens't exist.]

Hello Guenter,

On Fri, May 06, 2022 at 07:12:44AM -0700, Guenter Roeck wrote:
> On Fri, May 06, 2022 at 02:23:11PM +0200, Alexander Stein wrote:
> > Am Freitag, 6. Mai 2022, 12:23:01 CEST schrieb Uwe Kleine-König:
> > > See
> > > https://lore.kernel.org/linux-pwm/20180806155129.cjcc7okmwtaujf43@pengutronix.de/
> > > for one of the previous discussions.
> > 
> > Thanks for the link. I took a look into it. I'm on your side here, IMHO 
> > pwm_disable() implies that the PWM perphery is disabled, including any clocks 
> > or powerdomain. This is what pwm-imx27 actually does. This might lead to a, 
> > probably platform dependent, (undefined?) state of the PWM output pin.
> > This implies it is not possible to disable the PWM periphery for inverted 
> > signals, if the disabled state is not the inactive level. You know all about 
> > it already.
> > Then again from pwm-fan side I want be able to disable the FAN, turning of 
> > regulator and PWM, so powersaving is possible. That's what this patch is 
> > about. This is similar also what pwm_bl is doing.
> > Independent of the exact semantics, it makes sense to disable the regulator in 
> > pwm-fan as well when the fan shall be disabled.
> 
> There are fans which never stop if pwm==0, such as some CPU fans. I don't

I assume with pwm==0 you actually mean duty_cycle == 0?

> think it is a good idea to force those off by turning off their power. The
> problem in the driver is that it treats pwm==0 as "disable pwm", not as
> "set pwm output to 0", Part of the probem may be that the ABI doesn't have
> a good representation for "disable pwm output", which is what is really
> wanted/needed here.

Disable pwm output == set pwm output to High-Z? Not all PWMs are able to
provide that.

> I think the best solution would be to implement and
> use pwmX_enable, and define in the driver documentation that pwm1_enable=0
> reflects "disable pwm" and pwm1_enable=1 reflects "emable manual pwm
> control:. At the same time, stop associating "pwm==0" with "disable pwm",
> but just set the pwm output value to 0.

Are you talking about the PWM framework here, or only the pwm-fan
driver?

I'd expect there are better names than pwm1_enable for the intended
semantic.

Best regards
Uwe

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]