From e138400d1c19a561eaf9f23b0cadc07226684561 Mon Sep 17 00:00:00 2001
From: Boyan Karatotev <boyan.karatotev@arm.com>
Date: Fri, 18 Nov 2022 14:17:17 +0000
Subject: [PATCH 01/10] fix: unify fallthrough annotations

Compiling with -Wimplicit-fallthrough=3 (enabled by -Wextra) produces
many warnings about fallthrough comments either missing or being wrong.
Unify the comments so we comply with -Wextra.

Note that Coverity recommends against using the __attribute__ directive.
Also, zlib does not build with a higher value of -Wimplicit-fallthrough.
Finally, compilers strip comments before expanding macros. As such,
checkpatch's fallthrough annotation (or higher levels of the flag) isn't
really possible.

Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: I060cf4f8dc04c02cbb45cf4ceb69569a8369ccee
---
 drivers/imx/usdhc/imx_usdhc.c           | 6 ++++--
 drivers/renesas/common/emmc/emmc_cmd.c  | 5 ++---
 lib/libc/snprintf.c                     | 1 +
 plat/mediatek/common/mtk_smc_handlers.c | 1 +
 plat/nxp/common/psci/plat_psci.c        | 6 +++---
 services/std_svc/spmd/spmd_main.c       | 6 ++++--
 6 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/imx/usdhc/imx_usdhc.c b/drivers/imx/usdhc/imx_usdhc.c
index 07f55b784..49dfc076d 100644
--- a/drivers/imx/usdhc/imx_usdhc.c
+++ b/drivers/imx/usdhc/imx_usdhc.c
@@ -136,7 +136,8 @@ static int imx_usdhc_send_cmd(struct mmc_cmd *cmd)
 		break;
 	case MMC_CMD(18):
 		multiple = 1;
-		/* fall thru for read op */
+		/* for read op */
+		/* fallthrough */
 	case MMC_CMD(17):
 	case MMC_CMD(8):
 		mixctl |= MIXCTRL_DTDSEL;
@@ -144,7 +145,8 @@ static int imx_usdhc_send_cmd(struct mmc_cmd *cmd)
 		break;
 	case MMC_CMD(25):
 		multiple = 1;
-		/* fall thru for data op flag */
+		/* for data op flag */
+		/* fallthrough */
 	case MMC_CMD(24):
 		data = 1;
 		break;
diff --git a/drivers/renesas/common/emmc/emmc_cmd.c b/drivers/renesas/common/emmc/emmc_cmd.c
index d255bffc9..02fc26bba 100644
--- a/drivers/renesas/common/emmc/emmc_cmd.c
+++ b/drivers/renesas/common/emmc/emmc_cmd.c
@@ -254,8 +254,7 @@ EMMC_ERROR_CODE emmc_exec_cmd(uint32_t error_mask, uint32_t *response)
 				(SD_INFO2_ALL_ERR | SD_INFO2_CLEAR));
 
 			state = ESTATE_ISSUE_CMD;
-			/* through */
-
+			/* fallthrough */
 		case ESTATE_ISSUE_CMD:
 			/* ARG */
 			SETR_32(SD_ARG, mmc_drv_obj.cmd_info.arg);
@@ -454,8 +453,8 @@ EMMC_ERROR_CODE emmc_exec_cmd(uint32_t error_mask, uint32_t *response)
 				SETR_32(SD_STOP, 0x00000000U);
 				mmc_drv_obj.during_dma_transfer = FALSE;
 			}
-			/* through */
 
