* [PATCH 1/9] fs: move struct fstype_info definition to top of file
2026-05-18 5:57 [PATCH 0/9] fs: add change date to ls output Heinrich Schuchardt
@ 2026-05-18 5:57 ` Heinrich Schuchardt
2026-05-19 18:55 ` Simon Glass
2026-05-18 5:57 ` [PATCH 2/9] fs: print change date in directory listing for FAT Heinrich Schuchardt
` (8 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2026-05-18 5:57 UTC (permalink / raw)
To: Tom Rini
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs,
Heinrich Schuchardt
Structure definitions should precede code using them.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
fs/fs.c | 86 ++++++++++++++++++++++++++++-----------------------------
1 file changed, 43 insertions(+), 43 deletions(-)
diff --git a/fs/fs.c b/fs/fs.c
index 8ea50a6c13c..fe62b71c83c 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -38,6 +38,49 @@ static int fs_dev_part;
static struct disk_partition fs_partition;
static int fs_type = FS_TYPE_ANY;
+struct fstype_info {
+ int fstype;
+ char *name;
+ /*
+ * Is it legal to pass NULL as .probe()'s fs_dev_desc parameter? This
+ * should be false in most cases. For "virtual" filesystems which
+ * aren't based on a U-Boot block device (e.g. sandbox), this can be
+ * set to true. This should also be true for the dummy entry at the end
+ * of fstypes[], since that is essentially a "virtual" (non-existent)
+ * filesystem.
+ */
+ bool null_dev_desc_ok;
+ int (*probe)(struct blk_desc *fs_dev_desc,
+ struct disk_partition *fs_partition);
+ int (*ls)(const char *dirname);
+ int (*exists)(const char *filename);
+ int (*size)(const char *filename, loff_t *size);
+ int (*read)(const char *filename, void *buf, loff_t offset,
+ loff_t len, loff_t *actread);
+ int (*write)(const char *filename, void *buf, loff_t offset,
+ loff_t len, loff_t *actwrite);
+ void (*close)(void);
+ int (*uuid)(char *uuid_str);
+ /*
+ * Open a directory stream. On success return 0 and directory
+ * stream pointer via 'dirsp'. On error, return -errno. See
+ * fs_opendir().
+ */
+ int (*opendir)(const char *filename, struct fs_dir_stream **dirsp);
+ /*
+ * Read next entry from directory stream. On success return 0
+ * and directory entry pointer via 'dentp'. On error return
+ * -errno. See fs_readdir().
+ */
+ int (*readdir)(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
+ /* see fs_closedir() */
+ void (*closedir)(struct fs_dir_stream *dirs);
+ int (*unlink)(const char *filename);
+ int (*mkdir)(const char *dirname);
+ int (*ln)(const char *filename, const char *target);
+ int (*rename)(const char *old_path, const char *new_path);
+};
+
void fs_set_type(int type)
{
fs_type = type;
@@ -147,49 +190,6 @@ static inline int fs_rename_unsupported(const char *old_path,
return -1;
}
-struct fstype_info {
- int fstype;
- char *name;
- /*
- * Is it legal to pass NULL as .probe()'s fs_dev_desc parameter? This
- * should be false in most cases. For "virtual" filesystems which
- * aren't based on a U-Boot block device (e.g. sandbox), this can be
- * set to true. This should also be true for the dummy entry at the end
- * of fstypes[], since that is essentially a "virtual" (non-existent)
- * filesystem.
- */
- bool null_dev_desc_ok;
- int (*probe)(struct blk_desc *fs_dev_desc,
- struct disk_partition *fs_partition);
- int (*ls)(const char *dirname);
- int (*exists)(const char *filename);
- int (*size)(const char *filename, loff_t *size);
- int (*read)(const char *filename, void *buf, loff_t offset,
- loff_t len, loff_t *actread);
- int (*write)(const char *filename, void *buf, loff_t offset,
- loff_t len, loff_t *actwrite);
- void (*close)(void);
- int (*uuid)(char *uuid_str);
- /*
- * Open a directory stream. On success return 0 and directory
- * stream pointer via 'dirsp'. On error, return -errno. See
- * fs_opendir().
- */
- int (*opendir)(const char *filename, struct fs_dir_stream **dirsp);
- /*
- * Read next entry from directory stream. On success return 0
- * and directory entry pointer via 'dentp'. On error return
- * -errno. See fs_readdir().
- */
- int (*readdir)(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
- /* see fs_closedir() */
- void (*closedir)(struct fs_dir_stream *dirs);
- int (*unlink)(const char *filename);
- int (*mkdir)(const char *dirname);
- int (*ln)(const char *filename, const char *target);
- int (*rename)(const char *old_path, const char *new_path);
-};
-
static struct fstype_info fstypes[] = {
#if CONFIG_IS_ENABLED(FS_FAT)
{
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 1/9] fs: move struct fstype_info definition to top of file
2026-05-18 5:57 ` [PATCH 1/9] fs: move struct fstype_info definition to top of file Heinrich Schuchardt
@ 2026-05-19 18:55 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2026-05-19 18:55 UTC (permalink / raw)
To: heinrich.schuchardt
Cc: Tom Rini, Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
On 2026-05-18T05:57:19, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
> fs: move struct fstype_info definition to top of file
>
> Structure definitions should precede code using them.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>
> fs/fs.c | 86 ++++++++++++++++++++++++++++++++---------------------------------
> 1 file changed, 43 insertions(+), 43 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/9] fs: print change date in directory listing for FAT
2026-05-18 5:57 [PATCH 0/9] fs: add change date to ls output Heinrich Schuchardt
2026-05-18 5:57 ` [PATCH 1/9] fs: move struct fstype_info definition to top of file Heinrich Schuchardt
@ 2026-05-18 5:57 ` Heinrich Schuchardt
2026-05-19 19:59 ` Simon Glass
2026-05-18 5:57 ` [PATCH 3/9] fs: ext4: print change date in directory listing Heinrich Schuchardt
` (7 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2026-05-18 5:57 UTC (permalink / raw)
To: Tom Rini
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs,
Heinrich Schuchardt
fs_ls_generic() displays file sizes but no timestamps. The FAT
filesystem stores a change date in every directory entry and already
populates fs_dirent::change_time in fat_readdir(). Print the date
alongside the file size for filesystems that provide it.
Add a u32 capability bitmap (caps) to struct fstype_info. Each bit
documents a property that the filesystem's readdir() implementation
guarantees:
FS_CAP_DATE BIT(0) change_time in fs_dirent is valid
fs_ls_generic() tests FS_CAP_DATE once before the loop to select a
consistent output format for the entire listing:
12345678 2024-03-15 09:30 filename.txt (FAT)
12345678 filename.txt (ext4, squashfs, ...)
Set FS_CAP_DATE for FAT. fat2rtc() loses the __maybe_unused annotation
since it is now called unconditionally outside XPL builds. The attr,
create_time, change_time, and access_time fields that were previously
only populated under CONFIG_EFI_LOADER are now populated whenever
CONFIG_XPL_BUILD is not set.
Extending the feature to another filesystem requires only adding
.caps = FS_CAP_DATE to its fstype_info entry and ensuring its readdir()
fills in fs_dirent::change_time.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
fs/fat/fat.c | 2 +-
fs/fs.c | 38 +++++++++++++++++++++++++++++++++++---
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index c1ccf30771a..7443f5952af 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -1539,7 +1539,7 @@ int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp)
memset(dent, 0, sizeof(*dent));
strcpy(dent->name, dir->itr.name);
- if (CONFIG_IS_ENABLED(EFI_LOADER)) {
+ if (!IS_ENABLED(CONFIG_XPL_BUILD)) {
dent->attr = dir->itr.dent->attr;
fat2rtc(le16_to_cpu(dir->itr.dent->cdate),
le16_to_cpu(dir->itr.dent->ctime), &dent->create_time);
diff --git a/fs/fs.c b/fs/fs.c
index fe62b71c83c..f8e4794c10e 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -38,6 +38,11 @@ static int fs_dev_part;
static struct disk_partition fs_partition;
static int fs_type = FS_TYPE_ANY;
+/*
+ * define FS_CAP_DATE - readdir() populates fs_dirent::change_time
+ */
+#define FS_CAP_DATE BIT(0)
+
struct fstype_info {
int fstype;
char *name;
@@ -50,6 +55,10 @@ struct fstype_info {
* filesystem.
*/
bool null_dev_desc_ok;
+#if !IS_ENABLED(CONFIG_XPL_BUILD)
+ /* Capability flags (FS_CAP_*) */
+ u32 caps;
+#endif
int (*probe)(struct blk_desc *fs_dev_desc,
struct disk_partition *fs_partition);
int (*ls)(const char *dirname);
@@ -98,10 +107,19 @@ static inline int fs_ls_unsupported(const char *dirname)
return -1;
}
+/* Forward declaration - defined after fstypes[] */
+static struct fstype_info *fs_get_info(int fstype);
+
/* generic implementation of ls in terms of opendir/readdir/closedir */
__maybe_unused
static int fs_ls_generic(const char *dirname)
{
+#if !IS_ENABLED(CONFIG_XPL_BUILD)
+ struct fstype_info *info = fs_get_info(fs_type);
+ bool has_date = !!(info->caps & FS_CAP_DATE);
+#else
+ bool has_date = false;
+#endif
struct fs_dir_stream *dirs;
struct fs_dirent *dent;
int nfiles = 0, ndirs = 0;
@@ -112,15 +130,26 @@ static int fs_ls_generic(const char *dirname)
while ((dent = fs_readdir(dirs))) {
if (dent->type == FS_DT_DIR) {
- printf(" %s/\n", dent->name);
+ printf(" ");
ndirs++;
} else if (dent->type == FS_DT_LNK) {
- printf(" <SYM> %s\n", dent->name);
+ printf(" <SYM> ");
nfiles++;
} else {
- printf(" %8lld %s\n", dent->size, dent->name);
+ printf(" %8lld ", dent->size);
nfiles++;
}
+ if (has_date)
+ printf("%04d-%02d-%02d %02d:%02d ",
+ dent->change_time.tm_year,
+ dent->change_time.tm_mon,
+ dent->change_time.tm_mday,
+ dent->change_time.tm_hour,
+ dent->change_time.tm_min);
+ if (dent->type == FS_DT_DIR)
+ printf("%s/\n", dent->name);
+ else
+ printf("%s\n", dent->name);
}
fs_closedir(dirs);
@@ -196,6 +225,9 @@ static struct fstype_info fstypes[] = {
.fstype = FS_TYPE_FAT,
.name = "fat",
.null_dev_desc_ok = false,
+#if !IS_ENABLED(CONFIG_XPL_BUILD)
+ .caps = FS_CAP_DATE,
+#endif
.probe = fat_set_blk_dev,
.close = fat_close,
.ls = fs_ls_generic,
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 2/9] fs: print change date in directory listing for FAT
2026-05-18 5:57 ` [PATCH 2/9] fs: print change date in directory listing for FAT Heinrich Schuchardt
@ 2026-05-19 19:59 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2026-05-19 19:59 UTC (permalink / raw)
To: heinrich.schuchardt
Cc: Tom Rini, Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
Hi Heinrich,
On 2026-05-18T05:57:19, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
> fs: print change date in directory listing for FAT
>
> fs_ls_generic() displays file sizes but no timestamps. The FAT
> filesystem stores a change date in every directory entry and already
> populates fs_dirent::change_time in fat_readdir(). Print the date
> alongside the file size for filesystems that provide it.
>
> Add a u32 capability bitmap (caps) to struct fstype_info. Each bit
> documents a property that the filesystem's readdir() implementation
> guarantees:
>
> FS_CAP_DATE BIT(0) change_time in fs_dirent is valid
>
> fs_ls_generic() tests FS_CAP_DATE once before the loop to select a
> consistent output format for the entire listing:
>
> 12345678 2024-03-15 09:30 filename.txt (FAT)
> 12345678 filename.txt (ext4, squashfs, ...)
>
> Set FS_CAP_DATE for FAT. fat2rtc() loses the __maybe_unused annotation
> [...]
>
> fs/fat/fat.c | 2 +-
> fs/fs.c | 38 +++++++++++++++++++++++++++++++++++---
> 2 files changed, 36 insertions(+), 4 deletions(-)
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> @@ -1539,7 +1539,7 @@ int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp)
>
> memset(dent, 0, sizeof(*dent));
> strcpy(dent->name, dir->itr.name);
> - if (CONFIG_IS_ENABLED(EFI_LOADER)) {
> + if (!IS_ENABLED(CONFIG_XPL_BUILD)) {
> dent->attr = dir->itr.dent->attr;
The commit message says fat2rtc() loses __maybe_unused, but the diff
does not touch fat2rtc()
> diff --git a/fs/fs.c b/fs/fs.c
> @@ -38,6 +38,11 @@ static int fs_dev_part;
> static struct disk_partition fs_partition;
> static int fs_type = FS_TYPE_ANY;
>
> +/*
> + * define FS_CAP_DATE - readdir() populates fs_dirent::change_time
> + */
> +#define FS_CAP_DATE BIT(0)
The leading 'define' reads like a half-finished kernel-doc. Please
make this a plain comment:
/* FS_CAP_DATE: readdir() populates fs_dirent::change_time */
> diff --git a/fs/fs.c b/fs/fs.c
> @@ -98,10 +107,19 @@ static inline int fs_ls_unsupported(const char *dirname)
> return -1;
> }
>
> +/* Forward declaration - defined after fstypes[] */
> +static struct fstype_info *fs_get_info(int fstype);
> +
Patch 1 moved struct fstype_info to the top precisely to avoid this.
Please move fs_get_info() up next to the struct (or just before
fs_ls_generic()) and drop the forward decl.
> diff --git a/fs/fs.c b/fs/fs.c
> @@ -112,15 +130,26 @@ static int fs_ls_generic(const char *dirname)
> while ((dent = fs_readdir(dirs))) {
> if (dent->type == FS_DT_DIR) {
> - printf(" %s/\n", dent->name);
> + printf(" ");
> ndirs++;
> } else if (dent->type == FS_DT_LNK) {
> - printf(" <SYM> %s\n", dent->name);
> + printf(" <SYM> ");
> nfiles++;
> } else {
> - printf(" %8lld %s\n", dent->size, dent->name);
> + printf(" %8lld ", dent->size);
> nfiles++;
> }
This silently narrows the gap between size and name from three spaces
to one for every filesystem, not only the ones opting into FS_CAP_DATE
That is why patch 9 has to fix up the erofs test, but ubifs, sandbox
etc. get the same churn for no functional gain. How about keeping the
original layout when has_date is false?
> diff --git a/fs/fs.c b/fs/fs.c
> @@ -112,15 +130,26 @@ static int fs_ls_generic(const char *dirname)
> + if (has_date)
> + printf("%04d-%02d-%02d %02d:%02d ",
> + dent->change_time.tm_year,
> + dent->change_time.tm_mon,
> + dent->change_time.tm_mday,
> + dent->change_time.tm_hour,
> + dent->change_time.tm_min);
If the date was never set, this presumably prints '0000-00-00 00:00
filename', which is more misleading than no date at all. It is is
unset it would be better to show nothing.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/9] fs: ext4: print change date in directory listing
2026-05-18 5:57 [PATCH 0/9] fs: add change date to ls output Heinrich Schuchardt
2026-05-18 5:57 ` [PATCH 1/9] fs: move struct fstype_info definition to top of file Heinrich Schuchardt
2026-05-18 5:57 ` [PATCH 2/9] fs: print change date in directory listing for FAT Heinrich Schuchardt
@ 2026-05-18 5:57 ` Heinrich Schuchardt
2026-05-20 20:42 ` Simon Glass
2026-05-18 5:57 ` [PATCH 4/9] fs: ext4: don't read time fields in XPL Heinrich Schuchardt
` (6 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2026-05-18 5:57 UTC (permalink / raw)
To: Tom Rini
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs,
Heinrich Schuchardt
Declare FS_CAP_DATE in the ext4 fstype_info entry so that fs_ls_generic()
displays the modification date alongside the file size:
4096 2024-03-15 09:30 filename.txt
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
fs/fs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/fs.c b/fs/fs.c
index f8e4794c10e..482a5523712 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -261,6 +261,9 @@ static struct fstype_info fstypes[] = {
.fstype = FS_TYPE_EXT,
.name = "ext4",
.null_dev_desc_ok = false,
+#if !IS_ENABLED(CONFIG_XPL_BUILD)
+ .caps = FS_CAP_DATE,
+#endif
.probe = ext4fs_probe,
.close = ext4fs_close,
.ls = fs_ls_generic,
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 3/9] fs: ext4: print change date in directory listing
2026-05-18 5:57 ` [PATCH 3/9] fs: ext4: print change date in directory listing Heinrich Schuchardt
@ 2026-05-20 20:42 ` Simon Glass
2026-05-21 0:46 ` Heinrich Schuchardt
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2026-05-20 20:42 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
Hi Heinrich,
On Mon, 18 May 2026 at 00:57, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> Declare FS_CAP_DATE in the ext4 fstype_info entry so that fs_ls_generic()
> displays the modification date alongside the file size:
>
> 4096 2024-03-15 09:30 filename.txt
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> fs/fs.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/fs.c b/fs/fs.c
> index f8e4794c10e..482a5523712 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -261,6 +261,9 @@ static struct fstype_info fstypes[] = {
> .fstype = FS_TYPE_EXT,
> .name = "ext4",
> .null_dev_desc_ok = false,
> +#if !IS_ENABLED(CONFIG_XPL_BUILD)
> + .caps = FS_CAP_DATE,
> +#endif
> .probe = ext4fs_probe,
> .close = ext4fs_close,
> .ls = fs_ls_generic,
> --
> 2.53.0
>
I would prefer having a head-file macro which expands to nothing for
xPL builds, rather than adding preprocessor macros.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/9] fs: ext4: print change date in directory listing
2026-05-20 20:42 ` Simon Glass
@ 2026-05-21 0:46 ` Heinrich Schuchardt
2026-05-21 15:38 ` Simon Glass
2026-05-29 20:01 ` Tom Rini
0 siblings, 2 replies; 25+ messages in thread
From: Heinrich Schuchardt @ 2026-05-21 0:46 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
On 5/20/26 22:42, Simon Glass wrote:
> Hi Heinrich,
>
> On Mon, 18 May 2026 at 00:57, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> Declare FS_CAP_DATE in the ext4 fstype_info entry so that fs_ls_generic()
>> displays the modification date alongside the file size:
>>
>> 4096 2024-03-15 09:30 filename.txt
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>> fs/fs.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/fs.c b/fs/fs.c
>> index f8e4794c10e..482a5523712 100644
>> --- a/fs/fs.c
>> +++ b/fs/fs.c
>> @@ -261,6 +261,9 @@ static struct fstype_info fstypes[] = {
>> .fstype = FS_TYPE_EXT,
>> .name = "ext4",
>> .null_dev_desc_ok = false,
>> +#if !IS_ENABLED(CONFIG_XPL_BUILD)
>> + .caps = FS_CAP_DATE,
>> +#endif
>> .probe = ext4fs_probe,
>> .close = ext4fs_close,
>> .ls = fs_ls_generic,
>> --
>> 2.53.0
>>
>
> I would prefer having a head-file macro which expands to nothing for
> xPL builds, rather than adding preprocessor macros.
>
> Regards,
> Simon
Hello Simon,
In the internet I could not find what a "head-file macro" might be.
As struct fstype_info is not defined in a header file, a preprocessor
macro defined in a header file would not make sense here.
Do you mean something like:
#if IS_ENABLED(CONFIG_XPL_BUILD)
#define FS_CAPS(flags) /* empty */
#else
#define FS_CAPS(flags) .caps = (flags),
#endif
static struct fstype_info fstypes[] = {
#if CONFIG_IS_ENABLED(FS_FAT)
{
.fstype = FS_TYPE_FAT,
.name = "fat",
.null_dev_desc_ok = false,
FS_CAPS(FS_CAP_DATE)
.probe = fat_set_blk_dev,
...
A line without a comma in the initializer is easily mistaken as
incorrect. I am not sure that a code reviewers life is made easier with
defining a new preprocessor macro.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/9] fs: ext4: print change date in directory listing
2026-05-21 0:46 ` Heinrich Schuchardt
@ 2026-05-21 15:38 ` Simon Glass
2026-05-29 20:01 ` Tom Rini
1 sibling, 0 replies; 25+ messages in thread
From: Simon Glass @ 2026-05-21 15:38 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, U-Boot Mailing List,
linux-erofs
Hi Heinrich,
On Wed, 20 May 2026, 19:46 Heinrich Schuchardt,
<heinrich.schuchardt@canonical.com> wrote:
>
> On 5/20/26 22:42, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Mon, 18 May 2026 at 00:57, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> Declare FS_CAP_DATE in the ext4 fstype_info entry so that fs_ls_generic()
> >> displays the modification date alongside the file size:
> >>
> >> 4096 2024-03-15 09:30 filename.txt
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >> ---
> >> fs/fs.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/fs/fs.c b/fs/fs.c
> >> index f8e4794c10e..482a5523712 100644
> >> --- a/fs/fs.c
> >> +++ b/fs/fs.c
> >> @@ -261,6 +261,9 @@ static struct fstype_info fstypes[] = {
> >> .fstype = FS_TYPE_EXT,
> >> .name = "ext4",
> >> .null_dev_desc_ok = false,
> >> +#if !IS_ENABLED(CONFIG_XPL_BUILD)
> >> + .caps = FS_CAP_DATE,
> >> +#endif
> >> .probe = ext4fs_probe,
> >> .close = ext4fs_close,
> >> .ls = fs_ls_generic,
> >> --
> >> 2.53.0
> >>
> >
> > I would prefer having a head-file macro which expands to nothing for
> > xPL builds, rather than adding preprocessor macros.
> >
> > Regards,
> > Simon
>
> Hello Simon,
>
> In the internet I could not find what a "head-file macro" might be.
Sorry I meant 'header-file macro'
>
> As struct fstype_info is not defined in a header file, a preprocessor
> macro defined in a header file would not make sense here.
>
> Do you mean something like:
>
> #if IS_ENABLED(CONFIG_XPL_BUILD)
> #define FS_CAPS(flags) /* empty */
> #else
> #define FS_CAPS(flags) .caps = (flags),
> #endif
>
> static struct fstype_info fstypes[] = {
> #if CONFIG_IS_ENABLED(FS_FAT)
> {
> .fstype = FS_TYPE_FAT,
> .name = "fat",
> .null_dev_desc_ok = false,
> FS_CAPS(FS_CAP_DATE)
> .probe = fat_set_blk_dev,
> ...
>
> A line without a comma in the initializer is easily mistaken as
> incorrect. I am not sure that a code reviewers life is made easier with
> defining a new preprocessor macro.
Firstly I wonder if it is actually worth making the field conditional?
We have a null_dev_desc_ok bool which could be turned into flags, if
you are trying to save space.
Ideally we would have a new Kconfig for this, so it is possible to
enable the feature in particular xPL builds.
If you don't like the example above, another option is:
CONFIG_IS_ENABLED(YOUR_OPTION, (FS_CAP_DATE,))
which we use in quite a few places now.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/9] fs: ext4: print change date in directory listing
2026-05-21 0:46 ` Heinrich Schuchardt
2026-05-21 15:38 ` Simon Glass
@ 2026-05-29 20:01 ` Tom Rini
1 sibling, 0 replies; 25+ messages in thread
From: Tom Rini @ 2026-05-29 20:01 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
[-- Attachment #1: Type: text/plain, Size: 2442 bytes --]
On Thu, May 21, 2026 at 02:46:51AM +0200, Heinrich Schuchardt wrote:
> On 5/20/26 22:42, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Mon, 18 May 2026 at 00:57, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> > >
> > > Declare FS_CAP_DATE in the ext4 fstype_info entry so that fs_ls_generic()
> > > displays the modification date alongside the file size:
> > >
> > > 4096 2024-03-15 09:30 filename.txt
> > >
> > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > ---
> > > fs/fs.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/fs/fs.c b/fs/fs.c
> > > index f8e4794c10e..482a5523712 100644
> > > --- a/fs/fs.c
> > > +++ b/fs/fs.c
> > > @@ -261,6 +261,9 @@ static struct fstype_info fstypes[] = {
> > > .fstype = FS_TYPE_EXT,
> > > .name = "ext4",
> > > .null_dev_desc_ok = false,
> > > +#if !IS_ENABLED(CONFIG_XPL_BUILD)
> > > + .caps = FS_CAP_DATE,
> > > +#endif
> > > .probe = ext4fs_probe,
> > > .close = ext4fs_close,
> > > .ls = fs_ls_generic,
> > > --
> > > 2.53.0
> > >
> >
> > I would prefer having a head-file macro which expands to nothing for
> > xPL builds, rather than adding preprocessor macros.
> >
> > Regards,
> > Simon
>
> Hello Simon,
>
> In the internet I could not find what a "head-file macro" might be.
>
> As struct fstype_info is not defined in a header file, a preprocessor macro
> defined in a header file would not make sense here.
>
> Do you mean something like:
>
> #if IS_ENABLED(CONFIG_XPL_BUILD)
> #define FS_CAPS(flags) /* empty */
> #else
> #define FS_CAPS(flags) .caps = (flags),
> #endif
>
> static struct fstype_info fstypes[] = {
> #if CONFIG_IS_ENABLED(FS_FAT)
> {
> .fstype = FS_TYPE_FAT,
> .name = "fat",
> .null_dev_desc_ok = false,
> FS_CAPS(FS_CAP_DATE)
> .probe = fat_set_blk_dev,
> ...
>
> A line without a comma in the initializer is easily mistaken as incorrect. I
> am not sure that a code reviewers life is made easier with defining a new
> preprocessor macro.
We have a lot of other examples like this in-tree already such as
ENV_NAME(..) so I think it's reasonable to make an FS_CAPS macro like
this.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/9] fs: ext4: don't read time fields in XPL
2026-05-18 5:57 [PATCH 0/9] fs: add change date to ls output Heinrich Schuchardt
` (2 preceding siblings ...)
2026-05-18 5:57 ` [PATCH 3/9] fs: ext4: print change date in directory listing Heinrich Schuchardt
@ 2026-05-18 5:57 ` Heinrich Schuchardt
2026-05-19 20:03 ` Simon Glass
2026-05-18 5:57 ` [PATCH 5/9] fs: ext4: set inode timestamps on write Heinrich Schuchardt
` (5 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2026-05-18 5:57 UTC (permalink / raw)
To: Tom Rini
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs,
Heinrich Schuchardt
The ext4 readdir implementation populates dent time fields in XML
builds though that information is never used.
Guard the three rtc_to_tm() calls with !IS_ENABLED(CONFIG_XPL_BUILD),
consistent with the FAT driver.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
fs/ext4/ext4fs.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c
index 3c79a889bc2..abf4a9835bc 100644
--- a/fs/ext4/ext4fs.c
+++ b/fs/ext4/ext4fs.c
@@ -319,9 +319,11 @@ int ext4fs_readdir(struct fs_dir_stream *fs_dirs, struct fs_dirent **dentp)
dent->type = FILETYPE_UNKNOWN;
}
- rtc_to_tm(fdiro.inode.atime, &dent->access_time);
- rtc_to_tm(fdiro.inode.ctime, &dent->create_time);
- rtc_to_tm(fdiro.inode.mtime, &dent->change_time);
+ if (!IS_ENABLED(CONFIG_XPL_BUILD)) {
+ rtc_to_tm(le32_to_cpu(fdiro.inode.atime), &dent->access_time);
+ rtc_to_tm(le32_to_cpu(fdiro.inode.ctime), &dent->create_time);
+ rtc_to_tm(le32_to_cpu(fdiro.inode.mtime), &dent->change_time);
+ }
dirs->fpos += le16_to_cpu(dirent.direntlen);
dent->size = fdiro.inode.size;
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 4/9] fs: ext4: don't read time fields in XPL
2026-05-18 5:57 ` [PATCH 4/9] fs: ext4: don't read time fields in XPL Heinrich Schuchardt
@ 2026-05-19 20:03 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2026-05-19 20:03 UTC (permalink / raw)
To: heinrich.schuchardt
Cc: Tom Rini, Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
Hi Heinrich,
On 2026-05-18T05:57:19, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
> fs: ext4: don't read time fields in XPL
>
> The ext4 readdir implementation populates dent time fields in XML
> builds though that information is never used.
>
> Guard the three rtc_to_tm() calls with !IS_ENABLED(CONFIG_XPL_BUILD),
> consistent with the FAT driver.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>
> fs/ext4/ext4fs.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
> diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c
> @@ -319,9 +319,11 @@ int ext4fs_readdir(struct fs_dir_stream *fs_dirs, struct fs_dirent **dentp)
> - rtc_to_tm(fdiro.inode.atime, &dent->access_time);
> - rtc_to_tm(fdiro.inode.ctime, &dent->create_time);
> - rtc_to_tm(fdiro.inode.mtime, &dent->change_time);
> + if (!IS_ENABLED(CONFIG_XPL_BUILD)) {
> + rtc_to_tm(le32_to_cpu(fdiro.inode.atime), &dent->access_time);
> + rtc_to_tm(le32_to_cpu(fdiro.inode.ctime), &dent->create_time);
> + rtc_to_tm(le32_to_cpu(fdiro.inode.mtime), &dent->change_time);
> + }
The le32_to_cpu() addition is a separate fix from the XPL guard,
right? Please mention the endianness fix in the commit message, or
better, split it into its own patch that can be backported
independently.
BTW dent->size on the next line has the same problem, doesn't it?
> fs: ext4: don't read time fields in XPL
>
> The ext4 readdir implementation populates dent time fields in XML
> builds though that information is never used.
Typo: XML should be xPL
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 5/9] fs: ext4: set inode timestamps on write
2026-05-18 5:57 [PATCH 0/9] fs: add change date to ls output Heinrich Schuchardt
` (3 preceding siblings ...)
2026-05-18 5:57 ` [PATCH 4/9] fs: ext4: don't read time fields in XPL Heinrich Schuchardt
@ 2026-05-18 5:57 ` Heinrich Schuchardt
2026-05-19 20:00 ` Simon Glass
2026-05-18 5:57 ` [PATCH 6/9] test: Probe RTC early in dm_test_host() Heinrich Schuchardt
` (4 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2026-05-18 5:57 UTC (permalink / raw)
To: Tom Rini
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs,
Heinrich Schuchardt
Replace the hardcoded zero timestamp in ext4fs_write() with the actual
current time obtained from the RTC when CONFIG_DM_RTC is enabled.
Per the ext2/3/4 specification (ext2 design document, section 4.2):
i_mtime last data modification time
i_ctime last inode change time (the 'c' stands for 'change', not
'create'; this is not a creation timestamp)
i_atime last access time
All three fields are set to the same value since writing data modifies
both the file content (mtime) and the inode metadata (ctime), and any
write also constitutes an access (atime). If no RTC is available, the
timestamp falls back to 2000-01-01 00:00:00 UTC, matching the FAT
write driver behaviour.
The ext4 i_crtime field (true file creation time, added in ext4) is
stored in the extra inode area beyond the base 128-byte inode and is
not modelled by struct ext2_inode, so it cannot be set here.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
fs/ext4/ext4_write.c | 41 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 39 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index 1abedcede72..c8a1ca63605 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -21,14 +21,47 @@
*/
#include <blk.h>
+#include <dm/uclass.h>
#include <log.h>
#include <malloc.h>
#include <memalign.h>
#include <part.h>
+#include <rtc.h>
#include <linux/stat.h>
#include <div64.h>
#include "ext4_common.h"
+/**
+ * define EXT4_TIMESTAMP_Y2K - 2000-01-01 00:00:00 UTC as a POSIX timestamp
+ *
+ * Used as a fallback timestamp when no RTC is available.
+ */
+#define EXT4_TIMESTAMP_Y2K 946684800
+
+/**
+ * ext4_current_timestamp() - get current time as a POSIX timestamp
+ *
+ * Returns the current time as seconds since the Unix epoch.
+ * Falls back to 2000-01-01 00:00:00 UTC if the RTC is unavailable,
+ * matching the behaviour of the FAT write driver.
+ */
+static time_t ext4_current_timestamp(void)
+{
+ if (CONFIG_IS_ENABLED(DM_RTC)) {
+ struct udevice *dev;
+ struct rtc_time tm;
+
+ uclass_first_device(UCLASS_RTC, &dev);
+ if (!dev)
+ goto fallback;
+ if (dm_rtc_get(dev, &tm))
+ goto fallback;
+ return rtc_mktime(&tm);
+ }
+fallback:
+ return EXT4_TIMESTAMP_Y2K;
+}
+
static inline void ext4fs_sb_free_inodes_inc(struct ext2_sblock *sb)
{
sb->free_inodes = cpu_to_le32(le32_to_cpu(sb->free_inodes) + 1);
@@ -854,7 +887,7 @@ int ext4fs_write(const char *fname, const char *buffer,
unsigned char *inode_buffer = NULL;
int parent_inodeno;
int inodeno;
- time_t timestamp = 0;
+ time_t timestamp = ext4_current_timestamp();
uint64_t bytes_reqd_for_file;
unsigned int blks_reqd_for_file;
@@ -979,7 +1012,11 @@ int ext4fs_write(const char *fname, const char *buffer,
}
if (existing_file_inode)
free(existing_file_inode);
- /* ToDo: Update correct time */
+ /*
+ * Note: ext4 i_crtime (true creation time) lives in the extended
+ * inode area beyond the base 128-byte inode and is not modelled
+ * by struct ext2_inode, so it cannot be set here.
+ */
file_inode->mtime = cpu_to_le32(timestamp);
file_inode->atime = cpu_to_le32(timestamp);
file_inode->ctime = cpu_to_le32(timestamp);
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 5/9] fs: ext4: set inode timestamps on write
2026-05-18 5:57 ` [PATCH 5/9] fs: ext4: set inode timestamps on write Heinrich Schuchardt
@ 2026-05-19 20:00 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2026-05-19 20:00 UTC (permalink / raw)
To: heinrich.schuchardt
Cc: Tom Rini, Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
Hi Heinrich,
On 2026-05-18T05:57:19, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
> fs: ext4: set inode timestamps on write
>
> Replace the hardcoded zero timestamp in ext4fs_write() with the actual
> current time obtained from the RTC when CONFIG_DM_RTC is enabled.
>
> Per the ext2/3/4 specification (ext2 design document, section 4.2):
> i_mtime last data modification time
> i_ctime last inode change time (the 'c' stands for 'change', not
> 'create'; this is not a creation timestamp)
> i_atime last access time
>
> All three fields are set to the same value since writing data modifies
> both the file content (mtime) and the inode metadata (ctime), and any
> write also constitutes an access (atime). If no RTC is available, the
> timestamp falls back to 2000-01-01 00:00:00 UTC, matching the FAT
> write driver behaviour.
>
> The ext4 i_crtime field (true file creation time, added in ext4) is
> stored in the extra inode area beyond the base 128-byte inode and is
> not modelled by struct ext2_inode, so it cannot be set here.
> [...]
>
> fs/ext4/ext4_write.c | 41 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 39 insertions(+), 2 deletions(-)
> diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
> @@ -21,14 +21,47 @@
> +/**
> + * define EXT4_TIMESTAMP_Y2K - 2000-01-01 00:00:00 UTC as a POSIX timestamp
> + *
> + * Used as a fallback timestamp when no RTC is available.
> + */
> +#define EXT4_TIMESTAMP_Y2K 946684800
'define' is not a valid kernel-doc directive, is it? I suggest a plain
C comment.
Also, the natural sentinel for an unknown ext4 time is 0 (Unix epoch),
which the read path already displays as 1970-01-01; picking Y2K means
files written by U-Boot show 2000 while pre-existing zero-stamped
inodes show 1970, which is jarring. The FAT-parity argument is pretty
weak since FAT cannot represent dates before 1980.
> diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
> @@ -21,14 +21,47 @@
> +static time_t ext4_current_timestamp(void)
> +{
> + if (CONFIG_IS_ENABLED(DM_RTC)) {
> + struct udevice *dev;
> + struct rtc_time tm;
> +
> + uclass_first_device(UCLASS_RTC, &dev);
> + if (!dev)
> + goto fallback;
> + if (dm_rtc_get(dev, &tm))
> + goto fallback;
> + return rtc_mktime(&tm);
> + }
> +fallback:
> + return EXT4_TIMESTAMP_Y2K;
> +}
The label sitting outside the if-block and jumped to from inside reads
oddly. Please drop the gotos:
if (CONFIG_IS_ENABLED(DM_RTC)) {
struct udevice *dev;
struct rtc_time tm;
uclass_first_device(UCLASS_RTC, &dev);
if (dev && !dm_rtc_get(dev, &tm))
return rtc_mktime(&tm);
}
return EXT4_TIMESTAMP_Y2K;
Also, this helper sits at the top of a 1000-line file but is only used
at the bottom from ext4fs_write() - please move it nearer to its
caller.
> diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
> @@ -979,7 +1012,11 @@ int ext4fs_write(const char *fname, const char *buffer,
> }
> if (existing_file_inode)
> free(existing_file_inode);
> - /* ToDo: Update correct time */
> + /*
> + * Note: ext4 i_crtime (true creation time) lives in the extended
> + * inode area beyond the base 128-byte inode and is not modelled
> + * by struct ext2_inode, so it cannot be set here.
> + */
> file_inode->mtime = cpu_to_le32(timestamp);
The comment describes a field this code does not touch, which is more
confusing than helpful. The i_mtime_extra/i_atime_extra/i_ctime_extra
fields (nanoseconds and the high bits past 2038) have the same
limitation and arguably matter more, since we are truncating the
timestamp to 32 bits via cpu_to_le32(). Drop the comment, or expand it
to mention all the unmodelled time fields.
> diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
> @@ -854,7 +887,7 @@ int ext4fs_write(const char *fname, const char *buffer,
> - time_t timestamp = 0;
> + time_t timestamp = ext4_current_timestamp();
Note the parent directory's mtime/ctime should also change when a file
is created or replaced, but g_parent_inode is written back unchanged a
few lines further down. Not related to this patch, but worth a
follow-up.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 6/9] test: Probe RTC early in dm_test_host()
2026-05-18 5:57 [PATCH 0/9] fs: add change date to ls output Heinrich Schuchardt
` (4 preceding siblings ...)
2026-05-18 5:57 ` [PATCH 5/9] fs: ext4: set inode timestamps on write Heinrich Schuchardt
@ 2026-05-18 5:57 ` Heinrich Schuchardt
2026-05-19 20:00 ` Simon Glass
2026-05-18 5:57 ` [PATCH 7/9] test: fs: allow optional date field in ls output assertion Heinrich Schuchardt
` (3 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2026-05-18 5:57 UTC (permalink / raw)
To: Tom Rini
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs,
Heinrich Schuchardt
The ext4 driver probes and reads the RTC which allocates memory.
Ensure that the device is already probed and read once in dm_test_hook()
to avoid false positives.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
test/dm/host.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/test/dm/host.c b/test/dm/host.c
index f577377da6a..32a9818248a 100644
--- a/test/dm/host.c
+++ b/test/dm/host.c
@@ -8,6 +8,7 @@
#include <dm.h>
#include <fs.h>
#include <os.h>
+#include <rtc.h>
#include <sandbox_host.h>
#include <asm/test.h>
#include <dm/device-internal.h>
@@ -26,6 +27,18 @@ static int dm_test_host(struct unit_test_state *uts)
ulong mem_start;
loff_t actwrite;
+ /*
+ * Probing and first read from the RTC allocates memory.
+ * Do it before the measurement.
+ */
+ if (CONFIG_IS_ENABLED(DM_RTC)) {
+ struct rtc_time tm;
+
+ uclass_first_device(UCLASS_RTC, &dev);
+ if (dev)
+ dm_rtc_get(dev, &tm);
+ }
+
ut_asserteq(-ENODEV, uclass_first_device_err(UCLASS_HOST, &dev));
ut_asserteq(-ENODEV, uclass_first_device_err(UCLASS_PARTITION, &part));
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 6/9] test: Probe RTC early in dm_test_host()
2026-05-18 5:57 ` [PATCH 6/9] test: Probe RTC early in dm_test_host() Heinrich Schuchardt
@ 2026-05-19 20:00 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2026-05-19 20:00 UTC (permalink / raw)
To: heinrich.schuchardt
Cc: Tom Rini, Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
Hi Heinrich,
On 2026-05-18T05:57:19, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
> test: Probe RTC early in dm_test_host()
>
> The ext4 driver probes and reads the RTC which allocates memory.
>
> Ensure that the device is already probed and read once in dm_test_hook()
> to avoid false positives.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>
> test/dm/host.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
> diff --git a/test/dm/host.c b/test/dm/host.c
> @@ -26,6 +27,18 @@ static int dm_test_host(struct unit_test_state *uts)
> ulong mem_start;
> loff_t actwrite;
>
> + /*
> + * Probing and first read from the RTC allocates memory.
> + * Do it before the measurement.
> + */
Please mention here (and in the commit message) which call inside this
test ends up touching the RTC. It is non-obvious that fs_write()
further down now reaches into the RTC via patch 5, and a future reader
will not understand why an RTC probe belongs in a host test.
> diff --git a/test/dm/host.c b/test/dm/host.c
> @@ -26,6 +27,18 @@ static int dm_test_host(struct unit_test_state *uts)
> + if (CONFIG_IS_ENABLED(DM_RTC)) {
> + struct rtc_time tm;
> +
> + uclass_first_device(UCLASS_RTC, &dev);
> + if (dev)
> + dm_rtc_get(dev, &tm);
> + }
Since this is a DM test you should assert each of these calls.
Also, would it be simpler to take a second mem_start sample after the
warm-up, so the RTC allocation is naturally outside the measured
window without needing to know it exists?
> Ensure that the device is already probed and read once in dm_test_hook()
> to avoid false positives.
Should be dm_test_host(). Also spell out that it is the
ut_check_delta() leak check at the end of the test that gets fooled by
the RTC's one-shot allocation.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 7/9] test: fs: allow optional date field in ls output assertion
2026-05-18 5:57 [PATCH 0/9] fs: add change date to ls output Heinrich Schuchardt
` (5 preceding siblings ...)
2026-05-18 5:57 ` [PATCH 6/9] test: Probe RTC early in dm_test_host() Heinrich Schuchardt
@ 2026-05-18 5:57 ` Heinrich Schuchardt
2026-05-19 20:00 ` Simon Glass
2026-05-18 5:57 ` [PATCH 8/9] test: env: " Heinrich Schuchardt
` (2 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2026-05-18 5:57 UTC (permalink / raw)
To: Tom Rini
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs,
Heinrich Schuchardt
fs_ls_generic() now prints a date between the file size and filename
when the filesystem sets FS_CAP_DATE (currently FAT and ext4). The
two regex patterns in test_fs1 used ' *' (zero or more spaces) to
match between the size and filename; that no longer matches when a
date is present.
Change ' *' to ' .*' so the pattern matches both the old format
(size + spaces + name) and the new format (size + spaces + date + name).
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
test/py/tests/test_fs/test_basic.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/test/py/tests/test_fs/test_basic.py b/test/py/tests/test_fs/test_basic.py
index 88b163ce305..5f2af9e21d3 100644
--- a/test/py/tests/test_fs/test_basic.py
+++ b/test/py/tests/test_fs/test_basic.py
@@ -26,8 +26,8 @@ class TestFsBasic(object):
output = ubman.run_command_list([
'host bind 0 %s' % fs_img,
'%sls host 0:0' % fs_cmd_prefix])
- assert(re.search('2621440000 *%s' % BIG_FILE, ''.join(output)))
- assert(re.search('1048576 *%s' % SMALL_FILE, ''.join(output)))
+ assert(re.search('2621440000 .*%s' % BIG_FILE, ''.join(output)))
+ assert(re.search('1048576 .*%s' % SMALL_FILE, ''.join(output)))
with ubman.log.section('Test Case 1b - ls (invalid dir)'):
# In addition, test with a nonexistent directory to see if we crash.
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 7/9] test: fs: allow optional date field in ls output assertion
2026-05-18 5:57 ` [PATCH 7/9] test: fs: allow optional date field in ls output assertion Heinrich Schuchardt
@ 2026-05-19 20:00 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2026-05-19 20:00 UTC (permalink / raw)
To: heinrich.schuchardt
Cc: Tom Rini, Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
On 2026-05-18T05:57:19, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
> test: fs: allow optional date field in ls output assertion
>
> fs_ls_generic() now prints a date between the file size and filename
> when the filesystem sets FS_CAP_DATE (currently FAT and ext4). The
> two regex patterns in test_fs1 used ' *' (zero or more spaces) to
> match between the size and filename; that no longer matches when a
> date is present.
>
> Change ' *' to ' .*' so the pattern matches both the old format
> (size + spaces + name) and the new format (size + spaces + date + name).
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>
> test/py/tests/test_fs/test_basic.py | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 8/9] test: env: allow optional date field in ls output assertion
2026-05-18 5:57 [PATCH 0/9] fs: add change date to ls output Heinrich Schuchardt
` (6 preceding siblings ...)
2026-05-18 5:57 ` [PATCH 7/9] test: fs: allow optional date field in ls output assertion Heinrich Schuchardt
@ 2026-05-18 5:57 ` Heinrich Schuchardt
2026-05-19 20:00 ` Simon Glass
2026-05-18 5:57 ` [PATCH 9/9] test: test_erofs: adjust expected ls output Heinrich Schuchardt
2026-05-18 18:15 ` [PATCH 0/9] fs: add change date to " Tom Rini
9 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2026-05-18 5:57 UTC (permalink / raw)
To: Tom Rini
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs,
Heinrich Schuchardt
fs_ls_generic() now prints a date between the file size and filename
when the filesystem sets FS_CAP_DATE (currently FAT and ext4).
Adjust the assert in test_env.py().
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
test/py/tests/test_env.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
index f8713a59ba9..e9d502148bc 100644
--- a/test/py/tests/test_env.py
+++ b/test/py/tests/test_env.py
@@ -523,7 +523,7 @@ def test_env_ext4(state_test_env):
assert 'Loading Environment from EXT4... OK' in response
response = c.run_command('ext4ls host 0:0')
- assert '8192 uboot.env' in response
+ assert(re.search('8192 .*uboot.env', ''.join(response)))
response = c.run_command('env info')
assert 'env_valid = valid' in response
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 8/9] test: env: allow optional date field in ls output assertion
2026-05-18 5:57 ` [PATCH 8/9] test: env: " Heinrich Schuchardt
@ 2026-05-19 20:00 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2026-05-19 20:00 UTC (permalink / raw)
To: heinrich.schuchardt
Cc: Tom Rini, Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
Hi Heinrich,
On 2026-05-18T05:57:19, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
> test: env: allow optional date field in ls output assertion
>
> fs_ls_generic() now prints a date between the file size and filename
> when the filesystem sets FS_CAP_DATE (currently FAT and ext4).
>
> Adjust the assert in test_env.py().
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>
> test/py/tests/test_env.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> @@ -523,7 +523,7 @@ def test_env_ext4(state_test_env):
> assert 'Loading Environment from EXT4... OK' in response
>
> response = c.run_command('ext4ls host 0:0')
> - assert '8192 uboot.env' in response
> + assert(re.search('8192 .*uboot.env', ''.join(response)))
run_command() returns a string, not a list. So ''.join(response)
iterates the characters and rebuilds the string; it is a no-op. Please
drop the join() and pass response directly. Also, the trailing '.' in
uboot.env is a regex metacharacter - escape it as uboot\\.env.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 9/9] test: test_erofs: adjust expected ls output
2026-05-18 5:57 [PATCH 0/9] fs: add change date to ls output Heinrich Schuchardt
` (7 preceding siblings ...)
2026-05-18 5:57 ` [PATCH 8/9] test: env: " Heinrich Schuchardt
@ 2026-05-18 5:57 ` Heinrich Schuchardt
2026-05-19 20:00 ` Simon Glass
2026-05-18 18:15 ` [PATCH 0/9] fs: add change date to " Tom Rini
9 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2026-05-18 5:57 UTC (permalink / raw)
To: Tom Rini
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs,
Heinrich Schuchardt
With the addition of the date field the space between columns in the ls
output has been reduced. Reflect this in the expected lines of the erofs
test.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
test/py/tests/test_fs/test_erofs.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/test/py/tests/test_fs/test_erofs.py b/test/py/tests/test_fs/test_erofs.py
index a2bb6b505f2..0531f99cd9c 100644
--- a/test/py/tests/test_fs/test_erofs.py
+++ b/test/py/tests/test_fs/test_erofs.py
@@ -73,8 +73,8 @@ def erofs_ls_at_root(ubman):
slash = ubman.run_command('erofsls host 0 /')
assert no_slash == slash
- expected_lines = ['./', '../', '4096 f4096', '7812 f7812', 'subdir/',
- '<SYM> symdir', '<SYM> symfile', '4 file(s), 3 dir(s)']
+ expected_lines = ['./', '../', '4096 f4096', '7812 f7812', 'subdir/',
+ '<SYM> symdir', '<SYM> symfile', '4 file(s), 3 dir(s)']
output = ubman.run_command('erofsls host 0')
for line in expected_lines:
@@ -84,7 +84,7 @@ def erofs_ls_at_subdir(ubman):
"""
Test if the path resolution works.
"""
- expected_lines = ['./', '../', '100 subdir-file', '1 file(s), 2 dir(s)']
+ expected_lines = ['./', '../', '100 subdir-file', '1 file(s), 2 dir(s)']
output = ubman.run_command('erofsls host 0 subdir')
for line in expected_lines:
assert line in output
@@ -97,7 +97,7 @@ def erofs_ls_at_symlink(ubman):
output_subdir = ubman.run_command('erofsls host 0 subdir')
assert output == output_subdir
- expected_lines = ['./', '../', '100 subdir-file', '1 file(s), 2 dir(s)']
+ expected_lines = ['./', '../', '100 subdir-file', '1 file(s), 2 dir(s)']
for line in expected_lines:
assert line in output
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 9/9] test: test_erofs: adjust expected ls output
2026-05-18 5:57 ` [PATCH 9/9] test: test_erofs: adjust expected ls output Heinrich Schuchardt
@ 2026-05-19 20:00 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2026-05-19 20:00 UTC (permalink / raw)
To: heinrich.schuchardt
Cc: Tom Rini, Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
Hi Heinrich,
On 2026-05-18T05:57:19, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
> test: test_erofs: adjust expected ls output
>
> With the addition of the date field the space between columns in the ls
> output has been reduced. Reflect this in the expected lines of the erofs
> test.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>
> test/py/tests/test_fs/test_erofs.py | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
> diff --git a/test/py/tests/test_fs/test_erofs.py b/test/py/tests/test_fs/test_erofs.py
> @@ -73,8 +73,8 @@ def erofs_ls_at_root(ubman):
> - expected_lines = ['./', '../', '4096 f4096', '7812 f7812', 'subdir/',
> - '<SYM> symdir', '<SYM> symfile', '4 file(s), 3 dir(s)']
> + expected_lines = ['./', '../', '4096 f4096', '7812 f7812', 'subdir/',
> + '<SYM> symdir', '<SYM> symfile', '4 file(s), 3 dir(s)']
Patches 7 and 8 took the flexible-regex approach (' .*' / ' +') so the
assertion does not need touching again next time the column layout
shifts. Since EROFS goes through fs_ls_generic() too, please do the
same here rather than hard-coding the single-space layout.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/9] fs: add change date to ls output
2026-05-18 5:57 [PATCH 0/9] fs: add change date to ls output Heinrich Schuchardt
` (8 preceding siblings ...)
2026-05-18 5:57 ` [PATCH 9/9] test: test_erofs: adjust expected ls output Heinrich Schuchardt
@ 2026-05-18 18:15 ` Tom Rini
2026-05-19 7:25 ` Heinrich Schuchardt
9 siblings, 1 reply; 25+ messages in thread
From: Tom Rini @ 2026-05-18 18:15 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
[-- Attachment #1: Type: text/plain, Size: 423 bytes --]
On Mon, May 18, 2026 at 07:57:19AM +0200, Heinrich Schuchardt wrote:
> The ls command currently only displays the size and name of files and
> directories.
>
> * Add the change date to the output on FAT and ext2/3/4.
> * Use the actual date when updating the change date in ext2/3/4
> file-systems.
What's the motivation for this change, and how much of a size impact
does this have in general?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 0/9] fs: add change date to ls output
2026-05-18 18:15 ` [PATCH 0/9] fs: add change date to " Tom Rini
@ 2026-05-19 7:25 ` Heinrich Schuchardt
2026-05-19 13:55 ` Tom Rini
0 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2026-05-19 7:25 UTC (permalink / raw)
To: Tom Rini
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
On 5/18/26 20:15, Tom Rini wrote:
> On Mon, May 18, 2026 at 07:57:19AM +0200, Heinrich Schuchardt wrote:
>
>> The ls command currently only displays the size and name of files and
>> directories.
>>
>> * Add the change date to the output on FAT and ext2/3/4.
>> * Use the actual date when updating the change date in ext2/3/4
>> file-systems.
>
> What's the motivation for this change, and how much of a size impact
> does this have in general?
>
For qemu_arm64_defconfig fs/fs.o shows a growth of 260 bytes in .text
and .data sections.
Change times let users immediately spot which files were modified most
recently (kernel images, device trees).
If a device stops booting, seeing a file’s change date helps determine
whether a recent change could be the cause.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/9] fs: add change date to ls output
2026-05-19 7:25 ` Heinrich Schuchardt
@ 2026-05-19 13:55 ` Tom Rini
0 siblings, 0 replies; 25+ messages in thread
From: Tom Rini @ 2026-05-19 13:55 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Simon Glass, Huang Jianan, Quentin Schulz, Tony Dinh,
Timo tp Preißl, Francois Berder, Andrew Goodbody,
Daniel Palmer, Varadarajan Narayanan, Sughosh Ganu,
Ilias Apalodimas, Peng Fan, Marek Vasut, u-boot, linux-erofs
[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]
On Tue, May 19, 2026 at 09:25:37AM +0200, Heinrich Schuchardt wrote:
> On 5/18/26 20:15, Tom Rini wrote:
> > On Mon, May 18, 2026 at 07:57:19AM +0200, Heinrich Schuchardt wrote:
> >
> > > The ls command currently only displays the size and name of files and
> > > directories.
> > >
> > > * Add the change date to the output on FAT and ext2/3/4.
> > > * Use the actual date when updating the change date in ext2/3/4
> > > file-systems.
> >
> > What's the motivation for this change, and how much of a size impact
> > does this have in general?
> >
>
> For qemu_arm64_defconfig fs/fs.o shows a growth of 260 bytes in .text and
> .data sections.
OK, can you please use something like
https://source.denx.de/u-boot/u-boot-extras/-/blob/master/contrib/trini/u-boot-size-test.sh?ref_type=heads
for the whole board?
> Change times let users immediately spot which files were modified most
> recently (kernel images, device trees).
>
> If a device stops booting, seeing a file’s change date helps determine
> whether a recent change could be the cause.
Yes, that is useful and more user friendly than hashing a file. But I do
worry about global size growth.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread