- Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH] ALSA: pcm_lib: avoid timing jitter in snd_pcm_read/write()
by David Dillow 05 Jul '10

05 Jul '10

When using poll() to wait for the next period -- or avail_min samples -- one gets a consistent delay for each system call that is usually just a little short of the selected period time. However, When using snd_pcm_read/write(), one gets a jittery delay that alternates between less than a millisecond and approximately two period times. This is caused by snd_pcm_lib_{read,write}1() transferring any available samples to the user's buffer and adjusting the application pointer prior to sleeping to the end of the current period. When the next period interrupt occurs, there is then less than avail_min samples remaining to be transferred in the period, so we end up sleeping until a second period occurs. This is solved by using runtime->twake as the number of samples needed for a wakeup in addition to selecting the proper wait queue to wake in snd_pcm_update_state(). This requires twake to be non-zero when used by snd_pcm_lib_{read,write}1() even if avail_min is zero. Signed-off-by: Dave Dillow <dave(a)thedillows.org> --- Hans, you may want this one as well for the SIS7019, as it gives more margin to deal with captured data on alternating calls into the stack. diff --git a/include/sound/pcm.h b/include/sound/pcm.h index dd76cde..83c6fa6 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -313,7 +313,7 @@ struct snd_pcm_runtime { struct snd_pcm_mmap_control *control; /* -- locking / scheduling -- */ - unsigned int twake: 1; /* do transfer (!poll) wakeup */ + snd_pcm_uframes_t twake; /* do transfer (!poll) wakeup if non-zero */ wait_queue_head_t sleep; /* poll sleep */ wait_queue_head_t tsleep; /* transfer sleep */ struct fasync_struct *fasync; diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index e9d98be..bcf95d3 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -287,8 +287,11 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream, return -EPIPE; } } - if (avail >= runtime->control->avail_min) - wake_up(runtime->twake ? &runtime->tsleep : &runtime->sleep); + if (runtime->twake) { + if (avail >= runtime->twake) + wake_up(&runtime->tsleep); + } else if (avail >= runtime->control->avail_min) + wake_up(&runtime->sleep); return 0; } @@ -1707,7 +1710,7 @@ EXPORT_SYMBOL(snd_pcm_period_elapsed); * The available space is stored on availp. When err = 0 and avail = 0 * on the capture stream, it indicates the stream is in DRAINING state. */ -static int wait_for_avail_min(struct snd_pcm_substream *substream, +static int wait_for_avail(struct snd_pcm_substream *substream, snd_pcm_uframes_t *availp) { struct snd_pcm_runtime *runtime = substream->runtime; @@ -1757,7 +1760,7 @@ static int wait_for_avail_min(struct snd_pcm_substream *substream, avail = snd_pcm_playback_avail(runtime); else avail = snd_pcm_capture_avail(runtime); - if (avail >= runtime->control->avail_min) + if (avail >= runtime->twake) break; } _endloop: @@ -1820,7 +1823,7 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, goto _end_unlock; } - runtime->twake = 1; + runtime->twake = runtime->control->avail_min ? : 1; while (size > 0) { snd_pcm_uframes_t frames, appl_ptr, appl_ofs; snd_pcm_uframes_t avail; @@ -1833,7 +1836,9 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, err = -EAGAIN; goto _end_unlock; } - err = wait_for_avail_min(substream, &avail); + runtime->twake = min_t(snd_pcm_uframes_t, size, + runtime->control->avail_min ? : 1); + err = wait_for_avail(substream, &avail); if (err < 0) goto _end_unlock; } @@ -2042,7 +2047,7 @@ static snd_pcm_sframes_t snd_pcm_lib_read1(struct snd_pcm_substream *substream, goto _end_unlock; } - runtime->twake = 1; + runtime->twake = runtime->control->avail_min ? : 1; while (size > 0) { snd_pcm_uframes_t frames, appl_ptr, appl_ofs; snd_pcm_uframes_t avail; @@ -2060,7 +2065,9 @@ static snd_pcm_sframes_t snd_pcm_lib_read1(struct snd_pcm_substream *substream, err = -EAGAIN; goto _end_unlock; } - err = wait_for_avail_min(substream, &avail); + runtime->twake = min_t(snd_pcm_uframes_t, size, + runtime->control->avail_min ? : 1); + err = wait_for_avail(substream, &avail); if (err < 0) goto _end_unlock; if (!avail)

5 12

[alsa-devel] [PATCH] ASoC: DaVinci: More accurate calculation for clock divider for McBSP (I2S)
by Raffaele Recalcati 05 Jul '10

05 Jul '10

From: Raffaele Recalcati <raffaele.recalcati(a)bticino.it> i2s_accurate_sck switch can be used to have a better approximate sampling frequency. The trade off is between more accurate clock (fast clock) and less accurate clock (slow clock). The waveform will be not symmetric. Probably it is possible to get a better algorithm for calculating the divider, trying to keep a slower clock as possible. This patch has been developed against the http://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-davinci.git git tree and has been tested on bmx board (similar to dm365 evm, but using uda1345 as external audio codec). Signed-off-by: Raffaele Recalcati <raffaele.recalcati(a)bticino.it> Signed-off-by: Davide Bonfanti <davide.bonfanti(a)bticino.it> --- arch/arm/mach-davinci/include/mach/asp.h | 33 ++++++++++++++++++++++++++++++ sound/soc/davinci/davinci-i2s.c | 28 ++++++++++++++++++++----- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/arch/arm/mach-davinci/include/mach/asp.h b/arch/arm/mach-davinci/include/mach/asp.h index 0847d21..5149abe 100644 --- a/arch/arm/mach-davinci/include/mach/asp.h +++ b/arch/arm/mach-davinci/include/mach/asp.h @@ -73,6 +73,39 @@ struct snd_platform_data { */ int clk_input_pin; + /* + * This flag works when both clock and FS are outputs for the cpu + * and makes clock more accurate (FS is not symmetrical and the + * clock is very fast. + * The clock becoming faster is named + * i2s continuous serial clocl (I2S_SCK) and it is an externally + * visible bit clock. + * + * first line : WordSelect + * second line : ContinuousSerialClock + * third line: SerialData + * + * SYMMETRICAL APPROACH: + * _______________________ LEFT + * _| RIGHT |______________________| + * _ _ _ _ _ _ _ _ + * _| |_| |_ x16 _| |_| |_| |_| |_ x16 _| |_| |_ + * _ _ _ _ _ _ _ _ + * _/ \_/ \_ ... _/ \_/ \_/ \_/ \_ ... _/ \_/ \_ + * \_/ \_/ \_/ \_/ \_/ \_/ \_/ \_/ + * + * ACCURATE CLOCK APPROACH: + * ______________ LEFT + * _| RIGHT |_______________________________| + * _ _ _ _ _ _ _ _ _ + * _| |_ x16 _| |_| |_ x16 _| |_| |_| |_| |_| |_| | + * _ _ _ _ dummy cycles + * _/ \_ ... _/ \_/ \_ ... _/ \__________________ + * \_/ \_/ \_/ \_/ + * + */ + bool i2s_accurate_sck; + /* McASP specific fields */ int tdm_slots; u8 op_mode; diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c index 17f594f..e6dcd81 100644 --- a/sound/soc/davinci/davinci-i2s.c +++ b/sound/soc/davinci/davinci-i2s.c @@ -122,6 +122,7 @@ static const unsigned char double_fmt[SNDRV_PCM_FORMAT_S32_LE + 1] = { }; struct davinci_mcbsp_dev { + struct device *dev; struct davinci_pcm_dma_params dma_params[2]; void __iomem *base; #define MOD_DSP_A 0 @@ -154,6 +155,7 @@ struct davinci_mcbsp_dev { unsigned int fmt; int clk_div; int clk_input_pin; + bool i2s_accurate_sck; }; static inline void davinci_mcbsp_write_reg(struct davinci_mcbsp_dev *dev, @@ -296,7 +298,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, pcr |= DAVINCI_MCBSP_PCR_SCLKME; break; default: - dev_err(&pdev->dev, "bad clk_input_pin\n"); + dev_err(dev->dev, "bad clk_input_pin\n"); return -EINVAL; } @@ -447,11 +449,23 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream, DAVINCI_MCBSP_SRGR_CLKSM; srgr |= DAVINCI_MCBSP_SRGR_FWID(mcbsp_word_length * 8 - 1); - /* symmetric waveforms */ - clk_div = freq / (mcbsp_word_length * 16) / - params->rate_num * params->rate_den; - srgr |= DAVINCI_MCBSP_SRGR_FPER(mcbsp_word_length * - 16 - 1); + if (dev->i2s_accurate_sck) { + clk_div = 256; + do { + framesize = (freq / (--clk_div)) / + params->rate_num * + params->rate_den; + } while (((framesize < 33) || (framesize > 4095)) && + (clk_div)); + clk_div--; + srgr |= DAVINCI_MCBSP_SRGR_FPER(framesize - 1); + } else { + /* symmetric waveforms */ + clk_div = freq / (mcbsp_word_length * 16) / + params->rate_num * params->rate_den; + srgr |= DAVINCI_MCBSP_SRGR_FPER(mcbsp_word_length * + 16 - 1); + } clk_div &= 0xFF; srgr |= clk_div; break; @@ -662,6 +676,7 @@ static int davinci_i2s_probe(struct platform_device *pdev) dev->dma_params[SNDRV_PCM_STREAM_CAPTURE].sram_size = pdata->sram_size_capture; dev->clk_input_pin = pdata->clk_input_pin; + dev->i2s_accurate_sck = pdata->i2s_accurate_sck; } dev->clk = clk_get(&pdev->dev, NULL); if (IS_ERR(dev->clk)) { @@ -694,6 +709,7 @@ static int davinci_i2s_probe(struct platform_device *pdev) goto err_free_mem; } dev->dma_params[SNDRV_PCM_STREAM_CAPTURE].channel = res->start; + dev->dev = &pdev->dev; davinci_i2s_dai.private_data = dev; davinci_i2s_dai.capture.dma_data = dev->dma_params; -- 1.7.0.4

1 0

Re: [alsa-devel] ALSA: hda - Don't check capture source mixer if no ADC is available
by Dan Carpenter 05 Jul '10

05 Jul '10

Commit fbe618f2168 "ALSA: hda - Don't check capture source mixer if no ADC is available" added support for when there is no ADC available and spec->adc_nids is NULL. If spec->adc_nids is NULL, can it cause a problem later when we call snd_hda_add_nid()? sound/pci/hda/patch_realtek.c +2643 alc_build_controls(96) error: we previously assumed 'spec->adc_nids' could be null. 2635 if (spec->cap_mixer) { 2636 const char *kname = kctl ? kctl->id.name : NULL; 2637 for (knew = spec->cap_mixer; knew->name; knew++) { 2638 if (kname && strcmp(knew->name, kname) == 0) 2639 continue; 2640 kctl = snd_hda_find_mixer_ctl(codec, knew->name); 2641 for (i = 0; kctl && i < kctl->count; i++) { 2642 err = snd_hda_add_nid(codec, kctl, i, 2643 spec->adc_nids[i]); ^^^^^^^^^^^^^^^^^ 2644 if (err < 0) 2645 return err; 2646 } 2647 } 2648 } regards, dan carpenter

2 1

[alsa-devel] linux vs c99 fixed width types
by Eliot Blennerhassett 05 Jul '10

05 Jul '10

Hi Takashi, & list it is quite likely that we will be moving our own codebase to use the C99 fixed width types rather than our own e.g. uint32_t vs HW32 etc. Currently HW32 is translated into u32 before I submit patches to you. Should I do the same for the C99 types, or let them pass through unaltered? regards -- Eliot Blennerhassett AudioScience Inc.

3 2

[alsa-devel] Update asihpi to hpi version 4.03.36
by linux＠audioscience.com 05 Jul '10

05 Jul '10

2 7

[alsa-devel] ASoC updates for 2.6.36
by Mark Brown 05 Jul '10

05 Jul '10

The following changes since commit e827e32efc313e1a061c81007e96fff439ccb802: Merge branch 'for-2.6.36' of git://git.kernel.org/pub/scm/linux/kernel/git/lrg/asoc-2.6 into topic/asoc (2010-06-24 11:11:41 +0200) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git for-2.6.36 Eric Bénard (1): ASoC: clean i.MX Kconfig Kuninori Morimoto (1): ASoC: ak4642: Add Digital Playback Volume control Mark Brown (1): Merge remote branch 'takashi/topic/asoc' into for-2.6.36 Vladimir Zapolskiy (4): ASoC: uda134x: replace a macro with a value in platform struct. ASoC: uda134x: fix bias level setup on initialization ASoC: uda134x: add DATA011 register found in codecs family ASoC: uda134x: correct bias level setup for codecs family include/sound/uda134x.h | 12 ++++++++ sound/soc/codecs/ak4642.c | 24 ++++++++++++++-- sound/soc/codecs/uda134x.c | 64 +++++++++++++++++++++++++++++-------------- sound/soc/codecs/uda134x.h | 5 ++- sound/soc/imx/Kconfig | 14 ++++----- 5 files changed, 85 insertions(+), 34 deletions(-)

