From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936636AbcHJSqr (ORCPT ); Wed, 10 Aug 2016 14:46:47 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:12481 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S938722AbcHJSqd (ORCPT ); Wed, 10 Aug 2016 14:46:33 -0400 X-AuditID: cbfec7f5-f792a6d000001302-39-57aae77d36ed Subject: Re: [PATCH v4 2/4] mfd: Add Samsung Exynos Low Power Audio Subsystem driver To: Lee Jones References: <1467738804-31460-1-git-send-email-s.nawrocki@samsung.com> <20160809150502.GQ5243@dell> <61d700f7-ef66-8278-54ea-8e6e2f1a029e@samsung.com> <20160809205031.GD1581@dell> Cc: broonie@kernel.org, robh@kernel.org, alsa-devel@alsa-project.org, devicetree@vger.kernel.org, ideal.song@samsung.com, inki.dae@samsung.com, b.zolnierkie@samsung.com, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, Beomho Seo From: Sylwester Nawrocki Message-id: Date: Wed, 10 Aug 2016 10:36:00 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-version: 1.0 In-reply-to: <20160809205031.GD1581@dell> Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrPLMWRmVeSWpSXmKPExsVy+t/xK7q1z1eFGxyYLmNx5eIhJouNM9az Wpz+tI3dYurDJ2wW84+cY7XY9fc+o8Wk+xNYLO5/PcpocXnXHDaLGef3MVn837OD3YHbY8Pn JjaPTas62TzuXNvD5tG3ZRWjx+dNcgGsUVw2Kak5mWWpRfp2CVwZiz4uYC3Yx1NxpLeFqYFx MlcXIyeHhICJROu6U4wQtpjEhXvr2boYuTiEBJYySuxd9oodwnnOKNHUfJoJpEpYIExi0fE1 QDYHh4iAisS5N+YQNdsZJRZM+cgE4jALTGGSOHlnFxtIA5uAoUTv0T6wFbwCdhKbFrxmAbFZ BFQlXh44DzZUVCBC4taqj1A1ghI/Jt8Dq+EU0JR48HgFK8gyZgF1iSlTckHCzALyEpvXvGWe wCgwC0nHLISqWUiqFjAyr2IUTS1NLihOSs810itOzC0uzUvXS87P3cQIiYavOxiXHrM6xCjA wajEw7uBaVW4EGtiWXFl7iFGCQ5mJRHe40+BQrwpiZVVqUX58UWlOanFhxilOViUxHln7nof IiSQnliSmp2aWpBaBJNl4uCUamBsXGaVcDNMXOE5h+XUa319O//O763w9XYwXBwWpSPL+SRQ QL22PjUz0tN2gZOIV9v0muakCp9T1iWBN1b1LRVUivHZLLon5dPU0LcyLvsW7pwWdjxtw+62 PTFPUo174qtfCHyX+3zanX+KSaiqWVL1lc79SRFv+53SeYNfbbFeNbejuyDvvBJLcUaioRZz UXEiANW/o4aCAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/09/2016 10:50 PM, Lee Jones wrote: > On Tue, 09 Aug 2016, Sylwester Nawrocki wrote: > >> > On 08/09/2016 05:05 PM, Lee Jones wrote: >>>> > >> +static SIMPLE_DEV_PM_OPS(lpass_pm_ops, exynos_lpass_suspend, >>>>> > >> > + exynos_lpass_resume); >>> > > Put this up by the PM functions. >> > >> > Sorry, I don't understand this comment, which PM functions do you >> > mean exactly? > *_suspend and *_resume. PM == Power Management. > > You also need to place the functions in side some CONFIG_PM_SLEEP > guards. Grep around, you'll see what I mean. OK, but couldn't we just leave it with that SIMPLE_DEV_PM_OPS() macro? It saves us an #ifdef ugliness and if I actually grep drivers/mfd most of the drivers use either SIMPLE_DEV_PM_OPS() or SET_SYSTEM_SLEEP_PM_OPS. Are you asking me to change the above to: static const struct dev_pm_ops exynos_lpass_pm_ops = { #ifdef CONFIG_PM_SLEEP .suspend = exynos_lpass_suspend, .resume = exynos_lpass_resume, .freeze = exynos_lpass_suspend, .thaw = exynos_lpass_resume, .poweroff = exynos_lpass_suspend, .restore = exynos_lpass_resume, #endif } or perhaps static const struct dev_pm_ops exynos_lpass_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(exynos_lpass_suspend, exynos_lpass_resume) } ? I agree about the CONFIG_PM_SLEEP guards. There is very little chance though the LPASS driver will ever be used with CONFIG_PM_SLEEP disabled. How about adding __maybe_unused attribute to the *_suspend/*_resume functions then? I can see this pattern used already in drivers/mfd. -- Thanks, Sylwester