Re: [PATCH v4 4/7] pwm: Add support for Azoteq IQS620A PWM generator

[Jeff LaBundy <jeff@labundy.com>]

Hi Uwe,

> > However, you bring up a really interesting point about preserving what may
> > have been done by the bootloader. The device holds itself in POR until its
> > supply reaches a sufficient level, so there isn't necessarily a functional
> > reason to manually issue a soft reset from the parent MFD driver.
> > 
> > I could get rid of the manual soft reset, and then simply sync both out_en
> > and duty_val in iqs620_pwm_probe which would allow iqs620_pwm_get_state to
> > pick up any changes made by the bootloader prior to the kernel coming up.
> 
> That sounds good. This way the PWM driver is independent of the MFD
> driver and does the right thing no matter if parent resets the chip or
> not.

Agreed on all counts.

>  
> > The only problem is that leds-pwm disables the pwm at start-up, so the end
> > result is the same anyway. Regardless of the behavior of any one consumer,
> > however, I'm slightly inclined to go with the second option as it seems to
> > be less restrictive and more maintainable. Let me know if you disagree.
> 
> With
> 
> 	default-state = "keep";
> 
> in your dt the LED shouldn't get disabled.

I see default-state defined as a common LED property, but leds-pwm doesn't
seem to use it unfortunately. Looking through its code, brightness is just
initialized to zero unconditionally.

This doesn't change what is the right thing to do, nor do I imagine it to
be a problem for typical use cases, just noting for completeness (however
if I am mistaken please let me know).

> 
> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Kind regards,
Jeff LaBundy