Re: [PATCH v2 14/15] drivers: thermal: tsens: Create function to return sign-extended temperature

From: Stephen Boyd
Date: Wed Aug 28 2019 - 11:44:43 EST


Quoting Amit Kucheria (2019-08-28 03:35:28)
> (Resending, replied only to Stephen by mistake)
>
> On Wed, Aug 28, 2019 at 6:08 AM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> >
> > Quoting Amit Kucheria (2019-08-27 05:14:10)
> > > @@ -310,6 +328,10 @@ int __init init_common(struct tsens_priv *priv)
> > > Â Â Â Â Â Â Â Â Â Â Â Â goto err_put_device;
> > > Â Â Â Â Â Â Â Â }
> > > Â Â Â Â }
> > > +
> > > + Â Â Â /* Save away resolution of signed temperature value for this IP */
> > > + Â Â Â priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields
> [LAST_TEMP_0].lsb;
> > > +
> >
> > Why not just calculate this in the function that uses it? Is there a
> > reason to stash it away in the struct?
>
> To avoid recalculating in an often-called function. It doesn't change for an IP
> version.
>
> We can't make it static either inside that function since the initializer isn't
> constant.
>

This sounds like a super micro optimization. It's a couple derefs and a
subtraction. If it isn't used anywhere else please just move it into the
function where it's used.