linux-kernel - Re: [PATCH v1 1/1] thermal: rcar_gen3_thermal: request IRQ after device initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <92504666-9e17-4c6d-fc74-6cf68b9f017b@linaro.org>
Date:   Tue, 16 Apr 2019 21:22:59 +0200
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     Jiada Wang <jiada_wang@...tor.com>, rui.zhang@...el.com,
        edubezval@...il.com
Cc:     linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/1] thermal: rcar_gen3_thermal: request IRQ after
 device initialization

On 11/04/2019 12:03, Jiada Wang wrote:
> Currently IRQ is remain enabled after .remove, later if device is probed,
> IRQ is requested before .thermal_init, this may cause IRQ function be
> triggered but not able to clear IRQ status, thus cause system to hang.
> 
> this patch by moving request of IRQ after device initialization to
> avoid this issue.

Why not disable the interrupt and clear the irq status in the .remove
callback, so the place is clean when probing again?


        struct rcar_gen3_thermal_priv *priv = dev_get_drvdata(dev);

        rcar_thermal_irq_set(priv, false);

should do the trick no ?

> Signed-off-by: Jiada Wang <jiada_wang@...tor.com>
> ---
>  drivers/thermal/rcar_gen3_thermal.c | 48 ++++++++++++++++-------------
>  1 file changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 88fa41cf16e8..4d095d7f9763 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -375,28 +375,6 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, priv);
>  
> -	/*
> -	 * Request 2 (of the 3 possible) IRQs, the driver only needs to
> -	 * to trigger on the low and high trip points of the current
> -	 * temp window at this point.
> -	 */
> -	for (i = 0; i < 2; i++) {
> -		irq = platform_get_irq(pdev, i);
> -		if (irq < 0)
> -			return irq;
> -
> -		irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:ch%d",
> -					 dev_name(dev), i);
> -		if (!irqname)
> -			return -ENOMEM;
> -
> -		ret = devm_request_threaded_irq(dev, irq, rcar_gen3_thermal_irq,
> -						rcar_gen3_thermal_irq_thread,
> -						IRQF_SHARED, irqname, priv);
> -		if (ret)
> -			return ret;
> -	}
> -
>  	pm_runtime_enable(dev);
>  	pm_runtime_get_sync(dev);
>  
> @@ -458,6 +436,32 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
>  		goto error_unregister;
>  	}
>  
> +	/*
> +	 * Request 2 (of the 3 possible) IRQs, the driver only needs to
> +	 * to trigger on the low and high trip points of the current
> +	 * temp window at this point.
> +	 */
> +	for (i = 0; i < 2; i++) {
> +		irq = platform_get_irq(pdev, i);
> +		if (irq < 0) {
> +			ret = irq;
> +			goto error_unregister;
> +		}
> +
> +		irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:ch%d",
> +					 dev_name(dev), i);
> +		if (!irqname) {
> +			ret = -ENOMEM;
> +			goto error_unregister;
> +		}
> +
> +		ret = devm_request_threaded_irq(dev, irq, rcar_gen3_thermal_irq,
> +						rcar_gen3_thermal_irq_thread,
> +						IRQF_SHARED, irqname, priv);
> +		if (ret)
> +			goto error_unregister;
> +	}
> +
>  	rcar_thermal_irq_set(priv, true);
>  
>  	return 0;
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog