| Message ID | 20200310170029.1648996-3-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | thermal: rcar_thermal: Update how temperature is read | expand |
Hi Niklas, On Tue, Mar 10, 2020 at 6:02 PM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> 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@ragnatech.se> Thanks for your patch! Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> 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
diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c index f379eb5f8b9ecd14..953dc28d1dd1d499 100644 --- a/drivers/thermal/rcar_thermal.c +++ b/drivers/thermal/rcar_thermal.c @@ -95,7 +95,6 @@ struct rcar_thermal_priv { struct mutex lock; struct list_head list; int id; - u32 ctemp; }; #define rcar_thermal_for_each_priv(pos, common) \ @@ -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; 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; } static int rcar_thermal_get_current_temp(struct rcar_thermal_priv *priv, int *temp) { - int tmp; - int ret; + int ctemp, tmp; - ret = rcar_thermal_update_temp(priv); - if (ret < 0) - return ret; + ctemp = rcar_thermal_update_temp(priv); + if (ctemp < 0) + return ctemp; mutex_lock(&priv->lock); if (priv->chip->ctemp_bands == 1) - tmp = MCELSIUS((priv->ctemp * 5) - 65); - else if (priv->ctemp < 24) - tmp = MCELSIUS(((priv->ctemp * 55) - 720) / 10); + tmp = MCELSIUS((ctemp * 5) - 65); + else if (ctemp < 24) + tmp = MCELSIUS(((ctemp * 55) - 720) / 10); else - tmp = MCELSIUS((priv->ctemp * 5) - 60); + tmp = MCELSIUS((ctemp * 5) - 60); mutex_unlock(&priv->lock); /* Guaranteed operating range is -45C to 125C. */
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@ragnatech.se> --- drivers/thermal/rcar_thermal.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-)