Re: [PATCH 3/6] brcmfmac: Add support for first trying to get a board specific nvram file

[Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx>]

On 10/9/2018 2:47 PM, Hans de Goede wrote:
> The nvram files which some brcmfmac chips need are board-specific. To be
> able to distribute these as part of linux-firmware, so that devices with
> such a wifi chip will work OOTB, multiple (one per board) versions must
> co-exist under /lib/firmware.
>
> This commit adds support for callers of the brcmfmac/firmware.c code to
> pass in a board_type parameter through the request structure.
>
> If that parameter is set then the code will first try to load
> chipmodel.board_type.txt before falling back to the old chipmodel.txt name.

minor comment below...

Reviewed-by: Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
>  .../broadcom/brcm80211/brcmfmac/firmware.c    | 27 ++++++++++++++++++-
>  .../broadcom/brcm80211/brcmfmac/firmware.h    |  1 +
>  2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> index 08aaf99fee34..6755b2388fbc 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> @@ -532,6 +532,31 @@ static int brcmf_fw_complete_request(const struct firmware *fw,
>  	return (cur->flags & BRCMF_FW_REQF_OPTIONAL) ? 0 : ret;
>  }
>
> +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 */
> +	if (cur->type == BRCMF_FW_TYPE_NVRAM && fwctx->req->board_type) {
> +		char alt_path[BRCMF_FW_NAME_LEN];
> +
> +		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);

why not string just "txt"?

> +		strlcat(alt_path, fwctx->req->board_type, BRCMF_FW_NAME_LEN);
> +		strlcat(alt_path, ".txt", BRCMF_FW_NAME_LEN);
> +
> +		ret = request_firmware(fw, alt_path, fwctx->dev);
> +		if (ret == 0)
> +			return ret;
> +	}
> +
> +	return request_firmware(fw, cur->path, fwctx->dev);
> +}
> +
>  static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
>  {
>  	struct brcmf_fw *fwctx = ctx;
> @@ -544,7 +569,7 @@ static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
>
>  	while (ret == 0 && ++fwctx->curpos < fwctx->req->n_items) {
>  		cur = &fwctx->req->items[fwctx->curpos];
> -		request_firmware(&fw, cur->path, fwctx->dev);
> +		brcmf_fw_request_firmware(&fw, fwctx);
>  		ret = brcmf_fw_complete_request(fw, ctx);
>  	}
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
> index 2893e56910f0..a0834be8864e 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
> @@ -70,6 +70,7 @@ struct brcmf_fw_request {
>  	u16 domain_nr;
>  	u16 bus_nr;
>  	u32 n_items;
> +	const char *board_type;
>  	struct brcmf_fw_item items[0];
>  };