Re: [PATCH v3] panel: Use pr_err(...) rather than printk(KERN_ERR ...)
Ryan-san, Joe-san, Thanks for your advice. I will try re-create the patch again. On Thu, Jul 12, 2012 at 2:43 PM, Joe Perches wrote: > On Thu, 2012-07-12 at 15:22 +1000, Ryan Mallon wrote: >> On 12/07/12 12:35, Toshiaki Yamane wrote: >> > This change is inspired by checkpatch. >> >> Your changelog needs to describe all of the changes you are making. The >> subject line only describes one. This patch is doing the following: >> >> - Converting printk(KERN_ERR to pr_err >> - Adding __func__ prefixes to printk lines >> - Refactoring split printk strings onto a single line >> >> There are a few other printks in this file which could be converted to >> pr_* to make the code more consistent. Perhaps a follow up patch? >> >> Typically for a sub-sequent version of a patch/series you should list >> the changes since the last round. Put these below the --- so that they >> don't become part of the change log, e.g.: >> >> Signed-off-by: Your name/email here >> --- >> >> Changes since v2: >> - Some stuff >> >> Changes since v1: >> - Some other stuff >> >> Some more comments below. > > And ideally cc the people that gave you notes/comments > on your previous patches too. > -- Regards, . . ... Yamane Toshiaki yamaneto...@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] panel: Use pr_err(...) rather than printk(KERN_ERR ...)
Ryan-san, Joe-san, Thanks for your advice. I will try re-create the patch again. On Thu, Jul 12, 2012 at 2:43 PM, Joe Perches j...@perches.com wrote: On Thu, 2012-07-12 at 15:22 +1000, Ryan Mallon wrote: On 12/07/12 12:35, Toshiaki Yamane wrote: This change is inspired by checkpatch. Your changelog needs to describe all of the changes you are making. The subject line only describes one. This patch is doing the following: - Converting printk(KERN_ERR to pr_err - Adding __func__ prefixes to printk lines - Refactoring split printk strings onto a single line There are a few other printks in this file which could be converted to pr_* to make the code more consistent. Perhaps a follow up patch? Typically for a sub-sequent version of a patch/series you should list the changes since the last round. Put these below the --- so that they don't become part of the change log, e.g.: Signed-off-by: Your name/email here --- Changes since v2: - Some stuff Changes since v1: - Some other stuff Some more comments below. And ideally cc the people that gave you notes/comments on your previous patches too. -- Regards, . . ... Yamane Toshiaki yamaneto...@gmail.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] panel: Use pr_err(...) rather than printk(KERN_ERR ...)
On Thu, 2012-07-12 at 15:22 +1000, Ryan Mallon wrote: > On 12/07/12 12:35, Toshiaki Yamane wrote: > > This change is inspired by checkpatch. > > Your changelog needs to describe all of the changes you are making. The > subject line only describes one. This patch is doing the following: > > - Converting printk(KERN_ERR to pr_err > - Adding __func__ prefixes to printk lines > - Refactoring split printk strings onto a single line > > There are a few other printks in this file which could be converted to > pr_* to make the code more consistent. Perhaps a follow up patch? > > Typically for a sub-sequent version of a patch/series you should list > the changes since the last round. Put these below the --- so that they > don't become part of the change log, e.g.: > > Signed-off-by: Your name/email here > --- > > Changes since v2: > - Some stuff > > Changes since v1: > - Some other stuff > > Some more comments below. And ideally cc the people that gave you notes/comments on your previous patches too. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] panel: Use pr_err(...) rather than printk(KERN_ERR ...)
On 12/07/12 12:35, Toshiaki Yamane wrote: > This change is inspired by checkpatch. Your changelog needs to describe all of the changes you are making. The subject line only describes one. This patch is doing the following: - Converting printk(KERN_ERR to pr_err - Adding __func__ prefixes to printk lines - Refactoring split printk strings onto a single line There are a few other printks in this file which could be converted to pr_* to make the code more consistent. Perhaps a follow up patch? Typically for a sub-sequent version of a patch/series you should list the changes since the last round. Put these below the --- so that they don't become part of the change log, e.g.: Signed-off-by: Your name/email here --- Changes since v2: - Some stuff Changes since v1: - Some other stuff Some more comments below. ~Ryan > > Signed-off-by: Toshiaki Yamane > --- > drivers/staging/panel/panel.c | 42 +--- > 1 files changed, 18 insertions(+), 24 deletions(-) > > diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c > index 7365089..a6d71fd 100644 > --- a/drivers/staging/panel/panel.c > +++ b/drivers/staging/panel/panel.c > @@ -34,6 +34,8 @@ > * > */ > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + If you are going to print __func__ on each line, you can do: #define pr_fmt(fmt) KBUILD_MODNAME "%s: " fmt, __func__ Do you really need to print the function name out everywhere though? > #include > > #include > @@ -1987,10 +1989,9 @@ static struct logical_input *panel_bind_key(char > *name, char *press, > struct logical_input *key; > > key = kzalloc(sizeof(struct logical_input), GFP_KERNEL); > - if (!key) { > - printk(KERN_ERR "panel: not enough memory\n"); > + if (!key) > return NULL; > - } > + > if (!input_name2mask(name, >mask, >value, _mask_i, >_mask_o)) { > kfree(key); > @@ -2030,10 +2031,9 @@ static struct logical_input *panel_bind_callback(char > *name, > struct logical_input *callback; > > callback = kmalloc(sizeof(struct logical_input), GFP_KERNEL); > - if (!callback) { > - printk(KERN_ERR "panel: not enough memory\n"); > + if (!callback) > return NULL; > - } > + > memset(callback, 0, sizeof(struct logical_input)); > if (!input_name2mask(name, >mask, >value, >_mask_i, _mask_o)) > @@ -2110,10 +2110,8 @@ static void panel_attach(struct parport *port) > return; > > if (pprt) { > - printk(KERN_ERR > -"panel_attach(): port->number=%d parport=%d, " > -"already registered !\n", > -port->number, parport); > + pr_err("%s: port->number=%d parport=%d, already registered !\n", > +__func__, port->number, parport); Nitpick - Could remove the space before the '!'. The original has it that, so no big deal if you want to leave it. > return; > } > > @@ -2122,16 +2120,14 @@ static void panel_attach(struct parport *port) > /*PARPORT_DEV_EXCL */ > 0, (void *)); > if (pprt == NULL) { > - pr_err("panel_attach(): port->number=%d parport=%d, " > -"parport_register_device() failed\n", > -port->number, parport); > + pr_err("%s: port->number=%d parport=%d, > parport_register_device() failed\n", > +__func__, port->number, parport); > return; > } > > if (parport_claim(pprt)) { > - printk(KERN_ERR > -"Panel: could not claim access to parport%d. " > -"Aborting.\n", parport); > + pr_err("%s: could not claim access to parport%d. Aborting.\n", > +__func__, parport); > goto err_unreg_device; > } > > @@ -2165,10 +2161,8 @@ static void panel_detach(struct parport *port) > return; > > if (!pprt) { > - printk(KERN_ERR > -"panel_detach(): port->number=%d parport=%d, " > -"nothing to unregister.\n", > -port->number, parport); > + pr_err("%s: port->number=%d parport=%d, nothing to > unregister.\n", > +__func__, port->number, parport); > return; > } > > @@ -2278,8 +2272,8 @@ int panel_init(void) > init_in_progress = 1; > > if (parport_register_driver(_driver)) { > - printk(KERN_ERR > -"Panel: could not register with parport. Aborting.\n"); > + pr_err("%s: could not register with parport. Aborting.\n", > +__func__); > return -EIO; > } > > @@ -2291,8 +2285,8 @@ int
[PATCH v3] panel: Use pr_err(...) rather than printk(KERN_ERR ...)
This change is inspired by checkpatch. Signed-off-by: Toshiaki Yamane --- drivers/staging/panel/panel.c | 42 +--- 1 files changed, 18 insertions(+), 24 deletions(-) diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c index 7365089..a6d71fd 100644 --- a/drivers/staging/panel/panel.c +++ b/drivers/staging/panel/panel.c @@ -34,6 +34,8 @@ * */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include @@ -1987,10 +1989,9 @@ static struct logical_input *panel_bind_key(char *name, char *press, struct logical_input *key; key = kzalloc(sizeof(struct logical_input), GFP_KERNEL); - if (!key) { - printk(KERN_ERR "panel: not enough memory\n"); + if (!key) return NULL; - } + if (!input_name2mask(name, >mask, >value, _mask_i, _mask_o)) { kfree(key); @@ -2030,10 +2031,9 @@ static struct logical_input *panel_bind_callback(char *name, struct logical_input *callback; callback = kmalloc(sizeof(struct logical_input), GFP_KERNEL); - if (!callback) { - printk(KERN_ERR "panel: not enough memory\n"); + if (!callback) return NULL; - } + memset(callback, 0, sizeof(struct logical_input)); if (!input_name2mask(name, >mask, >value, _mask_i, _mask_o)) @@ -2110,10 +2110,8 @@ static void panel_attach(struct parport *port) return; if (pprt) { - printk(KERN_ERR - "panel_attach(): port->number=%d parport=%d, " - "already registered !\n", - port->number, parport); + pr_err("%s: port->number=%d parport=%d, already registered !\n", + __func__, port->number, parport); return; } @@ -2122,16 +2120,14 @@ static void panel_attach(struct parport *port) /*PARPORT_DEV_EXCL */ 0, (void *)); if (pprt == NULL) { - pr_err("panel_attach(): port->number=%d parport=%d, " - "parport_register_device() failed\n", - port->number, parport); + pr_err("%s: port->number=%d parport=%d, parport_register_device() failed\n", + __func__, port->number, parport); return; } if (parport_claim(pprt)) { - printk(KERN_ERR - "Panel: could not claim access to parport%d. " - "Aborting.\n", parport); + pr_err("%s: could not claim access to parport%d. Aborting.\n", + __func__, parport); goto err_unreg_device; } @@ -2165,10 +2161,8 @@ static void panel_detach(struct parport *port) return; if (!pprt) { - printk(KERN_ERR - "panel_detach(): port->number=%d parport=%d, " - "nothing to unregister.\n", - port->number, parport); + pr_err("%s: port->number=%d parport=%d, nothing to unregister.\n", + __func__, port->number, parport); return; } @@ -2278,8 +2272,8 @@ int panel_init(void) init_in_progress = 1; if (parport_register_driver(_driver)) { - printk(KERN_ERR - "Panel: could not register with parport. Aborting.\n"); + pr_err("%s: could not register with parport. Aborting.\n", + __func__); return -EIO; } @@ -2291,8 +2285,8 @@ int panel_init(void) pprt = NULL; } parport_unregister_driver(_driver); - printk(KERN_ERR "Panel driver version " PANEL_VERSION - " disabled.\n"); + pr_err("%s: Panel driver version " PANEL_VERSION " disabled.\n", + __func__); return -ENODEV; } -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3] panel: Use pr_err(...) rather than printk(KERN_ERR ...)
This change is inspired by checkpatch. Signed-off-by: Toshiaki Yamane yamaneto...@gmail.com --- drivers/staging/panel/panel.c | 42 +--- 1 files changed, 18 insertions(+), 24 deletions(-) diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c index 7365089..a6d71fd 100644 --- a/drivers/staging/panel/panel.c +++ b/drivers/staging/panel/panel.c @@ -34,6 +34,8 @@ * */ +#define pr_fmt(fmt) KBUILD_MODNAME : fmt + #include linux/module.h #include linux/types.h @@ -1987,10 +1989,9 @@ static struct logical_input *panel_bind_key(char *name, char *press, struct logical_input *key; key = kzalloc(sizeof(struct logical_input), GFP_KERNEL); - if (!key) { - printk(KERN_ERR panel: not enough memory\n); + if (!key) return NULL; - } + if (!input_name2mask(name, key-mask, key-value, scan_mask_i, scan_mask_o)) { kfree(key); @@ -2030,10 +2031,9 @@ static struct logical_input *panel_bind_callback(char *name, struct logical_input *callback; callback = kmalloc(sizeof(struct logical_input), GFP_KERNEL); - if (!callback) { - printk(KERN_ERR panel: not enough memory\n); + if (!callback) return NULL; - } + memset(callback, 0, sizeof(struct logical_input)); if (!input_name2mask(name, callback-mask, callback-value, scan_mask_i, scan_mask_o)) @@ -2110,10 +2110,8 @@ static void panel_attach(struct parport *port) return; if (pprt) { - printk(KERN_ERR - panel_attach(): port-number=%d parport=%d, - already registered !\n, - port-number, parport); + pr_err(%s: port-number=%d parport=%d, already registered !\n, + __func__, port-number, parport); return; } @@ -2122,16 +2120,14 @@ static void panel_attach(struct parport *port) /*PARPORT_DEV_EXCL */ 0, (void *)pprt); if (pprt == NULL) { - pr_err(panel_attach(): port-number=%d parport=%d, - parport_register_device() failed\n, - port-number, parport); + pr_err(%s: port-number=%d parport=%d, parport_register_device() failed\n, + __func__, port-number, parport); return; } if (parport_claim(pprt)) { - printk(KERN_ERR - Panel: could not claim access to parport%d. - Aborting.\n, parport); + pr_err(%s: could not claim access to parport%d. Aborting.\n, + __func__, parport); goto err_unreg_device; } @@ -2165,10 +2161,8 @@ static void panel_detach(struct parport *port) return; if (!pprt) { - printk(KERN_ERR - panel_detach(): port-number=%d parport=%d, - nothing to unregister.\n, - port-number, parport); + pr_err(%s: port-number=%d parport=%d, nothing to unregister.\n, + __func__, port-number, parport); return; } @@ -2278,8 +2272,8 @@ int panel_init(void) init_in_progress = 1; if (parport_register_driver(panel_driver)) { - printk(KERN_ERR - Panel: could not register with parport. Aborting.\n); + pr_err(%s: could not register with parport. Aborting.\n, + __func__); return -EIO; } @@ -2291,8 +2285,8 @@ int panel_init(void) pprt = NULL; } parport_unregister_driver(panel_driver); - printk(KERN_ERR Panel driver version PANEL_VERSION - disabled.\n); + pr_err(%s: Panel driver version PANEL_VERSION disabled.\n, + __func__); return -ENODEV; } -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] panel: Use pr_err(...) rather than printk(KERN_ERR ...)
On 12/07/12 12:35, Toshiaki Yamane wrote: This change is inspired by checkpatch. Your changelog needs to describe all of the changes you are making. The subject line only describes one. This patch is doing the following: - Converting printk(KERN_ERR to pr_err - Adding __func__ prefixes to printk lines - Refactoring split printk strings onto a single line There are a few other printks in this file which could be converted to pr_* to make the code more consistent. Perhaps a follow up patch? Typically for a sub-sequent version of a patch/series you should list the changes since the last round. Put these below the --- so that they don't become part of the change log, e.g.: Signed-off-by: Your name/email here --- Changes since v2: - Some stuff Changes since v1: - Some other stuff Some more comments below. ~Ryan Signed-off-by: Toshiaki Yamane yamaneto...@gmail.com --- drivers/staging/panel/panel.c | 42 +--- 1 files changed, 18 insertions(+), 24 deletions(-) diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c index 7365089..a6d71fd 100644 --- a/drivers/staging/panel/panel.c +++ b/drivers/staging/panel/panel.c @@ -34,6 +34,8 @@ * */ +#define pr_fmt(fmt) KBUILD_MODNAME : fmt + If you are going to print __func__ on each line, you can do: #define pr_fmt(fmt) KBUILD_MODNAME %s: fmt, __func__ Do you really need to print the function name out everywhere though? #include linux/module.h #include linux/types.h @@ -1987,10 +1989,9 @@ static struct logical_input *panel_bind_key(char *name, char *press, struct logical_input *key; key = kzalloc(sizeof(struct logical_input), GFP_KERNEL); - if (!key) { - printk(KERN_ERR panel: not enough memory\n); + if (!key) return NULL; - } + if (!input_name2mask(name, key-mask, key-value, scan_mask_i, scan_mask_o)) { kfree(key); @@ -2030,10 +2031,9 @@ static struct logical_input *panel_bind_callback(char *name, struct logical_input *callback; callback = kmalloc(sizeof(struct logical_input), GFP_KERNEL); - if (!callback) { - printk(KERN_ERR panel: not enough memory\n); + if (!callback) return NULL; - } + memset(callback, 0, sizeof(struct logical_input)); if (!input_name2mask(name, callback-mask, callback-value, scan_mask_i, scan_mask_o)) @@ -2110,10 +2110,8 @@ static void panel_attach(struct parport *port) return; if (pprt) { - printk(KERN_ERR -panel_attach(): port-number=%d parport=%d, -already registered !\n, -port-number, parport); + pr_err(%s: port-number=%d parport=%d, already registered !\n, +__func__, port-number, parport); Nitpick - Could remove the space before the '!'. The original has it that, so no big deal if you want to leave it. return; } @@ -2122,16 +2120,14 @@ static void panel_attach(struct parport *port) /*PARPORT_DEV_EXCL */ 0, (void *)pprt); if (pprt == NULL) { - pr_err(panel_attach(): port-number=%d parport=%d, -parport_register_device() failed\n, -port-number, parport); + pr_err(%s: port-number=%d parport=%d, parport_register_device() failed\n, +__func__, port-number, parport); return; } if (parport_claim(pprt)) { - printk(KERN_ERR -Panel: could not claim access to parport%d. -Aborting.\n, parport); + pr_err(%s: could not claim access to parport%d. Aborting.\n, +__func__, parport); goto err_unreg_device; } @@ -2165,10 +2161,8 @@ static void panel_detach(struct parport *port) return; if (!pprt) { - printk(KERN_ERR -panel_detach(): port-number=%d parport=%d, -nothing to unregister.\n, -port-number, parport); + pr_err(%s: port-number=%d parport=%d, nothing to unregister.\n, +__func__, port-number, parport); return; } @@ -2278,8 +2272,8 @@ int panel_init(void) init_in_progress = 1; if (parport_register_driver(panel_driver)) { - printk(KERN_ERR -Panel: could not register with parport. Aborting.\n); + pr_err(%s: could not register with parport. Aborting.\n, +__func__); return -EIO; } @@ -2291,8 +2285,8 @@ int panel_init(void) pprt = NULL; }
Re: [PATCH v3] panel: Use pr_err(...) rather than printk(KERN_ERR ...)
On Thu, 2012-07-12 at 15:22 +1000, Ryan Mallon wrote: On 12/07/12 12:35, Toshiaki Yamane wrote: This change is inspired by checkpatch. Your changelog needs to describe all of the changes you are making. The subject line only describes one. This patch is doing the following: - Converting printk(KERN_ERR to pr_err - Adding __func__ prefixes to printk lines - Refactoring split printk strings onto a single line There are a few other printks in this file which could be converted to pr_* to make the code more consistent. Perhaps a follow up patch? Typically for a sub-sequent version of a patch/series you should list the changes since the last round. Put these below the --- so that they don't become part of the change log, e.g.: Signed-off-by: Your name/email here --- Changes since v2: - Some stuff Changes since v1: - Some other stuff Some more comments below. And ideally cc the people that gave you notes/comments on your previous patches too. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/