[PATCH v2 0/6] brcmstb_thermal updates for new processes

[PATCH v2 0/6] brcmstb_thermal updates for new processes

[Florian Fainelli]

Hi,

This patch series contains a bug fix for the existing platforms and then
paves the way for adding support for Broadcom STB's latest chips in 16nm
processes, and finally updates the driver with pecularities introduced
with the 16nm, like the lack of interrupt notification from the HW.

Please queue up the first patch for -stable if you want, thanks!

Changes in v2:

- kept defined constants in patch #1 and keep using them for subsequent
  patches
- add Reviewed-by tags to patches #3 through #6
- rebase against v5.5.-rc1

Florian Fainelli (6):
  thermal: brcmstb_thermal: Do not use DT coefficients
  thermal: brcmstb_thermal: Prepare to support a different process
  dt-bindings: thermal: Define BCM7216 thermal sensor compatible
  thermal: brcmstb_thermal: Add 16nm process thermal parameters
  thermal: brcmstb_thermal: Restructure interrupt registration
  thermal: brcmstb_thermal: Register different ops per process

 .../bindings/thermal/brcm,avs-tmon.txt        |  8 +-
 drivers/thermal/broadcom/brcmstb_thermal.c    | 99 ++++++++++++-------
 2 files changed, 67 insertions(+), 40 deletions(-)

-- 
2.17.1

[PATCH v2 1/6] thermal: brcmstb_thermal: Do not use DT coefficients

[Florian Fainelli]

At the time the brcmstb_thermal driver and its binding were merged, the
DT binding did not make the coefficients properties a mandatory one,
therefore all users of the brcmstb_thermal driver out there have a non
functional implementation with zero coefficients. Even if these
properties were provided, the formula used for computation is incorrect.

The coefficients are entirely process specific (right now, only 28nm is
supported) and not board or SoC specific, it is therefore appropriate to
hard code them in the driver given the compatibility string we are
probed with which has to be updated whenever a new process is
introduced.

We remove the existing coefficients definition since subsequent patches
are going to add support for a new process and will introduce new
coefficients as well.

Fixes: 9e03cf1b2dd5 ("thermal: add brcmstb AVS TMON driver")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/thermal/broadcom/brcmstb_thermal.c | 31 +++++++---------------
 1 file changed, 9 insertions(+), 22 deletions(-)

diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
index 5825ac581f56..680f1a070606 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -49,7 +49,7 @@
 #define AVS_TMON_TP_TEST_ENABLE		0x20
 
 /* Default coefficients */
-#define AVS_TMON_TEMP_SLOPE		-487
+#define AVS_TMON_TEMP_SLOPE		487
 #define AVS_TMON_TEMP_OFFSET		410040
 
 /* HW related temperature constants */
@@ -108,23 +108,12 @@ struct brcmstb_thermal_priv {
 	struct thermal_zone_device *thermal;
 };
 
-static void avs_tmon_get_coeffs(struct thermal_zone_device *tz, int *slope,
-				int *offset)
-{
-	*slope = thermal_zone_get_slope(tz);
-	*offset = thermal_zone_get_offset(tz);
-}
-
 /* Convert a HW code to a temperature reading (millidegree celsius) */
 static inline int avs_tmon_code_to_temp(struct thermal_zone_device *tz,
 					u32 code)
 {
-	const int val = code & AVS_TMON_TEMP_MASK;
-	int slope, offset;
-
-	avs_tmon_get_coeffs(tz, &slope, &offset);
-
-	return slope * val + offset;
+	return (AVS_TMON_TEMP_OFFSET -
+		(int)((code & AVS_TMON_TEMP_MAX) * AVS_TMON_TEMP_SLOPE));
 }
 
 /*
@@ -136,20 +125,18 @@ static inline int avs_tmon_code_to_temp(struct thermal_zone_device *tz,
 static inline u32 avs_tmon_temp_to_code(struct thermal_zone_device *tz,
 					int temp, bool low)
 {
-	int slope, offset;
-
 	if (temp < AVS_TMON_TEMP_MIN)
-		return AVS_TMON_TEMP_MAX; /* Maximum code value */
-
-	avs_tmon_get_coeffs(tz, &slope, &offset);
+		return AVS_TMON_TEMP_MAX;	/* Maximum code value */
 
