Re: [EXT] Re: [PATCH v2 net-next 1/2] bnx2x: Utilize firmware 7.13.21.0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Manish,


As a side note, it’d be great if you could use an email client, better supporting quoting.


Am 11.03.22 um 13:11 schrieb Manish Chopra:
-----Original Message-----
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Sent: Thursday, March 10, 2022 3:48 AM

[…]

On Wed, Mar 9, 2022 at 11:46 AM Manish Chopra <manishc@xxxxxxxxxxx>
wrote:

This has not changed anything functionally from driver/device perspective,
FW is still being loaded only when device is opened.
bnx2x_init_firmware() [I guess, perhaps the name is misleading] just
request_firmware() to prepare the metadata to be used when device will be
opened.

So how do you explain the report by Paul Menzel that things used to work and
no longer work now?

The issue which Paul mentioned had to do with "/lib/firmware/bnx2x/*
file not found" when driver probes, which was introduced by the patch
in subject, And the commit e13ad1443684 ("bnx2x: fix driver load from
initrd") fixes this issue. So things should work as it is with the
mentioned fixed commit.
No, your statement is incorrect. I already corrected it in a previous reply. The commit you mentioned was backported to 5.10.103. As we used that version, your commit was present.

The only discussion led by this problem now is why the
request_firmware() was moved early on [from open() to probe()] by the
patch in subject. I explained the intention to do this in my earlier
emails and let me add more details below -

Note that we have just moved request_firmware() logic, *not*
something significant which has to do with actual FW loading or
device initialization from the FW file data which could cause
significant functional change for this device/driver, FW load/init
part still stays in open flow.

Before the patch in subject, driver used to only work with
fixed/specific FW version file whose version was statically known to
the driver function at probe() time to take some decision to fail the
function probe early in the system if the function is supposed to run
with a FW version which is not the same version loaded on the device
by another PF (different ENV). Now when we sent this new FW patch (in
subject) then we got feedback from community to maintain backward
compatibility with older FW versions as well and we did it in same v2
patch legitimately, just that now we can work with both older or
newer FW file so we need this run time FW version information to
cache (based on request_firmware() return success value for an old FW
file or new FW file) which will be used in follow up probe() flows to
decide the function probe failure early If there could be FW version
mismatches against the loaded FW on the device by other PFs already

So we need to understand why we should not call request_firmware() in
probe or at least what's really harmful in doing that in probe() if
some of the follow up probe flows needs some of the metadata info
(like the run time FW versions info in this case which we get based
on request_firmware() return value), we could avoid this but we don't
want to add some ugly/unsuitable file APIs checks to know which FW
version file is available on the file system if there is already an
API request_firmware() available for this to be used.
Your patches broke loading the driver, and as a result – as seen from the pastes I provided – the network devices were not functional.

Please let us know. Thanks.

You can't do request_firmware() early. When you actually then push the
firmware to the device is immaterial - but request_firmware() has to be done
after the system is up and running.


Kind regards,

Paul



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux