All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] ath10k: export calibration data to user space
@ 2014-09-11 20:31 Kalle Valo
  2014-09-11 20:31 ` [RFC PATCH 1/3] ath10k: ignore diagnose pipes in ath10k_pci_ce_recv_data()/_send_done() Kalle Valo
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Kalle Valo @ 2014-09-11 20:31 UTC (permalink / raw)
  To: ath10k

Hi,

here's a small patchset exporting firmware calibration data to user space. This
makes it easier to debug various calibration data related problems. The first
patch is more or less a hack, not sure how to do that properly.

Please comment.

---

Kalle Valo (3):
      ath10k: ignore diagnose pipes in ath10k_pci_ce_recv_data()/_send_done()
      ath10k: add diag_read() to hif ops
      ath10k: add cal_data debugfs file


 drivers/net/wireless/ath/ath10k/debug.c |   81 +++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/hif.h   |   10 ++++
 drivers/net/wireless/ath/ath10k/hw.h    |    2 +
 drivers/net/wireless/ath/ath10k/pci.c   |   14 +++++
 4 files changed, 107 insertions(+)


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 1/3] ath10k: ignore diagnose pipes in ath10k_pci_ce_recv_data()/_send_done()
  2014-09-11 20:31 [RFC PATCH 0/3] ath10k: export calibration data to user space Kalle Valo
@ 2014-09-11 20:31 ` Kalle Valo
  2014-09-12  7:53   ` Michal Kazior
  2014-09-11 20:31 ` [RFC PATCH 2/3] ath10k: add diag_read() to hif ops Kalle Valo
  2014-09-11 20:31 ` [RFC PATCH 3/3] ath10k: add cal_data debugfs file Kalle Valo
  2 siblings, 1 reply; 14+ messages in thread
From: Kalle Valo @ 2014-09-11 20:31 UTC (permalink / raw)
  To: ath10k

As ath10k_pci_diag_read_mem() uses polling to receive data from CE, the CE callbacks
have to ignore the pipe used for diagnose reads. Otherwise ath10k crashes due
to NULL dereference and polling reads timeout.

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/pci.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index baeb98e78b1f..154451ab3e3c 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -819,6 +819,9 @@ static void ath10k_pci_ce_send_done(struct ath10k_ce_pipe *ce_state)
 	unsigned int nbytes;
 	unsigned int transfer_id;
 
+	if (ce_state->id == 7)
+		return;
+
 	while (ath10k_ce_completed_send_next(ce_state, &transfer_context,
 					     &ce_data, &nbytes,
 					     &transfer_id) == 0) {
@@ -844,6 +847,9 @@ static void ath10k_pci_ce_recv_data(struct ath10k_ce_pipe *ce_state)
 	unsigned int transfer_id;
 	unsigned int flags;
 
+	if (ce_state->id == 7)
+		return;
+
 	while (ath10k_ce_completed_recv_next(ce_state, &transfer_context,
 					     &ce_data, &nbytes, &transfer_id,
 					     &flags) == 0) {


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/3] ath10k: add diag_read() to hif ops
  2014-09-11 20:31 [RFC PATCH 0/3] ath10k: export calibration data to user space Kalle Valo
  2014-09-11 20:31 ` [RFC PATCH 1/3] ath10k: ignore diagnose pipes in ath10k_pci_ce_recv_data()/_send_done() Kalle Valo
@ 2014-09-11 20:31 ` Kalle Valo
  2014-09-12  9:57   ` Michal Kazior
  2014-09-11 20:31 ` [RFC PATCH 3/3] ath10k: add cal_data debugfs file Kalle Valo
  2 siblings, 1 reply; 14+ messages in thread
From: Kalle Valo @ 2014-09-11 20:31 UTC (permalink / raw)
  To: ath10k

diag_read() is used for reading from firmware memory via the diagnose window.
First user will be cal_data debugfs file.

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/hif.h |   10 ++++++++++
 drivers/net/wireless/ath/ath10k/pci.c |    8 ++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/hif.h b/drivers/net/wireless/ath/ath10k/hif.h
index 2ac7beacddca..0a8877d33f1f 100644
--- a/drivers/net/wireless/ath/ath10k/hif.h
+++ b/drivers/net/wireless/ath/ath10k/hif.h
@@ -43,6 +43,10 @@ struct ath10k_hif_ops {
 	int (*tx_sg)(struct ath10k *ar, u8 pipe_id,
 		     struct ath10k_hif_sg_item *items, int n_items);
 
+	/* read firmware memory through the diagnose interface */
+	int (*diag_read)(struct ath10k *ar, u32 address, void *buf,
+			 size_t buf_len);
+
 	/*
 	 * API to handle HIF-specific BMI message exchanges, this API is
 	 * synchronous and only allowed to be called from a context that
@@ -99,6 +103,12 @@ static inline int ath10k_hif_tx_sg(struct ath10k *ar, u8 pipe_id,
 	return ar->hif.ops->tx_sg(ar, pipe_id, items, n_items);
 }
 
+static inline int ath10k_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
+				       size_t buf_len)
+{
+	return ar->hif.ops->diag_read(ar, address, buf, buf_len);
+}
+
 static inline int ath10k_hif_exchange_bmi_msg(struct ath10k *ar,
 					      void *request, u32 request_len,
 					      void *response, u32 *response_len)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 154451ab3e3c..d3c51079034e 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -941,6 +941,13 @@ err:
 	return err;
 }
 
+static int ath10k_pci_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
+				    size_t buf_len)
+{
+	/* FIXME: locking */
+	return ath10k_pci_diag_read_mem(ar, address, buf, buf_len);
+}
+
 static u16 ath10k_pci_hif_get_free_queue_number(struct ath10k *ar, u8 pipe)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
@@ -1926,6 +1933,7 @@ static int ath10k_pci_hif_resume(struct ath10k *ar)
 
 static const struct ath10k_hif_ops ath10k_pci_hif_ops = {
 	.tx_sg			= ath10k_pci_hif_tx_sg,
+	.diag_read		= ath10k_pci_hif_diag_read,
 	.exchange_bmi_msg	= ath10k_pci_hif_exchange_bmi_msg,
 	.start			= ath10k_pci_hif_start,
 	.stop			= ath10k_pci_hif_stop,


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 3/3] ath10k: add cal_data debugfs file
  2014-09-11 20:31 [RFC PATCH 0/3] ath10k: export calibration data to user space Kalle Valo
  2014-09-11 20:31 ` [RFC PATCH 1/3] ath10k: ignore diagnose pipes in ath10k_pci_ce_recv_data()/_send_done() Kalle Valo
  2014-09-11 20:31 ` [RFC PATCH 2/3] ath10k: add diag_read() to hif ops Kalle Valo
@ 2014-09-11 20:31 ` Kalle Valo
  2014-09-11 20:53   ` Kalle Valo
                     ` (2 more replies)
  2 siblings, 3 replies; 14+ messages in thread
From: Kalle Valo @ 2014-09-11 20:31 UTC (permalink / raw)
  To: ath10k

Provide calibration data used by the firmware to user space via a debugfs file.
This makes it easier to debug calibration related problems.

Example:

sudo cp /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data 1.cal

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/debug.c |   81 +++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/hw.h    |    2 +
 2 files changed, 83 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index f948a4d8ee59..6500fd44f527 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -23,6 +23,7 @@
 
 #include "core.h"
 #include "debug.h"
+#include "hif.h"
 
 /* ms */
 #define ATH10K_DEBUG_HTT_STATS_INTERVAL 1000
@@ -1014,6 +1015,83 @@ static const struct file_operations fops_fw_dbglog = {
 	.llseek = default_llseek,
 };
 
+static int ath10k_cal_data_open(struct inode *inode, struct file *file)
+{
+	struct ath10k *ar = inode->i_private;
+	void *buf;
+	u32 hi_addr;
+	__le32 addr;
+	int ret;
+
+	mutex_lock(&ar->conf_mutex);
+
+	if (ar->state != ATH10K_STATE_ON &&
+	    ar->state != ATH10K_STATE_UTF) {
+		ret = -ENETDOWN;
+		goto err;
+	}
+
+	buf = vmalloc(QCA988X_CAL_DATA_LEN);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	hi_addr = host_interest_item_address(HI_ITEM(hi_board_data));
+
+	ret = ath10k_hif_diag_read(ar, hi_addr, &addr, sizeof(addr));
+	if (ret) {
+		ath10k_warn("failed to read hi_board_data address: %d\n", ret);
+		goto err_vfree;
+	}
+
+	ret = ath10k_hif_diag_read(ar, le32_to_cpu(addr), buf,
+				   QCA988X_CAL_DATA_LEN);
+	if (ret) {
+		ath10k_warn("failed to read calibration data: %d\n", ret);
+		goto err_vfree;
+	}
+
+	file->private_data = buf;
+
+	mutex_unlock(&ar->conf_mutex);
+
+	return 0;
+
+err_vfree:
+	vfree(buf);
+
+err:
+	mutex_unlock(&ar->conf_mutex);
+
+	return ret;
+}
+
+static ssize_t ath10k_cal_data_read(struct file *file, char __user *user_buf,
+				   size_t count, loff_t *ppos)
+{
+	void *buf = file->private_data;
+
+	return simple_read_from_buffer(user_buf, count, ppos,
+				       buf, QCA988X_CAL_DATA_LEN);
+}
+
+static int ath10k_cal_data_release(struct inode *inode,
+				   struct file *file)
+{
+	vfree(file->private_data);
+
+	return 0;
+}
+
+static const struct file_operations fops_cal_data = {
+	.open = ath10k_cal_data_open,
+	.read = ath10k_cal_data_read,
+	.release = ath10k_cal_data_release,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 int ath10k_debug_start(struct ath10k *ar)
 {
 	int ret;
@@ -1183,6 +1261,9 @@ int ath10k_debug_register(struct ath10k *ar)
 	debugfs_create_file("fw_dbglog", S_IRUSR, ar->debug.debugfs_phy,
 			    ar, &fops_fw_dbglog);
 
+	debugfs_create_file("cal_data", S_IRUSR, ar->debug.debugfs_phy,
+			    ar, &fops_cal_data);
+
 	if (config_enabled(CONFIG_ATH10K_DFS_CERTIFIED)) {
 		debugfs_create_file("dfs_simulate_radar", S_IWUSR,
 				    ar->debug.debugfs_phy, ar,
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 3cf5702c1e7e..006a9cbc60b2 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -43,6 +43,8 @@
 
 #define REG_DUMP_COUNT_QCA988X 60
 
+#define QCA988X_CAL_DATA_LEN		2116
+
 struct ath10k_fw_ie {
 	__le32 id;
 	__le32 len;


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 3/3] ath10k: add cal_data debugfs file
  2014-09-11 20:31 ` [RFC PATCH 3/3] ath10k: add cal_data debugfs file Kalle Valo
@ 2014-09-11 20:53   ` Kalle Valo
  2014-09-11 22:45   ` Ben Greear
  2014-09-12 10:11   ` Michal Kazior
  2 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2014-09-11 20:53 UTC (permalink / raw)
  To: ath10k

Kalle Valo <kvalo@qca.qualcomm.com> writes:

> Provide calibration data used by the firmware to user space via a debugfs file.
> This makes it easier to debug calibration related problems.
>
> Example:
>
> sudo cp /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data 1.cal
>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>

[...]

> +	ret = ath10k_hif_diag_read(ar, hi_addr, &addr, sizeof(addr));
> +	if (ret) {
> +		ath10k_warn("failed to read hi_board_data address: %d\n", ret);

This and rest of the fuction is missing 'ar' parameter. Obviously it's
too late for me :)

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 3/3] ath10k: add cal_data debugfs file
  2014-09-11 20:31 ` [RFC PATCH 3/3] ath10k: add cal_data debugfs file Kalle Valo
  2014-09-11 20:53   ` Kalle Valo
@ 2014-09-11 22:45   ` Ben Greear
  2014-09-12 11:25     ` Kalle Valo
  2014-09-12 10:11   ` Michal Kazior
  2 siblings, 1 reply; 14+ messages in thread
From: Ben Greear @ 2014-09-11 22:45 UTC (permalink / raw)
  To: Kalle Valo, ath10k

On 09/11/2014 01:31 PM, Kalle Valo wrote:
> Provide calibration data used by the firmware to user space via a debugfs file.
> This makes it easier to debug calibration related problems.
>
> Example:
>
> sudo cp /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data 1.cal

Is this data that could be made human readable, or does it need to
be parsed by some private tool?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 1/3] ath10k: ignore diagnose pipes in ath10k_pci_ce_recv_data()/_send_done()
  2014-09-11 20:31 ` [RFC PATCH 1/3] ath10k: ignore diagnose pipes in ath10k_pci_ce_recv_data()/_send_done() Kalle Valo
@ 2014-09-12  7:53   ` Michal Kazior
  2014-09-12  8:01     ` Kalle Valo
  0 siblings, 1 reply; 14+ messages in thread
From: Michal Kazior @ 2014-09-12  7:53 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath10k

On 11 September 2014 22:31, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> As ath10k_pci_diag_read_mem() uses polling to receive data from CE, the CE callbacks
> have to ignore the pipe used for diagnose reads. Otherwise ath10k crashes due
> to NULL dereference and polling reads timeout.
>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath10k/pci.c |    6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
> index baeb98e78b1f..154451ab3e3c 100644
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -819,6 +819,9 @@ static void ath10k_pci_ce_send_done(struct ath10k_ce_pipe *ce_state)
>         unsigned int nbytes;
>         unsigned int transfer_id;
>
> +       if (ce_state->id == 7)
> +               return;

I think you can check it in a saner way: (ce_state == ar_pci->ce_diag).

Another approach is to prevent this from happening in the first place
by never unmasking the diagnostic window copy engine pipe in
ath10k_ce_enable_interrupts() and preventing
ath10k_ce_per_engine_service() from being called in
ath10k_pci_hif_send_complete_check() for the diagnostic ce pipe.


Michał

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 1/3] ath10k: ignore diagnose pipes in ath10k_pci_ce_recv_data()/_send_done()
  2014-09-12  7:53   ` Michal Kazior
@ 2014-09-12  8:01     ` Kalle Valo
  0 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2014-09-12  8:01 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k

Michal Kazior <michal.kazior@tieto.com> writes:

> On 11 September 2014 22:31, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> As ath10k_pci_diag_read_mem() uses polling to receive data from CE, the CE callbacks
>> have to ignore the pipe used for diagnose reads. Otherwise ath10k crashes due
>> to NULL dereference and polling reads timeout.
>>
>> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
>> ---
>>  drivers/net/wireless/ath/ath10k/pci.c |    6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
>> index baeb98e78b1f..154451ab3e3c 100644
>> --- a/drivers/net/wireless/ath/ath10k/pci.c
>> +++ b/drivers/net/wireless/ath/ath10k/pci.c
>> @@ -819,6 +819,9 @@ static void ath10k_pci_ce_send_done(struct ath10k_ce_pipe *ce_state)
>>         unsigned int nbytes;
>>         unsigned int transfer_id;
>>
>> +       if (ce_state->id == 7)
>> +               return;
>
> I think you can check it in a saner way: (ce_state == ar_pci->ce_diag).
>
> Another approach is to prevent this from happening in the first place
> by never unmasking the diagnostic window copy engine pipe in
> ath10k_ce_enable_interrupts() and preventing
> ath10k_ce_per_engine_service() from being called in
> ath10k_pci_hif_send_complete_check() for the diagnostic ce pipe.

I like the latter approach and I'll try to implement that first. Thanks!

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 2/3] ath10k: add diag_read() to hif ops
  2014-09-11 20:31 ` [RFC PATCH 2/3] ath10k: add diag_read() to hif ops Kalle Valo
@ 2014-09-12  9:57   ` Michal Kazior
  2014-09-12 10:03     ` Kalle Valo
  0 siblings, 1 reply; 14+ messages in thread
From: Michal Kazior @ 2014-09-12  9:57 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath10k

On 11 September 2014 22:31, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> diag_read() is used for reading from firmware memory via the diagnose window.
> First user will be cal_data debugfs file.
[...]
> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
> index 154451ab3e3c..d3c51079034e 100644
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -941,6 +941,13 @@ err:
>         return err;
>  }
>
> +static int ath10k_pci_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
> +                                   size_t buf_len)
> +{
> +       /* FIXME: locking */
> +       return ath10k_pci_diag_read_mem(ar, address, buf, buf_len);

Hmm.. This can potentially collide with firmware crash as it uses CE
diag window as well.

I'm thinking entire bodies of diag_read_mem()/diag_writemem() should
be protected by ar_pci->ce_lock. It shouldn't be hard. As for
diag_read_mem():

 * s/ath10k_ce_rx_post_buf/__ath10k_ce_rx_post_buf/
 * s/ath10k_ce_send/ath10k_ce_send_nolock/
 * s/ath10k_ce_completed_send_next/ath10k_ce_completed_send_next_nolock/
 * s/ath10k_ce_completed_recv_next/ath10k_ce_completed_recv_next_nolock/

Note: Most _nolock stuff is static in ce.c so it needs to be changed
to be accessible in pci.c.


Michał

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 2/3] ath10k: add diag_read() to hif ops
  2014-09-12  9:57   ` Michal Kazior
@ 2014-09-12 10:03     ` Kalle Valo
  0 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2014-09-12 10:03 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k

Michal Kazior <michal.kazior@tieto.com> writes:

> On 11 September 2014 22:31, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> diag_read() is used for reading from firmware memory via the diagnose window.
>> First user will be cal_data debugfs file.
> [...]
>> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
>> index 154451ab3e3c..d3c51079034e 100644
>> --- a/drivers/net/wireless/ath/ath10k/pci.c
>> +++ b/drivers/net/wireless/ath/ath10k/pci.c
>> @@ -941,6 +941,13 @@ err:
>>         return err;
>>  }
>>
>> +static int ath10k_pci_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
>> +                                   size_t buf_len)
>> +{
>> +       /* FIXME: locking */
>> +       return ath10k_pci_diag_read_mem(ar, address, buf, buf_len);
>
> Hmm.. This can potentially collide with firmware crash as it uses CE
> diag window as well.

Yeah, I was worried something like that could happen.

> I'm thinking entire bodies of diag_read_mem()/diag_writemem() should
> be protected by ar_pci->ce_lock. It shouldn't be hard. As for
> diag_read_mem():
>
>  * s/ath10k_ce_rx_post_buf/__ath10k_ce_rx_post_buf/
>  * s/ath10k_ce_send/ath10k_ce_send_nolock/
>  * s/ath10k_ce_completed_send_next/ath10k_ce_completed_send_next_nolock/
>  * s/ath10k_ce_completed_recv_next/ath10k_ce_completed_recv_next_nolock/
>
> Note: Most _nolock stuff is static in ce.c so it needs to be changed
> to be accessible in pci.c.

Sounds like a good idea, I'll take a closer look and see how it works
out. On the downside this means that there will be one more function
call in hot path (I assume the compiler is able to inline the _nolock
static functions).

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 3/3] ath10k: add cal_data debugfs file
  2014-09-11 20:31 ` [RFC PATCH 3/3] ath10k: add cal_data debugfs file Kalle Valo
  2014-09-11 20:53   ` Kalle Valo
  2014-09-11 22:45   ` Ben Greear
@ 2014-09-12 10:11   ` Michal Kazior
  2014-09-12 10:50     ` Kalle Valo
  2 siblings, 1 reply; 14+ messages in thread
From: Michal Kazior @ 2014-09-12 10:11 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath10k

On 11 September 2014 22:31, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Provide calibration data used by the firmware to user space via a debugfs file.
> This makes it easier to debug calibration related problems.
>
> Example:
>
> sudo cp /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data 1.cal

How about having a generic r/w access to target memory via debugfs?
Userspace would deal with the extraction of the cal data. Or do you
want to make it easier for the user to provide the cal data without
any extra tools (and that sending entire unprocessed target memory
dump may introduce some privacy concerns)?


Michał

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 3/3] ath10k: add cal_data debugfs file
  2014-09-12 10:11   ` Michal Kazior
@ 2014-09-12 10:50     ` Kalle Valo
  2014-09-12 16:17       ` Ben Greear
  0 siblings, 1 reply; 14+ messages in thread
From: Kalle Valo @ 2014-09-12 10:50 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k

Michal Kazior <michal.kazior@tieto.com> writes:

> On 11 September 2014 22:31, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Provide calibration data used by the firmware to user space via a debugfs file.
>> This makes it easier to debug calibration related problems.
>>
>> Example:
>>
>> sudo cp /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data 1.cal
>
> How about having a generic r/w access to target memory via debugfs?
> Userspace would deal with the extraction of the cal data. 

Yeah, Yanbo is working on that kind of generic interface.

> Or do you want to make it easier for the user to provide the cal data
> without any extra tools (and that sending entire unprocessed target
> memory dump may introduce some privacy concerns)?

This (to make it easier) is exactly my motivation here. In some of the
problems, especially once we add support to get calibration data from
host instead of just OTP, it's important to get easily access to the
calibration data. User only needs to do a simple cp operation and send
the file to us for further investigation.

Of course we could use the generic target memory interface for this, but
it's just more complicated for an unexperienced user and that's why I
think we should have this interface. But to be honest I'm not 100% sure
yet so other comments are more than welcome.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 3/3] ath10k: add cal_data debugfs file
  2014-09-11 22:45   ` Ben Greear
