[PATCH 0/2] Enable reset_cpu() in SPL for ZynqMP

From: Lukas Funke lukas.funke@weidmueller.com
This series enables the CPU reset in the SPL for ZynqMP based platforms. This only works if CONFIG_SYSRESET is disabled. This is usually the case since the the regular sysreset requires bl31 firmware to be loaded in order to hand the sysreset over to PMU firmware. In SPL we can talk to the PMU firmware directly and request a CPU reset.
The series also introduces SPL_ZYNQMP_FIRMWARE Kconfig symbol in order to make 'CONFIG_IS_ENABLED()' work in SPL.
Lukas Funke (2): arm64: zynqmp: Add 'SPL_ZYNQMP_FIRMWARE' to Kconfig xilinx: zynqmp: Enable reset_cpu() in SPL
arch/arm/mach-zynqmp/Kconfig | 2 +- arch/arm/mach-zynqmp/aes.c | 2 ++ arch/arm/mach-zynqmp/cpu.c | 4 ++-- board/xilinx/zynqmp/zynqmp.c | 13 +++++++++++-- drivers/firmware/Kconfig | 5 +++++ drivers/mmc/zynq_sdhci.c | 4 ++-- drivers/pinctrl/Kconfig | 2 +- 7 files changed, 24 insertions(+), 8 deletions(-)

