* [PATCH v2 0/2] MIPS: Loongson: Fix some issues of cpu_hwmon.c
@ 2020-05-09 7:12 Tiezhu Yang
2020-05-09 7:12 ` [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c Tiezhu Yang
2020-05-09 7:12 ` [PATCH v2 2/2] MIPS: Loongson: Reduce possible loop times and add log in do_thermal_timer() Tiezhu Yang
0 siblings, 2 replies; 5+ messages in thread
From: Tiezhu Yang @ 2020-05-09 7:12 UTC (permalink / raw)
To: Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
Cc: linux-mips, linux-kernel, Xuefeng Li
v2:
- Drop the patch "MIPS: Loongson: Add hwmon support for generic CPU"
- Reduce possible loop times in do_thermal_timer()
Tiezhu Yang (2):
MIPS: Loongson: Cleanup cpu_hwmon.c
MIPS: Loongson: Reduce possible loop times and add log in
do_thermal_timer()
drivers/platform/mips/cpu_hwmon.c | 46 ++++++++++++++++++++-------------------
1 file changed, 24 insertions(+), 22 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c 2020-05-09 7:12 [PATCH v2 0/2] MIPS: Loongson: Fix some issues of cpu_hwmon.c Tiezhu Yang @ 2020-05-09 7:12 ` Tiezhu Yang 2020-05-09 9:52 ` Huacai Chen 2020-05-09 7:12 ` [PATCH v2 2/2] MIPS: Loongson: Reduce possible loop times and add log in do_thermal_timer() Tiezhu Yang 1 sibling, 1 reply; 5+ messages in thread From: Tiezhu Yang @ 2020-05-09 7:12 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; Signed-off-by: Tiezhu Yang <yangtiezhu@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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c 2020-05-09 7:12 ` [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c Tiezhu Yang @ 2020-05-09 9:52 ` Huacai Chen 2020-05-09 10:28 ` Tiezhu Yang 0 siblings, 1 reply; 5+ messages in thread From: Huacai Chen @ 2020-05-09 9:52 UTC (permalink / raw) To: Tiezhu Yang Cc: Thomas Bogendoerfer, Jiaxun Yang, open list:MIPS, LKML, Xuefeng Li Hi, Tiezhu, On Sat, May 9, 2020 at 3:13 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > > 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; I know you are doing things by checkpatch, but I really don't know why not initialise. I think initialise is more human-readable. > > 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); And so does this. > > 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; > > Signed-off-by: Tiezhu Yang <yangtiezhu@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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c 2020-05-09 9:52 ` Huacai Chen @ 2020-05-09 10:28 ` Tiezhu Yang 0 siblings, 0 replies; 5+ messages in thread From: Tiezhu Yang @ 2020-05-09 10:28 UTC (permalink / raw) To: Huacai Chen Cc: Thomas Bogendoerfer, Jiaxun Yang, open list:MIPS, LKML, Xuefeng Li On 05/09/2020 05:52 PM, Huacai Chen wrote: > Hi, Tiezhu, > > On Sat, May 9, 2020 at 3:13 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote: >> 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; > I know you are doing things by checkpatch, but I really don't know why > not initialise. I think initialise is more human-readable. Hi Huacai, Just for cleanup. I think the reason may be: The static variables are initialised to 0 by GCC. Explicit initialization is not necessary. > >> 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); > And so does this. The generic reason is octal is readable and S_<FOO> is unintelligible. Here is Linus' opinion: https://lkml.org/lkml/2016/8/2/1945 Thanks, Tiezhu Yang > >> 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; >> >> Signed-off-by: Tiezhu Yang <yangtiezhu@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 >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] MIPS: Loongson: Reduce possible loop times and add log in do_thermal_timer() 2020-05-09 7:12 [PATCH v2 0/2] MIPS: Loongson: Fix some issues of cpu_hwmon.c Tiezhu Yang 2020-05-09 7:12 ` [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c Tiezhu Yang @ 2020-05-09 7:12 ` Tiezhu Yang 1 sibling, 0 replies; 5+ messages in thread From: Tiezhu Yang @ 2020-05-09 7:12 UTC (permalink / raw) To: Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang Cc: linux-mips, linux-kernel, Xuefeng Li 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> --- v2: - Reduce possible loop times in do_thermal_timer() - Modify the patch subject and update the commit message 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-09 10:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-09 7:12 [PATCH v2 0/2] MIPS: Loongson: Fix some issues of cpu_hwmon.c Tiezhu Yang 2020-05-09 7:12 ` [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c Tiezhu Yang 2020-05-09 9:52 ` Huacai Chen 2020-05-09 10:28 ` Tiezhu Yang 2020-05-09 7:12 ` [PATCH v2 2/2] MIPS: Loongson: Reduce possible loop times and add log in do_thermal_timer() 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.