| Message ID | 20221229030106.3303205-10-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually | expand |
On 29.12.2022 04:00, Dmitry Baryshkov wrote: > Add a unified function using nvmem cells for parsing the calibration > data rather than parsing the calibration blob manually. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/thermal/qcom/tsens-v0_1.c | 15 ++++++ > drivers/thermal/qcom/tsens-v1.c | 11 ++++- > drivers/thermal/qcom/tsens.c | 76 +++++++++++++++++++++++++++++++ > drivers/thermal/qcom/tsens.h | 5 ++ > 4 files changed, 106 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c > index 579028ea48f4..6c9e491f9559 100644 > --- a/drivers/thermal/qcom/tsens-v0_1.c > +++ b/drivers/thermal/qcom/tsens-v0_1.c > @@ -229,6 +229,11 @@ static int calibrate_8916(struct tsens_priv *priv) > u32 p1[5], p2[5]; > int mode = 0; > u32 *qfprom_cdata, *qfprom_csel; > + int ret; > + > + ret = tsens_calibrate_nvmem(priv, 3); > + if (!ret) > + return 0; > > qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); > if (IS_ERR(qfprom_cdata)) > @@ -286,6 +291,11 @@ static int calibrate_8939(struct tsens_priv *priv) > int mode = 0; > u32 *qfprom_cdata; > u32 cdata[4]; > + int ret; > + > + ret = tsens_calibrate_common(priv); > + if (!ret) > + return 0; > > qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); > if (IS_ERR(qfprom_cdata)) > @@ -486,6 +496,11 @@ static int calibrate_9607(struct tsens_priv *priv) > u32 p1[5], p2[5]; > int mode = 0; > u32 *qfprom_cdata; > + int ret; > + > + ret = tsens_calibrate_common(priv); > + if (!ret) > + return 0; > > qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); > if (IS_ERR(qfprom_cdata)) > diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c > index 83c2853546d0..5bba75a845c5 100644 > --- a/drivers/thermal/qcom/tsens-v1.c > +++ b/drivers/thermal/qcom/tsens-v1.c > @@ -143,7 +143,11 @@ static int calibrate_v1(struct tsens_priv *priv) > u32 p1[10], p2[10]; > u32 mode = 0, lsb = 0, msb = 0; > u32 *qfprom_cdata; > - int i; > + int i, ret; > + > + ret = tsens_calibrate_common(priv); > + if (!ret) > + return 0; > > qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); > if (IS_ERR(qfprom_cdata)) > @@ -209,6 +213,11 @@ static int calibrate_8976(struct tsens_priv *priv) > u32 p1[11], p2[11]; > int mode = 0, tmp = 0; > u32 *qfprom_cdata; > + int ret; > + > + ret = tsens_calibrate_common(priv); > + if (!ret) > + return 0; > > qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); > if (IS_ERR(qfprom_cdata)) > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > index b191e19df93d..ce568a68de4a 100644 > --- a/drivers/thermal/qcom/tsens.c > +++ b/drivers/thermal/qcom/tsens.c > @@ -70,6 +70,82 @@ char *qfprom_read(struct device *dev, const char *cname) > return ret; > } > > +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift) > +{ > + u32 mode; > + u32 base1, base2; > + u32 p1[MAX_SENSORS], p2[MAX_SENSORS]; > + char name[] = "sXX_pY"; /* s10_p1 */ > + int i, ret; > + > + if (priv->num_sensors > MAX_SENSORS) > + return -EINVAL; > + > + ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode); > + if (ret == -ENOENT) > + dev_warn(priv->dev, "Please migrate to separate nvmem cells for calibration data\n"); > + if (ret < 0) > + return ret; > + > + dev_dbg(priv->dev, "calibration mode is %d\n", mode); > + > + ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1); > + if (ret < 0) > + return ret; > + > + ret = nvmem_cell_read_variable_le_u32(priv->dev, "base2", &base2); > + if (ret < 0) > + return ret; > + > + for (i = 0; i < priv->num_sensors; i++) { > + ret = snprintf(name, sizeof(name), "s%d_p1", priv->sensor[i].hw_id); I think you forgot to update the underscore to a hyphen here (unless the nvmem api does some magic internally). Konrad > + if (ret < 0) > + return ret; > + > + ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p1[i]); > + if (ret) > + return ret; > + > + ret = snprintf(name, sizeof(name), "s%d_p2", priv->sensor[i].hw_id); > + if (ret < 0) > + return ret; > + > + ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p2[i]); > + if (ret) > + return ret; > + } > + > + switch (mode) { > + case ONE_PT_CALIB: > + for (i = 0; i < priv->num_sensors; i++) > + p1[i] = p1[i] + (base1 << shift); > + break; > + case TWO_PT_CALIB: > + for (i = 0; i < priv->num_sensors; i++) > + p2[i] = (p2[i] + base2) << shift; > + fallthrough; > + case ONE_PT_CALIB2: > + for (i = 0; i < priv->num_sensors; i++) > + p1[i] = (p1[i] + base1) << shift; > + break; > + default: > + dev_dbg(priv->dev, "calibrationless mode\n"); > + for (i = 0; i < priv->num_sensors; i++) { > + p1[i] = 500; > + p2[i] = 780; > + } > + } > + > + compute_intercept_slope(priv, p1, p2, mode); > + > + return 0; > +} > + > +int tsens_calibrate_common(struct tsens_priv *priv) > +{ > + return tsens_calibrate_nvmem(priv, 2); > +} > + > /* > * Use this function on devices where slope and offset calculations > * depend on calibration data read from qfprom. On others the slope > diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h > index 7dd5fc246894..645ae02438fa 100644 > --- a/drivers/thermal/qcom/tsens.h > +++ b/drivers/thermal/qcom/tsens.h > @@ -6,6 +6,7 @@ > #ifndef __QCOM_TSENS_H__ > #define __QCOM_TSENS_H__ > > +#define NO_PT_CALIB 0x0 > #define ONE_PT_CALIB 0x1 > #define ONE_PT_CALIB2 0x2 > #define TWO_PT_CALIB 0x3 > @@ -17,6 +18,8 @@ > #define THRESHOLD_MAX_ADC_CODE 0x3ff > #define THRESHOLD_MIN_ADC_CODE 0x0 > > +#define MAX_SENSORS 16 > + > #include <linux/interrupt.h> > #include <linux/thermal.h> > #include <linux/regmap.h> > @@ -582,6 +585,8 @@ struct tsens_priv { > }; > > char *qfprom_read(struct device *dev, const char *cname); > +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift); > +int tsens_calibrate_common(struct tsens_priv *priv); > void compute_intercept_slope(struct tsens_priv *priv, u32 *pt1, u32 *pt2, u32 mode); > int init_common(struct tsens_priv *priv); > int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp);
On 29.12.2022 12:42, Dmitry Baryshkov wrote: > On Thu, 29 Dec 2022 at 12:47, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> >> >> On 29.12.2022 04:00, Dmitry Baryshkov wrote: >>> Add a unified function using nvmem cells for parsing the calibration >>> data rather than parsing the calibration blob manually. >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >>> drivers/thermal/qcom/tsens-v0_1.c | 15 ++++++ >>> drivers/thermal/qcom/tsens-v1.c | 11 ++++- >>> drivers/thermal/qcom/tsens.c | 76 +++++++++++++++++++++++++++++++ >>> drivers/thermal/qcom/tsens.h | 5 ++ >>> 4 files changed, 106 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c >>> index 579028ea48f4..6c9e491f9559 100644 >>> --- a/drivers/thermal/qcom/tsens-v0_1.c >>> +++ b/drivers/thermal/qcom/tsens-v0_1.c >>> @@ -229,6 +229,11 @@ static int calibrate_8916(struct tsens_priv *priv) >>> u32 p1[5], p2[5]; >>> int mode = 0; >>> u32 *qfprom_cdata, *qfprom_csel; >>> + int ret; >>> + >>> + ret = tsens_calibrate_nvmem(priv, 3); >>> + if (!ret) >>> + return 0; >>> >>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); >>> if (IS_ERR(qfprom_cdata)) >>> @@ -286,6 +291,11 @@ static int calibrate_8939(struct tsens_priv *priv) >>> int mode = 0; >>> u32 *qfprom_cdata; >>> u32 cdata[4]; >>> + int ret; >>> + >>> + ret = tsens_calibrate_common(priv); >>> + if (!ret) >>> + return 0; >>> >>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); >>> if (IS_ERR(qfprom_cdata)) >>> @@ -486,6 +496,11 @@ static int calibrate_9607(struct tsens_priv *priv) >>> u32 p1[5], p2[5]; >>> int mode = 0; >>> u32 *qfprom_cdata; >>> + int ret; >>> + >>> + ret = tsens_calibrate_common(priv); >>> + if (!ret) >>> + return 0; >>> >>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); >>> if (IS_ERR(qfprom_cdata)) >>> diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c >>> index 83c2853546d0..5bba75a845c5 100644 >>> --- a/drivers/thermal/qcom/tsens-v1.c >>> +++ b/drivers/thermal/qcom/tsens-v1.c >>> @@ -143,7 +143,11 @@ static int calibrate_v1(struct tsens_priv *priv) >>> u32 p1[10], p2[10]; >>> u32 mode = 0, lsb = 0, msb = 0; >>> u32 *qfprom_cdata; >>> - int i; >>> + int i, ret; >>> + >>> + ret = tsens_calibrate_common(priv); >>> + if (!ret) >>> + return 0; >>> >>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); >>> if (IS_ERR(qfprom_cdata)) >>> @@ -209,6 +213,11 @@ static int calibrate_8976(struct tsens_priv *priv) >>> u32 p1[11], p2[11]; >>> int mode = 0, tmp = 0; >>> u32 *qfprom_cdata; >>> + int ret; >>> + >>> + ret = tsens_calibrate_common(priv); >>> + if (!ret) >>> + return 0; >>> >>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); >>> if (IS_ERR(qfprom_cdata)) >>> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c >>> index b191e19df93d..ce568a68de4a 100644 >>> --- a/drivers/thermal/qcom/tsens.c >>> +++ b/drivers/thermal/qcom/tsens.c >>> @@ -70,6 +70,82 @@ char *qfprom_read(struct device *dev, const char *cname) >>> return ret; >>> } >>> >>> +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift) >>> +{ >>> + u32 mode; >>> + u32 base1, base2; >>> + u32 p1[MAX_SENSORS], p2[MAX_SENSORS]; >>> + char name[] = "sXX_pY"; /* s10_p1 */ >>> + int i, ret; >>> + >>> + if (priv->num_sensors > MAX_SENSORS) >>> + return -EINVAL; >>> + >>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode); >>> + if (ret == -ENOENT) >>> + dev_warn(priv->dev, "Please migrate to separate nvmem cells for calibration data\n"); >>> + if (ret < 0) >>> + return ret; >>> + >>> + dev_dbg(priv->dev, "calibration mode is %d\n", mode); >>> + >>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1); >>> + if (ret < 0) >>> + return ret; >>> + >>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, "base2", &base2); >>> + if (ret < 0) >>> + return ret; >>> + >>> + for (i = 0; i < priv->num_sensors; i++) { >>> + ret = snprintf(name, sizeof(name), "s%d_p1", priv->sensor[i].hw_id); >> I think you forgot to update the underscore to a hyphen here >> (unless the nvmem api does some magic internally). > > No. Please see the nvmem-cell-names property of the tsens nodes. It > uses underscores. Then OF code translates this sX_pY string into an > index in the nvmem-cells array or phandles. The sX-pY@ZZ node name is > not used during lookups at all. Right, I overlooked that! > >> >> Konrad >>> + if (ret < 0) >>> + return ret; >>> + >>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p1[i]); >>> + if (ret) >>> + return ret; >>> + >>> + ret = snprintf(name, sizeof(name), "s%d_p2", priv->sensor[i].hw_id); >>> + if (ret < 0) >>> + return ret; >>> + >>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p2[i]); >>> + if (ret) >>> + return ret; >>> + } >>> + >>> + switch (mode) { >>> + case ONE_PT_CALIB: >>> + for (i = 0; i < priv->num_sensors; i++) >>> + p1[i] = p1[i] + (base1 << shift); >>> + break; >>> + case TWO_PT_CALIB: >>> + for (i = 0; i < priv->num_sensors; i++) >>> + p2[i] = (p2[i] + base2) << shift; >>> + fallthrough; >>> + case ONE_PT_CALIB2: >>> + for (i = 0; i < priv->num_sensors; i++) >>> + p1[i] = (p1[i] + base1) << shift; >>> + break; >>> + default: >>> + dev_dbg(priv->dev, "calibrationless mode\n"); This could be a dev_warn, as we usually don't expect it to happen. Other than that: Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad >>> + for (i = 0; i < priv->num_sensors; i++) { >>> + p1[i] = 500; >>> + p2[i] = 780; >>> + } >>> + } >>> + >>> + compute_intercept_slope(priv, p1, p2, mode); >>> + >>> + return 0; >>> +} >>> + >>> +int tsens_calibrate_common(struct tsens_priv *priv) >>> +{ >>> + return tsens_calibrate_nvmem(priv, 2); >>> +} >>> + >>> /* >>> * Use this function on devices where slope and offset calculations >>> * depend on calibration data read from qfprom. On others the slope >>> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h >>> index 7dd5fc246894..645ae02438fa 100644 >>> --- a/drivers/thermal/qcom/tsens.h >>> +++ b/drivers/thermal/qcom/tsens.h >>> @@ -6,6 +6,7 @@ >>> #ifndef __QCOM_TSENS_H__ >>> #define __QCOM_TSENS_H__ >>> >>> +#define NO_PT_CALIB 0x0 >>> #define ONE_PT_CALIB 0x1 >>> #define ONE_PT_CALIB2 0x2 >>> #define TWO_PT_CALIB 0x3 >>> @@ -17,6 +18,8 @@ >>> #define THRESHOLD_MAX_ADC_CODE 0x3ff >>> #define THRESHOLD_MIN_ADC_CODE 0x0 >>> >>> +#define MAX_SENSORS 16 >>> + >>> #include <linux/interrupt.h> >>> #include <linux/thermal.h> >>> #include <linux/regmap.h> >>> @@ -582,6 +585,8 @@ struct tsens_priv { >>> }; >>> >>> char *qfprom_read(struct device *dev, const char *cname); >>> +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift); >>> +int tsens_calibrate_common(struct tsens_priv *priv); >>> void compute_intercept_slope(struct tsens_priv *priv, u32 *pt1, u32 *pt2, u32 mode); >>> int init_common(struct tsens_priv *priv); >>> int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp); > > >
On Thu, 29 Dec 2022 at 13:47, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > On 29.12.2022 12:42, Dmitry Baryshkov wrote: > > On Thu, 29 Dec 2022 at 12:47, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > >> On 29.12.2022 04:00, Dmitry Baryshkov wrote: > >>> Add a unified function using nvmem cells for parsing the calibration > >>> data rather than parsing the calibration blob manually. > >>> > >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >>> --- > >>> drivers/thermal/qcom/tsens-v0_1.c | 15 ++++++ > >>> drivers/thermal/qcom/tsens-v1.c | 11 ++++- > >>> drivers/thermal/qcom/tsens.c | 76 +++++++++++++++++++++++++++++++ > >>> drivers/thermal/qcom/tsens.h | 5 ++ > >>> 4 files changed, 106 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c > >>> index 579028ea48f4..6c9e491f9559 100644 > >>> --- a/drivers/thermal/qcom/tsens-v0_1.c > >>> +++ b/drivers/thermal/qcom/tsens-v0_1.c > >>> @@ -229,6 +229,11 @@ static int calibrate_8916(struct tsens_priv *priv) > >>> u32 p1[5], p2[5]; > >>> int mode = 0; > >>> u32 *qfprom_cdata, *qfprom_csel; > >>> + int ret; > >>> + > >>> + ret = tsens_calibrate_nvmem(priv, 3); > >>> + if (!ret) > >>> + return 0; > >>> > >>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); > >>> if (IS_ERR(qfprom_cdata)) > >>> @@ -286,6 +291,11 @@ static int calibrate_8939(struct tsens_priv *priv) > >>> int mode = 0; > >>> u32 *qfprom_cdata; > >>> u32 cdata[4]; > >>> + int ret; > >>> + > >>> + ret = tsens_calibrate_common(priv); > >>> + if (!ret) > >>> + return 0; > >>> > >>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); > >>> if (IS_ERR(qfprom_cdata)) > >>> @@ -486,6 +496,11 @@ static int calibrate_9607(struct tsens_priv *priv) > >>> u32 p1[5], p2[5]; > >>> int mode = 0; > >>> u32 *qfprom_cdata; > >>> + int ret; > >>> + > >>> + ret = tsens_calibrate_common(priv); > >>> + if (!ret) > >>> + return 0; > >>> > >>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); > >>> if (IS_ERR(qfprom_cdata)) > >>> diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c > >>> index 83c2853546d0..5bba75a845c5 100644 > >>> --- a/drivers/thermal/qcom/tsens-v1.c > >>> +++ b/drivers/thermal/qcom/tsens-v1.c > >>> @@ -143,7 +143,11 @@ static int calibrate_v1(struct tsens_priv *priv) > >>> u32 p1[10], p2[10]; > >>> u32 mode = 0, lsb = 0, msb = 0; > >>> u32 *qfprom_cdata; > >>> - int i; > >>> + int i, ret; > >>> + > >>> + ret = tsens_calibrate_common(priv); > >>> + if (!ret) > >>> + return 0; > >>> > >>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); > >>> if (IS_ERR(qfprom_cdata)) > >>> @@ -209,6 +213,11 @@ static int calibrate_8976(struct tsens_priv *priv) > >>> u32 p1[11], p2[11]; > >>> int mode = 0, tmp = 0; > >>> u32 *qfprom_cdata; > >>> + int ret; > >>> + > >>> + ret = tsens_calibrate_common(priv); > >>> + if (!ret) > >>> + return 0; > >>> > >>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); > >>> if (IS_ERR(qfprom_cdata)) > >>> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > >>> index b191e19df93d..ce568a68de4a 100644 > >>> --- a/drivers/thermal/qcom/tsens.c > >>> +++ b/drivers/thermal/qcom/tsens.c > >>> @@ -70,6 +70,82 @@ char *qfprom_read(struct device *dev, const char *cname) > >>> return ret; > >>> } > >>> > >>> +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift) > >>> +{ > >>> + u32 mode; > >>> + u32 base1, base2; > >>> + u32 p1[MAX_SENSORS], p2[MAX_SENSORS]; > >>> + char name[] = "sXX_pY"; /* s10_p1 */ > >>> + int i, ret; > >>> + > >>> + if (priv->num_sensors > MAX_SENSORS) > >>> + return -EINVAL; > >>> + > >>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode); > >>> + if (ret == -ENOENT) > >>> + dev_warn(priv->dev, "Please migrate to separate nvmem cells for calibration data\n"); > >>> + if (ret < 0) > >>> + return ret; > >>> + > >>> + dev_dbg(priv->dev, "calibration mode is %d\n", mode); > >>> + > >>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1); > >>> + if (ret < 0) > >>> + return ret; > >>> + > >>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, "base2", &base2); > >>> + if (ret < 0) > >>> + return ret; > >>> + > >>> + for (i = 0; i < priv->num_sensors; i++) { > >>> + ret = snprintf(name, sizeof(name), "s%d_p1", priv->sensor[i].hw_id); > >> I think you forgot to update the underscore to a hyphen here > >> (unless the nvmem api does some magic internally). > > > > No. Please see the nvmem-cell-names property of the tsens nodes. It > > uses underscores. Then OF code translates this sX_pY string into an > > index in the nvmem-cells array or phandles. The sX-pY@ZZ node name is > > not used during lookups at all. > Right, I overlooked that! > > > > >> > >> Konrad > >>> + if (ret < 0) > >>> + return ret; > >>> + > >>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p1[i]); > >>> + if (ret) > >>> + return ret; > >>> + > >>> + ret = snprintf(name, sizeof(name), "s%d_p2", priv->sensor[i].hw_id); > >>> + if (ret < 0) > >>> + return ret; > >>> + > >>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p2[i]); > >>> + if (ret) > >>> + return ret; > >>> + } > >>> + > >>> + switch (mode) { > >>> + case ONE_PT_CALIB: > >>> + for (i = 0; i < priv->num_sensors; i++) > >>> + p1[i] = p1[i] + (base1 << shift); > >>> + break; > >>> + case TWO_PT_CALIB: > >>> + for (i = 0; i < priv->num_sensors; i++) > >>> + p2[i] = (p2[i] + base2) << shift; > >>> + fallthrough; > >>> + case ONE_PT_CALIB2: > >>> + for (i = 0; i < priv->num_sensors; i++) > >>> + p1[i] = (p1[i] + base1) << shift; > >>> + break; > >>> + default: > >>> + dev_dbg(priv->dev, "calibrationless mode\n"); > This could be a dev_warn, as we usually don't expect it to happen. I'm not so sure here. Current code handles this case without any warnings, as one of the expected cases So, I don't think the rework should start emitting warnings. > Other than that: > > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > Konrad > >>> + for (i = 0; i < priv->num_sensors; i++) { > >>> + p1[i] = 500; > >>> + p2[i] = 780; > >>> + } > >>> + } > >>> + > >>> + compute_intercept_slope(priv, p1, p2, mode); > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +int tsens_calibrate_common(struct tsens_priv *priv) > >>> +{ > >>> + return tsens_calibrate_nvmem(priv, 2); > >>> +} > >>> + > >>> /* > >>> * Use this function on devices where slope and offset calculations > >>> * depend on calibration data read from qfprom. On others the slope > >>> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h > >>> index 7dd5fc246894..645ae02438fa 100644 > >>> --- a/drivers/thermal/qcom/tsens.h > >>> +++ b/drivers/thermal/qcom/tsens.h > >>> @@ -6,6 +6,7 @@ > >>> #ifndef __QCOM_TSENS_H__ > >>> #define __QCOM_TSENS_H__ > >>> > >>> +#define NO_PT_CALIB 0x0 > >>> #define ONE_PT_CALIB 0x1 > >>> #define ONE_PT_CALIB2 0x2 > >>> #define TWO_PT_CALIB 0x3 > >>> @@ -17,6 +18,8 @@ > >>> #define THRESHOLD_MAX_ADC_CODE 0x3ff > >>> #define THRESHOLD_MIN_ADC_CODE 0x0 > >>> > >>> +#define MAX_SENSORS 16 > >>> + > >>> #include <linux/interrupt.h> > >>> #include <linux/thermal.h> > >>> #include <linux/regmap.h> > >>> @@ -582,6 +585,8 @@ struct tsens_priv { > >>> }; > >>> > >>> char *qfprom_read(struct device *dev, const char *cname); > >>> +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift); > >>> +int tsens_calibrate_common(struct tsens_priv *priv); > >>> void compute_intercept_slope(struct tsens_priv *priv, u32 *pt1, u32 *pt2, u32 mode); > >>> int init_common(struct tsens_priv *priv); > >>> int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp); > > > > > >
On 29.12.2022 12:52, Dmitry Baryshkov wrote: > On Thu, 29 Dec 2022 at 13:47, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> On 29.12.2022 12:42, Dmitry Baryshkov wrote: >>> On Thu, 29 Dec 2022 at 12:47, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >>>> On 29.12.2022 04:00, Dmitry Baryshkov wrote: >>>>> Add a unified function using nvmem cells for parsing the calibration >>>>> data rather than parsing the calibration blob manually. >>>>> >>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>>> --- >>>>> drivers/thermal/qcom/tsens-v0_1.c | 15 ++++++ >>>>> drivers/thermal/qcom/tsens-v1.c | 11 ++++- >>>>> drivers/thermal/qcom/tsens.c | 76 +++++++++++++++++++++++++++++++ >>>>> drivers/thermal/qcom/tsens.h | 5 ++ >>>>> 4 files changed, 106 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c >>>>> index 579028ea48f4..6c9e491f9559 100644 >>>>> --- a/drivers/thermal/qcom/tsens-v0_1.c >>>>> +++ b/drivers/thermal/qcom/tsens-v0_1.c >>>>> @@ -229,6 +229,11 @@ static int calibrate_8916(struct tsens_priv *priv) >>>>> u32 p1[5], p2[5]; >>>>> int mode = 0; >>>>> u32 *qfprom_cdata, *qfprom_csel; >>>>> + int ret; >>>>> + >>>>> + ret = tsens_calibrate_nvmem(priv, 3); >>>>> + if (!ret) >>>>> + return 0; >>>>> >>>>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); >>>>> if (IS_ERR(qfprom_cdata)) >>>>> @@ -286,6 +291,11 @@ static int calibrate_8939(struct tsens_priv *priv) >>>>> int mode = 0; >>>>> u32 *qfprom_cdata; >>>>> u32 cdata[4]; >>>>> + int ret; >>>>> + >>>>> + ret = tsens_calibrate_common(priv); >>>>> + if (!ret) >>>>> + return 0; >>>>> >>>>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); >>>>> if (IS_ERR(qfprom_cdata)) >>>>> @@ -486,6 +496,11 @@ static int calibrate_9607(struct tsens_priv *priv) >>>>> u32 p1[5], p2[5]; >>>>> int mode = 0; >>>>> u32 *qfprom_cdata; >>>>> + int ret; >>>>> + >>>>> + ret = tsens_calibrate_common(priv); >>>>> + if (!ret) >>>>> + return 0; >>>>> >>>>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); >>>>> if (IS_ERR(qfprom_cdata)) >>>>> diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c >>>>> index 83c2853546d0..5bba75a845c5 100644 >>>>> --- a/drivers/thermal/qcom/tsens-v1.c >>>>> +++ b/drivers/thermal/qcom/tsens-v1.c >>>>> @@ -143,7 +143,11 @@ static int calibrate_v1(struct tsens_priv *priv) >>>>> u32 p1[10], p2[10]; >>>>> u32 mode = 0, lsb = 0, msb = 0; >>>>> u32 *qfprom_cdata; >>>>> - int i; >>>>> + int i, ret; >>>>> + >>>>> + ret = tsens_calibrate_common(priv); >>>>> + if (!ret) >>>>> + return 0; >>>>> >>>>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); >>>>> if (IS_ERR(qfprom_cdata)) >>>>> @@ -209,6 +213,11 @@ static int calibrate_8976(struct tsens_priv *priv) >>>>> u32 p1[11], p2[11]; >>>>> int mode = 0, tmp = 0; >>>>> u32 *qfprom_cdata; >>>>> + int ret; >>>>> + >>>>> + ret = tsens_calibrate_common(priv); >>>>> + if (!ret) >>>>> + return 0; >>>>> >>>>> qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); >>>>> if (IS_ERR(qfprom_cdata)) >>>>> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c >>>>> index b191e19df93d..ce568a68de4a 100644 >>>>> --- a/drivers/thermal/qcom/tsens.c >>>>> +++ b/drivers/thermal/qcom/tsens.c >>>>> @@ -70,6 +70,82 @@ char *qfprom_read(struct device *dev, const char *cname) >>>>> return ret; >>>>> } >>>>> >>>>> +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift) >>>>> +{ >>>>> + u32 mode; >>>>> + u32 base1, base2; >>>>> + u32 p1[MAX_SENSORS], p2[MAX_SENSORS]; >>>>> + char name[] = "sXX_pY"; /* s10_p1 */ >>>>> + int i, ret; >>>>> + >>>>> + if (priv->num_sensors > MAX_SENSORS) >>>>> + return -EINVAL; >>>>> + >>>>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode); >>>>> + if (ret == -ENOENT) >>>>> + dev_warn(priv->dev, "Please migrate to separate nvmem cells for calibration data\n"); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + dev_dbg(priv->dev, "calibration mode is %d\n", mode); >>>>> + >>>>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, "base2", &base2); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + for (i = 0; i < priv->num_sensors; i++) { >>>>> + ret = snprintf(name, sizeof(name), "s%d_p1", priv->sensor[i].hw_id); >>>> I think you forgot to update the underscore to a hyphen here >>>> (unless the nvmem api does some magic internally). >>> >>> No. Please see the nvmem-cell-names property of the tsens nodes. It >>> uses underscores. Then OF code translates this sX_pY string into an >>> index in the nvmem-cells array or phandles. The sX-pY@ZZ node name is >>> not used during lookups at all. >> Right, I overlooked that! >> >>> >>>> >>>> Konrad >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p1[i]); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + ret = snprintf(name, sizeof(name), "s%d_p2", priv->sensor[i].hw_id); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p2[i]); >>>>> + if (ret) >>>>> + return ret; >>>>> + } >>>>> + >>>>> + switch (mode) { >>>>> + case ONE_PT_CALIB: >>>>> + for (i = 0; i < priv->num_sensors; i++) >>>>> + p1[i] = p1[i] + (base1 << shift); >>>>> + break; >>>>> + case TWO_PT_CALIB: >>>>> + for (i = 0; i < priv->num_sensors; i++) >>>>> + p2[i] = (p2[i] + base2) << shift; >>>>> + fallthrough; >>>>> + case ONE_PT_CALIB2: >>>>> + for (i = 0; i < priv->num_sensors; i++) >>>>> + p1[i] = (p1[i] + base1) << shift; >>>>> + break; >>>>> + default: >>>>> + dev_dbg(priv->dev, "calibrationless mode\n"); >> This could be a dev_warn, as we usually don't expect it to happen. > > I'm not so sure here. Current code handles this case without any > warnings, as one of the expected cases So, I don't think the rework > should start emitting warnings. Right, perhaps that's something to consider at a different time. Konrad > >> Other than that: >> >> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> >> Konrad >>>>> + for (i = 0; i < priv->num_sensors; i++) { >>>>> + p1[i] = 500; >>>>> + p2[i] = 780; >>>>> + } >>>>> + } >>>>> + >>>>> + compute_intercept_slope(priv, p1, p2, mode); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int tsens_calibrate_common(struct tsens_priv *priv) >>>>> +{ >>>>> + return tsens_calibrate_nvmem(priv, 2); >>>>> +} >>>>> + >>>>> /* >>>>> * Use this function on devices where slope and offset calculations >>>>> * depend on calibration data read from qfprom. On others the slope >>>>> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h >>>>> index 7dd5fc246894..645ae02438fa 100644 >>>>> --- a/drivers/thermal/qcom/tsens.h >>>>> +++ b/drivers/thermal/qcom/tsens.h >>>>> @@ -6,6 +6,7 @@ >>>>> #ifndef __QCOM_TSENS_H__ >>>>> #define __QCOM_TSENS_H__ >>>>> >>>>> +#define NO_PT_CALIB 0x0 >>>>> #define ONE_PT_CALIB 0x1 >>>>> #define ONE_PT_CALIB2 0x2 >>>>> #define TWO_PT_CALIB 0x3 >>>>> @@ -17,6 +18,8 @@ >>>>> #define THRESHOLD_MAX_ADC_CODE 0x3ff >>>>> #define THRESHOLD_MIN_ADC_CODE 0x0 >>>>> >>>>> +#define MAX_SENSORS 16 >>>>> + >>>>> #include <linux/interrupt.h> >>>>> #include <linux/thermal.h> >>>>> #include <linux/regmap.h> >>>>> @@ -582,6 +585,8 @@ struct tsens_priv { >>>>> }; >>>>> >>>>> char *qfprom_read(struct device *dev, const char *cname); >>>>> +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift); >>>>> +int tsens_calibrate_common(struct tsens_priv *priv); >>>>> void compute_intercept_slope(struct tsens_priv *priv, u32 *pt1, u32 *pt2, u32 mode); >>>>> int init_common(struct tsens_priv *priv); >>>>> int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp); >>> >>> >>> > > >
diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c index 579028ea48f4..6c9e491f9559 100644 --- a/drivers/thermal/qcom/tsens-v0_1.c +++ b/drivers/thermal/qcom/tsens-v0_1.c @@ -229,6 +229,11 @@ static int calibrate_8916(struct tsens_priv *priv) u32 p1[5], p2[5]; int mode = 0; u32 *qfprom_cdata, *qfprom_csel; + int ret; + + ret = tsens_calibrate_nvmem(priv, 3); + if (!ret) + return 0; qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); if (IS_ERR(qfprom_cdata)) @@ -286,6 +291,11 @@ static int calibrate_8939(struct tsens_priv *priv) int mode = 0; u32 *qfprom_cdata; u32 cdata[4]; + int ret; + + ret = tsens_calibrate_common(priv); + if (!ret) + return 0; qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); if (IS_ERR(qfprom_cdata)) @@ -486,6 +496,11 @@ static int calibrate_9607(struct tsens_priv *priv) u32 p1[5], p2[5]; int mode = 0; u32 *qfprom_cdata; + int ret; + + ret = tsens_calibrate_common(priv); + if (!ret) + return 0; qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); if (IS_ERR(qfprom_cdata)) diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c index 83c2853546d0..5bba75a845c5 100644 --- a/drivers/thermal/qcom/tsens-v1.c +++ b/drivers/thermal/qcom/tsens-v1.c @@ -143,7 +143,11 @@ static int calibrate_v1(struct tsens_priv *priv) u32 p1[10], p2[10]; u32 mode = 0, lsb = 0, msb = 0; u32 *qfprom_cdata; - int i; + int i, ret; + + ret = tsens_calibrate_common(priv); + if (!ret) + return 0; qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); if (IS_ERR(qfprom_cdata)) @@ -209,6 +213,11 @@ static int calibrate_8976(struct tsens_priv *priv) u32 p1[11], p2[11]; int mode = 0, tmp = 0; u32 *qfprom_cdata; + int ret; + + ret = tsens_calibrate_common(priv); + if (!ret) + return 0; qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib"); if (IS_ERR(qfprom_cdata)) diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c index b191e19df93d..ce568a68de4a 100644 --- a/drivers/thermal/qcom/tsens.c +++ b/drivers/thermal/qcom/tsens.c @@ -70,6 +70,82 @@ char *qfprom_read(struct device *dev, const char *cname) return ret; } +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift) +{ + u32 mode; + u32 base1, base2; + u32 p1[MAX_SENSORS], p2[MAX_SENSORS]; + char name[] = "sXX_pY"; /* s10_p1 */ + int i, ret; + + if (priv->num_sensors > MAX_SENSORS) + return -EINVAL; + + ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode); + if (ret == -ENOENT) + dev_warn(priv->dev, "Please migrate to separate nvmem cells for calibration data\n"); + if (ret < 0) + return ret; + + dev_dbg(priv->dev, "calibration mode is %d\n", mode); + + ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1); + if (ret < 0) + return ret; + + ret = nvmem_cell_read_variable_le_u32(priv->dev, "base2", &base2); + if (ret < 0) + return ret; + + for (i = 0; i < priv->num_sensors; i++) { + ret = snprintf(name, sizeof(name), "s%d_p1", priv->sensor[i].hw_id); + if (ret < 0) + return ret; + + ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p1[i]); + if (ret) + return ret; + + ret = snprintf(name, sizeof(name), "s%d_p2", priv->sensor[i].hw_id); + if (ret < 0) + return ret; + + ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p2[i]); + if (ret) + return ret; + } + + switch (mode) { + case ONE_PT_CALIB: + for (i = 0; i < priv->num_sensors; i++) + p1[i] = p1[i] + (base1 << shift); + break; + case TWO_PT_CALIB: + for (i = 0; i < priv->num_sensors; i++) + p2[i] = (p2[i] + base2) << shift; + fallthrough; + case ONE_PT_CALIB2: + for (i = 0; i < priv->num_sensors; i++) + p1[i] = (p1[i] + base1) << shift; + break; + default: + dev_dbg(priv->dev, "calibrationless mode\n"); + for (i = 0; i < priv->num_sensors; i++) { + p1[i] = 500; + p2[i] = 780; + } + } + + compute_intercept_slope(priv, p1, p2, mode); + + return 0; +} + +int tsens_calibrate_common(struct tsens_priv *priv) +{ + return tsens_calibrate_nvmem(priv, 2); +} + /* * Use this function on devices where slope and offset calculations * depend on calibration data read from qfprom. On others the slope diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h index 7dd5fc246894..645ae02438fa 100644 --- a/drivers/thermal/qcom/tsens.h +++ b/drivers/thermal/qcom/tsens.h @@ -6,6 +6,7 @@ #ifndef __QCOM_TSENS_H__ #define __QCOM_TSENS_H__ +#define NO_PT_CALIB 0x0 #define ONE_PT_CALIB 0x1 #define ONE_PT_CALIB2 0x2 #define TWO_PT_CALIB 0x3 @@ -17,6 +18,8 @@ #define THRESHOLD_MAX_ADC_CODE 0x3ff #define THRESHOLD_MIN_ADC_CODE 0x0 +#define MAX_SENSORS 16 + #include <linux/interrupt.h> #include <linux/thermal.h> #include <linux/regmap.h> @@ -582,6 +585,8 @@ struct tsens_priv { }; char *qfprom_read(struct device *dev, const char *cname); +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift); +int tsens_calibrate_common(struct tsens_priv *priv); void compute_intercept_slope(struct tsens_priv *priv, u32 *pt1, u32 *pt2, u32 mode); int init_common(struct tsens_priv *priv); int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp);
Add a unified function using nvmem cells for parsing the calibration data rather than parsing the calibration blob manually. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/thermal/qcom/tsens-v0_1.c | 15 ++++++ drivers/thermal/qcom/tsens-v1.c | 11 ++++- drivers/thermal/qcom/tsens.c | 76 +++++++++++++++++++++++++++++++ drivers/thermal/qcom/tsens.h | 5 ++ 4 files changed, 106 insertions(+), 1 deletion(-)