| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Yjjgi4YJVYBnJTqK@google.com>
Date: Mon, 21 Mar 2022 13:31:07 -0700
From: Brian Norris <briannorris@...omium.org>
To: Francesco Dolcini <francesco.dolcini@...adex.com>
Cc: Amitkumar Karwar <amitkarwar@...il.com>,
Ganapathi Bhat <ganapathi017@...il.com>,
Sharvari Harisangam <sharvari.harisangam@....com>,
Xinming Hu <huxinming820@...il.com>,
linux-wireless@...r.kernel.org,
Andrejs Cainikovs <andrejs.cainikovs@...adex.com>,
Kalle Valo <kvalo@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
Jonas Dreßler <verdre@...d.nl>
Subject: Re: [RFC PATCH] mwifiex: Select firmware based on strapping
On Mon, Mar 21, 2022 at 05:10:03PM +0100, Francesco Dolcini wrote:
> From: Andrejs Cainikovs <andrejs.cainikovs@...adex.com>
>
> Some WiFi/Bluetooth modules might have different host connection
> options, allowing to either use SDIO for both WiFi and Bluetooth,
> or SDIO for WiFi and UART for Bluetooth. It is possible to detect
> whether a module has SDIO-SDIO or SDIO-UART connection by reading
> its host strap register.
>
> This change introduces a way to automatically select appropriate
> firmware depending of the connection method, and removes a need
> of symlinking or overwriting the original firmware file with a
> required one.
>
> Signed-off-by: Andrejs Cainikovs <andrejs.cainikovs@...adex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@...adex.com>
> ---
> Hi all,
>
> Current mwifiex_sdio implementation does not have strapping detection, which
> means there's no way system will automatically detect which firmware needs to
> be picked depending of the strapping. SD8997, in particular, can be strapped
> for sdiosdio (Wi-Fi over SDIO, Bluetooth over SDIO) or sdiouart (Wi-Fi over
> SDIO, Bluetooth over UART). What we do now - simply replace the
> original sdiosdio firmware file with the one supplied by NXP [1] for sdiouart.
>
> Of course, this is not clean, and by submitting this patch I would like to
> receive your comments regarding how it would be better to implement the
> strapping detection.
>
> [1] https://github.com/NXP/imx-firmware/blob/lf-5.10.52_2.1.0/nxp/FwImage_8997_SD/sdiouart8997_combo_v4.bin
>
> Francesco & Andrejs
>
> ---
> drivers/net/wireless/marvell/mwifiex/sdio.c | 17 ++++++++++++++++-
> drivers/net/wireless/marvell/mwifiex/sdio.h | 6 ++++++
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index bde9e4bbfffe..8670ded74c27 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -182,6 +182,9 @@ static const struct mwifiex_sdio_card_reg mwifiex_reg_sd8997 = {
> .host_int_rsr_reg = 0x4,
> .host_int_status_reg = 0x0C,
> .host_int_mask_reg = 0x08,
> + .host_strap_reg = 0xF4,
> + .host_strap_mask = 0x01,
> + .host_strap_value = 0x00,
Do you have any documentation or sources (e.g., a vendor driver) that
describe this register? If we don't have documentation on what exactly
this register means, we might need to be a bit more conservative on
using it.
Previous thread:
https://lore.kernel.org/linux-wireless/87a6ejj2np.fsf@bang-olufsen.dk/
I guess this links to some driver sources with similar definitions?
Seems like we could still use a bit better naming/macros to explicitly
call these out as "UART" and "SDIO" options, instead of just
"alternate".
> .status_reg_0 = 0xE8,
> .status_reg_1 = 0xE9,
> .sdio_int_mask = 0xff,
> @@ -402,6 +405,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8977 = {
>
> static const struct mwifiex_sdio_device mwifiex_sdio_sd8997 = {
> .firmware = SD8997_DEFAULT_FW_NAME,
> + .firmware_alt_strap = SD8997_SDIOUART_FW_NAME,
> .reg = &mwifiex_reg_sd8997,
> .max_ports = 32,
> .mp_agg_pkt_limit = 16,
> @@ -536,6 +540,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct sdio_device_id *id)
> struct mwifiex_sdio_device *data = (void *)id->driver_data;
>
> card->firmware = data->firmware;
> + card->firmware_alt_strap = data->firmware_alt_strap;
> card->reg = data->reg;
> card->max_ports = data->max_ports;
> card->mp_agg_pkt_limit = data->mp_agg_pkt_limit;
> @@ -2439,6 +2444,7 @@ static int mwifiex_register_dev(struct mwifiex_adapter *adapter)
> int ret;
> struct sdio_mmc_card *card = adapter->card;
> struct sdio_func *func = card->func;
> + const char *firmware = card->firmware;
>
> /* save adapter pointer in card */
> card->adapter = adapter;
> @@ -2455,7 +2461,15 @@ static int mwifiex_register_dev(struct mwifiex_adapter *adapter)
> return ret;
> }
>
> - strcpy(adapter->fw_name, card->firmware);
> + /* Select alternative firmware based on the strapping options */
> + if (card->firmware_alt_strap) {
> + u8 val;
> + mwifiex_read_reg(adapter, card->reg->host_strap_reg, &val);
> + if ((val & card->reg->host_strap_mask) == card->reg->host_strap_value)
> + firmware = card->firmware_alt_strap;
> + }
> + strcpy(adapter->fw_name, firmware);
> +
> if (card->fw_dump_enh) {
> adapter->mem_type_mapping_tbl = generic_mem_type_map;
> adapter->num_mem_types = 1;
> @@ -3157,3 +3171,4 @@ MODULE_FIRMWARE(SD8887_DEFAULT_FW_NAME);
> MODULE_FIRMWARE(SD8977_DEFAULT_FW_NAME);
> MODULE_FIRMWARE(SD8987_DEFAULT_FW_NAME);
> MODULE_FIRMWARE(SD8997_DEFAULT_FW_NAME);
> +MODULE_FIRMWARE(SD8997_SDIOUART_FW_NAME);
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h b/drivers/net/wireless/marvell/mwifiex/sdio.h
> index 5648512c9300..bfea4d5998b7 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.h
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
> @@ -39,6 +39,7 @@
> #define SD8977_DEFAULT_FW_NAME "mrvl/sdsd8977_combo_v2.bin"
> #define SD8987_DEFAULT_FW_NAME "mrvl/sd8987_uapsta.bin"
> #define SD8997_DEFAULT_FW_NAME "mrvl/sdsd8997_combo_v4.bin"
> +#define SD8997_SDIOUART_FW_NAME "nxp/sdiouart8997_combo_v4.bin"
This isn't your main issue, but just because companies buy and sell IP
doesn't mean we'll change the firmware paths. Qualcomm drivers still use
"ath" prefixes, for one ;)
Personally, I'd still keep the mrvl/ path. But that might be up to Kalle
and/or linux-firmware.git maintainers.
Brian
>
> #define BLOCK_MODE 1
> #define BYTE_MODE 0
Powered by blists - more mailing lists