Hi, On Mon, Jan 11, 2021 at 09:35:32PM +0100, Uwe Kleine-König wrote: > 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 > > > <clemens.gru...@pqgruber.com> 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.
I see. This would also allow PWMs initialized in the bootloader (e.g. backlights) to stay on between the bootloader and Linux and avoid flickering. If no one objects, I would then no longer reset period and duty cycles in the driver (and for our projects, reset them in the bootloader code to avoid leaving PWMs on after a kernel panic and watchdog reset, etc.) And if there is no pre-known state of the registers, we actually need the .get_state function fully implemented. Thanks, Clemens