+			/* fallthrough */
 		case ESTATE_ERROR:
 			if (err_not_care_flag == TRUE) {
 				mmc_drv_obj.during_cmd_processing = FALSE;
diff --git a/lib/libc/snprintf.c b/lib/libc/snprintf.c
index 6a2f0ba7f..0e3256cde 100644
--- a/lib/libc/snprintf.c
+++ b/lib/libc/snprintf.c
@@ -209,6 +209,7 @@ loop:
 				break;
 			case 'X':
 				capitalise = true;
+				/* fallthrough */
 			case 'x':
 				unum = get_unum_va_args(args, l_count);
 				unsigned_num_print(&s, n, &chars_printed,
diff --git a/plat/mediatek/common/mtk_smc_handlers.c b/plat/mediatek/common/mtk_smc_handlers.c
index 51a960fc3..92b3873c3 100644
--- a/plat/mediatek/common/mtk_smc_handlers.c
+++ b/plat/mediatek/common/mtk_smc_handlers.c
@@ -51,6 +51,7 @@
 		x3 = x3 & MASK_32_BIT; \
 		x4 = x4 & MASK_32_BIT; \
 	} \
+	/* fallthrough */ \
 	case _smc_id##_AARCH64: \
 	{ \
 		if (_smc_id##_descriptor_index < 0) { \
diff --git a/plat/nxp/common/psci/plat_psci.c b/plat/nxp/common/psci/plat_psci.c
index 9281e97cb..f6dd7b30e 100644
--- a/plat/nxp/common/psci/plat_psci.c
+++ b/plat/nxp/common/psci/plat_psci.c
@@ -350,7 +350,7 @@ static int _pwr_state_validate(uint32_t pwr_state,
 		else if (SOC_SYSTEM_STANDBY)
 			state->pwr_domain_state[PLAT_MAX_LVL] =
 				PLAT_MAX_RET_STATE;
-		 /* intentional fall-thru condition */
+		 /* fallthrough */
 	case PWR_STATE_LVL_SYS:
 		if (pwrdn && SOC_SYSTEM_PWR_DWN)
 			state->pwr_domain_state[PLAT_SYS_LVL] =
@@ -358,7 +358,7 @@ static int _pwr_state_validate(uint32_t pwr_state,
 		else if (SOC_SYSTEM_STANDBY)
 			state->pwr_domain_state[PLAT_SYS_LVL] =
 				PLAT_MAX_RET_STATE;
-		 /* intentional fall-thru condition */
+		 /* fallthrough */
 	case PWR_STATE_LVL_CLSTR:
 		if (pwrdn && SOC_CLUSTER_PWR_DWN)
 			state->pwr_domain_state[PLAT_CLSTR_LVL] =
@@ -366,7 +366,7 @@ static int _pwr_state_validate(uint32_t pwr_state,
 		else if (SOC_CLUSTER_STANDBY)
 			state->pwr_domain_state[PLAT_CLSTR_LVL] =
 				PLAT_MAX_RET_STATE;
-		 /* intentional fall-thru condition */
+		 /* fallthrough */
 	case PWR_STATE_LVL_CORE:
 		stat = PSCI_E_SUCCESS;
 
diff --git a/services/std_svc/spmd/spmd_main.c b/services/std_svc/spmd/spmd_main.c
index 7e6c89df3..afd0f2ea2 100644
--- a/services/std_svc/spmd/spmd_main.c
+++ b/services/std_svc/spmd/spmd_main.c
@@ -868,7 +868,8 @@ uint64_t spmd_smc_handler(uint32_t smc_fid,
 						     FFA_ERROR_NOT_SUPPORTED);
 		}
 
-		/* Fall through to forward the call to the other world */
+		/* Forward the call to the other world */
+		/* fallthrough */
 	case FFA_MSG_SEND:
 	case FFA_MSG_SEND_DIRECT_RESP_SMC64:
 	case FFA_MEM_DONATE_SMC32:
@@ -908,7 +909,8 @@ uint64_t spmd_smc_handler(uint32_t smc_fid,
 			spmd_spm_core_sync_exit(0ULL);
 		}
 
-		/* Fall through to forward the call to the other world */
+		/* Forward the call to the other world */
+		/* fallthrough */
 	case FFA_INTERRUPT:
 	case FFA_MSG_YIELD:
 		/* This interface must be invoked only by the Secure world */

From f4b8470feee4437fb3984baeee8c61ed91f63f51 Mon Sep 17 00:00:00 2001
From: Boyan Karatotev <boyan.karatotev@arm.com>
Date: Tue, 22 Nov 2022 14:31:41 +0000
Subject: [PATCH 02/10] fix: remove old-style declarations

TF-A wants to eventually enable -Wold-style-definition globally. Convert
the rare few instances where this is still the case.

Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: I9c450fc875cf097e6de2ed577ea3b085821c9f5e
---
 drivers/nxp/ddr/nxp-ddr/ddr.c             | 2 +-
 drivers/nxp/ddr/phy-gen2/messages.h       | 4 ++--
 plat/imx/common/imx_sip_handler.c         | 2 +-
 plat/imx/imx8qm/imx8qm_bl31_setup.c       | 2 +-
 plat/imx/imx8qm/imx8qm_psci.c             | 2 +-
 plat/imx/imx8qx/imx8qx_psci.c             | 2 +-
 plat/nvidia/tegra/soc/t186/plat_memctrl.c | 6 +++---
 plat/xilinx/common/ipi.c                  | 2 +-
 plat/xilinx/versal/versal_ipi.c           | 2 +-
 plat/xilinx/zynqmp/zynqmp_ipi.c           | 2 +-
 10 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/nxp/ddr/nxp-ddr/ddr.c b/drivers/nxp/ddr/nxp-ddr/ddr.c
index c051b3b25..faf20e963 100644
--- a/drivers/nxp/ddr/nxp-ddr/ddr.c
+++ b/drivers/nxp/ddr/nxp-ddr/ddr.c
@@ -269,7 +269,7 @@ static int cal_odt(const unsigned int clk,
 	unsigned int i;
 	const struct dynamic_odt *pdodt = NULL;
 
-	const static struct dynamic_odt *table[2][5] = {
+	static const struct dynamic_odt *table[2][5] = {
 		{single_S, single_D, NULL, NULL},
 		{dual_SS, dual_DD, NULL, NULL},
 	};
diff --git a/drivers/nxp/ddr/phy-gen2/messages.h b/drivers/nxp/ddr/phy-gen2/messages.h
index 7dec7df55..a2310f23b 100644
--- a/drivers/nxp/ddr/phy-gen2/messages.h
+++ b/drivers/nxp/ddr/phy-gen2/messages.h
@@ -13,7 +13,7 @@ struct phy_msg {
 	const char *msg;
 };
 
-const static struct phy_msg messages_1d[] = {
+static const struct phy_msg messages_1d[] = {
 	{0x00000001,
 	 "PMU1:prbsGenCtl:%x\n"
 	},
@@ -1239,7 +1239,7 @@ const static struct phy_msg messages_1d[] = {
 	},
 };
 
-const static struct phy_msg messages_2d[] = {
+static const struct phy_msg messages_2d[] = {
 	{0x00000001,
 	 "PMU0: Converting %d into an MR\n"
 	},
diff --git a/plat/imx/common/imx_sip_handler.c b/plat/imx/common/imx_sip_handler.c
index d4b3425af..ec8631a4f 100644
--- a/plat/imx/common/imx_sip_handler.c
+++ b/plat/imx/common/imx_sip_handler.c
@@ -20,7 +20,7 @@
 #if defined(PLAT_imx8qm) || defined(PLAT_imx8qx)
 
 #ifdef PLAT_imx8qm
-const static int ap_cluster_index[PLATFORM_CLUSTER_COUNT] = {
+static const int ap_cluster_index[PLATFORM_CLUSTER_COUNT] = {
 	SC_R_A53, SC_R_A72,
 };
 #endif
diff --git a/plat/imx/imx8qm/imx8qm_bl31_setup.c b/plat/imx/imx8qm/imx8qm_bl31_setup.c
index 68eb53422..bd7896a99 100644
--- a/plat/imx/imx8qm/imx8qm_bl31_setup.c
+++ b/plat/imx/imx8qm/imx8qm_bl31_setup.c
@@ -62,7 +62,7 @@ static entry_point_info_t bl33_image_ep_info;
 #error "Provide proper UART number in IMX_DEBUG_UART"
 #endif
 
-const static int imx8qm_cci_map[] = {
+static const int imx8qm_cci_map[] = {
 	CLUSTER0_CCI_SLVAE_IFACE,
 	CLUSTER1_CCI_SLVAE_IFACE
 };
diff --git a/plat/imx/imx8qm/imx8qm_psci.c b/plat/imx/imx8qm/imx8qm_psci.c
index bdba37c6e..dcc502ff8 100644
--- a/plat/imx/imx8qm/imx8qm_psci.c
+++ b/plat/imx/imx8qm/imx8qm_psci.c
@@ -26,7 +26,7 @@
 #define SYSTEM_PWR_STATE(state) \
 	((state)->pwr_domain_state[PLAT_MAX_PWR_LVL])
 
-const static int ap_core_index[PLATFORM_CORE_COUNT] = {
+static const int ap_core_index[PLATFORM_CORE_COUNT] = {
 	SC_R_A53_0, SC_R_A53_1, SC_R_A53_2,
 	SC_R_A53_3, SC_R_A72_0, SC_R_A72_1,
 };
diff --git a/plat/imx/imx8qx/imx8qx_psci.c b/plat/imx/imx8qx/imx8qx_psci.c
index aab3a2dae..5f0556665 100644
--- a/plat/imx/imx8qx/imx8qx_psci.c
+++ b/plat/imx/imx8qx/imx8qx_psci.c
@@ -18,7 +18,7 @@
 
 #include "../../common/sci/imx8_mu.h"
 
-const static int ap_core_index[PLATFORM_CORE_COUNT] = {
+static const int ap_core_index[PLATFORM_CORE_COUNT] = {
 	SC_R_A35_0, SC_R_A35_1, SC_R_A35_2, SC_R_A35_3
 };
 
diff --git a/plat/nvidia/tegra/soc/t186/plat_memctrl.c b/plat/nvidia/tegra/soc/t186/plat_memctrl.c
index 81de674bc..253301304 100644
--- a/plat/nvidia/tegra/soc/t186/plat_memctrl.c
+++ b/plat/nvidia/tegra/soc/t186/plat_memctrl.c
@@ -20,7 +20,7 @@ extern uint64_t tegra_bl31_phys_base;
 /*******************************************************************************
  * Array to hold stream_id override config register offsets
  ******************************************************************************/
-const static uint32_t tegra186_streamid_override_regs[] = {
+static const uint32_t tegra186_streamid_override_regs[] = {
 	MC_STREAMID_OVERRIDE_CFG_SDMMCRA,
 	MC_STREAMID_OVERRIDE_CFG_SDMMCRAA,
 	MC_STREAMID_OVERRIDE_CFG_SDMMCR,
@@ -34,7 +34,7 @@ const static uint32_t tegra186_streamid_override_regs[] = {
 /*******************************************************************************
  * Array to hold the security configs for stream IDs
  ******************************************************************************/
-const static mc_streamid_security_cfg_t tegra186_streamid_sec_cfgs[] = {
+static const mc_streamid_security_cfg_t tegra186_streamid_sec_cfgs[] = {
 	mc_make_sec_cfg(SCEW, NON_SECURE, NO_OVERRIDE, DISABLE),
 	mc_make_sec_cfg(AFIR, NON_SECURE, OVERRIDE, DISABLE),
 	mc_make_sec_cfg(AFIW, NON_SECURE, OVERRIDE, DISABLE),
@@ -112,7 +112,7 @@ const static mc_streamid_security_cfg_t tegra186_streamid_sec_cfgs[] = {
 /*******************************************************************************
  * Array to hold the transaction override configs
  ******************************************************************************/
-const static mc_txn_override_cfg_t tegra186_txn_override_cfgs[] = {
+static const mc_txn_override_cfg_t tegra186_txn_override_cfgs[] = {
 	mc_make_txn_override_cfg(BPMPW, CGID_TAG_ADR),
 	mc_make_txn_override_cfg(EQOSW, CGID_TAG_ADR),
 	mc_make_txn_override_cfg(NVJPGSWR, CGID_TAG_ADR),
diff --git a/plat/xilinx/common/ipi.c b/plat/xilinx/common/ipi.c
index 643889647..8fa7bc563 100644
--- a/plat/xilinx/common/ipi.c
+++ b/plat/xilinx/common/ipi.c
@@ -39,7 +39,7 @@
 #define IPI_BIT_MASK(I) (ipi_table[(I)].ipi_bit_mask)
 
 /* IPI configuration table */
-const static struct ipi_config *ipi_table;
+static const struct ipi_config *ipi_table;
 
 /* Total number of IPI */
 static uint32_t ipi_total;
diff --git a/plat/xilinx/versal/versal_ipi.c b/plat/xilinx/versal/versal_ipi.c
index f99af8211..d821929a8 100644
--- a/plat/xilinx/versal/versal_ipi.c
+++ b/plat/xilinx/versal/versal_ipi.c
@@ -19,7 +19,7 @@
 #include <lib/mmio.h>
 
 /* versal ipi configuration table */
-const static struct ipi_config versal_ipi_table[] = {
+static const struct ipi_config versal_ipi_table[] = {
 	/* A72 IPI */
 	[IPI_ID_APU] = {
 		.ipi_bit_mask = IPI0_TRIG_BIT,
diff --git a/plat/xilinx/zynqmp/zynqmp_ipi.c b/plat/xilinx/zynqmp/zynqmp_ipi.c
index 4ea3c6a33..acd31df0e 100644
--- a/plat/xilinx/zynqmp/zynqmp_ipi.c
+++ b/plat/xilinx/zynqmp/zynqmp_ipi.c
@@ -21,7 +21,7 @@
 #include <plat_private.h>
 
 /* Zynqmp ipi configuration table */
-const static struct ipi_config zynqmp_ipi_table[] = {
+static const struct ipi_config zynqmp_ipi_table[] = {
 	/* APU IPI */
 	{
 		.ipi_bit_mask = 0x1,

From d0b58c8a9bff3cabfdb59e052ab7eaecfe64b305 Mon Sep 17 00:00:00 2001
From: Boyan Karatotev <boyan.karatotev@arm.com>
Date: Tue, 22 Nov 2022 12:13:44 +0000
Subject: [PATCH 03/10] fix(zynqmp): remove redundant api_version check

The api_version is checked in pm_setup() and an error is returned. The
smc handlers will not be registered on error so doing the check again is
redundant. This also silences a warning when compiling with -Wextra.

Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: I09395e6a20e3f6eb22a1f81ec2f6bdf034eeb4bf
---
 plat/xilinx/zynqmp/pm_service/pm_svc_main.c | 26 +++++++++------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/plat/xilinx/zynqmp/pm_service/pm_svc_main.c b/plat/xilinx/zynqmp/pm_service/pm_svc_main.c
index b45ce6c76..42eec0d49 100644
--- a/plat/xilinx/zynqmp/pm_service/pm_svc_main.c
+++ b/plat/xilinx/zynqmp/pm_service/pm_svc_main.c
@@ -334,22 +334,18 @@ uint64_t pm_smc_handler(uint32_t smc_fid, uint64_t x1, uint64_t x2, uint64_t x3,
 		SMC_RET1(handle, (uint64_t)ret);
 
 	case PM_GET_API_VERSION:
-		/* Check is PM API version already verified */
-		if (pm_ctx.api_version >= PM_VERSION) {
-			if (ipi_irq_flag == 0U) {
-				/*
-				 * Enable IPI IRQ
-				 * assume the rich OS is OK to handle callback IRQs now.
-				 * Even if we were wrong, it would not enable the IRQ in
-				 * the GIC.
-				 */
-				pm_ipi_irq_enable(primary_proc);
-				ipi_irq_flag = 1U;
-			}
-			SMC_RET1(handle, (uint64_t)PM_RET_SUCCESS |
-				 ((uint64_t)pm_ctx.api_version << 32));
+		if (ipi_irq_flag == 0U) {
+			/*
+			 * Enable IPI IRQ
+			 * assume the rich OS is OK to handle callback IRQs now.
+			 * Even if we were wrong, it would not enable the IRQ in
+			 * the GIC.
+			 */
+			pm_ipi_irq_enable(primary_proc);
+			ipi_irq_flag = 1U;
 		}
-
+		SMC_RET1(handle, (uint64_t)PM_RET_SUCCESS |
+			 ((uint64_t)pm_ctx.api_version << 32));
 	case PM_FPGA_LOAD:
 		ret = pm_fpga_load(pm_arg[0], pm_arg[1], pm_arg[2], pm_arg[3]);
 		SMC_RET1(handle, (uint64_t)ret);

From 90c4b3b62d5303c22fdc5f65f0db784de0f4ac95 Mon Sep 17 00:00:00 2001
From: Boyan Karatotev <boyan.karatotev@arm.com>
Date: Tue, 22 Nov 2022 12:24:10 +0000
Subject: [PATCH 04/10] fix(renesas): align incompatible function pointers

secure_boot_api_f is defined to take uint32_t, uint32_t, and void *
parameters. However rom_secure_boot_api_f is defined to take uint32_t *,
uint32_t *, void *. These are incompatible and cause a warning when
compiling with -Wextra. Align the rom definition to the more generic
definition from where it's called.

Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: Ia030803b3c2335d220aff09fc0eef5c7615276aa
---
 drivers/renesas/common/rom/rom_api.c | 4 ++--
 drivers/renesas/common/rom/rom_api.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/renesas/common/rom/rom_api.c b/drivers/renesas/common/rom/rom_api.c
index fda28150e..4eede17ce 100644
--- a/drivers/renesas/common/rom/rom_api.c
+++ b/drivers/renesas/common/rom/rom_api.c
@@ -11,7 +11,7 @@
 #include "rcar_def.h"
 #include "rom_api.h"
 
-typedef uint32_t(*rom_secure_boot_api_f) (uint32_t *key, uint32_t *cert,
+typedef uint32_t(*rom_secure_boot_api_f) (uint32_t key, uint32_t cert,
 					  rom_read_flash_f pFuncReadFlash);
 
 typedef uint32_t(*rom_get_lcs_api_f) (uint32_t *lcs);
@@ -68,7 +68,7 @@ static uint32_t get_table_index(void)
 	return index;
 }
 
-uint32_t rcar_rom_secure_boot_api(uint32_t *key, uint32_t *cert,
+uint32_t rcar_rom_secure_boot_api(uint32_t key, uint32_t cert,
 			     rom_read_flash_f read_flash)
 {
 	static const uintptr_t rom_api_table[API_TABLE_MAX] = {
diff --git a/drivers/renesas/common/rom/rom_api.h b/drivers/renesas/common/rom/rom_api.h
index 1d5b03d7f..4b1008032 100644
--- a/drivers/renesas/common/rom/rom_api.h
+++ b/drivers/renesas/common/rom/rom_api.h
@@ -24,7 +24,7 @@
 #define LCS_FA					(0x7U)
 
 typedef uint32_t(*rom_read_flash_f) (uint64_t src, uint8_t *dst, uint32_t len);
-uint32_t rcar_rom_secure_boot_api(uint32_t *key, uint32_t *cert,
+uint32_t rcar_rom_secure_boot_api(uint32_t key, uint32_t cert,
 				  rom_read_flash_f f);
 uint32_t rcar_rom_get_lcs(uint32_t *lcs);
 

From 9f58bfbbe90d2891c289cd27ab7d2ede8b5572d4 Mon Sep 17 00:00:00 2001
From: Boyan Karatotev <boyan.karatotev@arm.com>
Date: Tue, 22 Nov 2022 14:05:18 +0000
Subject: [PATCH 05/10] fix(brcm): add braces around bodies of conditionals

On release builds EMMC_TRACE doesn't expand to anything. Some
conditionals with no braces end up with empty bodies. This produces a
warning when compiling with -Werror=empty-body (enabled by -Wextra).

Since TF-A coding guidelines require braces to comply with MISRA
guidelines anyway, add them in the whole file.

Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: Ib4e691efc7acdb8fb8692278c7a9772fc894f77f
---
 drivers/brcm/emmc/emmc_csl_sdcard.c           |  5 ++-
 drivers/brcm/emmc/emmc_pboot_hal_memory_drv.c | 40 ++++++++++++-------
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/drivers/brcm/emmc/emmc_csl_sdcard.c b/drivers/brcm/emmc/emmc_csl_sdcard.c
index 9e2c618d9..40bc4a058 100644
--- a/drivers/brcm/emmc/emmc_csl_sdcard.c
+++ b/drivers/brcm/emmc/emmc_csl_sdcard.c
@@ -479,10 +479,11 @@ int init_mmc_card(struct sd_handle *handle)
 			handle->device->cfg.blockSize = 512;
 		}
 
-		if (handle->device->ctrl.ocr & SD_CARD_HIGH_CAPACITY)
+		if (handle->device->ctrl.ocr & SD_CARD_HIGH_CAPACITY) {
 			EMMC_TRACE("Sector addressing\n");
-		else
+		} else {
 			EMMC_TRACE("Byte addressing\n");
+		}
 
 		EMMC_TRACE("Ext_CSD_storage[162]: 0x%02X  Ext_CSD_storage[179]: 0x%02X\n",
 			   emmc_global_buf_ptr->u.Ext_CSD_storage[162],
diff --git a/drivers/brcm/emmc/emmc_pboot_hal_memory_drv.c b/drivers/brcm/emmc/emmc_pboot_hal_memory_drv.c
index 68f93e75e..fcd499f16 100644
--- a/drivers/brcm/emmc/emmc_pboot_hal_memory_drv.c
+++ b/drivers/brcm/emmc/emmc_pboot_hal_memory_drv.c
@@ -278,8 +278,9 @@ struct sd_handle *sdio_init(void)
 
 	SDIO_base = EMMC_CTRL_REGS_BASE_ADDR;
 
-	if (SDIO_base == SDIO0_EMMCSDXC_SYSADDR)
+	if (SDIO_base == SDIO0_EMMCSDXC_SYSADDR) {
 		EMMC_TRACE(" ---> for SDIO 0 Controller\n\n");
+	}
 
 	memset(p_sdhandle, 0, sizeof(struct sd_handle));
 
@@ -290,8 +291,9 @@ struct sd_handle *sdio_init(void)
 	memset(p_sdhandle->card, 0, sizeof(struct sd_card_info));
 
 	if (chal_sd_start((CHAL_HANDLE *) p_sdhandle->device,
-			  SD_PIO_MODE, SDIO_base, SDIO_base) != SD_OK)
+			  SD_PIO_MODE, SDIO_base, SDIO_base) != SD_OK) {
 		return NULL;
+	}
 
 	set_config(p_sdhandle, SD_NORMAL_SPEED, MAX_CMD_RETRY, SD_DMA_OFF,
 		   SD_DMA_BOUNDARY_4K, EMMC_BLOCK_SIZE, EMMC_WFE_RETRY);
@@ -330,14 +332,16 @@ uint32_t sdio_read(struct sd_handle *p_sdhandle,
 	VERBOSE("EMMC READ: dst=0x%lx, src=0x%lx, size=0x%lx\n",
 			storage_addr, mem_addr, bytes_to_read);
 
-	if (storage_size < bytes_to_read)
+	if (storage_size < bytes_to_read) {
 		/* Don't have sufficient storage to complete the operation */
 		return 0;
+	}
 
 	/* Range check non high capacity memory */
 	if ((p_sdhandle->device->ctrl.ocr & SD_CARD_HIGH_CAPACITY) == 0) {
-		if (mem_addr > 0x80000000)
+		if (mem_addr > 0x80000000) {
 			return 0;
+		}
 	}
 
 	/* High capacity card use block address mode */
@@ -384,10 +388,11 @@ uint32_t sdio_read(struct sd_handle *p_sdhandle,
 			/* Update Physical address */
 			outputBuf += manual_copy_size;
 
-			if (p_sdhandle->device->ctrl.ocr & SD_CARD_HIGH_CAPACITY)
+			if (p_sdhandle->device->ctrl.ocr & SD_CARD_HIGH_CAPACITY) {
 				blockAddr++;
-			else
+			} else {
 				blockAddr += blockSize;
+			}
 		} else {
 			return 0;
 		}
@@ -395,10 +400,11 @@ uint32_t sdio_read(struct sd_handle *p_sdhandle,
 
 	while (remSize >= blockSize) {
 
-		if (remSize >= SD_MAX_BLK_TRANSFER_LENGTH)
+		if (remSize >= SD_MAX_BLK_TRANSFER_LENGTH) {
 			readLen = SD_MAX_BLK_TRANSFER_LENGTH;
-		else
+		} else {
 			readLen = (remSize / blockSize) * blockSize;
+		}
 
 		/* Check for overflow */
 		if ((rdCount + readLen) > storage_size ||
@@ -409,10 +415,11 @@ uint32_t sdio_read(struct sd_handle *p_sdhandle,
 		}
 
 		if (!read_block(p_sdhandle, outputBuf, blockAddr, readLen)) {
-			if (p_sdhandle->device->ctrl.ocr & SD_CARD_HIGH_CAPACITY)
+			if (p_sdhandle->device->ctrl.ocr & SD_CARD_HIGH_CAPACITY) {
 				blockAddr += (readLen / blockSize);
-			else
+			} else {
 				blockAddr += readLen;
+			}
 
 			remSize -= readLen;
 			rdCount += readLen;
@@ -463,8 +470,9 @@ static uint32_t sdio_write(struct sd_handle *p_sdhandle, uintptr_t mem_addr,
 
 	/* range check non high capacity memory */
 	if ((p_sdhandle->device->ctrl.ocr & SD_CARD_HIGH_CAPACITY) == 0) {
-		if (mem_addr > 0x80000000)
+		if (mem_addr > 0x80000000) {
 			return 0;
+		}
 	}
 
 	/* the high capacity card use block address mode */
@@ -491,11 +499,12 @@ static uint32_t sdio_write(struct sd_handle *p_sdhandle, uintptr_t mem_addr,
 				blockAddr, p_sdhandle->device->cfg.blockSize)) {
 
 			if (remSize <
-			    (p_sdhandle->device->cfg.blockSize - offset))
+			    (p_sdhandle->device->cfg.blockSize - offset)) {
 				manual_copy_size = remSize;
-			else
+			} else {
 				manual_copy_size =
 				    p_sdhandle->device->cfg.blockSize - offset;
+			}
 
 			memcpy((void *)((uintptr_t)
 				(emmc_global_buf_ptr->u.tempbuf + offset)),
@@ -530,11 +539,12 @@ static uint32_t sdio_write(struct sd_handle *p_sdhandle, uintptr_t mem_addr,
 				inputBuf += manual_copy_size;
 
 				if (p_sdhandle->device->ctrl.ocr &
-				    SD_CARD_HIGH_CAPACITY)
+				    SD_CARD_HIGH_CAPACITY) {
 					blockAddr++;
-				else
+				} else {
 					blockAddr +=
 					    p_sdhandle->device->cfg.blockSize;
+				}
 			} else
 				return 0;
 		} else {

From 02af589cfa8d8aefaffeef3390e3fb8fdf51978f Mon Sep 17 00:00:00 2001
From: Boyan Karatotev <boyan.karatotev@arm.com>
Date: Tue, 22 Nov 2022 14:09:36 +0000
Subject: [PATCH 06/10] fix(st-usb): replace redundant checks with asserts

Returning enum usb_status in an enum usb_action function is wrong as
they have different meanings. However, usb_dwc2_ep0_out_start() and
usb_dwc2_activate_setup() only return USBD_OK so we will never get to
there. Replace these checks with asserts in case the code changes in
future.

This also silences a warning when compiling with -Wextra.

Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: I73dfd5c189a357544c15ceb3f4268da82ce272b9
---
 drivers/st/usb/stm32mp1_usb.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/st/usb/stm32mp1_usb.c b/drivers/st/usb/stm32mp1_usb.c
index 9a4969036..78890f5eb 100644
--- a/drivers/st/usb/stm32mp1_usb.c
+++ b/drivers/st/usb/stm32mp1_usb.c
@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: BSD-3-Clause
  */
 
+#include <assert.h>
 #include <stdint.h>
 
 #include <arch_helpers.h>
@@ -794,7 +795,7 @@ static enum usb_action usb_dwc2_it_handler(void *handle, uint32_t *param)
 	uint32_t epint;
 	uint32_t epnum;
 	uint32_t temp;
-	enum usb_status ret;
+	enum usb_status __unused ret;
 
 	if (usb_dwc2_get_mode(handle) != USB_OTG_MODE_DEVICE) {
 		return USB_NOTHING;
@@ -947,9 +948,7 @@ static enum usb_action usb_dwc2_it_handler(void *handle, uint32_t *param)
 
 		/* Setup EP0 to receive SETUP packets */
 		ret = usb_dwc2_ep0_out_start(handle);
-		if (ret != USBD_OK) {
-			return ret;
-		}
+		assert(ret == USBD_OK);
 
 		mmio_write_32(usb_base_addr + OTG_GINTSTS, OTG_GINTSTS_USBRST);
 
@@ -959,9 +958,7 @@ static enum usb_action usb_dwc2_it_handler(void *handle, uint32_t *param)
 	/* Handle enumeration done interrupt */
 	if ((usb_dwc2_read_int(handle) & OTG_GINTSTS_ENUMDNE) != 0U) {
 		ret = usb_dwc2_activate_setup(handle);
-		if (ret != USBD_OK) {
-			return ret;
-		}
+		assert(ret == USBD_OK);
 
 		mmio_clrbits_32(usb_base_addr + OTG_GUSBCFG, OTG_GUSBCFG_TRDT);
 

From 228b06a5352bc149fcbba06c7701203f5ce921d3 Mon Sep 17 00:00:00 2001
From: Boyan Karatotev <boyan.karatotev@arm.com>
Date: Tue, 22 Nov 2022 12:01:09 +0000
Subject: [PATCH 07/10] docs(porting-guide): update a reference

The BL31 part has been there forever and the PSCI reference is neither
at section 3.3 or directly below. Update this to locate the section more
easily.

Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: I9a86e4ef13d1ac5da743917493f63ddd7690e087
---
 docs/getting_started/porting-guide.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/getting_started/porting-guide.rst b/docs/getting_started/porting-guide.rst
index aa57e1db6..783691426 100644
--- a/docs/getting_started/porting-guide.rst
+++ b/docs/getting_started/porting-guide.rst
@@ -2135,7 +2135,7 @@ CPUs. BL31 executes at EL3 and is responsible for:
 
 #. Providing runtime firmware services. Currently, BL31 only implements a
    subset of the Power State Coordination Interface (PSCI) API as a runtime
-   service. See Section 3.3 below for details of porting the PSCI
+   service. See :ref:`psci_in_bl31` below for details of porting the PSCI
    implementation.
 
 #. Optionally passing control to the BL32 image, pre-loaded at a platform-
@@ -2544,6 +2544,8 @@ Function: bool plat_get_entropy(uint64_t \*out) [mandatory]
 This function writes entropy into storage provided by the caller. If no entropy
 is available, it must return false and the storage must not be written.
 
+.. _psci_in_bl31:
+
 Power State Coordination Interface (in BL31)
 --------------------------------------------
 

From d75a9ecdaa8caa7f845297958a3f606b66bc2264 Mon Sep 17 00:00:00 2001
From: Boyan Karatotev <boyan.karatotev@arm.com>
Date: Mon, 21 Nov 2022 14:16:43 +0000
Subject: [PATCH 08/10] build: include -Wextra in generic builds

TF-A is more strict with compiler warnings in comparison to other
projects (notably Linux) for security and -Wextra enables a lot of
desirable warnings. This patch enables -Wextra by default (from W=1
previously) and reorganises the warning levels so that they can useful
when enabled and not just a build failure. This will help us move
towards fixing the warnings that are too many to fix at once and
enabling all W={1, 2} warnings.

The warning levels get new meanings:
* W=1: warnings we want the generic build to include but are too time
  consuming to fix at the moment. They re-enable warnings taken out for
  generic builds.
* W=2: warnings we want the generic build to include but cannot be
  enabled due to external libraries.
* W=3: warnings that are informative but not necessary and generally too
  verbose and frequently ignored.

Quality expectations for new contributions mean that generally they
should have no warnings up to W=2. To allow code to be developed with
them in mind, -Werror is disabled when W=x is set. This way enabling
warnings will not just fail the build due to technicalities we have and
contributors will be able to actually see if they get any.

Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: Ieb15ddd635d458a956a34b0f9d0ea2f81b9c0745
---
 Makefile | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 1ddb7b844..03105d700 100644
--- a/Makefile
+++ b/Makefile
@@ -350,27 +350,51 @@ ASFLAGS_aarch64		=	$(march64-directive)
 # General warnings
 WARNINGS		:=	-Wall -Wmissing-include-dirs -Wunused	\
 				-Wdisabled-optimization -Wvla -Wshadow	\
-				-Wno-unused-parameter -Wredundant-decls
+				-Wredundant-decls
+# stricter warnings
+WARNINGS		+=	-Wextra -Wno-trigraphs
+# too verbose for generic build
+WARNINGS		+=	-Wno-missing-field-initializers \
+				-Wno-type-limits -Wno-sign-compare \
+# on clang this flag gets reset if -Wextra is set after it. No difference on gcc
+WARNINGS		+=	-Wno-unused-parameter
 
 # Additional warnings
-# Level 1
-WARNING1 := -Wextra
-WARNING1 += -Wmissing-format-attribute
-WARNING1 += -Wmissing-prototypes
-WARNING1 += -Wold-style-definition
+# Level 1 - infrequent warnings we should have none of
+# full -Wextra
+WARNING1 += -Wsign-compare
+WARNING1 += -Wtype-limits
+WARNING1 += -Wmissing-field-initializers
 
-# Level 2
-WARNING2 := -Waggregate-return
-WARNING2 += -Wcast-align
-WARNING2 += -Wnested-externs
+# Level 2 - problematic warnings that we want
+# zlib, compiler-rt, coreboot, and mbdedtls blow up with these
+# TODO: disable just for them and move into default build
+WARNING2 += -Wold-style-definition
+WARNING2 += -Wmissing-prototypes
+WARNING2 += -Wmissing-format-attribute
+# TF-A aims to comply with this eventually. Effort too large at present
+WARNING2 += -Wundef
 
+# Level 3 - very pedantic, frequently ignored
 WARNING3 := -Wbad-function-cast
+WARNING3 += -Waggregate-return
+WARNING3 += -Wnested-externs
+WARNING3 += -Wcast-align
 WARNING3 += -Wcast-qual
 WARNING3 += -Wconversion
 WARNING3 += -Wpacked
 WARNING3 += -Wpointer-arith
 WARNING3 += -Wswitch-default
 
+# Setting W is quite verbose and most warnings will be pre-existing issues
+# outside of the contributor's control. Don't fail the build on them so warnings
+# can be seen and hopefully addressed
+ifdef W
+ifneq (${W},0)
+E	 ?= 0
+endif
+endif
+
 ifeq (${W},1)
 WARNINGS += $(WARNING1)
 else ifeq (${W},2)

From d141e638442ea37ae64b5ddd375349aaa1870c2d Mon Sep 17 00:00:00 2001
From: Boyan Karatotev <boyan.karatotev@arm.com>
Date: Mon, 21 Nov 2022 14:49:05 +0000
Subject: [PATCH 09/10] build: add -Wunused-const-variable=2 to W=2

TF-A is quite strict with warnings and redundant code. This flag
furthers this so it would be useful to have it. Add it to W=2 as it sets
off a few platforms which require a somewhat involved fix.

Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: Id52b3d477b4ada7dd69a36101ab22c575ab4ef19
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index 03105d700..6d7865a82 100644
--- a/Makefile
+++ b/Makefile
@@ -374,6 +374,8 @@ WARNING2 += -Wmissing-prototypes
 WARNING2 += -Wmissing-format-attribute
 # TF-A aims to comply with this eventually. Effort too large at present
 WARNING2 += -Wundef
+# currently very involved and many platforms set this off
+WARNING2 += -Wunused-const-variable=2
 
 # Level 3 - very pedantic, frequently ignored
 WARNING3 := -Wbad-function-cast

From 291be198faf634c2cde6812ac9a59b1573b71806 Mon Sep 17 00:00:00 2001
From: Boyan Karatotev <boyan.karatotev@arm.com>
Date: Wed, 7 Dec 2022 10:26:48 +0000
Subject: [PATCH 10/10] docs: describe the new warning levels

When -Wextra was added, the warning levels changed their meaning. Add a
description in the build option section and leave the security hardening
section as mostly a pointer to it.

Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: Iabf2f598d0bf3e865c9b991c5d44d2acb9572bd5
---
 docs/getting_started/build-options.rst | 45 ++++++++++++++++++++++++++
 docs/process/security-hardening.rst    | 35 ++------------------
 2 files changed, 48 insertions(+), 32 deletions(-)

diff --git a/docs/getting_started/build-options.rst b/docs/getting_started/build-options.rst
index 402de1361..c4667cafc 100644
--- a/docs/getting_started/build-options.rst
+++ b/docs/getting_started/build-options.rst
@@ -219,6 +219,12 @@ Common build options
 
 -  ``E``: Boolean option to make warnings into errors. Default is 1.
 
+   When specifying higher warnings levels (``W=1`` and higher), this option
+   defaults to 0. This is done to encourage contributors to use them, as they
+   are expected to produce warnings that would otherwise fail the build. New
+   contributions are still expected to build with ``W=0`` and ``E=1`` (the
+   default).
+
 -  ``EL3_PAYLOAD_BASE``: This option enables booting an EL3 payload instead of
    the normal boot flow. It must specify the entry point address of the EL3
    payload. Please refer to the "Booting an EL3 payload" section for more
@@ -955,6 +961,43 @@ Common build options
    regrouped and put in the root Makefile. This flag can take the values 0 to 3,
    each level enabling more warning options. Default is 0.
 
+   This option is closely related to the ``E`` option, which enables
+   ``-Werror``.
+
+   - ``W=0`` (default)
+
+     Enables a wide assortment of warnings, most notably ``-Wall`` and
+     ``-Wextra``, as well as various bad practices and things that are likely to
+     result in errors. Includes some compiler specific flags. No warnings are
+     expected at this level for any build.
+
+   - ``W=1``
+
+     Enables warnings we want the generic build to include but are too time
+     consuming to fix at the moment. It re-enables warnings taken out for
+     ``W=0`` builds (a few of the ``-Wextra`` additions). This level is expected
+     to eventually be merged into ``W=0``. Some warnings are expected on some
+     builds, but new contributions should not introduce new ones.
+
+   - ``W=2`` (recommended)
+
+    Enables warnings we want the generic build to include but cannot be enabled
+    due to external libraries. This level is expected to eventually be merged
+    into ``W=0``. Lots of warnings are expected, primarily from external
+    libraries like zlib and compiler-rt, but new controbutions should not
+    introduce new ones.
+
+   - ``W=3``
+
+     Enables warnings that are informative but not necessary and generally too
+     verbose and frequently ignored. A very large number of warnings are
+     expected.
+
+   The exact set of warning flags depends on the compiler and TF-A warning
+   level, however they are all succinctly set in the top-level Makefile. Please
+   refer to the `GCC`_ or `Clang`_ documentation for more information on the
+   individual flags.
+
 -  ``WARMBOOT_ENABLE_DCACHE_EARLY`` : Boolean option to enable D-cache early on
    the CPU after warm boot. This is applicable for platforms which do not
    require interconnect programming to enable cache coherency (eg: single
@@ -1162,3 +1205,5 @@ Firmware update options
 .. _DEN0115: https://developer.arm.com/docs/den0115/latest
 .. _PSA FW update specification: https://developer.arm.com/documentation/den0118/a/
 .. _PSA DRTM specification: https://developer.arm.com/documentation/den0113/a
+.. _GCC: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
+.. _Clang: https://clang.llvm.org/docs/DiagnosticsReference.html
diff --git a/docs/process/security-hardening.rst b/docs/process/security-hardening.rst
index 507046f2e..f9618db08 100644
--- a/docs/process/security-hardening.rst
+++ b/docs/process/security-hardening.rst
@@ -131,38 +131,9 @@ Several build options can be used to check for security issues. Refer to the
   overflows.
 
 - The ``W`` build flag can be used to enable a number of compiler warning
-  options to detect potentially incorrect code.
-
-  - W=0 (default value)
-
-    The ``Wunused`` with ``Wno-unused-parameter``, ``Wdisabled-optimization``
-    and ``Wvla`` flags are enabled.
-
-    The ``Wunused-but-set-variable``, ``Wmaybe-uninitialized`` and
-    ``Wpacked-bitfield-compat`` are GCC specific flags that are also enabled.
-
-  - W=1
-
-    Adds ``Wextra``, ``Wmissing-format-attribute``, ``Wmissing-prototypes``,
-    ``Wold-style-definition`` and ``Wunused-const-variable``.
-
-  - W=2
-
-    Adds ``Waggregate-return``, ``Wcast-align``, ``Wnested-externs``,
-    ``Wshadow``, ``Wlogical-op``.
-
-  - W=3
-
-    Adds ``Wbad-function-cast``, ``Wcast-qual``, ``Wconversion``, ``Wpacked``,
-    ``Wpointer-arith``, ``Wredundant-decls`` and
-    ``Wswitch-default``.
-
-  Refer to the GCC or Clang documentation for more information on the individual
-  options: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html and
-  https://clang.llvm.org/docs/DiagnosticsReference.html.
-
-  NB: The ``Werror`` flag is enabled by default in TF-A and can be disabled by
-  setting the ``E`` build flag to 0.
+  options to detect potentially incorrect code. TF-A is tested with ``W=0`` but
+  it is recommended to develop against ``W=2`` (which will eventually become the
+  default).
 
 .. rubric:: References