From: Dmitry Osipenko <digetx@gmail.com>
To: Linus Walleij <linus.walleij@linaro.org>,
Arend van Spriel <aspriel@gmail.com>,
Franky Lin <franky.lin@broadcom.com>,
Hante Meuleman <hante.meuleman@broadcom.com>,
Chi-hsien Lin <chi-hsien.lin@infineon.com>,
Wright Feng <wright.feng@infineon.com>,
Chung-hsien Hsu <chung-hsien.hsu@infineon.com>
Cc: linux-wireless@vger.kernel.org, Kalle Valo <kvalo@codeaurora.org>,
phone-devel@vger.kernel.org, newbyte@disroot.org,
Stephan Gerhold <stephan@gerhold.net>
Subject: Re: [PATCH] brcmfmac: firmware: Allow per-board firmware binaries
Date: Tue, 3 Aug 2021 18:28:48 +0300 [thread overview]
Message-ID: <fca09928-72a8-a573-a862-7c486a2094d6@gmail.com> (raw)
In-Reply-To: <20210711231659.255479-1-linus.walleij@linaro.org>
12.07.2021 02:16, Linus Walleij пишет:
> After some crashes in the 3D engine (!) on the Samsung GT-I8530
> it turns out that the main firmware file can be device dependent,
> something that was previously only handled for the NVRAM
> parameter file.
>
> Rewrite the code a bit so we can a per-board suffixed firmware
> binary as well, if this does not exist we fall back to the
> canonical firmware name.
>
> Example: a 4330 device with the OF board compatible is
> "samsung,gavini". We will first try
> "brcmfmac4330-sdio.samsung,gavini.bin" then "brcmfmac4330-sdio.bin"
> if that does not work.
>
> Cc: phone-devel@vger.kernel.org
> Cc: newbyte@disroot.org
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> .../broadcom/brcm80211/brcmfmac/firmware.c | 53 +++++++++++++++----
> 1 file changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> index d40104b8df55..adfdfc654b10 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> @@ -594,28 +594,47 @@ static int brcmf_fw_complete_request(const struct firmware *fw,
> return (cur->flags & BRCMF_FW_REQF_OPTIONAL) ? 0 : ret;
> }
>
> +static char *brcm_alt_fw_path(const char *path, const char *board_type)
> +{
> + char alt_path[BRCMF_FW_NAME_LEN];
> + char suffix[5];
> +
> + strscpy(alt_path, path, BRCMF_FW_NAME_LEN);
> + /* At least one character + suffix */
> + if (strlen(alt_path) < 5)
> + return NULL;
> +
> + /* strip .txt or .bin at the end */
> + strscpy(suffix, alt_path + strlen(alt_path) - 4, 5);
> + alt_path[strlen(alt_path) - 4] = 0;
> + strlcat(alt_path, ".", BRCMF_FW_NAME_LEN);
> + strlcat(alt_path, board_type, BRCMF_FW_NAME_LEN);
> + strlcat(alt_path, suffix, BRCMF_FW_NAME_LEN);
> +
> + return kstrdup(alt_path, GFP_KERNEL);
> +}
> +
> static int brcmf_fw_request_firmware(const struct firmware **fw,
> struct brcmf_fw *fwctx)
> {
> struct brcmf_fw_item *cur = &fwctx->req->items[fwctx->curpos];
> int ret;
>
> - /* nvram files are board-specific, first try a board-specific path */
> + /* Files can be board-specific, first try a board-specific path */
> if (cur->type == BRCMF_FW_TYPE_NVRAM && fwctx->req->board_type) {
> - char alt_path[BRCMF_FW_NAME_LEN];
> + char *alt_path;
>
> - strlcpy(alt_path, cur->path, BRCMF_FW_NAME_LEN);
> - /* strip .txt at the end */
> - alt_path[strlen(alt_path) - 4] = 0;
> - strlcat(alt_path, ".", BRCMF_FW_NAME_LEN);
> - strlcat(alt_path, fwctx->req->board_type, BRCMF_FW_NAME_LEN);
> - strlcat(alt_path, ".txt", BRCMF_FW_NAME_LEN);
> + alt_path = brcm_alt_fw_path(cur->path, fwctx->req->board_type);
> + if (!alt_path)
> + goto fallback;
>
> ret = request_firmware(fw, alt_path, fwctx->dev);
> + kfree(alt_path);
> if (ret == 0)
> return ret;
> }
>
> +fallback:
> return request_firmware(fw, cur->path, fwctx->dev);
> }
>
> @@ -660,6 +679,7 @@ int brcmf_fw_get_firmwares(struct device *dev, struct brcmf_fw_request *req,
> {
> struct brcmf_fw_item *first = &req->items[0];
> struct brcmf_fw *fwctx;
> + char *alt_path;
> int ret;
>
> brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(dev));
> @@ -677,9 +697,20 @@ int brcmf_fw_get_firmwares(struct device *dev, struct brcmf_fw_request *req,
> fwctx->req = req;
> fwctx->done = fw_cb;
>
> - ret = request_firmware_nowait(THIS_MODULE, true, first->path,
> - fwctx->dev, GFP_KERNEL, fwctx,
> - brcmf_fw_request_done);
> + /* First try alternative board-specific path if any */
> + alt_path = brcm_alt_fw_path(first->path, fwctx->req->board_type);
> + if (alt_path) {
> + ret = request_firmware_nowait(THIS_MODULE, true, alt_path,
> + fwctx->dev, GFP_KERNEL, fwctx,
> + brcmf_fw_request_done);
> + kfree(alt_path);
> + }
> + /* Else try canonical path */
> + if (ret) {
> + ret = request_firmware_nowait(THIS_MODULE, true, first->path,
> + fwctx->dev, GFP_KERNEL, fwctx,
> + brcmf_fw_request_done);
> + }
> if (ret < 0)
> brcmf_fw_request_done(NULL, fwctx);
>
>
Hi,
I'm getting this error on Nexus 7 (bcm4330) and Acer A500 (bcm4329)
using today's -next, WiFi doesn't work at all:
brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000): clkctl 0x50
Reverting this patch helps, any ideas? Maybe the missing firmware
creates the offending delay somewhere. Adding the per-board FW file
makes WiFi to work, there is no error message.
Linus, have you tested this patch without the per-board file?
BTW, we also need the per-board FW for Nexus 7 because stock bcm4330
firmware doesn't work properly on Nexus. It sees networks, but can't
connect anywhere.
I'm also aware about Lenovo tablet with bcm4330 that doesn't work at all
until Bluetooth FW is loaded. So the FW support status is not ideal yet,
but the per-board FW is a step forward!
next prev parent reply other threads:[~2021-08-03 15:30 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-11 23:16 [PATCH] brcmfmac: firmware: Allow per-board firmware binaries Linus Walleij
2021-07-30 9:21 ` Linus Walleij
2021-07-30 17:58 ` Kalle Valo
2021-08-01 10:27 ` Kalle Valo
2021-08-03 15:28 ` Dmitry Osipenko [this message]
2021-08-03 15:53 ` Dmitry Osipenko
2021-08-03 23:28 ` Linus Walleij
2021-08-04 8:48 ` Arend van Spriel
2021-08-04 9:35 ` Linus Walleij
2021-08-04 11:32 ` Arend van Spriel
2021-08-04 19:18 ` Linus Walleij
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fca09928-72a8-a573-a862-7c486a2094d6@gmail.com \
--to=digetx@gmail.com \
--cc=aspriel@gmail.com \
--cc=chi-hsien.lin@infineon.com \
--cc=chung-hsien.hsu@infineon.com \
--cc=franky.lin@broadcom.com \
--cc=hante.meuleman@broadcom.com \
--cc=kvalo@codeaurora.org \
--cc=linus.walleij@linaro.org \
--cc=linux-wireless@vger.kernel.org \
--cc=newbyte@disroot.org \
--cc=phone-devel@vger.kernel.org \
--cc=stephan@gerhold.net \
--cc=wright.feng@infineon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.