devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] brcmstb_thermal updates for new processes
@ 2019-10-30 18:21 Florian Fainelli
  2019-10-30 18:21 ` [PATCH 1/6] thermal: brcmstb_thermal: Do not use DT coefficients Florian Fainelli
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Florian Fainelli @ 2019-10-30 18:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Markus Mayer,
	maintainer:BROADCOM STB AVS TMON DRIVER, Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Amit Kucheria, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

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!

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    | 108 ++++++++++--------
 2 files changed, 67 insertions(+), 49 deletions(-)

-- 
2.17.1


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

* [PATCH 1/6] thermal: brcmstb_thermal: Do not use DT coefficients
  2019-10-30 18:21 [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
@ 2019-10-30 18:21 ` Florian Fainelli
  2019-12-04  8:41   ` Amit Kucheria
  2019-10-30 18:21 ` [PATCH 2/6] thermal: brcmstb_thermal: Prepare to support a different process Florian Fainelli
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Florian Fainelli @ 2019-10-30 18:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Markus Mayer,
	maintainer:BROADCOM STB AVS TMON DRIVER, Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Amit Kucheria, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

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 | 37 ++++------------------
 1 file changed, 6 insertions(+), 31 deletions(-)

diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
index 5825ac581f56..42482af0422e 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -48,15 +48,6 @@
 #define AVS_TMON_TEMP_INT_CODE		0x1c
 #define AVS_TMON_TP_TEST_ENABLE		0x20
 
-/* Default coefficients */
-#define AVS_TMON_TEMP_SLOPE		-487
-#define AVS_TMON_TEMP_OFFSET		410040
-
-/* HW related temperature constants */
-#define AVS_TMON_TEMP_MAX		0x3ff
-#define AVS_TMON_TEMP_MIN		-88161
-#define AVS_TMON_TEMP_MASK		AVS_TMON_TEMP_MAX
-
 enum avs_tmon_trip_type {
 	TMON_TRIP_TYPE_LOW = 0,
 	TMON_TRIP_TYPE_HIGH,
@@ -108,23 +99,11 @@ 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 (410040 - (int)((code & 0x3FF) * 487));
 }
 
 /*
@@ -136,20 +115,16 @@ 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);
+	if (temp < -88161)
+		return 0x3FF;	/* Maximum code value */
 
-	if (temp >= offset)
+	if (temp >= 410040)
 		return 0;	/* Minimum code value */
 
 	if (low)
-		return (u32)(DIV_ROUND_UP(offset - temp, abs(slope)));
+		return (u32)(DIV_ROUND_UP(410040 - temp, 487));
 	else
-		return (u32)((offset - temp) / abs(slope));
+		return (u32)((410040 - temp) / 487);
 }
 
 static int brcmstb_get_temp(void *data, int *temp)
-- 
2.17.1


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

