From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751933AbbJWGdv (ORCPT ); Fri, 23 Oct 2015 02:33:51 -0400 Received: from m50-138.163.com ([123.125.50.138]:34427 "EHLO m50-138.163.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750781AbbJWGdu (ORCPT ); Fri, 23 Oct 2015 02:33:50 -0400 Subject: Re: [PATCH v3 2/3] thermal: rockchip: ensure the otp states before resetting the controller To: Doug Anderson References: <1445565296-31517-1-git-send-email-wxt@rock-chips.com> <1445565296-31517-3-git-send-email-wxt@rock-chips.com> Cc: Caesar Wang , Heiko Stuebner , "linux-pm@vger.kernel.org" , Dmitry Torokhov , "linux-kernel@vger.kernel.org" , Eduardo Valentin , "open list:ARM/Rockchip SoC..." , Zhang Rui , "linux-arm-kernel@lists.infradead.org" From: Caesar Wang Message-ID: <5629D45A.1090407@163.com> Date: Fri, 23 Oct 2015 14:31:54 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CM-TRANSID: C9GowABnaRJa1ClWAqUVAA--.888S2 X-Coremail-Antispam: 1Uf129KBjvJXoWxZr1fKF4xAF15tFWDJw1xGrg_yoWrXF18pr s0gF1Ykr4DJF4jq3W2ywsFyrZYya17t3yUWryrG3s8Zr9xAFyfKFyxKryYv3sYvFn3WF1Y vr1UK398urWDZrJanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07bUrWrUUUUU= X-Originating-IP: [58.22.7.114] X-CM-SenderInfo: 5vdv3yhhz03qqrwthudrp/1tbiRQqclVWBMzkj9wABsN Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 在 2015年10月23日 12:04, Doug Anderson 写道: > Caesar, > > On Thu, Oct 22, 2015 at 9:54 PM, Caesar Wang wrote: >> We need the OTP pin is gpio state before resetting the TSADC controller, >> since the tshut polarity will generate a high signal. >> >> Says: >> The TSHUT temperature is setting more than 80 degree, the >> default tshut polarity is high. >> >> If T > 80C, the OTP output the high signal. >> If T < 80C, the OTP output the low signal. >> >> On the moment, the tshut polarity will be low in a short period of time >> if the TSADC controller is reset. >> >> So: >> If T < 80C, the OTP output the High Signal. >> If T > 80C, the OTP output the Low Signal. >> >> In some cases, the OTP pin is connected to the PMIC, maybe the PMIC can >> accept the reset response time to avoid this issue. >> In other words, the system will be always reboot if we >> make the OTP pin is connected the others IC to control the power. >> >> Signed-off-by: Caesar Wang >> >> --- >> >> Changes in v3: >> - Add the pinctrl state for in the suspend/resume. >> >> Changes in v2: None >> Changes in v1: None >> >> drivers/thermal/rockchip_thermal.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c >> index c89ffb2..3b8fbda 100644 >> --- a/drivers/thermal/rockchip_thermal.c >> +++ b/drivers/thermal/rockchip_thermal.c >> @@ -642,6 +642,8 @@ static int __maybe_unused rockchip_thermal_suspend(struct device *dev) >> clk_disable(thermal->pclk); >> clk_disable(thermal->clk); >> >> + pinctrl_pm_select_sleep_state(dev); >> + >> return 0; >> } >> >> @@ -678,6 +680,8 @@ static int __maybe_unused rockchip_thermal_resume(struct device *dev) >> for (i = 0; i < ARRAY_SIZE(thermal->sensors); i++) >> rockchip_thermal_toggle_sensor(&thermal->sensors[i], true); >> >> + pinctrl_pm_select_default_state(dev); >> + >> return 0; >> } > The patch looks totally fine, but the description is a little > confusing. Reading this patch it's all about adding support for the > "sleep" state in the tsadc driver, but nothing in the description > talks about that. I'd expect something like: > > thermal: rockchip: support the sleep pinctrl state to avoid glitches in s2r > > When we come out of system suspend state (S3) the tsadc will have been > reset and back at its default state. While reprogramming the tsadc > it's possible that we'll glitch the output and unintentionally cause > the "over temperature" GPIO to be asserted. Since the over > temperature GPIO is often hooked up to something that will cause a > reboot or shutdown in hardware, this glitch can be catastrophic on > some boards. > > We'll add support for selecting the "sleep" pinctrl state at suspend > time. Boards can use this to effectively disable the tsadc at suspend > time and avoid glitches when the system is resumed. Thanks Doug to take your time reviewing this series patchs. The commit is very good for this patch. > > --- > > Note that although this pinctrl approach is fine IMHO, I am left > wondering whether we could just change the tsadc init sequence to > avoid the glitch. I can't easily test myself, but if we can program > the temperatures before re-enabling the tsadc would it avoid the > problem too? It's the chip behaviour, the glitches is aways occured by reset controller. The best way need change to the gpio state before reset the controller. > Like could we just swap things like: > > thermal->chip->set_tshut_temp(id, thermal->regs, > thermal->hw_shut_temp); > thermal->chip->set_tshut_mode(id, thermal->regs, > thermal->tshut_mode); > > > Does that help? It didn't work on box board. > > > -Doug > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip