linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data
@ 2023-04-06 14:58 Bryan O'Donoghue
  2023-04-06 14:58 ` [PATCH 1/3] thermal/drivers/tsens: Add error/debug prints to calibration read Bryan O'Donoghue
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Bryan O'Donoghue @ 2023-04-06 14:58 UTC (permalink / raw)
  To: amitk, thara.gopinath, agross, andersson, konrad.dybcio, rafael,
	daniel.lezcano, rui.zhang, dmitry.baryshkov
  Cc: linux-pm, linux-arm-msm, linux-kernel, Bryan O'Donoghue

On MSM8939 the last sensor has calibration data that cannot be extracted in
one big read.

Rather than have a lot of MSM8939 specific code this series makes a generic
modification to allow any other calibration data that is non-contiguous to
be extracted and recovered.

For example s9-p2 takes bits 1-5 from @4b and bit 13 from @4d. The bit from
bit13 then becomes the sixth bit in the calibration data.

tsens_s9_p2: s9-p2@4b {
    reg = <0x4b 0x1>;
    bits = <1 5>;
};

tsens_s9_p2_msb: s9-p2-msb@4d {
    reg = <0x4d 0x1>;
    bits = <13 1>;
};

A register desciptor is introduced in the driver which takes the place of
the previous unsigned int hw_ids array in struct tsens_plat_data.

This new structure contains the previous hardware id and two variables
p1_shift and p2_shift.

If p1_shift or p2_shift is non-zero then this tells
tsens_read_calibration() to search for sX-pY-msb where msb means "most
significant bits".

The value at p1_shift/p2_shift is then used to right shift the value read
from sX-pY-msb and or that value into the base value from sX-pY.

The nvmem 'bits' field provides the mask.

Bryan O'Donoghue (3):
  thermal/drivers/tsens: Add error/debug prints to calibration read
  thermal/drivers/tsens: Describe sensor registers via a structure
  thermal/drivers/tsens: Extract and shift-in optional MSB

 drivers/thermal/qcom/tsens-v0_1.c | 56 +++++++++++++++++++++++++++++--
 drivers/thermal/qcom/tsens.c      | 50 ++++++++++++++++++++++++---
 drivers/thermal/qcom/tsens.h      | 16 ++++++++-
 3 files changed, 115 insertions(+), 7 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] thermal/drivers/tsens: Add error/debug prints to calibration read
  2023-04-06 14:58 [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data Bryan O'Donoghue
@ 2023-04-06 14:58 ` Bryan O'Donoghue
  2023-04-06 17:36   ` Dmitry Baryshkov
  2023-04-06 14:58 ` [PATCH 2/3] thermal/drivers/tsens: Describe sensor registers via a structure Bryan O'Donoghue
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Bryan O'Donoghue @ 2023-04-06 14:58 UTC (permalink / raw)
  To: amitk, thara.gopinath, agross, andersson, konrad.dybcio, rafael,
	daniel.lezcano, rui.zhang, dmitry.baryshkov
  Cc: linux-pm, linux-arm-msm, linux-kernel, Bryan O'Donoghue

Add in some dev_dbg() to aid in viewing of raw calibration data extracted.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/thermal/qcom/tsens.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index d3218127e617d..7165b0bfe8b9f 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -115,8 +115,12 @@ int tsens_read_calibration(struct tsens_priv *priv, int shift, u32 *p1, u32 *p2,
 			return ret;
 
 		ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p1[i]);
-		if (ret)
+		if (ret) {
+			dev_err(priv->dev, "Failed to read %s\n", name);
 			return ret;
+		}
+
+		dev_dbg(priv->dev, "%s 0x%x\n", name, p1[i]);
 
 		ret = snprintf(name, sizeof(name), "s%d_p2%s", priv->sensor[i].hw_id,
 			       backup ? "_backup" : "");
@@ -124,8 +128,12 @@ int tsens_read_calibration(struct tsens_priv *priv, int shift, u32 *p1, u32 *p2,
 			return ret;
 
 		ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p2[i]);
