| Message ID | 1475699037-136212-1-git-send-email-mfaltesek@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
Marty Faltesek <mfaltesek@google.com> writes: > Caching calibration data allows it to be accessed when the > device is not active. > > --- Signed-off-by missing. Can you send it as a reply to this message and I'll add it to v3? > drivers/net/wireless/ath/ath10k/core.h | 1 + > drivers/net/wireless/ath/ath10k/debug.c | 72 ++++++++++++++------------------- > 2 files changed, 31 insertions(+), 42 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h > index 6e5aa2d..7274ebe 100644 > --- a/drivers/net/wireless/ath/ath10k/core.h > +++ b/drivers/net/wireless/ath/ath10k/core.h > @@ -452,6 +452,7 @@ struct ath10k_debug { > u32 nf_cal_period; > > struct ath10k_fw_crash_data *fw_crash_data; > + void *cal_data; To properly document locking I'll move this under the "protected by conf_mutex" comment. > -static int ath10k_debug_cal_data_open(struct inode *inode, struct file *file) > +static int ath10k_debug_cal_data_fetch(struct ath10k *ar) > { > - struct ath10k *ar = inode->i_private; > - void *buf; > u32 hi_addr; > __le32 addr; > int ret; > > - mutex_lock(&ar->conf_mutex); I'll add lockdep_assert_held() here. > static ssize_t ath10k_debug_cal_data_read(struct file *file, > - char __user *user_buf, > - size_t count, loff_t *ppos) > + char __user *user_buf, > + size_t count, loff_t *ppos) > { > struct ath10k *ar = file->private_data; > - void *buf = file->private_data; > > return simple_read_from_buffer(user_buf, count, ppos, > - buf, ar->hw_params.cal_data_len); > -} I'll add locking to this function. > @@ -2343,7 +2327,9 @@ int ath10k_debug_create(struct ath10k *ar) > ar->debug.fw_crash_data = vzalloc(sizeof(*ar->debug.fw_crash_data)); > if (!ar->debug.fw_crash_data) > return -ENOMEM; > - > + ar->debug.cal_data = vzalloc(ar->hw_params.cal_data_len); > + if (!ar->debug.cal_data) > + return -ENOMEM; > INIT_LIST_HEAD(&ar->debug.fw_stats.pdevs); > INIT_LIST_HEAD(&ar->debug.fw_stats.vdevs); > INIT_LIST_HEAD(&ar->debug.fw_stats.peers); > @@ -2355,7 +2341,9 @@ int ath10k_debug_create(struct ath10k *ar) > void ath10k_debug_destroy(struct ath10k *ar) > { > vfree(ar->debug.fw_crash_data); > + vfree(ar->debug.cal_data); > ar->debug.fw_crash_data = NULL; > + ar->debug.cal_data = NULL; Actually I gave you a bad advice, this won't work as ar->hw_params.cal_data_len is not yet initialised, we initialise it only after we have probed the capabilities during the first firmware boot. I changed the allocation to a fixed length buffer for now, it's only few kBs virtual memory anyway so it shouldn't matter to anyone. I have now v3 ready and tested. I'll just need your Signed-off-by and I can then submit it.
ath10k: cache calibration data when the core is stopped Caching calibration data allows it to be accessed when the device is not active. Signed-off-by: Marty Faltesek <mfaltesek@google.com> On Mon, Oct 10, 2016 at 10:54 AM, Valo, Kalle <kvalo@qca.qualcomm.com> wrote: > Marty Faltesek <mfaltesek@google.com> writes: > >> Caching calibration data allows it to be accessed when the >> device is not active. >> >> --- > > Signed-off-by missing. Can you send it as a reply to this message and > I'll add it to v3? > >> drivers/net/wireless/ath/ath10k/core.h | 1 + >> drivers/net/wireless/ath/ath10k/debug.c | 72 ++++++++++++++------------------- >> 2 files changed, 31 insertions(+), 42 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h >> index 6e5aa2d..7274ebe 100644 >> --- a/drivers/net/wireless/ath/ath10k/core.h >> +++ b/drivers/net/wireless/ath/ath10k/core.h >> @@ -452,6 +452,7 @@ struct ath10k_debug { >> u32 nf_cal_period; >> >> struct ath10k_fw_crash_data *fw_crash_data; >> + void *cal_data; > > To properly document locking I'll move this under the "protected by > conf_mutex" comment. > >> -static int ath10k_debug_cal_data_open(struct inode *inode, struct file *file) >> +static int ath10k_debug_cal_data_fetch(struct ath10k *ar) >> { >> - struct ath10k *ar = inode->i_private; >> - void *buf; >> u32 hi_addr; >> __le32 addr; >> int ret; >> >> - mutex_lock(&ar->conf_mutex); > > I'll add lockdep_assert_held() here. > >> static ssize_t ath10k_debug_cal_data_read(struct file *file, >> - char __user *user_buf, >> - size_t count, loff_t *ppos) >> + char __user *user_buf, >> + size_t count, loff_t *ppos) >> { >> struct ath10k *ar = file->private_data; >> - void *buf = file->private_data; >> >> return simple_read_from_buffer(user_buf, count, ppos, >> - buf, ar->hw_params.cal_data_len); >> -} > > I'll add locking to this function. > >> @@ -2343,7 +2327,9 @@ int ath10k_debug_create(struct ath10k *ar) >> ar->debug.fw_crash_data = vzalloc(sizeof(*ar->debug.fw_crash_data)); >> if (!ar->debug.fw_crash_data) >> return -ENOMEM; >> - >> + ar->debug.cal_data = vzalloc(ar->hw_params.cal_data_len); >> + if (!ar->debug.cal_data) >> + return -ENOMEM; >> INIT_LIST_HEAD(&ar->debug.fw_stats.pdevs); >> INIT_LIST_HEAD(&ar->debug.fw_stats.vdevs); >> INIT_LIST_HEAD(&ar->debug.fw_stats.peers); >> @@ -2355,7 +2341,9 @@ int ath10k_debug_create(struct ath10k *ar) >> void ath10k_debug_destroy(struct ath10k *ar) >> { >> vfree(ar->debug.fw_crash_data); >> + vfree(ar->debug.cal_data); >> ar->debug.fw_crash_data = NULL; >> + ar->debug.cal_data = NULL; > > Actually I gave you a bad advice, this won't work as > ar->hw_params.cal_data_len is not yet initialised, we initialise it only > after we have probed the capabilities during the first firmware boot. I > changed the allocation to a fixed length buffer for now, it's only few > kBs virtual memory anyway so it shouldn't matter to anyone. > > I have now v3 ready and tested. I'll just need your Signed-off-by and I > can then submit it. > > -- > Kalle Valo
Marty Faltesek <mfaltesek@google.com> writes: > ath10k: cache calibration data when the core is stopped > > Caching calibration data allows it to be accessed when the > device is not active. > > Signed-off-by: Marty Faltesek <mfaltesek@google.com> Thanks, I'll now send v3.
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h index 6e5aa2d..7274ebe 100644 --- a/drivers/net/wireless/ath/ath10k/core.h +++ b/drivers/net/wireless/ath/ath10k/core.h @@ -452,6 +452,7 @@ struct ath10k_debug { u32 nf_cal_period; struct ath10k_fw_crash_data *fw_crash_data; + void *cal_data; }; enum ath10k_state { diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c index 832da6e..668074c 100644 --- a/drivers/net/wireless/ath/ath10k/debug.c +++ b/drivers/net/wireless/ath/ath10k/debug.c @@ -1451,75 +1451,58 @@ static const struct file_operations fops_fw_dbglog = { .llseek = default_llseek, }; -static int ath10k_debug_cal_data_open(struct inode *inode, struct file *file) +static int ath10k_debug_cal_data_fetch(struct ath10k *ar) { - 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(ar->hw_params.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(ar, "failed to read hi_board_data address: %d\n", ret); - goto err_vfree; + ath10k_warn(ar, "failed to read hi_board_data address: %d\n", + ret); + return ret; } - ret = ath10k_hif_diag_read(ar, le32_to_cpu(addr), buf, + ret = ath10k_hif_diag_read(ar, le32_to_cpu(addr), ar->debug.cal_data, ar->hw_params.cal_data_len); if (ret) { ath10k_warn(ar, "failed to read calibration data: %d\n", ret); - goto err_vfree; + return ret; } - file->private_data = buf; + return 0; +} - mutex_unlock(&ar->conf_mutex); +static int ath10k_debug_cal_data_open(struct inode *inode, struct file *file) +{ + struct ath10k *ar = inode->i_private; - return 0; + mutex_lock(&ar->conf_mutex); -err_vfree: - vfree(buf); + if (ar->state == ATH10K_STATE_ON || + ar->state == ATH10K_STATE_UTF) { + ath10k_debug_cal_data_fetch(ar); + } -err: + file->private_data = ar; mutex_unlock(&ar->conf_mutex); - return ret; + return 0; } static ssize_t ath10k_debug_cal_data_read(struct file *file, - char __user *user_buf, - size_t count, loff_t *ppos) + char __user *user_buf, + size_t count, loff_t *ppos) { struct ath10k *ar = file->private_data; - void *buf = file->private_data; return simple_read_from_buffer(user_buf, count, ppos, - buf, ar->hw_params.cal_data_len); -} - -static int ath10k_debug_cal_data_release(struct inode *inode, - struct file *file) -{ - vfree(file->private_data); - - return 0; + ar->debug.cal_data, + ar->hw_params.cal_data_len); } static ssize_t ath10k_write_ani_enable(struct file *file, @@ -1580,7 +1563,6 @@ static const struct file_operations fops_ani_enable = { static const struct file_operations fops_cal_data = { .open = ath10k_debug_cal_data_open, .read = ath10k_debug_cal_data_read, - .release = ath10k_debug_cal_data_release, .owner = THIS_MODULE, .llseek = default_llseek, }; @@ -1932,6 +1914,8 @@ void ath10k_debug_stop(struct ath10k *ar) { lockdep_assert_held(&ar->conf_mutex); + ath10k_debug_cal_data_fetch(ar); + /* Must not use _sync to avoid deadlock, we do that in * ath10k_debug_destroy(). The check for htt_stats_mask is to avoid * warning from del_timer(). */ @@ -2343,7 +2327,9 @@ int ath10k_debug_create(struct ath10k *ar) ar->debug.fw_crash_data = vzalloc(sizeof(*ar->debug.fw_crash_data)); if (!ar->debug.fw_crash_data) return -ENOMEM; - + ar->debug.cal_data = vzalloc(ar->hw_params.cal_data_len); + if (!ar->debug.cal_data) + return -ENOMEM; INIT_LIST_HEAD(&ar->debug.fw_stats.pdevs); INIT_LIST_HEAD(&ar->debug.fw_stats.vdevs); INIT_LIST_HEAD(&ar->debug.fw_stats.peers); @@ -2355,7 +2341,9 @@ int ath10k_debug_create(struct ath10k *ar) void ath10k_debug_destroy(struct ath10k *ar) { vfree(ar->debug.fw_crash_data); + vfree(ar->debug.cal_data); ar->debug.fw_crash_data = NULL; + ar->debug.cal_data = NULL; ath10k_debug_fw_stats_reset(ar);