Re: [PATCH v3 1/2] thermal: rcar_gen3_thermal: fix interrupt type

[Eugeniu Rosca]

Hi Jiada,

On Tue, Apr 23, 2019 at 11:14:13PM +0900, Jiada Wang wrote:
> Currently IRQF_SHARED type interrupt line is allocated, but it
> is not appropriate, as the interrupt line isn't shared between
> different devices, instead IRQF_ONESHOT is the proper type.
> 
> By changing interrupt type to IRQF_ONESHOT, now irq handler is
> no longer needed, as clear of interrupt status can be done in
> threaded interrupt context.
> 
> Because IRQF_ONESHOT type interrupt line is kept disabled until
> the threaded handler has been run, so there is no need to protect
> read/write of REG_GEN3_IRQSTR with lock.
> 
> Signed-off-by: Jiada Wang <jiada_wang@xxxxxxxxxx>
> ---
>  drivers/thermal/rcar_gen3_thermal.c | 34 +++++------------------------
>  1 file changed, 6 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 88fa41cf16e8..ebbaef5a3cf1 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -231,39 +231,17 @@ static void rcar_thermal_irq_set(struct rcar_gen3_thermal_priv *priv, bool on)
>  static irqreturn_t rcar_gen3_thermal_irq(int irq, void *data)
>  {
>  	struct rcar_gen3_thermal_priv *priv = data;
> +	int i;
>  	u32 status;
> -	int i, ret = IRQ_HANDLED;

nit: why not keeping 'int i' on the same line?

>  
> -	spin_lock(&priv->lock);
[..]
> -	spin_unlock(&priv->lock);
[..]
> -	spin_lock_irqsave(&priv->lock, flags);
[..]
> -	spin_unlock_irqrestore(&priv->lock, flags);
[..]

FWIW, after applying the patch, the 'spinlock_t lock' member of
'struct rcar_gen3_thermal_priv' is no longer used in the driver, except
to initialize it in probe. Is there any usage expected in future or
should it be zapped?

-- 
Best regards,
Eugeniu.