Re: [PATCH 4/9] drivers/pwm: Add helper to configure pwm using clock divisor and duty percent

From: Thierry Reding <thierry.reding@gmail.com>
To: Shobhit Kumar <shobhit.kumar@linux.intel.com>
Cc: Alexandre Courbot <gnurou@gmail.com>,
	Samuel Ortiz <sameo@linux.intel.com>,
	Jani Nikula <jani.nikula@intel.com>,
	Shobhit Kumar <shobhit.kumar@intel.com>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [PATCH 4/9] drivers/pwm: Add helper to configure pwm using clock divisor and duty percent
Date: Fri, 10 Apr 2015 10:29:55 +0200	[thread overview]
Message-ID: <20150410082954.GA4139@ulmo.nvidia.com> (raw)
In-Reply-To: <551B9022.107@linux.intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 2390 bytes --]

On Wed, Apr 01, 2015 at 11:58:50AM +0530, Shobhit Kumar wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 03/24/2015 01:53 PM, Thierry Reding wrote:
> > On Fri, Mar 13, 2015 at 07:28:02PM +0530, Shobhit Kumar wrote:
> >> Some chips instead of using period_ns and duty_ns can be
> >> configured using the clock divisor and duty percent. Adds an
> >> alternative configuration method for such chips
> > 
> > I don't see a need to introduce this alternative configuration 
> > mechanism. Most, of not all, of the other drivers program a clock 
> > divisor and some percentage of the duty cycle as well and it should
> > be easy to convert to that internally from the period and
> > duty_cycle parameters that you get in ->config().
> 
> Perhaps. Probably I misunderstood but as per Documentation/pwm.txt, it
> is suggested that rather than calculating in the driver, we can add
> additional helpers. So I tried doing just that. And it also means that
> the consumer(which is directly aware of the percent it wants) has to
> do the calculation and pass as ns values and we internally again
> convert back to percentage ?

Yes. The interface assumes that you'll pass in absolute values for the
period and duty cycle. Existing drivers, such as pwm-backlight, already
convert a percentage or other internal representation to these absolute
values. If your driver internally works with percent you can easily
convert to that from the absolute values.

The documentation only makes a suggestion. I think it'd be fine if you
kept this conversion internal to the driver. We can turn it into a more
generic helper if a second driver appears that needs the same
conversion.

> > Adding an alternative means of configuring the PWM also means that
> > every user driver now potentially needs to support both the
> > traditional and the alternative way because PWM providers may not
> > implement both.
> 
> I just assumed either or implementation should suffice. Even in my
> implementation the error checks assumes either of the two should be
> available else to fail the pwmchip_add

Your implementation requires that users call either pwm_config() or
pwm_config_alternate(). PWM drivers may only have to implement either
callback, but users will be required to support both (or otherwise
only work with a subset of PWM drivers).

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx