Re: [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c

From: Huacai Chen
Date: Sat May 09 2020 - 05:45:17 EST

Next message: Thomas Gleixner: "Re: Missing CLOCK_BOOTTIME_RAW?"
Previous message: Ming Lei: "Re: [RFC PATCH v2 1/7] block: Extand commit_rqs() to do batch processing"
In reply to: Tiezhu Yang: "[PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c"
Next in thread: Tiezhu Yang: "Re: [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi, Tiezhu,

On Sat, May 9, 2020 at 3:13 PM Tiezhu Yang <yangtiezhu@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> ---
> 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
>

Next message: Thomas Gleixner: "Re: Missing CLOCK_BOOTTIME_RAW?"
Previous message: Ming Lei: "Re: [RFC PATCH v2 1/7] block: Extand commit_rqs() to do batch processing"
In reply to: Tiezhu Yang: "[PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c"
Next in thread: Tiezhu Yang: "Re: [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]