Armada thermal: improvements and A7K/A8K SoCs support

From: Baruch Siach <[email protected]>

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

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

Signed-off-by Baruch Siach <[email protected]>

[<[email protected]>: reword, additional details]

Signed-off-by Miquel Raynal <[email protected]>

---
 .../devicetree/bindings/thermal/armada-thermal.txt | 24 +++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

Documentation/devicetree/bindings/thermal/armada-thermal.txt +2 -1

index 24aacf8948c5..9b7b2c03cc6f 100644
@@ -7,17 +7,31 @@ Required properties:
 		marvell,armada375-thermal
 		marvell,armada380-thermal
 		marvell,armadaxp-thermal
+		marvell,armada-ap806-thermal
+		marvell,armada-cp110-thermal
 
 - reg:		Device's register space.
 		Two entries are expected, see the examples below.
-		The first one is required for the sensor register;

Documentation/devicetree/bindings/thermal/armada-thermal.txt +9 -2

-Example:
+Examples:
 
+	/* Legacy bindings */
 	thermal at d0018300 {
 		compatible = "marvell,armada370-thermal";
-                reg = <0xd0018300 0x4
+		reg = <0xd0018300 0x4
 		       0xd0018304 0x4>;
 	};
+
+	ap_thermal: thermal at 6f8084 {
+		compatible = "marvell,armada-ap806-thermal";
+		reg = <0x6f808C 0x4>,
+		      <0x6f8084 0x8>;
+	};

Hi Miqu?l,

Quoted 45 lines · Miquel Raynal

From: Baruch Siach <[email protected]>

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

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

Signed-off-by Baruch Siach <[email protected]>

[<[email protected]>: reword, additional details]

Signed-off-by Miquel Raynal <[email protected]>

---
 .../devicetree/bindings/thermal/armada-thermal.txt | 24

+++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt index 24aacf8948c5..9b7b2c03cc6f 100644 --- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt +++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt @@ -7,17 +7,31 @@ Required properties: marvell,armada375-thermal marvell,armada380-thermal marvell,armadaxp-thermal

Documentation/devicetree/bindings/thermal/armada-thermal.txt +2 -3

+		marvell,armada-ap806-thermal
+		marvell,armada-cp110-thermal
 
 - reg:		Device's register space.
 		Two entries are expected, see the examples below.
-		The first one is required for the sensor register;
-		the second one is required for the control register
-		to be used for sensor initialization (a.k.a.

calibration).

(quoted diff) +2 -0

+		The first one points to the status register (4B).
+		The second one points to the control registers

(8B).

(quoted diff) +1 -0

+		Note: with legacy bindings, the second entry

pointed

(quoted diff) +1 -0

+		only to the so called "control MSB" ("control 1"),

was

(quoted diff) +1 -0

+		4B wide and did not let the possibility to reach

the

Documentation/devicetree/bindings/thermal/armada-thermal.txt Required properties: · line 22

Quoted 3 lines

Documentation/devicetree/bindings/thermal/armada-thermal.txt +3 -0

+		"control LSB" ("control 0") register. This is only
+		allowed for compatibility reasons in Armada
+		370/375/38x/XP DT nodes.  

"allowed" is not the right term, IMO. Legacy compatibles MUST point to the MSB control register to preserve compatibility with existing DTs.

The original patch had a list of legacy and non-legacy compatibles. I think we need to keep them.

baruch

Quoted 16 lines

Documentation/devicetree/bindings/thermal/armada-thermal.txt +9 -2

-Example:
+Examples:
 
+	/* Legacy bindings */
 	thermal at d0018300 {
 		compatible = "marvell,armada370-thermal";
-                reg = <0xd0018300 0x4
+		reg = <0xd0018300 0x4
 		       0xd0018304 0x4>;
 	};
+
+	ap_thermal: thermal at 6f8084 {
+		compatible = "marvell,armada-ap806-thermal";
+		reg = <0x6f808C 0x4>,
+		      <0x6f8084 0x8>;
+	};  

Quoted 54 lines · On Mon, 18 Dec 2017 22:33:24 +0200 Baruch Siach <[email protected]> wrote:

Quoted 45 lines · Miquel Raynal

From: Baruch Siach <[email protected]>

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

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

Signed-off-by Baruch Siach <[email protected]>

[<[email protected]>: reword, additional details]

Signed-off-by Miquel Raynal <[email protected]>

---
 .../devicetree/bindings/thermal/armada-thermal.txt | 24

+++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt index 24aacf8948c5..9b7b2c03cc6f 100644 --- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt +++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt @@ -7,17 +7,31 @@ Required properties: marvell,armada375-thermal marvell,armada380-thermal marvell,armadaxp-thermal

Documentation/devicetree/bindings/thermal/armada-thermal.txt +2 -3

+		marvell,armada-ap806-thermal
+		marvell,armada-cp110-thermal
 
 - reg:		Device's register space.
 		Two entries are expected, see the examples below.
-		The first one is required for the sensor register;
-		the second one is required for the control register
-		to be used for sensor initialization (a.k.a.

calibration).

(quoted diff) +2 -0

+		The first one points to the status register (4B).
+		The second one points to the control registers

(8B).

(quoted diff) +1 -0

+		Note: with legacy bindings, the second entry

pointed

(quoted diff) +1 -0

+		only to the so called "control MSB" ("control 1"),

was

(quoted diff) +1 -0

+		4B wide and did not let the possibility to reach

the

Documentation/devicetree/bindings/thermal/armada-thermal.txt Required properties: · line 22

Quoted 3 lines

Documentation/devicetree/bindings/thermal/armada-thermal.txt +3 -0

+		"control LSB" ("control 0") register. This is only
+		allowed for compatibility reasons in Armada
+		370/375/38x/XP DT nodes.  

"allowed" is not the right term, IMO. Legacy compatibles MUST point to the MSB control register to preserve compatibility with existing DTs.

Quoted 2 lines

The original patch had a list of legacy and non-legacy compatibles. I think we need to keep them.

Maybe I should reword this paragraph because we both agree on the meaning:

"
Note: Legacy bindings are only supported with Armada 370/375/38x/XP compatibles. The second memory resource entry only points to "control MSB/control 1", is 4 bytes wide and is preventing any access to "control LSB/control 0".
"

Does this sounds better to you?

Hi Miqu?l,

Quoted 76 lines · Miquel RAYNAL

Quoted 61 lines · On Mon, 18 Dec 2017 22:33:24 +0200 Baruch Siach <[email protected]> wrote:

Quoted 53 lines · Miquel Raynal

From: Baruch Siach <[email protected]>

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

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

Signed-off-by Baruch Siach <[email protected]>

[<[email protected]>: reword, additional details]

Signed-off-by Miquel Raynal <[email protected]>

---
 .../devicetree/bindings/thermal/armada-thermal.txt | 24

+++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt index 24aacf8948c5..9b7b2c03cc6f 100644 --- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
+++
b/Documentation/devicetree/bindings/thermal/armada-thermal.txt

(quoted diff) +0 -0

@@ -7,17 +7,31 @@ Required properties:

marvell,armada375-thermal marvell,armada380-thermal marvell,armadaxp-thermal

Documentation/devicetree/bindings/thermal/armada-thermal.txt +2 -0

+		marvell,armada-ap806-thermal
+		marvell,armada-cp110-thermal
 
 - reg:		Device's register space.
 		Two entries are expected, see the examples

below.

(quoted diff) +0 -1

-		The first one is required for the sensor

register;

(quoted diff) +0 -1

-		the second one is required for the control

register

(quoted diff) +0 -1

-		to be used for sensor initialization (a.k.a.

calibration).

(quoted diff) +1 -0

+		The first one points to the status register

(4B).

(quoted diff) +1 -0

+		The second one points to the control registers

(8B).

(quoted diff) +1 -0

+		Note: with legacy bindings, the second entry

pointed

(quoted diff) +1 -0

