Hi Kalle,
On 16/01/16 11:57, Kalle Valo wrote:
> Marc Zyngier <marc.zyngier@xxxxxxx> writes:
>
>> David, Hante,
>>
>> On 13/01/16 02:51, David Miller wrote:
>>
>> [...]
>>
>>> Hante Meuleman (33):
>> [...]
>>> brcmfmac: Move all module parameters to one place
>
> As a reminder to myself this is the commit id:
>
> 7d34b0560567 brcmfmac: Move all module parameters to one place
>
>> This particular patch breaks one of my boxes in a spectacular way:
>>
>> [ 3.602155] Unable to handle kernel paging request at virtual address 000027e4
>> [ 3.602160] pgd = c0003000
>> [ 3.602169] [000027e4] *pgd=80000040004003, *pmd=00000000
>> [ 3.602181] Internal error: Oops: 206 [#1] PREEMPT SMP ARM
>
> [...]
>
>> This is caused by this hunk:
>>
>> @@ -890,7 +887,8 @@ static void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev)
>> if (!sdiodev->sg_support)
>> return;
>>
>> - nents = max_t(uint, BRCMF_DEFAULT_RXGLOM_SIZE, brcmf_sdiod_txglomsz);
>> + nents = max_t(uint, BRCMF_DEFAULT_RXGLOM_SIZE,
>> + sdiodev->bus_if->drvr->settings->sdiod_txglomsz);
>> nents += (nents >> 4) + 1;
>>
>> WARN_ON(nents > sdiodev->max_segment_count);
>>
>> were drvr->settings is NULL (as the settings allocation seems to be done
>> much later). The fix is not completely obvious to me (probably requires
>> pushing the call to brcmf_mp_device_attach() down into the various bus
>> specific functions). An alternative would be to restore the txglomsz
>> parameter as it was before and not rely on settings being allocated.
>
> Should we revert the patch or can you Hante fix this? The revert doesn't
> seem to be trivial so I would appreciate if someone could send a patch.
I've worked out a partial revert (see below) that allows my system to
boot, but I'd rather see a proper fix from the maintainer of this code.
Thanks,
M.
>From 4dcd43e859ebf4453da90de2034c37174bb88d13 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@xxxxxxx>
Date: Mon, 18 Jan 2016 11:16:36 +0000
Subject: [PATCH] brcmfmac: Partial revert of module parameter rework
Commit 7d34b0560567 ("brcmfmac: Move all module parameters to one
place") reworked the brcmfmac driver parameters and moved them to
a dynamically allocated structure.
It turns out that at least one parameter (sdiod_txglomsz) is required
before this structure is allocated, leading to a crash.
This patch reverts the handling of that parameter until the maintainer
of this driver can sort it out for good.
Cc: Arend Van Spriel <arend@xxxxxxxxxxxx>
Cc: Hante Meuleman <meuleman@xxxxxxxxxxxx>
Cc: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 12 +++++++-----
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 7 -------
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h | 2 --
3 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 5363739..410a664 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -47,8 +47,6 @@
#include "debug.h"
#include "sdio.h"
#include "of.h"
-#include "core.h"
-#include "common.h"
#define SDIOH_API_ACCESS_RETRY_LIMIT 2
@@ -59,6 +57,7 @@
/* Maximum milliseconds to wait for F2 to come up */
#define SDIO_WAIT_F2RDY 3000
+#define BRCMF_DEFAULT_TXGLOM_SIZE 32 /* max tx frames in glom chain */
#define BRCMF_DEFAULT_RXGLOM_SIZE 32 /* max rx frames in glom chain */
struct brcmf_sdiod_freezer {
@@ -69,6 +68,10 @@ struct brcmf_sdiod_freezer {
struct completion resumed;
};
+static int brcmf_sdiod_txglomsz = BRCMF_DEFAULT_TXGLOM_SIZE;
+module_param_named(txglomsz, brcmf_sdiod_txglomsz, int, 0);
+MODULE_PARM_DESC(txglomsz, "maximum tx packet chain size [SDIO]");
+
static irqreturn_t brcmf_sdiod_oob_irqhandler(int irq, void *dev_id)
{
struct brcmf_bus *bus_if = dev_get_drvdata(dev_id);
@@ -887,8 +890,7 @@ static void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev)
if (!sdiodev->sg_support)
return;
- nents = max_t(uint, BRCMF_DEFAULT_RXGLOM_SIZE,
- sdiodev->bus_if->drvr->settings->sdiod_txglomsz);
+ nents = max_t(uint, BRCMF_DEFAULT_RXGLOM_SIZE, brcmf_sdiod_txglomsz);
nents += (nents >> 4) + 1;
WARN_ON(nents > sdiodev->max_segment_count);
@@ -900,7 +902,7 @@ static void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev)
sdiodev->sg_support = false;
}
- sdiodev->txglomsz = sdiodev->bus_if->drvr->settings->sdiod_txglomsz;
+ sdiodev->txglomsz = brcmf_sdiod_txglomsz;
}
#ifdef CONFIG_PM_SLEEP
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 4265b50..1f57c42 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -35,12 +35,6 @@ const u8 ALLFFMAC[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
/* boost value for RSSI_DELTA in preferred join selection */
#define BRCMF_JOIN_PREF_RSSI_BOOST 8
-#define BRCMF_DEFAULT_TXGLOM_SIZE 32 /* max tx frames in glom chain */
-
-static int brcmf_sdiod_txglomsz = BRCMF_DEFAULT_TXGLOM_SIZE;
-module_param_named(txglomsz, brcmf_sdiod_txglomsz, int, 0);
-MODULE_PARM_DESC(txglomsz, "Maximum tx packet chain size [SDIO]");
-
/* Debug level configuration. See debug.h for bits, sysfs modifiable */
int brcmf_msg_level;
module_param_named(debug, brcmf_msg_level, int, S_IRUSR | S_IWUSR);
@@ -234,7 +228,6 @@ int brcmf_mp_device_attach(struct brcmf_pub *drvr)
return -ENOMEM;
}
- drvr->settings->sdiod_txglomsz = brcmf_sdiod_txglomsz;
drvr->settings->p2p_enable = !!brcmf_p2p_enable;
drvr->settings->feature_disable = brcmf_feature_disable;
drvr->settings->fcmode = brcmf_fcmode;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index 3b0a63b..05ad4b8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -41,7 +41,6 @@ extern struct brcmf_mp_global_t brcmf_mp_global;
/**
* struct brcmf_mp_device - Device module paramaters.
*
- * @sdiod_txglomsz: SDIO txglom size.
* @joinboost_5g_rssi: 5g rssi booost for preferred join selection.
* @p2p_enable: Legacy P2P0 enable (old wpa_supplicant).
* @feature_disable: Feature_disable bitmask.
@@ -49,7 +48,6 @@ extern struct brcmf_mp_global_t brcmf_mp_global;
* @roamoff: Firmware roaming off?
*/
struct brcmf_mp_device {
- int sdiod_txglomsz;
int joinboost_5g_rssi;
bool p2p_enable;
int feature_disable;
--
2.1.4
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
