[PATCH v3 0/3] MIPS: Loongson: Fix some issues of cpu_hwmon.c

[PATCH v3 0/3] MIPS: Loongson: Fix some issues of cpu_hwmon.c

[Zhi Li]

v2:
  - Drop the patch "MIPS: Loongson: Add hwmon support for generic CPU"
  - Reduce possible loop times in do_thermal_timer()

v3:
  - Fix boot warning about hwmon_device_register()

Tiezhu Yang (2):
  MIPS: Loongson: Cleanup cpu_hwmon.c
  MIPS: Loongson: Reduce possible loop times and add log in
    do_thermal_timer()

Zhi Li (1):
  MIPS: Loongson: Fix boot warning about hwmon_device_register()

 drivers/platform/mips/cpu_hwmon.c | 66 +++++++++++++++------------------------
 1 file changed, 25 insertions(+), 41 deletions(-)

-- 
2.1.0

[PATCH v3 1/3] MIPS: Loongson: Cleanup cpu_hwmon.c

[Zhi Li]

From: Tiezhu Yang <yangtiezhu@loongson.cn>

Fix the following checkpatch warnings and errors:

ERROR: do not initialise statics to 0
+static int csr_temp_enable = 0;

WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static SENSOR_DEVICE_ATTR(name, S_IRUGO, get_hwmon_name, NULL, 0);

WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, get_cpu_temp, NULL, 1);

WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, cpu_temp_label, NULL, 1);

WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, get_cpu_temp, NULL, 2);

WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static SENSOR_DEVICE_ATTR(temp2_label, S_IRUGO, cpu_temp_label, NULL, 2);

WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, get_cpu_temp, NULL, 3);

WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static SENSOR_DEVICE_ATTR(temp3_label, S_IRUGO, cpu_temp_label, NULL, 3);

WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, get_cpu_temp, NULL, 4);

WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static SENSOR_DEVICE_ATTR(temp4_label, S_IRUGO, cpu_temp_label, NULL, 4);

WARNING: Missing a blank line after declarations
+	int id = (to_sensor_dev_attr(attr))->index - 1;
+	return sprintf(buf, "CPU %d Temperature\n", id);

WARNING: Missing a blank line after declarations
+	int value = loongson3_cpu_temp(id);
+	return sprintf(buf, "%d\n", value);

ERROR: spaces required around that '=' (ctx:VxV)
+	for (i=0; i<nr_packages; i++)
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
+	for (i=0; i<nr_packages; i++)
 	           ^

ERROR: spaces required around that '=' (ctx:VxV)
+	for (i=0; i<nr_packages; i++)
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
+	for (i=0; i<nr_packages; i++)
 	           ^

