[PATCH 1/5] mwifiex: add fw_dump debugfs file

[PATCH 1/5] mwifiex: add fw_dump debugfs file

[Bing Zhao]

From: Amitkumar Karwar <akarwar@marvell.com>

This option be useful to dump firmware memory for debugging
purpose. Actual code to dump firmware momory for SDIO and PCIe
chipsets will be added later.

Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
 drivers/net/wireless/mwifiex/README    |  7 +++++++
 drivers/net/wireless/mwifiex/debugfs.c | 25 +++++++++++++++++++++++++
 drivers/net/wireless/mwifiex/main.h    |  1 +
 3 files changed, 33 insertions(+)

diff --git a/drivers/net/wireless/mwifiex/README b/drivers/net/wireless/mwifiex/README
index b9242c3..3b55ce5 100644
--- a/drivers/net/wireless/mwifiex/README
+++ b/drivers/net/wireless/mwifiex/README
@@ -200,4 +200,11 @@ getlog
 
 	cat getlog
 
+fw_dump
+	This command is used to dump firmware memory into files.
+	Separate file will be created for each memory segment.
+	Usage:
+
+	cat fw_dump
+
 ===============================================================================
diff --git a/drivers/net/wireless/mwifiex/debugfs.c b/drivers/net/wireless/mwifiex/debugfs.c
index b8a49aa..7b419bb 100644
--- a/drivers/net/wireless/mwifiex/debugfs.c
+++ b/drivers/net/wireless/mwifiex/debugfs.c
@@ -257,6 +257,29 @@ free_and_exit:
 }
 
 /*
+ * Proc firmware dump read handler.
+ *
+ * This function is called when the 'fw_dump' file is opened for
+ * reading.
+ * This function dumps firmware memory in different files
+ * (ex. DTCM, ITCM, SQRAM etc.) based on the the segments for
+ * debugging.
+ */
+static ssize_t
+mwifiex_fw_dump_read(struct file *file, char __user *ubuf,
+		     size_t count, loff_t *ppos)
+{
+	struct mwifiex_private *priv = file->private_data;
+
+	if (!priv->adapter->if_ops.fw_dump)
+		return -EIO;
+
+	priv->adapter->if_ops.fw_dump(priv->adapter);
+
+	return 0;
+}
+
+/*
  * Proc getlog file read handler.
  *
  * This function is called when the 'getlog' file is opened for reading
@@ -699,6 +722,7 @@ static const struct file_operations mwifiex_dfs_##name##_fops = {       \
 MWIFIEX_DFS_FILE_READ_OPS(info);
 MWIFIEX_DFS_FILE_READ_OPS(debug);
 MWIFIEX_DFS_FILE_READ_OPS(getlog);
+MWIFIEX_DFS_FILE_READ_OPS(fw_dump);
 MWIFIEX_DFS_FILE_OPS(regrdwr);
 MWIFIEX_DFS_FILE_OPS(rdeeprom);
 
@@ -722,6 +746,7 @@ mwifiex_dev_debugfs_init(struct mwifiex_private *priv)
 	MWIFIEX_DFS_ADD_FILE(getlog);
 	MWIFIEX_DFS_ADD_FILE(regrdwr);
 	MWIFIEX_DFS_ADD_FILE(rdeeprom);
+	MWIFIEX_DFS_ADD_FILE(fw_dump);
 }
 
 /*
diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index ae8b042..3418119 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -672,6 +672,7 @@ struct mwifiex_if_ops {
 	int (*init_fw_port) (struct mwifiex_adapter *);
 	int (*dnld_fw) (struct mwifiex_adapter *, struct mwifiex_fw_image *);
 	void (*card_reset) (struct mwifiex_adapter *);
+	void (*fw_dump)(struct mwifiex_adapter *);
 	int (*clean_pcie_ring) (struct mwifiex_adapter *adapter);
 };
 
-- 
1.8.2.3

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

[Bing Zhao]

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(+)

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 */
-- 
1.8.2.3

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

[Kalle Valo]

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.

-- 
Kalle Valo

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

[John W. Linville]

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
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

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

[Bing Zhao]

> > 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

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

[Johannes Berg]

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

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

[Kalle Valo]

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?

-- 
Kalle Valo

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

[Johannes Berg]

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

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

[John W. Linville]

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?

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

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

[Kalle Valo]

"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.

-- 
Kalle Valo

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

[Johannes Berg]

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

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

[Amitkumar Karwar]

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

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

[Johannes Berg]

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

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

[Amitkumar Karwar]

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

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

