Re: [PATCH] brcmfmac: check if we can support used firmware API version

[Arend Van Spriel <arend.vanspriel@broadcom.com>]

On 5-1-2017 11:06, Rafał Miłecki wrote:
> On 5 January 2017 at 10:50, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 3-1-2017 17:11, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Every new firmware API will most likely require changes in our code to
>>> support it. Right now we support 2 versions only. Refuse to init if we
>>> detect newer version.
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>> Hi Arend,
>>>
>>> I think you were concerned about possible firmware API changes. Please
>>> review this patch, I hope it's a proper check for running unsupported
>>> firmware version which could result in broken communication between host
>>> driver and a device.
>>> ---
>>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> index 0babfc7..c69ae84 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> @@ -6816,6 +6816,11 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
>>>               brcmf_err("Failed to get D11 version (%d)\n", err);
>>>               goto priv_out;
>>>       }
>>> +     if (io_type > BRCMU_D11AC_IOTYPE) {
>>> +             brcmf_err("Unsupported IO version %d\n", io_type);
>>> +             goto priv_out;
>>> +     }
>>
>> I prefer to have this in brcmu_d11_attach() and have it return an error.
> 
> My too, but since you made brcmu_d11_attach part of *utils* and new IO
> version support may require driver changes, I didn't want to mess
> these two.
> 
> If it was up to me, I would keep brcmu_d11_attach in brcmfmac code and
> then add a proper check in this function indeed.
> 
> Is there any reason you made brcmu_d11_attach part of utils?

The plan was/is to add 11ac support in brcmsmac so it would need the
brcmu_d11_*() functions.

Regards,
Arend