2 2

[alsa-devel] PC Beeps not working with HDA Intel module
by Shérab 05 Jul '10

05 Jul '10

Dear all, The problem is on a computer with an HDA Intel sound card, Intel G45 DEVCTG chip and appears will all version s of the kernel. Running e.g. beep 440 produces a beep whose audible frequency is very low, definitely not 440 Hz. However, it is worth notiing that grub is able to produce a beep of the right frequency at boot time. Can this problem be fixed ? Should you need any additional bit of information or testing, please ask. Many thanks in advance for your help, Shérab.

2 1

[alsa-devel] Full duplex code sample?
by robert 05 Jul '10

05 Jul '10

Dear All, Is there an example code on how to program a full duplex audio transmission? I did search the internet but it is full of Paul Davis advising to use Jack instead of ALSA. I think I would rather use ALSA for my purposes. Cheers, Robert

2 1

[alsa-devel] [RFC][PATCH] snd-usb: improving support for CM6206
by Adrian Pardini 05 Jul '10

05 Jul '10

Hi all, I'm working in my spare time to get better support for this chip. Right now it works but some features present in the datasheet are missing. This patch gives the option to select the source for the "headphone" jack. I have some concerns with the way I accomplished this and I'd like if someone could kick me in the right direction. In particular with the caching of some register values. Added another field to usb_mixer_interface, maybe it's not the best place. Also, according to the datasheet, in order to read back the register I must issue a request as done in snd_usb_cm106_write_int_reg() and the result will come in the HID input report. For this it doesn't matter but I'm planning on making some controls for spdif and I'll need it. Where should I tap to get that information?. To be done (I need help): * Mapping of streams: Front is ok. Rear_L jack is assigned to "Center", Rear_R jack gets "Speaker Woofer". "Center/Bass" jack is managed with the mixer "Speaker". Traced the circuit and the connections are fine, how do I switch them from ALSA? (nb: not with a ttable, from the driver) To be done (I think I'll somehow get this done on my own): * Input levels are reported with more resolution than the real. Add another case to build_feature_ctl(). Not very important now. * Engaging of the "mute" controls: now I can't mute left or right channel without muting everything. Have to play more with mixer_maps. Kind regards and thanks a lot for your time. Signed off by: Adrian Pardini <adrian.pardini(a)solar.org.ar> --- diff --git a/alsa-kernel/usb/mixer.h b/alsa-kernel/usb/mixer.h index 26c636c..291ba4b 100644 --- a/alsa-kernel/usb/mixer.h +++ b/alsa-kernel/usb/mixer.h @@ -22,6 +22,7 @@ struct usb_mixer_interface { u8 audigy2nx_leds[3]; u8 xonar_u1_status; + u16 cm6206reg2; }; #define MAX_CHANNELS 10 /* max logical channels */ diff --git a/usb/mixer_quirks.c b/usb/mixer_quirks.c index 794cdbd..8adeef0 100644 --- a/usb/mixer_quirks.c +++ b/usb/mixer_quirks.c @@ -360,6 +360,129 @@ void snd_emuusb_set_samplerate(struct snd_usb_audio *chip, } } +/* + * C-Media CM106/CM106+ have four 16-bit internal registers that are nicely + * documented in the device's data sheet. + */ +int snd_usb_cm106_write_int_reg(struct usb_device *dev, int reg, u16 value) +{ + u8 buf[4]; + buf[0] = 0x20; + buf[1] = value & 0xff; + buf[2] = (value >> 8) & 0xff; + buf[3] = reg; + return snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), USB_REQ_SET_CONFIGURATION, + USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_ENDPOINT, + 0, 0, &buf, 4, 1000); +} + +static int snd_cm6206_headphone_source_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) +{ + ucontrol->value.integer.value[0] = kcontrol->private_value; + return 0; +} + +static int snd_cm6206_headphone_source_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) +{ + struct usb_mixer_interface *mixer = snd_kcontrol_chip(kcontrol); + struct usb_device *dev = mixer->chip->dev; + + u16 reg2 = mixer->cm6206reg2; + u16 idx = kcontrol->private_value = ucontrol->value.integer.value[0]; + + reg2 = (u16)( (((1<<7) | (idx<<5))<<8) | (reg2 & 0x1800) ); + + snd_usb_cm106_write_int_reg(dev, 2, reg2); + mixer->cm6206reg2 = reg2; + return 1; +} + +static int snd_cm6206_headphone_mute_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) +{ + ucontrol->value.integer.value[0] = kcontrol->private_value & (1<<11); + ucontrol->value.integer.value[1] = kcontrol->private_value & (1<<12); + return 0; +} + +static int snd_cm6206_headphone_mute_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) +{ + struct usb_mixer_interface *mixer = snd_kcontrol_chip(kcontrol); + struct usb_device *dev = mixer->chip->dev; + + u16 reg2 = mixer->cm6206reg2; + u16 mute; + + mute = ucontrol->value.integer.value[1] ? (1<<12) : 0; + mute |= ucontrol->value.integer.value[0] ? (1<<11) : 0; + kcontrol->private_value = mute; + + reg2 = (u16)(mute | (reg2 & ~0x1800)); + + snd_usb_cm106_write_int_reg(dev, 2, reg2); + mixer->cm6206reg2 = reg2; + return 1; +} + +static int snd_cm6206_headphone_source_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char *const names[] = { + "Side", + "Surround", + "Center", + "Front" + }; + + uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; + uinfo->count = 1; + uinfo->value.enumerated.items = 4; + if (uinfo->value.enumerated.item > 3) + uinfo->value.enumerated.item = 3; + strcpy(uinfo->value.enumerated.name, names[uinfo->value.enumerated.item]); + return 0; +} + +#define snd_cm6206_headphone_mute_info snd_ctl_boolean_stereo_info + +static struct snd_kcontrol_new cm6206_controls[] = { + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .access = SNDRV_CTL_ELEM_ACCESS_READWRITE, + .name = "Headphone Source", + .info = snd_cm6206_headphone_source_info, + .get = snd_cm6206_headphone_source_get, + .put = snd_cm6206_headphone_source_put, + .private_value = 3 + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .access = SNDRV_CTL_ELEM_ACCESS_READWRITE, + .name = "Headphone", + .info = snd_cm6206_headphone_mute_info, + .get = snd_cm6206_headphone_mute_get, + .put = snd_cm6206_headphone_mute_put, + .private_value = 24 + } +}; + +static int snd_cm6206_controls_create(struct usb_mixer_interface *mixer) +{ + int i, err; + + for (i=0; i < ARRAY_SIZE(cm6206_controls); ++i) { + err = snd_ctl_add(mixer->chip->card, + snd_ctl_new1(&cm6206_controls[i], mixer)); + if (err < 0) + return err; + } + mixer->cm6206reg2 = 0xf800; /* Headphone source=front, no mute. */ + return 0; +} + int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer) { int err; @@ -385,6 +508,11 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer) return err; } + if (mixer->chip->usb_id == USB_ID(0x0d8c, 0x0102)) { + err = snd_cm6206_controls_create(mixer); + if (err < 0) + return err; + } return 0; } @@ -415,4 +543,3 @@ void snd_usb_mixer_rc_memory_change(struct usb_mixer_interface *mixer, break; } } - diff --git a/alsa-kernel/usb/mixer_quirks.h b/alsa-kernel/usb/mixer_quirks.h index bdbfab0..bc5d577 100644 --- a/alsa-kernel/usb/mixer_quirks.h +++ b/alsa-kernel/usb/mixer_quirks.h @@ -9,5 +9,7 @@ void snd_emuusb_set_samplerate(struct snd_usb_audio *chip, void snd_usb_mixer_rc_memory_change(struct usb_mixer_interface *mixer, int unitid); +int snd_usb_cm106_write_int_reg(struct usb_device *dev, int reg, u16 value); + #endif /* SND_USB_MIXER_QUIRKS_H */ diff --git a/usb/quirks.c b/usb/quirks.c index bf97ccd..ab57d58 100644 --- a/usb/quirks.c +++ b/usb/quirks.c @@ -376,22 +376,6 @@ static int snd_usb_audigy2nx_boot_quirk(struct usb_device *dev) return 0; } -/* - * C-Media CM106/CM106+ have four 16-bit internal registers that are nicely - * documented in the device's data sheet. - */ -static int snd_usb_cm106_write_int_reg(struct usb_device *dev, int reg, u16 value) -{ - u8 buf[4]; - buf[0] = 0x20; - buf[1] = value & 0xff; - buf[2] = (value >> 8) & 0xff; - buf[3] = reg; - return snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), USB_REQ_SET_CONFIGURATION, - USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_ENDPOINT, - 0, 0, &buf, 4, 1000); -} - static int snd_usb_cm106_boot_quirk(struct usb_device *dev) { /* -- Adrian. http://elesquinazotango.com.ar http://www.noalcodigodescioli.blogspot.com/

1 0

Re: [alsa-devel] [PATCH 2/6] sound/oss: convert to unlocked_ioctl
by Sam Ravnborg 04 Jul '10

04 Jul '10

> > diff --git a/sound/oss/au1550_ac97.c b/sound/oss/au1550_ac97.c > index c1070e3..3547450 100644 > --- a/sound/oss/au1550_ac97.c > +++ b/sound/oss/au1550_ac97.c > @@ -824,22 +824,26 @@ mixdev_ioctl(struct ac97_codec *codec, unsigned int cmd, > return codec->mixer_ioctl(codec, cmd, arg); > } > > -static int > -au1550_ioctl_mixdev(struct inode *inode, struct file *file, > - unsigned int cmd, unsigned long arg) > +static long > +au1550_ioctl_mixdev(struct file *file, unsigned int cmd, unsigned long arg) > { > struct au1550_state *s = (struct au1550_state *)file->private_data; > struct ac97_codec *codec = s->codec; > + int ret; > + > + lock_kernel(); > + ret = mixdev_ioctl(codec, cmd, arg); > + unlock_kernel(); > > - return mixdev_ioctl(codec, cmd, arg); > + return ret; > } General comment... In several places you declare a local variable of type "int", but the function return a long. I know that mixdev_ioctl() return an int so nothing is lost but it just looks wrong. > diff --git a/sound/oss/dmasound/dmasound_core.c b/sound/oss/dmasound/dmasound_core.c > index 3f3c3f7..b5464fb 100644 > --- a/sound/oss/dmasound/dmasound_core.c > +++ b/sound/oss/dmasound/dmasound_core.c > @@ -337,8 +337,8 @@ static int mixer_release(struct inode *inode, struct file *file) > unlock_kernel(); > return 0; > } > -static int mixer_ioctl(struct inode *inode, struct file *file, u_int cmd, > - u_long arg) > + > +static long mixer_ioctl(struct file *file, u_int cmd, u_long arg) > { > if (_SIOC_DIR(cmd) & _SIOC_WRITE) > mixer.modify_counter++; > @@ -362,11 +362,22 @@ static int mixer_ioctl(struct inode *inode, struct file *file, u_int cmd, > return -EINVAL; > } > > +static long mixer_unlocked_ioctl(struct file *file, u_int cmd, u_long arg) > +{ > + int ret; > + > + lock_kernel(); > + ret = mixer_ioctl(file, cmd, arg); > + unlock_kernel(); > + > + return ret; > +} Here it looks like a potential but. mixer_ioctl() return a long which is stored in an int and then we return a long. > -static int sq_ioctl(struct inode *inode, struct file *file, u_int cmd, > - u_long arg) > +static long sq_ioctl(struct file *file, u_int cmd, u_long arg) > { > int val, result; > u_long fmt; > @@ -1114,18 +1124,29 @@ static int sq_ioctl(struct inode *inode, struct file *file, u_int cmd, > return IOCTL_OUT(arg,val); > > default: > - return mixer_ioctl(inode, file, cmd, arg); > + return mixer_ioctl(file, cmd, arg); > } > return -EINVAL; > } > > +static long sq_unlocked_ioctl(struct file *file, u_int cmd, u_long arg) > +{ > + int ret; > + > + lock_kernel(); > + ret = sq_ioctl(file, cmd, arg); > + unlock_kernel(); > + > + return ret; > +} ditto: long -> int -> long Sam

2 1