ERROR: spaces required around that '=' (ctx:VxV)
+	for (i=0; i<nr_packages; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
+	for (i=0; i<nr_packages; i++) {
 	           ^

WARNING: line over 80 characters
+		csr_temp_enable = csr_readl(LOONGSON_CSR_FEATURES) & LOONGSON_CSRF_TEMP;

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Signed-off-by: Zhi Li <lizhi01@loongson.cn>
---
 drivers/platform/mips/cpu_hwmon.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/mips/cpu_hwmon.c b/drivers/platform/mips/cpu_hwmon.c
index 0d27cb7..fa42b13 100644
--- a/drivers/platform/mips/cpu_hwmon.c
+++ b/drivers/platform/mips/cpu_hwmon.c
@@ -11,7 +11,7 @@
 #include <loongson_hwmon.h>
 #include <loongson_regs.h>
 
-static int csr_temp_enable = 0;
+static int csr_temp_enable;
 
 /*
  * Loongson-3 series cpu has two sensors inside,
@@ -44,7 +44,7 @@ int loongson3_cpu_temp(int cpu)
 	case PRID_REV_LOONGSON3A_R3_0:
 	case PRID_REV_LOONGSON3A_R3_1:
 	default:
-		reg = (reg & 0xffff)*731/0x4000 - 273;
+		reg = (reg & 0xffff) * 731 / 0x4000 - 273;
 		break;
 	}
 
@@ -57,7 +57,7 @@ static struct device *cpu_hwmon_dev;
 
 static ssize_t get_hwmon_name(struct device *dev,
 			struct device_attribute *attr, char *buf);
-static SENSOR_DEVICE_ATTR(name, S_IRUGO, get_hwmon_name, NULL, 0);
+static SENSOR_DEVICE_ATTR(name, 0444, get_hwmon_name, NULL, 0);
 
 static struct attribute *cpu_hwmon_attributes[] = {
 	&sensor_dev_attr_name.dev_attr.attr,
@@ -81,14 +81,14 @@ static ssize_t get_cpu_temp(struct device *dev,
 static ssize_t cpu_temp_label(struct device *dev,
 			struct device_attribute *attr, char *buf);
 
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, get_cpu_temp, NULL, 1);
-static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, cpu_temp_label, NULL, 1);
-static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, get_cpu_temp, NULL, 2);
-static SENSOR_DEVICE_ATTR(temp2_label, S_IRUGO, cpu_temp_label, NULL, 2);
-static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, get_cpu_temp, NULL, 3);
-static SENSOR_DEVICE_ATTR(temp3_label, S_IRUGO, cpu_temp_label, NULL, 3);
-static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, get_cpu_temp, NULL, 4);
-static SENSOR_DEVICE_ATTR(temp4_label, S_IRUGO, cpu_temp_label, NULL, 4);
+static SENSOR_DEVICE_ATTR(temp1_input, 0444, get_cpu_temp, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp1_label, 0444, cpu_temp_label, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_input, 0444, get_cpu_temp, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp2_label, 0444, cpu_temp_label, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_input, 0444, get_cpu_temp, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp3_label, 0444, cpu_temp_label, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp4_input, 0444, get_cpu_temp, NULL, 4);
+static SENSOR_DEVICE_ATTR(temp4_label, 0444, cpu_temp_label, NULL, 4);
 
 static const struct attribute *hwmon_cputemp[4][3] = {
 	{
@@ -117,6 +117,7 @@ static ssize_t cpu_temp_label(struct device *dev,
 			struct device_attribute *attr, char *buf)
 {
 	int id = (to_sensor_dev_attr(attr))->index - 1;
+
 	return sprintf(buf, "CPU %d Temperature\n", id);
 }
 
@@ -125,6 +126,7 @@ static ssize_t get_cpu_temp(struct device *dev,
 {
 	int id = (to_sensor_dev_attr(attr))->index - 1;
 	int value = loongson3_cpu_temp(id);
+
 	return sprintf(buf, "%d\n", value);
 }
 
@@ -132,7 +134,7 @@ static int create_sysfs_cputemp_files(struct kobject *kobj)
 {
 	int i, ret = 0;
 
-	for (i=0; i<nr_packages; i++)
+	for (i = 0; i < nr_packages; i++)
 		ret = sysfs_create_files(kobj, hwmon_cputemp[i]);
 
 	return ret;
@@ -142,7 +144,7 @@ static void remove_sysfs_cputemp_files(struct kobject *kobj)
 {
 	int i;
 
-	for (i=0; i<nr_packages; i++)
+	for (i = 0; i < nr_packages; i++)
 		sysfs_remove_files(kobj, hwmon_cputemp[i]);
 }
 
@@ -153,7 +155,7 @@ static void do_thermal_timer(struct work_struct *work)
 {
 	int i, value, temp_max = 0;
 
-	for (i=0; i<nr_packages; i++) {
+	for (i = 0; i < nr_packages; i++) {
 		value = loongson3_cpu_temp(i);
 		if (value > temp_max)
 			temp_max = value;
@@ -172,7 +174,8 @@ static int __init loongson_hwmon_init(void)
 	pr_info("Loongson Hwmon Enter...\n");
 
 	if (cpu_has_csr())
-		csr_temp_enable = csr_readl(LOONGSON_CSR_FEATURES) & LOONGSON_CSRF_TEMP;
+		csr_temp_enable = csr_readl(LOONGSON_CSR_FEATURES) &
+				  LOONGSON_CSRF_TEMP;
 
 	cpu_hwmon_dev = hwmon_device_register(NULL);
 	if (IS_ERR(cpu_hwmon_dev)) {
-- 
2.1.0

[PATCH v3 2/3] MIPS: Loongson: Reduce possible loop times and add log in do_thermal_timer()

[Zhi Li]

From: Tiezhu Yang <yangtiezhu@loongson.cn>

Once the temperature of any CPUs is too high, it can power off immediately,
no need to check the rest of CPUs, and it is better to print a log before
power off, this is useful when analysis the abnormal issues.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Signed-off-by: Zhi Li <lizhi01@loongson.cn>
---
 drivers/platform/mips/cpu_hwmon.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/mips/cpu_hwmon.c b/drivers/platform/mips/cpu_hwmon.c
index fa42b13..c9f35e5 100644
--- a/drivers/platform/mips/cpu_hwmon.c
+++ b/drivers/platform/mips/cpu_hwmon.c
@@ -153,18 +153,17 @@ static struct delayed_work thermal_work;
 
 static void do_thermal_timer(struct work_struct *work)
 {
-	int i, value, temp_max = 0;
+	int i, value;
 
 	for (i = 0; i < nr_packages; i++) {
 		value = loongson3_cpu_temp(i);
-		if (value > temp_max)
-			temp_max = value;
+		if (value > CPU_THERMAL_THRESHOLD) {
+			pr_emerg("Power off due to high temp: %d\n", value);
+			orderly_poweroff(true);
+		}
 	}
 
-	if (temp_max <= CPU_THERMAL_THRESHOLD)
-		schedule_delayed_work(&thermal_work, msecs_to_jiffies(5000));
-	else
-		orderly_poweroff(true);
+	schedule_delayed_work(&thermal_work, msecs_to_jiffies(5000));
 }
 
 static int __init loongson_hwmon_init(void)
-- 
2.1.0

[PATCH v3 3/3] MIPS: Loongson: Fix boot warning about hwmon_device_register()

[Zhi Li]

Replace hwmon_device_register() with hwmon_device_register_with_info()
to fix the following boot warning :

[    9.029924] Loongson Hwmon Enter...
[    9.106850] (NULL device *): hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().

Signed-off-by: Zhi Li <lizhi01@loongson.cn>
---
 drivers/platform/mips/cpu_hwmon.c | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/platform/mips/cpu_hwmon.c b/drivers/platform/mips/cpu_hwmon.c
index c9f35e5..386389f 100644
--- a/drivers/platform/mips/cpu_hwmon.c
+++ b/drivers/platform/mips/cpu_hwmon.c
@@ -55,9 +55,7 @@ int loongson3_cpu_temp(int cpu)
 static int nr_packages;
 static struct device *cpu_hwmon_dev;
 
-static ssize_t get_hwmon_name(struct device *dev,
-			struct device_attribute *attr, char *buf);
-static SENSOR_DEVICE_ATTR(name, 0444, get_hwmon_name, NULL, 0);
+static SENSOR_DEVICE_ATTR(name, 0444, NULL, NULL, 0);
 
 static struct attribute *cpu_hwmon_attributes[] = {
 	&sensor_dev_attr_name.dev_attr.attr,
@@ -69,13 +67,6 @@ static struct attribute_group cpu_hwmon_attribute_group = {
 	.attrs = cpu_hwmon_attributes,
 };
 
-/* Hwmon device get name */
-static ssize_t get_hwmon_name(struct device *dev,
-			struct device_attribute *attr, char *buf)
-{
-	return sprintf(buf, "cpu-hwmon\n");
-}
-
 static ssize_t get_cpu_temp(struct device *dev,
 			struct device_attribute *attr, char *buf);
 static ssize_t cpu_temp_label(struct device *dev,
@@ -176,7 +167,7 @@ static int __init loongson_hwmon_init(void)
 		csr_temp_enable = csr_readl(LOONGSON_CSR_FEATURES) &
 				  LOONGSON_CSRF_TEMP;
 
-	cpu_hwmon_dev = hwmon_device_register(NULL);
+	cpu_hwmon_dev = hwmon_device_register_with_info(NULL, "cpu_hwmon", NULL, NULL, NULL);
 	if (IS_ERR(cpu_hwmon_dev)) {
 		ret = PTR_ERR(cpu_hwmon_dev);
 		pr_err("hwmon_device_register fail!\n");
@@ -186,13 +177,6 @@ static int __init loongson_hwmon_init(void)
 	nr_packages = loongson_sysconf.nr_cpus /
 		loongson_sysconf.cores_per_package;
 
-	ret = sysfs_create_group(&cpu_hwmon_dev->kobj,
-				&cpu_hwmon_attribute_group);
-	if (ret) {
-		pr_err("fail to create loongson hwmon!\n");
-		goto fail_sysfs_create_group_hwmon;
-	}
-
 	ret = create_sysfs_cputemp_files(&cpu_hwmon_dev->kobj);
 	if (ret) {
 		pr_err("fail to create cpu temperature interface!\n");
@@ -207,8 +191,6 @@ static int __init loongson_hwmon_init(void)
 fail_create_sysfs_cputemp_files:
 	sysfs_remove_group(&cpu_hwmon_dev->kobj,
 				&cpu_hwmon_attribute_group);
-
-fail_sysfs_create_group_hwmon:
 	hwmon_device_unregister(cpu_hwmon_dev);
 
 fail_hwmon_device_register:
-- 
2.1.0

Re: [PATCH v3 0/3] MIPS: Loongson: Fix some issues of cpu_hwmon.c

[Thomas Bogendoerfer]

On Thu, Jul 09, 2020 at 11:55:18AM +0800, Zhi Li wrote:
> v2:
>   - Drop the patch "MIPS: Loongson: Add hwmon support for generic CPU"
>   - Reduce possible loop times in do_thermal_timer()
> 
> v3:
>   - Fix boot warning about hwmon_device_register()
> 
> Tiezhu Yang (2):
>   MIPS: Loongson: Cleanup cpu_hwmon.c
>   MIPS: Loongson: Reduce possible loop times and add log in
>     do_thermal_timer()
> 
> Zhi Li (1):
>   MIPS: Loongson: Fix boot warning about hwmon_device_register()
> 
>  drivers/platform/mips/cpu_hwmon.c | 66 +++++++++++++++------------------------

series applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]