From owner-freebsd-bugs@FreeBSD.ORG Sun Nov 16 11:20:18 2003 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 92B7316A4DE for ; Sun, 16 Nov 2003 11:20:18 -0800 (PST) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id A7C1C43FAF for ; Sun, 16 Nov 2003 11:20:16 -0800 (PST) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.12.9/8.12.9) with ESMTP id hAGJKGFY049248 for ; Sun, 16 Nov 2003 11:20:16 -0800 (PST) (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.9/8.12.9/Submit) id hAGJKGbL049246; Sun, 16 Nov 2003 11:20:16 -0800 (PST) (envelope-from gnats) Resent-Date: Sun, 16 Nov 2003 11:20:16 -0800 (PST) Resent-Message-Id: <200311161920.hAGJKGbL049246@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Mathew Kanner Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id B994D16A4D0 for ; Sun, 16 Nov 2003 11:13:53 -0800 (PST) Received: from hak.cnd.mcgill.ca (hak.cnd.mcgill.ca [132.216.11.133]) by mx1.FreeBSD.org (Postfix) with ESMTP id 8848643FBD for ; Sun, 16 Nov 2003 11:13:52 -0800 (PST) (envelope-from mat@hak.cnd.mcgill.ca) Received: from hak.cnd.mcgill.ca (localhost [127.0.0.1]) by hak.cnd.mcgill.ca (8.12.9/8.12.8) with ESMTP id hAGJBRk4077205 for ; Sun, 16 Nov 2003 14:11:27 -0500 (EST) (envelope-from mat@hak.cnd.mcgill.ca) Received: (from mat@localhost) by hak.cnd.mcgill.ca (8.12.9/8.12.8/Submit) id hAGJBRWF077204 for FreeBSD-gnats-submit@freebsd.org; Sun, 16 Nov 2003 14:11:27 -0500 (EST) Message-Id: <20031116191127.GJ68875@cnd.mcgill.ca> Date: Sun, 16 Nov 2003 14:11:27 -0500 From: Mathew Kanner To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Subject: kern/59349: patch to including locking for es137x sound driver X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 16 Nov 2003 19:20:18 -0000 >Number: 59349 >Category: kern >Synopsis: patch to including locking for es137x sound driver >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: change-request >Submitter-Id: current-users >Arrival-Date: Sun Nov 16 11:20:16 PST 2003 >Closed-Date: >Last-Modified: >Originator: Mathew Kanner >Release: FreeBSD 5.x >Organization: >Environment: System: FreeBSD tube.mine.nu 5.1-CURRENT-20031021-JPSNAP FreeBSD 5.1-CURRENT-20031021-JPSNAP #0: Tue Oct 21 01:07:08 GMT 2003 root@ushi.jp.freebsd.org:/usr/obj/usr/src/sys/GENERIC i386 >Description: While working with pav@freebsd to solve the poping and craking problem, we experimented with adding locking to the driver. Although it didn't help, I think it belongs in the driver anyway because it moves it to being MPSAFE. I'm filing this here for safe keeping. Note the missing mtx_destroy. >How-To-Repeat: >Fix: --5gxpn/Q6ypwruk0T Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="es137xlock.patch" --- es137x.c.old Sat Nov 15 02:02:10 2003 +++ es137x.c Sat Nov 15 14:18:35 2003 @@ -118,6 +118,7 @@ u_long ctrl; u_long sctrl; struct es_chinfo pch, rch; + struct mtx *lock; }; /* -------------------------------------------------------------------- */ @@ -284,6 +285,7 @@ struct es_chinfo *ch = data; struct es_info *es = ch->parent; + snd_mtxlock(es->lock); if (dir == PCMDIR_PLAY) { bus_space_write_1(es->st, es->sh, ES1370_REG_MEMPAGE, ES1370_REG_DAC2_FRAMEADR >> 8); bus_space_write_4(es->st, es->sh, ES1370_REG_DAC2_FRAMEADR & 0xff, sndbuf_getbufaddr(ch->buffer)); @@ -294,6 +296,7 @@ bus_space_write_4(es->st, es->sh, ES1370_REG_ADC_FRAMECNT & 0xff, (ch->bufsz >> 2) - 1); } ch->dir = dir; + snd_mtxunlock(es->lock); return 0; } @@ -303,6 +306,7 @@ struct es_chinfo *ch = data; struct es_info *es = ch->parent; + snd_mtxlock(es->lock); if (ch->dir == PCMDIR_PLAY) { es->sctrl &= ~SCTRL_P2FMT; if (format & AFMT_S16_LE) es->sctrl |= SCTRL_P2SEB; @@ -314,6 +318,7 @@ } bus_space_write_4(es->st, es->sh, ES1370_REG_SERIAL_CONTROL, es->sctrl); ch->fmt = format; + snd_mtxunlock(es->lock); return 0; } @@ -323,10 +328,12 @@ struct es_chinfo *ch = data; struct es_info *es = ch->parent; + snd_mtxlock(es->lock); es->ctrl &= ~CTRL_PCLKDIV; es->ctrl |= DAC2_SRTODIV(speed) << CTRL_SH_PCLKDIV; bus_space_write_4(es->st, es->sh, ES1370_REG_CONTROL, es->ctrl); /* rec/play speeds locked together - should indicate in flags */ + snd_mtxunlock(es->lock); return speed; /* XXX calc real speed */ } @@ -335,12 +342,16 @@ { struct es_chinfo *ch = data; struct es_info *es = ch->parent; + int i; + snd_mtxlock(es->lock); if (ch->dir == PCMDIR_PLAY) { - return es1371_dac_rate(es, speed, 3 - ch->num); /* play */ + i = es1371_dac_rate(es, speed, 3 - ch->num); /* play */ } else { - return es1371_adc_rate(es, speed, 1); /* record */ + i = es1371_adc_rate(es, speed, 1); /* record */ } + snd_mtxunlock(es->lock); + return i; } static int @@ -365,6 +376,7 @@ if (go == PCMTRIG_EMLDMAWR || go == PCMTRIG_EMLDMARD) return 0; + snd_mtxlock(es->lock); cnt = (ch->blksz / sndbuf_getbps(ch->buffer)) - 1; if (ch->dir == PCMDIR_PLAY) { @@ -391,6 +403,7 @@ } bus_space_write_4(es->st, es->sh, ES1370_REG_SERIAL_CONTROL, es->sctrl); bus_space_write_4(es->st, es->sh, ES1370_REG_CONTROL, es->ctrl); + snd_mtxunlock(es->lock); return 0; } @@ -406,8 +419,10 @@ else reg = ES1370_REG_ADC_FRAMECNT; + snd_mtxlock(es->lock); bus_space_write_4(es->st, es->sh, ES1370_REG_MEMPAGE, reg >> 8); cnt = bus_space_read_4(es->st, es->sh, reg & 0x000000ff) >> 16; + snd_mtxunlock(es->lock); /* cnt is longwords */ return cnt << 2; } @@ -453,8 +468,12 @@ struct es_info *es = p; unsigned intsrc, sctrl; + snd_mtxlock(es->lock); intsrc = bus_space_read_4(es->st, es->sh, ES1370_REG_STATUS); - if ((intsrc & STAT_INTR) == 0) return; + if ((intsrc & STAT_INTR) == 0) { + snd_mtxunlock(es->lock); + return; + } sctrl = es->sctrl; if (intsrc & STAT_ADC) sctrl &= ~SCTRL_R1INTEN; @@ -463,6 +482,7 @@ bus_space_write_4(es->st, es->sh, ES1370_REG_SERIAL_CONTROL, sctrl); bus_space_write_4(es->st, es->sh, ES1370_REG_SERIAL_CONTROL, es->sctrl); + snd_mtxunlock(es->lock); if (intsrc & STAT_ADC) chn_intr(es->rch.channel); if (intsrc & STAT_DAC1); @@ -845,7 +865,7 @@ device_printf(dev, "cannot allocate softc\n"); return ENXIO; } - + es->lock = snd_mtxcreate(device_get_nameunit(dev), "sound softc"); es->dev = dev; mapped = 0; data = pci_read_config(dev, PCIR_COMMAND, 2); @@ -908,7 +928,7 @@ es->irqid = 0; es->irq = bus_alloc_resource(dev, SYS_RES_IRQ, &es->irqid, 0, ~0, 1, RF_ACTIVE | RF_SHAREABLE); - if (!es->irq || snd_setup_intr(dev, es->irq, 0, es_intr, es, &es->ih)) { + if (!es->irq || snd_setup_intr(dev, es->irq, INTR_MPSAFE, es_intr, es, &es->ih)) { device_printf(dev, "unable to map interrupt\n"); goto bad; } --5gxpn/Q6ypwruk0T-- >Release-Note: >Audit-Trail: >Unformatted: --5gxpn/Q6ypwruk0T Content-Type: text/plain; charset=us-ascii Content-Disposition: inline