On Tue, 26 Oct 2021 12:37:16 -0700 Manish Chopra wrote:
Commit 0050dcf3e848 ("bnx2x: Add FW 7.13.20.0") added a new .bin firmware file to linux-firmware.git tree. This new firmware addresses few important issues and enhancements as mentioned below -
- Support direct invalidation of FP HSI Ver per function ID, required for invalidating FP HSI Ver prior to each VF start, as there is no VF
start
- BRB hardware block parity error detection support for the driver
- Fix the FCOE underrun flow
- Fix PSOD during FCoE BFS over the NIC ports after preboot driver
This patch incorporates this new firmware 7.13.20.0 in bnx2x driver.
How is this expected to work? Your drivers seems to select a very specific FW version:
/* Check FW version */ offset = be32_to_cpu(fw_hdr->fw_version.offset); fw_ver = firmware->data + offset; if ((fw_ver[0] != BCM_5710_FW_MAJOR_VERSION) || (fw_ver[1] != BCM_5710_FW_MINOR_VERSION) || (fw_ver[2] != BCM_5710_FW_REVISION_VERSION) || (fw_ver[3] != BCM_5710_FW_ENGINEERING_VERSION)) { BNX2X_ERR("Bad FW version:%d.%d.%d.%d. Should be %d.%d.%d.%d\n", fw_ver[0], fw_ver[1], fw_ver[2], fw_ver[3], BCM_5710_FW_MAJOR_VERSION, BCM_5710_FW_MINOR_VERSION, BCM_5710_FW_REVISION_VERSION, BCM_5710_FW_ENGINEERING_VERSION); return -EINVAL; }
so this change has a dependency on user updating their /lib/firmware.
Is it really okay to break the systems for people who do not have that FW version with a stable backport?
Greg, do you have general guidance for this or is it subsystem by subsystem?
I have been pushing back on a similar change for the Marvell Prestera driver, which also loads the firmware from /lib/firmware and they are proposing to break the ABI to the firmware, and not support older version.
I don't like this. As Jakub points out, you are going to break systems which don't update the firmware and the kernel at the same time. I really would prefer you support two versions of the firmware, and detect what features it supports to runtime.
Andrew