* [PATCH 2/6] thermal: brcmstb_thermal: Prepare to support a different process
  2019-10-30 18:21 [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
  2019-10-30 18:21 ` [PATCH 1/6] thermal: brcmstb_thermal: Do not use DT coefficients Florian Fainelli
@ 2019-10-30 18:21 ` Florian Fainelli
  2019-10-30 18:21 ` [PATCH 3/6] dt-bindings: thermal: Define BCM7216 thermal sensor compatible Florian Fainelli
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Florian Fainelli @ 2019-10-30 18:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Markus Mayer,
	maintainer:BROADCOM STB AVS TMON DRIVER, Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Amit Kucheria, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

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 | 49 +++++++++++++++++-----
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
index 42482af0422e..2ad4eeb79f9c 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -93,17 +93,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 (410040 - (int)((code & 0x3FF) * 487));
+	int offset = priv->temp_params.offset;
+	int mult = priv->temp_params.mult;
+
+	return (offset - (int)((code & 0x3FF) * mult));
 }
 
 /*
@@ -112,19 +122,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 < -88161)
 		return 0x3FF;	/* Maximum code value */
 
-	if (temp >= 410040)
+	if (temp >= offset)
 		return 0;	/* Minimum code value */
 
 	if (low)
-		return (u32)(DIV_ROUND_UP(410040 - temp, 487));
+		return (u32)(DIV_ROUND_UP(offset - temp, mult));
 	else
-		return (u32)((410040 - temp) / 487);
+		return (u32)((offset - temp) / mult);
 }
 
 static int brcmstb_get_temp(void *data, int *temp)
@@ -142,7 +155,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
@@ -176,7 +189,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,
@@ -189,7 +202,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;
@@ -206,7 +219,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)
@@ -270,19 +283,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;
@@ -292,6 +318,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


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

* [PATCH 3/6] dt-bindings: thermal: Define BCM7216 thermal sensor compatible
  2019-10-30 18:21 [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
  2019-10-30 18:21 ` [PATCH 1/6] thermal: brcmstb_thermal: Do not use DT coefficients Florian Fainelli
  2019-10-30 18:21 ` [PATCH 2/6] thermal: brcmstb_thermal: Prepare to support a different process Florian Fainelli
@ 2019-10-30 18:21 ` Florian Fainelli
  2019-11-06  3:55   ` Rob Herring
  2019-10-30 18:21 ` [PATCH 4/6] thermal: brcmstb_thermal: Add 16nm process thermal parameters Florian Fainelli
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Florian Fainelli @ 2019-10-30 18:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Markus Mayer,
	maintainer:BROADCOM STB AVS TMON DRIVER, Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Amit Kucheria, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

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

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


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

* [PATCH 4/6] thermal: brcmstb_thermal: Add 16nm process thermal parameters
  2019-10-30 18:21 [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
                   ` (2 preceding siblings ...)
  2019-10-30 18:21 ` [PATCH 3/6] dt-bindings: thermal: Define BCM7216 thermal sensor compatible Florian Fainelli
@ 2019-10-30 18:21 ` Florian Fainelli
  2019-12-04  8:43   ` Amit Kucheria
  2019-10-30 18:21 ` [PATCH 5/6] thermal: brcmstb_thermal: Restructure interrupt registration Florian Fainelli
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Florian Fainelli @ 2019-10-30 18:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Markus Mayer,
	maintainer:BROADCOM STB AVS TMON DRIVER, Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Amit Kucheria, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

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

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 2ad4eeb79f9c..41d4a142707c 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -283,12 +283,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


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

* [PATCH 5/6] thermal: brcmstb_thermal: Restructure interrupt registration
  2019-10-30 18:21 [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
                   ` (3 preceding siblings ...)
  2019-10-30 18:21 ` [PATCH 4/6] thermal: brcmstb_thermal: Add 16nm process thermal parameters Florian Fainelli
@ 2019-10-30 18:21 ` Florian Fainelli
  2019-12-04  9:38   ` Amit Kucheria
  2019-10-30 18:21 ` [PATCH 6/6] thermal: brcmstb_thermal: Register different ops per process Florian Fainelli
  2019-11-20 18:43 ` [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
  6 siblings, 1 reply; 18+ messages in thread
From: Florian Fainelli @ 2019-10-30 18:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Markus Mayer,
	maintainer:BROADCOM STB AVS TMON DRIVER, Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Amit Kucheria, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

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.

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 41d4a142707c..64f715053ce9 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -339,16 +339,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


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

* [PATCH 6/6] thermal: brcmstb_thermal: Register different ops per process
  2019-10-30 18:21 [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
                   ` (4 preceding siblings ...)
  2019-10-30 18:21 ` [PATCH 5/6] thermal: brcmstb_thermal: Restructure interrupt registration Florian Fainelli
@ 2019-10-30 18:21 ` Florian Fainelli
  2019-12-04  9:38   ` Amit Kucheria
  2019-11-20 18:43 ` [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
  6 siblings, 1 reply; 18+ messages in thread
From: Florian Fainelli @ 2019-10-30 18:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Markus Mayer,
	maintainer:BROADCOM STB AVS TMON DRIVER, Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Amit Kucheria, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

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

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 64f715053ce9..a75a335d1bb3 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -96,6 +96,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 {
@@ -278,19 +279,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[] = {
@@ -329,7 +336,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


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

* Re: [PATCH 3/6] dt-bindings: thermal: Define BCM7216 thermal sensor compatible
  2019-10-30 18:21 ` [PATCH 3/6] dt-bindings: thermal: Define BCM7216 thermal sensor compatible Florian Fainelli
@ 2019-11-06  3:55   ` Rob Herring
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2019-11-06  3:55 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-kernel, Florian Fainelli, Markus Mayer,
	maintainer:BROADCOM STB AVS TMON DRIVER, Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Amit Kucheria, Mark Rutland,
	open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

On Wed, 30 Oct 2019 11:21:29 -0700, Florian Fainelli wrote:
> BCM7216 is a 16nm process STB chip, which requires a different
> compatible string to differentiate different temperature formulas.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  .../devicetree/bindings/thermal/brcm,avs-tmon.txt         | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 0/6] brcmstb_thermal updates for new processes
  2019-10-30 18:21 [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
                   ` (5 preceding siblings ...)
  2019-10-30 18:21 ` [PATCH 6/6] thermal: brcmstb_thermal: Register different ops per process Florian Fainelli
@ 2019-11-20 18:43 ` Florian Fainelli
  2019-12-04  4:50   ` Florian Fainelli
  6 siblings, 1 reply; 18+ messages in thread
From: Florian Fainelli @ 2019-11-20 18:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Markus Mayer, maintainer:BROADCOM STB AVS TMON DRIVER, Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Amit Kucheria, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE



On 10/30/2019 11:21 AM, 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!

Ping?

> 
> 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    | 108 ++++++++++--------
>  2 files changed, 67 insertions(+), 49 deletions(-)
> 

-- 
Florian

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

* Re: [PATCH 0/6] brcmstb_thermal updates for new processes
  2019-11-20 18:43 ` [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
@ 2019-12-04  4:50   ` Florian Fainelli
  2019-12-09  9:04     ` Daniel Lezcano
  0 siblings, 1 reply; 18+ messages in thread
From: Florian Fainelli @ 2019-12-04  4:50 UTC (permalink / raw)
  To: linux-kernel, Zhang Rui, Eduardo Valentin, Amit Kucheria
  Cc: Markus Mayer, maintainer:BROADCOM STB AVS TMON DRIVER,
	Daniel Lezcano, Rob Herring, Mark Rutland,
	open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE



On 11/20/2019 10:43 AM, Florian Fainelli wrote:
> 
> 
> On 10/30/2019 11:21 AM, 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!
> 
> Ping?

Rui, anyone?

> 
>>
>> 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    | 108 ++++++++++--------
>>  2 files changed, 67 insertions(+), 49 deletions(-)
>>
> 

-- 
Florian

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

* Re: [PATCH 1/6] thermal: brcmstb_thermal: Do not use DT coefficients
  2019-10-30 18:21 ` [PATCH 1/6] thermal: brcmstb_thermal: Do not use DT coefficients Florian Fainelli
@ 2019-12-04  8:41   ` Amit Kucheria
  2019-12-04 17:41     ` Florian Fainelli
  0 siblings, 1 reply; 18+ messages in thread
From: Amit Kucheria @ 2019-12-04  8:41 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: LKML, Markus Mayer, maintainer:BROADCOM STB AVS TMON DRIVER,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

On Wed, Oct 30, 2019 at 11:51 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> 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")

I think you should fix the computation formula as the first patch and
then merge the rest of this patch into your second patch.

I don't think the intermediate state of converting named constants to
magic numbers is needed just to convert it over to another set of
parameters.

Regards,
Amit

> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/thermal/broadcom/brcmstb_thermal.c | 37 ++++------------------
>  1 file changed, 6 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
> index 5825ac581f56..42482af0422e 100644
> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
> @@ -48,15 +48,6 @@
>  #define AVS_TMON_TEMP_INT_CODE         0x1c
>  #define AVS_TMON_TP_TEST_ENABLE                0x20
>
> -/* Default coefficients */
> -#define AVS_TMON_TEMP_SLOPE            -487
> -#define AVS_TMON_TEMP_OFFSET           410040
> -
> -/* HW related temperature constants */
> -#define AVS_TMON_TEMP_MAX              0x3ff
> -#define AVS_TMON_TEMP_MIN              -88161
> -#define AVS_TMON_TEMP_MASK             AVS_TMON_TEMP_MAX
> -
>  enum avs_tmon_trip_type {
>         TMON_TRIP_TYPE_LOW = 0,
>         TMON_TRIP_TYPE_HIGH,
> @@ -108,23 +99,11 @@ 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 (410040 - (int)((code & 0x3FF) * 487));
>  }
>
>  /*
> @@ -136,20 +115,16 @@ 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);
> +       if (temp < -88161)
> +               return 0x3FF;   /* Maximum code value */
>
> -       if (temp >= offset)
> +       if (temp >= 410040)
>                 return 0;       /* Minimum code value */
>
>         if (low)
> -               return (u32)(DIV_ROUND_UP(offset - temp, abs(slope)));
> +               return (u32)(DIV_ROUND_UP(410040 - temp, 487));
>         else
> -               return (u32)((offset - temp) / abs(slope));
> +               return (u32)((410040 - temp) / 487);
>  }
>
>  static int brcmstb_get_temp(void *data, int *temp)
> --
> 2.17.1
>

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

* Re: [PATCH 4/6] thermal: brcmstb_thermal: Add 16nm process thermal parameters
  2019-10-30 18:21 ` [PATCH 4/6] thermal: brcmstb_thermal: Add 16nm process thermal parameters Florian Fainelli
@ 2019-12-04  8:43   ` Amit Kucheria
  0 siblings, 0 replies; 18+ messages in thread
From: Amit Kucheria @ 2019-12-04  8:43 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: LKML, Markus Mayer, maintainer:BROADCOM STB AVS TMON DRIVER,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

On Wed, Oct 30, 2019 at 11:52 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> Match the 7216 compatible string in order to derive the correct 16nm
> process thermal parameters to obtain correct readings.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

> ---
>  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 2ad4eeb79f9c..41d4a142707c 100644
> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
> @@ -283,12 +283,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
>

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

* Re: [PATCH 5/6] thermal: brcmstb_thermal: Restructure interrupt registration
  2019-10-30 18:21 ` [PATCH 5/6] thermal: brcmstb_thermal: Restructure interrupt registration Florian Fainelli
@ 2019-12-04  9:38   ` Amit Kucheria
  0 siblings, 0 replies; 18+ messages in thread
From: Amit Kucheria @ 2019-12-04  9:38 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: LKML, Markus Mayer, maintainer:BROADCOM STB AVS TMON DRIVER,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

On Wed, Oct 30, 2019 at 11:52 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> 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.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

> ---
>  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 41d4a142707c..64f715053ce9 100644
> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
> @@ -339,16 +339,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
>

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

* Re: [PATCH 6/6] thermal: brcmstb_thermal: Register different ops per process
  2019-10-30 18:21 ` [PATCH 6/6] thermal: brcmstb_thermal: Register different ops per process Florian Fainelli
@ 2019-12-04  9:38   ` Amit Kucheria
  0 siblings, 0 replies; 18+ messages in thread
From: Amit Kucheria @ 2019-12-04  9:38 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: LKML, Markus Mayer, maintainer:BROADCOM STB AVS TMON DRIVER,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

On Wed, Oct 30, 2019 at 11:52 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> 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
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

> ---
>  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 64f715053ce9..a75a335d1bb3 100644
> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
> @@ -96,6 +96,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 {
> @@ -278,19 +279,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[] = {
> @@ -329,7 +336,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
>

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

* Re: [PATCH 1/6] thermal: brcmstb_thermal: Do not use DT coefficients
  2019-12-04  8:41   ` Amit Kucheria
@ 2019-12-04 17:41     ` Florian Fainelli
  2019-12-04 19:01       ` Amit Kucheria
  0 siblings, 1 reply; 18+ messages in thread
From: Florian Fainelli @ 2019-12-04 17:41 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: LKML, Markus Mayer, maintainer:BROADCOM STB AVS TMON DRIVER,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

On 12/4/19 12:41 AM, Amit Kucheria wrote:
> On Wed, Oct 30, 2019 at 11:51 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> 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")
> 
> I think you should fix the computation formula as the first patch and
> then merge the rest of this patch into your second patch.
> 
> I don't think the intermediate state of converting named constants to
> magic numbers is needed just to convert it over to another set of
> parameters.

ok, so you would rather see this patch using the defined constants while
fixing the formula, and in a subsequent patch getting rid of the
constant names since they are going to be added to a per-process table,
does that sound reasonable?

> 
> Regards,
> Amit
> 
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  drivers/thermal/broadcom/brcmstb_thermal.c | 37 ++++------------------
>>  1 file changed, 6 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
>> index 5825ac581f56..42482af0422e 100644
>> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
>> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
>> @@ -48,15 +48,6 @@
>>  #define AVS_TMON_TEMP_INT_CODE         0x1c
>>  #define AVS_TMON_TP_TEST_ENABLE                0x20
>>
>> -/* Default coefficients */
>> -#define AVS_TMON_TEMP_SLOPE            -487
>> -#define AVS_TMON_TEMP_OFFSET           410040
>> -
>> -/* HW related temperature constants */
>> -#define AVS_TMON_TEMP_MAX              0x3ff
>> -#define AVS_TMON_TEMP_MIN              -88161
>> -#define AVS_TMON_TEMP_MASK             AVS_TMON_TEMP_MAX
>> -
>>  enum avs_tmon_trip_type {
>>         TMON_TRIP_TYPE_LOW = 0,
>>         TMON_TRIP_TYPE_HIGH,
>> @@ -108,23 +99,11 @@ 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 (410040 - (int)((code & 0x3FF) * 487));
>>  }
>>
>>  /*
>> @@ -136,20 +115,16 @@ 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);
>> +       if (temp < -88161)
>> +               return 0x3FF;   /* Maximum code value */
>>
>> -       if (temp >= offset)
>> +       if (temp >= 410040)
>>                 return 0;       /* Minimum code value */
>>
>>         if (low)
>> -               return (u32)(DIV_ROUND_UP(offset - temp, abs(slope)));
>> +               return (u32)(DIV_ROUND_UP(410040 - temp, 487));
>>         else
>> -               return (u32)((offset - temp) / abs(slope));
>> +               return (u32)((410040 - temp) / 487);
>>  }
>>
>>  static int brcmstb_get_temp(void *data, int *temp)
>> --
>> 2.17.1
>>


-- 
Florian

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

* Re: [PATCH 1/6] thermal: brcmstb_thermal: Do not use DT coefficients
  2019-12-04 17:41     ` Florian Fainelli
@ 2019-12-04 19:01       ` Amit Kucheria
  0 siblings, 0 replies; 18+ messages in thread
From: Amit Kucheria @ 2019-12-04 19:01 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: LKML, Markus Mayer, maintainer:BROADCOM STB AVS TMON DRIVER,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

On Wed, Dec 4, 2019 at 11:12 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 12/4/19 12:41 AM, Amit Kucheria wrote:
> > On Wed, Oct 30, 2019 at 11:51 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>
> >> 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")
> >
> > I think you should fix the computation formula as the first patch and
> > then merge the rest of this patch into your second patch.
> >
> > I don't think the intermediate state of converting named constants to
> > magic numbers is needed just to convert it over to another set of
> > parameters.
>
> ok, so you would rather see this patch using the defined constants while
> fixing the formula, and in a subsequent patch getting rid of the
> constant names since they are going to be added to a per-process table,
> does that sound reasonable?

Yes, that way the first patch would fix a bug (and with a Fixes tag
potentially gets backported to stable), the second would convert over
to the per-process table.

Regards,
Amit

> >
> > Regards,
> > Amit
> >
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >>  drivers/thermal/broadcom/brcmstb_thermal.c | 37 ++++------------------
> >>  1 file changed, 6 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
> >> index 5825ac581f56..42482af0422e 100644
> >> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
> >> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
> >> @@ -48,15 +48,6 @@
> >>  #define AVS_TMON_TEMP_INT_CODE         0x1c
> >>  #define AVS_TMON_TP_TEST_ENABLE                0x20
> >>
> >> -/* Default coefficients */
> >> -#define AVS_TMON_TEMP_SLOPE            -487
> >> -#define AVS_TMON_TEMP_OFFSET           410040
> >> -
> >> -/* HW related temperature constants */
> >> -#define AVS_TMON_TEMP_MAX              0x3ff
> >> -#define AVS_TMON_TEMP_MIN              -88161
> >> -#define AVS_TMON_TEMP_MASK             AVS_TMON_TEMP_MAX
> >> -
> >>  enum avs_tmon_trip_type {
> >>         TMON_TRIP_TYPE_LOW = 0,
> >>         TMON_TRIP_TYPE_HIGH,
> >> @@ -108,23 +99,11 @@ 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 (410040 - (int)((code & 0x3FF) * 487));
> >>  }
> >>l
> >>  /*
> >> @@ -136,20 +115,16 @@ 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);
> >> +       if (temp < -88161)
> >> +               return 0x3FF;   /* Maximum code value */
> >>
> >> -       if (temp >= offset)
> >> +       if (temp >= 410040)
> >>                 return 0;       /* Minimum code value */
> >>
> >>         if (low)
> >> -               return (u32)(DIV_ROUND_UP(offset - temp, abs(slope)));
> >> +               return (u32)(DIV_ROUND_UP(410040 - temp, 487));
> >>         else
> >> -               return (u32)((offset - temp) / abs(slope));
> >> +               return (u32)((410040 - temp) / 487);
> >>  }
> >>
> >>  static int brcmstb_get_temp(void *data, int *temp)
> >> --
> >> 2.17.1
> >>
>
>
> --
> Florian

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

* Re: [PATCH 0/6] brcmstb_thermal updates for new processes
  2019-12-04  4:50   ` Florian Fainelli
@ 2019-12-09  9:04     ` Daniel Lezcano
  2019-12-09 17:36       ` Florian Fainelli
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Lezcano @ 2019-12-09  9:04 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel, Zhang Rui, Eduardo Valentin,
	Amit Kucheria
  Cc: Markus Mayer, maintainer:BROADCOM STB AVS TMON DRIVER,
	Rob Herring, Mark Rutland,
	open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE


Hi Florian,

On 04/12/2019 05:50, Florian Fainelli wrote:
> 
> 
> On 11/20/2019 10:43 AM, Florian Fainelli wrote:
>>
>>
>> On 10/30/2019 11:21 AM, 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!
>>
>> Ping?
> 
> Rui, anyone?

Given comments on patch 1, I'm waiting for the V2.



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


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

* Re: [PATCH 0/6] brcmstb_thermal updates for new processes
  2019-12-09  9:04     ` Daniel Lezcano
@ 2019-12-09 17:36       ` Florian Fainelli
  0 siblings, 0 replies; 18+ messages in thread
From: Florian Fainelli @ 2019-12-09 17:36 UTC (permalink / raw)
  To: Daniel Lezcano, Florian Fainelli, linux-kernel, Zhang Rui,
	Eduardo Valentin, Amit Kucheria
  Cc: Markus Mayer, maintainer:BROADCOM STB AVS TMON DRIVER,
	Rob Herring, Mark Rutland,
	open list:BROADCOM STB AVS TMON DRIVER,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE

On 12/9/19 1:04 AM, Daniel Lezcano wrote:
> 
> Hi Florian,
> 
> On 04/12/2019 05:50, Florian Fainelli wrote:
>>
>>
>> On 11/20/2019 10:43 AM, Florian Fainelli wrote:
>>>
>>>
>>> On 10/30/2019 11:21 AM, 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!
>>>
>>> Ping?
>>
>> Rui, anyone?
> 
> Given comments on patch 1, I'm waiting for the V2.

Yes, I will follow up with a v2 later this week. Thanks
-- 
Florian

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

end of thread, other threads:[~2019-12-09 17:36 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-30 18:21 [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
2019-10-30 18:21 ` [PATCH 1/6] thermal: brcmstb_thermal: Do not use DT coefficients Florian Fainelli
2019-12-04  8:41   ` Amit Kucheria
2019-12-04 17:41     ` Florian Fainelli
2019-12-04 19:01       ` Amit Kucheria
2019-10-30 18:21 ` [PATCH 2/6] thermal: brcmstb_thermal: Prepare to support a different process Florian Fainelli
2019-10-30 18:21 ` [PATCH 3/6] dt-bindings: thermal: Define BCM7216 thermal sensor compatible Florian Fainelli
2019-11-06  3:55   ` Rob Herring
2019-10-30 18:21 ` [PATCH 4/6] thermal: brcmstb_thermal: Add 16nm process thermal parameters Florian Fainelli
2019-12-04  8:43   ` Amit Kucheria
2019-10-30 18:21 ` [PATCH 5/6] thermal: brcmstb_thermal: Restructure interrupt registration Florian Fainelli
2019-12-04  9:38   ` Amit Kucheria
2019-10-30 18:21 ` [PATCH 6/6] thermal: brcmstb_thermal: Register different ops per process Florian Fainelli
2019-12-04  9:38   ` Amit Kucheria
2019-11-20 18:43 ` [PATCH 0/6] brcmstb_thermal updates for new processes Florian Fainelli
2019-12-04  4:50   ` Florian Fainelli
2019-12-09  9:04     ` Daniel Lezcano
2019-12-09 17:36       ` Florian Fainelli

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