Re: [PATCH] riscv: mpfs: Read and store FPGA design information on MPFS hardware.

On 8/18/26 05:29, Conor Dooley wrote:
> Hey Nathan,
>
> Sorry for missing this originally.
>
> On Fri, Jul 24, 2026 at 01:33:50PM -0400, Nathan Whitehorn wrote:
>> When working with systems with multiple FPGA firmwares, it is sometimes
>> useful to be able to use the same U-Boot and script conditional behavior
>> based on the FPGA firmware ID and version number. This, allows, for
>> example, flashing new FPGA images without flashing the bootloader and
>> letting U-Boot and the Linux kernel adapt appropriately.
>>
>> This does three things with the information:
>> 1. Prints the FPGA design ID and version to the console as part of U-Boot
>>     startup, which is helpful to a human working with the board.
>> 2. Stores the same information, plus the already-acquired FPGA serial
>>     number, in a new set of environment variables (pf_serial_number,
>>     pf_design_id, pf_design_ver [the version], and pf_design_backl
>>     [minimum-version backlevel]). These can be used in a U-Boot script to
>>     load different kernels, device trees, etc. depending on the currently
>>     installed firmware, which allows U-Boot and the kernel to adapt
>>     cleanly to FPGA firmware updates that change accessible peripherals
>>     etc.
> These two I don't really mind, I can see the value in it.
>
>> 3. Stores the same information in the device tree (under /soc) as
>>     fpga-design-version, fpga-design, and fpga-design-version-backlevel.
>>     These things are also available through the Linux system-controller
>>     driver directly, but it is handy to have them in the device tree as
>>     well.
> This I don't on the other hand. If linux needs it, it can go and get the
> information itself and if userspace needs it, I'd rather there was a
> small driver made to expose it on a dedicated interface rather than
> buried somewhere in the devicetree portion of sysfs. If it is just
> informational, the info has been emitted by U-Boot already.
> I'd also rather U-Boot didn't add undocumented properties to the
> devicetree.

Fair enough. It has been convenient for our use, but with the 
environment variables, we can also do this from a U-Boot script easily 
enough if we still want it. I'll remove this part for v2.

>> Signed-off-by: Nathan Whitehorn <[email protected]>
>> ---
>>   board/microchip/mpfs_generic/mpfs_generic.c | 31 ++++++++++++
>>   drivers/misc/mpfs_syscontroller.c           | 52 +++++++++++++++++++++
>>   include/mpfs-mailbox.h                      |  1 +
>>   3 files changed, 84 insertions(+)
>>
>> diff --git a/board/microchip/mpfs_generic/mpfs_generic.c b/board/microchip/mpfs_generic/mpfs_generic.c
>> index f57f5f4046b..469e914e083 100644
>> --- a/board/microchip/mpfs_generic/mpfs_generic.c
>> +++ b/board/microchip/mpfs_generic/mpfs_generic.c
>> @@ -114,7 +114,10 @@ int board_late_init(void)
>>   {
>>   	u32 ret;
>>   	int node;
>> +	int idx;
>>   	u8 device_serial_number[16] = {0};
>> +	char serialstring[33], designid[33];
>> +	u16 designver, designbacklevel;
>>   	void *blob = (void *)gd->fdt_blob;
>>   	struct udevice *dev;
>>   	struct mpfs_sys_serv *sys_serv_priv;
>> @@ -144,6 +147,24 @@ int board_late_init(void)
>>   		return -EINVAL;
>>   	}
>>   
>> +	/* Store design info and serial number in environment */
>> +	ret = mpfs_syscontroller_read_design_info(sys_serv_priv, designid, &designver, &designbacklevel);
>> +	if (ret) {
>> +		printf("Cannot read device design information\n");
>> +		return -EINVAL;
>> +	}
> blank line here please
>
>> +	designid[32] = 0;
> What is this doing?

This is meant to ensure NULL termination of the string.

>
>> +	for (idx = 0; idx < 16; idx++)
>> +		sprintf(&serialstring[2*idx], "%02x", device_serial_number[idx]);
> blank line here please.

Will do.

>
>> +	env_set("pf_serial_number", serialstring);
> Contrary to what I said yesterday, why not just make this
> "serial_number"? Not like the code is going to run on a non-polarfire.
>
>> +	env_set("pf_design_id", designid);
> Ditto here and below.

Sounds good to me.

>
>> +	env_set_ulong("pf_design_ver", designver);
>> +	env_set_ulong("pf_design_backl", designbacklevel);
>> +
>> +	printf("FPGA Design name: %s\n", designid);
>> +	printf("FPGA Serial: %s\n", serialstring);
>> +	printf("Design version number %d (backlevel %d)\n", designver, designbacklevel);
>> +
>>   	/* Update MAC address with device serial number */
>>   	mac_addr[0] = 0x00;
>>   	mac_addr[1] = 0x04;
>> @@ -182,6 +203,16 @@ int ft_board_setup(void *blob, struct bd_info *bd)
>>   	u32 ret;
>>   	int node;
>>   
>> +	node = fdt_path_offset(blob, "/soc");
> Putting in the soc node doesn't really make sense anyway, the soc node
> is really the devices on axi buses.
> If this were to go anywhere, it should be in the root node.

I will just drop this part of the patch.
-Nathan

