| Message ID | 20230315103950.2679317-1-stephan.gerhold@kernkonzept.com |
|---|---|
| Headers | show |
| Series | thermal: qcom: tsens: Fix MDM9607, add MSM8909 | expand |
On 15.03.2023 11:39, Stephan Gerhold wrote: > The old single-cell parsing code was removed for MSM8939, MDM9607 and > MSM8976 but for some reason the structs defining the bit positions etc > were kept around (unused). Drop them now. > > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Fixes: 51d78b8b1beb ("thermal/drivers/tsens: Drop single-cell code for mdm9607") > Fixes: dfadb4599ab0 ("thermal/drivers/tsens: Drop single-cell code for msm8939") > Fixes: 3a908971f7cb ("thermal/drivers/tsens: Drop single-cell code for msm8976/msm8956") > Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com> > --- > Changes in v3: None > Changes in v2: New patch > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > drivers/thermal/qcom/tsens-v0_1.c | 36 ------------------------------- > drivers/thermal/qcom/tsens-v1.c | 22 ------------------- > 2 files changed, 58 deletions(-) > > diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c > index e89c6f39a3ae..106d26076e3f 100644 > --- a/drivers/thermal/qcom/tsens-v0_1.c > +++ b/drivers/thermal/qcom/tsens-v0_1.c > @@ -39,26 +39,6 @@ struct tsens_legacy_calibration_format tsens_8916_nvmem = { > }, > }; > > -struct tsens_legacy_calibration_format tsens_8939_nvmem = { > - .base_len = 8, > - .base_shift = 2, > - .sp_len = 6, > - .mode = { 12, 0 }, > - .invalid = { 12, 2 }, > - .base = { { 0, 0 }, { 1, 24 } }, > - .sp = { > - { { 12, 3 }, { 12, 9 } }, > - { { 12, 15 }, { 12, 21 } }, > - { { 12, 27 }, { 13, 1 } }, > - { { 13, 7 }, { 13, 13 } }, > - { { 13, 19 }, { 13, 25 } }, > - { { 0, 8 }, { 0, 14 } }, > - { { 0, 20 }, { 0, 26 } }, > - { { 1, 0 }, { 1, 6 } }, > - { { 1, 12 }, { 1, 18 } }, > - }, > -}; > - > struct tsens_legacy_calibration_format tsens_8974_nvmem = { > .base_len = 8, > .base_shift = 2, > @@ -103,22 +83,6 @@ struct tsens_legacy_calibration_format tsens_8974_backup_nvmem = { > }, > }; > > -struct tsens_legacy_calibration_format tsens_9607_nvmem = { > - .base_len = 8, > - .base_shift = 2, > - .sp_len = 6, > - .mode = { 2, 20 }, > - .invalid = { 2, 22 }, > - .base = { { 0, 0 }, { 2, 12 } }, > - .sp = { > - { { 0, 8 }, { 0, 14 } }, > - { { 0, 20 }, { 0, 26 } }, > - { { 1, 0 }, { 1, 6 } }, > - { { 1, 12 }, { 1, 18 } }, > - { { 2, 0 }, { 2, 6 } }, > - }, > -}; > - > static int calibrate_8916(struct tsens_priv *priv) > { > u32 p1[5], p2[5]; > diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c > index b822a426066d..51322430f1fe 100644 > --- a/drivers/thermal/qcom/tsens-v1.c > +++ b/drivers/thermal/qcom/tsens-v1.c > @@ -42,28 +42,6 @@ struct tsens_legacy_calibration_format tsens_qcs404_nvmem = { > }, > }; > > -struct tsens_legacy_calibration_format tsens_8976_nvmem = { > - .base_len = 8, > - .base_shift = 2, > - .sp_len = 6, > - .mode = { 4, 0 }, > - .invalid = { 4, 2 }, > - .base = { { 0, 0 }, { 2, 8 } }, > - .sp = { > - { { 0, 8 }, { 0, 14 } }, > - { { 0, 20 }, { 0, 26 } }, > - { { 1, 0 }, { 1, 6 } }, > - { { 1, 12 }, { 1, 18 } }, > - { { 2, 8 }, { 2, 14 } }, > - { { 2, 20 }, { 2, 26 } }, > - { { 3, 0 }, { 3, 6 } }, > - { { 3, 12 }, { 3, 18 } }, > - { { 4, 2 }, { 4, 9 } }, > - { { 4, 14 }, { 4, 21 } }, > - { { 4, 26 }, { 5, 1 } }, > - }, > -}; > - > static int calibrate_v1(struct tsens_priv *priv) > { > u32 p1[10], p2[10];
On 15.03.2023 11:39, Stephan Gerhold wrote: > The MSM8909 SoC has 5 thermal sensors in a TSENS v0.1 block. Like > MDM9607 it uses a non-standard default slope value of 3000 [1] and needs > per-sensor "correction factors" to workaround issues with the factory > calibration [2]. > > [1]: https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/blob/LA.UM.7.7.c26-09100-8x09.0/arch/arm/boot/dts/qcom/msm8909.dtsi#L476 > [2]: https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/commit/6df022c6d0c2c1b4a5a6c2124dba4d57910c0911 > > Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com> > --- > Changes in v3: None > Changes in v2: > - Rewrite on top of per-sensor nvmem cell changes that landed in 6.3 > --- > drivers/thermal/qcom/tsens-v0_1.c | 32 ++++++++++++++++++++++++++++++- > drivers/thermal/qcom/tsens.c | 3 +++ > drivers/thermal/qcom/tsens.h | 2 +- > 3 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c > index e69889dd524a..2fda1ff4f6a6 100644 > --- a/drivers/thermal/qcom/tsens-v0_1.c > +++ b/drivers/thermal/qcom/tsens-v0_1.c > @@ -207,6 +207,23 @@ static int calibrate_8974(struct tsens_priv *priv) > return 0; > } > > +static int __init init_8909(struct tsens_priv *priv) > +{ > + int i; > + > + for (i = 0; i < priv->num_sensors; ++i) > + priv->sensor[i].slope = 3000; > + I think assigning 0 here explicitly to priv->sensor[0].p12_calib_offset, while unnecessary, would make it a bit more obvious. Either way: Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > + priv->sensor[1].p1_calib_offset = -10; > + priv->sensor[1].p2_calib_offset = -6; > + priv->sensor[3].p1_calib_offset = -9; > + priv->sensor[3].p2_calib_offset = -9; > + priv->sensor[4].p1_calib_offset = -8; > + priv->sensor[4].p2_calib_offset = -10; > + > + return init_common(priv); > +} > + > static int __init init_8939(struct tsens_priv *priv) { > priv->sensor[0].slope = 2911; > priv->sensor[1].slope = 2789; > @@ -243,7 +260,7 @@ static int __init init_9607(struct tsens_priv *priv) > return init_common(priv); > } > > -/* v0.1: 8916, 8939, 8974, 9607 */ > +/* v0.1: 8909, 8916, 8939, 8974, 9607 */ > > static struct tsens_features tsens_v0_1_feat = { > .ver_major = VER_0_1, > @@ -292,6 +309,19 @@ static const struct reg_field tsens_v0_1_regfields[MAX_REGFIELDS] = { > [TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0), > }; > > +static const struct tsens_ops ops_8909 = { > + .init = init_8909, > + .calibrate = tsens_calibrate_common, > + .get_temp = get_temp_common, > +}; > + > +struct tsens_plat_data data_8909 = { > + .num_sensors = 5, > + .ops = &ops_8909, > + .feat = &tsens_v0_1_feat, > + .fields = tsens_v0_1_regfields, > +}; > + > static const struct tsens_ops ops_8916 = { > .init = init_common, > .calibrate = calibrate_8916, > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > index 32d2907f76e5..a04179247b34 100644 > --- a/drivers/thermal/qcom/tsens.c > +++ b/drivers/thermal/qcom/tsens.c > @@ -1109,6 +1109,9 @@ static const struct of_device_id tsens_table[] = { > }, { > .compatible = "qcom,mdm9607-tsens", > .data = &data_9607, > + }, { > + .compatible = "qcom,msm8909-tsens", > + .data = &data_8909, > }, { > .compatible = "qcom,msm8916-tsens", > .data = &data_8916, > diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h > index 880be6be5c3f..c88287dede96 100644 > --- a/drivers/thermal/qcom/tsens.h > +++ b/drivers/thermal/qcom/tsens.h > @@ -639,7 +639,7 @@ int get_temp_common(const struct tsens_sensor *s, int *temp); > extern struct tsens_plat_data data_8960; > > /* TSENS v0.1 targets */ > -extern struct tsens_plat_data data_8916, data_8939, data_8974, data_9607; > +extern struct tsens_plat_data data_8909, data_8916, data_8939, data_8974, data_9607; > > /* TSENS v1 targets */ > extern struct tsens_plat_data data_tsens_v1, data_8976, data_8956;
On Sat, Mar 18, 2023 at 03:02:35PM +0100, Konrad Dybcio wrote: > On 15.03.2023 11:39, Stephan Gerhold wrote: > > According to the msm-3.18 vendor kernel from Qualcomm, mdm9607 needs > > "correction factors" to adjust for additional offsets observed after the > > factory calibration values in the fuses [1, 2]. > > > > The fixed offsets should be applied unless there is a special > > calibration mode value that indicates that no offsets are needed [3]. > > > > Note that the new calibration mode values are called differently in this > > patch compared to the vendor kernel: > > - TSENS_TWO_POINT_CALIB_N_WA -> ONE_PT_CALIB2_NO_OFFSET > > - TSENS_TWO_POINT_CALIB_N_OFFSET_WA -> TWO_PT_CALIB_NO_OFFSET > > This is because close inspection of the calibration function [3] reveals > > that TSENS_TWO_POINT_CALIB_N_WA is actually a "one point" calibration > > because the if statements skip all "point2" related code for it. > > > > [1]: https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/commit/d9d2db1b82bf3f72f5de0803d55e6849eb5b671e > > [2]: https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/commit/d75aef53a760e8ff7bac54049d00c8b2ee1b193e > > [3]: https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/blob/LE.UM.4.3.2.r1-04200-9x07/drivers/thermal/msm-tsens.c#L2987-3136 > > > > Cc: Konrad Dybcio <konrad.dybcio@linaro.org> > > Fixes: a2149ab815fc ("thermal/drivers/qcom/tsens-v0_1: Add support for MDM9607") > > Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com> > > --- > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > BTW, did you notice some crazy readouts or would this have gone > unnoticed had you not dug in the code? > I'm afraid it would likely have remained unnoticed. I think these offsets only make a small difference but it's still good to have them for slightly more accurate readings. In v1 of this series I had the offsets for MSM8909 already (hardcoded into the old calibration function with all the bit shifts/masks etc). It was more coincidence that I checked MDM9607 for v2 because I had to make the code more generic with the new per-sensor nvmem cells. ;) Thanks, Stephan