On Mon, 29 Jul 2024 10:59:04 +0200, Ivan Orlov wrote:
--- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h
(snip)
+/*
- This structure describes the userspace-driven timer. Such timers are purely virtual,
- and can only be triggered from software (for instance, by userspace application).
- */
+struct snd_utimer_info {
- /*
* To pretend being a normal timer, we need to know the frame rate and* the period size in frames.*/- snd_pcm_uframes_t frame_rate;
- snd_pcm_uframes_t period_size;
The units in timer API should be independent from PCM. So use the explicit type such as __u64 here (so that you don't need the compat ioctl conversion, too).
- unsigned int id;
+};
We often put some reserved fields for future extension. But I'm not sure whether it's needed at this time for this kind of simple interface, though.
#define SNDRV_TIMER_IOCTL_PVERSION _IOR('T', 0x00, int) #define SNDRV_TIMER_IOCTL_NEXT_DEVICE _IOWR('T', 0x01, struct snd_timer_id) #define SNDRV_TIMER_IOCTL_TREAD_OLD _IOW('T', 0x02, int) @@ -990,6 +1005,8 @@ struct snd_timer_status { #define SNDRV_TIMER_IOCTL_CONTINUE _IO('T', 0xa2) #define SNDRV_TIMER_IOCTL_PAUSE _IO('T', 0xa3) #define SNDRV_TIMER_IOCTL_TREAD64 _IOW('T', 0xa4, int) +#define SNDRV_TIMER_IOCTL_CREATE _IOWR('T', 0xa5, struct snd_utimer_info) +#define SNDRV_TIMER_IOCTL_TRIGGER _IO('T', 0xa6)
Once after adding the new API, don't forget to bump the protocol version defined in SNDRV_TIMER_VERSION.
--- a/sound/core/timer.c +++ b/sound/core/timer.c
(snip)
+#ifdef CONFIG_SND_UTIMER +/*
- Since userspace-driven timers are passed to userspace, we need to have an identifier
- which will allow us to use them (basically, the subdevice number of udriven timer).
- */
+DEFINE_IDA(snd_utimer_ids);
Missing static.
+static int snd_utimer_create(struct snd_utimer_info *utimer_info,
struct snd_utimer **r_utimer)+{
(snip)
- err = snd_timer_new(NULL, utimer->name, &tid, &timer);
- if (err < 0) {
pr_err("Can't create userspace-driven timer\n");goto err_timer_new;- }
- timer->module = THIS_MODULE;
- timer->hw = timer_hw;
- timer->hw.resolution = NANO / utimer_info->frame_rate * utimer_info->period_size;
A sanity check is definitely needed for parameters like this. e.g. you'd hit a zero-division Oops with this code. Also, the resolution should be neither too small nor too high.
+static int snd_utimer_ioctl_create(struct file *file,
struct snd_utimer_info __user *_utimer_info)+{
- struct snd_utimer *utimer;
- struct snd_utimer_info *utimer_info;
- int err;
- utimer_info = memdup_user(_utimer_info, sizeof(*utimer_info));
- if (IS_ERR(utimer_info))
return PTR_ERR(no_free_ptr(utimer_info));
no_free_ptr() is used only for the automatic cleanup stuff.
+static int snd_utimer_ioctl_create(struct file *file,
struct snd_utimer_info __user *_utimer_info)+{
- return -EINVAL;
Better to keep -ENOTTY?
thanks,
Takashi