From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx1.molgen.mpg.de (mx3.molgen.mpg.de [141.14.17.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CA8BC33D5 for ; Fri, 11 Mar 2022 13:00:30 +0000 (UTC) Received: from [192.168.0.7] (ip5f5ae8da.dynamic.kabel-deutschland.de [95.90.232.218]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: pmenzel) by mx.molgen.mpg.de (Postfix) with ESMTPSA id 7C54061EA1927; Fri, 11 Mar 2022 14:00:21 +0100 (CET) Message-ID: Date: Fri, 11 Mar 2022 14:00:20 +0100 Precedence: bulk X-Mailing-List: regressions@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.2 Subject: Re: [EXT] Re: [PATCH v2 net-next 1/2] bnx2x: Utilize firmware 7.13.21.0 Content-Language: en-US To: Manish Chopra Cc: Linus Torvalds , Jakub Kicinski , "netdev@vger.kernel.org" , Ariel Elior , Alok Prasad , Prabhakar Kushwaha , "David S. Miller" , Greg KH , "stable@vger.kernel.org" , "it+netdev@molgen.mpg.de" , "regressions@lists.linux.dev" References: <20211217165552.746-1-manishc@marvell.com> From: Paul Menzel In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 >> Sent: Thursday, March 10, 2022 3:48 AM […] >> On Wed, Mar 9, 2022 at 11:46 AM Manish Chopra >> 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