From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding 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 Message-ID: <20150410082954.GA4139@ulmo.nvidia.com> References: <1426177893-17945-5-git-send-email-shobhit.kumar@intel.com> <1426255082-25002-1-git-send-email-shobhit.kumar@intel.com> <20150324082357.GA17183@ulmo.nvidia.com> <551B9022.107@linux.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1972029787==" Return-path: Received: from mail-pa0-f42.google.com (mail-pa0-f42.google.com [209.85.220.42]) by gabe.freedesktop.org (Postfix) with ESMTP id 435496E8CA for ; Fri, 10 Apr 2015 01:30:00 -0700 (PDT) Received: by paboj16 with SMTP id oj16so15097999pab.0 for ; Fri, 10 Apr 2015 01:30:00 -0700 (PDT) In-Reply-To: <551B9022.107@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Shobhit Kumar Cc: Alexandre Courbot , Samuel Ortiz , Jani Nikula , Shobhit Kumar , intel-gfx , Daniel Vetter , Linus Walleij List-Id: intel-gfx@lists.freedesktop.org --===============1972029787== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="7AUc2qLy4jB3hD7Z" Content-Disposition: inline --7AUc2qLy4jB3hD7Z Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 01, 2015 at 11:58:50AM +0530, Shobhit Kumar wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 >=20 > 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 > >=20 > > I don't see a need to introduce this alternative configuration=20 > > mechanism. Most, of not all, of the other drivers program a clock=20 > > 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(). >=20 > 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. >=20 > 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 --7AUc2qLy4jB3hD7Z Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJVJ4n/AAoJEN0jrNd/PrOhjYEP+QGbvbt788SvHzYX/zksFGhL +7AqMRoqKmr1rF6q9Xxr97YlWDmBLiZeJKFfMZrnKGFtsPWjWvpZkmPjZNd2yzck wtV5OXL7H6wQU5NGJJN2bA8BP+t8q9dVclQjbcw4MaYZnqaPN/7zLhWfSWCPc8hl GiPZt1iLYqAouKJnQqt2vOUWIcCwzDWMe0FvwFSD6z5miNEl+HFBsduXaJVcaGV4 E4iS6EifZ7GHAvkbmNqtSvWLWu5wS5spZcOv+KV+ISqtXY52uuFX7GvvTs2h3a4r 7Mu0u/kK/FBeWAHGBjS4kimnVycoOjbMcxTQH2uyxFFt3/TxAbCOuwZL3lSZuNq2 Y7xIauyUrqMsDJmZJSOwonZ5AMZ3qrf/zHRR1Ezp5H61CD9L0dLSufvtXK7RS+vq Z/KoAnxIaammdX6zwvp4A2GFHMJypV+Hty8JWwjjEE6RuFCb3tzSr6VOb8vo78vW ZdF4NlhAVI0IpEuxBieYTAxUm/1P9gMZumi1qAHhn/D3+oagC0wohzCBzL2jIZcP 6h7wAkxFrWSqTuTIZB99wz9S5Gb1br1DttvaQ7q+ESz04AicX2aXuP3aJLOJ4UtO wEu6Bkc4nwawWY7O+UZvrYZZ6iLWqVfxKmhY7tH6llS+UswKAnPYVqNVt5uLAt0I 1mOhkvuFMBdcTi65pQx5 =IqQK -----END PGP SIGNATURE----- --7AUc2qLy4jB3hD7Z-- --===============1972029787== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============1972029787==--