* [PATCH 1/2] mtd: nand: generalized error messages with __func__
@ 2011-05-25 21:59 Brian Norris
2011-05-25 21:59 ` [PATCH 2/2] mtd: nand: multi-line comment style fixups Brian Norris
2011-05-26 5:14 ` [PATCH 1/2] mtd: nand: generalized error messages with __func__ Igor Grinberg
0 siblings, 2 replies; 13+ messages in thread
From: Brian Norris @ 2011-05-25 21:59 UTC (permalink / raw)
To: linux-mtd; +Cc: Brian Norris, David Woodhouse, Artem Bityutskiy
These simple printk error messages can be a little simpler to maintain
when they use the __func__ identifier.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
drivers/mtd/nand/nand_bbt.c | 8 ++++----
drivers/mtd/nand/rtc_from4.c | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index ccbeaa1..7936a6c 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -1164,7 +1164,7 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
/* Allocate memory (2bit per block) and clear the memory bad block table */
this->bbt = kzalloc(len, GFP_KERNEL);
if (!this->bbt) {
- printk(KERN_ERR "nand_scan_bbt: Out of memory\n");
+ printk(KERN_ERR "%s: Out of memory\n", __func__);
return -ENOMEM;
}
@@ -1187,7 +1187,7 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
len += (len >> this->page_shift) * mtd->oobsize;
buf = vmalloc(len);
if (!buf) {
- printk(KERN_ERR "nand_bbt: Out of memory\n");
+ printk(KERN_ERR "%s: Out of memory\n", __func__);
kfree(this->bbt);
this->bbt = NULL;
return -ENOMEM;
@@ -1237,7 +1237,7 @@ int nand_update_bbt(struct mtd_info *mtd, loff_t offs)
len += (len >> this->page_shift) * mtd->oobsize;
buf = kmalloc(len, GFP_KERNEL);
if (!buf) {
- printk(KERN_ERR "nand_update_bbt: Out of memory\n");
+ printk(KERN_ERR "%s: Out of memory\n", __func__);
return -ENOMEM;
}
@@ -1351,7 +1351,7 @@ static int nand_create_default_bbt_descr(struct nand_chip *this)
}
bd = kzalloc(sizeof(*bd), GFP_KERNEL);
if (!bd) {
- printk(KERN_ERR "nand_create_default_bbt_descr: Out of memory\n");
+ printk(KERN_ERR "%s: Out of memory\n", __func__);
return -ENOMEM;
}
bd->options = this->options & BBT_SCAN_OPTIONS;
diff --git a/drivers/mtd/nand/rtc_from4.c b/drivers/mtd/nand/rtc_from4.c
index c9f9127..7e02c94 100644
--- a/drivers/mtd/nand/rtc_from4.c
+++ b/drivers/mtd/nand/rtc_from4.c
@@ -444,7 +444,7 @@ static int rtc_from4_errstat(struct mtd_info *mtd, struct nand_chip *this,
len = mtd->writesize;
buf = kmalloc(len, GFP_KERNEL);
if (!buf) {
- printk(KERN_ERR "rtc_from4_errstat: Out of memory!\n");
+ printk(KERN_ERR "%s: Out of memory!\n", __func__);
er_stat = 1;
goto out;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] mtd: nand: multi-line comment style fixups
2011-05-25 21:59 [PATCH 1/2] mtd: nand: generalized error messages with __func__ Brian Norris
@ 2011-05-25 21:59 ` Brian Norris
2011-06-06 14:26 ` Artem Bityutskiy
2011-05-26 5:14 ` [PATCH 1/2] mtd: nand: generalized error messages with __func__ Igor Grinberg
1 sibling, 1 reply; 13+ messages in thread
From: Brian Norris @ 2011-05-25 21:59 UTC (permalink / raw)
To: linux-mtd; +Cc: Brian Norris, David Woodhouse, Brian Norris, Artem Bityutskiy
From: Brian Norris <norris@broadcom.com>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
drivers/mtd/nand/nand_base.c | 50 ++++++++++++++++++++++++++---------------
drivers/mtd/nand/nand_bbt.c | 38 ++++++++++++++++++++-----------
2 files changed, 56 insertions(+), 32 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index a46e9bb..f591c21 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -410,7 +410,8 @@ static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
else {
nand_get_device(chip, mtd, FL_WRITING);
- /* Write to first two pages and to byte 1 and 6 if necessary.
+ /*
+ * Write to first two pages and to byte 1 and 6 if necessary.
* If we write to more than one location, the first error
* encountered quits the procedure. We write two bytes per
* location, so we dont have to mess with 16 bit access.
@@ -625,8 +626,10 @@ static void nand_command(struct mtd_info *mtd, unsigned int command,
return;
}
}
- /* Apply this short delay always to ensure that we do wait tWB in
- * any case on any machine. */
+ /*
+ * Apply this short delay always to ensure that we do wait tWB in
+ * any case on any machine.
+ */
ndelay(100);
nand_wait_ready(mtd);
@@ -747,8 +750,10 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
}
}
- /* Apply this short delay always to ensure that we do wait tWB in
- * any case on any machine. */
+ /*
+ * Apply this short delay always to ensure that we do wait tWB in
+ * any case on any machine.
+ */
ndelay(100);
nand_wait_ready(mtd);
@@ -859,8 +864,10 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *chip)
led_trigger_event(nand_led_trigger, LED_FULL);
- /* Apply this short delay always to ensure that we do wait tWB in
- * any case on any machine. */
+ /*
+ * Apply this short delay always to ensure that we do wait tWB in
+ * any case on any machine.
+ */
ndelay(100);
if ((state == FL_ERASING) && (chip->options & NAND_IS_AND))
@@ -1187,9 +1194,10 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
for (i = 0; i < eccfrag_len ; i += chip->ecc.bytes, p += chip->ecc.size)
chip->ecc.calculate(mtd, p, &chip->buffers->ecccalc[i]);
- /* The performance is faster if to position offsets
- according to ecc.pos. Let make sure here that
- there are no gaps in ecc positions */
+ /*
+ * The performance is faster if we position offsets according to
+ * ecc.pos. Let's make sure that there are no gaps in ecc positions.
+ */
for (i = 0; i < eccfrag_len - 1; i++) {
if (eccpos[i + start_step * chip->ecc.bytes] + 1 !=
eccpos[i + start_step * chip->ecc.bytes + 1]) {
@@ -1552,7 +1560,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
chip->select_chip(mtd, chipnr);
}
- /* Check, if the chip supports auto page increment
+ /*
+ * Check, if the chip supports auto page increment
* or if we have hit a block boundary.
*/
if (!NAND_CANAUTOINCR(chip) || !(page & blkcheck))
@@ -1830,7 +1839,8 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
chip->select_chip(mtd, chipnr);
}
- /* Check, if the chip supports auto page increment
+ /*
+ * Check, if the chip supports auto page increment
* or if we have hit a block boundary.
*/
if (!NAND_CANAUTOINCR(chip) || !(page & blkcheck))
@@ -2924,7 +2934,8 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
*maf_id = chip->read_byte(mtd);
*dev_id = chip->read_byte(mtd);
- /* Try again to make sure, as some systems the bus-hold or other
+ /*
+ * Try again to make sure, as some systems the bus-hold or other
* interface concerns can cause random data which looks like a
* possibly credible NAND flash to appear. If the two results do
* not match, ignore the device completely.
@@ -2973,7 +2984,7 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
chip->chipsize = (uint64_t)type->chipsize << 20;
if (!type->pagesize && chip->init_size) {
- /* set the pagesize, oobsize, erasesize by the driver*/
+ /* set the pagesize, oobsize, erasesize by the driver */
busw = chip->init_size(mtd, chip, id_data);
} else if (!type->pagesize) {
int extid;
@@ -3057,8 +3068,9 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
chip->options &= ~NAND_CHIPOPTIONS_MSK;
chip->options |= type->options & NAND_CHIPOPTIONS_MSK;
- /* Check if chip is a not a samsung device. Do not clear the
- * options for chips which are not having an extended id.
+ /*
+ * Check if chip is not a Samsung device. Do not clear the
+ * options for chips which do not have an extended id.
*/
if (*maf_id != NAND_MFR_SAMSUNG && !type->pagesize)
chip->options &= ~NAND_SAMSUNG_LP_OPTIONS;
@@ -3481,9 +3493,11 @@ int nand_scan_tail(struct mtd_info *mtd)
}
EXPORT_SYMBOL(nand_scan_tail);
-/* is_module_text_address() isn't exported, and it's mostly a pointless
+/*
+ * is_module_text_address() isn't exported, and it's mostly a pointless
* test if this is a module _anyway_ -- they'd have to try _really_ hard
- * to call us from in-kernel code if the core NAND support is modular. */
+ * to call us from in-kernel code if the core NAND support is modular.
+ */
#ifdef MODULE
#define caller_is_module() (1)
#else
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 7936a6c..64a3cd5 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -258,8 +258,10 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,
mtd->ecc_stats.bbtblocks++;
continue;
}
- /* Leave it for now, if its matured we can move this
- * message to MTD_DEBUG_LEVEL0 */
+ /*
+ * Leave it for now, if it's matured we can
+ * move this message to MTD_DEBUG_LEVEL0
+ */
printk(KERN_DEBUG "nand_read_bbt: Bad block at 0x%012llx\n",
(loff_t)((offs << 2) + (act >> 1)) << this->bbt_erase_shift);
/* Factory marked bad or worn out ? */
@@ -529,8 +531,10 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
}
if (chip == -1) {
- /* Note that numblocks is 2 * (real numblocks) here, see i+=2
- * below as it makes shifting and masking less painful */
+ /*
+ * Note that numblocks is 2 * (real numblocks) here, see i+=2
+ * below as it makes shifting and masking less painful
+ */
numblocks = mtd->size >> (this->bbt_erase_shift - 1);
startblock = 0;
from = 0;
@@ -732,7 +736,8 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
/* Loop through the chips */
for (; chip < nrchips; chip++) {
- /* There was already a version of the table, reuse the page
+ /*
+ * There was already a version of the table, reuse the page
* This applies for absolute placement too, as we have the
* page nr. in td->pages.
*/
@@ -1082,9 +1087,11 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
update = 1;
block += 2;
}
- /* If we want reserved blocks to be recorded to flash, and some
- new ones have been marked, then we need to update the stored
- bbts. This should only happen once. */
+ /*
+ * If we want reserved blocks to be recorded to flash, and some
+ * new ones have been marked, then we need to update the stored
+ * bbts. This should only happen once.
+ */
if (update && td->reserved_block_code)
nand_update_bbt(mtd, (loff_t)(block - 2) << (this->bbt_erase_shift - 1));
}
@@ -1168,7 +1175,8 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
return -ENOMEM;
}
- /* If no primary table decriptor is given, scan the device
+ /*
+ * If no primary table decriptor is given, scan the device
* to build a memory based bad block table
*/
if (!td) {
@@ -1272,8 +1280,10 @@ int nand_update_bbt(struct mtd_info *mtd, loff_t offs)
return res;
}
-/* Define some generic bad / good block scan pattern which are used
- * while scanning a device for factory marked good / bad blocks. */
+/*
+ * Define some generic bad / good block scan pattern which are used
+ * while scanning a device for factory marked good / bad blocks.
+ */
static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
static uint8_t scan_agand_pattern[] = { 0x1C, 0x71, 0xC7, 0x1C, 0x71, 0xC7 };
@@ -1285,8 +1295,7 @@ static struct nand_bbt_descr agand_flashbased = {
.pattern = scan_agand_pattern
};
-/* Generic flash bbt decriptors
-*/
+/* Generic flash bbt decriptors */
static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
@@ -1375,7 +1384,8 @@ int nand_default_bbt(struct mtd_info *mtd)
{
struct nand_chip *this = mtd->priv;
- /* Default for AG-AND. We must use a flash based
+ /*
+ * Default for AG-AND. We must use a flash based
* bad block table as the devices have factory marked
* _good_ blocks. Erasing those blocks leads to loss
* of the good / bad information, so we _must_ store
--
1.7.0.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: nand: generalized error messages with __func__
2011-05-25 21:59 [PATCH 1/2] mtd: nand: generalized error messages with __func__ Brian Norris
2011-05-25 21:59 ` [PATCH 2/2] mtd: nand: multi-line comment style fixups Brian Norris
@ 2011-05-26 5:14 ` Igor Grinberg
2011-05-26 5:16 ` Artem Bityutskiy
1 sibling, 1 reply; 13+ messages in thread
From: Igor Grinberg @ 2011-05-26 5:14 UTC (permalink / raw)
To: Brian Norris; +Cc: David Woodhouse, linux-mtd, Artem Bityutskiy
On 05/26/11 00:59, Brian Norris wrote:
> These simple printk error messages can be a little simpler to maintain
> when they use the __func__ identifier.
While this is a good thing you are doing, I'd suggest using pr_err macro
and may be even pr_fmt.
pr_err() will save you from the need to define the log level each time.
pr_fmt will save you the need to add %s: and __func__.
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
> drivers/mtd/nand/nand_bbt.c | 8 ++++----
> drivers/mtd/nand/rtc_from4.c | 2 +-
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
> index ccbeaa1..7936a6c 100644
> --- a/drivers/mtd/nand/nand_bbt.c
> +++ b/drivers/mtd/nand/nand_bbt.c
> @@ -1164,7 +1164,7 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
> /* Allocate memory (2bit per block) and clear the memory bad block table */
> this->bbt = kzalloc(len, GFP_KERNEL);
> if (!this->bbt) {
> - printk(KERN_ERR "nand_scan_bbt: Out of memory\n");
> + printk(KERN_ERR "%s: Out of memory\n", __func__);
> return -ENOMEM;
> }
>
> @@ -1187,7 +1187,7 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
> len += (len >> this->page_shift) * mtd->oobsize;
> buf = vmalloc(len);
> if (!buf) {
> - printk(KERN_ERR "nand_bbt: Out of memory\n");
> + printk(KERN_ERR "%s: Out of memory\n", __func__);
> kfree(this->bbt);
> this->bbt = NULL;
> return -ENOMEM;
> @@ -1237,7 +1237,7 @@ int nand_update_bbt(struct mtd_info *mtd, loff_t offs)
> len += (len >> this->page_shift) * mtd->oobsize;
> buf = kmalloc(len, GFP_KERNEL);
> if (!buf) {
> - printk(KERN_ERR "nand_update_bbt: Out of memory\n");
> + printk(KERN_ERR "%s: Out of memory\n", __func__);
> return -ENOMEM;
> }
>
> @@ -1351,7 +1351,7 @@ static int nand_create_default_bbt_descr(struct nand_chip *this)
> }
> bd = kzalloc(sizeof(*bd), GFP_KERNEL);
> if (!bd) {
> - printk(KERN_ERR "nand_create_default_bbt_descr: Out of memory\n");
> + printk(KERN_ERR "%s: Out of memory\n", __func__);
> return -ENOMEM;
> }
> bd->options = this->options & BBT_SCAN_OPTIONS;
> diff --git a/drivers/mtd/nand/rtc_from4.c b/drivers/mtd/nand/rtc_from4.c
> index c9f9127..7e02c94 100644
> --- a/drivers/mtd/nand/rtc_from4.c
> +++ b/drivers/mtd/nand/rtc_from4.c
> @@ -444,7 +444,7 @@ static int rtc_from4_errstat(struct mtd_info *mtd, struct nand_chip *this,
> len = mtd->writesize;
> buf = kmalloc(len, GFP_KERNEL);
> if (!buf) {
> - printk(KERN_ERR "rtc_from4_errstat: Out of memory!\n");
> + printk(KERN_ERR "%s: Out of memory!\n", __func__);
> er_stat = 1;
> goto out;
> }
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: nand: generalized error messages with __func__
2011-05-26 5:14 ` [PATCH 1/2] mtd: nand: generalized error messages with __func__ Igor Grinberg
@ 2011-05-26 5:16 ` Artem Bityutskiy
2011-05-31 18:52 ` Brian Norris
0 siblings, 1 reply; 13+ messages in thread
From: Artem Bityutskiy @ 2011-05-26 5:16 UTC (permalink / raw)
To: Igor Grinberg; +Cc: David Woodhouse, Brian Norris, linux-mtd
On Thu, 2011-05-26 at 08:14 +0300, Igor Grinberg wrote:
> On 05/26/11 00:59, Brian Norris wrote:
>
> > These simple printk error messages can be a little simpler to maintain
> > when they use the __func__ identifier.
>
> While this is a good thing you are doing, I'd suggest using pr_err macro
> and may be even pr_fmt.
>
> pr_err() will save you from the need to define the log level each time.
> pr_fmt will save you the need to add %s: and __func__.
Right, but in this _particular_ case the best thing to do is to just
kill the error messages - if any of the allocation functions fail they
print a large scary warning with a backtrace anyway, and the backtrace
will contain the caller function names.
Brian, would you please instead just zap the prints?
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: nand: generalized error messages with __func__
2011-05-26 5:16 ` Artem Bityutskiy
@ 2011-05-31 18:52 ` Brian Norris
2011-05-31 20:19 ` Mike Frysinger
2011-06-01 10:30 ` Artem Bityutskiy
0 siblings, 2 replies; 13+ messages in thread
From: Brian Norris @ 2011-05-31 18:52 UTC (permalink / raw)
To: dedekind1; +Cc: David Woodhouse, linux-mtd, Igor Grinberg
Hi,
Sorry if this is a second message anyone, I didn't hit Reply-All.
On Wed, May 25, 2011 at 10:16 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> Right, but in this _particular_ case the best thing to do is to just
> kill the error messages - if any of the allocation functions fail they
> print a large scary warning with a backtrace anyway, and the backtrace
> will contain the caller function names.
>
> Brian, would you please instead just zap the prints?
Sure, can do. In general, then, is it best practice to simply 'return
-ENOMEM' or similar (without any printing) in all MTD code?
> On Thu, 2011-05-26 at 08:14 +0300, Igor Grinberg wrote:
>> On 05/26/11 00:59, Brian Norris wrote:
>> pr_err() will save you from the need to define the log level each time.
>> pr_fmt will save you the need to add %s: and __func__.
Also, it seems that the second statement is not necessarily true. In
fact, in include/linux/printk.h, we default to the following
declaration:
#define pr_fmt(fmt) fmt
So it seems that, when we desire to print out the relevant function
name, we should manually use __func__ instead of relying on
pr_fmt...or should we define our own pr_fmt in include/linux/mtd.h?
Brian
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: nand: generalized error messages with __func__
2011-05-31 18:52 ` Brian Norris
@ 2011-05-31 20:19 ` Mike Frysinger
2011-06-01 10:30 ` Artem Bityutskiy
1 sibling, 0 replies; 13+ messages in thread
From: Mike Frysinger @ 2011-05-31 20:19 UTC (permalink / raw)
To: Brian Norris; +Cc: linux-mtd, David Woodhouse, Igor Grinberg, dedekind1
On Tue, May 31, 2011 at 14:52, Brian Norris wrote:
>> On Thu, 2011-05-26 at 08:14 +0300, Igor Grinberg wrote:
>>> On 05/26/11 00:59, Brian Norris wrote:
>>> pr_err() will save you from the need to define the log level each time.
>>> pr_fmt will save you the need to add %s: and __func__.
>
> Also, it seems that the second statement is not necessarily true. In
> fact, in include/linux/printk.h, we default to the following
> declaration:
> #define pr_fmt(fmt) fmt
> So it seems that, when we desire to print out the relevant function
> name, we should manually use __func__ instead of relying on
> pr_fmt...or should we define our own pr_fmt in include/linux/mtd.h?
while it might run afoul of some people's sense of style, it is
technically possible. try something like this:
#define pr_fmt(fmt) "%s: " fmt, __func__
-mike
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: nand: generalized error messages with __func__
2011-05-31 18:52 ` Brian Norris
2011-05-31 20:19 ` Mike Frysinger
@ 2011-06-01 10:30 ` Artem Bityutskiy
2011-06-01 18:37 ` Brian Norris
1 sibling, 1 reply; 13+ messages in thread
From: Artem Bityutskiy @ 2011-06-01 10:30 UTC (permalink / raw)
To: Brian Norris; +Cc: David Woodhouse, linux-mtd, Igor Grinberg
Hi,
On Tue, 2011-05-31 at 11:52 -0700, Brian Norris wrote:
> > Brian, would you please instead just zap the prints?
>
> Sure, can do. In general, then, is it best practice to simply 'return
> -ENOMEM' or similar (without any printing) in all MTD code?
Yes.
> > On Thu, 2011-05-26 at 08:14 +0300, Igor Grinberg wrote:
> >> On 05/26/11 00:59, Brian Norris wrote:
> >> pr_err() will save you from the need to define the log level each time.
> >> pr_fmt will save you the need to add %s: and __func__.
>
> Also, it seems that the second statement is not necessarily true. In
> fact, in include/linux/printk.h, we default to the following
> declaration:
> #define pr_fmt(fmt) fmt
To be more precise:
#ifndef pr_fmt
#define pr_fmt(fmt) fmt
#endif
> So it seems that, when we desire to print out the relevant function
> name, we should manually use __func__ instead of relying on
> pr_fmt...or should we define our own pr_fmt in include/linux/mtd.h?
Yes, before you include files.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: nand: generalized error messages with __func__
2011-06-01 10:30 ` Artem Bityutskiy
@ 2011-06-01 18:37 ` Brian Norris
2011-06-03 15:35 ` Artem Bityutskiy
2011-06-07 7:26 ` Igor Grinberg
0 siblings, 2 replies; 13+ messages in thread
From: Brian Norris @ 2011-06-01 18:37 UTC (permalink / raw)
To: dedekind1; +Cc: David Woodhouse, linux-mtd, Igor Grinberg
On Wed, Jun 1, 2011 at 3:30 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> or should we define our own pr_fmt in include/linux/mtd.h?
>
> Yes, before you include files.
Well, I've tried this, and as I suspected, I can't just include it in
mtd.h at the top, since we can't guarantee that <linux/mtd/mtd.h> is
going to be the first included header in other files. Specifically, in
include/linux/mtd/nand.h, we include other headers before mtd.h and so
printk.h has already defined pr_fmt() for us, so this doesn't work.
I'm not sure if there's a nice place to define pr_fmt() right now...
Brian
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: nand: generalized error messages with __func__
2011-06-01 18:37 ` Brian Norris
@ 2011-06-03 15:35 ` Artem Bityutskiy
2011-06-07 7:26 ` Igor Grinberg
1 sibling, 0 replies; 13+ messages in thread
From: Artem Bityutskiy @ 2011-06-03 15:35 UTC (permalink / raw)
To: Brian Norris; +Cc: David Woodhouse, linux-mtd, Igor Grinberg
On Wed, 2011-06-01 at 11:37 -0700, Brian Norris wrote:
> On Wed, Jun 1, 2011 at 3:30 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> >> or should we define our own pr_fmt in include/linux/mtd.h?
> >
> > Yes, before you include files.
>
> Well, I've tried this, and as I suspected, I can't just include it in
> mtd.h at the top, since we can't guarantee that <linux/mtd/mtd.h> is
> going to be the first included header in other files.
In any case we do not want to blow the kernel binary size by too many
__func__ strings. They should be used only when needed.
> Specifically, in
> include/linux/mtd/nand.h, we include other headers before mtd.h and so
> printk.h has already defined pr_fmt() for us, so this doesn't work.
> I'm not sure if there's a nice place to define pr_fmt() right now...
.c files, AFAIU.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] mtd: nand: multi-line comment style fixups
2011-05-25 21:59 ` [PATCH 2/2] mtd: nand: multi-line comment style fixups Brian Norris
@ 2011-06-06 14:26 ` Artem Bityutskiy
2011-06-06 15:21 ` Brian Norris
0 siblings, 1 reply; 13+ messages in thread
From: Artem Bityutskiy @ 2011-06-06 14:26 UTC (permalink / raw)
To: Brian Norris; +Cc: David Woodhouse, linux-mtd, Brian Norris
On Wed, 2011-05-25 at 14:59 -0700, Brian Norris wrote:
> From: Brian Norris <norris@broadcom.com>
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
I kind of rejected the first patch in this series, but forgot about this
one. I've now pushed it to the l2 tree with additional comments
clean-ups from me - just did them while looking at your patch and to the
code, and accidentally used git commit --amend, so they were merged with
yours. But this should not be a big deal - we ended up with one big
comments cleanup patch.
Thanks - it is in the l2-mtd-2.6.git now.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] mtd: nand: multi-line comment style fixups
2011-06-06 14:26 ` Artem Bityutskiy
@ 2011-06-06 15:21 ` Brian Norris
0 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2011-06-06 15:21 UTC (permalink / raw)
To: dedekind1; +Cc: David Woodhouse, linux-mtd, Brian Norris
On Mon, Jun 6, 2011 at 7:26 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> I kind of rejected the first patch in this series, but forgot about this
> one. I've now pushed it to the l2 tree with additional comments
> clean-ups from me - just did them while looking at your patch and to the
> code, and accidentally used git commit --amend, so they were merged with
> yours. But this should not be a big deal - we ended up with one big
> comments cleanup patch.
>
> Thanks - it is in the l2-mtd-2.6.git now.
No problem. Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: nand: generalized error messages with __func__
2011-06-01 18:37 ` Brian Norris
2011-06-03 15:35 ` Artem Bityutskiy
@ 2011-06-07 7:26 ` Igor Grinberg
2011-06-07 22:57 ` Brian Norris
1 sibling, 1 reply; 13+ messages in thread
From: Igor Grinberg @ 2011-06-07 7:26 UTC (permalink / raw)
To: Brian Norris; +Cc: David Woodhouse, linux-mtd, dedekind1
On 06/01/11 21:37, Brian Norris wrote:
> On Wed, Jun 1, 2011 at 3:30 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>>> or should we define our own pr_fmt in include/linux/mtd.h?
>> Yes, before you include files.
> Well, I've tried this, and as I suspected, I can't just include it in
> mtd.h at the top, since we can't guarantee that <linux/mtd/mtd.h> is
> going to be the first included header in other files. Specifically, in
> include/linux/mtd/nand.h, we include other headers before mtd.h and so
> printk.h has already defined pr_fmt() for us, so this doesn't work.
> I'm not sure if there's a nice place to define pr_fmt() right now...
You should not define pr_fmt() in .h file.
pr_fmt() should be defined in _.c_ file at the top - _before any include is done_.
And yes, like Artem already said, _only_ in those _.c_ files where we want to use it.
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: nand: generalized error messages with __func__
2011-06-07 7:26 ` Igor Grinberg
@ 2011-06-07 22:57 ` Brian Norris
0 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2011-06-07 22:57 UTC (permalink / raw)
To: Igor Grinberg; +Cc: David Woodhouse, linux-mtd, dedekind1
On Tue, Jun 7, 2011 at 12:26 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> You should not define pr_fmt() in .h file.
> pr_fmt() should be defined in _.c_ file at the top - _before any include is done_.
> And yes, like Artem already said, _only_ in those _.c_ files where we want to use it.
Hmm, well I guess I'll try defining pr_fmt() in nand_bbt.c only. Look
for the following patchset shortly:
* removing those printk's when a memory allocations fail
* changing other printk's to pr_*'s
* changing some DEBUG messages to dev_dbg
* define pr_fmt() to include __func__ in debug messages for nand_bbt.c
The only inconsistency with the 4th patch is that it adds the __func__
prefix to some print messages that didn't have them previously. Thus,
I'd be fine if the 4th patch is dropped.
Thanks,
Brian
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-06-07 22:57 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-25 21:59 [PATCH 1/2] mtd: nand: generalized error messages with __func__ Brian Norris
2011-05-25 21:59 ` [PATCH 2/2] mtd: nand: multi-line comment style fixups Brian Norris
2011-06-06 14:26 ` Artem Bityutskiy
2011-06-06 15:21 ` Brian Norris
2011-05-26 5:14 ` [PATCH 1/2] mtd: nand: generalized error messages with __func__ Igor Grinberg
2011-05-26 5:16 ` Artem Bityutskiy
2011-05-31 18:52 ` Brian Norris
2011-05-31 20:19 ` Mike Frysinger
2011-06-01 10:30 ` Artem Bityutskiy
2011-06-01 18:37 ` Brian Norris
2011-06-03 15:35 ` Artem Bityutskiy
2011-06-07 7:26 ` Igor Grinberg
2011-06-07 22:57 ` Brian Norris
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.