On 1/16/26 16:59, Sean Anderson wrote:
On 1/16/26 03:17, Michal Simek wrote:On 1/6/26 23:42, Sean Anderson wrote:Although the pinctrl pm requests are implemented in the PMU firmware, PM_QUERY_DATA is actually implemented in ATF. In SPL (or when running in EL3), ATF is not yet running, so we need to implement this API ourselves. Do the bare minimum, allowing SPL to enumerate functions, but don't bother with groups. Groups take up a lot of space, and can be emulated with pins. For example, a node like display-port { mux { groups = "dpaux0_1"; function = "dpaux0"; }; }; can be replaced by display-port { mux { pins = "MIO34", "MIO35", "MIO36", "MIO37"; function = "dpaux0"; }; }; While this isn't backwards-compatible with existing devicetrees, it's more than enough for SPL where we may only need to mux one or two pins. Signed-off-by: Sean Anderson <[email protected]> --- drivers/firmware/firmware-zynqmp.c | 100 +++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index f8a9945c1da..77911757177 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -427,6 +427,102 @@ U_BOOT_DRIVER(zynqmp_power) = { }; #endif +static const char *const pinctrl_functions[] = { + "can0", + "can1", + "ethernet0", + "ethernet1", + "ethernet2", + "ethernet3", + "gemtsu0", + "gpio0", + "i2c0", + "i2c1", + "mdio0", + "mdio1", + "mdio2", + "mdio3", + "qspi0", + "qspi_fbclk", + "qspi_ss", + "spi0", + "spi1", + "spi0_ss", + "spi1_ss", + "sdio0", + "sdio0_pc", + "sdio0_cd", + "sdio0_wp", + "sdio1", + "sdio1_pc", + "sdio1_cd", + "sdio1_wp", + "nand0", + "nand0_ce", + "nand0_rb", + "nand0_dqs", + "ttc0_clk", + "ttc0_wav", + "ttc1_clk", + "ttc1_wav", + "ttc2_clk", + "ttc2_wav", + "ttc3_clk", + "ttc3_wav", + "uart0", + "uart1", + "usb0", + "usb1", + "swdt0_clk", + "swdt0_rst", + "swdt1_clk", + "swdt1_rst", + "pmu0", + "pcie0", + "csu0", + "dpaux0", + "pjtag0", + "trace0", + "trace0_clk", + "testscan0", +}; + +/* + * PM_QUERY_DATA is implemented by ATF and not the PMU firmware, so we have to + * emulate it in SPL. Just implement functions/pins since the groups take up a + * lot of rodata and are mostly superfluous. + */ +static int zynqmp_pm_query_data(enum pm_query_id qid, u32 arg1, u32 arg2, + u32 *ret_payload) +{ + switch (qid) { + case PM_QID_PINCTRL_GET_NUM_PINS: + ret_payload[1] = 78;Macro for this value please.Why? This value is used exactly once, and its semantics can be directly inferred from the previous line.
But it is not clear what this is referring to. I expect it is amount of MIO PINs but macro name would make it clear that it is not random number.
+ ret_payload[0] = 0; + return 0; + case PM_QID_PINCTRL_GET_NUM_FUNCTIONS: + ret_payload[1] = ARRAY_SIZE(pinctrl_functions); + ret_payload[0] = 0; + return 0; + case PM_QID_PINCTRL_GET_NUM_FUNCTION_GROUPS: + ret_payload[1] = 0; + ret_payload[0] = 0; + return 0; + case PM_QID_PINCTRL_GET_FUNCTION_NAME: + memset(ret_payload, 0, 16);Where is this 16 coming from? I expect this is max number of chars of function.Yes. It comes from ATF. I can move MAX_FUNC_NAME_LEN to include/zynqmp_firmware.h so we can use it here.+ strcpy((char *)ret_payload, pinctrl_functions[arg1]);you should check that arg1 is between 0 and array size not to read value behind.This is guaranteed by the loop condition in zynqmp_pinctrl_probe. I can add an assert, but we have one internal user so we don't really need validation beyond what would be necessary for something like
But you never know who use/call this code in future.
for (i = 0; i < ARRAY_SIZE(pinctrl_functions); i++) pinctrl_functions[i];+ return 0; + case PM_QID_PINCTRL_GET_FUNCTION_GROUPS: + case PM_QID_PINCTRL_GET_PIN_GROUPS: + memset(ret_payload + 1, 0xff, 12);Where is this 12 coming from? Macro for it.NUM_GROUPS_PER_RESP * sizeof(u16)
much better.
+ ret_payload[0] = 0; + return 0; + default: + ret_payload[0] = 1; + return 1; + } +} + smc_call_handler_t __data smc_call_handler; static int smc_call_legacy(u32 api_id, u32 arg0, u32 arg1, u32 arg2, @@ -493,6 +589,10 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, __func__, current_el(), api_id, arg0, arg1, arg2, arg3, arg4, arg5); if (IS_ENABLED(CONFIG_XPL_BUILD) || current_el() == 3) { + if (IS_ENABLED(CONFIG_ARCH_ZYNQMP) && + IS_ENABLED(CONFIG_PINCTRL_ZYNQMP) &&This logic have to change a little bit. There is also CONFIG_SPL_PINCTRL symbol which likely needs to be enabled. But if it is not still this code ends in SPL and just extend binary.PINCTRL_ZYNQMP is not enabled in any defconfig for U-Boot proper, so it will not normally be enabled in SPL either.
It is in xilinx_zynqmp_kria_defconfig Thanks, Michal
