Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Date:   Fri, 13 Oct 2023 15:58:30 +0100
From:   Sean Young <sean@mess.org>
To:     Thierry Reding <thierry.reding@gmail.com>
Cc:     linux-media@vger.kernel.org,
        Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>,
        Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= 
        <u.kleine-koenig@pengutronix.de>, linux-pwm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] pwm: make it possible to apply pwm changes in
 atomic context
Message-ID: <ZSlbFukZKGNpR5PM@gofer.mess.org>
References: <cover.1697193646.git.sean@mess.org>
 <9c0f1616fca5b218336b9321bfefe7abb7e1749f.1697193646.git.sean@mess.org>
 <ZSkvTKr42sUZImiM@orome.fritz.box>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZSkvTKr42sUZImiM@orome.fritz.box>
Precedence: bulk

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