[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

Thu Apr 4 06:49:28 UTC 2019

On 04. 04. 19 7:38, Mike Looijmans wrote:
> On 03-04-19 23:18, Luca Ceresoli wrote:
>> Hi Mike,
>>
>> On 03/04/19 13:24, Mike Looijmans wrote:
>>> On 29-03-19 13:22, Luca Ceresoli wrote:
>>>> Hi Michal,
>>>>
>>>> thanks for the feedback.
>>>>
>>>> On 27/03/19 16:03, Michal Simek wrote:
>>>>> On 21. 03. 19 16:48, Luca Ceresoli wrote:
>>>>>> Optionally allow U-Boot to load at the PMU firmware configuration object
>>>>>> into the Power Management Unit (PMU) on Xilinx ZynqMP.
>>>>>>
>>>>>> The configuration object is required by the PMU FW to enable most SoC
>>>>>> peripherals. So far the only way to boot using U-Boot SPL was to hard-code
>>>>>> the configuration object in the PMU firmware. Allow a different boot
>>>>>> process, where the PMU FW is equal for any ZynqMP chip and its
>>>>>> configuration is passed at runtime by U-Boot SPL.
>>>>>>
>>>>>> All the code for Inter-processor communication with the PMU is isolated in
>>>>>> a new file (pmu_ipc.c). The code is inspired by the same feature as
>>>>>> implemented in the Xilinx First Stage Bootloader (FSBL) and Arm Trusted
>>>>>> Firmware:
>>>>>>
>>>>>>    * https://github.com/Xilinx/embeddedsw/blob/fb647e6b4c00f5154eba52a88b948195b6f1dc2b/lib/sw_apps/zynqmp_fsbl/src/xfsbl_misc_drivers.c#L295
>>>>>>    * https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/plat/xilinx/zynqmp/pm_service/pm_api_sys.c#L357
>>>>>>
>>>>>> The load is logged on the console during boot:
>>>>>>
>>>>>>     U-Boot SPL 2018.01 (Mar 20 2019 - 08:12:21)
>>>>>>     Loading PMUFW cfg obj (2008 bytes)
>>>>>>     EL Level:	EL3
>>>>>>     ...
>>>>>>
>>>>>> Signed-off-by: Luca Ceresoli <luca at lucaceresoli.net>
>>>> [...]
>>>>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.c b/board/xilinx/zynqmp/pmu_ipc.c
>>>>>> new file mode 100644
>>>>>> index 000000000000..6306d33d6f17
>>>>>> --- /dev/null
>>>>>> +++ b/board/xilinx/zynqmp/pmu_ipc.c
>>>>>> @@ -0,0 +1,116 @@
>>>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>>>> +/*
>>>>>> + * Inter-Processor Communication with the Platform Management Unit (PMU)
>>>>>> + * firmware.
>>>> [...]
>>>>>> +static int pmu_ipc_request(const u32 *req, size_t req_len,
>>>>>> +			   u32 *res, size_t res_maxlen)
>>>>>> +{
>>>>>> +	u32 status;
>>>>>> +
>>>>>> +	if (req_len > PMUFW_PAYLOAD_ARG_CNT ||
>>>>>> +	    res_maxlen > PMUFW_PAYLOAD_ARG_CNT)
>>>>>> +		return -EINVAL;
>>>>>> +
>>>>>> +	pmu_ipc_send_request(req, req_len);
>>>>>> +
>>>>>> +	/* Raise Inter-Processor Interrupt to PMU and wait for response */
>>>>>> +	writel(IPI_BIT_MASK_PMU0, IPI_REG_BASE_APU + IPI_REG_OFFSET_TRIG);
>>>>>> +	do {
>>>>>> +		status = readl(IPI_REG_BASE_APU + IPI_REG_OFFSET_OBR);
>>>>>> +	} while (status & IPI_BIT_MASK_PMU0);
>>>>>> +
>>>>>> +	pmu_ipc_read_response(res, res_maxlen);
>>>>>> +
>>>>>> +	return 0;
>>>>>> +}
>>>>>
>>>>> All above should be mailbox driver. It means this should go to
>>>>> drivers/mailbox and be split to mbox send/recv functions.
>>>>
>>>> Oh, wow, there's a mailbox uclass! I'll have a look into it.
>>>>
>>>>> But I have no problem to use this configuration in the first patch and
>>>>> move to mbox driver in separate patch.
>>>>
>>>> Good to know, I'll use this option in case it takes too long to make it
>>>> a proper mailbox driver.
>>>>
>>>>>> +/**
>>>>>> + * Send a configuration object to the PMU firmware.
>>>>>> + *
>>>>>> + * @cfg_obj Pointer to the configuration object
>>>>>> + * @size    Size of @cfg_obj in bytes
>>>>>> + */
>>>>>> +void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size)
>>>>>> +{
>>>>>> +	const u32 *ocm = (u32 *)CONFIG_SPL_TEXT_BASE;
>>>>>> +	const u32 request[] = {
>>>>>> +		PMUFW_CMD_SET_CONFIGURATION,
>>>>>> +		CONFIG_SPL_TEXT_BASE
>>>>>> +	};
>>>>>> +	u32 response;
>>>>>> +	int err;
>>>>>> +
>>>>>> +	printf("Loading PMUFW cfg obj (%ld bytes)\n", size);
>>>>>> +
>>>>>> +	memcpy(ocm, cfg_obj, size);
>>>>>> +
>>>>>> +	err = pmu_ipc_request(request,  ARRAY_SIZE(request), &response, 1);
>>>>>> +	if (err)
>>>>>> +		panic("Cannot load PMUFW configuration object (%d)\n", err);
>>>>>> +	if (response != 0)
>>>>>> +		panic("PMUFW returned 0x%08x status!\n", response);
>>>>>> +}
>>>>>
>>>>> And this can stay here or go to arch/arm/mach-zynq/
>>>>
>>>> Ok, I'll move it to arch/arm/mach-zynq/pmu.c or so.
>>>>
>>>> I assume "zynq" here means the whole zynq family, including zynqmp.
>>>>
>>>>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.h b/board/xilinx/zynqmp/pmu_ipc.h
>>>>>> new file mode 100644
>>>>>> index 000000000000..37bb72c1b20a
>>>>>> --- /dev/null
>>>>>> +++ b/board/xilinx/zynqmp/pmu_ipc.h
>>>>>> @@ -0,0 +1,14 @@
>>>>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>>>>> +/*
>>>>>> + * (C) Copyright 2019 Luca Ceresoli
>>>>>> + * Luca Ceresoli <luca at lucaceresoli.net>
>>>>>> + */
>>>>>> +
>>>>>> +#ifndef __ZYNQMP_PMU_IPC_H__
>>>>>> +#define __ZYNQMP_PMU_IPC_H__
>>>>>> +
>>>>>> +#include <linux/types.h>
>>>>>> +
>>>>>> +void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
>>>>>> +
>>>>>
>>>>>
>>>>> arch/arm/mach-zynqmp/include/mach/sys_proto.h should be fine.
>>>>
>>>> Ok, but I guess you mean s/zynqmp/zynq/, as above.
>>>>
>>>>>> +#endif /* __ZYNQMP_PMU_IPC_H__ */
>>>>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>>>>> index 5e1d2116bc32..1d5e25961863 100644
>>>>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>>>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>>>>> @@ -4,6 +4,8 @@
>>>>>>     * Michal Simek <michal.simek at xilinx.com>
>>>>>>     */
>>>>>>    
>>>>>> +#include "pmu_ipc.h"
>>>>>> +
>>>>>>    #include <common.h>
>>>>>>    #include <sata.h>
>>>>>>    #include <ahci.h>
>>>>>> @@ -302,6 +304,10 @@ static char *zynqmp_get_silicon_idcode_name(void)
>>>>>>    }
>>>>>>    #endif
>>>>>>    
>>>>>> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
>>>>>> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
>>>>>> +#endif
>>>>>> +
>>>>>>    int board_early_init_f(void)
>>>>>>    {
>>>>>>    	int ret = 0;
>>>>>> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>>>>>>    
>>>>>>    int board_init(void)
>>>>>>    {
>>>>>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>>>>>> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
>>>>>> +					sizeof(XPm_ConfigObject));
>>>>>> +#endif
>>>>>
>>>>> As we discussed over IRC. I think that this should be simply bin
>>>>> firmware file compare to C built by u-boot.
>>>>
>>>> Sure. I have a working prototype that uses a binary blob. It still needs
>>>> a decent way to produce a blob and to be updated according to your review.
>>>
>>> It should be doable to write a Python script to parse the C file and create an
>>> equivalent binary (using "struct" module) which is just an array of integers
>>> in the end. That avoids the need for a microblaze C compiler...
>>
>> There's no need for a microblaze compiler. pm_cfg_obj.c is simply
>> declaring a u32 array and some #defines, any C compiler is enough.
>>
>> That said, my current solution (a trivial main.c that compiles the u32
>> array and outputs it to a binary file) is not nice at all, and it
>> requires a pm_defs.h file.
>>
>> The python script you mention looks way better from a user perspective,
>> although the parsing might be a bit fragile. I'll consider it, thanks
>> for the suggestion.
>>
>> In the past I even prototyped a python script that parses the Vivado
>> .xpr project file and produces a pm_cfg_obj.c. It avoided the need to
>> run the Xilinx XSDK just to generate pm_cfg_obj.c. It might also be
>> extended to produce a .bin directly, or a self-standing .c file that
>> doesn't need pm_defs.h, thus removing any licensing issue. But it never
>> grew complete to handle all cases. Obvious, since *I* don't even know
>> all of the cases. :)
> 
> 
> Another approach would be to simply create and include a "god mode" config 
> object that just allows access to all periferals. As far as I can see, such a 
> config object would just work on all boards. There's nothing really board 
> specific in the config object, and it's rather lame anyway to have to go and 
> compile a new bootloader just because you want to use a SPI controller...
> 
> The config object I compile into the pmu firmware is of that type anyway.

Most of that stuff should be the same for all boards. But there are some
stuff which can be just board specific.

Thanks,
Michal