-		if (ret)
+		if (ret) {
+			dev_err(priv->dev, "Failed to read %s\n", name);
 			return ret;
+		}
+
+		dev_dbg(priv->dev, "%s 0x%x\n", name, p2[i]);
 	}
 
 	switch (mode) {
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] thermal/drivers/tsens: Describe sensor registers via a structure
  2023-04-06 14:58 [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data Bryan O'Donoghue
  2023-04-06 14:58 ` [PATCH 1/3] thermal/drivers/tsens: Add error/debug prints to calibration read Bryan O'Donoghue
@ 2023-04-06 14:58 ` Bryan O'Donoghue
  2023-04-06 17:43   ` Dmitry Baryshkov
  2023-04-06 14:58 ` [PATCH 3/3] thermal/drivers/tsens: Extract and shift-in optional MSB Bryan O'Donoghue
  2023-04-06 16:20 ` [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data Stephan Gerhold
  3 siblings, 1 reply; 9+ messages in thread
From: Bryan O'Donoghue @ 2023-04-06 14:58 UTC (permalink / raw)
  To: amitk, thara.gopinath, agross, andersson, konrad.dybcio, rafael,
	daniel.lezcano, rui.zhang, dmitry.baryshkov
  Cc: linux-pm, linux-arm-msm, linux-kernel, Bryan O'Donoghue

Define sensor identifiers and optional shifts in a single data-structure.
This facilitates extraction of calibration data from non-contiguous
addresses.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/thermal/qcom/tsens-v0_1.c | 56 +++++++++++++++++++++++++++++--
 drivers/thermal/qcom/tsens.c      |  5 +--
 drivers/thermal/qcom/tsens.h      | 16 ++++++++-
 3 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c
index e89c6f39a3aea..c20c002d98650 100644
--- a/drivers/thermal/qcom/tsens-v0_1.c
+++ b/drivers/thermal/qcom/tsens-v0_1.c
@@ -319,10 +319,31 @@ static const struct tsens_ops ops_8916 = {
 	.get_temp	= get_temp_common,
 };
 
+struct tsens_reg_data reg_8916[] = {
+	{
+		.id = 0,
+	},
+	{
+		.id = 1,
+	},
+	{
+		.id = 2,
+	},
+	{
+		.id = 3,
+	},
+	{
+		.id = 4,
+	},
+	{
+		.id = 5,
+	},
+};
+
 struct tsens_plat_data data_8916 = {
 	.num_sensors	= 5,
 	.ops		= &ops_8916,
-	.hw_ids		= (unsigned int []){0, 1, 2, 4, 5 },
+	.reg		= reg_8916,
 
 	.feat		= &tsens_v0_1_feat,
 	.fields	= tsens_v0_1_regfields,
@@ -334,10 +355,41 @@ static const struct tsens_ops ops_8939 = {
 	.get_temp	= get_temp_common,
 };
 
+struct tsens_reg_data reg_8939[] = {
+	{
+		.id = 0,
+	},
+	{
+		.id = 1,
+	},
+	{
+		.id = 2,
+	},
+	{
+		.id = 3,
+	},
+	{
+		.id = 5,
+	},
+	{
+		.id = 6,
+	},
+	{
+		.id = 7,
+	},
+	{
+		.id = 8,
+	},
+	{
+		.id = 9,
+		.p2_shift = 8,
+	},
+};
+
 struct tsens_plat_data data_8939 = {
 	.num_sensors	= 9,
 	.ops		= &ops_8939,
-	.hw_ids		= (unsigned int []){ 0, 1, 2, 3, 5, 6, 7, 8, 9, /* 10 */ },
+	.reg		= reg_8939,
 
 	.feat		= &tsens_v0_1_feat,
 	.fields	= tsens_v0_1_regfields,
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 7165b0bfe8b9f..a260f563b4889 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -1274,13 +1274,14 @@ static int tsens_probe(struct platform_device *pdev)
 	priv->num_sensors = num_sensors;
 	priv->ops = data->ops;
 	for (i = 0;  i < priv->num_sensors; i++) {
-		if (data->hw_ids)
-			priv->sensor[i].hw_id = data->hw_ids[i];
+		if (data->reg)
+			priv->sensor[i].hw_id = data->reg[i].id;
 		else
 			priv->sensor[i].hw_id = i;
 	}
 	priv->feat = data->feat;
 	priv->fields = data->fields;
+	priv->reg = data->reg;
 
 	platform_set_drvdata(pdev, priv);
 
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index dba9cd38f637c..31f67da03bce6 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -517,18 +517,31 @@ struct tsens_features {
 	int trip_max_temp;
 };
 
+/**
+ * struct tsens_reg_data - describes register data retrieved non-contiguously
+ * @id: thermal sensor identifier
+ * @p1_shift: When non-zero is the # of bits to right shift p1 MSB by
+ * @p2_shift: When non-zero is the # of bits to right shift p2 MSB by
+ */
+struct tsens_reg_data {
+	unsigned int id;
+	unsigned int p1_shift;
+	unsigned int p2_shift;
+};
+
 /**
  * struct tsens_plat_data - tsens compile-time platform data
  * @num_sensors: Number of sensors supported by platform
  * @ops: operations the tsens instance supports
  * @hw_ids: Subset of sensors ids supported by platform, if not the first n
+ * @reg: Describe sensor id and calibration shifts
  * @feat: features of the IP
  * @fields: bitfield locations
  */
 struct tsens_plat_data {
 	const u32		num_sensors;
 	const struct tsens_ops	*ops;
-	unsigned int		*hw_ids;
+	struct tsens_reg_data	*reg;
 	struct tsens_features	*feat;
 	const struct reg_field		*fields;
 };
@@ -575,6 +588,7 @@ struct tsens_priv {
 	struct regmap_field		*rf[MAX_REGFIELDS];
 	struct tsens_context		ctx;
 	struct tsens_features		*feat;
+	struct tsens_reg_data		*reg;
 	const struct reg_field		*fields;
 	const struct tsens_ops		*ops;
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] thermal/drivers/tsens: Extract and shift-in optional MSB
  2023-04-06 14:58 [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data Bryan O'Donoghue
  2023-04-06 14:58 ` [PATCH 1/3] thermal/drivers/tsens: Add error/debug prints to calibration read Bryan O'Donoghue
  2023-04-06 14:58 ` [PATCH 2/3] thermal/drivers/tsens: Describe sensor registers via a structure Bryan O'Donoghue
@ 2023-04-06 14:58 ` Bryan O'Donoghue
  2023-04-06 17:47   ` Dmitry Baryshkov
  2023-04-06 16:20 ` [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data Stephan Gerhold
  3 siblings, 1 reply; 9+ messages in thread
From: Bryan O'Donoghue @ 2023-04-06 14:58 UTC (permalink / raw)
  To: amitk, thara.gopinath, agross, andersson, konrad.dybcio, rafael,
	daniel.lezcano, rui.zhang, dmitry.baryshkov
  Cc: linux-pm, linux-arm-msm, linux-kernel, Bryan O'Donoghue

In msm8939 some of the sensor calibration data traverses byte boundaries.
Two examples of this are thermal sensor 2 point 1 and sensor 9 point 2.

For sensor 2 point 1 we can get away with a simple read traversing byte
boundaries as the calibration most significant bits are adjacent to the
least significant across the byte boundary.

In this case a read starting at the end of the first byte for nine bits
will deliver up the data we want.

In the case of sensor 9 point 2 however, the most significant bits are not
adjacent and so therefore we need to perform two reads and or the bits
together.

If reg.p1_shift or reg.p2_shift is set then automatically search for
pX_sY_msb in the dts applying pX_shift as a right shift or into the pX_sY
value.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/thermal/qcom/tsens.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index a260f563b4889..eff2c8671c343 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -74,6 +74,7 @@ int tsens_read_calibration(struct tsens_priv *priv, int shift, u32 *p1, u32 *p2,
 {
 	u32 mode;
 	u32 base1, base2;
+	u32 msb;
 	char name[] = "sXX_pY_backup"; /* s10_p1_backup */
 	int i, ret;
 
@@ -122,6 +123,22 @@ int tsens_read_calibration(struct tsens_priv *priv, int shift, u32 *p1, u32 *p2,
 
 		dev_dbg(priv->dev, "%s 0x%x\n", name, p1[i]);
 
+		if (priv->reg && priv->reg[i].p1_shift) {
+			ret = snprintf(name, sizeof(name), "s%d_p1_msb",
+				       priv->sensor[i].hw_id);
+			if (ret < 0)
+				return ret;
+
+			ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &msb);
+			if (ret) {
+				dev_err(priv->dev, "Failed to read %s\n", name);
+				return ret;
+			}
+
+			dev_dbg(priv->dev, "%s 0x%x\n", name, msb);
+			p1[i] |= msb >> priv->reg[i].p1_shift;
+		}
+
 		ret = snprintf(name, sizeof(name), "s%d_p2%s", priv->sensor[i].hw_id,
 			       backup ? "_backup" : "");
 		if (ret < 0)
@@ -134,6 +151,22 @@ int tsens_read_calibration(struct tsens_priv *priv, int shift, u32 *p1, u32 *p2,
 		}
 
 		dev_dbg(priv->dev, "%s 0x%x\n", name, p2[i]);
+
+		if (priv->reg && priv->reg[i].p2_shift) {
+			ret = snprintf(name, sizeof(name), "s%d_p2_msb",
+				       priv->sensor[i].hw_id);
+			if (ret < 0)
+				return ret;
+
+			ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &msb);
+			if (ret) {
+				dev_err(priv->dev, "Failed to read %s\n", name);
+				return ret;
+			}
+
+			dev_dbg(priv->dev, "%s 0x%x\n", name, msb);
+			p2[i] |= msb >> priv->reg[i].p2_shift;
+		}
 	}
 
 	switch (mode) {
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data
  2023-04-06 14:58 [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data Bryan O'Donoghue
                   ` (2 preceding siblings ...)
  2023-04-06 14:58 ` [PATCH 3/3] thermal/drivers/tsens: Extract and shift-in optional MSB Bryan O'Donoghue
@ 2023-04-06 16:20 ` Stephan Gerhold
  2023-04-06 22:36   ` Bryan O'Donoghue
  3 siblings, 1 reply; 9+ messages in thread
From: Stephan Gerhold @ 2023-04-06 16:20 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: amitk, thara.gopinath, agross, andersson, konrad.dybcio, rafael,
	daniel.lezcano, rui.zhang, dmitry.baryshkov, linux-pm,
	linux-arm-msm, linux-kernel

On Thu, Apr 06, 2023 at 03:58:47PM +0100, Bryan O'Donoghue wrote:
> On MSM8939 the last sensor has calibration data that cannot be extracted in
> one big read.
> 
> Rather than have a lot of MSM8939 specific code this series makes a generic
> modification to allow any other calibration data that is non-contiguous to
> be extracted and recovered.
> 
> For example s9-p2 takes bits 1-5 from @4b and bit 13 from @4d. The bit from
> bit13 then becomes the sixth bit in the calibration data.
> 
> tsens_s9_p2: s9-p2@4b {
>     reg = <0x4b 0x1>;
>     bits = <1 5>;
> };
> 
> tsens_s9_p2_msb: s9-p2-msb@4d {
>     reg = <0x4d 0x1>;
>     bits = <13 1>;
> };

As far as I can tell the sensor with the non-contiguous calibration data
is the one with hwid=10, so do you mean s10-p2 instead of s9-p2 here?

It's easy to mix up the numbering: Since hwid=4 is missing for MSM8939,
the sensor 9 in the calibration code downstream (TSENS9_8939_POINT*)
actually refers to hwid=10. hwid=9 is sensor 8 in the calibration code
(TSENS8_8939_POINT*).

Sensor hwid=10 was disabled for MSM8939 in the tsens driver because it
seems unused, only exists on MSM8939 v3.0, and specifically to avoid
having to handle this non-contiguous calibration data, see commit
903238a33c11 ("thermal/drivers/tsens: limit num_sensors to 9 for msm8939"):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=903238a33c116edf5f64f7a3fd246e6169cccfa6

> On msm8939 last (hwid=10) sensor was added in the hw revision 3.0.
> Calibration data for it was placed outside of the main calibration  
> data blob, so it is not accessible by the current blob-parsing code.
>
> Moreover data for the sensor's p2 is not contiguous in the fuses. This
> makes it hard to use nvmem_cell API to parse calibration data in a
> generic way.
>
> Since the sensor doesn't seem to be actually used by the existing
> hardware, disable the sensor for now.
>
> Fixes: 332bc8ebab2c ("thermal: qcom: tsens-v0_1: Add support for MSM8939")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

So with sensor hwid=10 disabled, I think this patch series is actually
not needed? :)

Thanks,
Stephan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] thermal/drivers/tsens: Add error/debug prints to calibration read
  2023-04-06 14:58 ` [PATCH 1/3] thermal/drivers/tsens: Add error/debug prints to calibration read Bryan O'Donoghue
@ 2023-04-06 17:36   ` Dmitry Baryshkov
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Baryshkov @ 2023-04-06 17:36 UTC (permalink / raw)
  To: Bryan O'Donoghue, amitk, thara.gopinath, agross, andersson,
	konrad.dybcio, rafael, daniel.lezcano, rui.zhang
  Cc: linux-pm, linux-arm-msm, linux-kernel

On 06/04/2023 17:58, Bryan O'Donoghue wrote:
> Add in some dev_dbg() to aid in viewing of raw calibration data extracted.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   drivers/thermal/qcom/tsens.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] thermal/drivers/tsens: Describe sensor registers via a structure
  2023-04-06 14:58 ` [PATCH 2/3] thermal/drivers/tsens: Describe sensor registers via a structure Bryan O'Donoghue
@ 2023-04-06 17:43   ` Dmitry Baryshkov
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Baryshkov @ 2023-04-06 17:43 UTC (permalink / raw)
  To: Bryan O'Donoghue, amitk, thara.gopinath, agross, andersson,
	konrad.dybcio, rafael, daniel.lezcano, rui.zhang
  Cc: linux-pm, linux-arm-msm, linux-kernel

On 06/04/2023 17:58, Bryan O'Donoghue wrote:
> Define sensor identifiers and optional shifts in a single data-structure.
> This facilitates extraction of calibration data from non-contiguous
> addresses.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   drivers/thermal/qcom/tsens-v0_1.c | 56 +++++++++++++++++++++++++++++--
>   drivers/thermal/qcom/tsens.c      |  5 +--
>   drivers/thermal/qcom/tsens.h      | 16 ++++++++-
>   3 files changed, 72 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c
> index e89c6f39a3aea..c20c002d98650 100644
> --- a/drivers/thermal/qcom/tsens-v0_1.c
> +++ b/drivers/thermal/qcom/tsens-v0_1.c
> @@ -319,10 +319,31 @@ static const struct tsens_ops ops_8916 = {
>   	.get_temp	= get_temp_common,
>   };
>   
> +struct tsens_reg_data reg_8916[] = {
> +	{
> +		.id = 0,
> +	},
> +	{
> +		.id = 1,
> +	},
> +	{
> +		.id = 2,
> +	},
> +	{
> +		.id = 3,
> +	},
> +	{
> +		.id = 4,
> +	},
> +	{
> +		.id = 5,
> +	},
> +};
> +
>   struct tsens_plat_data data_8916 = {
>   	.num_sensors	= 5,
>   	.ops		= &ops_8916,
> -	.hw_ids		= (unsigned int []){0, 1, 2, 4, 5 },
> +	.reg		= reg_8916,
>   
>   	.feat		= &tsens_v0_1_feat,
>   	.fields	= tsens_v0_1_regfields,
> @@ -334,10 +355,41 @@ static const struct tsens_ops ops_8939 = {
>   	.get_temp	= get_temp_common,
>   };
>   
> +struct tsens_reg_data reg_8939[] = {
> +	{
> +		.id = 0,
> +	},
> +	{
> +		.id = 1,
> +	},
> +	{
> +		.id = 2,
> +	},
> +	{
> +		.id = 3,
> +	},
> +	{
> +		.id = 5,
> +	},
> +	{
> +		.id = 6,
> +	},
> +	{
> +		.id = 7,
> +	},
> +	{
> +		.id = 8,
> +	},
> +	{
> +		.id = 9,

Sensor 9 is contiguous. It's sensor with hwid=10, who requires two reads.

> +		.p2_shift = 8,
> +	},
> +};
> +
>   struct tsens_plat_data data_8939 = {
>   	.num_sensors	= 9,
>   	.ops		= &ops_8939,
> -	.hw_ids		= (unsigned int []){ 0, 1, 2, 3, 5, 6, 7, 8, 9, /* 10 */ },
> +	.reg		= reg_8939,
>   
>   	.feat		= &tsens_v0_1_feat,
>   	.fields	= tsens_v0_1_regfields,
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 7165b0bfe8b9f..a260f563b4889 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -1274,13 +1274,14 @@ static int tsens_probe(struct platform_device *pdev)
>   	priv->num_sensors = num_sensors;
>   	priv->ops = data->ops;
>   	for (i = 0;  i < priv->num_sensors; i++) {
> -		if (data->hw_ids)
> -			priv->sensor[i].hw_id = data->hw_ids[i];
> +		if (data->reg)
> +			priv->sensor[i].hw_id = data->reg[i].id;
>   		else
>   			priv->sensor[i].hw_id = i;
>   	}
>   	priv->feat = data->feat;
>   	priv->fields = data->fields;
> +	priv->reg = data->reg;
>   
>   	platform_set_drvdata(pdev, priv);
>   
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index dba9cd38f637c..31f67da03bce6 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -517,18 +517,31 @@ struct tsens_features {
>   	int trip_max_temp;
>   };
>   
> +/**
> + * struct tsens_reg_data - describes register data retrieved non-contiguously
> + * @id: thermal sensor identifier
> + * @p1_shift: When non-zero is the # of bits to right shift p1 MSB by
> + * @p2_shift: When non-zero is the # of bits to right shift p2 MSB by
> + */
> +struct tsens_reg_data {
> +	unsigned int id;
> +	unsigned int p1_shift;
> +	unsigned int p2_shift;
> +};
> +
>   /**
>    * struct tsens_plat_data - tsens compile-time platform data
>    * @num_sensors: Number of sensors supported by platform
>    * @ops: operations the tsens instance supports
>    * @hw_ids: Subset of sensors ids supported by platform, if not the first n
> + * @reg: Describe sensor id and calibration shifts
>    * @feat: features of the IP
>    * @fields: bitfield locations
>    */
>   struct tsens_plat_data {
>   	const u32		num_sensors;
>   	const struct tsens_ops	*ops;
> -	unsigned int		*hw_ids;
> +	struct tsens_reg_data	*reg;
>   	struct tsens_features	*feat;
>   	const struct reg_field		*fields;
>   };
> @@ -575,6 +588,7 @@ struct tsens_priv {
>   	struct regmap_field		*rf[MAX_REGFIELDS];
>   	struct tsens_context		ctx;
>   	struct tsens_features		*feat;
> +	struct tsens_reg_data		*reg;
>   	const struct reg_field		*fields;
>   	const struct tsens_ops		*ops;
>   

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] thermal/drivers/tsens: Extract and shift-in optional MSB
  2023-04-06 14:58 ` [PATCH 3/3] thermal/drivers/tsens: Extract and shift-in optional MSB Bryan O'Donoghue
@ 2023-04-06 17:47   ` Dmitry Baryshkov
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Baryshkov @ 2023-04-06 17:47 UTC (permalink / raw)
  To: Bryan O'Donoghue, amitk, thara.gopinath, agross, andersson,
	konrad.dybcio, rafael, daniel.lezcano, rui.zhang
  Cc: linux-pm, linux-arm-msm, linux-kernel

On 06/04/2023 17:58, Bryan O'Donoghue wrote:
> In msm8939 some of the sensor calibration data traverses byte boundaries.
> Two examples of this are thermal sensor 2 point 1 and sensor 9 point 2.
> 
> For sensor 2 point 1 we can get away with a simple read traversing byte
> boundaries as the calibration most significant bits are adjacent to the
> least significant across the byte boundary.
> 
> In this case a read starting at the end of the first byte for nine bits
> will deliver up the data we want.
> 
> In the case of sensor 9 point 2 however, the most significant bits are not
> adjacent and so therefore we need to perform two reads and or the bits
> together.
> 
> If reg.p1_shift or reg.p2_shift is set then automatically search for
> pX_sY_msb in the dts applying pX_shift as a right shift or into the pX_sY
> value.

I think that having this in the common code is a bit of an overkill. No 
other platform has this 'peculiarity' up to now. So, it might be better 
to add 8939-specific calibration function that calls 
tsens_read_calibration(), mixes in the s10_p2_msb and then calls 
compute_intercept_slope().

> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   drivers/thermal/qcom/tsens.c | 33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index a260f563b4889..eff2c8671c343 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -74,6 +74,7 @@ int tsens_read_calibration(struct tsens_priv *priv, int shift, u32 *p1, u32 *p2,
>   {
>   	u32 mode;
>   	u32 base1, base2;
> +	u32 msb;
>   	char name[] = "sXX_pY_backup"; /* s10_p1_backup */
>   	int i, ret;
>   
> @@ -122,6 +123,22 @@ int tsens_read_calibration(struct tsens_priv *priv, int shift, u32 *p1, u32 *p2,
>   
>   		dev_dbg(priv->dev, "%s 0x%x\n", name, p1[i]);
>   
> +		if (priv->reg && priv->reg[i].p1_shift) {
> +			ret = snprintf(name, sizeof(name), "s%d_p1_msb",
> +				       priv->sensor[i].hw_id);
> +			if (ret < 0)
> +				return ret;
> +
> +			ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &msb);
> +			if (ret) {
> +				dev_err(priv->dev, "Failed to read %s\n", name);
> +				return ret;
> +			}
> +
> +			dev_dbg(priv->dev, "%s 0x%x\n", name, msb);
> +			p1[i] |= msb >> priv->reg[i].p1_shift;
> +		}
> +
>   		ret = snprintf(name, sizeof(name), "s%d_p2%s", priv->sensor[i].hw_id,
>   			       backup ? "_backup" : "");
>   		if (ret < 0)
> @@ -134,6 +151,22 @@ int tsens_read_calibration(struct tsens_priv *priv, int shift, u32 *p1, u32 *p2,
>   		}
>   
>   		dev_dbg(priv->dev, "%s 0x%x\n", name, p2[i]);
> +
> +		if (priv->reg && priv->reg[i].p2_shift) {
> +			ret = snprintf(name, sizeof(name), "s%d_p2_msb",
> +				       priv->sensor[i].hw_id);
> +			if (ret < 0)
> +				return ret;
> +
> +			ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &msb);
> +			if (ret) {
> +				dev_err(priv->dev, "Failed to read %s\n", name);
> +				return ret;
> +			}
> +
> +			dev_dbg(priv->dev, "%s 0x%x\n", name, msb);
> +			p2[i] |= msb >> priv->reg[i].p2_shift;
> +		}
>   	}
>   
>   	switch (mode) {

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data
  2023-04-06 16:20 ` [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data Stephan Gerhold
@ 2023-04-06 22:36   ` Bryan O'Donoghue
  0 siblings, 0 replies; 9+ messages in thread
From: Bryan O'Donoghue @ 2023-04-06 22:36 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: amitk, thara.gopinath, agross, andersson, konrad.dybcio, rafael,
	daniel.lezcano, rui.zhang, dmitry.baryshkov, linux-pm,
	linux-arm-msm, linux-kernel

On 06/04/2023 17:20, Stephan Gerhold wrote:
>> Reviewed-by: Bryan O'Donoghue<bryan.odonoghue@linaro.org>
> So with sensor hwid=10 disabled, I think this patch series is actually
> not needed? 😄

I can hardly be expected to remember back to January ..

dropping

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-04-06 22:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-06 14:58 [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data Bryan O'Donoghue
2023-04-06 14:58 ` [PATCH 1/3] thermal/drivers/tsens: Add error/debug prints to calibration read Bryan O'Donoghue
2023-04-06 17:36   ` Dmitry Baryshkov
2023-04-06 14:58 ` [PATCH 2/3] thermal/drivers/tsens: Describe sensor registers via a structure Bryan O'Donoghue
2023-04-06 17:43   ` Dmitry Baryshkov
2023-04-06 14:58 ` [PATCH 3/3] thermal/drivers/tsens: Extract and shift-in optional MSB Bryan O'Donoghue
2023-04-06 17:47   ` Dmitry Baryshkov
2023-04-06 16:20 ` [PATCH 0/3] drivers/thermal/qcom/tsens: Add ability to read and shift-in non-contiguous calibration data Stephan Gerhold
2023-04-06 22:36   ` Bryan O'Donoghue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).