| Message ID | 20141124164208.29422.76134.stgit@potku.adurom.net (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
Kalle Valo <kvalo@qca.qualcomm.com> writes: > + ret = copy_from_user(buf, user_buf, count); > + if (ret) > + goto err_free_copy; > + > + ret = ath10k_hif_diag_write(ar, *ppos, buf, count); > + if (ret) { > + ath10k_warn(ar, "failed to write address 0x%08x via diagnose window from debugfs: %d\n", > + (u32)(*ppos), ret); > + goto err_free_copy; > + } > + > + *ppos += count; > + ret = count; > + > +err_free_copy: > + vfree(buf); > + return ret; This introduces a new smatch warning: drivers/net/wireless/ath/ath10k/debug.c:1100 ath10k_mem_value_write() warn: maybe return -EFAULT instead of the bytes remaining? I think it should be like this: ret = copy_from_user(buf, user_buf, count); if (ret) { ret = -EFAULT; goto err_free_copy; } Comments?
Kalle Valo <kvalo@qca.qualcomm.com> writes: > +static ssize_t ath10k_mem_value_read(struct file *file, > + char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct ath10k *ar = file->private_data; > + u8 *buf; > + int ret; > + > + if (*ppos < 0) > + return -EINVAL; > + > + if (!count) > + return 0; > + > + buf = vmalloc(count); > + if (!buf) > + return -ENOMEM; > + > + ret = ath10k_hif_diag_read(ar, *ppos, buf, count); > + if (ret) { > + ath10k_warn(ar, "failed to read address 0x%08x via diagnose window from debugfs: %d\n", > + (u32)(*ppos), ret); > + goto read_err; > + } > + [...] > +static ssize_t ath10k_mem_value_write(struct file *file, > + const char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct ath10k *ar = file->private_data; > + u8 *buf; > + int ret; > + > + if (*ppos < 0) > + return -EINVAL; > + > + if (!count) > + return 0; > + > + buf = vmalloc(count); > + if (!buf) > + return -ENOMEM; > + > + ret = copy_from_user(buf, user_buf, count); > + if (ret) > + goto err_free_copy; > + > + ret = ath10k_hif_diag_write(ar, *ppos, buf, count); > + if (ret) { > + ath10k_warn(ar, "failed to write address 0x%08x via diagnose window from debugfs: %d\n", > + (u32)(*ppos), ret); > + goto err_free_copy; > + } In these two functions we also need to check ar->state.
On 25 November 2014 at 09:41, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > Kalle Valo <kvalo@qca.qualcomm.com> writes: > >> +static ssize_t ath10k_mem_value_read(struct file *file, >> + char __user *user_buf, >> + size_t count, loff_t *ppos) >> +{ [...] >> +static ssize_t ath10k_mem_value_write(struct file *file, >> + const char __user *user_buf, >> + size_t count, loff_t *ppos) [...] > In these two functions we also need to check ar->state. It should work but the pci_wake (I've mentioned in the other mail) is tricky though. Micha?
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c index b500cd619c05..3988220e3e66 100644 --- a/drivers/net/wireless/ath/ath10k/debug.c +++ b/drivers/net/wireless/ath/ath10k/debug.c @@ -1024,6 +1024,89 @@ static const struct file_operations fops_reg_value = { .llseek = default_llseek, }; +static ssize_t ath10k_mem_value_read(struct file *file, + char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct ath10k *ar = file->private_data; + u8 *buf; + int ret; + + if (*ppos < 0) + return -EINVAL; + + if (!count) + return 0; + + buf = vmalloc(count); + if (!buf) + return -ENOMEM; + + ret = ath10k_hif_diag_read(ar, *ppos, buf, count); + if (ret) { + ath10k_warn(ar, "failed to read address 0x%08x via diagnose window from debugfs: %d\n", + (u32)(*ppos), ret); + goto read_err; + } + + ret = copy_to_user(user_buf, buf, count); + if (ret == count) + return -EFAULT; + + count -= ret; + *ppos += count; + ret = count; + +read_err: + vfree(buf); + return ret; +} + +static ssize_t ath10k_mem_value_write(struct file *file, + const char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct ath10k *ar = file->private_data; + u8 *buf; + int ret; + + if (*ppos < 0) + return -EINVAL; + + if (!count) + return 0; + + buf = vmalloc(count); + if (!buf) + return -ENOMEM; + + ret = copy_from_user(buf, user_buf, count); + if (ret) + goto err_free_copy; + + ret = ath10k_hif_diag_write(ar, *ppos, buf, count); + if (ret) { + ath10k_warn(ar, "failed to write address 0x%08x via diagnose window from debugfs: %d\n", + (u32)(*ppos), ret); + goto err_free_copy; + } + + *ppos += count; + ret = count; + +err_free_copy: + vfree(buf); + return ret; +} + +static const struct file_operations fops_mem_value = { + .read = ath10k_mem_value_read, + .write = ath10k_mem_value_write, + .open = simple_open, + .owner = THIS_MODULE, + .llseek = default_llseek, +}; + static int ath10k_debug_htt_stats_req(struct ath10k *ar) { u64 cookie; @@ -1731,6 +1814,9 @@ int ath10k_debug_register(struct ath10k *ar) debugfs_create_file("reg_value", S_IRUSR | S_IWUSR, ar->debug.debugfs_phy, ar, &fops_reg_value); + debugfs_create_file("mem_value", S_IRUSR | S_IWUSR, + ar->debug.debugfs_phy, ar, &fops_mem_value); + debugfs_create_file("chip_id", S_IRUSR, ar->debug.debugfs_phy, ar, &fops_chip_id); diff --git a/drivers/net/wireless/ath/ath10k/hif.h b/drivers/net/wireless/ath/ath10k/hif.h index bad071906540..6ac552304546 100644 --- a/drivers/net/wireless/ath/ath10k/hif.h +++ b/drivers/net/wireless/ath/ath10k/hif.h @@ -48,6 +48,8 @@ struct ath10k_hif_ops { int (*diag_read)(struct ath10k *ar, u32 address, void *buf, size_t buf_len); + int (*diag_write)(struct ath10k *ar, u32 address, const void *data, + int nbytes); /* * API to handle HIF-specific BMI message exchanges, this API is * synchronous and only allowed to be called from a context that @@ -113,6 +115,15 @@ static inline int ath10k_hif_diag_read(struct ath10k *ar, u32 address, void *buf return ar->hif.ops->diag_read(ar, address, buf, buf_len); } +static inline int ath10k_hif_diag_write(struct ath10k *ar, u32 address, + const void *data, int nbytes) +{ + if (!ar->hif.ops->diag_write) + return -EOPNOTSUPP; + + return ar->hif.ops->diag_write(ar, address, data, nbytes); +} + 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 328ab6c5d053..0816098af578 100644 --- a/drivers/net/wireless/ath/ath10k/pci.c +++ b/drivers/net/wireless/ath/ath10k/pci.c @@ -1988,6 +1988,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, + .diag_write = ath10k_pci_diag_write_mem, .exchange_bmi_msg = ath10k_pci_hif_exchange_bmi_msg, .start = ath10k_pci_hif_start, .stop = ath10k_pci_hif_stop,