From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S268817AbUHaTGw (ORCPT ); Tue, 31 Aug 2004 15:06:52 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S268816AbUHaTFt (ORCPT ); Tue, 31 Aug 2004 15:05:49 -0400 Received: from cantor.suse.de ([195.135.220.2]:9364 "EHLO Cantor.suse.de") by vger.kernel.org with ESMTP id S268807AbUHaTE0 (ORCPT ); Tue, 31 Aug 2004 15:04:26 -0400 Date: Tue, 31 Aug 2004 21:02:46 +0200 Message-ID: From: Takashi Iwai To: Ingo Molnar Cc: Lee Revell , Mark_H_Johnson@raytheon.com, "K.R. Foley" , linux-kernel , Felipe Alfaro Solana , Daniel Schmitt , alsa-devel Subject: Re: [patch] voluntary-preempt-2.6.9-rc1-bk4-Q5 In-Reply-To: <20040831184846.GB25485@elte.hu> References: <1093972819.5403.8.camel@krustophenia.net> <20040831184846.GB25485@elte.hu> User-Agent: Wanderlust/2.10.1 (Watching The Wheels) SEMI/1.14.5 (Awara-Onsen) FLIM/1.14.5 (Demachiyanagi) APEL/10.6 MULE XEmacs/21.4 (patch 15) (Security Through Obscurity) (i386-suse-linux) MIME-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org At Tue, 31 Aug 2004 20:48:46 +0200, Ingo Molnar wrote: > > > * Takashi Iwai wrote: > > > Does the attached patch fix this problem? > > > > > > Takashi > > > > --- linux/sound/pci/ens1370.c 25 Aug 2004 09:57:03 -0000 1.64 > > +++ linux/sound/pci/ens1370.c 31 Aug 2004 18:17:45 -0000 > > @@ -513,6 +513,7 @@ > > r = inl(ES_REG(ensoniq, 1371_SMPRATE)); > > if ((r & ES_1371_SRC_RAM_BUSY) == 0) > > return r; > > + cond_resched(); > > but ... snd_es1371_wait_src_ready() is being called with > ensoniq->reg_lock held and you must not reschedule with a spinlock held. Oh yes, you're right. I overlooked that. > cond_resched_lock(&ensoniq->req_lock) might not crash immediately, but > is it really safe to release the driver lock at this point? The lock can be released, but then *_rate() functions may become racy with each other. Ok, the second try. Takashi --- linux/sound/pci/ens1370.c 25 Aug 2004 09:57:03 -0000 1.64 +++ linux/sound/pci/ens1370.c 31 Aug 2004 19:00:33 -0000 @@ -372,6 +372,7 @@ struct _snd_ensoniq { spinlock_t reg_lock; + struct semaphore src_mutex; int irq; @@ -513,6 +514,7 @@ r = inl(ES_REG(ensoniq, 1371_SMPRATE)); if ((r & ES_1371_SRC_RAM_BUSY) == 0) return r; + cond_resched(); } snd_printk("wait source ready timeout 0x%lx [0x%x]\n", ES_REG(ensoniq, 1371_SMPRATE), r); return 0; @@ -696,6 +698,7 @@ { unsigned int n, truncm, freq, result; + down(&ensoniq->src_mutex); n = rate / 3000; if ((1 << n) & ((1 << 15) | (1 << 13) | (1 << 11) | (1 << 9))) n--; @@ -719,12 +722,14 @@ snd_es1371_src_write(ensoniq, ES_SMPREG_ADC + ES_SMPREG_VFREQ_FRAC, freq & 0x7fff); snd_es1371_src_write(ensoniq, ES_SMPREG_VOL_ADC, n << 8); snd_es1371_src_write(ensoniq, ES_SMPREG_VOL_ADC + 1, n << 8); + up(&ensoniq->src_mutex); } static void snd_es1371_dac1_rate(ensoniq_t * ensoniq, unsigned int rate) { unsigned int freq, r; + down(&ensoniq->src_mutex); freq = ((rate << 15) + 1500) / 3000; r = (snd_es1371_wait_src_ready(ensoniq) & (ES_1371_SRC_DISABLE | ES_1371_DIS_P2 | ES_1371_DIS_R1)) | ES_1371_DIS_P1; outl(r, ES_REG(ensoniq, 1371_SMPRATE)); @@ -734,12 +739,14 @@ snd_es1371_src_write(ensoniq, ES_SMPREG_DAC1 + ES_SMPREG_VFREQ_FRAC, freq & 0x7fff); r = (snd_es1371_wait_src_ready(ensoniq) & (ES_1371_SRC_DISABLE | ES_1371_DIS_P2 | ES_1371_DIS_R1)); outl(r, ES_REG(ensoniq, 1371_SMPRATE)); + up(&ensoniq->src_mutex); } static void snd_es1371_dac2_rate(ensoniq_t * ensoniq, unsigned int rate) { unsigned int freq, r; + down(&ensoniq->src_mutex); freq = ((rate << 15) + 1500) / 3000; r = (snd_es1371_wait_src_ready(ensoniq) & (ES_1371_SRC_DISABLE | ES_1371_DIS_P1 | ES_1371_DIS_R1)) | ES_1371_DIS_P2; outl(r, ES_REG(ensoniq, 1371_SMPRATE)); @@ -749,6 +756,7 @@ snd_es1371_src_write(ensoniq, ES_SMPREG_DAC2 + ES_SMPREG_VFREQ_FRAC, freq & 0x7fff); r = (snd_es1371_wait_src_ready(ensoniq) & (ES_1371_SRC_DISABLE | ES_1371_DIS_P1 | ES_1371_DIS_R1)); outl(r, ES_REG(ensoniq, 1371_SMPRATE)); + up(&ensoniq->src_mutex); } #endif /* CHIP1371 */ @@ -870,11 +878,12 @@ case 44100: ensoniq->ctrl |= ES_1370_WTSRSEL(3); break; default: snd_BUG(); } -#else - snd_es1371_dac1_rate(ensoniq, runtime->rate); #endif outl(ensoniq->ctrl, ES_REG(ensoniq, CONTROL)); spin_unlock_irq(&ensoniq->reg_lock); +#ifndef CHIP1370 + snd_es1371_dac1_rate(ensoniq, runtime->rate); +#endif return 0; } @@ -908,11 +917,12 @@ ensoniq->ctrl |= ES_1370_PCLKDIVO(ES_1370_SRTODIV(runtime->rate)); ensoniq->u.es1370.pclkdiv_lock |= ES_MODE_PLAY2; } -#else - snd_es1371_dac2_rate(ensoniq, runtime->rate); #endif outl(ensoniq->ctrl, ES_REG(ensoniq, CONTROL)); spin_unlock_irq(&ensoniq->reg_lock); +#ifndef CHIP1370 + snd_es1371_dac2_rate(ensoniq, runtime->rate); +#endif return 0; } @@ -944,11 +954,12 @@ ensoniq->ctrl |= ES_1370_PCLKDIVO(ES_1370_SRTODIV(runtime->rate)); ensoniq->u.es1370.pclkdiv_lock |= ES_MODE_CAPTURE; } -#else - snd_es1371_adc_rate(ensoniq, runtime->rate); #endif outl(ensoniq->ctrl, ES_REG(ensoniq, CONTROL)); spin_unlock_irq(&ensoniq->reg_lock); +#ifndef CHIP1370 + snd_es1371_adc_rate(ensoniq, runtime->rate); +#endif return 0; } @@ -1886,6 +1897,7 @@ if (ensoniq == NULL) return -ENOMEM; spin_lock_init(&ensoniq->reg_lock); + init_MUTEX(&ensoniq->src_mutex); ensoniq->card = card; ensoniq->pci = pci; ensoniq->irq = -1;