+		only to the so called "control MSB" ("control

1"), was

(quoted diff) +1 -0

+		4B wide and did not let the possibility to

reach the

(quoted diff) +1 -0

+		"control LSB" ("control 0") register. This is

only

Documentation/devicetree/bindings/thermal/armada-thermal.txt Required properties: · line 22

Quoted 2 lines

Documentation/devicetree/bindings/thermal/armada-thermal.txt +2 -0

+		allowed for compatibility reasons in Armada
+		370/375/38x/XP DT nodes.    

"allowed" is not the right term, IMO. Legacy compatibles MUST point to the MSB control register to preserve compatibility with existing DTs.

Quoted 2 lines

The original patch had a list of legacy and non-legacy compatibles. I think we need to keep them.

Maybe I should reword this paragraph because we both agree on the meaning:

"
Note: Legacy bindings are only supported with Armada 370/375/38x/XP compatibles. The second memory resource entry only points to "control MSB/control 1", is 4 bytes wide and is preventing any access to "control LSB/control 0".
"

Quoted 1 line

Does this sounds better to you?

I think we need to explicitly list the affected compatible strings. Something like:

For backwards compatibility reasons, the compatibles marvell,armada370-thermal, marvell,armada380-thermal, and marvell,armadaxp-thermal must point to "control MSB/control 1",
with size of 4. All other compatibles must point to "control LSB/control 0" with size of 8.

But I think you are right that we can use the size of the control registers to tell whether e.g. marvell,armada380-thermal is of the old binding of the new one. So maybe the "allow" language is more correct. But let's make it explicit to avoid any doubt. How about:

The compatibles marvell,armada370-thermal,
marvell,armada380-thermal, and marvell,armadaxp-thermal must point to "control MSB/control 1", with size of 4 (deprecated binding), or point to "control LSB/control 0" with size of 8 (current binding). All other compatibles must point to "control LSB/control 0" with size of 8.

baruch

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

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

Signed-off-by Miquel Raynal <[email protected]>

Reviewed-by Gregory CLEMENT <[email protected]>

---

Hi Miqu?l,

Quoted 17 lines · Miquel Raynal

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

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

Signed-off-by Miquel Raynal <[email protected]>

Reviewed-by Gregory CLEMENT <[email protected]>

---

[...]

Quoted 11 lines

(quoted diff) +2 -0

+	/*
+	 * Legacy DT bindings only described "control1" register

(also referred

(quoted diff) +1 -0

+	 * as "control MSB" on old documentation). New bindings

cover

(quoted diff) +1 -0

+	 * "control0/control LSB" and "control1/control MSB"

registers within

drivers/thermal/armada_thermal.c +4 -0

+	 * the same resource, which is then of size 8 instead of 4.
+	 */
+	if (resource_size(res) == LEGACY_CONTROL_MEM_LEN) {
+		/* ->control0 unavailable in this configuration */

drivers/thermal/armada_thermal.c static int armada_thermal_probe(struct platform_device *pdev) · line 303

Quoted 5 lines

drivers/thermal/armada_thermal.c +5 -0

+		priv->control1 = control + LEGACY_CONTROL1_OFFSET;
+	} else {
+		priv->control0 = control + CONTROL0_OFFSET;
+		priv->control1 = control + CONTROL1_OFFSET;
+	}  

The needs_control0 field that you mentioned in the cover page is missing here.

baruch

Quoted 32 lines · On Mon, 18 Dec 2017 22:35:42 +0200 Baruch Siach <[email protected]> wrote:

Quoted 17 lines · Miquel Raynal

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

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

Signed-off-by Miquel Raynal <[email protected]>

Reviewed-by Gregory CLEMENT <[email protected]>

---

[...]

Quoted 11 lines

(quoted diff) +2 -0

+	/*
+	 * Legacy DT bindings only described "control1" register

(also referred

(quoted diff) +1 -0

+	 * as "control MSB" on old documentation). New bindings

cover

(quoted diff) +1 -0

+	 * "control0/control LSB" and "control1/control MSB"

registers within

drivers/thermal/armada_thermal.c +4 -0

+	 * the same resource, which is then of size 8 instead of 4.
+	 */
+	if (resource_size(res) == LEGACY_CONTROL_MEM_LEN) {
+		/* ->control0 unavailable in this configuration */

Quoted 8 lines

drivers/thermal/armada_thermal.c static int armada_thermal_probe(struct platform_device *pdev) · line 303

Quoted 5 lines

drivers/thermal/armada_thermal.c +5 -0

+		priv->control1 = control + LEGACY_CONTROL1_OFFSET;
+	} else {
+		priv->control0 = control + CONTROL0_OFFSET;
+		priv->control1 = control + CONTROL1_OFFSET;
+	}  

The needs_control0 field that you mentioned in the cover page is missing here.

Yes, at this point nobody actually *needs* control0 so the limitation is added with the patch that introduce ap806 support as it is the first compatible that needs both control0 and control1 to work correctly. Does this bother you?

Hi Miqu?l,

Quoted 46 lines · Miquel RAYNAL

Quoted 35 lines · On Mon, 18 Dec 2017 22:35:42 +0200 Baruch Siach <[email protected]> wrote:

Quoted 17 lines · Miquel Raynal

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

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

Signed-off-by Miquel Raynal <[email protected]>

Reviewed-by Gregory CLEMENT

<[email protected]> ---

[...]

Quoted 14 lines

(quoted diff) +2 -0

+	/*
+	 * Legacy DT bindings only described "control1"

register (also referred

(quoted diff) +1 -0

+	 * as "control MSB" on old documentation). New bindings

cover

(quoted diff) +1 -0

+	 * "control0/control LSB" and "control1/control MSB"

registers within

(quoted diff) +1 -0

+	 * the same resource, which is then of size 8 instead

of 4.

drivers/thermal/armada_thermal.c +3 -0

+	 */
+	if (resource_size(res) == LEGACY_CONTROL_MEM_LEN) {
+		/* ->control0 unavailable in this

configuration */

(quoted diff) +1 -0

+		priv->control1 = control +

Quoted 8 lines

drivers/thermal/armada_thermal.c static int armada_thermal_probe(struct platform_device *pdev) · line 303

Quoted 5 lines

LEGACY_CONTROL1_OFFSET;

drivers/thermal/armada_thermal.c +4 -0

+	} else {
+		priv->control0 = control + CONTROL0_OFFSET;
+		priv->control1 = control + CONTROL1_OFFSET;
+	}    

The needs_control0 field that you mentioned in the cover page is missing here.

Quoted 4 lines

Yes, at this point nobody actually *needs* control0 so the limitation is added with the patch that introduce ap806 support as it is the first compatible that needs both control0 and control1 to work correctly. Does this bother you?

No. It is just that we agreed to have a verification here that the size of the control registers resource matches the binding. I thought that the needs_control0 field that you mention in the cover page is meant to implement that.

necessary. It would just make sure that no one introduces a DT with the wrong resource size.

baruch

Quoted 52 lines · On Tue, 19 Dec 2017 07:51:54 +0200 Baruch Siach <[email protected]> wrote:

Quoted 46 lines · Miquel RAYNAL

Quoted 35 lines · On Mon, 18 Dec 2017 22:35:42 +0200 Baruch Siach <[email protected]> wrote:

Quoted 17 lines · Miquel Raynal

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

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

Signed-off-by Miquel Raynal <[email protected]>

Reviewed-by Gregory CLEMENT

<[email protected]> ---

[...]

Quoted 14 lines

(quoted diff) +2 -0

+	/*
+	 * Legacy DT bindings only described "control1"

register (also referred

(quoted diff) +1 -0

+	 * as "control MSB" on old documentation). New bindings

cover

(quoted diff) +1 -0

+	 * "control0/control LSB" and "control1/control MSB"

registers within

(quoted diff) +1 -0

+	 * the same resource, which is then of size 8 instead

of 4.

drivers/thermal/armada_thermal.c +3 -0

+	 */
+	if (resource_size(res) == LEGACY_CONTROL_MEM_LEN) {
+		/* ->control0 unavailable in this

configuration */

(quoted diff) +1 -0

+		priv->control1 = control +

Quoted 8 lines

drivers/thermal/armada_thermal.c static int armada_thermal_probe(struct platform_device *pdev) · line 303

Quoted 5 lines

LEGACY_CONTROL1_OFFSET;

drivers/thermal/armada_thermal.c +4 -0

+	} else {
+		priv->control0 = control + CONTROL0_OFFSET;
+		priv->control1 = control + CONTROL1_OFFSET;
+	}    

The needs_control0 field that you mentioned in the cover page is missing here.

Quoted 4 lines

Yes, at this point nobody actually *needs* control0 so the limitation is added with the patch that introduce ap806 support as it is the first compatible that needs both control0 and control1 to work correctly. Does this bother you?

Quoted 4 lines

No. It is just that we agreed to have a verification here that the size of the control registers resource matches the binding. I thought that the needs_control0 field that you mention in the cover page is meant to implement that.

That is absolutely right, but at this point in the series, the supported compatible strings are "marvell,armada[370|375|38x|xp]-thermal". All of them can use both bindings so I don't see the point to have a needs_control0 field in this patch. It is introduced in the next patch that adds support for ap806 by only supporting the new bindings though.

Quoted 2 lines

necessary. It would just make sure that no one introduces a DT with the wrong resource size.

Not sure I understand what exactly you wanna check, can you give me an example?

Hi Miqu?l,

Quoted 60 lines · Miquel RAYNAL

Quoted 53 lines · On Tue, 19 Dec 2017 07:51:54 +0200 Baruch Siach <[email protected]> wrote:

Quoted 47 lines · Miquel RAYNAL

Quoted 36 lines · On Mon, 18 Dec 2017 22:35:42 +0200 Baruch Siach <[email protected]> wrote:

Quoted 17 lines · Miquel Raynal

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

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

Signed-off-by: Miquel Raynal <[email protected]> Reviewed-by: Gregory CLEMENT <[email protected]> ---

[...]

Quoted 14 lines

(quoted diff) +2 -0

+	/*
+	 * Legacy DT bindings only described "control1"

register (also referred

(quoted diff) +1 -0

+	 * as "control MSB" on old documentation). New

bindings cover

(quoted diff) +1 -0

+	 * "control0/control LSB" and "control1/control

MSB" registers within

(quoted diff) +1 -0

+	 * the same resource, which is then of size 8

instead of 4.

drivers/thermal/armada_thermal.c +3 -0

+	 */
+	if (resource_size(res) == LEGACY_CONTROL_MEM_LEN) {
+		/* ->control0 unavailable in this

configuration */

(quoted diff) +1 -0

+		priv->control1 = control +

Quoted 8 lines

drivers/thermal/armada_thermal.c static int armada_thermal_probe(struct platform_device *pdev) · line 303

Quoted 5 lines

LEGACY_CONTROL1_OFFSET;

drivers/thermal/armada_thermal.c +4 -0

+	} else {
+		priv->control0 = control + CONTROL0_OFFSET;
+		priv->control1 = control + CONTROL1_OFFSET;
+	}      

The needs_control0 field that you mentioned in the cover page is missing here.

Quoted 4 lines

Yes, at this point nobody actually *needs* control0 so the limitation is added with the patch that introduce ap806 support as it is the first compatible that needs both control0 and control1 to work correctly. Does this bother you?

Quoted 4 lines

No. It is just that we agreed to have a verification here that the size of the control registers resource matches the binding. I thought that the needs_control0 field that you mention in the cover page is meant to implement that.

Quoted 6 lines

That is absolutely right, but at this point in the series, the supported compatible strings are "marvell,armada[370|375|38x|xp]-thermal". All of them can use both bindings so I don't see the point to have a needs_control0 field in this patch. It is introduced in the next patch that adds support for ap806 by only supporting the new bindings though.

OK. Makes sense.

Quoted 5 lines

Quoted 2 lines

necessary. It would just make sure that no one introduces a DT with the wrong resource size.

Not sure I understand what exactly you wanna check, can you give me an example?

I wrote that before it occurred to me that we can use the control registers size the distinguish between the old binding and the new one.

I still think it would be nice to add needs_control0=true to armada375_data, for consistency with the ap806 and cp110.

baruch

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

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/thermal/armada_thermal.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

drivers/thermal/armada_thermal.c +8 -8

index f422563e617c..198485fa77f2 100644
@@ -51,7 +51,7 @@ struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
 struct armada_thermal_priv {
-	void __iomem *sensor;
+	void __iomem *status;
 	void __iomem *control0;
 	void __iomem *control1;
 	struct armada_thermal_data *data;

@@ -98,9 +98,9 @@ static void armadaxp_init_sensor(struct platform_device *pdev,
 	writel(reg, priv->control1);
 
 	/* Enable the sensor */
-	reg = readl_relaxed(priv->sensor);
+	reg = readl_relaxed(priv->status);
 	reg &= ~PMU_TM_DISABLE_MASK;
-	writel(reg, priv->sensor);
+	writel(reg, priv->status);
 }
 
 static void armada370_init_sensor(struct platform_device *pdev,

@@ -156,7 +156,7 @@ static void armada380_init_sensor(struct platform_device *pdev,
 
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
-	u32 reg = readl_relaxed(priv->sensor);
+	u32 reg = readl_relaxed(priv->status);
 
 	return reg & priv->data->is_valid_bit;
 }

@@ -175,7 +175,7 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 		return -EIO;
 	}
 
-	reg = readl_relaxed(priv->sensor);
+	reg = readl_relaxed(priv->status);
 	reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;
 
 	/* Get formula coeficients */

@@ -277,9 +277,9 @@ static int armada_thermal_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	priv->sensor = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(priv->sensor))
-		return PTR_ERR(priv->sensor);
+	priv->status = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->status))
+		return PTR_ERR(priv->status);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	control = devm_ioremap_resource(&pdev->dev, res);

