Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933427AbdDFRCI (ORCPT ); Thu, 6 Apr 2017 13:02:08 -0400 Received: from hqemgate16.nvidia.com ([216.228.121.65]:14602 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752417AbdDFRCA (ORCPT ); Thu, 6 Apr 2017 13:02:00 -0400 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Thu, 06 Apr 2017 10:01:58 -0700 Message-ID: <58E66F8F.1030802@nvidia.com> Date: Thu, 6 Apr 2017 22:10:47 +0530 From: Laxman Dewangan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: Jon Hunter , , CC: , , , , Subject: Re: [PATCH V4 4/4] pwm: tegra: Add support to configure pin state in suspends/resume References: <1491488461-24621-1-git-send-email-ldewangan@nvidia.com> <1491488461-24621-5-git-send-email-ldewangan@nvidia.com> In-Reply-To: X-Originating-IP: [10.19.65.30] X-ClientProxiedBy: DRHKMAIL103.nvidia.com (10.25.59.17) To bgmail102.nvidia.com (10.25.59.11) Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2320 Lines: 69 Oops, it was actually v2. On Thursday 06 April 2017 08:47 PM, Jon Hunter wrote: > On 06/04/17 15:21, Laxman Dewangan wrote: >> In some of NVIDIA Tegra's platform, PWM controller is used to >> control the PWM controlled regulators. PWM signal is connected to >> the VID pin of the regulator where duty cycle of PWM signal decide >> the voltage level of the regulator output. >> >> The tristate (high impedance of PWM pin form Tegra) also define >> one of the state of PWM regulator which needs to be configure in >> suspend state of system. >> >> Add support to configure the pin state via pinctrl frameworks in >> suspend and active state of the system. >> >> Signed-off-by: Laxman Dewangan >> --- >> Changes from v1: >> - Use standard pinctrl names for sleep and active state. >> - Use API pinctrl_pm_select_*() >> >> drivers/pwm/pwm-tegra.c | 22 ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> >> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c >> index e9c4de5..af1bd4f 100644 >> --- a/drivers/pwm/pwm-tegra.c >> +++ b/drivers/pwm/pwm-tegra.c >> @@ -29,6 +29,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> >> @@ -256,6 +257,22 @@ static int tegra_pwm_remove(struct platform_device *pdev) >> return pwmchip_remove(&pc->chip); >> } >> >> +#ifdef CONFIG_PM_SLEEP >> +static int tegra_pwm_suspend(struct device *dev) >> +{ >> + pinctrl_pm_select_sleep_state(dev); > Why not return the error code here? As the pin state in suspend is optional, I dont want to return error if the sleep state is not available. However, it seems pinctrl take care of retuning success if there is no sleep state. By seeing code. Let me test this on different condition and it it works fine then we can return the return of pinctrl_pm_select_*() BTW, it should be OK to have pwm_tegra_resume/suspend wrapper, not directly use the pinctrl_pm_select_* in pm ops suspend/resume. The prototype matches. > > By the way, do you plan to include patches to populate the bindings for > the pwm devices? I am planning to populate the GPU regulator which is PWM based. This will only populate the regulator.