2018-05-16 12:12:30

by Arend Van Spriel

[permalink] [raw]
Subject: [PATCH V2 0/6] brcmfmac: coredump functionality and fixes

This series is intended for 4.18:

* make ALLFFMAC variable static.
* support user-space initiated coredump.

The first patch in this series applies to the master branch of the
wireless-drivers-next repository. The remaining patches related to coredump
functionality are dependent upon a commit present since v4.17-rc3:

commit ed4564babeeee4fb19fe4ec0beabe29754e380f9
Author: Arend van Spriel <[email protected]>
Date: Sun Apr 8 23:57:07 2018 +0200

drivers: change struct device_driver::coredump() return type to void

This patch is not in w-d-next yet.

Arend van Spriel (3):
brcmfmac: move ALLFFMAC variable in flowring module
brcmfmac: add support for sysfs initiated coredump
mwifiex: support sysfs initiated device coredump

Franky Lin (3):
brcmfmac: validate user provided data for memdump before copying
brcmfmac: trigger memory dump upon firmware halt signal
brcmfmac: trigger memory dump on SDIO firmware halt message

.../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 1 +
.../net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 2 ++
.../wireless/broadcom/brcm80211/brcmfmac/common.c | 2 --
.../wireless/broadcom/brcm80211/brcmfmac/common.h | 2 --
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 8 ++++++
.../wireless/broadcom/brcm80211/brcmfmac/debug.c | 3 ++-
.../broadcom/brcm80211/brcmfmac/flowring.c | 2 ++
.../wireless/broadcom/brcm80211/brcmfmac/pcie.c | 6 +++++
.../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 4 ++-
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 1 +
drivers/net/wireless/marvell/mwifiex/debugfs.c | 31 +---------------------
drivers/net/wireless/marvell/mwifiex/pcie.c | 18 +++++++++++--
drivers/net/wireless/marvell/mwifiex/sdio.c | 12 +++++++++
drivers/net/wireless/marvell/mwifiex/usb.c | 13 +++++++++
14 files changed, 67 insertions(+), 38 deletions(-)

--
1.9.1


2018-05-16 13:50:16

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH V2 3/6] mwifiex: support sysfs initiated device coredump

Arend van Spriel <[email protected]> writes:

> Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
> it is possible to initiate a device coredump from user-space. This
> patch adds support for it adding the .coredump() driver callback.
> As there is no longer a need to initiate it through debugfs remove
> that code.
>
> Signed-off-by: Arend van Spriel <[email protected]>
> ---
> V2:
> - export mwifiex_send_cmd() needed by mwifiex_usb.ko.
> ---
> drivers/net/wireless/marvell/mwifiex/cmdevt.c | 1 +
> drivers/net/wireless/marvell/mwifiex/debugfs.c | 31 +-------------------------
> drivers/net/wireless/marvell/mwifiex/pcie.c | 18 +++++++++++++--
> drivers/net/wireless/marvell/mwifiex/sdio.c | 12 ++++++++++
> drivers/net/wireless/marvell/mwifiex/usb.c | 13 +++++++++++
> 5 files changed, 43 insertions(+), 32 deletions(-)

You forgot to CC mwifiex maintainers, doing it now. Full patch here:

https://patchwork.kernel.org/patch/10403717/

--
Kalle Valo

2018-05-16 18:52:53

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 3/6] mwifiex: support sysfs initiated device coredump

On 5/16/2018 3:50 PM, Kalle Valo wrote:
> Arend van Spriel <[email protected]> writes:
>
>> Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
>> it is possible to initiate a device coredump from user-space. This
>> patch adds support for it adding the .coredump() driver callback.
>> As there is no longer a need to initiate it through debugfs remove
>> that code.
>>
>> Signed-off-by: Arend van Spriel <[email protected]>
>> ---
>> V2:
>> - export mwifiex_send_cmd() needed by mwifiex_usb.ko.
>> ---
>> drivers/net/wireless/marvell/mwifiex/cmdevt.c | 1 +
>> drivers/net/wireless/marvell/mwifiex/debugfs.c | 31 +-------------------------
>> drivers/net/wireless/marvell/mwifiex/pcie.c | 18 +++++++++++++--
>> drivers/net/wireless/marvell/mwifiex/sdio.c | 12 ++++++++++
>> drivers/net/wireless/marvell/mwifiex/usb.c | 13 +++++++++++
>> 5 files changed, 43 insertions(+), 32 deletions(-)
>
> You forgot to CC mwifiex maintainers, doing it now. Full patch here:
>
> https://patchwork.kernel.org/patch/10403717/

