[alsa-devel] [PATCH 0/4] ALSA: usb: misc consistency fixes
Hi,
when working on the hiFace driver (which I plan on re-sending later today BTW) I've been reading the code of other drivers and I've stumbled on a couple of minor inconsistencies which were also spotted during the review rounds for the hiFace driver.
I am sending some patches to address these issues, so that new drivers authors won't do the same mistakes I did when copying code over.
Please take a closer look at the ones about vmalloc buffers, do we need an actual Tested-by for these?
Patches are against 3.10-rc6.
Thanks, Antonio
Antonio Ospite (4): ALSA: snd-usb-caiaq: remove the unused snd_card_used variable ALSA: snd-usb-caiaq: use vmalloc buffers ALSA: snd-usb-6fire: use vmalloc buffers ALSA: usb: uniform style used in MODULE_SUPPORTED_DEVICE()
sound/usb/6fire/chip.c | 2 +- sound/usb/6fire/pcm.c | 12 +++++------- sound/usb/caiaq/audio.c | 12 +++++------- sound/usb/caiaq/device.c | 31 +++++++++++++++---------------- sound/usb/usx2y/usbusx2y.c | 2 +- 5 files changed, 27 insertions(+), 32 deletions(-)
The snd_card_used variable is only read but never written, remove it.
Signed-off-by: Antonio Ospite ao2@amarulasolutions.com --- sound/usb/caiaq/device.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/usb/caiaq/device.c b/sound/usb/caiaq/device.c index 48b63cc..23cf55d 100644 --- a/sound/usb/caiaq/device.c +++ b/sound/usb/caiaq/device.c @@ -57,7 +57,6 @@ MODULE_SUPPORTED_DEVICE("{{Native Instruments, RigKontrol2}," static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-max */ static char* id[SNDRV_CARDS] = SNDRV_DEFAULT_STR; /* Id for this card */ static bool enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_PNP; /* Enable this card */ -static int snd_card_used[SNDRV_CARDS];
module_param_array(index, int, NULL, 0444); MODULE_PARM_DESC(index, "Index value for the caiaq sound device"); @@ -388,7 +387,7 @@ static int create_card(struct usb_device *usb_dev, struct snd_usb_caiaqdev *cdev;
for (devnum = 0; devnum < SNDRV_CARDS; devnum++) - if (enable[devnum] && !snd_card_used[devnum]) + if (enable[devnum]) break;
if (devnum >= SNDRV_CARDS)
For USB devices it's not necessary to allocate physically contiguous buffers.
Signed-off-by: Antonio Ospite ao2@amarulasolutions.com --- sound/usb/caiaq/audio.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c index c191618..7103b09 100644 --- a/sound/usb/caiaq/audio.c +++ b/sound/usb/caiaq/audio.c @@ -183,14 +183,15 @@ static int snd_usb_caiaq_substream_close(struct snd_pcm_substream *substream) static int snd_usb_caiaq_pcm_hw_params(struct snd_pcm_substream *sub, struct snd_pcm_hw_params *hw_params) { - return snd_pcm_lib_malloc_pages(sub, params_buffer_bytes(hw_params)); + return snd_pcm_lib_alloc_vmalloc_buffer(sub, + params_buffer_bytes(hw_params)); }
static int snd_usb_caiaq_pcm_hw_free(struct snd_pcm_substream *sub) { struct snd_usb_caiaqdev *cdev = snd_pcm_substream_chip(sub); deactivate_substream(cdev, sub); - return snd_pcm_lib_free_pages(sub); + return snd_pcm_lib_free_vmalloc_buffer(sub); }
/* this should probably go upstream */ @@ -345,7 +346,9 @@ static struct snd_pcm_ops snd_usb_caiaq_ops = { .hw_free = snd_usb_caiaq_pcm_hw_free, .prepare = snd_usb_caiaq_pcm_prepare, .trigger = snd_usb_caiaq_pcm_trigger, - .pointer = snd_usb_caiaq_pcm_pointer + .pointer = snd_usb_caiaq_pcm_pointer, + .page = snd_pcm_lib_get_vmalloc_page, + .mmap = snd_pcm_lib_mmap_vmalloc, };
static void check_for_elapsed_periods(struct snd_usb_caiaqdev *cdev, @@ -852,11 +855,6 @@ int snd_usb_caiaq_audio_init(struct snd_usb_caiaqdev *cdev) snd_pcm_set_ops(cdev->pcm, SNDRV_PCM_STREAM_CAPTURE, &snd_usb_caiaq_ops);
- snd_pcm_lib_preallocate_pages_for_all(cdev->pcm, - SNDRV_DMA_TYPE_CONTINUOUS, - snd_dma_continuous_data(GFP_KERNEL), - MAX_BUFFER_SIZE, MAX_BUFFER_SIZE); - cdev->data_cb_info = kmalloc(sizeof(struct snd_usb_caiaq_cb_info) * N_URBS, GFP_KERNEL);
For USB devices it's not necessary to allocate physically contiguous buffers.
Signed-off-by: Antonio Ospite ao2@amarulasolutions.com --- sound/usb/6fire/pcm.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/sound/usb/6fire/pcm.c b/sound/usb/6fire/pcm.c index 40dd50a..c5b9cac 100644 --- a/sound/usb/6fire/pcm.c +++ b/sound/usb/6fire/pcm.c @@ -450,13 +450,13 @@ static int usb6fire_pcm_close(struct snd_pcm_substream *alsa_sub) static int usb6fire_pcm_hw_params(struct snd_pcm_substream *alsa_sub, struct snd_pcm_hw_params *hw_params) { - return snd_pcm_lib_malloc_pages(alsa_sub, - params_buffer_bytes(hw_params)); + return snd_pcm_lib_alloc_vmalloc_buffer(alsa_sub, + params_buffer_bytes(hw_params)); }
static int usb6fire_pcm_hw_free(struct snd_pcm_substream *alsa_sub) { - return snd_pcm_lib_free_pages(alsa_sub); + return snd_pcm_lib_free_vmalloc_buffer(alsa_sub); }
static int usb6fire_pcm_prepare(struct snd_pcm_substream *alsa_sub) @@ -560,6 +560,8 @@ static struct snd_pcm_ops pcm_ops = { .prepare = usb6fire_pcm_prepare, .trigger = usb6fire_pcm_trigger, .pointer = usb6fire_pcm_pointer, + .page = snd_pcm_lib_get_vmalloc_page, + .mmap = snd_pcm_lib_mmap_vmalloc, };
static void usb6fire_pcm_init_urb(struct pcm_urb *urb, @@ -622,10 +624,6 @@ int usb6fire_pcm_init(struct sfire_chip *chip) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &pcm_ops); snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &pcm_ops);
- ret = snd_pcm_lib_preallocate_pages_for_all(pcm, - SNDRV_DMA_TYPE_CONTINUOUS, - snd_dma_continuous_data(GFP_KERNEL), - MAX_BUFSIZE, MAX_BUFSIZE); if (ret) { kfree(rt); snd_printk(KERN_ERR PREFIX
In sound/usb/card.c and sound/usb/misc/ua101.c there are no spaces between the vendor and the device names, use this style in the other drivers too.
This also helps keeping consistency when new drivers copies from the ones already in the mainline tree.
Signed-off-by: Antonio Ospite ao2@amarulasolutions.com --- sound/usb/6fire/chip.c | 2 +- sound/usb/caiaq/device.c | 28 ++++++++++++++-------------- sound/usb/usx2y/usbusx2y.c | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/sound/usb/6fire/chip.c b/sound/usb/6fire/chip.c index 4394ae7..c39c779 100644 --- a/sound/usb/6fire/chip.c +++ b/sound/usb/6fire/chip.c @@ -30,7 +30,7 @@ MODULE_AUTHOR("Torsten Schenk torsten.schenk@zoho.com"); MODULE_DESCRIPTION("TerraTec DMX 6Fire USB audio driver"); MODULE_LICENSE("GPL v2"); -MODULE_SUPPORTED_DEVICE("{{TerraTec, DMX 6Fire USB}}"); +MODULE_SUPPORTED_DEVICE("{{TerraTec,DMX 6Fire USB}}");
static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-max */ static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR; /* Id for card */ diff --git a/sound/usb/caiaq/device.c b/sound/usb/caiaq/device.c index 23cf55d..1a61dd1 100644 --- a/sound/usb/caiaq/device.c +++ b/sound/usb/caiaq/device.c @@ -39,20 +39,20 @@ MODULE_AUTHOR("Daniel Mack daniel@caiaq.de"); MODULE_DESCRIPTION("caiaq USB audio"); MODULE_LICENSE("GPL"); -MODULE_SUPPORTED_DEVICE("{{Native Instruments, RigKontrol2}," - "{Native Instruments, RigKontrol3}," - "{Native Instruments, Kore Controller}," - "{Native Instruments, Kore Controller 2}," - "{Native Instruments, Audio Kontrol 1}," - "{Native Instruments, Audio 2 DJ}," - "{Native Instruments, Audio 4 DJ}," - "{Native Instruments, Audio 8 DJ}," - "{Native Instruments, Traktor Audio 2}," - "{Native Instruments, Session I/O}," - "{Native Instruments, GuitarRig mobile}," - "{Native Instruments, Traktor Kontrol X1}," - "{Native Instruments, Traktor Kontrol S4}," - "{Native Instruments, Maschine Controller}}"); +MODULE_SUPPORTED_DEVICE("{{Native Instruments,RigKontrol2}," + "{Native Instruments,RigKontrol3}," + "{Native Instruments,Kore Controller}," + "{Native Instruments,Kore Controller 2}," + "{Native Instruments,Audio Kontrol 1}," + "{Native Instruments,Audio 2 DJ}," + "{Native Instruments,Audio 4 DJ}," + "{Native Instruments,Audio 8 DJ}," + "{Native Instruments,Traktor Audio 2}," + "{Native Instruments,Session I/O}," + "{Native Instruments,GuitarRig mobile}," + "{Native Instruments,Traktor Kontrol X1}," + "{Native Instruments,Traktor Kontrol S4}," + "{Native Instruments,Maschine Controller}}");
static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-max */ static char* id[SNDRV_CARDS] = SNDRV_DEFAULT_STR; /* Id for this card */ diff --git a/sound/usb/usx2y/usbusx2y.c b/sound/usb/usx2y/usbusx2y.c index 9af7c1f..1f9bbd5 100644 --- a/sound/usb/usx2y/usbusx2y.c +++ b/sound/usb/usx2y/usbusx2y.c @@ -150,7 +150,7 @@ MODULE_AUTHOR("Karsten Wiese annabellesgarden@yahoo.de"); MODULE_DESCRIPTION("TASCAM "NAME_ALLCAPS" Version 0.8.7.2"); MODULE_LICENSE("GPL"); -MODULE_SUPPORTED_DEVICE("{{TASCAM(0x1604), "NAME_ALLCAPS"(0x8001)(0x8005)(0x8007) }}"); +MODULE_SUPPORTED_DEVICE("{{TASCAM(0x1604),"NAME_ALLCAPS"(0x8001)(0x8005)(0x8007)}}");
static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-max */ static char* id[SNDRV_CARDS] = SNDRV_DEFAULT_STR; /* Id for this card */
At Fri, 21 Jun 2013 13:11:47 +0200, Antonio Ospite wrote:
Hi,
when working on the hiFace driver (which I plan on re-sending later today BTW) I've been reading the code of other drivers and I've stumbled on a couple of minor inconsistencies which were also spotted during the review rounds for the hiFace driver.
I am sending some patches to address these issues, so that new drivers authors won't do the same mistakes I did when copying code over.
Please take a closer look at the ones about vmalloc buffers, do we need an actual Tested-by for these?
Well, these drivers do just simple memcpy() from the buffer, so this must work with vmalloced buffer like the generic usb-audio driver.
So I took all patches now. Let me know if this causes unexpected results.
thanks,
Takashi
Patches are against 3.10-rc6.
Thanks, Antonio
Antonio Ospite (4): ALSA: snd-usb-caiaq: remove the unused snd_card_used variable ALSA: snd-usb-caiaq: use vmalloc buffers ALSA: snd-usb-6fire: use vmalloc buffers ALSA: usb: uniform style used in MODULE_SUPPORTED_DEVICE()
sound/usb/6fire/chip.c | 2 +- sound/usb/6fire/pcm.c | 12 +++++------- sound/usb/caiaq/audio.c | 12 +++++------- sound/usb/caiaq/device.c | 31 +++++++++++++++---------------- sound/usb/usx2y/usbusx2y.c | 2 +- 5 files changed, 27 insertions(+), 32 deletions(-)
-- Antonio Ospite http://ao2.it
A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing?
participants (2)
-
Antonio Ospite -
Takashi Iwai