From: Lukas Funke lukas.funke@weidmueller.com
In order to make CONFIG_IS_ENABLED() work with 'ZYNQMP_FIRMWARE' introduce an additional Kconfig 'SPL_ZYNQMP_FIRMWARE' which is selected if and only if ZYNQMP_FIRMWARE is enabled. Driver are adapted such that they build with and without the config being set.
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com ---
arch/arm/mach-zynqmp/Kconfig | 2 +- arch/arm/mach-zynqmp/aes.c | 2 ++ arch/arm/mach-zynqmp/cpu.c | 4 ++-- board/xilinx/zynqmp/zynqmp.c | 4 ++-- drivers/firmware/Kconfig | 5 +++++ drivers/mmc/zynq_sdhci.c | 4 ++-- drivers/pinctrl/Kconfig | 2 +- 7 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig index 0d2238ace1e..304dce45439 100644 --- a/arch/arm/mach-zynqmp/Kconfig +++ b/arch/arm/mach-zynqmp/Kconfig @@ -36,7 +36,7 @@ config PMUFW_INIT_FILE
config ZYNQMP_SPL_PM_CFG_OBJ_FILE string "PMU firmware configuration object to load at runtime by SPL" - depends on SPL + depends on SPL && ZYNQMP_FIRMWARE help Path to a binary PMU firmware configuration object to be linked into U-Boot SPL and loaded at runtime into the PMU firmware. diff --git a/arch/arm/mach-zynqmp/aes.c b/arch/arm/mach-zynqmp/aes.c index 8a2b7fdcbe9..100dde2f372 100644 --- a/arch/arm/mach-zynqmp/aes.c +++ b/arch/arm/mach-zynqmp/aes.c @@ -17,6 +17,7 @@
int zynqmp_aes_operation(struct zynqmp_aes *aes) { +#if CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) u32 ret_payload[PAYLOAD_ARG_CNT]; int ret;
@@ -54,6 +55,7 @@ int zynqmp_aes_operation(struct zynqmp_aes *aes) ret, ret_payload[1]); return -EIO; } +#endif
return 0; } diff --git a/arch/arm/mach-zynqmp/cpu.c b/arch/arm/mach-zynqmp/cpu.c index 6ae27894ecd..3515be257a5 100644 --- a/arch/arm/mach-zynqmp/cpu.c +++ b/arch/arm/mach-zynqmp/cpu.c @@ -188,7 +188,7 @@ int zynqmp_mmio_write(const u32 address, { if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) return zynqmp_mmio_rawwrite(address, mask, value); -#if defined(CONFIG_ZYNQMP_FIRMWARE) +#if CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) else return xilinx_pm_request(PM_MMIO_WRITE, address, mask, value, 0, NULL); @@ -207,7 +207,7 @@ int zynqmp_mmio_read(const u32 address, u32 *value) if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) { ret = zynqmp_mmio_rawread(address, value); } -#if defined(CONFIG_ZYNQMP_FIRMWARE) +#if CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) else { u32 ret_payload[PAYLOAD_ARG_CNT];
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index f370fb7347a..95a134b972d 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -147,14 +147,14 @@ int board_init(void) int ret; #endif
-#if defined(CONFIG_SPL_BUILD) +#if defined(CONFIG_SPL_BUILD) && CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) /* Check *at build time* if the filename is an non-empty string */ if (sizeof(CONFIG_ZYNQMP_SPL_PM_CFG_OBJ_FILE) > 1) zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj, zynqmp_pm_cfg_obj_size); #endif
-#if defined(CONFIG_ZYNQMP_FIRMWARE) +#if CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) struct udevice *dev;
uclass_get_device_by_name(UCLASS_FIRMWARE, "power-management", &dev); diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 8789b1ea141..ae785d55d54 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -30,6 +30,7 @@ config TI_SCI_PROTOCOL config ZYNQMP_FIRMWARE bool "ZynqMP Firmware interface" select FIRMWARE + select SPL_ZYNQMP_FIRMWARE if SPL help Firmware interface driver is used by different drivers to communicate with the firmware for @@ -37,6 +38,10 @@ config ZYNQMP_FIRMWARE Say yes to enable ZynqMP firmware interface driver. If in doubt, say N.
+config SPL_ZYNQMP_FIRMWARE + bool "Enable ZynqMP Firmware interface in SPL" + depends on FIRMWARE && SPL + config ARM_SMCCC_FEATURES bool "Arm SMCCC features discovery" depends on ARM_PSCI_FW diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 935540d1719..3e4cae60c17 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -992,7 +992,7 @@ static const struct sdhci_ops arasan_ops = { }; #endif
-#if defined(CONFIG_ARCH_ZYNQMP) && defined(CONFIG_ZYNQMP_FIRMWARE) +#if defined(CONFIG_ARCH_ZYNQMP) && CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) static int sdhci_zynqmp_set_dynamic_config(struct arasan_sdhci_priv *priv, struct udevice *dev) { @@ -1094,7 +1094,7 @@ static int arasan_sdhci_probe(struct udevice *dev)
host = priv->host;
-#if defined(CONFIG_ARCH_ZYNQMP) && defined(CONFIG_ZYNQMP_FIRMWARE) +#if defined(CONFIG_ARCH_ZYNQMP) && CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) if (device_is_compatible(dev, "xlnx,zynqmp-8.9a")) { ret = zynqmp_pm_is_function_supported(PM_IOCTL, IOCTL_SET_SD_CONFIG); diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index a1d53cfbdbe..d2ccfb724af 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -335,7 +335,7 @@ config PINCTRL_K210
config PINCTRL_ZYNQMP bool "Xilinx ZynqMP pin control driver" - depends on DM && PINCTRL_GENERIC && ARCH_ZYNQMP + depends on DM && PINCTRL_GENERIC && ARCH_ZYNQMP && ZYNQMP_FIRMWARE default y help Support pin multiplexing control on Xilinx ZynqMP. The driver uses

On 6/3/24 15:34, lukas.funke-oss@weidmueller.com wrote:
From: Lukas Funke lukas.funke@weidmueller.com
In order to make CONFIG_IS_ENABLED() work with 'ZYNQMP_FIRMWARE' introduce an additional Kconfig 'SPL_ZYNQMP_FIRMWARE' which is selected if and only if ZYNQMP_FIRMWARE is enabled. Driver are adapted such that they build with and without the config being set.
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com
arch/arm/mach-zynqmp/Kconfig | 2 +- arch/arm/mach-zynqmp/aes.c | 2 ++ arch/arm/mach-zynqmp/cpu.c | 4 ++-- board/xilinx/zynqmp/zynqmp.c | 4 ++-- drivers/firmware/Kconfig | 5 +++++ drivers/mmc/zynq_sdhci.c | 4 ++-- drivers/pinctrl/Kconfig | 2 +- 7 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig index 0d2238ace1e..304dce45439 100644 --- a/arch/arm/mach-zynqmp/Kconfig +++ b/arch/arm/mach-zynqmp/Kconfig @@ -36,7 +36,7 @@ config PMUFW_INIT_FILE
config ZYNQMP_SPL_PM_CFG_OBJ_FILE string "PMU firmware configuration object to load at runtime by SPL"
- depends on SPL
- depends on SPL && ZYNQMP_FIRMWARE
Isn't this SPL_ZYNQMP_FIRMWARE?
help Path to a binary PMU firmware configuration object to be linked into U-Boot SPL and loaded at runtime into the PMU firmware. diff --git a/arch/arm/mach-zynqmp/aes.c b/arch/arm/mach-zynqmp/aes.c index 8a2b7fdcbe9..100dde2f372 100644 --- a/arch/arm/mach-zynqmp/aes.c +++ b/arch/arm/mach-zynqmp/aes.c @@ -17,6 +17,7 @@
int zynqmp_aes_operation(struct zynqmp_aes *aes) { +#if CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) u32 ret_payload[PAYLOAD_ARG_CNT]; int ret;
@@ -54,6 +55,7 @@ int zynqmp_aes_operation(struct zynqmp_aes *aes) ret, ret_payload[1]); return -EIO; } +#endif
return 0;
Very likely you want to return error if functionality is not present.
} diff --git a/arch/arm/mach-zynqmp/cpu.c b/arch/arm/mach-zynqmp/cpu.c index 6ae27894ecd..3515be257a5 100644 --- a/arch/arm/mach-zynqmp/cpu.c +++ b/arch/arm/mach-zynqmp/cpu.c @@ -188,7 +188,7 @@ int zynqmp_mmio_write(const u32 address, { if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) return zynqmp_mmio_rawwrite(address, mask, value); -#if defined(CONFIG_ZYNQMP_FIRMWARE) +#if CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) else return xilinx_pm_request(PM_MMIO_WRITE, address, mask, value, 0, NULL); @@ -207,7 +207,7 @@ int zynqmp_mmio_read(const u32 address, u32 *value) if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) { ret = zynqmp_mmio_rawread(address, value); } -#if defined(CONFIG_ZYNQMP_FIRMWARE) +#if CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) else { u32 ret_payload[PAYLOAD_ARG_CNT];
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index f370fb7347a..95a134b972d 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -147,14 +147,14 @@ int board_init(void) int ret; #endif
-#if defined(CONFIG_SPL_BUILD) +#if defined(CONFIG_SPL_BUILD) && CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) /* Check *at build time* if the filename is an non-empty string */ if (sizeof(CONFIG_ZYNQMP_SPL_PM_CFG_OBJ_FILE) > 1) zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj, zynqmp_pm_cfg_obj_size); #endif
-#if defined(CONFIG_ZYNQMP_FIRMWARE) +#if CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) struct udevice *dev;
uclass_get_device_by_name(UCLASS_FIRMWARE, "power-management", &dev); diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 8789b1ea141..ae785d55d54 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -30,6 +30,7 @@ config TI_SCI_PROTOCOL config ZYNQMP_FIRMWARE bool "ZynqMP Firmware interface" select FIRMWARE
- select SPL_ZYNQMP_FIRMWARE if SPL
I don't think this is correct. Why you should connect SPL symbol with non SPL one? I would just remove line above that I can enable/disable firmware in SPL or U-Boot separately.
help Firmware interface driver is used by different drivers to communicate with the firmware for @@ -37,6 +38,10 @@ config ZYNQMP_FIRMWARE Say yes to enable ZynqMP firmware interface driver. If in doubt, say N.
+config SPL_ZYNQMP_FIRMWARE
- bool "Enable ZynqMP Firmware interface in SPL"
Can we remove "Enable" at the beginning? Or add Enable it to ZYNQMP_FIRMWARE.
- depends on FIRMWARE && SPL
- config ARM_SMCCC_FEATURES bool "Arm SMCCC features discovery" depends on ARM_PSCI_FW
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 935540d1719..3e4cae60c17 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -992,7 +992,7 @@ static const struct sdhci_ops arasan_ops = { }; #endif
-#if defined(CONFIG_ARCH_ZYNQMP) && defined(CONFIG_ZYNQMP_FIRMWARE) +#if defined(CONFIG_ARCH_ZYNQMP) && CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) static int sdhci_zynqmp_set_dynamic_config(struct arasan_sdhci_priv *priv, struct udevice *dev) { @@ -1094,7 +1094,7 @@ static int arasan_sdhci_probe(struct udevice *dev)
host = priv->host;
-#if defined(CONFIG_ARCH_ZYNQMP) && defined(CONFIG_ZYNQMP_FIRMWARE) +#if defined(CONFIG_ARCH_ZYNQMP) && CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE) if (device_is_compatible(dev, "xlnx,zynqmp-8.9a")) { ret = zynqmp_pm_is_function_supported(PM_IOCTL, IOCTL_SET_SD_CONFIG); diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index a1d53cfbdbe..d2ccfb724af 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -335,7 +335,7 @@ config PINCTRL_K210
config PINCTRL_ZYNQMP bool "Xilinx ZynqMP pin control driver"
- depends on DM && PINCTRL_GENERIC && ARCH_ZYNQMP
- depends on DM && PINCTRL_GENERIC && ARCH_ZYNQMP && ZYNQMP_FIRMWARE default y help Support pin multiplexing control on Xilinx ZynqMP. The driver uses
M

From: Lukas Funke lukas.funke@weidmueller.com
This commit enables SPL to reset the CPU via PMU-firmware. The usual reset mechanism requires bl31 to be loaded which may not be the case in SPL.
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com ---
board/xilinx/zynqmp/zynqmp.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 95a134b972d..99f5c178c1d 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -40,6 +40,7 @@ #include <linux/bitops.h> #include <linux/delay.h> #include <linux/sizes.h> +#include <dt-bindings/reset/xlnx-zynqmp-resets.h> #include "../common/board.h"
#include "pm_cfg_obj.h" @@ -285,6 +286,14 @@ int dram_init(void) #if !CONFIG_IS_ENABLED(SYSRESET) void reset_cpu(void) { + if (!CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE)) { + log_warning("reset failed: ZYNQMP_FIRMWARE disabled"); + return; + } + + xilinx_pm_request(PM_RESET_ASSERT, + ZYNQMP_PM_RESET_START + ZYNQMP_RESET_SOFT, + PM_RESET_ACTION_ASSERT, 0, 0, NULL); } #endif

On 6/3/24 15:34, lukas.funke-oss@weidmueller.com wrote:
From: Lukas Funke lukas.funke@weidmueller.com
This commit enables SPL to reset the CPU via PMU-firmware. The usual reset mechanism requires bl31 to be loaded which may not be the case in SPL.
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com
board/xilinx/zynqmp/zynqmp.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 95a134b972d..99f5c178c1d 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -40,6 +40,7 @@ #include <linux/bitops.h> #include <linux/delay.h> #include <linux/sizes.h> +#include <dt-bindings/reset/xlnx-zynqmp-resets.h> #include "../common/board.h"
#include "pm_cfg_obj.h" @@ -285,6 +286,14 @@ int dram_init(void) #if !CONFIG_IS_ENABLED(SYSRESET) void reset_cpu(void) {
- if (!CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE)) {
log_warning("reset failed: ZYNQMP_FIRMWARE disabled");
return;
- }
- xilinx_pm_request(PM_RESET_ASSERT,
ZYNQMP_PM_RESET_START + ZYNQMP_RESET_SOFT,
PM_RESET_ACTION_ASSERT, 0, 0, NULL);
If you disable ZYNQMP_FIRMWARE xilinx_pm_request() should fail.
we should likely fix it in drivers/mmc/zynq_sdhci.c:114
Thanks, Michal

On 03.06.2024 16:32, Michal Simek wrote:
On 6/3/24 15:34, lukas.funke-oss@weidmueller.com wrote:
From: Lukas Funke lukas.funke@weidmueller.com
This commit enables SPL to reset the CPU via PMU-firmware. The usual reset mechanism requires bl31 to be loaded which may not be the case in SPL.
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com
board/xilinx/zynqmp/zynqmp.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 95a134b972d..99f5c178c1d 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -40,6 +40,7 @@ #include <linux/bitops.h> #include <linux/delay.h> #include <linux/sizes.h> +#include <dt-bindings/reset/xlnx-zynqmp-resets.h> #include "../common/board.h" #include "pm_cfg_obj.h" @@ -285,6 +286,14 @@ int dram_init(void) #if !CONFIG_IS_ENABLED(SYSRESET) void reset_cpu(void) { + if (!CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE)) { + log_warning("reset failed: ZYNQMP_FIRMWARE disabled"); + return; + }
+ xilinx_pm_request(PM_RESET_ASSERT, + ZYNQMP_PM_RESET_START + ZYNQMP_RESET_SOFT, + PM_RESET_ACTION_ASSERT, 0, 0, NULL);
If you disable ZYNQMP_FIRMWARE xilinx_pm_request() should fail.
we should likely fix it in drivers/mmc/zynq_sdhci.c:114
That's an odd place for a default implementation. I'd like to move the functions to a common location but I've no idea where. That's probably why the last developer moved it here. Any suggestions?
Thanks, Michal

On 6/3/24 16:50, Lukas Funke wrote:
On 03.06.2024 16:32, Michal Simek wrote:
On 6/3/24 15:34, lukas.funke-oss@weidmueller.com wrote:
From: Lukas Funke lukas.funke@weidmueller.com
This commit enables SPL to reset the CPU via PMU-firmware. The usual reset mechanism requires bl31 to be loaded which may not be the case in SPL.
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com
board/xilinx/zynqmp/zynqmp.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 95a134b972d..99f5c178c1d 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -40,6 +40,7 @@ #include <linux/bitops.h> #include <linux/delay.h> #include <linux/sizes.h> +#include <dt-bindings/reset/xlnx-zynqmp-resets.h> #include "../common/board.h" #include "pm_cfg_obj.h" @@ -285,6 +286,14 @@ int dram_init(void) #if !CONFIG_IS_ENABLED(SYSRESET) void reset_cpu(void) { + if (!CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE)) { + log_warning("reset failed: ZYNQMP_FIRMWARE disabled"); + return; + }
+ xilinx_pm_request(PM_RESET_ASSERT, + ZYNQMP_PM_RESET_START + ZYNQMP_RESET_SOFT, + PM_RESET_ACTION_ASSERT, 0, 0, NULL);
If you disable ZYNQMP_FIRMWARE xilinx_pm_request() should fail.
we should likely fix it in drivers/mmc/zynq_sdhci.c:114
That's an odd place for a default implementation. I'd like to move the functions to a common location but I've no idea where. That's probably why the last developer moved it here. Any suggestions?
static inline to include/zynqmp_firmware.h?
M

On 03.06.2024 17:08, Michal Simek wrote:
On 6/3/24 16:50, Lukas Funke wrote:
On 03.06.2024 16:32, Michal Simek wrote:
On 6/3/24 15:34, lukas.funke-oss@weidmueller.com wrote:
From: Lukas Funke lukas.funke@weidmueller.com
This commit enables SPL to reset the CPU via PMU-firmware. The usual reset mechanism requires bl31 to be loaded which may not be the case in SPL.
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com
board/xilinx/zynqmp/zynqmp.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 95a134b972d..99f5c178c1d 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -40,6 +40,7 @@ #include <linux/bitops.h> #include <linux/delay.h> #include <linux/sizes.h> +#include <dt-bindings/reset/xlnx-zynqmp-resets.h> #include "../common/board.h" #include "pm_cfg_obj.h" @@ -285,6 +286,14 @@ int dram_init(void) #if !CONFIG_IS_ENABLED(SYSRESET) void reset_cpu(void) { + if (!CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE)) { + log_warning("reset failed: ZYNQMP_FIRMWARE disabled"); + return; + }
+ xilinx_pm_request(PM_RESET_ASSERT, + ZYNQMP_PM_RESET_START + ZYNQMP_RESET_SOFT, + PM_RESET_ACTION_ASSERT, 0, 0, NULL);
If you disable ZYNQMP_FIRMWARE xilinx_pm_request() should fail.
we should likely fix it in drivers/mmc/zynq_sdhci.c:114
That's an odd place for a default implementation. I'd like to move the functions to a common location but I've no idea where. That's probably why the last developer moved it here. Any suggestions?
static inline to include/zynqmp_firmware.h?
Sound like the right place.
Currently zynqmp is not buildable when disabling CONFIG_ZYNQMP_FIRMWARE. I'm not sure if it even makes sense to disable it. Maybe this should be enforced!?
However, I'll drop the 1/2 patch and convert 'CONFIG_IS_ENABLED' to 'IS_ENABLED' in v2 to get finish this series. Refactoring should be addressed in another series.
Best regards - Lukas
M

On 6/4/24 11:12, Lukas Funke wrote:
On 03.06.2024 17:08, Michal Simek wrote:
On 6/3/24 16:50, Lukas Funke wrote:
On 03.06.2024 16:32, Michal Simek wrote:
On 6/3/24 15:34, lukas.funke-oss@weidmueller.com wrote:
From: Lukas Funke lukas.funke@weidmueller.com
This commit enables SPL to reset the CPU via PMU-firmware. The usual reset mechanism requires bl31 to be loaded which may not be the case in SPL.
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com
board/xilinx/zynqmp/zynqmp.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 95a134b972d..99f5c178c1d 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -40,6 +40,7 @@ #include <linux/bitops.h> #include <linux/delay.h> #include <linux/sizes.h> +#include <dt-bindings/reset/xlnx-zynqmp-resets.h> #include "../common/board.h" #include "pm_cfg_obj.h" @@ -285,6 +286,14 @@ int dram_init(void) #if !CONFIG_IS_ENABLED(SYSRESET) void reset_cpu(void) { + if (!CONFIG_IS_ENABLED(ZYNQMP_FIRMWARE)) { + log_warning("reset failed: ZYNQMP_FIRMWARE disabled"); + return; + }
+ xilinx_pm_request(PM_RESET_ASSERT, + ZYNQMP_PM_RESET_START + ZYNQMP_RESET_SOFT, + PM_RESET_ACTION_ASSERT, 0, 0, NULL);
If you disable ZYNQMP_FIRMWARE xilinx_pm_request() should fail.
we should likely fix it in drivers/mmc/zynq_sdhci.c:114
That's an odd place for a default implementation. I'd like to move the functions to a common location but I've no idea where. That's probably why the last developer moved it here. Any suggestions?
static inline to include/zynqmp_firmware.h?
Sound like the right place.
Currently zynqmp is not buildable when disabling CONFIG_ZYNQMP_FIRMWARE. I'm not sure if it even makes sense to disable it. Maybe this should be enforced!?
However, I'll drop the 1/2 patch and convert 'CONFIG_IS_ENABLED' to 'IS_ENABLED' in v2 to get finish this series. Refactoring should be addressed in another series.
In past it was mandatory and then it was changed by this patch because mini u-boot configurations don't need it.
commit 71efd45a5fc70e29e391e0b57c700de8708ae6d9 Author: Michal Simek michal.simek@amd.com AuthorDate: Fri Jan 14 13:08:42 2022 +0100 Commit: Michal Simek michal.simek@amd.com CommitDate: Wed Jan 19 11:36:11 2022 +0100
arm64: zynqmp: Change firmware dependency
In case of mini U-Boot configurations there is no need to enable firmware driver which just consume space for nothing. That's why add an option to disable it.
Signed-off-by: Michal Simek michal.simek@xilinx.com Link: https://lore.kernel.org/r/d439399160ff3374f2b39f54f7dd70fa8c8bfea0.164216212...
Back to your question. Even if we skip mini u-boot cases there could be still value not to use firmware interface but it is not tested or used at this stage. But if you simply used fixed clocks it should be possible to use it.
That's why imply has been used not to force everybody to use it but also it is necessary to say that I don't want to block others to do it.
Thanks, Michal
participants (3)
-
Lukas Funke
-
lukas.funke-oss@weidmueller.com
-
Michal Simek