Hello, On Thu, Dec 17, 2020 at 06:43:04PM +0100, Clemens Gruber wrote: > On Wed, Dec 16, 2020 at 11:00:59PM -0500, Sven Van Asbroeck wrote: > > On Wed, Dec 16, 2020 at 7:53 AM Clemens Gruber > > <[email protected]> wrote: > > > > > > Implements .get_state to read-out the current hardware state. > > > > > > > I am not convinced that we actually need this. > > > > Looking at the pwm core, .get_state() is only called right after .request(), > > to initialize the cached value of the state. The core then uses the cached > > value throughout, it'll never read out the h/w again, until the next > > .request(). > > > > In our case, we know that the state right after request is always disabled, > > because: > > - we disable all pwm channels on probe (in PATCH v5 4/7) > > - .free() disables the pwm channel > > > > Conclusion: .get_state() will always return "pwm disabled", so why do we > > bother reading out the h/w? > > If there are no plans for the PWM core to call .get_state more often in > the future, we could just read out the period and return 0 duty and > disabled. > > Thierry, Uwe, what's your take on this?
I have some plans here. In the past I tried to implement them (see commit 01ccf903edd65f6421612321648fa5a7f4b7cb10), but this failed (commit 40a6b9a00930fd6b59aa2eb6135abc2efe5440c3). > > Of course, if we choose to leave the pwm enabled after .free(), then > > .get_state() can even be left out! Do we want that? Genuine question, I do > > not know the answer. > > I do not think we should leave it enabled after free. It is less > complicated if we know that unrequested channels are not in use. My position here is: A consumer should disable a PWM before calling pwm_put. The driver should however not enforce this and so should not modify the hardware state in .free(). Also .probe should not change the PWM configuration. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
