Re: [PATCH] brcmfmac: Use request_firmware_direct for the clm_blob

From: Kalle Valo <kvalo@codeaurora.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>,
	Franky Lin <franky.lin@broadcom.com>,
	Hante Meuleman <hante.meuleman@broadcom.com>,
	Chi-Hsien Lin <chi-hsien.lin@cypress.com>,
	Wright Feng <wright.feng@cypress.com>,
	linux-wireless@vger.kernel.org,
	brcm80211-dev-list.pdl@broadcom.com
Subject: Re: [PATCH] brcmfmac: Use request_firmware_direct for the clm_blob
Date: Mon, 07 Jan 2019 13:52:49 +0200	[thread overview]
Message-ID: <87wongsmzy.fsf@kamboji.qca.qualcomm.com> (raw)
In-Reply-To: <20190107113401.6824-1-hdegoede@redhat.com> (Hans de Goede's message of "Mon, 7 Jan 2019 12:34:01 +0100")

Hans de Goede <hdegoede@redhat.com> writes:

> The linux-firmware brcmfmac firmware files contain an embedded table with
> per country allowed channels and strength info.
>
> These versions of the firmware are specially build for linux-firmware,
> the firmware files directly available from Broadcom / Cypress rely on
> a separate clm_blob file for this info.
>
> For some unknown reason Broadcom / Cypress refuse to provide the standard
> firmware files + clm_blob files it uses elsewhere for inclusion into
> linux-firmware, instead relying on these special builds with the clm_blob
> info embedded. This means that the linux-firmware firmware versions often
> lag behind, but I digress.
>
> The brcmfmac driver does support the separate clm_blob file and always
> tries to load this. Currently we use request_firmware for this. This means
> that on any standard install, using the standard combo of linux-kernel +
> linux-firmware, we will get a warning:
> "Direct firmware load for ... failed with error -2"
>
> On top of this, brcmfmac itself prints: "no clm_blob available (err=-2),
> device may have limited channels available" and we will get a slow
> fallback to the userspace firmware loading mechanism.
>
> This commit fixes both almost any brcmfmac device logging the warning
> (leaving the brcmfmac info message in pace), as well as the slow and
> unnecesary fallback by switching to request_firmware_direct for
> the clm_blob.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> index 0bb16bf574e3..e0a5e78ee437 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> @@ -149,7 +149,7 @@ static int brcmf_c_process_clm_blob(struct brcmf_if *ifp)
>  		return err;
>  	}
>  
> -	err = request_firmware(&clm, clm_name, bus->dev);
> +	err = request_firmware_direct(&clm, clm_name, bus->dev);

If you just want to avoid the warning shouldn't this be
firmware_request_nowarn()? On ath10k we also did the workaround using
_direct() variant but that caused problems with openwrt.

-- 
Kalle Valo