diff mbox

[2/5] mwifiex: add firmware dump feature for PCIe

Message ID 1397760423-11455-2-git-send-email-bzhao@marvell.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Bing Zhao April 17, 2014, 6:47 p.m. UTC
From: Amitkumar Karwar <akarwar@marvell.com>

Firmware dump feature is added for PCIe based chipsets.
Separate file will be created at /var/log/fw_dump_*
for each memory segment.

Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
 drivers/net/wireless/mwifiex/cmdevt.c |   3 +
 drivers/net/wireless/mwifiex/main.c   |   2 +
 drivers/net/wireless/mwifiex/main.h   |   2 +
 drivers/net/wireless/mwifiex/pcie.c   | 227 ++++++++++++++++++++++++++++++++++
 drivers/net/wireless/mwifiex/pcie.h   |  27 ++++
 5 files changed, 261 insertions(+)

Comments

Kalle Valo April 24, 2014, 9:08 a.m. UTC | #1
Bing Zhao <bzhao@marvell.com> writes:

> From: Amitkumar Karwar <akarwar@marvell.com>
>
> Firmware dump feature is added for PCIe based chipsets.
> Separate file will be created at /var/log/fw_dump_*
> for each memory segment.
>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> Signed-off-by: Bing Zhao <bzhao@marvell.com>

Sorry for commenting so late, I was on holidays and then got lagged
behind with email.

> +			memset(filename, 0, sizeof(filename));
> +			memcpy(filename, name_prefix, strlen(name_prefix));
> +			strcat(filename, entry->mem_name);
> +			do_gettimeofday(&t);
> +			entry->file_mem = filp_open(filename, O_CREAT | O_RDWR,
> +						    0644);
> +			if (IS_ERR(entry->file_mem)) {
> +				dev_info(adapter->dev,
> +					 "Create %s file failed at %s, opening another dir /tmp\n",
> +					 entry->mem_name, filename);
> +				memset(filename, 0, sizeof(filename));
> +				sprintf(filename, "%s%s", "/tmp/fw_dump_",
> +					entry->mem_name);
> +				entry->file_mem =
> +					filp_open(filename,
> +						  O_CREAT | O_RDWR, 0644);
> +			}
> +			if (!IS_ERR(entry->file_mem)) {
> +				dev_info(adapter->dev,
> +					 "Start to save the output : %u.%06u, please wait...\n",
> +					 (u32)t.tv_sec, (u32)t.tv_usec);

Like I said in my previous email, IMHO a wireless driver should not save
any files to the filesystem. For example, I don't see any other uses of
filp_open() in drivers/net. Ugly hacks like this belong to staging
drivers, not to proper upstream drivers.
John W. Linville April 25, 2014, 1:37 a.m. UTC | #2
On Thu, Apr 24, 2014 at 12:08:59PM +0300, Kalle Valo wrote:
> Bing Zhao <bzhao@marvell.com> writes:
> 
> > From: Amitkumar Karwar <akarwar@marvell.com>
> >
> > Firmware dump feature is added for PCIe based chipsets.
> > Separate file will be created at /var/log/fw_dump_*
> > for each memory segment.
> >
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > Signed-off-by: Bing Zhao <bzhao@marvell.com>
> 
> Sorry for commenting so late, I was on holidays and then got lagged
> behind with email.
> 
> > +			memset(filename, 0, sizeof(filename));
> > +			memcpy(filename, name_prefix, strlen(name_prefix));
> > +			strcat(filename, entry->mem_name);
> > +			do_gettimeofday(&t);
> > +			entry->file_mem = filp_open(filename, O_CREAT | O_RDWR,
> > +						    0644);
> > +			if (IS_ERR(entry->file_mem)) {
> > +				dev_info(adapter->dev,
> > +					 "Create %s file failed at %s, opening another dir /tmp\n",
> > +					 entry->mem_name, filename);
> > +				memset(filename, 0, sizeof(filename));
> > +				sprintf(filename, "%s%s", "/tmp/fw_dump_",
> > +					entry->mem_name);
> > +				entry->file_mem =
> > +					filp_open(filename,
> > +						  O_CREAT | O_RDWR, 0644);
> > +			}
> > +			if (!IS_ERR(entry->file_mem)) {
> > +				dev_info(adapter->dev,
> > +					 "Start to save the output : %u.%06u, please wait...\n",
> > +					 (u32)t.tv_sec, (u32)t.tv_usec);
> 
> Like I said in my previous email, IMHO a wireless driver should not save
> any files to the filesystem. For example, I don't see any other uses of
> filp_open() in drivers/net. Ugly hacks like this belong to staging
> drivers, not to proper upstream drivers.

I'm sorry for letting this slip through.  I must have had too much rum or something...

I'm reverting this patch in wireless-next -- drivers should not be
making filesystem calls like that.  Even if you can argue for doing
so somehow, where the file would go would be a policy decision that
should not be encoded in the driver.

John
Bing Zhao April 25, 2014, 3:35 a.m. UTC | #3
> > Sorry for commenting so late, I was on holidays and then got lagged
> > behind with email.

Hi Kalle,

Thanks for your comments.

> > Like I said in my previous email, IMHO a wireless driver should not save
> > any files to the filesystem. For example, I don't see any other uses of
> > filp_open() in drivers/net. Ugly hacks like this belong to staging
> > drivers, not to proper upstream drivers.
> 
> I'm sorry for letting this slip through.  I must have had too much rum or something...
> 
> I'm reverting this patch in wireless-next -- drivers should not be
> making filesystem calls like that.  Even if you can argue for doing
> so somehow, where the file would go would be a policy decision that
> should not be encoded in the driver.

Hi John,

Sorry about the hassle. We will find a proper way to implement this.

Thanks,
Bing




--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg April 25, 2014, 8:09 a.m. UTC | #4
On Thu, 2014-04-24 at 20:35 -0700, Bing Zhao wrote:

> Sorry about the hassle. We will find a proper way to implement this.

FWIW, a pretty simple way is to trigger a uevent or so and have a
debugfs file that all the info can be read out from in userspace.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kalle Valo April 30, 2014, 7:41 a.m. UTC | #5
Johannes Berg <johannes@sipsolutions.net> writes:

> On Thu, 2014-04-24 at 20:35 -0700, Bing Zhao wrote:
>
>> Sorry about the hassle. We will find a proper way to implement this.
>
> FWIW, a pretty simple way is to trigger a uevent or so and have a
> debugfs file that all the info can be read out from in userspace.

I think we need a similar feature in ath10k as well. Should we have a
generic interface this, would that make any sense?
Johannes Berg April 30, 2014, 8:29 a.m. UTC | #6
On Wed, 2014-04-30 at 10:41 +0300, Kalle Valo wrote:
> Johannes Berg <johannes@sipsolutions.net> writes:
> 
> > On Thu, 2014-04-24 at 20:35 -0700, Bing Zhao wrote:
> >
> >> Sorry about the hassle. We will find a proper way to implement this.
> >
> > FWIW, a pretty simple way is to trigger a uevent or so and have a
> > debugfs file that all the info can be read out from in userspace.
> 
> I think we need a similar feature in ath10k as well. Should we have a
> generic interface this, would that make any sense?

We have it in iwlwifi as well - we were considering a generic uevent but
beyond that I don't think there's much that could be done?

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John W. Linville April 30, 2014, 1:22 p.m. UTC | #7
On Wed, Apr 30, 2014 at 10:29:53AM +0200, Johannes Berg wrote:
> On Wed, 2014-04-30 at 10:41 +0300, Kalle Valo wrote:
> > Johannes Berg <johannes@sipsolutions.net> writes:
> > 
> > > On Thu, 2014-04-24 at 20:35 -0700, Bing Zhao wrote:
> > >
> > >> Sorry about the hassle. We will find a proper way to implement this.
> > >
> > > FWIW, a pretty simple way is to trigger a uevent or so and have a
> > > debugfs file that all the info can be read out from in userspace.
> > 
> > I think we need a similar feature in ath10k as well. Should we have a
> > generic interface this, would that make any sense?
> 
> We have it in iwlwifi as well - we were considering a generic uevent but
> beyond that I don't think there's much that could be done?

There is an ethtool op for that, no?
Kalle Valo April 30, 2014, 4:13 p.m. UTC | #8
Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2014-04-30 at 10:41 +0300, Kalle Valo wrote:
>> Johannes Berg <johannes@sipsolutions.net> writes:
>> 
>> > On Thu, 2014-04-24 at 20:35 -0700, Bing Zhao wrote:
>> >
>> >> Sorry about the hassle. We will find a proper way to implement this.
>> >
>> > FWIW, a pretty simple way is to trigger a uevent or so and have a
>> > debugfs file that all the info can be read out from in userspace.
>> 
>> I think we need a similar feature in ath10k as well. Should we have a
>> generic interface this, would that make any sense?
>
> We have it in iwlwifi as well - we were considering a generic uevent but
> beyond that I don't think there's much that could be done?

I was thinking that it would be nice to avoid rewriting a separate user
space dumping tool for each driver and instead have a comman way to
retrieve the blobs from the driver.
Kalle Valo April 30, 2014, 4:14 p.m. UTC | #9
"John W. Linville" <linville@tuxdriver.com> writes:

> On Wed, Apr 30, 2014 at 10:29:53AM +0200, Johannes Berg wrote:
>> On Wed, 2014-04-30 at 10:41 +0300, Kalle Valo wrote:
>> > Johannes Berg <johannes@sipsolutions.net> writes:
>> > 
>> > > On Thu, 2014-04-24 at 20:35 -0700, Bing Zhao wrote:
>> > >
>> > >> Sorry about the hassle. We will find a proper way to implement this.
>> > >
>> > > FWIW, a pretty simple way is to trigger a uevent or so and have a
>> > > debugfs file that all the info can be read out from in userspace.
>> > 
>> > I think we need a similar feature in ath10k as well. Should we have a
>> > generic interface this, would that make any sense?
>> 
>> We have it in iwlwifi as well - we were considering a generic uevent but
>> beyond that I don't think there's much that could be done?
>
> There is an ethtool op for that, no?

Do you mean this one:

 * @get_dump_flag: Get dump flag indicating current dump length, version,
 * 		   and flag of the device.
 * @get_dump_data: Get dump data.
 * @set_dump: Set dump specific flags to the device.

	int	(*get_dump_flag)(struct net_device *, struct ethtool_dump *);
	int	(*get_dump_data)(struct net_device *,
				 struct ethtool_dump *, void *);
	int	(*set_dump)(struct net_device *, struct ethtool_dump *);

Yeah, maybe we could use that in wireless as well.
Johannes Berg May 5, 2014, 12:57 p.m. UTC | #10
On Wed, 2014-04-30 at 19:14 +0300, Kalle Valo wrote:

> >> We have it in iwlwifi as well - we were considering a generic uevent but
> >> beyond that I don't think there's much that could be done?
> >
> > There is an ethtool op for that, no?
> 
> Do you mean this one:
> 
>  * @get_dump_flag: Get dump flag indicating current dump length, version,
>  * 		   and flag of the device.
>  * @get_dump_data: Get dump data.
>  * @set_dump: Set dump specific flags to the device.
> 
> 	int	(*get_dump_flag)(struct net_device *, struct ethtool_dump *);
> 	int	(*get_dump_data)(struct net_device *,
> 				 struct ethtool_dump *, void *);
> 	int	(*set_dump)(struct net_device *, struct ethtool_dump *);
> 
> Yeah, maybe we could use that in wireless as well.

What was the intended use case for this though? Neither the header file
nor the ethtool documentation seem particularly clear, other than saying
"firmware dump" - we'll want to include driver data as well though?

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg May 5, 2014, 12:58 p.m. UTC | #11
On Wed, 2014-04-30 at 19:13 +0300, Kalle Valo wrote:

> I was thinking that it would be nice to avoid rewriting a separate user
> space dumping tool for each driver and instead have a comman way to
> retrieve the blobs from the driver.

Ours is pretty much just "cat /path/to/some/debugfs/file > wherever",
but indeed it would make sense to have a common uevent and dump
mechanism.

The ethtool dump thing seems reasonable to me, who wants to implement
it? The generic uevent should probably be some cfg80211 API then I
guess?

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amitkumar Karwar May 5, 2014, 2:19 p.m. UTC | #12
Hi Johannes,

> >
> > 	int	(*get_dump_flag)(struct net_device *, struct ethtool_dump *);
> > 	int	(*get_dump_data)(struct net_device *,
> > 				 struct ethtool_dump *, void *);
> > 	int	(*set_dump)(struct net_device *, struct ethtool_dump *);
> >
> > Yeah, maybe we could use that in wireless as well.
> 
> What was the intended use case for this though?

In mwifiex, we are interested in providing separate firmware dump file to user for each memory segment. Ex. ITCM, DTCM, SQRAM etc.

It looks simpler with ethtool ops instead of creating multiple debugfs files.
 
1) User can set index for required memory segment.

#ethtool --set-dump wlan0 3

2) Get the details

#ethtool --get-dump wlan0
flag: 3, version:xyz, length:13450

3) Dump the info in ITCM.log file

#ethtool --get-dump wlan0 data ITCM.log


Common uevent through cfg80211 API to notify firmware dump is over would be useful for us.

Thanks,
Amitkumar Karwar
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg May 5, 2014, 3:04 p.m. UTC | #13
On Mon, 2014-05-05 at 07:19 -0700, Amitkumar Karwar wrote:

> In mwifiex, we are interested in providing separate firmware dump file to user for each memory segment. Ex. ITCM, DTCM, SQRAM etc.

We have multiple things as well, firmware memory, last driver
commands, ...

> It looks simpler with ethtool ops instead of creating multiple debugfs files.

What we decided to do is encode the file with an IE-like structure so we
can extend the format in the future.
 
> 1) User can set index for required memory segment.
> 
> #ethtool --set-dump wlan0 3
> 
> 2) Get the details
> 
> #ethtool --get-dump wlan0
> flag: 3, version:xyz, length:13450
> 
> 3) Dump the info in ITCM.log file
> 
> #ethtool --get-dump wlan0 data ITCM.log

This works, though I'd be worried about getting a consistent snapshot,
no?

> Common uevent through cfg80211 API to notify firmware dump is over would be useful for us.

That makes some sense.


The way our code works is that it stores all the desired data on an
error, triggers the event & restart, and then you can request the data
later.

I'm not sure ethtool dump is appropriate for that - it seems to be more
used for "get a dump right now during operation" type scenarios?

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amitkumar Karwar May 5, 2014, 3:49 p.m. UTC | #14
Hi Johannes,

> > 3) Dump the info in ITCM.log file
> >
> > #ethtool --get-dump wlan0 data ITCM.log
> 
> This works, though I'd be worried about getting a consistent snapshot,
> no?

Actually we don't need consistent snapshots for firmware dump in mwifiex. 
When command/Tx data timeout occurs, one time firmware memory dump (total ~1.1Mb) is sufficient for debugging. 

> 
> > Common uevent through cfg80211 API to notify firmware dump is over would
> be useful for us.
> 
> That makes some sense.
> 
> 
> The way our code works is that it stores all the desired data on an
> error, triggers the event & restart, and then you can request the data
> later.
> 
> I'm not sure ethtool dump is appropriate for that - it seems to be more
> used for "get a dump right now during operation" type scenarios?
 
We are planning to use ethtool to selectively get required info when common cfg80211 API notifies that firmware dump is over. If the operation is 
in progress, we can return 0 to ethtool.

Thanks,
Amit
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/wireless/mwifiex/cmdevt.c b/drivers/net/wireless/mwifiex/cmdevt.c
index 8dee6c8..421322f 100644
--- a/drivers/net/wireless/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/mwifiex/cmdevt.c
@@ -960,6 +960,9 @@  mwifiex_cmd_timeout_func(unsigned long function_context)
 	if (adapter->hw_status == MWIFIEX_HW_STATUS_INITIALIZING)
 		mwifiex_init_fw_complete(adapter);
 
+	if (adapter->if_ops.fw_dump)
+		adapter->if_ops.fw_dump(adapter);
+
 	if (adapter->if_ops.card_reset)
 		adapter->if_ops.card_reset(adapter);
 }
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index cbabc12..6bc645a 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -881,6 +881,8 @@  mwifiex_add_card(void *card, struct semaphore *sem,
 		goto err_kmalloc;
 
 	INIT_WORK(&adapter->main_work, mwifiex_main_work_queue);
+	if (adapter->if_ops.iface_work)
+		INIT_WORK(&adapter->iface_work, adapter->if_ops.iface_work);
 
 	/* Register the device. Fill up the private data structure with relevant
 	   information from the card. */
diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index 3418119..d70457b 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -674,6 +674,7 @@  struct mwifiex_if_ops {
 	void (*card_reset) (struct mwifiex_adapter *);
 	void (*fw_dump)(struct mwifiex_adapter *);
 	int (*clean_pcie_ring) (struct mwifiex_adapter *adapter);
+	void (*iface_work)(struct work_struct *work);
 };
 
 struct mwifiex_adapter {
@@ -809,6 +810,7 @@  struct mwifiex_adapter {
 	bool ext_scan;
 	u8 fw_api_ver;
 	u8 fw_key_api_major_ver, fw_key_api_minor_ver;
+	struct work_struct iface_work;
 };
 
 int mwifiex_init_lock_list(struct mwifiex_adapter *adapter);
diff --git a/drivers/net/wireless/mwifiex/pcie.c b/drivers/net/wireless/mwifiex/pcie.c
index c2cfeec..51989b3 100644
--- a/drivers/net/wireless/mwifiex/pcie.c
+++ b/drivers/net/wireless/mwifiex/pcie.c
@@ -37,6 +37,9 @@  static struct mwifiex_if_ops pcie_ops;
 
 static struct semaphore add_remove_card_sem;
 
+/* enum mwifiex_pcie_work_flags bitmap */
+static unsigned long pcie_work_flags;
+
 static int
 mwifiex_map_pci_memory(struct mwifiex_adapter *adapter, struct sk_buff *skb,
 		       size_t size, int flags)
@@ -221,6 +224,8 @@  static void mwifiex_pcie_remove(struct pci_dev *pdev)
 	if (!adapter || !adapter->priv_num)
 		return;
 
+	cancel_work_sync(&adapter->iface_work);
+
 	if (user_rmmod) {
 #ifdef CONFIG_PM_SLEEP
 		if (adapter->is_suspended)
@@ -307,6 +312,17 @@  static int mwifiex_read_reg(struct mwifiex_adapter *adapter, int reg, u32 *data)
 	return 0;
 }
 
+/* This function reads u8 data from PCIE card register. */
+static int mwifiex_read_reg_byte(struct mwifiex_adapter *adapter,
+				 int reg, u8 *data)
+{
+	struct pcie_service_card *card = adapter->card;
+
+	*data = ioread8(card->pci_mmap1 + reg);
+
+	return 0;
+}
+
 /*
  * This function adds delay loop to ensure FW is awake before proceeding.
  */
@@ -2172,6 +2188,215 @@  static int mwifiex_pcie_host_to_card(struct mwifiex_adapter *adapter, u8 type,
 	return 0;
 }
 
+/* This function read/write firmware */
+static enum rdwr_status
+mwifiex_pcie_rdwr_firmware(struct mwifiex_adapter *adapter, u8 doneflag)
+{
+	int ret, tries;
+	u8 ctrl_data;
+
+	ret = mwifiex_write_reg(adapter, DEBUG_DUMP_CTRL_REG, DEBUG_HOST_READY);
+	if (ret) {
+		dev_err(adapter->dev, "PCIE write err\n");
+		return RDWR_STATUS_FAILURE;
+	}
+
+	for (tries = 0; tries < MAX_POLL_TRIES; tries++) {
+		mwifiex_read_reg_byte(adapter, DEBUG_DUMP_CTRL_REG, &ctrl_data);
+		if (ctrl_data == DEBUG_FW_DONE)
+			return RDWR_STATUS_SUCCESS;
+		if (doneflag && ctrl_data == doneflag)
+			return RDWR_STATUS_DONE;
+		if (ctrl_data != DEBUG_HOST_READY) {
+			dev_info(adapter->dev,
+				 "The ctrl reg was changed, re-try again!\n");
+			mwifiex_write_reg(adapter, DEBUG_DUMP_CTRL_REG,
+					  DEBUG_HOST_READY);
+			if (ret) {
+				dev_err(adapter->dev, "PCIE write err\n");
+				return RDWR_STATUS_FAILURE;
+			}
+		}
+		usleep_range(100, 200);
+	}
+
+	dev_err(adapter->dev, "Fail to pull ctrl_data\n");
+	return RDWR_STATUS_FAILURE;
+}
+
+/* This function dump firmware memory to file */
+static void mwifiex_pcie_fw_dump_work(struct work_struct *work)
+{
+	struct mwifiex_adapter *adapter =
+			container_of(work, struct mwifiex_adapter, iface_work);
+	unsigned int reg, reg_start, reg_end;
+	u8 *dbg_ptr;
+	struct timeval t;
+	u8 dump_num = 0, idx, i, read_reg, doneflag = 0;
+	enum rdwr_status stat;
+	u32 memory_size;
+	u8 filename[MAX_FULL_NAME_LEN];
+	mm_segment_t fs;
+	loff_t pos;
+	u8 *end_ptr;
+	u8 *name_prefix = "/var/log/fw_dump_";
+	struct memory_type_mapping mem_type_mapping_tbl[] = {
+		{"ITCM", NULL, NULL, 0xF0},
+		{"DTCM", NULL, NULL, 0xF1},
+		{"SQRAM", NULL, NULL, 0xF2},
+		{"IRAM", NULL, NULL, 0xF3},
+	};
+
+	if (!adapter) {
+		dev_err(adapter->dev, "Could not dump firmwware info\n");
+		return;
+	}
+
+	do_gettimeofday(&t);
+	dev_info(adapter->dev, "== mwifiex firmware dump start: %u.%06u ==\n",
+		 (u32)t.tv_sec, (u32)t.tv_usec);
+
+	/* Read the number of the memories which will dump */
+	stat = mwifiex_pcie_rdwr_firmware(adapter, doneflag);
+	if (stat == RDWR_STATUS_FAILURE)
+		goto done;
+
+	reg = DEBUG_DUMP_START_REG;
+	mwifiex_read_reg_byte(adapter, reg, &dump_num);
+
+	/* Read the length of every memory which will dump */
+	for (idx = 0; idx < dump_num; idx++) {
+		struct memory_type_mapping *entry = &mem_type_mapping_tbl[idx];
+
+		stat = mwifiex_pcie_rdwr_firmware(adapter, doneflag);
+		if (stat == RDWR_STATUS_FAILURE)
+			goto done;
+
+		memory_size = 0;
+		reg = DEBUG_DUMP_START_REG;
+		for (i = 0; i < 4; i++) {
+			mwifiex_read_reg_byte(adapter, reg, &read_reg);
+			memory_size |= (read_reg << (i * 8));
+			reg++;
+		}
+
+		if (memory_size == 0) {
+			dev_info(adapter->dev, "Firmware dump Finished!\n");
+			break;
+		}
+
+		dev_info(adapter->dev,
+			 "%s_SIZE=0x%x\n", entry->mem_name, memory_size);
+		entry->mem_ptr = vmalloc(memory_size + 1);
+		if (!entry->mem_ptr) {
+			dev_err(adapter->dev,
+				"Vmalloc %s failed\n", entry->mem_name);
+			goto done;
+		}
+		dbg_ptr = entry->mem_ptr;
+		end_ptr = dbg_ptr + memory_size;
+
+		doneflag = entry->done_flag;
+		do_gettimeofday(&t);
+		dev_info(adapter->dev, "Start %s output %u.%06u, please wait...\n",
+			 entry->mem_name, (u32)t.tv_sec, (u32)t.tv_usec);
+
+		do {
+			stat = mwifiex_pcie_rdwr_firmware(adapter, doneflag);
+			if (RDWR_STATUS_FAILURE == stat)
+				goto done;
+
+			reg_start = DEBUG_DUMP_START_REG;
+			reg_end = DEBUG_DUMP_END_REG;
+			for (reg = reg_start; reg <= reg_end; reg++) {
+				mwifiex_read_reg_byte(adapter, reg, dbg_ptr);
+				if (dbg_ptr < end_ptr)
+					dbg_ptr++;
+				else
+					dev_err(adapter->dev,
+						"Allocated buf not enough\n");
+			}
+
+			if (stat != RDWR_STATUS_DONE)
+				continue;
+
+			dev_info(adapter->dev, "%s done: size=0x%lx\n",
+				 entry->mem_name, dbg_ptr - entry->mem_ptr);
+			memset(filename, 0, sizeof(filename));
+			memcpy(filename, name_prefix, strlen(name_prefix));
+			strcat(filename, entry->mem_name);
+			do_gettimeofday(&t);
+			entry->file_mem = filp_open(filename, O_CREAT | O_RDWR,
+						    0644);
+			if (IS_ERR(entry->file_mem)) {
+				dev_info(adapter->dev,
+					 "Create %s file failed at %s, opening another dir /tmp\n",
+					 entry->mem_name, filename);
+				memset(filename, 0, sizeof(filename));
+				sprintf(filename, "%s%s", "/tmp/fw_dump_",
+					entry->mem_name);
+				entry->file_mem =
+					filp_open(filename,
+						  O_CREAT | O_RDWR, 0644);
+			}
+			if (!IS_ERR(entry->file_mem)) {
+				dev_info(adapter->dev,
+					 "Start to save the output : %u.%06u, please wait...\n",
+					 (u32)t.tv_sec, (u32)t.tv_usec);
+				fs = get_fs();
+				set_fs(KERNEL_DS);
+				pos = 0;
+				vfs_write(entry->file_mem,
+					  (char __user *)entry->mem_ptr,
+					  memory_size, &pos);
+				filp_close(entry->file_mem, NULL);
+				set_fs(fs);
+				dev_info(adapter->dev,
+					 "The output %s have been saved to file successfully!\n",
+					 entry->mem_name);
+			} else {
+				dev_err(adapter->dev,
+					"Failed to create file %s\n", filename);
+			}
+			vfree(entry->mem_ptr);
+			entry->mem_ptr = NULL;
+			break;
+		} while (true);
+	}
+	do_gettimeofday(&t);
+	dev_info(adapter->dev, "== mwifiex firmware dump end: %u.%06u ==\n",
+		 (u32)t.tv_sec, (u32)t.tv_usec);
+
+done:
+	for (idx = 0; idx < ARRAY_SIZE(mem_type_mapping_tbl); idx++) {
+		struct memory_type_mapping *entry = &mem_type_mapping_tbl[idx];
+
+		if (entry->mem_ptr) {
+			vfree(entry->mem_ptr);
+			entry->mem_ptr = NULL;
+		}
+	}
+
+	return;
+}
+
+static void mwifiex_pcie_work(struct work_struct *work)
+{
+	if (test_and_clear_bit(MWIFIEX_PCIE_WORK_FW_DUMP, &pcie_work_flags))
+		mwifiex_pcie_fw_dump_work(work);
+}
+
+/* This function dumps FW information */
+static void mwifiex_pcie_fw_dump(struct mwifiex_adapter *adapter)
+{
+	if (test_bit(MWIFIEX_PCIE_WORK_FW_DUMP, &pcie_work_flags))
+		return;
+
+	set_bit(MWIFIEX_PCIE_WORK_FW_DUMP, &pcie_work_flags);
+
+	schedule_work(&adapter->iface_work);
+}
+
 /*
  * This function initializes the PCI-E host memory space, WCB rings, etc.
  *
@@ -2393,6 +2618,8 @@  static struct mwifiex_if_ops pcie_ops = {
 	.cleanup_mpa_buf =		NULL,
 	.init_fw_port =			mwifiex_pcie_init_fw_port,
 	.clean_pcie_ring =		mwifiex_clean_pcie_ring_buf,
+	.fw_dump =			mwifiex_pcie_fw_dump,
+	.iface_work =			mwifiex_pcie_work,
 };
 
 /*
diff --git a/drivers/net/wireless/mwifiex/pcie.h b/drivers/net/wireless/mwifiex/pcie.h
index e8ec561..3abba32 100644
--- a/drivers/net/wireless/mwifiex/pcie.h
+++ b/drivers/net/wireless/mwifiex/pcie.h
@@ -100,6 +100,28 @@ 
 #define MWIFIEX_DEF_SLEEP_COOKIE			0xBEEFBEEF
 #define MWIFIEX_MAX_DELAY_COUNT				5
 
+#define DEBUG_DUMP_CTRL_REG				0xCF4
+#define DEBUG_DUMP_START_REG				0xCF8
+#define DEBUG_DUMP_END_REG				0xCFF
+#define DEBUG_HOST_READY				0xEE
+#define DEBUG_FW_DONE					0xFF
+
+#define MAX_NAME_LEN					8
+#define MAX_FULL_NAME_LEN				32
+
+struct memory_type_mapping {
+	u8 mem_name[MAX_NAME_LEN];
+	u8 *mem_ptr;
+	struct file *file_mem;
+	u8 done_flag;
+};
+
+enum rdwr_status {
+	RDWR_STATUS_SUCCESS = 0,
+	RDWR_STATUS_FAILURE = 1,
+	RDWR_STATUS_DONE = 2
+};
+
 struct mwifiex_pcie_card_reg {
 	u16 cmd_addr_lo;
 	u16 cmd_addr_hi;
@@ -322,4 +344,9 @@  mwifiex_pcie_txbd_not_full(struct pcie_service_card *card)
 
 	return 0;
 }
+
+enum mwifiex_pcie_work_flags {
+	MWIFIEX_PCIE_WORK_FW_DUMP,
+};
+
 #endif /* _MWIFIEX_PCIE_H */