On Fri, Oct 20, 2023 at 02:11:48PM +0200, Uwe Kleine-König wrote: > Hello Daniel, > > On Fri, Oct 20, 2023 at 12:27:27PM +0100, Daniel Thompson wrote: > > On Wed, Oct 18, 2023 at 11:07:41PM +0200, Uwe Kleine-König wrote: > > > Hello Philipp, > > > > > > On Thu, Jun 08, 2023 at 04:11:14PM +0200, Philipp Zabel wrote: > > > > The initial PWM state returned by pwm_init_state() has a duty cycle > > > > of 0 ns. > > > > > > This is only true for drivers without a .get_state() callback, isn't it? > > > > pwm_init_state() explicitly zeros the duty-cycle in order to avoid > > problems when the default args have a different period to the currently > > applied config: > > https://elixir.bootlin.com/linux/latest/source/include/linux/pwm.h#L174 > > Ah right, pwm_init_state() is strange in a different way than I > remembered :-) pwm_get_state() is only called to get .enabled set > appropriately. > > Looking at the callers: > > <snip> > - drivers/video/backlight/lm3630a_bl.c > explictily sets .enabled before calling pwm_apply_state() > > - drivers/video/backlight/lp855x_bl.c > explictily sets .enabled before calling pwm_apply_state() > > - drivers/video/backlight/pwm_bl.c > This is the one we currently discuss. I think even with the patch > applied it uses the .enabled value returned by pwm_init_state() but > it shouldn't. Agreed. > So all consumers using pwm_init_state() either don't use the .enabled > value returned by pwm_init_state() or at least shouldn't do that. Looking a little deeper in the PWM code, it looks to me like pwm_bl.c could just use pwm_adjust_config() during probe to transition between bootloader settings and kernel settings! Daniel.
