Hi Ulf,

On 2017-03-22 13:00, Ulf Hansson wrote:

On 22 March 2017 at 12:35, Marek Szyprowski [off-list ref] wrote:

quoted

This patch adds support for runtime PM to Exynos Audio SubSystem driver to
enable full support for audio power domain on Exynos5 SoCs. The main change
is moving register saving and restoring code from system sleep PM ops to
runtime PM ops and implementing system sleep PM ops with generic
pm_runtime_force_suspend/resume helpers. Runtime PM of the Exynos AudSS
device is managed from clock core depending on the preparation status
of the provided clocks.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Reviewed-by: Ulf Hansson <redacted>

Minor nitpick below.

[...]

quoted

-#ifdef CONFIG_PM_SLEEP
  static unsigned long reg_save[][2] = {
         { ASS_CLK_SRC,  0 },
         { ASS_CLK_DIV,  0 },
         { ASS_CLK_GATE, 0 },
  };

-static int exynos_audss_clk_suspend(struct device *dev)
+static int __maybe_unused exynos_audss_clk_suspend(struct device *dev)

Instead of using "__maybe_unused", I would rather change from
"CONFIG_PM_SLEEP" to "CONFIG_PM". However, no strong opinion.

Perhaps you need something like this for patch 3/4 as well?

Frankly, this matters only for compile-time tests. All Exynos platforms
require CONFIG_PM, so that code will be always used in runtime.

For compile-time tests probably it is better to use __maybe_unused approach
to have higher coverage.

quoted

  {
         int i;

@@ -54,7 +54,7 @@ static int exynos_audss_clk_suspend(struct device *dev)
         return 0;
  }

[...]

Kind regards
Uffe

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland