Re: [PATCH v2 1/3] pwm: make it possible to apply pwm changes in atomic context

[Sean Young <sean@mess.org>]

On Fri, Oct 13, 2023 at 01:51:40PM +0200, Thierry Reding wrote:
> On Fri, Oct 13, 2023 at 11:46:14AM +0100, Sean Young wrote:
> [...]
> > diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> > index d2f9f690a9c1..93f166ab03c1 100644
> > --- a/include/linux/pwm.h
> > +++ b/include/linux/pwm.h
> > @@ -267,6 +267,7 @@ struct pwm_capture {
> >   * @get_state: get the current PWM state. This function is only
> >   *	       called once per PWM device when the PWM chip is
> >   *	       registered.
> > + * @atomic: can the driver execute pwm_apply_state in atomic context
> >   * @owner: helps prevent removal of modules exporting active PWMs
> >   */
> >  struct pwm_ops {
> > @@ -278,6 +279,7 @@ struct pwm_ops {
> >  		     const struct pwm_state *state);
> >  	int (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,
> >  			 struct pwm_state *state);
> > +	bool atomic;
> >  	struct module *owner;
> >  };
> 
> As I mentioned earlier, this really belongs in struct pwm_chip rather
> than struct pwm_ops. I know that Uwe said this is unlikely to happen,
> and that may be true, but at the same time it's not like I'm asking
> much. Whether you put this in struct pwm_ops or struct pwm_chip is
> about the same amount of code, and putting it into pwm_chip is much
> more flexible, so it's really a no-brainer.

Happy to change this of course. I changed it and then changed it back after
Uwe's comment, I'll fix this in the next version.

One tiny advantage is that pwm_ops is static const while pwm_chip is
allocated per-pwm, so will need instructions for setting the value. Having
said that, the difference is tiny, it's a single bool.


Sean