[alsa-devel] [PATCH 00/14] Refactoring ALSA core device management
Hi,
this is a series of relatively small patches for refactoring the ALSA core device management. I experimented a similar thing some time ago but didn't merge it in the end due to too intrusive changes. At this time, the approach is rather minimalistic: only the device registration part is replaced by embedding the struct device into each ALSA device object.
To the outside of ALSA core, there shouldn't be many visible changes. The only possible breakage would be an access to some renamed / dropped fields by a downstream driver. But it must be easy to correct.
Some patches in the series are merely cleanup patches that can be applied individually, but included just for completeness.
The whole patches are found in test/snd-device branch of sound git tree, too.
Takashi
===
Takashi Iwai (14): ALSA: Allow to pass the device object to snd_register_device*() ALSA: control: Provide a helper to look for the preferred subdevice ALSA: Add a helper to initialize device ALSA: control: Embed struct device ALSA: hwdep: Embed struct device ALSA: pcm: Embed struct device ALSA: rawmidi: Embed struct device ALSA: rawmidi: Use rawmidi device file for kernel messages ALSA: timer: Propagate the error at initialization ALSA: timer: Handle the device directly ALSA: seq: Handle the device directly ALSA: compress: Embed struct device ALSA: Simplify snd_device_register() variants ALSA: Drop snd_get_device() helper
include/sound/compress_driver.h | 4 +- include/sound/control.h | 11 +++- include/sound/core.h | 41 ++---------- include/sound/hwdep.h | 3 +- include/sound/pcm.h | 2 +- include/sound/rawmidi.h | 4 +- sound/aoa/soundbus/i2sbus/pcm.c | 5 +- sound/core/compress_offload.c | 26 ++++++-- sound/core/control.c | 66 +++++++++++-------- sound/core/hwdep.c | 88 ++++++++++--------------- sound/core/init.c | 19 ++++++ sound/core/pcm.c | 70 ++++++-------------- sound/core/rawmidi.c | 47 +++++++------- sound/core/seq/seq_clientmgr.c | 14 +++- sound/core/sound.c | 112 ++++++++++---------------------- sound/core/timer.c | 42 ++++++++---- sound/pci/hda/hda_controller.c | 3 +- sound/pci/hda/hda_hwdep.c | 7 +- sound/soc/intel/sst-mfld-platform-pcm.c | 1 - 19 files changed, 265 insertions(+), 300 deletions(-)
This is a preliminary patch for the further work on embedding struct device into each sound device instance. It changes snd_register_device*() helpers to receive the device object directly for skipping creating a device there.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/core.h | 14 +++++++------- sound/core/hwdep.c | 3 ++- sound/core/pcm.c | 2 +- sound/core/sound.c | 54 +++++++++++++++++++++++++++++++++------------------- 4 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/include/sound/core.h b/include/sound/core.h index 1df3f2fe5350..39d14234961d 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -186,6 +186,7 @@ struct snd_minor { int type; /* SNDRV_DEVICE_TYPE_XXX */ int card; /* card number */ int device; /* device number */ + bool created; const struct file_operations *f_ops; /* file operations */ void *private_data; /* private data for f_ops->open */ struct device *dev; /* device for sysfs */ @@ -206,12 +207,10 @@ extern struct class *sound_class;
void snd_request_card(int card);
-int snd_register_device_for_dev(int type, struct snd_card *card, - int dev, +int snd_register_device_for_dev(int type, struct snd_card *card, int dev, const struct file_operations *f_ops, - void *private_data, - const char *name, - struct device *device); + void *private_data, struct device *device, + struct device *parent, const char *name);
/** * snd_register_device - Register the ALSA device file for the card @@ -236,8 +235,9 @@ static inline int snd_register_device(int type, struct snd_card *card, int dev, const char *name) { return snd_register_device_for_dev(type, card, dev, f_ops, - private_data, name, - snd_card_get_device_link(card)); + private_data, NULL, + snd_card_get_device_link(card), + name); }
int snd_unregister_device(int type, struct snd_card *card, int dev); diff --git a/sound/core/hwdep.c b/sound/core/hwdep.c index 69459e5f712e..85096a150eda 100644 --- a/sound/core/hwdep.c +++ b/sound/core/hwdep.c @@ -433,7 +433,8 @@ static int snd_hwdep_dev_register(struct snd_device *device) dev = snd_card_get_device_link(hwdep->card); err = snd_register_device_for_dev(SNDRV_DEVICE_TYPE_HWDEP, hwdep->card, hwdep->device, - &snd_hwdep_f_ops, hwdep, name, dev); + &snd_hwdep_f_ops, hwdep, + NULL, dev, name); if (err < 0) { dev_err(dev, "unable to register hardware dependent device %i:%i\n", diff --git a/sound/core/pcm.c b/sound/core/pcm.c index cfc56c806964..dba5180e5b80 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -1115,7 +1115,7 @@ static int snd_pcm_dev_register(struct snd_device *device) err = snd_register_device_for_dev(devtype, pcm->card, pcm->device, &snd_pcm_f_ops[cidx], - pcm, str, dev); + pcm, NULL, dev, str); if (err < 0) { list_del(&pcm->list); mutex_unlock(®ister_mutex); diff --git a/sound/core/sound.c b/sound/core/sound.c index f1333060bf1c..ea1af1acdbe9 100644 --- a/sound/core/sound.c +++ b/sound/core/sound.c @@ -248,8 +248,9 @@ static int snd_kernel_minor(int type, struct snd_card *card, int dev) * @dev: the device index * @f_ops: the file operations * @private_data: user pointer for f_ops->open() - * @name: the device file name - * @device: the &struct device to link this new device to + * @device: the device to register, NULL to create a new one + * @parent: the &struct device to link this new device to (only for device=NULL) + * @name: the device file name (only for device=NULL) * * Registers an ALSA device file for the given card. * The operators have to be set in reg parameter. @@ -258,14 +259,13 @@ static int snd_kernel_minor(int type, struct snd_card *card, int dev) */ int snd_register_device_for_dev(int type, struct snd_card *card, int dev, const struct file_operations *f_ops, - void *private_data, - const char *name, struct device *device) + void *private_data, struct device *device, + struct device *parent, const char *name) { int minor; + int err = 0; struct snd_minor *preg;
- if (snd_BUG_ON(!name)) - return -EINVAL; preg = kmalloc(sizeof *preg, GFP_KERNEL); if (preg == NULL) return -ENOMEM; @@ -284,23 +284,32 @@ int snd_register_device_for_dev(int type, struct snd_card *card, int dev, minor = -EBUSY; #endif if (minor < 0) { - mutex_unlock(&sound_mutex); - kfree(preg); - return minor; + err = minor; + goto error; } - snd_minors[minor] = preg; - preg->dev = device_create(sound_class, device, MKDEV(major, minor), - private_data, "%s", name); - if (IS_ERR(preg->dev)) { - snd_minors[minor] = NULL; - mutex_unlock(&sound_mutex); - minor = PTR_ERR(preg->dev); - kfree(preg); - return minor; + + if (device) { + preg->created = false; + preg->dev = device; + device->devt = MKDEV(major, minor); + err = device_add(device); + } else { + preg->created = true; + preg->dev = device_create(sound_class, parent, + MKDEV(major, minor), private_data, + "%s", name); + if (IS_ERR(preg->dev)) + err = PTR_ERR(preg->dev); } + if (err < 0) + goto error;
+ snd_minors[minor] = preg; + error: mutex_unlock(&sound_mutex); - return 0; + if (err < 0) + kfree(preg); + return err; }
EXPORT_SYMBOL(snd_register_device_for_dev); @@ -337,6 +346,7 @@ static int find_snd_minor(int type, struct snd_card *card, int dev) int snd_unregister_device(int type, struct snd_card *card, int dev) { int minor; + struct snd_minor *preg;
mutex_lock(&sound_mutex); minor = find_snd_minor(type, card, dev); @@ -345,7 +355,11 @@ int snd_unregister_device(int type, struct snd_card *card, int dev) return -EINVAL; }
- device_destroy(sound_class, MKDEV(major, minor)); + preg = snd_minors[minor]; + if (preg && !preg->created) + device_del(preg->dev); + else + device_destroy(sound_class, MKDEV(major, minor));
kfree(snd_minors[minor]); snd_minors[minor] = NULL;
Instead of open-coding the search over the control file loop, provide a helper function for the preferred subdevice assigned to the current process.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/control.h | 11 +++++++++-- sound/core/control.c | 27 ++++++++++++++++++++++++--- sound/core/pcm.c | 15 +++------------ sound/core/rawmidi.c | 14 ++------------ 4 files changed, 38 insertions(+), 29 deletions(-)
diff --git a/include/sound/control.h b/include/sound/control.h index 042613938a1d..75f3054023f7 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -93,12 +93,17 @@ struct snd_kctl_event {
struct pid;
+enum { + SND_CTL_SUBDEV_PCM, + SND_CTL_SUBDEV_RAWMIDI, + SND_CTL_SUBDEV_ITEMS, +}; + struct snd_ctl_file { struct list_head list; /* list of all control files */ struct snd_card *card; struct pid *pid; - int prefer_pcm_subdevice; - int prefer_rawmidi_subdevice; + int preferred_subdevice[SND_CTL_SUBDEV_ITEMS]; wait_queue_head_t change_sleep; spinlock_t read_lock; struct fasync_struct *fasync; @@ -138,6 +143,8 @@ int snd_ctl_unregister_ioctl_compat(snd_kctl_ioctl_func_t fcn); #define snd_ctl_unregister_ioctl_compat(fcn) #endif
+int snd_ctl_get_preferred_subdevice(struct snd_card *card, int type); + static inline unsigned int snd_ctl_get_ioffnum(struct snd_kcontrol *kctl, struct snd_ctl_elem_id *id) { return id->numid - kctl->id.numid; diff --git a/sound/core/control.c b/sound/core/control.c index bb96a467e88d..cd246a0bcd55 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -50,7 +50,7 @@ static int snd_ctl_open(struct inode *inode, struct file *file) unsigned long flags; struct snd_card *card; struct snd_ctl_file *ctl; - int err; + int i, err;
err = nonseekable_open(inode, file); if (err < 0) @@ -79,8 +79,8 @@ static int snd_ctl_open(struct inode *inode, struct file *file) init_waitqueue_head(&ctl->change_sleep); spin_lock_init(&ctl->read_lock); ctl->card = card; - ctl->prefer_pcm_subdevice = -1; - ctl->prefer_rawmidi_subdevice = -1; + for (i = 0; i < SND_CTL_SUBDEV_ITEMS; i++) + ctl->preferred_subdevice[i] = -1; ctl->pid = get_pid(task_pid(current)); file->private_data = ctl; write_lock_irqsave(&card->ctl_files_rwlock, flags); @@ -1607,6 +1607,27 @@ static int snd_ctl_fasync(int fd, struct file * file, int on) return fasync_helper(fd, file, on, &ctl->fasync); }
+/* return the preferred subdevice number if already assigned; + * otherwise return -1 + */ +int snd_ctl_get_preferred_subdevice(struct snd_card *card, int type) +{ + struct snd_ctl_file *kctl; + int subdevice = -1; + + read_lock(&card->ctl_files_rwlock); + list_for_each_entry(kctl, &card->ctl_files, list) { + if (kctl->pid == task_pid(current)) { + subdevice = kctl->preferred_subdevice[type]; + if (subdevice != -1) + break; + } + } + read_unlock(&card->ctl_files_rwlock); + return subdevice; +} +EXPORT_SYMBOL_GPL(snd_ctl_get_preferred_subdevice); + /* * ioctl32 compat */ diff --git a/sound/core/pcm.c b/sound/core/pcm.c index dba5180e5b80..1b7c473720fa 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -161,7 +161,7 @@ static int snd_pcm_control_ioctl(struct snd_card *card, if (get_user(val, (int __user *)arg)) return -EFAULT; - control->prefer_pcm_subdevice = val; + control->preferred_subdevice[SND_CTL_SUBDEV_PCM] = val; return 0; } } @@ -901,9 +901,8 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, struct snd_pcm_str * pstr; struct snd_pcm_substream *substream; struct snd_pcm_runtime *runtime; - struct snd_ctl_file *kctl; struct snd_card *card; - int prefer_subdevice = -1; + int prefer_subdevice; size_t size;
if (snd_BUG_ON(!pcm || !rsubstream)) @@ -914,15 +913,7 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, return -ENODEV;
card = pcm->card; - read_lock(&card->ctl_files_rwlock); - list_for_each_entry(kctl, &card->ctl_files, list) { - if (kctl->pid == task_pid(current)) { - prefer_subdevice = kctl->prefer_pcm_subdevice; - if (prefer_subdevice != -1) - break; - } - } - read_unlock(&card->ctl_files_rwlock); + prefer_subdevice = snd_ctl_get_preferred_subdevice(card, SND_CTL_SUBDEV_PCM);
switch (stream) { case SNDRV_PCM_STREAM_PLAYBACK: diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c index 6fc71a4c8a51..be18162c380f 100644 --- a/sound/core/rawmidi.c +++ b/sound/core/rawmidi.c @@ -369,7 +369,6 @@ static int snd_rawmidi_open(struct inode *inode, struct file *file) struct snd_rawmidi *rmidi; struct snd_rawmidi_file *rawmidi_file = NULL; wait_queue_t wait; - struct snd_ctl_file *kctl;
if ((file->f_flags & O_APPEND) && !(file->f_flags & O_NONBLOCK)) return -EINVAL; /* invalid combination */ @@ -413,16 +412,7 @@ static int snd_rawmidi_open(struct inode *inode, struct file *file) init_waitqueue_entry(&wait, current); add_wait_queue(&rmidi->open_wait, &wait); while (1) { - subdevice = -1; - read_lock(&card->ctl_files_rwlock); - list_for_each_entry(kctl, &card->ctl_files, list) { - if (kctl->pid == task_pid(current)) { - subdevice = kctl->prefer_rawmidi_subdevice; - if (subdevice != -1) - break; - } - } - read_unlock(&card->ctl_files_rwlock); + subdevice = snd_ctl_get_preferred_subdevice(card, SND_CTL_SUBDEV_RAWMIDI); err = rawmidi_open_priv(rmidi, subdevice, fflags, rawmidi_file); if (err >= 0) break; @@ -862,7 +852,7 @@ static int snd_rawmidi_control_ioctl(struct snd_card *card, if (get_user(val, (int __user *)argp)) return -EFAULT; - control->prefer_rawmidi_subdevice = val; + control->preferred_subdevice[SND_CTL_SUBDEV_RAWMIDI] = val; return 0; } case SNDRV_CTL_IOCTL_RAWMIDI_INFO:
Introduce a new helper function snd_device_initialize() to initialize the device object for sound devices.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/core.h | 2 ++ sound/core/init.c | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+)
diff --git a/include/sound/core.h b/include/sound/core.h index 39d14234961d..de7a878217d7 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -207,6 +207,8 @@ extern struct class *sound_class;
void snd_request_card(int card);
+void snd_device_initialize(struct device *dev, struct snd_card *card); + int snd_register_device_for_dev(int type, struct snd_card *card, int dev, const struct file_operations *f_ops, void *private_data, struct device *device, diff --git a/sound/core/init.c b/sound/core/init.c index 074875d68c15..2f730efe97b6 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -157,6 +157,25 @@ static int get_slot_from_bitmask(int mask, int (*check)(struct module *, int), return mask; /* unchanged */ }
+static void default_release(struct device *dev) +{ +} + +/** + * snd_device_initialize - Initialize struct device for sound devices + * @dev: device to initialize + * @card: card to assign, optional + */ +void snd_device_initialize(struct device *dev, struct snd_card *card) +{ + device_initialize(dev); + if (card) + dev->parent = &card->card_dev; + dev->class = sound_class; + dev->release = default_release; +} +EXPORT_SYMBOL_GPL(snd_device_initialize); + static int snd_card_do_free(struct snd_card *card); static const struct attribute_group *card_dev_attr_groups[];
On 02/02/2015 11:24 AM, Takashi Iwai wrote:
diff --git a/sound/core/init.c b/sound/core/init.c index 074875d68c15..2f730efe97b6 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -157,6 +157,25 @@ static int get_slot_from_bitmask(int mask, int (*check)(struct module *, int), return mask; /* unchanged */ }
+static void default_release(struct device *dev) +{ +}
A empty release callback is pretty much always wrong and typically causes use-after-free bugs. It might be correct in this case, but there should at least be a comment explaining why it is correct. And on the long run things should probably be re-factored to do all memory freeing in a subdevice specific release function.
- Lars
At Mon, 02 Feb 2015 14:31:40 +0100, Lars-Peter Clausen wrote:
On 02/02/2015 11:24 AM, Takashi Iwai wrote:
diff --git a/sound/core/init.c b/sound/core/init.c index 074875d68c15..2f730efe97b6 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -157,6 +157,25 @@ static int get_slot_from_bitmask(int mask, int (*check)(struct module *, int), return mask; /* unchanged */ }
+static void default_release(struct device *dev) +{ +}
A empty release callback is pretty much always wrong and typically causes use-after-free bugs. It might be correct in this case, but there should at least be a comment explaining why it is correct.
Right, a bit more explanation would be better: we have already dev_free callback in snd_device chain, and this would do almost all job. That's why we can leave empty as default there.
And on the long run things should probably be re-factored to do all memory freeing in a subdevice specific release function.
This patchset is one step toward the simplification, indeed.
But, what do you have in mind when you write "a subsystem specific release function"?
thanks,
Takashi
On 02/02/2015 02:40 PM, Takashi Iwai wrote: [...]
And on the long run things should probably be re-factored to do all memory freeing in a subdevice specific release function.
This patchset is one step toward the simplification, indeed.
But, what do you have in mind when you write "a subsystem specific release function"?
"subdev specific". What you already do for e.g. hwdep and MIDI, but for all of the subdevice types, so the empty default function is no longer needed.
The kfree(container_of(...)) stuff.
- Lars
At Mon, 02 Feb 2015 14:43:02 +0100, Lars-Peter Clausen wrote:
On 02/02/2015 02:40 PM, Takashi Iwai wrote: [...]
And on the long run things should probably be re-factored to do all memory freeing in a subdevice specific release function.
This patchset is one step toward the simplification, indeed.
But, what do you have in mind when you write "a subsystem specific release function"?
"subdev specific". What you already do for e.g. hwdep and MIDI, but for all of the subdevice types, so the empty default function is no longer needed.
The kfree(container_of(...)) stuff.
I see. Yes, the whole device_free stuff should be moved to each release function ideally. But as of now, doing it is buggy because of the lack of kobject dependency chain between card and subsystem objects. We'd need to manage the dependency chain among card and subsystems, not only the refcount of opened subsystem files.
Maybe I'll start working on it with the next patchset once after this patchset is merged. Another TODO is to rewrite dev_disconnect and dev_regsiter call chain with another method, e.g. the standard notifier chain. Then we can get rid of snd_device stuff.
Takashi
This patch embeds a struct device for the control device into the card object and avoid the device creation at registration time.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/core.h | 1 + sound/core/control.c | 40 ++++++++++++++++------------------------ 2 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/include/sound/core.h b/include/sound/core.h index de7a878217d7..4b7e04e85e16 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -109,6 +109,7 @@ struct snd_card { private data */ struct list_head devices; /* devices */
+ struct device ctl_dev; /* control device */ unsigned int last_numid; /* last used numeric ID */ struct rw_semaphore controls_rwsem; /* controls list lock */ rwlock_t ctl_files_rwlock; /* ctl_files list lock */ diff --git a/sound/core/control.c b/sound/core/control.c index cd246a0bcd55..e214fabbc671 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1660,19 +1660,10 @@ static const struct file_operations snd_ctl_f_ops = static int snd_ctl_dev_register(struct snd_device *device) { struct snd_card *card = device->device_data; - int err, cardnum; - char name[16];
- if (snd_BUG_ON(!card)) - return -ENXIO; - cardnum = card->number; - if (snd_BUG_ON(cardnum < 0 || cardnum >= SNDRV_CARDS)) - return -ENXIO; - sprintf(name, "controlC%i", cardnum); - if ((err = snd_register_device(SNDRV_DEVICE_TYPE_CONTROL, card, -1, - &snd_ctl_f_ops, card, name)) < 0) - return err; - return 0; + return snd_register_device_for_dev(SNDRV_DEVICE_TYPE_CONTROL, card, + -1, &snd_ctl_f_ops, card, + &card->ctl_dev, NULL, NULL); }
/* @@ -1682,13 +1673,6 @@ static int snd_ctl_dev_disconnect(struct snd_device *device) { struct snd_card *card = device->device_data; struct snd_ctl_file *ctl; - int err, cardnum; - - if (snd_BUG_ON(!card)) - return -ENXIO; - cardnum = card->number; - if (snd_BUG_ON(cardnum < 0 || cardnum >= SNDRV_CARDS)) - return -ENXIO;
read_lock(&card->ctl_files_rwlock); list_for_each_entry(ctl, &card->ctl_files, list) { @@ -1697,10 +1681,7 @@ static int snd_ctl_dev_disconnect(struct snd_device *device) } read_unlock(&card->ctl_files_rwlock);
- if ((err = snd_unregister_device(SNDRV_DEVICE_TYPE_CONTROL, - card, -1)) < 0) - return err; - return 0; + return snd_unregister_device(SNDRV_DEVICE_TYPE_CONTROL, card, -1); }
/* @@ -1717,6 +1698,7 @@ static int snd_ctl_dev_free(struct snd_device *device) snd_ctl_remove(card, control); } up_write(&card->controls_rwsem); + put_device(&card->ctl_dev); return 0; }
@@ -1731,10 +1713,20 @@ int snd_ctl_create(struct snd_card *card) .dev_register = snd_ctl_dev_register, .dev_disconnect = snd_ctl_dev_disconnect, }; + int err;
if (snd_BUG_ON(!card)) return -ENXIO; - return snd_device_new(card, SNDRV_DEV_CONTROL, card, &ops); + if (snd_BUG_ON(card->number < 0 || card->number >= SNDRV_CARDS)) + return -ENXIO; + + snd_device_initialize(&card->ctl_dev, card); + dev_set_name(&card->ctl_dev, "controlC%d", card->number); + + err = snd_device_new(card, SNDRV_DEV_CONTROL, card, &ops); + if (err < 0) + put_device(&card->ctl_dev); + return err; }
/*
Like the previous patch, this one embeds the device object into hwdep object. For a proper object lifecycle, it's freed in the release callback.
This also allows us to create sysfs entries via passing to the groups field of the device without explicit function calls. Since each driver can see the device and touch its groups field directly, we don't need to delegate in hwdep core any longer. So, remove the groups field from snd_hwdep, and let the user (in this case only hda_hwdep.c) modify the device groups.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/hwdep.h | 3 +- sound/core/hwdep.c | 82 ++++++++++++++++++----------------------------- sound/pci/hda/hda_hwdep.c | 7 ++-- 3 files changed, 38 insertions(+), 54 deletions(-)
diff --git a/include/sound/hwdep.h b/include/sound/hwdep.h index ae04a3ec9c77..ab9fcb2f97f0 100644 --- a/include/sound/hwdep.h +++ b/include/sound/hwdep.h @@ -68,8 +68,7 @@ struct snd_hwdep { wait_queue_head_t open_wait; void *private_data; void (*private_free) (struct snd_hwdep *hwdep); - struct device *dev; - const struct attribute_group **groups; + struct device dev;
struct mutex open_mutex; int used; /* reference counter */ diff --git a/sound/core/hwdep.c b/sound/core/hwdep.c index 85096a150eda..506387ba645d 100644 --- a/sound/core/hwdep.c +++ b/sound/core/hwdep.c @@ -38,7 +38,6 @@ MODULE_LICENSE("GPL"); static LIST_HEAD(snd_hwdep_devices); static DEFINE_MUTEX(register_mutex);
-static int snd_hwdep_free(struct snd_hwdep *hwdep); static int snd_hwdep_dev_free(struct snd_device *device); static int snd_hwdep_dev_register(struct snd_device *device); static int snd_hwdep_dev_disconnect(struct snd_device *device); @@ -345,6 +344,11 @@ static const struct file_operations snd_hwdep_f_ops = .mmap = snd_hwdep_mmap, };
+static void release_hwdep_device(struct device *dev) +{ + kfree(container_of(dev, struct snd_hwdep, dev)); +} + /** * snd_hwdep_new - create a new hwdep instance * @card: the card instance @@ -378,48 +382,49 @@ int snd_hwdep_new(struct snd_card *card, char *id, int device, dev_err(card->dev, "hwdep: cannot allocate\n"); return -ENOMEM; } + + init_waitqueue_head(&hwdep->open_wait); + mutex_init(&hwdep->open_mutex); hwdep->card = card; hwdep->device = device; if (id) strlcpy(hwdep->id, id, sizeof(hwdep->id)); + + snd_device_initialize(&hwdep->dev, card); + hwdep->dev.release = release_hwdep_device; + dev_set_name(&hwdep->dev, "hwC%iD%i", card->number, device); #ifdef CONFIG_SND_OSSEMUL hwdep->oss_type = -1; #endif - if ((err = snd_device_new(card, SNDRV_DEV_HWDEP, hwdep, &ops)) < 0) { - snd_hwdep_free(hwdep); + + err = snd_device_new(card, SNDRV_DEV_HWDEP, hwdep, &ops); + if (err < 0) { + put_device(&hwdep->dev); return err; } - init_waitqueue_head(&hwdep->open_wait); - mutex_init(&hwdep->open_mutex); + if (rhwdep) *rhwdep = hwdep; return 0; } EXPORT_SYMBOL(snd_hwdep_new);
-static int snd_hwdep_free(struct snd_hwdep *hwdep) +static int snd_hwdep_dev_free(struct snd_device *device) { + struct snd_hwdep *hwdep = device->device_data; if (!hwdep) return 0; if (hwdep->private_free) hwdep->private_free(hwdep); - kfree(hwdep); + put_device(&hwdep->dev); return 0; }
-static int snd_hwdep_dev_free(struct snd_device *device) -{ - struct snd_hwdep *hwdep = device->device_data; - return snd_hwdep_free(hwdep); -} - static int snd_hwdep_dev_register(struct snd_device *device) { struct snd_hwdep *hwdep = device->device_data; struct snd_card *card = hwdep->card; - struct device *dev; int err; - char name[32];
mutex_lock(®ister_mutex); if (snd_hwdep_search(card, hwdep->device)) { @@ -427,54 +432,31 @@ static int snd_hwdep_dev_register(struct snd_device *device) return -EBUSY; } list_add_tail(&hwdep->list, &snd_hwdep_devices); - sprintf(name, "hwC%iD%i", hwdep->card->number, hwdep->device); - dev = hwdep->dev; - if (!dev) - dev = snd_card_get_device_link(hwdep->card); err = snd_register_device_for_dev(SNDRV_DEVICE_TYPE_HWDEP, hwdep->card, hwdep->device, &snd_hwdep_f_ops, hwdep, - NULL, dev, name); + &hwdep->dev, NULL, NULL); if (err < 0) { - dev_err(dev, - "unable to register hardware dependent device %i:%i\n", - card->number, hwdep->device); + dev_err(&hwdep->dev, "unable to register\n"); list_del(&hwdep->list); mutex_unlock(®ister_mutex); return err; }
- if (hwdep->groups) { - struct device *d = snd_get_device(SNDRV_DEVICE_TYPE_HWDEP, - hwdep->card, hwdep->device); - if (d) { - if (hwdep->private_data) - dev_set_drvdata(d, hwdep->private_data); - err = sysfs_create_groups(&d->kobj, hwdep->groups); - if (err < 0) - dev_warn(dev, - "hwdep %d:%d: cannot create sysfs groups\n", - card->number, hwdep->device); - put_device(d); - } - } - #ifdef CONFIG_SND_OSSEMUL hwdep->ossreg = 0; if (hwdep->oss_type >= 0) { - if ((hwdep->oss_type == SNDRV_OSS_DEVICE_TYPE_DMFM) && (hwdep->device != 0)) { - dev_warn(dev, + if (hwdep->oss_type == SNDRV_OSS_DEVICE_TYPE_DMFM && + hwdep->device) + dev_warn(&hwdep->dev, "only hwdep device 0 can be registered as OSS direct FM device!\n"); - } else { - if (snd_register_oss_device(hwdep->oss_type, - card, hwdep->device, - &snd_hwdep_f_ops, hwdep) < 0) { - dev_err(dev, - "unable to register OSS compatibility device %i:%i\n", - card->number, hwdep->device); - } else - hwdep->ossreg = 1; - } + else if (snd_register_oss_device(hwdep->oss_type, + card, hwdep->device, + &snd_hwdep_f_ops, hwdep) < 0) + dev_warn(&hwdep->dev, + "unable to register OSS compatibility device\n"); + else + hwdep->ossreg = 1; } #endif mutex_unlock(®ister_mutex); diff --git a/sound/pci/hda/hda_hwdep.c b/sound/pci/hda/hda_hwdep.c index 014a7849e8fd..11b5a42b4ec8 100644 --- a/sound/pci/hda/hda_hwdep.c +++ b/sound/pci/hda/hda_hwdep.c @@ -109,7 +109,6 @@ int snd_hda_create_hwdep(struct hda_codec *codec) hwdep->iface = SNDRV_HWDEP_IFACE_HDA; hwdep->private_data = codec; hwdep->exclusive = 1; - hwdep->groups = snd_hda_dev_attr_groups;
hwdep->ops.open = hda_hwdep_open; hwdep->ops.ioctl = hda_hwdep_ioctl; @@ -118,7 +117,11 @@ int snd_hda_create_hwdep(struct hda_codec *codec) #endif
/* link to codec */ - hwdep->dev = &codec->dev; + hwdep->dev.parent = &codec->dev; + + /* for sysfs */ + hwdep->dev.groups = snd_hda_dev_attr_groups; + dev_set_drvdata(&hwdep->dev, codec);
return 0; }
Like previous patches, at this time we embed the struct device into PCM object. However, this needs a bit more caution: struct snd_pcm doesn't own one device but two, for both playback and capture! Thus not struct snd_pcm but struct snd_pcm_str object contains the device.
Along with this change, pcm->dev field is dropped for avoiding confusion. It was meant to point to a non-standard parent. But, since now we can touch each struct device directly, we can manipulate the parent field easily there, too.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/pcm.h | 2 +- sound/aoa/soundbus/i2sbus/pcm.c | 5 ++++- sound/core/pcm.c | 38 +++++++++++++-------------------- sound/pci/hda/hda_controller.c | 3 ++- sound/soc/intel/sst-mfld-platform-pcm.c | 1 - 5 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index b429b73e875e..735bd0cc7347 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -449,6 +449,7 @@ struct snd_pcm_str { #endif #endif struct snd_kcontrol *chmap_kctl; /* channel-mapping controls */ + struct device dev; };
struct snd_pcm { @@ -465,7 +466,6 @@ struct snd_pcm { wait_queue_head_t open_wait; void *private_data; void (*private_free) (struct snd_pcm *pcm); - struct device *dev; /* actual hw device this belongs to */ bool internal; /* pcm is for internal use only */ bool nonatomic; /* whole PCM operations are in non-atomic context */ #if defined(CONFIG_SND_PCM_OSS) || defined(CONFIG_SND_PCM_OSS_MODULE) diff --git a/sound/aoa/soundbus/i2sbus/pcm.c b/sound/aoa/soundbus/i2sbus/pcm.c index 7b74a4ba75f8..a02b7b8d3532 100644 --- a/sound/aoa/soundbus/i2sbus/pcm.c +++ b/sound/aoa/soundbus/i2sbus/pcm.c @@ -968,7 +968,6 @@ i2sbus_attach_codec(struct soundbus_dev *dev, struct snd_card *card, printk(KERN_DEBUG "i2sbus: failed to create pcm\n"); goto out_put_ci_module; } - dev->pcm->dev = &dev->ofdev.dev; }
/* ALSA yet again sucks. @@ -988,6 +987,8 @@ i2sbus_attach_codec(struct soundbus_dev *dev, struct snd_card *card, goto out_put_ci_module; snd_pcm_set_ops(dev->pcm, SNDRV_PCM_STREAM_PLAYBACK, &i2sbus_playback_ops); + dev->pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].dev.parent = + &dev->ofdev.dev; i2sdev->out.created = 1; }
@@ -1003,6 +1004,8 @@ i2sbus_attach_codec(struct soundbus_dev *dev, struct snd_card *card, goto out_put_ci_module; snd_pcm_set_ops(dev->pcm, SNDRV_PCM_STREAM_CAPTURE, &i2sbus_record_ops); + dev->pcm->streams[SNDRV_PCM_STREAM_CAPTURE].dev.parent = + &dev->ofdev.dev; i2sdev->in.created = 1; }
diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 1b7c473720fa..4d5120f7a8ab 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -673,6 +673,8 @@ static inline int snd_pcm_substream_proc_init(struct snd_pcm_substream *substrea static inline int snd_pcm_substream_proc_done(struct snd_pcm_substream *substream) { return 0; } #endif /* CONFIG_SND_VERBOSE_PROCFS */
+static const struct attribute_group *pcm_dev_attr_groups[]; + /** * snd_pcm_new_stream - create a new PCM stream * @pcm: the pcm instance @@ -698,7 +700,15 @@ int snd_pcm_new_stream(struct snd_pcm *pcm, int stream, int substream_count) pstr->stream = stream; pstr->pcm = pcm; pstr->substream_count = substream_count; - if (substream_count > 0 && !pcm->internal) { + if (!substream_count) + return 0; + + snd_device_initialize(&pstr->dev, pcm->card); + pstr->dev.groups = pcm_dev_attr_groups; + dev_set_name(&pstr->dev, "pcmC%iD%i%c", pcm->card->number, pcm->device, + stream == SNDRV_PCM_STREAM_PLAYBACK ? 'p' : 'c'); + + if (!pcm->internal) { err = snd_pcm_stream_proc_init(pstr); if (err < 0) { pcm_err(pcm, "Error in snd_pcm_stream_proc_init\n"); @@ -868,6 +878,8 @@ static void snd_pcm_free_stream(struct snd_pcm_str * pstr) kfree(setup); } #endif + if (pstr->substream_count) + put_device(&pstr->dev); }
static int snd_pcm_free(struct snd_pcm *pcm) @@ -1069,9 +1081,7 @@ static int snd_pcm_dev_register(struct snd_device *device) int cidx, err; struct snd_pcm_substream *substream; struct snd_pcm_notify *notify; - char str[16]; struct snd_pcm *pcm; - struct device *dev;
if (snd_BUG_ON(!device || !device->device_data)) return -ENXIO; @@ -1088,42 +1098,24 @@ static int snd_pcm_dev_register(struct snd_device *device) continue; switch (cidx) { case SNDRV_PCM_STREAM_PLAYBACK: - sprintf(str, "pcmC%iD%ip", pcm->card->number, pcm->device); devtype = SNDRV_DEVICE_TYPE_PCM_PLAYBACK; break; case SNDRV_PCM_STREAM_CAPTURE: - sprintf(str, "pcmC%iD%ic", pcm->card->number, pcm->device); devtype = SNDRV_DEVICE_TYPE_PCM_CAPTURE; break; } - /* device pointer to use, pcm->dev takes precedence if - * it is assigned, otherwise fall back to card's device - * if possible */ - dev = pcm->dev; - if (!dev) - dev = snd_card_get_device_link(pcm->card); /* register pcm */ err = snd_register_device_for_dev(devtype, pcm->card, pcm->device, &snd_pcm_f_ops[cidx], - pcm, NULL, dev, str); + pcm, &pcm->streams[cidx].dev, + NULL, NULL); if (err < 0) { list_del(&pcm->list); mutex_unlock(®ister_mutex); return err; }
- dev = snd_get_device(devtype, pcm->card, pcm->device); - if (dev) { - err = sysfs_create_groups(&dev->kobj, - pcm_dev_attr_groups); - if (err < 0) - dev_warn(dev, - "pcm %d:%d: cannot create sysfs groups\n", - pcm->card->number, pcm->device); - put_device(dev); - } - for (substream = pcm->streams[cidx].substream; substream; substream = substream->next) snd_pcm_timer_init(substream); } diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c index 0cfc9c8c4b4e..712ec5ceba46 100644 --- a/sound/pci/hda/hda_controller.c +++ b/sound/pci/hda/hda_controller.c @@ -939,7 +939,8 @@ static int azx_attach_pcm_stream(struct hda_bus *bus, struct hda_codec *codec, chip->card->dev, size, MAX_PREALLOC_SIZE); /* link to codec */ - pcm->dev = &codec->dev; + for (s = 0; s < 2; s++) + pcm->streams[s].dev.parent = &codec->dev; return 0; }
diff --git a/sound/soc/intel/sst-mfld-platform-pcm.c b/sound/soc/intel/sst-mfld-platform-pcm.c index a1a8d9d91539..2d80c4e12997 100644 --- a/sound/soc/intel/sst-mfld-platform-pcm.c +++ b/sound/soc/intel/sst-mfld-platform-pcm.c @@ -645,7 +645,6 @@ static struct snd_pcm_ops sst_platform_ops = {
static void sst_pcm_free(struct snd_pcm *pcm) { - dev_dbg(pcm->dev, "sst_pcm_free called\n"); snd_pcm_lib_preallocate_free_for_all(pcm); }
Like previous patches, this changes the device management for rawmidi, embedding the struct device into struct snd_rawmidi. The required change is more or less same as hwdep device.
The currently unused dev field is reused as the new embedded struct field now.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/rawmidi.h | 4 +++- sound/core/rawmidi.c | 24 +++++++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/include/sound/rawmidi.h b/include/sound/rawmidi.h index 311dafe6cc4b..f6cbef78db62 100644 --- a/include/sound/rawmidi.h +++ b/include/sound/rawmidi.h @@ -28,6 +28,7 @@ #include <linux/wait.h> #include <linux/mutex.h> #include <linux/workqueue.h> +#include <linux/device.h>
#if defined(CONFIG_SND_SEQUENCER) || defined(CONFIG_SND_SEQUENCER_MODULE) #include <sound/seq_device.h> @@ -139,7 +140,8 @@ struct snd_rawmidi { struct mutex open_mutex; wait_queue_head_t open_wait;
- struct snd_info_entry *dev; + struct device dev; + struct snd_info_entry *proc_entry;
#if defined(CONFIG_SND_SEQUENCER) || defined(CONFIG_SND_SEQUENCER_MODULE) diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c index be18162c380f..932396c81035 100644 --- a/sound/core/rawmidi.c +++ b/sound/core/rawmidi.c @@ -1443,6 +1443,11 @@ static int snd_rawmidi_alloc_substreams(struct snd_rawmidi *rmidi, return 0; }
+static void release_rawmidi_device(struct device *dev) +{ + kfree(container_of(dev, struct snd_rawmidi, dev)); +} + /** * snd_rawmidi_new - create a rawmidi instance * @card: the card instance @@ -1487,6 +1492,11 @@ int snd_rawmidi_new(struct snd_card *card, char *id, int device,
if (id != NULL) strlcpy(rmidi->id, id, sizeof(rmidi->id)); + + snd_device_initialize(&rmidi->dev, card); + rmidi->dev.release = release_rawmidi_device; + dev_set_name(&rmidi->dev, "midiC%iD%i", card->number, device); + if ((err = snd_rawmidi_alloc_substreams(rmidi, &rmidi->streams[SNDRV_RAWMIDI_STREAM_INPUT], SNDRV_RAWMIDI_STREAM_INPUT, @@ -1538,7 +1548,7 @@ static int snd_rawmidi_free(struct snd_rawmidi *rmidi) snd_rawmidi_free_substreams(&rmidi->streams[SNDRV_RAWMIDI_STREAM_OUTPUT]); if (rmidi->private_free) rmidi->private_free(rmidi); - kfree(rmidi); + put_device(&rmidi->dev); return 0; }
@@ -1571,12 +1581,12 @@ static int snd_rawmidi_dev_register(struct snd_device *device) return -EBUSY; } list_add_tail(&rmidi->list, &snd_rawmidi_devices); - sprintf(name, "midiC%iD%i", rmidi->card->number, rmidi->device); - if ((err = snd_register_device(SNDRV_DEVICE_TYPE_RAWMIDI, - rmidi->card, rmidi->device, - &snd_rawmidi_f_ops, rmidi, name)) < 0) { - rmidi_err(rmidi, "unable to register rawmidi device %i:%i\n", - rmidi->card->number, rmidi->device); + err = snd_register_device_for_dev(SNDRV_DEVICE_TYPE_RAWMIDI, + rmidi->card, rmidi->device, + &snd_rawmidi_f_ops, rmidi, + &rmidi->dev, NULL, NULL); + if (err < 0) { + rmidi_err(rmidi, "unable to register\n"); list_del(&rmidi->list); mutex_unlock(®ister_mutex); return err;
... instead of card's device. This will be helpful to distinguish errors from multiple rawmidi devices on a single card.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/rawmidi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c index 932396c81035..bccbf7e375d6 100644 --- a/sound/core/rawmidi.c +++ b/sound/core/rawmidi.c @@ -57,11 +57,11 @@ static LIST_HEAD(snd_rawmidi_devices); static DEFINE_MUTEX(register_mutex);
#define rmidi_err(rmidi, fmt, args...) \ - dev_err((rmidi)->card->dev, fmt, ##args) + dev_err(&(rmidi)->dev, fmt, ##args) #define rmidi_warn(rmidi, fmt, args...) \ - dev_warn((rmidi)->card->dev, fmt, ##args) + dev_warn(&(rmidi)->dev, fmt, ##args) #define rmidi_dbg(rmidi, fmt, args...) \ - dev_dbg((rmidi)->card->dev, fmt, ##args) + dev_dbg(&(rmidi)->dev, fmt, ##args)
static struct snd_rawmidi *snd_rawmidi_search(struct snd_card *card, int device) {
... instead of just printing errors.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/timer.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/sound/core/timer.c b/sound/core/timer.c index 777a45e08e53..28b673d4f812 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -1942,6 +1942,15 @@ static const struct file_operations snd_timer_f_ops = .fasync = snd_timer_user_fasync, };
+/* unregister the system timer */ +static void snd_timer_free_all(void) +{ + struct snd_timer *timer, *n; + + list_for_each_entry_safe(timer, n, &snd_timer_list, device_list) + snd_timer_free(timer); +} + /* * ENTRY functions */ @@ -1955,25 +1964,28 @@ static int __init alsa_timer_init(void) "system timer"); #endif
- if ((err = snd_timer_register_system()) < 0) + err = snd_timer_register_system(); + if (err < 0) { pr_err("ALSA: unable to register system timer (%i)\n", err); - if ((err = snd_register_device(SNDRV_DEVICE_TYPE_TIMER, NULL, 0, - &snd_timer_f_ops, NULL, "timer")) < 0) + return err; + } + + err = snd_register_device(SNDRV_DEVICE_TYPE_TIMER, NULL, 0, + &snd_timer_f_ops, NULL, "timer"); + if (err < 0) { pr_err("ALSA: unable to register timer device (%i)\n", err); + snd_timer_free_all(); + return err; + } + snd_timer_proc_init(); return 0; }
static void __exit alsa_timer_exit(void) { - struct list_head *p, *n; - snd_unregister_device(SNDRV_DEVICE_TYPE_TIMER, NULL, 0); - /* unregister the system timer */ - list_for_each_safe(p, n, &snd_timer_list) { - struct snd_timer *timer = list_entry(p, struct snd_timer, device_list); - snd_timer_free(timer); - } + snd_timer_free_all(); snd_timer_proc_done(); #ifdef SNDRV_OSS_INFO_DEV_TIMERS snd_oss_info_unregister(SNDRV_OSS_INFO_DEV_TIMERS, SNDRV_CARDS - 1);
This is a relatively straightforward change, using the struct device directly for managing the ALSA timer device.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/timer.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/sound/core/timer.c b/sound/core/timer.c index 28b673d4f812..dae40ac11e04 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -1951,6 +1951,8 @@ static void snd_timer_free_all(void) snd_timer_free(timer); }
+static struct device timer_dev; + /* * ENTRY functions */ @@ -1959,6 +1961,9 @@ static int __init alsa_timer_init(void) { int err;
+ snd_device_initialize(&timer_dev, NULL); + dev_set_name(&timer_dev, "timer"); + #ifdef SNDRV_OSS_INFO_DEV_TIMERS snd_oss_info_register(SNDRV_OSS_INFO_DEV_TIMERS, SNDRV_CARDS - 1, "system timer"); @@ -1967,14 +1972,17 @@ static int __init alsa_timer_init(void) err = snd_timer_register_system(); if (err < 0) { pr_err("ALSA: unable to register system timer (%i)\n", err); + put_device(&timer_dev); return err; }
- err = snd_register_device(SNDRV_DEVICE_TYPE_TIMER, NULL, 0, - &snd_timer_f_ops, NULL, "timer"); + err = snd_register_device_for_dev(SNDRV_DEVICE_TYPE_TIMER, NULL, 0, + &snd_timer_f_ops, NULL, + &timer_dev, NULL, NULL); if (err < 0) { pr_err("ALSA: unable to register timer device (%i)\n", err); snd_timer_free_all(); + put_device(&timer_dev); return err; }
@@ -1986,6 +1994,7 @@ static void __exit alsa_timer_exit(void) { snd_unregister_device(SNDRV_DEVICE_TYPE_TIMER, NULL, 0); snd_timer_free_all(); + put_device(&timer_dev); snd_timer_proc_done(); #ifdef SNDRV_OSS_INFO_DEV_TIMERS snd_oss_info_unregister(SNDRV_OSS_INFO_DEV_TIMERS, SNDRV_CARDS - 1);
Like the previous change for the timer device, this patch changes the device management for the ALSA sequencer device using the struct device directly.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/seq/seq_clientmgr.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 225c73152ee9..65b320ec66f1 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2571,6 +2571,8 @@ static const struct file_operations snd_seq_f_ops = .compat_ioctl = snd_seq_ioctl_compat, };
+static struct device seq_dev; + /* * register sequencer device */ @@ -2578,12 +2580,18 @@ int __init snd_sequencer_device_init(void) { int err;
+ snd_device_initialize(&seq_dev, NULL); + dev_set_name(&seq_dev, "seq"); + if (mutex_lock_interruptible(®ister_mutex)) return -ERESTARTSYS;
- if ((err = snd_register_device(SNDRV_DEVICE_TYPE_SEQUENCER, NULL, 0, - &snd_seq_f_ops, NULL, "seq")) < 0) { + err = snd_register_device_for_dev(SNDRV_DEVICE_TYPE_SEQUENCER, NULL, 0, + &snd_seq_f_ops, NULL, + &seq_dev, NULL, NULL); + if (err < 0) { mutex_unlock(®ister_mutex); + put_device(&seq_dev); return err; } @@ -2600,4 +2608,5 @@ int __init snd_sequencer_device_init(void) void __exit snd_sequencer_device_done(void) { snd_unregister_device(SNDRV_DEVICE_TYPE_SEQUENCER, NULL, 0); + put_device(&seq_dev); }
Like previous patches, this one embeds the struct device into struct snd_compr. As the dev field wasn't used beforehand, it's reused as the new device struct.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/compress_driver.h | 4 ++-- sound/core/compress_offload.c | 24 +++++++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h index 396e8f73670a..1d0593b52573 100644 --- a/include/sound/compress_driver.h +++ b/include/sound/compress_driver.h @@ -134,7 +134,7 @@ struct snd_compr_ops { /** * struct snd_compr: Compressed device * @name: DSP device name - * @dev: Device pointer + * @dev: associated device instance * @ops: pointer to DSP callbacks * @private_data: pointer to DSP pvt data * @card: sound card pointer @@ -144,7 +144,7 @@ struct snd_compr_ops { */ struct snd_compr { const char *name; - struct device *dev; + struct device dev; struct snd_compr_ops *ops; void *private_data; struct snd_card *card; diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index 89028fab64fd..cb58c3f7f80c 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -868,12 +868,13 @@ static int snd_compress_dev_register(struct snd_device *device) return -EBADFD; compr = device->device_data;
- sprintf(str, "comprC%iD%i", compr->card->number, compr->device); pr_debug("reg %s for device %s, direction %d\n", str, compr->name, compr->direction); /* register compressed device */ - ret = snd_register_device(SNDRV_DEVICE_TYPE_COMPRESS, compr->card, - compr->device, &snd_compr_file_ops, compr, str); + ret = snd_register_device_for_dev(SNDRV_DEVICE_TYPE_COMPRESS, + compr->card, compr->device, + &snd_compr_file_ops, compr, + &compr->dev, NULL, NULL); if (ret < 0) { pr_err("snd_register_device failed\n %d", ret); return ret; @@ -892,6 +893,15 @@ static int snd_compress_dev_disconnect(struct snd_device *device) return 0; }
+static int snd_compress_dev_free(struct snd_device *device) +{ + struct snd_compr *compr; + + compr = device->device_data; + put_device(&compr->dev); + return 0; +} + /* * snd_compress_new: create new compress device * @card: sound card pointer @@ -903,7 +913,7 @@ int snd_compress_new(struct snd_card *card, int device, int dirn, struct snd_compr *compr) { static struct snd_device_ops ops = { - .dev_free = NULL, + .dev_free = snd_compress_dev_free, .dev_register = snd_compress_dev_register, .dev_disconnect = snd_compress_dev_disconnect, }; @@ -911,6 +921,10 @@ int snd_compress_new(struct snd_card *card, int device, compr->card = card; compr->device = device; compr->direction = dirn; + + snd_device_initialize(&compr->dev, card); + dev_set_name(&compr->dev, "comprC%iD%i", card->number, device); + return snd_device_new(card, SNDRV_DEV_COMPRESS, compr, &ops); } EXPORT_SYMBOL_GPL(snd_compress_new); @@ -948,7 +962,7 @@ int snd_compress_register(struct snd_compr *device) { int retval;
- if (device->name == NULL || device->dev == NULL || device->ops == NULL) + if (device->name == NULL || device->ops == NULL) return -EINVAL;
pr_debug("Registering compressed device %s\n", device->name);
Now that all callers have been replaced with snd_device_register_for_dev(), let's drop the obsolete device registration code and concentrate only on the code handling struct device directly. That said,
- remove the old snd_device_register(), - rename snd_device_register_for_dev() with snd_device_register(), - drop superfluous arguments from snd_device_register(), - change snd_unregister_device() to pass the device pointer directly
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/core.h | 39 +++------------------------ sound/core/compress_offload.c | 10 +++---- sound/core/control.c | 7 +++-- sound/core/hwdep.c | 9 +++---- sound/core/pcm.c | 21 ++++----------- sound/core/rawmidi.c | 11 ++++---- sound/core/seq/seq_clientmgr.c | 7 +++-- sound/core/sound.c | 61 ++++++++++++++++-------------------------- sound/core/timer.c | 7 +++-- 9 files changed, 54 insertions(+), 118 deletions(-)
diff --git a/include/sound/core.h b/include/sound/core.h index 4b7e04e85e16..67ac309bb218 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -187,7 +187,6 @@ struct snd_minor { int type; /* SNDRV_DEVICE_TYPE_XXX */ int card; /* card number */ int device; /* device number */ - bool created; const struct file_operations *f_ops; /* file operations */ void *private_data; /* private data for f_ops->open */ struct device *dev; /* device for sysfs */ @@ -210,40 +209,10 @@ void snd_request_card(int card);
void snd_device_initialize(struct device *dev, struct snd_card *card);
-int snd_register_device_for_dev(int type, struct snd_card *card, int dev, - const struct file_operations *f_ops, - void *private_data, struct device *device, - struct device *parent, const char *name); - -/** - * snd_register_device - Register the ALSA device file for the card - * @type: the device type, SNDRV_DEVICE_TYPE_XXX - * @card: the card instance - * @dev: the device index - * @f_ops: the file operations - * @private_data: user pointer for f_ops->open() - * @name: the device file name - * - * Registers an ALSA device file for the given card. - * The operators have to be set in reg parameter. - * - * This function uses the card's device pointer to link to the - * correct &struct device. - * - * Return: Zero if successful, or a negative error code on failure. - */ -static inline int snd_register_device(int type, struct snd_card *card, int dev, - const struct file_operations *f_ops, - void *private_data, - const char *name) -{ - return snd_register_device_for_dev(type, card, dev, f_ops, - private_data, NULL, - snd_card_get_device_link(card), - name); -} - -int snd_unregister_device(int type, struct snd_card *card, int dev); +int snd_register_device(int type, struct snd_card *card, int dev, + const struct file_operations *f_ops, + void *private_data, struct device *device); +int snd_unregister_device(struct device *dev); void *snd_lookup_minor_data(unsigned int minor, int type); struct device *snd_get_device(int type, struct snd_card *card, int dev);
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index cb58c3f7f80c..b123c42e7dc8 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -871,10 +871,9 @@ static int snd_compress_dev_register(struct snd_device *device) pr_debug("reg %s for device %s, direction %d\n", str, compr->name, compr->direction); /* register compressed device */ - ret = snd_register_device_for_dev(SNDRV_DEVICE_TYPE_COMPRESS, - compr->card, compr->device, - &snd_compr_file_ops, compr, - &compr->dev, NULL, NULL); + ret = snd_register_device(SNDRV_DEVICE_TYPE_COMPRESS, + compr->card, compr->device, + &snd_compr_file_ops, compr, &compr->dev); if (ret < 0) { pr_err("snd_register_device failed\n %d", ret); return ret; @@ -888,8 +887,7 @@ static int snd_compress_dev_disconnect(struct snd_device *device) struct snd_compr *compr;
compr = device->device_data; - snd_unregister_device(SNDRV_DEVICE_TYPE_COMPRESS, compr->card, - compr->device); + snd_unregister_device(&compr->dev); return 0; }
diff --git a/sound/core/control.c b/sound/core/control.c index e214fabbc671..60caba1f2211 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1661,9 +1661,8 @@ static int snd_ctl_dev_register(struct snd_device *device) { struct snd_card *card = device->device_data;
- return snd_register_device_for_dev(SNDRV_DEVICE_TYPE_CONTROL, card, - -1, &snd_ctl_f_ops, card, - &card->ctl_dev, NULL, NULL); + return snd_register_device(SNDRV_DEVICE_TYPE_CONTROL, card, -1, + &snd_ctl_f_ops, card, &card->ctl_dev); }
/* @@ -1681,7 +1680,7 @@ static int snd_ctl_dev_disconnect(struct snd_device *device) } read_unlock(&card->ctl_files_rwlock);
- return snd_unregister_device(SNDRV_DEVICE_TYPE_CONTROL, card, -1); + return snd_unregister_device(&card->ctl_dev); }
/* diff --git a/sound/core/hwdep.c b/sound/core/hwdep.c index 506387ba645d..84244a5143cf 100644 --- a/sound/core/hwdep.c +++ b/sound/core/hwdep.c @@ -432,10 +432,9 @@ static int snd_hwdep_dev_register(struct snd_device *device) return -EBUSY; } list_add_tail(&hwdep->list, &snd_hwdep_devices); - err = snd_register_device_for_dev(SNDRV_DEVICE_TYPE_HWDEP, - hwdep->card, hwdep->device, - &snd_hwdep_f_ops, hwdep, - &hwdep->dev, NULL, NULL); + err = snd_register_device(SNDRV_DEVICE_TYPE_HWDEP, + hwdep->card, hwdep->device, + &snd_hwdep_f_ops, hwdep, &hwdep->dev); if (err < 0) { dev_err(&hwdep->dev, "unable to register\n"); list_del(&hwdep->list); @@ -480,7 +479,7 @@ static int snd_hwdep_dev_disconnect(struct snd_device *device) if (hwdep->ossreg) snd_unregister_oss_device(hwdep->oss_type, hwdep->card, hwdep->device); #endif - snd_unregister_device(SNDRV_DEVICE_TYPE_HWDEP, hwdep->card, hwdep->device); + snd_unregister_device(&hwdep->dev); list_del_init(&hwdep->list); mutex_unlock(&hwdep->open_mutex); mutex_unlock(®ister_mutex); diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 4d5120f7a8ab..0345e53a340c 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -1105,11 +1105,9 @@ static int snd_pcm_dev_register(struct snd_device *device) break; } /* register pcm */ - err = snd_register_device_for_dev(devtype, pcm->card, - pcm->device, - &snd_pcm_f_ops[cidx], - pcm, &pcm->streams[cidx].dev, - NULL, NULL); + err = snd_register_device(devtype, pcm->card, pcm->device, + &snd_pcm_f_ops[cidx], pcm, + &pcm->streams[cidx].dev); if (err < 0) { list_del(&pcm->list); mutex_unlock(®ister_mutex); @@ -1132,7 +1130,7 @@ static int snd_pcm_dev_disconnect(struct snd_device *device) struct snd_pcm *pcm = device->device_data; struct snd_pcm_notify *notify; struct snd_pcm_substream *substream; - int cidx, devtype; + int cidx;
mutex_lock(®ister_mutex); if (list_empty(&pcm->list)) @@ -1155,16 +1153,7 @@ static int snd_pcm_dev_disconnect(struct snd_device *device) notify->n_disconnect(pcm); } for (cidx = 0; cidx < 2; cidx++) { - devtype = -1; - switch (cidx) { - case SNDRV_PCM_STREAM_PLAYBACK: - devtype = SNDRV_DEVICE_TYPE_PCM_PLAYBACK; - break; - case SNDRV_PCM_STREAM_CAPTURE: - devtype = SNDRV_DEVICE_TYPE_PCM_CAPTURE; - break; - } - snd_unregister_device(devtype, pcm->card, pcm->device); + snd_unregister_device(&pcm->streams[cidx].dev); if (pcm->streams[cidx].chmap_kctl) { snd_ctl_remove(pcm->card, pcm->streams[cidx].chmap_kctl); pcm->streams[cidx].chmap_kctl = NULL; diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c index bccbf7e375d6..b5a748596fc4 100644 --- a/sound/core/rawmidi.c +++ b/sound/core/rawmidi.c @@ -1581,10 +1581,9 @@ static int snd_rawmidi_dev_register(struct snd_device *device) return -EBUSY; } list_add_tail(&rmidi->list, &snd_rawmidi_devices); - err = snd_register_device_for_dev(SNDRV_DEVICE_TYPE_RAWMIDI, - rmidi->card, rmidi->device, - &snd_rawmidi_f_ops, rmidi, - &rmidi->dev, NULL, NULL); + err = snd_register_device(SNDRV_DEVICE_TYPE_RAWMIDI, + rmidi->card, rmidi->device, + &snd_rawmidi_f_ops, rmidi, &rmidi->dev); if (err < 0) { rmidi_err(rmidi, "unable to register\n"); list_del(&rmidi->list); @@ -1593,7 +1592,7 @@ static int snd_rawmidi_dev_register(struct snd_device *device) } if (rmidi->ops && rmidi->ops->dev_register && (err = rmidi->ops->dev_register(rmidi)) < 0) { - snd_unregister_device(SNDRV_DEVICE_TYPE_RAWMIDI, rmidi->card, rmidi->device); + snd_unregister_device(&rmidi->dev); list_del(&rmidi->list); mutex_unlock(®ister_mutex); return err; @@ -1681,7 +1680,7 @@ static int snd_rawmidi_dev_disconnect(struct snd_device *device) rmidi->ossreg = 0; } #endif /* CONFIG_SND_OSSEMUL */ - snd_unregister_device(SNDRV_DEVICE_TYPE_RAWMIDI, rmidi->card, rmidi->device); + snd_unregister_device(&rmidi->dev); mutex_unlock(&rmidi->open_mutex); mutex_unlock(®ister_mutex); return 0; diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 65b320ec66f1..2b62cd021bc5 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2586,9 +2586,8 @@ int __init snd_sequencer_device_init(void) if (mutex_lock_interruptible(®ister_mutex)) return -ERESTARTSYS;
- err = snd_register_device_for_dev(SNDRV_DEVICE_TYPE_SEQUENCER, NULL, 0, - &snd_seq_f_ops, NULL, - &seq_dev, NULL, NULL); + err = snd_register_device(SNDRV_DEVICE_TYPE_SEQUENCER, NULL, 0, + &snd_seq_f_ops, NULL, &seq_dev); if (err < 0) { mutex_unlock(®ister_mutex); put_device(&seq_dev); @@ -2607,6 +2606,6 @@ int __init snd_sequencer_device_init(void) */ void __exit snd_sequencer_device_done(void) { - snd_unregister_device(SNDRV_DEVICE_TYPE_SEQUENCER, NULL, 0); + snd_unregister_device(&seq_dev); put_device(&seq_dev); } diff --git a/sound/core/sound.c b/sound/core/sound.c index ea1af1acdbe9..515e62506541 100644 --- a/sound/core/sound.c +++ b/sound/core/sound.c @@ -242,30 +242,30 @@ static int snd_kernel_minor(int type, struct snd_card *card, int dev) #endif
/** - * snd_register_device_for_dev - Register the ALSA device file for the card + * snd_register_device - Register the ALSA device file for the card * @type: the device type, SNDRV_DEVICE_TYPE_XXX * @card: the card instance * @dev: the device index * @f_ops: the file operations * @private_data: user pointer for f_ops->open() - * @device: the device to register, NULL to create a new one - * @parent: the &struct device to link this new device to (only for device=NULL) - * @name: the device file name (only for device=NULL) + * @device: the device to register * * Registers an ALSA device file for the given card. * The operators have to be set in reg parameter. * * Return: Zero if successful, or a negative error code on failure. */ -int snd_register_device_for_dev(int type, struct snd_card *card, int dev, - const struct file_operations *f_ops, - void *private_data, struct device *device, - struct device *parent, const char *name) +int snd_register_device(int type, struct snd_card *card, int dev, + const struct file_operations *f_ops, + void *private_data, struct device *device) { int minor; int err = 0; struct snd_minor *preg;
+ if (snd_BUG_ON(!device)) + return -EINVAL; + preg = kmalloc(sizeof *preg, GFP_KERNEL); if (preg == NULL) return -ENOMEM; @@ -288,19 +288,9 @@ int snd_register_device_for_dev(int type, struct snd_card *card, int dev, goto error; }
- if (device) { - preg->created = false; - preg->dev = device; - device->devt = MKDEV(major, minor); - err = device_add(device); - } else { - preg->created = true; - preg->dev = device_create(sound_class, parent, - MKDEV(major, minor), private_data, - "%s", name); - if (IS_ERR(preg->dev)) - err = PTR_ERR(preg->dev); - } + preg->dev = device; + device->devt = MKDEV(major, minor); + err = device_add(device); if (err < 0) goto error;
@@ -311,8 +301,7 @@ int snd_register_device_for_dev(int type, struct snd_card *card, int dev, kfree(preg); return err; } - -EXPORT_SYMBOL(snd_register_device_for_dev); +EXPORT_SYMBOL(snd_register_device);
/* find the matching minor record * return the index of snd_minor, or -1 if not found @@ -343,30 +332,26 @@ static int find_snd_minor(int type, struct snd_card *card, int dev) * * Return: Zero if successful, or a negative error code on failure. */ -int snd_unregister_device(int type, struct snd_card *card, int dev) +int snd_unregister_device(struct device *dev) { int minor; struct snd_minor *preg;
mutex_lock(&sound_mutex); - minor = find_snd_minor(type, card, dev); - if (minor < 0) { - mutex_unlock(&sound_mutex); - return -EINVAL; + for (minor = 0; minor < ARRAY_SIZE(snd_minors); ++minor) { + preg = snd_minors[minor]; + if (preg && preg->dev == dev) { + snd_minors[minor] = NULL; + device_del(dev); + kfree(preg); + break; + } } - - preg = snd_minors[minor]; - if (preg && !preg->created) - device_del(preg->dev); - else - device_destroy(sound_class, MKDEV(major, minor)); - - kfree(snd_minors[minor]); - snd_minors[minor] = NULL; mutex_unlock(&sound_mutex); + if (minor >= ARRAY_SIZE(snd_minors)) + return -ENOENT; return 0; } - EXPORT_SYMBOL(snd_unregister_device);
/** diff --git a/sound/core/timer.c b/sound/core/timer.c index dae40ac11e04..9f0c703ef081 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -1976,9 +1976,8 @@ static int __init alsa_timer_init(void) return err; }
- err = snd_register_device_for_dev(SNDRV_DEVICE_TYPE_TIMER, NULL, 0, - &snd_timer_f_ops, NULL, - &timer_dev, NULL, NULL); + err = snd_register_device(SNDRV_DEVICE_TYPE_TIMER, NULL, 0, + &snd_timer_f_ops, NULL, &timer_dev); if (err < 0) { pr_err("ALSA: unable to register timer device (%i)\n", err); snd_timer_free_all(); @@ -1992,7 +1991,7 @@ static int __init alsa_timer_init(void)
static void __exit alsa_timer_exit(void) { - snd_unregister_device(SNDRV_DEVICE_TYPE_TIMER, NULL, 0); + snd_unregister_device(&timer_dev); snd_timer_free_all(); put_device(&timer_dev); snd_timer_proc_done();
Since the device is no longer hidden but embedded into each component, we no longer need snd_get_device(). Let's drop it and relevant codes.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/core.h | 1 - sound/core/sound.c | 43 ------------------------------------------- 2 files changed, 44 deletions(-)
diff --git a/include/sound/core.h b/include/sound/core.h index 67ac309bb218..58882bfacdd7 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -214,7 +214,6 @@ int snd_register_device(int type, struct snd_card *card, int dev, void *private_data, struct device *device); int snd_unregister_device(struct device *dev); void *snd_lookup_minor_data(unsigned int minor, int type); -struct device *snd_get_device(int type, struct snd_card *card, int dev);
#ifdef CONFIG_SND_OSSEMUL int snd_register_oss_device(int type, struct snd_card *card, int dev, diff --git a/sound/core/sound.c b/sound/core/sound.c index 515e62506541..6f8ea40a7a8e 100644 --- a/sound/core/sound.c +++ b/sound/core/sound.c @@ -303,24 +303,6 @@ int snd_register_device(int type, struct snd_card *card, int dev, } EXPORT_SYMBOL(snd_register_device);
-/* find the matching minor record - * return the index of snd_minor, or -1 if not found - */ -static int find_snd_minor(int type, struct snd_card *card, int dev) -{ - int cardnum, minor; - struct snd_minor *mptr; - - cardnum = card ? card->number : -1; - for (minor = 0; minor < ARRAY_SIZE(snd_minors); ++minor) - if ((mptr = snd_minors[minor]) != NULL && - mptr->type == type && - mptr->card == cardnum && - mptr->device == dev) - return minor; - return -1; -} - /** * snd_unregister_device - unregister the device on the given card * @type: the device type, SNDRV_DEVICE_TYPE_XXX @@ -354,31 +336,6 @@ int snd_unregister_device(struct device *dev) } EXPORT_SYMBOL(snd_unregister_device);
-/** - * snd_get_device - get the assigned device to the given type and device number - * @type: the device type, SNDRV_DEVICE_TYPE_XXX - * @card:the card instance - * @dev: the device index - * - * The caller needs to release it via put_device() after using it. - */ -struct device *snd_get_device(int type, struct snd_card *card, int dev) -{ - int minor; - struct device *d = NULL; - - mutex_lock(&sound_mutex); - minor = find_snd_minor(type, card, dev); - if (minor >= 0) { - d = snd_minors[minor]->dev; - if (d) - get_device(d); - } - mutex_unlock(&sound_mutex); - return d; -} -EXPORT_SYMBOL(snd_get_device); - #ifdef CONFIG_PROC_FS /* * INFO PART
Dne 2.2.2015 v 11:24 Takashi Iwai napsal(a):
Hi,
this is a series of relatively small patches for refactoring the ALSA core device management. I experimented a similar thing some time ago but didn't merge it in the end due to too intrusive changes. At this time, the approach is rather minimalistic: only the device registration part is replaced by embedding the struct device into each ALSA device object.
Looks good. Thanks. For all patches:
Reviewed-by: Jaroslav Kysela perex@perex.cz
At Mon, 02 Feb 2015 12:01:57 +0100, Jaroslav Kysela wrote:
Dne 2.2.2015 v 11:24 Takashi Iwai napsal(a):
Hi,
this is a series of relatively small patches for refactoring the ALSA core device management. I experimented a similar thing some time ago but didn't merge it in the end due to too intrusive changes. At this time, the approach is rather minimalistic: only the device registration part is replaced by embedding the struct device into each ALSA device object.
Looks good. Thanks. For all patches:
Reviewed-by: Jaroslav Kysela perex@perex.cz
Thanks, updated the changelog in test/snd-device branch now.
Takashi
participants (3)
-
Jaroslav Kysela
-
Lars-Peter Clausen
-
Takashi Iwai