Armada thermal: improvements and A7K/A8K SoCs support
Subsystems (3)
v6 cover Armada thermal: improvements and A7K/A8K SoCs support
Hi,
This series takes over Baruch's series by integrating his patches about supporting thermal on Armada 7K and 8K SoCs within a larger series with several improvements on the armada_thermal.c driver.
For now, Armada 380 and CP110 compatibles share the same initialization routine but this will probably change in the near future when adding support for overheat interrupts.
DT bindings documentation is updated to match existing code.
Armada AP806 and CP110 DT are also updated with thermal nodes and have already been accepted (thus absent in this series).
Thank you,
Miqu?l
Changes since v5:
- Added Grogory's Tested-by tags for 64-bit platforms in patches:
"thermal: armada: Add support for Armada AP806" "thermal: armada: Add support for Armada CP110"
- Fixed a compilation issue with 32-bit toolchains introduced in patch:
"thermal: armada: Add support for Armada AP806"
Baruch Siach (4):
dt-bindings: thermal: Describe Armada AP806 and CP110
thermal: armada: Use msleep for long delays
thermal: armada: Add support for Armada AP806
thermal: armada: Add support for Armada CP110
Miquel Raynal (7):
thermal: armada: Simplify the check of the validity bit
thermal: armada: Clarify control registers accesses
thermal: armada: Use real status register name
thermal: armada: Update Kconfig and module description
thermal: armada: Change sensors trim default value
thermal: armada: Wait sensors validity before exiting the init
callback
thermal: armada: Give meaningful names to the thermal zones
.../devicetree/bindings/thermal/armada-thermal.txt | 37 ++- drivers/thermal/Kconfig | 4 +- drivers/thermal/armada_thermal.c | 257 +++++++++++++++------ 3 files changed, 218 insertions(+), 80 deletions(-)
Signature · 1 line
2.11.0
PATCH v6 1/11 dt-bindings: thermal: Describe Armada AP806 and CP110
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.
--- .../devicetree/bindings/thermal/armada-thermal.txt | 37 +++++++++++++++------- 1 file changed, 25 insertions(+), 12 deletions(-)
Documentation/devicetree/bindings/thermal/armada-thermal.txt +25 -12
@@ -2,22 +2,35 @@ Required properties: -- compatible: Should be set to one of the following: - marvell,armada370-thermal - marvell,armada375-thermal - marvell,armada380-thermal - marvell,armadaxp-thermal +- compatible: Should be set to one of the following: + * marvell,armada370-thermal + * marvell,armada375-thermal + * marvell,armada380-thermal + * marvell,armadaxp-thermal + * marvell,armada-ap806-thermal + * marvell,armada-cp110-thermal -- reg: Device's register space. - Two entries are expected, see the examples below. - The first one is required for the sensor register; - the second one is required for the control register - to be used for sensor initialization (a.k.a. calibration). +- reg: Device's register space. + Two entries are expected, see the examples below. The first one points + to the status register (4B). The second one points to the control + registers (8B). + Note: The compatibles marvell,armada370-thermal, + marvell,armada380-thermal, and marvell,armadaxp-thermal must point to + "control MSB/control 1", with size of 4 (deprecated binding), or point + to "control LSB/control 0" with size of 8 (current binding). All other + compatibles must point to "control LSB/control 0" with size of 8. -Example: +Examples: + /* Legacy bindings */ thermal at d0018300 { compatible = "marvell,armada370-thermal"; - reg = <0xd0018300 0x4 + reg = <0xd0018300 0x4 0xd0018304 0x4>; }; + + ap_thermal: thermal at 6f8084 { + compatible = "marvell,armada-ap806-thermal"; + reg = <0x6f808C 0x4>, + <0x6f8084 0x8>; + };
Signature · 1 line
2.11.0
PATCH v6 1/11 dt-bindings: thermal: Describe Armada AP806 and CP110
Hi Rob,
Quoted 5 lines · On Fri, 22 Dec 2017 10:32:16 +0100 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.
Quoted 8 lines
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]>
I forgot to add your:
in this series, sorry about that.
Kind regards,
Miqu?l
Quoted 61 lines
--- .../devicetree/bindings/thermal/armada-thermal.txt | 37+++++++++++++++------- 1 file changed, 25 insertions(+), 12 deletions(-)
diff --git
a/Documentation/devicetree/bindings/thermal/armada-thermal.txtb/Documentation/devicetree/bindings/thermal/armada-thermal.txtindex24aacf8948c5..e0d013a2e66d100644 ---a/Documentation/devicetree/bindings/thermal/armada-thermal.txt+++b/Documentation/devicetree/bindings/thermal/armada-thermal.txt@@ -2,22 +2,35 @@ Required properties:-- compatible: Should be set to one of the following:
Documentation/devicetree/bindings/thermal/armada-thermal.txt +7
-9- marvell,armada370-thermal - marvell,armada375-thermal - marvell,armada380-thermal - marvell,armadaxp-thermal +- compatible: Should be set to one of the following: + * marvell,armada370-thermal + * marvell,armada375-thermal + * marvell,armada380-thermal + * marvell,armadaxp-thermal + * marvell,armada-ap806-thermal + * marvell,armada-cp110-thermal -- reg: Device's register space. - Two entries are expected, see the examples below. - The first one is required for the sensor register; - the second one is required for the control register - to be used for sensor initialization (a.k.a.calibration). +- reg: Device's register space.
+ Two entries are expected, see the examples below. The first one pointsDocumentation/devicetree/bindings/thermal/armada-thermal.txt +4
-0+ to the status register (4B). The second one points to the control + registers (8B). + Note: The compatibles marvell,armada370-thermal, + marvell,armada380-thermal, and marvell,armadaxp-thermal must pointto
+ "control MSB/control 1", with size of 4 (deprecated binding), or point
+ to "control LSB/control 0" with size of 8 (current binding). All otherDocumentation/devicetree/bindings/thermal/armada-thermal.txt +10
-2+ compatibles must point to "control LSB/control 0" with size of 8. -Example: +Examples: + /* Legacy bindings */ thermal at d0018300 { compatible = "marvell,armada370-thermal"; - reg = <0xd0018300 0x4 + reg = <0xd0018300 0x4 0xd0018304 0x4>; }; + + ap_thermal: thermal at 6f8084 { + compatible = "marvell,armada-ap806-thermal"; + reg = <0x6f808C 0x4>, + <0x6f8084 0x8>; + };
Signature · 3 lines
Miquel Raynal, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH v6 2/11 thermal: armada: Use msleep for long delays
In reply to Miquel RaynalFrom: Baruch Siach <[email protected]>
Use msleep for long (> 10ms) delays, instead of the busy waiting mdelay. All delays are called from the probe routine, where scheduling is allowed.
--- drivers/thermal/armada_thermal.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/thermal/armada_thermal.c +4 -4
@@ -113,7 +113,7 @@ static void armada370_init_sensor(struct platform_device *pdev, reg &= ~PMU_TDC0_START_CAL_MASK ;writel (reg, priv->control); -mdelay (10 ); +msleep (10 ); } static void armada375_init_sensor(struct platform_device *pdev,
@@ -127,11 +127,11 @@ static void armada375_init_sensor(struct platform_device *pdev, reg &= ~A375_HW_RESETn;writel (reg, priv->control +4 ); -mdelay (20 ); +msleep (20 ); reg |= A375_HW_RESETn;writel (reg, priv->control +4 ); -mdelay (50 ); +msleep (50 ); } static void armada380_init_sensor(struct platform_device *pdev,
@@ -143,7 +143,7 @@ static void armada380_init_sensor(struct platform_device *pdev,if (!(reg &A380_HW_RESET )) { reg |=A380_HW_RESET ;writel (reg, priv->control); -mdelay (10 ); +msleep (10 ); } }
Signature · 1 line
2.11.0
PATCH v6 3/11 thermal: armada: Simplify the check of the validity bit
In reply to Miquel RaynalAll Armada SoCs use one bit to declare if the sensor values are valid. This bit moves across the versions of the IP.
The method until then was to do both a shift and compare with an useless flag of "0x1". It is clearer and quicker to directly save the value that must be ANDed instead of the bit position and do a single bitwise AND operation.
--- drivers/thermal/armada_thermal.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/thermal/armada_thermal.c +6 -8
@@ -24,8 +24,6 @@#include <linux/of_device.h> #include <linux/thermal.h> -#define THERMAL_VALID_MASK 0x1 -/* Thermal Manager Control and Status Register */ #define PMU_TDC0_SW_RST_MASK (0x1 << 1)#define PMU_TM_DISABLE_OFFS 0
@@ -67,7 +65,7 @@ struct armada_thermal_data {/* Register shift and mask to access the sensor temperature */ unsigned int temp_shift;unsigned int temp_mask; -unsigned int is_valid_shift; +u32 is_valid_bit; }; static void armadaxp_init_sensor(struct platform_device *pdev,
@@ -149,9 +147,9 @@ static void armada380_init_sensor(struct platform_device *pdev,static bool armada_is_valid (struct armada_thermal_priv *priv) { -unsigned long reg =readl_relaxed (priv->sensor); +u32 reg =readl_relaxed (priv->sensor); -return (reg >> priv->data->is_valid_shift) &THERMAL_VALID_MASK ; +return reg & priv->data->is_valid_bit; } static int armada_get_temp(struct thermal_zone_device *thermal,
@@ -199,7 +197,7 @@ static const struct armada_thermal_data armadaxp_data = {static const struct armada_thermal_data armada370_data = { .is_valid = armada_is_valid, .init_sensor = armada370_init_sensor, - .is_valid_shift =9 , + .is_valid_bit =BIT (9 ), .temp_shift =10 , .temp_mask =0x1ff , .coef_b =3153000000UL ,
@@ -210,7 +208,7 @@ static const struct armada_thermal_data armada370_data = {static const struct armada_thermal_data armada375_data = { .is_valid = armada_is_valid, .init_sensor = armada375_init_sensor, - .is_valid_shift =10 , + .is_valid_bit =BIT (10 ), .temp_shift =0 , .temp_mask =0x1ff , .coef_b =3171900000UL ,
@@ -221,7 +219,7 @@ static const struct armada_thermal_data armada375_data = {static const struct armada_thermal_data armada380_data = { .is_valid = armada_is_valid, .init_sensor = armada380_init_sensor, - .is_valid_shift =10 , + .is_valid_bit =BIT (10 ), .temp_shift =0 , .temp_mask =0x3ff , .coef_b =1172499100UL ,
Signature · 1 line
2.11.0
PATCH v6 4/11 thermal: armada: Clarify control registers accesses
In reply to Miquel RaynalBindings 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.
--- drivers/thermal/armada_thermal.c | 76 ++++++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 22 deletions(-)
drivers/thermal/armada_thermal.c +54 -22
@@ -39,12 +39,21 @@#define A375_HW_RESETn BIT(8)#define A380_HW_RESET BIT(8) +/* Legacy bindings */ +#define LEGACY_CONTROL_MEM_LEN 0x4 + +/* Current bindings with the 2 control registers under the same memory area */ +#define LEGACY_CONTROL1_OFFSET 0x0 +#define CONTROL0_OFFSET 0x0 +#define CONTROL1_OFFSET 0x4 +struct armada_thermal_data ;/* Marvell EBU Thermal Sensor Dev Structure */ struct armada_thermal_priv {void __iomem *sensor; -void __iomem *control; +void __iomem *control0; +void __iomem *control1;struct armada_thermal_data *data; };
@@ -66,27 +75,28 @@ struct armada_thermal_data {unsigned int temp_shift;unsigned int temp_mask;u32 is_valid_bit; +bool needs_control0; };static void armadaxp_init_sensor (struct platform_device *pdev,struct armada_thermal_priv *priv) { -unsigned long reg; +u32 reg; - reg =readl_relaxed (priv->control); + reg =readl_relaxed (priv->control1); reg |=PMU_TDC0_OTF_CAL_MASK ; -writel (reg, priv->control); +writel (reg, priv->control1);/* Reference calibration value */ reg &= ~PMU_TDC0_REF_CAL_CNT_MASK ; reg |= (0xf1 <<PMU_TDC0_REF_CAL_CNT_OFFS ); -writel (reg, priv->control); +writel (reg, priv->control1);/* Reset the sensor */ - reg =readl_relaxed (priv->control); -writel ((reg |PMU_TDC0_SW_RST_MASK ), priv->control); + reg =readl_relaxed (priv->control1); +writel ((reg |PMU_TDC0_SW_RST_MASK ), priv->control1); -writel (reg, priv->control); +writel (reg, priv->control1);/* Enable the sensor */ reg =readl_relaxed (priv->sensor);
@@ -97,19 +107,19 @@ static void armadaxp_init_sensor(struct platform_device *pdev,static void armada370_init_sensor (struct platform_device *pdev,struct armada_thermal_priv *priv) { -unsigned long reg; +u32 reg; - reg =readl_relaxed (priv->control); + reg =readl_relaxed (priv->control1); reg |=PMU_TDC0_OTF_CAL_MASK ; -writel (reg, priv->control); +writel (reg, priv->control1);/* Reference calibration value */ reg &= ~PMU_TDC0_REF_CAL_CNT_MASK ; reg |= (0xf1 <<PMU_TDC0_REF_CAL_CNT_OFFS ); -writel (reg, priv->control); +writel (reg, priv->control1); reg &= ~PMU_TDC0_START_CAL_MASK ; -writel (reg, priv->control); +writel (reg, priv->control1);msleep (10 ); }
@@ -117,30 +127,30 @@ static void armada370_init_sensor(struct platform_device *pdev,static void armada375_init_sensor (struct platform_device *pdev,struct armada_thermal_priv *priv) { -unsigned long reg; +u32 reg; - reg =readl (priv->control +4 ); + reg =readl (priv->control1); reg &= ~(A375_UNIT_CONTROL_MASK <<A375_UNIT_CONTROL_SHIFT ); reg &= ~A375_READOUT_INVERT ; reg &= ~A375_HW_RESETn; -writel (reg, priv->control +4 ); +writel (reg, priv->control1);msleep (20 ); reg |= A375_HW_RESETn; -writel (reg, priv->control +4 ); +writel (reg, priv->control1);msleep (50 ); }static void armada380_init_sensor (struct platform_device *pdev,struct armada_thermal_priv *priv) { -unsigned long reg =readl_relaxed (priv->control); +u32 reg =readl_relaxed (priv->control1);/* Reset hardware once */ if (!(reg &A380_HW_RESET )) { reg |=A380_HW_RESET ; -writel (reg, priv->control); +writel (reg, priv->control1);msleep (10 ); } }
@@ -214,6 +224,7 @@ static const struct armada_thermal_data armada375_data = { .coef_b = 3171900000UL, .coef_m = 10000000UL, .coef_div = 13616, + .needs_control0 = true, };static const struct armada_thermal_data armada380_data = {
@@ -253,6 +264,7 @@ MODULE_DEVICE_TABLE(of, armada_thermal_id_table);static int armada_thermal_probe (struct platform_device *pdev) { +void __iomem *control =NULL ;struct thermal_zone_device *thermal;const struct of_device_id *match;struct armada_thermal_priv *priv;
@@ -272,11 +284,31 @@ static int armada_thermal_probe(struct platform_device *pdev)return PTR_ERR (priv->sensor); res =platform_get_resource (pdev,IORESOURCE_MEM ,1 ); - priv->control =devm_ioremap_resource (&pdev->dev, res); -if (IS_ERR (priv->control)) -return PTR_ERR (priv->control); + control =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (control)) +return PTR_ERR (control); priv->data = (struct armada_thermal_data *)match->data; + +/* +* Legacy DT bindings only described "control1" register (also referred +* as "control MSB" on old documentation). New bindings cover +* "control0/control LSB" and "control1/control MSB" registers within +* the same resource, which is then of size 8 instead of 4. +*/ +if (resource_size (res) ==LEGACY_CONTROL_MEM_LEN ) { +/* ->control0 unavailable in this configuration */ +if (priv->data->needs_control0) { +dev_err (&pdev->dev,"No access to control0 register\n" ); +return -EINVAL ; + } + + priv->control1 = control +LEGACY_CONTROL1_OFFSET ; + }else { + priv->control0 = control +CONTROL0_OFFSET ; + priv->control1 = control +CONTROL1_OFFSET ; + } + priv->data->init_sensor (pdev, priv); thermal = thermal_zone_device_register("armada_thermal", 0, 0,
Signature · 1 line
2.11.0
PATCH v6 4/11 thermal: armada: Clarify control registers accesses
Hi Miquel,
Quoted 6 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.
Quoted 8 lines
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]>
I tested that there was no regression on an Armada XP based board:
PlatHome OpenBlocks AX3-4, and on an Armada 388 one: SolidRun Clearfog A1. For both of them it was OK:
Gregory
Quoted 183 lines
--- drivers/thermal/armada_thermal.c | 76 ++++++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 22 deletions(-)drivers/thermal/armada_thermal.c +54
-22@@ -39,12 +39,21 @@#define A375_HW_RESETn BIT(8)#define A380_HW_RESET BIT(8) +/* Legacy bindings */ +#define LEGACY_CONTROL_MEM_LEN 0x4 + +/* Current bindings with the 2 control registers under the same memory area */ +#define LEGACY_CONTROL1_OFFSET 0x0 +#define CONTROL0_OFFSET 0x0 +#define CONTROL1_OFFSET 0x4 +struct armada_thermal_data ;/* Marvell EBU Thermal Sensor Dev Structure */ struct armada_thermal_priv {void __iomem *sensor; -void __iomem *control; +void __iomem *control0; +void __iomem *control1;struct armada_thermal_data *data; };@@ -66,27 +75,28 @@ struct armada_thermal_data {unsigned int temp_shift;unsigned int temp_mask;u32 is_valid_bit; +bool needs_control0; };static void armadaxp_init_sensor (struct platform_device *pdev,struct armada_thermal_priv *priv) { -unsigned long reg; +u32 reg; - reg =readl_relaxed (priv->control); + reg =readl_relaxed (priv->control1); reg |=PMU_TDC0_OTF_CAL_MASK ; -writel (reg, priv->control); +writel (reg, priv->control1);/* Reference calibration value */ reg &= ~PMU_TDC0_REF_CAL_CNT_MASK ; reg |= (0xf1 <<PMU_TDC0_REF_CAL_CNT_OFFS ); -writel (reg, priv->control); +writel (reg, priv->control1);/* Reset the sensor */ - reg =readl_relaxed (priv->control); -writel ((reg |PMU_TDC0_SW_RST_MASK ), priv->control); + reg =readl_relaxed (priv->control1); +writel ((reg |PMU_TDC0_SW_RST_MASK ), priv->control1); -writel (reg, priv->control); +writel (reg, priv->control1);/* Enable the sensor */ reg =readl_relaxed (priv->sensor);@@ -97,19 +107,19 @@ static void armadaxp_init_sensor(struct platform_device *pdev,static void armada370_init_sensor (struct platform_device *pdev,struct armada_thermal_priv *priv) { -unsigned long reg; +u32 reg; - reg =readl_relaxed (priv->control); + reg =readl_relaxed (priv->control1); reg |=PMU_TDC0_OTF_CAL_MASK ; -writel (reg, priv->control); +writel (reg, priv->control1);/* Reference calibration value */ reg &= ~PMU_TDC0_REF_CAL_CNT_MASK ; reg |= (0xf1 <<PMU_TDC0_REF_CAL_CNT_OFFS ); -writel (reg, priv->control); +writel (reg, priv->control1); reg &= ~PMU_TDC0_START_CAL_MASK ; -writel (reg, priv->control); +writel (reg, priv->control1);msleep (10 ); }@@ -117,30 +127,30 @@ static void armada370_init_sensor(struct platform_device *pdev,static void armada375_init_sensor (struct platform_device *pdev,struct armada_thermal_priv *priv) { -unsigned long reg; +u32 reg; - reg =readl (priv->control +4 ); + reg =readl (priv->control1); reg &= ~(A375_UNIT_CONTROL_MASK <<A375_UNIT_CONTROL_SHIFT ); reg &= ~A375_READOUT_INVERT ; reg &= ~A375_HW_RESETn; -writel (reg, priv->control +4 ); +writel (reg, priv->control1);msleep (20 ); reg |= A375_HW_RESETn; -writel (reg, priv->control +4 ); +writel (reg, priv->control1);msleep (50 ); }static void armada380_init_sensor (struct platform_device *pdev,struct armada_thermal_priv *priv) { -unsigned long reg =readl_relaxed (priv->control); +u32 reg =readl_relaxed (priv->control1);/* Reset hardware once */ if (!(reg &A380_HW_RESET )) { reg |=A380_HW_RESET ; -writel (reg, priv->control); +writel (reg, priv->control1);msleep (10 ); } }@@ -214,6 +224,7 @@ static const struct armada_thermal_data armada375_data = { .coef_b = 3171900000UL, .coef_m = 10000000UL, .coef_div = 13616, + .needs_control0 = true, };static const struct armada_thermal_data armada380_data = {@@ -253,6 +264,7 @@ MODULE_DEVICE_TABLE(of, armada_thermal_id_table);static int armada_thermal_probe (struct platform_device *pdev) { +void __iomem *control =NULL ;struct thermal_zone_device *thermal;const struct of_device_id *match;struct armada_thermal_priv *priv;@@ -272,11 +284,31 @@ static int armada_thermal_probe(struct platform_device *pdev)return PTR_ERR (priv->sensor); res =platform_get_resource (pdev,IORESOURCE_MEM ,1 ); - priv->control =devm_ioremap_resource (&pdev->dev, res); -if (IS_ERR (priv->control)) -return PTR_ERR (priv->control); + control =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (control)) +return PTR_ERR (control); priv->data = (struct armada_thermal_data *)match->data; + +/* +* Legacy DT bindings only described "control1" register (also referred +* as "control MSB" on old documentation). New bindings cover +* "control0/control LSB" and "control1/control MSB" registers within +* the same resource, which is then of size 8 instead of 4. +*/ +if (resource_size (res) ==LEGACY_CONTROL_MEM_LEN ) { +/* ->control0 unavailable in this configuration */ +if (priv->data->needs_control0) { +dev_err (&pdev->dev,"No access to control0 register\n" ); +return -EINVAL ; + } + + priv->control1 = control +LEGACY_CONTROL1_OFFSET ; + }else { + priv->control0 = control +CONTROL0_OFFSET ; + priv->control1 = control +CONTROL1_OFFSET ; + } + priv->data->init_sensor (pdev, priv); thermal = thermal_zone_device_register("armada_thermal", 0, 0,Signature · 1 line
2.11.0
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH v6 5/11 thermal: armada: Use real status register name
In reply to Miquel RaynalThree 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.
--- drivers/thermal/armada_thermal.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/thermal/armada_thermal.c +8 -8
@@ -51,7 +51,7 @@ struct armada_thermal_data;/* Marvell EBU Thermal Sensor Dev Structure */ struct armada_thermal_priv { -void __iomem *sensor; +void __iomem *status;void __iomem *control0;void __iomem *control1;struct armada_thermal_data *data;
@@ -99,9 +99,9 @@ static void armadaxp_init_sensor(struct platform_device *pdev,writel (reg, priv->control1);/* Enable the sensor */ - reg =readl_relaxed (priv->sensor); + reg =readl_relaxed (priv->status); reg &= ~PMU_TM_DISABLE_MASK ; -writel (reg, priv->sensor); +writel (reg, priv->status); } static void armada370_init_sensor(struct platform_device *pdev,
@@ -157,7 +157,7 @@ static void armada380_init_sensor(struct platform_device *pdev,static bool armada_is_valid (struct armada_thermal_priv *priv) { -u32 reg =readl_relaxed (priv->sensor); +u32 reg =readl_relaxed (priv->status);return reg & priv->data->is_valid_bit; }
@@ -176,7 +176,7 @@ static int armada_get_temp(struct thermal_zone_device *thermal,return -EIO ; } - reg =readl_relaxed (priv->sensor); + reg =readl_relaxed (priv->status); reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;/* Get formula coeficients */
@@ -279,9 +279,9 @@ static int armada_thermal_probe(struct platform_device *pdev)return -ENOMEM ; res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); - priv->sensor =devm_ioremap_resource (&pdev->dev, res); -if (IS_ERR (priv->sensor)) -return PTR_ERR (priv->sensor); + priv->status =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (priv->status)) +return PTR_ERR (priv->status); res =platform_get_resource (pdev,IORESOURCE_MEM ,1 ); control =devm_ioremap_resource (&pdev->dev, res);
Signature · 1 line
2.11.0
PATCH v6 6/11 thermal: armada: Add support for Armada AP806
In reply to Miquel RaynalFrom: 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.
[<[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 '/']
--- drivers/thermal/armada_thermal.c | 78 +++++++++++++++++++++++++++++++--------- 1 file changed, 62 insertions(+), 16 deletions(-)
drivers/thermal/armada_thermal.c +62 -16
@@ -47,6 +47,11 @@#define CONTROL0_OFFSET 0x0#define CONTROL1_OFFSET 0x4 +/* TSEN refers to the temperature sensors within the AP */ +#define CONTROL0_TSEN_START BIT(0) +#define CONTROL0_TSEN_RESET BIT(1) +#define CONTROL0_TSEN_ENABLE BIT(2) +struct armada_thermal_data ;/* Marvell EBU Thermal Sensor Dev Structure */
@@ -66,10 +71,11 @@ struct armada_thermal_data {bool (*is_valid)(struct armada_thermal_priv *);/* Formula coeficients: temp = (b - m * reg) / div */ -unsigned long coef_b; -unsigned long coef_m; -unsigned long coef_div; +s64 coef_b; +s64 coef_m; +u32 coef_div;bool inverted; +bool signed_sample;/* Register shift and mask to access the sensor temperature */ unsigned int temp_shift;
@@ -155,6 +161,18 @@ static void armada380_init_sensor(struct platform_device *pdev, } } +static void armada_ap806_init_sensor (struct platform_device *pdev, +struct armada_thermal_priv *priv) +{ +u32 reg; + + reg =readl_relaxed (priv->control0); + reg &= ~CONTROL0_TSEN_RESET ; + reg |=CONTROL0_TSEN_START |CONTROL0_TSEN_ENABLE ; +writel (reg, priv->control0); +msleep (10 ); +} +static bool armada_is_valid (struct armada_thermal_priv *priv) {u32 reg =readl_relaxed (priv->status);
@@ -163,11 +181,12 @@ static bool armada_is_valid(struct armada_thermal_priv *priv) }static int armada_get_temp (struct thermal_zone_device *thermal, -int *temp) +int *temperature) {struct armada_thermal_priv *priv = thermal->devdata; -unsigned long reg; -unsigned long m, b, div; +u32 reg, div; +s64 sample, b, m; +u64 tmp;/* Valid check */ if (priv->data->is_valid && !priv->data->is_valid (priv)) {
@@ -178,6 +197,11 @@ static int armada_get_temp(struct thermal_zone_device *thermal, reg =readl_relaxed (priv->status); reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask; +if (priv->data->signed_sample) +/* The most significant bit is the sign bit */ + sample =sign_extend32 (reg,fls (priv->data->temp_mask) -1 ); +else + sample = reg;/* Get formula coeficients */ b = priv->data->coef_b;
@@ -185,9 +209,13 @@ static int armada_get_temp(struct thermal_zone_device *thermal, div = priv->data->coef_div;if (priv->data->inverted) - *temp = ((m * reg) - b) / div; + tmp = (m * sample) - b;else - *temp = (b - (m * reg)) / div; + tmp = b - (m * sample); + +do_div (tmp, div); + *temperature = (int )tmp; +return 0 ; }
@@ -199,8 +227,8 @@ static const struct armada_thermal_data armadaxp_data = { .init_sensor = armadaxp_init_sensor, .temp_shift = 10, .temp_mask = 0x1ff, - .coef_b = 3153000000UL, - .coef_m = 10000000UL, + .coef_b = 3153000000ULL, + .coef_m = 10000000ULL, .coef_div = 13825, };
@@ -210,8 +238,8 @@ static const struct armada_thermal_data armada370_data = { .is_valid_bit = BIT(9), .temp_shift = 10, .temp_mask = 0x1ff, - .coef_b = 3153000000UL, - .coef_m = 10000000UL, + .coef_b = 3153000000ULL, + .coef_m = 10000000ULL, .coef_div = 13825, };
@@ -221,8 +249,8 @@ static const struct armada_thermal_data armada375_data = { .is_valid_bit = BIT(10), .temp_shift = 0, .temp_mask = 0x1ff, - .coef_b = 3171900000UL, - .coef_m = 10000000UL, + .coef_b = 3171900000ULL, + .coef_m = 10000000ULL, .coef_div = 13616, .needs_control0 = true, };
@@ -233,12 +261,26 @@ static const struct armada_thermal_data armada380_data = { .is_valid_bit = BIT(10), .temp_shift = 0, .temp_mask = 0x3ff, - .coef_b = 1172499100UL, - .coef_m = 2000096UL, + .coef_b = 1172499100ULL, + .coef_m = 2000096ULL, .coef_div = 4201, .inverted = true, }; +static const struct armada_thermal_data armada_ap806_data = { + .is_valid = armada_is_valid, + .init_sensor = armada_ap806_init_sensor, + .is_valid_bit =BIT (16 ), + .temp_shift =0 , + .temp_mask =0x3ff , + .coef_b =-150000LL , + .coef_m =423ULL , + .coef_div =1 , + .inverted = true, + .signed_sample = true, + .needs_control0 = true, +}; +static const struct of_device_id armada_thermal_id_table[] = { { .compatible ="marvell,armadaxp-thermal" ,
@@ -257,6 +299,10 @@ static const struct of_device_id armada_thermal_id_table[] = { .data = &armada380_data, }, { + .compatible = "marvell,armada-ap806-thermal", + .data = &armada_ap806_data, + }, + {/* sentinel */ }, };
Signature · 1 line
2.11.0
PATCH v6 6/11 thermal: armada: Add support for Armada AP806
Hi Miqu?l,
Quoted 18 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.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_divinstead of the traditionnal '/']Signed-off-by Miquel Raynal <[email protected]>Reviewed-by Gregory CLEMENT <[email protected]>Tested-by Gregory CLEMENT <[email protected]>---
[..]
Quoted 27 lines
static int armada_get_temp(struct
thermal_zone_device*thermal,drivers/thermal/armada_thermal.c +9
-3- int *temp) + int *temperature) {struct armada_thermal_priv *priv = thermal->devdata; -unsigned long reg; -unsigned long m, b, div; +u32 reg, div; +s64 sample, b, m; +u64 tmp;/* Valid check */ if (priv->data->is_valid && !priv->data->is_valid (priv)) {@@ -178,6 +197,11 @@ static int armada_get_temp(struct thermal_zone_device *thermal, reg =readl_relaxed (priv->status); reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask; +if (priv->data->signed_sample) +/* The most significant bit is the sign bit */ + sample =sign_extend32 (reg,fls (priv->data->temp_mask) -1 ); +else + sample = reg;/* Get formula coeficients */ b = priv->data->coef_b;@@ -185,9 +209,13 @@ static int armada_get_temp(struct thermal_zone_device *thermal, div = priv->data->coef_div;
drivers/thermal/armada_thermal.c static int armada_get_temp(struct thermal_zone_device *thermal, · line 217
Quoted 10 lines
drivers/thermal/armada_thermal.c +5
-2if (priv->data->inverted) - *temp = ((m * reg) - b) / div; + tmp = (m * sample) - b;else - *temp = (b - (m * reg)) / div; + tmp = b - (m * sample); + +do_div (tmp, div); + *temperature = (int )tmp;
Nitpick: why not (untested)
#include <linux/math64.h>
if (priv->data->inverted)
*temp = div_s64((m * sample) - b, div);
else
*temp = div_s64(b - (m * sample), div);baruch
Quoted 3 lines
drivers/thermal/armada_thermal.c +1
-0+return 0 ; }
Signature · 3 lines
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -PATCH v6 6/11 thermal: armada: Add support for Armada AP806
Hi Baruch,
Quoted 66 lines · On Fri, 22 Dec 2017 12:14:26 +0200 Baruch Siach <[email protected]> wrote:
Hi Miqu?l,
Quoted 18 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.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_divinstead of the traditionnal '/'] Signed-off-by: Miquel Raynal <[email protected]> Reviewed-by: Gregory CLEMENT <[email protected]> Tested-by: Gregory CLEMENT <[email protected]> ---[..]
Quoted 29 lines
static int armada_get_temp(struct
thermal_zone_device*thermal,drivers/thermal/armada_thermal.c +9
-3- int *temp) + int *temperature) {struct armada_thermal_priv *priv = thermal->devdata; -unsigned long reg; -unsigned long m, b, div; +u32 reg, div; +s64 sample, b, m; +u64 tmp;/* Valid check */ if (priv->data->is_valid && !priv->data->is_valid (priv)) {@@ -178,6 +197,11 @@ static int armada_get_temp(structthermal_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 +0
-0/* Get formula coeficients */ b = priv->data->coef_b;@@ -185,9 +209,13 @@ static int armada_get_temp(structthermal_zone_device *thermal, div = priv->data->coef_div;drivers/thermal/armada_thermal.c static int armada_get_temp(struct · line 217
Quoted 10 lines
drivers/thermal/armada_thermal.c +5
-2if (priv->data->inverted) - *temp = ((m * reg) - b) / div; + tmp = (m * sample) - b;else - *temp = (b - (m * reg)) / div; + tmp = b - (m * sample); + +do_div (tmp, div); + *temperature = (int )tmp;Nitpick: why not (untested)
Quoted 6 lines
#include <
linux/math64.h>if (priv->data->inverted) *temp = div_s64((m * sample) - b, div); else *temp = div_s64(b - (m * sample), div);
Indeed I could also use div_s64, but the result must be unsigned anyway.
But this does all the operations on the same line, maybe this is more readable, I will update it and send (hopefully) the last version :)
Cheers,
Miqu?l
Quoted 7 lines
baruch
Quoted 3 lines
drivers/thermal/armada_thermal.c +1
-0+return 0 ; }
Signature · 3 lines
Miquel Raynal, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH v6 6/11 thermal: armada: Add support for Armada AP806
Hi Miqu?l,
Quoted 66 lines · Miquel RAYNAL
Quoted 64 lines · On Fri, 22 Dec 2017 12:14:26 +0200 Baruch Siach <[email protected]> wrote:
Quoted 18 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.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_divinstead of the traditionnal '/'] Signed-off-by: Miquel Raynal <[email protected]> Reviewed-by: Gregory CLEMENT <[email protected]> Tested-by: Gregory CLEMENT <[email protected]> ---[..]
Quoted 29 lines
static int armada_get_temp(struct
thermal_zone_device*thermal,drivers/thermal/armada_thermal.c +9
-3- int *temp) + int *temperature) {struct armada_thermal_priv *priv = thermal->devdata; -unsigned long reg; -unsigned long m, b, div; +u32 reg, div; +s64 sample, b, m; +u64 tmp;/* Valid check */ if (priv->data->is_valid && !priv->data->is_valid (priv)) {@@ -178,6 +197,11 @@ static int armada_get_temp(structthermal_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 +0
-0/* Get formula coeficients */ b = priv->data->coef_b;@@ -185,9 +209,13 @@ static int armada_get_temp(structthermal_zone_device *thermal, div = priv->data->coef_div;drivers/thermal/armada_thermal.c static int armada_get_temp(struct · line 217
Quoted 10 lines
drivers/thermal/armada_thermal.c +5
-2if (priv->data->inverted) - *temp = ((m * reg) - b) / div; + tmp = (m * sample) - b;else - *temp = (b - (m * reg)) / div; + tmp = b - (m * sample); + +do_div (tmp, div); + *temperature = (int )tmp;Nitpick: why not (untested)
Quoted 8 lines
Quoted 6 lines
#include <
linux/math64.h>if (priv->data->inverted) *temp = div_s64((m * sample) - b, div); else *temp = div_s64(b - (m * sample), div);Indeed I could also use
div_s64, but the result must be unsigned anyway.
*temp is signed, as far as I can see. Temperature can go below zero, at least in the Celsius scale.
Quoted 2 lines
But this does all the operations on the same line, maybe this is more readable, I will update it and send (hopefully) the last version :)
baruch
Signature · 3 lines
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -PATCH v6 7/11 thermal: armada: Add support for Armada CP110
In reply to Miquel RaynalFrom: Baruch Siach <[email protected]>
The CP110 component is integrated in the Armada 8k and 7k lines of processors.
[<[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]
--- drivers/thermal/armada_thermal.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-)
drivers/thermal/armada_thermal.c +26 -7
@@ -37,7 +37,6 @@#define A375_UNIT_CONTROL_MASK 0x7#define A375_READOUT_INVERT BIT(15)#define A375_HW_RESETn BIT(8) -#define A380_HW_RESET BIT(8)/* Legacy bindings */ #define LEGACY_CONTROL_MEM_LEN 0x4
@@ -52,6 +51,10 @@#define CONTROL0_TSEN_RESET BIT(1)#define CONTROL0_TSEN_ENABLE BIT(2) +/* EXT_TSEN refers to the external temperature sensors, out of the AP */ +#define CONTROL1_EXT_TSEN_SW_RESET BIT(7) +#define CONTROL1_EXT_TSEN_HW_RESETn BIT(8) +struct armada_thermal_data ;/* Marvell EBU Thermal Sensor Dev Structure */
@@ -153,12 +156,11 @@ static void armada380_init_sensor(struct platform_device *pdev, {u32 reg =readl_relaxed (priv->control1); -/* Reset hardware once */ -if (!(reg &A380_HW_RESET )) { - reg |=A380_HW_RESET ; -writel (reg, priv->control1); -msleep (10 ); - } +/* Disable the HW/SW reset */ + reg |= CONTROL1_EXT_TSEN_HW_RESETn; + reg &= ~CONTROL1_EXT_TSEN_SW_RESET ; +writel (reg, priv->control1); +msleep (10 ); } static void armada_ap806_init_sensor(struct platform_device *pdev,
@@ -281,6 +283,19 @@ static const struct armada_thermal_data armada_ap806_data = { .needs_control0 = true, }; +static const struct armada_thermal_data armada_cp110_data = { + .is_valid = armada_is_valid, + .init_sensor = armada380_init_sensor, + .is_valid_bit =BIT (10 ), + .temp_shift =0 , + .temp_mask =0x3ff , + .coef_b =1172499100ULL , + .coef_m =2000096ULL , + .coef_div =4201 , + .inverted = true, + .needs_control0 = true, +}; +static const struct of_device_id armada_thermal_id_table[] = { { .compatible ="marvell,armadaxp-thermal" ,
@@ -303,6 +318,10 @@ static const struct of_device_id armada_thermal_id_table[] = { .data = &armada_ap806_data, }, { + .compatible = "marvell,armada-cp110-thermal", + .data = &armada_cp110_data, + }, + {/* sentinel */ }, };
Signature · 1 line
2.11.0
PATCH v6 8/11 thermal: armada: Update Kconfig and module description
In reply to Miquel RaynalUpdate 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).
--- drivers/thermal/Kconfig | 4 ++-- drivers/thermal/armada_thermal.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/thermal/Kconfig +2 -2
@@ -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
@@ -1,5 +1,5 @@/* -* Marvell Armada 370/XP thermal sensor driver +* Marvell EBU Armada SoCs thermal sensor driver * * Copyright (C) 2013 Marvell *
@@ -411,5 +411,5 @@ static struct platform_driver armada_thermal_driver = {module_platform_driver (armada_thermal_driver);MODULE_AUTHOR ("Ezequiel Garcia <[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
PATCH v6 9/11 thermal: armada: Change sensors trim default value
In reply to Miquel RaynalErrata #132698 highlights an error in the default value of Tc trim. Set this parameter to b'011.
--- drivers/thermal/armada_thermal.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
drivers/thermal/armada_thermal.c +13 -0
@@ -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
PATCH v6 10/11 thermal: armada: Wait sensors validity before exiting the init callback
In reply to Miquel RaynalThe 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.
--- drivers/thermal/armada_thermal.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
drivers/thermal/armada_thermal.c +20 -3
@@ -23,6 +23,7 @@#include <linux/platform_device.h> #include <linux/of_device.h> #include <linux/thermal.h> +#include <linux/iopoll.h> /* Thermal Manager Control and Status Register */ #define PMU_TDC0_SW_RST_MASK (0x1 << 1)
@@ -59,6 +60,9 @@#define CONTROL1_EXT_TSEN_SW_RESET BIT(7)#define CONTROL1_EXT_TSEN_HW_RESETn BIT(8) +#define STATUS_POLL_PERIOD_US 1000 +#define STATUS_POLL_TIMEOUT_US 100000 +struct armada_thermal_data ;/* Marvell EBU Thermal Sensor Dev Structure */
@@ -155,6 +159,16 @@ static void armada375_init_sensor(struct platform_device *pdev,msleep (50 ); } +static void armada_wait_sensor_validity (struct armada_thermal_priv *priv) +{ +u32 reg; + +readl_relaxed_poll_timeout (priv->status, reg, + reg & priv->data->is_valid_bit, +STATUS_POLL_PERIOD_US , +STATUS_POLL_TIMEOUT_US ); +} +static void armada380_init_sensor (struct platform_device *pdev,struct armada_thermal_priv *priv) {
@@ -164,7 +178,6 @@ static void armada380_init_sensor(struct platform_device *pdev, reg |= CONTROL1_EXT_TSEN_HW_RESETn; reg &= ~CONTROL1_EXT_TSEN_SW_RESET ;writel (reg, priv->control1); -msleep (10 );/* Set Tsen Tc Trim to correct default value (errata #132698) */ if (priv->control0) {
@@ -172,8 +185,10 @@ static void armada380_init_sensor(struct platform_device *pdev, reg &= ~CONTROL0_TSEN_TC_TRIM_MASK ; reg |=CONTROL0_TSEN_TC_TRIM_VAL ;writel (reg, priv->control0); -msleep (10 ); } + +/* Wait the sensors to be valid or the core will warn the user */ +armada_wait_sensor_validity (priv); } static void armada_ap806_init_sensor(struct platform_device *pdev,
@@ -185,7 +200,9 @@ static void armada_ap806_init_sensor(struct platform_device *pdev, reg &= ~CONTROL0_TSEN_RESET ; reg |=CONTROL0_TSEN_START |CONTROL0_TSEN_ENABLE ;writel (reg, priv->control0); -msleep (10 ); + +/* Wait the sensors to be valid or the core will warn the user */ +armada_wait_sensor_validity (priv); }static bool armada_is_valid (struct armada_thermal_priv *priv)
Signature · 1 line
2.11.0
PATCH v6 11/11 thermal: armada: Give meaningful names to the thermal zones
In reply to Miquel RaynalAfter 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.
--- drivers/thermal/armada_thermal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/thermal/armada_thermal.c +2 -2
@@ -406,8 +406,8 @@ static int armada_thermal_probe(struct platform_device *pdev) priv->data->init_sensor (pdev, priv); - thermal =thermal_zone_device_register ("armada_thermal" ,0 ,0 , - priv, &ops,NULL ,0 ,0 ); + thermal =thermal_zone_device_register (dev_name (&pdev->dev),0 ,0 , priv, + &ops,NULL ,0 ,0 );if (IS_ERR (thermal)) {dev_err (&pdev->dev,"Failed to register thermal zone device\n" );
Signature · 1 line
2.11.0
PATCH v6 11/11 thermal: armada: Give meaningful names to the thermal zones
Hi Miquel,
Quoted 9 lines · Miquel Raynal
After registration to the thermal core, sysfs will make one entry per instance of the driver in
/sys/class/thermal_zoneXand/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.
Quoted 6 lines
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]>Reviewed-by Gregory CLEMENT <[email protected]>
And here I also tested that when all the series was applied there was no regression on an Armada XP based board: PlatHome OpenBlocks AX3-4, and on an Armada 388 one: SolidRun Clearfog A1. hera again for both of them it was OK:
Gregory
Quoted 22 lines
--- drivers/thermal/armada_thermal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/thermal/armada_thermal.c +2
-2@@ -406,8 +406,8 @@ static int armada_thermal_probe(struct platform_device *pdev) priv->data->init_sensor (pdev, priv); - thermal =thermal_zone_device_register ("armada_thermal" ,0 ,0 , - priv, &ops,NULL ,0 ,0 ); + thermal =thermal_zone_device_register (dev_name (&pdev->dev),0 ,0 , priv, + &ops,NULL ,0 ,0 );if (IS_ERR (thermal)) {dev_err (&pdev->dev,"Failed to register thermal zone device\n" );Signature · 1 line
2.11.0
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH v6 11/11 thermal: armada: Give meaningful names to the thermal zones
Hello Gregory,
Quoted 19 lines · On Fri, 22 Dec 2017 16:36:04 +0100 Gregory CLEMENT…
Hi Miquel,
Quoted 14 lines · Miquel Raynal
After registration to the thermal core, sysfs will make one entry per instance of the driver in
/sys/class/thermal_zoneXand/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.
Quoted 7 lines
Quoted 2 lines
Signed-off-by Miquel Raynal <[email protected]>Reviewed-by Gregory CLEMENT <[email protected]>And here I also tested that when all the series was applied there was no regression on an Armada XP based board: PlatHome OpenBlocks AX3-4, and on an Armada 388 one: SolidRun Clearfog A1. hera again for both of them it was OK:
Thank you for testing, on my side I checked the last version (minor changes) with both an Armada-385-AP and an Armada-8040-DB.
I will resend the series very soon.
Thanks,
Miqu?l
Quoted 31 lines
Tested-by Gregory CLEMENT <[email protected]>Gregory
Quoted 25 lines
--- drivers/thermal/armada_thermal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)(quoted diff) +0
-0
b/drivers/thermal/armada_thermal.cindexea958e651312..454137f78eb3100644 ---a/drivers/thermal/armada_thermal.c
+++b/drivers/thermal/armada_thermal.c(quoted diff) +2
-2@@ -406,8 +406,8 @@ static int armada_thermal_probe(struct platform_device *pdev) priv->data->init_sensor(pdev, priv); - thermal = thermal_zone_device_register("armada_thermal", 0, 0, - priv, &ops, NULL, 0, 0); + thermal = thermal_zone_device_register(dev_name(&pdev->dev), 0, 0, priv, + &ops, NULL, 0, 0);drivers/thermal/armada_thermal.c +0
-0if (IS_ERR (thermal)) { dev_err(&pdev->dev, "Failed to register thermal zonedevice\n"); --
2.11.0
Signature · 3 lines
Miquel Raynal, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com