Thanks, Kalle

You would almost think I am not used to submit patches for drivers I do
not actively maintain ;-)

Gr. AvS

2018-05-16 12:12:33

by Arend Van Spriel

[permalink] [raw]
Subject: [PATCH V2 2/6] brcmfmac: add support for sysfs initiated coredump

The driver already supports device coredump initiated by firmware
event. Since commit 3c47d19ff4dc ("drivers: base: add coredump driver
ops") it is also possible to initiate it from user-space through
sysfs. This patch adds support for SDIO and PCIe devices.

Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 1 +
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 2 ++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 8 ++++++++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 1 +
4 files changed, 12 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index a191541..d2f788d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -1165,6 +1165,7 @@ static int brcmf_ops_sdio_resume(struct device *dev)
#ifdef CONFIG_PM_SLEEP
.pm = &brcmf_sdio_pm_ops,
#endif /* CONFIG_PM_SLEEP */
+ .coredump = brcmf_dev_coredump,
},
};

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
index 27e693e..c496518 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
@@ -250,6 +250,8 @@ int brcmf_bus_get_fwname(struct brcmf_bus *bus, const char *ext,
void brcmf_detach(struct device *dev);
/* Indication from bus module that dongle should be reset */
void brcmf_dev_reset(struct device *dev);
+/* Request from bus module to initiate a coredump */
+void brcmf_dev_coredump(struct device *dev);

/* Configure the "global" bus state used by upper layers */
void brcmf_bus_change_state(struct brcmf_bus *bus, enum brcmf_bus_state state);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 8d4511e..72954fd 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -1180,6 +1180,14 @@ void brcmf_dev_reset(struct device *dev)
brcmf_fil_cmd_int_set(drvr->iflist[0], BRCMF_C_TERMINATED, 1);
}

+void brcmf_dev_coredump(struct device *dev)
+{
+ struct brcmf_bus *bus_if = dev_get_drvdata(dev);
+
+ if (brcmf_debug_create_memdump(bus_if, NULL, 0) < 0)
+ brcmf_dbg(TRACE, "failed to create coredump\n");
+}
+
void brcmf_detach(struct device *dev)
{
s32 i;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index f0797ae..5baa837 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -2044,6 +2044,7 @@ static int brcmf_pcie_pm_leave_D3(struct device *dev)
#ifdef CONFIG_PM
.driver.pm = &brcmf_pciedrvr_pm,
#endif
+ .driver.coredump = brcmf_dev_coredump,
};


--
1.9.1

2018-05-16 12:12:34

by Arend Van Spriel

[permalink] [raw]
Subject: [PATCH V2 1/6] brcmfmac: move ALLFFMAC variable in flowring module

The only user of ALLFFMAC is the flowring module so no need to
expose it in a header file.

Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
Hi Kalle,

I recall Joe Perches posted patches dealing with broadcast address declarations,
but don't know what happened with those. In patchwork [1] is says 'Not Applicable'
whatever that means. I noticed Johannes set similar mac80211 change to
'Awaiting Upstream'. Anyway, when applying this patch the patch in [1] needs to
be reworked.

Regards,
Arend

[1] https://patchwork.kernel.org/patch/10318857/
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 --
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h | 2 --
drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c | 2 ++
3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 105b877..cd36510 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -36,8 +36,6 @@
MODULE_DESCRIPTION("Broadcom 802.11 wireless LAN fullmac driver.");
MODULE_LICENSE("Dual BSD/GPL");

