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

On 8/21/26 11:24, [email protected] wrote:
> On Tue, 2026-08-18 at 08:48 -0400, Nathan Whitehorn wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>> know the content is safe
>>
>> 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 two 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 (serial_number,
>>     design_id, design_ver [the version], and design_backlevel
>>     [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.
>>
>> Signed-off-by: Nathan Whitehorn <[email protected]>
>> ---
>> Changes for v2:
>> - Rename environment variables to remove the pf_ prefix
>> - Remove portions of the patch that added properties to the device
>> tree
>> - Style changes
>> - Rebase after a5f93037f28624c612288e2d97604d73e03af5a3
>>
>> Thanks to Conor Dooley and Tim Ouyang for helpful suggestions.
>>
>>   board/microchip/mpfs_generic/mpfs_generic.c | 26 ++++++++++
>>   drivers/misc/mpfs_syscontroller.c           | 54
>> +++++++++++++++++++++
>>   include/mpfs-mailbox.h                      |  2 +
>>   3 files changed, 82 insertions(+)
>>
> Hi Nathan,
>
> Happy with the patch other than Conor's styling suggestions and one NIK
> of my own seen below..
>
> Other than those:
> Acked-by: Jamie Gibbons <[email protected]>
>
> Thanks for the patch.
>> diff --git a/board/microchip/mpfs_generic/mpfs_generic.c
>> b/board/microchip/mpfs_generic/mpfs_generic.c
>> index f57f5f4046b..4423dd1e38c 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,29 @@ int board_late_init(void)
>>                  return -EINVAL;
>>          }
>>
>> +       /* Store design info and serial number in environment */
>> +       memset(designid, 0, sizeof(designid));
>> +       memset(serialstring, 0, sizeof(serialstring));
>> +
>> +       ret = mpfs_syscontroller_read_design_info(sys_serv_priv,
>> designid,
>> +                       &designver, &designbacklevel);
>> +       if (ret) {
>> +               printf("Cannot read device design information\n");
>> +               return -EINVAL;
>> +       }
>> +       for (idx = 0; idx < 16; idx++)
>> +               sprintf(&serialstring[2*idx], "%02x",
>> device_serial_number[idx]);
>> +
>> +       env_set("serial_number", serialstring);
>> +       env_set("design_id", designid);
>> +       env_set_ulong("design_ver", designver);
>> +       env_set_ulong("design_backlevel", 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;
>> diff --git a/drivers/misc/mpfs_syscontroller.c
>> b/drivers/misc/mpfs_syscontroller.c
>> index b9aea7e1181..07a788db76e 100644
>> --- a/drivers/misc/mpfs_syscontroller.c
>> +++ b/drivers/misc/mpfs_syscontroller.c
>> @@ -152,6 +152,60 @@ int mpfs_syscontroller_read_sernum(struct
>> mpfs_sys_serv *sys_serv_priv, u8 *devi
>>   }
>>   EXPORT_SYMBOL(mpfs_syscontroller_read_sernum);
>>
>> +/**
>> + * mpfs_syscontroller_read_design_info() - Use system service to
>> read FPGA design info
>> + * @sys_serv_priv:     system service private data
>> + * @designid:          30-byte string name of top module in FPGA
>> logic design
>> + *                     NB: null termination is not guaranteed
>> + * @designver:         version number of the FPGA design
>> + * @backlevel:         programmed minimum allowed FPGA design
>> version
>> + *
>> + * 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,
> Could you put this as a define at the top of the file with the others
> instead of having it hardcoded here please.


Thank you! That's a good point and is fixed in the v3 I am about to send.

On this one, I will also note that the existing serial number service 
has as its opcode "CMD_OPCODE", which seems uninformative. It might be a 
good idea to rename it in the future. I haven't touched it for now.
-Nathan

>
> Thanks,
> Jamie.
>> +               .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 = mpfs_syscontroller_recv_response(sys_serv_priv-
>>> sys_controller, &msg, timeoutsecs);
>> +       if (ret)
>> +               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;
>>

-- 
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