Date:   Thu, 2 Mar 2023 10:47:02 +0100
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Daniel Lezcano <daniel.lezcano@linaro.org>
Cc:     rafael@kernel.org, linux-pm@vger.kernel.org,
        linux-kernel@vger.kernel.org, Amit Kucheria <amitk@kernel.org>,
        Zhang Rui <rui.zhang@intel.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Niklas =?utf-8?Q?S=C3=B6derlund?= 
        <niklas.soderlund+renesas@ragnatech.se>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        "open list:TEGRA ARCHITECTURE SUPPORT" <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH v5 16/18] thermal/drivers/tegra: Remove unneeded lock
 when setting a trip point
Message-ID: <ZABwllXuTHbUhnue@orome>
References: <20230301201446.3713334-1-daniel.lezcano@linaro.org>
 <20230301201446.3713334-17-daniel.lezcano@linaro.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="4/U54hRVG0VAzoU8"
Content-Disposition: inline
In-Reply-To: <20230301201446.3713334-17-daniel.lezcano@linaro.org>
User-Agent: Mutt/2.2.9 (2022-11-12)
Precedence: bulk


--4/U54hRVG0VAzoU8
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 01, 2023 at 09:14:44PM +0100, Daniel Lezcano wrote:
> The function tegra_tsensor_enable_hw_channel() takes the thermal zone
> lock to prevent "a potential" race with a call to set_trips()
> callback.
>=20
> The driver must not play with the thermal framework core code
> internals.
>=20
> The tegra_tsensor_enable_hw_channel() is called by:
>=20
>  - the suspend / resume callbacks
>  - the probe function after the thermal zones are registered
>=20
> 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.
>=20
> 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().
>=20
>  - 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.
>=20
>  - At resume time, the thermal zone is updated at PM_POST_SUSPEND,
>    thus the driver has already set the TH2 temperature.
>=20
>  - 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.
>=20
> Enable the channels before setting up the interrupt. We close the
> potential race window without using the thermal zone's lock.
>=20
> 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(-)

Acked-by: Thierry Reding <treding@nvidia.com>

--4/U54hRVG0VAzoU8
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmQAcJYACgkQ3SOs138+
s6HAow//bXdkecSf8BK7JCcXO2dmg2HCFrd0N4Kt/cY/86hVZKL4rmqgEz8gqwaG
k/PhFwYLrLWqdenFyA47E1rj5Np2uQy6b5+iS6345abC3k0nA/j6wwDzNOXCiQKn
vMC7ZJwJx2nZxmaFiv46z2SJ3k8sdA3ew6dACrRXUAtbOerUZtdOKU0JQjzyIKBo
JURmhfexudw1oVkg8GqUmpc1dfO8pJ40jDu93QoPS1y/JPPzy5lJ6sjxaWRbELa8
tRSza/pyNm3fkJp18iOvoj3IIoVYLIuvT7shF9DuAI7FL+zXDU4o3JBs1jvBcqUE
dzQY/MNyeekQFwiq1XNxc42JzBLSWWfeuwV/LSNyfyvoYieHXaWCcjPSqK/X+JmD
UcRvrKc8rO0q6XkWneMk1RTzMyKNw17LofYHWt1WFHt82zJxTa8wSE6z2Rvx8ebK
e/MrYzuGrPiyzxzwugtVYu0fOUzdFlEip9s/iFHrhn8x/1QKR4tloJEUd5c557ap
bhUvsjYcy2zyjWVUE0gg2RbT4QG8TkO7EjpUkIFwDJIHxn1ASVlV9eE6L8TOan9P
lrGfvfmu+NrsHWjhhr1wNbxyQeA/ns83CV3nAKcz+5pxMLjtzWfpbLoqSx6e9H6G
6YekX1Yizxss8/93IskqGB5I9Y9xzE36+y1iqnTFO2XjQXmZbYg=
=jAUw
-----END PGP SIGNATURE-----

--4/U54hRVG0VAzoU8--