[PATCH v2 6/6] test: py: fru: add a test for the fru command

2022-08-15 Thread Jae Hyun Yoo
Add a simple test for the 'fru' command.

Signed-off-by: Jae Hyun Yoo 
---
Changes from v1:
 * Newly added in v2. (Heinrich)

 configs/sandbox64_defconfig|  1 +
 configs/sandbox_defconfig  |  1 +
 configs/sandbox_flattree_defconfig |  1 +
 configs/sandbox_noinst_defconfig   |  1 +
 configs/sandbox_spl_defconfig  |  1 +
 configs/sandbox_vpl_defconfig  |  1 +
 test/py/tests/test_fru.py  | 47 ++
 7 files changed, 53 insertions(+)
 create mode 100644 test/py/tests/test_fru.py

diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 6553568e7684..9059e3c1c91d 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -83,6 +83,7 @@ CONFIG_CMD_CBFS=y
 CONFIG_CMD_CRAMFS=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_MTDPARTS=y
+CONFIG_CMD_FRU=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index eba7bcbb483b..293e39706cf2 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -115,6 +115,7 @@ CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_SQUASHFS=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_CMD_STACKPROTECTOR_TEST=y
+CONFIG_CMD_FRU=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
diff --git a/configs/sandbox_flattree_defconfig 
b/configs/sandbox_flattree_defconfig
index 6d62feeb08a9..e71379599140 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -66,6 +66,7 @@ CONFIG_CMD_REGULATOR=y
 CONFIG_CMD_TPM=y
 CONFIG_CMD_TPM_TEST=y
 CONFIG_CMD_EXT4_WRITE=y
+CONFIG_CMD_FRU=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 9ee70c29c1a5..0b7f8bd9b3cc 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -84,6 +84,7 @@ CONFIG_CMD_TPM_TEST=y
 CONFIG_CMD_CBFS=y
 CONFIG_CMD_CRAMFS=y
 CONFIG_CMD_EXT4_WRITE=y
+CONFIG_CMD_FRU=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index ec2d26d4436b..05235ec1c493 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -84,6 +84,7 @@ CONFIG_CMD_TPM_TEST=y
 CONFIG_CMD_CBFS=y
 CONFIG_CMD_CRAMFS=y
 CONFIG_CMD_EXT4_WRITE=y
+CONFIG_CMD_FRU=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
diff --git a/configs/sandbox_vpl_defconfig b/configs/sandbox_vpl_defconfig
index 0d946b4ad779..7d4b808a5b49 100644
--- a/configs/sandbox_vpl_defconfig
+++ b/configs/sandbox_vpl_defconfig
@@ -96,6 +96,7 @@ CONFIG_CMD_TPM_TEST=y
 CONFIG_CMD_CBFS=y
 CONFIG_CMD_CRAMFS=y
 CONFIG_CMD_EXT4_WRITE=y