>
> Cheers,
> Conor.
>
>> +	if (node >= 0) {
>> +		if (env_get("pf_design_ver") != NULL)
>> +			fdt_setprop(blob, node, "fpga-design-version", env_get("pf_design_ver"), strlen(env_get("pf_design_id")) + 1);
>> +		if (env_get("pf_design_backl") != NULL)
>> +			fdt_setprop(blob, node, "fpga-design-version-backlevel", env_get("pf_design_backl"), strlen(env_get("pf_design_backl")) + 1);
>> +		if (env_get("pf_design_id") != NULL)
>> +			fdt_setprop(blob, node, "fpga-design", env_get("pf_design_id"), strlen(env_get("pf_design_id")) + 1);
>> +	}
>> +
>>   	node = fdt_path_offset(blob, "/soc/ethernet@20110000");
>>   	if (node >= 0) {
>>   		ret = fdt_setprop(blob, node, "local-mac-address", mac_addr, 6);
>> diff --git a/drivers/misc/mpfs_syscontroller.c b/drivers/misc/mpfs_syscontroller.c
>> index f608d5518b0..ef03ce7b262 100644
>> --- a/drivers/misc/mpfs_syscontroller.c
>> +++ b/drivers/misc/mpfs_syscontroller.c
>> @@ -129,6 +129,58 @@ int mpfs_syscontroller_read_sernum(struct mpfs_sys_serv *sys_serv_priv, u8 *devi
>>   }
>>   EXPORT_SYMBOL(mpfs_syscontroller_read_sernum);
>>   
>> +/**
>> + * mpfs_syscontroller_read_sernum() - Use system service to read the device serial number
>> + * @sys_serv_priv:	system service private data
>> + * @device_serial_number:	device serial number
>> + *
>> + * Return: 0 if all went ok, else return appropriate error
>> + */
>> +int mpfs_syscontroller_read_design_info(struct mpfs_sys_serv *sys_serv_priv, u8 *designid, u16 *designver, u16 *backlevel)
>> +{
>> +	unsigned long timeoutsecs = 300;
>> +	u8 data[36];
>> +	int ret;
>> +
>> +	struct mpfs_mss_response response = {
>> +		.resp_status = 0U,
>> +		.resp_msg = (u32 *)data,
>> +		.resp_size = sizeof(data)};
>> +	struct mpfs_mss_msg msg = {
>> +		.cmd_opcode = 2,
>> +		.cmd_data_size = CMD_DATA_SIZE,
>> +		.response = &response,
>> +		.cmd_data = CMD_DATA,
>> +		.mbox_offset = MBOX_OFFSET,
>> +		.resp_offset = RESP_OFFSET};
>> +
>> +	ret = mpfs_syscontroller_run_service(sys_serv_priv->sys_controller, &msg);
>> +	if (ret) {
>> +		dev_err(sys_serv_priv->sys_controller->chan.dev, "Service failed: %d, abort\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Receive the response */
>> +	ret = mbox_recv(&sys_serv_priv->sys_controller->chan, &msg, timeoutsecs);
>> +	if (ret) {
>> +		dev_err(sys_serv_priv->sys_controller->chan.dev, "Service failed: %d, abort. Failure: %u\n", ret, msg.response->resp_status);
>> +		return ret;
>> +	}
>> +
>> +	debug("%s: Read successful %s\n",
>> +	      __func__, sys_serv_priv->sys_controller->chan.dev->name);
>> +
>> +	if (designid != NULL)
>> +		memcpy(designid, &data[2], 30);
>> +	if (designver != NULL)
>> +		memcpy(designver, &data[32], 2);
>> +	if (backlevel != NULL)
>> +		memcpy(backlevel, &data[34], 2);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(mpfs_syscontroller_read_design_info);
>> +
>>   static u16 mpfs_syscontroller_service_spi_copy(struct mpfs_sys_serv *sys_serv_priv, u64 dst_addr, u32 src_addr, u32 length)
>>   {
>>   	int ret;
>> diff --git a/include/mpfs-mailbox.h b/include/mpfs-mailbox.h
>> index c0ff327a4ce..5c1ade3e918 100644
>> --- a/include/mpfs-mailbox.h
>> +++ b/include/mpfs-mailbox.h
>> @@ -59,6 +59,7 @@ struct mpfs_sys_serv {
>>   
>>   int mpfs_syscontroller_run_service(struct mpfs_syscontroller_priv *sys_controller, struct mpfs_mss_msg *msg);
>>   int mpfs_syscontroller_read_sernum(struct mpfs_sys_serv *sys_serv_priv, u8 *device_serial_number);
>> +int mpfs_syscontroller_read_design_info(struct mpfs_sys_serv *sys_serv_priv, u8 *designid, u16 *designver, u16 *backlevel);
>>   void mpfs_syscontroller_process_dtbo(struct mpfs_sys_serv *sys_serv_priv);
>>   struct mpfs_syscontroller_priv *mpfs_syscontroller_get(struct udevice *dev);
>>   
>> -- 
>> 2.34.1
>>

-- 
Nathan Whitehorn (he/him)
Associate Professor
Department of Physics and Astronomy
Michigan State University
Biomedical and Physical Sciences 3225
East Lansing, MI 48824
(517) 884-5563