On Thu, Jan 9, 2020 at 11:07 PM Akinobu Mita <akinobu.mita@xxxxxxxxx> wrote: > > 2020年1月9日(木) 1:13 Rob Herring <robh@xxxxxxxxxx>: > > > > On Sun, Dec 22, 2019 at 11:10:22PM +0900, Akinobu Mita wrote: > > > The pwm-fan driver leaves the fun running when shutting down the system. > > > (On the other hand the gpio-fan driver stops it.) > > > > Seemms like we should have consistent behavior independent of what the > > underlying implementation uses. Is there actually a case you'd want to > > leave the fan on? It seems strange to disable in suspend and leave on in > > shutdown. > > I agree. I was trying to keep the current behavior unchanged, so I added > "disable-state-shutdown" property. But I can't think of any case we want > to leave the fun on in shutdown. > > So it's better to change the shutdown behavior and remove the option > completely or add "retain-state-shutdown" property instead. > (The "retain-state-shutdown" property is inspired by gpio-leds) I would just turn off the fan in shutdown and see if anyone complains. > > Wouldn't the shutdown state depend if the PWM off state is high or low? > > IIRC, i.MX PWM has a quirk that the PWM disabled state is high. Doesn't > > It could be possible to affect the shutdown behavior for pwm-fan. > There are three i.MX PWM drivers (pwm-imx1, pwm-imx27, and pwm-tpm). > Do you remember which one actually have such limitation? No. I believe the fix was to use pinctrl modes to force the state. And I think the issue was for suspend rather than shutdown (but it seems unlikely you'd want the fan off in suspend and on in shutdown). > > Maybe it should be handled by the PWM controller/chip driver and PWM core. > From the perspective of PWM user drivers for now, we have nothing to do > other than setting duty cycle zero and then disable PWM for stopping the > pwm-fan. > > > it also depend on what the PWM driver does in shutdown? > > I think so. But as far as I can see, no PWM drivers implement shutdown > callback. Do they need to? I'd assume most SoC's are powered off or put in reset which makes the PWN pin go low. Rob