-const u8 ALLFFMAC[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
-
#define BRCMF_DEFAULT_SCAN_CHANNEL_TIME 40
#define BRCMF_DEFAULT_SCAN_UNASSOC_TIME 40

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index ef91461..a34642c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -19,8 +19,6 @@
#include <linux/platform_data/brcmfmac.h>
#include "fwil_types.h"

-extern const u8 ALLFFMAC[ETH_ALEN];
-
#define BRCMF_FW_ALTPATH_LEN 256

/* Definitions for the module global and device specific settings are defined
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
index d0b738d..d0d8b32 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
@@ -46,6 +46,8 @@
3
};

+static const u8 ALLFFMAC[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
+

static bool
brcmf_flowring_is_tdls_mac(struct brcmf_flowring *flow, u8 mac[ETH_ALEN])
--
1.9.1

2018-05-23 15:57:00

by Kalle Valo

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH V2 3/6] mwifiex: support sysfs initiated device coredump

Ganapathi Bhat <[email protected]> writes:

> Hi Kalle,
>
>> Arend van Spriel <[email protected]> writes:
>>
>> > On 5/23/2018 9:54 AM, Kalle Valo wrote:
>> >> Ganapathi Bhat <[email protected]> writes:
>> >>
>> >>> In V2:
>> >>>
>> >>> return ret;
>> >>> }
>> >>> +EXPORT_SYMBOL_GPL(mwifiex_send_cmd);
>> >>>
>> >>> Instead of exporting 'mwifiex_send_cmd' we can prepare a wrapper for
>> >>> the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex
>> module. Let
>> >>> me know if we can send a follow up.
>> >>
>> >> So what's the plan? Can I apply this patch or should I drop it?
>> >
>> > Sorry. Had to reply that earlier. When adding the export I figured it
>> > might not be desirable. If Ganapathi can fix it with a follow up patch
>> > that would ok, right?
>>
>> A follow up patch sounds good to me.
>
> Sorry for confusions. I assume you will be margining the original
> patch and want me to send the follow-up right?

Correct. I applied Arend's patch now so you can send your follow-up
patch.

--
Kalle Valo

2018-05-23 08:17:44

by Kalle Valo

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH V2 3/6] mwifiex: support sysfs initiated device coredump

Arend van Spriel <[email protected]> writes:

> On 5/23/2018 9:54 AM, Kalle Valo wrote:
>> Ganapathi Bhat <[email protected]> writes:
>>
>>> In V2:
>>>
>>> return ret;
>>> }
>>> +EXPORT_SYMBOL_GPL(mwifiex_send_cmd);
>>>
>>> Instead of exporting 'mwifiex_send_cmd' we can prepare a wrapper for
>>> the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex module. Let me
>>> know if we can send a follow up.
>>
>> So what's the plan? Can I apply this patch or should I drop it?
>
> Sorry. Had to reply that earlier. When adding the export I figured it
> might not be desirable. If Ganapathi can fix it with a follow up patch
> that would ok, right?

A follow up patch sounds good to me.

> Than you can apply it, I would say.

Good, thanks.

--
Kalle Valo

2018-05-21 08:19:42

by Ganapathi Bhat

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH V2 3/6] mwifiex: support sysfs initiated device coredump

Hi arend.vanspriel,
>
> ----------------------------------------------------------------------
> On 5/16/2018 3:50 PM, Kalle Valo wrote:
> > Arend van Spriel <[email protected]> writes:
> >
> >> Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
> >> it is possible to initiate a device coredump from user-space. This
> >> patch adds support for it adding the .coredump() driver callback.
> >> As there is no longer a need to initiate it through debugfs remove
> >> that code.
> >>
> >> Signed-off-by: Arend van Spriel <[email protected]>
> >> ---
> >> V2:
> >> - export mwifiex_send_cmd() needed by mwifiex_usb.ko.
> >> ---
> >> drivers/net/wireless/marvell/mwifiex/cmdevt.c | 1 +
> >> drivers/net/wireless/marvell/mwifiex/debugfs.c | 31 +----------------------
> ---
> >> drivers/net/wireless/marvell/mwifiex/pcie.c | 18 +++++++++++++--
> >> drivers/net/wireless/marvell/mwifiex/sdio.c | 12 ++++++++++
> >> drivers/net/wireless/marvell/mwifiex/usb.c | 13 +++++++++++
> >> 5 files changed, 43 insertions(+), 32 deletions(-)
> >
> > You forgot to CC mwifiex maintainers, doing it now. Full patch here:
> >
> > https://patchwork.kernel.org/patch/10403717/
>
> Thanks, Kalle
>
> You would almost think I am not used to submit patches for drivers I do not
> actively maintain ;-)
>
> Gr. AvS
In V2:

return ret;
}
+EXPORT_SYMBOL_GPL(mwifiex_send_cmd);

Instead of exporting 'mwifiex_send_cmd' we can prepare a wrapper for the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex module. Let me know if we can send a follow up.

Thanks,
Ganapathi

2018-05-23 15:54:39

by Kalle Valo

[permalink] [raw]
Subject: Re: [V2,1/6] brcmfmac: move ALLFFMAC variable in flowring module

Arend Van Spriel <[email protected]> wrote:

> The only user of ALLFFMAC is the flowring module so no need to
> expose it in a header file.
>
> Reviewed-by: Hante Meuleman <[email protected]>
> Reviewed-by: Pieter-Paul Giesberts <[email protected]>
> Reviewed-by: Franky Lin <[email protected]>
> Signed-off-by: Arend van Spriel <[email protected]>

6 patches applied to wireless-drivers-next.git, thanks.

f8793c26fe58 brcmfmac: move ALLFFMAC variable in flowring module
8e072168f75e brcmfmac: add support for sysfs initiated coredump
21c5c83ce833 mwifiex: support sysfs initiated device coredump
d2af9b566554 brcmfmac: validate user provided data for memdump before copying
8a3ab2f38f16 brcmfmac: trigger memory dump upon firmware halt signal
b8248236e927 brcmfmac: trigger memory dump on SDIO firmware halt message

--
https://patchwork.kernel.org/patch/10403713/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2018-05-16 12:12:30

by Arend Van Spriel

[permalink] [raw]
Subject: [PATCH V2 6/6] brcmfmac: trigger memory dump on SDIO firmware halt message

From: Franky Lin <[email protected]>

Attempt to dump dongle memory for debug upon receiving firmware halt
message through dongle to host mail box interrupt.

Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 412a05b..c99a191 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -1072,8 +1072,10 @@ static u32 brcmf_sdio_hostmail(struct brcmf_sdio *bus)
bus->sdcnt.f1regdata += 2;

/* dongle indicates the firmware has halted/crashed */
- if (hmb_data & HMB_DATA_FWHALT)
+ if (hmb_data & HMB_DATA_FWHALT) {
brcmf_err("mailbox indicates firmware halted\n");
+ brcmf_dev_coredump(&sdiod->func1->dev);
+ }

/* Dongle recomposed rx frames, accept them again */
if (hmb_data & HMB_DATA_NAKHANDLED) {
--
1.9.1

2018-05-23 11:24:21

by Ganapathi Bhat

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH V2 3/6] mwifiex: support sysfs initiated device coredump

Hi Kalle,

> Arend van Spriel <[email protected]> writes:
>
> > On 5/23/2018 9:54 AM, Kalle Valo wrote:
> >> Ganapathi Bhat <[email protected]> writes:
> >>
> >>> In V2:
> >>>
> >>> return ret;
> >>> }
> >>> +EXPORT_SYMBOL_GPL(mwifiex_send_cmd);
> >>>
> >>> Instead of exporting 'mwifiex_send_cmd' we can prepare a wrapper for
> >>> the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex
> module. Let
> >>> me know if we can send a follow up.
> >>
> >> So what's the plan? Can I apply this patch or should I drop it?
> >
> > Sorry. Had to reply that earlier. When adding the export I figured it
> > might not be desirable. If Ganapathi can fix it with a follow up patch
> > that would ok, right?
>
> A follow up patch sounds good to me.
Sorry for confusions. I assume you will be margining the original patch and want me to send the follow-up right?
>
Regards,
Ganapathi

2018-05-23 08:11:55

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH V2 3/6] mwifiex: support sysfs initiated device coredump

On 5/23/2018 9:54 AM, Kalle Valo wrote:
> Ganapathi Bhat <[email protected]> writes:
>
>> In V2:
>>
>> return ret;
>> }
>> +EXPORT_SYMBOL_GPL(mwifiex_send_cmd);
>>
>> Instead of exporting 'mwifiex_send_cmd' we can prepare a wrapper for
>> the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex module. Let me
>> know if we can send a follow up.
>
> So what's the plan? Can I apply this patch or should I drop it?

Sorry. Had to reply that earlier. When adding the export I figured it
might not be desirable. If Ganapathi can fix it with a follow up patch
that would ok, right? Than you can apply it, I would say.

Regards,
Arend

2018-05-23 07:54:09

by Kalle Valo

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH V2 3/6] mwifiex: support sysfs initiated device coredump

Ganapathi Bhat <[email protected]> writes:

> In V2:
>
> return ret;
> }
> +EXPORT_SYMBOL_GPL(mwifiex_send_cmd);
>
> Instead of exporting 'mwifiex_send_cmd' we can prepare a wrapper for
> the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex module. Let me
> know if we can send a follow up.

So what's the plan? Can I apply this patch or should I drop it?

--
Kalle Valo

2018-05-16 12:12:34

by Arend Van Spriel

[permalink] [raw]
Subject: [PATCH V2 4/6] brcmfmac: validate user provided data for memdump before copying

From: Franky Lin <[email protected]>

In patch "brcmfmac: add support for sysfs initiated coredump", a new
scenario of brcmf_debug_create_memdump was added in which the user of
the function might not necessarily provide prefix data. Hence the
function should not assume the data is always valid and should perform a
check before copying.

Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.c
index 5048320..489b5df 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.c
@@ -40,7 +40,8 @@ int brcmf_debug_create_memdump(struct brcmf_bus *bus, const void *data,
if (!dump)
return -ENOMEM;

- memcpy(dump, data, len);
+ if (data && len > 0)
+ memcpy(dump, data, len);
err = brcmf_bus_get_memdump(bus, dump + len, ramsize);
if (err) {
vfree(dump);
--
1.9.1

2018-05-16 12:12:32

by Arend Van Spriel

[permalink] [raw]
Subject: [PATCH V2 3/6] mwifiex: support sysfs initiated device coredump

Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
it is possible to initiate a device coredump from user-space. This
patch adds support for it adding the .coredump() driver callback.
As there is no longer a need to initiate it through debugfs remove
that code.

Signed-off-by: Arend van Spriel <[email protected]>
---
V2:
- export mwifiex_send_cmd() needed by mwifiex_usb.ko.
---
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 1 +
drivers/net/wireless/marvell/mwifiex/debugfs.c | 31 +-------------------------
drivers/net/wireless/marvell/mwifiex/pcie.c | 18 +++++++++++++--
drivers/net/wireless/marvell/mwifiex/sdio.c | 12 ++++++++++
drivers/net/wireless/marvell/mwifiex/usb.c | 13 +++++++++++
5 files changed, 43 insertions(+), 32 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 9cfcdf6..8be1e69 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -674,6 +674,7 @@ int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,

return ret;
}
+EXPORT_SYMBOL_GPL(mwifiex_send_cmd);

/*
* This function queues a command to the command pending queue.
diff --git a/drivers/net/wireless/marvell/mwifiex/debugfs.c b/drivers/net/wireless/marvell/mwifiex/debugfs.c
index db2872d..0745393 100644
--- a/drivers/net/wireless/marvell/mwifiex/debugfs.c
+++ b/drivers/net/wireless/marvell/mwifiex/debugfs.c
@@ -154,34 +154,6 @@
}

/*
- * Proc device dump read handler.
- *
- * This function is called when the 'device_dump' file is opened for
- * reading.
- * This function dumps driver information and firmware memory segments
- * (ex. DTCM, ITCM, SQRAM etc.) for
- * debugging.
- */
-static ssize_t
-mwifiex_device_dump_read(struct file *file, char __user *ubuf,
- size_t count, loff_t *ppos)
-{
- struct mwifiex_private *priv = file->private_data;
-
- /* For command timeouts, USB firmware will automatically emit
- * firmware dump events, so we don't implement device_dump().
- * For user-initiated dumps, we trigger it ourselves.
- */
- if (priv->adapter->iface_type == MWIFIEX_USB)
- mwifiex_send_cmd(priv, HostCmd_CMD_FW_DUMP_EVENT,
- HostCmd_ACT_GEN_SET, 0, NULL, true);
- else
- priv->adapter->if_ops.device_dump(priv->adapter);
-
- return 0;
-}
-
-/*
* Proc getlog file read handler.
*
* This function is called when the 'getlog' file is opened for reading
@@ -980,7 +952,6 @@
MWIFIEX_DFS_FILE_READ_OPS(info);
MWIFIEX_DFS_FILE_READ_OPS(debug);
MWIFIEX_DFS_FILE_READ_OPS(getlog);
-MWIFIEX_DFS_FILE_READ_OPS(device_dump);
MWIFIEX_DFS_FILE_OPS(regrdwr);
MWIFIEX_DFS_FILE_OPS(rdeeprom);
MWIFIEX_DFS_FILE_OPS(memrw);
@@ -1011,7 +982,7 @@
MWIFIEX_DFS_ADD_FILE(getlog);
MWIFIEX_DFS_ADD_FILE(regrdwr);
MWIFIEX_DFS_ADD_FILE(rdeeprom);
- MWIFIEX_DFS_ADD_FILE(device_dump);
+
MWIFIEX_DFS_ADD_FILE(memrw);
MWIFIEX_DFS_ADD_FILE(hscfg);
MWIFIEX_DFS_ADD_FILE(histogram);
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 7538543..0c42b72 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -320,6 +320,19 @@ static void mwifiex_pcie_shutdown(struct pci_dev *pdev)
return;
}

+static void mwifiex_pcie_coredump(struct device *dev)
+{
+ struct pci_dev *pdev;
+ struct pcie_service_card *card;
+
+ pdev = container_of(dev, struct pci_dev, dev);
+ card = pci_get_drvdata(pdev);
+
+ if (!test_and_set_bit(MWIFIEX_IFACE_WORK_DEVICE_DUMP,
+ &card->work_flags))
+ schedule_work(&card->work);
+}
+
static const struct pci_device_id mwifiex_ids[] = {
{
PCIE_VENDOR_ID_MARVELL, PCIE_DEVICE_ID_MARVELL_88W8766P,
@@ -415,11 +428,12 @@ static SIMPLE_DEV_PM_OPS(mwifiex_pcie_pm_ops, mwifiex_pcie_suspend,
.id_table = mwifiex_ids,
.probe = mwifiex_pcie_probe,
.remove = mwifiex_pcie_remove,
-#ifdef CONFIG_PM_SLEEP
.driver = {
+ .coredump = mwifiex_pcie_coredump,
+#ifdef CONFIG_PM_SLEEP
.pm = &mwifiex_pcie_pm_ops,
- },
#endif
+ },
.shutdown = mwifiex_pcie_shutdown,
.err_handler = &mwifiex_pcie_err_handler,
};
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index a828801..47d2dcc 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -466,6 +466,17 @@ static int mwifiex_sdio_suspend(struct device *dev)
return ret;
}

+static void mwifiex_sdio_coredump(struct device *dev)
+{
+ struct sdio_func *func = dev_to_sdio_func(dev);
+ struct sdio_mmc_card *card;
+
+ card = sdio_get_drvdata(func);
+ if (!test_and_set_bit(MWIFIEX_IFACE_WORK_DEVICE_DUMP,
+ &card->work_flags))
+ schedule_work(&card->work);
+}
+
/* Device ID for SD8786 */
#define SDIO_DEVICE_ID_MARVELL_8786 (0x9116)
/* Device ID for SD8787 */
@@ -515,6 +526,7 @@ static int mwifiex_sdio_suspend(struct device *dev)
.remove = mwifiex_sdio_remove,
.drv = {
.owner = THIS_MODULE,
+ .coredump = mwifiex_sdio_coredump,
.pm = &mwifiex_sdio_pm_ops,
}
};
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 4bc2448..7aa39878 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -653,6 +653,16 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
usb_put_dev(interface_to_usbdev(intf));
}

