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