[Kalle Valo]

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

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

[Johannes Berg]

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

[PATCH 3/5] mwifiex: increase tx/rx AMPDU window sizes for STA 11n mode

[Bing Zhao]

From: Amitkumar Karwar <akarwar@marvell.com>

This will help to aggregate more packets which yields better
throughput results.

Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Avinash Patil <patila@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
 drivers/net/wireless/mwifiex/decl.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/decl.h b/drivers/net/wireless/mwifiex/decl.h
index e7b3e16..b8e15ee 100644
--- a/drivers/net/wireless/mwifiex/decl.h
+++ b/drivers/net/wireless/mwifiex/decl.h
@@ -42,8 +42,8 @@
 #define MWIFIEX_MAX_TX_BASTREAM_SUPPORTED	2
 #define MWIFIEX_MAX_RX_BASTREAM_SUPPORTED	16
 
-#define MWIFIEX_STA_AMPDU_DEF_TXWINSIZE        16
-#define MWIFIEX_STA_AMPDU_DEF_RXWINSIZE        32
+#define MWIFIEX_STA_AMPDU_DEF_TXWINSIZE        64
+#define MWIFIEX_STA_AMPDU_DEF_RXWINSIZE        64
 #define MWIFIEX_UAP_AMPDU_DEF_TXWINSIZE        32
 #define MWIFIEX_UAP_AMPDU_DEF_RXWINSIZE        16
 #define MWIFIEX_11AC_STA_AMPDU_DEF_TXWINSIZE   32
-- 
1.8.2.3

[PATCH 4/5] mwifiex: increase tx/rx AMPDU window sizes for STA 11ac mode

[Bing Zhao]

From: Amitkumar Karwar <akarwar@marvell.com>

This will help to aggregate more packets which yields better
throughput results for 11ac chipsets.

Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Avinash Patil <patila@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
 drivers/net/wireless/mwifiex/decl.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/decl.h b/drivers/net/wireless/mwifiex/decl.h
index b8e15ee..38da6ff 100644
--- a/drivers/net/wireless/mwifiex/decl.h
+++ b/drivers/net/wireless/mwifiex/decl.h
@@ -46,8 +46,8 @@
 #define MWIFIEX_STA_AMPDU_DEF_RXWINSIZE        64
 #define MWIFIEX_UAP_AMPDU_DEF_TXWINSIZE        32
 #define MWIFIEX_UAP_AMPDU_DEF_RXWINSIZE        16
-#define MWIFIEX_11AC_STA_AMPDU_DEF_TXWINSIZE   32
-#define MWIFIEX_11AC_STA_AMPDU_DEF_RXWINSIZE   48
+#define MWIFIEX_11AC_STA_AMPDU_DEF_TXWINSIZE   64
+#define MWIFIEX_11AC_STA_AMPDU_DEF_RXWINSIZE   64
 #define MWIFIEX_11AC_UAP_AMPDU_DEF_TXWINSIZE   48
 #define MWIFIEX_11AC_UAP_AMPDU_DEF_RXWINSIZE   32
 
-- 
1.8.2.3

[PATCH 5/5] mwifiex: enable aggregation for TID 6 and 7 streams

[Bing Zhao]

From: Amitkumar Karwar <akarwar@marvell.com>

Currently AMSDU and AMPDU aggregation is enabled for TID 0 to
TID 5 streams. Lets enable it for remaining two streams also.

Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Avinash Patil <patila@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
 drivers/net/wireless/mwifiex/wmm.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/wmm.c b/drivers/net/wireless/mwifiex/wmm.c
index 0a7cc74..94b6c74 100644
--- a/drivers/net/wireless/mwifiex/wmm.c
+++ b/drivers/net/wireless/mwifiex/wmm.c
@@ -426,15 +426,6 @@ mwifiex_wmm_init(struct mwifiex_adapter *adapter)
 							priv->tos_to_tid_inv[i];
 		}
 
-		priv->aggr_prio_tbl[6].amsdu
-					= priv->aggr_prio_tbl[6].ampdu_ap
-					= priv->aggr_prio_tbl[6].ampdu_user
-					= BA_STREAM_NOT_ALLOWED;
-
-		priv->aggr_prio_tbl[7].amsdu = priv->aggr_prio_tbl[7].ampdu_ap
-					= priv->aggr_prio_tbl[7].ampdu_user
-					= BA_STREAM_NOT_ALLOWED;
-
 		mwifiex_set_ba_params(priv);
 		mwifiex_reset_11n_rx_seq_num(priv);
 
-- 
1.8.2.3