+static void mwifiex_usb_coredump(struct device *dev)
+{
+ struct usb_interface *intf = to_usb_interface(dev);
+ struct usb_card_rec *card = usb_get_intfdata(intf);
+
+ mwifiex_send_cmd(mwifiex_get_priv(card->adapter, MWIFIEX_BSS_ROLE_ANY),
+ HostCmd_CMD_FW_DUMP_EVENT, HostCmd_ACT_GEN_SET, 0,
+ NULL, true);
+}
+
static struct usb_driver mwifiex_usb_driver = {
.name = "mwifiex_usb",
.probe = mwifiex_usb_probe,
@@ -661,6 +671,9 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
.suspend = mwifiex_usb_suspend,
.resume = mwifiex_usb_resume,
.soft_unbind = 1,
+ .drvwrap.driver = {
+ .coredump = mwifiex_usb_coredump,
+ },
};

static int mwifiex_write_data_sync(struct mwifiex_adapter *adapter, u8 *pbuf,
--
1.9.1

2018-05-16 13:36:26

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH V2 1/6] brcmfmac: move ALLFFMAC variable in flowring module

Arend van Spriel <[email protected]> writes:

> The only user of ALLFFMAC is the flowring module so no need to
> expose it in a header file.
>
> Reviewed-by: Hante Meuleman <[email protected]>
> Reviewed-by: Pieter-Paul Giesberts <[email protected]>
> Reviewed-by: Franky Lin <[email protected]>
> Signed-off-by: Arend van Spriel <[email protected]>
> ---
> Hi Kalle,
>
> I recall Joe Perches posted patches dealing with broadcast address declarations,
> but don't know what happened with those. In patchwork [1] is says 'Not Applicable'
> whatever that means. I noticed Johannes set similar mac80211 change to
> 'Awaiting Upstream'. Anyway, when applying this patch the patch in [1] needs to
> be reworked.
>
> Regards,
> Arend
>
> [1] https://patchwork.kernel.org/patch/10318857/

