Re: [PATCH 2/2] pwm: Add GPIO PWM driver

[Vincent Whitchurch <vincent.whitchurch@axis.com>]

On Sat, Aug 15, 2020 at 09:50:32AM +0200, Uwe Kleine-König wrote:
> On Fri, Aug 14, 2020 at 05:55:13PM +0200, Vincent Whitchurch wrote:
> > Add a software PWM which toggles a GPIO from a high-resolution timer.
> > 
> > This will naturally not be as accurate or as efficient as a hardware
> > PWM, but it is useful in some cases.  I have for example used it for
> > evaluating LED brightness handling (via leds-pwm) on a board where the
> > LED was just hooked up to a GPIO, and for a simple verification of the
> > timer frequency on another platform.
> > 
> > Since high-resolution timers are used, sleeping gpio chips are not
> > supported and are rejected in the probe function.
> > 
> > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> 
> Nice idea. IMHO this can serve as example about what we expect from pwm
> drivers. There is some improvement necessary however to reach that
> state.

Thank you for the comments.  I think I've fixed all of them in v2.  Just
one point about this one:

> > +
> > +	return 0;
> > +}
> > +
> > +static const struct pwm_ops pwm_gpio_ops = {
> > +	.owner = THIS_MODULE,
> > +	.apply = pwm_gpio_apply,
> 
> Usually a .get_state callback is nice. Does it make sense to do
> something like:
> 
> 	if (driver is up)
> 		report current setting
> 	else
> 		val = gpio_get_value()
> 		report(period=1, duty_cycle=val, enabled=val, polarity=NORMAL)
> 
> ?

I implemented get_state() but I didn't do the gpio_get_value() thing
since the driver sets the gpio to a known value in probe().