[lkml]   [2020]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 1/3] pwm: pca9685: Switch to atomic API
On Sat, Nov 21, 2020 at 9:58 AM Clemens Gruber
<> wrote:
> I'd rather continue supporting this driver with !CONFIG_PM. (In our
> company we have a product with a !CONFIG_PM build using this driver)

Absolutely, makes sense. If you do add support for !CONFIG_PM, then it's
important that both PM and !PM cases get tested by you.

> I am thinking about the following solution:
> #ifdef CONFIG_PM
> /* Set runtime PM status according to chip sleep state */
> if (reg & MODE1_SLEEP)
> pm_runtime_set_suspended(..);
> else
> pm_runtime_set_active(..);
> pm_runtime_enable(..);
> #else
> /* If in SLEEP state on non-PM environments, wake the chip up */
> if (reg & MODE1_SLEEP)
> pca9685_set_sleep_mode(.., false)
> #endif

I don't think we need the #ifdef CONFIG_PM, because all pm_runtime_xxx
functions become no-ops when !CONFIG_PM.

Also, I believe "if (IS_ENABLED(CONFIG_XXX))" is preferred, because
it allows the compiler to syntax-check disabled code.

How about the following? It should be correct, short, and easy to understand.
Yes, there's one single unnecessary register write (+ 500us delay if !PM) when
the chip is already active on probe(). But maybe that's worth it if it makes
the code easier to understand?


pca9685_set_sleep_mode(pca, false);

return 0;


pca9685_set_sleep_mode(pca, true);

return 0;

> About the regmap cache: I looked into it and think it is a good idea but
> it's probably best to get these patches merged first and then rework the
> driver to using the regmap cache?

Good suggestion, I agree.

 \ /
  Last update: 2020-11-21 23:01    [W:0.049 / U:1.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site