Hi Niklas,
On Tue, Mar 10, 2020 at 6:02 PM Niklas Söderlund
<niklas.soderlund+renesas@xxxxxxxxxxxx> wrote:
> There is no need to cache the ctemp value in the private data structure
> as it's always prefetched before it's used. Remove it from the structure
> and have rcar_thermal_update_temp return the value instead of storing
> it.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
Thanks for your patch!
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
One suggestion below...
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -201,7 +200,6 @@ static int rcar_thermal_update_temp(struct rcar_thermal_priv *priv)
> struct device *dev = rcar_priv_to_dev(priv);
> int i;
> u32 ctemp, old, new;
> - int ret = -EINVAL;
It might make sense to change ctemp to int, and preinitialize it to
-EINVAL...
>
> mutex_lock(&priv->lock);
>
> @@ -247,32 +245,28 @@ static int rcar_thermal_update_temp(struct rcar_thermal_priv *priv)
> ((ctemp - 1) << 0)));
> }
>
> - dev_dbg(dev, "thermal%d %d -> %d\n", priv->id, priv->ctemp, ctemp);
> -
> - priv->ctemp = ctemp;
> - ret = 0;
> err_out_unlock:
> mutex_unlock(&priv->lock);
> - return ret;
> +
> + return ctemp ? ctemp : -EINVAL;
... so you can just return ctemp here.
> }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
