Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WDT initialization code #300

Open
vamoirid opened this issue Jun 13, 2024 · 1 comment
Open

WDT initialization code #300

vamoirid opened this issue Jun 13, 2024 · 1 comment

Comments

@vamoirid
Copy link

So, I would like to raise some comments on the FSBL code and especially the following parts:

  1. XFsbl_BoardInit() - https://github.com/Xilinx/embeddedsw/blob/master/lib/sw_apps/zynqmp_fsbl/src/xfsbl_initialization.c#L393
  2. XFsbl_ResetValidation() - https://github.com/Xilinx/embeddedsw/blob/master/lib/sw_apps/zynqmp_fsbl/src/xfsbl_initialization.c#L401
  3. XFsbl_InitWdt() - https://github.com/Xilinx/embeddedsw/blob/master/lib/sw_apps/zynqmp_fsbl/src/xfsbl_initialization.c#L888

What is currently implemented in the FSBL is that first, the XFsbl_BoardInit() function is being called where users can write their own initialization code for their specific board. Then after this function is complete, the XFsbl_ResetValidation() is being called in order to validate the reset reason and trigger specific actions in case of error (i.e. WDT timeout). Then, the WDT is being enabled in the XFsbl_PrimaryBootDeviceInit() function. To me, the order of execution of those functions looks problematic. Let me explain.

  1. First of all, I would expect the Initialization of WDT to take place ASAP. What if one of the external devices in the XFsbl_BoardInit() hangs the code? The WDT wouldn't be enabled yet and so this fallback wouldn't be implemented. Of course, I can accept an answer like "correct code should always understand errors and fallback" but in general, I think that the Initialization of a safety mechanism like the WDT should definitely take place before we start initializing devices, especially ones that are outside of the chip like the code for the Si570.

  2. The ResetValidation() should be something that takes place definitely before the XFsbl_BoardInit() function. The XFsbl_BoardInit() function is a function that includes in the very well written and tested FSBL code, some extra functionality that usually has to do with external devices. If there was an error with an external device, why to try again to run XFsbl_BoardInit() (after the board has resetted) and not check initially the Reset Reason in XFsbl_ResetValidation() and then decide if the FSBL should continue or fallback?

So to sum up, my proposal is to move the WDT initialization code ASAP in the execution order of FSBL and definitely before the XFsbl_BoardInit() function and to move as well the XFsbl_ResetValidation() before the XFsbl_BoardInit(). With those changes, in case of any error or bad code in XFsbl_BoardInit() function, the user will always be safe and rebooted by the WDT.

Please let me know what you think.

Cheers,
Vasilis

@SaratCS
Copy link

SaratCS commented Sep 2, 2024

@vamoirid , thanks for your input. We'll evaluate and address as required in upcoming release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants