[U-Boot] [PATCHv2 0/4] support remote system update on Intel Stratix10 SoC

[Richard Gong]

Hi Simon,

Thanks for your comments!

On 11/4/19 3:03 PM, Simon Goldschmidt wrote:
> Am 30.10.2019 um 21:34 schrieb richard.gong at linux.intel.com:
>> From: Richard Gong <richard.gong at intel.com>
>>
>> This is 2nd submission of Intel Remote System Update patches.
> 
> Ok, so what has changed since v1? You'd normally add a changelog 
> (ideally to both this cover-letter and to each patch). Have a look at 
> patman in tools/patman, that allows you to create those changelogs more 
> easily.
> Sorry I missed that. I will add change details in the next submissions.
>>
>> The Intel Remote System Update (RSU) provides a way for users to update
>> the QSPI configuration bitstream of a Intel Stratix10 SoC device with
>> significantly reduced risk of corrupting the bitstream storage and
>> bricking the system.
>>
>> The patchset adds RSU support which allows user to perform a complete set
>> of RSU operations via provided console commands.
>>
>> The patches have reviewed by other colleagues at Intel.
>>
>> Richard Gong (4):
>>    arm: socfpga: stratix10: add RSU mailbox support
>>    drivers: firmware: add RSU support for Stratix10 SoC
>>    dirvers: firmware: add console commands for RSU support
>>    arm: socfpga: stratix10: add environment variables for RSU support
>>
>>   arch/arm/mach-socfpga/include/mach/mailbox_s10.h |   36 +-
>>   arch/arm/mach-socfpga/mailbox_s10.c              |   79 ++
> 
> Do you have plans to move this code to drivers/mailbox?
> 
Yes, will do after RSU upstream.

>>   arch/arm/mach-socfpga/misc_s10.c                 |    9 +
>>   drivers/firmware/Kconfig                         |   11 +
>>   drivers/firmware/Makefile                        |    1 +
>>   drivers/firmware/rsu.c                           |  662 ++++++++++++++
>>   drivers/firmware/rsu_ll_qspi.c                   | 1050 
>> ++++++++++++++++++++++
>>   drivers/firmware/rsu_misc.c                      |  817 
>> +++++++++++++++++
>>   drivers/firmware/rsu_s10.c                       |  874 
>> ++++++++++++++++++
> 
> Ok, so you started to add this "RSU" as a driver, that's probably fine 
> (sorry I did not find the time to review that, yet).
> 
> However, I think you'd need a more specific name, probably including 
> 'intel' or something to make this more clear.
> 
How about intel_rsu.c, intel_rsu_misc.c, intel_rsu_ll_qspi.c and 
intel_rsu_s10.c?

> I'll hope to find the time to review the rest this week.
> 
> Regards,
> Simon
> 
>>   include/intel/rsu.h                              |  291 ++++++
>>   include/intel/rsu_ll.h                           |   72 ++
>>   include/intel/rsu_misc.h                         |   60 ++
>>   include/intel/rsu_s10.h                          |   46 +
>>   13 files changed, 3998 insertions(+), 10 deletions(-)
>>   create mode 100644 drivers/firmware/rsu.c
>>   create mode 100644 drivers/firmware/rsu_ll_qspi.c
>>   create mode 100644 drivers/firmware/rsu_misc.c
>>   create mode 100644 drivers/firmware/rsu_s10.c
>>   create mode 100644 include/intel/rsu.h
>>   create mode 100644 include/intel/rsu_ll.h
>>   create mode 100644 include/intel/rsu_misc.h
>>   create mode 100644 include/intel/rsu_s10.h
>>
>