[U-Boot] [PATCH] fs/fat: debug-print file read position during file_fat_read_at()

In order to make the debug print in file_fat_read_at() a tad more useful, show the offset the file is being read at alongside the filename.
Suggested-by: Tero Kristo t-kristo@ti.com Signed-off-by: Andreas Dannenberg dannenberg@ti.com ---
Small addition but helpful nevertheless as none of the other debug prints embedded into fat.c seems to output this info. Without that addition it would just tell you the same file name a couple of times when reading from an DTB/ITB file for example. With the offset being shown it's easier to correlate/debug the loading process.
Oddly (and I haven't fully debugged this) but in order to really get _any_ of the debug() prints working in fat.c in my particulat setup, in addition to the usual '#define DEBUG' I also had to explicitly define _DEBUG to '1', something that should already be taken care of by log.c...
fs/fat/fat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 4efe8a3eda..4b722fc5ca 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -1095,7 +1095,7 @@ int file_fat_read_at(const char *filename, loff_t pos, void *buffer, if (ret) goto out_free_both;
- debug("reading %s\n", filename); + debug("reading %s at pos %llu\n", filename, pos); ret = get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread);
out_free_both:

On 08/14/2018 04:35 AM, Andreas Dannenberg wrote:
In order to make the debug print in file_fat_read_at() a tad more useful, show the offset the file is being read at alongside the filename.
Suggested-by: Tero Kristo t-kristo@ti.com Signed-off-by: Andreas Dannenberg dannenberg@ti.com
Small addition but helpful nevertheless as none of the other debug prints embedded into fat.c seems to output this info. Without that addition it would just tell you the same file name a couple of times when reading from an DTB/ITB file for example. With the offset being shown it's easier to correlate/debug the loading process.
Oddly (and I haven't fully debugged this) but in order to really get _any_ of the debug() prints working in fat.c in my particulat setup, in addition to the usual '#define DEBUG' I also had to explicitly define _DEBUG to '1', something that should already be taken care of by log.c...
fs/fat/fat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 4efe8a3eda..4b722fc5ca 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -1095,7 +1095,7 @@ int file_fat_read_at(const char *filename, loff_t pos, void *buffer, if (ret) goto out_free_both;
- debug("reading %s\n", filename);
- debug("reading %s at pos %llu\n", filename, pos); ret = get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread);
out_free_both:
This seems to duplicate [PATCH 1/1] fat: provide position in debug message https://lists.denx.de/pipermail/u-boot/2018-August/337850.html
The only difference is that you print a decimal number instead of a hex number.
Using %llu is consistent with fs/fat/fat.c:331: debug("Read position past EOF: %llu\n", pos);
So I will withdraw my patch.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de

Heinrich,
On Tue, Aug 14, 2018 at 07:40:14AM +0200, Heinrich Schuchardt wrote:
On 08/14/2018 04:35 AM, Andreas Dannenberg wrote:
In order to make the debug print in file_fat_read_at() a tad more useful, show the offset the file is being read at alongside the filename.
Suggested-by: Tero Kristo t-kristo@ti.com Signed-off-by: Andreas Dannenberg dannenberg@ti.com
Small addition but helpful nevertheless as none of the other debug prints embedded into fat.c seems to output this info. Without that addition it would just tell you the same file name a couple of times when reading from an DTB/ITB file for example. With the offset being shown it's easier to correlate/debug the loading process.
Oddly (and I haven't fully debugged this) but in order to really get _any_ of the debug() prints working in fat.c in my particulat setup, in addition to the usual '#define DEBUG' I also had to explicitly define _DEBUG to '1', something that should already be taken care of by log.c...
fs/fat/fat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 4efe8a3eda..4b722fc5ca 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -1095,7 +1095,7 @@ int file_fat_read_at(const char *filename, loff_t pos, void *buffer, if (ret) goto out_free_both;
- debug("reading %s\n", filename);
- debug("reading %s at pos %llu\n", filename, pos); ret = get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread);
out_free_both:
This seems to duplicate [PATCH 1/1] fat: provide position in debug message https://lists.denx.de/pipermail/u-boot/2018-August/337850.html
Interesting, I had specifically searched for this to be potentially there already, but I searched by prefix "fs/fat" which you used in your earlier patch, however the prefix had changed in your later patch :)
The only difference is that you print a decimal number instead of a hex number.
Using %llu is consistent with fs/fat/fat.c:331: debug("Read position past EOF: %llu\n", pos);
Yes that's exactly why I used decimal. Also when using hex IMHO it would have been better to prefix with 0x to minimize confusion, something that got me into trouble in my early U-Boot days (it's often not clear what is hex and what is dec, but things have since improved on that front).
-- Andreas Dannenberg Texas Instruments Inc
So I will withdraw my patch.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de

On Mon, Aug 13, 2018 at 09:35:15PM -0500, Andreas Dannenberg wrote:
In order to make the debug print in file_fat_read_at() a tad more useful, show the offset the file is being read at alongside the filename.
Suggested-by: Tero Kristo t-kristo@ti.com Signed-off-by: Andreas Dannenberg dannenberg@ti.com Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
Applied to u-boot/master, thanks!
participants (3)
-
Andreas Dannenberg
-
Heinrich Schuchardt
-
Tom Rini