Re: [RFC PATCH] drivers: bootcount: Add support for FAT filesystem

On Thu, Jun 06, 2024 at 05:34:55PM +0200, Quentin Schulz wrote:

Hi Vasileios,

On 6/5/24 8:10 PM, Vasileios Amoiridis wrote:

...

[You don't often get email from vassilisamir@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

From: Vasileios Amoiridis vasileios.amoiridis@cern.ch

Add support to save boot count variable in a file in a FAT filesystem.

Signed-off-by: Vasileios Amoiridis vasileios.amoiridis@cern.ch

doc/README.bootcount | 12 ++++---- drivers/bootcount/Kconfig | 49 +++++++++++++++++++++++-------- drivers/bootcount/Makefile | 2 +- drivers/bootcount/bootcount_ext.c | 18 ++++++++---- 4 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/doc/README.bootcount b/doc/README.bootcount index f6c5f82f98..cce66d4d70 100644 --- a/doc/README.bootcount +++ b/doc/README.bootcount @@ -23,15 +23,17 @@ 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. +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 3c56253b1e..a39a7556bb 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 filesystem"
  

•   depends on FS_EXT4 || FS_FAT
     help
       Add support for maintaining boot count in a file on an EXT
       filesystem.
  

@@ -177,6 +176,30 @@ config BOOTCOUNT_BOOTLIMIT counter being cleared. If set to 0, do not set a boot limit in the environment.

+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 maintaining boot count in a file on an EXT
  

•     filesystem.
  

+config BOOTCOUNT_FAT

•   bool "Boot counter on FAT filesystem"
  

•   depends on FS_FAT
  

•   select FAT_WRITE
  

•   help
  

•     Add support for maintaining boot count in a file on a FAT
  

•     filesystem.
  

+endchoice +endif

• config SYS_BOOTCOUNT_SINGLEWORD bool "Use single word to pack boot count and magic value" depends on BOOTCOUNT_GENERIC

@@ -184,26 +207,26 @@ 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 +config SYS_BOOTCOUNT_FS_INTERFACE string "Interface on which to find boot counter EXT filesystem" default "mmc"

•   depends on BOOTCOUNT_EXT
  

•   depends on BOOTCOUNT_FS
     help
       Set the interface to use when locating the filesystem to use for the
       boot counter.
  
  

-config SYS_BOOTCOUNT_EXT_DEVPART +config SYS_BOOTCOUNT_FS_DEVPART string "Partition of the boot counter EXT filesystem" default "0:1"

•   depends on BOOTCOUNT_EXT
  

•   depends on BOOTCOUNT_FS
     help
       Set the partition to use when locating the filesystem to use for the
       boot counter.
  
  

-config SYS_BOOTCOUNT_EXT_NAME +config SYS_BOOTCOUNT_FS_NAME string "Path and filename of the EXT filesystem based boot counter" default "/boot/failures"

•   depends on BOOTCOUNT_EXT
  

•   depends on BOOTCOUNT_FS
     help
       Set the filename and path of the file used to store the boot counter.
  
  

@@ -211,18 +234,18 @@ config SYS_BOOTCOUNT_ADDR hex "RAM address used for reading and writing the boot counter" default 0x44E3E000 if BOOTCOUNT_AM33XX || BOOTCOUNT_AM33XX_NVMEM default 0xE0115FF8 if ARCH_LS1043A || ARCH_LS1021A

•   depends on BOOTCOUNT_AM33XX || BOOTCOUNT_GENERIC || BOOTCOUNT_EXT || \
  

•   depends on BOOTCOUNT_AM33XX || BOOTCOUNT_GENERIC || BOOTCOUNT_FS || \
                BOOTCOUNT_AM33XX_NVMEM
     help
       Set the address used for reading and writing the boot counter.
  
  

  config SYS_BOOTCOUNT_MAGIC hex "Magic value for the boot counter"

•   default 0xB001C041 if BOOTCOUNT_GENERIC || BOOTCOUNT_EXT || \
  

•   default 0xB001C041 if BOOTCOUNT_GENERIC || BOOTCOUNT_FS || \
                           BOOTCOUNT_AM33XX || BOOTCOUNT_ENV || \
                           BOOTCOUNT_RAM || BOOTCOUNT_AT91 || DM_BOOTCOUNT
     default 0xB0 if BOOTCOUNT_AM33XX_NVMEM
  

•   depends on BOOTCOUNT_GENERIC || BOOTCOUNT_EXT || \
  

•   depends on BOOTCOUNT_GENERIC || BOOTCOUNT_FS || \
                BOOTCOUNT_AM33XX || BOOTCOUNT_ENV || \
                BOOTCOUNT_RAM || BOOTCOUNT_AT91 || DM_BOOTCOUNT || \
                BOOTCOUNT_AM33XX_NVMEM
  

diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile index e7771f5b36..c0427434d7 100644 --- a/drivers/bootcount/Makefile +++ b/drivers/bootcount/Makefile @@ -6,7 +6,7 @@ obj-$(CONFIG_BOOTCOUNT_AT91) += bootcount_at91.o obj-$(CONFIG_BOOTCOUNT_AM33XX) += bootcount_davinci.o obj-$(CONFIG_BOOTCOUNT_RAM) += bootcount_ram.o obj-$(CONFIG_BOOTCOUNT_ENV) += bootcount_env.o -obj-$(CONFIG_BOOTCOUNT_EXT) += bootcount_ext.o +obj-$(CONFIG_BOOTCOUNT_FS) += bootcount_ext.o obj-$(CONFIG_BOOTCOUNT_AM33XX_NVMEM) += bootcount_nvmem.o

obj-$(CONFIG_DM_BOOTCOUNT) += bootcount-uclass.o diff --git a/drivers/bootcount/bootcount_ext.c b/drivers/bootcount/bootcount_ext.c index 9639e638e9..660bea2db2 100644 --- a/drivers/bootcount/bootcount_ext.c +++ b/drivers/bootcount/bootcount_ext.c @@ -19,14 +19,20 @@ typedef struct {

static u8 upgrade_available = 1;

+#ifdef BOOTCOUNT_EXT +static u8 bootcount_fstype = FS_TYPE_EXT; +#else +static u8 bootcount_fstype = FS_TYPE_FAT; +#endif

I know nothing about filesystems, but I was wondering if we couldn't just use FS_TYPE_ANY here? Do we really need to specify which FS_TYPE to use? Are there some limitations, etc.?

Cheers, Quentin

Well, I am not very accustomed to the U-Boot code yet but by looking at the fs_set_blk_dev() and fs_write() code, you might be right. I will test with FS_TYPE_ANY and I will let you know.

Cheers, Vasilis