@ 2014-09-12 11:25     ` Kalle Valo
  0 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2014-09-12 11:25 UTC (permalink / raw)
  To: Ben Greear; +Cc: ath10k

Ben Greear <greearb@candelatech.com> writes:

> On 09/11/2014 01:31 PM, Kalle Valo wrote:
>> Provide calibration data used by the firmware to user space via a debugfs file.
>> This makes it easier to debug calibration related problems.
>>
>> Example:
>>
>> sudo cp /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data 1.cal
>
> Is this data that could be made human readable, or does it need to
> be parsed by some private tool?

A private tool is needed for this.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 3/3] ath10k: add cal_data debugfs file
  2014-09-12 10:50     ` Kalle Valo
@ 2014-09-12 16:17       ` Ben Greear
  0 siblings, 0 replies; 14+ messages in thread
From: Ben Greear @ 2014-09-12 16:17 UTC (permalink / raw)
  To: Kalle Valo; +Cc: Michal Kazior, ath10k

On 09/12/2014 03:50 AM, Kalle Valo wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
> 
>> On 11 September 2014 22:31, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>>> Provide calibration data used by the firmware to user space via a debugfs file.
>>> This makes it easier to debug calibration related problems.
>>>
>>> Example:
>>>
>>> sudo cp /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data 1.cal
>>
>> How about having a generic r/w access to target memory via debugfs?
>> Userspace would deal with the extraction of the cal data. 
> 
> Yeah, Yanbo is working on that kind of generic interface.
> 
>> Or do you want to make it easier for the user to provide the cal data
>> without any extra tools (and that sending entire unprocessed target
>> memory dump may introduce some privacy concerns)?
> 
> This (to make it easier) is exactly my motivation here. In some of the
> problems, especially once we add support to get calibration data from
> host instead of just OTP, it's important to get easily access to the
> calibration data. User only needs to do a simple cp operation and send
> the file to us for further investigation.
> 
> Of course we could use the generic target memory interface for this, but
> it's just more complicated for an unexperienced user and that's why I
> think we should have this interface. But to be honest I'm not 100% sure
> yet so other comments are more than welcome.

Different firmware may place this at different locations, so I definitely
like a well-defined interface to grab the requested data instead of having to
try to grab it out of a full ram dump.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-09-12 16:18 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-11 20:31 [RFC PATCH 0/3] ath10k: export calibration data to user space Kalle Valo
2014-09-11 20:31 ` [RFC PATCH 1/3] ath10k: ignore diagnose pipes in ath10k_pci_ce_recv_data()/_send_done() Kalle Valo
2014-09-12  7:53   ` Michal Kazior
2014-09-12  8:01     ` Kalle Valo
2014-09-11 20:31 ` [RFC PATCH 2/3] ath10k: add diag_read() to hif ops Kalle Valo
2014-09-12  9:57   ` Michal Kazior
2014-09-12 10:03     ` Kalle Valo
2014-09-11 20:31 ` [RFC PATCH 3/3] ath10k: add cal_data debugfs file Kalle Valo
2014-09-11 20:53   ` Kalle Valo
2014-09-11 22:45   ` Ben Greear
2014-09-12 11:25     ` Kalle Valo
2014-09-12 10:11   ` Michal Kazior
2014-09-12 10:50     ` Kalle Valo
2014-09-12 16:17       ` Ben Greear

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.