'Not Applicable' means that I have assumed that the patch is not
suitable for my tree for whatever reason, I guess in this case because
it was part of a bigger patchset.

--
Kalle Valo

2018-05-16 12:12:32

by Arend Van Spriel

[permalink] [raw]
Subject: [PATCH V2 5/6] brcmfmac: trigger memory dump upon firmware halt signal

From: Franky Lin <[email protected]>

PCIe dongle firmware signals a halt/trap through mailbox interrupt.
Trigger a memory dump upon receiving such signal could help to provide
useful information for issue debug.

Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 5baa837..45928b5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -182,6 +182,7 @@ enum brcmf_pcie_state {
#define BRCMF_D2H_DEV_D3_ACK 0x00000001
#define BRCMF_D2H_DEV_DS_ENTER_REQ 0x00000002
#define BRCMF_D2H_DEV_DS_EXIT_NOTE 0x00000004
+#define BRCMF_D2H_DEV_FWHALT 0x10000000

#define BRCMF_H2D_HOST_D3_INFORM 0x00000001
#define BRCMF_H2D_HOST_DS_ACK 0x00000002
@@ -717,6 +718,10 @@ static void brcmf_pcie_handle_mb_data(struct brcmf_pciedev_info *devinfo)
devinfo->mbdata_completed = true;
wake_up(&devinfo->mbdata_resp_wait);
}
+ if (dtoh_mb_data & BRCMF_D2H_DEV_FWHALT) {
+ brcmf_dbg(PCIE, "D2H_MB_DATA: FW HALT\n");
+ brcmf_dev_coredump(&devinfo->pdev->dev);
+ }
}


--
1.9.1