-	if (temp >= offset)
+	if (temp >= AVS_TMON_TEMP_OFFSET)
 		return 0;	/* Minimum code value */
 
 	if (low)
-		return (u32)(DIV_ROUND_UP(offset - temp, abs(slope)));
+		return (u32)(DIV_ROUND_UP(AVS_TMON_TEMP_OFFSET - temp,
+					  AVS_TMON_TEMP_SLOPE));
 	else
-		return (u32)((offset - temp) / abs(slope));
+		return (u32)((AVS_TMON_TEMP_OFFSET - temp) /
+			      AVS_TMON_TEMP_SLOPE);
 }
 
 static int brcmstb_get_temp(void *data, int *temp)
-- 
2.17.1

[PATCH v2 2/6] thermal: brcmstb_thermal: Prepare to support a different process

[Florian Fainelli]

The driver is currently assuming that it is operating with a 28nm
process chip, which has a specific formula to convert temperature to a
code and vice versa. Update the code to support providing two key
values: offset and multiplier to derive the correct formulas.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/thermal/broadcom/brcmstb_thermal.c | 54 ++++++++++++++++------
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
index 680f1a070606..68f89f7c7e7f 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -102,18 +102,27 @@ static struct avs_tmon_trip avs_tmon_trips[] = {
 	},
 };
 
+struct brcmstb_thermal_params {
+	unsigned int offset;
+	unsigned int mult;
+};
+
 struct brcmstb_thermal_priv {
 	void __iomem *tmon_base;
 	struct device *dev;
 	struct thermal_zone_device *thermal;
+	/* Process specific thermal parameters used for calculations */
+	struct brcmstb_thermal_params temp_params;
 };
 
 /* Convert a HW code to a temperature reading (millidegree celsius) */
-static inline int avs_tmon_code_to_temp(struct thermal_zone_device *tz,
+static inline int avs_tmon_code_to_temp(struct brcmstb_thermal_priv *priv,
 					u32 code)
 {
-	return (AVS_TMON_TEMP_OFFSET -
-		(int)((code & AVS_TMON_TEMP_MAX) * AVS_TMON_TEMP_SLOPE));
+	int offset = priv->temp_params.offset;
+	int mult = priv->temp_params.mult;
+
+	return (offset - (int)((code & AVS_TMON_TEMP_MASK) * mult));
 }
 
 /*
@@ -122,21 +131,22 @@ static inline int avs_tmon_code_to_temp(struct thermal_zone_device *tz,
  * @temp: temperature to convert
  * @low: if true, round toward the low side
  */
-static inline u32 avs_tmon_temp_to_code(struct thermal_zone_device *tz,
+static inline u32 avs_tmon_temp_to_code(struct brcmstb_thermal_priv *priv,
 					int temp, bool low)
 {
+	int offset = priv->temp_params.offset;
+	int mult = priv->temp_params.mult;
+
 	if (temp < AVS_TMON_TEMP_MIN)
-		return AVS_TMON_TEMP_MAX;	/* Maximum code value */
+		return AVS_TMON_TEMP_MASK;	/* Maximum code value */
 
-	if (temp >= AVS_TMON_TEMP_OFFSET)
+	if (temp >= offset)
 		return 0;	/* Minimum code value */
 
 	if (low)
-		return (u32)(DIV_ROUND_UP(AVS_TMON_TEMP_OFFSET - temp,
-					  AVS_TMON_TEMP_SLOPE));
+		return (u32)(DIV_ROUND_UP(offset - temp, mult));
 	else
-		return (u32)((AVS_TMON_TEMP_OFFSET - temp) /
-			      AVS_TMON_TEMP_SLOPE);
+		return (u32)((offset - temp) / mult);
 }
 
 static int brcmstb_get_temp(void *data, int *temp)
@@ -154,7 +164,7 @@ static int brcmstb_get_temp(void *data, int *temp)
 
 	val = (val & AVS_TMON_STATUS_data_msk) >> AVS_TMON_STATUS_data_shift;
 
-	t = avs_tmon_code_to_temp(priv->thermal, val);
+	t = avs_tmon_code_to_temp(priv, val);
 	if (t < 0)
 		*temp = 0;
 	else
@@ -188,7 +198,7 @@ static int avs_tmon_get_trip_temp(struct brcmstb_thermal_priv *priv,
 	val &= trip->reg_msk;
 	val >>= trip->reg_shift;
 
-	return avs_tmon_code_to_temp(priv->thermal, val);
+	return avs_tmon_code_to_temp(priv, val);
 }
 
 static void avs_tmon_set_trip_temp(struct brcmstb_thermal_priv *priv,
@@ -201,7 +211,7 @@ static void avs_tmon_set_trip_temp(struct brcmstb_thermal_priv *priv,
 	dev_dbg(priv->dev, "set temp %d to %d\n", type, temp);
 
 	/* round toward low temp for the low interrupt */
-	val = avs_tmon_temp_to_code(priv->thermal, temp,
+	val = avs_tmon_temp_to_code(priv, temp,
 				    type == TMON_TRIP_TYPE_LOW);
 
 	val <<= trip->reg_shift;
@@ -218,7 +228,7 @@ static int avs_tmon_get_intr_temp(struct brcmstb_thermal_priv *priv)
 	u32 val;
 
 	val = __raw_readl(priv->tmon_base + AVS_TMON_TEMP_INT_CODE);
-	return avs_tmon_code_to_temp(priv->thermal, val);
+	return avs_tmon_code_to_temp(priv, val);
 }
 
 static irqreturn_t brcmstb_tmon_irq_thread(int irq, void *data)
@@ -282,19 +292,32 @@ static const struct thermal_zone_of_device_ops of_ops = {
 	.set_trips	= brcmstb_set_trips,
 };
 
+static const struct brcmstb_thermal_params brcmstb_28nm_params = {
+	.offset	= 410040,
+	.mult	= 487,
+};
+
 static const struct of_device_id brcmstb_thermal_id_table[] = {
-	{ .compatible = "brcm,avs-tmon" },
+	{ .compatible = "brcm,avs-tmon", .data = &brcmstb_28nm_params },
 	{},
 };
 MODULE_DEVICE_TABLE(of, brcmstb_thermal_id_table);
 
 static int brcmstb_thermal_probe(struct platform_device *pdev)
 {
+	const struct brcmstb_thermal_params *params;
+	const struct of_device_id *of_id = NULL;
 	struct thermal_zone_device *thermal;
 	struct brcmstb_thermal_priv *priv;
 	struct resource *res;
 	int irq, ret;
 
+	of_id = of_match_node(brcmstb_thermal_id_table, pdev->dev.of_node);
+	if (!of_id || !of_id->data)
+		return -EINVAL;
+
+	params = of_id->data;
+
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
@@ -304,6 +327,7 @@ static int brcmstb_thermal_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->tmon_base))
 		return PTR_ERR(priv->tmon_base);
 
+	memcpy(&priv->temp_params, params, sizeof(priv->temp_params));
 	priv->dev = &pdev->dev;
 	platform_set_drvdata(pdev, priv);
 
-- 
2.17.1

[PATCH v2 3/6] dt-bindings: thermal: Define BCM7216 thermal sensor compatible

[Florian Fainelli]

BCM7216 is a 16nm process STB chip, which requires a different
compatible string to differentiate different temperature formulas.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../devicetree/bindings/thermal/brcm,avs-tmon.txt         | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/brcm,avs-tmon.txt b/Documentation/devicetree/bindings/thermal/brcm,avs-tmon.txt
index 43a9ed545944..74a9ef09db8b 100644
--- a/Documentation/devicetree/bindings/thermal/brcm,avs-tmon.txt
+++ b/Documentation/devicetree/bindings/thermal/brcm,avs-tmon.txt
@@ -3,9 +3,13 @@
 Thermal management core, provided by the AVS TMON hardware block.
 
 Required properties:
-- compatible: must be "brcm,avs-tmon" and/or "brcm,avs-tmon-bcm7445"
+- compatible: must be one of:
+	"brcm,avs-tmon-bcm7216"
+	"brcm,avs-tmon-bcm7445"
+	"brcm,avs-tmon"
 - reg: address range for the AVS TMON registers
-- interrupts: temperature monitor interrupt, for high/low threshold triggers
+- interrupts: temperature monitor interrupt, for high/low threshold triggers,
+	      required except for "brcm,avs-tmon-bcm7216"
 - interrupt-names: should be "tmon"
 
 Example:
-- 
2.17.1

[PATCH v2 4/6] thermal: brcmstb_thermal: Add 16nm process thermal parameters

[Florian Fainelli]

Match the 7216 compatible string in order to derive the correct 16nm
process thermal parameters to obtain correct readings.

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/thermal/broadcom/brcmstb_thermal.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
index 68f89f7c7e7f..74d94f01b1b9 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -292,12 +292,18 @@ static const struct thermal_zone_of_device_ops of_ops = {
 	.set_trips	= brcmstb_set_trips,
 };
 
+static const struct brcmstb_thermal_params brcmstb_16nm_params = {
+	.offset	= 457829,
+	.mult	= 557,
+};
+
 static const struct brcmstb_thermal_params brcmstb_28nm_params = {
 	.offset	= 410040,
 	.mult	= 487,
 };
 
 static const struct of_device_id brcmstb_thermal_id_table[] = {
+	{ .compatible = "brcm,avs-tmon-bcm7216", .data = &brcmstb_16nm_params },
 	{ .compatible = "brcm,avs-tmon", .data = &brcmstb_28nm_params },
 	{},
 };
-- 
2.17.1

[PATCH v2 5/6] thermal: brcmstb_thermal: Restructure interrupt registration

[Florian Fainelli]

If we are successful grabbing the interrupt resource, then register an
interrupt handler, this makes it easier to support the interrupt as
being optional, which is it for 7216.

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/thermal/broadcom/brcmstb_thermal.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
index 74d94f01b1b9..47b622f33900 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -348,16 +348,15 @@ static int brcmstb_thermal_probe(struct platform_device *pdev)
 	priv->thermal = thermal;
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		dev_err(&pdev->dev, "could not get IRQ\n");
-		return irq;
-	}
-	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
-					brcmstb_tmon_irq_thread, IRQF_ONESHOT,
-					DRV_NAME, priv);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "could not request IRQ: %d\n", ret);
-		return ret;
+	if (irq >= 0) {
+		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+						brcmstb_tmon_irq_thread,
+						IRQF_ONESHOT,
+						DRV_NAME, priv);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "could not request IRQ: %d\n", ret);
+			return ret;
+		}
 	}
 
 	dev_info(&pdev->dev, "registered AVS TMON of-sensor driver\n");
-- 
2.17.1

[PATCH v2 6/6] thermal: brcmstb_thermal: Register different ops per process

[Florian Fainelli]

Since we do not have interrupts on BCM7216, we cannot have trip point
crossing, the thermal subsystem expects us to provide a NULL set_trips
operation in that case, so make it possible to provide per-process
thermal_zone_of_device_ops

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/thermal/broadcom/brcmstb_thermal.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
index 47b622f33900..8170ea1b8227 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -105,6 +105,7 @@ static struct avs_tmon_trip avs_tmon_trips[] = {
 struct brcmstb_thermal_params {
 	unsigned int offset;
 	unsigned int mult;
+	const struct thermal_zone_of_device_ops *of_ops;
 };
 
 struct brcmstb_thermal_priv {
@@ -287,19 +288,25 @@ static int brcmstb_set_trips(void *data, int low, int high)
 	return 0;
 }
 
-static const struct thermal_zone_of_device_ops of_ops = {
+static const struct thermal_zone_of_device_ops brcmstb_16nm_of_ops = {
 	.get_temp	= brcmstb_get_temp,
-	.set_trips	= brcmstb_set_trips,
 };
 
 static const struct brcmstb_thermal_params brcmstb_16nm_params = {
 	.offset	= 457829,
 	.mult	= 557,
+	.of_ops	= &brcmstb_16nm_of_ops,
+};
+
+static const struct thermal_zone_of_device_ops brcmstb_28nm_of_ops = {
+	.get_temp	= brcmstb_get_temp,
+	.set_trips	= brcmstb_set_trips,
 };
 
 static const struct brcmstb_thermal_params brcmstb_28nm_params = {
 	.offset	= 410040,
 	.mult	= 487,
+	.of_ops	= &brcmstb_28nm_of_ops,
 };
 
 static const struct of_device_id brcmstb_thermal_id_table[] = {
@@ -338,7 +345,7 @@ static int brcmstb_thermal_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, priv);
 
 	thermal = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, priv,
-						       &of_ops);
+						       priv->temp_params.of_ops);
 	if (IS_ERR(thermal)) {
 		ret = PTR_ERR(thermal);
 		dev_err(&pdev->dev, "could not register sensor: %d\n", ret);
-- 
2.17.1

Re: [PATCH v2 0/6] brcmstb_thermal updates for new processes

[Florian Fainelli]

On 12/11/2019 12:31 PM, Florian Fainelli wrote:
> Hi,
> 
> This patch series contains a bug fix for the existing platforms and then
> paves the way for adding support for Broadcom STB's latest chips in 16nm
> processes, and finally updates the driver with pecularities introduced
> with the 16nm, like the lack of interrupt notification from the HW.
> 
> Please queue up the first patch for -stable if you want, thanks!

Amit, Daniel, Rui, does this look acceptable to you now? Thank you

> 
> Changes in v2:
> 
> - kept defined constants in patch #1 and keep using them for subsequent
>   patches
> - add Reviewed-by tags to patches #3 through #6
> - rebase against v5.5.-rc1
> 
> Florian Fainelli (6):
>   thermal: brcmstb_thermal: Do not use DT coefficients
>   thermal: brcmstb_thermal: Prepare to support a different process
>   dt-bindings: thermal: Define BCM7216 thermal sensor compatible
>   thermal: brcmstb_thermal: Add 16nm process thermal parameters
>   thermal: brcmstb_thermal: Restructure interrupt registration
>   thermal: brcmstb_thermal: Register different ops per process
> 
>  .../bindings/thermal/brcm,avs-tmon.txt        |  8 +-
>  drivers/thermal/broadcom/brcmstb_thermal.c    | 99 ++++++++++++-------
>  2 files changed, 67 insertions(+), 40 deletions(-)
> 

-- 
Florian

Re: [PATCH v2 0/6] brcmstb_thermal updates for new processes

[Florian Fainelli]

On 12/26/19 7:54 AM, Florian Fainelli wrote:
> 
> 
> On 12/11/2019 12:31 PM, Florian Fainelli wrote:
>> Hi,
>>
>> This patch series contains a bug fix for the existing platforms and then
>> paves the way for adding support for Broadcom STB's latest chips in 16nm
>> processes, and finally updates the driver with pecularities introduced
>> with the 16nm, like the lack of interrupt notification from the HW.
>>
>> Please queue up the first patch for -stable if you want, thanks!
> 
> Amit, Daniel, Rui, does this look acceptable to you now? Thank you

Ping? Is this good to go now?
-- 
Florian

Re: [PATCH v2 2/6] thermal: brcmstb_thermal: Prepare to support a different process

[Daniel Lezcano]

On 11/12/2019 21:31, Florian Fainelli wrote:
> The driver is currently assuming that it is operating with a 28nm
> process chip, which has a specific formula to convert temperature to a
> code and vice versa. Update the code to support providing two key
> values: offset and multiplier to derive the correct formulas.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/thermal/broadcom/brcmstb_thermal.c | 54 ++++++++++++++++------
>  1 file changed, 39 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
> index 680f1a070606..68f89f7c7e7f 100644
> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
> @@ -102,18 +102,27 @@ static struct avs_tmon_trip avs_tmon_trips[] = {
>  	},
>  };
>  
> +struct brcmstb_thermal_params {
> +	unsigned int offset;
> +	unsigned int mult;
> +};
> +
>  struct brcmstb_thermal_priv {
>  	void __iomem *tmon_base;
>  	struct device *dev;
>  	struct thermal_zone_device *thermal;
> +	/* Process specific thermal parameters used for calculations */
> +	struct brcmstb_thermal_params temp_params;
>  };
>  
>  /* Convert a HW code to a temperature reading (millidegree celsius) */
> -static inline int avs_tmon_code_to_temp(struct thermal_zone_device *tz,
> +static inline int avs_tmon_code_to_temp(struct brcmstb_thermal_priv *priv,
>  					u32 code)
>  {
> -	return (AVS_TMON_TEMP_OFFSET -
> -		(int)((code & AVS_TMON_TEMP_MAX) * AVS_TMON_TEMP_SLOPE));
> +	int offset = priv->temp_params.offset;
> +	int mult = priv->temp_params.mult;
> +
> +	return (offset - (int)((code & AVS_TMON_TEMP_MASK) * mult));
>  }
>  
>  /*
> @@ -122,21 +131,22 @@ static inline int avs_tmon_code_to_temp(struct thermal_zone_device *tz,
>   * @temp: temperature to convert
>   * @low: if true, round toward the low side
>   */
> -static inline u32 avs_tmon_temp_to_code(struct thermal_zone_device *tz,
> +static inline u32 avs_tmon_temp_to_code(struct brcmstb_thermal_priv *priv,
>  					int temp, bool low)
>  {
> +	int offset = priv->temp_params.offset;
> +	int mult = priv->temp_params.mult;
> +
>  	if (temp < AVS_TMON_TEMP_MIN)
> -		return AVS_TMON_TEMP_MAX;	/* Maximum code value */
> +		return AVS_TMON_TEMP_MASK;	/* Maximum code value */

Why this change?

>  
> -	if (temp >= AVS_TMON_TEMP_OFFSET)
> +	if (temp >= offset)
>  		return 0;	/* Minimum code value */
>  
>  	if (low)
> -		return (u32)(DIV_ROUND_UP(AVS_TMON_TEMP_OFFSET - temp,
> -					  AVS_TMON_TEMP_SLOPE));
> +		return (u32)(DIV_ROUND_UP(offset - temp, mult));
>  	else
> -		return (u32)((AVS_TMON_TEMP_OFFSET - temp) /
> -			      AVS_TMON_TEMP_SLOPE);
> +		return (u32)((offset - temp) / mult);
>  }
>  
>  static int brcmstb_get_temp(void *data, int *temp)
> @@ -154,7 +164,7 @@ static int brcmstb_get_temp(void *data, int *temp)
>  
>  	val = (val & AVS_TMON_STATUS_data_msk) >> AVS_TMON_STATUS_data_shift;
>  
> -	t = avs_tmon_code_to_temp(priv->thermal, val);
> +	t = avs_tmon_code_to_temp(priv, val);
>  	if (t < 0)
>  		*temp = 0;
>  	else
> @@ -188,7 +198,7 @@ static int avs_tmon_get_trip_temp(struct brcmstb_thermal_priv *priv,
>  	val &= trip->reg_msk;
>  	val >>= trip->reg_shift;
>  
> -	return avs_tmon_code_to_temp(priv->thermal, val);
> +	return avs_tmon_code_to_temp(priv, val);
>  }
>  
>  static void avs_tmon_set_trip_temp(struct brcmstb_thermal_priv *priv,
> @@ -201,7 +211,7 @@ static void avs_tmon_set_trip_temp(struct brcmstb_thermal_priv *priv,
>  	dev_dbg(priv->dev, "set temp %d to %d\n", type, temp);
>  
>  	/* round toward low temp for the low interrupt */
> -	val = avs_tmon_temp_to_code(priv->thermal, temp,
> +	val = avs_tmon_temp_to_code(priv, temp,
>  				    type == TMON_TRIP_TYPE_LOW);
>  
>  	val <<= trip->reg_shift;
> @@ -218,7 +228,7 @@ static int avs_tmon_get_intr_temp(struct brcmstb_thermal_priv *priv)
>  	u32 val;
>  
>  	val = __raw_readl(priv->tmon_base + AVS_TMON_TEMP_INT_CODE);
> -	return avs_tmon_code_to_temp(priv->thermal, val);
> +	return avs_tmon_code_to_temp(priv, val);
>  }
>  
>  static irqreturn_t brcmstb_tmon_irq_thread(int irq, void *data)
> @@ -282,19 +292,32 @@ static const struct thermal_zone_of_device_ops of_ops = {
>  	.set_trips	= brcmstb_set_trips,
>  };
>  
> +static const struct brcmstb_thermal_params brcmstb_28nm_params = {
> +	.offset	= 410040,
> +	.mult	= 487,
> +};
> +
>  static const struct of_device_id brcmstb_thermal_id_table[] = {
> -	{ .compatible = "brcm,avs-tmon" },
> +	{ .compatible = "brcm,avs-tmon", .data = &brcmstb_28nm_params },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, brcmstb_thermal_id_table);
>  
>  static int brcmstb_thermal_probe(struct platform_device *pdev)
>  {
> +	const struct brcmstb_thermal_params *params;
> +	const struct of_device_id *of_id = NULL;
>  	struct thermal_zone_device *thermal;
>  	struct brcmstb_thermal_priv *priv;
>  	struct resource *res;
>  	int irq, ret;
>  
> +	of_id = of_match_node(brcmstb_thermal_id_table, pdev->dev.of_node);
> +	if (!of_id || !of_id->data)
> +		return -EINVAL;

of_device_get_match_data(&pdev->dev) ?

> +	params = of_id->data;
> +
>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;
> @@ -304,6 +327,7 @@ static int brcmstb_thermal_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->tmon_base))
>  		return PTR_ERR(priv->tmon_base);
>  
> +	memcpy(&priv->temp_params, params, sizeof(priv->temp_params));

Do you really need a copy here? Why not convert to a pointer and assign it?

>  	priv->dev = &pdev->dev;
>  	platform_set_drvdata(pdev, priv);
>  
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v2 2/6] thermal: brcmstb_thermal: Prepare to support a different process

[Florian Fainelli]

On 1/9/20 1:17 PM, Daniel Lezcano wrote:
> On 11/12/2019 21:31, Florian Fainelli wrote:
>> The driver is currently assuming that it is operating with a 28nm
>> process chip, which has a specific formula to convert temperature to a
>> code and vice versa. Update the code to support providing two key
>> values: offset and multiplier to derive the correct formulas.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  drivers/thermal/broadcom/brcmstb_thermal.c | 54 ++++++++++++++++------
>>  1 file changed, 39 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
>> index 680f1a070606..68f89f7c7e7f 100644
>> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
>> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
>> @@ -102,18 +102,27 @@ static struct avs_tmon_trip avs_tmon_trips[] = {
>>  	},
>>  };
>>  
>> +struct brcmstb_thermal_params {
>> +	unsigned int offset;
>> +	unsigned int mult;
>> +};
>> +
>>  struct brcmstb_thermal_priv {
>>  	void __iomem *tmon_base;
>>  	struct device *dev;
>>  	struct thermal_zone_device *thermal;
>> +	/* Process specific thermal parameters used for calculations */
>> +	struct brcmstb_thermal_params temp_params;
>>  };
>>  
>>  /* Convert a HW code to a temperature reading (millidegree celsius) */
>> -static inline int avs_tmon_code_to_temp(struct thermal_zone_device *tz,
>> +static inline int avs_tmon_code_to_temp(struct brcmstb_thermal_priv *priv,
>>  					u32 code)
>>  {
>> -	return (AVS_TMON_TEMP_OFFSET -
>> -		(int)((code & AVS_TMON_TEMP_MAX) * AVS_TMON_TEMP_SLOPE));
>> +	int offset = priv->temp_params.offset;
>> +	int mult = priv->temp_params.mult;
>> +
>> +	return (offset - (int)((code & AVS_TMON_TEMP_MASK) * mult));
>>  }
>>  
>>  /*
>> @@ -122,21 +131,22 @@ static inline int avs_tmon_code_to_temp(struct thermal_zone_device *tz,
>>   * @temp: temperature to convert
>>   * @low: if true, round toward the low side
>>   */
>> -static inline u32 avs_tmon_temp_to_code(struct thermal_zone_device *tz,
>> +static inline u32 avs_tmon_temp_to_code(struct brcmstb_thermal_priv *priv,
>>  					int temp, bool low)
>>  {
>> +	int offset = priv->temp_params.offset;
>> +	int mult = priv->temp_params.mult;
>> +
>>  	if (temp < AVS_TMON_TEMP_MIN)
>> -		return AVS_TMON_TEMP_MAX;	/* Maximum code value */
>> +		return AVS_TMON_TEMP_MASK;	/* Maximum code value */
> 
> Why this change?

No reason to change, thanks.

> 
>>  
>> -	if (temp >= AVS_TMON_TEMP_OFFSET)
>> +	if (temp >= offset)
>>  		return 0;	/* Minimum code value */
>>  
>>  	if (low)
>> -		return (u32)(DIV_ROUND_UP(AVS_TMON_TEMP_OFFSET - temp,
>> -					  AVS_TMON_TEMP_SLOPE));
>> +		return (u32)(DIV_ROUND_UP(offset - temp, mult));
>>  	else
>> -		return (u32)((AVS_TMON_TEMP_OFFSET - temp) /
>> -			      AVS_TMON_TEMP_SLOPE);
>> +		return (u32)((offset - temp) / mult);
>>  }
>>  
>>  static int brcmstb_get_temp(void *data, int *temp)
>> @@ -154,7 +164,7 @@ static int brcmstb_get_temp(void *data, int *temp)
>>  
>>  	val = (val & AVS_TMON_STATUS_data_msk) >> AVS_TMON_STATUS_data_shift;
>>  
>> -	t = avs_tmon_code_to_temp(priv->thermal, val);
>> +	t = avs_tmon_code_to_temp(priv, val);
>>  	if (t < 0)
>>  		*temp = 0;
>>  	else
>> @@ -188,7 +198,7 @@ static int avs_tmon_get_trip_temp(struct brcmstb_thermal_priv *priv,
>>  	val &= trip->reg_msk;
>>  	val >>= trip->reg_shift;
>>  
>> -	return avs_tmon_code_to_temp(priv->thermal, val);
>> +	return avs_tmon_code_to_temp(priv, val);
>>  }
>>  
>>  static void avs_tmon_set_trip_temp(struct brcmstb_thermal_priv *priv,
>> @@ -201,7 +211,7 @@ static void avs_tmon_set_trip_temp(struct brcmstb_thermal_priv *priv,
>>  	dev_dbg(priv->dev, "set temp %d to %d\n", type, temp);
>>  
>>  	/* round toward low temp for the low interrupt */
>> -	val = avs_tmon_temp_to_code(priv->thermal, temp,
>> +	val = avs_tmon_temp_to_code(priv, temp,
>>  				    type == TMON_TRIP_TYPE_LOW);
>>  
>>  	val <<= trip->reg_shift;
>> @@ -218,7 +228,7 @@ static int avs_tmon_get_intr_temp(struct brcmstb_thermal_priv *priv)
>>  	u32 val;
>>  
>>  	val = __raw_readl(priv->tmon_base + AVS_TMON_TEMP_INT_CODE);
>> -	return avs_tmon_code_to_temp(priv->thermal, val);
>> +	return avs_tmon_code_to_temp(priv, val);
>>  }
>>  
>>  static irqreturn_t brcmstb_tmon_irq_thread(int irq, void *data)
>> @@ -282,19 +292,32 @@ static const struct thermal_zone_of_device_ops of_ops = {
>>  	.set_trips	= brcmstb_set_trips,
>>  };
>>  
>> +static const struct brcmstb_thermal_params brcmstb_28nm_params = {
>> +	.offset	= 410040,
>> +	.mult	= 487,
>> +};
>> +
>>  static const struct of_device_id brcmstb_thermal_id_table[] = {
>> -	{ .compatible = "brcm,avs-tmon" },
>> +	{ .compatible = "brcm,avs-tmon", .data = &brcmstb_28nm_params },
>>  	{},
>>  };
>>  MODULE_DEVICE_TABLE(of, brcmstb_thermal_id_table);
>>  
>>  static int brcmstb_thermal_probe(struct platform_device *pdev)
>>  {
>> +	const struct brcmstb_thermal_params *params;
>> +	const struct of_device_id *of_id = NULL;
>>  	struct thermal_zone_device *thermal;
>>  	struct brcmstb_thermal_priv *priv;
>>  	struct resource *res;
>>  	int irq, ret;
>>  
>> +	of_id = of_match_node(brcmstb_thermal_id_table, pdev->dev.of_node);
>> +	if (!of_id || !of_id->data)
>> +		return -EINVAL;
> 
> of_device_get_match_data(&pdev->dev) ?

Yes.

> 
>> +	params = of_id->data;
>> +
>>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>>  	if (!priv)
>>  		return -ENOMEM;
>> @@ -304,6 +327,7 @@ static int brcmstb_thermal_probe(struct platform_device *pdev)
>>  	if (IS_ERR(priv->tmon_base))
>>  		return PTR_ERR(priv->tmon_base);
>>  
>> +	memcpy(&priv->temp_params, params, sizeof(priv->temp_params));
> 
> Do you really need a copy here? Why not convert to a pointer and assign it?

I would rather not reference data that is possibly __initconst or
__initdata, this is not the case here but I have been burned by this
before, I will change it to a pointer.

Thanks!
-- 
Florian