On Fri, May 06, 2022 at 04:29:13PM +0200, Uwe Kleine-König wrote: > [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@xxxxxxxxxxxxxx/ > > > > 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? > Correct. The "pwm" attribute sets the duty cycle. > > 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. > It is up to us to define whate it means exactly. If you are ok that "set duty cycle to 0" reflects "set duty cycle to 0, disable pwm, and turn off regulator", I would hope that you are ok with using the _enable attribute to do the same and leaving pwm==0 to do what it is supposed to do, ie to keep pwm control enabled and set the duty cycle to 0. Thanks, Guenter > > 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/ |
