Re: [PATCH v4 4/4] drivers/thermal/exymos: Use guard notation when acquiring mutex

[Krzysztof Kozlowski]

On 10/03/2025 15:34, Anand Moon wrote:
> Using guard notation makes the code more compact and error handling
> more robust by ensuring that mutexes are released in all code paths
> when control leaves critical section.
> 

Subject: typo, exynos

> Signed-off-by: Anand Moon <linux.amoon@xxxxxxxxx>
> ---
> v4: used DEFINE_GUARD macro to guard exynos_tmu_data structure.
>     However, incorporating guard(exynos_tmu_data)(data); results
>     in a recursive deadlock with the mutex during initialization, as this
>     data structure is common to all the code configurations of Exynos TMU
> v3: New patch

If you ever use cleanup or guards, you must build your code with recent
clang and W=1. Failure to do so means you ask reviewers manually to spot
issues not visible in the context, instead of using tools. It's a NAK
for me.

> ---
>  drivers/thermal/samsung/exynos_tmu.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index a71cde0a4b17e..85f88c5e0f11c 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -12,6 +12,7 @@
>   */
>  
>  #include <linux/clk.h>
> +#include <linux/cleanup.h>
>  #include <linux/io.h>
>  #include <linux/interrupt.h>
>  #include <linux/module.h>
> @@ -199,6 +200,9 @@ struct exynos_tmu_data {
>  	void (*tmu_clear_irqs)(struct exynos_tmu_data *data);
>  };
>  
> +DEFINE_GUARD(exynos_tmu_data, struct exynos_tmu_data *,

I do not understand why do you need custom guard.

> +	     mutex_lock(&_T->lock), mutex_unlock(&_T->lock))
> +
>  /*
>   * TMU treats temperature as a mapped temperature code.
>   * The temperature is converted differently depending on the calibration type.
> @@ -256,7 +260,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  	unsigned int status;
>  	int ret = 0;
>  
> -	mutex_lock(&data->lock);
> +	guard(mutex)(&data->lock);

Which you do not use... Please don't use cleanup.h if you do not know
it. It leads to bugs.


Best regards,
Krzysztof