From patchwork Tue Oct 24 19:40:20 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 10025409 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A8D68601E8 for ; Tue, 24 Oct 2017 19:40:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9843C28A10 for ; Tue, 24 Oct 2017 19:40:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8B15F28A31; Tue, 24 Oct 2017 19:40:28 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 893FC28A10 for ; Tue, 24 Oct 2017 19:40:27 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id 3D1E12673B2; Tue, 24 Oct 2017 21:40:25 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 5417A2673B5; Tue, 24 Oct 2017 21:40:23 +0200 (CEST) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 351DB266DB1 for ; Tue, 24 Oct 2017 21:40:21 +0200 (CEST) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id D400CAD4C; Tue, 24 Oct 2017 19:40:20 +0000 (UTC) Date: Tue, 24 Oct 2017 21:40:20 +0200 Message-ID: From: Takashi Iwai To: Takashi Iwai In-Reply-To: References: <20171024153458.GA112893@beast> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Cc: linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org, Takashi Sakamoto , Kees Cook Subject: Re: [alsa-devel] [PATCH] ALSA: sb: Convert timers to use timer_setup() X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, 24 Oct 2017 21:24:58 +0200, Takashi Iwai wrote: > > On Tue, 24 Oct 2017 17:34:58 +0200, > Kees Cook wrote: > > > > In preparation for unconditionally passing the struct timer_list pointer to > > all timer callbacks, switch to using the new timer_setup() and from_timer() > > to pass the timer pointer explicitly. > > > > Cc: Jaroslav Kysela > > Cc: Takashi Iwai > > Cc: Takashi Sakamoto > > Cc: alsa-devel@alsa-project.org > > Signed-off-by: Kees Cook > > --- > > include/sound/sb.h | 1 + > > sound/isa/sb/emu8000_pcm.c | 6 +++--- > > sound/isa/sb/sb8_midi.c | 12 ++++++------ > > 3 files changed, 10 insertions(+), 9 deletions(-) > > > > diff --git a/include/sound/sb.h b/include/sound/sb.h > > index bacefaee411a..4aef395189ec 100644 > > --- a/include/sound/sb.h > > +++ b/include/sound/sb.h > > @@ -83,6 +83,7 @@ struct snd_sb { > > unsigned int playback_format; > > unsigned int capture_format; > > struct timer_list midi_timer; > > + struct snd_rawmidi_substream *timer_substream; > > This is superfluous. It's identical with chip->midi_substream_output, > as this timer is used only for the output stream. ... and the revised patch is below. > > @@ -230,9 +230,9 @@ static void snd_sb8dsp_midi_output_trigger(struct snd_rawmidi_substream *substre > > spin_lock_irqsave(&chip->open_lock, flags); > > if (up) { > > if (!(chip->open & SB_OPEN_MIDI_OUTPUT_TRIGGER)) { > > - setup_timer(&chip->midi_timer, > > - snd_sb8dsp_midi_output_timer, > > - (unsigned long) substream); > > + chip->timer_substream = substream; > > + timer_setup(&chip->midi_timer, > > + snd_sb8dsp_midi_output_timer, 0); > > I think it's OK to replace straightforwardly like this for this > particular patch (so that the conversion becomes clearer), but ideally > speaking, timer_setup() should go into either *_output_open() or > snd_sb8dsp_midi() so that it's called only once per stream open or > once at creation. > > And, looking at the code closely now, it seems that del_timer_sync() > is missing at the close side, too, but maybe it didn't matter much for > such an old hardware :) In the end, I kept timer_setup() call placed as is in v1 patch. This call will be moved to the more appropriate place in another patch, as well as the additional call of missing del_timer_sync() at *_close(). thanks, Takashi -- 8< -- From: Kees Cook Subject: [PATCH v2] ALSA: sb: Convert timers to use timer_setup() In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. Signed-off-by: Kees Cook Signed-off-by: Takashi Iwai --- v1->v2: Re-use the existing chip->midi_substream_output instead of assigning a new field to struct snd_sb sound/isa/sb/emu8000_pcm.c | 6 +++--- sound/isa/sb/sb8_midi.c | 11 +++++------ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/sound/isa/sb/emu8000_pcm.c b/sound/isa/sb/emu8000_pcm.c index 8f34551abd8d..bc5af71d3bdb 100644 --- a/sound/isa/sb/emu8000_pcm.c +++ b/sound/isa/sb/emu8000_pcm.c @@ -193,9 +193,9 @@ static inline int emu8k_get_curpos(struct snd_emu8k_pcm *rec, int ch) * timer interrupt handler * check the current position and update the period if necessary. */ -static void emu8k_pcm_timer_func(unsigned long data) +static void emu8k_pcm_timer_func(struct timer_list *t) { - struct snd_emu8k_pcm *rec = (struct snd_emu8k_pcm *)data; + struct snd_emu8k_pcm *rec = from_timer(rec, t, timer); int ptr, delta; spin_lock(&rec->timer_lock); @@ -241,7 +241,7 @@ static int emu8k_pcm_open(struct snd_pcm_substream *subs) runtime->private_data = rec; spin_lock_init(&rec->timer_lock); - setup_timer(&rec->timer, emu8k_pcm_timer_func, (unsigned long)rec); + timer_setup(&rec->timer, emu8k_pcm_timer_func, 0); runtime->hw = emu8k_pcm_hw; runtime->hw.buffer_bytes_max = emu->mem_size - LOOP_BLANK_SIZE * 3; diff --git a/sound/isa/sb/sb8_midi.c b/sound/isa/sb/sb8_midi.c index bd672abb4854..05ba2c6b1a8c 100644 --- a/sound/isa/sb/sb8_midi.c +++ b/sound/isa/sb/sb8_midi.c @@ -209,10 +209,10 @@ static void snd_sb8dsp_midi_output_write(struct snd_rawmidi_substream *substream } } -static void snd_sb8dsp_midi_output_timer(unsigned long data) +static void snd_sb8dsp_midi_output_timer(struct timer_list *t) { - struct snd_rawmidi_substream *substream = (struct snd_rawmidi_substream *) data; - struct snd_sb * chip = substream->rmidi->private_data; + struct snd_sb *chip = from_timer(chip, t, midi_timer); + struct snd_rawmidi_substream *substream = chip->midi_substream_output; unsigned long flags; spin_lock_irqsave(&chip->open_lock, flags); @@ -230,9 +230,8 @@ static void snd_sb8dsp_midi_output_trigger(struct snd_rawmidi_substream *substre spin_lock_irqsave(&chip->open_lock, flags); if (up) { if (!(chip->open & SB_OPEN_MIDI_OUTPUT_TRIGGER)) { - setup_timer(&chip->midi_timer, - snd_sb8dsp_midi_output_timer, - (unsigned long) substream); + timer_setup(&chip->midi_timer, + snd_sb8dsp_midi_output_timer, 0); mod_timer(&chip->midi_timer, 1 + jiffies); chip->open |= SB_OPEN_MIDI_OUTPUT_TRIGGER; }