diff mbox series

[v5,09/20] thermal/drivers/tsens: Support using nvmem cells for calibration data

Message ID 20221229030106.3303205-10-dmitry.baryshkov@linaro.org (mailing list archive)
State Superseded
Headers show
Series thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually | expand

Commit Message

Dmitry Baryshkov Dec. 29, 2022, 3 a.m. UTC
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(-)

Comments

Konrad Dybcio Dec. 29, 2022, 10:47 a.m. UTC | #1
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);
Dmitry Baryshkov Dec. 29, 2022, 11:42 a.m. UTC | #2
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.

>
> 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);
Konrad Dybcio Dec. 29, 2022, 11:47 a.m. UTC | #3
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);
> 
> 
>
Dmitry Baryshkov Dec. 29, 2022, 11:52 a.m. UTC | #4
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);
> >
> >
> >
Konrad Dybcio Dec. 29, 2022, 12:23 p.m. UTC | #5
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 mbox series

Patch

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);