plat-stm32mp1: remove registering to platform shared_resources driver #7169
Conversation
core/drivers/stm32_iwdg.c
Outdated
| * with status="okay" and secure-status="disabled" as IWDG assigned to | ||
| * non-secure world. | ||
| */ | ||
| if (!(fdt_get_status(fdt, node) & DT_STATUS_OK_SEC)) |
There was a problem hiding this comment.
We don't need to manage this case. The firewall bus probe is already filtering out probe of non-secure peripherals, regardless of the secure-status.
There was a problem hiding this comment.
This is to manage existing stm32mp15xx-dhcom-som.dtsi and stm32mp15xx-dhcor-som.dtsi based boards.
There was a problem hiding this comment.
Ok but I don't know if it makes sense to support this. Maybe we should patch the DH and ST device trees and ask @jneuhauser to review?
There was a problem hiding this comment.
Actually, this is not needed. IWDG driver is not probed for instances assigned to non-secure since implementation of stm32_etzpc.c driver (stm32_etzpc_dt_probe_bus()) so we don't need to check the status here. I'll simply remove this test.
core/drivers/stm32_iwdg.c
Outdated
| } else { | ||
| stm32mp_register_secure_periph_iomem(iwdg->base.pa); | ||
|
|
||
| if (IS_ENABLED(CFG_WDT_SM_HANDLER)) { |
There was a problem hiding this comment.
To be honest, we don't support watchdog if we do not register it to the framework. We wouldn't want to start a wathcdog and never be able to ping it IMO
There was a problem hiding this comment.
Indeed. So you prefer we remove this condition and change the platform conf.mk to only enable CFG_STM32_IWDG when CFG_WDT_SM_HANDLER is enabled?
There was a problem hiding this comment.
Yes or static assert? Whatever your choice
| @@ -702,9 +687,7 @@ static TEE_Result initialize_pmic(const void *fdt, int pmic_node) | |||
| stm32mp_put_pmic(); | |||
|
|
|||
| if (dt_pmic_is_secure()) | |||
There was a problem hiding this comment.
I don't fancy we rely of secure-status in dt_pmic_is_secure()
There was a problem hiding this comment.
We can address this in a dedicated P-R, it's out of the scope of this series that only intend to get rid of plat-stm32mp1/shared_resources.c.
There was a problem hiding this comment.
I've created #7172 to address that point.
|
I've addressed the current review comments and appended a minor commit to remove some deprecated useless lines in stm32_iwdg.c driver. |
core/drivers/stm32_iwdg.c
Outdated
| * non-secure world. | ||
| */ | ||
| if (!(fdt_get_status(fdt, node) & DT_STATUS_OK_SEC)) | ||
| if (!IS_ENABLED(CFG_WDT_SM_HANDLER)) { |
There was a problem hiding this comment.
I'd prefer to have a compile time restriction (static assert or rule in conf.mk) rather than a driver probing to do nothing. + we may face issue with firewall config when probing firewall bus without wanting to support this peripheral
There was a problem hiding this comment.
Ok, i'll update plat-stm32mp*/conf.mk
etienne-lms
force-pushed
the
stm32-shres
branch
3 times, most recently
from
3bfa050 to
3ac0787
Compare
|
I've updated the series (after few bad manipulations). @GseoC, may I squash the fixup commits? |
|
Yes sure, please squash |
Driver stm32_iwdg only aims at exposing an OP-TEE watchdog service hence declare CFG_STM32_IWDG dependency on CFG_WDT and CFG_WDT_SM_HANDLER in stm32mp1 platform configuration file. Signed-off-by: Etienne Carriere <[email protected]>
Remove registering of STM32 IWDG driver to platform shared_resources driver that is deprecated since integration of the firewall framework in stm32mp1 platforms. Since this integration, OP-TEE only consider IWDG secure instances hence remove the useless code for IWDG assigned to non-secure world. As watchdog drivers are only used when registering to OP-TEE watchdog services (CFG_WDT_SM_HANDLER) simplify the code to always register IWDG instance and assert CFG_WDT_SM_HANDLER is enabled when IWDG driver is embedded. Signed-off-by: Etienne Carriere <[email protected]>
STM32 watchdog driver does not manage several instances of IWDG hence remove the useless code. To simplify code, remove stm32_iwdg_register() local function. Signed-off-by: Etienne Carriere <[email protected]>
Remove registering of STM32 RNG driver to shared_resources driver that is deprecated since integration of the firewall framework and will soon be removed. Signed-off-by: Etienne Carriere <[email protected]>
Remove registering of STM32 UART driver to shared_resources driver that is deprecated since integration of the firewall framework and will soon be removed. Signed-off-by: Etienne Carriere <[email protected]>
Remove registering of STM32 CRYP driver to shared_resources driver that is deprecated since integration of the firewall framework and will soon be removed. Signed-off-by: Etienne Carriere <[email protected]>
Remove registering of STM32MP1 PMIC driver to shared_resources driver that is deprecated since integration of the firewall framework and will soon be removed. Signed-off-by: Etienne Carriere <[email protected]>
etienne-lms
force-pushed
the
stm32-shres
branch
from
3ac0787 to
0df1333
Compare
|
Fixup commits squashed and series rebased. |
Since integration of the firewall framework in STM32MP1 platform drivers (mainly P-Rs #7066, #7102, #7107, #7108 and #7111), plat-stm32mp1/shared_resources.c driver is deprecated and needs to be removed.
This series removes the remaining calls to
stm32mp_register_secure_xxx()andstm32mp_register_non_secure_xxx()platform functions from STM32 drivers. Once this P-R and P-R #7111 are both merged, we will be able to fully remove plat-stm32mp1/shared_resources.c source file.