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

[Thierry Reding]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20150410/3122cd79/attachment.sig>