[PATCH v2 21/21] doc: fwu: make changes for supporting FWU Metadata version 2

2024-02-11 Thread Sughosh Ganu
Make changes to the FWU documentation to reflect the changes made with
migration of the FWU metadata to version 2.

Signed-off-by: Sughosh Ganu 
---

Changes since V1: None

 doc/board/socionext/developerbox.rst |  9 +++--
 doc/develop/uefi/fwu_updates.rst | 12 +---
 doc/usage/cmd/fwu_mdata.rst  | 12 
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/doc/board/socionext/developerbox.rst 
b/doc/board/socionext/developerbox.rst
index 46712c379b..d8c1bb4986 100644
--- a/doc/board/socionext/developerbox.rst
+++ b/doc/board/socionext/developerbox.rst
@@ -113,8 +113,6 @@ configs/synquacer_developerbox_defconfig enables default 
FWU configuration ::
  CONFIG_FWU_MULTI_BANK_UPDATE=y
  CONFIG_FWU_MDATA=y
  CONFIG_FWU_MDATA_MTD=y
- CONFIG_FWU_NUM_BANKS=2
- CONFIG_FWU_NUM_IMAGES_PER_BANK=1
  CONFIG_CMD_FWU_METADATA=y
 
 And build it::
@@ -126,10 +124,9 @@ And build it::
   make -j `noproc`
   cd ../
 
-By default, the CONFIG_FWU_NUM_BANKS and CONFIG_FWU_NUM_IMAGES_PER_BANKS are
-set to 2 and 1 respectively. This uses FIP (Firmware Image Package) type image
-which contains TF-A, U-Boot and OP-TEE (the OP-TEE is optional).
-You can use fiptool to compose the FIP image from those firmware images.
+This uses FIP (Firmware Image Package) type image which contains TF-A,
+U-Boot and OP-TEE (the OP-TEE is optional). You can use fiptool to
+compose the FIP image from those firmware images.
 
 Rebuild SCP firmware
 
diff --git a/doc/develop/uefi/fwu_updates.rst b/doc/develop/uefi/fwu_updates.rst
index e4709d82b4..7911c954d9 100644
--- a/doc/develop/uefi/fwu_updates.rst
+++ b/doc/develop/uefi/fwu_updates.rst
@@ -43,8 +43,6 @@ The feature can be enabled by specifying the following 
configs::
 CONFIG_FWU_MULTI_BANK_UPDATE=y
 CONFIG_FWU_MDATA=y
 CONFIG_FWU_MDATA_GPT_BLK=y
-CONFIG_FWU_NUM_BANKS=
-CONFIG_FWU_NUM_IMAGES_PER_BANK=
 
 in the .config file
 
@@ -94,12 +92,12 @@ of. Each GPT partition entry in the GPT header has two 
GUIDs::
 * UniquePartitionGUID
 
 The PartitionTypeGUID value should correspond to the
-``image_type_uuid`` field of the FWU metadata. This field is used to
+``image_type_guid`` field of the FWU metadata. This field is used to
 identify a given type of updatable firmware image, e.g. U-Boot,
 OP-TEE, FIP etc. This GUID should also be used for specifying the
 `--guid` parameter when generating the capsule.
 
-The UniquePartitionGUID value should correspond to the ``image_uuid``
+The UniquePartitionGUID value should correspond to the ``image_guid``
 field in the FWU metadata. This GUID is used to identify images of a
 given image type in different banks.
 
@@ -108,8 +106,8 @@ metadata partitions. This would be the PartitionTypeGUID 
for the
 metadata partitions. Similarly, the UEFI specification defines the ESP
 GUID to be be used.
 
-When generating the metadata, the ``image_type_uuid`` and the
-``image_uuid`` values should match the *PartitionTypeGUID* and the
+When generating the metadata, the ``image_type_guid`` and the
+``image_guid`` values should match the *PartitionTypeGUID* and the
 *UniquePartitionGUID* values respectively.
 
 Performing the Update
@@ -181,5 +179,5 @@ empty capsule would be::
 Links
 -
 
-* [1] https://developer.arm.com/documentation/den0118/a/ - FWU Specification
+* [1] https://developer.arm.com/documentation/den0118/ - FWU Specification
 * [2] 
https://git.codelinaro.org/linaro/dependable-boot/mbfw/uploads/6f7ddfe3be24e18d4319e108a758d02e/mbfw.pdf
 - Dependable Boot Specification
diff --git a/doc/usage/cmd/fwu_mdata.rst b/doc/usage/cmd/fwu_mdata.rst
index f1bf08fde1..1804422b33 100644
--- a/doc/usage/cmd/fwu_mdata.rst
+++ b/doc/usage/cmd/fwu_mdata.rst
@@ -26,10 +26,14 @@ The output may look like:
 
 => fwu_mdata_read
 FWU Metadata
-crc32: 0xec4fb997
-version: 0x1
-active_index: 0x0
-previous_active_index: 0x1
+crc32: 0x13c330
+version: 0x2
+active_index: 0x1
+previous_active_index: 0x0
+bank_state[0]: 0xfc
+bank_state[1]: 0xfc
+bank_state[2]: 0xff
+bank_state[3]: 0xff
 Image Info
 
 Image Type Guid: 19D5DF83-11B0-457B-BE2C-7559C13142A5
-- 
2.34.1



[PATCH v2 20/21] configs: fwu: re-enable FWU configs

2024-02-11 Thread Sughosh Ganu
Now that the migration to the FWU metadata version 2 is complete, the
feature can be re-enabled on platforms which had it enabled.

Signed-off-by: Sughosh Ganu 
---

Changes since V1: None

 configs/corstone1000_defconfig   | 2 ++
 configs/sandbox64_defconfig  | 1 +
 configs/synquacer_developerbox_defconfig | 3 +++
 3 files changed, 6 insertions(+)

diff --git a/configs/corstone1000_defconfig b/configs/corstone1000_defconfig
index c26a621175..a86ac12732 100644
--- a/configs/corstone1000_defconfig
+++ b/configs/corstone1000_defconfig
@@ -25,6 +25,7 @@ CONFIG_BOARD_LATE_INIT=y
 CONFIG_SYS_PROMPT="corstone1000# "
 CONFIG_SYS_MAXARGS=64
 # CONFIG_CMD_CONSOLE is not set
+CONFIG_CMD_FWU_METADATA=y
 CONFIG_CMD_BOOTZ=y
 # CONFIG_CMD_XIMG is not set
 CONFIG_CMD_GPT=y
@@ -67,3 +68,4 @@ CONFIG_FFA_SHARED_MM_BUF_OFFSET=0
 CONFIG_FFA_SHARED_MM_BUF_ADDR=0x0200
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_IGNORE_OSINDICATIONS=y
+CONFIG_FWU_MULTI_BANK_UPDATE=y
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 2863f9b189..d101cca6a7 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -273,6 +273,7 @@ CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
+CONFIG_FWU_MULTI_BANK_UPDATE=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
diff --git a/configs/synquacer_developerbox_defconfig 
b/configs/synquacer_developerbox_defconfig
index 616d410074..1bb55be0a7 100644
--- a/configs/synquacer_developerbox_defconfig
+++ b/configs/synquacer_developerbox_defconfig
@@ -17,6 +17,7 @@ CONFIG_SYS_BOOTM_LEN=0x80
 CONFIG_BOOTSTAGE_STASH_SIZE=4096
 CONFIG_HUSH_PARSER=y
 CONFIG_SYS_MAXARGS=128
+CONFIG_CMD_FWU_METADATA=y
 CONFIG_CMD_IMLS=y
 CONFIG_CMD_ERASEENV=y
 CONFIG_CMD_NVEDIT_EFI=y
@@ -51,6 +52,7 @@ CONFIG_DFU_TFTP=y
 CONFIG_DFU_MTD=y
 CONFIG_DFU_RAM=y
 CONFIG_DFU_SF=y
+CONFIG_FWU_MDATA_MTD=y
 CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_SYNQUACER=y
 CONFIG_MMC_SDHCI=y
@@ -93,3 +95,4 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_IGNORE_OSINDICATIONS=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
+CONFIG_FWU_MULTI_BANK_UPDATE=y
-- 
2.34.1



[PATCH v2 19/21] tools: mkfwumdata: fix the size parameter to the fwrite call

2024-02-11 Thread Sughosh Ganu
The fwrite call returns the number of bytes transferred as part of the
write only when the size parameter is 1. Pass the size parameter to
the library call as 1 so that the correct number of bytes transferred
are returned.

Fixes: fdd56bfd3ad ("tools: Add mkfwumdata tool for FWU metadata image")
Signed-off-by: Sughosh Ganu 
---

Changes since V1: New patch

 tools/mkfwumdata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/mkfwumdata.c b/tools/mkfwumdata.c
index ab07623e25..426d221ac7 100644
--- a/tools/mkfwumdata.c
+++ b/tools/mkfwumdata.c
@@ -338,7 +338,7 @@ fwu_make_mdata(size_t images, size_t banks, const char 
*vendor_file,
goto done_make;
}
 
-   ret = fwrite(mobj->mdata, mobj->size, 1, file);
+   ret = fwrite(mobj->mdata, 1, mobj->size, file);
if (ret != mobj->size)
ret = -errno;
else
-- 
2.34.1



[PATCH v2 18/21] tools: mkfwumdata: add logic to append vendor data to the FWU metadata

2024-02-11 Thread Sughosh Ganu
The version 2 of the FWU metadata allows for appending opaque vendor
specific data to the metadata structure. Add support for appending
this data to the metadata. The vendor specific data needs to be
provided through a file, passed through a command-line parameter.

Signed-off-by: Sughosh Ganu 
---

Changes since V1: New patch

 tools/mkfwumdata.c | 85 +-
 1 file changed, 77 insertions(+), 8 deletions(-)

diff --git a/tools/mkfwumdata.c b/tools/mkfwumdata.c
index fb847e3a78..ab07623e25 100644
--- a/tools/mkfwumdata.c
+++ b/tools/mkfwumdata.c
@@ -12,6 +12,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 /* Since we can not include fwu.h, redefine version here. */
@@ -30,7 +32,7 @@ typedef uint64_t u64;
 
 #include 
 
-static const char *opts_short = "b:i:a:p:gh";
+static const char *opts_short = "b:i:a:p:v:gh";
 
 static struct option options[] = {
{"banks", required_argument, NULL, 'b'},
@@ -38,6 +40,7 @@ static struct option options[] = {
{"guid", required_argument, NULL, 'g'},
{"active-bank", required_argument, NULL, 'a'},
{"previous-bank", required_argument, NULL, 'p'},
+   {"vendor-file", required_argument, NULL, 'v'},
{"help", no_argument, NULL, 'h'},
{NULL, 0, NULL, 0},
 };
@@ -51,6 +54,7 @@ static void print_usage(void)
"\t-a, --active-bank  Active bank (default=0)\n"
"\t-p, --previous-bankPrevious active bank 
(default=active_bank - 1)\n"
"\t-g, --guid  Use GUID instead of UUID\n"
+   "\t-v, --vendor-file   Vendor data file to append to 
the metadata\n"
"\t-h, --help  print a help message\n"
);
fprintf(stderr, "  UUIDs list syntax:\n"
@@ -69,13 +73,16 @@ struct fwu_mdata_object {
size_t images;
size_t banks;
size_t size;
+   size_t vsize;
+   void *vbuf;
struct fwu_mdata *mdata;
 };
 
 static int previous_bank, active_bank;
 static bool __use_guid;
 
-static struct fwu_mdata_object *fwu_alloc_mdata(size_t images, size_t banks)
+static struct fwu_mdata_object *fwu_alloc_mdata(size_t images, size_t banks,
+   size_t vendor_size)
 {
struct fwu_mdata_object *mobj;
 
@@ -87,16 +94,28 @@ static struct fwu_mdata_object *fwu_alloc_mdata(size_t 
images, size_t banks)
sizeof(struct fwu_fw_store_desc) +
(sizeof(struct fwu_image_entry) +
 sizeof(struct fwu_image_bank_info) * banks) * images;
+
+   mobj->size += vendor_size;
+   mobj->vsize = vendor_size;
mobj->images = images;
mobj->banks = banks;
 
mobj->mdata = calloc(1, mobj->size);
-   if (!mobj->mdata) {
-   free(mobj);
-   return NULL;
+   if (!mobj->mdata)
+   goto alloc_err;
+
+   if (vendor_size) {
+   mobj->vbuf = calloc(1, mobj->vsize);
+   if (!mobj->vbuf)
+   goto alloc_err;
}
 
return mobj;
+
+alloc_err:
+   free(mobj->mdata);
+   free(mobj);
+   return NULL;
 }
 
 static struct fwu_image_entry *
@@ -223,6 +242,7 @@ static int fwu_parse_fill_uuids(struct fwu_mdata_object 
*mobj, char *uuids[])
 {
struct fwu_mdata *mdata = mobj->mdata;
struct fwu_fw_store_desc *fw_desc;
+   char *vdata;
int i, ret;
 
mdata->version = FWU_MDATA_VERSION;
@@ -249,23 +269,65 @@ static int fwu_parse_fill_uuids(struct fwu_mdata_object 
*mobj, char *uuids[])
return ret;
}
 
+   if (mobj->vsize) {
+   vdata = (char *)mobj->mdata + (mobj->size - mobj->vsize);
+   memcpy(vdata, mobj->vbuf, mobj->vsize);
+   }
+
mdata->crc32 = crc32(0, (const unsigned char *)>version,
 mobj->size - sizeof(uint32_t));
 
return 0;
 }
 
+static int fwu_read_vendor_data(struct fwu_mdata_object *mobj,
+   const char *vendor_file)
+{
+   int ret = 0;
+   FILE *vfile = NULL;
+
+   vfile = fopen(vendor_file, "r");
+   if (!vfile) {
+   ret = -1;
+   goto out;
+   }
+
+   if (fread(mobj->vbuf, 1, mobj->vsize, vfile) != mobj->vsize)
+   ret = -1;
+
+out:
+   fclose(vfile);
+   return ret;
+}
+
 static int
-fwu_make_mdata(size_t images, size_t banks, char *uuids[], char *output)
+fwu_make_mdata(size_t images, size_t banks, const char *vendor_file,
+  char *uuids[], char *output)
 {
struct fwu_mdata_object *mobj;
FILE *file;
+   struct stat sbuf;
+   size_t vendor_size = 0;
int ret;
 
-   mobj = fwu_alloc_mdata(images, banks);
+   if (vendor_file) {
+   ret = stat(vendor_file, );
+   if (ret)
+   return -errno;
+
+ 

[PATCH v2 17/21] tools: mkfwumdata: migrate to metadata version 2

2024-02-11 Thread Sughosh Ganu
Migrate the metadata generation tool to generate the version 2
metadata.

Signed-off-by: Sughosh Ganu 
---

Changes since V1:
* Compute location of struct fwu_fw_store_desc using pointer
  arithmetic.

 tools/mkfwumdata.c | 45 ++---
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/tools/mkfwumdata.c b/tools/mkfwumdata.c
index 9732a8ddc5..fb847e3a78 100644
--- a/tools/mkfwumdata.c
+++ b/tools/mkfwumdata.c
@@ -14,12 +14,13 @@
 #include 
 #include 
 
-/* This will dynamically allocate the fwu_mdata */
-#define CONFIG_FWU_NUM_BANKS   0
-#define CONFIG_FWU_NUM_IMAGES_PER_BANK 0
-
 /* Since we can not include fwu.h, redefine version here. */
-#define FWU_MDATA_VERSION  1
+#define FWU_MDATA_VERSION  2
+
+#define MAX_BANKS  4
+
+#define BANK_INVALID   0xFF
+#define BANK_ACCEPTED  0xFC
 
 typedef uint8_t u8;
 typedef int16_t s16;
@@ -29,8 +30,6 @@ typedef uint64_t u64;
 
 #include 
 
-/* TODO: Endianness conversion may be required for some arch. */
-
 static const char *opts_short = "b:i:a:p:gh";
 
 static struct option options[] = {
@@ -48,7 +47,7 @@ static void print_usage(void)
fprintf(stderr, "Usage: mkfwumdata [options]  \n");
fprintf(stderr, "Options:\n"
"\t-i, --images   Number of images (mandatory)\n"
-   "\t-b, --banksNumber of banks (mandatory)\n"
+   "\t-b, --banksNumber of banks(1-4) 
(mandatory)\n"
"\t-a, --active-bank  Active bank (default=0)\n"
"\t-p, --previous-bankPrevious active bank 
(default=active_bank - 1)\n"
"\t-g, --guid  Use GUID instead of UUID\n"
@@ -85,6 +84,7 @@ static struct fwu_mdata_object *fwu_alloc_mdata(size_t 
images, size_t banks)
return NULL;
 
mobj->size = sizeof(struct fwu_mdata) +
+   sizeof(struct fwu_fw_store_desc) +
(sizeof(struct fwu_image_entry) +
 sizeof(struct fwu_image_bank_info) * banks) * images;
mobj->images = images;
@@ -105,6 +105,7 @@ fwu_get_image(struct fwu_mdata_object *mobj, size_t idx)
size_t offset;
 
offset = sizeof(struct fwu_mdata) +
+   sizeof(struct fwu_fw_store_desc) +
(sizeof(struct fwu_image_entry) +
 sizeof(struct fwu_image_bank_info) * mobj->banks) * idx;
 
@@ -117,6 +118,7 @@ fwu_get_bank(struct fwu_mdata_object *mobj, size_t img_idx, 
size_t bnk_idx)
size_t offset;
 
offset = sizeof(struct fwu_mdata) +
+   sizeof(struct fwu_fw_store_desc) +
(sizeof(struct fwu_image_entry) +
 sizeof(struct fwu_image_bank_info) * mobj->banks) * img_idx +
sizeof(struct fwu_image_entry) +
@@ -188,7 +190,7 @@ fwu_parse_fill_image_uuid(struct fwu_mdata_object *mobj,
return -EINVAL;
 
if (strcmp(uuid, "0") &&
-   uuid_guid_parse(uuid, (unsigned char *)>location_uuid) < 0)
+   uuid_guid_parse(uuid, (unsigned char *)>location_guid) < 0)
return -EINVAL;
 
/* Image type UUID */
@@ -196,7 +198,7 @@ fwu_parse_fill_image_uuid(struct fwu_mdata_object *mobj,
if (!uuid)
return -EINVAL;
 
-   if (uuid_guid_parse(uuid, (unsigned char *)>image_type_uuid) < 0)
+   if (uuid_guid_parse(uuid, (unsigned char *)>image_type_guid) < 0)
return -EINVAL;
 
/* Fill bank image-UUID */
@@ -210,7 +212,7 @@ fwu_parse_fill_image_uuid(struct fwu_mdata_object *mobj,
return -EINVAL;
 
if (strcmp(uuid, "0") &&
-   uuid_guid_parse(uuid, (unsigned char *)>image_uuid) < 
0)
+   uuid_guid_parse(uuid, (unsigned char *)>image_guid) < 
0)
return -EINVAL;
}
return 0;
@@ -220,11 +222,26 @@ fwu_parse_fill_image_uuid(struct fwu_mdata_object *mobj,
 static int fwu_parse_fill_uuids(struct fwu_mdata_object *mobj, char *uuids[])
 {
struct fwu_mdata *mdata = mobj->mdata;
+   struct fwu_fw_store_desc *fw_desc;
int i, ret;
 
mdata->version = FWU_MDATA_VERSION;
mdata->active_index = active_bank;
mdata->previous_active_index = previous_bank;
+   mdata->metadata_size = mobj->size;
+   mdata->desc_offset = sizeof(struct fwu_mdata);
+
+   for (i = 0; i < MAX_BANKS; i++)
+   mdata->bank_state[i] = i < mobj->banks ?
+   BANK_ACCEPTED : BANK_INVALID;
+
+   fw_desc = (struct fwu_fw_store_desc *)((u8 *)mdata + sizeof(*mdata));
+   fw_desc->num_banks = mobj->banks;
+   fw_desc->num_images = mobj->images;
+   fw_desc->img_entry_size = sizeof(struct fwu_image_entry) +
+   (sizeof(struct fwu_image_bank_info) * mobj->banks);
+   fw_desc->bank_info_entry_size =
+   

[PATCH v2 16/21] fwu: remove the config symbols for number of banks and images

2024-02-11 Thread Sughosh Ganu
With the migration to the FWU metadata version 2 structure, the values
of number of banks and number of images per bank are now obtained from
the metadata at runtime. Remove the now superfluous config symbols.

Signed-off-by: Sughosh Ganu 
---

Changes since V1: None

 arch/sandbox/Kconfig |  6 --
 board/armltd/corstone1000/corstone1000.c |  2 +-
 lib/fwu_updates/Kconfig  | 11 ---
 3 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/arch/sandbox/Kconfig b/arch/sandbox/Kconfig
index 0ce77de2fc..29013a5673 100644
--- a/arch/sandbox/Kconfig
+++ b/arch/sandbox/Kconfig
@@ -79,9 +79,3 @@ config SYS_FDT_LOAD_ADDR
  See `doc/arch/sandbox.rst` for more information.
 
 endmenu
-
-config FWU_NUM_BANKS
-   default 2
-
-config FWU_NUM_IMAGES_PER_BANK
-   default 2
diff --git a/board/armltd/corstone1000/corstone1000.c 
b/board/armltd/corstone1000/corstone1000.c
index 01c80aaf9d..15de738154 100644
--- a/board/armltd/corstone1000/corstone1000.c
+++ b/board/armltd/corstone1000/corstone1000.c
@@ -109,7 +109,7 @@ void fwu_plat_get_bootidx(uint *boot_idx)
 */
ret = fwu_get_active_index(boot_idx);
if (ret < 0) {
-   *boot_idx = CONFIG_FWU_NUM_BANKS;
+   *boot_idx = 0;
log_err("corstone1000: failed to read active index\n");
}
 }
diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig
index d35247d0e5..6bb94f88d3 100644
--- a/lib/fwu_updates/Kconfig
+++ b/lib/fwu_updates/Kconfig
@@ -12,17 +12,6 @@ menuconfig FWU_MULTI_BANK_UPDATE
 
 if FWU_MULTI_BANK_UPDATE
 
-config FWU_NUM_BANKS
-   int "Number of Banks defined by the platform"
-   help
- Define the number of banks of firmware images on a platform
-
-config FWU_NUM_IMAGES_PER_BANK
-   int "Number of firmware images per bank"
-   help
- Define the number of firmware images per bank. This value
- should be the same for all the banks.
-
 config FWU_TRIAL_STATE_CNT
int "Number of times system boots in Trial State"
default 3
-- 
2.34.1



[PATCH v2 15/21] test: fwu: align the FWU metadata access test with version 2

2024-02-11 Thread Sughosh Ganu
With the FWU metadata support having been migrated to version 2, make
corresponding changes to the test for accessing the FWU metadata.

Signed-off-by: Sughosh Ganu 
---

Changes since V1:
* Use the helper functions from the previous patch to access the
  image information in the metadata.

 test/dm/fwu_mdata.c|  56 ---
 test/dm/fwu_mdata_disk_image.h | 124 ++---
 2 files changed, 83 insertions(+), 97 deletions(-)

diff --git a/test/dm/fwu_mdata.c b/test/dm/fwu_mdata.c
index 52018f610f..cbeceaee93 100644
--- a/test/dm/fwu_mdata.c
+++ b/test/dm/fwu_mdata.c
@@ -88,10 +88,15 @@ static int write_mmc_blk_device(struct unit_test_state *uts)
return 0;
 }
 
-static int dm_test_fwu_mdata_read(struct unit_test_state *uts)
+static int fwu_mdata_access_setup(struct unit_test_state *uts,
+  struct fwu_mdata **mdata)
 {
+   u32 mdata_size;
struct udevice *dev;
-   struct fwu_mdata mdata = { 0 };
+
+   ut_assertok(setup_blk_device(uts));
+   ut_assertok(populate_mmc_disk_image(uts));
+   ut_assertok(write_mmc_blk_device(uts));
 
/*
 * Trigger lib/fwu_updates/fwu.c fwu_boottime_checks()
@@ -100,13 +105,24 @@ static int dm_test_fwu_mdata_read(struct unit_test_state 
*uts)
event_notify_null(EVT_MAIN_LOOP);
 
ut_assertok(uclass_first_device_err(UCLASS_FWU_MDATA, ));
-   ut_assertok(setup_blk_device(uts));
-   ut_assertok(populate_mmc_disk_image(uts));
-   ut_assertok(write_mmc_blk_device(uts));
 
-   ut_assertok(fwu_get_mdata());
+   ut_assertok(fwu_get_mdata_size(_size));
 
-   ut_asserteq(mdata.version, 0x1);
+   *mdata = malloc(mdata_size);
+   ut_assertnonnull(*mdata);
+
+   return 0;
+}
+
+static int dm_test_fwu_mdata_read(struct unit_test_state *uts)
+{
+   struct fwu_mdata *mdata = NULL;
+
+   fwu_mdata_access_setup(uts, );
+
+   ut_assertok(fwu_get_mdata(mdata));
+
+   ut_asserteq(mdata->version, 0x2);
 
return 0;
 }
@@ -114,29 +130,21 @@ DM_TEST(dm_test_fwu_mdata_read, UT_TESTF_SCAN_FDT);
 
 static int dm_test_fwu_mdata_write(struct unit_test_state *uts)
 {
+   u8 num_banks;
u32 active_idx;
-   struct udevice *dev;
-   struct fwu_mdata mdata = { 0 };
-
-   /*
-* Trigger lib/fwu_updates/fwu.c fwu_boottime_checks()
-* to populate g_dev global pointer in that library.
-*/
-   event_notify_null(EVT_MAIN_LOOP);
+   struct fwu_mdata *mdata = NULL;
 
-   ut_assertok(setup_blk_device(uts));
-   ut_assertok(populate_mmc_disk_image(uts));
-   ut_assertok(write_mmc_blk_device(uts));
-
-   ut_assertok(uclass_first_device_err(UCLASS_FWU_MDATA, ));
+   fwu_mdata_access_setup(uts, );
 
-   ut_assertok(fwu_get_mdata());
+   ut_assertok(fwu_get_mdata(mdata));
+   num_banks = fwu_get_fw_desc(mdata)->num_banks;
+   ut_asserteq(2, num_banks);
 
-   active_idx = (mdata.active_index + 1) % CONFIG_FWU_NUM_BANKS;
+   active_idx = (mdata->active_index + 1) % num_banks;
ut_assertok(fwu_set_active_index(active_idx));
 
-   ut_assertok(fwu_get_mdata());
-   ut_asserteq(mdata.active_index, active_idx);
+   ut_assertok(fwu_get_mdata(mdata));
+   ut_asserteq(mdata->active_index, active_idx);
 
return 0;
 }
diff --git a/test/dm/fwu_mdata_disk_image.h b/test/dm/fwu_mdata_disk_image.h
index b9803417c8..b15b06b9de 100644
--- a/test/dm/fwu_mdata_disk_image.h
+++ b/test/dm/fwu_mdata_disk_image.h
@@ -6,107 +6,85 @@
  */
 
 #define FWU_MDATA_DISK_IMG { 0x0001, { \
-   {0x01c0, "\x02\x00\xee\x02\x02\x00\x01\x00"}, /*  */ \
+   {0x01c0, "\x02\x00\xee\xff\xff\xff\x01\x00"}, /*  */ \
{0x01c8, "\x00\x00\x7f\x00\x00\x00\x00\x00"}, /*  */ \
{0x01f8, "\x00\x00\x00\x00\x00\x00\x55\xaa"}, /* ..U. */ \
{0x0200, "\x45\x46\x49\x20\x50\x41\x52\x54"}, /* EFI PART */ \
{0x0208, "\x00\x00\x01\x00\x5c\x00\x00\x00"}, /* \... */ \
-   {0x0210, "\xa6\xf6\x92\x20\x00\x00\x00\x00"}, /* ...  */ \
+   {0x0210, "\x52\x5f\x3a\xa1\x00\x00\x00\x00"}, /* R_:. */ \
{0x0218, "\x01\x00\x00\x00\x00\x00\x00\x00"}, /*  */ \
{0x0220, "\x7f\x00\x00\x00\x00\x00\x00\x00"}, /*  */ \
{0x0228, "\x22\x00\x00\x00\x00\x00\x00\x00"}, /* "... */ \
{0x0230, "\x5e\x00\x00\x00\x00\x00\x00\x00"}, /* ^... */ \
-   {0x0238, "\xde\x99\xa2\x7e\x46\x34\xeb\x47"}, /* ...~F4.G */ \
-   {0x0240, "\x87\xf6\x4f\x75\xe8\xd5\x7d\xc7"}, /* ..Ou..}. */ \
+   {0x0238, "\xf5\xf3\x9d\xb9\x92\xdd\x48\x60"}, /* ..H` */ \
+   {0x0240, "\x9a\x04\xe5\x2b\x11\xcb\x42\x61"}, /* ...+..Ba */ \
{0x0248, "\x02\x00\x00\x00\x00\x00\x00\x00"}, /*  */ \
{0x0250, "\x80\x00\x00\x00\x80\x00\x00\x00"}, /*  */ \
-   {0x0258, 

[PATCH v2 14/21] cmd: fwu: align the command with metadata version 2

2024-02-11 Thread Sughosh Ganu
Make changes to the fwu_mdata_read command to have it align with the
metadata version 2.

Signed-off-by: Sughosh Ganu 
---

Changes since V1:
* Use the helper functions from the previous patch to access the
  image information in the metadata.

 cmd/fwu_mdata.c | 45 +++--
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c
index 5ecda455df..6d5ed7a187 100644
--- a/cmd/fwu_mdata.c
+++ b/cmd/fwu_mdata.c
@@ -16,6 +16,8 @@
 static void print_mdata(struct fwu_mdata *mdata)
 {
int i, j;
+   uint8_t num_banks;
+   uint16_t num_images;
struct fwu_image_entry *img_entry;
struct fwu_image_bank_info *img_info;
 
@@ -25,15 +27,22 @@ static void print_mdata(struct fwu_mdata *mdata)
printf("active_index: %#x\n", mdata->active_index);
printf("previous_active_index: %#x\n", mdata->previous_active_index);
 
+   num_banks = fwu_get_fw_desc(mdata)->num_banks;
+   num_images = fwu_get_fw_desc(mdata)->num_images;
+
+   for (i = 0; i < 4; i++)
+   printf("bank_state[%d]: %#x\n", i, mdata->bank_state[i]);
+
printf("\tImage Info\n");
-   for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {
-   img_entry = >img_entry[i];
+
+   for (i = 0; i < num_images; i++) {
+   img_entry = fwu_img_entry_offset(mdata, i);
printf("\nImage Type Guid: %pUL\n",
-  _entry->image_type_uuid);
-   printf("Location Guid: %pUL\n", _entry->location_uuid);
-   for (j = 0; j < CONFIG_FWU_NUM_BANKS; j++) {
-   img_info = _entry->img_bank_info[j];
-   printf("Image Guid:  %pUL\n", _info->image_uuid);
+  _entry->image_type_guid);
+   printf("Location Guid: %pUL\n", _entry->location_guid);
+   for (j = 0; j < num_banks; j++) {
+   img_info = fwu_img_bank_info_offset(mdata, i, j);
+   printf("Image Guid:  %pUL\n", _info->image_guid);
printf("Image Acceptance: %s\n",
   img_info->accepted == 0x1 ? "yes" : "no");
}
@@ -43,19 +52,35 @@ static void print_mdata(struct fwu_mdata *mdata)
 int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag,
 int argc, char * const argv[])
 {
+   uint32_t mdata_size;
+   struct fwu_mdata *mdata = NULL;
int ret = CMD_RET_SUCCESS, res;
-   struct fwu_mdata mdata;
 
-   res = fwu_get_mdata();
+   res = fwu_get_mdata_size(_size);
+   if (res) {
+   log_err("Unable to get FWU metadata size\n");
+   ret = CMD_RET_FAILURE;
+   goto out;
+   }
+
+   mdata = malloc(mdata_size);
+   if (!mdata) {
+   log_err("Unable to allocate memory for FWU metadata\n");
+   ret = CMD_RET_FAILURE;
+   goto out;
+   }
+
+   res = fwu_get_mdata(mdata);
if (res < 0) {
log_err("Unable to get valid FWU metadata\n");
ret = CMD_RET_FAILURE;
goto out;
}
 
-   print_mdata();
+   print_mdata(mdata);
 
 out:
+   free(mdata);
return ret;
 }
 
-- 
2.34.1



[PATCH v2 13/21] efi_firmware: fwu: get the number of FWU banks at runtime

2024-02-11 Thread Sughosh Ganu
With the migration of the FWU metadata to version 2, the number of
banks are now obtained at runtime, instead of the config symbols. Make
use of the API to get the number of banks in the versioning
functions.

Signed-off-by: Sughosh Ganu 
---

Changes since V1: None

 lib/efi_loader/efi_firmware.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index c742e23268..26d2916ee1 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -207,7 +207,8 @@ void efi_firmware_fill_version_info(struct 
efi_firmware_image_descriptor *image_
u16 varname[13]; /* u"FmpState" */
efi_status_t ret;
efi_uintn_t size, expected_size;
-   uint num_banks = 1;
+   u8 num_banks = 1;
+   u16 __maybe_unused num_images;
uint active_index = 0;
struct fmp_state *var_state;
 
@@ -229,7 +230,9 @@ void efi_firmware_fill_version_info(struct 
efi_firmware_image_descriptor *image_
if (ret)
return;
 
-   num_banks = CONFIG_FWU_NUM_BANKS;
+   ret = fwu_get_banks_images(_banks, _images);
+   if (ret)
+   return;
}
 
size = num_banks * sizeof(*var_state);
@@ -379,7 +382,8 @@ efi_status_t efi_firmware_set_fmp_state_var(struct 
fmp_state *state, u8 image_in
 {
u16 varname[13]; /* u"FmpState" */
efi_status_t ret;
-   uint num_banks = 1;
+   u8 num_banks = 1;
+   u16 __maybe_unused num_images;
uint update_bank = 0;
efi_uintn_t size;
efi_guid_t *image_type_id;
@@ -398,7 +402,9 @@ efi_status_t efi_firmware_set_fmp_state_var(struct 
fmp_state *state, u8 image_in
if (ret)
return EFI_INVALID_PARAMETER;
 
-   num_banks = CONFIG_FWU_NUM_BANKS;
+   ret = fwu_get_banks_images(_banks, _images);
+   if (ret)
+   return EFI_INVALID_PARAMETER;
}
 
size = num_banks * sizeof(*var_state);
-- 
2.34.1



[PATCH v2 12/21] efi_firmware: fwu: do not read FWU metadata on sandbox

2024-02-11 Thread Sughosh Ganu
The FWU metadata is being read for populating the firmware image's
version information. The sandbox platform does not have the FWU
metadata on any of it's storage devices. Skip attempting to read the
FWU metadata on the sandbox platform.

Signed-off-by: Sughosh Ganu 
---

Changes since V1: None

 lib/efi_loader/efi_firmware.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index ba5aba098c..c742e23268 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -223,7 +223,8 @@ void efi_firmware_fill_version_info(struct 
efi_firmware_image_descriptor *image_
/* get the fw_version */
efi_create_indexed_name(varname, sizeof(varname), "FmpState",
fw_array->image_index);
-   if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
+   if (!IS_ENABLED(CONFIG_SANDBOX) &&
+   IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
ret = fwu_get_active_index(_index);
if (ret)
return;
@@ -391,7 +392,8 @@ efi_status_t efi_firmware_set_fmp_state_var(struct 
fmp_state *state, u8 image_in
efi_create_indexed_name(varname, sizeof(varname), "FmpState",
image_index);
 
-   if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
+   if (!IS_ENABLED(CONFIG_SANDBOX) &&
+   IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
ret = fwu_plat_get_update_index(_bank);
if (ret)
return EFI_INVALID_PARAMETER;
-- 
2.34.1



[PATCH v2 11/21] fwu: mtd: modify the DFU API's to align with metadata version 2

2024-02-11 Thread Sughosh Ganu
Make changes to the functions used for generating the DFU's alt
variable so that they are aligned with changes to the metadata version
2.

These changes include getting the number of banks and images per bank
at runtime from the metadata, as well as accessing the updatable image
information from the FWU MTD driver's private structure.

Signed-off-by: Sughosh Ganu 
---

Changes since V1:
* Use the helper functions from the previous patch to access the
  image information in the metadata.

 lib/fwu_updates/fwu_mtd.c | 81 +--
 1 file changed, 61 insertions(+), 20 deletions(-)

diff --git a/lib/fwu_updates/fwu_mtd.c b/lib/fwu_updates/fwu_mtd.c
index 69cd3d7001..50ff0d692d 100644
--- a/lib/fwu_updates/fwu_mtd.c
+++ b/lib/fwu_updates/fwu_mtd.c
@@ -15,16 +15,51 @@
 
 #include 
 
-struct fwu_mtd_image_info
-fwu_mtd_images[CONFIG_FWU_NUM_BANKS * CONFIG_FWU_NUM_IMAGES_PER_BANK];
+static struct fwu_mdata *mdata;
+
+static int read_mdata(void)
+{
+   int ret = 0;
+   u32 mdata_size;
+
+   ret = fwu_get_mdata_size(_size);
+   if (ret)
+   return ret;
+
+   mdata = malloc(mdata_size);
+   if (!mdata)
+   return -ENOMEM;
+
+   ret = fwu_get_mdata(mdata);
+   if (ret < 0) {
+   log_err("Failed to get the FWU mdata\n");
+   free(mdata);
+   mdata = NULL;
+   }
+
+   return ret;
+}
 
 static struct fwu_mtd_image_info *mtd_img_by_uuid(const char *uuidbuf)
 {
-   int num_images = ARRAY_SIZE(fwu_mtd_images);
+   u8 nbanks;
+   u16 nimages;
+   int num_images, ret;
+   struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(fwu_get_dev());
+   struct fwu_mtd_image_info *image_info = mtd_priv->fwu_mtd_images;
+
+   if (!image_info)
+   return NULL;
+
+   ret = fwu_get_banks_images(, );
+   if (ret)
+   return NULL;
+
+   num_images = nbanks * nimages;
 
for (int i = 0; i < num_images; i++)
-   if (!strcmp(uuidbuf, fwu_mtd_images[i].uuidbuf))
-   return _mtd_images[i];
+   if (!strcmp(uuidbuf, image_info[i].uuidbuf))
+   return _info[i];
 
return NULL;
 }
@@ -107,8 +142,9 @@ __weak int fwu_plat_get_alt_num(struct udevice *dev, 
efi_guid_t *image_id,
return fwu_mtd_get_alt_num(image_id, alt_num, "nor1");
 }
 
-static int gen_image_alt_info(char *buf, size_t len, int sidx,
- struct fwu_image_entry *img, struct mtd_info *mtd)
+static int gen_image_alt_info(char *buf, size_t len, int idx,
+ struct fwu_image_entry *img,
+ struct mtd_info *mtd, uint8_t num_banks)
 {
char *p = buf, *end = buf + len;
int i;
@@ -123,15 +159,15 @@ static int gen_image_alt_info(char *buf, size_t len, int 
sidx,
 * List the image banks in the FWU mdata and search the corresponding
 * partition based on partition's uuid.
 */
-   for (i = 0; i < CONFIG_FWU_NUM_BANKS; i++) {
+   for (i = 0; i < num_banks; i++) {
struct fwu_mtd_image_info *mtd_img_info;
struct fwu_image_bank_info *bank;
char uuidbuf[UUID_STR_LEN + 1];
u32 offset, size;
 
/* Query a partition by image UUID */
-   bank = >img_bank_info[i];
-   uuid_bin_to_str(bank->image_uuid.b, uuidbuf, 
UUID_STR_FORMAT_STD);
+   bank = fwu_img_bank_info_offset(mdata, idx, i);
+   uuid_bin_to_str(bank->image_guid.b, uuidbuf, 
UUID_STR_FORMAT_STD);
 
mtd_img_info = mtd_img_by_uuid(uuidbuf);
if (!mtd_img_info) {
@@ -150,7 +186,7 @@ static int gen_image_alt_info(char *buf, size_t len, int 
sidx,
}
}
 
-   if (i == CONFIG_FWU_NUM_BANKS)
+   if (i == num_banks)
return 0;
 
return -ENOENT;
@@ -158,24 +194,29 @@ static int gen_image_alt_info(char *buf, size_t len, int 
sidx,
 
 int fwu_gen_alt_info_from_mtd(char *buf, size_t len, struct mtd_info *mtd)
 {
-   struct fwu_mdata mdata;
+   u8 num_banks;
+   u16 num_images;
int i, l, ret;
+   struct fwu_image_entry *img_entry;
 
-   ret = fwu_get_mdata();
-   if (ret < 0) {
-   log_err("Failed to get the FWU mdata.\n");
-   return ret;
+   if (!mdata) {
+   ret = read_mdata();
+   if (ret)
+   return ret;
}
 
-   for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {
-   ret = gen_image_alt_info(buf, len, i * CONFIG_FWU_NUM_BANKS,
-_entry[i], mtd);
+   num_banks = fwu_get_fw_desc(mdata)->num_banks;
+   num_images = fwu_get_fw_desc(mdata)->num_images;
+   for (i = 0; i < num_images; i++) {
+   img_entry = fwu_img_entry_offset(mdata, i);
+   ret = 

[PATCH v2 10/21] capsule: accept a bank on a successful update

2024-02-11 Thread Sughosh Ganu
The version 2 of the FWU metadata maintains a bank_state field per
bank, which keeps an aggregate status of the bank. A bank can either
be in a valid, invalid, or accepted state.

Update the bank_state field of the metadata once the update has gone
through successfully(when skipping Trial State), or once the images in
the bank have been accepted.

Signed-off-by: Sughosh Ganu 
---

Changes since V1: None

 lib/efi_loader/efi_capsule.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 0e6a38b441..422bb11162 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -480,6 +480,12 @@ static __maybe_unused efi_status_t 
fwu_empty_capsule_process(
if (ret != EFI_SUCCESS)
log_err("Unable to set the Accept bit for the image 
%pUs\n",
image_guid);
+
+   status = fwu_bank_state_update(active_idx);
+   ret = fwu_to_efi_error(status);
+   if (ret != EFI_SUCCESS)
+   log_err("Unable to update the bank_state for bank %u\n",
+   active_idx);
}
 
return ret;
@@ -525,6 +531,10 @@ static __maybe_unused efi_status_t 
fwu_post_update_process(bool fw_accept_os)
status = fwu_trial_state_start(update_index);
if (status < 0)
ret = EFI_DEVICE_ERROR;
+   } else {
+   status = fwu_bank_state_update(update_index);
+   if (status < 0)
+   ret = EFI_DEVICE_ERROR;
}
}
 
-- 
2.34.1



[PATCH v2 09/21] fwu: add a function to put a bank in Trial State

2024-02-11 Thread Sughosh Ganu
The version 2 of the FWU metadata has a field in the top level
structure, called bank_state. This is used to keep the state a given
bank is in, either of valid(for trial state), invalid, or accepted.

Update this field when putting the platform in Trial State, in
addition to starting the TrialStateCtr variable by calling the
fwu_trial_state_start() function.

Signed-off-by: Sughosh Ganu 
---

Changes since V1: None

 include/fwu.h| 12 +---
 lib/efi_loader/efi_capsule.c |  2 +-
 lib/fwu_updates/fwu.c| 54 ++--
 3 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/include/fwu.h b/include/fwu.h
index 109ceb2610..1c6a5fcda9 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -342,15 +342,19 @@ u8 fwu_update_checks_pass(void);
 u8 fwu_empty_capsule_checks_pass(void);
 
 /**
- * fwu_trial_state_ctr_start() - Start the Trial State counter
+ * fwu_trial_state_start() - Put the platform in Trial State
+ * @update_index: Bank number to which images have been updated
  *
- * Start the counter to identify the platform booting in the
- * Trial State. The counter is implemented as an EFI variable.
+ * Put the platform in Trial State by starting the counter to
+ * identify the platform booting in the Trial State. The
+ * counter is implemented as an EFI variable. Secondly, set
+ * the bank_state in the metadata for the updated bank to Valid
+ * state.
  *
  * Return: 0 if OK, -ve on error
  *
  */
-int fwu_trial_state_ctr_start(void);
+int fwu_trial_state_start(uint update_index);
 
 /**
  * fwu_gen_alt_info_from_mtd() - Parse dfu_alt_info from metadata in mtd
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index de0d49ebeb..0e6a38b441 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -522,7 +522,7 @@ static __maybe_unused efi_status_t 
fwu_post_update_process(bool fw_accept_os)
} else {
log_debug("Successfully updated the active_index\n");
if (fw_accept_os) {
-   status = fwu_trial_state_ctr_start();
+   status = fwu_trial_state_start(update_index);
if (status < 0)
ret = EFI_DEVICE_ERROR;
}
diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
index 587ca779d3..a58c42bee2 100644
--- a/lib/fwu_updates/fwu.c
+++ b/lib/fwu_updates/fwu.c
@@ -672,6 +672,36 @@ out:
return ret;
 }
 
+static int fwu_trial_state_ctr_start(void)
+{
+   int ret;
+   u16 trial_state_ctr;
+
+   printf("%s: starting the TrialStateCtr\n", __func__);
+   trial_state_ctr = 0;
+   ret = trial_counter_update(_state_ctr);
+   if (ret)
+   log_err("Unable to initialise TrialStateCtr\n");
+
+   return ret;
+}
+
+static int fwu_set_bank_state_trial(uint update_index)
+{
+   int ret;
+   struct fwu_mdata *mdata = g_mdata;
+
+   mdata->bank_state[update_index] = FWU_BANK_VALID;
+
+   ret = fwu_sync_mdata(mdata, BOTH_PARTS);
+   if (ret) {
+   log_err("Unable to set bank_state for %d bank\n", update_index);
+   return ret;
+   }
+
+   return 0;
+}
+
 /**
  * fwu_bank_state_update() - Check and update the bank_state of the metadata
  * @update_index: Bank for which the bank_state needs to be updated
@@ -830,25 +860,31 @@ u8 fwu_empty_capsule_checks_pass(void)
 }
 
 /**
- * fwu_trial_state_ctr_start() - Start the Trial State counter
+ * fwu_trial_state_start() - Put the platform in Trial State
+ * @update_index: Bank number to which images have been updated
  *
- * Start the counter to identify the platform booting in the
- * Trial State. The counter is implemented as an EFI variable.
+ * Put the platform in Trial State by starting the counter to
+ * identify the platform booting in the Trial State. The
+ * counter is implemented as an EFI variable. Secondly, set
+ * the bank_state in the metadata for the updated bank to Valid
+ * state.
  *
  * Return: 0 if OK, -ve on error
  *
  */
-int fwu_trial_state_ctr_start(void)
+int fwu_trial_state_start(uint update_index)
 {
int ret;
-   u16 trial_state_ctr;
 
-   trial_state_ctr = 0;
-   ret = trial_counter_update(_state_ctr);
+   ret = fwu_trial_state_ctr_start();
if (ret)
-   log_err("Unable to initialise TrialStateCtr\n");
+   return ret;
 
-   return ret;
+   ret = fwu_set_bank_state_trial(update_index);
+   if (ret)
+   return ret;
+
+   return 0;
 }
 
 static int fwu_boottime_checks(void)
-- 
2.34.1



[PATCH v2 08/21] drivers: fwu: allocate memory for metadata copies

2024-02-11 Thread Sughosh Ganu
With migration of the FWU metadata access code to version 2, the size
of the metadata is obtained at runtime. Allocate memory for both the
metadata copies from the driver's probe function.

Signed-off-by: Sughosh Ganu 
---

Changes since V1: None

 drivers/fwu-mdata/gpt_blk.c | 4 
 drivers/fwu-mdata/raw_mtd.c | 4 
 2 files changed, 8 insertions(+)

diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c
index 97eac3611f..c2cb7ef7c3 100644
--- a/drivers/fwu-mdata/gpt_blk.c
+++ b/drivers/fwu-mdata/gpt_blk.c
@@ -159,6 +159,10 @@ static int fwu_mdata_gpt_blk_probe(struct udevice *dev)
 
priv->blk_dev = mdata_dev;
 
+   ret = fwu_mdata_copies_allocate();
+   if (ret)
+   return ret;
+
return 0;
 }
 
diff --git a/drivers/fwu-mdata/raw_mtd.c b/drivers/fwu-mdata/raw_mtd.c
index da36094644..d91518bf0a 100644
--- a/drivers/fwu-mdata/raw_mtd.c
+++ b/drivers/fwu-mdata/raw_mtd.c
@@ -260,6 +260,10 @@ static int fwu_mdata_mtd_probe(struct udevice *dev)
if (ret)
return ret;
 
+   ret = fwu_mdata_copies_allocate();
+   if (ret)
+   return ret;
+
/* Read the metadata to get number of banks and images */
ret = fwu_get_banks_images(, );
if (ret)
-- 
2.34.1



[PATCH v2 07/21] drivers: fwu: mtd: allocate buffer for image info dynamically

2024-02-11 Thread Sughosh Ganu
The FWU metadata access driver for MTD partitioned devices currently
uses a statically allocated array for storing the updatable image
information. This array depends on the number of banks and images per
bank. With migration of the FWU metadata to version 2, these
parameters are now obtained at runtime from the metadata.

Make changes to the FWU metadata access driver for MTD devices to
allocate memory for the image information dynamically in the driver's
probe function, after having obtained the number of banks and images
per bank by reading the metadata. Move the image information as part
of the driver's private structure, instead of using a global variable.

Signed-off-by: Sughosh Ganu 
---

Changes since V1: None

 drivers/fwu-mdata/raw_mtd.c | 71 -
 include/fwu.h   |  9 +
 2 files changed, 55 insertions(+), 25 deletions(-)

diff --git a/drivers/fwu-mdata/raw_mtd.c b/drivers/fwu-mdata/raw_mtd.c
index 9f3f1dc213..da36094644 100644
--- a/drivers/fwu-mdata/raw_mtd.c
+++ b/drivers/fwu-mdata/raw_mtd.c
@@ -12,22 +12,11 @@
 #include 
 #include 
 
-/* Internal helper structure to move data around */
-struct fwu_mdata_mtd_priv {
-   struct mtd_info *mtd;
-   char pri_label[50];
-   char sec_label[50];
-   u32 pri_offset;
-   u32 sec_offset;
-};
-
 enum fwu_mtd_op {
FWU_MTD_READ,
FWU_MTD_WRITE,
 };
 
-extern struct fwu_mtd_image_info fwu_mtd_images[];
-
 static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size)
 {
return !do_div(size, mtd->erasesize);
@@ -134,7 +123,7 @@ static int flash_partition_offset(struct udevice *dev, 
const char *part_name, fd
return (int)size;
 }
 
-static int fwu_mdata_mtd_of_to_plat(struct udevice *dev)
+static int get_fwu_mdata_dev(struct udevice *dev)
 {
struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev);
const fdt32_t *phandle_p = NULL;
@@ -144,8 +133,6 @@ static int fwu_mdata_mtd_of_to_plat(struct udevice *dev)
fdt_addr_t offset;
int ret, size;
u32 phandle;
-   ofnode bank;
-   int off_img;
 
/* Find the FWU mdata storage device */
phandle_p = ofnode_get_property(dev_ofnode(dev),
@@ -199,8 +186,28 @@ static int fwu_mdata_mtd_of_to_plat(struct udevice *dev)
return ret;
mtd_priv->sec_offset = offset;
 
-   off_img = 0;
+   return 0;
+}
+
+static int fwu_mtd_image_info_populate(struct udevice *dev, u8 nbanks,
+  u16 nimages)
+{
+   struct fwu_mtd_image_info *mtd_images;
+   struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev);
+   struct udevice *mtd_dev = mtd_priv->mtd->dev;
+   fdt_addr_t offset;
+   ofnode bank;
+   int off_img;
+   u32 total_images;
 
+   total_images = nbanks * nimages;
+   mtd_priv->fwu_mtd_images = malloc(sizeof(struct fwu_mtd_image_info) *
+ total_images);
+   if (!mtd_priv->fwu_mtd_images)
+   return -ENOMEM;
+
+   off_img = 0;
+   mtd_images = mtd_priv->fwu_mtd_images;
ofnode_for_each_subnode(bank, dev_ofnode(dev)) {
int bank_num, bank_offset, bank_size;
const char *bank_name;
@@ -219,8 +226,7 @@ static int fwu_mdata_mtd_of_to_plat(struct udevice *dev)
int image_num, image_offset, image_size;
const char *uuid;
 
-   if (off_img == CONFIG_FWU_NUM_BANKS *
-   CONFIG_FWU_NUM_IMAGES_PER_BANK) 
{
+   if (off_img == total_images) {
log_err("DT provides more images than 
configured!\n");
break;
}
@@ -230,11 +236,11 @@ static int fwu_mdata_mtd_of_to_plat(struct udevice *dev)
ofnode_read_u32(image, "offset", _offset);
ofnode_read_u32(image, "size", _size);
 
-   fwu_mtd_images[off_img].start = bank_offset + 
image_offset;
-   fwu_mtd_images[off_img].size = image_size;
-   fwu_mtd_images[off_img].bank_num = bank_num;
-   fwu_mtd_images[off_img].image_num = image_num;
-   strcpy(fwu_mtd_images[off_img].uuidbuf, uuid);
+   mtd_images[off_img].start = bank_offset + image_offset;
+   mtd_images[off_img].size = image_size;
+   mtd_images[off_img].bank_num = bank_num;
+   mtd_images[off_img].image_num = image_num;
+   strcpy(mtd_images[off_img].uuidbuf, uuid);
log_debug("\tImage%d: %s @0x%x\n\n",
  image_num, uuid, bank_offset + image_offset);
off_img++;
@@ -246,8 +252,24 @@ static int fwu_mdata_mtd_of_to_plat(struct udevice *dev)
 
 static 

[PATCH v2 06/21] fwu: add some API's for metadata version 2 access

2024-02-11 Thread Sughosh Ganu
There are certain fields added in version 2 of the FWU metadata
structure. Also, information like number of banks and number of images
per bank are also part of the metadata structure. Add functions to
access fields of the version 2 of the metadata structure.

Signed-off-by: Sughosh Ganu 
---

Changes since V1:
* Use the helper functions from the previous patch to access the
  image information in the metadata.

 include/fwu.h |  53 
 lib/fwu_updates/fwu.c | 144 ++
 2 files changed, 197 insertions(+)

diff --git a/include/fwu.h b/include/fwu.h
index 8f2492bb7e..ce8c98921a 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -153,6 +153,26 @@ int fwu_write_mdata(struct udevice *dev, struct fwu_mdata 
*mdata,
  */
 int fwu_get_mdata(struct fwu_mdata *mdata);
 
+/**
+ * fwu_mdata_copies_allocate() - Allocate memory for metadata
+ *
+ * Allocate memory for storing both the copies of the FWU metadata. The
+ * copies are then used as a cache for storing FWU metadata contents.
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int fwu_mdata_copies_allocate(void);
+
+/**
+ * fwu_get_mdata_size() - Get the FWU metadata size
+ *
+ * Get the size of the FWU metadata from the structure. This is later used
+ * to allocate memory for the structure.
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int fwu_get_mdata_size(uint32_t *mdata_size);
+
 /**
  * fwu_get_active_index() - Get active_index from the FWU metadata
  * @active_idxp: active_index value to be read
@@ -202,6 +222,18 @@ int fwu_get_dfu_alt_num(u8 image_index, u8 *alt_num);
  */
 int fwu_revert_boot_index(void);
 
+/**
+ * fwu_bank_state_update() - Check and update the bank_state of the metadata
+ * @update_index: Bank for which the bank_state needs to be updated
+ *
+ * Check that all the images for the given bank have been accepted, and if
+ * they are, set the status of the bank to Accepted in the bank_state field
+ * of the metadata.
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int fwu_bank_state_update(uint update_index);
+
 /**
  * fwu_accept_image() - Set the Acceptance bit for the image
  * @img_type_id: GUID of the image type for which the accepted bit is to be
@@ -335,4 +367,25 @@ int fwu_gen_alt_info_from_mtd(char *buf, size_t len, 
struct mtd_info *mtd);
  */
 int fwu_mtd_get_alt_num(efi_guid_t *image_guid, u8 *alt_num, const char 
*mtd_dev);
 
+/**
+ * fwu_get_banks_images() - Get the number of banks and images from the 
metadata
+ * @nbanks: Number of banks
+ * @nimages: Number of images per bank
+ *
+ * Get the values of number of banks and number of images per bank from the
+ * metadata.
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int fwu_get_banks_images(u8 *nbanks, u16 *nimages);
+
+/**
+ * fwu_get_dev() - Return the FWU metadata device
+ *
+ * Return the pointer to the FWU metadata device.
+ *
+ * Return: Pointer to the FWU metadata dev
+ */
+__maybe_unused struct udevice *fwu_get_dev(void);
+
 #endif /* _FWU_H_ */
diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
index 2da19c9003..587ca779d3 100644
--- a/lib/fwu_updates/fwu.c
+++ b/lib/fwu_updates/fwu.c
@@ -240,6 +240,114 @@ static inline int mdata_crc_check(struct fwu_mdata *mdata)
return calc_crc32 == mdata->crc32 ? 0 : -EINVAL;
 }
 
+/**
+ * fwu_mdata_copies_allocate() - Allocate memory for metadata
+ *
+ * Allocate memory for storing both the copies of the FWU metadata. The
+ * copies are then used as a cache for storing FWU metadata contents.
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int fwu_mdata_copies_allocate(void)
+{
+   int err;
+   uint32_t mdata_size;
+
+   if (g_mdata)
+   return 0;
+
+   err = fwu_get_mdata_size(_size);
+   if (err)
+   return err;
+
+   /*
+* Now allocate the total memory that would be needed for both
+* the copies.
+*/
+   g_mdata = calloc(2, mdata_size);
+   if (!g_mdata) {
+   log_err("Unable to allocate space for FWU metadata\n");
+   return -ENOMEM;
+   }
+
+   return 0;
+}
+
+/**
+ * fwu_get_mdata_size() - Get the FWU metadata size
+ *
+ * Get the size of the FWU metadata from the structure. This is later used
+ * to allocate memory for the structure.
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int fwu_get_mdata_size(uint32_t *mdata_size)
+{
+   int err = 0;
+   struct fwu_mdata mdata = { 0 };
+
+   if (g_mdata && !mdata_crc_check(g_mdata)) {
+   *mdata_size = g_mdata->metadata_size;
+   return 0;
+   }
+
+   err = fwu_read_mdata(g_dev, , 1, sizeof(struct fwu_mdata));
+   if (err) {
+   log_err("FWU metadata read failed\n");
+   return err;
+   }
+
+   if (mdata.version != 0x2) {
+   log_err("FWU metadata version %u. Expected value of 2\n",
+   mdata.version);
+   return -EINVAL;
+   }
+
+   *mdata_size = mdata.metadata_size;
+   if 

[PATCH v2 05/21] fwu: make changes to support version 2 of FWU metadata

2024-02-11 Thread Sughosh Ganu
Make changes to the FWU library functions which are used to access the
metadata structure to support version 2 of the metadata.

At a broad level, the following are the changes made
 - Use a pointer g_mdata instead of a variable, and allocate space for
   it at runtime.
 - Obtain the number of banks and number of images per bank from the
   metadata at runtime, instead of using config values.
 - Obtain the pointers to the fwu_image_entry and fwu_image_bank_info
   structures at runtime by pointer arithmetic using the helper
   functions.
 - Get the size of the metadata from the metadata structure, instead
   of using build-time value.

Signed-off-by: Sughosh Ganu 
---

Changes since V1:
* Use the helper functions from the previous patch to access the
  image information in the metadata.

 include/fwu.h |   6 +-
 lib/fwu_updates/fwu.c | 145 +++---
 2 files changed, 98 insertions(+), 53 deletions(-)

diff --git a/include/fwu.h b/include/fwu.h
index 7de462548c..8f2492bb7e 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -53,9 +53,13 @@ struct fwu_mdata_ops {
   bool primary, uint32_t size);
 };
 
-#define FWU_MDATA_VERSION  0x1
+#define FWU_MDATA_VERSION  0x2
 #define FWU_IMAGE_ACCEPTED 0x1
 
+#define FWU_BANK_INVALID   0xFF
+#define FWU_BANK_VALID 0xFE
+#define FWU_BANK_ACCEPTED  0xFC
+
 /*
 * GUID value defined in the FWU specification for identification
 * of the FWU metadata partition.
diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
index 5f1182a764..2da19c9003 100644
--- a/lib/fwu_updates/fwu.c
+++ b/lib/fwu_updates/fwu.c
@@ -17,7 +17,7 @@
 
 #include 
 
-static struct fwu_mdata g_mdata; /* = {0} makes uninit crc32 always invalid */
+static struct fwu_mdata *g_mdata;
 static struct udevice *g_dev;
 static u8 in_trial;
 static u8 boottime_check;
@@ -108,21 +108,9 @@ out:
 
 static int in_trial_state(struct fwu_mdata *mdata)
 {
-   u32 i, active_bank;
-   struct fwu_image_entry *img_entry;
-   struct fwu_image_bank_info *img_bank_info;
-
-   active_bank = mdata->active_index;
-   img_entry = >img_entry[0];
-   for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {
-   img_bank_info = _entry[i].img_bank_info[active_bank];
-   if (!img_bank_info->accepted) {
-   log_info("System booting in Trial State\n");
-   return 1;
-   }
-   }
+   u32 active_bank = mdata->active_index;
 
-   return 0;
+   return mdata->bank_state[active_bank] == FWU_BANK_VALID ? 1 : 0;
 }
 
 static int fwu_get_image_type_id(u8 image_index, efi_guid_t *image_type_id)
@@ -208,8 +196,9 @@ struct fwu_image_bank_info *fwu_img_bank_info_offset(struct 
fwu_mdata *mdata,
  */
 static int fwu_sync_mdata(struct fwu_mdata *mdata, int part)
 {
-   void *buf = >version;
int err;
+   uint32_t mdata_size;
+   void *buf = >version;
 
if (part == BOTH_PARTS) {
err = fwu_sync_mdata(mdata, SECONDARY_PART);
@@ -223,9 +212,10 @@ static int fwu_sync_mdata(struct fwu_mdata *mdata, int 
part)
 * and put the updated value in the FWU metadata crc32
 * field
 */
-   mdata->crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32));
+   mdata_size = mdata->metadata_size;
+   mdata->crc32 = crc32(0, buf, mdata_size - sizeof(u32));
 
-   err = fwu_write_mdata(g_dev, mdata, part == PRIMARY_PART);
+   err = fwu_write_mdata(g_dev, mdata, part == PRIMARY_PART, mdata_size);
if (err) {
log_err("Unable to write %s mdata\n",
part == PRIMARY_PART ?  "primary" : "secondary");
@@ -233,7 +223,7 @@ static int fwu_sync_mdata(struct fwu_mdata *mdata, int part)
}
 
/* update the cached copy of meta-data */
-   memcpy(_mdata, mdata, sizeof(struct fwu_mdata));
+   memcpy(g_mdata, mdata, mdata_size);
 
return 0;
 }
@@ -241,8 +231,12 @@ static int fwu_sync_mdata(struct fwu_mdata *mdata, int 
part)
 static inline int mdata_crc_check(struct fwu_mdata *mdata)
 {
void *buf = >version;
-   u32 calc_crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32));
+   u32 calc_crc32;
 
+   if (!mdata->metadata_size)
+   return -EINVAL;
+
+   calc_crc32 = crc32(0, buf, mdata->metadata_size - sizeof(u32));
return calc_crc32 == mdata->crc32 ? 0 : -EINVAL;
 }
 
@@ -259,27 +253,32 @@ static inline int mdata_crc_check(struct fwu_mdata *mdata)
 int fwu_get_mdata(struct fwu_mdata *mdata)
 {
int err;
+   uint32_t mdata_size;
bool parts_ok[2] = { false };
-   struct fwu_mdata s, *parts_mdata[2];
+   struct fwu_mdata *parts_mdata[2];
+
+   err = fwu_get_mdata_size(_size);
+   if (err)
+   return err;
 
-   parts_mdata[0] = _mdata;
-   parts_mdata[1] = 
+   parts_mdata[0] = g_mdata;
+   parts_mdata[1] = (struct fwu_mdata *)((char 

[PATCH v2 04/21] fwu: add helper functions for getting image description offsets

2024-02-11 Thread Sughosh Ganu
With migration of the FWU metadata structure to version 2, the size of
the structure is not determined at build time, but at run time.

This means, that the offsets for the structures describing firmware
images which are part of the FWU metadata(struct fwu_fw_store_desc,
struct fwu_image_entry and struct fwu_image_bank_info) need to be
computed at runtime. Add helper functions to get addresses of these
structures.

Signed-off-by: Sughosh Ganu 
---

Changes since V1:
* New patch needed based on comments from Ilias on the earlier
  version.


 include/fwu.h | 45 +
 lib/fwu_updates/fwu.c | 58 +++
 2 files changed, 103 insertions(+)

diff --git a/include/fwu.h b/include/fwu.h
index 1815bd0064..7de462548c 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -8,6 +8,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -81,6 +82,50 @@ struct fwu_mdata_ops {
EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
 0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
 
+
+/**
+ * fwu_get_fw_desc() - Return pointer to firmware descriptor store struct
+ * @mdata: Pointer to the FWU metadata
+ *
+ * Returns pointer to the firmware store descriptor of the FWU metadata
+ * containing information on updatable images.
+ *
+ * Return: Pointer to the struct fwu_fw_store_desc
+ */
+static inline struct fwu_fw_store_desc *fwu_get_fw_desc(struct fwu_mdata 
*mdata)
+{
+   return (struct fwu_fw_store_desc *)((u8 *)mdata + sizeof(*mdata));
+}
+
+/**
+ * fwu_img_entry_offset() - Get pointer to struct fwu_image_entry
+ * @mdata: Pointer to the FWU metadata
+ * @idx: Image index for which pointer is to be returned
+ *
+ * Fetches pointer to am array element of type struct fwu_image_entry.
+ * This returns back a pointer to a structure which is providing
+ * information on a updatable image.
+ *
+ * Return: Pointer to an array element of type struct fwu_image_entry
+ *
+ */
+struct fwu_image_entry *fwu_img_entry_offset(struct fwu_mdata *mdata, u16 idx);
+
+/**
+ * fwu_img_bank_info_offset() - Get pointer to struct fwu_image_bank_info
+ * @mdata: Pointer to the FWU metadata
+ * @idx: Image index for which information is needed
+ * @bank: Bank for which pointer is to be returned
+ *
+ * Fetches pointer to an array element of type struct fwu_image_bank_info
+ * for a given image. This returns back a pointer to a structure which
+ * is providing information for a given bank for a particular image.
+ *
+ * Return: Pointer to an array element of type fwu_image_bank_info
+ *
+ */
+struct fwu_image_bank_info *fwu_img_bank_info_offset(struct fwu_mdata *mdata,
+u16 idx, u8 bank);
 /**
  * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata()
  */
diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
index 86518108c2..5f1182a764 100644
--- a/lib/fwu_updates/fwu.c
+++ b/lib/fwu_updates/fwu.c
@@ -141,6 +141,64 @@ static int fwu_get_image_type_id(u8 image_index, 
efi_guid_t *image_type_id)
return -ENOENT;
 }
 
+/**
+ * fwu_img_entry_offset() - Get pointer to struct fwu_image_entry
+ * @mdata: Pointer to the FWU metadata
+ * @idx: Image index for which pointer is to be returned
+ *
+ * Fetches pointer to am array element of type struct fwu_image_entry.
+ * This returns back a pointer to a structure which is providing
+ * information on a updatable image.
+ *
+ * Return: Pointer to an array element of type struct fwu_image_entry
+ *
+ */
+struct fwu_image_entry *fwu_img_entry_offset(struct fwu_mdata *mdata, u16 idx)
+{
+   u8 num_banks;
+   size_t offset;
+
+   num_banks = fwu_get_fw_desc(mdata)->num_banks;
+
+   offset = sizeof(struct fwu_mdata) +
+   sizeof(struct fwu_fw_store_desc) +
+   (sizeof(struct fwu_image_entry) +
+sizeof(struct fwu_image_bank_info) * num_banks) * idx;
+
+   return (struct fwu_image_entry *)((char *)mdata + offset);
+}
+
+/**
+ * fwu_img_bank_info_offset() - Get pointer to struct fwu_image_bank_info
+ * @mdata: Pointer to the FWU metadata
+ * @idx: Image index for which information is needed
+ * @bank: Bank for which pointer is to be returned
+ *
+ * Fetches pointer to an array element of type struct fwu_image_bank_info
+ * for a given image. This returns back a pointer to a structure which
+ * is providing information for a given bank for a particular image.
+ *
+ * Return: Pointer to an array element of type fwu_image_bank_info
+ *
+ */
+struct fwu_image_bank_info *fwu_img_bank_info_offset(struct fwu_mdata *mdata,
+u16 idx, u8 bank)
+{
+   u8 num_banks;
+   size_t offset;
+
+   num_banks = fwu_get_fw_desc(mdata)->num_banks;
+
+   offset = sizeof(struct fwu_mdata) +
+   sizeof(struct fwu_fw_store_desc) +
+   (sizeof(struct fwu_image_entry) +
+sizeof(struct fwu_image_bank_info) * num_banks) 

[PATCH v2 03/21] drivers: fwu: add the size parameter to the metadata access API's

2024-02-11 Thread Sughosh Ganu
In version 2 of the metadata structure, the size of the structure
cannot be determined statically at build time. The structure is now
broken into the top level structure which contains a field indicating
the total size of the structure.

Add a size parameter to the metadata access API functions to indicate
the number of bytes to be accessed. This is then used to either read
the entire structure, or only the top level structure.

Signed-off-by: Sughosh Ganu 
---

Changes since V1: None

 drivers/fwu-mdata/fwu-mdata-uclass.c | 10 ++
 drivers/fwu-mdata/gpt_blk.c  | 23 +--
 drivers/fwu-mdata/raw_mtd.c  | 10 ++
 include/fwu.h| 14 ++
 4 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c 
b/drivers/fwu-mdata/fwu-mdata-uclass.c
index 0a8edaaa41..145479bab0 100644
--- a/drivers/fwu-mdata/fwu-mdata-uclass.c
+++ b/drivers/fwu-mdata/fwu-mdata-uclass.c
@@ -20,7 +20,8 @@
  *
  * Return: 0 if OK, -ve on error
  */
-int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary)
+int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary,
+  uint32_t size)
 {
const struct fwu_mdata_ops *ops = device_get_ops(dev);
 
@@ -29,7 +30,7 @@ int fwu_read_mdata(struct udevice *dev, struct fwu_mdata 
*mdata, bool primary)
return -ENOSYS;
}
 
-   return ops->read_mdata(dev, mdata, primary);
+   return ops->read_mdata(dev, mdata, primary, size);
 }
 
 /**
@@ -37,7 +38,8 @@ int fwu_read_mdata(struct udevice *dev, struct fwu_mdata 
*mdata, bool primary)
  *
  * Return: 0 if OK, -ve on error
  */
-int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary)
+int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary,
+   uint32_t size)
 {
const struct fwu_mdata_ops *ops = device_get_ops(dev);
 
@@ -46,7 +48,7 @@ int fwu_write_mdata(struct udevice *dev, struct fwu_mdata 
*mdata, bool primary)
return -ENOSYS;
}
 
-   return ops->write_mdata(dev, mdata, primary);
+   return ops->write_mdata(dev, mdata, primary, size);
 }
 
 UCLASS_DRIVER(fwu_mdata) = {
diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c
index c7284916c4..97eac3611f 100644
--- a/drivers/fwu-mdata/gpt_blk.c
+++ b/drivers/fwu-mdata/gpt_blk.c
@@ -81,15 +81,14 @@ static int gpt_get_mdata_disk_part(struct blk_desc *desc,
return -ENOENT;
 }
 
-static int gpt_read_write_mdata(struct blk_desc *desc,
-   struct fwu_mdata *mdata,
-   u8 access, u32 part_num)
+static int gpt_read_write_mdata(struct blk_desc *desc, struct fwu_mdata *mdata,
+   u8 access, u32 part_num, u32 size)
 {
int ret;
u32 len, blk_start, blkcnt;
struct disk_partition info;
 
-   ALLOC_CACHE_ALIGN_BUFFER_PAD(struct fwu_mdata, mdata_aligned, 1,
+   ALLOC_CACHE_ALIGN_BUFFER_PAD(u8, mdata_aligned, size,
 desc->blksz);
 
if (!mdata)
@@ -101,7 +100,7 @@ static int gpt_read_write_mdata(struct blk_desc *desc,
return -ENOENT;
}
 
-   len = sizeof(*mdata);
+   len = size;
blkcnt = BLOCK_CNT(len, desc);
if (blkcnt > info.size) {
log_debug("Block count exceeds FWU metadata partition size\n");
@@ -114,7 +113,7 @@ static int gpt_read_write_mdata(struct blk_desc *desc,
log_debug("Error reading FWU metadata from the 
device\n");
return -EIO;
}
-   memcpy(mdata, mdata_aligned, sizeof(struct fwu_mdata));
+   memcpy(mdata, mdata_aligned, size);
} else {
if (blk_dwrite(desc, blk_start, blkcnt, mdata) != blkcnt) {
log_debug("Error writing FWU metadata to the device\n");
@@ -164,7 +163,7 @@ static int fwu_mdata_gpt_blk_probe(struct udevice *dev)
 }
 
 static int fwu_gpt_read_mdata(struct udevice *dev, struct fwu_mdata *mdata,
- bool primary)
+ bool primary, u32 size)
 {
struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
struct blk_desc *desc = dev_get_uclass_plat(priv->blk_dev);
@@ -177,11 +176,13 @@ static int fwu_gpt_read_mdata(struct udevice *dev, struct 
fwu_mdata *mdata,
}
 
return gpt_read_write_mdata(desc, mdata, MDATA_READ,
-   primary ? g_mdata_part[0] : 
g_mdata_part[1]);
+   primary ?
+   g_mdata_part[0] : g_mdata_part[1],
+   size);
 }
 
 static int fwu_gpt_write_mdata(struct udevice *dev, struct fwu_mdata *mdata,
-  bool primary)
+  bool primary, u32 

[PATCH v2 02/21] fwu: metadata: migrate to version 2 of the structure

2024-02-11 Thread Sughosh Ganu
The latest version of the FWU specification [1] has changes to the
metadata structure. This is version 2 of the structure.

Primary changes include
 - bank_state field in the top level structure
 - Total metadata size in the top level structure
 - Image description structures now optional
 - Number of banks and images per bank values part of the structure

Migrate to the version 2 of the metadata structure.

[1] - https://developer.arm.com/documentation/den0118/latest/

Signed-off-by: Sughosh Ganu 
---

Changes since V1:
* Do not define flexible array members inside the structures.

 include/fwu_mdata.h | 56 +
 1 file changed, 42 insertions(+), 14 deletions(-)

diff --git a/include/fwu_mdata.h b/include/fwu_mdata.h
index 56189e2f40..9008bbc93f 100644
--- a/include/fwu_mdata.h
+++ b/include/fwu_mdata.h
@@ -11,7 +11,7 @@
 
 /**
  * struct fwu_image_bank_info - firmware image information
- * @image_uuid: Guid value of the image in this bank
+ * @image_guid: Guid value of the image in this bank
  * @accepted: Acceptance status of the image
  * @reserved: Reserved
  *
@@ -20,15 +20,15 @@
  * acceptance status
  */
 struct fwu_image_bank_info {
-   efi_guid_t  image_uuid;
+   efi_guid_t  image_guid;
uint32_t accepted;
uint32_t reserved;
 } __packed;
 
 /**
  * struct fwu_image_entry - information for a particular type of image
- * @image_type_uuid: Guid value for identifying the image type
- * @location_uuid: Guid of the storage volume where the image is located
+ * @image_type_guid: Guid value for identifying the image type
+ * @location_guid: Guid of the storage volume where the image is located
  * @img_bank_info: Array containing properties of images
  *
  * This structure contains information on various types of updatable
@@ -36,9 +36,30 @@ struct fwu_image_bank_info {
  * information per bank.
  */
 struct fwu_image_entry {
-   efi_guid_t image_type_uuid;
-   efi_guid_t location_uuid;
-   struct fwu_image_bank_info img_bank_info[CONFIG_FWU_NUM_BANKS];
+   efi_guid_t image_type_guid;
+   efi_guid_t location_guid;
+   // struct fwu_image_bank_info img_bank_info[];
+} __packed;
+
+/**
+ * struct fwu_fw_desc_store - FWU updatable image information
+ * @num_banks: Number of firmware banks
+ * num_images: Number of images per bank
+ * @img_entry_size: The size of the img_entry array
+ * @bank_info_entry_size: The size of the img_bank_info array
+ * @img_entry: Array of image entries each giving information on a image
+ *
+ * This image descriptor structure contains information on the number of
+ * updatable banks and images per bank. It also gives the total sizes of
+ * the fwu_image_entry and fwu_image_bank_info arrays.
+ */
+struct fwu_fw_store_desc {
+   uint8_t  num_banks;
+   uint8_t  reserved;
+   uint16_t num_images;
+   uint16_t img_entry_size;
+   uint16_t bank_info_entry_size;
+   // struct fwu_image_entry img_entry[];
 } __packed;
 
 /**
@@ -48,21 +69,28 @@ struct fwu_image_entry {
  * @active_index: Index of the bank currently used for booting images
  * @previous_active_inde: Index of the bank used before the current bank
  *being used for booting
- * @img_entry: Array of information on various firmware images that can
- * be updated
+ * @metadata_size: Size of the entire metadata structure, including the
+ * image descriptors
+ * @desc_offset: The offset from the start of this structure where the
+ *   image descriptor structure starts. 0 if absent
+ * @bank_state: State of each bank, valid, invalid or accepted
+ * @fw_desc: The structure describing the FWU updatable images
  *
- * This structure is used to store all the needed information for performing
+ * This is the top level structure used to store all information for performing
  * multi bank updates on the platform. This contains info on the bank being
- * used to boot along with the information needed for identification of
- * individual images
+ * used to boot along with the information on state of individual banks.
  */
 struct fwu_mdata {
uint32_t crc32;
uint32_t version;
uint32_t active_index;
uint32_t previous_active_index;
-
-   struct fwu_image_entry img_entry[CONFIG_FWU_NUM_IMAGES_PER_BANK];
+   uint32_t metadata_size;
+   uint16_t desc_offset;
+   uint16_t reserved1;
+   uint8_t  bank_state[4];
+   uint32_t reserved2;
+   // struct fwu_fw_store_desc fw_desc[];
 } __packed;
 
 #endif /* _FWU_MDATA_H_ */
-- 
2.34.1



[PATCH v2 01/21] configs: fwu: remove FWU configs for metadata V2 migration

2024-02-11 Thread Sughosh Ganu
The FWU metadata is to be migrated to version 2. Disable the FWU
feature on platforms that enable it for the migration.

Signed-off-by: Sughosh Ganu 
Reviewed-by: Ilias Apalodimas 
---

Changes since V1: None

 configs/corstone1000_defconfig   | 2 --
 configs/sandbox64_defconfig  | 1 -
 configs/synquacer_developerbox_defconfig | 4 
 3 files changed, 7 deletions(-)

diff --git a/configs/corstone1000_defconfig b/configs/corstone1000_defconfig
index a86ac12732..c26a621175 100644
--- a/configs/corstone1000_defconfig
+++ b/configs/corstone1000_defconfig
@@ -25,7 +25,6 @@ CONFIG_BOARD_LATE_INIT=y
 CONFIG_SYS_PROMPT="corstone1000# "
 CONFIG_SYS_MAXARGS=64
 # CONFIG_CMD_CONSOLE is not set
-CONFIG_CMD_FWU_METADATA=y
 CONFIG_CMD_BOOTZ=y
 # CONFIG_CMD_XIMG is not set
 CONFIG_CMD_GPT=y
@@ -68,4 +67,3 @@ CONFIG_FFA_SHARED_MM_BUF_OFFSET=0
 CONFIG_FFA_SHARED_MM_BUF_ADDR=0x0200
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_IGNORE_OSINDICATIONS=y
-CONFIG_FWU_MULTI_BANK_UPDATE=y
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index d101cca6a7..2863f9b189 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -273,7 +273,6 @@ CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
-CONFIG_FWU_MULTI_BANK_UPDATE=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
diff --git a/configs/synquacer_developerbox_defconfig 
b/configs/synquacer_developerbox_defconfig
index 2a0407de40..616d410074 100644
--- a/configs/synquacer_developerbox_defconfig
+++ b/configs/synquacer_developerbox_defconfig
@@ -11,14 +11,12 @@ CONFIG_DM_GPIO=y
 CONFIG_DEFAULT_DEVICE_TREE="synquacer-sc2a11-developerbox"
 CONFIG_SYS_LOAD_ADDR=0x8000
 CONFIG_TARGET_DEVELOPERBOX=y
-CONFIG_FWU_NUM_IMAGES_PER_BANK=1
 CONFIG_AHCI=y
 CONFIG_FIT=y
 CONFIG_SYS_BOOTM_LEN=0x80
 CONFIG_BOOTSTAGE_STASH_SIZE=4096
 CONFIG_HUSH_PARSER=y
 CONFIG_SYS_MAXARGS=128
-CONFIG_CMD_FWU_METADATA=y
 CONFIG_CMD_IMLS=y
 CONFIG_CMD_ERASEENV=y
 CONFIG_CMD_NVEDIT_EFI=y
@@ -53,7 +51,6 @@ CONFIG_DFU_TFTP=y
 CONFIG_DFU_MTD=y
 CONFIG_DFU_RAM=y
 CONFIG_DFU_SF=y
-CONFIG_FWU_MDATA_MTD=y
 CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_SYNQUACER=y
 CONFIG_MMC_SDHCI=y
@@ -96,4 +93,3 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_IGNORE_OSINDICATIONS=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
-CONFIG_FWU_MULTI_BANK_UPDATE=y
-- 
2.34.1



[PATCH v2 00/21] FWU: Migrate FWU metadata to version 2

2024-02-11 Thread Sughosh Ganu


The following patches migrate the FWU metadata access code to version
2 of the structure. This is based on the structure definition as
defined in the latest rev of the FWU Multi Bank Update specification
[1].

Since the version 1 of the structure has currently been adopted on a
couple of platforms, it was decided to have a clean migration of the
metadata to version 2 only, instead of supporting both the versions of
the structure. Also, based on consultations with the main author of
the specification, it is expected that any further changes in the
structure would be minor tweaks, and not be significant. Hence a
migration to version 2.

Similar migration is also being done in TF-A, including migrating the
ST platform port to support version 2 of the metadata structure [2].

@Michal, I tested the metadata for the two image per bank case, and it
works fine on the ST board. Kindly test this on your board as well.

@Kojima-san, Please help in testing the version 2 on your
board. Thanks.


[1] - https://developer.arm.com/documentation/den0118/latest/
[2] - 
https://review.trustedfirmware.org/q/topic:%22topics/fwu_metadata_v2_migration%22

Changes since V1:

* Do not define flexible array members inside the structures.
* Access the image information related fields in the metadata using
  the helper functions defined in an earlier patch.
* Access fwu_fw_store_desc structure using pointer arithmetic.


Sughosh Ganu (21):
  configs: fwu: remove FWU configs for metadata V2 migration
  fwu: metadata: migrate to version 2 of the structure
  drivers: fwu: add the size parameter to the metadata access API's
  fwu: add helper functions for getting image description offsets
  fwu: make changes to support version 2 of FWU metadata
  fwu: add some API's for metadata version 2 access
  drivers: fwu: mtd: allocate buffer for image info dynamically
  drivers: fwu: allocate memory for metadata copies
  fwu: add a function to put a bank in Trial State
  capsule: accept a bank on a successful update
  fwu: mtd: modify the DFU API's to align with metadata version 2
  efi_firmware: fwu: do not read FWU metadata on sandbox
  efi_firmware: fwu: get the number of FWU banks at runtime
  cmd: fwu: align the command with metadata version 2
  test: fwu: align the FWU metadata access test with version 2
  fwu: remove the config symbols for number of banks and images
  tools: mkfwumdata: migrate to metadata version 2
  tools: mkfwumdata: add logic to append vendor data to the FWU metadata
  tools: mkfwumdata: fix the size parameter to the fwrite call
  configs: fwu: re-enable FWU configs
  doc: fwu: make changes for supporting FWU Metadata version 2

 arch/sandbox/Kconfig |   6 -
 board/armltd/corstone1000/corstone1000.c |   2 +-
 cmd/fwu_mdata.c  |  45 ++-
 configs/synquacer_developerbox_defconfig |   1 -
 doc/board/socionext/developerbox.rst |   9 +-
 doc/develop/uefi/fwu_updates.rst |  12 +-
 doc/usage/cmd/fwu_mdata.rst  |  12 +-
 drivers/fwu-mdata/fwu-mdata-uclass.c |  10 +-
 drivers/fwu-mdata/gpt_blk.c  |  27 +-
 drivers/fwu-mdata/raw_mtd.c  |  85 +++--
 include/fwu.h| 139 +++-
 include/fwu_mdata.h  |  56 +++-
 lib/efi_loader/efi_capsule.c |  12 +-
 lib/efi_loader/efi_firmware.c|  20 +-
 lib/fwu_updates/Kconfig  |  11 -
 lib/fwu_updates/fwu.c| 401 +++
 lib/fwu_updates/fwu_mtd.c|  81 +++--
 test/dm/fwu_mdata.c  |  56 ++--
 test/dm/fwu_mdata_disk_image.h   | 124 +++
 tools/mkfwumdata.c   | 132 ++--
 20 files changed, 924 insertions(+), 317 deletions(-)

-- 
2.34.1




Re: [PATCH v2 1/7] common: avb_verify: don't call mmc_switch_part for SD

2024-02-11 Thread Dan Carpenter
On Fri, Feb 09, 2024 at 08:20:39PM +0100, Igor Opaniuk wrote:
> From: Igor Opaniuk 
> 
> mmc_switch_part() is used for switching between hw partitions
> on eMMC (boot0, boot1, user, rpmb).
> There is no need to do that for SD card.
> 

Is this a clean up or a bugfix?  What are the visible effects for the
user?

regards,
dan carpenter



Re: [PATCH] button: qcom-pmic: demote "unknown button" message to debug

2024-02-11 Thread Sumit Garg
On Fri, 9 Feb 2024 at 20:44, Caleb Connolly  wrote:
>
> This message isn't an error (there can be a watchdog subnode for example)
> but it shouldn't be printed unless this driver is being debugged. Demote
> it to a debug print.
>
> Signed-off-by: Caleb Connolly 
> ---
>  drivers/button/button-qcom-pmic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Sumit Garg 

-Sumit

> diff --git a/drivers/button/button-qcom-pmic.c 
> b/drivers/button/button-qcom-pmic.c
> index 34a976d1e6c6..e6702139ca2d 100644
> --- a/drivers/button/button-qcom-pmic.c
> +++ b/drivers/button/button-qcom-pmic.c
> @@ -133,7 +133,7 @@ static int button_qcom_pmic_bind(struct udevice *parent)
> } else if (NODE_IS_RESIN(node)) {
> uc_plat->label = "vol_down";
> } else {
> -   printf("Unknown button node '%s' should be 'pwrkey' 
> or 'resin'\n",
> +   debug("Unknown button node '%s' should be 'pwrkey' or 
> 'resin'\n",
>ofnode_get_name(node));
> device_unbind(dev);
> }
> --
> 2.43.0
>


[PATCH] mtd: spi-nor: Add NO_CHIP_ERASE flag

2024-02-11 Thread Tejas Bhumkar
Since the opcode SPINOR_OP_CHIP_ERASE (0xc7) is not supported
for the mt35xu02g flash, the NO_CHIP_ERASE flag has been added
to enable sector erase functionality instead.

Signed-off-by: Tejas Bhumkar 
---
 drivers/mtd/spi/spi-nor-ids.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index 38a287487e..bfbdf7570b 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -329,7 +329,7 @@ const struct flash_info spi_nor_ids[] = {
{ INFO("mt35xu512aba", 0x2c5b1a, 0,  128 * 1024,  512, USE_FSR | 
SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES | SPI_NOR_OCTAL_DTR_READ) },
 #endif /* CONFIG_SPI_FLASH_MT35XU */
{ INFO6("mt35xu01g",  0x2c5b1b, 0x104100, 128 * 1024,  1024, USE_FSR | 
SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },
-   { INFO("mt35xu02g",  0x2c5b1c, 0, 128 * 1024,  2048, USE_FSR | 
SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mt35xu02g",  0x2c5b1c, 0, 128 * 1024,  2048, USE_FSR | 
SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES | NO_CHIP_ERASE) },
 #endif
 #ifdef CONFIG_SPI_FLASH_SPANSION   /* SPANSION */
/* Spansion/Cypress -- single (large) sector size only, at least
-- 
2.27.0



[PATCH 2/2] rockchip: rk3399: Update stack and bss addresses

2024-02-11 Thread Jonas Karlman
With the stack and text base used by U-Boot SPL and proper on RK3399
there is a high likelihood of overlapping when U-Boot proper + FDT nears
1 MiB in size.

Currently the following memory layout is used:
- 0x (@0 MiB): U-Boot SPL text base
- 0x0004 (@256 KiB): TF-A
- 0x0020 (@2 MiB): U-Boot proper text base
- 0x0030 (@3 MiB): U-Boot proper init stack
- 0x0040 (@4 MiB): U-Boot SPL init stack
- 0x0040 (@4 MiB): U-Boot SPL bss
- 0x0200 (@64 MiB): U-Boot SPL reloc stack
- 0x0840 (@132 MiB): optional OPTEE

SPL load TF-A, U-Boot proper and optional OPTEE after SPL stack has
relocated, meaning that there is room for close to 2 MiB (@2-4 MiB).
However, the initial stack used by U-Boot proper is limiting the size
of U-Boot proper + FDT to below 1 MiB (@2-3 MiB).

Instead change to use the following memory layout:
- 0x (@0 MiB): U-Boot SPL text base
- 0x0004 (@256 KiB): TF-A
- 0x0020 (@2 MiB): U-Boot proper text base
- 0x0040 (@4 MiB): U-Boot SPL init stack
- 0x0080 (@8 MiB): U-Boot proper init stack (changed)
- 0x0200 (@32 MiB): U-Boot SPL bss (changed)
- 0x0400 (@64 MiB): U-Boot SPL reloc stack
- 0x0840 (@132 MiB): optional OPTEE

This should leave room for loading and running a U-Boot proper close
to 6 MiB (@2-8 MiB) in size. When U-Boot proper has relocated is should
be possible to use @2-132 MiB and @164 MiB+ for scripts, kernel etc.

With the moved SPL reloc stack it is also possible to use a much larger
malloc area in SPL, e.g. using default SPL_STACK_R_MALLOC_SIMPLE_LEN.

Signed-off-by: Jonas Karlman 
---
 configs/chromebook_bob_defconfig | 2 +-
 configs/chromebook_kevin_defconfig   | 2 +-
 configs/eaidk-610-rk3399_defconfig   | 4 ++--
 configs/evb-rk3399_defconfig | 4 ++--
 configs/ficus-rk3399_defconfig   | 2 +-
 configs/firefly-rk3399_defconfig | 4 ++--
 configs/khadas-edge-captain-rk3399_defconfig | 4 ++--
 configs/khadas-edge-rk3399_defconfig | 4 ++--
 configs/khadas-edge-v-rk3399_defconfig   | 4 ++--
 configs/leez-rk3399_defconfig| 4 ++--
 configs/nanopc-t4-rk3399_defconfig   | 4 ++--
 configs/nanopi-m4-2gb-rk3399_defconfig   | 4 ++--
 configs/nanopi-m4-rk3399_defconfig   | 4 ++--
 configs/nanopi-m4b-rk3399_defconfig  | 4 ++--
 configs/nanopi-neo4-rk3399_defconfig | 4 ++--
 configs/nanopi-r4s-rk3399_defconfig  | 4 ++--
 configs/orangepi-rk3399_defconfig| 4 ++--
 configs/pinebook-pro-rk3399_defconfig| 4 ++--
 configs/pinephone-pro-rk3399_defconfig   | 4 ++--
 configs/puma-rk3399_defconfig| 2 +-
 configs/roc-pc-mezzanine-rk3399_defconfig| 4 ++--
 configs/roc-pc-rk3399_defconfig  | 4 ++--
 configs/rock-4c-plus-rk3399_defconfig| 4 ++--
 configs/rock-4se-rk3399_defconfig| 4 ++--
 configs/rock-pi-4-rk3399_defconfig   | 4 ++--
 configs/rock-pi-4c-rk3399_defconfig  | 4 ++--
 configs/rock-pi-n10-rk3399pro_defconfig  | 4 ++--
 configs/rock960-rk3399_defconfig | 4 ++--
 configs/rockpro64-rk3399_defconfig   | 4 ++--
 29 files changed, 54 insertions(+), 54 deletions(-)

diff --git a/configs/chromebook_bob_defconfig b/configs/chromebook_bob_defconfig
index bffa0d33b75c..1fffc8b9e273 100644
--- a/configs/chromebook_bob_defconfig
+++ b/configs/chromebook_bob_defconfig
@@ -6,7 +6,7 @@ CONFIG_TEXT_BASE=0x0020
 CONFIG_SPL_GPIO=y
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
-CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x30
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80
 CONFIG_SF_DEFAULT_SPEED=2000
 CONFIG_ENV_OFFSET=0x3F8000
 CONFIG_DEFAULT_DEVICE_TREE="rk3399-gru-bob"
diff --git a/configs/chromebook_kevin_defconfig 
b/configs/chromebook_kevin_defconfig
index e14fc358c504..c9ffd3d60dd3 100644
--- a/configs/chromebook_kevin_defconfig
+++ b/configs/chromebook_kevin_defconfig
@@ -6,7 +6,7 @@ CONFIG_TEXT_BASE=0x0020
 CONFIG_SPL_GPIO=y
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
-CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x30
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80
 CONFIG_SF_DEFAULT_SPEED=2000
 CONFIG_ENV_OFFSET=0x3F8000
 CONFIG_DEFAULT_DEVICE_TREE="rk3399-gru-kevin"
diff --git a/configs/eaidk-610-rk3399_defconfig 
b/configs/eaidk-610-rk3399_defconfig
index 22ad98b95ad3..d6ba6f74b5b5 100644
--- a/configs/eaidk-610-rk3399_defconfig
+++ b/configs/eaidk-610-rk3399_defconfig
@@ -5,7 +5,7 @@ CONFIG_ARCH_ROCKCHIP=y
 CONFIG_TEXT_BASE=0x0020
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
-CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x30
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80
 CONFIG_ENV_OFFSET=0x3F8000
 CONFIG_DEFAULT_DEVICE_TREE="rk3399-eaidk-610"
 CONFIG_ROCKCHIP_RK3399=y
@@ -20,7 +20,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_SPL_MAX_SIZE=0x2e000
 CONFIG_SPL_PAD_TO=0x7f8000
 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
-CONFIG_SPL_BSS_START_ADDR=0x40

[PATCH 1/2] rockchip: rk3328: Update stack addresses

2024-02-11 Thread Jonas Karlman
With the stack and text base used by U-Boot SPL and proper on RK3328
there is a high likelihood of overlapping when U-Boot proper + FDT nears
1 MiB in size.

Currently the following memory layout is used:
- 0x (@0 MiB): U-Boot SPL text base
- 0x0004 (@256 KiB): TF-A
- 0x0020 (@2 MiB): U-Boot proper text base
- 0x0030 (@3 MiB): U-Boot proper init stack
- 0x0040 (@4 MiB): U-Boot SPL init stack
- 0x0060 (@6 MiB): U-Boot SPL reloc stack
- 0x0200 (@32 MiB): U-Boot SPL bss
- 0x0840 (@132 MiB): optional OPTEE

SPL load TF-A, U-Boot proper and optional OPTEE after SPL stack has
relocated, meaning that there is room for close to 4 MiB (@2-6 MiB).
However, the initial stack used by U-Boot proper is limiting the size
of U-Boot proper + FDT to below 1 MiB (@2-3 MiB).

Instead change to use the following memory layout:
- 0x (@0 MiB): U-Boot SPL text base
- 0x0004 (@256 KiB): TF-A
- 0x0020 (@2 MiB): U-Boot proper text base
- 0x0040 (@4 MiB): U-Boot SPL init stack
- 0x0080 (@8 MiB): U-Boot proper init stack (changed)
- 0x0200 (@32 MiB): U-Boot SPL bss
- 0x0400 (@64 MiB): U-Boot SPL reloc stack (changed)
- 0x0840 (@132 MiB): optional OPTEE

This should leave room for loading and running a U-Boot proper close
to 6 MiB (@2-8 MiB) in size. When U-Boot proper has relocated is should
be possible to use @2-132 MiB and @164 MiB+ for scripts, kernel etc.

Also update SYS_MALLOC_F_LEN to use a 16 KiB malloc area running in SPL
pre-reloc and proper pre-reloc stage. Same size that is used on RK3399.

Signed-off-by: Jonas Karlman 
---
 arch/arm/mach-rockchip/rk3328/Kconfig | 2 +-
 configs/evb-rk3328_defconfig  | 4 +---
 configs/nanopi-r2c-plus-rk3328_defconfig  | 4 ++--
 configs/nanopi-r2c-rk3328_defconfig   | 4 ++--
 configs/nanopi-r2s-rk3328_defconfig   | 4 ++--
 configs/orangepi-r1-plus-lts-rk3328_defconfig | 4 ++--
 configs/orangepi-r1-plus-rk3328_defconfig | 4 ++--
 configs/roc-cc-rk3328_defconfig   | 4 ++--
 configs/rock-pi-e-rk3328_defconfig| 4 +---
 configs/rock64-rk3328_defconfig   | 4 ++--
 10 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-rockchip/rk3328/Kconfig 
b/arch/arm/mach-rockchip/rk3328/Kconfig
index d5cb649ae6ba..681a1a624da2 100644
--- a/arch/arm/mach-rockchip/rk3328/Kconfig
+++ b/arch/arm/mach-rockchip/rk3328/Kconfig
@@ -22,7 +22,7 @@ config SYS_SOC
default "rk3328"
 
 config SYS_MALLOC_F_LEN
-   default 0x2000
+   default 0x4000
 
 config SPL_LIBCOMMON_SUPPORT
default y
diff --git a/configs/evb-rk3328_defconfig b/configs/evb-rk3328_defconfig
index 004cbd33e80a..975ba72a8274 100644
--- a/configs/evb-rk3328_defconfig
+++ b/configs/evb-rk3328_defconfig
@@ -5,7 +5,7 @@ CONFIG_ARCH_ROCKCHIP=y
 CONFIG_TEXT_BASE=0x0020
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
-CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x30
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80
 CONFIG_SF_DEFAULT_SPEED=2000
 CONFIG_ENV_OFFSET=0x3F8000
 CONFIG_DEFAULT_DEVICE_TREE="rk3328-evb"
@@ -16,7 +16,6 @@ CONFIG_TPL_LIBGENERIC_SUPPORT=y
 CONFIG_SPL_STACK_R_ADDR=0x400
 CONFIG_SPL_STACK=0x40
 CONFIG_TPL_SYS_MALLOC_F_LEN=0x800
-CONFIG_SPL_SYS_MALLOC_F_LEN=0x4000
 CONFIG_DEBUG_UART_BASE=0xFF13
 CONFIG_DEBUG_UART_CLOCK=2400
 CONFIG_SYS_LOAD_ADDR=0x800800
@@ -38,7 +37,6 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
 # CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
 # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
 CONFIG_SPL_STACK_R=y
-CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1
 CONFIG_SPL_ATF=y
 CONFIG_SPL_ATF_NO_PLATFORM_PARAM=y
 CONFIG_TPL_SYS_MALLOC_SIMPLE=y
diff --git a/configs/nanopi-r2c-plus-rk3328_defconfig 
b/configs/nanopi-r2c-plus-rk3328_defconfig
index 2f0d253b4b7c..7dfa8686651d 100644
--- a/configs/nanopi-r2c-plus-rk3328_defconfig
+++ b/configs/nanopi-r2c-plus-rk3328_defconfig
@@ -6,7 +6,7 @@ CONFIG_TEXT_BASE=0x0020
 CONFIG_SPL_GPIO=y
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
-CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x30
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80
 CONFIG_SF_DEFAULT_SPEED=2000
 CONFIG_ENV_OFFSET=0x3F8000
 CONFIG_DEFAULT_DEVICE_TREE="rk3328-nanopi-r2c-plus"
@@ -15,7 +15,7 @@ CONFIG_ROCKCHIP_RK3328=y
 CONFIG_TPL_ROCKCHIP_COMMON_BOARD=y
 CONFIG_TPL_LIBCOMMON_SUPPORT=y
 CONFIG_TPL_LIBGENERIC_SUPPORT=y
-CONFIG_SPL_STACK_R_ADDR=0x60
+CONFIG_SPL_STACK_R_ADDR=0x400
 CONFIG_SPL_STACK=0x40
 CONFIG_TPL_SYS_MALLOC_F_LEN=0x800
 CONFIG_DEBUG_UART_BASE=0xFF13
diff --git a/configs/nanopi-r2c-rk3328_defconfig 
b/configs/nanopi-r2c-rk3328_defconfig
index ca13eb873a72..ba98c152639c 100644
--- a/configs/nanopi-r2c-rk3328_defconfig
+++ b/configs/nanopi-r2c-rk3328_defconfig
@@ -6,7 +6,7 @@ CONFIG_TEXT_BASE=0x0020
 CONFIG_SPL_GPIO=y
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
-CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x30
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80
 

[PATCH 0/2] rockchip: Update stack and bss addresses on RK3328 and RK3399

2024-02-11 Thread Jonas Karlman
With the stack and text base used by U-Boot SPL and proper on RK3328 and
RK3399 there is a high likelihood of overlapping when U-Boot proper +
FDT nears 1 MiB in size.

Trying to run U-Boot proper close to 1 MiB in size with debug logging
something similar to following can be observed:

  FDT 002fc4e0 gd 002fddf0
  FDT overlap
  resetting ...
  System reset not supported on this platform
  ### ERROR ### Please RESET the board ###

Fix this by changing stack and bss addresses used on RK3328 and RK3399.
The addresses chosen for stack and bss is a combination from both boards.

This series depends on the following two series:
- rockchip: rk3328: Update defconfigs, DTs and enable boot from SPI [1]
- rockchip: Read cpuid and generate MAC address from efuse for RK3328
  and RK3399 [2]

[1] https://patchwork.ozlabs.org/cover/1895947/
[1] https://patchwork.ozlabs.org/cover/1897310/

Jonas Karlman (2):
  rockchip: rk3328: Update stack addresses
  rockchip: rk3399: Update stack and bss addresses

 arch/arm/mach-rockchip/rk3328/Kconfig | 2 +-
 configs/chromebook_bob_defconfig  | 2 +-
 configs/chromebook_kevin_defconfig| 2 +-
 configs/eaidk-610-rk3399_defconfig| 4 ++--
 configs/evb-rk3328_defconfig  | 4 +---
 configs/evb-rk3399_defconfig  | 4 ++--
 configs/ficus-rk3399_defconfig| 2 +-
 configs/firefly-rk3399_defconfig  | 4 ++--
 configs/khadas-edge-captain-rk3399_defconfig  | 4 ++--
 configs/khadas-edge-rk3399_defconfig  | 4 ++--
 configs/khadas-edge-v-rk3399_defconfig| 4 ++--
 configs/leez-rk3399_defconfig | 4 ++--
 configs/nanopc-t4-rk3399_defconfig| 4 ++--
 configs/nanopi-m4-2gb-rk3399_defconfig| 4 ++--
 configs/nanopi-m4-rk3399_defconfig| 4 ++--
 configs/nanopi-m4b-rk3399_defconfig   | 4 ++--
 configs/nanopi-neo4-rk3399_defconfig  | 4 ++--
 configs/nanopi-r2c-plus-rk3328_defconfig  | 4 ++--
 configs/nanopi-r2c-rk3328_defconfig   | 4 ++--
 configs/nanopi-r2s-rk3328_defconfig   | 4 ++--
 configs/nanopi-r4s-rk3399_defconfig   | 4 ++--
 configs/orangepi-r1-plus-lts-rk3328_defconfig | 4 ++--
 configs/orangepi-r1-plus-rk3328_defconfig | 4 ++--
 configs/orangepi-rk3399_defconfig | 4 ++--
 configs/pinebook-pro-rk3399_defconfig | 4 ++--
 configs/pinephone-pro-rk3399_defconfig| 4 ++--
 configs/puma-rk3399_defconfig | 2 +-
 configs/roc-cc-rk3328_defconfig   | 4 ++--
 configs/roc-pc-mezzanine-rk3399_defconfig | 4 ++--
 configs/roc-pc-rk3399_defconfig   | 4 ++--
 configs/rock-4c-plus-rk3399_defconfig | 4 ++--
 configs/rock-4se-rk3399_defconfig | 4 ++--
 configs/rock-pi-4-rk3399_defconfig| 4 ++--
 configs/rock-pi-4c-rk3399_defconfig   | 4 ++--
 configs/rock-pi-e-rk3328_defconfig| 4 +---
 configs/rock-pi-n10-rk3399pro_defconfig   | 4 ++--
 configs/rock64-rk3328_defconfig   | 4 ++--
 configs/rock960-rk3399_defconfig  | 4 ++--
 configs/rockpro64-rk3399_defconfig| 4 ++--
 39 files changed, 71 insertions(+), 75 deletions(-)

-- 
2.43.0



Re: [PATCH v2] arm: sunxi: Reduce inrush current on Olimex A20-OLinuXino_MICRO configs

2024-02-11 Thread Andre Przywara
On Sun, 11 Feb 2024 21:52:13 +0100
Philippe Coval  wrote:

Hi Philippe,

> This change fix reboot, both configurations
> were tested on my Olimex A20 Micro Rev E with debian-12.
> 
> This issue was also present and fixed on A20 Lime2 (in 8311e84b18),
> quoting Olliver Schinagl:
> 
> The lime2 features a too large capacitor on the LDO3 output, which
> causes the PMIC to shutdown when enabling power. To be able to still
> boot up however, we must gradually enable power on LDO3 for this board.
> We do this by enabling both the inrush quirk and the maximum slope the
> AXP209 supports.
> 
> Link: https://bugs.debian.org/1060752
> Cc: Olliver Schinagl 
> Cc: Priit Laes 
> Cc: Maxime Ripard 
> Signed-off-by: Philippe Coval 

Thanks for the changes, this looks good to me now.

Reviewed-by: Andre Przywara 

I will queue this for the sunxi/master.

Cheers,
Andre

> ---
> 
> Changes in v2:
> - Add qirk to eMMC configuration of same machine
> 
>  configs/A20-OLinuXino_MICRO-eMMC_defconfig | 1 +
>  configs/A20-OLinuXino_MICRO_defconfig  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/configs/A20-OLinuXino_MICRO-eMMC_defconfig 
> b/configs/A20-OLinuXino_MICRO-eMMC_defconfig
> index ca5869f43d..2f26b0ca01 100644
> --- a/configs/A20-OLinuXino_MICRO-eMMC_defconfig
> +++ b/configs/A20-OLinuXino_MICRO-eMMC_defconfig
> @@ -20,6 +20,7 @@ CONFIG_ETH_DESIGNWARE=y
>  CONFIG_MII=y
>  CONFIG_SUN7I_GMAC=y
>  CONFIG_SUN7I_GMAC_FORCE_TXERR=y
> +CONFIG_AXP_ALDO3_INRUSH_QUIRK=y
>  CONFIG_AXP_ALDO3_VOLT=2800
>  CONFIG_AXP_ALDO4_VOLT=2800
>  CONFIG_SCSI=y
> diff --git a/configs/A20-OLinuXino_MICRO_defconfig 
> b/configs/A20-OLinuXino_MICRO_defconfig
> index db4270f9b2..673ab85c8a 100644
> --- a/configs/A20-OLinuXino_MICRO_defconfig
> +++ b/configs/A20-OLinuXino_MICRO_defconfig
> @@ -20,6 +20,7 @@ CONFIG_ETH_DESIGNWARE=y
>  CONFIG_MII=y
>  CONFIG_SUN7I_GMAC=y
>  CONFIG_SUN7I_GMAC_FORCE_TXERR=y
> +CONFIG_AXP_ALDO3_INRUSH_QUIRK=y
>  CONFIG_AXP_ALDO3_VOLT=2800
>  CONFIG_AXP_ALDO4_VOLT=2800
>  CONFIG_SCSI=y



Re: [PATCH RFC] sunxi: pinephone: detect existed magnetometer and fixup dtb

2024-02-11 Thread Andre Przywara
On Mon, 12 Feb 2024 01:38:36 +0300
Andrey Skvortsov  wrote:

Hi Andrey,

> thank you for the valuable feedback!
> 
> On 24-02-11 13:13, Andre Przywara wrote:
> > On Sun, 11 Feb 2024 12:28:24 +0300
> > Andrey Skvortsov  wrote:
> > 
> > Hi Andrey,
> > 
> > thanks for taking care of this upstream!
> >   
> > > In newer 1.2 PinePhone board revisions LIS3MDL magnetometer was replaced 
> > > by
> > > AF8133J. They use the same PB1 pin in different modes.  
> 
> > > 
> > > AF8133J's driver isn't upstreamed yet, but it will be soon. Therefore this
> > > is RFC patch. I'd like to know whether selected approach will be accepted
> > > in u-boot before submiting coresponding dts changes to the Linux kernel.
> > > Any feedback on this change would be very welcome.  
> > 
> > So I think this is the right approach: Since the SPL has no use of this
> > device, and it's a rather small DT change, we definitely want to do this
> > in U-Boot proper. And I believe that U-Boot (proper) is indeed the best
> > place to do those adjustments, since it's built for one board anyway,
> > so anything board specific belongs into there (and not into TF-A or the
> > SPL, which are more tailored to one *SoC*). Also we are not so size
> > sensitive in proper.
> > And I also like the fact that it's protected by a board specific
> > definition, and even better that it's an already existing one.
> > 
> > Oh, and I am not really convinced this is the right approach here, but
> > maybe have a look at doc/usage/cmd/extension.rst, for another related
> > mechanism.  
> 
> Thanks for the tip. I've looked at it after your suggestion and tried
> to implement the same logic using extension command. Here are my
> thoughts about that:
> 
> 1. integrated magnetometer is part of the device and making it
> extension with separate dtbo sounds a bit strange.
> 
> 2. if I'd create dtbo for new AF8133J magnetometer, then I'd call it
> like other dts files for the device:
> 'sun50i-a64-pinephone-1.2-af8133j.dtbo. This is 38 characters and 
> currently overlay name is limited to 32 bytes. What do you think about
> increasing overlay name?
> 
> 3. extensions are not supported for extlinux boot flow at all. This is Debian
> case, that I'm working with. And this is a major problem for me.
> 
> 4. I've looked how EFI boot flow is made and I see that extensions are
> not applied to fdtcontroladdr, only to loaded fdt to
> fdt_addr_r. Extlinux uses fdtcontroladdr always.
> 
> 5. When distribution supplies fdt with extlinux, when supplied fdt is
> used and no extensions are applied to it. This is my case as well.
> 
> 6. I can't apply extensions to fdtcontroladdr. When I've tried to
> apply working extension to fdtcontroladdr, then I get a crash.
> I have to copy fdt from fdtcontroladdr to fdt_addr_r
> and then apply extension to fdt_addr_r and when it works. Maybe this
> is something sunxi-specific.

Not really. The DTB is appended to the end of the U-Boot image, and I
believe there is something important immediately after it, like the
heap or the gd or something. Some years back this used to work, but not
anymore, for quite a while now.
So to apply overlays, do a "fdt move $fdtcontroladdr $fdt_addr_r"
first, then use $fdt_addr_r.
But that's just for clarification, nothing that solves the above
problems.

> Overall extensions seems like a nice feature for capes, extension boards for
> pogo pins and so on. But I'm not sure, that it's the right choice in
> this case.

Yeah, many thanks for the extensive research and nice summary! I was
already expecting something along those lines, but wasn't sure. So
thanks for saving me some time.

This confirms that we should go the route you already sketched.

> > Some smaller comments below ...
> >   
> > > 
> > >  board/sunxi/board.c | 26 ++
> > >  configs/pinephone_defconfig |  1 +
> > >  2 files changed, 27 insertions(+)
> > > 
> > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > index 1305302060..a4bfa24400 100644
> > > --- a/board/sunxi/board.c
> > > +++ b/board/sunxi/board.c
> > > @@ -15,6 +15,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -920,6 +921,28 @@ static void bluetooth_dt_fixup(void *blob)
> > >  "local-bd-address", bdaddr, ETH_ALEN, 1);
> > >  }
> > >  
> > > +#ifdef CONFIG_PINEPHONE_DT_SELECTION  
> > 
> > Can you move that line into the function? That would for one avoid the
> > protection in the caller below, and secondly make it easier to add
> > other boards, if a need arises for them. The two #defines don't hurt or
> > change the code, so just keep them outside.  
> 
> > > +
> > > +#define PINEPHONE_LIS3MDL_I2C_ADDR   0x1E
> > > +#define PINEPHONE_LIS3MDL_I2C_BUS1 /* I2C1 */
> > > +
> > > +static   void board_dt_fixup(void *blob)
> > > +{
> > > + struct udevice *bus, *dev;
> > > +  
> > 
> > (have the #ifdef here)  
> 
> Ok, then I mark local 

Re: [PATCH RFC] sunxi: pinephone: detect existed magnetometer and fixup dtb

2024-02-11 Thread Andrey Skvortsov
Hi Andre,

thank you for the valuable feedback!

On 24-02-11 13:13, Andre Przywara wrote:
> On Sun, 11 Feb 2024 12:28:24 +0300
> Andrey Skvortsov  wrote:
> 
> Hi Andrey,
> 
> thanks for taking care of this upstream!
> 
> > In newer 1.2 PinePhone board revisions LIS3MDL magnetometer was replaced by
> > AF8133J. They use the same PB1 pin in different modes.

> > 
> > AF8133J's driver isn't upstreamed yet, but it will be soon. Therefore this
> > is RFC patch. I'd like to know whether selected approach will be accepted
> > in u-boot before submiting coresponding dts changes to the Linux kernel.
> > Any feedback on this change would be very welcome.
> 
> So I think this is the right approach: Since the SPL has no use of this
> device, and it's a rather small DT change, we definitely want to do this
> in U-Boot proper. And I believe that U-Boot (proper) is indeed the best
> place to do those adjustments, since it's built for one board anyway,
> so anything board specific belongs into there (and not into TF-A or the
> SPL, which are more tailored to one *SoC*). Also we are not so size
> sensitive in proper.
> And I also like the fact that it's protected by a board specific
> definition, and even better that it's an already existing one.
> 
> Oh, and I am not really convinced this is the right approach here, but
> maybe have a look at doc/usage/cmd/extension.rst, for another related
> mechanism.

Thanks for the tip. I've looked at it after your suggestion and tried
to implement the same logic using extension command. Here are my
thoughts about that:

1. integrated magnetometer is part of the device and making it
extension with separate dtbo sounds a bit strange.

2. if I'd create dtbo for new AF8133J magnetometer, then I'd call it
like other dts files for the device:
'sun50i-a64-pinephone-1.2-af8133j.dtbo. This is 38 characters and 
currently overlay name is limited to 32 bytes. What do you think about
increasing overlay name?

3. extensions are not supported for extlinux boot flow at all. This is Debian
case, that I'm working with. And this is a major problem for me.

4. I've looked how EFI boot flow is made and I see that extensions are
not applied to fdtcontroladdr, only to loaded fdt to
fdt_addr_r. Extlinux uses fdtcontroladdr always.

5. When distribution supplies fdt with extlinux, when supplied fdt is
used and no extensions are applied to it. This is my case as well.

6. I can't apply extensions to fdtcontroladdr. When I've tried to
apply working extension to fdtcontroladdr, then I get a crash.
I have to copy fdt from fdtcontroladdr to fdt_addr_r
and then apply extension to fdt_addr_r and when it works. Maybe this
is something sunxi-specific.

Overall extensions seems like a nice feature for capes, extension boards for
pogo pins and so on. But I'm not sure, that it's the right choice in
this case.

> Some smaller comments below ...
> 
> > 
> >  board/sunxi/board.c | 26 ++
> >  configs/pinephone_defconfig |  1 +
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > index 1305302060..a4bfa24400 100644
> > --- a/board/sunxi/board.c
> > +++ b/board/sunxi/board.c
> > @@ -15,6 +15,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -920,6 +921,28 @@ static void bluetooth_dt_fixup(void *blob)
> >"local-bd-address", bdaddr, ETH_ALEN, 1);
> >  }
> >  
> > +#ifdef CONFIG_PINEPHONE_DT_SELECTION
> 
> Can you move that line into the function? That would for one avoid the
> protection in the caller below, and secondly make it easier to add
> other boards, if a need arises for them. The two #defines don't hurt or
> change the code, so just keep them outside.

> > +
> > +#define PINEPHONE_LIS3MDL_I2C_ADDR 0x1E
> > +#define PINEPHONE_LIS3MDL_I2C_BUS  1 /* I2C1 */
> > +
> > +static void board_dt_fixup(void *blob)
> > +{
> > +   struct udevice *bus, *dev;
> > +
> 
> (have the #ifdef here)

Ok, then I mark local variables with __maybe_unused and remove #ifdef
at board_dt_fixup call.

> 
> > +   if (!fdt_node_check_compatible(blob, 0, "pine64,pinephone-1.2"))
> 
> Please have curly braces around that.

I'll fix that in v2. 

> So I'd say please send the (disabled) DT node addition patch to the
> kernel MLs, then send this patch to U-Boot.

Do you mean patch with disabled AF8133J DT node? Right?
If so, then it was the plan.

1. upstream AF8133J driver to the Linux kernel (on-going)
2. find acceptable solution for u-boot to handle different
   magnetometers (on-going in this thread) 
3. upstream necessary dts changes to the Linux kernel
4. upstream previously discussed changes to u-boot.


> 
> Cheers,
> Andre
> 
> > +   if (!uclass_get_device_by_seq(UCLASS_I2C, 
> > PINEPHONE_LIS3MDL_I2C_BUS, )) {
> > +   dm_i2c_probe(bus, PINEPHONE_LIS3MDL_I2C_ADDR, 0, );
> > +   fdt_set_status_by_compatible(
> > +

Re: [PATCH v2 16/20] board: rockchip: add Theobroma-Systems RK3588 Jaguar SBC

2024-02-11 Thread Jonas Karlman
Hi Quentin,

On 2024-02-09 10:50, Quentin Schulz wrote:
> From: Quentin Schulz 
> 
> JAGUAR is a Single-Board Computer (SBC) based around the rk3588 SoC and
> is targeting Autonomous Mobile Robots (AMR).
> 
> It features:
>  * LPDDR4X (up to 16GB)
>  * 1Gbps Ethernet on RJ45 connector (KSZ9031 or KSZ9131)
>  * PCIe 3.0 4-lane on M.2 M-key connector
>  * PCIe 2.1 1-lane on M.2 E-key
>  * USB 2.0 on M.2 E-key
>  * 2x USB3 OTG type-c ports with DP Alt-Mode
>  * USB2 host port
>  * HDMI output
>  * 2x camera connectors, each exposing:
>* 2-lane MIPI-CSI
>* 1v2, 1v8, 2v8 power rails
>* I2C bus
>* GPIOs
>  * PPS input
>  * CAN
>  * RS485 UART
>  * FAN connector
>  * SD card slot
>  * eMMC (up to 256GB)
>  * RTC backup battery
>  * Companion microcontroller
>* ISL1208 RTC emulation
>* AMC6821 PWM emulation
>* On/off buzzer control
>  * Secure Element
>  * 80-pin Mezzanine connector for daughterboards:
>* GPIOs
>* 1Gbps Ethernet
>* PCIe 2.1 1-lane
>* 2x 2-lane MIPI-CSI
>* ADC channel
>* I2C bus
>* PWM
>* UART
>* SPI
>* SDIO
>* CAN
>* I2S
>* 1v8, 3v3, 5v0, dc-in (12-24V) power rails
> 
> The Device Tree comes from next-20240110 Linux kernel.
> 
> Cc: Quentin Schulz 
> Signed-off-by: Quentin Schulz 
> ---
>  arch/arm/dts/Makefile  |   1 +
>  arch/arm/dts/rk3588-jaguar-u-boot.dtsi |  38 +
>  arch/arm/dts/rk3588-jaguar.dts | 803 
> +
>  arch/arm/mach-rockchip/rk3588/Kconfig  |  28 +
>  board/theobroma-systems/jaguar_rk3588/Kconfig  |  16 +
>  board/theobroma-systems/jaguar_rk3588/MAINTAINERS  |  13 +
>  board/theobroma-systems/jaguar_rk3588/Makefile |  10 +
>  .../jaguar_rk3588/jaguar_rk3588.c  |  52 ++
>  configs/jaguar-rk3588_defconfig| 115 +++
>  doc/board/index.rst|   1 +
>  doc/board/rockchip/rockchip.rst|   1 +
>  doc/board/theobroma-systems/index.rst  |   9 +
>  doc/board/theobroma-systems/jaguar_rk3588.rst  | 100 +++
>  include/configs/jaguar_rk3588.h|  15 +
>  14 files changed, 1202 insertions(+)
> 
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 3beb8f1b9d4..7c103655e1d 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -193,6 +193,7 @@ dtb-$(CONFIG_ROCKCHIP_RK3588) += \
>   rk3588-edgeble-neu6a-io.dtb \
>   rk3588-edgeble-neu6b-io.dtb \
>   rk3588-evb1-v10.dtb \
> + rk3588-jaguar.dtb \
>   rk3588-nanopc-t6.dtb \
>   rk3588s-orangepi-5.dtb \
>   rk3588-orangepi-5-plus.dtb \
> diff --git a/arch/arm/dts/rk3588-jaguar-u-boot.dtsi 
> b/arch/arm/dts/rk3588-jaguar-u-boot.dtsi
> new file mode 100644
> index 000..a0207c265c9
> --- /dev/null
> +++ b/arch/arm/dts/rk3588-jaguar-u-boot.dtsi
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2023 Theobroma Systems Design und Consulting GmbH
> + */
> +
> +#include "rk3588-u-boot.dtsi"
> +#include 
> +#include 
> +#include 
> +#include 

These dt-bindings includes should not be needed in this file.

> +
> +/ {
> + chosen {
> + u-boot,spl-boot-order = "same-as-spl", , 

Is there specific reasoning behind why eMMC is listed before SD-card?

rk3588-u-boot.dtsi should now contain a default with a same-as-spl >
SD-card > eMMC order.

Also guess in practice it should not matter mutch, if TPL/SPL is running
from eMMC it will read FIT from eMMC > eMMC > SD-card. And if TPL/SPL is
running from SD-card it will read FIT from SD-card > eMMC > SD-card.

Main difference would be if TPL/SPL is running from SPI, then it will
read FIT from SPI > eMMC > SD-card instead of SPI > SD-card > eMMC.

In my mind anything after same-as-spl would mostly be used for recovery.
And preferring SD-card may ease in such situation?

> + };
> +};
> +
> +_pwrseq {
> + bootph-all;
> +};
> +
> +_reset {
> + bootph-all;
> +};
> +
> + {
> + bootph-all;
> +};
> +
> + {
> + bootph-all;

I am guessing that emmc_pwrseq, gpio0 and gpio2 would not be needed in
TPL so bootph-all could be too inclusive, for now I guess this does not
really matter because an external TPL blob is used anyway.

Regards,
Jonas

> +};
> +
> + {
> + /* U-Boot currently cannot handle anything below HS200 for eMMC on 
> RK3588 */
> + /delete-property/ mmc-ddr-1_8v;
> + /delete-property/ cap-mmc-highspeed;
> +};

[snip]


Re: [PATCH v2 15/20] rockchip: rk3588: bind MMC controllers in U-Boot proper pre-reloc

2024-02-11 Thread Jonas Karlman
Hi Quentin,

On 2024-02-09 10:50, Quentin Schulz wrote:
> From: Quentin Schulz 
> 
> Since commit 9e644284ab81 ("dm: core: Report bootph-pre-ram/sram node as
> pre-reloc after relocation"), bootph-pre-ram doesn't make U-Boot proper
> bind the device before relocation.
> 
> While this is usually not much of an issue, it is when there's a lookup
> for devices by code running before the relocation. Such is the case of
> env_init() which calls env_driver_lookup() which calls
> env_get_location() which is a weak symbol and may call
> arch_env_get_location() also a weak symbol. Those are two functions that
> may traverse UCLASS to find some devices (e.g.
> board/theobroma-systems/common/common.c:arch_env_get_location()).
> 
> This allows something in the env_init() call stack to be able to use
> uclasses for SD and eMMC controller on RK3588S/RK3588. This aligns the
> behavior with what seems to be all SoCs except RK356x family.
> 
> Additionally, if any other env function (e.g. env_load) were to be used
> before relocation, this is also required as otherwise it wouldn't be
> able to find the MMC device(s).
> 
> Cc: Quentin Schulz 
> Signed-off-by: Quentin Schulz 
> ---
>  arch/arm/dts/rk3588s-u-boot.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/dts/rk3588s-u-boot.dtsi 
> b/arch/arm/dts/rk3588s-u-boot.dtsi
> index bf3b1ea8a3c..3f194e8ffbd 100644
> --- a/arch/arm/dts/rk3588s-u-boot.dtsi
> +++ b/arch/arm/dts/rk3588s-u-boot.dtsi
> @@ -187,12 +187,12 @@
>  };
>  
>   {
> - bootph-pre-ram;
> + bootph-all;

I know that there is no TPL for rk3588 so this does not matter at this
time but I would suggest you instead add bootph-some-ram for both these
instead of changing them to bootph-all.

We are not expected to need the MMC devices in TPL, and I am working on
a series that changes from bootph-all to bootph-pre-ram+bootph-some-ram
combo in an upcoming rk3399 cleanup series.

Regards,
Jonas

>   u-boot,spl-fifo-mode;
>  };
>  
>   {
> - bootph-pre-ram;
> + bootph-all;
>   u-boot,spl-fifo-mode;
>  };
>  
> 



Re: [PULL] u-boot-sh/master-779h0-r2

2024-02-11 Thread Tom Rini
On Sun, Feb 11, 2024 at 06:31:59PM +0100, Marek Vasut wrote:

> The following changes since commit d7aaaf4223d0a2f9f8c9eed47d7431860b3116d8:
> 
>   Merge tag 'u-boot-dfu-20240209' of 
> https://source.denx.de/u-boot/custodians/u-boot-dfu (2024-02-09 09:00:42 
> -0500)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-sh.git master-779h0-r2
> 
> for you to fetch changes up to 53066deccbedf02439309b0d4aca9f0be853a8da:
> 
>   ARM: renesas: Add Renesas R8A779H0 V4M Gray Hawk board code (2024-02-10 
> 17:08:06 +0100)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


[PATCH v2] arm: sunxi: Reduce inrush current on Olimex A20-OLinuXino_MICRO configs

2024-02-11 Thread Philippe Coval
This change fix reboot, both configurations
were tested on my Olimex A20 Micro Rev E with debian-12.

This issue was also present and fixed on A20 Lime2 (in 8311e84b18),
quoting Olliver Schinagl:

The lime2 features a too large capacitor on the LDO3 output, which
causes the PMIC to shutdown when enabling power. To be able to still
boot up however, we must gradually enable power on LDO3 for this board.
We do this by enabling both the inrush quirk and the maximum slope the
AXP209 supports.

Link: https://bugs.debian.org/1060752
Cc: Olliver Schinagl 
Cc: Priit Laes 
Cc: Maxime Ripard 
Signed-off-by: Philippe Coval 
---

Changes in v2:
- Add qirk to eMMC configuration of same machine

 configs/A20-OLinuXino_MICRO-eMMC_defconfig | 1 +
 configs/A20-OLinuXino_MICRO_defconfig  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/configs/A20-OLinuXino_MICRO-eMMC_defconfig 
b/configs/A20-OLinuXino_MICRO-eMMC_defconfig
index ca5869f43d..2f26b0ca01 100644
--- a/configs/A20-OLinuXino_MICRO-eMMC_defconfig
+++ b/configs/A20-OLinuXino_MICRO-eMMC_defconfig
@@ -20,6 +20,7 @@ CONFIG_ETH_DESIGNWARE=y
 CONFIG_MII=y
 CONFIG_SUN7I_GMAC=y
 CONFIG_SUN7I_GMAC_FORCE_TXERR=y
+CONFIG_AXP_ALDO3_INRUSH_QUIRK=y
 CONFIG_AXP_ALDO3_VOLT=2800
 CONFIG_AXP_ALDO4_VOLT=2800
 CONFIG_SCSI=y
diff --git a/configs/A20-OLinuXino_MICRO_defconfig 
b/configs/A20-OLinuXino_MICRO_defconfig
index db4270f9b2..673ab85c8a 100644
--- a/configs/A20-OLinuXino_MICRO_defconfig
+++ b/configs/A20-OLinuXino_MICRO_defconfig
@@ -20,6 +20,7 @@ CONFIG_ETH_DESIGNWARE=y
 CONFIG_MII=y
 CONFIG_SUN7I_GMAC=y
 CONFIG_SUN7I_GMAC_FORCE_TXERR=y
+CONFIG_AXP_ALDO3_INRUSH_QUIRK=y
 CONFIG_AXP_ALDO3_VOLT=2800
 CONFIG_AXP_ALDO4_VOLT=2800
 CONFIG_SCSI=y
-- 
2.43.0



[PATCH] doc: fix mistyped "env flags" command

2024-02-11 Thread Thomas Weißschuh
Signed-off-by: Thomas Weißschuh 
---
 doc/usage/cmd/env.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/usage/cmd/env.rst b/doc/usage/cmd/env.rst
index a859e32798cb..a7e21693a672 100644
--- a/doc/usage/cmd/env.rst
+++ b/doc/usage/cmd/env.rst
@@ -350,7 +350,7 @@ edit
 exists
 CONFIG_CMD_ENV_EXISTS
 
-flsgs
+flags
 CONFIG_CMD_ENV_FLAGS
 
 erase

---
base-commit: d7aaaf4223d0a2f9f8c9eed47d7431860b3116d8
change-id: 20240211-doc-flsgs-31daddb198ec

Best regards,
-- 
Thomas Weißschuh 



Re: inconsistent wget behavior

2024-02-11 Thread Paul Liu
Hi Fabio,

I'm on vacation now (Chinese new year). I hope I'll find some time to
revive my imx8 board.
I've tried sandbox and qemu. Both of them are not reproducible. I'm
wondering if it could be some packet loss that causes the issue. Because
sandbox and qemu there won't be any missing packets because of loopback
devices.

Yours,
Paul



On Thu, 8 Feb 2024 at 02:00, Fabio Estevam  wrote:

> Hi Paul,
>
> On Wed, Jan 10, 2024 at 9:20 AM Fabio Estevam  wrote:
>
> > One colleague from you at Linaro was able to reproduce the bug:
> >
> >
> https://lore.kernel.org/u-boot/cadq0-x_cj1ecn67u3sefcz-jm4obsymzka+jazrca3ekq84...@mail.gmail.com/
> >
> > It is not specific to i.MX.
>
> Have you had a chance to look into this wget problem?
>


[PATCH v1] cmd: md5sum: use hash_command

2024-02-11 Thread Igor Opaniuk
From: Igor Opaniuk 

Drop old implementation and use hash_command() instead, as
how it's currently done for crc32 and sha1sum cmds.

Test:
=> md5sum 0x6000 0x200
md5 for 6000 ... 61ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050

Signed-off-by: Igor Opaniuk 
---

 cmd/md5sum.c | 149 ---
 1 file changed, 9 insertions(+), 140 deletions(-)

diff --git a/cmd/md5sum.c b/cmd/md5sum.c
index 0f0e1d3dd68..618265e8d50 100644
--- a/cmd/md5sum.c
+++ b/cmd/md5sum.c
@@ -7,7 +7,6 @@
  * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
  */
 
-#include 
 #include 
 #include 
 #include 
@@ -15,158 +14,28 @@
 #include 
 #include 
 
-/*
- * Store the resulting sum to an address or variable
- */
-static void store_result(const u8 *sum, const char *dest)
-{
-   unsigned int i;
-
-   if (*dest == '*') {
-   u8 *ptr;
-
-   ptr = (u8 *)hextoul(dest + 1, NULL);
-   for (i = 0; i < 16; i++)
-   *ptr++ = sum[i];
-   } else {
-   char str_output[33];
-   char *str_ptr = str_output;
-
-   for (i = 0; i < 16; i++) {
-   sprintf(str_ptr, "%02x", sum[i]);
-   str_ptr += 2;
-   }
-   env_set(dest, str_output);
-   }
-}
-
-#ifdef CONFIG_MD5SUM_VERIFY
-static int parse_verify_sum(char *verify_str, u8 *vsum)
-{
-   if (*verify_str == '*') {
-   u8 *ptr;
-
-   ptr = (u8 *)hextoul(verify_str + 1, NULL);
-   memcpy(vsum, ptr, 16);
-   } else {
-   unsigned int i;
-   char *vsum_str;
-
-   if (strlen(verify_str) == 32)
-   vsum_str = verify_str;
-   else {
-   vsum_str = env_get(verify_str);
-   if (vsum_str == NULL || strlen(vsum_str) != 32)
-   return 1;
-   }
-
-   for (i = 0; i < 16; i++) {
-   char *nullp = vsum_str + (i + 1) * 2;
-   char end = *nullp;
-
-   *nullp = '\0';
-   *(u8 *)(vsum + i) =
-   hextoul(vsum_str + (i * 2), NULL);
-   *nullp = end;
-   }
-   }
-   return 0;
-}
-
-int do_md5sum(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+static int do_md5sum(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
 {
-   ulong addr, len;
-   unsigned int i;
-   u8 output[16];
-   u8 vsum[16];
-   int verify = 0;
+   int flags = HASH_FLAG_ENV;
int ac;
-   char * const *av;
-   void *buf;
+   char *const *av;
 
if (argc < 3)
return CMD_RET_USAGE;
 
av = argv + 1;
ac = argc - 1;
-   if (strcmp(*av, "-v") == 0) {
-   verify = 1;
+   if (IS_ENABLED(CONFIG_MD5SUM_VERIFY) && strcmp(*av, "-v") == 0) {
+   flags |= HASH_FLAG_VERIFY;
av++;
ac--;
-   if (ac < 3)
-   return CMD_RET_USAGE;
}
 
-   addr = hextoul(*av++, NULL);
-   len = hextoul(*av++, NULL);
-
-   buf = map_sysmem(addr, len);
-   md5_wd(buf, len, output, CHUNKSZ_MD5);
-   unmap_sysmem(buf);
-
-   if (!verify) {
-   printf("md5 for %08lx ... %08lx ==> ", addr, addr + len - 1);
-   for (i = 0; i < 16; i++)
-   printf("%02x", output[i]);
-   printf("\n");
-
-   if (ac > 2)
-   store_result(output, *av);
-   } else {
-   char *verify_str = *av++;
-
-   if (parse_verify_sum(verify_str, vsum)) {
-   printf("ERROR: %s does not contain a valid md5 sum\n",
-   verify_str);
-   return 1;
-   }
-   if (memcmp(output, vsum, 16) != 0) {
-   printf("md5 for %08lx ... %08lx ==> ", addr,
-   addr + len - 1);
-   for (i = 0; i < 16; i++)
-   printf("%02x", output[i]);
-   printf(" != ");
-   for (i = 0; i < 16; i++)
-   printf("%02x", vsum[i]);
-   printf(" ** ERROR **\n");
-   return 1;
-   }
-   }
-
-   return 0;
-}
-#else
-static int do_md5sum(struct cmd_tbl *cmdtp, int flag, int argc,
-char *const argv[])
-{
-   unsigned long addr, len;
-   unsigned int i;
-   u8 output[16];
-   void *buf;
-
-   if (argc < 3)
-   return CMD_RET_USAGE;
-
-   addr = hextoul(argv[1], NULL);
-   len = hextoul(argv[2], NULL);
-
-   buf = map_sysmem(addr, len);
-   md5_wd(buf, len, output, 

Re: [PATCH] doc/develop/codingstyle.rst: Clarify include section

2024-02-11 Thread Igor Opaniuk
Hi Tom,

On Fri, Feb 9, 2024 at 3:52 PM Tom Rini  wrote:
>
> Rework the section about includes slightly. We should not be using
> common.h anywhere, so remove that from examples and ask people to send
> patches removing it when found. Doing this also means we need to reword
> other parts of this section. Be clearer about using alphabetical
> ordering.
>
> Signed-off-by: Tom Rini 
> ---
>  doc/develop/codingstyle.rst | 23 +++
>  1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst
> index b25bfbd271f0..cdf5aedfcbb0 100644
> --- a/doc/develop/codingstyle.rst
> +++ b/doc/develop/codingstyle.rst
> @@ -108,30 +108,29 @@ expected size, or that particular members appear at the 
> right offset.
>  Include files
>  -
>
> -You should follow this ordering in U-Boot. The common.h header (which is 
> going
> -away at some point) should always be first, followed by other headers in 
> order,
> -then headers with directories, then local files:
> +You should follow this ordering in U-Boot. In all cases, they should be 
> listed
> +in alphabetical order. First comes headers which are located directly in our
> +top-level include diretory. This excludes the common.h header file which is 
> to
> +be removed. Second are headers within subdirectories, Finally directory-local
> +includes should be listed. See this example:
>
>  .. code-block:: C
>
> -   #include 
> #include 
> #include 
> #include 
> #include 
> -   #include 
> +   #include 
> #include 
> #include 
> #include "local.h"
>
> -Within that order, sort your includes.
> -
> -It is important to include common.h first since it provides basic features 
> used
> -by most files, e.g. CONFIG options.
> -
>  For files that need to be compiled for the host (e.g. tools), you need to use
> -``#ifndef USE_HOSTCC`` to avoid including common.h since it includes a lot of
> -internal U-Boot things. See common/image.c for an example.
> +``#ifndef USE_HOSTCC`` to avoid including U-Boot specific include files.  See
> +common/image.c for an example.
> +
> +If you encounter ccode which still uses  a patch to remove that and
nitpick: s/ccode/code/g
> +replace it with any required include files directly is much appreciated.
>
>  If your file uses driver model, include  in the C file. Do not include
>  dm.h in a header file. Try to use forward declarations (e.g. ``struct
> --
> 2.34.1
>

Reviewed-by: Igor Opaniuk 

-- 
Best regards - Atentamente - Meilleures salutations

Igor Opaniuk

mailto: igor.opan...@gmail.com
skype: igor.opanyuk
http://ua.linkedin.com/in/iopaniuk


[PATCH 3/3] ARM: renesas: Enable LTO on R-Car

2024-02-11 Thread Marek Vasut
Enable LTO globally on Renesas R-Car platforms. This has been enabled
on a subset of boards already, but at this point it is safe to enable
it globally. This saves units or tens of kiB from the resulting build.

Signed-off-by: Marek Vasut 
---
Cc: Niklas Söderlund 
Cc: Paul Barker 
---
 arch/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 6b072be2463..fde85dc0d53 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1076,6 +1076,7 @@ config ARCH_RMOBILE
select DM
select DM_SERIAL
select GPIO_EXTRA_HEADER
+   select LTO
imply BOARD_EARLY_INIT_F
imply CMD_DM
imply FAT_WRITE
-- 
2.43.0



[PATCH 2/3] ARM: renesas: Set R-Car Gen2 board size limit to 512 kiB

2024-02-11 Thread Marek Vasut
The maximum size of u-boot.img on R-Car Gen2 is 0x8 or 512 kiB,
set the limit to avoid overflows as new functionality gets pulled in.

Signed-off-by: Marek Vasut 
---
Cc: Niklas Söderlund 
Cc: Paul Barker 
---
 Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Kconfig b/Kconfig
index 00ed1ecc173..5710934000f 100644
--- a/Kconfig
+++ b/Kconfig
@@ -500,13 +500,14 @@ config BUILD_TARGET
 
 config HAS_BOARD_SIZE_LIMIT
bool "Define a maximum size for the U-Boot image"
-   default y if RCAR_64
+   default y if RCAR_32 || RCAR_64
help
  In some cases, we need to enforce a hard limit on how big the U-Boot
  image itself can be.
 
 config BOARD_SIZE_LIMIT
int "Maximum size of the U-Boot image in bytes"
+   default 524288 if RCAR_32
default 1048576 if RCAR_64
depends on HAS_BOARD_SIZE_LIMIT
help
-- 
2.43.0



[PATCH 1/3] ARM: renesas: Disable EFI on R-Car Gen2

2024-02-11 Thread Marek Vasut
These systems are unlikely to use EFI as this functionality has not been
enabled until it got pulled in by Kconfig default. This functionality
does add some 60-70 kiB to the u-boot.img size, which overflows the size
limit. Disable it.

Signed-off-by: Marek Vasut 
---
Cc: Niklas Söderlund 
Cc: Paul Barker 
---
 configs/alt_defconfig | 2 ++
 configs/blanche_defconfig | 2 ++
 configs/gose_defconfig| 2 ++
 configs/koelsch_defconfig | 2 ++
 configs/lager_defconfig   | 2 ++
 configs/porter_defconfig  | 2 ++
 configs/silk_defconfig| 2 ++
 configs/stout_defconfig   | 2 ++
 8 files changed, 16 insertions(+)

diff --git a/configs/alt_defconfig b/configs/alt_defconfig
index 31f38bd3a5b..35707a65960 100644
--- a/configs/alt_defconfig
+++ b/configs/alt_defconfig
@@ -68,6 +68,7 @@ CONFIG_CMD_FAT=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_MTDIDS_DEFAULT="nor0=spi0.0"
 
CONFIG_MTDPARTS_DEFAULT="mtdparts=spi0.0:256k(u-boot-spl),512k(u-boot-env1),512k(u-boot-env2),768k(u-boot),-(user)"
+CONFIG_PARTITION_UUIDS=y
 CONFIG_OF_CONTROL=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
@@ -106,3 +107,4 @@ CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
 CONFIG_USB_STORAGE=y
 CONFIG_SYS_TIMER_COUNTS_DOWN=y
+# CONFIG_EFI_LOADER is not set
diff --git a/configs/blanche_defconfig b/configs/blanche_defconfig
index 59e9b3af846..f328af84dfa 100644
--- a/configs/blanche_defconfig
+++ b/configs/blanche_defconfig
@@ -44,6 +44,7 @@ CONFIG_CMD_EXT2=y
 CONFIG_CMD_EXT4=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_FAT=y
+CONFIG_PARTITION_UUIDS=y
 CONFIG_OF_CONTROL=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_IS_IN_FLASH=y
@@ -82,3 +83,4 @@ CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
 CONFIG_USB_STORAGE=y
 CONFIG_SYS_TIMER_COUNTS_DOWN=y
+# CONFIG_EFI_LOADER is not set
diff --git a/configs/gose_defconfig b/configs/gose_defconfig
index 4220b9349b7..92bb5911765 100644
--- a/configs/gose_defconfig
+++ b/configs/gose_defconfig
@@ -68,6 +68,7 @@ CONFIG_CMD_FAT=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_MTDIDS_DEFAULT="nor0=spi0.0"
 
CONFIG_MTDPARTS_DEFAULT="mtdparts=spi0.0:256k(u-boot-spl),512k(u-boot-env1),512k(u-boot-env2),768k(u-boot),-(user)"
+CONFIG_PARTITION_UUIDS=y
 CONFIG_OF_CONTROL=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
@@ -104,3 +105,4 @@ CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
 CONFIG_USB_STORAGE=y
 CONFIG_SYS_TIMER_COUNTS_DOWN=y
+# CONFIG_EFI_LOADER is not set
diff --git a/configs/koelsch_defconfig b/configs/koelsch_defconfig
index d39533cc393..8d9c3b94499 100644
--- a/configs/koelsch_defconfig
+++ b/configs/koelsch_defconfig
@@ -68,6 +68,7 @@ CONFIG_CMD_FAT=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_MTDIDS_DEFAULT="nor0=spi0.0"
 
CONFIG_MTDPARTS_DEFAULT="mtdparts=spi0.0:256k(u-boot-spl),512k(u-boot-env1),512k(u-boot-env2),768k(u-boot),-(user)"
+CONFIG_PARTITION_UUIDS=y
 CONFIG_OF_CONTROL=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
@@ -104,3 +105,4 @@ CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
 CONFIG_USB_STORAGE=y
 CONFIG_SYS_TIMER_COUNTS_DOWN=y
+# CONFIG_EFI_LOADER is not set
diff --git a/configs/lager_defconfig b/configs/lager_defconfig
index da2ba58c6b0..d00314c60e4 100644
--- a/configs/lager_defconfig
+++ b/configs/lager_defconfig
@@ -68,6 +68,7 @@ CONFIG_CMD_FAT=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_MTDIDS_DEFAULT="nor0=spi0.0"
 
CONFIG_MTDPARTS_DEFAULT="mtdparts=spi0.0:256k(u-boot-spl),512k(u-boot-env1),512k(u-boot-env2),768k(u-boot),-(user)"
+CONFIG_PARTITION_UUIDS=y
 CONFIG_OF_CONTROL=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
@@ -106,3 +107,4 @@ CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
 CONFIG_USB_STORAGE=y
 CONFIG_SYS_TIMER_COUNTS_DOWN=y
+# CONFIG_EFI_LOADER is not set
diff --git a/configs/porter_defconfig b/configs/porter_defconfig
index 0a805deff65..a343c8e83de 100644
--- a/configs/porter_defconfig
+++ b/configs/porter_defconfig
@@ -68,6 +68,7 @@ CONFIG_CMD_FAT=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_MTDIDS_DEFAULT="nor0=spi0.0"
 
CONFIG_MTDPARTS_DEFAULT="mtdparts=spi0.0:256k(u-boot-spl),512k(u-boot-env1),512k(u-boot-env2),768k(u-boot),-(user)"
+CONFIG_PARTITION_UUIDS=y
 CONFIG_OF_CONTROL=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
@@ -104,3 +105,4 @@ CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
 CONFIG_USB_STORAGE=y
 CONFIG_SYS_TIMER_COUNTS_DOWN=y
+# CONFIG_EFI_LOADER is not set
diff --git a/configs/silk_defconfig b/configs/silk_defconfig
index 7c6b5b59c93..5f08ae0fcf9 100644
--- a/configs/silk_defconfig
+++ b/configs/silk_defconfig
@@ -68,6 +68,7 @@ CONFIG_CMD_FAT=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_MTDIDS_DEFAULT="nor0=spi0.0"
 
CONFIG_MTDPARTS_DEFAULT="mtdparts=spi0.0:256k(u-boot-spl),512k(u-boot-env1),512k(u-boot-env2),768k(u-boot),-(user)"
+CONFIG_PARTITION_UUIDS=y
 CONFIG_OF_CONTROL=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
@@ -106,3 +107,4 @@ CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
 CONFIG_USB_STORAGE=y
 CONFIG_SYS_TIMER_COUNTS_DOWN=y
+# CONFIG_EFI_LOADER is not set
diff --git a/configs/stout_defconfig b/configs/stout_defconfig
index 8fad272cf55..0375630adea 100644
--- 

[PULL] u-boot-sh/master-779h0-r2

2024-02-11 Thread Marek Vasut
The following changes since commit d7aaaf4223d0a2f9f8c9eed47d7431860b3116d8:

  Merge tag 'u-boot-dfu-20240209' of 
https://source.denx.de/u-boot/custodians/u-boot-dfu (2024-02-09 09:00:42 -0500)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-sh.git master-779h0-r2

for you to fetch changes up to 53066deccbedf02439309b0d4aca9f0be853a8da:

  ARM: renesas: Add Renesas R8A779H0 V4M Gray Hawk board code (2024-02-10 
17:08:06 +0100)


Duy Nguyen (2):
  dt-bindings: clock: Add R8A779H0 V4M CPG Core Clock Definitions
  dt-bindings: power: Add R8A779H0 V4M SYSC power domain definitions

Hai Pham (8):
  clk: renesas: Add R8A779H0 V4M clock tables
  pinctrl: renesas: Add R8A779H0 V4M PFC tables
  ARM: renesas: Add R8A779H0 V4M Kconfig entry and PRR ID
  mtd: spi: renesas: Add R8A779H0 V4M support
  ARM: dts: renesas: Add Renesas R8A779H0 V4M SoC support
  ARM: dts: renesas: Add Renesas R8A779H0 V4M DT extras
  ARM: dts: renesas: Add Renesas Gray Hawk boards support
  ARM: renesas: Add Renesas R8A779H0 V4M Gray Hawk board code

Marek Vasut (1):
  clk: renesas: Implement R8A779H0 V4M PLL7 support

 arch/arm/dts/Makefile  |3 +-
 arch/arm/dts/r8a779h0-gray-hawk-cpu.dtsi   |  166 +
 arch/arm/dts/r8a779h0-gray-hawk-csi-dsi.dtsi   |   15 +
 arch/arm/dts/r8a779h0-gray-hawk-ethernet.dtsi  |   15 +
 arch/arm/dts/r8a779h0-gray-hawk-u-boot.dts |   42 +
 arch/arm/dts/r8a779h0-gray-hawk.dts|   25 +
 arch/arm/dts/r8a779h0-u-boot.dtsi  |   27 +
 arch/arm/dts/r8a779h0.dtsi |  460 +++
 arch/arm/mach-rmobile/Kconfig.rcar4|   13 +
 arch/arm/mach-rmobile/cpu_info.c   |1 +
 arch/arm/mach-rmobile/include/mach/rmobile.h   |1 +
 board/renesas/grayhawk/Kconfig |   15 +
 board/renesas/grayhawk/MAINTAINERS |5 +
 board/renesas/grayhawk/Makefile|9 +
 board/renesas/grayhawk/grayhawk.c  |   66 +
 configs/r8a779h0_grayhawk_defconfig|   75 +
 drivers/clk/renesas/Kconfig|6 +
 drivers/clk/renesas/Makefile   |1 +
 drivers/clk/renesas/clk-rcar-gen3.c|6 +
 drivers/clk/renesas/r8a779h0-cpg-mssr.c|  293 ++
 drivers/clk/renesas/rcar-gen3-cpg.h|3 +
 drivers/pinctrl/renesas/Kconfig|6 +
 drivers/pinctrl/renesas/Makefile   |1 +
 drivers/pinctrl/renesas/pfc-r8a779h0.c | 3969 
 drivers/pinctrl/renesas/pfc.c  |   11 +
 drivers/pinctrl/renesas/sh_pfc.h   |1 +
 drivers/spi/renesas_rpc_spi.c  |3 +-
 include/configs/grayhawk.h |   14 +
 .../dt-bindings/clock/renesas,r8a779h0-cpg-mssr.h  |   96 +
 include/dt-bindings/power/renesas,r8a779h0-sysc.h  |   49 +
 30 files changed, 5395 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/dts/r8a779h0-gray-hawk-cpu.dtsi
 create mode 100644 arch/arm/dts/r8a779h0-gray-hawk-csi-dsi.dtsi
 create mode 100644 arch/arm/dts/r8a779h0-gray-hawk-ethernet.dtsi
 create mode 100644 arch/arm/dts/r8a779h0-gray-hawk-u-boot.dts
 create mode 100644 arch/arm/dts/r8a779h0-gray-hawk.dts
 create mode 100644 arch/arm/dts/r8a779h0-u-boot.dtsi
 create mode 100644 arch/arm/dts/r8a779h0.dtsi
 create mode 100644 board/renesas/grayhawk/Kconfig
 create mode 100644 board/renesas/grayhawk/MAINTAINERS
 create mode 100644 board/renesas/grayhawk/Makefile
 create mode 100644 board/renesas/grayhawk/grayhawk.c
 create mode 100644 configs/r8a779h0_grayhawk_defconfig
 create mode 100644 drivers/clk/renesas/r8a779h0-cpg-mssr.c
 create mode 100644 drivers/pinctrl/renesas/pfc-r8a779h0.c
 create mode 100644 include/configs/grayhawk.h
 create mode 100644 include/dt-bindings/clock/renesas,r8a779h0-cpg-mssr.h
 create mode 100644 include/dt-bindings/power/renesas,r8a779h0-sysc.h


Re: [PATCH v1] armv8: crypto: SHA-512 using ARMv8 Crypto Extensions

2024-02-11 Thread Igor Opaniuk
Hi Tom,

On Sun, Feb 11, 2024 at 1:37 AM Tom Rini  wrote:
>
> On Sat, Feb 10, 2024 at 01:07:09PM +0100, Igor Opaniuk wrote:
>
> > From: Igor Opaniuk 
> >
> > Add support for the SHA-512 Secure Hash Algorithm which uses ARMv8 Crypto
> > Extensions. The CPU should support ARMv8.2 instruction set and implement
> > SHA512H, SHA512H2, SHA512SU0, and SHA512SU1 instructions.
> >
> > This information can be obtained from ID_AA64ISAR0_EL1 (AArch64 Instruction
> > Set Attribute Register 0), bits [15:12] should be 0b0010 [1], that
> > indicates support for SHA512* instructions in AArch64 state. As not all
> > ARMv8-base SoCs support that, ARMV8_CE_SHA512 is left disabled by
> > default for now.
> >
> > Tested in QEMU for ARMv8 with compiled-in SHA-2 support.
> > Even on emulated cpu the hashing speed increase was visible:
> >
> > With CE usage:
> > => time hash sha512 0x4020 0x200
> > Calculate hash
> > Calculate hash
> > sha512 for 4020 ... 421f ==> 1aeae269f4eb7c37...
> >
> > time: 0.215 seconds
> >
> > Without CE usage:
> > => time hash sha512 0x4020 0x200
> > sha512 for 4020 ... 421f ==> 1aeae269f4eb7c37...
> >
> > time: 0.356 seconds
> >
> > Real HW tests should provide much more improvement and objective results
> > with 10x speed increase at least.
> >
> > The implementation is based on original implementation from Ard Biesheuvel 
> > in
> > Linux kernel [2]
> >
> > [1] 
> > https://developer.arm.com/documentation/ddi0601/2023-12/AArch64-Registers/ID-AA64ISAR0-EL1--AArch64-Instruction-Set-Attribute-Register-0
> > [2] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/crypto/sha2-ce-core.S
> >
> > CC: Ard Biesheuvel 
> > CC: Loic Poulain 
> > Signed-off-by: Igor Opaniuk 
> [snip]
> > diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig
> > index 9f0fb369f7..fd5c26421b 100644
> > --- a/arch/arm/cpu/armv8/Kconfig
> > +++ b/arch/arm/cpu/armv8/Kconfig
> > @@ -204,6 +204,11 @@ config ARMV8_CE_SHA256
> >   bool "SHA-256 digest algorithm (ARMv8 Crypto Extensions)"
> >   default y if SHA256
> >
> > +config ARMV8_CE_SHA512
> > + bool "SHA-512 digest algorithm (ARMv8 Crypto Extensions)"
> > + depends on SHA512
> > + default n
>
> Like the sha256 one, this should be default y I think, the performance
> improvement is likely worth the size increase.
That was done on purpose, as SHA512* instructions (comparing to SHA256*)
were introduced only in ARMv8.2-A, and most of the currently supported ARMv8
SoCs in U-Boot don't support it.
We probably would end up with most of them reporting Synchronous
Abort crashes.

As Marc suggested in the previous email, we should have
both versions compiled-in (sw and hw-accelerated) and dynamic selection of
proper version in runtime based on CPU capabilities, and I plan to
address that in future in a separate patch series.

>
> --
> Tom

Regards,
Igor

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk
Senior Software Engineer, Embedded & Security
E: igor.opan...@foundries.io
W: www.foundries.io


Re: [PATCH v1] armv8: crypto: SHA-512 using ARMv8 Crypto Extensions

2024-02-11 Thread Igor Opaniuk
Hello Marc,

On Sun, Feb 11, 2024 at 12:12 AM Marc Zyngier  wrote:
>
> [Fixing Ard's email address for something more current.]
>
> On Sat, 10 Feb 2024 12:07:09 +,
> Igor Opaniuk  wrote:
> >
> > From: Igor Opaniuk 
> >
> > Add support for the SHA-512 Secure Hash Algorithm which uses ARMv8 Crypto
> > Extensions. The CPU should support ARMv8.2 instruction set and implement
> > SHA512H, SHA512H2, SHA512SU0, and SHA512SU1 instructions.
> >
> > This information can be obtained from ID_AA64ISAR0_EL1 (AArch64 Instruction
> > Set Attribute Register 0), bits [15:12] should be 0b0010 [1], that
> > indicates support for SHA512* instructions in AArch64 state. As not all
> > ARMv8-base SoCs support that, ARMV8_CE_SHA512 is left disabled by
> > default for now.
>
> But since you can actually probe it at runtime, what's the problem?

That actually was my initial plan, I just decided to move one step
one step at a time and address that in the next patch series.

>
> > Tested in QEMU for ARMv8 with compiled-in SHA-2 support.
> > Even on emulated cpu the hashing speed increase was visible:
>
> Unfortunately, QEMU is not a good oracle for optimisations, and is
> more akin to rolling a dice. In your case, you *should* see an
> improvement, but this should be evaluated on bare metal.
I fully agree with you here, but unfortunately it turned out
that the only board with ARMv8.2-ready SoC (Cortex A55). I have now
at hand doesn't support SHA512* instructions, but after all decided
so send the patch to get rid of the feeling that it was all in vain  :)

Maybe I added a bit of confusion to the commit message,
as the initial idea was about functional validation (that it works in
QEMU at least). I didn't want to make any comparison
in a virtualized environment as it obviously didn't make any sense.

>
> >
> > With CE usage:
> > => time hash sha512 0x4020 0x200
> > Calculate hash
> > Calculate hash
> > sha512 for 4020 ... 421f ==> 1aeae269f4eb7c37...
> >
> > time: 0.215 seconds
> >
> > Without CE usage:
> > => time hash sha512 0x4020 0x200
> > sha512 for 4020 ... 421f ==> 1aeae269f4eb7c37...
> >
> > time: 0.356 seconds
> >
> > Real HW tests should provide much more improvement and objective results
> > with 10x speed increase at least.
> >
> > The implementation is based on original implementation from Ard Biesheuvel 
> > in
> > Linux kernel [2]
> >
> > [1] 
> > https://developer.arm.com/documentation/ddi0601/2023-12/AArch64-Registers/ID-AA64ISAR0-EL1--AArch64-Instruction-Set-Attribute-Register-0
> > [2] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/crypto/sha2-ce-core.S
> >
> > CC: Ard Biesheuvel 
> > CC: Loic Poulain 
> > Signed-off-by: Igor Opaniuk 
> > ---
> >
> >  arch/arm/cpu/armv8/Kconfig  |   5 +
> >  arch/arm/cpu/armv8/Makefile |   1 +
> >  arch/arm/cpu/armv8/sha512_ce_core.S | 210 
> >  arch/arm/cpu/armv8/sha512_ce_glue.c |  20 +++
> >  lib/sha512.c|   6 +-
> >  5 files changed, 240 insertions(+), 2 deletions(-)
> >  create mode 100644 arch/arm/cpu/armv8/sha512_ce_core.S
> >  create mode 100644 arch/arm/cpu/armv8/sha512_ce_glue.c
> >
> > diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig
> > index 9f0fb369f7..fd5c26421b 100644
> > --- a/arch/arm/cpu/armv8/Kconfig
> > +++ b/arch/arm/cpu/armv8/Kconfig
> > @@ -204,6 +204,11 @@ config ARMV8_CE_SHA256
> >   bool "SHA-256 digest algorithm (ARMv8 Crypto Extensions)"
> >   default y if SHA256
> >
> > +config ARMV8_CE_SHA512
> > + bool "SHA-512 digest algorithm (ARMv8 Crypto Extensions)"
> > + depends on SHA512
> > + default n
> > +
> >  endif
> >
> >  endif
> > diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
> > index bba4f570db..3894f2bb2a 100644
> > --- a/arch/arm/cpu/armv8/Makefile
> > +++ b/arch/arm/cpu/armv8/Makefile
> > @@ -45,3 +45,4 @@ obj-$(CONFIG_TARGET_BCMNS3) += bcmns3/
> >  obj-$(CONFIG_XEN) += xen/
> >  obj-$(CONFIG_ARMV8_CE_SHA1) += sha1_ce_glue.o sha1_ce_core.o
> >  obj-$(CONFIG_ARMV8_CE_SHA256) += sha256_ce_glue.o sha256_ce_core.o
> > +obj-$(CONFIG_ARMV8_CE_SHA512) += sha512_ce_glue.o sha512_ce_core.o
> > \ No newline at end of file
> > diff --git a/arch/arm/cpu/armv8/sha512_ce_core.S 
> > b/arch/arm/cpu/armv8/sha512_ce_core.S
> > new file mode 100644
> > index 00..906291f35b
> > --- /dev/null
> > +++ b/arch/arm/cpu/armv8/sha512_ce_core.S
> > @@ -0,0 +1,210 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * sha512-ce-core.S - core SHA-384/SHA-512 transform using v8 Crypto
> > + * Extensions
> > + *
> > + * Copyright (C) 2018 Linaro Ltd 
> > + * Copyright (C) 2024 Igor Opaniuk 
> > + */
> > +
> > + #include 
> > + #include 
> > + #include 
> > + #include 
> > +
> > + .macro  adr_l, dst, sym
> > + adrp\dst, \sym
> > + add \dst, \dst, :lo12:\sym
> > + .endm
> > +
> > + .irp

Re: [PATCH] sunxi: defconfig: Add pstore support for pinephone

2024-02-11 Thread Andrey Skvortsov
Hi Peter,

On 24-02-11 16:11, Peter Robinson wrote:
> On Sun, 11 Feb 2024 at 16:02, Andrey Skvortsov
>  wrote:
> >
> > In general any DRAM address, that isn't overwritten during a boot is
> > suitable for pstore.
> 
> What's this functionality providing? The details below give the
> technical reasons for the memory address but I'm not sure what
> enabling pstore provides to PinePhone users.
>
Good point. pstore allows users to catch kernel crashes and report
them to developers. It's enabled by default in distribution kernels
like Debian.

CONFIG_PSTORE=y
CONFIG_PSTORE_RAM=m

systemd has service that automatically handles pstore and saves them
in /var/lib/pstore for later usage.

Modern (Android) phones have pstore enabled usually to get information
about kernel crash, since it's the simplest way to get kernel
backtrace on mobile device without serial console.

> > Range from 0x4000 - 0x5000 is heavily used by u-boot for
> > internal use and to load kernel, fdt, fdto, scripts, pxefile and ramdisk
> > later in the boot process. Ramdisk start address is 0x4FF0,
> > initramfs for kernel with some hacking features and debug info enabled
> > can take more than 100Mb and final address will be around 0x5800.
> > Address 0x6100 will most likely not overlap with that.
> >
> > Signed-off-by: Andrey Skvortsov 
> > ---
> >  configs/pinephone_defconfig | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
> > index 9d39204a43..7b80cc3131 100644
> > --- a/configs/pinephone_defconfig
> > +++ b/configs/pinephone_defconfig
> > @@ -10,6 +10,8 @@ CONFIG_DRAM_ZQ=3881949
> >  CONFIG_MMC_SUNXI_SLOT_EXTRA=2
> >  CONFIG_PINEPHONE_DT_SELECTION=y
> >  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> > +CONFIG_CMD_PSTORE=y
> > +CONFIG_CMD_PSTORE_MEM_ADDR=0x6100
> >  CONFIG_OF_LIST="sun50i-a64-pinephone-1.1 sun50i-a64-pinephone-1.2"
> >  CONFIG_LED_STATUS=y
> >  CONFIG_LED_STATUS_GPIO=y
> > --
> > 2.43.0
> >

-- 
Best regards,
Andrey Skvortsov


Re: [PATCH] sunxi: defconfig: Add pstore support for pinephone

2024-02-11 Thread Peter Robinson
On Sun, 11 Feb 2024 at 16:02, Andrey Skvortsov
 wrote:
>
> In general any DRAM address, that isn't overwritten during a boot is
> suitable for pstore.

What's this functionality providing? The details below give the
technical reasons for the memory address but I'm not sure what
enabling pstore provides to PinePhone users.

> Range from 0x4000 - 0x5000 is heavily used by u-boot for
> internal use and to load kernel, fdt, fdto, scripts, pxefile and ramdisk
> later in the boot process. Ramdisk start address is 0x4FF0,
> initramfs for kernel with some hacking features and debug info enabled
> can take more than 100Mb and final address will be around 0x5800.
> Address 0x6100 will most likely not overlap with that.
>
> Signed-off-by: Andrey Skvortsov 
> ---
>  configs/pinephone_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
> index 9d39204a43..7b80cc3131 100644
> --- a/configs/pinephone_defconfig
> +++ b/configs/pinephone_defconfig
> @@ -10,6 +10,8 @@ CONFIG_DRAM_ZQ=3881949
>  CONFIG_MMC_SUNXI_SLOT_EXTRA=2
>  CONFIG_PINEPHONE_DT_SELECTION=y
>  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> +CONFIG_CMD_PSTORE=y
> +CONFIG_CMD_PSTORE_MEM_ADDR=0x6100
>  CONFIG_OF_LIST="sun50i-a64-pinephone-1.1 sun50i-a64-pinephone-1.2"
>  CONFIG_LED_STATUS=y
>  CONFIG_LED_STATUS_GPIO=y
> --
> 2.43.0
>


[PATCH] sunxi: defconfig: Add pstore support for pinephone

2024-02-11 Thread Andrey Skvortsov
In general any DRAM address, that isn't overwritten during a boot is
suitable for pstore.

Range from 0x4000 - 0x5000 is heavily used by u-boot for
internal use and to load kernel, fdt, fdto, scripts, pxefile and ramdisk
later in the boot process. Ramdisk start address is 0x4FF0,
initramfs for kernel with some hacking features and debug info enabled
can take more than 100Mb and final address will be around 0x5800.
Address 0x6100 will most likely not overlap with that.

Signed-off-by: Andrey Skvortsov 
---
 configs/pinephone_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
index 9d39204a43..7b80cc3131 100644
--- a/configs/pinephone_defconfig
+++ b/configs/pinephone_defconfig
@@ -10,6 +10,8 @@ CONFIG_DRAM_ZQ=3881949
 CONFIG_MMC_SUNXI_SLOT_EXTRA=2
 CONFIG_PINEPHONE_DT_SELECTION=y
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
+CONFIG_CMD_PSTORE=y
+CONFIG_CMD_PSTORE_MEM_ADDR=0x6100
 CONFIG_OF_LIST="sun50i-a64-pinephone-1.1 sun50i-a64-pinephone-1.2"
 CONFIG_LED_STATUS=y
 CONFIG_LED_STATUS_GPIO=y
-- 
2.43.0



Re: [PATCH] doc: rockchip: Clarify the rkspi image format further

2024-02-11 Thread Johan Jonker



On 2/10/24 19:18, Dragan Simic wrote:
> As discussed on the U-Boot mailing list, [1][2] only some Rockchip SoCs
> suffer from a bug in their BROMs that requires a specific format for their

This not a bug.

> SPI images, which was the reason for the rkspi format to be introduced.
> 
> Improve the description of the rkspi format a bit to mention this, for
> future reference and to make understanding it easier.
> 
> [1] 
> https://lore.kernel.org/u-boot/c32129ba-db25-4b9d-9a4a-032d88dfb...@kwiboo.se/
> [2] 
> https://lore.kernel.org/u-boot/CACdvmAjfCWicRd=lkkyob7fzo79afkuqky0e1hbb0zyjuoo...@mail.gmail.com/
> 
> Signed-off-by: Dragan Simic 
> ---
>  doc/README.rockchip | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/README.rockchip b/doc/README.rockchip
> index 84caff8a24d3..e0c5c395ec6a 100644
> --- a/doc/README.rockchip
> +++ b/doc/README.rockchip
> @@ -649,7 +649,12 @@ sector is used. The header is the same as with rksd and 
> the maximum size is

>  also 32KB (before spreading). The image should be written to the start of
>  SPI flash.
>  

On rk3066 an image should start before page address:
NAND : 50
SPI  : 13
EMMC :  1

Not sure about start limits for other SoC's. (Kever ??)

Also rk3066 and rk3188 use RC4 crypto.

> -See above for instructions on how to write a SPI image.
> +Only the BROMs of some Rockchip SoCs, such as the RK3399, suffer from a bug
> +that mandates the above-described data spreading, thus requiring the rkspi
> +format to be used for their SPI images. Rockchip SoCs that don't suffer
> +from this bug use the rksd format for their SPI images.
> +

This not a bug. It's a design choice to reuse the reading function in the boot 
ROM shared by EMMC, NAND and SPI etc. due to size restrictions.

See NAND example:
https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/blob/master/drivers/block/rkmtd.c?ref_type=heads#L259

This NAND code also applies to SPI with minor tweaks.
Code will never read more then 2k regardless of the page size.

===
This info must be written in a more neutral wording.(no bugs involved)
Just describe the 2 formats and the SoC's that use it.

Johan

===
Extra:

Could someone cleanup README.rockchip
The file README.rockchip has most building scenarios covered by now.
Please have a look if all the boot log listing in that file are still needed or 
can be removed. 

===

Is there a need for SPI programmer code a la RKMTD with common base and block 
interface tools?

===

Is someone able to extract rk3568 boot ROM code in order to study NAND boot 
behavior for a future RKMTD NFC V9 upgrade?

> +See the section above for instructions on how to write an SPI image.
>  
>  rkmux.py
>  
> 


Re: [PATCH] doc: rockchip: Clarify the rkspi image format further

2024-02-11 Thread Dragan Simic

Hello Johan,

On 2024-02-11 15:07, Johan Jonker wrote:

On 2/10/24 19:18, Dragan Simic wrote:
As discussed on the U-Boot mailing list, [1][2] only some Rockchip 
SoCs
suffer from a bug in their BROMs that requires a specific format for 
their


This not a bug.

SPI images, which was the reason for the rkspi format to be 
introduced.


Improve the description of the rkspi format a bit to mention this, for
future reference and to make understanding it easier.

[1] 
https://lore.kernel.org/u-boot/c32129ba-db25-4b9d-9a4a-032d88dfb...@kwiboo.se/
[2] 
https://lore.kernel.org/u-boot/CACdvmAjfCWicRd=lkkyob7fzo79afkuqky0e1hbb0zyjuoo...@mail.gmail.com/


Signed-off-by: Dragan Simic 
---
 doc/README.rockchip | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/doc/README.rockchip b/doc/README.rockchip
index 84caff8a24d3..e0c5c395ec6a 100644
--- a/doc/README.rockchip
+++ b/doc/README.rockchip
@@ -649,7 +649,12 @@ sector is used. The header is the same as with 
rksd and the maximum size is


 also 32KB (before spreading). The image should be written to the 
start of

 SPI flash.



On rk3066 an image should start before page address:
NAND : 50
SPI  : 13
EMMC :  1

Not sure about start limits for other SoC's. (Kever ??)

Also rk3066 and rk3188 use RC4 crypto.


-See above for instructions on how to write a SPI image.
+Only the BROMs of some Rockchip SoCs, such as the RK3399, suffer from 
a bug
+that mandates the above-described data spreading, thus requiring the 
rkspi
+format to be used for their SPI images. Rockchip SoCs that don't 
suffer

+from this bug use the rksd format for their SPI images.
+


This not a bug. It's a design choice to reuse the reading function in
the boot ROM shared by EMMC, NAND and SPI etc. due to size
restrictions.

See NAND example:
https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/blob/master/drivers/block/rkmtd.c?ref_type=heads#L259

This NAND code also applies to SPI with minor tweaks.
Code will never read more then 2k regardless of the page size.

===
This info must be written in a more neutral wording.(no bugs involved)
Just describe the 2 formats and the SoC's that use it.


Thank you for the clarification!  I'll make sure to get the
wording adjusted to be more neutral.


===
Extra:

Could someone cleanup README.rockchip
The file README.rockchip has most building scenarios covered by now.
Please have a look if all the boot log listing in that file are still
needed or can be removed.


There's much more in README.rockchip that should be cleaned
up and refreshed a bit.  I'll convert the file into the RST
format and do a few cleanups.  After that, everyone will be
more than welcome to perform more cleanups.


===

Is there a need for SPI programmer code a la RKMTD with common base
and block interface tools?

===

Is someone able to extract rk3568 boot ROM code in order to study NAND
boot behavior for a future RKMTD NFC V9 upgrade?


+See the section above for instructions on how to write an SPI image.

 rkmux.py
 



[PATCH v4] rng: Add Turris Mox rTWM RNG driver

2024-02-11 Thread Max Resch
A RNG driver for Armada 3720 boards running the Turris Mox rWTM firmware
from CZ.NIC in the secure processor.

Signed-off-by: Max Resch 
---

Changes in v4:
 - wrongful/missing git rebase

Changes in v3:
 - More meaningful variable names in accordance with review

Changes in v2:
 - Removed ring buffer implementation

 drivers/rng/Kconfig   |   8 +++
 drivers/rng/Makefile  |   1 +
 drivers/rng/turris_rwtm_rng.c | 122 ++
 3 files changed, 131 insertions(+)
 create mode 100644 drivers/rng/turris_rwtm_rng.c

diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
index a89c899568..cd72852a47 100644
--- a/drivers/rng/Kconfig
+++ b/drivers/rng/Kconfig
@@ -105,4 +105,12 @@ config RNG_JH7110
help
  Enable True Random Number Generator in StarFive JH7110 SoCs.
 
+config RNG_TURRIS_RWTM
+   bool "Turris Mox TRNG in Secure Processor"
+   depends on DM_RNG && ARMADA_3700
+   help
+ Use TRNG in Turris Mox Secure Processor Firmware. Can be used
+ on other Armada-3700 devices (like EspressoBin) if Secure
+ Firmware from CZ.NIC is used.
+
 endif
diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
index 7e64c4cdfc..ecae1a3da3 100644
--- a/drivers/rng/Makefile
+++ b/drivers/rng/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_RNG_SMCCC_TRNG) += smccc_trng.o
 obj-$(CONFIG_RNG_ARM_RNDR) += arm_rndr.o
 obj-$(CONFIG_TPM_RNG) += tpm_rng.o
 obj-$(CONFIG_RNG_JH7110) += jh7110_rng.o
+obj-$(CONFIG_RNG_TURRIS_RWTM) += turris_rwtm_rng.o
diff --git a/drivers/rng/turris_rwtm_rng.c b/drivers/rng/turris_rwtm_rng.c
new file mode 100644
index 00..ec2cb0bca3
--- /dev/null
+++ b/drivers/rng/turris_rwtm_rng.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause
+/*
+ * Copyright (c) 2024, Max Resch
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* size of entropy buffer */
+#define RNG_BUFFER_SIZE128U
+
+struct turris_rwtm_rng_priv {
+   phys_addr_t buffer;
+};
+
+static int turris_rwtm_rng_fill_entropy(phys_addr_t entropy, size_t size)
+{
+   u32 args[3] = { 1, (u32)entropy, size };
+   int ret;
+
+   /* flush data cache */
+   flush_dcache_range(entropy, entropy + size);
+
+   /*
+* get entropy
+* args[0] = 1 copies BYTES array in args[1] of length args[2]
+*/
+   ret = mbox_do_cmd(MBOX_CMD_GET_RANDOM, args, 3, NULL, 0);
+   if (ret < 0)
+   return ret;
+
+   /* invalidate data cache */
+   invalidate_dcache_range(entropy, entropy + size);
+
+   return 0;
+}
+
+static int turris_rwtm_rng_random_read(struct udevice *dev, void *data, size_t 
count)
+{
+   struct turris_rwtm_rng_priv *priv = dev_get_priv(dev);
+   phys_addr_t phys;
+   size_t size;
+   int ret;
+
+   phys = priv->buffer;
+
+   while (count) {
+   size = min_t(size_t, RNG_BUFFER_SIZE, count);
+
+   ret = turris_rwtm_rng_fill_entropy(phys, size);
+
+   memcpy(data, (void *)phys, size);
+   count -= size;
+   data = (u8 *)data + size;
+   }
+
+   return 0;
+}
+
+static int turris_rwtm_rng_probe(struct udevice *dev)
+{
+   struct turris_rwtm_rng_priv *priv = dev_get_priv(dev);
+   u32 args[] = { 0 };
+   int ret;
+
+   /*
+* check if the random command is supported
+* args[0] = 0 would copy 16 DWORDS entropy to out but we ignore them
+*/
+   ret = mbox_do_cmd(MBOX_CMD_GET_RANDOM, args, ARRAY_SIZE(args), NULL, 0);
+
+   if (ret < 0)
+   return ret;
+
+   /* entropy buffer */
+   priv->buffer = 0;
+
+   /* buffer address need to be aligned */
+   dma_alloc_coherent(RNG_BUFFER_SIZE, (unsigned long *)>buffer);
+   if (!priv->buffer)
+   return -ENOMEM;
+
+   return 0;
+}
+
+static int turris_rwtm_rng_remove(struct udevice *dev)
+{
+   struct turris_rwtm_rng_priv *priv = dev_get_priv(dev);
+   phys_addr_t phys = priv->buffer;
+
+   dma_free_coherent((void *)phys);
+
+   return 0;
+}
+
+static const struct dm_rng_ops turris_rwtm_rng_ops = {
+   .read = turris_rwtm_rng_random_read,
+};
+
+/*
+ * only Turris MOX firmware has the RNG but allow all probable devices to be
+ * probed the default firmware will just reject the probe
+ */
+static const struct udevice_id turris_rwtm_rng_match[] = {
+   { .compatible = "cznic,turris-mox-rwtm" },
+   { .compatible = "marvell,armada-3700-rwtm-firmware" },
+   {},
+};
+
+U_BOOT_DRIVER(turris_rwtm_rng) = {
+   .name   = "turris-rwtm-rng",
+   .id = UCLASS_RNG,
+   .of_match = turris_rwtm_rng_match,
+   .ops= _rwtm_rng_ops,
+   .probe  = turris_rwtm_rng_probe,
+   .remove = turris_rwtm_rng_remove,
+   .priv_auto = sizeof(struct turris_rwtm_rng_priv),
+};
-- 
2.43.0



Re: [PATCH RFC] sunxi: pinephone: detect existed magnetometer and fixup dtb

2024-02-11 Thread Andre Przywara
On Sun, 11 Feb 2024 12:28:24 +0300
Andrey Skvortsov  wrote:

Hi Andrey,

thanks for taking care of this upstream!

> In newer 1.2 PinePhone board revisions LIS3MDL magnetometer was replaced by
> AF8133J. They use the same PB1 pin in different modes.
> 
> LIS3MDL uses it as an gpio input to handle interrupt.
> AF8133J uses it as an gpio output as a reset signal.
> 
> It wasn't possible at runtime to enable both device tree
> nodes and detect supported sensor at probe time.
> 
> AF8133J has reset pin (PB1) connected to the SoC. By default AF8133J
> is in a reset state and don't respond to probe request on I2C
> bus. Extra code would be needed to handle reset signal. Therefore this
> code uses LIS3MDL magnetometer instead of AF8133J.

Thanks for the research and explanation, that makes it much easier to
reason about!

> Introducing new dts 1.2b with AF8133J sensor would require probing in
> SPL. That would lead to pulling in into SPL I2C controller driver,
> RSB controller driver, introducing new AXP803 driver to power-up
> sensors for probe. It's working, but SPL is pretty size-constrained on
> A64 and doesn't have much space. Therefore fdt fixup is done in U-Boot
> proper without introducing new board revision and new dts.

I agree on that, especially if it's indeed just a matter of flipping
two "status" properties.

> Signed-off-by: Andrey Skvortsov 
> ---
> 
> AF8133J's driver isn't upstreamed yet, but it will be soon. Therefore this
> is RFC patch. I'd like to know whether selected approach will be accepted
> in u-boot before submiting coresponding dts changes to the Linux kernel.
> Any feedback on this change would be very welcome.

So I think this is the right approach: Since the SPL has no use of this
device, and it's a rather small DT change, we definitely want to do this
in U-Boot proper. And I believe that U-Boot (proper) is indeed the best
place to do those adjustments, since it's built for one board anyway,
so anything board specific belongs into there (and not into TF-A or the
SPL, which are more tailored to one *SoC*). Also we are not so size
sensitive in proper.
And I also like the fact that it's protected by a board specific
definition, and even better that it's an already existing one.

Oh, and I am not really convinced this is the right approach here, but
maybe have a look at doc/usage/cmd/extension.rst, for another related
mechanism.

Some smaller comments below ...

> 
>  board/sunxi/board.c | 26 ++
>  configs/pinephone_defconfig |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 1305302060..a4bfa24400 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -920,6 +921,28 @@ static void bluetooth_dt_fixup(void *blob)
>  "local-bd-address", bdaddr, ETH_ALEN, 1);
>  }
>  
> +#ifdef CONFIG_PINEPHONE_DT_SELECTION

Can you move that line into the function? That would for one avoid the
protection in the caller below, and secondly make it easier to add
other boards, if a need arises for them. The two #defines don't hurt or
change the code, so just keep them outside.

> +
> +#define PINEPHONE_LIS3MDL_I2C_ADDR   0x1E
> +#define PINEPHONE_LIS3MDL_I2C_BUS1 /* I2C1 */
> +
> +static   void board_dt_fixup(void *blob)
> +{
> + struct udevice *bus, *dev;
> +

(have the #ifdef here)

> + if (!fdt_node_check_compatible(blob, 0, "pine64,pinephone-1.2"))

Please have curly braces around that.

So I'd say please send the (disabled) DT node addition patch to the
kernel MLs, then send this patch to U-Boot.

Cheers,
Andre

> + if (!uclass_get_device_by_seq(UCLASS_I2C, 
> PINEPHONE_LIS3MDL_I2C_BUS, )) {
> + dm_i2c_probe(bus, PINEPHONE_LIS3MDL_I2C_ADDR, 0, );
> + fdt_set_status_by_compatible(
> + blob, "st,lis3mdl-magn",
> + dev ? FDT_STATUS_OKAY  : FDT_STATUS_DISABLED);
> + fdt_set_status_by_compatible(
> + blob, "voltafield,af8133j",
> + dev ? FDT_STATUS_DISABLED : FDT_STATUS_OKAY);
> + }
> +}
> +#endif
> +
>  int ft_board_setup(void *blob, struct bd_info *bd)
>  {
>   int __maybe_unused r;
> @@ -934,6 +957,9 @@ int ft_board_setup(void *blob, struct bd_info *bd)
>  
>   bluetooth_dt_fixup(blob);
>  
> +#ifdef CONFIG_PINEPHONE_DT_SELECTION
> + board_dt_fixup(blob);
> +#endif
>  #ifdef CONFIG_VIDEO_DT_SIMPLEFB
>   r = sunxi_simplefb_setup(blob);
>   if (r)
> diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
> index eebc676901..457e7ee1e7 100644
> --- a/configs/pinephone_defconfig
> +++ b/configs/pinephone_defconfig
> @@ -12,6 +12,7 @@ CONFIG_MMC_SUNXI_SLOT_EXTRA=2
>  CONFIG_PINEPHONE_DT_SELECTION=y
>  # 

Re: [PATCH] clk: renesas: Fix broken clocks on all Gen2 boards

2024-02-11 Thread Geert Uytterhoeven
Hi Niklas,

Thanks for your patch!

On Fri, Feb 9, 2024 at 10:22 PM Niklas Söderlund
 wrote:
> To prepare support for multiple register layouts pointers to register
> tables where added to struct cpg_mssr_info. These pointers are suppose

supposed

> to be filled in at probe time and no intended change in behavior was
> intended.
>
> However the new pointers where only filled in by some paths of the

were

> driver implemented in clk-rcar-gen3.c. The path implemented in
> clk-rcar-gen2.c was not updated leaving the pointers uninitialized
> leading to a crash when trying to probe the clocks.
>
> Fix this by filling in the pointers in the Gen2 code path with the
> values used before they where moved to struct cpg_mssr_info.

were

> Fixes: d413214fb748 ("clk: renesas: Add register pointers into struct 
> cpg_mssr_info")
> Signed-off-by: Niklas Söderlund 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


[PATCH RFC] sunxi: pinephone: detect existed magnetometer and fixup dtb

2024-02-11 Thread Andrey Skvortsov
In newer 1.2 PinePhone board revisions LIS3MDL magnetometer was replaced by
AF8133J. They use the same PB1 pin in different modes.

LIS3MDL uses it as an gpio input to handle interrupt.
AF8133J uses it as an gpio output as a reset signal.

It wasn't possible at runtime to enable both device tree
nodes and detect supported sensor at probe time.

AF8133J has reset pin (PB1) connected to the SoC. By default AF8133J
is in a reset state and don't respond to probe request on I2C
bus. Extra code would be needed to handle reset signal. Therefore this
code uses LIS3MDL magnetometer instead of AF8133J.

Introducing new dts 1.2b with AF8133J sensor would require probing in
SPL. That would lead to pulling in into SPL I2C controller driver,
RSB controller driver, introducing new AXP803 driver to power-up
sensors for probe. It's working, but SPL is pretty size-constrained on
A64 and doesn't have much space. Therefore fdt fixup is done in U-Boot
proper without introducing new board revision and new dts.

Signed-off-by: Andrey Skvortsov 
---

AF8133J's driver isn't upstreamed yet, but it will be soon. Therefore this
is RFC patch. I'd like to know whether selected approach will be accepted
in u-boot before submiting coresponding dts changes to the Linux kernel.
Any feedback on this change would be very welcome.

 board/sunxi/board.c | 26 ++
 configs/pinephone_defconfig |  1 +
 2 files changed, 27 insertions(+)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 1305302060..a4bfa24400 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -920,6 +921,28 @@ static void bluetooth_dt_fixup(void *blob)
   "local-bd-address", bdaddr, ETH_ALEN, 1);
 }
 
+#ifdef CONFIG_PINEPHONE_DT_SELECTION
+
+#define PINEPHONE_LIS3MDL_I2C_ADDR 0x1E
+#define PINEPHONE_LIS3MDL_I2C_BUS  1 /* I2C1 */
+
+static void board_dt_fixup(void *blob)
+{
+   struct udevice *bus, *dev;
+
+   if (!fdt_node_check_compatible(blob, 0, "pine64,pinephone-1.2"))
+   if (!uclass_get_device_by_seq(UCLASS_I2C, 
PINEPHONE_LIS3MDL_I2C_BUS, )) {
+   dm_i2c_probe(bus, PINEPHONE_LIS3MDL_I2C_ADDR, 0, );
+   fdt_set_status_by_compatible(
+   blob, "st,lis3mdl-magn",
+   dev ? FDT_STATUS_OKAY  : FDT_STATUS_DISABLED);
+   fdt_set_status_by_compatible(
+   blob, "voltafield,af8133j",
+   dev ? FDT_STATUS_DISABLED : FDT_STATUS_OKAY);
+   }
+}
+#endif
+
 int ft_board_setup(void *blob, struct bd_info *bd)
 {
int __maybe_unused r;
@@ -934,6 +957,9 @@ int ft_board_setup(void *blob, struct bd_info *bd)
 
bluetooth_dt_fixup(blob);
 
+#ifdef CONFIG_PINEPHONE_DT_SELECTION
+   board_dt_fixup(blob);
+#endif
 #ifdef CONFIG_VIDEO_DT_SIMPLEFB
r = sunxi_simplefb_setup(blob);
if (r)
diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
index eebc676901..457e7ee1e7 100644
--- a/configs/pinephone_defconfig
+++ b/configs/pinephone_defconfig
@@ -12,6 +12,7 @@ CONFIG_MMC_SUNXI_SLOT_EXTRA=2
 CONFIG_PINEPHONE_DT_SELECTION=y
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
 CONFIG_OF_LIST="sun50i-a64-pinephone-1.1 sun50i-a64-pinephone-1.2"
+CONFIG_SYS_I2C_MVTWSI=y
 CONFIG_LED_STATUS=y
 CONFIG_LED_STATUS_GPIO=y
 CONFIG_LED_STATUS0=y
-- 
2.43.0



Re: [PATCH] doc: rockchip: Clarify the rkspi image format further

2024-02-11 Thread Dragan Simic

Hello Tom,

On 2024-02-11 00:41, Tom Rini wrote:

On Sat, Feb 10, 2024 at 07:18:10PM +0100, Dragan Simic wrote:
As discussed on the U-Boot mailing list, [1][2] only some Rockchip 
SoCs
suffer from a bug in their BROMs that requires a specific format for 
their
SPI images, which was the reason for the rkspi format to be 
introduced.


Improve the description of the rkspi format a bit to mention this, for
future reference and to make understanding it easier.

[1] 
https://lore.kernel.org/u-boot/c32129ba-db25-4b9d-9a4a-032d88dfb...@kwiboo.se/
[2] 
https://lore.kernel.org/u-boot/CACdvmAjfCWicRd=lkkyob7fzo79afkuqky0e1hbb0zyjuoo...@mail.gmail.com/


Signed-off-by: Dragan Simic 
---
 doc/README.rockchip | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)


Can we please convert this content to rst under doc/board/rockchip/ 
then

enhance it?


Sure, I'll do that, but it will take some time.