Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1376748

Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept

From Thierry Reding <thierry.reding@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept
Date 2016-04-12 14:30 +0200
Message-ID <rn5Ca-7oG-17@gated-at.bofh.it> (permalink)
References <riuBc-28V-5@gated-at.bofh.it> <riuUy-2hW-11@gated-at.bofh.it> <rn4PL-6Jq-9@gated-at.bofh.it> <rn5iO-7dg-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

On Tue, Apr 12, 2016 at 02:04:12PM +0200, Boris Brezillon wrote:
> On Tue, 12 Apr 2016 13:39:12 +0200
> Thierry Reding <thierry.reding@gmail.com> wrote:
> 
> > On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> > > Currently the PWM core mixes the current PWM state with the per-platform
> > > reference config (specified through the PWM lookup table, DT definition or
> > > directly hardcoded in PWM drivers).
> > > 
> > > Create a pwm_args struct to store this reference config, so that PWM users
> > > can differentiate the current config from the reference one.
> > > 
> > > Patch all places where pwm->args should be initialized. We keep the
> > > pwm_set_polarity/period() calls until all PWM users are patched to
> > > use pwm_args instead of pwm_get_period/polarity().
> > 
> > Perhaps a helper would be useful? Something like:
> > 
> > 	static inline void
> > 	pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
> > 	{
> > 		pwm_set_duty_cycle(pwm, args->duty_cycle);
> > 		pwm_set_period(pwm, args->period);
> > 	}
> > 
> > ? That would make it slightly easier to get rid of it again after all
> > clients have been converted.
> 
> Sure. I'll add this helper.
> 
> > 
> > With the exception of pwm-clps711x all of these args are set at of_xlate
> > time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
> > might even be possible to move this call to the core, so that removal of
> > it will be a one-liner.
> 
> Not sure I get that one. Some drivers are implementing their own
> ->of_xlate() method, how would you get rid of this pwm_apply_args() in
> those custom implementations?

I was proposing to have pwm_apply_args() called from the core.
of_pwm_get() is where ->of_xlate() is called from, and the lookup table
arguments would be applied in pwm_get(). Taking into account clps711x,
which sets the arguments in ->request() it might be possible to simply
call pwm_apply_args() from pwm_device_request(), since that's also
called by all other request functions, even the legacy ones.

That said, the amount of code to modify isn't that large, so I'm fine if
you want to keep sprinkling the calls across multiple files, especially
since it's temporary.

Thierry

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept Thierry Reding <thierry.reding@gmail.com> - 2016-04-12 13:40 +0200
  Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-12 14:10 +0200
    Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept Thierry Reding <thierry.reding@gmail.com> - 2016-04-12 14:30 +0200
      Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-12 15:00 +0200
  Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-12 15:10 +0200
    Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept Thierry Reding <thierry.reding@gmail.com> - 2016-04-12 15:20 +0200

csiph-web