From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: rafael@kernel.org, daniel.lezcano@linaro.org
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
"Thierry Reding" <thierry.reding@gmail.com>,
"Amit Kucheria" <amitk@kernel.org>,
"Zhang Rui" <rui.zhang@intel.com>,
"Jonathan Hunter" <jonathanh@nvidia.com>,
"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
"AngeloGioacchino Del Regno"
<angelogioacchino.delregno@collabora.com>,
"Florian Fainelli" <f.fainelli@gmail.com>,
"Jernej Skrabec" <jernej.skrabec@gmail.com>,
linux-tegra@vger.kernel.org (open list:TEGRA ARCHITECTURE
SUPPORT)
Subject: [PATCH v5 16/18] thermal/drivers/tegra: Remove unneeded lock when setting a trip point
Date: Wed, 1 Mar 2023 21:14:44 +0100 [thread overview]
Message-ID: <20230301201446.3713334-17-daniel.lezcano@linaro.org> (raw)
In-Reply-To: <20230301201446.3713334-1-daniel.lezcano@linaro.org>
The function tegra_tsensor_enable_hw_channel() takes the thermal zone
lock to prevent "a potential" race with a call to set_trips()
callback.
The driver must not play with the thermal framework core code
internals.
The tegra_tsensor_enable_hw_channel() is called by:
- the suspend / resume callbacks
- the probe function after the thermal zones are registered
The thermal zone lock taken in this function is supposed to protect
from a call to the set_trips() callback which writes in the same
register.
The potential race is when suspend / resume are called at the same
time as set_trips. This one is called only in
thermal_zone_device_update().
- At suspend time, the 'in_suspend' is set, thus the
thermal_zone_device_update() bails out immediately and set_trips is
not called during this moment.
- At resume time, the thermal zone is updated at PM_POST_SUSPEND,
thus the driver has already set the TH2 temperature.
- At probe time, we register the thermal zone and then we set the
TH2. The only scenario I can see so far is the interrupt fires, the
thermal_zone_update() is called exactly at the moment
tegra_tsensor_enable_hw_channel() a few lines after registering it.
Enable the channels before setting up the interrupt. We close the
potential race window without using the thermal zone's lock.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Suggested-by: Thierry Reding <thierry.reding@gmail.com>
---
drivers/thermal/tegra/tegra30-tsensor.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/thermal/tegra/tegra30-tsensor.c b/drivers/thermal/tegra/tegra30-tsensor.c
index 4b2ea17910cd..cb584a5735ed 100644
--- a/drivers/thermal/tegra/tegra30-tsensor.c
+++ b/drivers/thermal/tegra/tegra30-tsensor.c
@@ -359,9 +359,6 @@ static int tegra_tsensor_enable_hw_channel(const struct tegra_tsensor *ts,
tegra_tsensor_get_hw_channel_trips(tzd, &hot_trip, &crit_trip);
- /* prevent potential racing with tegra_tsensor_set_trips() */
- mutex_lock(&tzd->lock);
-
dev_info_once(ts->dev, "ch%u: PMC emergency shutdown trip set to %dC\n",
id, DIV_ROUND_CLOSEST(crit_trip, 1000));
@@ -404,8 +401,6 @@ static int tegra_tsensor_enable_hw_channel(const struct tegra_tsensor *ts,
val |= FIELD_PREP(TSENSOR_SENSOR0_CONFIG0_INTR_THERMAL_RST_EN, 1);
writel_relaxed(val, tsc->regs + TSENSOR_SENSOR0_CONFIG0);
- mutex_unlock(&tzd->lock);
-
err = thermal_zone_device_enable(tzd);
if (err) {
dev_err(ts->dev, "ch%u: failed to enable zone: %d\n", id, err);
@@ -585,6 +580,20 @@ static int tegra_tsensor_probe(struct platform_device *pdev)
return err;
}
+ /*
+ * Enable the channels before setting the interrupt so
+ * set_trips() can not be called while we are setting up the
+ * register TSENSOR_SENSOR0_CONFIG1. With this we close a
+ * potential race window where we are setting up the TH2 and
+ * the temperature hits TH1 resulting to an update of the
+ * TSENSOR_SENSOR0_CONFIG1 register in the ISR.
+ */
+ for (i = 0; i < ARRAY_SIZE(ts->ch); i++) {
+ err = tegra_tsensor_enable_hw_channel(ts, i);
+ if (err)
+ return err;
+ }
+
err = devm_request_threaded_irq(&pdev->dev, irq, NULL,
tegra_tsensor_isr, IRQF_ONESHOT,
"tegra_tsensor", ts);
@@ -592,12 +601,6 @@ static int tegra_tsensor_probe(struct platform_device *pdev)
return dev_err_probe(&pdev->dev, err,
"failed to request interrupt\n");
- for (i = 0; i < ARRAY_SIZE(ts->ch); i++) {
- err = tegra_tsensor_enable_hw_channel(ts, i);
- if (err)
- return err;
- }
-
return 0;
}
--
2.34.1
next prev parent reply other threads:[~2023-03-01 20:17 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-01 20:14 [PATCH v5 00/18] Self-encapsulate the thermal zone device structure Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 01/18] thermal/core: Add a thermal zone 'devdata' accessor Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 02/18] thermal/core: Use the thermal zone 'devdata' accessor in thermal located drivers Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano
2023-03-03 11:28 ` Daniel Lezcano
2023-03-03 11:28 ` Daniel Lezcano
2023-03-03 11:55 ` Linus Walleij
2023-03-03 11:55 ` Linus Walleij
2023-03-03 15:47 ` Heiko Stübner
2023-03-03 15:47 ` Heiko Stübner
2023-03-03 16:21 ` Kunihiko Hayashi
2023-03-03 16:21 ` Kunihiko Hayashi
2023-03-01 20:14 ` [PATCH v5 03/18] thermal/core: Use the thermal zone 'devdata' accessor in hwmon " Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 04/18] thermal/core: Use the thermal zone 'devdata' accessor in remaining drivers Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 05/18] thermal/core: Show a debug message when get_temp() fails Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 06/18] thermal: Remove debug or error messages in get_temp() ops Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano
2023-03-03 15:47 ` Heiko Stübner
2023-03-03 15:47 ` Heiko Stübner
2023-03-03 15:47 ` Heiko Stübner
2023-03-01 20:14 ` [PATCH v5 07/18] thermal/hwmon: Do not set no_hwmon before calling thermal_add_hwmon_sysfs() Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano
2023-03-03 15:48 ` Heiko Stübner
2023-03-03 15:48 ` Heiko Stübner
2023-03-03 15:48 ` Heiko Stübner
2023-03-01 20:14 ` [PATCH v5 08/18] thermal/hwmon: Use the right device for devm_thermal_add_hwmon_sysfs() Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 09/18] thermal: Don't use 'device' internal thermal zone structure field Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 10/18] thermal/core: Add thermal_zone_device structure 'type' accessor Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 11/18] thermal/drivers/spear: Don't use tz->device but pdev->dev Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 12/18] thermal: Add a thermal zone id accessor Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 13/18] thermal: Use thermal_zone_device_type() accessor Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 14/18] thermal/drivers/da9062: Don't access the thermal zone device fields Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 15/18] thermal/hwmon: Use the thermal_core.h header Daniel Lezcano
2023-03-01 20:14 ` Daniel Lezcano [this message]
2023-03-02 9:47 ` [PATCH v5 16/18] thermal/drivers/tegra: Remove unneeded lock when setting a trip point Thierry Reding
2023-03-02 10:00 ` Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 17/18] thermal/drivers/acerhdf: Make interval setting only at module load time Daniel Lezcano
2023-03-01 20:14 ` [PATCH v5 18/18] thermal/drivers/acerhdf: Remove pointless governor test Daniel Lezcano
2023-03-03 9:24 ` [PATCH v5 00/18] Self-encapsulate the thermal zone device structure Daniel Lezcano
2023-03-03 19:48 ` Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230301201446.3713334-17-daniel.lezcano@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=amitk@kernel.org \
--cc=angelogioacchino.delregno@collabora.com \
--cc=f.fainelli@gmail.com \
--cc=jernej.skrabec@gmail.com \
--cc=jonathanh@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=niklas.soderlund+renesas@ragnatech.se \
--cc=rafael@kernel.org \
--cc=rui.zhang@intel.com \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.