Re: [PATCH v3 2/2] thermal: qcom: tsens-v1: Add support for MSM8992/4 TSENS

[Dmitry Baryshkov]

On Tue, 28 Jun 2022 at 16:19, Konrad Dybcio
<konrad.dybcio@xxxxxxxxxxxxxx> wrote:
>
>
>
> On 27.06.2022 17:15, Dmitry Baryshkov wrote:
> > On Sun, 1 May 2022 at 23:21, Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx> wrote:
> >>
> >> MSM8994, despite being heavily based on MSM8974, uses the
> >> 1.2 version of TSENS. Also, 8994 being 8994, it has a custom
> >> way of calculating the slope.
> >>
> >> MSM8992 in turn is a cut-down version of MSM8994 and uses
> >> the same TSENS hardware, albeit with a different set of sensors.
> >>
> >> Also tested on 8976 (by a person who didn't want to be named)
> >> to make sure the 11->16 max_sensors changes didn't break anything.
> >>
> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx>
> >> ---
> >> Changes since v2:
> >>
> >> - don't use slope before it's initialized (whoops!)
> >> - don't re-assign the same value to p[0..15]
> >> - free calib_rsel
> >> - use the same ops for 8992 and 8994
> >>
> >>  drivers/thermal/qcom/tsens-v1.c | 293 ++++++++++++++++++++++++++++++--
> >>  drivers/thermal/qcom/tsens.c    |   6 +
> >>  drivers/thermal/qcom/tsens.h    |   2 +-
> >>  3 files changed, 288 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
> >> index 573e261ccca7..58864962f370 100644
> >> --- a/drivers/thermal/qcom/tsens-v1.c
> >> +++ b/drivers/thermal/qcom/tsens-v1.c
> >> @@ -142,6 +142,99 @@
> >>  #define CAL_SEL_MASK   7
> >>  #define CAL_SEL_SHIFT  0
> >>
> >> +/* eeprom layout data for 8994 */
> >> +#define MSM8994_BASE0_MASK     0x3ff
> >> +#define MSM8994_BASE1_MASK     0xffc00
> >> +#define MSM8994_BASE0_SHIFT    0
> >> +#define MSM8994_BASE1_SHIFT    10
> >
> > Generally I feel that all the _MASK values can be better generated
> > using the GENMASK and newly defined _SHIFT value.
> Yes, I agree, however I think it'd be better to do it for
> all the defines in this file at once in a separate commit.

Then I'd suggest fixing the file first and adding 8992/4 on top of that.

>
> Konrad
> >
> >> +
> >> +#define MSM8994_S0_MASK        0xf00000
> >> +#define MSM8994_S1_MASK        0xf000000
> >> +#define MSM8994_S2_MASK        0xf0000000
> >> +#define MSM8994_S3_MASK        0xf
> >> +#define MSM8994_S4_MASK        0xf0
> >> +#define MSM8994_S5_MASK        0xf00
> >> +#define MSM8994_S6_MASK        0xf000
> >> +#define MSM8994_S7_MASK        0xf0000
> >> +#define MSM8994_S8_MASK        0xf00000
> >> +#define MSM8994_S9_MASK        0xf000000
> >> +#define MSM8994_S10_MASK       0xf0000000
> >> +#define MSM8994_S11_MASK       0xf
> >> +#define MSM8994_S12_MASK       0xf0
> >> +#define MSM8994_S13_MASK       0xf00
> >> +#define MSM8994_S14_MASK       0xf000
> >> +#define MSM8994_S15_MASK       0xf0000
> >> +
> >> +#define MSM8994_S0_SHIFT       20
> >> +#define MSM8994_S1_SHIFT       24
> >> +#define MSM8994_S2_SHIFT       28
> >> +#define MSM8994_S3_SHIFT       0
> >> +#define MSM8994_S4_SHIFT       4
> >> +#define MSM8994_S5_SHIFT       8
> >> +#define MSM8994_S6_SHIFT       12
> >> +#define MSM8994_S7_SHIFT       16
> >> +#define MSM8994_S8_SHIFT       20
> >> +#define MSM8994_S9_SHIFT       24
> >> +#define MSM8994_S10_SHIFT      28
> >> +#define MSM8994_S11_SHIFT      0
> >> +#define MSM8994_S12_SHIFT      4
> >> +#define MSM8994_S13_SHIFT      8
> >> +#define MSM8994_S14_SHIFT      12
> >> +#define MSM8994_S15_SHIFT      16
> >
> > [skipped the rest]
> >



-- 
With best wishes
Dmitry