[PATCH v6 00/11] Armada thermal: improvements and A7K/A8K SoCs support

[PATCH v6 00/11] Armada thermal: improvements and A7K/A8K SoCs support

[Miquel Raynal]

Hi,

This series takes over Baruch's series by integrating his patches about
supporting thermal on Armada 7K and 8K SoCs within a larger series with
several improvements on the armada_thermal.c driver.

For now, Armada 380 and CP110 compatibles share the same initialization
routine but this will probably change in the near future when adding
support for overheat interrupts.

DT bindings documentation is updated to match existing code.

Armada AP806 and CP110 DT are also updated with thermal nodes and have
already been accepted (thus absent in this series).

Thank you,
Miqu?l


Changes since v5:
  - Added Grogory's Tested-by tags for 64-bit platforms in patches:
        "thermal: armada: Add support for Armada AP806"
	"thermal: armada: Add support for Armada CP110"
  - Fixed a compilation issue with 32-bit toolchains introduced in patch:
        "thermal: armada: Add support for Armada AP806"


Baruch Siach (4):
  dt-bindings: thermal: Describe Armada AP806 and CP110
  thermal: armada: Use msleep for long delays
  thermal: armada: Add support for Armada AP806
  thermal: armada: Add support for Armada CP110

Miquel Raynal (7):
  thermal: armada: Simplify the check of the validity bit
  thermal: armada: Clarify control registers accesses
  thermal: armada: Use real status register name
  thermal: armada: Update Kconfig and module description
  thermal: armada: Change sensors trim default value
  thermal: armada: Wait sensors validity before exiting the init
    callback
  thermal: armada: Give meaningful names to the thermal zones

 .../devicetree/bindings/thermal/armada-thermal.txt |  37 ++-
 drivers/thermal/Kconfig                            |   4 +-
 drivers/thermal/armada_thermal.c                   | 257 +++++++++++++++------
 3 files changed, 218 insertions(+), 80 deletions(-)

-- 
2.11.0

[PATCH v6 01/11] dt-bindings: thermal: Describe Armada AP806 and CP110

[Miquel Raynal]

From: Baruch Siach <baruch@tkos.co.il>

Add compatible strings for AP806 and CP110 that are part of the Armada
8k/7k line of SoCs.

Add a note on the differences in the size of the control area in
different bindings. This is an existing difference between the Armada
375 binding and the other boards already supported. The new AP806 and
CP110 bindings are similar to the existing Armada 375 in this regard.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
[<miquel.raynal@free-electrons.com>: reword, additional details]
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
---
 .../devicetree/bindings/thermal/armada-thermal.txt | 37 +++++++++++++++-------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
index 24aacf8948c5..e0d013a2e66d 100644
--- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
@@ -2,22 +2,35 @@
 
 Required properties:
 
-- compatible:	Should be set to one of the following:
-		marvell,armada370-thermal
-		marvell,armada375-thermal
-		marvell,armada380-thermal
-		marvell,armadaxp-thermal
+- compatible: Should be set to one of the following:
+    * marvell,armada370-thermal
+    * marvell,armada375-thermal
+    * marvell,armada380-thermal
+    * marvell,armadaxp-thermal
+    * marvell,armada-ap806-thermal
+    * marvell,armada-cp110-thermal
 
-- reg:		Device's register space.
-		Two entries are expected, see the examples below.
-		The first one is required for the sensor register;
-		the second one is required for the control register
-		to be used for sensor initialization (a.k.a. calibration).
+- reg: Device's register space.
+  Two entries are expected, see the examples below. The first one points
+  to the status register (4B). The second one points to the control
+  registers (8B).
+  Note: The compatibles marvell,armada370-thermal,
+  marvell,armada380-thermal, and marvell,armadaxp-thermal must point to
+  "control MSB/control 1", with size of 4 (deprecated binding), or point
+  to "control LSB/control 0" with size of 8 (current binding). All other
+  compatibles must point to "control LSB/control 0" with size of 8.
 
-Example:
+Examples:
 
+	/* Legacy bindings */
 	thermal at d0018300 {
 		compatible = "marvell,armada370-thermal";
-                reg = <0xd0018300 0x4
+		reg = <0xd0018300 0x4
 		       0xd0018304 0x4>;
 	};
+
+	ap_thermal: thermal at 6f8084 {
+		compatible = "marvell,armada-ap806-thermal";
+		reg = <0x6f808C 0x4>,
+		      <0x6f8084 0x8>;
+	};
-- 
2.11.0

[PATCH v6 01/11] dt-bindings: thermal: Describe Armada AP806 and CP110

[Miquel RAYNAL]

Hi Rob,


On Fri, 22 Dec 2017 10:32:16 +0100
Miquel Raynal <miquel.raynal@free-electrons.com> wrote:

> From: Baruch Siach <baruch@tkos.co.il>
> 
> Add compatible strings for AP806 and CP110 that are part of the Armada
> 8k/7k line of SoCs.
> 
> Add a note on the differences in the size of the control area in
> different bindings. This is an existing difference between the Armada
> 375 binding and the other boards already supported. The new AP806 and
> CP110 bindings are similar to the existing Armada 375 in this regard.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> [<miquel.raynal@free-electrons.com>: reword, additional details]
> Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>

I forgot to add your:

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

in this series, sorry about that.

Kind regards,
Miqu?l


> ---
>  .../devicetree/bindings/thermal/armada-thermal.txt | 37
> +++++++++++++++------- 1 file changed, 25 insertions(+), 12
> deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> b/Documentation/devicetree/bindings/thermal/armada-thermal.txt index
> 24aacf8948c5..e0d013a2e66d 100644 ---
> a/Documentation/devicetree/bindings/thermal/armada-thermal.txt +++
> b/Documentation/devicetree/bindings/thermal/armada-thermal.txt @@
> -2,22 +2,35 @@ Required properties:
>  
> -- compatible:	Should be set to one of the following:
> -		marvell,armada370-thermal
> -		marvell,armada375-thermal
> -		marvell,armada380-thermal
> -		marvell,armadaxp-thermal
> +- compatible: Should be set to one of the following:
> +    * marvell,armada370-thermal
> +    * marvell,armada375-thermal
> +    * marvell,armada380-thermal
> +    * marvell,armadaxp-thermal
> +    * marvell,armada-ap806-thermal
> +    * marvell,armada-cp110-thermal
>  
> -- reg:		Device's register space.
> -		Two entries are expected, see the examples below.
> -		The first one is required for the sensor register;
> -		the second one is required for the control register
> -		to be used for sensor initialization (a.k.a.
> calibration). +- reg: Device's register space.
> +  Two entries are expected, see the examples below. The first one
> points
> +  to the status register (4B). The second one points to the control
> +  registers (8B).
> +  Note: The compatibles marvell,armada370-thermal,
> +  marvell,armada380-thermal, and marvell,armadaxp-thermal must point
> to
> +  "control MSB/control 1", with size of 4 (deprecated binding), or
> point
> +  to "control LSB/control 0" with size of 8 (current binding). All
> other
> +  compatibles must point to "control LSB/control 0" with size of 8.
>  
> -Example:
> +Examples:
>  
> +	/* Legacy bindings */
>  	thermal at d0018300 {
>  		compatible = "marvell,armada370-thermal";
> -                reg = <0xd0018300 0x4
> +		reg = <0xd0018300 0x4
>  		       0xd0018304 0x4>;
>  	};
> +
> +	ap_thermal: thermal at 6f8084 {
> +		compatible = "marvell,armada-ap806-thermal";
> +		reg = <0x6f808C 0x4>,
> +		      <0x6f8084 0x8>;
> +	};



