flash: Introduce functions for filling flash regions info

EC_CMD_FLASH_INFO requires flash_bank_array to be defined if flash
banks have different size. When using Zephyr the array is not
available. In this case we should use the Zephyr flash API.

Moving code that depends on flash_bank_array will make easier to
to enable support for variable bank sizes in CrosEC with Zephyr.
The change also makes existing code easier to read and understand.

BUG=b:239712345
BRANCH=none
TEST=make buildall -j

Signed-off-by: Patryk Duda <pdk@semihalf.com>
Change-Id: I4da19c3c302a9b64cfa4c851b3bdac10b7c438d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3932162
Reviewed-by: Bobby Casey <bobbycasey@google.com>
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Code-Coverage: Zoss <zoss-cl-coverage@prod.google.com>
Commit-Queue: Patryk Duda <patrykd@google.com>
Tested-by: Patryk Duda <patrykd@google.com>

2 files changed, 72 insertions, 24 deletions

diff --git a/common/flash.c b/common/flash.c
index 90c6a4c989..2649787101 100644
--- a/common/flash.c
+++ b/common/flash.c
@@ -218,6 +218,42 @@ int crec_flash_bank_start_offset(int bank)
 	return offset;
 }
 
+int crec_flash_response_fill_banks(struct ec_response_flash_info_2 *r,
+				   int num_banks)
+{
+	const struct ec_flash_bank *banks = flash_bank_array;
+	int banks_to_copy = MIN(ARRAY_SIZE(flash_bank_array), num_banks);
+
+	if (num_banks < 1)
+		return EC_RES_INVALID_PARAM;
+
+	memcpy(r->banks, banks, banks_to_copy * sizeof(struct ec_flash_bank));
+	r->num_banks_desc = banks_to_copy;
+	r->num_banks_total = ARRAY_SIZE(flash_bank_array);
+
+	return EC_RES_SUCCESS;
+}
+#else /* CONFIG_FLASH_MULTIPLE_REGION */
+#if CONFIG_FLASH_BANK_SIZE < CONFIG_FLASH_ERASE_SIZE
+#error "Flash: Bank size expected bigger or equal to erase size."
+#endif
+int crec_flash_response_fill_banks(struct ec_response_flash_info_2 *r,
+				   int num_banks)
+{
+	if (num_banks < 1)
+		return EC_RES_INVALID_PARAM;
+
+	r->banks[0].count = crec_flash_total_banks();
+	r->banks[0].size_exp = __fls(CONFIG_FLASH_BANK_SIZE);
+	r->banks[0].write_size_exp = __fls(CONFIG_FLASH_WRITE_SIZE);
+	r->banks[0].erase_size_exp = __fls(CONFIG_FLASH_ERASE_SIZE);
+	r->banks[0].protect_size_exp = __fls(CONFIG_FLASH_BANK_SIZE);
+
+	r->num_banks_desc = 1;
+	r->num_banks_total = 1;
+
+	return EC_RES_SUCCESS;
+}
 #endif /* CONFIG_FLASH_MULTIPLE_REGION */
 
 int crec_flash_total_banks(void)
@@ -1253,32 +1289,16 @@ static enum ec_status flash_command_get_info(struct host_cmd_handler_args *args)
 {
 	const struct ec_params_flash_info_2 *p_2 = args->params;
 	struct ec_response_flash_info_2 *r_2 = args->response;
-#ifdef CONFIG_FLASH_MULTIPLE_REGION
-	int banks_size = ARRAY_SIZE(flash_bank_array);
-	const struct ec_flash_bank *banks = flash_bank_array;
-#else
+#ifndef CONFIG_FLASH_MULTIPLE_REGION
 	struct ec_response_flash_info_1 *r_1 = args->response;
-#if CONFIG_FLASH_BANK_SIZE < CONFIG_FLASH_ERASE_SIZE
-#error "Flash: Bank size expected bigger or equal to erase size."
-#endif
-	struct ec_flash_bank single_bank = {
-		.count = CONFIG_FLASH_SIZE_BYTES / CONFIG_FLASH_BANK_SIZE,
-		.size_exp = __fls(CONFIG_FLASH_BANK_SIZE),
-		.write_size_exp = __fls(CONFIG_FLASH_WRITE_SIZE),
-		.erase_size_exp = __fls(CONFIG_FLASH_ERASE_SIZE),
-		.protect_size_exp = __fls(CONFIG_FLASH_BANK_SIZE),
-	};
-	int banks_size = 1;
-	const struct ec_flash_bank *banks = &single_bank;
 #endif
-	int banks_len;
-	int ideal_size;
+	int res;
 
 	/*
 	 * Compute the ideal amount of data for the host to send us,
 	 * based on the maximum response size and the ideal write size.
 	 */
-	ideal_size =
+	int ideal_size =
 		(args->response_max - sizeof(struct ec_params_flash_write)) &
 		~(CONFIG_FLASH_WRITE_IDEAL_SIZE - 1);
 	/*
@@ -1303,11 +1323,16 @@ static enum ec_status flash_command_get_info(struct host_cmd_handler_args *args)
 		r_2->flags |= EC_FLASH_INFO_SELECT_REQUIRED;
 #endif
 		r_2->write_ideal_size = ideal_size;
-		r_2->num_banks_total = banks_size;
-		r_2->num_banks_desc = MIN(banks_size, p_2->num_banks_desc);
-		banks_len = r_2->num_banks_desc * sizeof(struct ec_flash_bank);
-		memcpy(r_2->banks, banks, banks_len);
-		args->response_size += banks_len;
+		/*
+		 * Fill r_2->num_banks_desc, r_2->num_banks_total and
+		 * r_2->banks.
+		 */
+		res = crec_flash_response_fill_banks(r_2, p_2->num_banks_desc);
+		if (res != EC_RES_SUCCESS)
+			return res;
+
+		args->response_size +=
+			r_2->num_banks_desc * sizeof(struct ec_flash_bank);
 		return EC_RES_SUCCESS;
 	}
 #ifdef CONFIG_FLASH_MULTIPLE_REGION
diff --git a/include/flash.h b/include/flash.h
index 5448bc9c4b..9a498bce82 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -80,6 +80,29 @@ void crec_flash_print_region_info(void);
  */
 int crec_flash_total_banks(void);
 
+/**
+ * Fill flash info response structure (version 2)
+ *
+ * The function is responsible for filling 'num_banks_desc', 'num_banks_total'
+ * and 'banks' fields with information about flash layout.
+ *
+ * We are passing the whole response structure because it is marked
+ * as '__ec_align4', so it's packed, and should be aligned also but on most
+ * systems it's not because CONFIG_HOSTCMD_OPTION is not enabled. It means that
+ * the structure can be placed at ANY address. Passing the response structure
+ * gives information to the compiler how members should be accessed.
+ * Passing pointer to structure member is an error, and compiler will warn
+ * about it. Taking pointer to structure member, passing it as uint8_t and
+ * casting it is dangerous because the compiler will assume that the address
+ * is aligned and you won't get any warning about it.
+ *
+ * @param pointer to flash info version 2 response structure
+ * @param size of 'banks' array inside response structure
+ * @return EC_RES_SUCCESS or other error code.
+ */
+int crec_flash_response_fill_banks(struct ec_response_flash_info_2 *r,
+				   int num_banks);
+
 /* Persistent protection state flash offset / size / bank */
 #if defined(CONFIG_FLASH_PSTATE) && defined(CONFIG_FLASH_PSTATE_BANK)