lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 21 Nov 2020 17:00:10 -0500 From: Sven Van Asbroeck <thesven73@...il.com> To: Clemens Gruber <clemens.gruber@...ruber.com> Cc: linux-pwm@...r.kernel.org, Thierry Reding <thierry.reding@...il.com>, Uwe Kleine-König <u.kleine-koenig@...gutronix.de>, Lee Jones <lee.jones@...aro.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Mika Westerberg <mika.westerberg@...ux.intel.com>, David Jander <david@...tonic.nl> Subject: Re: [PATCH 1/3] pwm: pca9685: Switch to atomic API On Sat, Nov 21, 2020 at 9:58 AM Clemens Gruber <clemens.gruber@...ruber.com> 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? probe() { ... pm_runtime_set_active(&client->dev); pm_runtime_enable(&client->dev); if (!IS_ENABLED(CONFIG_PM)) pca9685_set_sleep_mode(pca, false); return 0; } remove() { ... pm_runtime_disable(&client->dev); if (!IS_ENABLED(CONFIG_PM)) 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.
Powered by blists - more mailing lists