-- 
Miquel Raynal, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v6 02/11] thermal: armada: Use msleep for long delays

[Miquel Raynal]

From: Baruch Siach <baruch@tkos.co.il>

Use msleep for long (> 10ms) delays, instead of the busy waiting mdelay.
All delays are called from the probe routine, where scheduling is
allowed.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 706d74798cbe..6c4af2622d4f 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -113,7 +113,7 @@ static void armada370_init_sensor(struct platform_device *pdev,
 	reg &= ~PMU_TDC0_START_CAL_MASK;
 	writel(reg, priv->control);
 
-	mdelay(10);
+	msleep(10);
 }
 
 static void armada375_init_sensor(struct platform_device *pdev,
@@ -127,11 +127,11 @@ static void armada375_init_sensor(struct platform_device *pdev,
 	reg &= ~A375_HW_RESETn;
 
 	writel(reg, priv->control + 4);
-	mdelay(20);
+	msleep(20);
 
 	reg |= A375_HW_RESETn;
 	writel(reg, priv->control + 4);
-	mdelay(50);
+	msleep(50);
 }
 
 static void armada380_init_sensor(struct platform_device *pdev,
@@ -143,7 +143,7 @@ static void armada380_init_sensor(struct platform_device *pdev,
 	if (!(reg & A380_HW_RESET)) {
 		reg |= A380_HW_RESET;
 		writel(reg, priv->control);
-		mdelay(10);
+		msleep(10);
 	}
 }
 
-- 
2.11.0

[PATCH v6 03/11] thermal: armada: Simplify the check of the validity bit

[Miquel Raynal]

All Armada SoCs use one bit to declare if the sensor values are valid.
This bit moves across the versions of the IP.

The method until then was to do both a shift and compare with an useless
flag of "0x1". It is clearer and quicker to directly save the value that
must be ANDed instead of the bit position and do a single bitwise AND
operation.

Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 6c4af2622d4f..f350d7efd35a 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -24,8 +24,6 @@
 #include <linux/of_device.h>
 #include <linux/thermal.h>
 
-#define THERMAL_VALID_MASK		0x1
-
 /* Thermal Manager Control and Status Register */
 #define PMU_TDC0_SW_RST_MASK		(0x1 << 1)
 #define PMU_TM_DISABLE_OFFS		0
@@ -67,7 +65,7 @@ struct armada_thermal_data {
 	/* Register shift and mask to access the sensor temperature */
 	unsigned int temp_shift;
 	unsigned int temp_mask;
-	unsigned int is_valid_shift;
+	u32 is_valid_bit;
 };
 
 static void armadaxp_init_sensor(struct platform_device *pdev,
@@ -149,9 +147,9 @@ static void armada380_init_sensor(struct platform_device *pdev,
 
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
-	unsigned long reg = readl_relaxed(priv->sensor);
+	u32 reg = readl_relaxed(priv->sensor);
 
-	return (reg >> priv->data->is_valid_shift) & THERMAL_VALID_MASK;
+	return reg & priv->data->is_valid_bit;
 }
 
 static int armada_get_temp(struct thermal_zone_device *thermal,
@@ -199,7 +197,7 @@ static const struct armada_thermal_data armadaxp_data = {
 static const struct armada_thermal_data armada370_data = {
 	.is_valid = armada_is_valid,
 	.init_sensor = armada370_init_sensor,
-	.is_valid_shift = 9,
+	.is_valid_bit = BIT(9),
 	.temp_shift = 10,
 	.temp_mask = 0x1ff,
 	.coef_b = 3153000000UL,
@@ -210,7 +208,7 @@ static const struct armada_thermal_data armada370_data = {
 static const struct armada_thermal_data armada375_data = {
 	.is_valid = armada_is_valid,
 	.init_sensor = armada375_init_sensor,
-	.is_valid_shift = 10,
+	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x1ff,
 	.coef_b = 3171900000UL,
@@ -221,7 +219,7 @@ static const struct armada_thermal_data armada375_data = {
 static const struct armada_thermal_data armada380_data = {
 	.is_valid = armada_is_valid,
 	.init_sensor = armada380_init_sensor,
-	.is_valid_shift = 10,
+	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
 	.coef_b = 1172499100UL,
-- 
2.11.0

[PATCH v6 04/11] thermal: armada: Clarify control registers accesses

[Miquel Raynal]

Bindings were incomplete for a long time by only exposing one of the two
available control registers. To ease the migration to the full bindings
(already in use for the Armada 375 SoC), rename the pointers for
clarification. This way, it will only be needed to add another pointer
to access the other control register when the time comes.

This avoids dangerous situations where the offset 0 of the control
area can be either one register or the other depending on the bindings
used. After this change, device trees of other SoCs could be migrated to
the "full" bindings if they may benefit from features from the
unaccessible register, without any change in the driver.

Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 76 ++++++++++++++++++++++++++++------------
 1 file changed, 54 insertions(+), 22 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index f350d7efd35a..d58376eba6d9 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -39,12 +39,21 @@
 #define A375_HW_RESETn			BIT(8)
 #define A380_HW_RESET			BIT(8)
 
+/* Legacy bindings */
+#define LEGACY_CONTROL_MEM_LEN		0x4
+
+/* Current bindings with the 2 control registers under the same memory area */
+#define LEGACY_CONTROL1_OFFSET		0x0
+#define CONTROL0_OFFSET			0x0
+#define CONTROL1_OFFSET			0x4
+
 struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
 struct armada_thermal_priv {
 	void __iomem *sensor;
-	void __iomem *control;
+	void __iomem *control0;
+	void __iomem *control1;
 	struct armada_thermal_data *data;
 };
 
@@ -66,27 +75,28 @@ struct armada_thermal_data {
 	unsigned int temp_shift;
 	unsigned int temp_mask;
 	u32 is_valid_bit;
+	bool needs_control0;
 };
 
 static void armadaxp_init_sensor(struct platform_device *pdev,
 				 struct armada_thermal_priv *priv)
 {
-	unsigned long reg;
+	u32 reg;
 
-	reg = readl_relaxed(priv->control);
+	reg = readl_relaxed(priv->control1);
 	reg |= PMU_TDC0_OTF_CAL_MASK;
-	writel(reg, priv->control);
+	writel(reg, priv->control1);
 
 	/* Reference calibration value */
 	reg &= ~PMU_TDC0_REF_CAL_CNT_MASK;
 	reg |= (0xf1 << PMU_TDC0_REF_CAL_CNT_OFFS);
-	writel(reg, priv->control);
+	writel(reg, priv->control1);
 
 	/* Reset the sensor */
-	reg = readl_relaxed(priv->control);
-	writel((reg | PMU_TDC0_SW_RST_MASK), priv->control);
+	reg = readl_relaxed(priv->control1);
+	writel((reg | PMU_TDC0_SW_RST_MASK), priv->control1);
 
-	writel(reg, priv->control);
+	writel(reg, priv->control1);
 
 	/* Enable the sensor */
 	reg = readl_relaxed(priv->sensor);
@@ -97,19 +107,19 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
 static void armada370_init_sensor(struct platform_device *pdev,
 				  struct armada_thermal_priv *priv)
 {
-	unsigned long reg;
+	u32 reg;
 
-	reg = readl_relaxed(priv->control);
+	reg = readl_relaxed(priv->control1);
 	reg |= PMU_TDC0_OTF_CAL_MASK;
-	writel(reg, priv->control);
+	writel(reg, priv->control1);
 
 	/* Reference calibration value */
 	reg &= ~PMU_TDC0_REF_CAL_CNT_MASK;
 	reg |= (0xf1 << PMU_TDC0_REF_CAL_CNT_OFFS);
-	writel(reg, priv->control);
+	writel(reg, priv->control1);
 
 	reg &= ~PMU_TDC0_START_CAL_MASK;
-	writel(reg, priv->control);
+	writel(reg, priv->control1);
 
 	msleep(10);
 }
@@ -117,30 +127,30 @@ static void armada370_init_sensor(struct platform_device *pdev,
 static void armada375_init_sensor(struct platform_device *pdev,
 				  struct armada_thermal_priv *priv)
 {
-	unsigned long reg;
+	u32 reg;
 
-	reg = readl(priv->control + 4);
+	reg = readl(priv->control1);
 	reg &= ~(A375_UNIT_CONTROL_MASK << A375_UNIT_CONTROL_SHIFT);
 	reg &= ~A375_READOUT_INVERT;
 	reg &= ~A375_HW_RESETn;
 
-	writel(reg, priv->control + 4);
+	writel(reg, priv->control1);
 	msleep(20);
 
 	reg |= A375_HW_RESETn;
-	writel(reg, priv->control + 4);
+	writel(reg, priv->control1);
 	msleep(50);
 }
 
 static void armada380_init_sensor(struct platform_device *pdev,
 				  struct armada_thermal_priv *priv)
 {
-	unsigned long reg = readl_relaxed(priv->control);
+	u32 reg = readl_relaxed(priv->control1);
 
 	/* Reset hardware once */
 	if (!(reg & A380_HW_RESET)) {
 		reg |= A380_HW_RESET;
-		writel(reg, priv->control);
+		writel(reg, priv->control1);
 		msleep(10);
 	}
 }
@@ -214,6 +224,7 @@ static const struct armada_thermal_data armada375_data = {
 	.coef_b = 3171900000UL,
 	.coef_m = 10000000UL,
 	.coef_div = 13616,
+	.needs_control0 = true,
 };
 
 static const struct armada_thermal_data armada380_data = {
@@ -253,6 +264,7 @@ MODULE_DEVICE_TABLE(of, armada_thermal_id_table);
 
 static int armada_thermal_probe(struct platform_device *pdev)
 {
+	void __iomem *control = NULL;
 	struct thermal_zone_device *thermal;
 	const struct of_device_id *match;
 	struct armada_thermal_priv *priv;
@@ -272,11 +284,31 @@ static int armada_thermal_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->sensor);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	priv->control = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(priv->control))
-		return PTR_ERR(priv->control);
+	control = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(control))
+		return PTR_ERR(control);
 
 	priv->data = (struct armada_thermal_data *)match->data;
+
+	/*
+	 * Legacy DT bindings only described "control1" register (also referred
+	 * as "control MSB" on old documentation). New bindings cover
+	 * "control0/control LSB" and "control1/control MSB" registers within
+	 * the same resource, which is then of size 8 instead of 4.
+	 */
+	if (resource_size(res) == LEGACY_CONTROL_MEM_LEN) {
+		/* ->control0 unavailable in this configuration */
+		if (priv->data->needs_control0) {
+			dev_err(&pdev->dev, "No access to control0 register\n");
+			return -EINVAL;
+		}
+
+		priv->control1 = control + LEGACY_CONTROL1_OFFSET;
+	} else {
+		priv->control0 = control + CONTROL0_OFFSET;
+		priv->control1 = control + CONTROL1_OFFSET;
+	}
+
 	priv->data->init_sensor(pdev, priv);
 
 	thermal = thermal_zone_device_register("armada_thermal", 0, 0,
-- 
2.11.0

[PATCH v6 04/11] thermal: armada: Clarify control registers accesses

[Gregory CLEMENT]

Hi Miquel,
 
 On ven., d?c. 22 2017, Miquel Raynal <miquel.raynal@free-electrons.com> wrote:

> Bindings were incomplete for a long time by only exposing one of the two
> available control registers. To ease the migration to the full bindings
> (already in use for the Armada 375 SoC), rename the pointers for
> clarification. This way, it will only be needed to add another pointer
> to access the other control register when the time comes.
>
> This avoids dangerous situations where the offset 0 of the control
> area can be either one register or the other depending on the bindings
> used. After this change, device trees of other SoCs could be migrated to
> the "full" bindings if they may benefit from features from the
> unaccessible register, without any change in the driver.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

I tested that there was no regression on an Armada XP based board:
PlatHome OpenBlocks AX3-4, and on an Armada 388 one: SolidRun Clearfog
A1. For both of them it was OK:

Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Gregory

> ---
>  drivers/thermal/armada_thermal.c | 76 ++++++++++++++++++++++++++++------------
>  1 file changed, 54 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index f350d7efd35a..d58376eba6d9 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -39,12 +39,21 @@
>  #define A375_HW_RESETn			BIT(8)
>  #define A380_HW_RESET			BIT(8)
>  
> +/* Legacy bindings */
> +#define LEGACY_CONTROL_MEM_LEN		0x4
> +
> +/* Current bindings with the 2 control registers under the same memory area */
> +#define LEGACY_CONTROL1_OFFSET		0x0
> +#define CONTROL0_OFFSET			0x0
> +#define CONTROL1_OFFSET			0x4
> +
>  struct armada_thermal_data;
>  
>  /* Marvell EBU Thermal Sensor Dev Structure */
>  struct armada_thermal_priv {
>  	void __iomem *sensor;
> -	void __iomem *control;
> +	void __iomem *control0;
> +	void __iomem *control1;
>  	struct armada_thermal_data *data;
>  };
>  
> @@ -66,27 +75,28 @@ struct armada_thermal_data {
>  	unsigned int temp_shift;
>  	unsigned int temp_mask;
>  	u32 is_valid_bit;
> +	bool needs_control0;
>  };
>  
>  static void armadaxp_init_sensor(struct platform_device *pdev,
>  				 struct armada_thermal_priv *priv)
>  {
> -	unsigned long reg;
> +	u32 reg;
>  
> -	reg = readl_relaxed(priv->control);
> +	reg = readl_relaxed(priv->control1);
>  	reg |= PMU_TDC0_OTF_CAL_MASK;
> -	writel(reg, priv->control);
> +	writel(reg, priv->control1);
>  
>  	/* Reference calibration value */
>  	reg &= ~PMU_TDC0_REF_CAL_CNT_MASK;
>  	reg |= (0xf1 << PMU_TDC0_REF_CAL_CNT_OFFS);
> -	writel(reg, priv->control);
> +	writel(reg, priv->control1);
>  
>  	/* Reset the sensor */
> -	reg = readl_relaxed(priv->control);
> -	writel((reg | PMU_TDC0_SW_RST_MASK), priv->control);
> +	reg = readl_relaxed(priv->control1);
> +	writel((reg | PMU_TDC0_SW_RST_MASK), priv->control1);
>  
> -	writel(reg, priv->control);
> +	writel(reg, priv->control1);
>  
>  	/* Enable the sensor */
>  	reg = readl_relaxed(priv->sensor);
> @@ -97,19 +107,19 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
>  static void armada370_init_sensor(struct platform_device *pdev,
>  				  struct armada_thermal_priv *priv)
>  {
> -	unsigned long reg;
> +	u32 reg;
>  
> -	reg = readl_relaxed(priv->control);
> +	reg = readl_relaxed(priv->control1);
>  	reg |= PMU_TDC0_OTF_CAL_MASK;
> -	writel(reg, priv->control);
> +	writel(reg, priv->control1);
>  
>  	/* Reference calibration value */
>  	reg &= ~PMU_TDC0_REF_CAL_CNT_MASK;
>  	reg |= (0xf1 << PMU_TDC0_REF_CAL_CNT_OFFS);
> -	writel(reg, priv->control);
> +	writel(reg, priv->control1);
>  
>  	reg &= ~PMU_TDC0_START_CAL_MASK;
> -	writel(reg, priv->control);
> +	writel(reg, priv->control1);
>  
>  	msleep(10);
>  }
> @@ -117,30 +127,30 @@ static void armada370_init_sensor(struct platform_device *pdev,
>  static void armada375_init_sensor(struct platform_device *pdev,
>  				  struct armada_thermal_priv *priv)
>  {
> -	unsigned long reg;
> +	u32 reg;
>  
> -	reg = readl(priv->control + 4);
> +	reg = readl(priv->control1);
>  	reg &= ~(A375_UNIT_CONTROL_MASK << A375_UNIT_CONTROL_SHIFT);
>  	reg &= ~A375_READOUT_INVERT;
>  	reg &= ~A375_HW_RESETn;
>  
> -	writel(reg, priv->control + 4);
> +	writel(reg, priv->control1);
>  	msleep(20);
>  
>  	reg |= A375_HW_RESETn;
> -	writel(reg, priv->control + 4);
> +	writel(reg, priv->control1);
>  	msleep(50);
>  }
>  
>  static void armada380_init_sensor(struct platform_device *pdev,
>  				  struct armada_thermal_priv *priv)
>  {
> -	unsigned long reg = readl_relaxed(priv->control);
> +	u32 reg = readl_relaxed(priv->control1);
>  
>  	/* Reset hardware once */
>  	if (!(reg & A380_HW_RESET)) {
>  		reg |= A380_HW_RESET;
> -		writel(reg, priv->control);
> +		writel(reg, priv->control1);
>  		msleep(10);
>  	}
>  }
> @@ -214,6 +224,7 @@ static const struct armada_thermal_data armada375_data = {
>  	.coef_b = 3171900000UL,
>  	.coef_m = 10000000UL,
>  	.coef_div = 13616,
> +	.needs_control0 = true,
>  };
>  
>  static const struct armada_thermal_data armada380_data = {
> @@ -253,6 +264,7 @@ MODULE_DEVICE_TABLE(of, armada_thermal_id_table);
>  
>  static int armada_thermal_probe(struct platform_device *pdev)
>  {
> +	void __iomem *control = NULL;
>  	struct thermal_zone_device *thermal;
>  	const struct of_device_id *match;
>  	struct armada_thermal_priv *priv;
> @@ -272,11 +284,31 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  		return PTR_ERR(priv->sensor);
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> -	priv->control = devm_ioremap_resource(&pdev->dev, res);
> -	if (IS_ERR(priv->control))
> -		return PTR_ERR(priv->control);
> +	control = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(control))
> +		return PTR_ERR(control);
>  
>  	priv->data = (struct armada_thermal_data *)match->data;
> +
> +	/*
> +	 * Legacy DT bindings only described "control1" register (also referred
> +	 * as "control MSB" on old documentation). New bindings cover
> +	 * "control0/control LSB" and "control1/control MSB" registers within
> +	 * the same resource, which is then of size 8 instead of 4.
> +	 */
> +	if (resource_size(res) == LEGACY_CONTROL_MEM_LEN) {
> +		/* ->control0 unavailable in this configuration */
> +		if (priv->data->needs_control0) {
> +			dev_err(&pdev->dev, "No access to control0 register\n");
> +			return -EINVAL;
> +		}
> +
> +		priv->control1 = control + LEGACY_CONTROL1_OFFSET;
> +	} else {
> +		priv->control0 = control + CONTROL0_OFFSET;
> +		priv->control1 = control + CONTROL1_OFFSET;
> +	}
> +
>  	priv->data->init_sensor(pdev, priv);
>  
>  	thermal = thermal_zone_device_register("armada_thermal", 0, 0,
> -- 
> 2.11.0
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH v6 05/11] thermal: armada: Use real status register name

[Miquel Raynal]

Three 32-bit registers are used to drive the thermal IP: control0,
control1 and status. The two control registers share the same name both
in the documentation and in the code, while the latter is referred as
"sensor" in the code. Rename this pointer to be called "status" in order
to be aligned with the documentation.

Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index d58376eba6d9..ceebabf45c53 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -51,7 +51,7 @@ struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
 struct armada_thermal_priv {
-	void __iomem *sensor;
+	void __iomem *status;
 	void __iomem *control0;
 	void __iomem *control1;
 	struct armada_thermal_data *data;
@@ -99,9 +99,9 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
 	writel(reg, priv->control1);
 
 	/* Enable the sensor */
-	reg = readl_relaxed(priv->sensor);
+	reg = readl_relaxed(priv->status);
 	reg &= ~PMU_TM_DISABLE_MASK;
-	writel(reg, priv->sensor);
+	writel(reg, priv->status);
 }
 
 static void armada370_init_sensor(struct platform_device *pdev,
@@ -157,7 +157,7 @@ static void armada380_init_sensor(struct platform_device *pdev,
 
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
-	u32 reg = readl_relaxed(priv->sensor);
+	u32 reg = readl_relaxed(priv->status);
 
 	return reg & priv->data->is_valid_bit;
 }
@@ -176,7 +176,7 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 		return -EIO;
 	}
 
-	reg = readl_relaxed(priv->sensor);
+	reg = readl_relaxed(priv->status);
 	reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;
 
 	/* Get formula coeficients */
@@ -279,9 +279,9 @@ static int armada_thermal_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	priv->sensor = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(priv->sensor))
-		return PTR_ERR(priv->sensor);
+	priv->status = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->status))
+		return PTR_ERR(priv->status);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	control = devm_ioremap_resource(&pdev->dev, res);
-- 
2.11.0

[PATCH v6 06/11] thermal: armada: Add support for Armada AP806

[Miquel Raynal]

From: Baruch Siach <baruch@tkos.co.il>

The AP806 component is integrated in the Armada 8K and 7K lines of
processors.

The thermal sensor sample field on the status register is a signed
value. Extend armada_get_temp() and the driver structure to handle
signed values.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
[<miquel.raynal@free-electrons.com>: Changes when applying over the
previous patches, including the register names changes, also switched
the coefficients values to s64 instead of unsigned long to deal with
negative values and used do_div instead of the traditionnal '/']
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 78 +++++++++++++++++++++++++++++++---------
 1 file changed, 62 insertions(+), 16 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index ceebabf45c53..28ff2d0412ac 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -47,6 +47,11 @@
 #define CONTROL0_OFFSET			0x0
 #define CONTROL1_OFFSET			0x4
 
+/* TSEN refers to the temperature sensors within the AP */
+#define CONTROL0_TSEN_START		BIT(0)
+#define CONTROL0_TSEN_RESET		BIT(1)
+#define CONTROL0_TSEN_ENABLE		BIT(2)
+
 struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
@@ -66,10 +71,11 @@ struct armada_thermal_data {
 	bool (*is_valid)(struct armada_thermal_priv *);
 
 	/* Formula coeficients: temp = (b - m * reg) / div */
-	unsigned long coef_b;
-	unsigned long coef_m;
-	unsigned long coef_div;
+	s64 coef_b;
+	s64 coef_m;
+	u32 coef_div;
 	bool inverted;
+	bool signed_sample;
 
 	/* Register shift and mask to access the sensor temperature */
 	unsigned int temp_shift;
@@ -155,6 +161,18 @@ static void armada380_init_sensor(struct platform_device *pdev,
 	}
 }
 
+static void armada_ap806_init_sensor(struct platform_device *pdev,
+				     struct armada_thermal_priv *priv)
+{
+	u32 reg;
+
+	reg = readl_relaxed(priv->control0);
+	reg &= ~CONTROL0_TSEN_RESET;
+	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
+	writel(reg, priv->control0);
+	msleep(10);
+}
+
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
 	u32 reg = readl_relaxed(priv->status);
@@ -163,11 +181,12 @@ static bool armada_is_valid(struct armada_thermal_priv *priv)
 }
 
 static int armada_get_temp(struct thermal_zone_device *thermal,
-			  int *temp)
+			   int *temperature)
 {
 	struct armada_thermal_priv *priv = thermal->devdata;
-	unsigned long reg;
-	unsigned long m, b, div;
+	u32 reg, div;
+	s64 sample, b, m;
+	u64 tmp;
 
 	/* Valid check */
 	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
@@ -178,6 +197,11 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 
 	reg = readl_relaxed(priv->status);
 	reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;
+	if (priv->data->signed_sample)
+		/* The most significant bit is the sign bit */
+		sample = sign_extend32(reg, fls(priv->data->temp_mask) - 1);
+	else
+		sample = reg;
 
 	/* Get formula coeficients */
 	b = priv->data->coef_b;
@@ -185,9 +209,13 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 	div = priv->data->coef_div;
 
 	if (priv->data->inverted)
-		*temp = ((m * reg) - b) / div;
+		tmp = (m * sample) - b;
 	else
-		*temp = (b - (m * reg)) / div;
+		tmp = b - (m * sample);
+
+	do_div(tmp, div);
+	*temperature = (int)tmp;
+
 	return 0;
 }
 
@@ -199,8 +227,8 @@ static const struct armada_thermal_data armadaxp_data = {
 	.init_sensor = armadaxp_init_sensor,
 	.temp_shift = 10,
 	.temp_mask = 0x1ff,
-	.coef_b = 3153000000UL,
-	.coef_m = 10000000UL,
+	.coef_b = 3153000000ULL,
+	.coef_m = 10000000ULL,
 	.coef_div = 13825,
 };
 
@@ -210,8 +238,8 @@ static const struct armada_thermal_data armada370_data = {
 	.is_valid_bit = BIT(9),
 	.temp_shift = 10,
 	.temp_mask = 0x1ff,
-	.coef_b = 3153000000UL,
-	.coef_m = 10000000UL,
+	.coef_b = 3153000000ULL,
+	.coef_m = 10000000ULL,
 	.coef_div = 13825,
 };
 
@@ -221,8 +249,8 @@ static const struct armada_thermal_data armada375_data = {
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x1ff,
-	.coef_b = 3171900000UL,
-	.coef_m = 10000000UL,
+	.coef_b = 3171900000ULL,
+	.coef_m = 10000000ULL,
 	.coef_div = 13616,
 	.needs_control0 = true,
 };
@@ -233,12 +261,26 @@ static const struct armada_thermal_data armada380_data = {
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
-	.coef_b = 1172499100UL,
-	.coef_m = 2000096UL,
+	.coef_b = 1172499100ULL,
+	.coef_m = 2000096ULL,
 	.coef_div = 4201,
 	.inverted = true,
 };
 
+static const struct armada_thermal_data armada_ap806_data = {
+	.is_valid = armada_is_valid,
+	.init_sensor = armada_ap806_init_sensor,
+	.is_valid_bit = BIT(16),
+	.temp_shift = 0,
+	.temp_mask = 0x3ff,
+	.coef_b = -150000LL,
+	.coef_m = 423ULL,
+	.coef_div = 1,
+	.inverted = true,
+	.signed_sample = true,
+	.needs_control0 = true,
+};
+
 static const struct of_device_id armada_thermal_id_table[] = {
 	{
 		.compatible = "marvell,armadaxp-thermal",
@@ -257,6 +299,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.data       = &armada380_data,
 	},
 	{
+		.compatible = "marvell,armada-ap806-thermal",
+		.data       = &armada_ap806_data,
+	},
+	{
 		/* sentinel */
 	},
 };
-- 
2.11.0

[PATCH v6 06/11] thermal: armada: Add support for Armada AP806

[Baruch Siach]

Hi Miqu?l,

On Fri, Dec 22, 2017 at 10:32:21AM +0100, Miquel Raynal wrote:
> From: Baruch Siach <baruch@tkos.co.il>
> 
> The AP806 component is integrated in the Armada 8K and 7K lines of
> processors.
> 
> The thermal sensor sample field on the status register is a signed
> value. Extend armada_get_temp() and the driver structure to handle
> signed values.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> [<miquel.raynal@free-electrons.com>: Changes when applying over the
> previous patches, including the register names changes, also switched
> the coefficients values to s64 instead of unsigned long to deal with
> negative values and used do_div instead of the traditionnal '/']
> Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---

[..]

>  static int armada_get_temp(struct thermal_zone_device *thermal,
> -			  int *temp)
> +			   int *temperature)
>  {
>  	struct armada_thermal_priv *priv = thermal->devdata;
> -	unsigned long reg;
> -	unsigned long m, b, div;
> +	u32 reg, div;
> +	s64 sample, b, m;
> +	u64 tmp;
>  
>  	/* Valid check */
>  	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
> @@ -178,6 +197,11 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
>  
>  	reg = readl_relaxed(priv->status);
>  	reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;
> +	if (priv->data->signed_sample)
> +		/* The most significant bit is the sign bit */
> +		sample = sign_extend32(reg, fls(priv->data->temp_mask) - 1);
> +	else
> +		sample = reg;
>  
>  	/* Get formula coeficients */
>  	b = priv->data->coef_b;
> @@ -185,9 +209,13 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
>  	div = priv->data->coef_div;
>  
>  	if (priv->data->inverted)
> -		*temp = ((m * reg) - b) / div;
> +		tmp = (m * sample) - b;
>  	else
> -		*temp = (b - (m * reg)) / div;
> +		tmp = b - (m * sample);
> +
> +	do_div(tmp, div);
> +	*temperature = (int)tmp;

Nitpick: why not (untested)

#include <linux/math64.h>

  if (priv->data->inverted)
    *temp = div_s64((m * sample) - b, div);
  else
    *temp = div_s64(b - (m * sample), div);

baruch

> +
>  	return 0;
>  }

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[PATCH v6 06/11] thermal: armada: Add support for Armada AP806

[Miquel RAYNAL]

Hi Baruch,

On Fri, 22 Dec 2017 12:14:26 +0200
Baruch Siach <baruch@tkos.co.il> wrote:

> Hi Miqu?l,
> 
> On Fri, Dec 22, 2017 at 10:32:21AM +0100, Miquel Raynal wrote:
> > From: Baruch Siach <baruch@tkos.co.il>
> > 
> > The AP806 component is integrated in the Armada 8K and 7K lines of
> > processors.
> > 
> > The thermal sensor sample field on the status register is a signed
> > value. Extend armada_get_temp() and the driver structure to handle
> > signed values.
> > 
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > [<miquel.raynal@free-electrons.com>: Changes when applying over the
> > previous patches, including the register names changes, also
> > switched the coefficients values to s64 instead of unsigned long to
> > deal with negative values and used do_div instead of the
> > traditionnal '/'] Signed-off-by: Miquel Raynal
> > <miquel.raynal@free-electrons.com> Reviewed-by: Gregory CLEMENT
> > <gregory.clement@free-electrons.com> Tested-by: Gregory CLEMENT
> > <gregory.clement@free-electrons.com> ---  
> 
> [..]
> 
> >  static int armada_get_temp(struct thermal_zone_device *thermal,
> > -			  int *temp)
> > +			   int *temperature)
> >  {
> >  	struct armada_thermal_priv *priv = thermal->devdata;
> > -	unsigned long reg;
> > -	unsigned long m, b, div;
> > +	u32 reg, div;
> > +	s64 sample, b, m;
> > +	u64 tmp;
> >  
> >  	/* Valid check */
> >  	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
> > @@ -178,6 +197,11 @@ static int armada_get_temp(struct
> > thermal_zone_device *thermal, 
> >  	reg = readl_relaxed(priv->status);
> >  	reg = (reg >> priv->data->temp_shift) &
> > priv->data->temp_mask;
> > +	if (priv->data->signed_sample)
> > +		/* The most significant bit is the sign bit */
> > +		sample = sign_extend32(reg,
> > fls(priv->data->temp_mask) - 1);
> > +	else
> > +		sample = reg;
> >  
> >  	/* Get formula coeficients */
> >  	b = priv->data->coef_b;
> > @@ -185,9 +209,13 @@ static int armada_get_temp(struct
> > thermal_zone_device *thermal, div = priv->data->coef_div;
> >  
> >  	if (priv->data->inverted)
> > -		*temp = ((m * reg) - b) / div;
> > +		tmp = (m * sample) - b;
> >  	else
> > -		*temp = (b - (m * reg)) / div;
> > +		tmp = b - (m * sample);
> > +
> > +	do_div(tmp, div);
> > +	*temperature = (int)tmp;  
> 
> Nitpick: why not (untested)
> 
> #include <linux/math64.h>
> 
>   if (priv->data->inverted)
>     *temp = div_s64((m * sample) - b, div);
>   else
>     *temp = div_s64(b - (m * sample), div);

Indeed I could also use div_s64, but the result must be unsigned anyway.

But this does all the operations on the same line, maybe this is more
readable, I will update it and send (hopefully) the last version :)

Cheers,
Miqu?l

> 
> baruch
> 
> > +
> >  	return 0;
> >  }  
> 



-- 
Miquel Raynal, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v6 06/11] thermal: armada: Add support for Armada AP806

[Baruch Siach]

Hi Miqu?l,

On Fri, Dec 22, 2017 at 11:49:48AM +0100, Miquel RAYNAL wrote:
> On Fri, 22 Dec 2017 12:14:26 +0200
> Baruch Siach <baruch@tkos.co.il> wrote:
> > On Fri, Dec 22, 2017 at 10:32:21AM +0100, Miquel Raynal wrote:
> > > From: Baruch Siach <baruch@tkos.co.il>
> > > 
> > > The AP806 component is integrated in the Armada 8K and 7K lines of
> > > processors.
> > > 
> > > The thermal sensor sample field on the status register is a signed
> > > value. Extend armada_get_temp() and the driver structure to handle
> > > signed values.
> > > 
> > > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > > [<miquel.raynal@free-electrons.com>: Changes when applying over the
> > > previous patches, including the register names changes, also
> > > switched the coefficients values to s64 instead of unsigned long to
> > > deal with negative values and used do_div instead of the
> > > traditionnal '/'] Signed-off-by: Miquel Raynal
> > > <miquel.raynal@free-electrons.com> Reviewed-by: Gregory CLEMENT
> > > <gregory.clement@free-electrons.com> Tested-by: Gregory CLEMENT
> > > <gregory.clement@free-electrons.com> ---  
> > 
> > [..]
> > 
> > >  static int armada_get_temp(struct thermal_zone_device *thermal,
> > > -			  int *temp)
> > > +			   int *temperature)
> > >  {
> > >  	struct armada_thermal_priv *priv = thermal->devdata;
> > > -	unsigned long reg;
> > > -	unsigned long m, b, div;
> > > +	u32 reg, div;
> > > +	s64 sample, b, m;
> > > +	u64 tmp;
> > >  
> > >  	/* Valid check */
> > >  	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
> > > @@ -178,6 +197,11 @@ static int armada_get_temp(struct
> > > thermal_zone_device *thermal, 
> > >  	reg = readl_relaxed(priv->status);
> > >  	reg = (reg >> priv->data->temp_shift) &
> > > priv->data->temp_mask;
> > > +	if (priv->data->signed_sample)
> > > +		/* The most significant bit is the sign bit */
> > > +		sample = sign_extend32(reg,
> > > fls(priv->data->temp_mask) - 1);
> > > +	else
> > > +		sample = reg;
> > >  
> > >  	/* Get formula coeficients */
> > >  	b = priv->data->coef_b;
> > > @@ -185,9 +209,13 @@ static int armada_get_temp(struct
> > > thermal_zone_device *thermal, div = priv->data->coef_div;
> > >  
> > >  	if (priv->data->inverted)
> > > -		*temp = ((m * reg) - b) / div;
> > > +		tmp = (m * sample) - b;
> > >  	else
> > > -		*temp = (b - (m * reg)) / div;
> > > +		tmp = b - (m * sample);
> > > +
> > > +	do_div(tmp, div);
> > > +	*temperature = (int)tmp;  
> > 
> > Nitpick: why not (untested)
> > 
> > #include <linux/math64.h>
> > 
> >   if (priv->data->inverted)
> >     *temp = div_s64((m * sample) - b, div);
> >   else
> >     *temp = div_s64(b - (m * sample), div);
> 
> Indeed I could also use div_s64, but the result must be unsigned anyway.

*temp is signed, as far as I can see. Temperature can go below zero, at least 
in the Celsius scale.

> But this does all the operations on the same line, maybe this is more
> readable, I will update it and send (hopefully) the last version :)

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[PATCH v6 07/11] thermal: armada: Add support for Armada CP110

[Miquel Raynal]

From: Baruch Siach <baruch@tkos.co.il>

The CP110 component is integrated in the Armada 8k and 7k lines of
processors.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
[<miquel.raynal@free-electrons.com>: renamed the register pointers as
well as some definitions related to the new register names and
simplified the init sequence for Armada 380]
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 28ff2d0412ac..93f7c0eb5141 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -37,7 +37,6 @@
 #define A375_UNIT_CONTROL_MASK		0x7
 #define A375_READOUT_INVERT		BIT(15)
 #define A375_HW_RESETn			BIT(8)
-#define A380_HW_RESET			BIT(8)
 
 /* Legacy bindings */
 #define LEGACY_CONTROL_MEM_LEN		0x4
@@ -52,6 +51,10 @@
 #define CONTROL0_TSEN_RESET		BIT(1)
 #define CONTROL0_TSEN_ENABLE		BIT(2)
 
+/* EXT_TSEN refers to the external temperature sensors, out of the AP */
+#define CONTROL1_EXT_TSEN_SW_RESET	BIT(7)
+#define CONTROL1_EXT_TSEN_HW_RESETn	BIT(8)
+
 struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
@@ -153,12 +156,11 @@ static void armada380_init_sensor(struct platform_device *pdev,
 {
 	u32 reg = readl_relaxed(priv->control1);
 
-	/* Reset hardware once */
-	if (!(reg & A380_HW_RESET)) {
-		reg |= A380_HW_RESET;
-		writel(reg, priv->control1);
-		msleep(10);
-	}
+	/* Disable the HW/SW reset */
+	reg |= CONTROL1_EXT_TSEN_HW_RESETn;
+	reg &= ~CONTROL1_EXT_TSEN_SW_RESET;
+	writel(reg, priv->control1);
+	msleep(10);
 }
 
 static void armada_ap806_init_sensor(struct platform_device *pdev,
@@ -281,6 +283,19 @@ static const struct armada_thermal_data armada_ap806_data = {
 	.needs_control0 = true,
 };
 
+static const struct armada_thermal_data armada_cp110_data = {
+	.is_valid = armada_is_valid,
+	.init_sensor = armada380_init_sensor,
+	.is_valid_bit = BIT(10),
+	.temp_shift = 0,
+	.temp_mask = 0x3ff,
+	.coef_b = 1172499100ULL,
+	.coef_m = 2000096ULL,
+	.coef_div = 4201,
+	.inverted = true,
+	.needs_control0 = true,
+};
+
 static const struct of_device_id armada_thermal_id_table[] = {
 	{
 		.compatible = "marvell,armadaxp-thermal",
@@ -303,6 +318,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.data       = &armada_ap806_data,
 	},
 	{
+		.compatible = "marvell,armada-cp110-thermal",
+		.data       = &armada_cp110_data,
+	},
+	{
 		/* sentinel */
 	},
 };
-- 
2.11.0

[PATCH v6 08/11] thermal: armada: Update Kconfig and module description

[Miquel Raynal]

Update Armada thermal driver Kconfig entry as well as the driver's
MODULE_DESCRIPTION content, now that 64-bit SoCs are also supported,
eg. Armada 7K and Armada 8K.

Use the generic term "Marvell EBU Armada SoCs" instead of listing all
the supported SoCs everywhere (excepted in the Kconfig description,
where it is useful to have a list).

Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/thermal/Kconfig          | 4 ++--
 drivers/thermal/armada_thermal.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 315ae2926e20..b6adc54b96f1 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -301,13 +301,13 @@ config DB8500_THERMAL
 	  thermal zone if trip points reached.
 
 config ARMADA_THERMAL
-	tristate "Armada 370/XP thermal management"
+	tristate "Marvell EBU Armada SoCs thermal management"
 	depends on ARCH_MVEBU || COMPILE_TEST
 	depends on HAS_IOMEM
 	depends on OF
 	help
 	  Enable this option if you want to have support for thermal management
-	  controller present in Armada 370 and Armada XP SoC.
+	  controller present in Marvell EBU Armada SoCs (370,375,XP,38x,7K,8K).
 
 config DA9062_THERMAL
 	tristate "DA9062/DA9061 Dialog Semiconductor thermal driver"
diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 93f7c0eb5141..537f6fdc9059 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -1,5 +1,5 @@
 /*
- * Marvell Armada 370/XP thermal sensor driver
+ * Marvell EBU Armada SoCs thermal sensor driver
  *
  * Copyright (C) 2013 Marvell
  *
@@ -411,5 +411,5 @@ static struct platform_driver armada_thermal_driver = {
 module_platform_driver(armada_thermal_driver);
 
 MODULE_AUTHOR("Ezequiel Garcia <ezequiel.garcia@free-electrons.com>");
-MODULE_DESCRIPTION("Armada 370/XP thermal driver");
+MODULE_DESCRIPTION("Marvell EBU Armada SoCs thermal driver");
 MODULE_LICENSE("GPL v2");
-- 
2.11.0

[PATCH v6 09/11] thermal: armada: Change sensors trim default value

[Miquel Raynal]

Errata #132698 highlights an error in the default value of Tc trim.
Set this parameter to b'011.

Suggested-by: David Sniatkiwicz <davidsn@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 537f6fdc9059..a3feaef7db20 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -46,6 +46,10 @@
 #define CONTROL0_OFFSET			0x0
 #define CONTROL1_OFFSET			0x4
 
+/* Errata fields */
+#define CONTROL0_TSEN_TC_TRIM_MASK	0x7
+#define CONTROL0_TSEN_TC_TRIM_VAL	0x3
+
 /* TSEN refers to the temperature sensors within the AP */
 #define CONTROL0_TSEN_START		BIT(0)
 #define CONTROL0_TSEN_RESET		BIT(1)
@@ -161,6 +165,15 @@ static void armada380_init_sensor(struct platform_device *pdev,
 	reg &= ~CONTROL1_EXT_TSEN_SW_RESET;
 	writel(reg, priv->control1);
 	msleep(10);
+
+	/* Set Tsen Tc Trim to correct default value (errata #132698) */
+	if (priv->control0) {
+		reg = readl_relaxed(priv->control0);
+		reg &= ~CONTROL0_TSEN_TC_TRIM_MASK;
+		reg |= CONTROL0_TSEN_TC_TRIM_VAL;
+		writel(reg, priv->control0);
+		msleep(10);
+	}
 }
 
 static void armada_ap806_init_sensor(struct platform_device *pdev,
-- 
2.11.0

[PATCH v6 10/11] thermal: armada: Wait sensors validity before exiting the init callback

[Miquel Raynal]

The thermal core will check for sensors validity right after the
initialization callback has returned. As the initialization routine make
a reset, the sensors are not ready immediately and the core spawns an
error in the dmesg. Avoid this annoying situation by polling on the
validity bit before exiting from these routines. This also avoid the use
of blind sleeps.

Suggested-by: David Sniatkiwicz <davidsn@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index a3feaef7db20..ea958e651312 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/of_device.h>
 #include <linux/thermal.h>
+#include <linux/iopoll.h>
 
 /* Thermal Manager Control and Status Register */
 #define PMU_TDC0_SW_RST_MASK		(0x1 << 1)
@@ -59,6 +60,9 @@
 #define CONTROL1_EXT_TSEN_SW_RESET	BIT(7)
 #define CONTROL1_EXT_TSEN_HW_RESETn	BIT(8)
 
+#define STATUS_POLL_PERIOD_US		1000
+#define STATUS_POLL_TIMEOUT_US		100000
+
 struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
@@ -155,6 +159,16 @@ static void armada375_init_sensor(struct platform_device *pdev,
 	msleep(50);
 }
 
+static void armada_wait_sensor_validity(struct armada_thermal_priv *priv)
+{
+	u32 reg;
+
+	readl_relaxed_poll_timeout(priv->status, reg,
+				   reg & priv->data->is_valid_bit,
+				   STATUS_POLL_PERIOD_US,
+				   STATUS_POLL_TIMEOUT_US);
+}
+
 static void armada380_init_sensor(struct platform_device *pdev,
 				  struct armada_thermal_priv *priv)
 {
@@ -164,7 +178,6 @@ static void armada380_init_sensor(struct platform_device *pdev,
 	reg |= CONTROL1_EXT_TSEN_HW_RESETn;
 	reg &= ~CONTROL1_EXT_TSEN_SW_RESET;
 	writel(reg, priv->control1);
-	msleep(10);
 
 	/* Set Tsen Tc Trim to correct default value (errata #132698) */
 	if (priv->control0) {
@@ -172,8 +185,10 @@ static void armada380_init_sensor(struct platform_device *pdev,
 		reg &= ~CONTROL0_TSEN_TC_TRIM_MASK;
 		reg |= CONTROL0_TSEN_TC_TRIM_VAL;
 		writel(reg, priv->control0);
-		msleep(10);
 	}
+
+	/* Wait the sensors to be valid or the core will warn the user */
+	armada_wait_sensor_validity(priv);
 }
 
 static void armada_ap806_init_sensor(struct platform_device *pdev,
@@ -185,7 +200,9 @@ static void armada_ap806_init_sensor(struct platform_device *pdev,
 	reg &= ~CONTROL0_TSEN_RESET;
 	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
 	writel(reg, priv->control0);
-	msleep(10);
+
+	/* Wait the sensors to be valid or the core will warn the user */
+	armada_wait_sensor_validity(priv);
 }
 
 static bool armada_is_valid(struct armada_thermal_priv *priv)
-- 
2.11.0

[PATCH v6 11/11] thermal: armada: Give meaningful names to the thermal zones

[Miquel Raynal]

After registration to the thermal core, sysfs will make one entry
per instance of the driver in /sys/class/thermal_zoneX and
/sys/class/hwmon/hwmonX, X being the index of the instance, all of them
having the type/name "armada_thermal".

Until now there was only one thermal zone per SoC but SoCs like Armada
A7K and Armada A8K have respectively two and three thermal zones (one
per AP and one per CP) and this number is subject to grow in the future.

Use dev_name() instead of the "armada_thermal" string to get a
meaningful name and be able to identify the thermal zones from
userspace.

Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index ea958e651312..454137f78eb3 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -406,8 +406,8 @@ static int armada_thermal_probe(struct platform_device *pdev)
 
 	priv->data->init_sensor(pdev, priv);
 
-	thermal = thermal_zone_device_register("armada_thermal", 0, 0,
-					       priv, &ops, NULL, 0, 0);
+	thermal = thermal_zone_device_register(dev_name(&pdev->dev), 0, 0, priv,
+					       &ops, NULL, 0, 0);
 	if (IS_ERR(thermal)) {
 		dev_err(&pdev->dev,
 			"Failed to register thermal zone device\n");
-- 
2.11.0

[PATCH v6 11/11] thermal: armada: Give meaningful names to the thermal zones

[Gregory CLEMENT]

Hi Miquel,
 
 On ven., d?c. 22 2017, Miquel Raynal <miquel.raynal@free-electrons.com> wrote:

> After registration to the thermal core, sysfs will make one entry
> per instance of the driver in /sys/class/thermal_zoneX and
> /sys/class/hwmon/hwmonX, X being the index of the instance, all of them
> having the type/name "armada_thermal".
>
> Until now there was only one thermal zone per SoC but SoCs like Armada
> A7K and Armada A8K have respectively two and three thermal zones (one
> per AP and one per CP) and this number is subject to grow in the future.
>
> Use dev_name() instead of the "armada_thermal" string to get a
> meaningful name and be able to identify the thermal zones from
> userspace.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

And here I also tested that when all the series was applied there was no
regression on an Armada XP based board: PlatHome OpenBlocks AX3-4, and
on an Armada 388 one: SolidRun Clearfog A1. hera again for both of them
it was OK:

Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Gregory

> ---
>  drivers/thermal/armada_thermal.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index ea958e651312..454137f78eb3 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -406,8 +406,8 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  
>  	priv->data->init_sensor(pdev, priv);
>  
> -	thermal = thermal_zone_device_register("armada_thermal", 0, 0,
> -					       priv, &ops, NULL, 0, 0);
> +	thermal = thermal_zone_device_register(dev_name(&pdev->dev), 0, 0, priv,
> +					       &ops, NULL, 0, 0);
>  	if (IS_ERR(thermal)) {
>  		dev_err(&pdev->dev,
>  			"Failed to register thermal zone device\n");
> -- 
> 2.11.0
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH v6 11/11] thermal: armada: Give meaningful names to the thermal zones

[Miquel RAYNAL]

Hello Gregory,

On Fri, 22 Dec 2017 16:36:04 +0100
Gregory CLEMENT <gregory.clement@free-electrons.com> wrote:

> Hi Miquel,
>  
>  On ven., d?c. 22 2017, Miquel Raynal
> <miquel.raynal@free-electrons.com> wrote:
> 
> > After registration to the thermal core, sysfs will make one entry
> > per instance of the driver in /sys/class/thermal_zoneX and
> > /sys/class/hwmon/hwmonX, X being the index of the instance, all of
> > them having the type/name "armada_thermal".
> >
> > Until now there was only one thermal zone per SoC but SoCs like
> > Armada A7K and Armada A8K have respectively two and three thermal
> > zones (one per AP and one per CP) and this number is subject to
> > grow in the future.
> >
> > Use dev_name() instead of the "armada_thermal" string to get a
> > meaningful name and be able to identify the thermal zones from
> > userspace.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
> > Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>  
> 
> And here I also tested that when all the series was applied there was
> no regression on an Armada XP based board: PlatHome OpenBlocks AX3-4,
> and on an Armada 388 one: SolidRun Clearfog A1. hera again for both
> of them it was OK:

Thank you for testing, on my side I checked the last version (minor
changes) with both an Armada-385-AP and an Armada-8040-DB.

I will resend the series very soon.

Thanks,
Miqu?l

> 
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> 
> Gregory
> 
> > ---
> >  drivers/thermal/armada_thermal.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/thermal/armada_thermal.c
> > b/drivers/thermal/armada_thermal.c index ea958e651312..454137f78eb3
> > 100644 --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -406,8 +406,8 @@ static int armada_thermal_probe(struct
> > platform_device *pdev) 
> >  	priv->data->init_sensor(pdev, priv);
> >  
> > -	thermal = thermal_zone_device_register("armada_thermal",
> > 0, 0,
> > -					       priv, &ops, NULL,
> > 0, 0);
> > +	thermal =
> > thermal_zone_device_register(dev_name(&pdev->dev), 0, 0, priv,
> > +					       &ops, NULL, 0, 0);
> >  	if (IS_ERR(thermal)) {
> >  		dev_err(&pdev->dev,
> >  			"Failed to register thermal zone
> > device\n"); -- 
> > 2.11.0
> >  
> 



-- 
Miquel Raynal, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com