Search Linux Wireless

Re: [PATCH] brcmfmac: firmware: Allow per-board firmware binaries

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

 



On August 3, 2021 5:53:14 PM Dmitry Osipenko <digetx@xxxxxxxxx> wrote:

03.08.2021 18:28, Dmitry Osipenko пишет:
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@xxxxxxxxxxxxxxx
Cc: newbyte@xxxxxxxxxxx
Cc: Stephan Gerhold <stephan@xxxxxxxxxxx>
Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
---
.../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);

This return 0 immediately, despite of the missing firmware file. It's
not a blocking FW request.

Right. I didn't get to looking at this earlier, but indeed the check whether the requested firmware exists is done in another thread context so the return value here only indicates whether the firmware request could be scheduled or not.

My first reaction to the patch was to reject it, but leaning towards supporting this now. OEMs tend to get tailor-made firmware in terms of features. Depending on their requirements they get their mix of firmware features. That such difference lead to a crash in 3d engine is somewhat surprising. I am curious if we can debug this more and learn how a firmware variant could cause such a crash. Maybe some DMA issue?

Regards,
Arend




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux