[2/3] thermal: rcar_thermal: Do not store ctemp in rcar_thermal_priv
diff mbox series

Message ID 20200310170029.1648996-3-niklas.soderlund+renesas@ragnatech.se
State New
Delegated to: Daniel Lezcano
Headers show
Series
  • thermal: rcar_thermal: Update how temperature is read
Related show

Commit Message

Niklas Söderlund March 10, 2020, 5 p.m. UTC
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(-)

Comments

Geert Uytterhoeven March 11, 2020, 12:40 p.m. UTC | #1
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

Patch
diff mbox series

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. */