From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4558DC001B0 for ; Thu, 29 Jun 2023 21:11:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230046AbjF2VK7 (ORCPT ); Thu, 29 Jun 2023 17:10:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51596 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231497AbjF2VKp (ORCPT ); Thu, 29 Jun 2023 17:10:45 -0400 Received: from madras.collabora.co.uk (madras.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e5ab]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F00202D4C; Thu, 29 Jun 2023 14:10:43 -0700 (PDT) Received: from notapiano (zone.collabora.co.uk [167.235.23.81]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: nfraprado) by madras.collabora.co.uk (Postfix) with ESMTPSA id CBDD066070D1; Thu, 29 Jun 2023 22:10:38 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1688073041; bh=JCh8QNgbheBN9vzum2hXOH+Xccw6yemZ2xjeKair+aY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=awtycltug4rEIpIWPHSmCJ0Ar7z77OoWwDGCry2DH+yxVYb/zvGXFn4MY6+0VYWHz KsjdF6RlG839AVnwTUXG5EhlfY57qurBilxeD4c48tgwS66/uibtEDaER8NzfwVG5S JHXvf7vP2vcIQKryhP7Nc2aPRt+dbb+oXgMTCIsXDUC66CsEfWU9LrVu+nJyGWIw83 BSZf9ksQr+YcWLUt9cu1rx4wlaf479EvT0yP1Jab0CD+ftjnz/ZU+t4SXMd1MUetTU H2jTI5x4AB+Dy+grt0C/nvXBZA132sNpsVnx645CtNY3W9M2+wrzL+iw7ra+tbJehg JGugr9zpCYcOQ== Date: Thu, 29 Jun 2023 17:10:34 -0400 From: =?utf-8?B?TsOtY29sYXMgRi4gUi4gQS4=?= Prado To: Daniel Lezcano Cc: kernel@collabora.com, Alexandre Mergnat , Balsam CHIHI , Chen-Yu Tsai , Alexandre Bailon , AngeloGioacchino Del Regno , Amit Kucheria , Matthias Brugger , "Rafael J. Wysocki" , Zhang Rui , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-pm@vger.kernel.org Subject: Re: [PATCH v2 5/6] thermal/drivers/mediatek/lvts_thermal: Don't leave threshold zeroed Message-ID: <819b9c10-044c-4a2b-b8f2-6ddd633bbf8d@notapiano> References: <20230504004852.627049-1-nfraprado@collabora.com> <20230504004852.627049-6-nfraprado@collabora.com> <5d810151-3a3b-51af-bbc1-0ee45668bcc4@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <5d810151-3a3b-51af-bbc1-0ee45668bcc4@linaro.org> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jun 02, 2023 at 11:17:27AM +0200, Daniel Lezcano wrote: > On 04/05/2023 02:48, Nícolas F. R. A. Prado wrote: > > The thermal framework might leave the low threshold unset if there > > aren't any lower trip points. This leaves the register zeroed, which > > translates to a very high temperature for the low threshold. The > > interrupt for this threshold is then immediately triggered, and the > > state machine gets stuck, preventing any other temperature monitoring > > interrupts to ever trigger. > > > > (The same happens by not setting the Cold or Hot to Normal thresholds > > when using those) > > > > Set the unused threshold to a valid low value. This value was chosen so > > that for any valid golden temperature read from the efuse, when the > > value is converted to raw and back again to milliCelsius, the result > > doesn't underflow. > > > > Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver") > > Signed-off-by: Nícolas F. R. A. Prado > > > > --- > > > > Changes in v2: > > - Added this commit > > > > drivers/thermal/mediatek/lvts_thermal.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c > > index efd1e938e1c2..951a4cb75ef6 100644 > > --- a/drivers/thermal/mediatek/lvts_thermal.c > > +++ b/drivers/thermal/mediatek/lvts_thermal.c > > @@ -82,6 +82,8 @@ > > #define LVTS_HW_SHUTDOWN_MT8195 105000 > > #define LVTS_HW_SHUTDOWN_MT8192 105000 > > +#define LVTS_MINIUM_THRESHOLD 20000 > > MINIMUM > > So if the thermal zone reaches 20°C, the interrupt fires, the set_trips sets > again 20°C but the interrupt won't fire until the temperature goes above > 20°C and then crosses the temperature low threshold the way down again? Well, actually, set_trips won't even be called since from the thermal framework's perspective we haven't crossed trip points, ie in __thermal_zone_set_trips(): /* No need to change trip points */ if (tz->prev_low_trip == low && tz->prev_high_trip == high) return; But in any case, yes, the interrupt will fire, the temperature will get updated in the framework, and that's it. It will only fire again when a threshold is crossed again (either by the temperature rising and falling again below this minimum, or rising beyond the high treshold). So basically at most this will cause a spurious interrupt when the temperature gets low enough. I do get 34-36C on all sensors when idling though, so I doubt that temperature is even reachable. Besides, we don't really have another option here if we want working interrupts, the threshold needs to be set to a valid value, and this is the lowest I've found. And thanks for all the feedback! I'll prepare a v3 based on your comments. Thanks, Nícolas From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9D8B0EB64DD for ; Thu, 29 Jun 2023 21:11:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=c1btAG/Z5LcL5Qn/caSIRsYTYtyGLG8ww58ddJcSr1w=; b=f6e3XaGb4L0R7Q f3vzdiYqOOPEfQl2Zkt1xZhEqAAL6JA5UbT0s8xMp37Yt1KvzEGbIQGZwkZ1IoI2TiW8gcLMlZn62 Q8LA7cwc0UquyGPSYqLDmiSR+XVtd3RCnVaqshbr+IMEWMDulu/DQAnhdOqQK4FymG7edIqbVIQRw SPfMwbt+qtwU31F50U08jNaQQAd+DJ2sNgs79T8dQOYcNKHP9HMR2yQPrZ2Gsyd/L3+MylAryQGcP Muo9ffa0YEi7EUJ8tqpNegLtzkMFFcRntkrVC3t5dP8wIDrt0mpzXNeQulPj68SZRHcx6bGibMtux En1wO1N7Soom4e/mzSrA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qEyud-002Bu7-31; Thu, 29 Jun 2023 21:10:47 +0000 Received: from madras.collabora.co.uk ([46.235.227.172]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qEyub-002BsD-18; Thu, 29 Jun 2023 21:10:46 +0000 Received: from notapiano (zone.collabora.co.uk [167.235.23.81]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: nfraprado) by madras.collabora.co.uk (Postfix) with ESMTPSA id CBDD066070D1; Thu, 29 Jun 2023 22:10:38 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1688073041; bh=JCh8QNgbheBN9vzum2hXOH+Xccw6yemZ2xjeKair+aY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=awtycltug4rEIpIWPHSmCJ0Ar7z77OoWwDGCry2DH+yxVYb/zvGXFn4MY6+0VYWHz KsjdF6RlG839AVnwTUXG5EhlfY57qurBilxeD4c48tgwS66/uibtEDaER8NzfwVG5S JHXvf7vP2vcIQKryhP7Nc2aPRt+dbb+oXgMTCIsXDUC66CsEfWU9LrVu+nJyGWIw83 BSZf9ksQr+YcWLUt9cu1rx4wlaf479EvT0yP1Jab0CD+ftjnz/ZU+t4SXMd1MUetTU H2jTI5x4AB+Dy+grt0C/nvXBZA132sNpsVnx645CtNY3W9M2+wrzL+iw7ra+tbJehg JGugr9zpCYcOQ== Date: Thu, 29 Jun 2023 17:10:34 -0400 From: =?utf-8?B?TsOtY29sYXMgRi4gUi4gQS4=?= Prado To: Daniel Lezcano Cc: kernel@collabora.com, Alexandre Mergnat , Balsam CHIHI , Chen-Yu Tsai , Alexandre Bailon , AngeloGioacchino Del Regno , Amit Kucheria , Matthias Brugger , "Rafael J. Wysocki" , Zhang Rui , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-pm@vger.kernel.org Subject: Re: [PATCH v2 5/6] thermal/drivers/mediatek/lvts_thermal: Don't leave threshold zeroed Message-ID: <819b9c10-044c-4a2b-b8f2-6ddd633bbf8d@notapiano> References: <20230504004852.627049-1-nfraprado@collabora.com> <20230504004852.627049-6-nfraprado@collabora.com> <5d810151-3a3b-51af-bbc1-0ee45668bcc4@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5d810151-3a3b-51af-bbc1-0ee45668bcc4@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230629_141045_606921_0233A5FF X-CRM114-Status: GOOD ( 26.83 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Jun 02, 2023 at 11:17:27AM +0200, Daniel Lezcano wrote: > On 04/05/2023 02:48, N=EDcolas F. R. A. Prado wrote: > > The thermal framework might leave the low threshold unset if there > > aren't any lower trip points. This leaves the register zeroed, which > > translates to a very high temperature for the low threshold. The > > interrupt for this threshold is then immediately triggered, and the > > state machine gets stuck, preventing any other temperature monitoring > > interrupts to ever trigger. > > = > > (The same happens by not setting the Cold or Hot to Normal thresholds > > when using those) > > = > > Set the unused threshold to a valid low value. This value was chosen so > > that for any valid golden temperature read from the efuse, when the > > value is converted to raw and back again to milliCelsius, the result > > doesn't underflow. > > = > > Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage The= rmal Sensor driver") > > Signed-off-by: N=EDcolas F. R. A. Prado > > = > > --- > > = > > Changes in v2: > > - Added this commit > > = > > drivers/thermal/mediatek/lvts_thermal.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > = > > diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/= mediatek/lvts_thermal.c > > index efd1e938e1c2..951a4cb75ef6 100644 > > --- a/drivers/thermal/mediatek/lvts_thermal.c > > +++ b/drivers/thermal/mediatek/lvts_thermal.c > > @@ -82,6 +82,8 @@ > > #define LVTS_HW_SHUTDOWN_MT8195 105000 > > #define LVTS_HW_SHUTDOWN_MT8192 105000 > > +#define LVTS_MINIUM_THRESHOLD 20000 > = > MINIMUM > = > So if the thermal zone reaches 20=B0C, the interrupt fires, the set_trips= sets > again 20=B0C but the interrupt won't fire until the temperature goes above > 20=B0C and then crosses the temperature low threshold the way down again? Well, actually, set_trips won't even be called since from the thermal framework's perspective we haven't crossed trip points, ie in __thermal_zone_set_trips(): /* No need to change trip points */ if (tz->prev_low_trip =3D=3D low && tz->prev_high_trip =3D=3D high) return; But in any case, yes, the interrupt will fire, the temperature will get upd= ated in the framework, and that's it. It will only fire again when a threshold is crossed again (either by the temperature rising and falling again below this minimum, or rising beyond the high treshold). So basically at most this will cause a spurious interrupt when the temperat= ure gets low enough. I do get 34-36C on all sensors when idling though, so I do= ubt that temperature is even reachable. Besides, we don't really have another o= ption here if we want working interrupts, the threshold needs to be set to a valid value, and this is the lowest I've found. And thanks for all the feedback! I'll prepare a v3 based on your comments. Thanks, N=EDcolas _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel