Thread (16 messages) 16 messages, 4 authors, 2015-12-15

Re: [PATCH 4/8 v4] thermal: rcar: retern error rcar_thermal_get_temp() if no ctemp update

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: 2015-12-14 23:55:39
Also in: linux-pm, linux-sh, lkml

Hi Eduardo
quoted
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -199,9 +199,9 @@ static int rcar_thermal_update_temp(struct rcar_thermal_priv *priv)
 
 	dev_dbg(dev, "thermal%d  %d -> %d\n", priv->id, priv->ctemp, ctemp);
 
-	priv->ctemp = ctemp;
 	ret = 0;
 err_out_unlock:
+	priv->ctemp = ctemp;
 	mutex_unlock(&priv->lock);
 	return ret;
I believe the problem here is actually the lack of error
handling/propagation. Are you sure you want to write to parameter
in the fail path ?

rcar_thermal_update_temp already returns error code when it fails
to read temperature. Don't you think it would make more sense to fix the
places that call rcar_thermal_update_temp to properly handle its return
value and propagate that error code when necessary?
Thanks. But the logic is that
priv->ctemp will be 0 in error cases, and it will be reported as
dev_err() in rcar_thermal_get_current_temp()

The problem was that priv->ctemp will not be updated
in error case of rcar_thermal_update_temp().
but user can call get_temp (it doesn't call
rcar_thermal_update_temp() if interrupt was supported).
In such case, user will get old thermal.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help