[PATCH 1/4] pwm: Add new helper to initialize a pwm_state variable with defaults

[u.kleine-koenig@pengutronix.de (Uwe Kleine-König)]

Hello Thierry,

On Wed, Nov 14, 2018 at 01:32:36PM +0100, Thierry Reding wrote:
> On Fri, Oct 26, 2018 at 08:41:55PM +0200, Uwe Kleine-K?nig wrote:
> > This helps to convert drivers from the legacy API to pwm_apply_state without
> > having to make the aware of the configured polarity (and in some cases even
> > period).
> > 
> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > ---
> >  include/linux/pwm.h | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> So if I understand your intention correctly here you want consumers to
> be able to get at the default state, which would be the state as it was
> "configured" in DT or the PWM lookup table so that you can modify that
> state (well, really just setting the duty-cycle) before you actually
> send the state to the hardware.
> 
> So the difference to pwm_apply_args() is that you don't want the driver
> to specifically program a duty cycle of 0 before setting the actual duty
> cycle that you want to set.
> 
> Is that about right?

Yes, and a resulting advantage is that the apply callback is only called
once.
 
> Isn't that exactly what pwm_init_state() already does? pwm-backlight
> uses it for exactly the same purpose that you seem to be using it in
> RX1950 backlight control.

I didn't use pwm_init_state because that doesn't give a helpful value
for state->enabled. (I think it is: if the driver provides a get_state
callback it is whatever that one set, otherwise it's off.)
Also the name pwm_init_state isn't that good IMHO. Just from the name
I'd expect it to be something like

	memset(state, 0, sizeof(&state));

instead of something more informed that depends on hardware state and/or
data provided by a device tree.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |