| Message ID | 1405989293-6629-3-git-send-email-marex@denx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Scott Wood |
| Headers | show |
On Tue, 2014-07-22 at 02:34 +0200, Marek Vasut wrote:
> Printing u32 with %02x is just a bad idea, fix it.
Why is it "just a bad idea" if the values aren't expected to exceed
0xff?
-Scott
On Tuesday, July 29, 2014 at 01:35:57 AM, Scott Wood wrote: > On Tue, 2014-07-22 at 02:34 +0200, Marek Vasut wrote: > > Printing u32 with %02x is just a bad idea, fix it. > > Why is it "just a bad idea" if the values aren't expected to exceed > 0xff? NAND_CMD_DEPLETE1 is 0x100 for example. I doubt anyone will use AND with this controller, but I'd be much happier to see the print properly matching the variable we're printing. Best regards, Marek Vasut
On Tue, 2014-07-29 at 02:09 +0200, Marek Vasut wrote: > On Tuesday, July 29, 2014 at 01:35:57 AM, Scott Wood wrote: > > On Tue, 2014-07-22 at 02:34 +0200, Marek Vasut wrote: > > > Printing u32 with %02x is just a bad idea, fix it. > > > > Why is it "just a bad idea" if the values aren't expected to exceed > > 0xff? > > NAND_CMD_DEPLETE1 is 0x100 for example. I doubt anyone will use AND with this > controller, but I'd be much happier to see the print properly matching the > variable we're printing. It will match it. %02x doesn't restrict the output to two characters; it just makes sure there are at least two characters. -Scott
On Tuesday, July 29, 2014 at 02:12:47 AM, Scott Wood wrote: > On Tue, 2014-07-29 at 02:09 +0200, Marek Vasut wrote: > > On Tuesday, July 29, 2014 at 01:35:57 AM, Scott Wood wrote: > > > On Tue, 2014-07-22 at 02:34 +0200, Marek Vasut wrote: > > > > Printing u32 with %02x is just a bad idea, fix it. > > > > > > Why is it "just a bad idea" if the values aren't expected to exceed > > > 0xff? > > > > NAND_CMD_DEPLETE1 is 0x100 for example. I doubt anyone will use AND with > > this controller, but I'd be much happier to see the print properly > > matching the variable we're printing. > > It will match it. %02x doesn't restrict the output to two characters; > it just makes sure there are at least two characters. The output with %02x in this case is 0xffffffXY , so there is something really wrong going on. With %08x, the output is as expected (because that does match ths size of the variable). Best regards, Marek Vasut
On Tue, 2014-07-29 at 03:11 +0200, Marek Vasut wrote: > On Tuesday, July 29, 2014 at 02:12:47 AM, Scott Wood wrote: > > On Tue, 2014-07-29 at 02:09 +0200, Marek Vasut wrote: > > > On Tuesday, July 29, 2014 at 01:35:57 AM, Scott Wood wrote: > > > > On Tue, 2014-07-22 at 02:34 +0200, Marek Vasut wrote: > > > > > Printing u32 with %02x is just a bad idea, fix it. > > > > > > > > Why is it "just a bad idea" if the values aren't expected to exceed > > > > 0xff? > > > > > > NAND_CMD_DEPLETE1 is 0x100 for example. I doubt anyone will use AND with > > > this controller, but I'd be much happier to see the print properly > > > matching the variable we're printing. > > > > It will match it. %02x doesn't restrict the output to two characters; > > it just makes sure there are at least two characters. > > The output with %02x in this case is 0xffffffXY , so there is something really > wrong going on. With %08x, the output is as expected (because that does match > ths size of the variable). Is that for NAND_CMD_NONE which is -1? Or something else? Also regarding the changelog, neither of these values are declared as u32. -Scott
On Tuesday, July 29, 2014 at 03:22:29 AM, Scott Wood wrote: > On Tue, 2014-07-29 at 03:11 +0200, Marek Vasut wrote: > > On Tuesday, July 29, 2014 at 02:12:47 AM, Scott Wood wrote: > > > On Tue, 2014-07-29 at 02:09 +0200, Marek Vasut wrote: > > > > On Tuesday, July 29, 2014 at 01:35:57 AM, Scott Wood wrote: > > > > > On Tue, 2014-07-22 at 02:34 +0200, Marek Vasut wrote: > > > > > > Printing u32 with %02x is just a bad idea, fix it. > > > > > > > > > > Why is it "just a bad idea" if the values aren't expected to exceed > > > > > 0xff? > > > > > > > > NAND_CMD_DEPLETE1 is 0x100 for example. I doubt anyone will use AND > > > > with this controller, but I'd be much happier to see the print > > > > properly matching the variable we're printing. > > > > > > It will match it. %02x doesn't restrict the output to two characters; > > > it just makes sure there are at least two characters. > > > > The output with %02x in this case is 0xffffffXY , so there is something > > really wrong going on. With %08x, the output is as expected (because > > that does match ths size of the variable). > > Is that for NAND_CMD_NONE which is -1? Or something else? OK, now it makes sense. > Also regarding the changelog, neither of these values are declared as > u32. They're int and unsigned int, which on ARMv7 and lower (this is armv4) is 32bit with GCC. Best regards, Marek Vasut
diff --git a/drivers/mtd/nand/s3c2410_nand.c b/drivers/mtd/nand/s3c2410_nand.c index db87d07..b607cc5 100644 --- a/drivers/mtd/nand/s3c2410_nand.c +++ b/drivers/mtd/nand/s3c2410_nand.c @@ -43,7 +43,7 @@ static void s3c2410_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl) struct nand_chip *chip = mtd->priv; struct s3c2410_nand *nand = s3c2410_get_base_nand(); - debug("hwcontrol(): 0x%02x 0x%02x\n", cmd, ctrl); + debug("hwcontrol(): 0x%08x 0x%08x\n", cmd, ctrl); if (ctrl & NAND_CTRL_CHANGE) { ulong IO_ADDR_W = (ulong)nand;
Printing u32 with %02x is just a bad idea, fix it. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Kyungmin Park <kyungmin.park@samsung.com> Cc: Lukasz Majewski <l.majewski@samsung.com> Cc: Minkyu Kang <mk7.kang@samsung.com> Cc: Scott Wood <scottwood@freescale.com> Cc: Vladimir Zapolskiy <vz@mleia.com> --- drivers/mtd/nand/s3c2410_nand.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)