* [PATCH 15/16] Staging: quickstart: Add QS_INFO and QS_ERR macros for logs
@ 2012-01-09 22:23 Szymon Janc
2012-01-11 11:56 ` Dan Carpenter
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Szymon Janc @ 2012-01-09 22:23 UTC (permalink / raw)
To: kernel-janitors
Signed-off-by: Szymon Janc <szymon@janc.net.pl>
---
drivers/staging/quickstart/quickstart.c | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/quickstart/quickstart.c b/drivers/staging/quickstart/quickstart.c
index 51a6d0c..c757ef6 100644
--- a/drivers/staging/quickstart/quickstart.c
+++ b/drivers/staging/quickstart/quickstart.c
@@ -44,6 +44,9 @@ MODULE_LICENSE("GPL");
#define QUICKSTART_PF_DRIVER_NAME "quickstart"
#define QUICKSTART_PF_DEVICE_NAME "quickstart"
+#define QS_INFO(fmt, arg...) printk(KERN_INFO "quickstart: " fmt "\n", ##arg)
+#define QS_ERR(fmt, arg...) printk(KERN_ERR "quickstart: " fmt "\n", ##arg)
+
/*
* There will be two events:
* 0x02 - A hot button was pressed while device was off/sleeping.
@@ -164,6 +167,7 @@ static void quickstart_acpi_notify(acpi_handle handle, u32 event, void *data)
input_sync(quickstart_input);
break;
default:
+ QS_ERR("Unexpected ACPI event notify (%u)", event);
break;
}
}
@@ -181,8 +185,7 @@ static int quickstart_acpi_ghid(struct quickstart_acpi *quickstart)
status = acpi_evaluate_object(quickstart->device->handle, "GHID", NULL,
&buffer);
if (ACPI_FAILURE(status)) {
- printk(KERN_ERR "quickstart: %s GHID method failed.\n",
- quickstart->button->name);
+ QS_ERR("%s GHID method failed", quickstart->button->name);
return -EINVAL;
}
@@ -205,8 +208,7 @@ static int quickstart_acpi_ghid(struct quickstart_acpi *quickstart)
quickstart->button->id = *(uint64_t *)buffer.pointer;
break;
default:
- printk(KERN_ERR "quickstart: %s GHID method returned buffer "
- "of unexpected length %u\n",
+ QS_ERR("%s GHID method returned buffer of unexpected length %u",
quickstart->button->name, buffer.length);
ret = -EINVAL;
break;
@@ -267,7 +269,7 @@ static int quickstart_acpi_add(struct acpi_device *device)
quickstart_acpi_notify,
quickstart);
if (ACPI_FAILURE(status)) {
- printk(KERN_ERR "quickstart: Notify handler install error\n");
+ QS_ERR("Notify handler install error");
ret = -ENODEV;
goto fail_installnotify;
}
@@ -307,7 +309,7 @@ static int quickstart_acpi_remove(struct acpi_device *device, int type)
status = acpi_remove_notify_handler(device->handle, ACPI_ALL_NOTIFY,
quickstart_acpi_notify);
if (ACPI_FAILURE(status))
- printk(KERN_ERR "quickstart: Error removing notify handler\n");
+ QS_ERR("Error removing notify handler");
kfree(quickstart);
@@ -433,8 +435,7 @@ static int __init quickstart_init(void)
if (ret)
goto fail_input;
- printk(KERN_INFO "quickstart: ACPI Direct App Launch ver %s\n",
- QUICKSTART_VERSION);
+ QS_INFO("ACPI Direct App Launch ver %s", QUICKSTART_VERSION);
return 0;
fail_input:
--
1.7.8.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 15/16] Staging: quickstart: Add QS_INFO and QS_ERR macros for logs
2012-01-09 22:23 [PATCH 15/16] Staging: quickstart: Add QS_INFO and QS_ERR macros for logs Szymon Janc
@ 2012-01-11 11:56 ` Dan Carpenter
2012-01-11 17:55 ` Szymon Janc
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2012-01-11 11:56 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]
On Mon, Jan 09, 2012 at 11:23:34PM +0100, Szymon Janc wrote:
> Signed-off-by: Szymon Janc <szymon@janc.net.pl>
> ---
> drivers/staging/quickstart/quickstart.c | 17 +++++++++--------
> 1 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/quickstart/quickstart.c b/drivers/staging/quickstart/quickstart.c
> index 51a6d0c..c757ef6 100644
> --- a/drivers/staging/quickstart/quickstart.c
> +++ b/drivers/staging/quickstart/quickstart.c
> @@ -44,6 +44,9 @@ MODULE_LICENSE("GPL");
> #define QUICKSTART_PF_DRIVER_NAME "quickstart"
> #define QUICKSTART_PF_DEVICE_NAME "quickstart"
>
> +#define QS_INFO(fmt, arg...) printk(KERN_INFO "quickstart: " fmt "\n", ##arg)
> +#define QS_ERR(fmt, arg...) printk(KERN_ERR "quickstart: " fmt "\n", ##arg)
> +
No. Don't do this. You can do same thing with pr_err() so you
don't need to create your own macros. Just add this to the front:
#define pr_fmt(fmt) "quickstart: " fmt
We already have a million standard ways of printing stuff, we don't
need a million and two.
regards,
dan carpenter
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 15/16] Staging: quickstart: Add QS_INFO and QS_ERR macros for logs
2012-01-09 22:23 [PATCH 15/16] Staging: quickstart: Add QS_INFO and QS_ERR macros for logs Szymon Janc
2012-01-11 11:56 ` Dan Carpenter
@ 2012-01-11 17:55 ` Szymon Janc
2012-01-11 20:51 ` Joe Perches
2012-01-11 22:22 ` Szymon Janc
3 siblings, 0 replies; 5+ messages in thread
From: Szymon Janc @ 2012-01-11 17:55 UTC (permalink / raw)
To: kernel-janitors
Hi,
> > #define QUICKSTART_PF_DRIVER_NAME "quickstart"
> > #define QUICKSTART_PF_DEVICE_NAME "quickstart"
> >
> > +#define QS_INFO(fmt, arg...) printk(KERN_INFO "quickstart: " fmt "\n",
> > ##arg) +#define QS_ERR(fmt, arg...) printk(KERN_ERR "quickstart: " fmt
> > "\n", ##arg) +
>
> No. Don't do this. You can do same thing with pr_err() so you
> don't need to create your own macros. Just add this to the front:
>
> #define pr_fmt(fmt) "quickstart: " fmt
>
> We already have a million standard ways of printing stuff, we don't
> need a million and two.
Ok, and thanks for review! I'll send V2 shortly.
--
Szymon K. Janc
szymon@janc.net.pl
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 15/16] Staging: quickstart: Add QS_INFO and QS_ERR macros for logs
2012-01-09 22:23 [PATCH 15/16] Staging: quickstart: Add QS_INFO and QS_ERR macros for logs Szymon Janc
2012-01-11 11:56 ` Dan Carpenter
2012-01-11 17:55 ` Szymon Janc
@ 2012-01-11 20:51 ` Joe Perches
2012-01-11 22:22 ` Szymon Janc
3 siblings, 0 replies; 5+ messages in thread
From: Joe Perches @ 2012-01-11 20:51 UTC (permalink / raw)
To: kernel-janitors
On Wed, 2012-01-11 at 18:55 +0100, Szymon Janc wrote:
> > > #define QUICKSTART_PF_DRIVER_NAME "quickstart"
> > > #define QUICKSTART_PF_DEVICE_NAME "quickstart"
> > >
> > > +#define QS_INFO(fmt, arg...) printk(KERN_INFO "quickstart: " fmt "\n",
> > > ##arg) +#define QS_ERR(fmt, arg...) printk(KERN_ERR "quickstart: " fmt
> > > "\n", ##arg) +
> >
> > No. Don't do this. You can do same thing with pr_err() so you
> > don't need to create your own macros. Just add this to the front:
> >
> > #define pr_fmt(fmt) "quickstart: " fmt
> >
> > We already have a million standard ways of printing stuff, we don't
> > need a million and two.
>
> Ok, and thanks for review! I'll send V2 shortly.
Before you send V2, this is better as:
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 15/16] Staging: quickstart: Add QS_INFO and QS_ERR macros for logs
2012-01-09 22:23 [PATCH 15/16] Staging: quickstart: Add QS_INFO and QS_ERR macros for logs Szymon Janc
` (2 preceding siblings ...)
2012-01-11 20:51 ` Joe Perches
@ 2012-01-11 22:22 ` Szymon Janc
3 siblings, 0 replies; 5+ messages in thread
From: Szymon Janc @ 2012-01-11 22:22 UTC (permalink / raw)
To: kernel-janitors
Signed-off-by: Szymon Janc <szymon@janc.net.pl>
---
drivers/staging/quickstart/quickstart.c | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/quickstart/quickstart.c b/drivers/staging/quickstart/quickstart.c
index 51a6d0c..c757ef6 100644
--- a/drivers/staging/quickstart/quickstart.c
+++ b/drivers/staging/quickstart/quickstart.c
@@ -44,6 +44,9 @@ MODULE_LICENSE("GPL");
#define QUICKSTART_PF_DRIVER_NAME "quickstart"
#define QUICKSTART_PF_DEVICE_NAME "quickstart"
+#define QS_INFO(fmt, arg...) printk(KERN_INFO "quickstart: " fmt "\n", ##arg)
+#define QS_ERR(fmt, arg...) printk(KERN_ERR "quickstart: " fmt "\n", ##arg)
+
/*
* There will be two events:
* 0x02 - A hot button was pressed while device was off/sleeping.
@@ -164,6 +167,7 @@ static void quickstart_acpi_notify(acpi_handle handle, u32 event, void *data)
input_sync(quickstart_input);
break;
default:
+ QS_ERR("Unexpected ACPI event notify (%u)", event);
break;
}
}
@@ -181,8 +185,7 @@ static int quickstart_acpi_ghid(struct quickstart_acpi *quickstart)
status = acpi_evaluate_object(quickstart->device->handle, "GHID", NULL,
&buffer);
if (ACPI_FAILURE(status)) {
- printk(KERN_ERR "quickstart: %s GHID method failed.\n",
- quickstart->button->name);
+ QS_ERR("%s GHID method failed", quickstart->button->name);
return -EINVAL;
}
@@ -205,8 +208,7 @@ static int quickstart_acpi_ghid(struct quickstart_acpi *quickstart)
quickstart->button->id = *(uint64_t *)buffer.pointer;
break;
default:
- printk(KERN_ERR "quickstart: %s GHID method returned buffer "
- "of unexpected length %u\n",
+ QS_ERR("%s GHID method returned buffer of unexpected length %u",
quickstart->button->name, buffer.length);
ret = -EINVAL;
break;
@@ -267,7 +269,7 @@ static int quickstart_acpi_add(struct acpi_device *device)
quickstart_acpi_notify,
quickstart);
if (ACPI_FAILURE(status)) {
- printk(KERN_ERR "quickstart: Notify handler install error\n");
+ QS_ERR("Notify handler install error");
ret = -ENODEV;
goto fail_installnotify;
}
@@ -307,7 +309,7 @@ static int quickstart_acpi_remove(struct acpi_device *device, int type)
status = acpi_remove_notify_handler(device->handle, ACPI_ALL_NOTIFY,
quickstart_acpi_notify);
if (ACPI_FAILURE(status))
- printk(KERN_ERR "quickstart: Error removing notify handler\n");
+ QS_ERR("Error removing notify handler");
kfree(quickstart);
@@ -433,8 +435,7 @@ static int __init quickstart_init(void)
if (ret)
goto fail_input;
- printk(KERN_INFO "quickstart: ACPI Direct App Launch ver %s\n",
- QUICKSTART_VERSION);
+ QS_INFO("ACPI Direct App Launch ver %s", QUICKSTART_VERSION);
return 0;
fail_input:
--
1.7.8.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-01-11 22:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-09 22:23 [PATCH 15/16] Staging: quickstart: Add QS_INFO and QS_ERR macros for logs Szymon Janc
2012-01-11 11:56 ` Dan Carpenter
2012-01-11 17:55 ` Szymon Janc
2012-01-11 20:51 ` Joe Perches
2012-01-11 22:22 ` Szymon Janc
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox