Re: [RESEND PATCH v1 1/2] thermal: qcom-spmi-temp-alarm: add support for GEN2 rev 1 PMIC peripherals
On Tue, Jul 28, 2020 at 06:08:28PM -0700, Stephen Boyd wrote:
> Quoting Guru Das Srinagesh (2020-07-24 10:46:10)
> > diff --git a/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
> > b/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
> > index bf7bae4..05a9601 100644
> > --- a/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
> > +++ b/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
> > @@ -38,26 +39,30 @@
> >
> > #define ALARM_CTRL_FORCE_ENABLEBIT(7)
> >
> > -/*
> > - * Trip point values based on threshold control
> > - * 0 = {105 C, 125 C, 145 C}
> > - * 1 = {110 C, 130 C, 150 C}
> > - * 2 = {115 C, 135 C, 155 C}
> > - * 3 = {120 C, 140 C, 160 C}
> > -*/
> > -#define TEMP_STAGE_STEP2 /* Stage step:
> > 20.000 C */
> > -#define TEMP_STAGE_HYSTERESIS 2000
> > +#define THRESH_COUNT 4
> > +#define STAGE_COUNT3
> > +
> > +/* Over-temperature trip point values in mC */
> > +static const long temp_map_gen1[THRESH_COUNT][STAGE_COUNT] = {
> > + {105000, 125000, 145000},
>
> Please add a space after { and before }
Done.
>
> > + {11, 13, 15},
> > + {115000, 135000, 155000},
> > + {12, 14, 16},
> > +};
> > +
> > +static const long temp_map_gen2_v1[THRESH_COUNT][STAGE_COUNT] = {
> > + { 9, 11, 14},
>
> Almost.
Done.
>
> > + { 95000, 115000, 145000},
> > + {10, 12, 15},
> > + {105000, 125000, 155000},
> > +};
> >
> > -#define TEMP_THRESH_MIN105000 /* Threshold Min:
> > 105 C */
> > -#define TEMP_THRESH_STEP 5000/* Threshold step: 5 C */
> > +#define TEMP_THRESH_STEP 5000 /* Threshold step: 5 C */
> >
> > #define THRESH_MIN 0
> > #define THRESH_MAX 3
> >
> > -/* Stage 2 Threshold Min: 125 C */
> > -#define STAGE2_THRESHOLD_MIN 125000
> > -/* Stage 2 Threshold Max: 140 C */
> > -#define STAGE2_THRESHOLD_MAX 14
> > +#define TEMP_STAGE_HYSTERESIS 2000
> >
> > /* Temperature in Milli Celsius reported during stage 0 if no ADC is
> > present */
> > #define DEFAULT_TEMP 37000
> > @@ -77,6 +82,7 @@ struct qpnp_tm_chip {
> > boolinitialized;
> >
> > struct iio_channel *adc;
> > + const long
> > (*temp_map)[THRESH_COUNT][STAGE_COUNT];
>
> It can be negative Celsius?
It's not unsigned because it has to match struct qpnp_tm_chip's temp
member, which is long - hope I understood your concern correctly. The
temperatures themselves cannot be negative.
>
> > };
> >
> > /* This array maps from GEN2 alarm state to GEN1 alarm stage */
> > @@ -101,6 +107,23 @@ static int qpnp_tm_write(struct qpnp_tm_chip *chip,
> > u16 addr, u8 data)
> > }
> >
> > /**
> > + * qpnp_tm_decode_temp() - return temperature in mC corresponding to the
> > + * specified over-temperature stage
> > + * @chip: Pointer to the qpnp_tm chip
> > + * @stage: Over-temperature stage
> > + *
> > + * Return: temperature in mC
> > + */
> > +static long qpnp_tm_decode_temp(struct qpnp_tm_chip *chip, unsigned int
> > stage)
> > +{
> > + if (!chip->temp_map || chip->thresh >= THRESH_COUNT || stage == 0
> > + || stage > STAGE_COUNT)
>
> Nitpick: The || goes on the line above.
Done.
>
> > + return 0;
> > +
> > + return (*chip->temp_map)[chip->thresh][stage - 1];
> > +}
> > +
> > +/**
> > * qpnp_tm_get_temp_stage() - return over-temperature stage
> > * @chip: Pointer to the qpnp_tm chip
> > *
Re: [RESEND PATCH v1 1/2] thermal: qcom-spmi-temp-alarm: add support for GEN2 rev 1 PMIC peripherals
Quoting Guru Das Srinagesh (2020-07-24 10:46:10)
> diff --git a/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
> b/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
> index bf7bae4..05a9601 100644
> --- a/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
> +++ b/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
> @@ -38,26 +39,30 @@
>
> #define ALARM_CTRL_FORCE_ENABLEBIT(7)
>
> -/*
> - * Trip point values based on threshold control
> - * 0 = {105 C, 125 C, 145 C}
> - * 1 = {110 C, 130 C, 150 C}
> - * 2 = {115 C, 135 C, 155 C}
> - * 3 = {120 C, 140 C, 160 C}
> -*/
> -#define TEMP_STAGE_STEP2 /* Stage step: 20.000
> C */
> -#define TEMP_STAGE_HYSTERESIS 2000
> +#define THRESH_COUNT 4
> +#define STAGE_COUNT3
> +
> +/* Over-temperature trip point values in mC */
> +static const long temp_map_gen1[THRESH_COUNT][STAGE_COUNT] = {
> + {105000, 125000, 145000},
Please add a space after { and before }
> + {11, 13, 15},
> + {115000, 135000, 155000},
> + {12, 14, 16},
> +};
> +
> +static const long temp_map_gen2_v1[THRESH_COUNT][STAGE_COUNT] = {
> + { 9, 11, 14},
Almost.
> + { 95000, 115000, 145000},
> + {10, 12, 15},
> + {105000, 125000, 155000},
> +};
>
> -#define TEMP_THRESH_MIN105000 /* Threshold Min: 105
> C */
> -#define TEMP_THRESH_STEP 5000/* Threshold step: 5 C */
> +#define TEMP_THRESH_STEP 5000 /* Threshold step: 5 C */
>
> #define THRESH_MIN 0
> #define THRESH_MAX 3
>
> -/* Stage 2 Threshold Min: 125 C */
> -#define STAGE2_THRESHOLD_MIN 125000
> -/* Stage 2 Threshold Max: 140 C */
> -#define STAGE2_THRESHOLD_MAX 14
> +#define TEMP_STAGE_HYSTERESIS 2000
>
> /* Temperature in Milli Celsius reported during stage 0 if no ADC is present
> */
> #define DEFAULT_TEMP 37000
> @@ -77,6 +82,7 @@ struct qpnp_tm_chip {
> boolinitialized;
>
> struct iio_channel *adc;
> + const long
> (*temp_map)[THRESH_COUNT][STAGE_COUNT];
It can be negative Celsius?
> };
>
> /* This array maps from GEN2 alarm state to GEN1 alarm stage */
> @@ -101,6 +107,23 @@ static int qpnp_tm_write(struct qpnp_tm_chip *chip, u16
> addr, u8 data)
> }
>
> /**
> + * qpnp_tm_decode_temp() - return temperature in mC corresponding to the
> + * specified over-temperature stage
> + * @chip: Pointer to the qpnp_tm chip
> + * @stage: Over-temperature stage
> + *
> + * Return: temperature in mC
> + */
> +static long qpnp_tm_decode_temp(struct qpnp_tm_chip *chip, unsigned int
> stage)
> +{
> + if (!chip->temp_map || chip->thresh >= THRESH_COUNT || stage == 0
> + || stage > STAGE_COUNT)
Nitpick: The || goes on the line above.
> + return 0;
> +
> + return (*chip->temp_map)[chip->thresh][stage - 1];
> +}
> +
> +/**
> * qpnp_tm_get_temp_stage() - return over-temperature stage
> * @chip: Pointer to the qpnp_tm chip
> *
[RESEND PATCH v1 1/2] thermal: qcom-spmi-temp-alarm: add support for GEN2 rev 1 PMIC peripherals
From: David Collins
Add support for TEMP_ALARM GEN2 PMIC peripherals with digital
major revision 1. This revision utilizes a different temperature
threshold mapping than earlier revisions.
Signed-off-by: David Collins
Signed-off-by: Guru Das Srinagesh
---
drivers/thermal/qcom/qcom-spmi-temp-alarm.c | 91 +++--
1 file changed, 61 insertions(+), 30 deletions(-)
diff --git a/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
b/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
index bf7bae4..05a9601 100644
--- a/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
+++ b/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
@@ -17,6 +17,7 @@
#include "../thermal_core.h"
+#define QPNP_TM_REG_DIG_MAJOR 0x01
#define QPNP_TM_REG_TYPE 0x04
#define QPNP_TM_REG_SUBTYPE0x05
#define QPNP_TM_REG_STATUS 0x08
@@ -38,26 +39,30 @@
#define ALARM_CTRL_FORCE_ENABLEBIT(7)
-/*
- * Trip point values based on threshold control
- * 0 = {105 C, 125 C, 145 C}
- * 1 = {110 C, 130 C, 150 C}
- * 2 = {115 C, 135 C, 155 C}
- * 3 = {120 C, 140 C, 160 C}
-*/
-#define TEMP_STAGE_STEP2 /* Stage step: 20.000 C
*/
-#define TEMP_STAGE_HYSTERESIS 2000
+#define THRESH_COUNT 4
+#define STAGE_COUNT3
+
+/* Over-temperature trip point values in mC */
+static const long temp_map_gen1[THRESH_COUNT][STAGE_COUNT] = {
+ {105000, 125000, 145000},
+ {11, 13, 15},
+ {115000, 135000, 155000},
+ {12, 14, 16},
+};
+
+static const long temp_map_gen2_v1[THRESH_COUNT][STAGE_COUNT] = {
+ { 9, 11, 14},
+ { 95000, 115000, 145000},
+ {10, 12, 15},
+ {105000, 125000, 155000},
+};
-#define TEMP_THRESH_MIN105000 /* Threshold Min: 105 C
*/
-#define TEMP_THRESH_STEP 5000/* Threshold step: 5 C */
+#define TEMP_THRESH_STEP 5000 /* Threshold step: 5 C */
#define THRESH_MIN 0
#define THRESH_MAX 3
-/* Stage 2 Threshold Min: 125 C */
-#define STAGE2_THRESHOLD_MIN 125000
-/* Stage 2 Threshold Max: 140 C */
-#define STAGE2_THRESHOLD_MAX 14
+#define TEMP_STAGE_HYSTERESIS 2000
/* Temperature in Milli Celsius reported during stage 0 if no ADC is present */
#define DEFAULT_TEMP 37000
@@ -77,6 +82,7 @@ struct qpnp_tm_chip {
boolinitialized;
struct iio_channel *adc;
+ const long (*temp_map)[THRESH_COUNT][STAGE_COUNT];
};
/* This array maps from GEN2 alarm state to GEN1 alarm stage */
@@ -101,6 +107,23 @@ static int qpnp_tm_write(struct qpnp_tm_chip *chip, u16
addr, u8 data)
}
/**
+ * qpnp_tm_decode_temp() - return temperature in mC corresponding to the
+ * specified over-temperature stage
+ * @chip: Pointer to the qpnp_tm chip
+ * @stage: Over-temperature stage
+ *
+ * Return: temperature in mC
+ */
+static long qpnp_tm_decode_temp(struct qpnp_tm_chip *chip, unsigned int stage)
+{
+ if (!chip->temp_map || chip->thresh >= THRESH_COUNT || stage == 0
+ || stage > STAGE_COUNT)
+ return 0;
+
+ return (*chip->temp_map)[chip->thresh][stage - 1];
+}
+
+/**
* qpnp_tm_get_temp_stage() - return over-temperature stage
* @chip: Pointer to the qpnp_tm chip
*
@@ -149,14 +172,12 @@ static int qpnp_tm_update_temp_no_adc(struct qpnp_tm_chip
*chip)
if (stage_new > stage_old) {
/* increasing stage, use lower bound */
- chip->temp = (stage_new - 1) * TEMP_STAGE_STEP +
-chip->thresh * TEMP_THRESH_STEP +
-TEMP_STAGE_HYSTERESIS + TEMP_THRESH_MIN;
+ chip->temp = qpnp_tm_decode_temp(chip, stage_new)
+ + TEMP_STAGE_HYSTERESIS;
} else if (stage_new < stage_old) {
/* decreasing stage, use upper bound */
- chip->temp = stage_new * TEMP_STAGE_STEP +
-chip->thresh * TEMP_THRESH_STEP -
-TEMP_STAGE_HYSTERESIS + TEMP_THRESH_MIN;
+ chip->temp = qpnp_tm_decode_temp(chip, stage_new + 1)
+ - TEMP_STAGE_HYSTERESIS;
}
chip->stage = stage;
@@ -199,26 +220,28 @@ static int qpnp_tm_get_temp(void *data, int *temp)
static int qpnp_tm_update_critical_trip_temp(struct qpnp_tm_chip *chip,
int temp)
{
- u8 reg;
+ long stage2_threshold_min = (*chip->temp_map)[THRESH_MIN][1];
+ long stage2_threshold_max = (*chip->temp_map)[THRESH_MAX][1];
bool disable_s2_shutdown = false;
+ u8 reg;
WARN_ON(!mutex_is_locked(>lock));
/*
* Default: S2 and S3 shutdown
