[PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Miquel Raynal]

The only capability of the Armada thermal driver is currently just to
read one sensor (the default one) per AP and one per CP. Actually,
there is one sensor per core in the AP806 plus one sensor in the
thermal IP itself. The CP110 just features one thermal sensor in its
own thermal IP.

This series first improves the readability of this driver, then adds
support for multi-channel thermal IPs. The bindings and the
device-trees are updated accordingly.

Thank you,
Miquèl


Changes since v3:
=================
- Removed the mention about the overheat interrupt in the commit log of
  "dt-bindings: thermal: armada: add reference to new bindings". This
  interrupt will be added in another series.

Changes since v2:
=================
* Fixed typos in "thermal: armada: convert driver to syscon  register
  accesses" commit log.
* Added Rob's RB tags after changing the node names both in the
  Documentation and in the DT files this way:
  s/xx_thermal: xx_thermal@yy/xx_thermal: thermal-sensor@yy/
* Dropped the patch that renamed labels in the Documentation on Rob's
  notice.
* Uniformized the type of temperature variables to 'int' as suggested in
  a previous commit from Sascha Hauer:
  17e8351a7739 ("thermal: consistently use int for temperatures")

Changes since v1:
=================
* Added register ranges in the thermal nodes.
* Added these ranges in the documentation also.
* In the bindings, used the "Must be one of" formula before listing the
  compatibles.
* Removed the overheat interrupt support that depends on another
  series. This work is independent.
* Rebased on top of v4.18-rc1.


Miquel Raynal (23):
  thermal: armada: add a function that sanitizes the thermal zone name
  thermal: armada: remove useless register accesses
  thermal: armada: remove misleading comments
  thermal: armada: rename the initialization routine
  thermal: armada: dissociate a380 and cp110 ->init() hooks
  thermal: armada: average over samples to avoid glitches
  thermal: armada: convert driver to syscon register accesses
  thermal: armada: use the resource managed registration helper
    alternative
  thermal: armada: add multi-channel sensors support
  thermal: armada: remove sensors validity from the IP initialization
  thermal: armada: move validity check out of the read function
  thermal: armada: get rid of the ->is_valid() pointer
  dt-bindings: cp110: rename cp110 syscon file
  dt-bindings: ap806: prepare the syscon file to list other syscons
    nodes
  dt-bindings: cp110: prepare the syscon file to list other syscons
    nodes
  dt-bindings: ap806: add the thermal node in the syscon file
  dt-bindings: cp110: update documentation since DT de-duplication
  dt-bindings: cp110: add the thermal node in the syscon file
  dt-bindings: thermal: armada: add reference to new bindings
  arm64: dts: marvell: move AP806/CP110 thermal nodes into a new syscon
  arm64: dts: marvell: add thermal-zone node in ap806 DTSI file
  arm64: dts: marvell: add macro to make distinction between node names
  arm64: dts: marvell: add thermal-zone node in cp110 DTSI file

 .../arm/marvell/ap806-system-controller.txt        |  48 +-
 ...controller0.txt => cp110-system-controller.txt} |  61 ++-
 .../devicetree/bindings/thermal/armada-thermal.txt |   5 +
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi      |  74 ++-
 arch/arm64/boot/dts/marvell/armada-common.dtsi     |   1 +
 arch/arm64/boot/dts/marvell/armada-cp110.dtsi      |  37 +-
 drivers/thermal/armada_thermal.c                   | 534 ++++++++++++++++-----
 7 files changed, 606 insertions(+), 154 deletions(-)
 rename Documentation/devicetree/bindings/arm/marvell/{cp110-system-controller0.txt => cp110-system-controller.txt} (85%)

-- 
2.14.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Miquel Raynal]

The only capability of the Armada thermal driver is currently just to
read one sensor (the default one) per AP and one per CP. Actually,
there is one sensor per core in the AP806 plus one sensor in the
thermal IP itself. The CP110 just features one thermal sensor in its
own thermal IP.

This series first improves the readability of this driver, then adds
support for multi-channel thermal IPs. The bindings and the
device-trees are updated accordingly.

Thank you,
Miqu?l


Changes since v3:
=================
- Removed the mention about the overheat interrupt in the commit log of
  "dt-bindings: thermal: armada: add reference to new bindings". This
  interrupt will be added in another series.

Changes since v2:
=================
* Fixed typos in "thermal: armada: convert driver to syscon  register
  accesses" commit log.
* Added Rob's RB tags after changing the node names both in the
  Documentation and in the DT files this way:
  s/xx_thermal: xx_thermal at yy/xx_thermal: thermal-sensor at yy/
* Dropped the patch that renamed labels in the Documentation on Rob's
  notice.
* Uniformized the type of temperature variables to 'int' as suggested in
  a previous commit from Sascha Hauer:
  17e8351a7739 ("thermal: consistently use int for temperatures")

Changes since v1:
=================
* Added register ranges in the thermal nodes.
* Added these ranges in the documentation also.
* In the bindings, used the "Must be one of" formula before listing the
  compatibles.
* Removed the overheat interrupt support that depends on another
  series. This work is independent.
* Rebased on top of v4.18-rc1.


Miquel Raynal (23):
  thermal: armada: add a function that sanitizes the thermal zone name
  thermal: armada: remove useless register accesses
  thermal: armada: remove misleading comments
  thermal: armada: rename the initialization routine
  thermal: armada: dissociate a380 and cp110 ->init() hooks
  thermal: armada: average over samples to avoid glitches
  thermal: armada: convert driver to syscon register accesses
  thermal: armada: use the resource managed registration helper
    alternative
  thermal: armada: add multi-channel sensors support
  thermal: armada: remove sensors validity from the IP initialization
  thermal: armada: move validity check out of the read function
  thermal: armada: get rid of the ->is_valid() pointer
  dt-bindings: cp110: rename cp110 syscon file
  dt-bindings: ap806: prepare the syscon file to list other syscons
    nodes
  dt-bindings: cp110: prepare the syscon file to list other syscons
    nodes
  dt-bindings: ap806: add the thermal node in the syscon file
  dt-bindings: cp110: update documentation since DT de-duplication
  dt-bindings: cp110: add the thermal node in the syscon file
  dt-bindings: thermal: armada: add reference to new bindings
  arm64: dts: marvell: move AP806/CP110 thermal nodes into a new syscon
  arm64: dts: marvell: add thermal-zone node in ap806 DTSI file
  arm64: dts: marvell: add macro to make distinction between node names
  arm64: dts: marvell: add thermal-zone node in cp110 DTSI file

 .../arm/marvell/ap806-system-controller.txt        |  48 +-
 ...controller0.txt => cp110-system-controller.txt} |  61 ++-
 .../devicetree/bindings/thermal/armada-thermal.txt |   5 +
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi      |  74 ++-
 arch/arm64/boot/dts/marvell/armada-common.dtsi     |   1 +
 arch/arm64/boot/dts/marvell/armada-cp110.dtsi      |  37 +-
 drivers/thermal/armada_thermal.c                   | 534 ++++++++++++++++-----
 7 files changed, 606 insertions(+), 154 deletions(-)
 rename Documentation/devicetree/bindings/arm/marvell/{cp110-system-controller0.txt => cp110-system-controller.txt} (85%)

-- 
2.14.1

[PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Miquel Raynal]

Thermal zone names must follow certain rules imposed by the framework.
They are limited in length and shall not have any hyphen '-'.

This is done in a separate function for future use in another location.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 4c275ec10ac5..077e8e562306 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -70,6 +70,7 @@ struct armada_thermal_priv {
 	void __iomem *status;
 	void __iomem *control0;
 	void __iomem *control1;
+	char zone_name[THERMAL_NAME_LENGTH];
 	struct armada_thermal_data *data;
 };
 
@@ -353,6 +354,36 @@ static const struct of_device_id armada_thermal_id_table[] = {
 };
 MODULE_DEVICE_TABLE(of, armada_thermal_id_table);
 
+static void armada_set_sane_name(struct platform_device *pdev,
+				 struct armada_thermal_priv *priv)
+{
+	const char *name = dev_name(&pdev->dev);
+	char *insane_char;
+
+	if (strlen(name) > THERMAL_NAME_LENGTH) {
+		/*
+		 * When inside a system controller, the device name has the
+		 * form: f06f8000.system-controller:ap-thermal so stripping
+		 * after the ':' should give us a shorter but meaningful name.
+		 */
+		name = strrchr(name, ':');
+		if (!name)
+			name = "armada_thermal";
+		else
+			name++;
+	}
+
+	/* Save the name locally */
+	strncpy(priv->zone_name, name, THERMAL_NAME_LENGTH);
+
+	/* Then check there are no '-' or hwmon core will complain */
+	do {
+		insane_char = strpbrk(priv->zone_name, "-");
+		if (insane_char)
+			*insane_char = '_';
+	} while (insane_char);
+}
+
 static int armada_thermal_probe(struct platform_device *pdev)
 {
 	void __iomem *control = NULL;
@@ -381,6 +412,9 @@ static int armada_thermal_probe(struct platform_device *pdev)
 
 	priv->data = (struct armada_thermal_data *)match->data;
 
+	/* Ensure device name is correct for the thermal core */
+	armada_set_sane_name(pdev, priv);
+
 	/*
 	 * Legacy DT bindings only described "control1" register (also referred
 	 * as "control MSB" on old documentation). New bindings cover
@@ -402,7 +436,7 @@ static int armada_thermal_probe(struct platform_device *pdev)
 
 	priv->data->init_sensor(pdev, priv);
 
-	thermal = thermal_zone_device_register(dev_name(&pdev->dev), 0, 0, priv,
+	thermal = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
 					       &ops, NULL, 0, 0);
 	if (IS_ERR(thermal)) {
 		dev_err(&pdev->dev,
-- 
2.14.1

[PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Miquel Raynal]

Thermal zone names must follow certain rules imposed by the framework.
They are limited in length and shall not have any hyphen '-'.

This is done in a separate function for future use in another location.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 4c275ec10ac5..077e8e562306 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -70,6 +70,7 @@ struct armada_thermal_priv {
 	void __iomem *status;
 	void __iomem *control0;
 	void __iomem *control1;
+	char zone_name[THERMAL_NAME_LENGTH];
 	struct armada_thermal_data *data;
 };
 
@@ -353,6 +354,36 @@ static const struct of_device_id armada_thermal_id_table[] = {
 };
 MODULE_DEVICE_TABLE(of, armada_thermal_id_table);
 
+static void armada_set_sane_name(struct platform_device *pdev,
+				 struct armada_thermal_priv *priv)
+{
+	const char *name = dev_name(&pdev->dev);
+	char *insane_char;
+
+	if (strlen(name) > THERMAL_NAME_LENGTH) {
+		/*
+		 * When inside a system controller, the device name has the
+		 * form: f06f8000.system-controller:ap-thermal so stripping
+		 * after the ':' should give us a shorter but meaningful name.
+		 */
+		name = strrchr(name, ':');
+		if (!name)
+			name = "armada_thermal";
+		else
+			name++;
+	}
+
+	/* Save the name locally */
+	strncpy(priv->zone_name, name, THERMAL_NAME_LENGTH);
+
+	/* Then check there are no '-' or hwmon core will complain */
+	do {
+		insane_char = strpbrk(priv->zone_name, "-");
+		if (insane_char)
+			*insane_char = '_';
+	} while (insane_char);
+}
+
 static int armada_thermal_probe(struct platform_device *pdev)
 {
 	void __iomem *control = NULL;
@@ -381,6 +412,9 @@ static int armada_thermal_probe(struct platform_device *pdev)
 
 	priv->data = (struct armada_thermal_data *)match->data;
 
+	/* Ensure device name is correct for the thermal core */
+	armada_set_sane_name(pdev, priv);
+
 	/*
 	 * Legacy DT bindings only described "control1" register (also referred
 	 * as "control MSB" on old documentation). New bindings cover
@@ -402,7 +436,7 @@ static int armada_thermal_probe(struct platform_device *pdev)
 
 	priv->data->init_sensor(pdev, priv);
 
-	thermal = thermal_zone_device_register(dev_name(&pdev->dev), 0, 0, priv,
+	thermal = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
 					       &ops, NULL, 0, 0);
 	if (IS_ERR(thermal)) {
 		dev_err(&pdev->dev,
-- 
2.14.1

[PATCH v3 02/23] thermal: armada: remove useless register accesses

[Miquel Raynal]

Prepare the migration to use regmaps by first simplifying the
initialization functions: avoid unnecessary write/read cycles on
configuration registers.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 077e8e562306..6fdb90b3c001 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -103,16 +103,13 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
 
 	reg = readl_relaxed(priv->control1);
 	reg |= PMU_TDC0_OTF_CAL_MASK;
-	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->control1);
 
 	/* Reset the sensor */
-	reg = readl_relaxed(priv->control1);
-	writel((reg | PMU_TDC0_SW_RST_MASK), priv->control1);
+	reg |= PMU_TDC0_SW_RST_MASK;
 
 	writel(reg, priv->control1);
 
@@ -129,14 +126,13 @@ static void armada370_init_sensor(struct platform_device *pdev,
 
 	reg = readl_relaxed(priv->control1);
 	reg |= PMU_TDC0_OTF_CAL_MASK;
-	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->control1);
 
 	reg &= ~PMU_TDC0_START_CAL_MASK;
+
 	writel(reg, priv->control1);
 
 	msleep(10);
-- 
2.14.1

[PATCH v3 02/23] thermal: armada: remove useless register accesses

[Miquel Raynal]

Prepare the migration to use regmaps by first simplifying the
initialization functions: avoid unnecessary write/read cycles on
configuration registers.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 077e8e562306..6fdb90b3c001 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -103,16 +103,13 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
 
 	reg = readl_relaxed(priv->control1);
 	reg |= PMU_TDC0_OTF_CAL_MASK;
-	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->control1);
 
 	/* Reset the sensor */
-	reg = readl_relaxed(priv->control1);
-	writel((reg | PMU_TDC0_SW_RST_MASK), priv->control1);
+	reg |= PMU_TDC0_SW_RST_MASK;
 
 	writel(reg, priv->control1);
 
@@ -129,14 +126,13 @@ static void armada370_init_sensor(struct platform_device *pdev,
 
 	reg = readl_relaxed(priv->control1);
 	reg |= PMU_TDC0_OTF_CAL_MASK;
-	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->control1);
 
 	reg &= ~PMU_TDC0_START_CAL_MASK;
+
 	writel(reg, priv->control1);
 
 	msleep(10);
-- 
2.14.1

[PATCH v3 03/23] thermal: armada: remove misleading comments

[Miquel Raynal]

On older versions of this thermal IP, TSEN referred as the internal
sensor in the thermal IP while EXT_TSEN referred as sensors outside of
this IP, ie in the CPUs most of the time. The bit names in the
specifications do not follow this rule anymore, so remove these comments
that are misleading.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 6fdb90b3c001..18155e43e67f 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -51,12 +51,10 @@
 #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)
 #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)
 
-- 
2.14.1

[PATCH v3 03/23] thermal: armada: remove misleading comments

[Miquel Raynal]

On older versions of this thermal IP, TSEN referred as the internal
sensor in the thermal IP while EXT_TSEN referred as sensors outside of
this IP, ie in the CPUs most of the time. The bit names in the
specifications do not follow this rule anymore, so remove these comments
that are misleading.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 6fdb90b3c001..18155e43e67f 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -51,12 +51,10 @@
 #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)
 #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)
 
-- 
2.14.1

[PATCH v3 04/23] thermal: armada: rename the initialization routine

[Miquel Raynal]

Calling a hook ->init_sensor() while what is initialized is the IP
itself and not the sensors is misleading. Rename the hook ->init() to
avoid any confusion in later work bringing multi-sensors support.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 18155e43e67f..3d22a6016b04 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -73,9 +73,9 @@ struct armada_thermal_priv {
 };
 
 struct armada_thermal_data {
-	/* Initialize the sensor */
-	void (*init_sensor)(struct platform_device *pdev,
-			    struct armada_thermal_priv *);
+	/* Initialize the thermal IC */
+	void (*init)(struct platform_device *pdev,
+		     struct armada_thermal_priv *priv);
 
 	/* Test for a valid sensor value (optional) */
 	bool (*is_valid)(struct armada_thermal_priv *);
@@ -94,8 +94,8 @@ struct armada_thermal_data {
 	bool needs_control0;
 };
 
-static void armadaxp_init_sensor(struct platform_device *pdev,
-				 struct armada_thermal_priv *priv)
+static void armadaxp_init(struct platform_device *pdev,
+			  struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
@@ -117,8 +117,8 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
 	writel(reg, priv->status);
 }
 
-static void armada370_init_sensor(struct platform_device *pdev,
-				  struct armada_thermal_priv *priv)
+static void armada370_init(struct platform_device *pdev,
+			   struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
@@ -136,8 +136,8 @@ static void armada370_init_sensor(struct platform_device *pdev,
 	msleep(10);
 }
 
-static void armada375_init_sensor(struct platform_device *pdev,
-				  struct armada_thermal_priv *priv)
+static void armada375_init(struct platform_device *pdev,
+			   struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
@@ -164,8 +164,8 @@ static void armada_wait_sensor_validity(struct armada_thermal_priv *priv)
 				   STATUS_POLL_TIMEOUT_US);
 }
 
-static void armada380_init_sensor(struct platform_device *pdev,
-				  struct armada_thermal_priv *priv)
+static void armada380_init(struct platform_device *pdev,
+			   struct armada_thermal_priv *priv)
 {
 	u32 reg = readl_relaxed(priv->control1);
 
@@ -186,8 +186,8 @@ static void armada380_init_sensor(struct platform_device *pdev,
 	armada_wait_sensor_validity(priv);
 }
 
-static void armada_ap806_init_sensor(struct platform_device *pdev,
-				     struct armada_thermal_priv *priv)
+static void armada_ap806_init(struct platform_device *pdev,
+			      struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
@@ -247,7 +247,7 @@ static struct thermal_zone_device_ops ops = {
 };
 
 static const struct armada_thermal_data armadaxp_data = {
-	.init_sensor = armadaxp_init_sensor,
+	.init = armadaxp_init,
 	.temp_shift = 10,
 	.temp_mask = 0x1ff,
 	.coef_b = 3153000000ULL,
@@ -257,7 +257,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,
+	.init = armada370_init,
 	.is_valid_bit = BIT(9),
 	.temp_shift = 10,
 	.temp_mask = 0x1ff,
@@ -268,7 +268,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,
+	.init = armada375_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x1ff,
@@ -280,7 +280,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,
+	.init = armada380_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
@@ -292,7 +292,7 @@ static const struct armada_thermal_data armada380_data = {
 
 static const struct armada_thermal_data armada_ap806_data = {
 	.is_valid = armada_is_valid,
-	.init_sensor = armada_ap806_init_sensor,
+	.init = armada_ap806_init,
 	.is_valid_bit = BIT(16),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
@@ -306,7 +306,7 @@ static const struct armada_thermal_data armada_ap806_data = {
 
 static const struct armada_thermal_data armada_cp110_data = {
 	.is_valid = armada_is_valid,
-	.init_sensor = armada380_init_sensor,
+	.init = armada380_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
@@ -428,7 +428,7 @@ static int armada_thermal_probe(struct platform_device *pdev)
 		priv->control1 = control + CONTROL1_OFFSET;
 	}
 
-	priv->data->init_sensor(pdev, priv);
+	priv->data->init(pdev, priv);
 
 	thermal = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
 					       &ops, NULL, 0, 0);
-- 
2.14.1

[PATCH v3 04/23] thermal: armada: rename the initialization routine

[Miquel Raynal]

Calling a hook ->init_sensor() while what is initialized is the IP
itself and not the sensors is misleading. Rename the hook ->init() to
avoid any confusion in later work bringing multi-sensors support.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 18155e43e67f..3d22a6016b04 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -73,9 +73,9 @@ struct armada_thermal_priv {
 };
 
 struct armada_thermal_data {
-	/* Initialize the sensor */
-	void (*init_sensor)(struct platform_device *pdev,
-			    struct armada_thermal_priv *);
+	/* Initialize the thermal IC */
+	void (*init)(struct platform_device *pdev,
+		     struct armada_thermal_priv *priv);
 
 	/* Test for a valid sensor value (optional) */
 	bool (*is_valid)(struct armada_thermal_priv *);
@@ -94,8 +94,8 @@ struct armada_thermal_data {
 	bool needs_control0;
 };
 
-static void armadaxp_init_sensor(struct platform_device *pdev,
-				 struct armada_thermal_priv *priv)
+static void armadaxp_init(struct platform_device *pdev,
+			  struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
@@ -117,8 +117,8 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
 	writel(reg, priv->status);
 }
 
-static void armada370_init_sensor(struct platform_device *pdev,
-				  struct armada_thermal_priv *priv)
+static void armada370_init(struct platform_device *pdev,
+			   struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
@@ -136,8 +136,8 @@ static void armada370_init_sensor(struct platform_device *pdev,
 	msleep(10);
 }
 
-static void armada375_init_sensor(struct platform_device *pdev,
-				  struct armada_thermal_priv *priv)
+static void armada375_init(struct platform_device *pdev,
+			   struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
@@ -164,8 +164,8 @@ static void armada_wait_sensor_validity(struct armada_thermal_priv *priv)
 				   STATUS_POLL_TIMEOUT_US);
 }
 
-static void armada380_init_sensor(struct platform_device *pdev,
-				  struct armada_thermal_priv *priv)
+static void armada380_init(struct platform_device *pdev,
+			   struct armada_thermal_priv *priv)
 {
 	u32 reg = readl_relaxed(priv->control1);
 
@@ -186,8 +186,8 @@ static void armada380_init_sensor(struct platform_device *pdev,
 	armada_wait_sensor_validity(priv);
 }
 
-static void armada_ap806_init_sensor(struct platform_device *pdev,
-				     struct armada_thermal_priv *priv)
+static void armada_ap806_init(struct platform_device *pdev,
+			      struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
@@ -247,7 +247,7 @@ static struct thermal_zone_device_ops ops = {
 };
 
 static const struct armada_thermal_data armadaxp_data = {
-	.init_sensor = armadaxp_init_sensor,
+	.init = armadaxp_init,
 	.temp_shift = 10,
 	.temp_mask = 0x1ff,
 	.coef_b = 3153000000ULL,
@@ -257,7 +257,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,
+	.init = armada370_init,
 	.is_valid_bit = BIT(9),
 	.temp_shift = 10,
 	.temp_mask = 0x1ff,
@@ -268,7 +268,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,
+	.init = armada375_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x1ff,
@@ -280,7 +280,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,
+	.init = armada380_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
@@ -292,7 +292,7 @@ static const struct armada_thermal_data armada380_data = {
 
 static const struct armada_thermal_data armada_ap806_data = {
 	.is_valid = armada_is_valid,
-	.init_sensor = armada_ap806_init_sensor,
+	.init = armada_ap806_init,
 	.is_valid_bit = BIT(16),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
@@ -306,7 +306,7 @@ static const struct armada_thermal_data armada_ap806_data = {
 
 static const struct armada_thermal_data armada_cp110_data = {
 	.is_valid = armada_is_valid,
-	.init_sensor = armada380_init_sensor,
+	.init = armada380_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
@@ -428,7 +428,7 @@ static int armada_thermal_probe(struct platform_device *pdev)
 		priv->control1 = control + CONTROL1_OFFSET;
 	}
 
-	priv->data->init_sensor(pdev, priv);
+	priv->data->init(pdev, priv);
 
 	thermal = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
 					       &ops, NULL, 0, 0);
-- 
2.14.1

[PATCH v3 05/23] thermal: armada: dissociate a380 and cp110 ->init() hooks

[Miquel Raynal]

Until now, Armada 380 and CP110 could share the same ->init() function
because their use was identical.

Prepare the support of multi-sensors support and overheat interrupt
feature by separating the initialization paths before they actually
diverge.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 3d22a6016b04..9291ea3ad2f7 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -200,6 +200,12 @@ static void armada_ap806_init(struct platform_device *pdev,
 	armada_wait_sensor_validity(priv);
 }
 
+static void armada_cp110_init(struct platform_device *pdev,
+			      struct armada_thermal_priv *priv)
+{
+	armada380_init(pdev, priv);
+}
+
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
 	u32 reg = readl_relaxed(priv->status);
@@ -306,7 +312,7 @@ static const struct armada_thermal_data armada_ap806_data = {
 
 static const struct armada_thermal_data armada_cp110_data = {
 	.is_valid = armada_is_valid,
-	.init = armada380_init,
+	.init = armada_cp110_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
-- 
2.14.1

[PATCH v3 05/23] thermal: armada: dissociate a380 and cp110 ->init() hooks

[Miquel Raynal]

Until now, Armada 380 and CP110 could share the same ->init() function
because their use was identical.

Prepare the support of multi-sensors support and overheat interrupt
feature by separating the initialization paths before they actually
diverge.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 3d22a6016b04..9291ea3ad2f7 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -200,6 +200,12 @@ static void armada_ap806_init(struct platform_device *pdev,
 	armada_wait_sensor_validity(priv);
 }
 
+static void armada_cp110_init(struct platform_device *pdev,
+			      struct armada_thermal_priv *priv)
+{
+	armada380_init(pdev, priv);
+}
+
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
 	u32 reg = readl_relaxed(priv->status);
@@ -306,7 +312,7 @@ static const struct armada_thermal_data armada_ap806_data = {
 
 static const struct armada_thermal_data armada_cp110_data = {
 	.is_valid = armada_is_valid,
-	.init = armada380_init,
+	.init = armada_cp110_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
-- 
2.14.1

[PATCH v3 06/23] thermal: armada: average over samples to avoid glitches

[Miquel Raynal]

Configure the sample frequency and number of averaged samples.

This is needed for two reasons:
1/ To be bootloader independent.
2/ To prepare the introduction of multi-sensors support by preventing
   inconsistencies when reading temperatures that could be a mean of
   samples took from different sensors.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 9291ea3ad2f7..1f9706d96a0d 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -54,7 +54,12 @@
 #define CONTROL0_TSEN_START		BIT(0)
 #define CONTROL0_TSEN_RESET		BIT(1)
 #define CONTROL0_TSEN_ENABLE		BIT(2)
+#define CONTROL0_TSEN_AVG_BYPASS	BIT(6)
+#define CONTROL0_TSEN_OSR_SHIFT		24
+#define CONTROL0_TSEN_OSR_MAX		0x3
 
+#define CONTROL1_TSEN_AVG_SHIFT		0
+#define CONTROL1_TSEN_AVG_MASK		0x7
 #define CONTROL1_EXT_TSEN_SW_RESET	BIT(7)
 #define CONTROL1_EXT_TSEN_HW_RESETn	BIT(8)
 
@@ -194,6 +199,13 @@ static void armada_ap806_init(struct platform_device *pdev,
 	reg = readl_relaxed(priv->control0);
 	reg &= ~CONTROL0_TSEN_RESET;
 	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
+
+	/* Sample every ~2ms */
+	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
+
+	/* Enable average (2 samples by default) */
+	reg &= ~CONTROL0_TSEN_AVG_BYPASS;
+
 	writel(reg, priv->control0);
 
 	/* Wait the sensors to be valid or the core will warn the user */
@@ -203,7 +215,20 @@ static void armada_ap806_init(struct platform_device *pdev,
 static void armada_cp110_init(struct platform_device *pdev,
 			      struct armada_thermal_priv *priv)
 {
+	u32 reg;
+
 	armada380_init(pdev, priv);
+
+	/* Sample every ~2ms */
+	reg = readl_relaxed(priv->control0);
+	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
+	writel(reg, priv->control0);
+
+	/* Average the output value over 2^1 = 2 samples */
+	reg = readl_relaxed(priv->control1);
+	reg &= ~CONTROL1_TSEN_AVG_MASK << CONTROL1_TSEN_AVG_SHIFT;
+	reg |= 1 << CONTROL1_TSEN_AVG_SHIFT;
+	writel(reg, priv->control1);
 }
 
 static bool armada_is_valid(struct armada_thermal_priv *priv)
-- 
2.14.1

[PATCH v3 06/23] thermal: armada: average over samples to avoid glitches

[Miquel Raynal]

Configure the sample frequency and number of averaged samples.

This is needed for two reasons:
1/ To be bootloader independent.
2/ To prepare the introduction of multi-sensors support by preventing
   inconsistencies when reading temperatures that could be a mean of
   samples took from different sensors.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 9291ea3ad2f7..1f9706d96a0d 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -54,7 +54,12 @@
 #define CONTROL0_TSEN_START		BIT(0)
 #define CONTROL0_TSEN_RESET		BIT(1)
 #define CONTROL0_TSEN_ENABLE		BIT(2)
+#define CONTROL0_TSEN_AVG_BYPASS	BIT(6)
+#define CONTROL0_TSEN_OSR_SHIFT		24
+#define CONTROL0_TSEN_OSR_MAX		0x3
 
+#define CONTROL1_TSEN_AVG_SHIFT		0
+#define CONTROL1_TSEN_AVG_MASK		0x7
 #define CONTROL1_EXT_TSEN_SW_RESET	BIT(7)
 #define CONTROL1_EXT_TSEN_HW_RESETn	BIT(8)
 
@@ -194,6 +199,13 @@ static void armada_ap806_init(struct platform_device *pdev,
 	reg = readl_relaxed(priv->control0);
 	reg &= ~CONTROL0_TSEN_RESET;
 	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
+
+	/* Sample every ~2ms */
+	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
+
+	/* Enable average (2 samples by default) */
+	reg &= ~CONTROL0_TSEN_AVG_BYPASS;
+
 	writel(reg, priv->control0);
 
 	/* Wait the sensors to be valid or the core will warn the user */
@@ -203,7 +215,20 @@ static void armada_ap806_init(struct platform_device *pdev,
 static void armada_cp110_init(struct platform_device *pdev,
 			      struct armada_thermal_priv *priv)
 {
+	u32 reg;
+
 	armada380_init(pdev, priv);
+
+	/* Sample every ~2ms */
+	reg = readl_relaxed(priv->control0);
+	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
+	writel(reg, priv->control0);
+
+	/* Average the output value over 2^1 = 2 samples */
+	reg = readl_relaxed(priv->control1);
+	reg &= ~CONTROL1_TSEN_AVG_MASK << CONTROL1_TSEN_AVG_SHIFT;
+	reg |= 1 << CONTROL1_TSEN_AVG_SHIFT;
+	writel(reg, priv->control1);
 }
 
 static bool armada_is_valid(struct armada_thermal_priv *priv)
-- 
2.14.1

[PATCH v3 07/23] thermal: armada: convert driver to syscon register accesses

[Miquel Raynal]

Until recently, only one register was referenced in MVEBU thermal IP
node. Recent changes added a second entry pointing to another
register right next to it. We cannot know for sure that we will not
have to access other registers. That will be actually the case when
overheat interrupt feature will come, where it will be needed to access
DFX registers in the same area.

This approach is not scalable so instead of adding consinuously memory
areas in the DT (and change the DT bindings, while keeping backward
compatibility), move the thermal node into a wider syscon from which it
will be possible to also configure the thermal interrupt.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 197 +++++++++++++++++++++++++--------------
 1 file changed, 128 insertions(+), 69 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 1f9706d96a0d..be346c6afde2 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -24,6 +24,8 @@
 #include <linux/of_device.h>
 #include <linux/thermal.h>
 #include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 
 /* Thermal Manager Control and Status Register */
 #define PMU_TDC0_SW_RST_MASK		(0x1 << 1)
@@ -39,14 +41,6 @@
 #define A375_READOUT_INVERT		BIT(15)
 #define A375_HW_RESETn			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
-
 /* Errata fields */
 #define CONTROL0_TSEN_TC_TRIM_MASK	0x7
 #define CONTROL0_TSEN_TC_TRIM_VAL	0x3
@@ -70,9 +64,7 @@ struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
 struct armada_thermal_priv {
-	void __iomem *status;
-	void __iomem *control0;
-	void __iomem *control1;
+	struct regmap *syscon;
 	char zone_name[THERMAL_NAME_LENGTH];
 	struct armada_thermal_data *data;
 };
@@ -96,15 +88,20 @@ struct armada_thermal_data {
 	unsigned int temp_shift;
 	unsigned int temp_mask;
 	u32 is_valid_bit;
-	bool needs_control0;
+
+	/* Syscon access */
+	unsigned int syscon_control0_off;
+	unsigned int syscon_control1_off;
+	unsigned int syscon_status_off;
 };
 
 static void armadaxp_init(struct platform_device *pdev,
 			  struct armada_thermal_priv *priv)
 {
+	struct armada_thermal_data *data = priv->data;
 	u32 reg;
 
-	reg = readl_relaxed(priv->control1);
+	regmap_read(priv->syscon, data->syscon_control1_off, &reg);
 	reg |= PMU_TDC0_OTF_CAL_MASK;
 
 	/* Reference calibration value */
@@ -114,29 +111,31 @@ static void armadaxp_init(struct platform_device *pdev,
 	/* Reset the sensor */
 	reg |= PMU_TDC0_SW_RST_MASK;
 
-	writel(reg, priv->control1);
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
 
 	/* Enable the sensor */
-	reg = readl_relaxed(priv->status);
+	regmap_read(priv->syscon, data->syscon_status_off, &reg);
 	reg &= ~PMU_TM_DISABLE_MASK;
-	writel(reg, priv->status);
+	regmap_write(priv->syscon, data->syscon_status_off, reg);
 }
 
 static void armada370_init(struct platform_device *pdev,
 			   struct armada_thermal_priv *priv)
 {
+	struct armada_thermal_data *data = priv->data;
 	u32 reg;
 
-	reg = readl_relaxed(priv->control1);
+	regmap_read(priv->syscon, data->syscon_control1_off, &reg);
 	reg |= PMU_TDC0_OTF_CAL_MASK;
 
 	/* Reference calibration value */
 	reg &= ~PMU_TDC0_REF_CAL_CNT_MASK;
 	reg |= (0xf1 << PMU_TDC0_REF_CAL_CNT_OFFS);
 
+	/* Reset the sensor */
 	reg &= ~PMU_TDC0_START_CAL_MASK;
 
-	writel(reg, priv->control1);
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
 
 	msleep(10);
 }
@@ -144,18 +143,20 @@ static void armada370_init(struct platform_device *pdev,
 static void armada375_init(struct platform_device *pdev,
 			   struct armada_thermal_priv *priv)
 {
+	struct armada_thermal_data *data = priv->data;
 	u32 reg;
 
-	reg = readl(priv->control1);
+	regmap_read(priv->syscon, data->syscon_control1_off, &reg);
 	reg &= ~(A375_UNIT_CONTROL_MASK << A375_UNIT_CONTROL_SHIFT);
 	reg &= ~A375_READOUT_INVERT;
 	reg &= ~A375_HW_RESETn;
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
 
-	writel(reg, priv->control1);
 	msleep(20);
 
 	reg |= A375_HW_RESETn;
-	writel(reg, priv->control1);
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
+
 	msleep(50);
 }
 
@@ -163,29 +164,29 @@ 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);
+	regmap_read_poll_timeout(priv->syscon, priv->data->syscon_status_off,
+				 reg, reg & priv->data->is_valid_bit,
+				 STATUS_POLL_PERIOD_US,
+				 STATUS_POLL_TIMEOUT_US);
 }
 
 static void armada380_init(struct platform_device *pdev,
 			   struct armada_thermal_priv *priv)
 {
-	u32 reg = readl_relaxed(priv->control1);
+	struct armada_thermal_data *data = priv->data;
+	u32 reg;
 
 	/* Disable the HW/SW reset */
+	regmap_read(priv->syscon, data->syscon_control1_off, &reg);
 	reg |= CONTROL1_EXT_TSEN_HW_RESETn;
 	reg &= ~CONTROL1_EXT_TSEN_SW_RESET;
-	writel(reg, priv->control1);
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
 
 	/* 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);
-	}
+	regmap_read(priv->syscon, data->syscon_control0_off, &reg);
+	reg &= ~CONTROL0_TSEN_TC_TRIM_MASK;
+	reg |= CONTROL0_TSEN_TC_TRIM_VAL;
+	regmap_write(priv->syscon, data->syscon_control0_off, reg);
 
 	/* Wait the sensors to be valid or the core will warn the user */
 	armada_wait_sensor_validity(priv);
@@ -194,9 +195,10 @@ static void armada380_init(struct platform_device *pdev,
 static void armada_ap806_init(struct platform_device *pdev,
 			      struct armada_thermal_priv *priv)
 {
+	struct armada_thermal_data *data = priv->data;
 	u32 reg;
 
-	reg = readl_relaxed(priv->control0);
+	regmap_read(priv->syscon, data->syscon_control0_off, &reg);
 	reg &= ~CONTROL0_TSEN_RESET;
 	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
 
@@ -206,7 +208,7 @@ static void armada_ap806_init(struct platform_device *pdev,
 	/* Enable average (2 samples by default) */
 	reg &= ~CONTROL0_TSEN_AVG_BYPASS;
 
-	writel(reg, priv->control0);
+	regmap_write(priv->syscon, data->syscon_control0_off, reg);
 
 	/* Wait the sensors to be valid or the core will warn the user */
 	armada_wait_sensor_validity(priv);
@@ -215,25 +217,28 @@ static void armada_ap806_init(struct platform_device *pdev,
 static void armada_cp110_init(struct platform_device *pdev,
 			      struct armada_thermal_priv *priv)
 {
+	struct armada_thermal_data *data = priv->data;
 	u32 reg;
 
 	armada380_init(pdev, priv);
 
 	/* Sample every ~2ms */
-	reg = readl_relaxed(priv->control0);
+	regmap_read(priv->syscon, data->syscon_control0_off, &reg);
 	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
-	writel(reg, priv->control0);
+	regmap_write(priv->syscon, data->syscon_control0_off, reg);
 
 	/* Average the output value over 2^1 = 2 samples */
-	reg = readl_relaxed(priv->control1);
+	regmap_read(priv->syscon, data->syscon_control1_off, &reg);
 	reg &= ~CONTROL1_TSEN_AVG_MASK << CONTROL1_TSEN_AVG_SHIFT;
 	reg |= 1 << CONTROL1_TSEN_AVG_SHIFT;
-	writel(reg, priv->control1);
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
 }
 
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
-	u32 reg = readl_relaxed(priv->status);
+	u32 reg;
+
+	regmap_read(priv->syscon, priv->data->syscon_status_off, &reg);
 
 	return reg & priv->data->is_valid_bit;
 }
@@ -252,7 +257,7 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 		return -EIO;
 	}
 
-	reg = readl_relaxed(priv->status);
+	regmap_read(priv->syscon, priv->data->syscon_status_off, &reg);
 	reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;
 	if (priv->data->signed_sample)
 		/* The most significant bit is the sign bit */
@@ -284,6 +289,8 @@ static const struct armada_thermal_data armadaxp_data = {
 	.coef_b = 3153000000ULL,
 	.coef_m = 10000000ULL,
 	.coef_div = 13825,
+	.syscon_status_off = 0xb0,
+	.syscon_control1_off = 0xd0,
 };
 
 static const struct armada_thermal_data armada370_data = {
@@ -295,6 +302,8 @@ static const struct armada_thermal_data armada370_data = {
 	.coef_b = 3153000000ULL,
 	.coef_m = 10000000ULL,
 	.coef_div = 13825,
+	.syscon_status_off = 0x0,
+	.syscon_control1_off = 0x4,
 };
 
 static const struct armada_thermal_data armada375_data = {
@@ -306,7 +315,9 @@ static const struct armada_thermal_data armada375_data = {
 	.coef_b = 3171900000ULL,
 	.coef_m = 10000000ULL,
 	.coef_div = 13616,
-	.needs_control0 = true,
+	.syscon_status_off = 0x78,
+	.syscon_control0_off = 0x7c,
+	.syscon_control1_off = 0x80,
 };
 
 static const struct armada_thermal_data armada380_data = {
@@ -319,6 +330,9 @@ static const struct armada_thermal_data armada380_data = {
 	.coef_m = 2000096ULL,
 	.coef_div = 4201,
 	.inverted = true,
+	.syscon_control0_off = 0x70,
+	.syscon_control1_off = 0x74,
+	.syscon_status_off = 0x78,
 };
 
 static const struct armada_thermal_data armada_ap806_data = {
@@ -332,7 +346,9 @@ static const struct armada_thermal_data armada_ap806_data = {
 	.coef_div = 1,
 	.inverted = true,
 	.signed_sample = true,
-	.needs_control0 = true,
+	.syscon_control0_off = 0x84,
+	.syscon_control1_off = 0x88,
+	.syscon_status_off = 0x8C,
 };
 
 static const struct armada_thermal_data armada_cp110_data = {
@@ -345,7 +361,9 @@ static const struct armada_thermal_data armada_cp110_data = {
 	.coef_m = 2000096ULL,
 	.coef_div = 4201,
 	.inverted = true,
-	.needs_control0 = true,
+	.syscon_control0_off = 0x70,
+	.syscon_control1_off = 0x74,
+	.syscon_status_off = 0x78,
 };
 
 static const struct of_device_id armada_thermal_id_table[] = {
@@ -379,6 +397,57 @@ static const struct of_device_id armada_thermal_id_table[] = {
 };
 MODULE_DEVICE_TABLE(of, armada_thermal_id_table);
 
+static const struct regmap_config armada_thermal_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.fast_io = true,
+};
+
+static int armada_thermal_probe_legacy(struct platform_device *pdev,
+				       struct armada_thermal_priv *priv)
+{
+	struct armada_thermal_data *data = priv->data;
+	struct resource *res;
+	void __iomem *base;
+
+	/* First memory region points towards the status register */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (IS_ERR(res))
+		return PTR_ERR(res);
+
+	/*
+	 * Edit the resource start address and length to map over all the
+	 * registers, instead of pointing at them one by one.
+	 */
+	res->start -= data->syscon_status_off;
+	res->end = res->start + max(data->syscon_status_off,
+				    max(data->syscon_control0_off,
+					data->syscon_control1_off)) +
+		   sizeof(unsigned int) - 1;
+
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->syscon = devm_regmap_init_mmio(&pdev->dev, base,
+					     &armada_thermal_regmap_config);
+	if (IS_ERR(priv->syscon))
+		return PTR_ERR(priv->syscon);
+
+	return 0;
+}
+
+static int armada_thermal_probe_syscon(struct platform_device *pdev,
+				       struct armada_thermal_priv *priv)
+{
+	priv->syscon = syscon_node_to_regmap(pdev->dev.parent->of_node);
+	if (IS_ERR(priv->syscon))
+		return PTR_ERR(priv->syscon);
+
+	return 0;
+}
+
 static void armada_set_sane_name(struct platform_device *pdev,
 				 struct armada_thermal_priv *priv)
 {
@@ -411,11 +480,10 @@ static void armada_set_sane_name(struct platform_device *pdev,
 
 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;
-	struct resource *res;
+	int ret;
 
 	match = of_match_device(armada_thermal_id_table, &pdev->dev);
 	if (!match)
@@ -425,16 +493,6 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	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);
-	if (IS_ERR(control))
-		return PTR_ERR(control);
-
 	priv->data = (struct armada_thermal_data *)match->data;
 
 	/* Ensure device name is correct for the thermal core */
@@ -442,22 +500,23 @@ static int armada_thermal_probe(struct platform_device *pdev)
 
 	/*
 	 * Legacy DT bindings only described "control1" register (also referred
-	 * as "control MSB" on old documentation). New bindings cover
+	 * as "control MSB" on old documentation). Then, bindings moved to cover
 	 * "control0/control LSB" and "control1/control MSB" registers within
-	 * the same resource, which is then of size 8 instead of 4.
+	 * the same resource, which was then of size 8 instead of 4.
+	 *
+	 * The logic of defining sporadic registers is broken. For instance, it
+	 * blocked the addition of the overheat interrupt feature that needed
+	 * another resource somewhere else in the same memory area. One solution
+	 * is to define an overall system controller and put the thermal node
+	 * into it, which requires the use of regmaps across all the driver.
 	 */
-	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;
-		}
+	if (IS_ERR(syscon_node_to_regmap(pdev->dev.parent->of_node)))
+		ret = armada_thermal_probe_legacy(pdev, priv);
+	else
+		ret = armada_thermal_probe_syscon(pdev, priv);
 
-		priv->control1 = control + LEGACY_CONTROL1_OFFSET;
-	} else {
-		priv->control0 = control + CONTROL0_OFFSET;
-		priv->control1 = control + CONTROL1_OFFSET;
-	}
+	if (ret)
+		return ret;
 
 	priv->data->init(pdev, priv);
 
-- 
2.14.1

[PATCH v3 07/23] thermal: armada: convert driver to syscon register accesses

[Miquel Raynal]

Until recently, only one register was referenced in MVEBU thermal IP
node. Recent changes added a second entry pointing to another
register right next to it. We cannot know for sure that we will not
have to access other registers. That will be actually the case when
overheat interrupt feature will come, where it will be needed to access
DFX registers in the same area.

This approach is not scalable so instead of adding consinuously memory
areas in the DT (and change the DT bindings, while keeping backward
compatibility), move the thermal node into a wider syscon from which it
will be possible to also configure the thermal interrupt.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 197 +++++++++++++++++++++++++--------------
 1 file changed, 128 insertions(+), 69 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 1f9706d96a0d..be346c6afde2 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -24,6 +24,8 @@
 #include <linux/of_device.h>
 #include <linux/thermal.h>
 #include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 
 /* Thermal Manager Control and Status Register */
 #define PMU_TDC0_SW_RST_MASK		(0x1 << 1)
@@ -39,14 +41,6 @@
 #define A375_READOUT_INVERT		BIT(15)
 #define A375_HW_RESETn			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
-
 /* Errata fields */
 #define CONTROL0_TSEN_TC_TRIM_MASK	0x7
 #define CONTROL0_TSEN_TC_TRIM_VAL	0x3
@@ -70,9 +64,7 @@ struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
 struct armada_thermal_priv {
-	void __iomem *status;
-	void __iomem *control0;
-	void __iomem *control1;
+	struct regmap *syscon;
 	char zone_name[THERMAL_NAME_LENGTH];
 	struct armada_thermal_data *data;
 };
@@ -96,15 +88,20 @@ struct armada_thermal_data {
 	unsigned int temp_shift;
 	unsigned int temp_mask;
 	u32 is_valid_bit;
-	bool needs_control0;
+
+	/* Syscon access */
+	unsigned int syscon_control0_off;
+	unsigned int syscon_control1_off;
+	unsigned int syscon_status_off;
 };
 
 static void armadaxp_init(struct platform_device *pdev,
 			  struct armada_thermal_priv *priv)
 {
+	struct armada_thermal_data *data = priv->data;
 	u32 reg;
 
-	reg = readl_relaxed(priv->control1);
+	regmap_read(priv->syscon, data->syscon_control1_off, &reg);
 	reg |= PMU_TDC0_OTF_CAL_MASK;
 
 	/* Reference calibration value */
@@ -114,29 +111,31 @@ static void armadaxp_init(struct platform_device *pdev,
 	/* Reset the sensor */
 	reg |= PMU_TDC0_SW_RST_MASK;
 
-	writel(reg, priv->control1);
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
 
 	/* Enable the sensor */
-	reg = readl_relaxed(priv->status);
+	regmap_read(priv->syscon, data->syscon_status_off, &reg);
 	reg &= ~PMU_TM_DISABLE_MASK;
-	writel(reg, priv->status);
+	regmap_write(priv->syscon, data->syscon_status_off, reg);
 }
 
 static void armada370_init(struct platform_device *pdev,
 			   struct armada_thermal_priv *priv)
 {
+	struct armada_thermal_data *data = priv->data;
 	u32 reg;
 
-	reg = readl_relaxed(priv->control1);
+	regmap_read(priv->syscon, data->syscon_control1_off, &reg);
 	reg |= PMU_TDC0_OTF_CAL_MASK;
 
 	/* Reference calibration value */
 	reg &= ~PMU_TDC0_REF_CAL_CNT_MASK;
 	reg |= (0xf1 << PMU_TDC0_REF_CAL_CNT_OFFS);
 
+	/* Reset the sensor */
 	reg &= ~PMU_TDC0_START_CAL_MASK;
 
-	writel(reg, priv->control1);
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
 
 	msleep(10);
 }
@@ -144,18 +143,20 @@ static void armada370_init(struct platform_device *pdev,
 static void armada375_init(struct platform_device *pdev,
 			   struct armada_thermal_priv *priv)
 {
+	struct armada_thermal_data *data = priv->data;
 	u32 reg;
 
-	reg = readl(priv->control1);
+	regmap_read(priv->syscon, data->syscon_control1_off, &reg);
 	reg &= ~(A375_UNIT_CONTROL_MASK << A375_UNIT_CONTROL_SHIFT);
 	reg &= ~A375_READOUT_INVERT;
 	reg &= ~A375_HW_RESETn;
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
 
-	writel(reg, priv->control1);
 	msleep(20);
 
 	reg |= A375_HW_RESETn;
-	writel(reg, priv->control1);
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
+
 	msleep(50);
 }
 
@@ -163,29 +164,29 @@ 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);
+	regmap_read_poll_timeout(priv->syscon, priv->data->syscon_status_off,
+				 reg, reg & priv->data->is_valid_bit,
+				 STATUS_POLL_PERIOD_US,
+				 STATUS_POLL_TIMEOUT_US);
 }
 
 static void armada380_init(struct platform_device *pdev,
 			   struct armada_thermal_priv *priv)
 {
-	u32 reg = readl_relaxed(priv->control1);
+	struct armada_thermal_data *data = priv->data;
+	u32 reg;
 
 	/* Disable the HW/SW reset */
+	regmap_read(priv->syscon, data->syscon_control1_off, &reg);
 	reg |= CONTROL1_EXT_TSEN_HW_RESETn;
 	reg &= ~CONTROL1_EXT_TSEN_SW_RESET;
-	writel(reg, priv->control1);
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
 
 	/* 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);
-	}
+	regmap_read(priv->syscon, data->syscon_control0_off, &reg);
+	reg &= ~CONTROL0_TSEN_TC_TRIM_MASK;
+	reg |= CONTROL0_TSEN_TC_TRIM_VAL;
+	regmap_write(priv->syscon, data->syscon_control0_off, reg);
 
 	/* Wait the sensors to be valid or the core will warn the user */
 	armada_wait_sensor_validity(priv);
@@ -194,9 +195,10 @@ static void armada380_init(struct platform_device *pdev,
 static void armada_ap806_init(struct platform_device *pdev,
 			      struct armada_thermal_priv *priv)
 {
+	struct armada_thermal_data *data = priv->data;
 	u32 reg;
 
-	reg = readl_relaxed(priv->control0);
+	regmap_read(priv->syscon, data->syscon_control0_off, &reg);
 	reg &= ~CONTROL0_TSEN_RESET;
 	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
 
@@ -206,7 +208,7 @@ static void armada_ap806_init(struct platform_device *pdev,
 	/* Enable average (2 samples by default) */
 	reg &= ~CONTROL0_TSEN_AVG_BYPASS;
 
-	writel(reg, priv->control0);
+	regmap_write(priv->syscon, data->syscon_control0_off, reg);
 
 	/* Wait the sensors to be valid or the core will warn the user */
 	armada_wait_sensor_validity(priv);
@@ -215,25 +217,28 @@ static void armada_ap806_init(struct platform_device *pdev,
 static void armada_cp110_init(struct platform_device *pdev,
 			      struct armada_thermal_priv *priv)
 {
+	struct armada_thermal_data *data = priv->data;
 	u32 reg;
 
 	armada380_init(pdev, priv);
 
 	/* Sample every ~2ms */
-	reg = readl_relaxed(priv->control0);
+	regmap_read(priv->syscon, data->syscon_control0_off, &reg);
 	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
-	writel(reg, priv->control0);
+	regmap_write(priv->syscon, data->syscon_control0_off, reg);
 
 	/* Average the output value over 2^1 = 2 samples */
-	reg = readl_relaxed(priv->control1);
+	regmap_read(priv->syscon, data->syscon_control1_off, &reg);
 	reg &= ~CONTROL1_TSEN_AVG_MASK << CONTROL1_TSEN_AVG_SHIFT;
 	reg |= 1 << CONTROL1_TSEN_AVG_SHIFT;
-	writel(reg, priv->control1);
+	regmap_write(priv->syscon, data->syscon_control1_off, reg);
 }
 
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
-	u32 reg = readl_relaxed(priv->status);
+	u32 reg;
+
+	regmap_read(priv->syscon, priv->data->syscon_status_off, &reg);
 
 	return reg & priv->data->is_valid_bit;
 }
@@ -252,7 +257,7 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 		return -EIO;
 	}
 
-	reg = readl_relaxed(priv->status);
+	regmap_read(priv->syscon, priv->data->syscon_status_off, &reg);
 	reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;
 	if (priv->data->signed_sample)
 		/* The most significant bit is the sign bit */
@@ -284,6 +289,8 @@ static const struct armada_thermal_data armadaxp_data = {
 	.coef_b = 3153000000ULL,
 	.coef_m = 10000000ULL,
 	.coef_div = 13825,
+	.syscon_status_off = 0xb0,
+	.syscon_control1_off = 0xd0,
 };
 
 static const struct armada_thermal_data armada370_data = {
@@ -295,6 +302,8 @@ static const struct armada_thermal_data armada370_data = {
 	.coef_b = 3153000000ULL,
 	.coef_m = 10000000ULL,
 	.coef_div = 13825,
+	.syscon_status_off = 0x0,
+	.syscon_control1_off = 0x4,
 };
 
 static const struct armada_thermal_data armada375_data = {
@@ -306,7 +315,9 @@ static const struct armada_thermal_data armada375_data = {
 	.coef_b = 3171900000ULL,
 	.coef_m = 10000000ULL,
 	.coef_div = 13616,
-	.needs_control0 = true,
+	.syscon_status_off = 0x78,
+	.syscon_control0_off = 0x7c,
+	.syscon_control1_off = 0x80,
 };
 
 static const struct armada_thermal_data armada380_data = {
@@ -319,6 +330,9 @@ static const struct armada_thermal_data armada380_data = {
 	.coef_m = 2000096ULL,
 	.coef_div = 4201,
 	.inverted = true,
+	.syscon_control0_off = 0x70,
+	.syscon_control1_off = 0x74,
+	.syscon_status_off = 0x78,
 };
 
 static const struct armada_thermal_data armada_ap806_data = {
@@ -332,7 +346,9 @@ static const struct armada_thermal_data armada_ap806_data = {
 	.coef_div = 1,
 	.inverted = true,
 	.signed_sample = true,
-	.needs_control0 = true,
+	.syscon_control0_off = 0x84,
+	.syscon_control1_off = 0x88,
+	.syscon_status_off = 0x8C,
 };
 
 static const struct armada_thermal_data armada_cp110_data = {
@@ -345,7 +361,9 @@ static const struct armada_thermal_data armada_cp110_data = {
 	.coef_m = 2000096ULL,
 	.coef_div = 4201,
 	.inverted = true,
-	.needs_control0 = true,
+	.syscon_control0_off = 0x70,
+	.syscon_control1_off = 0x74,
+	.syscon_status_off = 0x78,
 };
 
 static const struct of_device_id armada_thermal_id_table[] = {
@@ -379,6 +397,57 @@ static const struct of_device_id armada_thermal_id_table[] = {
 };
 MODULE_DEVICE_TABLE(of, armada_thermal_id_table);
 
+static const struct regmap_config armada_thermal_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.fast_io = true,
+};
+
+static int armada_thermal_probe_legacy(struct platform_device *pdev,
+				       struct armada_thermal_priv *priv)
+{
+	struct armada_thermal_data *data = priv->data;
+	struct resource *res;
+	void __iomem *base;
+
+	/* First memory region points towards the status register */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (IS_ERR(res))
+		return PTR_ERR(res);
+
+	/*
+	 * Edit the resource start address and length to map over all the
+	 * registers, instead of pointing at them one by one.
+	 */
+	res->start -= data->syscon_status_off;
+	res->end = res->start + max(data->syscon_status_off,
+				    max(data->syscon_control0_off,
+					data->syscon_control1_off)) +
+		   sizeof(unsigned int) - 1;
+
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->syscon = devm_regmap_init_mmio(&pdev->dev, base,
+					     &armada_thermal_regmap_config);
+	if (IS_ERR(priv->syscon))
+		return PTR_ERR(priv->syscon);
+
+	return 0;
+}
+
+static int armada_thermal_probe_syscon(struct platform_device *pdev,
+				       struct armada_thermal_priv *priv)
+{
+	priv->syscon = syscon_node_to_regmap(pdev->dev.parent->of_node);
+	if (IS_ERR(priv->syscon))
+		return PTR_ERR(priv->syscon);
+
+	return 0;
+}
+
 static void armada_set_sane_name(struct platform_device *pdev,
 				 struct armada_thermal_priv *priv)
 {
@@ -411,11 +480,10 @@ static void armada_set_sane_name(struct platform_device *pdev,
 
 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;
-	struct resource *res;
+	int ret;
 
 	match = of_match_device(armada_thermal_id_table, &pdev->dev);
 	if (!match)
@@ -425,16 +493,6 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	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);
-	if (IS_ERR(control))
-		return PTR_ERR(control);
-
 	priv->data = (struct armada_thermal_data *)match->data;
 
 	/* Ensure device name is correct for the thermal core */
@@ -442,22 +500,23 @@ static int armada_thermal_probe(struct platform_device *pdev)
 
 	/*
 	 * Legacy DT bindings only described "control1" register (also referred
-	 * as "control MSB" on old documentation). New bindings cover
+	 * as "control MSB" on old documentation). Then, bindings moved to cover
 	 * "control0/control LSB" and "control1/control MSB" registers within
-	 * the same resource, which is then of size 8 instead of 4.
+	 * the same resource, which was then of size 8 instead of 4.
+	 *
+	 * The logic of defining sporadic registers is broken. For instance, it
+	 * blocked the addition of the overheat interrupt feature that needed
+	 * another resource somewhere else in the same memory area. One solution
+	 * is to define an overall system controller and put the thermal node
+	 * into it, which requires the use of regmaps across all the driver.
 	 */
-	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;
-		}
+	if (IS_ERR(syscon_node_to_regmap(pdev->dev.parent->of_node)))
+		ret = armada_thermal_probe_legacy(pdev, priv);
+	else
+		ret = armada_thermal_probe_syscon(pdev, priv);
 
-		priv->control1 = control + LEGACY_CONTROL1_OFFSET;
-	} else {
-		priv->control0 = control + CONTROL0_OFFSET;
-		priv->control1 = control + CONTROL1_OFFSET;
-	}
+	if (ret)
+		return ret;
 
 	priv->data->init(pdev, priv);
 
-- 
2.14.1

[PATCH v3 08/23] thermal: armada: use the resource managed registration helper alternative

[Miquel Raynal]

Current use of thermal_zone_device_register() triggers a warning at boot
and should be replaced by devm_thermal_zone_of_sensor_register(). This
allows better handling of multiple thermal zones for later multi-sensors
support.

Also change the driver data to embed a new structure to make the
difference between legacy data (which needs to be cleaned) and
syscon-related data.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 123 ++++++++++++++++++++++++++++++++-------
 1 file changed, 101 insertions(+), 22 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index be346c6afde2..dd894312dedf 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -64,6 +64,7 @@ struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
 struct armada_thermal_priv {
+	struct device *dev;
 	struct regmap *syscon;
 	char zone_name[THERMAL_NAME_LENGTH];
 	struct armada_thermal_data *data;
@@ -95,6 +96,26 @@ struct armada_thermal_data {
 	unsigned int syscon_status_off;
 };
 
+struct armada_drvdata {
+	enum drvtype {
+		LEGACY,
+		SYSCON
+	} type;
+	union {
+		struct armada_thermal_priv *priv;
+		struct thermal_zone_device *tz;
+	} data;
+};
+
+/*
+ * struct armada_thermal_sensor - hold the information of one thermal sensor
+ * @thermal: pointer to the local private structure
+ * @tzd: pointer to the thermal zone device
+ */
+struct armada_thermal_sensor {
+	struct armada_thermal_priv *priv;
+};
+
 static void armadaxp_init(struct platform_device *pdev,
 			  struct armada_thermal_priv *priv)
 {
@@ -243,16 +264,14 @@ static bool armada_is_valid(struct armada_thermal_priv *priv)
 	return reg & priv->data->is_valid_bit;
 }
 
-static int armada_get_temp(struct thermal_zone_device *thermal,
-			   int *temp)
+static int armada_read_sensor(struct armada_thermal_priv *priv, int *temp)
 {
-	struct armada_thermal_priv *priv = thermal->devdata;
 	u32 reg, div;
 	s64 sample, b, m;
 
 	/* Valid check */
 	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
-		dev_err(&thermal->device,
+		dev_err(priv->dev,
 			"Temperature sensor reading not valid\n");
 		return -EIO;
 	}
@@ -278,7 +297,33 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static struct thermal_zone_device_ops ops = {
+static int armada_get_temp_legacy(struct thermal_zone_device *thermal,
+				  int *temp)
+{
+	struct armada_thermal_priv *priv = thermal->devdata;
+	int ret;
+
+	/* Do the actual reading */
+	ret = armada_read_sensor(priv, temp);
+
+	return ret;
+}
+
+static struct thermal_zone_device_ops legacy_ops = {
+	.get_temp = armada_get_temp_legacy,
+};
+
+static int armada_get_temp(void *_sensor, int *temp)
+{
+	struct armada_thermal_sensor *sensor = _sensor;
+	struct armada_thermal_priv *priv = sensor->priv;
+	int ret;
+
+	/* Do the actual reading */
+	return armada_read_sensor(priv, &temp);
+}
+
+static struct thermal_zone_of_device_ops of_ops = {
 	.get_temp = armada_get_temp,
 };
 
@@ -480,7 +525,9 @@ static void armada_set_sane_name(struct platform_device *pdev,
 
 static int armada_thermal_probe(struct platform_device *pdev)
 {
-	struct thermal_zone_device *thermal;
+	struct thermal_zone_device *tz;
+	struct armada_thermal_sensor *sensors;
+	struct armada_drvdata *drvdata;
 	const struct of_device_id *match;
 	struct armada_thermal_priv *priv;
 	int ret;
@@ -493,11 +540,13 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
+	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
 	priv->data = (struct armada_thermal_data *)match->data;
 
-	/* Ensure device name is correct for the thermal core */
-	armada_set_sane_name(pdev, priv);
-
 	/*
 	 * Legacy DT bindings only described "control1" register (also referred
 	 * as "control MSB" on old documentation). Then, bindings moved to cover
@@ -510,35 +559,65 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	 * is to define an overall system controller and put the thermal node
 	 * into it, which requires the use of regmaps across all the driver.
 	 */
-	if (IS_ERR(syscon_node_to_regmap(pdev->dev.parent->of_node)))
+	if (IS_ERR(syscon_node_to_regmap(pdev->dev.parent->of_node))) {
+		/* Ensure device name is correct for the thermal core */
+		armada_set_sane_name(pdev, priv);
+
 		ret = armada_thermal_probe_legacy(pdev, priv);
-	else
-		ret = armada_thermal_probe_syscon(pdev, priv);
+		if (ret)
+			return ret;
 
+		priv->data->init(pdev, priv);
+
+		tz = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
+						  &legacy_ops, NULL, 0, 0);
+		if (IS_ERR(tz)) {
+			dev_err(&pdev->dev,
+				"Failed to register thermal zone device\n");
+			return PTR_ERR(tz);
+		}
+
+		drvdata->type = LEGACY;
+		drvdata->data.tz = tz;
+		platform_set_drvdata(pdev, drvdata);
+
+		return 0;
+	}
+
+	ret = armada_thermal_probe_syscon(pdev, priv);
 	if (ret)
 		return ret;
 
 	priv->data->init(pdev, priv);
+	drvdata->type = SYSCON;
+	drvdata->data.priv = priv;
+	platform_set_drvdata(pdev, drvdata);
 
-	thermal = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
-					       &ops, NULL, 0, 0);
-	if (IS_ERR(thermal)) {
+	sensors = devm_kzalloc(&pdev->dev, sizeof(struct armada_thermal_sensor),
+			       GFP_KERNEL);
+	if (!sensors)
+		return -ENOMEM;
+
+	sensors->priv = priv;
+
+	tz = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, sensors,
+						  &of_ops);
+	if (IS_ERR(tz)) {
 		dev_err(&pdev->dev,
-			"Failed to register thermal zone device\n");
-		return PTR_ERR(thermal);
+			"Failed to register thermal sensor (err: %ld)\n",
+			PTR_ERR(tz));
+		return PTR_ERR(tz);
 	}
 
-	platform_set_drvdata(pdev, thermal);
-
 	return 0;
 }
 
 static int armada_thermal_exit(struct platform_device *pdev)
 {
-	struct thermal_zone_device *armada_thermal =
-		platform_get_drvdata(pdev);
+	struct armada_drvdata *drvdata = platform_get_drvdata(pdev);
 
-	thermal_zone_device_unregister(armada_thermal);
+	if (drvdata->type == LEGACY)
+		thermal_zone_device_unregister(drvdata->data.tz);
 
 	return 0;
 }
-- 
2.14.1

[PATCH v3 08/23] thermal: armada: use the resource managed registration helper alternative

[Miquel Raynal]

Current use of thermal_zone_device_register() triggers a warning at boot
and should be replaced by devm_thermal_zone_of_sensor_register(). This
allows better handling of multiple thermal zones for later multi-sensors
support.

Also change the driver data to embed a new structure to make the
difference between legacy data (which needs to be cleaned) and
syscon-related data.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 123 ++++++++++++++++++++++++++++++++-------
 1 file changed, 101 insertions(+), 22 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index be346c6afde2..dd894312dedf 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -64,6 +64,7 @@ struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
 struct armada_thermal_priv {
+	struct device *dev;
 	struct regmap *syscon;
 	char zone_name[THERMAL_NAME_LENGTH];
 	struct armada_thermal_data *data;
@@ -95,6 +96,26 @@ struct armada_thermal_data {
 	unsigned int syscon_status_off;
 };
 
+struct armada_drvdata {
+	enum drvtype {
+		LEGACY,
+		SYSCON
+	} type;
+	union {
+		struct armada_thermal_priv *priv;
+		struct thermal_zone_device *tz;
+	} data;
+};
+
+/*
+ * struct armada_thermal_sensor - hold the information of one thermal sensor
+ * @thermal: pointer to the local private structure
+ * @tzd: pointer to the thermal zone device
+ */
+struct armada_thermal_sensor {
+	struct armada_thermal_priv *priv;
+};
+
 static void armadaxp_init(struct platform_device *pdev,
 			  struct armada_thermal_priv *priv)
 {
@@ -243,16 +264,14 @@ static bool armada_is_valid(struct armada_thermal_priv *priv)
 	return reg & priv->data->is_valid_bit;
 }
 
-static int armada_get_temp(struct thermal_zone_device *thermal,
-			   int *temp)
+static int armada_read_sensor(struct armada_thermal_priv *priv, int *temp)
 {
-	struct armada_thermal_priv *priv = thermal->devdata;
 	u32 reg, div;
 	s64 sample, b, m;
 
 	/* Valid check */
 	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
-		dev_err(&thermal->device,
+		dev_err(priv->dev,
 			"Temperature sensor reading not valid\n");
 		return -EIO;
 	}
@@ -278,7 +297,33 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static struct thermal_zone_device_ops ops = {
+static int armada_get_temp_legacy(struct thermal_zone_device *thermal,
+				  int *temp)
+{
+	struct armada_thermal_priv *priv = thermal->devdata;
+	int ret;
+
+	/* Do the actual reading */
+	ret = armada_read_sensor(priv, temp);
+
+	return ret;
+}
+
+static struct thermal_zone_device_ops legacy_ops = {
+	.get_temp = armada_get_temp_legacy,
+};
+
+static int armada_get_temp(void *_sensor, int *temp)
+{
+	struct armada_thermal_sensor *sensor = _sensor;
+	struct armada_thermal_priv *priv = sensor->priv;
+	int ret;
+
+	/* Do the actual reading */
+	return armada_read_sensor(priv, &temp);
+}
+
+static struct thermal_zone_of_device_ops of_ops = {
 	.get_temp = armada_get_temp,
 };
 
@@ -480,7 +525,9 @@ static void armada_set_sane_name(struct platform_device *pdev,
 
 static int armada_thermal_probe(struct platform_device *pdev)
 {
-	struct thermal_zone_device *thermal;
+	struct thermal_zone_device *tz;
+	struct armada_thermal_sensor *sensors;
+	struct armada_drvdata *drvdata;
 	const struct of_device_id *match;
 	struct armada_thermal_priv *priv;
 	int ret;
@@ -493,11 +540,13 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
+	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
 	priv->data = (struct armada_thermal_data *)match->data;
 
-	/* Ensure device name is correct for the thermal core */
-	armada_set_sane_name(pdev, priv);
-
 	/*
 	 * Legacy DT bindings only described "control1" register (also referred
 	 * as "control MSB" on old documentation). Then, bindings moved to cover
@@ -510,35 +559,65 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	 * is to define an overall system controller and put the thermal node
 	 * into it, which requires the use of regmaps across all the driver.
 	 */
-	if (IS_ERR(syscon_node_to_regmap(pdev->dev.parent->of_node)))
+	if (IS_ERR(syscon_node_to_regmap(pdev->dev.parent->of_node))) {
+		/* Ensure device name is correct for the thermal core */
+		armada_set_sane_name(pdev, priv);
+
 		ret = armada_thermal_probe_legacy(pdev, priv);
-	else
-		ret = armada_thermal_probe_syscon(pdev, priv);
+		if (ret)
+			return ret;
 
+		priv->data->init(pdev, priv);
+
+		tz = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
+						  &legacy_ops, NULL, 0, 0);
+		if (IS_ERR(tz)) {
+			dev_err(&pdev->dev,
+				"Failed to register thermal zone device\n");
+			return PTR_ERR(tz);
+		}
+
+		drvdata->type = LEGACY;
+		drvdata->data.tz = tz;
+		platform_set_drvdata(pdev, drvdata);
+
+		return 0;
+	}
+
+	ret = armada_thermal_probe_syscon(pdev, priv);
 	if (ret)
 		return ret;
 
 	priv->data->init(pdev, priv);
+	drvdata->type = SYSCON;
+	drvdata->data.priv = priv;
+	platform_set_drvdata(pdev, drvdata);
 
-	thermal = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
-					       &ops, NULL, 0, 0);
-	if (IS_ERR(thermal)) {
+	sensors = devm_kzalloc(&pdev->dev, sizeof(struct armada_thermal_sensor),
+			       GFP_KERNEL);
+	if (!sensors)
+		return -ENOMEM;
+
+	sensors->priv = priv;
+
+	tz = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, sensors,
+						  &of_ops);
+	if (IS_ERR(tz)) {
 		dev_err(&pdev->dev,
-			"Failed to register thermal zone device\n");
-		return PTR_ERR(thermal);
+			"Failed to register thermal sensor (err: %ld)\n",
+			PTR_ERR(tz));
+		return PTR_ERR(tz);
 	}
 
-	platform_set_drvdata(pdev, thermal);
-
 	return 0;
 }
 
 static int armada_thermal_exit(struct platform_device *pdev)
 {
-	struct thermal_zone_device *armada_thermal =
-		platform_get_drvdata(pdev);
+	struct armada_drvdata *drvdata = platform_get_drvdata(pdev);
 
-	thermal_zone_device_unregister(armada_thermal);
+	if (drvdata->type == LEGACY)
+		thermal_zone_device_unregister(drvdata->data.tz);
 
 	return 0;
 }
-- 
2.14.1

[PATCH v3 09/23] thermal: armada: add multi-channel sensors support

[Miquel Raynal]

MVEBU thermal IP supports multiple channels. Each channel may have
several sensors but for now each channel is wired to only one thermal
sensor. The first channel always points to the so called internal
sensor, within the thermal IP. There is usually one more channel (with
one sensor each time) per CPU. The code has been written to support
possible evolutions of the ap806 IP that would embed more CPUs and thus
more channels to select. Each channel should be referenced in the device
tree as an independent thermal zone.

Add the possibility to read each of these sensors through sysfs by
registering all the sensors (translated in "thermal_zone"). Also add a
mutex on these accesses to avoid read conflicts (only one channel/sensor
may be selected and read at a time).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 131 +++++++++++++++++++++++++++++++++------
 1 file changed, 111 insertions(+), 20 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index dd894312dedf..eef8947b9b20 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -49,8 +49,13 @@
 #define CONTROL0_TSEN_RESET		BIT(1)
 #define CONTROL0_TSEN_ENABLE		BIT(2)
 #define CONTROL0_TSEN_AVG_BYPASS	BIT(6)
+#define CONTROL0_TSEN_CHAN_SHIFT	13
+#define CONTROL0_TSEN_CHAN_MASK		0xF
 #define CONTROL0_TSEN_OSR_SHIFT		24
 #define CONTROL0_TSEN_OSR_MAX		0x3
+#define CONTROL0_TSEN_MODE_SHIFT	30
+#define CONTROL0_TSEN_MODE_EXTERNAL	0x2
+#define CONTROL0_TSEN_MODE_MASK		0x3
 
 #define CONTROL1_TSEN_AVG_SHIFT		0
 #define CONTROL1_TSEN_AVG_MASK		0x7
@@ -67,7 +72,9 @@ struct armada_thermal_priv {
 	struct device *dev;
 	struct regmap *syscon;
 	char zone_name[THERMAL_NAME_LENGTH];
+	struct mutex update_lock;
 	struct armada_thermal_data *data;
+	int current_channel;
 };
 
 struct armada_thermal_data {
@@ -94,6 +101,9 @@ struct armada_thermal_data {
 	unsigned int syscon_control0_off;
 	unsigned int syscon_control1_off;
 	unsigned int syscon_status_off;
+
+	/* One sensor is in the thermal IC, the others are in the CPUs if any */
+	unsigned int cpu_nr;
 };
 
 struct armada_drvdata {
@@ -111,9 +121,11 @@ struct armada_drvdata {
  * struct armada_thermal_sensor - hold the information of one thermal sensor
  * @thermal: pointer to the local private structure
  * @tzd: pointer to the thermal zone device
+ * @id: identifier of the thermal sensor
  */
 struct armada_thermal_sensor {
 	struct armada_thermal_priv *priv;
+	int id;
 };
 
 static void armadaxp_init(struct platform_device *pdev,
@@ -181,14 +193,15 @@ static void armada375_init(struct platform_device *pdev,
 	msleep(50);
 }
 
-static void armada_wait_sensor_validity(struct armada_thermal_priv *priv)
+static int armada_wait_sensor_validity(struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
-	regmap_read_poll_timeout(priv->syscon, priv->data->syscon_status_off,
-				 reg, reg & priv->data->is_valid_bit,
-				 STATUS_POLL_PERIOD_US,
-				 STATUS_POLL_TIMEOUT_US);
+	return regmap_read_poll_timeout(priv->syscon,
+					priv->data->syscon_status_off, reg,
+					reg & priv->data->is_valid_bit,
+					STATUS_POLL_PERIOD_US,
+					STATUS_POLL_TIMEOUT_US);
 }
 
 static void armada380_init(struct platform_device *pdev,
@@ -264,6 +277,58 @@ static bool armada_is_valid(struct armada_thermal_priv *priv)
 	return reg & priv->data->is_valid_bit;
 }
 
+/* There is currently no board with more than one sensor per channel */
+static int armada_select_channel(struct armada_thermal_priv *priv, int channel)
+{
+	struct armada_thermal_data *data = priv->data;
+	u32 ctrl0;
+
+	if (channel < 0 || channel > priv->data->cpu_nr)
+		return -EINVAL;
+
+	if (priv->current_channel == channel)
+		return 0;
+
+	/* Stop the measurements */
+	regmap_read(priv->syscon, data->syscon_control0_off, &ctrl0);
+	ctrl0 &= ~CONTROL0_TSEN_START;
+	regmap_write(priv->syscon, data->syscon_control0_off, ctrl0);
+
+	/* Reset the mode, internal sensor will be automatically selected */
+	ctrl0 &= ~(CONTROL0_TSEN_MODE_MASK << CONTROL0_TSEN_MODE_SHIFT);
+
+	/* Other channels are external and should be selected accordingly */
+	if (channel) {
+		/* Change the mode to external */
+		ctrl0 |= CONTROL0_TSEN_MODE_EXTERNAL <<
+			 CONTROL0_TSEN_MODE_SHIFT;
+		/* Select the sensor */
+		ctrl0 &= ~(CONTROL0_TSEN_CHAN_MASK << CONTROL0_TSEN_CHAN_SHIFT);
+		ctrl0 |= (channel - 1) << CONTROL0_TSEN_CHAN_SHIFT;
+	}
+
+	/* Actually set the mode/channel */
+	regmap_write(priv->syscon, data->syscon_control0_off, ctrl0);
+	priv->current_channel = channel;
+
+	/* Re-start the measurements */
+	ctrl0 |= CONTROL0_TSEN_START;
+	regmap_write(priv->syscon, data->syscon_control0_off, ctrl0);
+
+	/*
+	 * The IP has a latency of ~15ms, so after updating the selected source,
+	 * we must absolutely wait for the sensor validity bit to ensure we read
+	 * actual data.
+	 */
+	if (armada_wait_sensor_validity(priv)) {
+		dev_err(priv->dev,
+			"Temperature sensor reading not valid\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
 static int armada_read_sensor(struct armada_thermal_priv *priv, int *temp)
 {
 	u32 reg, div;
@@ -319,8 +384,20 @@ static int armada_get_temp(void *_sensor, int *temp)
 	struct armada_thermal_priv *priv = sensor->priv;
 	int ret;
 
+	mutex_lock(&priv->update_lock);
+
+	/* Select the desired channel */
+	ret = armada_select_channel(priv, sensor->id);
+	if (ret)
+		goto unlock_mutex;
+
 	/* Do the actual reading */
-	return armada_read_sensor(priv, &temp);
+	ret = armada_read_sensor(priv, &temp);
+
+unlock_mutex:
+	mutex_unlock(&priv->update_lock);
+
+	return ret;
 }
 
 static struct thermal_zone_of_device_ops of_ops = {
@@ -394,6 +471,7 @@ static const struct armada_thermal_data armada_ap806_data = {
 	.syscon_control0_off = 0x84,
 	.syscon_control1_off = 0x88,
 	.syscon_status_off = 0x8C,
+	.cpu_nr = 4,
 };
 
 static const struct armada_thermal_data armada_cp110_data = {
@@ -526,10 +604,11 @@ static void armada_set_sane_name(struct platform_device *pdev,
 static int armada_thermal_probe(struct platform_device *pdev)
 {
 	struct thermal_zone_device *tz;
-	struct armada_thermal_sensor *sensors;
+	struct armada_thermal_sensor *sensor;
 	struct armada_drvdata *drvdata;
 	const struct of_device_id *match;
 	struct armada_thermal_priv *priv;
+	int sensor_id;
 	int ret;
 
 	match = of_match_device(armada_thermal_id_table, &pdev->dev);
@@ -547,6 +626,8 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	priv->dev = &pdev->dev;
 	priv->data = (struct armada_thermal_data *)match->data;
 
+	mutex_init(&priv->update_lock);
+
 	/*
 	 * Legacy DT bindings only described "control1" register (also referred
 	 * as "control MSB" on old documentation). Then, bindings moved to cover
@@ -588,25 +669,35 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	priv->current_channel = -1;
 	priv->data->init(pdev, priv);
 	drvdata->type = SYSCON;
 	drvdata->data.priv = priv;
 	platform_set_drvdata(pdev, drvdata);
 
-	sensors = devm_kzalloc(&pdev->dev, sizeof(struct armada_thermal_sensor),
-			       GFP_KERNEL);
-	if (!sensors)
-		return -ENOMEM;
+	/*
+	 * There is one channel for the IC and one per CPU (if any), each
+	 * channel has one sensor.
+	 */
+	for (sensor_id = 0; sensor_id <= priv->data->cpu_nr; sensor_id++) {
+		sensor = devm_kzalloc(&pdev->dev,
+				      sizeof(struct armada_thermal_sensor),
+				      GFP_KERNEL);
+		if (!sensor)
+			return -ENOMEM;
 
-	sensors->priv = priv;
-
-	tz = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, sensors,
-						  &of_ops);
-	if (IS_ERR(tz)) {
-		dev_err(&pdev->dev,
-			"Failed to register thermal sensor (err: %ld)\n",
-			PTR_ERR(tz));
-		return PTR_ERR(tz);
+		/* Register the sensor */
+		sensor->priv = priv;
+		sensor->id = sensor_id;
+		tz = devm_thermal_zone_of_sensor_register(&pdev->dev,
+							  sensor->id, sensor,
+							  &of_ops);
+		if (IS_ERR(tz)) {
+			dev_info(&pdev->dev, "Thermal sensor %d unavailable\n",
+				 sensor_id);
+			devm_kfree(&pdev->dev, sensor);
+			continue;
+		}
 	}
 
 	return 0;
-- 
2.14.1

[PATCH v3 09/23] thermal: armada: add multi-channel sensors support

[Miquel Raynal]

MVEBU thermal IP supports multiple channels. Each channel may have
several sensors but for now each channel is wired to only one thermal
sensor. The first channel always points to the so called internal
sensor, within the thermal IP. There is usually one more channel (with
one sensor each time) per CPU. The code has been written to support
possible evolutions of the ap806 IP that would embed more CPUs and thus
more channels to select. Each channel should be referenced in the device
tree as an independent thermal zone.

Add the possibility to read each of these sensors through sysfs by
registering all the sensors (translated in "thermal_zone"). Also add a
mutex on these accesses to avoid read conflicts (only one channel/sensor
may be selected and read at a time).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 131 +++++++++++++++++++++++++++++++++------
 1 file changed, 111 insertions(+), 20 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index dd894312dedf..eef8947b9b20 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -49,8 +49,13 @@
 #define CONTROL0_TSEN_RESET		BIT(1)
 #define CONTROL0_TSEN_ENABLE		BIT(2)
 #define CONTROL0_TSEN_AVG_BYPASS	BIT(6)
+#define CONTROL0_TSEN_CHAN_SHIFT	13
+#define CONTROL0_TSEN_CHAN_MASK		0xF
 #define CONTROL0_TSEN_OSR_SHIFT		24
 #define CONTROL0_TSEN_OSR_MAX		0x3
+#define CONTROL0_TSEN_MODE_SHIFT	30
+#define CONTROL0_TSEN_MODE_EXTERNAL	0x2
+#define CONTROL0_TSEN_MODE_MASK		0x3
 
 #define CONTROL1_TSEN_AVG_SHIFT		0
 #define CONTROL1_TSEN_AVG_MASK		0x7
@@ -67,7 +72,9 @@ struct armada_thermal_priv {
 	struct device *dev;
 	struct regmap *syscon;
 	char zone_name[THERMAL_NAME_LENGTH];
+	struct mutex update_lock;
 	struct armada_thermal_data *data;
+	int current_channel;
 };
 
 struct armada_thermal_data {
@@ -94,6 +101,9 @@ struct armada_thermal_data {
 	unsigned int syscon_control0_off;
 	unsigned int syscon_control1_off;
 	unsigned int syscon_status_off;
+
+	/* One sensor is in the thermal IC, the others are in the CPUs if any */
+	unsigned int cpu_nr;
 };
 
 struct armada_drvdata {
@@ -111,9 +121,11 @@ struct armada_drvdata {
  * struct armada_thermal_sensor - hold the information of one thermal sensor
  * @thermal: pointer to the local private structure
  * @tzd: pointer to the thermal zone device
+ * @id: identifier of the thermal sensor
  */
 struct armada_thermal_sensor {
 	struct armada_thermal_priv *priv;
+	int id;
 };
 
 static void armadaxp_init(struct platform_device *pdev,
@@ -181,14 +193,15 @@ static void armada375_init(struct platform_device *pdev,
 	msleep(50);
 }
 
-static void armada_wait_sensor_validity(struct armada_thermal_priv *priv)
+static int armada_wait_sensor_validity(struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
-	regmap_read_poll_timeout(priv->syscon, priv->data->syscon_status_off,
-				 reg, reg & priv->data->is_valid_bit,
-				 STATUS_POLL_PERIOD_US,
-				 STATUS_POLL_TIMEOUT_US);
+	return regmap_read_poll_timeout(priv->syscon,
+					priv->data->syscon_status_off, reg,
+					reg & priv->data->is_valid_bit,
+					STATUS_POLL_PERIOD_US,
+					STATUS_POLL_TIMEOUT_US);
 }
 
 static void armada380_init(struct platform_device *pdev,
@@ -264,6 +277,58 @@ static bool armada_is_valid(struct armada_thermal_priv *priv)
 	return reg & priv->data->is_valid_bit;
 }
 
+/* There is currently no board with more than one sensor per channel */
+static int armada_select_channel(struct armada_thermal_priv *priv, int channel)
+{
+	struct armada_thermal_data *data = priv->data;
+	u32 ctrl0;
+
+	if (channel < 0 || channel > priv->data->cpu_nr)
+		return -EINVAL;
+
+	if (priv->current_channel == channel)
+		return 0;
+
+	/* Stop the measurements */
+	regmap_read(priv->syscon, data->syscon_control0_off, &ctrl0);
+	ctrl0 &= ~CONTROL0_TSEN_START;
+	regmap_write(priv->syscon, data->syscon_control0_off, ctrl0);
+
+	/* Reset the mode, internal sensor will be automatically selected */
+	ctrl0 &= ~(CONTROL0_TSEN_MODE_MASK << CONTROL0_TSEN_MODE_SHIFT);
+
+	/* Other channels are external and should be selected accordingly */
+	if (channel) {
+		/* Change the mode to external */
+		ctrl0 |= CONTROL0_TSEN_MODE_EXTERNAL <<
+			 CONTROL0_TSEN_MODE_SHIFT;
+		/* Select the sensor */
+		ctrl0 &= ~(CONTROL0_TSEN_CHAN_MASK << CONTROL0_TSEN_CHAN_SHIFT);
+		ctrl0 |= (channel - 1) << CONTROL0_TSEN_CHAN_SHIFT;
+	}
+
+	/* Actually set the mode/channel */
+	regmap_write(priv->syscon, data->syscon_control0_off, ctrl0);
+	priv->current_channel = channel;
+
+	/* Re-start the measurements */
+	ctrl0 |= CONTROL0_TSEN_START;
+	regmap_write(priv->syscon, data->syscon_control0_off, ctrl0);
+
+	/*
+	 * The IP has a latency of ~15ms, so after updating the selected source,
+	 * we must absolutely wait for the sensor validity bit to ensure we read
+	 * actual data.
+	 */
+	if (armada_wait_sensor_validity(priv)) {
+		dev_err(priv->dev,
+			"Temperature sensor reading not valid\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
 static int armada_read_sensor(struct armada_thermal_priv *priv, int *temp)
 {
 	u32 reg, div;
@@ -319,8 +384,20 @@ static int armada_get_temp(void *_sensor, int *temp)
 	struct armada_thermal_priv *priv = sensor->priv;
 	int ret;
 
+	mutex_lock(&priv->update_lock);
+
+	/* Select the desired channel */
+	ret = armada_select_channel(priv, sensor->id);
+	if (ret)
+		goto unlock_mutex;
+
 	/* Do the actual reading */
-	return armada_read_sensor(priv, &temp);
+	ret = armada_read_sensor(priv, &temp);
+
+unlock_mutex:
+	mutex_unlock(&priv->update_lock);
+
+	return ret;
 }
 
 static struct thermal_zone_of_device_ops of_ops = {
@@ -394,6 +471,7 @@ static const struct armada_thermal_data armada_ap806_data = {
 	.syscon_control0_off = 0x84,
 	.syscon_control1_off = 0x88,
 	.syscon_status_off = 0x8C,
+	.cpu_nr = 4,
 };
 
 static const struct armada_thermal_data armada_cp110_data = {
@@ -526,10 +604,11 @@ static void armada_set_sane_name(struct platform_device *pdev,
 static int armada_thermal_probe(struct platform_device *pdev)
 {
 	struct thermal_zone_device *tz;
-	struct armada_thermal_sensor *sensors;
+	struct armada_thermal_sensor *sensor;
 	struct armada_drvdata *drvdata;
 	const struct of_device_id *match;
 	struct armada_thermal_priv *priv;
+	int sensor_id;
 	int ret;
 
 	match = of_match_device(armada_thermal_id_table, &pdev->dev);
@@ -547,6 +626,8 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	priv->dev = &pdev->dev;
 	priv->data = (struct armada_thermal_data *)match->data;
 
+	mutex_init(&priv->update_lock);
+
 	/*
 	 * Legacy DT bindings only described "control1" register (also referred
 	 * as "control MSB" on old documentation). Then, bindings moved to cover
@@ -588,25 +669,35 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	priv->current_channel = -1;
 	priv->data->init(pdev, priv);
 	drvdata->type = SYSCON;
 	drvdata->data.priv = priv;
 	platform_set_drvdata(pdev, drvdata);
 
-	sensors = devm_kzalloc(&pdev->dev, sizeof(struct armada_thermal_sensor),
-			       GFP_KERNEL);
-	if (!sensors)
-		return -ENOMEM;
+	/*
+	 * There is one channel for the IC and one per CPU (if any), each
+	 * channel has one sensor.
+	 */
+	for (sensor_id = 0; sensor_id <= priv->data->cpu_nr; sensor_id++) {
+		sensor = devm_kzalloc(&pdev->dev,
+				      sizeof(struct armada_thermal_sensor),
+				      GFP_KERNEL);
+		if (!sensor)
+			return -ENOMEM;
 
-	sensors->priv = priv;
-
-	tz = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, sensors,
-						  &of_ops);
-	if (IS_ERR(tz)) {
-		dev_err(&pdev->dev,
-			"Failed to register thermal sensor (err: %ld)\n",
-			PTR_ERR(tz));
-		return PTR_ERR(tz);
+		/* Register the sensor */
+		sensor->priv = priv;
+		sensor->id = sensor_id;
+		tz = devm_thermal_zone_of_sensor_register(&pdev->dev,
+							  sensor->id, sensor,
+							  &of_ops);
+		if (IS_ERR(tz)) {
+			dev_info(&pdev->dev, "Thermal sensor %d unavailable\n",
+				 sensor_id);
+			devm_kfree(&pdev->dev, sensor);
+			continue;
+		}
 	}
 
 	return 0;
-- 
2.14.1

[PATCH v3 10/23] thermal: armada: remove sensors validity from the IP initialization

[Miquel Raynal]

When using new bindings with multiple sensors, sensor validity is
checked twice because sensor selection also checks for the validity.

Remove the redundant call from the IP initialization helper and move it
to the legacy probe section where it is still needed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index eef8947b9b20..7eafc9400fbe 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -221,9 +221,6 @@ static void armada380_init(struct platform_device *pdev,
 	reg &= ~CONTROL0_TSEN_TC_TRIM_MASK;
 	reg |= CONTROL0_TSEN_TC_TRIM_VAL;
 	regmap_write(priv->syscon, data->syscon_control0_off, reg);
-
-	/* Wait the sensors to be valid or the core will warn the user */
-	armada_wait_sensor_validity(priv);
 }
 
 static void armada_ap806_init(struct platform_device *pdev,
@@ -243,9 +240,6 @@ static void armada_ap806_init(struct platform_device *pdev,
 	reg &= ~CONTROL0_TSEN_AVG_BYPASS;
 
 	regmap_write(priv->syscon, data->syscon_control0_off, reg);
-
-	/* Wait the sensors to be valid or the core will warn the user */
-	armada_wait_sensor_validity(priv);
 }
 
 static void armada_cp110_init(struct platform_device *pdev,
@@ -650,6 +644,9 @@ static int armada_thermal_probe(struct platform_device *pdev)
 
 		priv->data->init(pdev, priv);
 
+		/* Wait the sensors to be valid */
+		armada_wait_sensor_validity(priv);
+
 		tz = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
 						  &legacy_ops, NULL, 0, 0);
 		if (IS_ERR(tz)) {
-- 
2.14.1

[PATCH v3 10/23] thermal: armada: remove sensors validity from the IP initialization

[Miquel Raynal]

When using new bindings with multiple sensors, sensor validity is
checked twice because sensor selection also checks for the validity.

Remove the redundant call from the IP initialization helper and move it
to the legacy probe section where it is still needed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index eef8947b9b20..7eafc9400fbe 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -221,9 +221,6 @@ static void armada380_init(struct platform_device *pdev,
 	reg &= ~CONTROL0_TSEN_TC_TRIM_MASK;
 	reg |= CONTROL0_TSEN_TC_TRIM_VAL;
 	regmap_write(priv->syscon, data->syscon_control0_off, reg);
-
-	/* Wait the sensors to be valid or the core will warn the user */
-	armada_wait_sensor_validity(priv);
 }
 
 static void armada_ap806_init(struct platform_device *pdev,
@@ -243,9 +240,6 @@ static void armada_ap806_init(struct platform_device *pdev,
 	reg &= ~CONTROL0_TSEN_AVG_BYPASS;
 
 	regmap_write(priv->syscon, data->syscon_control0_off, reg);
-
-	/* Wait the sensors to be valid or the core will warn the user */
-	armada_wait_sensor_validity(priv);
 }
 
 static void armada_cp110_init(struct platform_device *pdev,
@@ -650,6 +644,9 @@ static int armada_thermal_probe(struct platform_device *pdev)
 
 		priv->data->init(pdev, priv);
 
+		/* Wait the sensors to be valid */
+		armada_wait_sensor_validity(priv);
+
 		tz = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
 						  &legacy_ops, NULL, 0, 0);
 		if (IS_ERR(tz)) {
-- 
2.14.1

[PATCH v3 11/23] thermal: armada: move validity check out of the read function

[Miquel Raynal]

Sensor selection when using multiple sensors already checks for the
sensor validity. Move it to the legacy ->get_temp() hook, where it is
still needed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 7eafc9400fbe..7f8185e5816e 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -328,13 +328,6 @@ static int armada_read_sensor(struct armada_thermal_priv *priv, int *temp)
 	u32 reg, div;
 	s64 sample, b, m;
 
-	/* Valid check */
-	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
-		dev_err(priv->dev,
-			"Temperature sensor reading not valid\n");
-		return -EIO;
-	}
-
 	regmap_read(priv->syscon, priv->data->syscon_status_off, &reg);
 	reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;
 	if (priv->data->signed_sample)
@@ -362,6 +355,13 @@ static int armada_get_temp_legacy(struct thermal_zone_device *thermal,
 	struct armada_thermal_priv *priv = thermal->devdata;
 	int ret;
 
+	/* Valid check */
+	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
+		dev_err(priv->dev,
+			"Temperature sensor reading not valid\n");
+		return -EIO;
+	}
+
 	/* Do the actual reading */
 	ret = armada_read_sensor(priv, temp);
 
-- 
2.14.1

[PATCH v3 11/23] thermal: armada: move validity check out of the read function

[Miquel Raynal]

Sensor selection when using multiple sensors already checks for the
sensor validity. Move it to the legacy ->get_temp() hook, where it is
still needed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 7eafc9400fbe..7f8185e5816e 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -328,13 +328,6 @@ static int armada_read_sensor(struct armada_thermal_priv *priv, int *temp)
 	u32 reg, div;
 	s64 sample, b, m;
 
-	/* Valid check */
-	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
-		dev_err(priv->dev,
-			"Temperature sensor reading not valid\n");
-		return -EIO;
-	}
-
 	regmap_read(priv->syscon, priv->data->syscon_status_off, &reg);
 	reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;
 	if (priv->data->signed_sample)
@@ -362,6 +355,13 @@ static int armada_get_temp_legacy(struct thermal_zone_device *thermal,
 	struct armada_thermal_priv *priv = thermal->devdata;
 	int ret;
 
+	/* Valid check */
+	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
+		dev_err(priv->dev,
+			"Temperature sensor reading not valid\n");
+		return -EIO;
+	}
+
 	/* Do the actual reading */
 	ret = armada_read_sensor(priv, temp);
 
-- 
2.14.1

[PATCH v3 12/23] thermal: armada: get rid of the ->is_valid() pointer

[Miquel Raynal]

The implementation of armada_is_valid() is very simple and is the same
across all the versions of the IP since the ->is_valid_bit has been
introduced. Simplify the structure by getting rid of the function
pointer and calling directly the function.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 7f8185e5816e..a05ec01d6848 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -82,9 +82,6 @@ struct armada_thermal_data {
 	void (*init)(struct platform_device *pdev,
 		     struct armada_thermal_priv *priv);
 
-	/* Test for a valid sensor value (optional) */
-	bool (*is_valid)(struct armada_thermal_priv *);
-
 	/* Formula coeficients: temp = (b - m * reg) / div */
 	s64 coef_b;
 	s64 coef_m;
@@ -266,6 +263,9 @@ static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
+	if (!priv->data->is_valid_bit)
+		return true;
+
 	regmap_read(priv->syscon, priv->data->syscon_status_off, &reg);
 
 	return reg & priv->data->is_valid_bit;
@@ -356,7 +356,7 @@ static int armada_get_temp_legacy(struct thermal_zone_device *thermal,
 	int ret;
 
 	/* Valid check */
-	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
+	if (armada_is_valid(priv)) {
 		dev_err(priv->dev,
 			"Temperature sensor reading not valid\n");
 		return -EIO;
@@ -410,7 +410,6 @@ static const struct armada_thermal_data armadaxp_data = {
 };
 
 static const struct armada_thermal_data armada370_data = {
-	.is_valid = armada_is_valid,
 	.init = armada370_init,
 	.is_valid_bit = BIT(9),
 	.temp_shift = 10,
@@ -423,7 +422,6 @@ static const struct armada_thermal_data armada370_data = {
 };
 
 static const struct armada_thermal_data armada375_data = {
-	.is_valid = armada_is_valid,
 	.init = armada375_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
@@ -437,7 +435,6 @@ static const struct armada_thermal_data armada375_data = {
 };
 
 static const struct armada_thermal_data armada380_data = {
-	.is_valid = armada_is_valid,
 	.init = armada380_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
@@ -452,7 +449,6 @@ static const struct armada_thermal_data armada380_data = {
 };
 
 static const struct armada_thermal_data armada_ap806_data = {
-	.is_valid = armada_is_valid,
 	.init = armada_ap806_init,
 	.is_valid_bit = BIT(16),
 	.temp_shift = 0,
@@ -469,7 +465,6 @@ static const struct armada_thermal_data armada_ap806_data = {
 };
 
 static const struct armada_thermal_data armada_cp110_data = {
-	.is_valid = armada_is_valid,
 	.init = armada_cp110_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
-- 
2.14.1

[PATCH v3 12/23] thermal: armada: get rid of the ->is_valid() pointer

[Miquel Raynal]

The implementation of armada_is_valid() is very simple and is the same
across all the versions of the IP since the ->is_valid_bit has been
introduced. Simplify the structure by getting rid of the function
pointer and calling directly the function.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/thermal/armada_thermal.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 7f8185e5816e..a05ec01d6848 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -82,9 +82,6 @@ struct armada_thermal_data {
 	void (*init)(struct platform_device *pdev,
 		     struct armada_thermal_priv *priv);
 
-	/* Test for a valid sensor value (optional) */
-	bool (*is_valid)(struct armada_thermal_priv *);
-
 	/* Formula coeficients: temp = (b - m * reg) / div */
 	s64 coef_b;
 	s64 coef_m;
@@ -266,6 +263,9 @@ static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
 	u32 reg;
 
+	if (!priv->data->is_valid_bit)
+		return true;
+
 	regmap_read(priv->syscon, priv->data->syscon_status_off, &reg);
 
 	return reg & priv->data->is_valid_bit;
@@ -356,7 +356,7 @@ static int armada_get_temp_legacy(struct thermal_zone_device *thermal,
 	int ret;
 
 	/* Valid check */
-	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
+	if (armada_is_valid(priv)) {
 		dev_err(priv->dev,
 			"Temperature sensor reading not valid\n");
 		return -EIO;
@@ -410,7 +410,6 @@ static const struct armada_thermal_data armadaxp_data = {
 };
 
 static const struct armada_thermal_data armada370_data = {
-	.is_valid = armada_is_valid,
 	.init = armada370_init,
 	.is_valid_bit = BIT(9),
 	.temp_shift = 10,
@@ -423,7 +422,6 @@ static const struct armada_thermal_data armada370_data = {
 };
 
 static const struct armada_thermal_data armada375_data = {
-	.is_valid = armada_is_valid,
 	.init = armada375_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
@@ -437,7 +435,6 @@ static const struct armada_thermal_data armada375_data = {
 };
 
 static const struct armada_thermal_data armada380_data = {
-	.is_valid = armada_is_valid,
 	.init = armada380_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
@@ -452,7 +449,6 @@ static const struct armada_thermal_data armada380_data = {
 };
 
 static const struct armada_thermal_data armada_ap806_data = {
-	.is_valid = armada_is_valid,
 	.init = armada_ap806_init,
 	.is_valid_bit = BIT(16),
 	.temp_shift = 0,
@@ -469,7 +465,6 @@ static const struct armada_thermal_data armada_ap806_data = {
 };
 
 static const struct armada_thermal_data armada_cp110_data = {
-	.is_valid = armada_is_valid,
 	.init = armada_cp110_init,
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
-- 
2.14.1

[PATCH v3 13/23] dt-bindings: cp110: rename cp110 syscon file

[Miquel Raynal]

There is no need to give numbers to system controllers inside the
documentation as the syscons use the same compatibles. Furthermore, this
approach does not scale very well and would force the creation of a new
file each time a new syscon is added in the device tree.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../marvell/{cp110-system-controller0.txt => cp110-system-controller.txt} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/arm/marvell/{cp110-system-controller0.txt => cp110-system-controller.txt} (100%)

diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
similarity index 100%
rename from Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt
rename to Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
-- 
2.14.1

[PATCH v3 13/23] dt-bindings: cp110: rename cp110 syscon file

[Miquel Raynal]

There is no need to give numbers to system controllers inside the
documentation as the syscons use the same compatibles. Furthermore, this
approach does not scale very well and would force the creation of a new
file each time a new syscon is added in the device tree.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../marvell/{cp110-system-controller0.txt => cp110-system-controller.txt} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/arm/marvell/{cp110-system-controller0.txt => cp110-system-controller.txt} (100%)

diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
similarity index 100%
rename from Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt
rename to Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
-- 
2.14.1

[PATCH v3 14/23] dt-bindings: ap806: prepare the syscon file to list other syscons nodes

[Miquel Raynal]

There are multiple system controllers in AP806. Because all syscon nodes
use the same compatible, it is pertinent to use this same file to list
IPs inside it. Thus, change the header to be more generic.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/arm/marvell/ap806-system-controller.txt           | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
index 0b887440e08a..a856eb9a4e05 100644
--- a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
@@ -2,14 +2,14 @@ Marvell Armada AP806 System Controller
 ======================================
 
 The AP806 is one of the two core HW blocks of the Marvell Armada 7K/8K
-SoCs. It contains a system controller, which provides a number
-registers giving access to numerous features: clocks, pin-muxing and
-many other SoC configuration items. This DT binding allows to describe
-this system controller.
+SoCs. It contains system controllers, which provide several registers
+giving access to numerous features: clocks, pin-muxing and many other
+SoC configuration items. This DT binding allows to describe these
+system controllers.
 
 For the top level node:
  - compatible: must be: "syscon", "simple-mfd";
-  - reg: register area of the AP806 system controller
+ - reg: register area of the AP806 system controller
 
 Clocks:
 -------
-- 
2.14.1

[PATCH v3 14/23] dt-bindings: ap806: prepare the syscon file to list other syscons nodes

[Miquel Raynal]

There are multiple system controllers in AP806. Because all syscon nodes
use the same compatible, it is pertinent to use this same file to list
IPs inside it. Thus, change the header to be more generic.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/arm/marvell/ap806-system-controller.txt           | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
index 0b887440e08a..a856eb9a4e05 100644
--- a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
@@ -2,14 +2,14 @@ Marvell Armada AP806 System Controller
 ======================================
 
 The AP806 is one of the two core HW blocks of the Marvell Armada 7K/8K
-SoCs. It contains a system controller, which provides a number
-registers giving access to numerous features: clocks, pin-muxing and
-many other SoC configuration items. This DT binding allows to describe
-this system controller.
+SoCs. It contains system controllers, which provide several registers
+giving access to numerous features: clocks, pin-muxing and many other
+SoC configuration items. This DT binding allows to describe these
+system controllers.
 
 For the top level node:
  - compatible: must be: "syscon", "simple-mfd";
-  - reg: register area of the AP806 system controller
+ - reg: register area of the AP806 system controller
 
 Clocks:
 -------
-- 
2.14.1

[PATCH v3 15/23] dt-bindings: cp110: prepare the syscon file to list other syscons nodes

[Miquel Raynal]

There are multiple system controllers in CP110. Because all syscon nodes
use the same compatible, it is pertinent to use this same file to list
IPs inside it. Thus, change the header to be more generic, and align
with AP806 file.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/arm/marvell/cp110-system-controller.txt       | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
index 29cdbae6c5ac..56e7fb1153e7 100644
--- a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
@@ -1,15 +1,15 @@
-Marvell Armada CP110 System Controller 0
-========================================
+Marvell Armada CP110 System Controller
+======================================
 
 The CP110 is one of the two core HW blocks of the Marvell Armada 7K/8K
-SoCs. It contains two sets of system control registers, System
-Controller 0 and System Controller 1. This Device Tree binding allows
-to describe the first system controller, which provides registers to
-configure various aspects of the SoC.
+SoCs. It contains system controllers, which provide several registers
+giving access to numerous features: clocks, pin-muxing and many other
+SoC configuration items. This DT binding allows to describe these
+system controllers.
 
 For the top level node:
  - compatible: must be: "syscon", "simple-mfd";
- - reg: register area of the CP110 system controller 0
+ - reg: register area of the CP110 system controller
 
 Clocks:
 -------
-- 
2.14.1

[PATCH v3 15/23] dt-bindings: cp110: prepare the syscon file to list other syscons nodes

[Miquel Raynal]

There are multiple system controllers in CP110. Because all syscon nodes
use the same compatible, it is pertinent to use this same file to list
IPs inside it. Thus, change the header to be more generic, and align
with AP806 file.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/arm/marvell/cp110-system-controller.txt       | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
index 29cdbae6c5ac..56e7fb1153e7 100644
--- a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
@@ -1,15 +1,15 @@
-Marvell Armada CP110 System Controller 0
-========================================
+Marvell Armada CP110 System Controller
+======================================
 
 The CP110 is one of the two core HW blocks of the Marvell Armada 7K/8K
-SoCs. It contains two sets of system control registers, System
-Controller 0 and System Controller 1. This Device Tree binding allows
-to describe the first system controller, which provides registers to
-configure various aspects of the SoC.
+SoCs. It contains system controllers, which provide several registers
+giving access to numerous features: clocks, pin-muxing and many other
+SoC configuration items. This DT binding allows to describe these
+system controllers.
 
 For the top level node:
  - compatible: must be: "syscon", "simple-mfd";
- - reg: register area of the CP110 system controller 0
+ - reg: register area of the CP110 system controller
 
 Clocks:
 -------
-- 
2.14.1

[PATCH v3 16/23] dt-bindings: ap806: add the thermal node in the syscon file

[Miquel Raynal]

Explain the thermal bindings now that the thermal IP is described being
inside of a system controller. Add a reference to the thermal-zone node.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../arm/marvell/ap806-system-controller.txt        | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
index a856eb9a4e05..3fd21bb7cb37 100644
--- a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
@@ -11,6 +11,9 @@ For the top level node:
  - compatible: must be: "syscon", "simple-mfd";
  - reg: register area of the AP806 system controller
 
+SYSTEM CONTROLLER 0
+===================
+
 Clocks:
 -------
 
@@ -98,3 +101,38 @@ ap_syscon: system-controller@6f4000 {
 		gpio-ranges = <&ap_pinctrl 0 0 19>;
 	};
 };
+
+SYSTEM CONTROLLER 1
+===================
+
+Thermal:
+--------
+
+For common binding part and usage, refer to
+Documentation/devicetree/bindings/thermal/thermal.txt
+
+The thermal IP can probe the temperature all around the processor. It
+may feature several channels, each of them wired to one sensor.
+
+Required properties:
+- compatible: must be one of:
+  * marvell,armada-ap806-thermal
+- reg: register range associated with the thermal functions.
+
+Optional properties:
+- #thermal-sensor-cells: shall be <1> when thermal-zones subnodes refer
+  to this IP and represents the channel ID. There is one sensor per
+  channel. O refers to the thermal IP internal channel, while positive
+  IDs refer to each CPU.
+
+Example:
+ap_syscon1: system-controller@6f8000 {
+	compatible = "syscon", "simple-mfd";
+	reg = <0x6f8000 0x1000>;
+
+	ap_thermal: thermal-sensor@80 {
+		compatible = "marvell,armada-ap806-thermal";
+		reg = <0x80 0x10>;
+		#thermal-sensor-cells = <1>;
+	};
+};
-- 
2.14.1

[PATCH v3 16/23] dt-bindings: ap806: add the thermal node in the syscon file

[Miquel Raynal]

Explain the thermal bindings now that the thermal IP is described being
inside of a system controller. Add a reference to the thermal-zone node.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../arm/marvell/ap806-system-controller.txt        | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
index a856eb9a4e05..3fd21bb7cb37 100644
--- a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
@@ -11,6 +11,9 @@ For the top level node:
  - compatible: must be: "syscon", "simple-mfd";
  - reg: register area of the AP806 system controller
 
+SYSTEM CONTROLLER 0
+===================
+
 Clocks:
 -------
 
@@ -98,3 +101,38 @@ ap_syscon: system-controller at 6f4000 {
 		gpio-ranges = <&ap_pinctrl 0 0 19>;
 	};
 };
+
+SYSTEM CONTROLLER 1
+===================
+
+Thermal:
+--------
+
+For common binding part and usage, refer to
+Documentation/devicetree/bindings/thermal/thermal.txt
+
+The thermal IP can probe the temperature all around the processor. It
+may feature several channels, each of them wired to one sensor.
+
+Required properties:
+- compatible: must be one of:
+  * marvell,armada-ap806-thermal
+- reg: register range associated with the thermal functions.
+
+Optional properties:
+- #thermal-sensor-cells: shall be <1> when thermal-zones subnodes refer
+  to this IP and represents the channel ID. There is one sensor per
+  channel. O refers to the thermal IP internal channel, while positive
+  IDs refer to each CPU.
+
+Example:
+ap_syscon1: system-controller at 6f8000 {
+	compatible = "syscon", "simple-mfd";
+	reg = <0x6f8000 0x1000>;
+
+	ap_thermal: thermal-sensor at 80 {
+		compatible = "marvell,armada-ap806-thermal";
+		reg = <0x80 0x10>;
+		#thermal-sensor-cells = <1>;
+	};
+};
-- 
2.14.1

[PATCH v3 17/23] dt-bindings: cp110: update documentation since DT de-duplication

[Miquel Raynal]

CP110 master/slave DT files have been merged in a DT de-duplication work
merged in v4.16. Update the syscon documentation accordingly to match
the current state of the DT nodes.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../bindings/arm/marvell/cp110-system-controller.txt           | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
index 56e7fb1153e7..54b0d64ce819 100644
--- a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
@@ -163,26 +163,26 @@ Required properties:
 
 Example:
 
-cpm_syscon0: system-controller@440000 {
+CP110_LABEL(syscon0): system-controller@440000 {
 	compatible = "syscon", "simple-mfd";
 	reg = <0x440000 0x1000>;
 
-	cpm_clk: clock {
+	CP110_LABEL(clk): clock {
 		compatible = "marvell,cp110-clock";
 		#clock-cells = <2>;
 	};
 
-	cpm_pinctrl: pinctrl {
+	CP110_LABEL(pinctrl): pinctrl {
 		compatible = "marvell,armada-8k-cpm-pinctrl";
 	};
 
-	cpm_gpio1: gpio@100 {
+	CP110_LABEL(gpio1): gpio@100 {
 		compatible = "marvell,armada-8k-gpio";
 		offset = <0x100>;
 		ngpios = <32>;
 		gpio-controller;
 		#gpio-cells = <2>;
-		gpio-ranges = <&cpm_pinctrl 0 0 32>;
+		gpio-ranges = <&CP110_LABEL(pinctrl) 0 0 32>;
 	};
 
 };
-- 
2.14.1

[PATCH v3 17/23] dt-bindings: cp110: update documentation since DT de-duplication

[Miquel Raynal]

CP110 master/slave DT files have been merged in a DT de-duplication work
merged in v4.16. Update the syscon documentation accordingly to match
the current state of the DT nodes.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../bindings/arm/marvell/cp110-system-controller.txt           | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
index 56e7fb1153e7..54b0d64ce819 100644
--- a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
@@ -163,26 +163,26 @@ Required properties:
 
 Example:
 
-cpm_syscon0: system-controller at 440000 {
+CP110_LABEL(syscon0): system-controller at 440000 {
 	compatible = "syscon", "simple-mfd";
 	reg = <0x440000 0x1000>;
 
-	cpm_clk: clock {
+	CP110_LABEL(clk): clock {
 		compatible = "marvell,cp110-clock";
 		#clock-cells = <2>;
 	};
 
-	cpm_pinctrl: pinctrl {
+	CP110_LABEL(pinctrl): pinctrl {
 		compatible = "marvell,armada-8k-cpm-pinctrl";
 	};
 
-	cpm_gpio1: gpio at 100 {
+	CP110_LABEL(gpio1): gpio at 100 {
 		compatible = "marvell,armada-8k-gpio";
 		offset = <0x100>;
 		ngpios = <32>;
 		gpio-controller;
 		#gpio-cells = <2>;
-		gpio-ranges = <&cpm_pinctrl 0 0 32>;
+		gpio-ranges = <&CP110_LABEL(pinctrl) 0 0 32>;
 	};
 
 };
-- 
2.14.1

[PATCH v3 18/23] dt-bindings: cp110: add the thermal node in the syscon file

[Miquel Raynal]

Explain the thermal bindings now that the thermal IP is described being
inside of a system controller. Add a reference to the thermal-zone node.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../arm/marvell/cp110-system-controller.txt        | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
index 54b0d64ce819..81ce742d2760 100644
--- a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
@@ -11,6 +11,9 @@ For the top level node:
  - compatible: must be: "syscon", "simple-mfd";
  - reg: register area of the CP110 system controller
 
+SYSTEM CONTROLLER 0
+===================
+
 Clocks:
 -------
 
@@ -186,3 +189,37 @@ CP110_LABEL(syscon0): system-controller@440000 {
 	};
 
 };
+
+SYSTEM CONTROLLER 1
+===================
+
+Thermal:
+--------
+
+The thermal IP can probe the temperature all around the processor. It
+may feature several channels, each of them wired to one sensor.
+
+For common binding part and usage, refer to
+Documentation/devicetree/bindings/thermal/thermal.txt
+
+Required properties:
+- compatible: must be one of:
+  * marvell,armada-cp110-thermal
+- reg: register range associated with the thermal functions.
+
+Optional properties:
+- #thermal-sensor-cells: shall be <1> when thermal-zones subnodes refer
+  to this IP and represents the channel ID. There is one sensor per
+  channel. O refers to the thermal IP internal channel.
+
+Example:
+CP110_LABEL(syscon1): system-controller@6f8000 {
+	compatible = "syscon", "simple-mfd";
+	reg = <0x6f8000 0x1000>;
+
+	CP110_LABEL(thermal): thermal-sensor@70 {
+		compatible = "marvell,armada-cp110-thermal";
+		reg = <0x70 0x10>;
+		#thermal-sensor-cells = <1>;
+	};
+};
-- 
2.14.1

[PATCH v3 18/23] dt-bindings: cp110: add the thermal node in the syscon file

[Miquel Raynal]

Explain the thermal bindings now that the thermal IP is described being
inside of a system controller. Add a reference to the thermal-zone node.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../arm/marvell/cp110-system-controller.txt        | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
index 54b0d64ce819..81ce742d2760 100644
--- a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
@@ -11,6 +11,9 @@ For the top level node:
  - compatible: must be: "syscon", "simple-mfd";
  - reg: register area of the CP110 system controller
 
+SYSTEM CONTROLLER 0
+===================
+
 Clocks:
 -------
 
@@ -186,3 +189,37 @@ CP110_LABEL(syscon0): system-controller at 440000 {
 	};
 
 };
+
+SYSTEM CONTROLLER 1
+===================
+
+Thermal:
+--------
+
+The thermal IP can probe the temperature all around the processor. It
+may feature several channels, each of them wired to one sensor.
+
+For common binding part and usage, refer to
+Documentation/devicetree/bindings/thermal/thermal.txt
+
+Required properties:
+- compatible: must be one of:
+  * marvell,armada-cp110-thermal
+- reg: register range associated with the thermal functions.
+
+Optional properties:
+- #thermal-sensor-cells: shall be <1> when thermal-zones subnodes refer
+  to this IP and represents the channel ID. There is one sensor per
+  channel. O refers to the thermal IP internal channel.
+
+Example:
+CP110_LABEL(syscon1): system-controller at 6f8000 {
+	compatible = "syscon", "simple-mfd";
+	reg = <0x6f8000 0x1000>;
+
+	CP110_LABEL(thermal): thermal-sensor at 70 {
+		compatible = "marvell,armada-cp110-thermal";
+		reg = <0x70 0x10>;
+		#thermal-sensor-cells = <1>;
+	};
+};
-- 
2.14.1

[PATCH v3 19/23] dt-bindings: thermal: armada: add reference to new bindings

[Miquel Raynal]

New bindings (using a syscon) are available for AP806 and CP110
compatibles. Add a reference to these files from the original
documentation.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/thermal/armada-thermal.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
index e0d013a2e66d..f3b441100890 100644
--- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
@@ -10,6 +10,11 @@ Required properties:
     * marvell,armada-ap806-thermal
     * marvell,armada-cp110-thermal
 
+Note: these bindings are deprecated for AP806/CP110 and should instead
+follow the rules described in:
+Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
+Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
+
 - 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
-- 
2.14.1

[PATCH v3 19/23] dt-bindings: thermal: armada: add reference to new bindings

[Miquel Raynal]

New bindings (using a syscon) are available for AP806 and CP110
compatibles. Add a reference to these files from the original
documentation.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/thermal/armada-thermal.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
index e0d013a2e66d..f3b441100890 100644
--- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
@@ -10,6 +10,11 @@ Required properties:
     * marvell,armada-ap806-thermal
     * marvell,armada-cp110-thermal
 
+Note: these bindings are deprecated for AP806/CP110 and should instead
+follow the rules described in:
+Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
+Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
+
 - 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
-- 
2.14.1

[PATCH v3 20/23] arm64: dts: marvell: move AP806/CP110 thermal nodes into a new syscon

[Miquel Raynal]

New bindings impose to declare the thermal IP from within a new syscon.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi | 14 ++++++++++----
 arch/arm64/boot/dts/marvell/armada-cp110.dtsi | 18 ++++++++++++------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
index 4f2a704615b0..45662fc09d18 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
@@ -258,10 +258,16 @@
 				};
 			};
 
-			ap_thermal: thermal@6f808c {
-				compatible = "marvell,armada-ap806-thermal";
-				reg = <0x6f808c 0x4>,
-				      <0x6f8084 0x8>;
+			ap_syscon1: system-controller@6f8000 {
+				compatible = "syscon", "simple-mfd";
+				reg = <0x6f8000 0x1000>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				ap_thermal: thermal-sensor@80 {
+					compatible = "marvell,armada-ap806-thermal";
+					reg = <0x80 0x10>;
+				};
 			};
 		};
 	};
diff --git a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
index ab31ba20aa2a..1b813a95149c 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
@@ -177,12 +177,6 @@
 			interrupts = <77 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
-		CP110_LABEL(thermal): thermal@400078 {
-			compatible = "marvell,armada-cp110-thermal";
-			reg = <0x400078 0x4>,
-			<0x400070 0x8>;
-		};
-
 		CP110_LABEL(syscon0): system-controller@440000 {
 			compatible = "syscon", "simple-mfd";
 			reg = <0x440000 0x2000>;
@@ -223,6 +217,18 @@
 			};
 		};
 
+		CP110_LABEL(syscon1): system-controller@400000 {
+			compatible = "syscon", "simple-mfd";
+			reg = <0x400000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			CP110_LABEL(thermal): thermal-sensor@70 {
+				compatible = "marvell,armada-cp110-thermal";
+				reg = <0x70 0x10>;
+			};
+		};
+
 		CP110_LABEL(usb3_0): usb3@500000 {
 			compatible = "marvell,armada-8k-xhci",
 			"generic-xhci";
-- 
2.14.1

[PATCH v3 20/23] arm64: dts: marvell: move AP806/CP110 thermal nodes into a new syscon

[Miquel Raynal]

New bindings impose to declare the thermal IP from within a new syscon.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi | 14 ++++++++++----
 arch/arm64/boot/dts/marvell/armada-cp110.dtsi | 18 ++++++++++++------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
index 4f2a704615b0..45662fc09d18 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
@@ -258,10 +258,16 @@
 				};
 			};
 
-			ap_thermal: thermal at 6f808c {
-				compatible = "marvell,armada-ap806-thermal";
-				reg = <0x6f808c 0x4>,
-				      <0x6f8084 0x8>;
+			ap_syscon1: system-controller at 6f8000 {
+				compatible = "syscon", "simple-mfd";
+				reg = <0x6f8000 0x1000>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				ap_thermal: thermal-sensor at 80 {
+					compatible = "marvell,armada-ap806-thermal";
+					reg = <0x80 0x10>;
+				};
 			};
 		};
 	};
diff --git a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
index ab31ba20aa2a..1b813a95149c 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
@@ -177,12 +177,6 @@
 			interrupts = <77 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
-		CP110_LABEL(thermal): thermal at 400078 {
-			compatible = "marvell,armada-cp110-thermal";
-			reg = <0x400078 0x4>,
-			<0x400070 0x8>;
-		};
-
 		CP110_LABEL(syscon0): system-controller at 440000 {
 			compatible = "syscon", "simple-mfd";
 			reg = <0x440000 0x2000>;
@@ -223,6 +217,18 @@
 			};
 		};
 
+		CP110_LABEL(syscon1): system-controller at 400000 {
+			compatible = "syscon", "simple-mfd";
+			reg = <0x400000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			CP110_LABEL(thermal): thermal-sensor at 70 {
+				compatible = "marvell,armada-cp110-thermal";
+				reg = <0x70 0x10>;
+			};
+		};
+
 		CP110_LABEL(usb3_0): usb3 at 500000 {
 			compatible = "marvell,armada-8k-xhci",
 			"generic-xhci";
-- 
2.14.1

[PATCH v3 21/23] arm64: dts: marvell: add thermal-zone node in ap806 DTSI file

[Miquel Raynal]

Add a thermal-zone node and fill in all the sensors available in an
ap806 (one in the IC plus one per CPU).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi | 60 +++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
index 45662fc09d18..fe56c044c8f0 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
@@ -6,6 +6,7 @@
  */
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/thermal/thermal.h>
 
 /dts-v1/;
 
@@ -267,8 +268,67 @@
 				ap_thermal: thermal-sensor@80 {
 					compatible = "marvell,armada-ap806-thermal";
 					reg = <0x80 0x10>;
+					#thermal-sensor-cells = <1>;
 				};
 			};
 		};
 	};
+
+	/*
+	 * The thermal IP features one internal sensor plus, if applicable, one
+	 * remote channel wired to one sensor per CPU.
+	 *
+	 * The cooling maps are always empty as there are no cooling devices.
+	 */
+	thermal-zones {
+		ap_thermal_ic: ap-thermal-ic {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&ap_thermal 0>;
+
+			trips {	};
+			cooling-maps { };
+		};
+
+		ap_thermal_cpu1: ap-thermal-cpu1 {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&ap_thermal 1>;
+
+			trips { };
+			cooling-maps { };
+		};
+
+		ap_thermal_cpu2: ap-thermal-cpu2 {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&ap_thermal 2>;
+
+			trips { };
+			cooling-maps { };
+		};
+
+		ap_thermal_cpu3: ap-thermal-cpu3 {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&ap_thermal 3>;
+
+			trips { };
+			cooling-maps { };
+		};
+
+		ap_thermal_cpu4: ap-thermal-cpu4 {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&ap_thermal 4>;
+
+			trips { };
+			cooling-maps { };
+		};
+	};
 };
-- 
2.14.1

[PATCH v3 21/23] arm64: dts: marvell: add thermal-zone node in ap806 DTSI file

[Miquel Raynal]

Add a thermal-zone node and fill in all the sensors available in an
ap806 (one in the IC plus one per CPU).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi | 60 +++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
index 45662fc09d18..fe56c044c8f0 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
@@ -6,6 +6,7 @@
  */
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/thermal/thermal.h>
 
 /dts-v1/;
 
@@ -267,8 +268,67 @@
 				ap_thermal: thermal-sensor at 80 {
 					compatible = "marvell,armada-ap806-thermal";
 					reg = <0x80 0x10>;
+					#thermal-sensor-cells = <1>;
 				};
 			};
 		};
 	};
+
+	/*
+	 * The thermal IP features one internal sensor plus, if applicable, one
+	 * remote channel wired to one sensor per CPU.
+	 *
+	 * The cooling maps are always empty as there are no cooling devices.
+	 */
+	thermal-zones {
+		ap_thermal_ic: ap-thermal-ic {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&ap_thermal 0>;
+
+			trips {	};
+			cooling-maps { };
+		};
+
+		ap_thermal_cpu1: ap-thermal-cpu1 {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&ap_thermal 1>;
+
+			trips { };
+			cooling-maps { };
+		};
+
+		ap_thermal_cpu2: ap-thermal-cpu2 {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&ap_thermal 2>;
+
+			trips { };
+			cooling-maps { };
+		};
+
+		ap_thermal_cpu3: ap-thermal-cpu3 {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&ap_thermal 3>;
+
+			trips { };
+			cooling-maps { };
+		};
+
+		ap_thermal_cpu4: ap-thermal-cpu4 {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&ap_thermal 4>;
+
+			trips { };
+			cooling-maps { };
+		};
+	};
 };
-- 
2.14.1

[PATCH v3 22/23] arm64: dts: marvell: add macro to make distinction between node names

[Miquel Raynal]

Because the label is different between CPs, the full path of a node is
unique. However, when referring to the end of the path only (the node
name), this name is not unique anymore.

The *thermal_zone_of_sensor_register() functions of the thermal core
present this limitation and prevent having a thermal-zone per CP.

Add a macro to make the distinction between node names to solve this
situation.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-common.dtsi b/arch/arm64/boot/dts/marvell/armada-common.dtsi
index d5e8aedec188..b29c6405d214 100644
--- a/arch/arm64/boot/dts/marvell/armada-common.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-common.dtsi
@@ -7,4 +7,5 @@
 #define PASTER(x, y) x ## y
 #define EVALUATOR(x, y) PASTER(x, y)
 #define CP110_LABEL(name) EVALUATOR(CP110_NAME, EVALUATOR(_, name))
+#define CP110_NODE_NAME(name) EVALUATOR(CP110_NAME, EVALUATOR(-, name))
 #define ADDRESSIFY(addr) EVALUATOR(0x, addr)
-- 
2.14.1

[PATCH v3 22/23] arm64: dts: marvell: add macro to make distinction between node names

[Miquel Raynal]

Because the label is different between CPs, the full path of a node is
unique. However, when referring to the end of the path only (the node
name), this name is not unique anymore.

The *thermal_zone_of_sensor_register() functions of the thermal core
present this limitation and prevent having a thermal-zone per CP.

Add a macro to make the distinction between node names to solve this
situation.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-common.dtsi b/arch/arm64/boot/dts/marvell/armada-common.dtsi
index d5e8aedec188..b29c6405d214 100644
--- a/arch/arm64/boot/dts/marvell/armada-common.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-common.dtsi
@@ -7,4 +7,5 @@
 #define PASTER(x, y) x ## y
 #define EVALUATOR(x, y) PASTER(x, y)
 #define CP110_LABEL(name) EVALUATOR(CP110_NAME, EVALUATOR(_, name))
+#define CP110_NODE_NAME(name) EVALUATOR(CP110_NAME, EVALUATOR(-, name))
 #define ADDRESSIFY(addr) EVALUATOR(0x, addr)
-- 
2.14.1

[PATCH v3 23/23] arm64: dts: marvell: add thermal-zone node in cp110 DTSI file

[Miquel Raynal]

Add a thermal-zone node and fill in all the sensors available in a
cp110 (only one in the thermal IP).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-cp110.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
index 1b813a95149c..e054ca0d184d 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
@@ -6,6 +6,7 @@
  */
 
 #include <dt-bindings/interrupt-controller/mvebu-icu.h>
+#include <dt-bindings/thermal/thermal.h>
 
 #include "armada-common.dtsi"
 
@@ -19,6 +20,23 @@
 	 * save one indentation level
 	 */
 	CP110_NAME: CP110_NAME { };
+
+	/*
+	 * CPs only have one sensor in the thermal IC.
+	 *
+	 * The cooling maps are empty as there are no cooling devices.
+	 */
+	thermal-zones {
+		CP110_LABEL(thermal_ic): CP110_NODE_NAME(thermal-ic) {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&CP110_LABEL(thermal) 0>;
+
+			trips {	};
+			cooling-maps { };
+		};
+	};
 };
 
 &CP110_NAME {
@@ -226,6 +244,7 @@
 			CP110_LABEL(thermal): thermal-sensor@70 {
 				compatible = "marvell,armada-cp110-thermal";
 				reg = <0x70 0x10>;
+				#thermal-sensor-cells = <1>;
 			};
 		};
 
-- 
2.14.1

[PATCH v3 23/23] arm64: dts: marvell: add thermal-zone node in cp110 DTSI file

[Miquel Raynal]

Add a thermal-zone node and fill in all the sensors available in a
cp110 (only one in the thermal IP).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-cp110.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
index 1b813a95149c..e054ca0d184d 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
@@ -6,6 +6,7 @@
  */
 
 #include <dt-bindings/interrupt-controller/mvebu-icu.h>
+#include <dt-bindings/thermal/thermal.h>
 
 #include "armada-common.dtsi"
 
@@ -19,6 +20,23 @@
 	 * save one indentation level
 	 */
 	CP110_NAME: CP110_NAME { };
+
+	/*
+	 * CPs only have one sensor in the thermal IC.
+	 *
+	 * The cooling maps are empty as there are no cooling devices.
+	 */
+	thermal-zones {
+		CP110_LABEL(thermal_ic): CP110_NODE_NAME(thermal-ic) {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&CP110_LABEL(thermal) 0>;
+
+			trips {	};
+			cooling-maps { };
+		};
+	};
 };
 
 &CP110_NAME {
@@ -226,6 +244,7 @@
 			CP110_LABEL(thermal): thermal-sensor at 70 {
 				compatible = "marvell,armada-cp110-thermal";
 				reg = <0x70 0x10>;
+				#thermal-sensor-cells = <1>;
 			};
 		};
 
-- 
2.14.1

Re: [PATCH v3 19/23] dt-bindings: thermal: armada: add reference to new bindings

[Rob Herring]

On Mon, Jul 16, 2018 at 04:42:02PM +0200, Miquel Raynal wrote:
> New bindings (using a syscon) are available for AP806 and CP110
> compatibles. Add a reference to these files from the original
> documentation.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/thermal/armada-thermal.txt | 5 +++++
>  1 file changed, 5 insertions(+)

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

[PATCH v3 19/23] dt-bindings: thermal: armada: add reference to new bindings

[Rob Herring]

On Mon, Jul 16, 2018 at 04:42:02PM +0200, Miquel Raynal wrote:
> New bindings (using a syscon) are available for AP806 and CP110
> compatibles. Add a reference to these files from the original
> documentation.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/thermal/armada-thermal.txt | 5 +++++
>  1 file changed, 5 insertions(+)

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

Re: [PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Miquel Raynal]

Hi Eduardo,

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
16:41:43 +0200:

> The only capability of the Armada thermal driver is currently just to
> read one sensor (the default one) per AP and one per CP. Actually,
> there is one sensor per core in the AP806 plus one sensor in the
> thermal IP itself. The CP110 just features one thermal sensor in its
> own thermal IP.
> 
> This series first improves the readability of this driver, then adds
> support for multi-channel thermal IPs. The bindings and the
> device-trees are updated accordingly.
> 
> Thank you,
> Miquèl

I know I sent this series on monday but there was almost no change on
it (see below), do you think you will have the time to look at it
before it's too late for the merge window?

Thank you,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Miquel Raynal]

Hi Eduardo,

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
16:41:43 +0200:

> The only capability of the Armada thermal driver is currently just to
> read one sensor (the default one) per AP and one per CP. Actually,
> there is one sensor per core in the AP806 plus one sensor in the
> thermal IP itself. The CP110 just features one thermal sensor in its
> own thermal IP.
> 
> This series first improves the readability of this driver, then adds
> support for multi-channel thermal IPs. The bindings and the
> device-trees are updated accordingly.
> 
> Thank you,
> Miqu?l

I know I sent this series on monday but there was almost no change on
it (see below), do you think you will have the time to look at it
before it's too late for the merge window?

Thank you,
Miqu?l

Re: [PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Miquel Raynal]

Hi Eduardo, Zhang,

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
17:28:49 +0200:

> Hi Eduardo,
> 
> Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> 16:41:43 +0200:
> 
> > The only capability of the Armada thermal driver is currently just to
> > read one sensor (the default one) per AP and one per CP. Actually,
> > there is one sensor per core in the AP806 plus one sensor in the
> > thermal IP itself. The CP110 just features one thermal sensor in its
> > own thermal IP.
> > 
> > This series first improves the readability of this driver, then adds
> > support for multi-channel thermal IPs. The bindings and the
> > device-trees are updated accordingly.
> > 
> > Thank you,
> > Miquèl  
> 
> I know I sent this series on monday but there was almost no change on
> it (see below), do you think you will have the time to look at it
> before it's too late for the merge window?

I know a little bit what it is to be on the maintainer side of a
subsystem but these changes are laying since April, 21st and I really
expect to get them merged. Zhang, as Eduardo seems to be overloaded
this release, could you please take the series?

Of course if there are things to address I'll be happy to do so.

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Miquel Raynal]

Hi Eduardo, Zhang,

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
17:28:49 +0200:

> Hi Eduardo,
> 
> Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> 16:41:43 +0200:
> 
> > The only capability of the Armada thermal driver is currently just to
> > read one sensor (the default one) per AP and one per CP. Actually,
> > there is one sensor per core in the AP806 plus one sensor in the
> > thermal IP itself. The CP110 just features one thermal sensor in its
> > own thermal IP.
> > 
> > This series first improves the readability of this driver, then adds
> > support for multi-channel thermal IPs. The bindings and the
> > device-trees are updated accordingly.
> > 
> > Thank you,
> > Miqu?l  
> 
> I know I sent this series on monday but there was almost no change on
> it (see below), do you think you will have the time to look at it
> before it's too late for the merge window?

I know a little bit what it is to be on the maintainer side of a
subsystem but these changes are laying since April, 21st and I really
expect to get them merged. Zhang, as Eduardo seems to be overloaded
this release, could you please take the series?

Of course if there are things to address I'll be happy to do so.

Thanks,
Miqu?l

Re: [PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Zhang Rui]

On 三, 2018-07-25 at 09:52 +0200, Miquel Raynal wrote:
> Hi Eduardo, Zhang,
> 
> Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
> 17:28:49 +0200:
> 
> > 
> > Hi Eduardo,
> > 
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> > 16:41:43 +0200:
> > 
> > > 
> > > The only capability of the Armada thermal driver is currently
> > > just to
> > > read one sensor (the default one) per AP and one per CP.
> > > Actually,
> > > there is one sensor per core in the AP806 plus one sensor in the
> > > thermal IP itself. The CP110 just features one thermal sensor in
> > > its
> > > own thermal IP.
> > > 
> > > This series first improves the readability of this driver, then
> > > adds
> > > support for multi-channel thermal IPs. The bindings and the
> > > device-trees are updated accordingly.
> > > 
> > > Thank you,
> > > Miquèl  
> > I know I sent this series on monday but there was almost no change
> > on
> > it (see below), do you think you will have the time to look at it
> > before it's too late for the merge window?
> I know a little bit what it is to be on the maintainer side of a
> subsystem but these changes are laying since April, 21st and I really
> expect to get them merged. Zhang, as Eduardo seems to be overloaded
> this release, could you please take the series?
> 
> Of course if there are things to address I'll be happy to do so.
> 
I had a sync with Eduardo during last merge window, and we agreed to
send separate git pull requests to Linus since this release and see how
it works.
This means that all the soc thermal driver patches should go to
Eduardo' tree and then Linus' tree directly.

Anyway, let's ping Eduardo and see if he can review/take the patches.

thanks,
rui
> Thanks,
> Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Zhang Rui]

On ?, 2018-07-25 at 09:52 +0200, Miquel Raynal wrote:
> Hi Eduardo, Zhang,
> 
> Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
> 17:28:49 +0200:
> 
> > 
> > Hi Eduardo,
> > 
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> > 16:41:43 +0200:
> > 
> > > 
> > > The only capability of the Armada thermal driver is currently
> > > just to
> > > read one sensor (the default one) per AP and one per CP.
> > > Actually,
> > > there is one sensor per core in the AP806 plus one sensor in the
> > > thermal IP itself. The CP110 just features one thermal sensor in
> > > its
> > > own thermal IP.
> > > 
> > > This series first improves the readability of this driver, then
> > > adds
> > > support for multi-channel thermal IPs. The bindings and the
> > > device-trees are updated accordingly.
> > > 
> > > Thank you,
> > > Miqu?l??
> > I know I sent this series on monday but there was almost no change
> > on
> > it (see below), do you think you will have the time to look at it
> > before it's too late for the merge window?
> I know a little bit what it is to be on the maintainer side of a
> subsystem but these changes are laying since April, 21st and I really
> expect to get them merged. Zhang, as Eduardo seems to be overloaded
> this release, could you please take the series?
> 
> Of course if there are things to address I'll be happy to do so.
> 
I had a sync with Eduardo during last merge window, and we agreed to
send separate git pull requests to Linus since this release and see how
it works.
This means that all the soc thermal driver patches should go to
Eduardo' tree and then Linus' tree directly.

Anyway, let's ping Eduardo and see if he can review/take the patches.

thanks,
rui
> Thanks,
> Miqu?l

Re: [PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Eduardo Valentin]

Hello,

On Thu, Jul 26, 2018 at 10:00:41AM +0800, Zhang Rui wrote:
> On 三, 2018-07-25 at 09:52 +0200, Miquel Raynal wrote:
> > Hi Eduardo, Zhang,
> > 
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
> > 17:28:49 +0200:
> > 
> > > 
> > > Hi Eduardo,
> > > 
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> > > 16:41:43 +0200:
> > > 
> > > > 
> > > > The only capability of the Armada thermal driver is currently
> > > > just to
> > > > read one sensor (the default one) per AP and one per CP.
> > > > Actually,
> > > > there is one sensor per core in the AP806 plus one sensor in the
> > > > thermal IP itself. The CP110 just features one thermal sensor in
> > > > its
> > > > own thermal IP.
> > > > 
> > > > This series first improves the readability of this driver, then
> > > > adds
> > > > support for multi-channel thermal IPs. The bindings and the
> > > > device-trees are updated accordingly.
> > > > 
> > > > Thank you,
> > > > Miquèl  
> > > I know I sent this series on monday but there was almost no change
> > > on
> > > it (see below), do you think you will have the time to look at it
> > > before it's too late for the merge window?
> > I know a little bit what it is to be on the maintainer side of a
> > subsystem but these changes are laying since April, 21st and I really
> > expect to get them merged. Zhang, as Eduardo seems to be overloaded
> > this release, could you please take the series?
> > 
> > Of course if there are things to address I'll be happy to do so.
> > 
> I had a sync with Eduardo during last merge window, and we agreed to
> send separate git pull requests to Linus since this release and see how
> it works.
> This means that all the soc thermal driver patches should go to
> Eduardo' tree and then Linus' tree directly.
> 
> Anyway, let's ping Eduardo and see if he can review/take the patches.

Sorry fellows, but I was off camping in a place with no electricity/no
internet. I looked at this series before, and I do not remember anything
that could block it to go. Let me restart on working on it.

> 
> thanks,
> rui
> > Thanks,
> > Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Eduardo Valentin]

Hello,

On Thu, Jul 26, 2018 at 10:00:41AM +0800, Zhang Rui wrote:
> On ?, 2018-07-25 at 09:52 +0200, Miquel Raynal wrote:
> > Hi Eduardo, Zhang,
> > 
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
> > 17:28:49 +0200:
> > 
> > > 
> > > Hi Eduardo,
> > > 
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> > > 16:41:43 +0200:
> > > 
> > > > 
> > > > The only capability of the Armada thermal driver is currently
> > > > just to
> > > > read one sensor (the default one) per AP and one per CP.
> > > > Actually,
> > > > there is one sensor per core in the AP806 plus one sensor in the
> > > > thermal IP itself. The CP110 just features one thermal sensor in
> > > > its
> > > > own thermal IP.
> > > > 
> > > > This series first improves the readability of this driver, then
> > > > adds
> > > > support for multi-channel thermal IPs. The bindings and the
> > > > device-trees are updated accordingly.
> > > > 
> > > > Thank you,
> > > > Miqu?l??
> > > I know I sent this series on monday but there was almost no change
> > > on
> > > it (see below), do you think you will have the time to look at it
> > > before it's too late for the merge window?
> > I know a little bit what it is to be on the maintainer side of a
> > subsystem but these changes are laying since April, 21st and I really
> > expect to get them merged. Zhang, as Eduardo seems to be overloaded
> > this release, could you please take the series?
> > 
> > Of course if there are things to address I'll be happy to do so.
> > 
> I had a sync with Eduardo during last merge window, and we agreed to
> send separate git pull requests to Linus since this release and see how
> it works.
> This means that all the soc thermal driver patches should go to
> Eduardo' tree and then Linus' tree directly.
> 
> Anyway, let's ping Eduardo and see if he can review/take the patches.

Sorry fellows, but I was off camping in a place with no electricity/no
internet. I looked at this series before, and I do not remember anything
that could block it to go. Let me restart on working on it.

> 
> thanks,
> rui
> > Thanks,
> > Miqu?l

Re: [PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Eduardo Valentin]

On Thu, Jul 26, 2018 at 02:14:43PM -0700, Eduardo Valentin wrote:
> Hello,
> 
> On Thu, Jul 26, 2018 at 10:00:41AM +0800, Zhang Rui wrote:
> > On 三, 2018-07-25 at 09:52 +0200, Miquel Raynal wrote:
> > > Hi Eduardo, Zhang,
> > > 
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
> > > 17:28:49 +0200:
> > > 
> > > > 
> > > > Hi Eduardo,
> > > > 
> > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> > > > 16:41:43 +0200:
> > > > 
> > > > > 
> > > > > The only capability of the Armada thermal driver is currently
> > > > > just to
> > > > > read one sensor (the default one) per AP and one per CP.
> > > > > Actually,
> > > > > there is one sensor per core in the AP806 plus one sensor in the
> > > > > thermal IP itself. The CP110 just features one thermal sensor in
> > > > > its
> > > > > own thermal IP.
> > > > > 
> > > > > This series first improves the readability of this driver, then
> > > > > adds
> > > > > support for multi-channel thermal IPs. The bindings and the
> > > > > device-trees are updated accordingly.
> > > > > 
> > > > > Thank you,
> > > > > Miquèl  
> > > > I know I sent this series on monday but there was almost no change
> > > > on
> > > > it (see below), do you think you will have the time to look at it
> > > > before it's too late for the merge window?
> > > I know a little bit what it is to be on the maintainer side of a
> > > subsystem but these changes are laying since April, 21st and I really
> > > expect to get them merged. Zhang, as Eduardo seems to be overloaded
> > > this release, could you please take the series?
> > > 
> > > Of course if there are things to address I'll be happy to do so.
> > > 
> > I had a sync with Eduardo during last merge window, and we agreed to
> > send separate git pull requests to Linus since this release and see how
> > it works.
> > This means that all the soc thermal driver patches should go to
> > Eduardo' tree and then Linus' tree directly.
> > 
> > Anyway, let's ping Eduardo and see if he can review/take the patches.
> 
> Sorry fellows, but I was off camping in a place with no electricity/no
> internet. I looked at this series before, and I do not remember anything
> that could block it to go. Let me restart on working on it.

Let me take that back. I got some compilation error.

> 
> > 
> > thanks,
> > rui
> > > Thanks,
> > > Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Eduardo Valentin]

On Thu, Jul 26, 2018 at 02:14:43PM -0700, Eduardo Valentin wrote:
> Hello,
> 
> On Thu, Jul 26, 2018 at 10:00:41AM +0800, Zhang Rui wrote:
> > On ?, 2018-07-25 at 09:52 +0200, Miquel Raynal wrote:
> > > Hi Eduardo, Zhang,
> > > 
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
> > > 17:28:49 +0200:
> > > 
> > > > 
> > > > Hi Eduardo,
> > > > 
> > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> > > > 16:41:43 +0200:
> > > > 
> > > > > 
> > > > > The only capability of the Armada thermal driver is currently
> > > > > just to
> > > > > read one sensor (the default one) per AP and one per CP.
> > > > > Actually,
> > > > > there is one sensor per core in the AP806 plus one sensor in the
> > > > > thermal IP itself. The CP110 just features one thermal sensor in
> > > > > its
> > > > > own thermal IP.
> > > > > 
> > > > > This series first improves the readability of this driver, then
> > > > > adds
> > > > > support for multi-channel thermal IPs. The bindings and the
> > > > > device-trees are updated accordingly.
> > > > > 
> > > > > Thank you,
> > > > > Miqu?l??
> > > > I know I sent this series on monday but there was almost no change
> > > > on
> > > > it (see below), do you think you will have the time to look at it
> > > > before it's too late for the merge window?
> > > I know a little bit what it is to be on the maintainer side of a
> > > subsystem but these changes are laying since April, 21st and I really
> > > expect to get them merged. Zhang, as Eduardo seems to be overloaded
> > > this release, could you please take the series?
> > > 
> > > Of course if there are things to address I'll be happy to do so.
> > > 
> > I had a sync with Eduardo during last merge window, and we agreed to
> > send separate git pull requests to Linus since this release and see how
> > it works.
> > This means that all the soc thermal driver patches should go to
> > Eduardo' tree and then Linus' tree directly.
> > 
> > Anyway, let's ping Eduardo and see if he can review/take the patches.
> 
> Sorry fellows, but I was off camping in a place with no electricity/no
> internet. I looked at this series before, and I do not remember anything
> that could block it to go. Let me restart on working on it.

Let me take that back. I got some compilation error.

> 
> > 
> > thanks,
> > rui
> > > Thanks,
> > > Miqu?l

Re: [PATCH v3 08/23] thermal: armada: use the resource managed registration helper alternative

[Eduardo Valentin]

Hey,

On Mon, Jul 16, 2018 at 04:41:51PM +0200, Miquel Raynal wrote:
> Current use of thermal_zone_device_register() triggers a warning at boot
> and should be replaced by devm_thermal_zone_of_sensor_register(). This
> allows better handling of multiple thermal zones for later multi-sensors
> support.
> 
> Also change the driver data to embed a new structure to make the
> difference between legacy data (which needs to be cleaned) and
> syscon-related data.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/thermal/armada_thermal.c | 123 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 101 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index be346c6afde2..dd894312dedf 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -64,6 +64,7 @@ struct armada_thermal_data;
>  
>  /* Marvell EBU Thermal Sensor Dev Structure */
>  struct armada_thermal_priv {
> +	struct device *dev;
>  	struct regmap *syscon;
>  	char zone_name[THERMAL_NAME_LENGTH];
>  	struct armada_thermal_data *data;
> @@ -95,6 +96,26 @@ struct armada_thermal_data {
>  	unsigned int syscon_status_off;
>  };
>  
> +struct armada_drvdata {
> +	enum drvtype {
> +		LEGACY,
> +		SYSCON
> +	} type;
> +	union {
> +		struct armada_thermal_priv *priv;
> +		struct thermal_zone_device *tz;
> +	} data;
> +};
> +
> +/*
> + * struct armada_thermal_sensor - hold the information of one thermal sensor
> + * @thermal: pointer to the local private structure
> + * @tzd: pointer to the thermal zone device
> + */
> +struct armada_thermal_sensor {
> +	struct armada_thermal_priv *priv;
> +};
> +
>  static void armadaxp_init(struct platform_device *pdev,
>  			  struct armada_thermal_priv *priv)
>  {
> @@ -243,16 +264,14 @@ static bool armada_is_valid(struct armada_thermal_priv *priv)
>  	return reg & priv->data->is_valid_bit;
>  }
>  
> -static int armada_get_temp(struct thermal_zone_device *thermal,
> -			   int *temp)
> +static int armada_read_sensor(struct armada_thermal_priv *priv, int *temp)
>  {
> -	struct armada_thermal_priv *priv = thermal->devdata;
>  	u32 reg, div;
>  	s64 sample, b, m;
>  
>  	/* Valid check */
>  	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
> -		dev_err(&thermal->device,
> +		dev_err(priv->dev,
>  			"Temperature sensor reading not valid\n");
>  		return -EIO;
>  	}
> @@ -278,7 +297,33 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
>  	return 0;
>  }
>  
> -static struct thermal_zone_device_ops ops = {
> +static int armada_get_temp_legacy(struct thermal_zone_device *thermal,
> +				  int *temp)
> +{
> +	struct armada_thermal_priv *priv = thermal->devdata;
> +	int ret;
> +
> +	/* Do the actual reading */
> +	ret = armada_read_sensor(priv, temp);
> +
> +	return ret;
> +}
> +
> +static struct thermal_zone_device_ops legacy_ops = {
> +	.get_temp = armada_get_temp_legacy,
> +};
> +
> +static int armada_get_temp(void *_sensor, int *temp)
> +{
> +	struct armada_thermal_sensor *sensor = _sensor;
> +	struct armada_thermal_priv *priv = sensor->priv;
> +	int ret;


The above variable:
drivers/thermal/armada_thermal.c:320:6: warning: unused variable ‘ret’ [-Wunused-variable]
  int ret;
      ^~~
cc1: some warnings being treated as errors
scripts/Makefile.build:323: recipe for target 'drivers/thermal/armada_thermal.o' failed


> +
> +	/* Do the actual reading */
> +	return armada_read_sensor(priv, &temp);

I think you meant 

+	return armada_read_sensor(priv, temp);

here right?


I obviously get:

  CC [M]  drivers/thermal/armada_thermal.o
drivers/thermal/armada_thermal.c: In function ‘armada_get_temp’:
drivers/thermal/armada_thermal.c:323:34: error: passing argument 2 of ‘armada_read_sensor’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  return armada_read_sensor(priv, &temp);
                                  ^
drivers/thermal/armada_thermal.c:267:12: note: expected ‘int *’ but argument is of type ‘int **’
 static int armada_read_sensor(struct armada_thermal_priv *priv, int *temp)
            ^~~~~~~~~~~~~~~~~~


> +}
> +
> +static struct thermal_zone_of_device_ops of_ops = {
>  	.get_temp = armada_get_temp,
>  };
>  
> @@ -480,7 +525,9 @@ static void armada_set_sane_name(struct platform_device *pdev,
>  
>  static int armada_thermal_probe(struct platform_device *pdev)
>  {
> -	struct thermal_zone_device *thermal;
> +	struct thermal_zone_device *tz;
> +	struct armada_thermal_sensor *sensors;
> +	struct armada_drvdata *drvdata;
>  	const struct of_device_id *match;
>  	struct armada_thermal_priv *priv;
>  	int ret;
> @@ -493,11 +540,13 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  	if (!priv)
>  		return -ENOMEM;
>  
> +	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dev = &pdev->dev;
>  	priv->data = (struct armada_thermal_data *)match->data;
>  
> -	/* Ensure device name is correct for the thermal core */
> -	armada_set_sane_name(pdev, priv);
> -
>  	/*
>  	 * Legacy DT bindings only described "control1" register (also referred
>  	 * as "control MSB" on old documentation). Then, bindings moved to cover
> @@ -510,35 +559,65 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  	 * is to define an overall system controller and put the thermal node
>  	 * into it, which requires the use of regmaps across all the driver.
>  	 */
> -	if (IS_ERR(syscon_node_to_regmap(pdev->dev.parent->of_node)))
> +	if (IS_ERR(syscon_node_to_regmap(pdev->dev.parent->of_node))) {
> +		/* Ensure device name is correct for the thermal core */
> +		armada_set_sane_name(pdev, priv);
> +
>  		ret = armada_thermal_probe_legacy(pdev, priv);
> -	else
> -		ret = armada_thermal_probe_syscon(pdev, priv);
> +		if (ret)
> +			return ret;
>  
> +		priv->data->init(pdev, priv);
> +
> +		tz = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
> +						  &legacy_ops, NULL, 0, 0);
> +		if (IS_ERR(tz)) {
> +			dev_err(&pdev->dev,
> +				"Failed to register thermal zone device\n");
> +			return PTR_ERR(tz);
> +		}
> +
> +		drvdata->type = LEGACY;
> +		drvdata->data.tz = tz;
> +		platform_set_drvdata(pdev, drvdata);
> +
> +		return 0;
> +	}
> +
> +	ret = armada_thermal_probe_syscon(pdev, priv);
>  	if (ret)
>  		return ret;
>  
>  	priv->data->init(pdev, priv);
> +	drvdata->type = SYSCON;
> +	drvdata->data.priv = priv;
> +	platform_set_drvdata(pdev, drvdata);
>  
> -	thermal = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
> -					       &ops, NULL, 0, 0);
> -	if (IS_ERR(thermal)) {
> +	sensors = devm_kzalloc(&pdev->dev, sizeof(struct armada_thermal_sensor),
> +			       GFP_KERNEL);
> +	if (!sensors)
> +		return -ENOMEM;
> +
> +	sensors->priv = priv;
> +
> +	tz = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, sensors,
> +						  &of_ops);
> +	if (IS_ERR(tz)) {
>  		dev_err(&pdev->dev,
> -			"Failed to register thermal zone device\n");
> -		return PTR_ERR(thermal);
> +			"Failed to register thermal sensor (err: %ld)\n",
> +			PTR_ERR(tz));
> +		return PTR_ERR(tz);
>  	}
>  
> -	platform_set_drvdata(pdev, thermal);
> -
>  	return 0;
>  }
>  
>  static int armada_thermal_exit(struct platform_device *pdev)
>  {
> -	struct thermal_zone_device *armada_thermal =
> -		platform_get_drvdata(pdev);
> +	struct armada_drvdata *drvdata = platform_get_drvdata(pdev);
>  
> -	thermal_zone_device_unregister(armada_thermal);
> +	if (drvdata->type == LEGACY)
> +		thermal_zone_device_unregister(drvdata->data.tz);
>  
>  	return 0;
>  }
> -- 
> 2.14.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 08/23] thermal: armada: use the resource managed registration helper alternative

[Eduardo Valentin]

Hey,

On Mon, Jul 16, 2018 at 04:41:51PM +0200, Miquel Raynal wrote:
> Current use of thermal_zone_device_register() triggers a warning at boot
> and should be replaced by devm_thermal_zone_of_sensor_register(). This
> allows better handling of multiple thermal zones for later multi-sensors
> support.
> 
> Also change the driver data to embed a new structure to make the
> difference between legacy data (which needs to be cleaned) and
> syscon-related data.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/thermal/armada_thermal.c | 123 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 101 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index be346c6afde2..dd894312dedf 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -64,6 +64,7 @@ struct armada_thermal_data;
>  
>  /* Marvell EBU Thermal Sensor Dev Structure */
>  struct armada_thermal_priv {
> +	struct device *dev;
>  	struct regmap *syscon;
>  	char zone_name[THERMAL_NAME_LENGTH];
>  	struct armada_thermal_data *data;
> @@ -95,6 +96,26 @@ struct armada_thermal_data {
>  	unsigned int syscon_status_off;
>  };
>  
> +struct armada_drvdata {
> +	enum drvtype {
> +		LEGACY,
> +		SYSCON
> +	} type;
> +	union {
> +		struct armada_thermal_priv *priv;
> +		struct thermal_zone_device *tz;
> +	} data;
> +};
> +
> +/*
> + * struct armada_thermal_sensor - hold the information of one thermal sensor
> + * @thermal: pointer to the local private structure
> + * @tzd: pointer to the thermal zone device
> + */
> +struct armada_thermal_sensor {
> +	struct armada_thermal_priv *priv;
> +};
> +
>  static void armadaxp_init(struct platform_device *pdev,
>  			  struct armada_thermal_priv *priv)
>  {
> @@ -243,16 +264,14 @@ static bool armada_is_valid(struct armada_thermal_priv *priv)
>  	return reg & priv->data->is_valid_bit;
>  }
>  
> -static int armada_get_temp(struct thermal_zone_device *thermal,
> -			   int *temp)
> +static int armada_read_sensor(struct armada_thermal_priv *priv, int *temp)
>  {
> -	struct armada_thermal_priv *priv = thermal->devdata;
>  	u32 reg, div;
>  	s64 sample, b, m;
>  
>  	/* Valid check */
>  	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
> -		dev_err(&thermal->device,
> +		dev_err(priv->dev,
>  			"Temperature sensor reading not valid\n");
>  		return -EIO;
>  	}
> @@ -278,7 +297,33 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
>  	return 0;
>  }
>  
> -static struct thermal_zone_device_ops ops = {
> +static int armada_get_temp_legacy(struct thermal_zone_device *thermal,
> +				  int *temp)
> +{
> +	struct armada_thermal_priv *priv = thermal->devdata;
> +	int ret;
> +
> +	/* Do the actual reading */
> +	ret = armada_read_sensor(priv, temp);
> +
> +	return ret;
> +}
> +
> +static struct thermal_zone_device_ops legacy_ops = {
> +	.get_temp = armada_get_temp_legacy,
> +};
> +
> +static int armada_get_temp(void *_sensor, int *temp)
> +{
> +	struct armada_thermal_sensor *sensor = _sensor;
> +	struct armada_thermal_priv *priv = sensor->priv;
> +	int ret;


The above variable:
drivers/thermal/armada_thermal.c:320:6: warning: unused variable ?ret? [-Wunused-variable]
  int ret;
      ^~~
cc1: some warnings being treated as errors
scripts/Makefile.build:323: recipe for target 'drivers/thermal/armada_thermal.o' failed


> +
> +	/* Do the actual reading */
> +	return armada_read_sensor(priv, &temp);

I think you meant 

+	return armada_read_sensor(priv, temp);

here right?


I obviously get:

  CC [M]  drivers/thermal/armada_thermal.o
drivers/thermal/armada_thermal.c: In function ?armada_get_temp?:
drivers/thermal/armada_thermal.c:323:34: error: passing argument 2 of ?armada_read_sensor? from incompatible pointer type [-Werror=incompatible-pointer-types]
  return armada_read_sensor(priv, &temp);
                                  ^
drivers/thermal/armada_thermal.c:267:12: note: expected ?int *? but argument is of type ?int **?
 static int armada_read_sensor(struct armada_thermal_priv *priv, int *temp)
            ^~~~~~~~~~~~~~~~~~


> +}
> +
> +static struct thermal_zone_of_device_ops of_ops = {
>  	.get_temp = armada_get_temp,
>  };
>  
> @@ -480,7 +525,9 @@ static void armada_set_sane_name(struct platform_device *pdev,
>  
>  static int armada_thermal_probe(struct platform_device *pdev)
>  {
> -	struct thermal_zone_device *thermal;
> +	struct thermal_zone_device *tz;
> +	struct armada_thermal_sensor *sensors;
> +	struct armada_drvdata *drvdata;
>  	const struct of_device_id *match;
>  	struct armada_thermal_priv *priv;
>  	int ret;
> @@ -493,11 +540,13 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  	if (!priv)
>  		return -ENOMEM;
>  
> +	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dev = &pdev->dev;
>  	priv->data = (struct armada_thermal_data *)match->data;
>  
> -	/* Ensure device name is correct for the thermal core */
> -	armada_set_sane_name(pdev, priv);
> -
>  	/*
>  	 * Legacy DT bindings only described "control1" register (also referred
>  	 * as "control MSB" on old documentation). Then, bindings moved to cover
> @@ -510,35 +559,65 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  	 * is to define an overall system controller and put the thermal node
>  	 * into it, which requires the use of regmaps across all the driver.
>  	 */
> -	if (IS_ERR(syscon_node_to_regmap(pdev->dev.parent->of_node)))
> +	if (IS_ERR(syscon_node_to_regmap(pdev->dev.parent->of_node))) {
> +		/* Ensure device name is correct for the thermal core */
> +		armada_set_sane_name(pdev, priv);
> +
>  		ret = armada_thermal_probe_legacy(pdev, priv);
> -	else
> -		ret = armada_thermal_probe_syscon(pdev, priv);
> +		if (ret)
> +			return ret;
>  
> +		priv->data->init(pdev, priv);
> +
> +		tz = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
> +						  &legacy_ops, NULL, 0, 0);
> +		if (IS_ERR(tz)) {
> +			dev_err(&pdev->dev,
> +				"Failed to register thermal zone device\n");
> +			return PTR_ERR(tz);
> +		}
> +
> +		drvdata->type = LEGACY;
> +		drvdata->data.tz = tz;
> +		platform_set_drvdata(pdev, drvdata);
> +
> +		return 0;
> +	}
> +
> +	ret = armada_thermal_probe_syscon(pdev, priv);
>  	if (ret)
>  		return ret;
>  
>  	priv->data->init(pdev, priv);
> +	drvdata->type = SYSCON;
> +	drvdata->data.priv = priv;
> +	platform_set_drvdata(pdev, drvdata);
>  
> -	thermal = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
> -					       &ops, NULL, 0, 0);
> -	if (IS_ERR(thermal)) {
> +	sensors = devm_kzalloc(&pdev->dev, sizeof(struct armada_thermal_sensor),
> +			       GFP_KERNEL);
> +	if (!sensors)
> +		return -ENOMEM;
> +
> +	sensors->priv = priv;
> +
> +	tz = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, sensors,
> +						  &of_ops);
> +	if (IS_ERR(tz)) {
>  		dev_err(&pdev->dev,
> -			"Failed to register thermal zone device\n");
> -		return PTR_ERR(thermal);
> +			"Failed to register thermal sensor (err: %ld)\n",
> +			PTR_ERR(tz));
> +		return PTR_ERR(tz);
>  	}
>  
> -	platform_set_drvdata(pdev, thermal);
> -
>  	return 0;
>  }
>  
>  static int armada_thermal_exit(struct platform_device *pdev)
>  {
> -	struct thermal_zone_device *armada_thermal =
> -		platform_get_drvdata(pdev);
> +	struct armada_drvdata *drvdata = platform_get_drvdata(pdev);
>  
> -	thermal_zone_device_unregister(armada_thermal);
> +	if (drvdata->type == LEGACY)
> +		thermal_zone_device_unregister(drvdata->data.tz);
>  
>  	return 0;
>  }
> -- 
> 2.14.1
> 

Re: [PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Eduardo Valentin]

On Thu, Jul 26, 2018 at 02:35:18PM -0700, Eduardo Valentin wrote:
> On Thu, Jul 26, 2018 at 02:14:43PM -0700, Eduardo Valentin wrote:
> > Hello,
> > 
> > On Thu, Jul 26, 2018 at 10:00:41AM +0800, Zhang Rui wrote:
> > > On 三, 2018-07-25 at 09:52 +0200, Miquel Raynal wrote:
> > > > Hi Eduardo, Zhang,
> > > > 
> > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
> > > > 17:28:49 +0200:
> > > > 
> > > > > 
> > > > > Hi Eduardo,
> > > > > 
> > > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> > > > > 16:41:43 +0200:
> > > > > 
> > > > > > 
> > > > > > The only capability of the Armada thermal driver is currently
> > > > > > just to
> > > > > > read one sensor (the default one) per AP and one per CP.
> > > > > > Actually,
> > > > > > there is one sensor per core in the AP806 plus one sensor in the
> > > > > > thermal IP itself. The CP110 just features one thermal sensor in
> > > > > > its
> > > > > > own thermal IP.
> > > > > > 
> > > > > > This series first improves the readability of this driver, then
> > > > > > adds
> > > > > > support for multi-channel thermal IPs. The bindings and the
> > > > > > device-trees are updated accordingly.
> > > > > > 
> > > > > > Thank you,
> > > > > > Miquèl  
> > > > > I know I sent this series on monday but there was almost no change
> > > > > on
> > > > > it (see below), do you think you will have the time to look at it
> > > > > before it's too late for the merge window?
> > > > I know a little bit what it is to be on the maintainer side of a
> > > > subsystem but these changes are laying since April, 21st and I really
> > > > expect to get them merged. Zhang, as Eduardo seems to be overloaded
> > > > this release, could you please take the series?
> > > > 
> > > > Of course if there are things to address I'll be happy to do so.
> > > > 
> > > I had a sync with Eduardo during last merge window, and we agreed to
> > > send separate git pull requests to Linus since this release and see how
> > > it works.
> > > This means that all the soc thermal driver patches should go to
> > > Eduardo' tree and then Linus' tree directly.
> > > 
> > > Anyway, let's ping Eduardo and see if he can review/take the patches.
> > 
> > Sorry fellows, but I was off camping in a place with no electricity/no
> > internet. I looked at this series before, and I do not remember anything
> > that could block it to go. Let me restart on working on it.
> 
> Let me take that back. I got some compilation error.


Given that I let this one fall into the cracks and that these are minor
issues, I fixed them manually, and applied up to patch 19. But next at
least compile test your series and run checkpatch on them.

Patches 20-23 need to go via your arch tree.

> 
> > 
> > > 
> > > thanks,
> > > rui
> > > > Thanks,
> > > > Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Eduardo Valentin]

On Thu, Jul 26, 2018 at 02:35:18PM -0700, Eduardo Valentin wrote:
> On Thu, Jul 26, 2018 at 02:14:43PM -0700, Eduardo Valentin wrote:
> > Hello,
> > 
> > On Thu, Jul 26, 2018 at 10:00:41AM +0800, Zhang Rui wrote:
> > > On ?, 2018-07-25 at 09:52 +0200, Miquel Raynal wrote:
> > > > Hi Eduardo, Zhang,
> > > > 
> > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
> > > > 17:28:49 +0200:
> > > > 
> > > > > 
> > > > > Hi Eduardo,
> > > > > 
> > > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> > > > > 16:41:43 +0200:
> > > > > 
> > > > > > 
> > > > > > The only capability of the Armada thermal driver is currently
> > > > > > just to
> > > > > > read one sensor (the default one) per AP and one per CP.
> > > > > > Actually,
> > > > > > there is one sensor per core in the AP806 plus one sensor in the
> > > > > > thermal IP itself. The CP110 just features one thermal sensor in
> > > > > > its
> > > > > > own thermal IP.
> > > > > > 
> > > > > > This series first improves the readability of this driver, then
> > > > > > adds
> > > > > > support for multi-channel thermal IPs. The bindings and the
> > > > > > device-trees are updated accordingly.
> > > > > > 
> > > > > > Thank you,
> > > > > > Miqu?l??
> > > > > I know I sent this series on monday but there was almost no change
> > > > > on
> > > > > it (see below), do you think you will have the time to look at it
> > > > > before it's too late for the merge window?
> > > > I know a little bit what it is to be on the maintainer side of a
> > > > subsystem but these changes are laying since April, 21st and I really
> > > > expect to get them merged. Zhang, as Eduardo seems to be overloaded
> > > > this release, could you please take the series?
> > > > 
> > > > Of course if there are things to address I'll be happy to do so.
> > > > 
> > > I had a sync with Eduardo during last merge window, and we agreed to
> > > send separate git pull requests to Linus since this release and see how
> > > it works.
> > > This means that all the soc thermal driver patches should go to
> > > Eduardo' tree and then Linus' tree directly.
> > > 
> > > Anyway, let's ping Eduardo and see if he can review/take the patches.
> > 
> > Sorry fellows, but I was off camping in a place with no electricity/no
> > internet. I looked at this series before, and I do not remember anything
> > that could block it to go. Let me restart on working on it.
> 
> Let me take that back. I got some compilation error.


Given that I let this one fall into the cracks and that these are minor
issues, I fixed them manually, and applied up to patch 19. But next at
least compile test your series and run checkpatch on them.

Patches 20-23 need to go via your arch tree.

> 
> > 
> > > 
> > > thanks,
> > > rui
> > > > Thanks,
> > > > Miqu?l

Re: [PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Miquel Raynal]

Hi Eduardo,

Eduardo Valentin <edubezval@gmail.com> wrote on Thu, 26 Jul 2018
15:14:24 -0700:

> On Thu, Jul 26, 2018 at 02:35:18PM -0700, Eduardo Valentin wrote:
> > On Thu, Jul 26, 2018 at 02:14:43PM -0700, Eduardo Valentin wrote:  
> > > Hello,
> > > 
> > > On Thu, Jul 26, 2018 at 10:00:41AM +0800, Zhang Rui wrote:  
> > > > On 三, 2018-07-25 at 09:52 +0200, Miquel Raynal wrote:  
> > > > > Hi Eduardo, Zhang,
> > > > > 
> > > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
> > > > > 17:28:49 +0200:
> > > > >   
> > > > > > 
> > > > > > Hi Eduardo,
> > > > > > 
> > > > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> > > > > > 16:41:43 +0200:
> > > > > >   
> > > > > > > 
> > > > > > > The only capability of the Armada thermal driver is currently
> > > > > > > just to
> > > > > > > read one sensor (the default one) per AP and one per CP.
> > > > > > > Actually,
> > > > > > > there is one sensor per core in the AP806 plus one sensor in the
> > > > > > > thermal IP itself. The CP110 just features one thermal sensor in
> > > > > > > its
> > > > > > > own thermal IP.
> > > > > > > 
> > > > > > > This series first improves the readability of this driver, then
> > > > > > > adds
> > > > > > > support for multi-channel thermal IPs. The bindings and the
> > > > > > > device-trees are updated accordingly.
> > > > > > > 
> > > > > > > Thank you,
> > > > > > > Miquèl    
> > > > > > I know I sent this series on monday but there was almost no change
> > > > > > on
> > > > > > it (see below), do you think you will have the time to look at it
> > > > > > before it's too late for the merge window?  
> > > > > I know a little bit what it is to be on the maintainer side of a
> > > > > subsystem but these changes are laying since April, 21st and I really
> > > > > expect to get them merged. Zhang, as Eduardo seems to be overloaded
> > > > > this release, could you please take the series?
> > > > > 
> > > > > Of course if there are things to address I'll be happy to do so.
> > > > >   
> > > > I had a sync with Eduardo during last merge window, and we agreed to
> > > > send separate git pull requests to Linus since this release and see how
> > > > it works.
> > > > This means that all the soc thermal driver patches should go to
> > > > Eduardo' tree and then Linus' tree directly.
> > > > 
> > > > Anyway, let's ping Eduardo and see if he can review/take the patches.  
> > > 
> > > Sorry fellows, but I was off camping in a place with no electricity/no
> > > internet. I looked at this series before, and I do not remember anything
> > > that could block it to go. Let me restart on working on it.  

No problem.

> > 
> > Let me take that back. I got some compilation error.  
> 
> 
> Given that I let this one fall into the cracks and that these are minor
> issues, I fixed them manually, and applied up to patch 19. But next at
> least compile test your series and run checkpatch on them.

I was really surprised of your remark but indeed there was a build
issue in the middle of the series that I did not see because I always
test with a last patch fixing it (this patch adds interrupt support, I
plan to send it for the next release). I'm very sorry for this
mistake, thanks for fixing it by yourself.

> 
> Patches 20-23 need to go via your arch tree.

Sure, they will enter in the next release now.

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Miquel Raynal]

Hi Eduardo,

Eduardo Valentin <edubezval@gmail.com> wrote on Thu, 26 Jul 2018
15:14:24 -0700:

> On Thu, Jul 26, 2018 at 02:35:18PM -0700, Eduardo Valentin wrote:
> > On Thu, Jul 26, 2018 at 02:14:43PM -0700, Eduardo Valentin wrote:  
> > > Hello,
> > > 
> > > On Thu, Jul 26, 2018 at 10:00:41AM +0800, Zhang Rui wrote:  
> > > > On ?, 2018-07-25 at 09:52 +0200, Miquel Raynal wrote:  
> > > > > Hi Eduardo, Zhang,
> > > > > 
> > > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 18 Jul 2018
> > > > > 17:28:49 +0200:
> > > > >   
> > > > > > 
> > > > > > Hi Eduardo,
> > > > > > 
> > > > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 16 Jul 2018
> > > > > > 16:41:43 +0200:
> > > > > >   
> > > > > > > 
> > > > > > > The only capability of the Armada thermal driver is currently
> > > > > > > just to
> > > > > > > read one sensor (the default one) per AP and one per CP.
> > > > > > > Actually,
> > > > > > > there is one sensor per core in the AP806 plus one sensor in the
> > > > > > > thermal IP itself. The CP110 just features one thermal sensor in
> > > > > > > its
> > > > > > > own thermal IP.
> > > > > > > 
> > > > > > > This series first improves the readability of this driver, then
> > > > > > > adds
> > > > > > > support for multi-channel thermal IPs. The bindings and the
> > > > > > > device-trees are updated accordingly.
> > > > > > > 
> > > > > > > Thank you,
> > > > > > > Miqu?l??  
> > > > > > I know I sent this series on monday but there was almost no change
> > > > > > on
> > > > > > it (see below), do you think you will have the time to look at it
> > > > > > before it's too late for the merge window?  
> > > > > I know a little bit what it is to be on the maintainer side of a
> > > > > subsystem but these changes are laying since April, 21st and I really
> > > > > expect to get them merged. Zhang, as Eduardo seems to be overloaded
> > > > > this release, could you please take the series?
> > > > > 
> > > > > Of course if there are things to address I'll be happy to do so.
> > > > >   
> > > > I had a sync with Eduardo during last merge window, and we agreed to
> > > > send separate git pull requests to Linus since this release and see how
> > > > it works.
> > > > This means that all the soc thermal driver patches should go to
> > > > Eduardo' tree and then Linus' tree directly.
> > > > 
> > > > Anyway, let's ping Eduardo and see if he can review/take the patches.  
> > > 
> > > Sorry fellows, but I was off camping in a place with no electricity/no
> > > internet. I looked at this series before, and I do not remember anything
> > > that could block it to go. Let me restart on working on it.  

No problem.

> > 
> > Let me take that back. I got some compilation error.  
> 
> 
> Given that I let this one fall into the cracks and that these are minor
> issues, I fixed them manually, and applied up to patch 19. But next at
> least compile test your series and run checkpatch on them.

I was really surprised of your remark but indeed there was a build
issue in the middle of the series that I did not see because I always
test with a last patch fixing it (this patch adds interrupt support, I
plan to send it for the next release). I'm very sorry for this
mistake, thanks for fixing it by yourself.

> 
> Patches 20-23 need to go via your arch tree.

Sure, they will enter in the next release now.

Thanks,
Miqu?l

Re: [PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> Thermal zone names must follow certain rules imposed by the framework.
> They are limited in length and shall not have any hyphen '-'.
> 
> This is done in a separate function for future use in another location.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Why do you have to provide a function to test that?

Logically, the one who did the change to add a thermal name, should
check its code works. Without a proper name that won't work.

So this function is testing something which should be already tested, no?



> ---
>  drivers/thermal/armada_thermal.c | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 4c275ec10ac5..077e8e562306 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -70,6 +70,7 @@ struct armada_thermal_priv {
>  	void __iomem *status;
>  	void __iomem *control0;
>  	void __iomem *control1;
> +	char zone_name[THERMAL_NAME_LENGTH];
>  	struct armada_thermal_data *data;
>  };
>  
> @@ -353,6 +354,36 @@ static const struct of_device_id armada_thermal_id_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, armada_thermal_id_table);
>  
> +static void armada_set_sane_name(struct platform_device *pdev,
> +				 struct armada_thermal_priv *priv)
> +{
> +	const char *name = dev_name(&pdev->dev);
> +	char *insane_char;
> +
> +	if (strlen(name) > THERMAL_NAME_LENGTH) {
> +		/*
> +		 * When inside a system controller, the device name has the
> +		 * form: f06f8000.system-controller:ap-thermal so stripping
> +		 * after the ':' should give us a shorter but meaningful name.
> +		 */
> +		name = strrchr(name, ':');
> +		if (!name)
> +			name = "armada_thermal";
> +		else
> +			name++;
> +	}
> +
> +	/* Save the name locally */
> +	strncpy(priv->zone_name, name, THERMAL_NAME_LENGTH);
> +
> +	/* Then check there are no '-' or hwmon core will complain */
> +	do {
> +		insane_char = strpbrk(priv->zone_name, "-");
> +		if (insane_char)
> +			*insane_char = '_';
> +	} while (insane_char);
> +}
> +
>  static int armada_thermal_probe(struct platform_device *pdev)
>  {
>  	void __iomem *control = NULL;
> @@ -381,6 +412,9 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  
>  	priv->data = (struct armada_thermal_data *)match->data;
>  
> +	/* Ensure device name is correct for the thermal core */
> +	armada_set_sane_name(pdev, priv);
> +
>  	/*
>  	 * Legacy DT bindings only described "control1" register (also referred
>  	 * as "control MSB" on old documentation). New bindings cover
> @@ -402,7 +436,7 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  
>  	priv->data->init_sensor(pdev, priv);
>  
> -	thermal = thermal_zone_device_register(dev_name(&pdev->dev), 0, 0, priv,
> +	thermal = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
>  					       &ops, NULL, 0, 0);
>  	if (IS_ERR(thermal)) {
>  		dev_err(&pdev->dev,
> 


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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> Thermal zone names must follow certain rules imposed by the framework.
> They are limited in length and shall not have any hyphen '-'.
> 
> This is done in a separate function for future use in another location.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Why do you have to provide a function to test that?

Logically, the one who did the change to add a thermal name, should
check its code works. Without a proper name that won't work.

So this function is testing something which should be already tested, no?



> ---
>  drivers/thermal/armada_thermal.c | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 4c275ec10ac5..077e8e562306 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -70,6 +70,7 @@ struct armada_thermal_priv {
>  	void __iomem *status;
>  	void __iomem *control0;
>  	void __iomem *control1;
> +	char zone_name[THERMAL_NAME_LENGTH];
>  	struct armada_thermal_data *data;
>  };
>  
> @@ -353,6 +354,36 @@ static const struct of_device_id armada_thermal_id_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, armada_thermal_id_table);
>  
> +static void armada_set_sane_name(struct platform_device *pdev,
> +				 struct armada_thermal_priv *priv)
> +{
> +	const char *name = dev_name(&pdev->dev);
> +	char *insane_char;
> +
> +	if (strlen(name) > THERMAL_NAME_LENGTH) {
> +		/*
> +		 * When inside a system controller, the device name has the
> +		 * form: f06f8000.system-controller:ap-thermal so stripping
> +		 * after the ':' should give us a shorter but meaningful name.
> +		 */
> +		name = strrchr(name, ':');
> +		if (!name)
> +			name = "armada_thermal";
> +		else
> +			name++;
> +	}
> +
> +	/* Save the name locally */
> +	strncpy(priv->zone_name, name, THERMAL_NAME_LENGTH);
> +
> +	/* Then check there are no '-' or hwmon core will complain */
> +	do {
> +		insane_char = strpbrk(priv->zone_name, "-");
> +		if (insane_char)
> +			*insane_char = '_';
> +	} while (insane_char);
> +}
> +
>  static int armada_thermal_probe(struct platform_device *pdev)
>  {
>  	void __iomem *control = NULL;
> @@ -381,6 +412,9 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  
>  	priv->data = (struct armada_thermal_data *)match->data;
>  
> +	/* Ensure device name is correct for the thermal core */
> +	armada_set_sane_name(pdev, priv);
> +
>  	/*
>  	 * Legacy DT bindings only described "control1" register (also referred
>  	 * as "control MSB" on old documentation). New bindings cover
> @@ -402,7 +436,7 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  
>  	priv->data->init_sensor(pdev, priv);
>  
> -	thermal = thermal_zone_device_register(dev_name(&pdev->dev), 0, 0, priv,
> +	thermal = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
>  					       &ops, NULL, 0, 0);
>  	if (IS_ERR(thermal)) {
>  		dev_err(&pdev->dev,
> 


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

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

Re: [PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Miquel Raynal]

Hi Daniel,

Daniel Lezcano <daniel.lezcano@linaro.org> wrote on Fri, 27 Jul 2018
13:34:19 +0200:

> On 16/07/2018 16:41, Miquel Raynal wrote:
> > Thermal zone names must follow certain rules imposed by the framework.
> > They are limited in length and shall not have any hyphen '-'.
> > 
> > This is done in a separate function for future use in another location.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> 
> Why do you have to provide a function to test that?
> 
> Logically, the one who did the change to add a thermal name, should
> check its code works. Without a proper name that won't work.

What do you mean "the one who did the change"?
I think the thermal core should not care that much to what is given as
name and should probably not be so strict.

Also, I don't choose what dev_name() returns, it's in the device tree
and the device tree do not care about the implementation, it's just a
descriptive file.

> 
> So this function is testing something which should be already tested, no?

I don't think it is. Without this function the probe will simply fail.

The explanation of what fails is in the code:

> > +		/*
> > +		 * When inside a system controller, the device name has the
> > +		 * form: f06f8000.system-controller:ap-thermal so stripping
> > +		 * after the ':' should give us a shorter but meaningful name.
> > +		 */
> > +		name = strrchr(name, ':');
> > +		if (!name)
> > +			name = "armada_thermal";
> > +		else
> > +			name++;

[...]

> > +
> > +	/* Then check there are no '-' or hwmon core will complain */

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Miquel Raynal]

Hi Daniel,

Daniel Lezcano <daniel.lezcano@linaro.org> wrote on Fri, 27 Jul 2018
13:34:19 +0200:

> On 16/07/2018 16:41, Miquel Raynal wrote:
> > Thermal zone names must follow certain rules imposed by the framework.
> > They are limited in length and shall not have any hyphen '-'.
> > 
> > This is done in a separate function for future use in another location.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> 
> Why do you have to provide a function to test that?
> 
> Logically, the one who did the change to add a thermal name, should
> check its code works. Without a proper name that won't work.

What do you mean "the one who did the change"?
I think the thermal core should not care that much to what is given as
name and should probably not be so strict.

Also, I don't choose what dev_name() returns, it's in the device tree
and the device tree do not care about the implementation, it's just a
descriptive file.

> 
> So this function is testing something which should be already tested, no?

I don't think it is. Without this function the probe will simply fail.

The explanation of what fails is in the code:

> > +		/*
> > +		 * When inside a system controller, the device name has the
> > +		 * form: f06f8000.system-controller:ap-thermal so stripping
> > +		 * after the ':' should give us a shorter but meaningful name.
> > +		 */
> > +		name = strrchr(name, ':');
> > +		if (!name)
> > +			name = "armada_thermal";
> > +		else
> > +			name++;

[...]

> > +
> > +	/* Then check there are no '-' or hwmon core will complain */

Thanks,
Miqu?l

Re: [PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Daniel Lezcano]

On 27/07/2018 13:52, Miquel Raynal wrote:
> Hi Daniel,
> 
> Daniel Lezcano <daniel.lezcano@linaro.org> wrote on Fri, 27 Jul 2018
> 13:34:19 +0200:
> 
>> On 16/07/2018 16:41, Miquel Raynal wrote:
>>> Thermal zone names must follow certain rules imposed by the framework.
>>> They are limited in length and shall not have any hyphen '-'.
>>>
>>> This is done in a separate function for future use in another location.
>>>
>>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
>>
>> Why do you have to provide a function to test that?
>>
>> Logically, the one who did the change to add a thermal name, should
>> check its code works. Without a proper name that won't work.
> 
> What do you mean "the one who did the change"?
> I think the thermal core should not care that much to what is given as
> name and should probably not be so strict.
> 
> Also, I don't choose what dev_name() returns, it's in the device tree
> and the device tree do not care about the implementation, it's just a
> descriptive file.
> 
>>
>> So this function is testing something which should be already tested, no?
> 
> I don't think it is. Without this function the probe will simply fail.
> 
> The explanation of what fails is in the code:
> 
>>> +		/*
>>> +		 * When inside a system controller, the device name has the
>>> +		 * form: f06f8000.system-controller:ap-thermal so stripping
>>> +		 * after the ':' should give us a shorter but meaningful name.
>>> +		 */
>>> +		name = strrchr(name, ':');
>>> +		if (!name)
>>> +			name = "armada_thermal";
>>> +		else
>>> +			name++;
> 
> [...]

I'm not in favor of this, potentially it can introduce a break which can
be exploited later.

You are creating the thermal zones manually from the driver but want to
rely on the temperature controller name to give a name to the thermal zone.

Why not create the thermal zone in the DT with the correct name and use
devm_thermal_zone_of_sensor_register ?

Or alternatively hardcode the thermal zone name ?







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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Daniel Lezcano]

On 27/07/2018 13:52, Miquel Raynal wrote:
> Hi Daniel,
> 
> Daniel Lezcano <daniel.lezcano@linaro.org> wrote on Fri, 27 Jul 2018
> 13:34:19 +0200:
> 
>> On 16/07/2018 16:41, Miquel Raynal wrote:
>>> Thermal zone names must follow certain rules imposed by the framework.
>>> They are limited in length and shall not have any hyphen '-'.
>>>
>>> This is done in a separate function for future use in another location.
>>>
>>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
>>
>> Why do you have to provide a function to test that?
>>
>> Logically, the one who did the change to add a thermal name, should
>> check its code works. Without a proper name that won't work.
> 
> What do you mean "the one who did the change"?
> I think the thermal core should not care that much to what is given as
> name and should probably not be so strict.
> 
> Also, I don't choose what dev_name() returns, it's in the device tree
> and the device tree do not care about the implementation, it's just a
> descriptive file.
> 
>>
>> So this function is testing something which should be already tested, no?
> 
> I don't think it is. Without this function the probe will simply fail.
> 
> The explanation of what fails is in the code:
> 
>>> +		/*
>>> +		 * When inside a system controller, the device name has the
>>> +		 * form: f06f8000.system-controller:ap-thermal so stripping
>>> +		 * after the ':' should give us a shorter but meaningful name.
>>> +		 */
>>> +		name = strrchr(name, ':');
>>> +		if (!name)
>>> +			name = "armada_thermal";
>>> +		else
>>> +			name++;
> 
> [...]

I'm not in favor of this, potentially it can introduce a break which can
be exploited later.

You are creating the thermal zones manually from the driver but want to
rely on the temperature controller name to give a name to the thermal zone.

Why not create the thermal zone in the DT with the correct name and use
devm_thermal_zone_of_sensor_register ?

Or alternatively hardcode the thermal zone name ?







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

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

Re: [PATCH v3 02/23] thermal: armada: remove useless register accesses

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> Prepare the migration to use regmaps by first simplifying the
> initialization functions: avoid unnecessary write/read cycles on
> configuration registers.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Good catch. I'm wondering why it was done this way ...

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>  drivers/thermal/armada_thermal.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 077e8e562306..6fdb90b3c001 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -103,16 +103,13 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
>  
>  	reg = readl_relaxed(priv->control1);
>  	reg |= PMU_TDC0_OTF_CAL_MASK;
> -	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->control1);
>  
>  	/* Reset the sensor */
> -	reg = readl_relaxed(priv->control1);
> -	writel((reg | PMU_TDC0_SW_RST_MASK), priv->control1);
> +	reg |= PMU_TDC0_SW_RST_MASK;
>  
>  	writel(reg, priv->control1);
>  
> @@ -129,14 +126,13 @@ static void armada370_init_sensor(struct platform_device *pdev,
>  
>  	reg = readl_relaxed(priv->control1);
>  	reg |= PMU_TDC0_OTF_CAL_MASK;
> -	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->control1);
>  
>  	reg &= ~PMU_TDC0_START_CAL_MASK;
> +
>  	writel(reg, priv->control1);
>  
>  	msleep(10);
> 


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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 02/23] thermal: armada: remove useless register accesses

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> Prepare the migration to use regmaps by first simplifying the
> initialization functions: avoid unnecessary write/read cycles on
> configuration registers.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Good catch. I'm wondering why it was done this way ...

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>  drivers/thermal/armada_thermal.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 077e8e562306..6fdb90b3c001 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -103,16 +103,13 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
>  
>  	reg = readl_relaxed(priv->control1);
>  	reg |= PMU_TDC0_OTF_CAL_MASK;
> -	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->control1);
>  
>  	/* Reset the sensor */
> -	reg = readl_relaxed(priv->control1);
> -	writel((reg | PMU_TDC0_SW_RST_MASK), priv->control1);
> +	reg |= PMU_TDC0_SW_RST_MASK;
>  
>  	writel(reg, priv->control1);
>  
> @@ -129,14 +126,13 @@ static void armada370_init_sensor(struct platform_device *pdev,
>  
>  	reg = readl_relaxed(priv->control1);
>  	reg |= PMU_TDC0_OTF_CAL_MASK;
> -	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->control1);
>  
>  	reg &= ~PMU_TDC0_START_CAL_MASK;
> +
>  	writel(reg, priv->control1);
>  
>  	msleep(10);
> 


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

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

Re: [PATCH v3 04/23] thermal: armada: rename the initialization routine

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> Calling a hook ->init_sensor() while what is initialized is the IP
> itself and not the sensors is misleading. Rename the hook ->init() to
> avoid any confusion in later work bringing multi-sensors support.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>


Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>


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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 04/23] thermal: armada: rename the initialization routine

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> Calling a hook ->init_sensor() while what is initialized is the IP
> itself and not the sensors is misleading. Rename the hook ->init() to
> avoid any confusion in later work bringing multi-sensors support.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>


Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>


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

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

Re: [PATCH v3 03/23] thermal: armada: remove misleading comments

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> On older versions of this thermal IP, TSEN referred as the internal
> sensor in the thermal IP while EXT_TSEN referred as sensors outside of
> this IP, ie in the CPUs most of the time. The bit names in the
> specifications do not follow this rule anymore, so remove these comments
> that are misleading.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 03/23] thermal: armada: remove misleading comments

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> On older versions of this thermal IP, TSEN referred as the internal
> sensor in the thermal IP while EXT_TSEN referred as sensors outside of
> this IP, ie in the CPUs most of the time. The bit names in the
> specifications do not follow this rule anymore, so remove these comments
> that are misleading.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

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

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

Re: [PATCH v3 05/23] thermal: armada: dissociate a380 and cp110 ->init() hooks

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> Until now, Armada 380 and CP110 could share the same ->init() function
> because their use was identical.
> 
> Prepare the support of multi-sensors support and overheat interrupt
> feature by separating the initialization paths before they actually
> diverge.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>


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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 05/23] thermal: armada: dissociate a380 and cp110 ->init() hooks

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> Until now, Armada 380 and CP110 could share the same ->init() function
> because their use was identical.
> 
> Prepare the support of multi-sensors support and overheat interrupt
> feature by separating the initialization paths before they actually
> diverge.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>


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

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

Re: [PATCH v3 02/23] thermal: armada: remove useless register accesses

[Ezequiel Garcia]

Hi Miquel,

You know, it is usually useful to Cc the author of the lines a patch touches :-)

On 16 July 2018 at 11:41, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Prepare the migration to use regmaps by first simplifying the
> initialization functions: avoid unnecessary write/read cycles on
> configuration registers.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/thermal/armada_thermal.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 077e8e562306..6fdb90b3c001 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -103,16 +103,13 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
>
>         reg = readl_relaxed(priv->control1);
>         reg |= PMU_TDC0_OTF_CAL_MASK;
> -       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->control1);
>
>         /* Reset the sensor */
> -       reg = readl_relaxed(priv->control1);
> -       writel((reg | PMU_TDC0_SW_RST_MASK), priv->control1);
> +       reg |= PMU_TDC0_SW_RST_MASK;
>
>         writel(reg, priv->control1);
>
> @@ -129,14 +126,13 @@ static void armada370_init_sensor(struct platform_device *pdev,
>
>         reg = readl_relaxed(priv->control1);
>         reg |= PMU_TDC0_OTF_CAL_MASK;
> -       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->control1);
>
>         reg &= ~PMU_TDC0_START_CAL_MASK;
> +
>         writel(reg, priv->control1);
>

IIRC, the documentation for this hardware block was a meager.

Are you sure there isn't any requirement for the calibration
start to be issued separately or something like that?
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 02/23] thermal: armada: remove useless register accesses

[Ezequiel Garcia]

Hi Miquel,

You know, it is usually useful to Cc the author of the lines a patch touches :-)

On 16 July 2018 at 11:41, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Prepare the migration to use regmaps by first simplifying the
> initialization functions: avoid unnecessary write/read cycles on
> configuration registers.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/thermal/armada_thermal.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 077e8e562306..6fdb90b3c001 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -103,16 +103,13 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
>
>         reg = readl_relaxed(priv->control1);
>         reg |= PMU_TDC0_OTF_CAL_MASK;
> -       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->control1);
>
>         /* Reset the sensor */
> -       reg = readl_relaxed(priv->control1);
> -       writel((reg | PMU_TDC0_SW_RST_MASK), priv->control1);
> +       reg |= PMU_TDC0_SW_RST_MASK;
>
>         writel(reg, priv->control1);
>
> @@ -129,14 +126,13 @@ static void armada370_init_sensor(struct platform_device *pdev,
>
>         reg = readl_relaxed(priv->control1);
>         reg |= PMU_TDC0_OTF_CAL_MASK;
> -       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->control1);
>
>         reg &= ~PMU_TDC0_START_CAL_MASK;
> +
>         writel(reg, priv->control1);
>

IIRC, the documentation for this hardware block was a meager.

Are you sure there isn't any requirement for the calibration
start to be issued separately or something like that?
-- 
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH v3 06/23] thermal: armada: average over samples to avoid glitches

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> Configure the sample frequency and number of averaged samples.
> 
> This is needed for two reasons:
> 1/ To be bootloader independent.
> 2/ To prepare the introduction of multi-sensors support by preventing
>    inconsistencies when reading temperatures that could be a mean of
>    samples took from different sensors.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/thermal/armada_thermal.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 9291ea3ad2f7..1f9706d96a0d 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -54,7 +54,12 @@
>  #define CONTROL0_TSEN_START		BIT(0)
>  #define CONTROL0_TSEN_RESET		BIT(1)
>  #define CONTROL0_TSEN_ENABLE		BIT(2)
> +#define CONTROL0_TSEN_AVG_BYPASS	BIT(6)
> +#define CONTROL0_TSEN_OSR_SHIFT		24
> +#define CONTROL0_TSEN_OSR_MAX		0x3
>  
> +#define CONTROL1_TSEN_AVG_SHIFT		0

Why shift by zero ?



> +#define CONTROL1_TSEN_AVG_MASK		0x7
>  #define CONTROL1_EXT_TSEN_SW_RESET	BIT(7)
>  #define CONTROL1_EXT_TSEN_HW_RESETn	BIT(8)
>  
> @@ -194,6 +199,13 @@ static void armada_ap806_init(struct platform_device *pdev,
>  	reg = readl_relaxed(priv->control0);
>  	reg &= ~CONTROL0_TSEN_RESET;
>  	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
> +
> +	/* Sample every ~2ms */
> +	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
> +
> +	/* Enable average (2 samples by default) */
> +	reg &= ~CONTROL0_TSEN_AVG_BYPASS;
> +
>  	writel(reg, priv->control0);
>  
>  	/* Wait the sensors to be valid or the core will warn the user */
> @@ -203,7 +215,20 @@ static void armada_ap806_init(struct platform_device *pdev,
>  static void armada_cp110_init(struct platform_device *pdev,
>  			      struct armada_thermal_priv *priv)
>  {
> +	u32 reg;
> +
>  	armada380_init(pdev, priv);
> +
> +	/* Sample every ~2ms */
> +	reg = readl_relaxed(priv->control0);
> +	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
> +	writel(reg, priv->control0);
> +
> +	/* Average the output value over 2^1 = 2 samples */
> +	reg = readl_relaxed(priv->control1);
> +	reg &= ~CONTROL1_TSEN_AVG_MASK << CONTROL1_TSEN_AVG_SHIFT;
> +	reg |= 1 << CONTROL1_TSEN_AVG_SHIFT;
> +	writel(reg, priv->control1);
>  }
>  
>  static bool armada_is_valid(struct armada_thermal_priv *priv)
> 


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

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 06/23] thermal: armada: average over samples to avoid glitches

[Daniel Lezcano]

On 16/07/2018 16:41, Miquel Raynal wrote:
> Configure the sample frequency and number of averaged samples.
> 
> This is needed for two reasons:
> 1/ To be bootloader independent.
> 2/ To prepare the introduction of multi-sensors support by preventing
>    inconsistencies when reading temperatures that could be a mean of
>    samples took from different sensors.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/thermal/armada_thermal.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 9291ea3ad2f7..1f9706d96a0d 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -54,7 +54,12 @@
>  #define CONTROL0_TSEN_START		BIT(0)
>  #define CONTROL0_TSEN_RESET		BIT(1)
>  #define CONTROL0_TSEN_ENABLE		BIT(2)
> +#define CONTROL0_TSEN_AVG_BYPASS	BIT(6)
> +#define CONTROL0_TSEN_OSR_SHIFT		24
> +#define CONTROL0_TSEN_OSR_MAX		0x3
>  
> +#define CONTROL1_TSEN_AVG_SHIFT		0

Why shift by zero ?



> +#define CONTROL1_TSEN_AVG_MASK		0x7
>  #define CONTROL1_EXT_TSEN_SW_RESET	BIT(7)
>  #define CONTROL1_EXT_TSEN_HW_RESETn	BIT(8)
>  
> @@ -194,6 +199,13 @@ static void armada_ap806_init(struct platform_device *pdev,
>  	reg = readl_relaxed(priv->control0);
>  	reg &= ~CONTROL0_TSEN_RESET;
>  	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
> +
> +	/* Sample every ~2ms */
> +	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
> +
> +	/* Enable average (2 samples by default) */
> +	reg &= ~CONTROL0_TSEN_AVG_BYPASS;
> +
>  	writel(reg, priv->control0);
>  
>  	/* Wait the sensors to be valid or the core will warn the user */
> @@ -203,7 +215,20 @@ static void armada_ap806_init(struct platform_device *pdev,
>  static void armada_cp110_init(struct platform_device *pdev,
>  			      struct armada_thermal_priv *priv)
>  {
> +	u32 reg;
> +
>  	armada380_init(pdev, priv);
> +
> +	/* Sample every ~2ms */
> +	reg = readl_relaxed(priv->control0);
> +	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
> +	writel(reg, priv->control0);
> +
> +	/* Average the output value over 2^1 = 2 samples */
> +	reg = readl_relaxed(priv->control1);
> +	reg &= ~CONTROL1_TSEN_AVG_MASK << CONTROL1_TSEN_AVG_SHIFT;
> +	reg |= 1 << CONTROL1_TSEN_AVG_SHIFT;
> +	writel(reg, priv->control1);
>  }
>  
>  static bool armada_is_valid(struct armada_thermal_priv *priv)
> 


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

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

Re: [PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Eduardo Valentin]

On Fri, Jul 27, 2018 at 12:59:28AM +0200, Miquel Raynal wrote:
> Hi Eduardo,
> 
> Eduardo Valentin <edubezval@gmail.com> wrote on Thu, 26 Jul 2018
> 15:14:24 -0700:
> 
> > 
> > 
> > Given that I let this one fall into the cracks and that these are minor
> > issues, I fixed them manually, and applied up to patch 19. But next at
> > least compile test your series and run checkpatch on them.
> 
> I was really surprised of your remark but indeed there was a build
> issue in the middle of the series that I did not see because I always
> test with a last patch fixing it (this patch adds interrupt support, I
> plan to send it for the next release). I'm very sorry for this
> mistake, thanks for fixing it by yourself.

Oh, you cannot do that :-). You have to diligently test each patch.

> 
> > 
> > Patches 20-23 need to go via your arch tree.
> 
> Sure, they will enter in the next release now.
> 
> Thanks,
> Miquèl

[PATCH v3 00/23] Add multi-channel support to Armada thermal driver

[Eduardo Valentin]

On Fri, Jul 27, 2018 at 12:59:28AM +0200, Miquel Raynal wrote:
> Hi Eduardo,
> 
> Eduardo Valentin <edubezval@gmail.com> wrote on Thu, 26 Jul 2018
> 15:14:24 -0700:
> 
> > 
> > 
> > Given that I let this one fall into the cracks and that these are minor
> > issues, I fixed them manually, and applied up to patch 19. But next at
> > least compile test your series and run checkpatch on them.
> 
> I was really surprised of your remark but indeed there was a build
> issue in the middle of the series that I did not see because I always
> test with a last patch fixing it (this patch adds interrupt support, I
> plan to send it for the next release). I'm very sorry for this
> mistake, thanks for fixing it by yourself.

Oh, you cannot do that :-). You have to diligently test each patch.

> 
> > 
> > Patches 20-23 need to go via your arch tree.
> 
> Sure, they will enter in the next release now.
> 
> Thanks,
> Miqu?l

Re: [PATCH v3 02/23] thermal: armada: remove useless register accesses

[Miquel Raynal]

Hi Ezequiel,

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote on Fri, 27 Jul
2018 13:13:20 -0300:

> Hi Miquel,
> 
> You know, it is usually useful to Cc the author of the lines a patch touches :-)

I have to admit I limited the CC list to the get_maintainers.pl output.
But for sure I can add you for further changes :)

> 
> On 16 July 2018 at 11:41, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > Prepare the migration to use regmaps by first simplifying the
> > initialization functions: avoid unnecessary write/read cycles on
> > configuration registers.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/thermal/armada_thermal.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> > index 077e8e562306..6fdb90b3c001 100644
> > --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -103,16 +103,13 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
> >
> >         reg = readl_relaxed(priv->control1);
> >         reg |= PMU_TDC0_OTF_CAL_MASK;
> > -       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->control1);
> >
> >         /* Reset the sensor */
> > -       reg = readl_relaxed(priv->control1);
> > -       writel((reg | PMU_TDC0_SW_RST_MASK), priv->control1);
> > +       reg |= PMU_TDC0_SW_RST_MASK;
> >
> >         writel(reg, priv->control1);
> >
> > @@ -129,14 +126,13 @@ static void armada370_init_sensor(struct platform_device *pdev,
> >
> >         reg = readl_relaxed(priv->control1);
> >         reg |= PMU_TDC0_OTF_CAL_MASK;
> > -       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->control1);
> >
> >         reg &= ~PMU_TDC0_START_CAL_MASK;
> > +
> >         writel(reg, priv->control1);
> >  
> 
> IIRC, the documentation for this hardware block was a meager.
> 
> Are you sure there isn't any requirement for the calibration
> start to be issued separately or something like that?

I didn't find any info about this, I don't think it will be a problem
if my understanding of this IP is correct (had a lot of interactions
with Marvell engineers) but cannot be 100% sure, if someone has a
problem we'll know where to look at.

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 02/23] thermal: armada: remove useless register accesses

[Miquel Raynal]

Hi Ezequiel,

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote on Fri, 27 Jul
2018 13:13:20 -0300:

> Hi Miquel,
> 
> You know, it is usually useful to Cc the author of the lines a patch touches :-)

I have to admit I limited the CC list to the get_maintainers.pl output.
But for sure I can add you for further changes :)

> 
> On 16 July 2018 at 11:41, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > Prepare the migration to use regmaps by first simplifying the
> > initialization functions: avoid unnecessary write/read cycles on
> > configuration registers.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/thermal/armada_thermal.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> > index 077e8e562306..6fdb90b3c001 100644
> > --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -103,16 +103,13 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
> >
> >         reg = readl_relaxed(priv->control1);
> >         reg |= PMU_TDC0_OTF_CAL_MASK;
> > -       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->control1);
> >
> >         /* Reset the sensor */
> > -       reg = readl_relaxed(priv->control1);
> > -       writel((reg | PMU_TDC0_SW_RST_MASK), priv->control1);
> > +       reg |= PMU_TDC0_SW_RST_MASK;
> >
> >         writel(reg, priv->control1);
> >
> > @@ -129,14 +126,13 @@ static void armada370_init_sensor(struct platform_device *pdev,
> >
> >         reg = readl_relaxed(priv->control1);
> >         reg |= PMU_TDC0_OTF_CAL_MASK;
> > -       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->control1);
> >
> >         reg &= ~PMU_TDC0_START_CAL_MASK;
> > +
> >         writel(reg, priv->control1);
> >  
> 
> IIRC, the documentation for this hardware block was a meager.
> 
> Are you sure there isn't any requirement for the calibration
> start to be issued separately or something like that?

I didn't find any info about this, I don't think it will be a problem
if my understanding of this IP is correct (had a lot of interactions
with Marvell engineers) but cannot be 100% sure, if someone has a
problem we'll know where to look at.

Thanks,
Miqu?l

Re: [PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Miquel Raynal]

Hi Daniel,

Daniel Lezcano <daniel.lezcano@linaro.org> wrote on Fri, 27 Jul 2018
17:29:52 +0200:

> On 27/07/2018 13:52, Miquel Raynal wrote:
> > Hi Daniel,
> > 
> > Daniel Lezcano <daniel.lezcano@linaro.org> wrote on Fri, 27 Jul 2018
> > 13:34:19 +0200:
> >   
> >> On 16/07/2018 16:41, Miquel Raynal wrote:  
> >>> Thermal zone names must follow certain rules imposed by the framework.
> >>> They are limited in length and shall not have any hyphen '-'.
> >>>
> >>> This is done in a separate function for future use in another location.
> >>>
> >>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>    
> >>
> >> Why do you have to provide a function to test that?
> >>
> >> Logically, the one who did the change to add a thermal name, should
> >> check its code works. Without a proper name that won't work.  
> > 
> > What do you mean "the one who did the change"?
> > I think the thermal core should not care that much to what is given as
> > name and should probably not be so strict.
> > 
> > Also, I don't choose what dev_name() returns, it's in the device tree
> > and the device tree do not care about the implementation, it's just a
> > descriptive file.
> >   
> >>
> >> So this function is testing something which should be already tested, no?  
> > 
> > I don't think it is. Without this function the probe will simply fail.
> > 
> > The explanation of what fails is in the code:
> >   
> >>> +		/*
> >>> +		 * When inside a system controller, the device name has the
> >>> +		 * form: f06f8000.system-controller:ap-thermal so stripping
> >>> +		 * after the ':' should give us a shorter but meaningful name.
> >>> +		 */
> >>> +		name = strrchr(name, ':');
> >>> +		if (!name)
> >>> +			name = "armada_thermal";
> >>> +		else
> >>> +			name++;  
> > 
> > [...]  
> 
> I'm not in favor of this, potentially it can introduce a break which can
> be exploited later.

I know, I don't like this neither but that was the best way IMHO to
handle the core's limitations.

> 
> You are creating the thermal zones manually from the driver but want to
> rely on the temperature controller name to give a name to the thermal zone.
> 
> Why not create the thermal zone in the DT with the correct name and use
> devm_thermal_zone_of_sensor_register ?

That's exactly what is done later in the series actually, but for DT
backward compatibility and in order to continue adding feature to this
driver, we must ensure there won't be any drawback for people using old
DT with deprecated thermal representation.


Thanks for reviewing, if you don't mind I could copy you in future
changes? Unfortunately for this series it's already been applied and I
would like not to do any deep changes before it's merged, but for sure
this function could be enhanced.

Kind regards,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Miquel Raynal]

Hi Daniel,

Daniel Lezcano <daniel.lezcano@linaro.org> wrote on Fri, 27 Jul 2018
17:29:52 +0200:

> On 27/07/2018 13:52, Miquel Raynal wrote:
> > Hi Daniel,
> > 
> > Daniel Lezcano <daniel.lezcano@linaro.org> wrote on Fri, 27 Jul 2018
> > 13:34:19 +0200:
> >   
> >> On 16/07/2018 16:41, Miquel Raynal wrote:  
> >>> Thermal zone names must follow certain rules imposed by the framework.
> >>> They are limited in length and shall not have any hyphen '-'.
> >>>
> >>> This is done in a separate function for future use in another location.
> >>>
> >>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>    
> >>
> >> Why do you have to provide a function to test that?
> >>
> >> Logically, the one who did the change to add a thermal name, should
> >> check its code works. Without a proper name that won't work.  
> > 
> > What do you mean "the one who did the change"?
> > I think the thermal core should not care that much to what is given as
> > name and should probably not be so strict.
> > 
> > Also, I don't choose what dev_name() returns, it's in the device tree
> > and the device tree do not care about the implementation, it's just a
> > descriptive file.
> >   
> >>
> >> So this function is testing something which should be already tested, no?  
> > 
> > I don't think it is. Without this function the probe will simply fail.
> > 
> > The explanation of what fails is in the code:
> >   
> >>> +		/*
> >>> +		 * When inside a system controller, the device name has the
> >>> +		 * form: f06f8000.system-controller:ap-thermal so stripping
> >>> +		 * after the ':' should give us a shorter but meaningful name.
> >>> +		 */
> >>> +		name = strrchr(name, ':');
> >>> +		if (!name)
> >>> +			name = "armada_thermal";
> >>> +		else
> >>> +			name++;  
> > 
> > [...]  
> 
> I'm not in favor of this, potentially it can introduce a break which can
> be exploited later.

I know, I don't like this neither but that was the best way IMHO to
handle the core's limitations.

> 
> You are creating the thermal zones manually from the driver but want to
> rely on the temperature controller name to give a name to the thermal zone.
> 
> Why not create the thermal zone in the DT with the correct name and use
> devm_thermal_zone_of_sensor_register ?

That's exactly what is done later in the series actually, but for DT
backward compatibility and in order to continue adding feature to this
driver, we must ensure there won't be any drawback for people using old
DT with deprecated thermal representation.


Thanks for reviewing, if you don't mind I could copy you in future
changes? Unfortunately for this series it's already been applied and I
would like not to do any deep changes before it's merged, but for sure
this function could be enhanced.

Kind regards,
Miqu?l

Re: [PATCH v3 06/23] thermal: armada: average over samples to avoid glitches

[Miquel Raynal]

Hi Daniel,

Daniel Lezcano <daniel.lezcano@linaro.org> wrote on Fri, 27 Jul 2018
18:29:01 +0200:

> On 16/07/2018 16:41, Miquel Raynal wrote:
> > Configure the sample frequency and number of averaged samples.
> > 
> > This is needed for two reasons:
> > 1/ To be bootloader independent.
> > 2/ To prepare the introduction of multi-sensors support by preventing
> >    inconsistencies when reading temperatures that could be a mean of
> >    samples took from different sensors.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/thermal/armada_thermal.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> > index 9291ea3ad2f7..1f9706d96a0d 100644
> > --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -54,7 +54,12 @@
> >  #define CONTROL0_TSEN_START		BIT(0)
> >  #define CONTROL0_TSEN_RESET		BIT(1)
> >  #define CONTROL0_TSEN_ENABLE		BIT(2)
> > +#define CONTROL0_TSEN_AVG_BYPASS	BIT(6)
> > +#define CONTROL0_TSEN_OSR_SHIFT		24
> > +#define CONTROL0_TSEN_OSR_MAX		0x3
> >  
> > +#define CONTROL1_TSEN_AVG_SHIFT		0  
> 
> Why shift by zero ?
> 

I know some people do not like it, it's a matter of taste, as this IP
registers documentation is a bit fuzzy, I wanted to make it clear that
it was the first region in the CONTROL1 register. It's optimized out by
the compiler anyway.

> 
> 
> > +#define CONTROL1_TSEN_AVG_MASK		0x7
> >  #define CONTROL1_EXT_TSEN_SW_RESET	BIT(7)
> >  #define CONTROL1_EXT_TSEN_HW_RESETn	BIT(8)
> >  
> > @@ -194,6 +199,13 @@ static void armada_ap806_init(struct platform_device *pdev,
> >  	reg = readl_relaxed(priv->control0);
> >  	reg &= ~CONTROL0_TSEN_RESET;
> >  	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
> > +
> > +	/* Sample every ~2ms */
> > +	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
> > +
> > +	/* Enable average (2 samples by default) */
> > +	reg &= ~CONTROL0_TSEN_AVG_BYPASS;
> > +
> >  	writel(reg, priv->control0);
> >  
> >  	/* Wait the sensors to be valid or the core will warn the user */
> > @@ -203,7 +215,20 @@ static void armada_ap806_init(struct platform_device *pdev,
> >  static void armada_cp110_init(struct platform_device *pdev,
> >  			      struct armada_thermal_priv *priv)
> >  {
> > +	u32 reg;
> > +
> >  	armada380_init(pdev, priv);
> > +
> > +	/* Sample every ~2ms */
> > +	reg = readl_relaxed(priv->control0);
> > +	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
> > +	writel(reg, priv->control0);
> > +
> > +	/* Average the output value over 2^1 = 2 samples */
> > +	reg = readl_relaxed(priv->control1);
> > +	reg &= ~CONTROL1_TSEN_AVG_MASK << CONTROL1_TSEN_AVG_SHIFT;
> > +	reg |= 1 << CONTROL1_TSEN_AVG_SHIFT;
> > +	writel(reg, priv->control1);
> >  }
> >  
> >  static bool armada_is_valid(struct armada_thermal_priv *priv)
> >   
> 
> 

Thanks
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 06/23] thermal: armada: average over samples to avoid glitches

[Miquel Raynal]

Hi Daniel,

Daniel Lezcano <daniel.lezcano@linaro.org> wrote on Fri, 27 Jul 2018
18:29:01 +0200:

> On 16/07/2018 16:41, Miquel Raynal wrote:
> > Configure the sample frequency and number of averaged samples.
> > 
> > This is needed for two reasons:
> > 1/ To be bootloader independent.
> > 2/ To prepare the introduction of multi-sensors support by preventing
> >    inconsistencies when reading temperatures that could be a mean of
> >    samples took from different sensors.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/thermal/armada_thermal.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> > index 9291ea3ad2f7..1f9706d96a0d 100644
> > --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -54,7 +54,12 @@
> >  #define CONTROL0_TSEN_START		BIT(0)
> >  #define CONTROL0_TSEN_RESET		BIT(1)
> >  #define CONTROL0_TSEN_ENABLE		BIT(2)
> > +#define CONTROL0_TSEN_AVG_BYPASS	BIT(6)
> > +#define CONTROL0_TSEN_OSR_SHIFT		24
> > +#define CONTROL0_TSEN_OSR_MAX		0x3
> >  
> > +#define CONTROL1_TSEN_AVG_SHIFT		0  
> 
> Why shift by zero ?
> 

I know some people do not like it, it's a matter of taste, as this IP
registers documentation is a bit fuzzy, I wanted to make it clear that
it was the first region in the CONTROL1 register. It's optimized out by
the compiler anyway.

> 
> 
> > +#define CONTROL1_TSEN_AVG_MASK		0x7
> >  #define CONTROL1_EXT_TSEN_SW_RESET	BIT(7)
> >  #define CONTROL1_EXT_TSEN_HW_RESETn	BIT(8)
> >  
> > @@ -194,6 +199,13 @@ static void armada_ap806_init(struct platform_device *pdev,
> >  	reg = readl_relaxed(priv->control0);
> >  	reg &= ~CONTROL0_TSEN_RESET;
> >  	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
> > +
> > +	/* Sample every ~2ms */
> > +	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
> > +
> > +	/* Enable average (2 samples by default) */
> > +	reg &= ~CONTROL0_TSEN_AVG_BYPASS;
> > +
> >  	writel(reg, priv->control0);
> >  
> >  	/* Wait the sensors to be valid or the core will warn the user */
> > @@ -203,7 +215,20 @@ static void armada_ap806_init(struct platform_device *pdev,
> >  static void armada_cp110_init(struct platform_device *pdev,
> >  			      struct armada_thermal_priv *priv)
> >  {
> > +	u32 reg;
> > +
> >  	armada380_init(pdev, priv);
> > +
> > +	/* Sample every ~2ms */
> > +	reg = readl_relaxed(priv->control0);
> > +	reg |= CONTROL0_TSEN_OSR_MAX << CONTROL0_TSEN_OSR_SHIFT;
> > +	writel(reg, priv->control0);
> > +
> > +	/* Average the output value over 2^1 = 2 samples */
> > +	reg = readl_relaxed(priv->control1);
> > +	reg &= ~CONTROL1_TSEN_AVG_MASK << CONTROL1_TSEN_AVG_SHIFT;
> > +	reg |= 1 << CONTROL1_TSEN_AVG_SHIFT;
> > +	writel(reg, priv->control1);
> >  }
> >  
> >  static bool armada_is_valid(struct armada_thermal_priv *priv)
> >   
> 
> 

Thanks
Miqu?l

Re: [PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Daniel Lezcano]

On 29/07/2018 21:27, Miquel Raynal wrote:
> Hi Daniel,

[ ... ]

> That's exactly what is done later in the series actually, but for DT
> backward compatibility and in order to continue adding feature to this
> driver, we must ensure there won't be any drawback for people using old
> DT with deprecated thermal representation.
> 
> 
> Thanks for reviewing, if you don't mind I could copy you in future
> changes? 

Yes, sure.

> Unfortunately for this series it's already been applied and I
> would like not to do any deep changes before it's merged, but for sure
> this function could be enhanced.

Or find a way to remove it ? :)


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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 01/23] thermal: armada: add a function that sanitizes the thermal zone name

[Daniel Lezcano]

On 29/07/2018 21:27, Miquel Raynal wrote:
> Hi Daniel,

[ ... ]

> That's exactly what is done later in the series actually, but for DT
> backward compatibility and in order to continue adding feature to this
> driver, we must ensure there won't be any drawback for people using old
> DT with deprecated thermal representation.
> 
> 
> Thanks for reviewing, if you don't mind I could copy you in future
> changes? 

Yes, sure.

> Unfortunately for this series it's already been applied and I
> would like not to do any deep changes before it's merged, but for sure
> this function could be enhanced.

Or find a way to remove it ? :)


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

[PATCH v3 13/23] dt-bindings: cp110: rename cp110 syscon file

[Miquel Raynal]

There is no need to give numbers to system controllers inside the
documentation as the syscons use the same compatibles. Furthermore, this
approach does not scale very well and would force the creation of a new
file each time a new syscon is added in the device tree.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../marvell/{cp110-system-controller0.txt => cp110-system-controller.txt} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/arm/marvell/{cp110-system-controller0.txt => cp110-system-controller.txt} (100%)

diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
similarity index 100%
rename from Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt
rename to Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
-- 
2.14.1

[PATCH v3 13/23] dt-bindings: cp110: rename cp110 syscon file

[Miquel Raynal]

There is no need to give numbers to system controllers inside the
documentation as the syscons use the same compatibles. Furthermore, this
approach does not scale very well and would force the creation of a new
file each time a new syscon is added in the device tree.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../marvell/{cp110-system-controller0.txt => cp110-system-controller.txt} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/arm/marvell/{cp110-system-controller0.txt => cp110-system-controller.txt} (100%)

diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
similarity index 100%
rename from Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt
rename to Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
-- 
2.14.1