* [PATCH] brcmfmac: check if we can support used firmware API version
@ 2017-01-03 16:11 Rafał Miłecki
2017-01-05 9:50 ` Arend Van Spriel
0 siblings, 1 reply; 4+ messages in thread
From: Rafał Miłecki @ 2017-01-03 16:11 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, Rafał Miłecki
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;
+ }
+
cfg->d11inf.io_type = (u8)io_type;
brcmu_d11_attach(&cfg->d11inf);
--
2.10.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] brcmfmac: check if we can support used firmware API version
2017-01-03 16:11 [PATCH] brcmfmac: check if we can support used firmware API version Rafał Miłecki
@ 2017-01-05 9:50 ` Arend Van Spriel
2017-01-05 10:06 ` Rafał Miłecki
0 siblings, 1 reply; 4+ messages in thread
From: Arend Van Spriel @ 2017-01-05 9:50 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo
Cc: Franky Lin, Hante Meuleman, Pieter-Paul Giesberts, Franky Lin,
linux-wireless, brcm80211-dev-list.pdl, Rafał Miłecki
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.
Now this actually does not cover the API version in its entirety. The
broadcom firmware API is a bit more complicated. Firmware commands may
use structured data where we may only add fields at the end to keep it
backward compatible and they are versioned if that can not be avoided.
As example see recent patch [1].
Regards,
Arend
[1] https://patchwork.kernel.org/patch/9442873/
> cfg->d11inf.io_type = (u8)io_type;
> brcmu_d11_attach(&cfg->d11inf);
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] brcmfmac: check if we can support used firmware API version
2017-01-05 9:50 ` Arend Van Spriel
@ 2017-01-05 10:06 ` Rafał Miłecki
2017-01-07 11:44 ` Arend Van Spriel
0 siblings, 1 reply; 4+ messages in thread
From: Rafał Miłecki @ 2017-01-05 10:06 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Kalle Valo, Franky Lin, Hante Meuleman, Pieter-Paul Giesberts,
Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Rafał Miłecki
On 5 January 2017 at 10:50, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 3-1-2017 17:11, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <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=C5=82 Mi=C5=82ecki <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?
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] brcmfmac: check if we can support used firmware API version
2017-01-05 10:06 ` Rafał Miłecki
@ 2017-01-07 11:44 ` Arend Van Spriel
0 siblings, 0 replies; 4+ messages in thread
From: Arend Van Spriel @ 2017-01-07 11:44 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Kalle Valo, Franky Lin, Hante Meuleman, Pieter-Paul Giesberts,
Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Rafał Miłecki
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-07 11:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-03 16:11 [PATCH] brcmfmac: check if we can support used firmware API version Rafał Miłecki
2017-01-05 9:50 ` Arend Van Spriel
2017-01-05 10:06 ` Rafał Miłecki
2017-01-07 11:44 ` Arend Van Spriel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox