A null pointer will be passed to a kfree() call after a kzalloc() call failed.
This code is useless. Thus delete the extra function call.
A goto statement is also no longer needed. Thus adjust an if branch.
Signed-off-by: Zheng Yongjun <zhengyongjun3@huawei.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Link: https://lore.kernel.org/r/20201222135113.20680-1-zhengyongjun3@huawei.com
The function parameter is not documented and either are any of the
other functions in this source file.
Fixes the following W=1 kernel build warning(s):
drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c:69: warning: Function parameter or member 'c' not described in 'is_nvram_char'
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: Franky Lin <franky.lin@broadcom.com>
Cc: Hante Meuleman <hante.meuleman@broadcom.com>
Cc: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
Cc: Wright Feng <wright.feng@cypress.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: brcm80211-dev-list.pdl@broadcom.com
Cc: brcm80211-dev-list@cypress.com
Cc: netdev@vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Link: https://lore.kernel.org/r/20200821071644.109970-13-lee.jones@linaro.org
With ISC license text in place under the LICENSES folder switch
to using the SPDX license identifier to refer to the ISC license.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct foo {
int stuff;
struct boo entry[];
};
size = sizeof(struct foo) + count * sizeof(struct boo);
instance = kzalloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
Notice that, in this case, variable reqsz is not necessary,
hence it is removed.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
During a normal brcmfmac lifetime brcmf_fw_alloc_request() is called
once only during the probe. It's safe to assume provided array is clear.
Further brcmfmac improvements may require calling it multiple times
though. This patch allows it by fixing invalid firmware paths like:
brcm/brcmfmac4366c-pcie.binbrcm/brcmfmac4366c-pcie.bin
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
The local variable chipname is string representation of chip id and revision
which is printed in the good flow of brcmf_fw_alloc_request(). Also use it
for the error path, ie. for unknown/unsupported devices.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Fix spelling mistakes in brcmfmac: "lenght" -> "length".
The typos are also in the special comment blocks which
translates to documentation.
Signed-off-by: Matteo Croce <mcroce@redhat.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
When CONFIG_NO_AUTO_INLINE is set, we get a false-postive warning
for the brcmf_fw_request_nvram_done() function, after gcc figures
out that brcmf_fw_nvram_from_efi() might not set the 'data_len'
variable, but fails to notice that it always returns NULL:
drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c: In function 'brcmf_fw_request_nvram_done':
drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c:560:11: error: 'data_len' may be used uninitialized in this function [-Werror=maybe-uninitialized]
Mark it 'inline' to force gcc to understand this.
Fixes: ce2e6db554 ("brcmfmac: Add support for getting nvram contents from EFI variables")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
I ended up tracking down some rather nasty issues with f2fs (and other
filesystem modules) constantly crashing on my kernel down to a
combination of out of bounds memory accesses, one of which was coming
from brcmfmac during module load:
[ 30.891382] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac4356-sdio for chip BCM4356/2
[ 30.894437] ==================================================================
[ 30.901581] BUG: KASAN: global-out-of-bounds in brcmf_fw_alloc_request+0x42c/0x480 [brcmfmac]
[ 30.909935] Read of size 1 at addr ffff2000024865df by task kworker/6:2/387
[ 30.916805]
[ 30.918261] CPU: 6 PID: 387 Comm: kworker/6:2 Tainted: G O 4.20.0-rc3Lyude-Test+ #19
[ 30.927251] Hardware name: amlogic khadas-vim2/khadas-vim2, BIOS 2018.07-rc2-armbian 09/11/2018
[ 30.935964] Workqueue: events brcmf_driver_register [brcmfmac]
[ 30.941641] Call trace:
[ 30.944058] dump_backtrace+0x0/0x3e8
[ 30.947676] show_stack+0x14/0x20
[ 30.950968] dump_stack+0x130/0x1c4
[ 30.954406] print_address_description+0x60/0x25c
[ 30.959066] kasan_report+0x1b4/0x368
[ 30.962683] __asan_report_load1_noabort+0x18/0x20
[ 30.967547] brcmf_fw_alloc_request+0x42c/0x480 [brcmfmac]
[ 30.967639] brcmf_sdio_probe+0x163c/0x2050 [brcmfmac]
[ 30.978035] brcmf_ops_sdio_probe+0x598/0xa08 [brcmfmac]
[ 30.983254] sdio_bus_probe+0x190/0x398
[ 30.983270] really_probe+0x2a0/0xa70
[ 30.983296] driver_probe_device+0x1b4/0x2d8
[ 30.994901] __driver_attach+0x200/0x280
[ 30.994914] bus_for_each_dev+0x10c/0x1a8
[ 30.994925] driver_attach+0x38/0x50
[ 30.994935] bus_add_driver+0x330/0x608
[ 30.994953] driver_register+0x140/0x388
[ 31.013965] sdio_register_driver+0x74/0xa0
[ 31.014076] brcmf_sdio_register+0x14/0x60 [brcmfmac]
[ 31.023177] brcmf_driver_register+0xc/0x18 [brcmfmac]
[ 31.023209] process_one_work+0x654/0x1080
[ 31.032266] worker_thread+0x4f0/0x1308
[ 31.032286] kthread+0x2a8/0x320
[ 31.039254] ret_from_fork+0x10/0x1c
[ 31.039269]
[ 31.044226] The buggy address belongs to the variable:
[ 31.044351] brcmf_firmware_path+0x11f/0xfffffffffffd3b40 [brcmfmac]
[ 31.055601]
[ 31.057031] Memory state around the buggy address:
[ 31.061800] ffff200002486480: 04 fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
[ 31.068983] ffff200002486500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 31.068993] >ffff200002486580: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
[ 31.068999] ^
[ 31.069017] ffff200002486600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 31.096521] ffff200002486680: 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa
[ 31.096528] ==================================================================
[ 31.096533] Disabling lock debugging due to kernel taint
It appears that when trying to determine the length of the string in the
alternate firmware path, we make the mistake of not handling the case
where the firmware path is empty correctly. Since strlen(mp_path) can
return 0, we'll end up accessing mp_path[-1] when the firmware_path
isn't provided through the module arguments.
So, fix this by just setting the end char to '\0' by default, and only
changing it if we have a non-zero length. Additionally, use strnlen()
with BRCMF_FW_ALTPATH_LEN instead of strlen() just to be extra safe.
Fixes: 2baa3aaee2 ("brcmfmac: introduce brcmf_fw_alloc_request() function")
Cc: Hante Meuleman <hante.meuleman@broadcom.com>
Cc: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Cc: Franky Lin <franky.lin@broadcom.com>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: Arend Van Spriel <arend.vanspriel@broadcom.com>
Cc: Himanshu Jha <himanshujha199640@gmail.com>
Cc: Dan Haab <dhaab@luxul.com>
Cc: Jia-Shyr Chuang <saint.chuang@cypress.com>
Cc: Ian Molton <ian@mnementh.co.uk>
Cc: <stable@vger.kernel.org> # v4.17+
Signed-off-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
In some cases the EFI-var stored nvram contains "ccode=ALL" or "ccode=XV"
to specify "worldwide" compatible settings, but these 2 ccode-s do not work
properly.
I've tested the different known "worldwide" ccode-s used in various nvram
sources with the latest firmwares from linux-firmware for various brcmfmac
models, here is a simplified (*) table with what each setting results in:
ALL: 12-14 disab, U-NII-1, U-NII-2 no-IR/radar, U-NII-3
XV: 12-14 no-IR, disables all 5G channels
XY: 12-13 enab, 14 disab, U-NII-1 enab, U-NII-2 no-IR/radar, U-NII-3 disab
X2: 12-13 no-IR, 14 dis, U-NII-1 no-IR, U-NII-2 no-IR/radar, U-NII-3 no-IR
Where 12,13,14 are 2.4G channels 12-14 and U-NII-1/2/3 are the 3 different
5G channel groups. no-IR is no-Initiate-Radiation, we will never send on
these channels without first having received valid wifi traffic there.
This immediately shows that both ALL and XV are not as worldwide as we want
them to be. ALL causes channels 12 and 13 to not be available and XV causes
all 5GHz channels to not be available. Also ALL unconditionally enables the
U-NII-1 and U-NII-3 5G groups, while we really should be using no-IR for
these.
This commit replace XV and ALL with X2, which allows usage of chan 12-13
and 5G channels, but only after receiving valid wifi traffic there first.
Note that this configure the firmware's channel limits, the kernels own
regulatory restrictions based on e.g. regulatory info received from the
access-point, will be applied on top of this.
This fixes channels 12+13 not working on the Asus T200TA and the Lenovo
Mixx 2 8 and 5G channels not working on the Asus T100HA.
This has been tested on the following models: Acer Iconia Tab8 w1-810,
Acer One 10, Asus T100CHI, Asus T100HA, Asus T100TA, Asus T200TA and a
Lenovo Mixx 2 8.
*) There are some exceptions to this table:
1) On really old firmware e.g. linux-firmware's 2011 brcmfmac4330-sdio.bin
ALL really means all, unconditionally enabling everything
2) The exact meaning might be influenced by setting the regrev nvram var.
Specifically using ccode=XV + regrev=1 on brcmfmac43241b4 leads to:
12-14 no-ir, U-NII-1 no-ir, U-NII-2 no-ir/radar, U-NII-3 no-ir
But only on the brcmfmac43241b4 and not on e.g. the brcmfmac43340
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Various X86 laptops with a SDIO attached brcmfmac wifi chip, store the
nvram contents in a special EFI variable. This commit adds support for
getting nvram directly from this EFI variable, without the user needing
to manually copy it.
This makes Wifi / Bluetooth work out of the box on these devices instead of
requiring manual setup.
This has been tested on the following models: Acer Iconia Tab8 w1-810,
Acer One 10, Asus T100CHI, Asus T100HA, Asus T100TA, Asus T200TA and a
Lenovo Mixx 2 8.
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
The "cur" variable is now only used for a debug print and we already
print the same info from brcmf_fw_complete_request(), so the debug print
does not provide any extra info and we can remove it.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
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.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Before this commit brcmf_fw_request_done would call
brcmf_fw_request_next_item to load the next item, which on an error would
call brcmf_fw_request_done, which if the error is recoverable (*) will
then continue calling brcmf_fw_request_next_item for the next item again
which on an error will call brcmf_fw_request_done again...
This does not blow up because we only have a limited number of items so
we never recurse too deep. But the recursion is still quite ugly and
frankly is giving me a headache, so lets fix this.
This commit fixes this by removing brcmf_fw_request_next_item and by
making brcmf_fw_get_firmwares and brcmf_fw_request_done directly call
firmware_request_nowait resp. firmware_request themselves.
*) brcmf_fw_request_nvram_done fallback path succeeds or
BRCMF_FW_REQF_OPTIONAL is set
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
brcmf_fw_request_next_item and brcmf_fw_request_done both have identical
code to complete the fw-request depending on the item-type.
This commit adds a new brcmf_fw_complete_request helper removing this code
duplication.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
The information in the firmware mapping does not need to be modified
so it can be static const.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
When nvram loading fails a double free occurred. Fix this and reorg the
code a little.
Fixes: d09ae51a4b ("brcmfmac: pass struct in brcmf_fw_get_firmwares()")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
The function was pretty minimal and now it is called only from one
place so just get rid of it.
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
The function is no longer used so removing it.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
The function brcmf_fw_alloc_request() takes a list of required files
and allocated the struct brcmf_fw_request instance accordingly. The
request can be modified by the caller before being passed to the
brcmf_fw_request_firmwares() function.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Make the function brcmf_fw_get_firmwares() a bit more easy to extend
using a structure to pass the request parameters.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Instead of defining individual filenames for firmware and nvram
use a basename and derive the names from that.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
When logging the chip id/revision information make use of
brcmf_chip_name() so it is always the same.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
For debugging some problems, it is useful to know the chip revision
add a brcmf_info message logging this.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
When firmware loading failed the code used to unbind the device provided
by the calling code. However, for the sdio driver two devices are bound
and both need to be released upon failure. The callback has been extended
with parameter to pass error code so add that in this commit upon firmware
loading failure.
Cc: stable@vger.kernel.org # 4.9.x-
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Extend the parameters in the firmware callback so it can be called
upon success and failure. This allows the caller to properly clear
all resources in the failure path. Right now the error code is
always zero, ie. success.
Cc: stable@vger.kernel.org # 4.9.x-
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Some nvram files/stores come without the boardrev information,
but firmware requires this to be set. When not found in nvram then
add a default boardrev string to the nvram data.
Reported-by: Rafal Milecki <zajec5@gmail.com>
Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <franky.lin@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Signed-off-by: Hante Meuleman <hante.meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Platform NVRAM (stored on a flash partition) has entries separated by a
NULL (\0) char. Our parsing code switches from VALUE state to IDLE
whenever it meets a NULL (\0). When that happens our IDLE handler should
simply consume it and analyze whatever is placed ahead.
This fixes harmless warnings spamming debugging output:
[ 155.165624] brcmfmac: brcmf_nvram_handle_idle warning: ln=1:col=20: ignoring invalid character
[ 155.180806] brcmfmac: brcmf_nvram_handle_idle warning: ln=1:col=44: ignoring invalid character
[ 155.195971] brcmfmac: brcmf_nvram_handle_idle warning: ln=1:col=63: ignoring invalid character
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Module parameters are defined in several files. Move them in one
place and make them device specific or global. This makes it
easier to override device specific settings by external data like
platform data in the future.
Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
All bus drivers (sdio, usb and pcie) require firmware files which
needs to be downloaded to the device, The definitions and mapping
of device id and revision to firmware and nvram file is done by
each bus driver. This patch creates common functions and defines
to simplify and unify the definition of these firmware and nvram
files and mapping.
Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
The release_firmware() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Part of reorganising wireless drivers directory and Kconfig. Note that I had to
edit Makefiles from subdirectories to use the new location.
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
2015-11-18 11:24:22 +02:00
Renamed from drivers/net/wireless/brcm80211/brcmfmac/firmware.c (Browse further)
