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@linux.intel.com:
From: Richard Gong richard.gong@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