Extend wait time in cable firmware download flow #513
Conversation
Signed-off-by: Stephen Sun <[email protected]>
|
@mihirpat1 Can you help review this PR? |
| @@ -415,6 +415,7 @@ def validate_fw_image(self): | |||
| cmd = bytearray(b'\x01\x07\x00\x00\x00\x00\x00\x00') | |||
| cmd[133-INIT_OFFSET] = self.cdb_chkcode(cmd) | |||
| self.write_cdb(cmd) | |||
| time.sleep(2) | |||
There was a problem hiding this comment.
@stephenxs instead of fixing just the firmware download is it better to fix the cdb1_chkstatus()? I see that cdb1_chkstatus() already has 60 seconds of wait time while checking CDB status. if there are i2c read errors then that will be considered as CDB is busy? So Why we need this fix here?
There was a problem hiding this comment.
@prgeor It retries in CDBCheckstatus after getting CDB status, like busy. But the I2C error can occur while reading CDB status.
We wait for an extra 2-second delay to ensure the cable can provide the status.
But we do not need to wait 2 seconds before checking every command's CDB status. It is fast for most commands, like writing EPL/RPL for downloading the firmware. The download process will be slowed down if we add wait logic there.
There was a problem hiding this comment.
@stephenxs instead of simply hardcoding the delay, can we make use of CMIS advertised timeout values for CDB command completion?
There was a problem hiding this comment.
Hi @prgeor
the motivation of this PR is to overcome I2C error messages under the current infra.
Of course, we can leverage timeout values that indicate how long the cable can provide CDB status at maximum, and then wait for cables for the time. However, it requires many more logic to handle, like,
- checking the capability of foreground/background processing
- tolerating the possibility that it is not supported
- reading those values in advance, storing them in the database or inside the platform API,
- representing the values in the CLI
- etc
It is beyond the scope of this PR but a new feature.
There was a problem hiding this comment.
@stephenxs need cleaner solution which works for all CMIS compliant vendors
Description
Extend the wait time in the cable firmware download flow
Motivation and Context
The cable firmware download flow is the following:
For each CMIS CDB command, it writes the command and then waits for the cable's response. Sometimes it takes several seconds for the cable to handle the command and provide the response. In that scenario, we must delay a few seconds to avoid error messages.
There has been a 2-second delay in step 1 but it's not sufficient on some platforms with certain cables.
We extend the delay to 5 seconds in step 1 and introduce a 2-second delay in step 3 to secure a successful cable firmware download without any error.
How Has This Been Tested?
Additional Information (Optional)