[alsa-devel] [PATCH v2 1/2] ALSA: Use priority list for managing device list
Basically, the device type specifies the priority of the device to be registered / freed, too. However, the priority value isn't well utilized but only it's checked as a group. This results in inconsistent register and free order (where each of them should be in reversed direction).
This patch simplifies the device list management code by simply inserting a list entry at creation time in an incremental order for the priority value. Since we can just follow the link for register, disconnect and free calls, we don't have to specify the group; so the whole enum definitions are also simplified as well.
The visible change to outside is that the priorities of some object types are revisited. For example, now the SNDRV_DEV_LOWLEVEL object is registered before others (control, PCM, etc) and, in return, released after others. Similarly, SNDRV_DEV_CODEC is in a lower priority than SNDRV_DEV_BUS for ensuring the dependency.
Also, the unused SNDRV_DEV_TOPLEVEL, SNDRV_DEV_LOWLEVEL_PRE and SNDRV_DEV_LOWLEVEL_NORMAL are removed as a cleanup.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/core.h | 28 ++++++++-------------- sound/core/device.c | 66 +++++++++++++++++++++++++++++----------------------- sound/core/init.c | 14 +++-------- 3 files changed, 50 insertions(+), 58 deletions(-)
diff --git a/include/sound/core.h b/include/sound/core.h index a3e3e89b63b6..5927378dc692 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -46,24 +46,22 @@ struct completion;
/* device allocation stuff */
-#define SNDRV_DEV_TYPE_RANGE_SIZE 0x1000 - +/* type of the object used in snd_device_*() + * this also defines the calling order + */ enum snd_device_type { - SNDRV_DEV_TOPLEVEL = 0, - SNDRV_DEV_CONTROL = 1, - SNDRV_DEV_LOWLEVEL_PRE = 2, - SNDRV_DEV_LOWLEVEL_NORMAL = 0x1000, + SNDRV_DEV_LOWLEVEL, + SNDRV_DEV_CONTROL, + SNDRV_DEV_INFO, + SNDRV_DEV_BUS, + SNDRV_DEV_CODEC, SNDRV_DEV_PCM, + SNDRV_DEV_COMPRESS, SNDRV_DEV_RAWMIDI, SNDRV_DEV_TIMER, SNDRV_DEV_SEQUENCER, SNDRV_DEV_HWDEP, - SNDRV_DEV_INFO, - SNDRV_DEV_BUS, - SNDRV_DEV_CODEC, SNDRV_DEV_JACK, - SNDRV_DEV_COMPRESS, - SNDRV_DEV_LOWLEVEL = 0x2000, };
enum snd_device_state { @@ -72,12 +70,6 @@ enum snd_device_state { SNDRV_DEV_DISCONNECTED, };
-enum snd_device_cmd { - SNDRV_DEV_CMD_PRE, - SNDRV_DEV_CMD_NORMAL, - SNDRV_DEV_CMD_POST, -}; - struct snd_device;
struct snd_device_ops { @@ -321,7 +313,7 @@ int snd_device_register_all(struct snd_card *card); int snd_device_disconnect(struct snd_card *card, void *device_data); int snd_device_disconnect_all(struct snd_card *card); int snd_device_free(struct snd_card *card, void *device_data); -int snd_device_free_all(struct snd_card *card, enum snd_device_cmd cmd); +int snd_device_free_all(struct snd_card *card);
/* isadma.c */
diff --git a/sound/core/device.c b/sound/core/device.c index 856bfdc79cee..53291ff21a09 100644 --- a/sound/core/device.c +++ b/sound/core/device.c @@ -45,6 +45,7 @@ int snd_device_new(struct snd_card *card, enum snd_device_type type, void *device_data, struct snd_device_ops *ops) { struct snd_device *dev; + struct list_head *p;
if (snd_BUG_ON(!card || !device_data || !ops)) return -ENXIO; @@ -53,17 +54,36 @@ int snd_device_new(struct snd_card *card, enum snd_device_type type, dev_err(card->dev, "Cannot allocate device, type=%d\n", type); return -ENOMEM; } + INIT_LIST_HEAD(&dev->list); dev->card = card; dev->type = type; dev->state = SNDRV_DEV_BUILD; dev->device_data = device_data; dev->ops = ops; - list_add(&dev->list, &card->devices); /* add to the head of list */ + + /* insert the entry in an incrementally sorted list */ + list_for_each_prev(p, &card->devices) { + struct snd_device *pdev = list_entry(p, struct snd_device, list); + if ((unsigned int)pdev->type <= (unsigned int)type) + break; + } + + list_add(&dev->list, p); return 0; } - EXPORT_SYMBOL(snd_device_new);
+static struct snd_device *look_for_dev(struct snd_card *card, void *device_data) +{ + struct snd_device *dev; + + list_for_each_entry(dev, &card->devices, list) + if (dev->device_data == device_data) + return dev; + + return NULL; +} + /** * snd_device_free - release the device from the card * @card: the card instance @@ -82,9 +102,8 @@ int snd_device_free(struct snd_card *card, void *device_data) if (snd_BUG_ON(!card || !device_data)) return -ENXIO; - list_for_each_entry(dev, &card->devices, list) { - if (dev->device_data != device_data) - continue; + dev = look_for_dev(card, device_data); + if (dev) { /* unlink */ list_del(&dev->list); if (dev->state == SNDRV_DEV_REGISTERED && @@ -103,7 +122,6 @@ int snd_device_free(struct snd_card *card, void *device_data) device_data, __builtin_return_address(0)); return -ENXIO; } - EXPORT_SYMBOL(snd_device_free);
/** @@ -125,9 +143,8 @@ int snd_device_disconnect(struct snd_card *card, void *device_data)
if (snd_BUG_ON(!card || !device_data)) return -ENXIO; - list_for_each_entry(dev, &card->devices, list) { - if (dev->device_data != device_data) - continue; + dev = look_for_dev(card, device_data); + if (dev) { if (dev->state == SNDRV_DEV_REGISTERED && dev->ops->dev_disconnect) { if (dev->ops->dev_disconnect(dev)) @@ -162,9 +179,8 @@ int snd_device_register(struct snd_card *card, void *device_data)
if (snd_BUG_ON(!card || !device_data)) return -ENXIO; - list_for_each_entry(dev, &card->devices, list) { - if (dev->device_data != device_data) - continue; + dev = look_for_dev(card, device_data); + if (dev) { if (dev->state == SNDRV_DEV_BUILD && dev->ops->dev_register) { if ((err = dev->ops->dev_register(dev)) < 0) return err; @@ -177,7 +193,6 @@ int snd_device_register(struct snd_card *card, void *device_data) snd_BUG(); return -ENXIO; } - EXPORT_SYMBOL(snd_device_register);
/* @@ -212,7 +227,7 @@ int snd_device_disconnect_all(struct snd_card *card)
if (snd_BUG_ON(!card)) return -ENXIO; - list_for_each_entry(dev, &card->devices, list) { + list_for_each_entry_reverse(dev, &card->devices, list) { if (snd_device_disconnect(card, dev->device_data) < 0) err = -ENXIO; } @@ -223,24 +238,17 @@ int snd_device_disconnect_all(struct snd_card *card) * release all the devices on the card. * called from init.c */ -int snd_device_free_all(struct snd_card *card, enum snd_device_cmd cmd) +int snd_device_free_all(struct snd_card *card) { - struct snd_device *dev; - int err; - unsigned int range_low, range_high, type; + struct snd_device *dev, *next; + int ret = 0;
if (snd_BUG_ON(!card)) return -ENXIO; - range_low = (unsigned int)cmd * SNDRV_DEV_TYPE_RANGE_SIZE; - range_high = range_low + SNDRV_DEV_TYPE_RANGE_SIZE - 1; - __again: - list_for_each_entry(dev, &card->devices, list) { - type = (unsigned int)dev->type; - if (type >= range_low && type <= range_high) { - if ((err = snd_device_free(card, dev->device_data)) < 0) - return err; - goto __again; - } + list_for_each_entry_safe_reverse(dev, next, &card->devices, list) { + int err = snd_device_free(card, dev->device_data); + if (err < 0) + ret = err; } - return 0; + return ret; } diff --git a/sound/core/init.c b/sound/core/init.c index b7085eb19079..9e7f17b72fb6 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -266,7 +266,7 @@ int snd_card_new(struct device *parent, int idx, const char *xid, return 0;
__error_ctl: - snd_device_free_all(card, SNDRV_DEV_CMD_PRE); + snd_device_free_all(card); __error: put_device(&card->card_dev); return err; @@ -454,16 +454,8 @@ static int snd_card_do_free(struct snd_card *card) if (snd_mixer_oss_notify_callback) snd_mixer_oss_notify_callback(card, SND_MIXER_OSS_NOTIFY_FREE); #endif - if (snd_device_free_all(card, SNDRV_DEV_CMD_PRE) < 0) { - dev_err(card->dev, "unable to free all devices (pre)\n"); - /* Fatal, but this situation should never occur */ - } - if (snd_device_free_all(card, SNDRV_DEV_CMD_NORMAL) < 0) { - dev_err(card->dev, "unable to free all devices (normal)\n"); - /* Fatal, but this situation should never occur */ - } - if (snd_device_free_all(card, SNDRV_DEV_CMD_POST) < 0) { - dev_err(card->dev, "unable to free all devices (post)\n"); + if (snd_device_free_all(card) < 0) { + dev_err(card->dev, "unable to free all devices\n"); /* Fatal, but this situation should never occur */ } if (card->private_free)
A few code cleanups and optimizations. In addition, drop snd_device_disconnect() that isn't used at all, and drop the return values from snd_device_free*().
Another slight difference by this change is that now the device state will become always SNDRV_DEV_REGISTERED no matter whether dev_register ops is present or not. It's for better consistency. There should be no impact for the current tree, as the state isn't checked.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/core.h | 5 +- sound/core/device.c | 128 ++++++++++++++++++++------------------------------- sound/core/init.c | 5 +- 3 files changed, 52 insertions(+), 86 deletions(-)
diff --git a/include/sound/core.h b/include/sound/core.h index 5927378dc692..8508c20ee420 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -310,10 +310,9 @@ int snd_device_new(struct snd_card *card, enum snd_device_type type, void *device_data, struct snd_device_ops *ops); int snd_device_register(struct snd_card *card, void *device_data); int snd_device_register_all(struct snd_card *card); -int snd_device_disconnect(struct snd_card *card, void *device_data); int snd_device_disconnect_all(struct snd_card *card); -int snd_device_free(struct snd_card *card, void *device_data); -int snd_device_free_all(struct snd_card *card); +void snd_device_free(struct snd_card *card, void *device_data); +void snd_device_free_all(struct snd_card *card);
/* isadma.c */
diff --git a/sound/core/device.c b/sound/core/device.c index 53291ff21a09..41bec3075ae5 100644 --- a/sound/core/device.c +++ b/sound/core/device.c @@ -73,6 +73,30 @@ int snd_device_new(struct snd_card *card, enum snd_device_type type, } EXPORT_SYMBOL(snd_device_new);
+static int __snd_device_disconnect(struct snd_device *dev) +{ + if (dev->state == SNDRV_DEV_REGISTERED) { + if (dev->ops->dev_disconnect && + dev->ops->dev_disconnect(dev)) + dev_err(dev->card->dev, "device disconnect failure\n"); + dev->state = SNDRV_DEV_DISCONNECTED; + } + return 0; +} + +static void __snd_device_free(struct snd_device *dev) +{ + /* unlink */ + list_del(&dev->list); + + __snd_device_disconnect(dev); + if (dev->ops->dev_free) { + if (dev->ops->dev_free(dev)) + dev_err(dev->card->dev, "device free failure\n"); + } + kfree(dev); +} + static struct snd_device *look_for_dev(struct snd_card *card, void *device_data) { struct snd_device *dev; @@ -92,71 +116,33 @@ static struct snd_device *look_for_dev(struct snd_card *card, void *device_data) * Removes the device from the list on the card and invokes the * callbacks, dev_disconnect and dev_free, corresponding to the state. * Then release the device. - * - * Return: Zero if successful, or a negative error code on failure or if the - * device not found. */ -int snd_device_free(struct snd_card *card, void *device_data) +void snd_device_free(struct snd_card *card, void *device_data) { struct snd_device *dev; if (snd_BUG_ON(!card || !device_data)) - return -ENXIO; + return; dev = look_for_dev(card, device_data); - if (dev) { - /* unlink */ - list_del(&dev->list); - if (dev->state == SNDRV_DEV_REGISTERED && - dev->ops->dev_disconnect) - if (dev->ops->dev_disconnect(dev)) - dev_err(card->dev, - "device disconnect failure\n"); - if (dev->ops->dev_free) { - if (dev->ops->dev_free(dev)) - dev_err(card->dev, "device free failure\n"); - } - kfree(dev); - return 0; - } - dev_dbg(card->dev, "device free %p (from %pF), not found\n", - device_data, __builtin_return_address(0)); - return -ENXIO; + if (dev) + __snd_device_free(dev); + else + dev_dbg(card->dev, "device free %p (from %pF), not found\n", + device_data, __builtin_return_address(0)); } EXPORT_SYMBOL(snd_device_free);
-/** - * snd_device_disconnect - disconnect the device - * @card: the card instance - * @device_data: the data pointer to disconnect - * - * Turns the device into the disconnection state, invoking - * dev_disconnect callback, if the device was already registered. - * - * Usually called from snd_card_disconnect(). - * - * Return: Zero if successful, or a negative error code on failure or if the - * device not found. - */ -int snd_device_disconnect(struct snd_card *card, void *device_data) +static int __snd_device_register(struct snd_device *dev) { - struct snd_device *dev; - - if (snd_BUG_ON(!card || !device_data)) - return -ENXIO; - dev = look_for_dev(card, device_data); - if (dev) { - if (dev->state == SNDRV_DEV_REGISTERED && - dev->ops->dev_disconnect) { - if (dev->ops->dev_disconnect(dev)) - dev_err(card->dev, - "device disconnect failure\n"); - dev->state = SNDRV_DEV_DISCONNECTED; + if (dev->state == SNDRV_DEV_BUILD) { + if (dev->ops->dev_register) { + int err = dev->ops->dev_register(dev); + if (err < 0) + return err; } - return 0; + dev->state = SNDRV_DEV_REGISTERED; } - dev_dbg(card->dev, "device disconnect %p (from %pF), not found\n", - device_data, __builtin_return_address(0)); - return -ENXIO; + return 0; }
/** @@ -175,21 +161,12 @@ int snd_device_disconnect(struct snd_card *card, void *device_data) int snd_device_register(struct snd_card *card, void *device_data) { struct snd_device *dev; - int err;
if (snd_BUG_ON(!card || !device_data)) return -ENXIO; dev = look_for_dev(card, device_data); - if (dev) { - if (dev->state == SNDRV_DEV_BUILD && dev->ops->dev_register) { - if ((err = dev->ops->dev_register(dev)) < 0) - return err; - dev->state = SNDRV_DEV_REGISTERED; - return 0; - } - dev_dbg(card->dev, "snd_device_register busy\n"); - return -EBUSY; - } + if (dev) + return __snd_device_register(dev); snd_BUG(); return -ENXIO; } @@ -207,11 +184,9 @@ int snd_device_register_all(struct snd_card *card) if (snd_BUG_ON(!card)) return -ENXIO; list_for_each_entry(dev, &card->devices, list) { - if (dev->state == SNDRV_DEV_BUILD && dev->ops->dev_register) { - if ((err = dev->ops->dev_register(dev)) < 0) - return err; - dev->state = SNDRV_DEV_REGISTERED; - } + err = __snd_device_register(dev); + if (err < 0) + return err; } return 0; } @@ -228,7 +203,7 @@ int snd_device_disconnect_all(struct snd_card *card) if (snd_BUG_ON(!card)) return -ENXIO; list_for_each_entry_reverse(dev, &card->devices, list) { - if (snd_device_disconnect(card, dev->device_data) < 0) + if (__snd_device_disconnect(dev) < 0) err = -ENXIO; } return err; @@ -238,17 +213,12 @@ int snd_device_disconnect_all(struct snd_card *card) * release all the devices on the card. * called from init.c */ -int snd_device_free_all(struct snd_card *card) +void snd_device_free_all(struct snd_card *card) { struct snd_device *dev, *next; - int ret = 0;
if (snd_BUG_ON(!card)) - return -ENXIO; - list_for_each_entry_safe_reverse(dev, next, &card->devices, list) { - int err = snd_device_free(card, dev->device_data); - if (err < 0) - ret = err; - } - return ret; + return; + list_for_each_entry_safe_reverse(dev, next, &card->devices, list) + __snd_device_free(dev); } diff --git a/sound/core/init.c b/sound/core/init.c index 9e7f17b72fb6..5ee83845c5de 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -454,10 +454,7 @@ static int snd_card_do_free(struct snd_card *card) if (snd_mixer_oss_notify_callback) snd_mixer_oss_notify_callback(card, SND_MIXER_OSS_NOTIFY_FREE); #endif - if (snd_device_free_all(card) < 0) { - dev_err(card->dev, "unable to free all devices\n"); - /* Fatal, but this situation should never occur */ - } + snd_device_free_all(card); if (card->private_free) card->private_free(card); snd_info_free_entry(card->proc_id);
participants (1)
-
Takashi Iwai