Re: [PATCH 3/7] thermal/drivers/qcom/tsens: Add support for IPQ5018 tsens

[Sricharan Ramabadhran <quic_srichara@xxxxxxxxxxx>]

<..>

> > +{
> > +       u32 p1[10], p2[10];
> > +       u32 *qfprom_cdata;
> > +       int mode;
> > +
> > +       qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
> > +       if (IS_ERR(qfprom_cdata))
> > +               return PTR_ERR(qfprom_cdata);
> > +
> > +       mode = tsens_read_calibration_legacy(priv, &tsens_ipq5018_nvmem,
> > +                                            p1, p2,
> > +                                            qfprom_cdata, NULL);
>
> No, this should be used only in _legacy_ cases. Please use
> tsens_calibrate_common() / tsens_calibrate_nvmem() / etc.

 ok.

> > +
> > +       fixup_ipq5018_points(mode, p1, p2);
> > +       compute_intercept_slope(priv, p1, p2, mode);
> > +       kfree(qfprom_cdata);
> > +
> > +       return 0;
> > +}
> > +
> >   static int calibrate_v1(struct tsens_priv *priv)
> >   {
> >          u32 p1[10], p2[10];
> > @@ -79,6 +132,18 @@ static struct tsens_features tsens_v1_feat = {
> >          .trip_max_temp  = 120000,
> >   };
> >
> > +static struct tsens_features tsens_v1_ipq5018_feat = {
> > +       .ver_major      = VER_1_X,
> > +       .crit_int       = 0,
> > +       .combo_int      = 0,
> > +       .adc            = 1,
> > +       .srot_split     = 1,
> > +       .max_sensors    = 11,
> > +       .trip_min_temp  = -40000,
> > +       .trip_max_temp  = 120000,
> > +       .no_early_init  = 1,
> > +};
> > +
> >   static const struct reg_field tsens_v1_regfields[MAX_REGFIELDS] = {
> >          /* ----- SROT ------ */
> >          /* VERSION */
> > @@ -150,6 +215,43 @@ static int __init init_8956(struct tsens_priv *priv) {
> >          return init_common(priv);
> >   }
> >
> > +static int init_ipq5018(struct tsens_priv *priv)
> > +{
> > +       int ret;
> > +       u32 mask;
> > +
> > +       init_common(priv);
> > +       if (!priv->tm_map)
> > +               return -ENODEV;
> > +
> > +       ret = regmap_field_write(priv->rf[TSENS_SW_RST], 1);
> > +       if (ret) {
> > +               dev_err(priv->dev, "Reset failed\n");
> > +               return ret;
> > +       }
> > +
> > +       mask = GENMASK(10, 0);
>
> #define this, then inline the variable. Or extract this codepiece into
> generic function which uses num_sensors to calculate the mask

 ok.

> > +       ret = regmap_field_update_bits(priv->rf[SENSOR_EN], mask, mask);
> > +       if (ret) {
> > +               dev_err(priv->dev, "Sensor Enable failed\n");
> > +               return ret;
> > +       }
> > +
> > +       ret = regmap_field_write(priv->rf[TSENS_EN], 1);
> > +       if (ret) {
> > +               dev_err(priv->dev, "Enable failed\n");
> > +               return ret;
> > +       }
> > +
> > +       ret = regmap_field_write(priv->rf[TSENS_SW_RST], 0);
> > +       if (ret) {
> > +               dev_err(priv->dev, "Reset failed\n");
>
> This error message is useless. You can not determine if it comes from
> this error or from setting the reset bit.

 ok, will drop it.

Regards,
 Sricharan