+CONFIG_CMD_FRU=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
diff --git a/test/py/tests/test_fru.py b/test/py/tests/test_fru.py
new file mode 100644
index ..e5e1d7d00639
--- /dev/null
+++ b/test/py/tests/test_fru.py
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+
+import pytest
+import u_boot_utils
+
+@pytest.mark.buildconfigspec('cmd_fru')
+def test_fru_board(u_boot_console):
+"""Test that fru command generates and captures board FRU information as
+expected."""
+
+ram_base = u_boot_utils.find_ram_base(u_boot_console)
+addr = '0x%x' % ram_base
+expected_response = 'rc:0'
+response = u_boot_console.run_command('fru generate -b ' + addr + ' abcd 
efgh ijkl mnop qrst uvwx; echo rc:$?')
+assert(expected_response in response)
+response = u_boot_console.run_command('fru capture ' + addr + '; echo 
rc:$?')
+assert(expected_response in response)
+response = u_boot_console.run_command('fru display')
+assert('Manufacturer Name: abcd' in response)
+assert('Product Name: efgh' in response)
+assert('Serial Number: ijkl' in response)
+assert('Part Number: mnop' in response)
+assert('File ID: qrst' in response)
+assert('Custom Type/Length: 0xc4' in response)
+
+@pytest.mark.buildconfigspec('cmd_fru')
+def test_fru_product(u_boot_console):
+"""Test that fru command generates and captures product FRU information as
+expected."""
+
+ram_base = u_boot_utils.find_ram_base(u_boot_console)
+addr = '0x%x' % ram_base
+expected_response = 'rc:0'
+response = u_boot_console.run_command('fru generate -p ' + addr + ' abcd 
efgh ijkl mnop qrst uvwx yz01 2345; echo rc:$?')
+assert(expected_response in response)
+response = u_boot_console.run_command('fru capture ' + addr + '; echo 
rc:$?')
+assert(expected_response in response)
+response = u_boot_console.run_command('fru display')
+assert('Manufacturer Name: abcd' in response)
+assert('Product Name: efgh' in response)
+assert('Part Number: ijkl' in response)
+assert('Version Number: mnop' in response)
+assert('Serial Number: qrst' 

[PATCH v2 4/6] cmd: fru: add product info area parsing support

2022-08-15 Thread Jae Hyun Yoo
Add product info area parsing support. Custom board fields can be
added dynamically using linked list so that each board support can
utilize them in their own custom way.

Signed-off-by: Jae Hyun Yoo 
---
Changes from v1:
 * Refactored using linked list instead of calling a custom parsing callback.

Changes from RFC:
 * Added manufacturer custom product info fields parsing flow.

 cmd/fru.c |  27 --
 include/fru.h |  32 ++-
 lib/fru_ops.c | 240 +++---
 3 files changed, 282 insertions(+), 17 deletions(-)

diff --git a/cmd/fru.c b/cmd/fru.c
index 11466339da6a..42bdaae09449 100644
--- a/cmd/fru.c
+++ b/cmd/fru.c
@@ -46,9 +46,19 @@ static int do_fru_generate(struct cmd_tbl *cmdtp, int flag, 
int argc,
int (*fru_generate)(const void *addr, int argc, char *const argv[]);
unsigned long addr;
const void *buf;
-   int ret;
+   int ret, maxargs;
+
+   if (!strncmp(argv[2], "-b", 3)) {
+   fru_generate = fru_board_generate;
+   maxargs = cmdtp->maxargs + FRU_BOARD_AREA_TOTAL_FIELDS;
+   } else if (!strncmp(argv[2], "-p", 3)) {
+   fru_generate = fru_product_generate;
+   maxargs = cmdtp->maxargs + FRU_PRODUCT_AREA_TOTAL_FIELDS;
+   } else {
+   return CMD_RET_USAGE;
+   }
 
-   if (argc < cmdtp->maxargs)
+   if (argc < maxargs)
return CMD_RET_USAGE;
 
addr = hextoul(argv[3], NULL);
@@ -63,7 +73,7 @@ static int do_fru_generate(struct cmd_tbl *cmdtp, int flag, 
int argc,
 static struct cmd_tbl cmd_fru_sub[] = {
U_BOOT_CMD_MKENT(capture, 3, 0, do_fru_capture, "", ""),
U_BOOT_CMD_MKENT(display, 2, 0, do_fru_display, "", ""),
-   U_BOOT_CMD_MKENT(board_gen, 8, 0, do_fru_generate, "", ""),
+   U_BOOT_CMD_MKENT(generate, 4, 0, do_fru_generate, "", ""),
 };
 
 static int do_fru(struct cmd_tbl *cmdtp, int flag, int argc,
@@ -91,11 +101,16 @@ static char fru_help_text[] =
"capture  - Parse and capture FRU table present at address.\n"
"fru display - Displays content of FRU table that was captured using\n"
"  fru capture command\n"
-   "fru board_gen\n"
-   "[custom ...] - Generate FRU\n"
-   "  format with board info area filled based on\n"
+   "fru generate -b\n"
+   "  [custom ...] - Generate FRU\n"
+   "format with board info area filled based on\n"
"parameters.  is pointing to place where FRU is\n"
"generated.\n"
+   "fru generate -p\n"
+   "  \n"
+   " [custom ...] - Generate FRU format with\n"
+   "product info area filled based on parameters. \n"
+   "is pointing to place where FRU is generated.\n"
;
 #endif
 
diff --git a/include/fru.h b/include/fru.h
index b158b80b5866..f60d6aed53bc 100644
--- a/include/fru.h
+++ b/include/fru.h
@@ -31,6 +31,12 @@ struct fru_board_info_header {
u8 time[3];
 } __packed;
 
+struct fru_product_info_header {
+   u8 ver;
+   u8 len;
+   u8 lang_code;
+} __packed;
+
 struct fru_board_info_member {
u8 type_len;
u8 *name;
@@ -64,6 +70,27 @@ struct fru_board_data {
struct list_head custom_fields;
 };
 
+struct fru_product_data {
+   u8 ver;
+   u8 len;
+   u8 lang_code;
+   u8 manufacturer_type_len;
+   u8 manufacturer_name[FRU_INFO_FIELD_LEN_MAX];
+   u8 product_name_type_len;
+   u8 product_name[FRU_INFO_FIELD_LEN_MAX];
+   u8 part_number_type_len;
+   u8 part_number[FRU_INFO_FIELD_LEN_MAX];
+   u8 version_number_type_len;
+   u8 version_number[FRU_INFO_FIELD_LEN_MAX];
+   u8 serial_number_type_len;
+   u8 serial_number[FRU_INFO_FIELD_LEN_MAX];
+   u8 asset_number_type_len;
+   u8 asset_number[FRU_INFO_FIELD_LEN_MAX];
+   u8 file_id_type_len;
+   u8 file_id[FRU_INFO_FIELD_LEN_MAX];
+   struct list_head custom_fields;
+};
+
 struct fru_multirec_hdr {
u8 rec_type;
u8 type;
@@ -85,6 +112,7 @@ struct fru_multirec_node {
 struct fru_table {
struct fru_common_hdr hdr;
struct fru_board_data brd;
+   struct fru_product_data prd;
struct list_head multi_recs;
bool captured;
 };
@@ -102,13 +130,15 @@ struct fru_table {
 
 /* This should be minimum of fields */
 #define FRU_BOARD_AREA_TOTAL_FIELDS5
+#define FRU_PRODUCT_AREA_TOTAL_FIELDS  7
 #define FRU_TYPELEN_TYPE_SHIFT 6
 #define FRU_TYPELEN_TYPE_BINARY0
 #define FRU_TYPELEN_TYPE_ASCII83
 
 int fru_display(int verbose);
 int fru_capture(const void *addr);
-int fru_generate(const void *addr, int argc, char *const argv[]);
+int fru_board_generate(const void *addr, int argc, char *const argv[]);
+int fru_product_generate(const void *addr, int argc, char *const 

[PATCH v2 5/6] doc: fru: add documentation for the fru command

2022-08-15 Thread Jae Hyun Yoo
Add a usage document for the 'fru' u-boot command.

Signed-off-by: Jae Hyun Yoo 
---
Changes from v1:
 * Newly added in v2. (Heinrich)

 doc/usage/cmd/fru.rst | 144 ++
 doc/usage/index.rst   |   1 +
 2 files changed, 145 insertions(+)
 create mode 100644 doc/usage/cmd/fru.rst

diff --git a/doc/usage/cmd/fru.rst b/doc/usage/cmd/fru.rst
new file mode 100644
index ..d65bbc6dcbba
--- /dev/null
+++ b/doc/usage/cmd/fru.rst
@@ -0,0 +1,144 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+fru command
+===
+
+Synopsis
+
+
+::
+
+fru capture 
+fru display
+fru generate -b   [ ...]
+fru generate -p 
[ ...]
+
+Description
+---
+
+The *fru* commands is used to generate, capture and display FRU (Field
+Replaceable Unit) information data.
+
+Capture
+~~~
+
+The *fru capture* command parses and captures FRU configuration table at
+a specified address.
+
+addr
+memory address which FRU configuration table is stored.
+
+Display
+~~~
+
+The *fru display* command displays FRU information that is parsed using
+fru capture command.
+
+Generate
+
+
+The *fru generate* command generates a FRU configuration table which has Board
+or Product Info Area using the given field parameters.
+
+-b
+generate FRU which has board info area.
+
+addr
+memory address which FRU configuration table will be stored.
+
+manufacturer
+board manufacturer string.
+
+board name
+board product name string.
+
+serial number
+board serial number string.
+
+serial number
+board serial number string.
+
+part number
+board part number string.
+
+file id
+FRU File ID string. The FRU File version field is a pre-defined
+field provided as a manufacturing aid for verifying the file that
+was used during manufacture or field update to load the FRU
+information. The content is manufacturer-specific.
+
+custom
+additional custom board info area fields, if any.
+
+-p
+generate FRU which has product info area.
+
+addr
+memory address which FRU configuration table will be stored.
+
+manufacturer
+product manufacturer string.
+
+board name
+product name string.
+
+part number
+product part/model number string.
+
+version number
+product version number string.
+
+serial number
+product serial number string.
+
+asset number
+asset tag.
+
+file id
+FRU File ID string. The FRU File version field is a pre-defined
+field provided as a manufacturing aid for verifying the file that
+was used during manufacture or field update to load the FRU
+information. The content is manufacturer-specific.
+
+custom
+additional custom product info area fields, if any.
+
+Example
+---
+
+::
+
+=> fru generate -b 9000 abc def ghi jkl mno prs tuv wxy
+=> fru capture 9000
+=> fru display
+*COMMON HEADER*
+Version:1
+*** No Internal Area ***
+*** No Chassis Info Area ***
+Board Area Offset:8
+*** No Product Info Area ***
+*** No MultiRecord Area ***
+*BOARD INFO*
+Version:1
+Board Area Length:40
+Time in Minutes from 0:00hrs 1/1/96: 0
+ Manufacturer Name: abc
+ Product Name: def
+ Serial Number: ghi
+ Part Number: jkl
+ File ID: mno
+ Custom Type/Length: 0xc3
+  : 70 72 73 prs
+ Custom Type/Length: 0xc3
+  : 74 75 76 tuv
+ Custom Type/Length: 0xc3
+  : 77 78 79 wxy
+*PRODUCT INFO*
+Version:0
+Product Area Length:0
+*MULTIRECORDS*
+
+Configuration
+-
+
+The fru command is only available if CONFIG_CMD_FRU=y.
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 28f9683a3e6f..e96a16356307 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -45,6 +45,7 @@ Shell commands
cmd/fatload
cmd/fdt
cmd/for
+   cmd/fru
cmd/gpio
cmd/load
cmd/loadm
-- 
2.25.1



[PATCH v2 1/6] xilinx: common: refactor FRU handling support

2022-08-15 Thread Jae Hyun Yoo
Refactor FRU handling support to remove Xilinx customization dependency.
With this change, single or multiple custom board fields and
multi-records can be added dynamically using linked list so that each
board support can utilize them in their own custom way.

It's a preparation change for moving the FRU command support to common
region.

Signed-off-by: Jae Hyun Yoo 
---
 board/xilinx/common/board.c   |  63 --
 board/xilinx/common/fru.c |  12 +-
 board/xilinx/common/fru.h |  76 +++-
 board/xilinx/common/fru_ops.c | 228 --
 4 files changed, 264 insertions(+), 115 deletions(-)

diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 9b4aded466ab..e979f0176273 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -88,6 +88,9 @@ int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
 #define EEPROM_HDR_NO_OF_MAC_ADDR  4
 #define EEPROM_HDR_ETH_ALENETH_ALEN
 #define EEPROM_HDR_UUID_LEN16
+#define EEPROM_MULTIREC_TYPE_XILINX_OEM0xD2
+#define EEPROM_MULTIREC_MAC_OFFSET 4
+#define EEPROM_MULTIREC_DUT_MACID  0x31
 
 struct xilinx_board_description {
u32 header;
@@ -116,6 +119,19 @@ struct xilinx_legacy_format {
char unused3[29]; /* 0xe3 */
 };
 
+struct xilinx_multirec_mac {
+   u8 xlnx_iana_id[3];
+   u8 ver;
+   u8 macid[EEPROM_HDR_NO_OF_MAC_ADDR][ETH_ALEN];
+};
+
+enum xilinx_board_custom_field {
+   brd_custom_field_rev = 0,
+   brd_custom_field_pcie,
+   brd_custom_field_uuid,
+   brd_custom_field_max
+};
+
 static void xilinx_eeprom_legacy_cleanup(char *eeprom, int size)
 {
int i;
@@ -200,9 +216,14 @@ static bool xilinx_detect_legacy(u8 *buffer)
 static int xilinx_read_eeprom_fru(struct udevice *dev, char *name,
  struct xilinx_board_description *desc)
 {
+   u8 parsed_macid[EEPROM_HDR_NO_OF_MAC_ADDR][ETH_ALEN] = { 0 };
+   struct fru_custom_info custom_info[brd_custom_field_max] = { 0 };
+   struct fru_custom_field_node *ci_node;
+   struct fru_multirec_node *mr_node;
+   const struct fru_table *fru_data;
int i, ret, eeprom_size;
u8 *fru_content;
-   u8 id = 0;
+   u8 id = 0, field = 0;
 
/* FIXME this is shortcut - if eeprom type is wrong it will fail */
eeprom_size = i2c_eeprom_size(dev);
@@ -237,30 +258,56 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, 
char *name,
goto end;
}
 
+   fru_data = fru_get_fru_data();
+
+   list_for_each_entry(ci_node, _data->brd.custom_fields, list) {
+   custom_info[field].type_len = ci_node->info.type_len;
+   memcpy(custom_info[field].data, ci_node->info.data,
+  ci_node->info.type_len & FRU_TYPELEN_LEN_MASK);
+   if (++field < brd_custom_field_max)
+   break;
+   }
+
+   list_for_each_entry(mr_node, _data->multi_recs, list) {
+   struct fru_multirec_hdr *hdr = _node->info.hdr;
+
+   if (hdr->rec_type == EEPROM_MULTIREC_TYPE_XILINX_OEM) {
+   struct xilinx_multirec_mac *mac;
+
+   mac = (struct xilinx_multirec_mac *)mr_node->info.data;
+   if (mac->ver == EEPROM_MULTIREC_DUT_MACID) {
+   int mac_len = hdr->len -
+ EEPROM_MULTIREC_MAC_OFFSET;
+
+   memcpy(parsed_macid, mac->macid, mac_len);
+   }
+   }
+   }
+
/* It is clear that FRU was captured and structures were filled */
-   strlcpy(desc->manufacturer, (char *)fru_data.brd.manufacturer_name,
+   strlcpy(desc->manufacturer, (char *)fru_data->brd.manufacturer_name,
sizeof(desc->manufacturer));
-   strlcpy(desc->uuid, (char *)fru_data.brd.uuid,
+   strlcpy(desc->uuid, (char *)custom_info[brd_custom_field_uuid].data,
sizeof(desc->uuid));
-   strlcpy(desc->name, (char *)fru_data.brd.product_name,
+   strlcpy(desc->name, (char *)fru_data->brd.product_name,
sizeof(desc->name));
for (i = 0; i < sizeof(desc->name); i++) {
if (desc->name[i] == ' ')
desc->name[i] = '\0';
}
-   strlcpy(desc->revision, (char *)fru_data.brd.rev,
+   strlcpy(desc->revision, (char *)custom_info[brd_custom_field_rev].data,
sizeof(desc->revision));
for (i = 0; i < sizeof(desc->revision); i++) {
if (desc->revision[i] == ' ')
desc->revision[i] = '\0';
}
-   strlcpy(desc->serial, (char *)fru_data.brd.serial_number,
+   strlcpy(desc->serial, (char *)fru_data->brd.serial_number,
sizeof(desc->serial));
 
while (id < EEPROM_HDR_NO_OF_MAC_ADDR) {
-   if (is_valid_ethaddr((const u8 

[PATCH v2 2/6] cmd: fru: move FRU handling support to common region

2022-08-15 Thread Jae Hyun Yoo
From: Graeme Gregory 

The FRU handling was added as a Xilinx board dependent support but it
is also useful for other boards, so this commit moves the FRU handling
support to the common region so that it can be enabled by CONFIG_CMD_FRU.

Signed-off-by: Graeme Gregory 
Signed-off-by: Jae Hyun Yoo 
---
Changes from v1:
 * Split out the custom field and multi-record handling part as a
   seperate patch. (Michal)
 * Moved fru_ops.c to lib. (Heinrich)
 * Added what does FRU stand for. (Heinrich)

Changes from RFC:
 * Made FRU typecode string table as a generic and sharable table. (Michal)
 * Made OEM multirecord parsing call happen only on customizable type IDs.
   (Michal)
 * Added manufacturer custom board info fields parsing flow. (Michal)

 board/xilinx/Kconfig   | 8 
 board/xilinx/common/Makefile   | 3 ---
 board/xilinx/common/board.c| 3 +--
 cmd/Kconfig| 8 
 cmd/Makefile   | 1 +
 {board/xilinx/common => cmd}/fru.c | 3 +--
 {board/xilinx/common => include}/fru.h | 0
 lib/Makefile   | 1 +
 {board/xilinx/common => lib}/fru_ops.c | 3 +--
 9 files changed, 13 insertions(+), 17 deletions(-)
 rename {board/xilinx/common => cmd}/fru.c (99%)
 rename {board/xilinx/common => include}/fru.h (100%)
 rename {board/xilinx/common => lib}/fru_ops.c (99%)

diff --git a/board/xilinx/Kconfig b/board/xilinx/Kconfig
index 17880661736d..110706b20fa3 100644
--- a/board/xilinx/Kconfig
+++ b/board/xilinx/Kconfig
@@ -74,11 +74,3 @@ config ZYNQ_GEM_I2C_MAC_OFFSET
  Set the MAC offset for i2C.
 
 endif
-
-config CMD_FRU
-   bool "FRU information for product"
-   help
- This option enables FRU commands to capture and display FRU
- information present in the device. The FRU Information is used
- to primarily to provide "inventory" information about the boards
- that the FRU Information Device is located on.
diff --git a/board/xilinx/common/Makefile b/board/xilinx/common/Makefile
index cdc3c9677432..e33baaae1159 100644
--- a/board/xilinx/common/Makefile
+++ b/board/xilinx/common/Makefile
@@ -8,6 +8,3 @@ obj-y   += board.o
 ifndef CONFIG_ARCH_ZYNQ
 obj-$(CONFIG_DISPLAY_CPUINFO) += cpu-info.o
 endif
-ifndef CONFIG_SPL_BUILD
-obj-$(CONFIG_CMD_FRU) += fru.o fru_ops.o
-endif
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index e979f0176273..e58b11d7f757 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -25,8 +26,6 @@
 #include 
 #include 
 
-#include "fru.h"
-
 #if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
 struct efi_fw_image fw_images[] = {
 #if defined(XILINX_BOOT_IMAGE_GUID)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 211ebe9c8783..29b2e11e1247 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1063,6 +1063,14 @@ config CMD_FPGAD
  fpga_get_reg() function. This functions similarly to the 'md'
  command.
 
+config CMD_FRU
+   bool "FRU information for product"
+   help
+ This option enables FRU (Field Replaceable Unit) commands to capture
+ and display FRU information present in the device. The FRU Information
+ is used to primarily to provide "inventory" information about the
+ boards that the FRU Information Device is located on.
+
 config CMD_FUSE
bool "fuse - support for the fuse subssystem"
help
diff --git a/cmd/Makefile b/cmd/Makefile
index 6e87522b62e8..58e353611a5a 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_CMD_SQUASHFS) += sqfs.o
 obj-$(CONFIG_CMD_FLASH) += flash.o
 obj-$(CONFIG_CMD_FPGA) += fpga.o
 obj-$(CONFIG_CMD_FPGAD) += fpgad.o
+obj-$(CONFIG_CMD_FRU) += fru.o
 obj-$(CONFIG_CMD_FS_GENERIC) += fs.o
 obj-$(CONFIG_CMD_FUSE) += fuse.o
 obj-$(CONFIG_CMD_GETTIME) += gettime.o
diff --git a/board/xilinx/common/fru.c b/cmd/fru.c
similarity index 99%
rename from board/xilinx/common/fru.c
rename to cmd/fru.c
index 0d72911df04d..2ec5012af5ac 100644
--- a/board/xilinx/common/fru.c
+++ b/cmd/fru.c
@@ -7,10 +7,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
-#include "fru.h"
-
 static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, int argc,
  char *const argv[])
 {
diff --git a/board/xilinx/common/fru.h b/include/fru.h
similarity index 100%
rename from board/xilinx/common/fru.h
rename to include/fru.h
diff --git a/lib/Makefile b/lib/Makefile
index e3deb1528794..ef8933ae3aee 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -39,6 +39,7 @@ obj-y += crc16-ccitt.o
 obj-$(CONFIG_ERRNO_STR) += errno_str.o
 obj-$(CONFIG_FIT) += fdtdec_common.o
 obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
+obj-$(CONFIG_CMD_FRU) += fru_ops.o
 obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o
 obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
 obj-$(CONFIG_SMBIOS_PARSER) += smbios-parser.o
diff --git 

[PATCH v2 3/6] cmd: fru: fix a sandbox segfault issue

2022-08-15 Thread Jae Hyun Yoo
This command doesn't work with sandbox because direct memory access
causes a segfault error. Fix it up using map_sysmem().

Signed-off-by: Jae Hyun Yoo 
---
Changes from v1:
 * Newly added in v2.

 board/xilinx/common/board.c |  2 +-
 cmd/fru.c   | 20 +---
 include/fru.h   |  4 ++--
 lib/fru_ops.c   | 17 -
 4 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index e58b11d7f757..3292083c5250 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -241,7 +241,7 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char 
*name,
goto end;
}
 
-   fru_capture((unsigned long)fru_content);
+   fru_capture(fru_content);
if (gd->flags & GD_FLG_RELOC || (_DEBUG && 
CONFIG_IS_ENABLED(DTB_RESELECT))) {
printf("Xilinx I2C FRU format at %s:\n", name);
ret = fru_display(0);
diff --git a/cmd/fru.c b/cmd/fru.c
index 2ec5012af5ac..11466339da6a 100644
--- a/cmd/fru.c
+++ b/cmd/fru.c
@@ -9,12 +9,15 @@
 #include 
 #include 
 #include 
+#include 
 
 static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, int argc,
  char *const argv[])
 {
unsigned long addr;
+   const void *buf;
char *endp;
+   int ret;
 
if (argc < cmdtp->maxargs)
return CMD_RET_USAGE;
@@ -23,7 +26,11 @@ static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, 
int argc,
if (*argv[1] == 0 || *endp != 0)
return -1;
 
-   return fru_capture(addr);
+   buf = map_sysmem(addr, 0);
+   ret = fru_capture(buf);
+   unmap_sysmem(buf);
+
+   return ret;
 }
 
 static int do_fru_display(struct cmd_tbl *cmdtp, int flag, int argc,
@@ -36,14 +43,21 @@ static int do_fru_display(struct cmd_tbl *cmdtp, int flag, 
int argc,
 static int do_fru_generate(struct cmd_tbl *cmdtp, int flag, int argc,
   char *const argv[])
 {
+   int (*fru_generate)(const void *addr, int argc, char *const argv[]);
unsigned long addr;
+   const void *buf;
+   int ret;
 
if (argc < cmdtp->maxargs)
return CMD_RET_USAGE;
 
-   addr = hextoul(argv[2], NULL);
+   addr = hextoul(argv[3], NULL);
+
+   buf = map_sysmem(addr, 0);
+   ret = fru_generate(buf, argc - 3, argv + 3);
+   unmap_sysmem(buf);
 
-   return fru_generate(addr, argc - 3, argv + 3);
+   return ret;
 }
 
 static struct cmd_tbl cmd_fru_sub[] = {
diff --git a/include/fru.h b/include/fru.h
index 41655864dbf5..b158b80b5866 100644
--- a/include/fru.h
+++ b/include/fru.h
@@ -107,8 +107,8 @@ struct fru_table {
 #define FRU_TYPELEN_TYPE_ASCII83
 
 int fru_display(int verbose);
-int fru_capture(unsigned long addr);
-int fru_generate(unsigned long addr, int argc, char *const argv[]);
+int fru_capture(const void *addr);
+int fru_generate(const void *addr, int argc, char *const argv[]);
 u8 fru_checksum(u8 *addr, u8 len);
 int fru_check_type_len(u8 type_len, u8 language, u8 *type);
 const struct fru_table *fru_get_fru_data(void);
diff --git a/lib/fru_ops.c b/lib/fru_ops.c
index c0360f219c37..6ed1388b2fc8 100644
--- a/lib/fru_ops.c
+++ b/lib/fru_ops.c
@@ -91,7 +91,7 @@ static u8 fru_gen_type_len(u8 *addr, char *name)
return 1 + len;
 }
 
-int fru_generate(unsigned long addr, int argc, char *const argv[])
+int fru_generate(const void *addr, int argc, char *const argv[])
 {
struct fru_common_hdr *header = (struct fru_common_hdr *)addr;
struct fru_board_info_header *board_info;
@@ -155,8 +155,8 @@ int fru_generate(unsigned long addr, int argc, char *const 
argv[])
 
debug("checksum %x(addr %x)\n", *member, len);
 
-   env_set_hex("fru_addr", addr);
-   env_set_hex("filesize", (unsigned long)member - addr + 1);
+   env_set_hex("fru_addr", (ulong)addr);
+   env_set_hex("filesize", (ulong)member - (ulong)addr + 1);
 
return 0;
 }
@@ -171,7 +171,7 @@ static void fru_delete_custom_fields(struct list_head 
*custom_fields)
}
 }
 
-static int fru_append_custom_info(unsigned long addr,
+static int fru_append_custom_info(const void *addr,
  struct list_head *custom_fields)
 {
struct fru_custom_info *info = (struct fru_custom_info *)addr;
@@ -190,7 +190,7 @@ static int fru_append_custom_info(unsigned long addr,
return 0;
 }
 
-static int fru_parse_board(unsigned long addr)
+static int fru_parse_board(const void *addr)
 {
u8 i, type;
int len;
@@ -268,8 +268,7 @@ static void fru_delete_multirecs(struct list_head 
*multi_recs)
}
 }
 
-static int fru_append_multirec(unsigned long addr,
-  struct list_head *multi_recs)
+static int fru_append_multirec(const void *addr, struct list_head *multi_recs)
 {
struct fru_multirec_info *mr_src = 

[PATCH v2 0/6] cmd/fru: move FRU handling support to common region

2022-08-15 Thread Jae Hyun Yoo
Hello,

The FRU handling was added as a Xilinx board dependent support but it
is also useful for other boards, so this commit moves the FRU handling
support to the common region so that it can be enabled by CONFIG_CMD_FRU.

To provide manufacturer specific custom info fields and multi-records
parsing, it refactors the FRU handling logic using linked list so that each
board support can utilize them in their own custom way. This series adds
'Product Info' parsing support, usage document and py test script too.

Please review!

Thanks,
Jae

Graeme Gregory (1):
  cmd: fru: move FRU handling support to common region

Jae Hyun Yoo (5):
  xilinx: common: refactor FRU handling support
  cmd: fru: fix a sandbox segfault issue
  cmd: fru: add product info area parsing support
  doc: fru: add documentation for the fru command
  test: py: fru: add a test for the fru command

 board/xilinx/Kconfig   |   8 -
 board/xilinx/common/Makefile   |   3 -
 board/xilinx/common/board.c|  68 ++-
 board/xilinx/common/fru.h  | 108 -
 board/xilinx/common/fru_ops.c  | 415 -
 cmd/Kconfig|   8 +
 cmd/Makefile   |   1 +
 {board/xilinx/common => cmd}/fru.c |  54 ++-
 configs/sandbox64_defconfig|   1 +
 configs/sandbox_defconfig  |   1 +
 configs/sandbox_flattree_defconfig |   1 +
 configs/sandbox_noinst_defconfig   |   1 +
 configs/sandbox_spl_defconfig  |   1 +
 configs/sandbox_vpl_defconfig  |   1 +
 doc/usage/cmd/fru.rst  | 144 ++
 doc/usage/index.rst|   1 +
 include/fru.h  | 146 ++
 lib/Makefile   |   1 +
 lib/fru_ops.c  | 725 +
 test/py/tests/test_fru.py  |  47 ++
 20 files changed, 1178 insertions(+), 557 deletions(-)
 delete mode 100644 board/xilinx/common/fru.h
 delete mode 100644 board/xilinx/common/fru_ops.c
 rename {board/xilinx/common => cmd}/fru.c (50%)
 create mode 100644 doc/usage/cmd/fru.rst
 create mode 100644 include/fru.h
 create mode 100644 lib/fru_ops.c
 create mode 100644 test/py/tests/test_fru.py

-- 
2.25.1



Re: [PATCH] [RFC] cmd: i2c: fix default address len for DM_I2C

2022-08-15 Thread Simon Glass
Hi Tim,

On Mon, 15 Aug 2022 at 11:48, Tim Harvey  wrote:
>
> On Sat, Aug 13, 2022 at 7:59 AM Simon Glass  wrote:
> >
> > Hi Tim,
> >
> > On Thu, 11 Aug 2022 at 11:57, Tim Harvey  wrote:
> > >
> > > According to the comment block "The default {addr} parameter is one byte
> > > (.1) which works well for memories and registers with 8 bits of address
> > > space."
> > >
> > > While this is true for legacy I2C a default length of -1 is being passed
> > > for DM_I2C which results in a usage error.
> > >
> > > Restore the documented behavior by always using a default alen of 1.
> > >
> > > Signed-off-by: Tim Harvey 
> > >
> > > This is an RFC as I'm unclear if we want to restore the legacy usage or
> > > enforce a new usage (in which case the comment block should be updated)
> > > and I'm not clear if this is documented in other places. If the decision
> > > is to enforce a new usage then it is unclear to me how to specifiy the
> > > default alen as there is no command for that (i2c alen [len]?).
> > > ---
> > >  cmd/i2c.c | 10 --
> > >  1 file changed, 10 deletions(-)
> > >
> >
> > Can you dig into this a little more on your board? DEFAULT_ADDR_LEN is
> > set to -1 so that by default it does not get set by the command, and
> > the existing alen is used.
> >
> > This is necessary for driver model, since the alen can be set by the
> > peripheral device and we don't want to overwrite it:
> >
> > if (!ret && alen != -1)
> > ret = i2c_set_chip_offset_len(dev, alen);
> >
>
> Simon,
>
> Here's some annotated debug prints added where chip_offset is passed/set:
> Model: Gateworks Venice GW73xx-0x i.MX8MM Development Kit
> DRAM:  1 GiB
> i2c_chip_of_to_plat gsc@20 offset_len=1
> i2c_chip_of_to_plat pmic@69 offset_len=1
> i2c_get_chip i2c@30a2 0x51 offset_len=1
> i2c_bind_driver i2c@30a2 offset_len=1
> i2c_set_chip_offset_len generic_51 offset_len=1
> dm_i2c_read generic_51 offset=0 offset_len=1
> i2c_setup_offset 0x51 offset=0 offset_len=1
> Core:  209 devices, 27 uclasses, devicetree: separate
> ...
> u-boot=> i2c dev 0 && i2c md 0x51 0 50
> Setting bus to 0
> i2c - I2C sub-system
>
> Usage:
> i2c bus [muxtype:muxaddr:muxchannel] - show I2C bus info
> ...
>
> So the chip I noticed this issue with is 0x51 which an atmel,24c02
> compatible eeprom which imx8mm-venice-gw700x.dtsi defines 4 of
> (i2c1-0x50, i2c1-0x51, i2c1-0x52, i2c1-0x53). You can see above
> i2c1-0x51 (i2c1=i2c@30a2) is accessed early as it holds the board
> model information and at that point in time it's accessed with alen=1
> (which I specify in board/gateworks/venice/eeprom.c:eeprom_read()) but
> by the time the 'i2c md 0x51 0 50' comes around
> i2c_get_chip_offset_len() returns 0 for this device. The other eeprom
> devices on i2c1 are never accessed by a driver so they would never
> have a 'default' alen set.

OK I see,

>
> Where is the initial setting of alen supposed to have come?

The "u-boot,i2c-offset-len" property in the device tree.

Regards,
Simon


>
> Best Regards,
>
> Tim
>
> >
> > > diff --git a/cmd/i2c.c b/cmd/i2c.c
> > > index bd04b14024be..c57271479e81 100644
> > > --- a/cmd/i2c.c
> > > +++ b/cmd/i2c.c
> > > @@ -118,17 +118,7 @@ static uchar i2c_no_probes[] = 
> > > CONFIG_SYS_I2C_NOPROBES;
> > >  #endif
> > >
> > >  #define DISP_LINE_LEN  16
> > > -
> > > -/*
> > > - * Default for driver model is to use the chip's existing address length.
> > > - * For legacy code, this is not stored, so we need to use a suitable
> > > - * default.
> > > - */
> > > -#if CONFIG_IS_ENABLED(DM_I2C)
> > > -#define DEFAULT_ADDR_LEN   (-1)
> > > -#else
> > >  #define DEFAULT_ADDR_LEN   1
> > > -#endif
> > >
> > >  #if CONFIG_IS_ENABLED(DM_I2C)
> > >  static struct udevice *i2c_cur_bus;
> > > --
> > > 2.25.1
> > >
> >
> > Regards,
> > Simon


Re: [PATCH] mtd: rawnand: fsl_elbc: Remove NAND_NO_SUBPAGE_WRITE flag

2022-08-15 Thread Pali Rohár
On Monday 15 August 2022 23:49:17 Michael Nazzareno Trimarchi wrote:
> Hi
> 
> Il lun 15 ago 2022, 10:01 Pali Rohár  ha scritto:
> 
> > Subpage write support for freescale eLBC NAND controller driver is
> > implemented in U-Boot and was fixes in the commit d3963721d93f ("nand: Sync
> > with Linux v4.1").
> >
> > So remove NAND_NO_SUBPAGE_WRITE flag from the fsl_elbc_nand.c driver. This
> > partially revert commit cb04c7723429 ("nand/fsl: add NAND_NO_SUBPAGE_WRITE
> > to eLBC and IFC drivers"), only eLBC driver part.
> >
> > With this change U-Boot with default settings can read from NAND UBIFS
> > image created on Linux with Linux default settings. Prior this change
> > U-Boot was unable to read from NAND UBIFS images created with Linux default
> > settings due to differnet UBI geometry.
> >
> > Linux kernel fsl_elbc_nand.c driver also does not set NAND_NO_SUBPAGE_WRITE
> > flag and has implemented subpage write support.
> >
> > Fixes: cb04c7723429 ("nand/fsl: add NAND_NO_SUBPAGE_WRITE to eLBC and IFC
> > drivers")
> > Fixes: d3963721d93f ("nand: Sync with Linux v4.1")
> > Signed-off-by: Pali Rohár 
> > ---
> > See also email thread:
> > https://lore.kernel.org/u-boot/20220807120027.2zz43afbqtqljhul@pali/t/#u
> > ---
> >  drivers/mtd/nand/raw/fsl_elbc_nand.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c
> > b/drivers/mtd/nand/raw/fsl_elbc_nand.c
> > index 48a3687f2728..e28670a4724a 100644
> > --- a/drivers/mtd/nand/raw/fsl_elbc_nand.c
> > +++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c
> > @@ -732,7 +732,6 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr,
> > struct udevice *dev)
> > nand->bbt_md = _mirror_descr;
> >
> > /* set up nand options */
> > -   nand->options = NAND_NO_SUBPAGE_WRITE;
> > nand->bbt_options = NAND_BBT_USE_FLASH;
> >
> > nand->controller = _ctrl->controller;
> >
> 
> Reviewed-by: Michael Trimarchi 
> 
> I was following the thread. Please confirm that you was able to test

Yes, I have tested this change on P2020 based board Turris 1.1. And
finally U-Boot by default was able to access UBI created by mainline
Linux with default parameters, just by calling 'ubi part rootfs'.
UBI created by sub-page of size 2048 can be still also read by U-Boot by
calling 'ubi part rootfs 2048'. For this purpose I already provided
distroboot patch to easily specify UBI header offset (which is by
default sub-page size):
https://lore.kernel.org/u-boot/20220807190422.20157-1-p...@kernel.org/

> Michael
> 
> > --
> > 2.20.1
> >
> >


Re: [PATCH] mtd: rawnand: fsl_elbc: Remove NAND_NO_SUBPAGE_WRITE flag

2022-08-15 Thread Michael Nazzareno Trimarchi
Hi

Il lun 15 ago 2022, 10:01 Pali Rohár  ha scritto:

> Subpage write support for freescale eLBC NAND controller driver is
> implemented in U-Boot and was fixes in the commit d3963721d93f ("nand: Sync
> with Linux v4.1").
>
> So remove NAND_NO_SUBPAGE_WRITE flag from the fsl_elbc_nand.c driver. This
> partially revert commit cb04c7723429 ("nand/fsl: add NAND_NO_SUBPAGE_WRITE
> to eLBC and IFC drivers"), only eLBC driver part.
>
> With this change U-Boot with default settings can read from NAND UBIFS
> image created on Linux with Linux default settings. Prior this change
> U-Boot was unable to read from NAND UBIFS images created with Linux default
> settings due to differnet UBI geometry.
>
> Linux kernel fsl_elbc_nand.c driver also does not set NAND_NO_SUBPAGE_WRITE
> flag and has implemented subpage write support.
>
> Fixes: cb04c7723429 ("nand/fsl: add NAND_NO_SUBPAGE_WRITE to eLBC and IFC
> drivers")
> Fixes: d3963721d93f ("nand: Sync with Linux v4.1")
> Signed-off-by: Pali Rohár 
> ---
> See also email thread:
> https://lore.kernel.org/u-boot/20220807120027.2zz43afbqtqljhul@pali/t/#u
> ---
>  drivers/mtd/nand/raw/fsl_elbc_nand.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c
> b/drivers/mtd/nand/raw/fsl_elbc_nand.c
> index 48a3687f2728..e28670a4724a 100644
> --- a/drivers/mtd/nand/raw/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c
> @@ -732,7 +732,6 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr,
> struct udevice *dev)
> nand->bbt_md = _mirror_descr;
>
> /* set up nand options */
> -   nand->options = NAND_NO_SUBPAGE_WRITE;
> nand->bbt_options = NAND_BBT_USE_FLASH;
>
> nand->controller = _ctrl->controller;
>

Reviewed-by: Michael Trimarchi 

I was following the thread. Please confirm that you was able to test

Michael

> --
> 2.20.1
>
>


[PATCH v2 3/3] arm: bcmbca: make bcm68360 driver depending on CONFIG_BCM6856

2022-08-15 Thread William Zhang
As CONFIG_ARCH_BCM68360 is replaced with CONFIG_BCM6856, update the
driver Kconfig to use the new config symbol

Signed-off-by: William Zhang 

---

(no changes since v1)

 drivers/gpio/Kconfig | 2 +-
 drivers/led/Kconfig  | 2 +-
 drivers/mtd/nand/raw/Kconfig | 2 +-
 drivers/spi/Kconfig  | 2 +-
 drivers/watchdog/Kconfig | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 83f4f5089992..9e00b48234ab 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -110,7 +110,7 @@ config BCM2835_GPIO
 
 config BCM6345_GPIO
bool "BCM6345 GPIO driver"
-   depends on DM_GPIO && (ARCH_BMIPS || ARCH_BCM68360 || \
+   depends on DM_GPIO && (ARCH_BMIPS || BCM6856 || \
   ARCH_BCM6858 || BCM63158 || \
   ARCH_BCM6753)
help
diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
index d777414dda8d..bd8f23fd9631 100644
--- a/drivers/led/Kconfig
+++ b/drivers/led/Kconfig
@@ -37,7 +37,7 @@ config LED_BCM6753
 
 config LED_BCM6858
bool "LED Support for BCM6858"
-   depends on LED && (ARCH_BCM68360 || ARCH_BCM6858 || BCM63158)
+   depends on LED && (BCM6856 || ARCH_BCM6858 || BCM63158)
help
  This option enables support for LEDs connected to the BCM6858
  HW has blinking capabilities and up to 32 LEDs can be controlled.
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 24c27b6ecf7f..5d006ca1ea07 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -103,7 +103,7 @@ config NAND_BRCMNAND_6753
 
 config NAND_BRCMNAND_68360
bool "Support Broadcom NAND controller on bcm68360"
-   depends on NAND_BRCMNAND && ARCH_BCM68360
+   depends on NAND_BRCMNAND && BCM6856
help
  Enable support for broadcom nand driver on bcm68360.
 
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 0a666eee80e7..978e5c86a420 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -91,7 +91,7 @@ config ATMEL_SPI
 
 config BCM63XX_HSSPI
bool "BCM63XX HSSPI driver"
-   depends on (ARCH_BMIPS || ARCH_BCM68360 || \
+   depends on (ARCH_BMIPS || BCM6856 || \
ARCH_BCM6858 || BCM63158)
help
  Enable the BCM6328 HSSPI driver. This driver can be used to
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index ff4d1ee530d2..6d559515b78b 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -128,7 +128,7 @@ config WDT_AT91
 
 config WDT_BCM6345
bool "BCM6345 watchdog timer support"
-   depends on WDT && (ARCH_BMIPS || ARCH_BCM68360 || \
+   depends on WDT && (ARCH_BMIPS || BCM6856 || \
   ARCH_BCM6858 || BCM63158 || \
   ARCH_BCM6753)
help
-- 
2.37.1



smime.p7s
Description: S/MIME Cryptographic Signature


[PATCH v2 2/3] arm: bcmbca: remove bcm68360 support under CONFIG_ARCH_BCM68360

2022-08-15 Thread William Zhang
BCM68360 is a variant within the BCM6856 chip family. Now that BCM6856
is supported under CONFIG_ARCH_BCMBCA and CONFIG_BCM6856, remove the
original ARCH_BCM68360 support and migrate its configuration and dts
settings. This includes:
  - Remove the bcm968360bg board folder. It is replaced by the generic
bcmbca board folder.
  - Merge the 68360.dtsi setting to the new 6856.dtsi file. Update board
dts with the new compatible string.
  - Merge broadcom_bcm968360bg.h setting to the new bcm96856.h file.

Signed-off-by: William Zhang 

---

Changes in v2:
- Bring Philippe Reynes copyright tag from 68360 dts to 6856 dts

 arch/arm/Kconfig |   7 -
 arch/arm/dts/Makefile|   6 +-
 arch/arm/dts/bcm68360.dtsi   | 217 ---
 arch/arm/dts/bcm6856.dtsi| 150 
 arch/arm/dts/bcm968360bg.dts |   6 +-
 board/broadcom/bcm968360bg/Kconfig   |  17 --
 board/broadcom/bcm968360bg/MAINTAINERS   |   6 -
 board/broadcom/bcm968360bg/Makefile  |   3 -
 board/broadcom/bcm968360bg/bcm968360bg.c |  62 ---
 configs/bcm968360bg_ram_defconfig|  10 +-
 include/configs/bcm96856.h   |   4 +
 include/configs/broadcom_bcm968360bg.h   |  32 
 12 files changed, 164 insertions(+), 356 deletions(-)
 delete mode 100644 arch/arm/dts/bcm68360.dtsi
 delete mode 100644 board/broadcom/bcm968360bg/Kconfig
 delete mode 100644 board/broadcom/bcm968360bg/MAINTAINERS
 delete mode 100644 board/broadcom/bcm968360bg/Makefile
 delete mode 100644 board/broadcom/bcm968360bg/bcm968360bg.c
 delete mode 100644 include/configs/broadcom_bcm968360bg.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index da4defa08466..3f124ab0ce85 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -672,12 +672,6 @@ config ARCH_BCM6753
select OF_CONTROL
imply CMD_DM
 
-config ARCH_BCM68360
-   bool "Broadcom BCM68360 family"
-   select DM
-   select OF_CONTROL
-   imply CMD_DM
-
 config ARCH_BCM6858
bool "Broadcom BCM6858 family"
select DM
@@ -2280,7 +2274,6 @@ source "board/armltd/vexpress/Kconfig"
 source "board/armltd/vexpress64/Kconfig"
 source "board/cortina/presidio-asic/Kconfig"
 source "board/broadcom/bcm96753ref/Kconfig"
-source "board/broadcom/bcm968360bg/Kconfig"
 source "board/broadcom/bcm968580xref/Kconfig"
 source "board/broadcom/bcmns3/Kconfig"
 source "board/cavium/thunderx/Kconfig"
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index a32bdf8c9f17..a0ea9fa6029d 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -1147,9 +1147,6 @@ dtb-$(CONFIG_ARCH_BCM283X) += \
bcm2837-rpi-cm3-io3.dtb \
bcm2711-rpi-4-b.dtb
 
-dtb-$(CONFIG_ARCH_BCM68360) += \
-   bcm968360bg.dtb
-
 dtb-$(CONFIG_ARCH_BCM6753) += \
bcm96753ref.dtb
 
@@ -1183,7 +1180,8 @@ dtb-$(CONFIG_BCM6813) += \
 dtb-$(CONFIG_BCM6846) += \
bcm96846.dtb
 dtb-$(CONFIG_BCM6856) += \
-   bcm96856.dtb
+   bcm96856.dtb \
+   bcm968360bg.dtb
 dtb-$(CONFIG_BCM6878) += \
bcm96878.dtb
 
diff --git a/arch/arm/dts/bcm68360.dtsi b/arch/arm/dts/bcm68360.dtsi
deleted file mode 100644
index 7bbe207794eb..
--- a/arch/arm/dts/bcm68360.dtsi
+++ /dev/null
@@ -1,217 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Copyright (C) 2020 Philippe Reynes 
- */
-
-#include "skeleton64.dtsi"
-
-/ {
-   compatible = "brcm,bcm68360";
-   #address-cells = <2>;
-   #size-cells = <2>;
-
-   aliases {
-   spi0 = 
-   };
-
-   cpus {
-   #address-cells = <2>;
-   #size-cells = <0>;
-   u-boot,dm-pre-reloc;
-
-   cpu0: cpu@0 {
-   compatible = "arm,cortex-a53", "arm,armv8";
-   device_type = "cpu";
-   reg = <0x0 0x0>;
-   next-level-cache = <>;
-   u-boot,dm-pre-reloc;
-   };
-
-   cpu1: cpu@1 {
-   compatible = "arm,cortex-a53", "arm,armv8";
-   device_type = "cpu";
-   reg = <0x0 0x1>;
-   next-level-cache = <>;
-   u-boot,dm-pre-reloc;
-   };
-
-   l2: l2-cache0 {
-   compatible = "cache";
-   u-boot,dm-pre-reloc;
-   };
-   };
-
-   clocks {
-   compatible = "simple-bus";
-   #address-cells = <2>;
-   #size-cells = <2>;
-   ranges;
-   u-boot,dm-pre-reloc;
-
-   periph_osc: periph-osc {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   clock-frequency = <2>;
-   u-boot,dm-pre-reloc;
-   };
-
-   hsspi_pll: hsspi-pll {
-   compatible = 

[PATCH v2 1/3] arm: bcmbca: add bcm6856 SoC support under CONFIG_ARCH_BCMBCA

2022-08-15 Thread William Zhang
BCM6856 is a Broadcom B53 based PON Gateway SoC. It is part of the BCA
(Broadband Carrier Access origin) chipset family. Like other Broadband
SoC, this patch adds it under CONFIG_BCM6856 chip config and
CONFIG_ARCH_BCMBCA platform config.

This initial support includes a bare-bone implementation and dts with
CPU subsystem, memory and Broadcom uart. This SoC is supported in the
linux-next git repository so the dts and dtsi files are copied from
linux.

The u-boot image can be loaded from flash or network to the entry point
address in the memory and boot from there to the console.

Signed-off-by: William Zhang 
---

(no changes since v1)

 MAINTAINERS  |   1 +
 arch/arm/dts/Makefile|   2 +
 arch/arm/dts/bcm6856.dtsi| 103 +++
 arch/arm/dts/bcm96856.dts|  30 +++
 arch/arm/mach-bcmbca/Kconfig |   8 ++
 arch/arm/mach-bcmbca/Makefile|   1 +
 arch/arm/mach-bcmbca/bcm6856/Kconfig |  17 
 arch/arm/mach-bcmbca/bcm6856/Makefile|   5 ++
 arch/arm/mach-bcmbca/bcm6856/mmu_table.c |  32 +++
 board/broadcom/bcmbca/Kconfig|   7 ++
 configs/bcm96856_defconfig   |  23 +
 include/configs/bcm96856.h   |  11 +++
 12 files changed, 240 insertions(+)
 create mode 100644 arch/arm/dts/bcm6856.dtsi
 create mode 100644 arch/arm/dts/bcm96856.dts
 create mode 100644 arch/arm/mach-bcmbca/bcm6856/Kconfig
 create mode 100644 arch/arm/mach-bcmbca/bcm6856/Makefile
 create mode 100644 arch/arm/mach-bcmbca/bcm6856/mmu_table.c
 create mode 100644 configs/bcm96856_defconfig
 create mode 100644 include/configs/bcm96856.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d0a5b2352cc8..1f50dad583ce 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -230,6 +230,7 @@ N:  bcm[9]?63178
 N: bcm[9]?6756
 N: bcm[9]?6813
 N: bcm[9]?6846
+N: bcm[9]?6856
 N: bcm[9]?6878
 
 ARM BROADCOM BCMSTB
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index c55bc3569662..a32bdf8c9f17 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -1182,6 +1182,8 @@ dtb-$(CONFIG_BCM6813) += \
bcm96813.dtb
 dtb-$(CONFIG_BCM6846) += \
bcm96846.dtb
+dtb-$(CONFIG_BCM6856) += \
+   bcm96856.dtb
 dtb-$(CONFIG_BCM6878) += \
bcm96878.dtb
 
diff --git a/arch/arm/dts/bcm6856.dtsi b/arch/arm/dts/bcm6856.dtsi
new file mode 100644
index ..0bce6497219f
--- /dev/null
+++ b/arch/arm/dts/bcm6856.dtsi
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2022 Broadcom Ltd.
+ */
+
+#include 
+#include 
+
+/ {
+   compatible = "brcm,bcm6856", "brcm,bcmbca";
+   #address-cells = <2>;
+   #size-cells = <2>;
+
+   interrupt-parent = <>;
+
+   cpus {
+   #address-cells = <2>;
+   #size-cells = <0>;
+
+   B53_0: cpu@0 {
+   compatible = "brcm,brahma-b53";
+   device_type = "cpu";
+   reg = <0x0 0x0>;
+   next-level-cache = <_0>;
+   enable-method = "psci";
+   };
+
+   B53_1: cpu@1 {
+   compatible = "brcm,brahma-b53";
+   device_type = "cpu";
+   reg = <0x0 0x1>;
+   next-level-cache = <_0>;
+   enable-method = "psci";
+   };
+
+   L2_0: l2-cache0 {
+   compatible = "cache";
+   };
+   };
+
+   timer {
+   compatible = "arm,armv8-timer";
+   interrupts = ,
+   ,
+   ,
+   ;
+   };
+
+   pmu: pmu {
+   compatible = "arm,cortex-a53-pmu";
+   interrupts = ,
+   ;
+   interrupt-affinity = <_0>, <_1>;
+   };
+
+   clocks: clocks {
+   periph_clk:periph-clk {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <2>;
+   };
+   };
+
+   psci {
+   compatible = "arm,psci-0.2";
+   method = "smc";
+   };
+
+   axi@8100 {
+   compatible = "simple-bus";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges = <0x0 0x0 0x8100 0x8000>;
+
+   gic: interrupt-controller@1000 {
+   compatible = "arm,gic-400";
+   #interrupt-cells = <3>;
+   interrupt-controller;
+   reg = <0x1000 0x1000>, /* GICD */
+   <0x2000 0x2000>, /* GICC */
+   <0x4000 0x2000>, /* GICH */
+   <0x6000 0x2000>; /* GICV */
+   interrupts = ;
+   };
+   };

[PATCH v2 0/3] arm: bcmbca: move bcm68360 support under CONFIG_ARCH_BCMBCA

2022-08-15 Thread William Zhang
BCM68360 is a variant for the main PON chip BCM6856 which is
part of the Broadcom BCA (Broadband Carrier Access origin) chipset
family. BCM68360 was originally added by Philippe before Broadcom
started to upstream the support for BCMBCA SoCs. The ARM based Broadband
SoC family is now supported under the unified ARCH_BCMBCA config. This
patch series migrate the BCM68360 support under the config of
ARCH_BCMBCA and BCM6856.

This patch series need to apply on top of my previous patch series [1].

[1] https://lists.denx.de/pipermail/u-boot/2022-August/491581.html

Changes in v2:
- Bring Philippe Reynes copyright tag from 68360 dts to 6856 dts

William Zhang (3):
  arm: bcmbca: add bcm6856 SoC support under CONFIG_ARCH_BCMBCA
  arm: bcmbca: remove bcm68360 support under CONFIG_ARCH_BCM68360
  arm: bcmbca: make bcm68360 driver depending on CONFIG_BCM6856

 MAINTAINERS  |   1 +
 arch/arm/Kconfig |   7 -
 arch/arm/dts/Makefile|   6 +-
 arch/arm/dts/bcm68360.dtsi   | 217 ---
 arch/arm/dts/bcm6856.dtsi| 253 +++
 arch/arm/dts/bcm968360bg.dts |   6 +-
 arch/arm/dts/bcm96856.dts|  30 +++
 arch/arm/mach-bcmbca/Kconfig |   8 +
 arch/arm/mach-bcmbca/Makefile|   1 +
 arch/arm/mach-bcmbca/bcm6856/Kconfig |  17 ++
 arch/arm/mach-bcmbca/bcm6856/Makefile|   5 +
 arch/arm/mach-bcmbca/bcm6856/mmu_table.c |  32 +++
 board/broadcom/bcm968360bg/Kconfig   |  17 --
 board/broadcom/bcm968360bg/MAINTAINERS   |   6 -
 board/broadcom/bcm968360bg/Makefile  |   3 -
 board/broadcom/bcm968360bg/bcm968360bg.c |  62 --
 board/broadcom/bcmbca/Kconfig|   7 +
 configs/bcm968360bg_ram_defconfig|  10 +-
 configs/bcm96856_defconfig   |  23 +++
 drivers/gpio/Kconfig |   2 +-
 drivers/led/Kconfig  |   2 +-
 drivers/mtd/nand/raw/Kconfig |   2 +-
 drivers/spi/Kconfig  |   2 +-
 drivers/watchdog/Kconfig |   2 +-
 include/configs/bcm96856.h   |  15 ++
 include/configs/broadcom_bcm968360bg.h   |  32 ---
 26 files changed, 408 insertions(+), 360 deletions(-)
 delete mode 100644 arch/arm/dts/bcm68360.dtsi
 create mode 100644 arch/arm/dts/bcm6856.dtsi
 create mode 100644 arch/arm/dts/bcm96856.dts
 create mode 100644 arch/arm/mach-bcmbca/bcm6856/Kconfig
 create mode 100644 arch/arm/mach-bcmbca/bcm6856/Makefile
 create mode 100644 arch/arm/mach-bcmbca/bcm6856/mmu_table.c
 delete mode 100644 board/broadcom/bcm968360bg/Kconfig
 delete mode 100644 board/broadcom/bcm968360bg/MAINTAINERS
 delete mode 100644 board/broadcom/bcm968360bg/Makefile
 delete mode 100644 board/broadcom/bcm968360bg/bcm968360bg.c
 create mode 100644 configs/bcm96856_defconfig
 create mode 100644 include/configs/bcm96856.h
 delete mode 100644 include/configs/broadcom_bcm968360bg.h

-- 
2.37.1



smime.p7s
Description: S/MIME Cryptographic Signature


Re: [BISECTED] BeagleBone Black doesn't boot after a58147c2dbbf

2022-08-15 Thread Matwey V. Kornilov
пн, 15 авг. 2022 г. в 20:53, Nishanth Menon :
>
> On 20:30-20220815, Matwey V. Kornilov wrote:
> > Hi Nishanth,
> >
> > I just reverted 0dba4586 and have the following diff in the config:
> >
> > diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig
> > index b500ed0fdd..b403901879 100644
> > --- a/configs/am335x_evm_defconfig
> > +++ b/configs/am335x_evm_defconfig
> > @@ -9,6 +9,9 @@ CONFIG_AM335X_USB0=y
> >  CONFIG_AM335X_USB0_PERIPHERAL=y
> >  CONFIG_AM335X_USB1=y
> >  CONFIG_SPL=y
> > +CONFIG_DEBUG_UART_BASE=0x44e09000
> > +CONFIG_DEBUG_UART_CLOCK=4800
> > +CONFIG_DEBUG_UART=y
> >  CONFIG_DISTRO_DEFAULTS=y
> >  CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> >  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x4030ff00
> > @@ -102,6 +105,9 @@ CONFIG_DRIVER_TI_CPSW=y
> >  CONFIG_DM_PMIC=y
> >  # CONFIG_SPL_DM_PMIC is not set
> >  CONFIG_PMIC_TPS65217=y
> > +CONFIG_DEBUG_UART_OMAP=y
> > +CONFIG_DEBUG_UART_SHIFT=2
> > +CONFIG_DEBUG_UART_ANNOUNCE=y
> >  CONFIG_SPI=y
> >  CONFIG_DM_SPI=y
> >  CONFIG_OMAP3_SPI=y
> >
> >
> > I've applied your patch and see the following output now:
>
> Thanks.
>
> >
> > 
> > ti_i2c_eeprom_get: 97: rc=0 header=0xee3355aa
> > ti_i2c_eeprom_get: 101: rc=0
> > ti_i2c_eeprom_get: 109: rc=0
> > ti_i2c_eeprom_get: 120: header=0xee3355aa
>
> 1 byte read operation passed here. so it never enters the 2 byte read op
> check. Is'nt this supposed to be a 2 byte addressing eeprom device?
>
> I wonder if changing that code to:
> (void)dm_i2c_read(dev, 0, (uint8_t *)_read, 4);
> if (hdr_read != header) {
> to:
>
> rc = dm_i2c_read(dev, 0, (uint8_t *)_read, 4);
> if (rc || (hdr_read != header) {
>
> might help?
>
> > ti_i2c_eeprom_get: 138: header=0xee3355aa
>
> > ti_i2c_eeprom_get: 143: rc=0
> > ti_i2c_eeprom_get: 191: Out OK
>
> So the header for sure matched, but not the data, I presume. Can we
> cross check with the updated debug printf("ep[%d]=0x%02x\n",i, ep[i]);
> that I added below?
>
> > Bad EEPROM or unknown board, cannot configure pinmux.
> >
>
> 8<---
> diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
> index ed34991377ee..34dfc1acb3a0 100644
> --- a/board/ti/common/board_detect.c
> +++ b/board/ti/common/board_detect.c
> @@ -90,13 +90,16 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, 
> int dev_addr,
> int rc;
>
>  #if CONFIG_IS_ENABLED(DM_I2C)
> +   int i;
> struct udevice *dev;
> struct udevice *bus;
>
> rc = uclass_get_device_by_seq(UCLASS_I2C, bus_addr, );
> +   printf("%s: %d: rc=%d header=0x%08x\n", __func__, __LINE__, rc, 
> header);
> if (rc)
> return rc;
> rc = dm_i2c_probe(bus, dev_addr, 0, );
> +   printf("%s: %d: rc=%d\n", __func__, __LINE__, rc);
> if (rc)
> return rc;
>
> @@ -104,6 +107,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, 
> int dev_addr,
>  * Read the header first then only read the other contents.
>  */
> rc = i2c_set_chip_offset_len(dev, 1);
> +   printf("%s: %d: rc=%d\n", __func__, __LINE__, rc);
> if (rc)
> return rc;
>
> @@ -114,6 +118,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, 
> int dev_addr,
>  * addressing works
>  */
> (void)dm_i2c_read(dev, 0, (uint8_t *)_read, 4);
> +   printf("%s: %d: header=0x%08x\n", __func__, __LINE__, hdr_read);
>
> /* Corrupted data??? */
> if (hdr_read != header) {
> @@ -122,24 +127,32 @@ static int __maybe_unused ti_i2c_eeprom_get(int 
> bus_addr, int dev_addr,
>  * 2 byte address (some newer boards need this..)
>  */
> rc = i2c_set_chip_offset_len(dev, 2);
> +   printf("%s: %d: rc=%d\n", __func__, __LINE__, rc);
> if (rc)
> return rc;
>
> rc = dm_i2c_read(dev, 0, (uint8_t *)_read, 4);
> +   printf("%s: %d: rc=%d\n", __func__, __LINE__, rc);
> if (rc)
> return rc;
> }
> +   printf("%s: %d: header=0x%08x\n", __func__, __LINE__, hdr_read);
> if (hdr_read != header)
> return -1;
>
> rc = dm_i2c_read(dev, 0, ep, size);
> +   printf("%s: %d: rc=%d\n", __func__, __LINE__, rc);
> if (rc)
> return rc;
> +
> +   for (i = 0; 

Re: [BISECTED] BeagleBone Black doesn't boot after a58147c2dbbf

2022-08-15 Thread Nishanth Menon
On 20:30-20220815, Matwey V. Kornilov wrote:
> Hi Nishanth,
> 
> I just reverted 0dba4586 and have the following diff in the config:
> 
> diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig
> index b500ed0fdd..b403901879 100644
> --- a/configs/am335x_evm_defconfig
> +++ b/configs/am335x_evm_defconfig
> @@ -9,6 +9,9 @@ CONFIG_AM335X_USB0=y
>  CONFIG_AM335X_USB0_PERIPHERAL=y
>  CONFIG_AM335X_USB1=y
>  CONFIG_SPL=y
> +CONFIG_DEBUG_UART_BASE=0x44e09000
> +CONFIG_DEBUG_UART_CLOCK=4800
> +CONFIG_DEBUG_UART=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x4030ff00
> @@ -102,6 +105,9 @@ CONFIG_DRIVER_TI_CPSW=y
>  CONFIG_DM_PMIC=y
>  # CONFIG_SPL_DM_PMIC is not set
>  CONFIG_PMIC_TPS65217=y
> +CONFIG_DEBUG_UART_OMAP=y
> +CONFIG_DEBUG_UART_SHIFT=2
> +CONFIG_DEBUG_UART_ANNOUNCE=y
>  CONFIG_SPI=y
>  CONFIG_DM_SPI=y
>  CONFIG_OMAP3_SPI=y
> 
> 
> I've applied your patch and see the following output now:

Thanks.

> 
> 
> ti_i2c_eeprom_get: 97: rc=0 header=0xee3355aa
> ti_i2c_eeprom_get: 101: rc=0
> ti_i2c_eeprom_get: 109: rc=0
> ti_i2c_eeprom_get: 120: header=0xee3355aa

1 byte read operation passed here. so it never enters the 2 byte read op
check. Is'nt this supposed to be a 2 byte addressing eeprom device?

I wonder if changing that code to:
(void)dm_i2c_read(dev, 0, (uint8_t *)_read, 4);
if (hdr_read != header) {
to:

rc = dm_i2c_read(dev, 0, (uint8_t *)_read, 4);
if (rc || (hdr_read != header) {

might help?

> ti_i2c_eeprom_get: 138: header=0xee3355aa

> ti_i2c_eeprom_get: 143: rc=0
> ti_i2c_eeprom_get: 191: Out OK

So the header for sure matched, but not the data, I presume. Can we
cross check with the updated debug printf("ep[%d]=0x%02x\n",i, ep[i]);
that I added below?

> Bad EEPROM or unknown board, cannot configure pinmux.
> 

8<---
diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
index ed34991377ee..34dfc1acb3a0 100644
--- a/board/ti/common/board_detect.c
+++ b/board/ti/common/board_detect.c
@@ -90,13 +90,16 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, 
int dev_addr,
int rc;
 
 #if CONFIG_IS_ENABLED(DM_I2C)
+   int i;
struct udevice *dev;
struct udevice *bus;
 
rc = uclass_get_device_by_seq(UCLASS_I2C, bus_addr, );
+   printf("%s: %d: rc=%d header=0x%08x\n", __func__, __LINE__, rc, header);
if (rc)
return rc;
rc = dm_i2c_probe(bus, dev_addr, 0, );
+   printf("%s: %d: rc=%d\n", __func__, __LINE__, rc);
if (rc)
return rc;
 
@@ -104,6 +107,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, 
int dev_addr,
 * Read the header first then only read the other contents.
 */
rc = i2c_set_chip_offset_len(dev, 1);
+   printf("%s: %d: rc=%d\n", __func__, __LINE__, rc);
if (rc)
return rc;
 
@@ -114,6 +118,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, 
int dev_addr,
 * addressing works
 */
(void)dm_i2c_read(dev, 0, (uint8_t *)_read, 4);
+   printf("%s: %d: header=0x%08x\n", __func__, __LINE__, hdr_read);
 
/* Corrupted data??? */
if (hdr_read != header) {
@@ -122,24 +127,32 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, 
int dev_addr,
 * 2 byte address (some newer boards need this..)
 */
rc = i2c_set_chip_offset_len(dev, 2);
+   printf("%s: %d: rc=%d\n", __func__, __LINE__, rc);
if (rc)
return rc;
 
rc = dm_i2c_read(dev, 0, (uint8_t *)_read, 4);
+   printf("%s: %d: rc=%d\n", __func__, __LINE__, rc);
if (rc)
return rc;
}
+   printf("%s: %d: header=0x%08x\n", __func__, __LINE__, hdr_read);
if (hdr_read != header)
return -1;
 
rc = dm_i2c_read(dev, 0, ep, size);
+   printf("%s: %d: rc=%d\n", __func__, __LINE__, rc);
if (rc)
return rc;
+
+   for (i = 0; i< size; i++)
+   printf("ep[%d]=0x%02x\n",i, ep[i]);
 #else
u32 byte;
 
gpi2c_init();
rc = ti_i2c_eeprom_init(bus_addr, dev_addr);
+   printf("%s: %d: rc=%d header=0x%08x\n", __func__, __LINE__, rc, header);
if (rc)
return rc;
 
@@ -157,6 +170,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, 
int dev_addr,
(void)i2c_read(dev_addr, 0x0, byte, (uint8_t *)_read, 4);
 
/* Corrupted data??? */
+   printf("%s: %d: header=0x%08x\n", __func__, __LINE__, hdr_read);
if (hdr_read != header) {
/*
 * read the eeprom header u

Re: [PATCH] [RFC] cmd: i2c: fix default address len for DM_I2C

2022-08-15 Thread Tim Harvey
On Sat, Aug 13, 2022 at 7:59 AM Simon Glass  wrote:
>
> Hi Tim,
>
> On Thu, 11 Aug 2022 at 11:57, Tim Harvey  wrote:
> >
> > According to the comment block "The default {addr} parameter is one byte
> > (.1) which works well for memories and registers with 8 bits of address
> > space."
> >
> > While this is true for legacy I2C a default length of -1 is being passed
> > for DM_I2C which results in a usage error.
> >
> > Restore the documented behavior by always using a default alen of 1.
> >
> > Signed-off-by: Tim Harvey 
> >
> > This is an RFC as I'm unclear if we want to restore the legacy usage or
> > enforce a new usage (in which case the comment block should be updated)
> > and I'm not clear if this is documented in other places. If the decision
> > is to enforce a new usage then it is unclear to me how to specifiy the
> > default alen as there is no command for that (i2c alen [len]?).
> > ---
> >  cmd/i2c.c | 10 --
> >  1 file changed, 10 deletions(-)
> >
>
> Can you dig into this a little more on your board? DEFAULT_ADDR_LEN is
> set to -1 so that by default it does not get set by the command, and
> the existing alen is used.
>
> This is necessary for driver model, since the alen can be set by the
> peripheral device and we don't want to overwrite it:
>
> if (!ret && alen != -1)
> ret = i2c_set_chip_offset_len(dev, alen);
>

Simon,

Here's some annotated debug prints added where chip_offset is passed/set:
Model: Gateworks Venice GW73xx-0x i.MX8MM Development Kit
DRAM:  1 GiB
i2c_chip_of_to_plat gsc@20 offset_len=1
i2c_chip_of_to_plat pmic@69 offset_len=1
i2c_get_chip i2c@30a2 0x51 offset_len=1
i2c_bind_driver i2c@30a2 offset_len=1
i2c_set_chip_offset_len generic_51 offset_len=1
dm_i2c_read generic_51 offset=0 offset_len=1
i2c_setup_offset 0x51 offset=0 offset_len=1
Core:  209 devices, 27 uclasses, devicetree: separate
...
u-boot=> i2c dev 0 && i2c md 0x51 0 50
Setting bus to 0
i2c - I2C sub-system

Usage:
i2c bus [muxtype:muxaddr:muxchannel] - show I2C bus info
...

So the chip I noticed this issue with is 0x51 which an atmel,24c02
compatible eeprom which imx8mm-venice-gw700x.dtsi defines 4 of
(i2c1-0x50, i2c1-0x51, i2c1-0x52, i2c1-0x53). You can see above
i2c1-0x51 (i2c1=i2c@30a2) is accessed early as it holds the board
model information and at that point in time it's accessed with alen=1
(which I specify in board/gateworks/venice/eeprom.c:eeprom_read()) but
by the time the 'i2c md 0x51 0 50' comes around
i2c_get_chip_offset_len() returns 0 for this device. The other eeprom
devices on i2c1 are never accessed by a driver so they would never
have a 'default' alen set.

Where is the initial setting of alen supposed to have come?

Best Regards,

Tim

>
> > diff --git a/cmd/i2c.c b/cmd/i2c.c
> > index bd04b14024be..c57271479e81 100644
> > --- a/cmd/i2c.c
> > +++ b/cmd/i2c.c
> > @@ -118,17 +118,7 @@ static uchar i2c_no_probes[] = CONFIG_SYS_I2C_NOPROBES;
> >  #endif
> >
> >  #define DISP_LINE_LEN  16
> > -
> > -/*
> > - * Default for driver model is to use the chip's existing address length.
> > - * For legacy code, this is not stored, so we need to use a suitable
> > - * default.
> > - */
> > -#if CONFIG_IS_ENABLED(DM_I2C)
> > -#define DEFAULT_ADDR_LEN   (-1)
> > -#else
> >  #define DEFAULT_ADDR_LEN   1
> > -#endif
> >
> >  #if CONFIG_IS_ENABLED(DM_I2C)
> >  static struct udevice *i2c_cur_bus;
> > --
> > 2.25.1
> >
>
> Regards,
> Simon


Re: [PATCH v2 00/24] blk: Rationalise the block interface

2022-08-15 Thread Simon Glass
Hi Johan,

On Sun, 14 Aug 2022 at 00:34, Johan Jonker  wrote:
>
>
>
> On 8/12/22 17:11, Simon Glass wrote:
> > Hi Johan,
> >
> > On Fri, 12 Aug 2022 at 07:51, Johan Jonker  wrote:
> >>
> >> Hi Simon and others,
> >>
> >> Some comments. Have a look if it's useful.
> >> Include is an example of how currently a new Rockchip external RKNAND FTL 
> >> block device must be included in U-boot.
> >> Changes must be made all over the place. EFI has now become a somewhat 
> >> "obligation" for block devices, then handling
> >> should be made more easy I propose.
> >>
> >> 1:
> >> The creation and removal of block devices is kind of dynamic with the 
> >> blk_create_device function.
> >> Is it possible to make the necessary functions in efi_device_path.c and 
> >> part.c more flexible as well by
> >> a new "blk_create_efi_device()" and "blk_remove_efi_device()" function? So 
> >> everything combined in one function.
> >
> > Do you mean to automatically create a device with the right uclass?
> > Yes I think that could be done, but I have not looked at it.
>
> Hi,
>
> Some comments...
> Reduced the number of public CC's.
> I have included a patch with call back function as example, so 
> efi_device_path.c and part.c don't have to be changed for every new block 
> device.
> Make EFI framework code without class specific things.
> TODO define { UCLASS_X, "x" }, in class driver and not in a fixed 
> array.
> Look up for x ==> UCLASS_X needs a lot more work which is out of 
> scope for my capacity now.

I'm not keen on callbacks, but we could use events if necessary.

Would you please send your patches based on my series and to the
mailing list? You can mark them RFC if you are unsure, but I really
cannot see myself replying to patches that are sent as an attachment
and don't appear in patchwork :-)

Regards,
Simon


Re: [PATCH v2 02/10] binman: Make compressed data header optional

2022-08-15 Thread Simon Glass
Hi Stefan,

On Mon, 15 Aug 2022 at 01:09, Stefan Herbrechtsmeier
 wrote:
>
> Hi Simon,
>
> Am 13.08.2022 um 16:59 schrieb Simon Glass:
> > Hi Stefan,
> >
> > On Mon, 8 Aug 2022 at 04:51, Stefan Herbrechtsmeier

[..]

> >>   # This is imported if needed
> >>   state = None
> >> @@ -17,6 +19,9 @@ class Entry_blob_dtb(Entry_blob):
> >>   This is a blob containing a device tree. The contents of the blob are
> >>   obtained from the list of available device-tree files, managed by the
> >>   'state' module.
> >> +
> >> +Additional attributes:
> >> +prepend: Header used (e.g. 'length'), 'none' if none
> >>   """
> >>   def __init__(self, section, etype, node):
> >>   # Put this here to allow entry-docs and help to work without 
> >> libfdt
> >> @@ -25,6 +30,12 @@ class Entry_blob_dtb(Entry_blob):
> >>
> >>   super().__init__(section, etype, node)
> >>
> >> +self.prepend = 'none'
> >
> > None ?
>
> I copy this from the compress attribute. You only need one check to
> support a missing value or a 'none' value. But I don't need this check
> and can use None.

OK I see. The idea there was that people might want to explicitly say
'none'. I;m not sure how use that is, particularly with prepend, but
I'm OK with either way.

>
> >
> >> +
> >> +def ReadNode(self):
> >> +super().ReadNode()
> >> +self.prepend = fdt_util.GetString(self._node, 'prepend', 'none')
> >
> > Can you drop the 'none' so that it uses None instead?
>
> Is 'none' a valid entry? Do we need to distinguish between 'none' and an
> invalid value?

Eventually we do...but for now bad things happen. See the TODO in
binman for some of that.
>
> > Aso we should check for a valid value here - e.g. it must be 'length'
> > and not something else, otherwise self.Raise()
>
> Okay. I will remove the 'none' and only support 'length'.

As above, up to you. I had forgotten about the compress thing.

Regards,
Simon


Re: [PATCH v3 3/8] cyclic: Integrate cyclic infrastructure into WATCHDOG_RESET

2022-08-15 Thread Simon Glass
Hi Stefan,

On Mon, 15 Aug 2022 at 10:16, Stefan Roese  wrote:
>
> Hi Simon,
>
> On 05.08.22 18:48, Simon Glass wrote:
> > Hi Stefan,
> >
> > On Fri, 5 Aug 2022 at 08:26, Stefan Roese  wrote:
> >>
> >> This patch integrates the main function responsible for calling all
> >> registered cyclic functions cyclic_run() into the common WATCHDOG_RESET
> >> macro. This guarantees that cyclic_run() is executed very often, which
> >> is necessary for the cyclic functions to get scheduled and executed at
> >> their configured periods.
> >>
> >> If CONFIG_WATCHDOG is not enabled, only cyclic_run() without calling
> >> watchdog_reset(). This guarantees that the cyclic functionality does not
> >> rely on CONFIG_WATCHDOG being enabled.
> >>
> >> Signed-off-by: Stefan Roese 
> >> ---
> >> v3:
> >> - No change
> >> v2:
> >> - No change
> >>
> >>   fs/cramfs/uncompress.c |  2 +-
> >>   include/watchdog.h | 23 ---
> >>   2 files changed, 21 insertions(+), 4 deletions(-)
> >
> > Reviewed-by: Simon Glass 
> >
> >>
> >> diff --git a/fs/cramfs/uncompress.c b/fs/cramfs/uncompress.c
> >> index f431cc46c1f7..38e10e2e4422 100644
> >> --- a/fs/cramfs/uncompress.c
> >> +++ b/fs/cramfs/uncompress.c
> >> @@ -62,7 +62,7 @@ int cramfs_uncompress_init (void)
> >>  stream.avail_in = 0;
> >>
> >>   #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
> >> -   stream.outcb = (cb_func) WATCHDOG_RESET;
> >> +   stream.outcb = (cb_func)watchdog_reset_func;
> >>   #else
> >>  stream.outcb = Z_NULL;
> >>   #endif /* CONFIG_HW_WATCHDOG */
> >> diff --git a/include/watchdog.h b/include/watchdog.h
> >> index 813cc8f2a5d3..0a9777edcbad 100644
> >> --- a/include/watchdog.h
> >> +++ b/include/watchdog.h
> >> @@ -11,6 +11,8 @@
> >>   #define _WATCHDOG_H_
> >>
> >>   #if !defined(__ASSEMBLY__)
> >> +#include 
> >> +
> >>   /*
> >>* Reset the watchdog timer, always returns 0
> >>*
> >> @@ -60,11 +62,16 @@ int init_func_watchdog_reset(void);
> >>  /* Don't require the watchdog to be enabled in 
> >> SPL */
> >>  #if defined(CONFIG_SPL_BUILD) &&\
> >>  !defined(CONFIG_SPL_WATCHDOG)
> >> -   #define WATCHDOG_RESET() {}
> >> +   #define WATCHDOG_RESET() { \
> >> +   cyclic_run(); \
> >> +   }
> >>  #else
> >>  extern void watchdog_reset(void);
> >>
> >> -   #define WATCHDOG_RESET watchdog_reset
> >> +   #define WATCHDOG_RESET() { \
> >> +   watchdog_reset(); \
> >> +   cyclic_run(); \
> >
> > Doesn't this create two function calls from every reset site? I worry
> > about code size.
>
> Good point. Even though the world build did not trigger any new problems
> with oversized images.
>
> > I suggest creating a new function like
> > check_watchdog() which you can define (in the C file) with
> > IS_ENABLED() as either empty, calling watchdog_reset() and/or calling
> > cyclic_run(). LTO should help here.
>
> I tried a bit to get clean up this ugly #ifdef construct. And move this
> as you suggested into a C file. Just now I noticed, that we don't have
> a matching C file, which is compiled in all cases. wdt-uclass.c and
> cyclic.c are both only compiled when actually enabled in Kconfig.
>
> So do you have some other / better idea on how to improve this?
>
> BTW: Moving the watchdog integration into the cyclic infrastructure in
> some follow-up patches will make all this much cleaner AFAICT.

If you are going to do this soon, then I suggest not working about it too much.

But you could create wdt_common.c or similar. It should be OK to
always compile it since the code will be dropped if nothing calls it.

Regards,
Simon


Re: binman warning/failure when blobs are missing

2022-08-15 Thread Simon Glass
Hi Tom,

On Sat, 13 Aug 2022 at 06:36, Tom Rini  wrote:
>
> On Fri, Aug 12, 2022 at 08:21:05PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 12 Aug 2022 at 10:11, Tom Rini  wrote:
> > >
> > > On Fri, Aug 12, 2022 at 09:11:11AM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 11 Aug 2022 at 19:03, Tom Rini  wrote:
> > > > >
> > > > > On Thu, Aug 11, 2022 at 06:08:51PM -0600, Simon Glass wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Thu, 11 Aug 2022 at 11:44, Tom Rini  wrote:
> > > > > > >
> > > > > > > On Thu, Aug 11, 2022 at 09:59:05AM -0700, Tim Harvey wrote:
> > > > > > > > On Thu, Aug 11, 2022 at 9:48 AM Tom Rini  
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Thu, Aug 11, 2022 at 09:27:44AM -0700, Tim Harvey wrote:
> > > > > > > > >
> > > > > > > > > > Greetings,
> > > > > > > > > >
> > > > > > > > > > After a couple of hours troubleshooting a bad boot image 
> > > > > > > > > > today I
> > > > > > > > > > realized the issue was that I had some 0 byte files for the 
> > > > > > > > > > lpddr4
> > > > > > > > > > training blobs that are part of the imx8mp binman created 
> > > > > > > > > > image.
> > > > > > > > > >
> > > > > > > > > > Digging in I found that if a blob referenced in the binman 
> > > > > > > > > > node is
> > > > > > > > > > missing a warning will be output but the missing files will 
> > > > > > > > > > be
> > > > > > > > > > 'created' as 0 byte files such that the next time you build 
> > > > > > > > > > you will
> > > > > > > > > > get no warning (but will have a non-working image). 
> > > > > > > > > > Additionally the
> > > > > > > > > > error does not cause a non-zero exit code.
> > > > > > > > > >
> > > > > > > > > > I'm not that fluent in python these days, and don't have 
> > > > > > > > > > the time for
> > > > > > > > > > a while to try and fix this but I figured I would at least 
> > > > > > > > > > send this
> > > > > > > > > > email in case someone else does.
> > > > > > > > > >
> > > > > > > > > > Example:
> > > > > > > > > > # rm *lpddr4*.bin # make sure lpddr4*.bin files referenced 
> > > > > > > > > > in binman
> > > > > > > > > > nodes are missing
> > > > > > > > > > # make distclean imx8mp_venice_defconfig flash.bin && echo 
> > > > > > > > > > "build ok"
> > > > > > > > > > ...
> > > > > > > > > >   BINMAN  flash.bin
> > > > > > > > > > Image 'main-section' is missing external blobs and is 
> > > > > > > > > > non-functional: ddr-1d-ime
> > > > > > > > > > m-fw ddr-1d-dmem-fw ddr-2d-imem-fw ddr-2d-dmem-fw
> > > > > > > > > > Image 'main-section' has faked external blobs and is 
> > > > > > > > > > non-functional: lpddr4_pmu_
> > > > > > > > > > train_1d_imem_202006.bin 
> > > > > > > > > > lpddr4_pmu_train_1d_dmem_202006.bin lpddr4_pmu_train_2d
> > > > > > > > > > _imem_202006.bin lpddr4_pmu_train_2d_dmem_202006.bin
> > > > > > > > > >
> > > > > > > > > > Some images are invalid
> > > > > > > > > > ^^^ excellent warning
> > > > > > > > > > build ok
> > > > > > > > > > ^^^ not so great that there is a successful exit code
> > > > > > > > > > # make flash.bin && echo "build ok"
> > > > > > > > > > ...
> > > > > > > > > >   BINMAN  flash.bin
> > > > > > > > > > build ok
> > > > > > > > > > ^^^ absolutely horrible that 0 byte files were created and 
> > > > > > > > > > thus
> > > > > > > > > > everything looks good this time around!
> > > > > > > > > > # stat -c "%s %n" lpddr4*.bin
> > > > > > > > > > 0 lpddr4_pmu_train_1d_dmem_202006.bin
> > > > > > > > > > 0 lpddr4_pmu_train_1d_imem_202006.bin
> > > > > > > > > > 0 lpddr4_pmu_train_2d_dmem_202006.bin
> > > > > > > > > > 0 lpddr4_pmu_train_2d_imem_202006.bin
> > > > > > > > >
> > > > > > > > > So, this isn't the first time someone has had this problem. 
> > > > > > > > > On the one
> > > > > > > > > hand, we need CI to pass, and not require fetching of 
> > > > > > > > > arbitrary further
> > > > > > > > > images to assemble the binary. On the other hand, we don't 
> > > > > > > > > want users
> > > > > > > > > spending a bunch of time because something didn't work and 
> > > > > > > > > the normal
> > > > > > > > > way of conveying THIS WON'T WORK is a non-zero exit status. 
> > > > > > > > > Can we
> > > > > > > > > easily make some flag for buildman or binman that we do set 
> > > > > > > > > in CI but
> > > > > > > > > won't be set by users?
> > > > > > > > >
> > > > > > > >
> > > > > > > > Tom,
> > > > > > > >
> > > > > > > > Ok - that makes sense as far as the exit status goes. It would 
> > > > > > > > be MUCH
> > > > > > > > easier to catch an error like this if binman didn't create 0 
> > > > > > > > byte
> > > > > > > > files for missing files so that you at least get the (ascii 
> > > > > > > > colored)
> > > > > > > > message indicating the image is wrong.
> > > > > >
> > > > > > Please see this:
> > > > > >
> > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20220807154708.1418967-2-...@chromium.org/
> > > > > >
> > > > > > > >
> > > > > > > > I do 

Re: [PATCH v2 2/2] patman: Add documentation to doc/

2022-08-15 Thread Simon Glass
Hi Ralph,

On Mon, 15 Aug 2022 at 08:41, Ralph Siemsen  wrote:
>
> Hello Heinrich,
>
> FYI, I had some trouble trying to apply your changes (patch seems to
> be mangled?).
>
> Only one small question for you, see below.
>
> On Wed, Aug 10, 2022 at 2:42 AM Heinrich Schuchardt  
> wrote:
> >
> > diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
> > index 52151f6f16..f2e6d7636f 100644
> > --- a/tools/patman/patman.rst
> > +++ b/tools/patman/patman.rst
> > @@ -1,19 +1,31 @@
> >   .. SPDX-License-Identifier: GPL-2.0+
> >   .. Copyright (c) 2011 The Chromium OS Authors
> > +.. Simon Glass 
> > +.. v1, v2, 19-Oct-11
> > +.. revised v3 24-Nov-11
> > +.. revised v4 04-Jul-2020, with Patchwork integration
> >
> >   Patman patch manager
> >   
> >
> >   This tool is a Python script which:
> > +
> >   - Creates patch directly from your branch
> > +
> >   - Cleans them up by removing unwanted tags
> > +
> >   - Inserts a cover letter with change lists
> > +
> >   - Runs the patches through checkpatch.pl and its own checks
> > +
> >   - Optionally emails them out to selected people
> >
> >   It also has some Patchwork features:
> > +
> >   - shows review tags from Patchwork so you can update your local patches
> > +
> >   - pulls these down into a new branch on request
> > +
> >   - lists comments received on a series
> >
> >   It is intended to automate patch creation and make it a less
> > @@ -41,12 +53,15 @@ This tool requires a certain way of working:
> >
> >   - Maintain a number of branches, one for each patch series you are
> > working on
> > +
> >   - Add tags into the commits within each branch to indicate where the
> > series should be sent, cover letter, version, etc. Most of these are
> > normally in the top commit so it is easy to change them with 'git
> > commit --amend'
> > +
> >   - Each branch tracks the upstream branch, so that this script can
> > automatically determine the number of commits in it (optional)
> > +
> >   - Check out a branch, and run this script to create and send out your
> > patches. Weeks later, change the patches and repeat, knowing that you
> > will get a consistent result each time.
>
> Is it necessary to have a blank line between each of the "bullet"
> points? It seems to work fine as long as there is one blank line
> separating the paragraph from the first bullet item.

It seems not, I will take a look.

>
> Otherwise, it looks good to me.

Regards,
Simon


Re: [PATCH] image-fit: don't set compression if it can't be read

2022-08-15 Thread Simon Glass
On Mon, 15 Aug 2022 at 04:39, Daniel Golle  wrote:
>
> fit_image_get_comp() should not set value -1 in case it can't read
> the compression node. Instead, leave the value untouched in that case
> as it can be absent and a default value previously defined by the
> caller of fit_image_get_comp() should be used.
>
> As a result the warning message
> WARNING: 'compression' nodes for ramdisks are deprecated, please fix your 
> .its file!
> no longer shows if the compression node is actually absent.
>
> Signed-off-by: Daniel Golle 
> ---
>  boot/bootm.c | 6 ++
>  boot/image-fit.c | 3 +--
>  cmd/ximg.c   | 7 ++-
>  3 files changed, 5 insertions(+), 11 deletions(-)

Reviewed-by: Simon Glass 


Re: [BISECTED] BeagleBone Black doesn't boot after a58147c2dbbf

2022-08-15 Thread Matwey V. Kornilov
Hi Nishanth,

I just reverted 0dba4586 and have the following diff in the config:

diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig
index b500ed0fdd..b403901879 100644
--- a/configs/am335x_evm_defconfig
+++ b/configs/am335x_evm_defconfig
@@ -9,6 +9,9 @@ CONFIG_AM335X_USB0=y
 CONFIG_AM335X_USB0_PERIPHERAL=y
 CONFIG_AM335X_USB1=y
 CONFIG_SPL=y
+CONFIG_DEBUG_UART_BASE=0x44e09000
+CONFIG_DEBUG_UART_CLOCK=4800
+CONFIG_DEBUG_UART=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x4030ff00
@@ -102,6 +105,9 @@ CONFIG_DRIVER_TI_CPSW=y
 CONFIG_DM_PMIC=y
 # CONFIG_SPL_DM_PMIC is not set
 CONFIG_PMIC_TPS65217=y
+CONFIG_DEBUG_UART_OMAP=y
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_DEBUG_UART_ANNOUNCE=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
 CONFIG_OMAP3_SPI=y


I've applied your patch and see the following output now:


ti_i2c_eeprom_get: 97: rc=0 header=0xee3355aa
ti_i2c_eeprom_get: 101: rc=0
ti_i2c_eeprom_get: 109: rc=0
ti_i2c_eeprom_get: 120: header=0xee3355aa
ti_i2c_eeprom_get: 138: header=0xee3355aa
ti_i2c_eeprom_get: 143: rc=0
ti_i2c_eeprom_get: 191: Out OK
Bad EEPROM or unknown board, cannot configure pinmux.



Re: [PATCH v3 5/8] cyclic: Add 'cyclic list' command

2022-08-15 Thread Stefan Roese

Hi Simon,

On 05.08.22 18:48, Simon Glass wrote:

On Fri, 5 Aug 2022 at 08:26, Stefan Roese  wrote:


This patch adds the cyclic command, which currently only supports the
'list' subcommand, to list all currently registered cyclic functions.
Here an example:

=> cyclic list
function: cyclic_demo, cpu-time: 7010 us, frequency: 99.80 times/s
function: cyclic_demo2, cpu-time: 1 us, frequency: 1.13 times/s

As you can see, the cpu-time is accounted, so that cyclic functions
that take too long might be discovered. Additionally the frequency is
logged.

Signed-off-by: Stefan Roese 
---
v3:
- No change
v2:
- Add depends on CYCLIC in Kconfig

  MAINTAINERS  |  1 +
  cmd/Kconfig  |  7 +++
  cmd/Makefile |  1 +
  cmd/cyclic.c | 40 
  4 files changed, 49 insertions(+)
  create mode 100644 cmd/cyclic.c


Reviewed-by: Simon Glass 



diff --git a/MAINTAINERS b/MAINTAINERS
index bfb5da16d78b..9d9d3fdd4069 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -791,6 +791,7 @@ F:  doc/arch/m68k.rst
  CYCLIC
  M: Stefan Roese 
  S: Maintained
+F: cmd/cyclic.c
  F: common/cyclic.c
  F: include/cyclic.h

diff --git a/cmd/Kconfig b/cmd/Kconfig
index a8260aa170d0..641c053003f0 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2494,6 +2494,13 @@ config CMD_CBSYSINFO
   memory by coreboot before jumping to U-Boot. It can be useful for
   debugging the beaaviour of coreboot or U-Boot.

+config CMD_CYCLIC
+   bool "cyclic - Show information about cyclic functions"
+   depends on CYCLIC


default y ?

It is nice for people to get the command by default rather than having
to hunt for it.


Good idea.


+   help
+ This enables the 'cyclic' command which provides information about
+ cyclic excution functions.


execution

Also needs another line or two of info - perhaps briefly explain what
these functions are for and point to docs?


Will change in the next version.

Thanks,
Stefan


+
  config CMD_DIAG
 bool "diag - Board diagnostics"
 help
diff --git a/cmd/Makefile b/cmd/Makefile
index 5e43a1e022e8..c5a4fc5a5cfc 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_CMD_DIAG) += diag.o
  endif
  obj-$(CONFIG_CMD_ADTIMG) += adtimg.o
  obj-$(CONFIG_CMD_ABOOTIMG) += abootimg.o
+obj-$(CONFIG_CMD_CYCLIC) += cyclic.o
  obj-$(CONFIG_CMD_EVENT) += event.o
  obj-$(CONFIG_CMD_EXTENSION) += extension_board.o
  obj-$(CONFIG_CMD_ECHO) += echo.o
diff --git a/cmd/cyclic.c b/cmd/cyclic.c
new file mode 100644
index ..1714db18e480
--- /dev/null
+++ b/cmd/cyclic.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * A general-purpose cyclic execution infrastructure, to allow "small"
+ * (run-time wise) functions to be executed at a specified frequency.
+ * Things like LED blinking or watchdog triggering are examples for such
+ * tasks.
+ *
+ * Copyright (C) 2022 Stefan Roese 
+ */
+
+#include 
+#include 
+#include 
+
+extern struct list_head cyclic_list;
+
+static int do_cyclic_list(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+   struct cyclic_struct *cyclic, *tmp;
+   uint64_t cnt, freq;
+
+   list_for_each_entry_safe(cyclic, tmp, _list, list) {
+   cnt = cyclic->run_cnt * 100ULL * 100ULL;
+   freq = cnt / (timer_get_us() - cyclic->start_time_us);
+   printf("function: %s, cpu-time: %lld us, frequency: %lld.%02lld 
times/s\n",
+  cyclic->name, cyclic->cpu_time_us,
+  freq / 100, freq % 100);
+   }
+
+   return 0;
+}
+
+#ifdef CONFIG_SYS_LONGHELP
+static char cyclic_help_text[] =
+   "cyclic list   - list cyclic functions";
+#endif
+
+U_BOOT_CMD_WITH_SUBCMDS(cyclic, "Cyclic", cyclic_help_text,
+   U_BOOT_SUBCMD_MKENT(list, 1, 1, do_cyclic_list));
--
2.37.1



Regards,
Simon


Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Re: [PATCH v3 8/8] cyclic: Add documentation

2022-08-15 Thread Stefan Roese

Hi Simon.

On 05.08.22 18:48, Simon Glass wrote:

Hi Stefan,

On Fri, 5 Aug 2022 at 08:26, Stefan Roese  wrote:


Add documentation for the cyclic function infrastructure, including the
cyclic command.

Signed-off-by: Stefan Roese 
---
v3:
- New patch

  doc/develop/cyclic.rst   | 50 
  doc/develop/index.rst|  1 +
  doc/usage/cmd/cyclic.rst | 45 
  doc/usage/index.rst  |  1 +
  4 files changed, 97 insertions(+)
  create mode 100644 doc/develop/cyclic.rst
  create mode 100644 doc/usage/cmd/cyclic.rst


Reviewed-by: Simon Glass 



diff --git a/doc/develop/cyclic.rst b/doc/develop/cyclic.rst
new file mode 100644
index ..f9cb2668b84c
--- /dev/null
+++ b/doc/develop/cyclic.rst
@@ -0,0 +1,50 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Cyclic functions
+
+
+The cyclic function execution infrastruture provides a way to periodically
+execute code, e.g. all 100ms. Examples for such functions might be LED


s/all/every/ ?


Thanks, changed in the next version.

Thanks,
Stefan


+blinking etc. The functions that are hooked into this cyclic list should
+be small timewise as otherwise the execution of the other code that relies
+on a high frequent polling (e.g. UART rx char ready check) might be
+delayed too much. To detect cyclic functions with a too long execution
+time, the Kconfig option `CONFIG_CYCLIC_MAX_CPU_TIME_US` is introduced,
+which configures the max allowed time for such a cyclic function. If it's
+execution time exceeds this time, this cyclic function will get removed
+from the cyclic list.


[..]

Regards,
SImon


Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Re: [PATCH v3 3/8] cyclic: Integrate cyclic infrastructure into WATCHDOG_RESET

2022-08-15 Thread Stefan Roese

Hi Simon,

On 05.08.22 18:48, Simon Glass wrote:

Hi Stefan,

On Fri, 5 Aug 2022 at 08:26, Stefan Roese  wrote:


This patch integrates the main function responsible for calling all
registered cyclic functions cyclic_run() into the common WATCHDOG_RESET
macro. This guarantees that cyclic_run() is executed very often, which
is necessary for the cyclic functions to get scheduled and executed at
their configured periods.

If CONFIG_WATCHDOG is not enabled, only cyclic_run() without calling
watchdog_reset(). This guarantees that the cyclic functionality does not
rely on CONFIG_WATCHDOG being enabled.

Signed-off-by: Stefan Roese 
---
v3:
- No change
v2:
- No change

  fs/cramfs/uncompress.c |  2 +-
  include/watchdog.h | 23 ---
  2 files changed, 21 insertions(+), 4 deletions(-)


Reviewed-by: Simon Glass 



diff --git a/fs/cramfs/uncompress.c b/fs/cramfs/uncompress.c
index f431cc46c1f7..38e10e2e4422 100644
--- a/fs/cramfs/uncompress.c
+++ b/fs/cramfs/uncompress.c
@@ -62,7 +62,7 @@ int cramfs_uncompress_init (void)
 stream.avail_in = 0;

  #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
-   stream.outcb = (cb_func) WATCHDOG_RESET;
+   stream.outcb = (cb_func)watchdog_reset_func;
  #else
 stream.outcb = Z_NULL;
  #endif /* CONFIG_HW_WATCHDOG */
diff --git a/include/watchdog.h b/include/watchdog.h
index 813cc8f2a5d3..0a9777edcbad 100644
--- a/include/watchdog.h
+++ b/include/watchdog.h
@@ -11,6 +11,8 @@
  #define _WATCHDOG_H_

  #if !defined(__ASSEMBLY__)
+#include 
+
  /*
   * Reset the watchdog timer, always returns 0
   *
@@ -60,11 +62,16 @@ int init_func_watchdog_reset(void);
 /* Don't require the watchdog to be enabled in SPL */
 #if defined(CONFIG_SPL_BUILD) &&\
 !defined(CONFIG_SPL_WATCHDOG)
-   #define WATCHDOG_RESET() {}
+   #define WATCHDOG_RESET() { \
+   cyclic_run(); \
+   }
 #else
 extern void watchdog_reset(void);

-   #define WATCHDOG_RESET watchdog_reset
+   #define WATCHDOG_RESET() { \
+   watchdog_reset(); \
+   cyclic_run(); \


Doesn't this create two function calls from every reset site? I worry
about code size.


Good point. Even though the world build did not trigger any new problems
with oversized images.


I suggest creating a new function like
check_watchdog() which you can define (in the C file) with
IS_ENABLED() as either empty, calling watchdog_reset() and/or calling
cyclic_run(). LTO should help here.


I tried a bit to get clean up this ugly #ifdef construct. And move this
as you suggested into a C file. Just now I noticed, that we don't have
a matching C file, which is compiled in all cases. wdt-uclass.c and
cyclic.c are both only compiled when actually enabled in Kconfig.

So do you have some other / better idea on how to improve this?

BTW: Moving the watchdog integration into the cyclic infrastructure in
some follow-up patches will make all this much cleaner AFAICT.

Thanks,
Stefan


+   }
 #endif
 #endif
 #else
@@ -74,11 +81,21 @@ int init_func_watchdog_reset(void);
 #if defined(__ASSEMBLY__)
 #define WATCHDOG_RESET /*XXX DO_NOT_DEL_THIS_COMMENT*/
 #else
-   #define WATCHDOG_RESET() {}
+   #define WATCHDOG_RESET() { \
+   cyclic_run(); \
+   }
 #endif /* __ASSEMBLY__ */
 #endif /* CONFIG_WATCHDOG && !__ASSEMBLY__ */
  #endif /* CONFIG_HW_WATCHDOG */

+#if !defined(__ASSEMBLY__)
+/* Currently only needed for fs/cramfs/uncompress.c */
+static inline void watchdog_reset_func(void)
+{
+   WATCHDOG_RESET();
+}
+#endif
+
  /*
   * Prototypes from $(CPU)/cpu.c.
   */
--
2.37.1



Regards,
Simon


Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


[PATCHv2 2/2] i2c: stm32f7: do not set the STOP condition on error

2022-08-15 Thread Jorge Ramirez-Ortiz
Sending the stop condition without waiting for transfer complete
has been found to lock the bus (BUSY) when NACKF is raised.

Tested accessing the NXP SE05X I2C device.
https://www.nxp.com/docs/en/application-note/AN12399.pdf

Signed-off-by: Jorge Ramirez-Ortiz 
Reviewed-by: Oleksandr Suvorov 
---
 drivers/i2c/stm32f7_i2c.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
index 3a727e68ac..14827e5cec 100644
--- a/drivers/i2c/stm32f7_i2c.c
+++ b/drivers/i2c/stm32f7_i2c.c
@@ -485,9 +485,11 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv 
*i2c_priv,
}
}
 
-   /* End of transfer, send stop condition */
-   mask = STM32_I2C_CR2_STOP;
-   setbits_le32(>cr2, mask);
+   if (!ret) {
+   /* End of transfer, send stop condition */
+   mask = STM32_I2C_CR2_STOP;
+   setbits_le32(>cr2, mask);
+   }
 
return stm32_i2c_check_end_of_message(i2c_priv);
 }
-- 
2.34.1



[PATCHv2 1/2] i2c: stm32f7: fix clearing the control register

2022-08-15 Thread Jorge Ramirez-Ortiz
Bits should be set to 0, not 1.

Signed-off-by: Jorge Ramirez-Ortiz 
---
 drivers/i2c/stm32f7_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
index bf2a6c9b4b..3a727e68ac 100644
--- a/drivers/i2c/stm32f7_i2c.c
+++ b/drivers/i2c/stm32f7_i2c.c
@@ -413,7 +413,7 @@ static int stm32_i2c_check_end_of_message(struct 
stm32_i2c_priv *i2c_priv)
setbits_le32(>icr, STM32_I2C_ICR_STOPCF);
 
/* Clear control register 2 */
-   setbits_le32(>cr2, STM32_I2C_CR2_RESET_MASK);
+   clrbits_le32(>cr2, STM32_I2C_CR2_RESET_MASK);
}
 
return ret;
-- 
2.34.1



Re: [PATCH v2 10/10] binman: Add zstd bintool

2022-08-15 Thread Stefan Herbrechtsmeier

Hi Simon,

Am 13.08.2022 um 16:59 schrieb Simon Glass:

Hi Stefan,

On Mon, 8 Aug 2022 at 04:52, Stefan Herbrechtsmeier
 wrote:


From: Stefan Herbrechtsmeier 

Add zstd bintool to binman to support on-the-fly compression.

Signed-off-by: Stefan Herbrechtsmeier 

---

Changes in v2:
- Added

  tools/binman/btool/zstd.py | 30 ++
  tools/binman/comp_util.py  | 18 +-
  tools/binman/etype/blob_dtb.py |  4 
  tools/binman/ftest.py  |  3 ++-
  4 files changed, 45 insertions(+), 10 deletions(-)
  create mode 100644 tools/binman/btool/zstd.py


Reviewed-by: Simon Glass 

But as you noted this does cause CI issues:

ERROR: binman.ftest.TestFunctional.testCompressions (subunit.RemotedTestCase)
binman.ftest.TestFunctional.testCompressions
--
testtools.testresult.real._StringException: Traceback (most recent call last):
TypeError: a bytes-like object is required, not 'NoneType'
==
FAIL: binman.ftest.TestFunctional.testVersions (subunit.RemotedTestCase)
binman.ftest.TestFunctional.testVersions
--
testtools.testresult.real._StringException: stdout: {{{
bzip2 - 1.0.8
gzip - 1.10
lz4 - v1.9.2
lzma - 9.22 beta
lzo - v1.04
xz - 5.2.4
zstd - unknown
}}}
Traceback (most recent call last):
AssertionError: Regex didn't match: '^v?[0-9]+[0-9.]*' not found in 'unknown'

One option is to check if zstd is available, using is_present() in
your testCompressions() function.


I have add a new function to check the present of the tools in each test.

Regards
  Stefan


Re: [PATCH v2 05/10] binman: Add BintoolPacker class to bintool

2022-08-15 Thread Stefan Herbrechtsmeier

Hi Simon,

Am 13.08.2022 um 16:59 schrieb Simon Glass:

Hi Stefan,

On Mon, 8 Aug 2022 at 04:51, Stefan Herbrechtsmeier
 wrote:


From: Stefan Herbrechtsmeier 

Add a bintools base class for packers which compression / decompression
entry contents.

Signed-off-by: Stefan Herbrechtsmeier 

---

Changes in v2:
- Added

  tools/binman/bintool.py | 94 +
  1 file changed, 94 insertions(+)



Reviewed-by: Simon Glass 

The one strange thing about this is that we don't support these
compression tools being missing, as we do with other tools. If 'lz4'
is needed and not present, binman just fails. This predates your
change, but I just noticed it.

I think this is OK though, at least for now. We could handle a missing
algo by returning empty data and marking the entry as 'missing', but I
don't see a great need for it at present. The compression tools are
widely available and easy to install.


I have add a commit to the series for it.


Re: [PATCH v2 2/2] patman: Add documentation to doc/

2022-08-15 Thread Ralph Siemsen
Hello Heinrich,

FYI, I had some trouble trying to apply your changes (patch seems to
be mangled?).

Only one small question for you, see below.

On Wed, Aug 10, 2022 at 2:42 AM Heinrich Schuchardt  wrote:
>
> diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
> index 52151f6f16..f2e6d7636f 100644
> --- a/tools/patman/patman.rst
> +++ b/tools/patman/patman.rst
> @@ -1,19 +1,31 @@
>   .. SPDX-License-Identifier: GPL-2.0+
>   .. Copyright (c) 2011 The Chromium OS Authors
> +.. Simon Glass 
> +.. v1, v2, 19-Oct-11
> +.. revised v3 24-Nov-11
> +.. revised v4 04-Jul-2020, with Patchwork integration
>
>   Patman patch manager
>   
>
>   This tool is a Python script which:
> +
>   - Creates patch directly from your branch
> +
>   - Cleans them up by removing unwanted tags
> +
>   - Inserts a cover letter with change lists
> +
>   - Runs the patches through checkpatch.pl and its own checks
> +
>   - Optionally emails them out to selected people
>
>   It also has some Patchwork features:
> +
>   - shows review tags from Patchwork so you can update your local patches
> +
>   - pulls these down into a new branch on request
> +
>   - lists comments received on a series
>
>   It is intended to automate patch creation and make it a less
> @@ -41,12 +53,15 @@ This tool requires a certain way of working:
>
>   - Maintain a number of branches, one for each patch series you are
> working on
> +
>   - Add tags into the commits within each branch to indicate where the
> series should be sent, cover letter, version, etc. Most of these are
> normally in the top commit so it is easy to change them with 'git
> commit --amend'
> +
>   - Each branch tracks the upstream branch, so that this script can
> automatically determine the number of commits in it (optional)
> +
>   - Check out a branch, and run this script to create and send out your
> patches. Weeks later, change the patches and repeat, knowing that you
> will get a consistent result each time.

Is it necessary to have a blank line between each of the "bullet"
points? It seems to work fine as long as there is one blank line
separating the paragraph from the first bullet item.

Otherwise, it looks good to me.

Ralph


Re: [PATCH v3 2/8] cyclic: Add basic support for cyclic function execution infrastruture

2022-08-15 Thread Stefan Roese

Hi Simon,

On 05.08.22 18:48, Simon Glass wrote:

Hi Stefan,

On Fri, 5 Aug 2022 at 08:26, Stefan Roese  wrote:


Add the basic infrastructure to periodically execute code, e.g. all
100ms. Examples for such functions might be LED blinking etc. The
functions that are hooked into this cyclic list should be small timewise
as otherwise the execution of the other code that relies on a high
frequent polling (e.g. UART rx char ready check) might be delayed too
much. This patch also adds the Kconfig option
CONFIG_CYCLIC_MAX_CPU_TIME_US, which configures the max allowed time
for such a cyclic function. If it's execution time exceeds this time,
this cyclic function will get removed from the cyclic list.

How is this cyclic functionality executed?
The following patch integrates the main function responsible for
calling all registered cyclic functions cyclic_run() into the
common WATCHDOG_RESET macro. This guarantees that cyclic_run() is
executed very often, which is necessary for the cyclic functions to
get scheduled and executed at their configured periods.

This cyclic infrastructure will be used by a board specific function on
the NIC23 MIPS Octeon board, which needs to check periodically, if a
PCIe FLR has occurred.

Signed-off-by: Stefan Roese 
---
v3:
- No change
v2:
- Also add cyclic_register() and cyclic_unregister() as empty functions
   when CONFIG_CYCLIC is not defined
- Misc minor changes

  MAINTAINERS  |   6 +++
  common/Kconfig   |  20 +
  common/Makefile  |   1 +
  common/cyclic.c  | 112 +++
  include/cyclic.h |  97 
  5 files changed, 236 insertions(+)
  create mode 100644 common/cyclic.c
  create mode 100644 include/cyclic.h


Reviewed-by: Simon Glass 

Can you document cyclic_struct - and perhaps 'struct cyclic_info' for
'struct cyclic' would be better so we don't have 'struct' in a struct?


Agreed. My word choice was definitely not optimal. Will change in the
next version.

Thanks,
Stefan


Re: [PATCH 2/2] i2c: stm32f7: do not set the STOP condition on error

2022-08-15 Thread Oleksandr Suvorov
Jorge,

On Mon, Aug 15, 2022 at 2:19 PM Jorge Ramirez-Ortiz  wrote:
>
> Sending the stop condition without waiting for TC has been
> found to lock the bus.
>
> Tested accessing the the NXP SE05X I2C device.

"the the" seems like a typo.

With this,
Reviewed-by: Oleksandr Suvorov 

> Signed-off-by: Jorge Ramirez-Ortiz 
> ---
>  drivers/i2c/stm32f7_i2c.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
> index 3a727e68ac..14827e5cec 100644
> --- a/drivers/i2c/stm32f7_i2c.c
> +++ b/drivers/i2c/stm32f7_i2c.c
> @@ -485,9 +485,11 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv 
> *i2c_priv,
> }
> }
>
> -   /* End of transfer, send stop condition */
> -   mask = STM32_I2C_CR2_STOP;
> -   setbits_le32(>cr2, mask);
> +   if (!ret) {
> +   /* End of transfer, send stop condition */
> +   mask = STM32_I2C_CR2_STOP;
> +   setbits_le32(>cr2, mask);
> +   }
>
> return stm32_i2c_check_end_of_message(i2c_priv);
>  }
> --
> 2.34.1
>


-- 
Best regards
Oleksandr

Oleksandr Suvorov
cryo...@gmail.com


[PATCH v3 8/8] fs: erofs: add unaligned read range handling

2022-08-15 Thread Qu Wenruo
I'm not an expert on erofs, but my quick glance didn't expose any
special handling on unaligned range, thus I think the U-boot erofs
driver doesn't really support unaligned read range.

This patch will add erofs_get_blocksize() so erofs can benefit from the
generic unaligned read support.

Cc: Huang Jianan 
Cc: linux-er...@lists.ozlabs.org
Signed-off-by: Qu Wenruo 
---
 fs/erofs/internal.h | 1 +
 fs/erofs/super.c| 6 ++
 fs/fs.c | 2 +-
 include/erofs.h | 1 +
 4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 4af7c91560cc..d368a6481bf1 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -83,6 +83,7 @@ struct erofs_sb_info {
u16 available_compr_algs;
u16 lz4_max_distance;
u32 checksum;
+   u32 blocksize;
u16 extra_devices;
union {
u16 devt_slotoff;   /* used for mkfs */
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 8277d9b53fb3..82625da59001 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -99,7 +99,13 @@ int erofs_read_superblock(void)
 
sbi.build_time = le64_to_cpu(dsb->build_time);
sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
+   sbi.blocksize = 1 << blkszbits;
 
memcpy(, dsb->uuid, sizeof(dsb->uuid));
return erofs_init_devices(, dsb);
 }
+
+int erofs_get_blocksize(const char *filename)
+{
+   return sbi.blocksize;
+}
diff --git a/fs/fs.c b/fs/fs.c
index 43c7128bcfc5..2ac43c05fcd8 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -376,7 +376,7 @@ static struct fstype_info fstypes[] = {
.readdir = erofs_readdir,
.ls = fs_ls_generic,
.read = erofs_read,
-   .get_blocksize = fs_get_blocksize_unsupported,
+   .get_blocksize = erofs_get_blocksize,
.size = erofs_size,
.close = erofs_close,
.closedir = erofs_closedir,
diff --git a/include/erofs.h b/include/erofs.h
index 1fbe82bf72cb..18bd6807c538 100644
--- a/include/erofs.h
+++ b/include/erofs.h
@@ -10,6 +10,7 @@ int erofs_probe(struct blk_desc *fs_dev_desc,
struct disk_partition *fs_partition);
 int erofs_read(const char *filename, void *buf, loff_t offset,
   loff_t len, loff_t *actread);
+int erofs_get_blocksize(const char *filename);
 int erofs_size(const char *filename, loff_t *size);
 int erofs_exists(const char *filename);
 void erofs_close(void);
-- 
2.37.1



[PATCH v3 7/8] fs: ubifs: rely on higher layer to do unaligned read

2022-08-15 Thread Qu Wenruo
Currently ubifs doesn't support unaligned read offset, thanks to the
recent _fs_read() work to handle unaligned read, we only need to
implement ubifs_get_blocksize() to take advantage of it.

Now ubifs can do unaligned read without any problem.

Signed-off-by: Qu Wenruo 
---
Unfortunately I can not test ubifs, as enabling UBI would cause compile
failure due to missing of  header.
---
 fs/fs.c   |  2 +-
 fs/ubifs/ubifs.c  | 13 -
 include/ubifs_uboot.h |  1 +
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index ea4325cd0b00..43c7128bcfc5 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -312,7 +312,7 @@ static struct fstype_info fstypes[] = {
.exists = ubifs_exists,
.size = ubifs_size,
.read = ubifs_read,
-   .get_blocksize = fs_get_blocksize_unsupported,
+   .get_blocksize = ubifs_get_blocksize,
.write = fs_write_unsupported,
.uuid = fs_uuid_unsupported,
.opendir = fs_opendir_unsupported,
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
index d3026e310168..a8ab556dd376 100644
--- a/fs/ubifs/ubifs.c
+++ b/fs/ubifs/ubifs.c
@@ -846,11 +846,9 @@ int ubifs_read(const char *filename, void *buf, loff_t 
offset,
 
*actread = 0;
 
-   if (offset & (PAGE_SIZE - 1)) {
-   printf("ubifs: Error offset must be a multiple of %d\n",
-  PAGE_SIZE);
-   return -1;
-   }
+   /* Higher layer should ensure it always pass page aligned range. */
+   assert(IS_ALIGNED(offset, PAGE_SIZE));
+   assert(IS_ALIGNED(size, PAGE_SIZE));
 
c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, UBI_READONLY);
/* ubifs_findfile will resolve symlinks, so we know that we get
@@ -920,6 +918,11 @@ out:
return err;
 }
 
+int ubifs_get_blocksize(const char *filename)
+{
+   return PAGE_SIZE;
+}
+
 void ubifs_close(void)
 {
 }
diff --git a/include/ubifs_uboot.h b/include/ubifs_uboot.h
index b025779d59ff..bcd21715314a 100644
--- a/include/ubifs_uboot.h
+++ b/include/ubifs_uboot.h
@@ -29,6 +29,7 @@ int ubifs_exists(const char *filename);
 int ubifs_size(const char *filename, loff_t *size);
 int ubifs_read(const char *filename, void *buf, loff_t offset,
   loff_t size, loff_t *actread);
+int ubifs_get_blocksize(const char *filename);
 void ubifs_close(void);
 
 #endif /* __UBIFS_UBOOT_H__ */
-- 
2.37.1



[PATCH v3 6/8] fs: fat: rely on higher layer to get block aligned read range

2022-08-15 Thread Qu Wenruo
Just implement fat_get_blocksize() for fat, so that fat_read_file()
always get a block aligned read range.

Unfortunately I'm not experienced enough to cleanup the fat code, thus
further cleanup is appreciated.

Cc: Tom Rini 
Signed-off-by: Qu Wenruo 
---
 fs/fat/fat.c  | 13 +
 fs/fs.c   |  2 +-
 include/fat.h |  1 +
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index dcceccbcee0a..e13035e8e6d1 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -1299,6 +1299,19 @@ int fat_read_file(const char *filename, void *buf, 
loff_t offset, loff_t len,
return ret;
 }
 
+int fat_get_blocksize(const char *filename)
+{
+   fsdata fsdata = {0};
+   int ret;
+
+   ret = get_fs_info();
+   if (ret)
+   return ret;
+
+   free(fsdata.fatbuf);
+   return fsdata.sect_size;
+}
+
 typedef struct {
struct fs_dir_stream parent;
struct fs_dirent dirent;
diff --git a/fs/fs.c b/fs/fs.c
index b26cc8e2e840..ea4325cd0b00 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -208,7 +208,7 @@ static struct fstype_info fstypes[] = {
.exists = fat_exists,
.size = fat_size,
.read = fat_read_file,
-   .get_blocksize = fs_get_blocksize_unsupported,
+   .get_blocksize = fat_get_blocksize,
 #if CONFIG_IS_ENABLED(FAT_WRITE)
.write = file_fat_write,
.unlink = fat_unlink,
diff --git a/include/fat.h b/include/fat.h
index a9756fb4cd1b..c03a2bebecef 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -201,6 +201,7 @@ int file_fat_detectfs(void);
 int fat_exists(const char *filename);
 int fat_size(const char *filename, loff_t *size);
 int file_fat_read(const char *filename, void *buffer, int maxsize);
+int fat_get_blocksize(const char *filename);
 int fat_set_blk_dev(struct blk_desc *rbdd, struct disk_partition *info);
 int fat_register_device(struct blk_desc *dev_desc, int part_no);
 
-- 
2.37.1



[PATCH v3 4/8] fs: btrfs: move the unaligned read code to _fs_read() for btrfs

2022-08-15 Thread Qu Wenruo
Unlike FUSE or kernel, U-boot filesystem code makes the underly fs code
to handle the unaligned read (aka, read range is not aligned to fs block
size).

This makes underlying fs code harder to implement, as  they have to handle
unaligned read all by themselves.

This patch will change the behavior, starting from btrfs, by moving the
unaligned read code into _fs_read().

The idea is pretty simple, if we have an unaligned read request, we
handle it in the following steps:

1. Grab the blocksize of the fs

2. Read the leading unaligned range
   We will read the block that @offset is in, and copy the
   requested part into buf.

   The the block we read covers the whole range, we just call it a day.

3. Read the remaining part
   The tailing part may be unaligned, but all fses handles the tailing
   part much easier than the leading unaligned part.

   As they just need to do a min(extent_size, start + len - cur) to
   calculate the real read size.

   In fact, for most file reading, the file size is not aligned and we
   need to handle the tailing part anyway.

There is a btrfs specific cleanup involved:

- In btrfs_file_read(), merge the tailing unaligned read into the main
  loop.
  Just reuse the existing read length calculation is enough.

- Remove read_and_truncate_page() call
  Since there is no explicit leading/tailing unaligned read anymore.

This has been tested with a proper randomly populated btrfs file, then
tried in sandbox mode with different aligned and unaligned range and
compare the output with md5sum.

Cc: Marek Behun 
Cc: linux-bt...@vger.kernel.org
Signed-off-by: Qu Wenruo 
---
 fs/btrfs/btrfs.c |  10 
 fs/btrfs/inode.c |  89 +++-
 fs/fs.c  | 130 ---
 include/btrfs.h  |   1 +
 4 files changed, 141 insertions(+), 89 deletions(-)

diff --git a/fs/btrfs/btrfs.c b/fs/btrfs/btrfs.c
index 74a992fa012d..ac0e972d0249 100644
--- a/fs/btrfs/btrfs.c
+++ b/fs/btrfs/btrfs.c
@@ -234,6 +234,10 @@ int btrfs_read(const char *file, void *buf, loff_t offset, 
loff_t len,
int ret;
 
ASSERT(fs_info);
+
+   /* Higher layer has ensures it never pass unaligned offset in. */
+   ASSERT(IS_ALIGNED(offset, fs_info->sectorsize));
+
ret = btrfs_lookup_path(fs_info->fs_root, BTRFS_FIRST_FREE_OBJECTID,
file, , , , 40);
if (ret < 0) {
@@ -275,6 +279,12 @@ int btrfs_read(const char *file, void *buf, loff_t offset, 
loff_t len,
return 0;
 }
 
+int btrfs_get_blocksize(const char *filename)
+{
+   ASSERT(current_fs_info);
+   return current_fs_info->sectorsize;
+}
+
 void btrfs_close(void)
 {
if (current_fs_info) {
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 40025662f250..f12be46f6262 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -617,44 +617,6 @@ check_next:
return 1;
 }
 
-static int read_and_truncate_page(struct btrfs_path *path,
- struct btrfs_file_extent_item *fi,
- int start, int len, char *dest)
-{
-   struct extent_buffer *leaf = path->nodes[0];
-   struct btrfs_fs_info *fs_info = leaf->fs_info;
-   u64 aligned_start = round_down(start, fs_info->sectorsize);
-   u8 extent_type;
-   char *buf;
-   int page_off = start - aligned_start;
-   int page_len = fs_info->sectorsize - page_off;
-   int ret;
-
-   ASSERT(start + len <= aligned_start + fs_info->sectorsize);
-   buf = malloc_cache_aligned(fs_info->sectorsize);
-   if (!buf)
-   return -ENOMEM;
-
-   extent_type = btrfs_file_extent_type(leaf, fi);
-   if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
-   ret = btrfs_read_extent_inline(path, fi, buf);
-   memcpy(dest, buf + page_off, min(page_len, ret));
-   free(buf);
-   return len;
-   }
-
-   ret = btrfs_read_extent_reg(path, fi,
-   round_down(start, fs_info->sectorsize),
-   fs_info->sectorsize, buf);
-   if (ret < 0) {
-   free(buf);
-   return ret;
-   }
-   memcpy(dest, buf + page_off, page_len);
-   free(buf);
-   return len;
-}
-
 int btrfs_file_read(struct btrfs_root *root, u64 ino, u64 file_offset, u64 len,
char *dest)
 {
@@ -663,7 +625,6 @@ int btrfs_file_read(struct btrfs_root *root, u64 ino, u64 
file_offset, u64 len,
struct btrfs_path path;
struct btrfs_key key;
u64 aligned_start = round_down(file_offset, fs_info->sectorsize);
-   u64 aligned_end = round_down(file_offset + len, fs_info->sectorsize);
u64 next_offset;
u64 cur = aligned_start;
int ret = 0;
@@ -673,34 +634,14 @@ int btrfs_file_read(struct btrfs_root *root, u64 ino, u64 
file_offset, u64 len,
/* Set the whole dest all zero, so we won't need to bother holes */
memset(dest, 0, len);
 
-  

[PATCH v3 5/8] fs: ext4: rely on _fs_read() to handle leading unaligned block read

2022-08-15 Thread Qu Wenruo
Just add ext4_get_blocksize() and a new assert() in ext4fs_read_file().

Signed-off-by: Qu Wenruo 
---
Several cleanup candicate:

1. ext_fs->blksz is never populated, thus it's always 0
   We can not easily grab blocksize just like btrfs in this case.

   Thus we have to go the same calculation in ext4fs_read_file().

2. can not easily cleanup @skipfirst in ext4fs_read_file()
   It seems to screw up with soft link read.
---
 fs/ext4/ext4fs.c | 22 ++
 fs/fs.c  |  2 +-
 include/ext4fs.h |  1 +
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c
index 4c89152ce4ad..b0568e77a895 100644
--- a/fs/ext4/ext4fs.c
+++ b/fs/ext4/ext4fs.c
@@ -69,6 +69,8 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos,
short status;
struct ext_block_cache cache;
 
+   assert(IS_ALIGNED(pos, blocksize));
+
ext_cache_init();
 
/* Adjust len so it we can't read past the end of the file. */
@@ -259,6 +261,26 @@ int ext4_read_file(const char *filename, void *buf, loff_t 
offset, loff_t len,
return ext4fs_read(buf, offset, len, len_read);
 }
 
+int ext4_get_blocksize(const char *filename)
+{
+   struct ext_filesystem *fs;
+   int log2blksz;
+   int log2_fs_blocksize;
+   loff_t file_len;
+   int ret;
+
+   ret = ext4fs_open(filename, _len);
+   if (ret < 0) {
+   printf("** File not found %s **\n", filename);
+   return -1;
+   }
+   fs = get_fs();
+   log2blksz = fs->dev_desc->log2blksz;
+   log2_fs_blocksize = LOG2_BLOCK_SIZE(ext4fs_file->data) - log2blksz;
+
+   return (1 << (log2_fs_blocksize + log2blksz));
+}
+
 int ext4fs_uuid(char *uuid_str)
 {
if (ext4fs_root == NULL)
diff --git a/fs/fs.c b/fs/fs.c
index 6d43d616de4b..b26cc8e2e840 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -237,7 +237,7 @@ static struct fstype_info fstypes[] = {
.exists = ext4fs_exists,
.size = ext4fs_size,
.read = ext4_read_file,
-   .get_blocksize = fs_get_blocksize_unsupported,
+   .get_blocksize = ext4_get_blocksize,
 #ifdef CONFIG_CMD_EXT4_WRITE
.write = ext4_write_file,
.ln = ext4fs_create_link,
diff --git a/include/ext4fs.h b/include/ext4fs.h
index cb5d9cc0a5c0..0f4cf32dcc2a 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -161,6 +161,7 @@ int ext4fs_probe(struct blk_desc *fs_dev_desc,
 struct disk_partition *fs_partition);
 int ext4_read_file(const char *filename, void *buf, loff_t offset, loff_t len,
   loff_t *actread);
+int ext4_get_blocksize(const char *filename);
 int ext4_read_superblock(char *buffer);
 int ext4fs_uuid(char *uuid_str);
 void ext_cache_init(struct ext_block_cache *cache);
-- 
2.37.1



[PATCH v3 3/8] fs: btrfs: fix a crash if specified range is beyond file size

2022-08-15 Thread Qu Wenruo
[BUG]
When try to read a range beyond file size, btrfs driver will cause
crash/segfault:

 => load host 0 $kernel_addr_r 5k_file 0 0x2000
 SEGFAULT

[CAUSE]
In btrfs_read(), if @len is 0, we will truncated it to file end, but if
file end is beyond our file size, this truncation will underflow @len,
making it -3K in this case.

And later that @len is used to memzero the output buffer, resulting
above crash.

[FIX]
Just error out if @offset is already beyond our file size.

Now it will fail properly with correct error message:

 => load host 0 $kernel_addr_r 5m_origin 0 0x2000
 BTRFS: Read range beyond file size, offset 8192 file size 5120

 Failed to load '5m_origin'

Signed-off-by: Qu Wenruo 
---
 fs/btrfs/btrfs.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/fs/btrfs/btrfs.c b/fs/btrfs/btrfs.c
index 309cd595d37d..74a992fa012d 100644
--- a/fs/btrfs/btrfs.c
+++ b/fs/btrfs/btrfs.c
@@ -252,6 +252,12 @@ int btrfs_read(const char *file, void *buf, loff_t offset, 
loff_t len,
return ret;
}
 
+   if (offset >= real_size) {
+   error("Read range beyond file size, offset %llu file size %llu",
+   offset, real_size);
+   return -EINVAL;
+   }
+
/*
 * If the length is 0 (meaning read the whole file) or the range is
 * beyond file size, truncate it to the end of the file.
-- 
2.37.1



[PATCH v3 2/8] fs: btrfs: fix a bug which no data get read if the length is not 0

2022-08-15 Thread Qu Wenruo
[BUG]
When testing with unaligned read, if a specific length is passed in,
btrfs driver will read out nothing:

 => load host 0 $kernel_addr_r 5k_file 0x1000 0
 0 bytes read in 0 ms

But if no length is passed in, it works fine, even if we pass a non-zero
length:

 => load host 0 $kernel_addr_r 5k_file 0 0x1000
 1024 bytes read in 0 ms

[CAUSE]
In btrfs_read() if we have a larger size than our file, we will try to
truncate it using the file size.

However the real file size is not initialized if @len is not zero, thus
we always truncate our length to 0, and cause the problem.

[FIX]
Fix it by just always do the file size check.

In fact btrfs_size() always follow soft link, thus it will return the
real file size correctly.

Signed-off-by: Qu Wenruo 
---
 fs/btrfs/btrfs.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/btrfs.c b/fs/btrfs/btrfs.c
index 4cdbbbe3d066..309cd595d37d 100644
--- a/fs/btrfs/btrfs.c
+++ b/fs/btrfs/btrfs.c
@@ -246,16 +246,17 @@ int btrfs_read(const char *file, void *buf, loff_t 
offset, loff_t len,
return -EINVAL;
}
 
-   if (!len) {
-   ret = btrfs_size(file, _size);
-   if (ret < 0) {
-   error("Failed to get inode size: %s", file);
-   return ret;
-   }
-   len = real_size;
+   ret = btrfs_size(file, _size);
+   if (ret < 0) {
+   error("Failed to get inode size: %s", file);
+   return ret;
}
 
-   if (len > real_size - offset)
+   /*
+* If the length is 0 (meaning read the whole file) or the range is
+* beyond file size, truncate it to the end of the file.
+*/
+   if (!len || len > real_size - offset)
len = real_size - offset;
 
ret = btrfs_file_read(root, ino, offset, len, buf);
-- 
2.37.1



[PATCH v3 1/8] fs: fat: unexport file_fat_read_at()

2022-08-15 Thread Qu Wenruo
That function is only utilized inside fat driver, unexport it.

Signed-off-by: Qu Wenruo 
---
 fs/fat/fat.c  | 4 ++--
 include/fat.h | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index df9ea2c028fc..dcceccbcee0a 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -1243,8 +1243,8 @@ out_free_itr:
return ret;
 }
 
-int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
-loff_t maxsize, loff_t *actread)
+static int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
+   loff_t maxsize, loff_t *actread)
 {
fsdata fsdata;
fat_itr *itr;
diff --git a/include/fat.h b/include/fat.h
index bd8e450b33a3..a9756fb4cd1b 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -200,8 +200,6 @@ static inline u32 sect_to_clust(fsdata *fsdata, int sect)
 int file_fat_detectfs(void);
 int fat_exists(const char *filename);
 int fat_size(const char *filename, loff_t *size);
-int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
-loff_t maxsize, loff_t *actread);
 int file_fat_read(const char *filename, void *buffer, int maxsize);
 int fat_set_blk_dev(struct blk_desc *rbdd, struct disk_partition *info);
 int fat_register_device(struct blk_desc *dev_desc, int part_no);
-- 
2.37.1



[PATCH v3 0/8] U-boot: fs: add generic unaligned read offset handling

2022-08-15 Thread Qu Wenruo
[CHANGELOG]
v3:
- Fix an error that we always return 0 actread bytes for unsupported fses
  For unsupported fses, we should also populate @total_read.
  Or we will just read the data but still return 0 for actually bytes.

  Now it pass all test_fs* cases.

v2
- Fix a linkage error where (U64 % U32) is called without proper helper
  Fix it with U64 & (U32 - 1), as the U32 value (@blocksize) should
  always be power of 2, thus (@blocksize - 1) is the mask we want to
  calculate the offset inside the block.

  Above change only affects the 4th patch, everything else is not
  touched.

RFC->v1:
- More (manual) testing
  Unfortunately, in the latest master (75967970850a), the fs-tests.sh
  always seems to hang at preparing the fs image.

  Thus still has to do manual testing, tested btrfs, ext4 and fat, with
  aligned and unaligned read, also added soft link read, all looks fine here.

  Extra testing is still appreciated.

- Two more btrfs specific bug fixes
  All exposed during manual tests

- Remove the tailing unaligned block handling
  In fact, all fses can easily handle such case, just a min() call is
  enough.

- Remove the support for sandboxfs
  Since it's using read() calls, really no need to do block alignment
  check.

- Enhanced blocksize check
  Ensure the returned blocksize is not only non-error, but also
  non-zero.


Qu Wenruo (8):
  fs: fat: unexport file_fat_read_at()
  fs: btrfs: fix a bug which no data get read if the length is not 0
  fs: btrfs: fix a crash if specified range is beyond file size
  fs: btrfs: move the unaligned read code to _fs_read() for btrfs
  fs: ext4: rely on _fs_read() to handle leading unaligned block read
  fs: fat: rely on higher layer to get block aligned read range
  fs: ubifs: rely on higher layer to do unaligned read
  fs: erofs: add unaligned read range handling

 fs/btrfs/btrfs.c  |  33 ---
 fs/btrfs/inode.c  |  89 +++--
 fs/erofs/internal.h   |   1 +
 fs/erofs/super.c  |   6 ++
 fs/ext4/ext4fs.c  |  22 +++
 fs/fat/fat.c  |  17 +-
 fs/fs.c   | 130 +++---
 fs/ubifs/ubifs.c  |  13 +++--
 include/btrfs.h   |   1 +
 include/erofs.h   |   1 +
 include/ext4fs.h  |   1 +
 include/fat.h |   3 +-
 include/ubifs_uboot.h |   1 +
 13 files changed, 212 insertions(+), 106 deletions(-)

-- 
2.37.1



Re: [PATCH 0/7] arm: dts: Create common imx8mn-u-boot

2022-08-15 Thread Adam Ford
On Sun, Aug 14, 2022 at 5:57 PM Fabio Estevam  wrote:
>
> Hi Adam,
>
> On Sun, Jul 31, 2022 at 8:46 PM Adam Ford  wrote:
> >
> > Every imx8mn board has a bunch of similar entries on their
> > respective board-u-boot.dtsi file to make the board bootable.
> > Instead of maintaining multiple files with duplicate code,
> > have them all point to a new, common file.  This file includes
> > the necessary nodes that were common to nearly all boards
> > and added spba1 to help faciliate SPL_DM_SERIAL.  This also
> > adds support for CONFIG_FSPI_CONF_HEADER which can be used
> > to generate flash.bin files for booting from FlexSPI.
> >
> > Adam Ford (7):
> >   arm: dts: imx8mn-u-boot: Create common imx8mn-u-boot.dtsi
> >   arm: dts: imx8mn-beacon-kit: Consolidate with imx8mn-u-boot
> >   arm: dts: imx8mn-bsh-smm-s2: Consolidate with imx8mn-u-boot
> >   arm: dts: imx8mn-ddr4-evk: Consolidate with imx8mn-u-boot
> >   arm: dts: imx8mn-evk: Consolidate with imx8mn-u-boot
> >   arm: dts: imx8mn-var-som-symphony: Consolidate with imx8mn-u-boot
> >   arm: dts: imx8mn-venice: Consolidate with imx8mn-u-boot
>
> For the series:
>
> Reviewed-by: Fabio Estevam 

Thanks!

Stefano,

If you want me to squash the 1 dependent patch, I can squash it into
this series and submit a V2, but getting this series applied should
fix a bunch of boards which don't boot properly.

adam


[PATCH 2/2] i2c: stm32f7: do not set the STOP condition on error

2022-08-15 Thread Jorge Ramirez-Ortiz
Sending the stop condition without waiting for TC has been
found to lock the bus.

Tested accessing the the NXP SE05X I2C device.

Signed-off-by: Jorge Ramirez-Ortiz 
---
 drivers/i2c/stm32f7_i2c.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
index 3a727e68ac..14827e5cec 100644
--- a/drivers/i2c/stm32f7_i2c.c
+++ b/drivers/i2c/stm32f7_i2c.c
@@ -485,9 +485,11 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv 
*i2c_priv,
}
}
 
-   /* End of transfer, send stop condition */
-   mask = STM32_I2C_CR2_STOP;
-   setbits_le32(>cr2, mask);
+   if (!ret) {
+   /* End of transfer, send stop condition */
+   mask = STM32_I2C_CR2_STOP;
+   setbits_le32(>cr2, mask);
+   }
 
return stm32_i2c_check_end_of_message(i2c_priv);
 }
-- 
2.34.1



[PATCH 1/2] i2c: stm32f7: fix clearing the control register

2022-08-15 Thread Jorge Ramirez-Ortiz
Bits should be set to 0, not 1.

Signed-off-by: Jorge Ramirez-Ortiz 
---
 drivers/i2c/stm32f7_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
index bf2a6c9b4b..3a727e68ac 100644
--- a/drivers/i2c/stm32f7_i2c.c
+++ b/drivers/i2c/stm32f7_i2c.c
@@ -413,7 +413,7 @@ static int stm32_i2c_check_end_of_message(struct 
stm32_i2c_priv *i2c_priv)
setbits_le32(>icr, STM32_I2C_ICR_STOPCF);
 
/* Clear control register 2 */
-   setbits_le32(>cr2, STM32_I2C_CR2_RESET_MASK);
+   clrbits_le32(>cr2, STM32_I2C_CR2_RESET_MASK);
}
 
return ret;
-- 
2.34.1



[PATCH] image-fit: don't set compression if it can't be read

2022-08-15 Thread Daniel Golle
fit_image_get_comp() should not set value -1 in case it can't read
the compression node. Instead, leave the value untouched in that case
as it can be absent and a default value previously defined by the
caller of fit_image_get_comp() should be used.

As a result the warning message
WARNING: 'compression' nodes for ramdisks are deprecated, please fix your .its 
file!
no longer shows if the compression node is actually absent.

Signed-off-by: Daniel Golle 
---
 boot/bootm.c | 6 ++
 boot/image-fit.c | 3 +--
 cmd/ximg.c   | 7 ++-
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 86dbfbcfed..b659825305 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -1024,10 +1024,8 @@ static int bootm_host_load_image(const void *fit, int 
req_image_type,
return -EINVAL;
}
 
-   if (fit_image_get_comp(fit, noffset, _comp)) {
-   puts("Can't get image compression!\n");
-   return -EINVAL;
-   }
+   if (fit_image_get_comp(fit, noffset, _comp))
+   image_comp = IH_COMP_NONE;
 
/* Allow the image to expand by a factor of 4, should be safe */
buf_size = (1 << 20) + len * 4;
diff --git a/boot/image-fit.c b/boot/image-fit.c
index df3e5df883..21dbd05118 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -477,7 +477,7 @@ void fit_print_contents(const void *fit)
 void fit_image_print(const void *fit, int image_noffset, const char *p)
 {
char *desc;
-   uint8_t type, arch, os, comp;
+   uint8_t type, arch, os, comp = IH_COMP_NONE;
size_t size;
ulong load, entry;
const void *data;
@@ -794,7 +794,6 @@ int fit_image_get_comp(const void *fit, int noffset, 
uint8_t *comp)
data = fdt_getprop(fit, noffset, FIT_COMP_PROP, );
if (data == NULL) {
fit_get_debug(fit, noffset, FIT_COMP_PROP, len);
-   *comp = -1;
return -1;
}
 
diff --git a/cmd/ximg.c b/cmd/ximg.c
index 65ba41320a..f84141ff45 100644
--- a/cmd/ximg.c
+++ b/cmd/ximg.c
@@ -171,11 +171,8 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, 
char *const argv[])
return 1;
}
 
-   if (fit_image_get_comp(fit_hdr, noffset, )) {
-   puts("Could not find script subimage "
-   "compression type\n");
-   return 1;
-   }
+   if (fit_image_get_comp(fit_hdr, noffset, ))
+   comp = IH_COMP_NONE;
 
data = (ulong)fit_data;
len = (ulong)fit_len;
-- 
2.37.1



RE: [RFC PATCH v4 28/28] board: keymile: common: Use environment to store IVM_* variables.

2022-08-15 Thread Holger Brunck
> Le lundi 20 juin 2022, 19:33:24 CEST Tom Rini a écrit :
> > On Mon, Jun 20, 2022 at 04:08:32PM +, Holger Brunck wrote:
> > > > > > On Fri, Jun 17, 2022 at 12:31:58AM +0200, Francis Laniel wrote:
> > > > > > > These boards used set_local_var() to store some variables as
> > > > > > > local shell.
> > > > > > > They then used get_local_var() to retrieve the variables values.
> > > > > > >
> > > > > > > Instead of using local shell variables, they should use
> > > > > > > environment ones (like a majority of board).
> > > > > > > So, this patch converts using local variables to environment ones.
> > > > >
> > > > > why do we need to change that? It is intended that we use this
> > > > > hush variable infrastructure from u-boot (common/hush.c) for our
> > > > > IVM data and not the standard env. We read the IVM at boot time
> > > > > and store these values in RAM. It is not intended to store them
> > > > > permanently in the flash or wherever the environment is saved.
> > > > > Especially in our case we have some boards where the environment
> > > > > is in a i2c EEPROM and we don't want to write down to the EEPROM
> each time when the board is starting.
> > > >
> > > > So, the whole series is about updating hush to bring in a new
> > > > baseline version of the shell, from busybox.
> > >
> > > Ok.
> > >
> > > > > > > Signed-off-by: Francis Laniel
> > > > > > > 
> > > > > > > ---
> > > > > > >
> > > > > > >  board/keymile/common/common.c | 8 
> > > > > > >  board/keymile/common/ivm.c| 9 +
> > > > > > >  2 files changed, 5 insertions(+), 12 deletions(-)
> > > > > > >
> > > > > > > diff --git a/board/keymile/common/common.c
> > > > > > > b/board/keymile/common/common.c index
> 3999f48719..72939af36e
> > > > > > > 100644
> > > > > > > --- a/board/keymile/common/common.c
> > > > > > > +++ b/board/keymile/common/common.c
> > > > > > > @@ -219,7 +219,7 @@ static int do_setboardid(struct cmd_tbl
> > > > > > > *cmdtp, int
> > > > > >
> > > > > > flag, int argc,
> > > > > >
> > > > > > > unsigned char buf[32];
> > > > > > > char *p;
> > > > > > >
> > > > > > > -   p = get_local_var("IVM_BoardId");
> > > > > > > +   p = env_get("IVM_BoardId");
> > > > > > >
> > > > > > > if (!p) {
> > > > > > >
> > > > > > > printf("can't get the IVM_Boardid\n");
> > > > > > > return 1;
> > > > > > >
> > > > > > > @@ -228,7 +228,7 @@ static int do_setboardid(struct cmd_tbl
> > > > > > > *cmdtp, int
> > > > > >
> > > > > > flag, int argc,
> > > > > >
> > > > > > > env_set("boardid", (char *)buf);
> > > > > > > printf("set boardid=%s\n", buf);
> > > > > > >
> > > > > > > -   p = get_local_var("IVM_HWKey");
> > > > > > > +   p = env_get("IVM_HWKey");
> > > > > > >
> > > > > > > if (!p) {
> > > > > > >
> > > > > > > printf("can't get the IVM_HWKey\n");
> > > > > > > return 1;
> > > > > > >
> > > > > > > @@ -272,14 +272,14 @@ static int do_checkboardidhwk(struct
> > > > > > > cmd_tbl
> > > > > >
> > > > > > *cmdtp, int flag, int argc,
> > > > > >
> > > > > > >  * first read out the real inventory values, these values are
> > > > > > >  * already stored in the local hush variables
> > > > > > >  */
> > > > > > >
> > > > > > > -   p = get_local_var("IVM_BoardId");
> > > > > > > +   p = env_get("IVM_BoardId");
> > > > > > >
> > > > > > > if (!p) {
> > > > > > >
> > > > > > > printf("can't get the IVM_Boardid\n");
> > > > > > > return 1;
> > > > > > >
> > > > > > > }
> > > > > > > rc = strict_strtoul(p, 16, );
> > > > > > >
> > > > > > > -   p = get_local_var("IVM_HWKey");
> > > > > > > +   p = env_get("IVM_HWKey");
> > > > > > >
> > > > > > > if (!p) {
> > > > > > >
> > > > > > > printf("can't get the IVM_HWKey\n");
> > > > > > > return 1;
> > > > > > >
> > > > > > > diff --git a/board/keymile/common/ivm.c
> > > > > > > b/board/keymile/common/ivm.c index 67db0c50f4..e266d7ce81
> > > > > > > 100644
> > > > > > > --- a/board/keymile/common/ivm.c
> > > > > > > +++ b/board/keymile/common/ivm.c
> > > > > > > @@ -44,14 +44,7 @@ static int ivm_calc_crc(unsigned char
> > > > > > > *buf, int
> > > > > > > len)
> > > > > > >
> > > > > > >  static int ivm_set_value(char *name, char *value)  {
> > > > > > >
> > > > > > > -   char tempbuf[256];
> > > > > > > -
> > > > > > > -   if (value) {
> > > > > > > -   sprintf(tempbuf, "%s=%s", name, value);
> > > > > > > -   return set_local_var(tempbuf, 0);
> > > > > > > -   }
> > > > > > > -   unset_local_var(name);
> > > > > > > -   return 0;
> > > > > > > +   return env_set(name, value);
> > > > >
> > > > > this means we are now writing always down to the permanent
> > > > > environment or? And this I would really like to avoid in our case.
> > > >
> > > > Note that "env_set" does not force a save of the running environment.
> > >
> > > Ah yes you are right. But still for the first boot of our board we
> > > will call saveenv 

Re: Have fsl_elbc_nand controllers subpage write support?

2022-08-15 Thread Pali Rohár
On Saturday 13 August 2022 12:36:37 Pali Rohár wrote:
> Hello!
> 
> On Sunday 07 August 2022 16:13:00 Pali Rohár wrote:
> > On Sunday 07 August 2022 14:00:27 Pali Rohár wrote:
> > > Hello Scott!
> > > 
> > > In past you disabled nand subpage write support for freescale eLBC and
> > > IFC U-Boot nand drivers in following patch:
> > > https://lore.kernel.org/u-boot/20121102234432.ga18...@buserror.net/
> > > 
> > > But in Linux kernel you disabled it only in freescale IFC nand driver:
> > > https://lore.kernel.org/linux-mtd/20130410223437.ga26...@home.buserror.net/
> > > 
> > > letting kernel's freescale eLBC driver to have enabled subpage write
> > > support.
> > > 
> > > I would like to ask, has freescale eLBC nand controller support for
> > > subpage writes? Or not and it should be disabled also in kernel?
> > > 
> > > Or has somebody else these details?
> > 
> > Now I found following commit http://git.kernel.org/torvalds/c/f034d87def51
> > which seems to be fixing/emulating subpage write support.
> 
> If there is not any objections I would send patch to U-boot eLBC nand
> controller driver which would remove that NAND_NO_SUBPAGE_WRITE too. As
> those subpage write functions are present also in U-Boot driver.
> 
> Any comments?

No objections, so U-Boot patch is here:
https://lore.kernel.org/u-boot/20220815080140.4048-1-p...@kernel.org/T/#u

> > > The main issue is that U-Boot nand and UBI support for boards with eLBC
> > > controllers is compatible with Linux kernel nand and UBI support because
> > > these two drivers calculate UBI geometry differently. UBI header offset
> > > is calculated from nand subpage size and nand subpage size obviously
> > > depends on the fact if subpage write is supported or not.
> > > 
> > > I would like to fix this issue, but I do not know if wrong information
> > > is in U-Boot driver or in Linux kernel driver.


[PATCH] mtd: rawnand: fsl_elbc: Remove NAND_NO_SUBPAGE_WRITE flag

2022-08-15 Thread Pali Rohár
Subpage write support for freescale eLBC NAND controller driver is
implemented in U-Boot and was fixes in the commit d3963721d93f ("nand: Sync
with Linux v4.1").

So remove NAND_NO_SUBPAGE_WRITE flag from the fsl_elbc_nand.c driver. This
partially revert commit cb04c7723429 ("nand/fsl: add NAND_NO_SUBPAGE_WRITE
to eLBC and IFC drivers"), only eLBC driver part.

With this change U-Boot with default settings can read from NAND UBIFS
image created on Linux with Linux default settings. Prior this change
U-Boot was unable to read from NAND UBIFS images created with Linux default
settings due to differnet UBI geometry.

Linux kernel fsl_elbc_nand.c driver also does not set NAND_NO_SUBPAGE_WRITE
flag and has implemented subpage write support.

Fixes: cb04c7723429 ("nand/fsl: add NAND_NO_SUBPAGE_WRITE to eLBC and IFC 
drivers")
Fixes: d3963721d93f ("nand: Sync with Linux v4.1")
Signed-off-by: Pali Rohár 
---
See also email thread:
https://lore.kernel.org/u-boot/20220807120027.2zz43afbqtqljhul@pali/t/#u
---
 drivers/mtd/nand/raw/fsl_elbc_nand.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c 
b/drivers/mtd/nand/raw/fsl_elbc_nand.c
index 48a3687f2728..e28670a4724a 100644
--- a/drivers/mtd/nand/raw/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c
@@ -732,7 +732,6 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr, struct 
udevice *dev)
nand->bbt_md = _mirror_descr;
 
/* set up nand options */
-   nand->options = NAND_NO_SUBPAGE_WRITE;
nand->bbt_options = NAND_BBT_USE_FLASH;
 
nand->controller = _ctrl->controller;
-- 
2.20.1



Re: [RFC PATCH 1/1] spl: introduce SPL_XIP to config

2022-08-15 Thread Nikita Shubin
Hello Sean!

On Sat, 13 Aug 2022 00:35:59 -0400
Sean Anderson  wrote:

> Hi Nikita,
> 
> On 8/11/22 5:37 AM, Nikita Shubin wrote:
> > From: Nikita Shubin 
> > 
> > U-Boot and SPL don't necessary share the same location, so we might
> > end with XIP SPL and U-Boot in "normal" memory.
> > 
> > This adds an option special for SPL to behave it in XIP manner and
> > don't use hart_lottery and available_harts_lock.
> > 
> > Signed-off-by: Nikita Shubin 
> > ---  
> 
> On a stylistic note, typically cover letters are not used for single
> patches (but feel free to use them for larger series).
> 
> >   arch/riscv/cpu/cpu.c | 2 +-
> >   arch/riscv/cpu/start.S   | 4 ++--
> >   arch/riscv/include/asm/global_data.h | 2 +-
> >   arch/riscv/lib/asm-offsets.c | 2 +-
> >   arch/riscv/lib/smp.c | 2 +-
> >   common/spl/Kconfig   | 5 +
> >   include/configs/scr7_vcu118.h| 2 ++
> >   7 files changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> > index 9f5fa0bcb3..89528748d7 100644
> > --- a/arch/riscv/cpu/cpu.c
> > +++ b/arch/riscv/cpu/cpu.c
> > @@ -19,7 +19,7 @@
> >* The variables here must be stored in the data section since
> > they are used
> >* before the bss section is available.
> >*/
> > -#ifndef CONFIG_XIP
> > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP)  
> Shouldn't this be
> 
> #if CONFIG_IS_ENABLED(XIP)
> 
> ?
> 
> ditto for the rest of these

I think you are right, i am a bit confused with CONFIG vs
CONFIG_IS_ENABLED.

> 
> >   u32 hart_lottery __section(".data") = 0;
> >   
> >   /*
> > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> > index 26cb877ed1..d824990778 100644
> > --- a/arch/riscv/cpu/start.S
> > +++ b/arch/riscv/cpu/start.S
> > @@ -122,7 +122,7 @@ call_board_init_f_0:
> >   call_harts_early_init:
> > jal harts_early_init
> >   
> > -#ifndef CONFIG_XIP
> > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP)
> > /*
> >  * Pick hart to initialize global data and run U-Boot.
> > The other harts
> >  * wait for initialization to complete.
> > @@ -155,7 +155,7 @@ call_harts_early_init:
> > /* save the boot hart id to global_data */
> > SREGtp, GD_BOOT_HART(gp)
> >   
> > -#ifndef CONFIG_XIP
> > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP)
> > la  t0, available_harts_lock
> > amoswap.w.rl zero, zero, 0(t0)
> >   
> > diff --git a/arch/riscv/include/asm/global_data.h
> > b/arch/riscv/include/asm/global_data.h index 9a146d1d49..d71e09c5ab
> > 100644 --- a/arch/riscv/include/asm/global_data.h
> > +++ b/arch/riscv/include/asm/global_data.h
> > @@ -30,7 +30,7 @@ int iccm[CONFIG_NR_CPUS];
> >   #if CONFIG_IS_ENABLED(SMP)
> > struct ipi_data ipi[CONFIG_NR_CPUS];
> >   #endif
> > -#ifndef CONFIG_XIP
> > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP)
> > ulong available_harts;
> >   #endif
> >   };
> > diff --git a/arch/riscv/lib/asm-offsets.c
> > b/arch/riscv/lib/asm-offsets.c index f1fe089b3d..bcb3c78654 100644
> > --- a/arch/riscv/lib/asm-offsets.c
> > +++ b/arch/riscv/lib/asm-offsets.c
> > @@ -16,7 +16,7 @@ int main(void)
> >   {
> > DEFINE(GD_BOOT_HART, offsetof(gd_t, arch.boot_hart));
> > DEFINE(GD_FIRMWARE_FDT_ADDR, offsetof(gd_t,
> > arch.firmware_fdt_addr)); -#ifndef CONFIG_XIP
> > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP)
> > DEFINE(GD_AVAILABLE_HARTS, offsetof(gd_t,
> > arch.available_harts)); #endif
> >   
> > diff --git a/arch/riscv/lib/smp.c b/arch/riscv/lib/smp.c
> > index ba992100ad..cef324954c 100644
> > --- a/arch/riscv/lib/smp.c
> > +++ b/arch/riscv/lib/smp.c
> > @@ -45,7 +45,7 @@ static int send_ipi_many(struct ipi_data *ipi,
> > int wait) continue;
> > }
> >   
> > -#ifndef CONFIG_XIP
> > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP)
> > /* skip if hart is not available */
> > if (!(gd->arch.available_harts & (1 << reg)))
> > continue;
> > diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> > index 07c03d611d..f24e423fc0 100644
> > --- a/common/spl/Kconfig
> > +++ b/common/spl/Kconfig
> > @@ -27,6 +27,11 @@ config SPL_FRAMEWORK
> >   supports MMC, NAND and YMODEM and other methods loading
> > of U-Boot and the Linux Kernel.  If unsure, say Y.
> >   
> > +config SPL_XIP
> > +   bool "Support SPL in XIP"
> > +   help
> > + Enable this if SPL is in XIP memory.  
> 
> Can you add another sentence or two? What is "XIP memory"? How does
> it differ from the standard boot process?

In this case i treat "XIP memory" as RO memory, if CONFIG_XIP is enabled
(at least for riscv start.S) we don't use such variables as
hart_lottery or available_harts_lock, both are locks. The problem is
that CONFIG_XIP also propagate to main U-Boot, not only SPL.

For example arch/riscv/cpu/start.S:
```
#ifndef CONFIG_XIP
la  t0, hart_lottery
li  t1, 1

Re: [PATCH v2 02/10] binman: Make compressed data header optional

2022-08-15 Thread Stefan Herbrechtsmeier

Hi Simon,

Am 13.08.2022 um 16:59 schrieb Simon Glass:

Hi Stefan,

On Mon, 8 Aug 2022 at 04:51, Stefan Herbrechtsmeier
 wrote:


From: Stefan Herbrechtsmeier 

Move the compressed data header handling into the dtb blob entry class
and make it optional. The header is uncommon, not supported by U-Boot
and incompatible with external compressed artifacts.

If needed the header could be enabled with the following
attribute beside the compress attribute:
   prepend = "length";

The header was introduced as part of commit eb0f4a4cb402 ("binman:
Support replacing data in a cbfs") to allow device tree entries to be
larger that the compressed contents. Regarding the commit "this is
necessary to cope with a compressed device tree being updated in such a
way that it shrinks after the entry size is already set (an obscure
case)". This case need to be fixed without influence any compressed data
by itself.

Signed-off-by: Stefan Herbrechtsmeier 

---

Changes in v2:
- Reworked to make the feature optional.

  tools/binman/cbfs_util.py |  8 ++---
  tools/binman/comp_util.py | 11 ++
  tools/binman/entries.rst  |  4 +++
  tools/binman/entry.py |  2 +-
  tools/binman/etype/blob_dtb.py| 21 
  tools/binman/ftest.py | 34 ---
  .../test/235_compress_prepend_length_dtb.dts  | 17 ++
  7 files changed, 78 insertions(+), 19 deletions(-)
  create mode 100644 tools/binman/test/235_compress_prepend_length_dtb.dts


I've been through this and I think it is a good change. We can use
your technique (property in the blob_dtb etype) to deal with any
future problems that come up.

But I would like to split this patch into three:

1. Add your blob_dtb property and the test
2. Change all uses of compress()/decompress() to call with add with_header=False
3. Drop the with_header arg from comp_util.py


Okay, I will split the commit.



A few more tweaks below.



diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py
index 9cad03886f..a1836f4ad3 100644
--- a/tools/binman/cbfs_util.py
+++ b/tools/binman/cbfs_util.py
@@ -241,9 +241,9 @@ class CbfsFile(object):
  """Handle decompressing data if necessary"""
  indata = self.data
  if self.compress == COMPRESS_LZ4:
-data = comp_util.decompress(indata, 'lz4', with_header=False)
+data = comp_util.decompress(indata, 'lz4')
  elif self.compress == COMPRESS_LZMA:
-data = comp_util.decompress(indata, 'lzma', with_header=False)
+data = comp_util.decompress(indata, 'lzma')
  else:
  data = indata
  self.memlen = len(data)
@@ -362,9 +362,9 @@ class CbfsFile(object):
  elif self.ftype == TYPE_RAW:
  orig_data = data
  if self.compress == COMPRESS_LZ4:
-data = comp_util.compress(orig_data, 'lz4', with_header=False)
+data = comp_util.compress(orig_data, 'lz4')
  elif self.compress == COMPRESS_LZMA:
-data = comp_util.compress(orig_data, 'lzma', with_header=False)
+data = comp_util.compress(orig_data, 'lzma')
  self.memlen = len(orig_data)
  self.data_len = len(data)
  attr = struct.pack(ATTR_COMPRESSION_FORMAT,
diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py
index dc76adab35..269bbf7975 100644
--- a/tools/binman/comp_util.py
+++ b/tools/binman/comp_util.py
@@ -3,7 +3,6 @@
  #
  """Utilities to compress and decompress data"""

-import struct
  import tempfile

  from binman import bintool
@@ -16,7 +15,7 @@ LZMA_ALONE = bintool.Bintool.create('lzma_alone')
  HAVE_LZMA_ALONE = LZMA_ALONE.is_present()


-def compress(indata, algo, with_header=True):
+def compress(indata, algo):
  """Compress some data using a given algorithm

  Note that for lzma this uses an old version of the algorithm, not that
@@ -41,12 +40,9 @@ def compress(indata, algo, with_header=True):
  data = LZMA_ALONE.compress(indata)
  else:
  raise ValueError("Unknown algorithm '%s'" % algo)
-if with_header:
-hdr = struct.pack('

None ?


I copy this from the compress attribute. You only need one check to 
support a missing value or a 'none' value. But I don't need this check 
and can use None.





+
+def ReadNode(self):
+super().ReadNode()
+self.prepend = fdt_util.GetString(self._node, 'prepend', 'none')


Can you drop the 'none' so that it uses None instead?


Is 'none' a valid entry? Do we need to distinguish between 'none' and an 
invalid value?



Aso we should check for a valid value here - e.g. it must be 'length'
and not something else, otherwise self.Raise()


Okay. I will remove the 'none' and only support 'length'.

Regards
  Stefan