DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A63FD20819
Subject: Re: [PATCH 3/6] brcmfmac: Add support for first trying to get a board
 specific nvram file
To:     Arend van Spriel <arend.vanspriel@broadcom.com>,
        Franky Lin <franky.lin@broadcom.com>,
        Hante Meuleman <hante.meuleman@broadcom.com>,
        Kalle Valo <kvalo@codeaurora.org>,
        Chi-Hsien Lin <chi-hsien.lin@cypress.com>,
        Wright Feng <wright.feng@cypress.com>
Cc:     linux-wireless@vger.kernel.org, brcm80211-dev-list.pdl@broadcom.com
References: <20181009124755.25402-1-hdegoede@redhat.com>
 <20181009124755.25402-3-hdegoede@redhat.com>
 <c0415f15-d3cf-2693-be61-26151c5c030b@broadcom.com>
From:   Hans de Goede <hdegoede@redhat.com>
Message-ID: <193d4020-1a7f-7a9e-1953-4da8a2509d89@redhat.com>
Date:   Mon, 5 Nov 2018 15:32:54 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.8.0
MIME-Version: 1.0
In-Reply-To: <c0415f15-d3cf-2693-be61-26151c5c030b@broadcom.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Hi,

Thank you for the reviews.

On 05-11-18 12:41, Arend van Spriel wrote:
> 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@broadcom.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  .../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"?

I'm not entirely sure what your question exactly is here?

Regards,

Hans


> 
>> +        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];
>>  };
>>
>>
>