Messages in this thread | | | Subject | Re: [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c | From | Tiezhu Yang <> | Date | Sat, 9 May 2020 18:28:15 +0800 |
| |
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 >>
| |