Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] PM / core: refactor PM_OPS initializers
To:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        Arnd Bergmann <arnd@arndb.de>
From:   Andrzej Hajda <a.hajda@samsung.com>
Message-ID: <9e1f4be2-436e-0d60-eb28-44c229a3178c@samsung.com>
Date:   Mon, 14 May 2018 14:07:54 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
        Thunderbird/52.7.0
MIME-Version: 1.0
In-Reply-To: <20180514082146.4318-1-a.hajda@samsung.com>
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <CGME20180514082201eucas1p1549ae992aeeef0906a2cccb5da96533f@eucas1p1.samsung.com>
        <20180514082146.4318-1-a.hajda@samsung.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 14.05.2018 10:21, Andrzej Hajda wrote:
> With current implementation of PM_OPS initializers users should annotate
> all PM callbacks with __maybe_unused attribute to prevent compiler from
> complaining in case respective option is not enabled. Using ternary
> operator with IS_ENABLED(symbol) as a condition allows to avoid marking
> these functions with __maybe_unused.
> Solution was tested successfully with multiple versions of gcc since
> 4.9.4 up to 7.2.1. No functional changes has been observed and callbacks
> were compiled out if not used, as before.

The kernel does not compile with the patch applied if the driver defines
PM callback inside "#ifdef CONFIG_PM(_SLEEP)" - initializers assume that
callbacks are defined, even if they are not used.
So if the idea is OK I should figure it out how to make proper
transition, any ideas welcome :)

Regards
Andrzej

>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  include/linux/pm.h | 61 ++++++++++++++++++----------------------------
>  1 file changed, 24 insertions(+), 37 deletions(-)
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index e723b78d8357..59f333922c15 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -313,50 +313,37 @@ struct dev_pm_ops {
>  	int (*runtime_idle)(struct device *dev);
>  };
>  
> -#ifdef CONFIG_PM_SLEEP
> +#define PM_SLEEP_PTR(ptr) (IS_ENABLED(CONFIG_PM_SLEEP) ? (ptr) : NULL)
> +#define PM_PTR(ptr) (IS_ENABLED(CONFIG_PM) ? (ptr) : NULL)
> +
>  #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -	.suspend = suspend_fn, \
> -	.resume = resume_fn, \
> -	.freeze = suspend_fn, \
> -	.thaw = resume_fn, \
> -	.poweroff = suspend_fn, \
> -	.restore = resume_fn,
> -#else
> -#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> -#endif
> +	.suspend = PM_SLEEP_PTR(suspend_fn), \
> +	.resume = PM_SLEEP_PTR(resume_fn), \
> +	.freeze = PM_SLEEP_PTR(suspend_fn), \
> +	.thaw = PM_SLEEP_PTR(resume_fn), \
> +	.poweroff = PM_SLEEP_PTR(suspend_fn), \
> +	.restore = PM_SLEEP_PTR(resume_fn),
>  
> -#ifdef CONFIG_PM_SLEEP
>  #define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -	.suspend_late = suspend_fn, \
> -	.resume_early = resume_fn, \
> -	.freeze_late = suspend_fn, \
> -	.thaw_early = resume_fn, \
> -	.poweroff_late = suspend_fn, \
> -	.restore_early = resume_fn,
> -#else
> -#define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> -#endif
> +	.suspend_late = PM_SLEEP_PTR(suspend_fn), \
> +	.resume_early = PM_SLEEP_PTR(resume_fn), \
> +	.freeze_late = PM_SLEEP_PTR(suspend_fn), \
> +	.thaw_early = PM_SLEEP_PTR(resume_fn), \
> +	.poweroff_late = PM_SLEEP_PTR(suspend_fn), \
> +	.restore_early = PM_SLEEP_PTR(resume_fn),
>  
> -#ifdef CONFIG_PM_SLEEP
>  #define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -	.suspend_noirq = suspend_fn, \
> -	.resume_noirq = resume_fn, \
> -	.freeze_noirq = suspend_fn, \
> -	.thaw_noirq = resume_fn, \
> -	.poweroff_noirq = suspend_fn, \
> -	.restore_noirq = resume_fn,
> -#else
> -#define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> -#endif
> +	.suspend_noirq = PM_SLEEP_PTR(suspend_fn), \
> +	.resume_noirq = PM_SLEEP_PTR(resume_fn), \
> +	.freeze_noirq = PM_SLEEP_PTR(suspend_fn), \
> +	.thaw_noirq = PM_SLEEP_PTR(resume_fn), \
> +	.poweroff_noirq = PM_SLEEP_PTR(suspend_fn), \
> +	.restore_noirq = PM_SLEEP_PTR(resume_fn),
>  
> -#ifdef CONFIG_PM
>  #define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
> -	.runtime_suspend = suspend_fn, \
> -	.runtime_resume = resume_fn, \
> -	.runtime_idle = idle_fn,
> -#else
> -#define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
> -#endif
> +	.runtime_suspend = PM_PTR(suspend_fn), \
> +	.runtime_resume = PM_PTR(resume_fn), \
> +	.runtime_idle = PM_PTR(idle_fn),
>  
>  /*
>   * Use this if you want to use the same suspend and resume callbacks for suspend