Hi Scott,
On 2018/11/02 18:26, Scott Bahling wrote:
Thanks for your PR[1] and it works well. But here I suggest that we can judge the frequency to call SNDRV_FIREWIRE_IOCTL_TASCAM_STATUS ioctl is surely less than the frequency on packet processing. If we attempt to keep low CPU usage in the packet processing, current implementation can be kept as is. Instead, a patch for handler of the ioctl is worth for commit.
Makes perfect sense.
For example, This patch will fix the issue.
$ git diff diff --git a/sound/firewire/tascam/tascam-hwdep.c b/sound/firewire/tascam/tascam-hwdep.c index ad164ad..dff7dc4 100644 --- a/sound/firewire/tascam/tascam-hwdep.c +++ b/sound/firewire/tascam/tascam-hwdep.c @@ -189,10 +189,23 @@ static int hwdep_unlock(struct snd_tscm *tscm) static int tscm_hwdep_status(struct snd_tscm *tscm, void __user *arg) { - if (copy_to_user(arg, tscm->status, sizeof(tscm->status))) - return -EFAULT; + struct snd_firewire_tascam_status *status; + int i; + int err = 0; - return 0; + status = kmalloc(sizeof(*status), GFP_KERNEL); + if (!status) + return -ENOMEM; + + for (i = 0; i < SNDRV_FIREWIRE_TASCAM_STATUS_COUNT; ++i) + status->data[i] = be32_to_cpu(tscm->status[i]); + + if (copy_to_user(arg, status, sizeof(*status))) + err = -EFAULT; + + kfree(status); + + return err; }I'd like you to test with this patch, before granting your PR.
I have tested and the patch above works as expected.
Thanks for your check. I pushed the above patch into my remote branch.
Well, soon, merge window for v4.20 (or v5.0) kernel will be closed, then we start working for next v4.21 (or v5.1) kernel. I have a plan to include changes required for the status and control message in this development period (not prepare yet) and the changes are the same in the remote branch. I'll add you to C.C list when posting patches for it.
If you require more functionality, please inform it to me in this development period.
Thanks
Takashi Sakamoto