All of lore.kernel.org
 help / color / mirror / Atom feed
From: Huacai Chen <chenhc@lemote.com>
To: Tiezhu Yang <yangtiezhu@loongson.cn>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Jiaxun Yang <jiaxun.yang@flygoat.com>,
	"open list:MIPS" <linux-mips@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Xuefeng Li <lixuefeng@loongson.cn>
Subject: Re: [PATCH v2 1/2] MIPS: Loongson: Cleanup cpu_hwmon.c
Date: Sat, 9 May 2020 17:52:56 +0800	[thread overview]
Message-ID: <CAAhV-H5zhwHgtbLXPuj5mcGnv5zfT7iRcxoBdk-rbPTn3vt=RQ@mail.gmail.com> (raw)
In-Reply-To: <1589008364-2294-2-git-send-email-yangtiezhu@loongson.cn>

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
>

  reply	other threads:[~2020-05-09  9:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAAhV-H5zhwHgtbLXPuj5mcGnv5zfT7iRcxoBdk-rbPTn3vt=RQ@mail.gmail.com' \
    --to=chenhc@lemote.com \
    --cc=jiaxun.yang@flygoat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=lixuefeng@loongson.cn \
    --cc=tsbogend@alpha.franken.de \
    --cc=yangtiezhu@loongson.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.