Signature · 1 line

2.11.0

From: Baruch Siach <[email protected]>

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

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

Signed-off-by Baruch Siach <[email protected]>

[<[email protected]>: Changes when applying over the previous patches, including the register names changes, also switched the coefficients values to s64 instead of unsigned long to deal with negative values and used do_div instead of the traditionnal '/']

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/thermal/armada_thermal.c | 80 ++++++++++++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 15 deletions(-)

drivers/thermal/armada_thermal.c +65 -15

index 198485fa77f2..ec29ea76b818 100644
@@ -47,6 +47,11 @@
 #define CONTROL0_OFFSET			0x0
 #define CONTROL1_OFFSET			0x4
 
+/* TSEN refers to the temperature sensors within the AP */
+#define CONTROL0_TSEN_START		BIT(0)
+#define CONTROL0_TSEN_RESET		BIT(1)
+#define CONTROL0_TSEN_ENABLE		BIT(2)
+
 struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */

@@ -66,15 +71,17 @@ struct armada_thermal_data {
 	bool (*is_valid)(struct armada_thermal_priv *);
 
 	/* Formula coeficients: temp = (b - m * reg) / div */
-	unsigned long coef_b;
-	unsigned long coef_m;
-	unsigned long coef_div;
+	s64 coef_b;
+	s64 coef_m;
+	u32 coef_div;
 	bool inverted;
+	bool signed_sample;
 
 	/* Register shift and mask to access the sensor temperature */
 	unsigned int temp_shift;
 	unsigned int temp_mask;
 	u32 is_valid_bit;
+	bool needs_control0;
 };
 
 static void armadaxp_init_sensor(struct platform_device *pdev,

@@ -154,6 +161,18 @@ static void armada380_init_sensor(struct platform_device *pdev,
 	}
 }
 
+static void armada_ap806_init_sensor(struct platform_device *pdev,
+				     struct armada_thermal_priv *priv)
+{
+	u32 reg;
+
+	reg = readl_relaxed(priv->control0);
+	reg &= ~CONTROL0_TSEN_RESET;
+	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
+	writel(reg, priv->control0);
+	msleep(10);
+}
+
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
 	u32 reg = readl_relaxed(priv->status);

@@ -165,8 +184,8 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 			  int *temp)
 {
 	struct armada_thermal_priv *priv = thermal->devdata;
-	unsigned long reg;
-	unsigned long m, b, div;
+	u32 reg, div;
+	s64 sample, b, m;
 
 	/* Valid check */
 	if (priv->data->is_valid && !priv->data->is_valid(priv)) {

@@ -177,6 +196,11 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 
 	reg = readl_relaxed(priv->status);
 	reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;
+	if (priv->data->signed_sample)
+		/* The most significant bit is the sign bit */
+		sample = sign_extend32(reg, fls(priv->data->temp_mask) - 1);
+	else
+		sample = reg;
 
 	/* Get formula coeficients */
 	b = priv->data->coef_b;

@@ -184,9 +208,12 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 	div = priv->data->coef_div;
 
 	if (priv->data->inverted)
-		*temp = ((m * reg) - b) / div;
+		*temp = (m * sample) - b;
 	else
-		*temp = (b - (m * reg)) / div;
+		*temp = b - (m * sample);
+
+	do_div(*temp, div);
+
 	return 0;
 }
 

@@ -198,8 +225,8 @@ static const struct armada_thermal_data armadaxp_data = {
 	.init_sensor = armadaxp_init_sensor,
 	.temp_shift = 10,
 	.temp_mask = 0x1ff,
-	.coef_b = 3153000000UL,
-	.coef_m = 10000000UL,
+	.coef_b = 3153000000ULL,
+	.coef_m = 10000000ULL,
 	.coef_div = 13825,
 };
 

@@ -209,8 +236,8 @@ static const struct armada_thermal_data armada370_data = {
 	.is_valid_bit = BIT(9),
 	.temp_shift = 10,
 	.temp_mask = 0x1ff,
-	.coef_b = 3153000000UL,
-	.coef_m = 10000000UL,
+	.coef_b = 3153000000ULL,
+	.coef_m = 10000000ULL,
 	.coef_div = 13825,
 };
 

@@ -220,8 +247,8 @@ static const struct armada_thermal_data armada375_data = {
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x1ff,
-	.coef_b = 3171900000UL,
-	.coef_m = 10000000UL,
+	.coef_b = 3171900000ULL,
+	.coef_m = 10000000ULL,
 	.coef_div = 13616,
 };
 

@@ -231,12 +258,26 @@ static const struct armada_thermal_data armada380_data = {
 	.is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
-	.coef_b = 1172499100UL,
-	.coef_m = 2000096UL,
+	.coef_b = 1172499100ULL,
+	.coef_m = 2000096ULL,
 	.coef_div = 4201,
 	.inverted = true,
 };
 
+static const struct armada_thermal_data armada_ap806_data = {
+	.is_valid = armada_is_valid,
+	.init_sensor = armada_ap806_init_sensor,
+	.is_valid_bit = BIT(16),
+	.temp_shift = 0,
+	.temp_mask = 0x3ff,
+	.coef_b = -150000LL,
+	.coef_m = 423ULL,
+	.coef_div = 1,
+	.inverted = true,
+	.signed_sample = true,
+	.needs_control0 = true,
+};
+
 static const struct of_device_id armada_thermal_id_table[] = {
 	{
 		.compatible = "marvell,armadaxp-thermal",

@@ -255,6 +296,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.data       = &armada380_data,
 	},
 	{
+		.compatible = "marvell,armada-ap806-thermal",
+		.data       = &armada_ap806_data,
+	},
+	{
 		/* sentinel */
 	},
 };

@@ -296,6 +341,11 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	 */
 	if (resource_size(res) == LEGACY_CONTROL_MEM_LEN) {
 		/* ->control0 unavailable in this configuration */
+		if (priv->data->needs_control0) {
+			dev_err(&pdev->dev, "No access to control0 register\n");
+			return -EINVAL;
+		}
+
 		priv->control1 = control + LEGACY_CONTROL1_OFFSET;
 	} else {
 		priv->control0 = control + CONTROL0_OFFSET;

Signature · 1 line

2.11.0

Hi Miquel,

Quoted 9 lines · Miquel Raynal

From: Baruch Siach <[email protected]>

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

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

Quoted 7 lines

Signed-off-by Baruch Siach <[email protected]>

[<[email protected]>: Changes when applying over the previous patches, including the register names changes, also switched the coefficients values to s64 instead of unsigned long to deal with negative values and used do_div instead of the traditionnal '/'] Signed-off-by: Miquel Raynal <[email protected]>

I am just a little concerned by the fac that the value pass though get_temp() is an int and now the intermediate calculation are in s64. But maybe I'm too picky for this,

Reviewed-by Gregory CLEMENT <[email protected]>

Gregory

Quoted 190 lines

---
 drivers/thermal/armada_thermal.c | 80

++++++++++++++++++++++++++++++++-------- 1 file changed, 65 insertions(+), 15 deletions(-)

(quoted diff) +0 -0

b/drivers/thermal/armada_thermal.c index 198485fa77f2..ec29ea76b818 100644 --- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c

drivers/thermal/armada_thermal.c +28 -5

@@ -47,6 +47,11 @@
 #define CONTROL0_OFFSET			0x0
 #define CONTROL1_OFFSET			0x4
 
+/* TSEN refers to the temperature sensors within the AP */
+#define CONTROL0_TSEN_START		BIT(0)
+#define CONTROL0_TSEN_RESET		BIT(1)
+#define CONTROL0_TSEN_ENABLE		BIT(2)
+
 struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */

@@ -66,15 +71,17 @@ struct armada_thermal_data {
 	bool (*is_valid)(struct armada_thermal_priv *);
 
 	/* Formula coeficients: temp = (b - m * reg) / div */
-	unsigned long coef_b;
-	unsigned long coef_m;
-	unsigned long coef_div;
+	s64 coef_b;
+	s64 coef_m;
+	u32 coef_div;
 	bool inverted;
+	bool signed_sample;
 
 	/* Register shift and mask to access the sensor
 temperature */ unsigned int temp_shift;
 	unsigned int temp_mask;
 	u32 is_valid_bit;
+	bool needs_control0;
 };
 
 static void armadaxp_init_sensor(struct platform_device *pdev,

@@ -154,6 +161,18 @@ static void armada380_init_sensor(struct
 platform_device *pdev, }
 }
 
+static void armada_ap806_init_sensor(struct platform_device *pdev,
+				     struct armada_thermal_priv
 *priv) +{
+	u32 reg;
+
+	reg = readl_relaxed(priv->control0);
+	reg &= ~CONTROL0_TSEN_RESET;
+	reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
+	writel(reg, priv->control0);
+	msleep(10);
+}
+
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
 	u32 reg = readl_relaxed(priv->status);

@@ -165,8 +184,8 @@ static int armada_get_temp(struct
 thermal_zone_device *thermal, int *temp)
 {
 	struct armada_thermal_priv *priv = thermal->devdata;
-	unsigned long reg;
-	unsigned long m, b, div;
+	u32 reg, div;
+	s64 sample, b, m;
 
 	/* Valid check */
 	if (priv->data->is_valid && !priv->data->is_valid(priv)) {

@@ -177,6 +196,11 @@ static int armada_get_temp(struct
 thermal_zone_device *thermal, 
 	reg = readl_relaxed(priv->status);
 	reg = (reg >> priv->data->temp_shift) &
 priv->data->temp_mask;
+	if (priv->data->signed_sample)
+		/* The most significant bit is the sign bit */
+		sample = sign_extend32(reg,
 fls(priv->data->temp_mask) - 1);
+	else
+		sample = reg;
 

drivers/thermal/armada_thermal.c +31 -10

 	/* Get formula coeficients */
 	b = priv->data->coef_b;

@@ -184,9 +208,12 @@ static int armada_get_temp(struct
 thermal_zone_device *thermal, div = priv->data->coef_div;
 
 	if (priv->data->inverted)
-		*temp = ((m * reg) - b) / div;
+		*temp = (m * sample) - b;
 	else
-		*temp = (b - (m * reg)) / div;
+		*temp = b - (m * sample);
+
+	do_div(*temp, div);
+
 	return 0;
 }
 

@@ -198,8 +225,8 @@ static const struct armada_thermal_data
 armadaxp_data = { .init_sensor = armadaxp_init_sensor,
 	.temp_shift = 10,
 	.temp_mask = 0x1ff,
-	.coef_b = 3153000000UL,
-	.coef_m = 10000000UL,
+	.coef_b = 3153000000ULL,
+	.coef_m = 10000000ULL,
 	.coef_div = 13825,
 };
 

@@ -209,8 +236,8 @@ static const struct armada_thermal_data
 armada370_data = { .is_valid_bit = BIT(9),
 	.temp_shift = 10,
 	.temp_mask = 0x1ff,
-	.coef_b = 3153000000UL,
-	.coef_m = 10000000UL,
+	.coef_b = 3153000000ULL,
+	.coef_m = 10000000ULL,
 	.coef_div = 13825,
 };
 

@@ -220,8 +247,8 @@ static const struct armada_thermal_data
 armada375_data = { .is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x1ff,
-	.coef_b = 3171900000UL,
-	.coef_m = 10000000UL,
+	.coef_b = 3171900000ULL,
+	.coef_m = 10000000ULL,
 	.coef_div = 13616,
 };
 

@@ -231,12 +258,26 @@ static const struct armada_thermal_data
 armada380_data = { .is_valid_bit = BIT(10),
 	.temp_shift = 0,
 	.temp_mask = 0x3ff,
-	.coef_b = 1172499100UL,
-	.coef_m = 2000096UL,
+	.coef_b = 1172499100ULL,
+	.coef_m = 2000096ULL,
 	.coef_div = 4201,
 	.inverted = true,
 };
 
+static const struct armada_thermal_data armada_ap806_data = {
+	.is_valid = armada_is_valid,
+	.init_sensor = armada_ap806_init_sensor,
+	.is_valid_bit = BIT(16),
+	.temp_shift = 0,
+	.temp_mask = 0x3ff,
+	.coef_b = -150000LL,
+	.coef_m = 423ULL,
+	.coef_div = 1,
+	.inverted = true,
+	.signed_sample = true,
+	.needs_control0 = true,
+};
+
 static const struct of_device_id armada_thermal_id_table[] = {
 	{
 		.compatible = "marvell,armadaxp-thermal",

@@ -255,6 +296,10 @@ static const struct of_device_id
 armada_thermal_id_table[] = { .data       = &armada380_data,
 	},
 	{
+		.compatible = "marvell,armada-ap806-thermal",
+		.data       = &armada_ap806_data,
+	},
+	{
 		/* sentinel */
 	},
 };

@@ -296,6 +341,11 @@ static int armada_thermal_probe(struct

platform_device *pdev) */

drivers/thermal/armada_thermal.c +2 -0

 	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");

drivers/thermal/armada_thermal.c +3 -0

+			return -EINVAL;
+		}
+
 		priv->control1 = control + LEGACY_CONTROL1_OFFSET;
 	} else {
 		priv->control0 = control + CONTROL0_OFFSET;

Signature · 1 line

2.11.0

From: Baruch Siach <[email protected]>

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

Signed-off-by Baruch Siach <[email protected]>

[<[email protected]>: renamed the register pointers as well as some definitions related to the new register names and simplified the init sequence for Armada 380]

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/thermal/armada_thermal.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

drivers/thermal/armada_thermal.c +26 -7

index ec29ea76b818..11a94ad66c35 100644
@@ -37,7 +37,6 @@
 #define A375_UNIT_CONTROL_MASK		0x7
 #define A375_READOUT_INVERT		BIT(15)
 #define A375_HW_RESETn			BIT(8)
-#define A380_HW_RESET			BIT(8)
 
 /* Legacy bindings */
 #define LEGACY_CONTROL_MEM_LEN		0x4

@@ -52,6 +51,10 @@
 #define CONTROL0_TSEN_RESET		BIT(1)
 #define CONTROL0_TSEN_ENABLE		BIT(2)
 
+/* EXT_TSEN refers to the external temperature sensors, out of the AP */
+#define CONTROL1_EXT_TSEN_SW_RESET	BIT(7)
+#define CONTROL1_EXT_TSEN_HW_RESETn	BIT(8)
+
 struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */

@@ -153,12 +156,11 @@ static void armada380_init_sensor(struct platform_device *pdev,
 {
 	u32 reg = readl_relaxed(priv->control1);
 
-	/* Reset hardware once */
-	if (!(reg & A380_HW_RESET)) {
-		reg |= A380_HW_RESET;
-		writel(reg, priv->control1);
-		msleep(10);
-	}
+	/* Disable the HW/SW reset */
+	reg |= CONTROL1_EXT_TSEN_HW_RESETn;
+	reg &= ~CONTROL1_EXT_TSEN_SW_RESET;
+	writel(reg, priv->control1);
+	msleep(10);
 }
 
 static void armada_ap806_init_sensor(struct platform_device *pdev,

@@ -278,6 +280,19 @@ static const struct armada_thermal_data armada_ap806_data = {
 	.needs_control0 = true,
 };
 
+static const struct armada_thermal_data armada_cp110_data = {
+	.is_valid = armada_is_valid,
+	.init_sensor = armada380_init_sensor,
+	.is_valid_bit = BIT(10),
+	.temp_shift = 0,
+	.temp_mask = 0x3ff,
+	.coef_b = 1172499100ULL,
+	.coef_m = 2000096ULL,
+	.coef_div = 4201,
+	.inverted = true,
+	.needs_control0 = true,
+};
+
 static const struct of_device_id armada_thermal_id_table[] = {
 	{
 		.compatible = "marvell,armadaxp-thermal",

@@ -300,6 +315,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.data       = &armada_ap806_data,
 	},
 	{
+		.compatible = "marvell,armada-cp110-thermal",
+		.data       = &armada_cp110_data,
+	},
+	{
 		/* sentinel */
 	},
 };

Signature · 1 line

2.11.0

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

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

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/thermal/Kconfig          | 4 ++--
 drivers/thermal/armada_thermal.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

drivers/thermal/Kconfig +2 -2

index 315ae2926e20..b6adc54b96f1 100644
@@ -301,13 +301,13 @@ config DB8500_THERMAL
 	  thermal zone if trip points reached.
 
 config ARMADA_THERMAL
-	tristate "Armada 370/XP thermal management"
+	tristate "Marvell EBU Armada SoCs thermal management"
 	depends on ARCH_MVEBU || COMPILE_TEST
 	depends on HAS_IOMEM
 	depends on OF
 	help
 	  Enable this option if you want to have support for thermal management
-	  controller present in Armada 370 and Armada XP SoC.
+	  controller present in Marvell EBU Armada SoCs (370,375,XP,38x,7K,8K).
 
 config DA9062_THERMAL
 	tristate "DA9062/DA9061 Dialog Semiconductor thermal driver"

drivers/thermal/armada_thermal.c +2 -2

index 11a94ad66c35..cef5c65c8f32 100644
@@ -1,5 +1,5 @@
 /*
- * Marvell Armada 370/XP thermal sensor driver
+ * Marvell EBU Armada SoCs thermal sensor driver
  *
  * Copyright (C) 2013 Marvell
  *

@@ -408,5 +408,5 @@ static struct platform_driver armada_thermal_driver = {
 module_platform_driver(armada_thermal_driver);
 
 MODULE_AUTHOR("Ezequiel Garcia <[email protected]>");
-MODULE_DESCRIPTION("Armada 370/XP thermal driver");
+MODULE_DESCRIPTION("Marvell EBU Armada SoCs thermal driver");
 MODULE_LICENSE("GPL v2");

Signature · 1 line

2.11.0

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

Suggested-by David Sniatkiwicz <[email protected]>

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/thermal/armada_thermal.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

drivers/thermal/armada_thermal.c +13 -0

index cef5c65c8f32..2eadd662591d 100644
@@ -46,6 +46,10 @@
 #define CONTROL0_OFFSET			0x0
 #define CONTROL1_OFFSET			0x4
 
+/* Errata fields */
+#define CONTROL0_TSEN_TC_TRIM_MASK	0x7
+#define CONTROL0_TSEN_TC_TRIM_VAL	0x3
+
 /* TSEN refers to the temperature sensors within the AP */
 #define CONTROL0_TSEN_START		BIT(0)
 #define CONTROL0_TSEN_RESET		BIT(1)

@@ -161,6 +165,15 @@ static void armada380_init_sensor(struct platform_device *pdev,
 	reg &= ~CONTROL1_EXT_TSEN_SW_RESET;
 	writel(reg, priv->control1);
 	msleep(10);
+
+	/* Set Tsen Tc Trim to correct default value (errata #132698) */
+	if (priv->control0) {
+		reg = readl_relaxed(priv->control0);
+		reg &= ~CONTROL0_TSEN_TC_TRIM_MASK;
+		reg |= CONTROL0_TSEN_TC_TRIM_VAL;
+		writel(reg, priv->control0);
+		msleep(10);
+	}
 }
 
 static void armada_ap806_init_sensor(struct platform_device *pdev,

Signature · 1 line

2.11.0

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

Suggested-by David Sniatkiwicz <[email protected]>

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/thermal/armada_thermal.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

drivers/thermal/armada_thermal.c +17 -2

index 2eadd662591d..4a5164ddffe7 100644
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/of_device.h>
 #include <linux/thermal.h>
+#include <linux/iopoll.h>
 
 /* Thermal Manager Control and Status Register */
 #define PMU_TDC0_SW_RST_MASK		(0x1 << 1)

@@ -59,6 +60,9 @@
 #define CONTROL1_EXT_TSEN_SW_RESET	BIT(7)
 #define CONTROL1_EXT_TSEN_HW_RESETn	BIT(8)
 
+#define STATUS_POLL_PERIOD_US		1000
+#define STATUS_POLL_TIMEOUT_US		100000
+
 struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */

@@ -155,6 +159,16 @@ static void armada375_init_sensor(struct platform_device *pdev,
 	msleep(50);
 }
 
+static void armada_wait_sensor_validity(struct armada_thermal_priv *priv)
+{
+	u32 reg;
+
+	readl_relaxed_poll_timeout(priv->status, reg,
+				   reg & priv->data->is_valid_bit,
+				   STATUS_POLL_PERIOD_US,
+				   STATUS_POLL_TIMEOUT_US);
+}
+
 static void armada380_init_sensor(struct platform_device *pdev,
 				  struct armada_thermal_priv *priv)
 {

@@ -164,7 +178,6 @@ static void armada380_init_sensor(struct platform_device *pdev,
 	reg |= CONTROL1_EXT_TSEN_HW_RESETn;
 	reg &= ~CONTROL1_EXT_TSEN_SW_RESET;
 	writel(reg, priv->control1);
-	msleep(10);
 
 	/* Set Tsen Tc Trim to correct default value (errata #132698) */
 	if (priv->control0) {

@@ -172,8 +185,10 @@ static void armada380_init_sensor(struct platform_device *pdev,
 		reg &= ~CONTROL0_TSEN_TC_TRIM_MASK;
 		reg |= CONTROL0_TSEN_TC_TRIM_VAL;
 		writel(reg, priv->control0);
-		msleep(10);
 	}
+
+	/* Wait the sensors to be valid or the core will warn the user */
+	armada_wait_sensor_validity(priv);
 }
 
 static void armada_ap806_init_sensor(struct platform_device *pdev,

drivers/thermal/armada_thermal.c +1 -0

 */
+	armada_wait_sensor_validity(priv);
 }
 

static bool armada_is_valid(struct armada_thermal_priv *priv)

Signature · 1 line

2.11.0

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

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

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

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/thermal/armada_thermal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/thermal/armada_thermal.c +2 -2

index 4a5164ddffe7..42ef80b3b5f8 100644
@@ -403,8 +403,8 @@ static int armada_thermal_probe(struct platform_device *pdev)
 
 	priv->data->init_sensor(pdev, priv);
 
-	thermal = thermal_zone_device_register("armada_thermal", 0, 0,
-					       priv, &ops, NULL, 0, 0);
+	thermal = thermal_zone_device_register(dev_name(&pdev->dev), 0, 0, priv,
+					       &ops, NULL, 0, 0);
 	if (IS_ERR(thermal)) {
 		dev_err(&pdev->dev,
 			"Failed to register thermal zone device\n");

Signature · 1 line

2.11.0

Add thermal DT nodes in AP806 and CP110 master/slave DTSI files.

Suggested-by David Sniatkiwicz <[email protected]>

Signed-off-by Miquel Raynal <[email protected]>

---
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi        | 6 ++++++
 arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 6 ++++++
 arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi  | 6 ++++++
 3 files changed, 18 insertions(+)

arch/arm64/boot/dts/marvell/armada-ap806.dtsi +6 -0

index 1c4dd8ab9ad5..bbc5a4d3acac 100644
@@ -285,6 +285,12 @@
 					gpio-ranges = <&ap_pinctrl 0 0 20>;
 				};
 			};
+
+			ap_thermal: thermal at 6f808C {
+				compatible = "marvell,armada-ap806-thermal";
+				reg = <0x6f808C 0x4>,
+				      <0x6f8084 0x8>;
+			};
 		};
 	};
 };

arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi +6 -0

index e3b64d03fbd8..ecbc76d26dff 100644
@@ -182,6 +182,12 @@
 				interrupts = <ICU_GRP_NSR 77 IRQ_TYPE_LEVEL_HIGH>;
 			};
 
+			cpm_thermal: thermal at 400078 {
+				compatible = "marvell,armada-cp110-thermal";
+				reg = <0x400078 0x4>,
+				      <0x400070 0x8>;
+			};
+
 			cpm_syscon0: system-controller at 440000 {
 				compatible = "syscon", "simple-mfd";
 				reg = <0x440000 0x2000>;

arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi +6 -0

index 0d51096c69f8..29ba32c68870 100644
@@ -182,6 +182,12 @@
 				interrupts = <ICU_GRP_NSR 77 IRQ_TYPE_LEVEL_HIGH>;
 			};
 
+			cps_thermal: thermal at 400078 {
+				compatible = "marvell,armada-cp110-thermal";
+				reg = <0x400078 0x4>,
+				      <0x400070 0x8>;
+			};
+
 			cps_syscon0: system-controller at 440000 {
 				compatible = "syscon", "simple-mfd";
 				reg = <0x440000 0x2000>;

Signature · 1 line

2.11.0