| Message ID | 64a3d07ebe5c4cfb4643d91f5f6605e8a4ffa48b.1566907161.git.amit.kucheria@linaro.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | thermal: qcom: tsens: Add interrupt support | expand |
Quoting Amit Kucheria (2019-08-27 05:14:10) > diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c > index ea2c46cc6a66a..06b44cfd5eab9 100644 > --- a/drivers/thermal/qcom/tsens-common.c > +++ b/drivers/thermal/qcom/tsens-common.c > @@ -84,13 +84,43 @@ static inline int code_to_degc(u32 adc_code, const struct tsens_sensor *s) > return degc; > } > > +/** > + * tsens_hw_to_mC - Return sign-extended temperature in mCelsius. > + * @s: Pointer to sensor struct sensor? This isn't golang! > + * @field: Index into regmap_field array pointing to temperature data > + * > + * This function handles temperature returned in ADC code or deciCelsius > + * depending on IP version. > + * > + * Return: Temperature in milliCelsius on success, a negative errno will > + * be returned in error cases > + */ > +static int tsens_hw_to_mC(struct tsens_sensor *s, int field) > +{ > + struct tsens_priv *priv = s->priv; > + u32 temp = 0; > + int ret; > + > + ret = regmap_field_read(priv->rf[field], &temp); > + if (ret) > + return ret; > + > + if (priv->feat->adc) { > + /* Convert temperature from ADC code to milliCelsius */ Nitpick: Move this comment above the if and drop the braces. > + return code_to_degc(temp, s) * 1000; > + } > + > + /* deciCelsius -> milliCelsius along with sign extension */ > + return sign_extend32(temp, priv->tempres) * 100; > +} > + > int get_temp_tsens_valid(struct tsens_sensor *s, int *temp) > { > struct tsens_priv *priv = s->priv; > int hw_id = s->hw_id; > u32 temp_idx = LAST_TEMP_0 + hw_id; > u32 valid_idx = VALID_0 + hw_id; > - u32 last_temp = 0, valid, mask; > + u32 valid; > int ret; > > ret = regmap_field_read(priv->rf[valid_idx], &valid); > @@ -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? > for (i = 0, j = VALID_0; i < priv->feat->max_sensors; i++, j++) { > priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map, > priv->fields[j]);
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@chromium.org> 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.
diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c index ea2c46cc6a66a..06b44cfd5eab9 100644 --- a/drivers/thermal/qcom/tsens-common.c +++ b/drivers/thermal/qcom/tsens-common.c @@ -84,13 +84,43 @@ static inline int code_to_degc(u32 adc_code, const struct tsens_sensor *s) return degc; } +/** + * tsens_hw_to_mC - Return sign-extended temperature in mCelsius. + * @s: Pointer to sensor struct + * @field: Index into regmap_field array pointing to temperature data + * + * This function handles temperature returned in ADC code or deciCelsius + * depending on IP version. + * + * Return: Temperature in milliCelsius on success, a negative errno will + * be returned in error cases + */ +static int tsens_hw_to_mC(struct tsens_sensor *s, int field) +{ + struct tsens_priv *priv = s->priv; + u32 temp = 0; + int ret; + + ret = regmap_field_read(priv->rf[field], &temp); + if (ret) + return ret; + + if (priv->feat->adc) { + /* Convert temperature from ADC code to milliCelsius */ + return code_to_degc(temp, s) * 1000; + } + + /* deciCelsius -> milliCelsius along with sign extension */ + return sign_extend32(temp, priv->tempres) * 100; +} + int get_temp_tsens_valid(struct tsens_sensor *s, int *temp) { struct tsens_priv *priv = s->priv; int hw_id = s->hw_id; u32 temp_idx = LAST_TEMP_0 + hw_id; u32 valid_idx = VALID_0 + hw_id; - u32 last_temp = 0, valid, mask; + u32 valid; int ret; ret = regmap_field_read(priv->rf[valid_idx], &valid); @@ -108,19 +138,7 @@ int get_temp_tsens_valid(struct tsens_sensor *s, int *temp) } /* Valid bit is set, OK to read the temperature */ - ret = regmap_field_read(priv->rf[temp_idx], &last_temp); - if (ret) - return ret; - - if (priv->feat->adc) { - /* Convert temperature from ADC code to milliCelsius */ - *temp = code_to_degc(last_temp, s) * 1000; - } else { - mask = GENMASK(priv->fields[LAST_TEMP_0].msb, - priv->fields[LAST_TEMP_0].lsb); - /* Convert temperature from deciCelsius to milliCelsius */ - *temp = sign_extend32(last_temp, fls(mask) - 1) * 100; - } + *temp = tsens_hw_to_mC(s, temp_idx); return 0; } @@ -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; + for (i = 0, j = VALID_0; i < priv->feat->max_sensors; i++, j++) { priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map, priv->fields[j]); diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h index e1d6af71b2b9a..409395e436f32 100644 --- a/drivers/thermal/qcom/tsens.h +++ b/drivers/thermal/qcom/tsens.h @@ -283,6 +283,7 @@ struct tsens_context { * struct tsens_priv - private data for each instance of the tsens IP * @dev: pointer to struct device * @num_sensors: number of sensors enabled on this device + * @tempres: number of bits used to represent signed temperature (resolution) * @tm_map: pointer to TM register address space * @srot_map: pointer to SROT register address space * @tm_offset: deal with old device trees that don't address TM and SROT @@ -299,6 +300,7 @@ struct tsens_context { struct tsens_priv { struct device *dev; u32 num_sensors; + u32 tempres; struct regmap *tm_map; struct regmap *srot_map; u32 tm_offset;
Hide the details of how to convert values read from TSENS HW to mCelsius behind a function. All versions of the IP can be supported as a result. Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> --- drivers/thermal/qcom/tsens-common.c | 50 +++++++++++++++++++++-------- drivers/thermal/qcom/tsens.h | 2 ++ 2 files changed, 38 insertions(+), 14 deletions(-)