All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] MIPS: Loongson: Fix some issues of cpu_hwmon.c
@ 2020-05-08 11:55 Tiezhu Yang
  2020-05-08 11:55 ` [PATCH 1/3] MIPS: Loongson: Cleanup cpu_hwmon.c Tiezhu Yang
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Tiezhu Yang @ 2020-05-08 11:55 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: linux-mips, linux-kernel, Xuefeng Li

Tiezhu Yang (3):
  MIPS: Loongson: Cleanup cpu_hwmon.c
  MIPS: Loongson: Add hwmon support for generic CPU
  MIPS: Loongson: Add log before power off due to high temperature

 drivers/platform/mips/cpu_hwmon.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

-- 
2.1.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] MIPS: Loongson: Cleanup cpu_hwmon.c
  2020-05-08 11:55 [PATCH 0/3] MIPS: Loongson: Fix some issues of cpu_hwmon.c Tiezhu Yang
@ 2020-05-08 11:55 ` Tiezhu Yang
  2020-05-08 11:55 ` [PATCH 2/3] MIPS: Loongson: Add hwmon support for generic CPU Tiezhu Yang
  2020-05-08 11:55 ` [PATCH 3/3] MIPS: Loongson: Add log before power off due to high temperature Tiezhu Yang
  2 siblings, 0 replies; 6+ messages in thread
From: Tiezhu Yang @ 2020-05-08 11:55 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: linux-mips, linux-kernel, Xuefeng Li

Fix the following checkpatch warnings and errors:

ERROR: do not initialise statics to 0
#14: FILE: drivers/platform/mips/cpu_hwmon.c:14:
+static int csr_temp_enable = 0;

WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
#60: FILE: drivers/platform/mips/cpu_hwmon.c:60:
+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'.
#84: FILE: drivers/platform/mips/cpu_hwmon.c:84:
+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'.
#85: FILE: drivers/platform/mips/cpu_hwmon.c:85:
+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'.
#86: FILE: drivers/platform/mips/cpu_hwmon.c:86:
+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'.
#87: FILE: drivers/platform/mips/cpu_hwmon.c:87:
+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'.
#88: FILE: drivers/platform/mips/cpu_hwmon.c:88:
+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'.
#89: FILE: drivers/platform/mips/cpu_hwmon.c:89:
+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'.
#90: FILE: drivers/platform/mips/cpu_hwmon.c:90:
+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'.
#91: FILE: drivers/platform/mips/cpu_hwmon.c:91:
+static SENSOR_DEVICE_ATTR(temp4_label, S_IRUGO, cpu_temp_label, NULL, 4);

WARNING: Missing a blank line after declarations
#120: FILE: drivers/platform/mips/cpu_hwmon.c:120:
+	int id = (to_sensor_dev_attr(attr))->index - 1;
+	return sprintf(buf, "CPU %d Temperature\n", id);

WARNING: Missing a blank line after declarations
#128: FILE: drivers/platform/mips/cpu_hwmon.c:128:
+	int value = loongson3_cpu_temp(id);
+	return sprintf(buf, "%d\n", value);

ERROR: spaces required around that '=' (ctx:VxV)
#135: FILE: drivers/platform/mips/cpu_hwmon.c:135:
+	for (i=0; i<nr_packages; i++)
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#135: FILE: drivers/platform/mips/cpu_hwmon.c:135:
+	for (i=0; i<nr_packages; i++)
 	           ^

ERROR: spaces required around that '=' (ctx:VxV)
#145: FILE: drivers/platform/mips/cpu_hwmon.c:145:
+	for (i=0; i<nr_packages; i++)
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#145: FILE: drivers/platform/mips/cpu_hwmon.c:145:
+	for (i=0; i<nr_packages; i++)
 	           ^

ERROR: spaces required around that '=' (ctx:VxV)
#156: FILE: drivers/platform/mips/cpu_hwmon.c:156:
+	for (i=0; i<nr_packages; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#156: FILE: drivers/platform/mips/cpu_hwmon.c:156:
+	for (i=0; i<nr_packages; i++) {
 	           ^

WARNING: line over 80 characters
#175: FILE: drivers/platform/mips/cpu_hwmon.c:175:
+		csr_temp_enable = csr_readl(LOONGSON_CSR_FEATURES) & LOONGSON_CSRF_TEMP;

total: 7 errors, 12 warnings, 231 lines checked

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 drivers/platform/mips/cpu_hwmon.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/mips/cpu_hwmon.c b/drivers/platform/mips/cpu_hwmon.c
index 0d27cb7..add5f52 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,
@@ -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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] MIPS: Loongson: Add hwmon support for generic CPU
  2020-05-08 11:55 [PATCH 0/3] MIPS: Loongson: Fix some issues of cpu_hwmon.c Tiezhu Yang
  2020-05-08 11:55 ` [PATCH 1/3] MIPS: Loongson: Cleanup cpu_hwmon.c Tiezhu Yang
@ 2020-05-08 11:55 ` Tiezhu Yang
  2020-05-08 12:51   ` WANG Xuerui
  2020-05-08 11:55 ` [PATCH 3/3] MIPS: Loongson: Add log before power off due to high temperature Tiezhu Yang
  2 siblings, 1 reply; 6+ messages in thread
From: Tiezhu Yang @ 2020-05-08 11:55 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: linux-mips, linux-kernel, Xuefeng Li

Add PRID_IMP_LOONGSON_64G case to enable hwmon support for Loongson
generic CPU such as 3A4000 and newer CPU.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 drivers/platform/mips/cpu_hwmon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/mips/cpu_hwmon.c b/drivers/platform/mips/cpu_hwmon.c
index add5f52..7b4bde1 100644
--- a/drivers/platform/mips/cpu_hwmon.c
+++ b/drivers/platform/mips/cpu_hwmon.c
@@ -43,6 +43,7 @@ int loongson3_cpu_temp(int cpu)
 		break;
 	case PRID_REV_LOONGSON3A_R3_0:
 	case PRID_REV_LOONGSON3A_R3_1:
+	case PRID_IMP_LOONGSON_64G:
 	default:
 		reg = (reg & 0xffff)*731/0x4000 - 273;
 		break;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] MIPS: Loongson: Add log before power off due to high temperature
  2020-05-08 11:55 [PATCH 0/3] MIPS: Loongson: Fix some issues of cpu_hwmon.c Tiezhu Yang
  2020-05-08 11:55 ` [PATCH 1/3] MIPS: Loongson: Cleanup cpu_hwmon.c Tiezhu Yang
  2020-05-08 11:55 ` [PATCH 2/3] MIPS: Loongson: Add hwmon support for generic CPU Tiezhu Yang
@ 2020-05-08 11:55 ` Tiezhu Yang
  2 siblings, 0 replies; 6+ messages in thread
From: Tiezhu Yang @ 2020-05-08 11:55 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: linux-mips, linux-kernel, Xuefeng Li

When the CPU temperature is too high, 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>
---
 drivers/platform/mips/cpu_hwmon.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/mips/cpu_hwmon.c b/drivers/platform/mips/cpu_hwmon.c
index 7b4bde1..6dfecd8 100644
--- a/drivers/platform/mips/cpu_hwmon.c
+++ b/drivers/platform/mips/cpu_hwmon.c
@@ -162,10 +162,12 @@ static void do_thermal_timer(struct work_struct *work)
 			temp_max = value;
 	}
 
-	if (temp_max <= CPU_THERMAL_THRESHOLD)
+	if (temp_max <= CPU_THERMAL_THRESHOLD) {
 		schedule_delayed_work(&thermal_work, msecs_to_jiffies(5000));
-	else
+	} else {
+		pr_emerg("Power off due to high temperature: %d\n", value);
 		orderly_poweroff(true);
+	}
 }
 
 static int __init loongson_hwmon_init(void)
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3] MIPS: Loongson: Add hwmon support for generic CPU
  2020-05-08 11:55 ` [PATCH 2/3] MIPS: Loongson: Add hwmon support for generic CPU Tiezhu Yang
@ 2020-05-08 12:51   ` WANG Xuerui
  2020-05-09  2:23     ` Tiezhu Yang
  0 siblings, 1 reply; 6+ messages in thread
From: WANG Xuerui @ 2020-05-08 12:51 UTC (permalink / raw)
  To: Tiezhu Yang, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: linux-mips, linux-kernel, Xuefeng Li

On 2020/5/8 19:55, Tiezhu Yang wrote:

> Add PRID_IMP_LOONGSON_64G case to enable hwmon support for Loongson
> generic CPU such as 3A4000 and newer CPU.
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>   drivers/platform/mips/cpu_hwmon.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/platform/mips/cpu_hwmon.c b/drivers/platform/mips/cpu_hwmon.c
> index add5f52..7b4bde1 100644
> --- a/drivers/platform/mips/cpu_hwmon.c
> +++ b/drivers/platform/mips/cpu_hwmon.c
> @@ -43,6 +43,7 @@ int loongson3_cpu_temp(int cpu)
>   		break;
>   	case PRID_REV_LOONGSON3A_R3_0:
>   	case PRID_REV_LOONGSON3A_R3_1:
> +	case PRID_IMP_LOONGSON_64G:
>   	default:
>   		reg = (reg & 0xffff)*731/0x4000 - 273;
>   		break;

Hi,

This is obviously wrong, as the value being matched is a PRID_REV. You 
can tell from the neighboring match arms.

Also, the LOONGSON_64G cores are 3A4000 and newer, that have CSR 
support. The csr_temp_enable flag is probed in loongson_hwmon_init, then 
the switch is simply never entered for these.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3] MIPS: Loongson: Add hwmon support for generic CPU
  2020-05-08 12:51   ` WANG Xuerui
@ 2020-05-09  2:23     ` Tiezhu Yang
  0 siblings, 0 replies; 6+ messages in thread
From: Tiezhu Yang @ 2020-05-09  2:23 UTC (permalink / raw)
  To: WANG Xuerui, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: linux-mips, linux-kernel, Xuefeng Li

On 05/08/2020 08:51 PM, WANG Xuerui wrote:
> On 2020/5/8 19:55, Tiezhu Yang wrote:
>
>> Add PRID_IMP_LOONGSON_64G case to enable hwmon support for Loongson
>> generic CPU such as 3A4000 and newer CPU.
>>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>>   drivers/platform/mips/cpu_hwmon.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/platform/mips/cpu_hwmon.c 
>> b/drivers/platform/mips/cpu_hwmon.c
>> index add5f52..7b4bde1 100644
>> --- a/drivers/platform/mips/cpu_hwmon.c
>> +++ b/drivers/platform/mips/cpu_hwmon.c
>> @@ -43,6 +43,7 @@ int loongson3_cpu_temp(int cpu)
>>           break;
>>       case PRID_REV_LOONGSON3A_R3_0:
>>       case PRID_REV_LOONGSON3A_R3_1:
>> +    case PRID_IMP_LOONGSON_64G:
>>       default:
>>           reg = (reg & 0xffff)*731/0x4000 - 273;
>>           break;
>
> Hi,
>
> This is obviously wrong, as the value being matched is a PRID_REV. You 
> can tell from the neighboring match arms.
>
> Also, the LOONGSON_64G cores are 3A4000 and newer, that have CSR 
> support. The csr_temp_enable flag is probed in loongson_hwmon_init, 
> then the switch is simply never entered for these.

Hi Xuerui,

Thanks for your review. You are right.

I notice that this feature has been done in the commit 7507445b1993
("MIPS: Loongson: Add Loongson-3A R4 basic support"). My initial aim
is to get CPU temperature for the Loongson generic CPU when the flag
csr_temp_enable is false, but this is just the default case. So this
patch is meaningless, please ignore it.

Because patch 3/3 depends on this patch 2/3, I will remake and send v2
patch series without this patch.

Thanks,
Tiezhu Yang


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-05-09  2:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-08 11:55 [PATCH 0/3] MIPS: Loongson: Fix some issues of cpu_hwmon.c Tiezhu Yang
2020-05-08 11:55 ` [PATCH 1/3] MIPS: Loongson: Cleanup cpu_hwmon.c Tiezhu Yang
2020-05-08 11:55 ` [PATCH 2/3] MIPS: Loongson: Add hwmon support for generic CPU Tiezhu Yang
2020-05-08 12:51   ` WANG Xuerui
2020-05-09  2:23     ` Tiezhu Yang
2020-05-08 11:55 ` [PATCH 3/3] MIPS: Loongson: Add log before power off due to high temperature Tiezhu Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.