On Tue, Jan 08, 2019 at 10:35:29AM +0100, Geert Uytterhoeven wrote: > Hi Uwe, > > On Tue, Jan 8, 2019 at 10:19 AM Uwe Kleine-König > <[email protected]> wrote: > > On Tue, Jan 08, 2019 at 09:56:28AM +0100, Geert Uytterhoeven wrote: > > > On Tue, Jan 8, 2019 at 8:48 AM Uwe Kleine-König > > > <[email protected]> wrote: > > > > Orthogonal to this patch I wonder what the intended behaviour for a pwm > > > > is on suspend. Should it stop oscilating unconditionally? Or should it > > > > only stop if the consumer stops it as part of its own suspend callback? > > > > > > I guess you mean system suspend, not runtime suspend, as the device is > > > runtime-resumed when a PWM is requested? > > > > I admit that suspend (both system and runtime) is a bit of a black box > > for me. So please take that into account when judging my statements. > > > > > Permitted behavior depends on the system: on R-Car Gen3 (arm64), PSCI > > > system > > > suspend will power down the SoC, so PWM output will stop for sure. > > > > > > On R-Car Gen2 (or R-Car Gen3 with s2idle instead of s2ram), the PM Domain > > > code will turn of the PWM module's clock. Hence it will stop oscillating, > > > unless > > > you take special countermeasures, like for modules that need to stay > > > powered > > > for wake-up handling. > > > > Whatever "suspend" here means, I want to prevent that a stopping pwm is > > a surprise for the consumer. So I think suspend should be inhibited if > > the consumer might expect the pwm to continue running but the pwm is > > about to stop. So if the suspend affects the consumer, too, it's the > > consumer that should be responsible to stop the pwm in a controlled > > manner. > > I think you can inhibit suspend by letting the .suspend() callback return an > error. However, I think the PWM driver cannot make that decision on its own. > Shutting down the PWM for a status LED is harmless, and the status LED being > enabled should not prevent a system suspend. > Shutting down e.g. a motor may be different.
Also this is something that should be implemented in the core (or at least consistently in all drivers). So I think it's not sensible to continue discussion that in the context of this rcar change. I added it to my list of stuff to discuss. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
