On 28 June 2022 17:23:58 GMT+03:00, Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx> wrote: >Convert the masks to use GENMASK. Tested by objdumping and making sure >the output is identical pre- and post this patch. > >Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx> >--- >Changes since v3: >* Add this patch > > drivers/thermal/qcom/tsens-v1.c | 107 ++++++++++++++++---------------- > 1 file changed, 54 insertions(+), 53 deletions(-) > >diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c >index 573e261ccca7..d6f0dec4bfa1 100644 >--- a/drivers/thermal/qcom/tsens-v1.c >+++ b/drivers/thermal/qcom/tsens-v1.c >@@ -3,6 +3,7 @@ > * Copyright (c) 2019, Linaro Limited > */ > >+#include <linux/bits.h> > #include <linux/bitops.h> > #include <linux/regmap.h> > #include <linux/delay.h> >@@ -22,34 +23,34 @@ > #define TM_HIGH_LOW_Sn_INT_THRESHOLD_OFF 0x0090 > > /* eeprom layout data for msm8956/76 (v1) */ >-#define MSM8976_BASE0_MASK 0xff >-#define MSM8976_BASE1_MASK 0xff >+#define MSM8976_BASE0_MASK GENMASK(7, 0) >+#define MSM8976_BASE1_MASK GENMASK(7, 0) > #define MSM8976_BASE1_SHIFT 8 Hmm. Is the BASE1_MASK correct? Should it be 0xff00 instead? (Yes, it's not a mistake in your patch, but let's make sure it is not a mistake). > >-#define MSM8976_S0_P1_MASK 0x3f00 >-#define MSM8976_S1_P1_MASK 0x3f00000 >-#define MSM8976_S2_P1_MASK 0x3f >-#define MSM8976_S3_P1_MASK 0x3f000 >-#define MSM8976_S4_P1_MASK 0x3f00 >-#define MSM8976_S5_P1_MASK 0x3f00000 >-#define MSM8976_S6_P1_MASK 0x3f >-#define MSM8976_S7_P1_MASK 0x3f000 >-#define MSM8976_S8_P1_MASK 0x1f8 >-#define MSM8976_S9_P1_MASK 0x1f8000 >-#define MSM8976_S10_P1_MASK 0xf8000000 >-#define MSM8976_S10_P1_MASK_1 0x1 >- >-#define MSM8976_S0_P2_MASK 0xfc000 >-#define MSM8976_S1_P2_MASK 0xfc000000 >-#define MSM8976_S2_P2_MASK 0xfc0 >-#define MSM8976_S3_P2_MASK 0xfc0000 >-#define MSM8976_S4_P2_MASK 0xfc000 >-#define MSM8976_S5_P2_MASK 0xfc000000 >-#define MSM8976_S6_P2_MASK 0xfc0 >-#define MSM8976_S7_P2_MASK 0xfc0000 >-#define MSM8976_S8_P2_MASK 0x7e00 >-#define MSM8976_S9_P2_MASK 0x7e00000 >-#define MSM8976_S10_P2_MASK 0x7e >+#define MSM8976_S0_P1_MASK GENMASK(13, 8) I was thinking about: #define MSM8976_MASK(shift) GENMASK((shift) + 5, (shift)) #define MSM8976_S1_P1_MASK MSM8976_MASK(MSM8976_S0_P1_SHIFT) But... While you are reworking this driver, could you please replace mask+shift statements with the FIELD_GET / FIELD_PREP? There is a huge chance that we can drop _SHIFT completely and just use newly defined masks from this patch. >+#define MSM8976_S1_P1_MASK GENMASK(25, 20) >+#define MSM8976_S2_P1_MASK GENMASK(5, 0) >+#define MSM8976_S3_P1_MASK GENMASK(17, 12) >+#define MSM8976_S4_P1_MASK GENMASK(13, 8) >+#define MSM8976_S5_P1_MASK GENMASK(25, 20) >+#define MSM8976_S6_P1_MASK GENMASK(5, 0) >+#define MSM8976_S7_P1_MASK GENMASK(17, 12) >+#define MSM8976_S8_P1_MASK GENMASK(8, 3) >+#define MSM8976_S9_P1_MASK GENMASK(20, 15) >+#define MSM8976_S10_P1_MASK GENMASK(31, 27) >+#define MSM8976_S10_P1_MASK_1 GENMASK(0, 0) >+ >+#define MSM8976_S0_P2_MASK GENMASK(19, 14) >+#define MSM8976_S1_P2_MASK GENMASK(31, 26) >+#define MSM8976_S2_P2_MASK GENMASK(11, 6) >+#define MSM8976_S3_P2_MASK GENMASK(23, 18) >+#define MSM8976_S4_P2_MASK GENMASK(19, 14) >+#define MSM8976_S5_P2_MASK GENMASK(31, 26) >+#define MSM8976_S6_P2_MASK GENMASK(11, 6) >+#define MSM8976_S7_P2_MASK GENMASK(23, 18) >+#define MSM8976_S8_P2_MASK GENMASK(14, 9) >+#define MSM8976_S9_P2_MASK GENMASK(26, 21) >+#define MSM8976_S10_P2_MASK GENMASK(6, 1) > > #define MSM8976_S0_P1_SHIFT 8 > #define MSM8976_S1_P1_SHIFT 20 >@@ -76,7 +77,7 @@ > #define MSM8976_S9_P2_SHIFT 21 > #define MSM8976_S10_P2_SHIFT 1 > >-#define MSM8976_CAL_SEL_MASK 0x3 >+#define MSM8976_CAL_SEL_MASK GENMASK(1, 0) > > #define MSM8976_CAL_DEGC_PT1 30 > #define MSM8976_CAL_DEGC_PT2 120 >@@ -84,34 +85,34 @@ > #define MSM8976_SLOPE_DEFAULT 3200 > > /* eeprom layout data for qcs404/405 (v1) */ >-#define BASE0_MASK 0x000007f8 >-#define BASE1_MASK 0x0007f800 >+#define BASE0_MASK GENMASK(10, 3) >+#define BASE1_MASK GENMASK(18, 11) > #define BASE0_SHIFT 3 > #define BASE1_SHIFT 11 > >-#define S0_P1_MASK 0x0000003f >-#define S1_P1_MASK 0x0003f000 >-#define S2_P1_MASK 0x3f000000 >-#define S3_P1_MASK 0x000003f0 >-#define S4_P1_MASK 0x003f0000 >-#define S5_P1_MASK 0x0000003f >-#define S6_P1_MASK 0x0003f000 >-#define S7_P1_MASK 0x3f000000 >-#define S8_P1_MASK 0x000003f0 >-#define S9_P1_MASK 0x003f0000 >- >-#define S0_P2_MASK 0x00000fc0 >-#define S1_P2_MASK 0x00fc0000 >-#define S2_P2_MASK_1_0 0xc0000000 >-#define S2_P2_MASK_5_2 0x0000000f >-#define S3_P2_MASK 0x0000fc00 >-#define S4_P2_MASK 0x0fc00000 >-#define S5_P2_MASK 0x00000fc0 >-#define S6_P2_MASK 0x00fc0000 >-#define S7_P2_MASK_1_0 0xc0000000 >-#define S7_P2_MASK_5_2 0x0000000f >-#define S8_P2_MASK 0x0000fc00 >-#define S9_P2_MASK 0x0fc00000 >+#define S0_P1_MASK GENMASK(5, 0) >+#define S1_P1_MASK GENMASK(17, 12) >+#define S2_P1_MASK GENMASK(29, 24) >+#define S3_P1_MASK GENMASK(9, 4) >+#define S4_P1_MASK GENMASK(21, 16) >+#define S5_P1_MASK GENMASK(5, 0) >+#define S6_P1_MASK GENMASK(17, 12) >+#define S7_P1_MASK GENMASK(29, 24) >+#define S8_P1_MASK GENMASK(9, 4) >+#define S9_P1_MASK GENMASK(21, 16) >+ >+#define S0_P2_MASK GENMASK(11, 6) >+#define S1_P2_MASK GENMASK(23, 18) >+#define S2_P2_MASK_1_0 GENMASK(31, 30) >+#define S2_P2_MASK_5_2 GENMASK(3, 0) >+#define S3_P2_MASK GENMASK(15, 10) >+#define S4_P2_MASK GENMASK(27, 22) >+#define S5_P2_MASK GENMASK(11, 6) >+#define S6_P2_MASK GENMASK(23, 18) >+#define S7_P2_MASK_1_0 GENMASK(31, 30) >+#define S7_P2_MASK_5_2 GENMASK(3, 0) >+#define S8_P2_MASK GENMASK(15, 10) >+#define S9_P2_MASK GENMASK(27, 22) > > #define S0_P1_SHIFT 0 > #define S0_P2_SHIFT 6 >@@ -139,7 +140,7 @@ > #define S9_P1_SHIFT 16 > #define S9_P2_SHIFT 22 > >-#define CAL_SEL_MASK 7 >+#define CAL_SEL_MASK GENMASK(2, 0) > #define CAL_SEL_SHIFT 0 > > static void compute_intercept_slope_8976(struct tsens_priv *priv, -- With best wishes Dmitry
