linux-kernel - Re: [PATCH][RESEND] acpi: fan.c: printk replacement

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAC-25o8XLy1S1g2KUk3E59m=Az6eVfOONgkMQmkkeikkneS4bg@mail.gmail.com>
Date:	Thu, 28 Aug 2014 08:53:45 -0400
From:	"edubezval@...il.com" <edubezval@...il.com>
To:	Sudip Mukherjee <sudipm.mukherjee@...il.com>
Cc:	Zhang Rui <rui.zhang@...el.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Len Brown <lenb@...nel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH][RESEND] acpi: fan.c: printk replacement

Hello Sudip,

On Thu, Aug 28, 2014 at 7:33 AM, Sudip Mukherjee
<sudipm.mukherjee@...il.com> wrote:
> printk replaced with corresponding dev_err and dev_info
> fixed one broken user-visible string
> multiine comment edited for correct commenting style
> asm/uaccess.h replaced with linux/uaccess.h
> PREFIX removed
>

Just a minor tip. When sending a new version of your patches do not
use the RESEND prefix in you subject line. RESEND means you did not
make changes on your patches and you are just resending it because,
for instance, people did not answer it. When you modify your changes
after, say updating it due to review comments, it is a good practice
to use 'vX' prefix, so those who are following the patch will
understand that this patch has something new from previous version. In
this case, I suppose the current patch should be '[PATCH v2]' instead
of [PATCH][RESEND].

BR,

> Signed-off-by: Sudip Mukherjee <sudip@...torindia.org>
> ---
>
> modified with reference to the discussion at https://lkml.org/lkml/2014/8/22/176
>
>  drivers/acpi/fan.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
> index 8acf53e..5328b10 100644
> --- a/drivers/acpi/fan.c
> +++ b/drivers/acpi/fan.c
> @@ -27,12 +27,10 @@
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/types.h>
> -#include <asm/uaccess.h>
> +#include <linux/uaccess.h>
>  #include <linux/thermal.h>
>  #include <linux/acpi.h>
>
> -#define PREFIX "ACPI: "
> -
>  #define ACPI_FAN_CLASS                 "fan"
>  #define ACPI_FAN_FILE_STATE            "state"
>
> @@ -127,8 +125,9 @@ static const struct thermal_cooling_device_ops fan_cooling_ops = {
>  };
>
>  /* --------------------------------------------------------------------------
> -                                 Driver Interface
> -   -------------------------------------------------------------------------- */
> + *                               Driver Interface
> + * --------------------------------------------------------------------------
> +*/
>
>  static int acpi_fan_add(struct acpi_device *device)
>  {
> @@ -143,7 +142,7 @@ static int acpi_fan_add(struct acpi_device *device)
>
>         result = acpi_bus_update_power(device->handle, NULL);
>         if (result) {
> -               printk(KERN_ERR PREFIX "Setting initial power state\n");
> +               dev_err(&device->dev, "Setting initial power state\n");
>                 goto end;
>         }
>
> @@ -168,10 +167,9 @@ static int acpi_fan_add(struct acpi_device *device)
>                                    &device->dev.kobj,
>                                    "device");
>         if (result)
> -               dev_err(&device->dev, "Failed to create sysfs link "
> -                       "'device'\n");
> +               dev_err(&device->dev, "Failed to create sysfs link 'device'\n");
>
> -       printk(KERN_INFO PREFIX "%s [%s] (%s)\n",
> +       dev_info(&device->dev, "ACPI: %s [%s] (%s)\n",
>                acpi_device_name(device), acpi_device_bid(device),
>                !device->power.state ? "on" : "off");
>
> @@ -217,7 +215,7 @@ static int acpi_fan_resume(struct device *dev)
>
>         result = acpi_bus_update_power(to_acpi_device(dev)->handle, NULL);
>         if (result)
> -               printk(KERN_ERR PREFIX "Error updating fan power state\n");
> +               dev_err(dev, "Error updating fan power state\n");
>
>         return result;
>  }
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Eduardo Bezerra Valentin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/