Add support of power management for stm32 SAES peripheral

[tbourgoi]

This PR adds support of power management in stm32 SAES driver.
We also properly manage the dependency between SAES and RNG.

[etienne-lms]

If SAES probe function is deferred, stm32_saes_parse_fdt() fills saes_pdata.base and the next call of stm32_saes_probe() will panic on the assertion. Before this change, stm32_saes_parse_fdt() used to first get the deferreable resources before getting SAES IOMEM base address. Please preserve this order.

[tbourgoi]

Nice catch, I think we can remove the assert.
It was to protect if we try to probe 2 instances of SAES but OP-TEE does not support several AES accelerator.
So It will fail during the register in crypto framework

[etienne-lms]

Instead of removing an assertion that serves a purpose, I suggest you modify your commit "drivers: crypto: stm32_saes: SAES depends on RNG clock":

@@ -1364,10 +1365,14 @@ static TEE_Result stm32_saes_parse_fdt(struct stm32_saes_platdata *pdata,
 	    dt_saes.reg_size == DT_INFO_INVALID_REG_SIZE)
 		return TEE_ERROR_BAD_PARAMETERS;
 
-	res = clk_dt_get_by_index(fdt, node, 0, &pdata->clk);
+	res = clk_dt_get_by_name(fdt, node, "bus", &pdata->clk);
 	if (res != TEE_SUCCESS)
 		return res;
 
+	return clk_dt_get_by_name(fdt, node, "rng", &pdata->clk_rng);
+	if (res != TEE_SUCCESS)
+		return res;
+
 	res = rstctrl_dt_get_by_index(fdt, node, 0, &pdata->reset);
 	if (res != TEE_SUCCESS && res != TEE_ERROR_ITEM_NOT_FOUND)
 		return res;

[etienne-lms]

Add a space char after # character and a . char at sentences end.
s/register/registered/
A would reword safety into consistency.

A last word, regarding this issue, can we let the DT define which of CRYP or SAES driver is to be probed, allowing both config to be enabled but panicking at OP-TEE initialization time if both are registered?
Note that build reveal an issue related to that:

core/drivers/crypto/stm32/cipher.c:96:1: error: function 'cryp_copy_state' could be declared with attribute 'noreturn' [-Werror,-Wmissing-noreturn]

This is due to the assert(IS_ENABLED(CFG_STM32_CRYP)) instruction in cryp_copy_state().
`

[tbourgoi]

I don't think it's a good idea to compile both drivers and let the DT choose.
Today the device tree in built in OP-TEE, so you know which peripheral will be enabled or not.
You are building OP-TEE for a dedicated device tree (not a list of device tree)

The user may try to enables CRYP and SAES and this will result in a crash of the plateform. The sooner the error the better I think.

SAES is DPA resistant (CRYP is not) that's why we enable it in OP-TEE for MP13.
Moreover, SAES is mandatory to use access the HUK of the SoC. Using CRYP instead of SAES would mean no HUK services in OP-TEE which reduce the feature set of OP-TEE.

[etienne-lms]

I get your point despite I think one day, we'd like to have OP-TEE's DTB provided at boot time, not built-in. I agree we can address that later.

Here i strongly suggest to report a error instead of overriding a config value (line 256), as you said, the sooner the error is reported, the better it is. Something like:

ifeq ($(call cfg-all-enabled,CFG_STM32MP13 CFG_STM32_CRYP CFG_STM32_SAES),y)
$(error Cannot embed both SAES and CRYP in STM32MP13)
endif

[etienne-lms]

I get your point despite I think one day, we'd like to have OP-TEE's DTB provided at boot time, not built-in. I agree we can address that later.

Here i strongly suggest to report a error instead of overriding a config value (line 256), as you said, the sooner the error is reported, the better it is. Something like:

ifeq ($(call cfg-all-enabled,CFG_STM32MP13 CFG_STM32_CRYP CFG_STM32_SAES),y)
$(error Cannot embed both SAES and CRYP in STM32MP13)
endif

[etienne-lms]

Instead of removing an assertion that serves a purpose, I suggest you modify your commit "drivers: crypto: stm32_saes: SAES depends on RNG clock":

@@ -1364,10 +1365,14 @@ static TEE_Result stm32_saes_parse_fdt(struct stm32_saes_platdata *pdata,
 	    dt_saes.reg_size == DT_INFO_INVALID_REG_SIZE)
 		return TEE_ERROR_BAD_PARAMETERS;
 
-	res = clk_dt_get_by_index(fdt, node, 0, &pdata->clk);
+	res = clk_dt_get_by_name(fdt, node, "bus", &pdata->clk);
 	if (res != TEE_SUCCESS)
 		return res;
 
+	return clk_dt_get_by_name(fdt, node, "rng", &pdata->clk_rng);
+	if (res != TEE_SUCCESS)
+		return res;
+
 	res = rstctrl_dt_get_by_index(fdt, node, 0, &pdata->reset);
 	if (res != TEE_SUCCESS && res != TEE_ERROR_ITEM_NOT_FOUND)
 		return res;

[etienne-lms]

nitpicking: could move the return TEE_ERROR_NOT_IMPLEMENTED instruction here. Simpler, better IMHO.

[etienne-lms]

Please see my comment at #6778 (comment).