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

[Simon Goldschmidt]

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.

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

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

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
>