Hi Vasileios,
On 6/10/24 8:51 PM, Vasileios Amoiridis wrote:
> Add support to save boot count variable in a file in a FAT filesystem.
>
> Signed-off-by: Vasileios Amoiridis <vassilisa...@gmail.com>
> ---
> doc/README.bootcount | 12 ++---
> drivers/bootcount/Kconfig | 53 +++++++++++++------
> drivers/bootcount/Makefile | 2 +-
> .../{bootcount_ext.c => bootcount_fs.c} | 12 ++---
> 4 files changed, 50 insertions(+), 29 deletions(-)
> rename drivers/bootcount/{bootcount_ext.c => bootcount_fs.c} (81%)
>
> diff --git a/doc/README.bootcount b/doc/README.bootcount
> index f6c5f82f..0f4ffb68 100644
> --- a/doc/README.bootcount
> +++ b/doc/README.bootcount
> @@ -23,15 +23,15 @@ It is the responsibility of some application code
(typically a Linux
> application) to reset the variable "bootcount" to 0 when the system
booted
> successfully, thus allowing for more boot cycles.
>
> -CONFIG_BOOTCOUNT_EXT
> +CONFIG_BOOTCOUNT_FS
> --------------------
>
> -This adds support for maintaining boot count in a file on an EXT
filesystem.
> -The file to use is defined by:
> +This adds support for maintaining boot count in a file on a filesystem.
> +Supported filesystems are FAT and EXT. The file to use is defined by:
>
> -CONFIG_SYS_BOOTCOUNT_EXT_INTERFACE
> -CONFIG_SYS_BOOTCOUNT_EXT_DEVPART
> -CONFIG_SYS_BOOTCOUNT_EXT_NAME
> +CONFIG_SYS_BOOTCOUNT_FS_INTERFACE
> +CONFIG_SYS_BOOTCOUNT_FS_DEVPART
> +CONFIG_SYS_BOOTCOUNT_FS_NAME
>
> The format of the file is:
>
> diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig
> index 3c56253b..d3679eb5 100644
> --- a/drivers/bootcount/Kconfig
> +++ b/drivers/bootcount/Kconfig
> @@ -25,10 +25,9 @@ config BOOTCOUNT_GENERIC
> Set to the address where the bootcount and bootcount magic
> will be stored.
>
> -config BOOTCOUNT_EXT
> - bool "Boot counter on EXT filesystem"
> - depends on FS_EXT4
> - select EXT4_WRITE
> +config BOOTCOUNT_FS
> + bool "Boot counter on a filesystem"
> + depends on FS_EXT4 || FS_FAT
Do we really need this 'depends on' here? Especially if we have a choice
below...
> help
> Add support for maintaining boot count in a file on an EXT
The help text is still mentioning EXT here.
I would recommend removing it, or listing the supported filesystems at
the moment. While I assume you tested with FAT, I assume that with
FS_ANY, any FS should be supported?
> filesystem.
> @@ -184,26 +183,48 @@ config SYS_BOOTCOUNT_SINGLEWORD
> This option enables packing boot count magic value and boot count
> into single word (32 bits).
>
> -config SYS_BOOTCOUNT_EXT_INTERFACE
> - string "Interface on which to find boot counter EXT filesystem"
> +if BOOTCOUNT_FS
> +choice
> + prompt "Filesystem type"
> + default BOOTCOUNT_EXT
> +
> +config BOOTCOUNT_EXT
> + bool "Boot counter on EXT filesystem"
> + depends on FS_EXT4
> + select EXT4_WRITE
> + help
> + Add support for maintaing boot counter in a file on EXT filesystem"
> +
> +config BOOTCOUNT_FAT
> + bool "Boot counter on FAT filesystem"
> + depends on FS_FAT
> + select FAT_WRITE
> + help
> + Add support for maintaing boot counter in a file on FAT filesystem"
> +
> +endchoice
> +endif
> +
Since we now support FS_ANY, do we really need this choice at all?
Alternatively, should it **really** be a choice and not just a bunch of
configs that depends on BOOTCOUNT_FS + whatever's needed to write on
that FS instead? I think we could have both BOOTCOUNT_EXT and
BOOTCOUNT_FAT set without issue?
Cheers,
Quentin
