From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757292AbcAOJu3 (ORCPT ); Fri, 15 Jan 2016 04:50:29 -0500 Received: from hqemgate15.nvidia.com ([216.228.121.64]:1223 "EHLO hqemgate15.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757260AbcAOJu0 convert rfc822-to-8bit (ORCPT ); Fri, 15 Jan 2016 04:50:26 -0500 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Fri, 15 Jan 2016 01:51:46 -0800 Message-ID: <5698C106.4080805@nvidia.com> Date: Fri, 15 Jan 2016 17:51:02 +0800 From: Wei Ni User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Thierry Reding CC: , , , , Subject: Re: [PATCH V1 08/10] thermal: tegra: add PM support References: <1452672043-475-1-git-send-email-wni@nvidia.com> <20160113155540.GH2588@ulmo> In-Reply-To: <20160113155540.GH2588@ulmo> X-Originating-IP: [10.19.224.146] X-ClientProxiedBy: HKMAIL104.nvidia.com (10.18.16.13) To HKMAIL101.nvidia.com (10.18.16.10) Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016年01月13日 23:55, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Wed, Jan 13, 2016 at 04:00:43PM +0800, Wei Ni wrote: >> Add suspend/resume function in soctherm driver. >> And enable it for Tegra124 and Tegra210. >> >> Signed-off-by: Wei Ni >> --- >> drivers/thermal/tegra/tegra124_soctherm.c | 9 ++ >> drivers/thermal/tegra/tegra210_soctherm.c | 9 ++ >> drivers/thermal/tegra/tegra_soctherm.c | 154 +++++++++++++++++++++--------- >> drivers/thermal/tegra/tegra_soctherm.h | 5 + >> 4 files changed, 130 insertions(+), 47 deletions(-) > > This all looks like it'd be a better candidate for runtime PM. The > implementation would be mostly the same, except that you'd need to > add a few more things to tegra_soctherm_probe() and use the runtime > PM API to enable/disable the device (clocks, resets, ...). System > sleep should then be taken care of automatically. We can not handle it as runtime PM, because this is used to monitor the temperatures, once they reach to critical value, it need to trigger operations to protect system, such as shutdown or throttle. If this device suspend in runtime, then it can't handle these events. > > Another comment irrespective of that... > >> diff --git a/drivers/thermal/tegra/tegra124_soctherm.c b/drivers/thermal/tegra/tegra124_soctherm.c >> index 13d7f4e3a3d7..04cb419dbabc 100644 >> --- a/drivers/thermal/tegra/tegra124_soctherm.c >> +++ b/drivers/thermal/tegra/tegra124_soctherm.c >> @@ -205,6 +205,14 @@ static const struct of_device_id tegra124_soctherm_of_match[] = { >> }; >> MODULE_DEVICE_TABLE(of, tegra_soctherm_of_match); >> >> +#ifdef CONFIG_PM_SLEEP >> +static SIMPLE_DEV_PM_OPS(tegra124_soctherm_pm, >> + soctherm_suspend, soctherm_resume); >> +#define TEGRA124_SOC_THERM_PM (&tegra124_soctherm_pm) >> +#else >> +#define TEGRA124_SOC_THERM_PM NULL >> +#endif > > I wouldn't bother with the #ifdef, just always define the structure. Ok, will remove them. > > Thierry > > * Unknown Key > * 0x7F3EB3A1 >