lkml.org 
[lkml]   [2019]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 14/15] drivers: thermal: tsens: Create function to return sign-extended temperature
    From
    Date
    Quoting Amit Kucheria (2019-07-25 15:18:49)
    > diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
    > index 7ab2e740a1da..13a875b99094 100644
    > --- a/drivers/thermal/qcom/tsens-common.c
    > +++ b/drivers/thermal/qcom/tsens-common.c
    > @@ -84,13 +84,35 @@ static inline int code_to_degc(u32 adc_code, const struct tsens_sensor *s)
    > return degc;
    > }
    >
    > +/**
    > + * tsens_hw_to_mC - Return properly sign extended temperature in mCelsius,

    Can you make this proper kernel-doc? Describe the arguments and have a
    "Return:" section.

    > + * whether in ADC code or deciCelsius depending on IP version.
    > + * This function handles the different widths of the signed integer across IPs.
    > + */
    > +static int tsens_hw_to_mC(char *str, struct tsens_sensor *s, int field, int temp)
    > +{
    > + struct tsens_priv *priv = s->priv;
    > + u32 mask;
    > +
    > + if (priv->feat->adc) {
    > + /* Convert temperature from ADC code to milliCelsius */
    > + return code_to_degc(temp, s) * 1000;
    > + } else {

    Please deindent and drop the else because there's a return above.

    > + mask = GENMASK(priv->fields[field].msb,
    > + priv->fields[field].lsb) >> priv->fields[field].lsb;

    Why is the mask generated, shifted right, sent into fls(), and then
    passed to sign_extend32? Shoudln't it be something like

    sign_extend32(temp, priv->fields[field].msg - priv->fiels[field].lsb - 1)

    > + dev_dbg(priv->dev, "%s: mask: %d\n", str, fls(mask));
    > + /* Convert temperature from deciCelsius to milliCelsius */
    > + return sign_extend32(temp, fls(mask) - 1) * 100;
    > + }
    > +}
    > +
    > @@ -112,15 +134,7 @@ int get_temp_tsens_valid(struct tsens_sensor *s, int *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;

    Oh the code is copied. Seems really complicated still.

    > - }
    > + *temp = tsens_hw_to_mC("get_temp", s, LAST_TEMP_0, last_temp);

    \
     
     \ /
      Last update: 2019-08-17 06:17    [W:4.030 / U:1.416 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site