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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ee50d097-6213-36dd-9966-bdd5b2bb953b@quicinc.com>
Date:   Wed, 30 Aug 2023 07:50:22 +0530
From:   Sricharan Ramabadhran <quic_srichara@...cinc.com>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC:     <agross@...nel.org>, <andersson@...nel.org>,
        <konrad.dybcio@...aro.org>, <srinivas.kandagatla@...aro.org>,
        <robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
        <conor+dt@...nel.org>, <thara.gopinath@...il.com>,
        <rafael@...nel.org>, <daniel.lezcano@...aro.org>,
        <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-pm@...r.kernel.org>
Subject: Re: [PATCH 3/7] thermal/drivers/qcom/tsens: Add support for IPQ5018
 tsens

<..>

>> +{
>> +       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