From: Pete Zaitcev <zaitcev@metabyte.com>
To: torvalds@transmeta.com
Cc: zaitcev@metabyte.com, perex@suze.cz, linux-kernel@vger.kernel.org
Subject: Patch for ymfpci in test12-pre7
Date: Sun, 10 Dec 2000 13:24:09 -0800 [thread overview]
Message-ID: <3A33F479.6E7B0C50@metabyte.com> (raw)
Hi, Linus:
The attached patch fixes the following problems with ymfpci in 2.4 tree:
1. Enumeration was wrong, this bit people with several soundcards
(Abhijit Menon-Sen).
2. Must use semaphore to guard open/close.
3. Old ymfpci locks up if compiled with CONFIG_SMP due to
recursive calls to spin_lock_irqsave().
4. Endianness buglet with ''rev'' (same as in ALSA).
Also, The patch removes P3 tagged messages that I do not
anticipate to use soon.
More improvements are in my queue (bugs from Pavel Roskin,
Simon Higgins, etc.), which I would like to put in later.
Thanks in advance,
--Pete
--- linux-2.4.0-pre12-test7/drivers/sound/ymfpci.h Fri Dec 8 22:45:29 2000
+++ linux-2.4.0-test12-pre7-p3/drivers/sound/ymfpci.h Sat Dec 9 23:36:14 2000
@@ -247,7 +247,7 @@
};
struct ymf_unit {
- unsigned int rev; /* PCI revision */
+ u8 rev; /* PCI revision */
void *reg_area_virt;
void *work_ptr; // +
@@ -275,13 +275,13 @@
u16 ac97_features;
struct pci_dev *pci;
- int inst; /* Unit number (instance) */
spinlock_t reg_lock;
spinlock_t voice_lock;
/* soundcore stuff */
int dev_audio;
+ struct semaphore open_sem;
struct list_head ymf_devs;
struct ymf_state *states[1]; // *
@@ -331,10 +331,6 @@
struct ymf_state {
struct ymf_unit *unit; /* backpointer */
-
- /* single open lock mechanism, only used for recording */
- struct semaphore open_sem;
- wait_queue_head_t open_wait;
/* virtual channel number */
int virt; // * unused a.t.m.
--- linux-2.4.0-pre12-test7/drivers/sound/ymfpci.c Fri Dec 8 22:45:29 2000
+++ linux-2.4.0-test12-pre7-p3/drivers/sound/ymfpci.c Sun Dec 10 12:55:35 2000
@@ -32,6 +32,9 @@
* ? merge ymf_pcm and state
* ? pcm interrupt no pointer
* ? underused structure members
+ * - Remove remaining P3 tags (debug messages).
+ * - Resolve XXX tagged questions.
+ * - Cannot play 5133Hz.
*/
#include <linux/module.h>
@@ -59,7 +62,7 @@
int pair, ymfpci_voice_t **rvoice);
static int ymfpci_voice_free(ymfpci_t *codec, ymfpci_voice_t *pvoice);
static int ymf_playback_prepare(ymfpci_t *codec, struct ymf_state *state);
-static int ymf_state_alloc(ymfpci_t *unit, int nvirt, int instance);
+static int ymf_state_alloc(ymfpci_t *unit, int nvirt);
static LIST_HEAD(ymf_devs);
@@ -602,11 +605,9 @@
char silence;
if ((ypcm = voice->ypcm) == NULL) {
-/* P3 */ printk("ymf_pcm_interrupt: voice %d: no ypcm\n", voice->number);
return;
}
if ((state = ypcm->state) == NULL) {
-/* P3 */ printk("ymf_pcm_interrupt: voice %d: no state\n", voice->number);
ypcm->running = 0; // lock it
return;
}
@@ -628,7 +629,7 @@
if (pos < 0 || pos >= dmabuf->dmasize) { /* ucode bug */
printk(KERN_ERR
"ymfpci%d: %d: runaway: hwptr %d dmasize %d\n",
- codec->inst, voice->number,
+ codec->dev_audio, voice->number,
dmabuf->hwptr, dmabuf->dmasize);
pos = 0;
}
@@ -645,7 +646,7 @@
if (dmabuf->count == 0) {
printk("ymfpci%d: %d: strain: hwptr %d\n",
- codec->inst, voice->number, dmabuf->hwptr);
+ codec->dev_audio, voice->number, dmabuf->hwptr);
ymf_playback_trigger(codec, ypcm, 0);
}
@@ -664,7 +665,7 @@
*/
printk("ymfpci%d: %d: lost: delta %d"
" hwptr %d swptr %d distance %d count %d\n",
- codec->inst, voice->number, delta,
+ codec->dev_audio, voice->number, delta,
dmabuf->hwptr, swptr, distance, dmabuf->count);
} else {
/*
@@ -672,7 +673,7 @@
*/
// printk("ymfpci%d: %d: done: delta %d"
// " hwptr %d swptr %d distance %d count %d\n",
-// codec->inst, voice->number, delta,
+// codec->dev_audio, voice->number, delta,
// dmabuf->hwptr, swptr, distance, dmabuf->count);
}
played = dmabuf->count;
@@ -738,7 +739,6 @@
{
if (ypcm->voices[0] == NULL) {
-/* P3 */ printk("ymfpci: trigger %d no voice\n", cmd);
return -EINVAL;
}
if (cmd != 0) {
@@ -911,7 +911,7 @@
if ((err = ymfpci_pcm_voice_alloc(ypcm, state->format.voices)) < 0) {
/* Cannot be unless we leak voices in ymf_release! */
printk(KERN_ERR "ymfpci%d: cannot allocate voice!\n",
- codec->inst);
+ codec->dev_audio);
return err;
}
@@ -1052,7 +1052,7 @@
}
}
-static int ymf_state_alloc(ymfpci_t *unit, int nvirt, int instance)
+static int ymf_state_alloc(ymfpci_t *unit, int nvirt)
{
ymfpci_pcm_t *ypcm;
struct ymf_state *state;
@@ -1062,7 +1062,6 @@
}
memset(state, 0, sizeof(struct ymf_state));
- init_waitqueue_head(&state->open_wait);
init_waitqueue_head(&state->dmabuf.wait);
ypcm = &state->ypcm;
@@ -1541,12 +1540,13 @@
return put_user(SOUND_VERSION, (int *)arg);
case SNDCTL_DSP_RESET:
- /* FIXME: spin_lock ? */
if (file->f_mode & FMODE_WRITE) {
ymf_wait_dac(state);
+ spin_lock_irqsave(&state->unit->reg_lock, flags);
dmabuf->ready = 0;
dmabuf->swptr = dmabuf->hwptr = 0;
dmabuf->count = dmabuf->total_bytes = 0;
+ spin_unlock_irqrestore(&state->unit->reg_lock, flags);
}
#if HAVE_RECORD
if (file->f_mode & FMODE_READ) {
@@ -1576,9 +1576,7 @@
case SNDCTL_DSP_SPEED: /* set smaple rate */
if (get_user(val, (int *)arg))
return -EFAULT;
- /* P3 */ /* printk("ymfpci: ioctl SNDCTL_DSP_SPEED %d\n", val); */
if (val >= 8000 && val <= 48000) {
- spin_lock_irqsave(&state->unit->reg_lock, flags);
if (file->f_mode & FMODE_WRITE) {
ymf_wait_dac(state);
}
@@ -1587,6 +1585,7 @@
stop_adc(state);
}
#endif
+ spin_lock_irqsave(&state->unit->reg_lock, flags);
dmabuf->ready = 0;
state->format.rate = val;
ymf_pcm_update_shift(&state->format);
@@ -1603,7 +1602,6 @@
case SNDCTL_DSP_STEREO: /* set stereo or mono channel */
if (get_user(val, (int *)arg))
return -EFAULT;
- /* P3 */ /* printk("ymfpci: ioctl SNDCTL_DSP_STEREO %d\n", val); */
if (file->f_mode & FMODE_WRITE) {
ymf_wait_dac(state);
spin_lock_irqsave(&state->unit->reg_lock, flags);
@@ -1625,7 +1623,6 @@
return 0;
case SNDCTL_DSP_GETBLKSIZE:
- /* P3 */ /* printk("ymfpci: ioctl SNDCTL_DSP_GETBLKSIZE\n"); */
if (file->f_mode & FMODE_WRITE) {
if ((val = prog_dmabuf(state, 0)))
return val;
@@ -1639,15 +1636,12 @@
return -EINVAL;
case SNDCTL_DSP_GETFMTS: /* Returns a mask of supported sample format*/
- /* P3 */ /* printk("ymfpci: ioctl SNDCTL_DSP_GETFMTS\n"); */
return put_user(AFMT_S16_LE|AFMT_U8, (int *)arg);
case SNDCTL_DSP_SETFMT: /* Select sample format */
if (get_user(val, (int *)arg))
return -EFAULT;
- /* P3 */ /* printk("ymfpci: ioctl SNDCTL_DSP_SETFMT 0x%x\n", val); */
if (val == AFMT_S16_LE || val == AFMT_U8) {
- spin_lock_irqsave(&state->unit->reg_lock, flags);
if (file->f_mode & FMODE_WRITE) {
ymf_wait_dac(state);
}
@@ -1656,6 +1650,7 @@
stop_adc(state);
}
#endif
+ spin_lock_irqsave(&state->unit->reg_lock, flags);
dmabuf->ready = 0;
state->format.format = val;
ymf_pcm_update_shift(&state->format);
@@ -1668,22 +1663,24 @@
return -EFAULT;
/* P3 */ /* printk("ymfpci: ioctl SNDCTL_DSP_CHANNELS 0x%x\n", val); */
if (val != 0) {
- spin_lock_irqsave(&state->unit->reg_lock, flags);
if (file->f_mode & FMODE_WRITE) {
ymf_wait_dac(state);
if (val == 1 || val == 2) {
+ spin_lock_irqsave(&state->unit->reg_lock, flags);
dmabuf->ready = 0;
state->format.voices = val;
ymf_pcm_update_shift(&state->format);
+ spin_unlock_irqrestore(&state->unit->reg_lock, flags);
}
}
#if HAVE_RECORD
if (file->f_mode & FMODE_READ) {
+ spin_lock_irqsave(&state->unit->reg_lock, flags);
stop_adc(state);
dmabuf->ready = 0;
+ spin_unlock_irqrestore(&state->unit->reg_lock, flags);
}
#endif
- spin_unlock_irqrestore(&state->unit->reg_lock, flags);
}
return put_user(state->format.voices, (int *)arg);
@@ -1737,7 +1734,6 @@
return 0;
case SNDCTL_DSP_GETOSPACE:
- /* P3 */ /* printk("ymfpci: ioctl SNDCTL_DSP_GETOSPACE\n"); */
if (!(file->f_mode & FMODE_WRITE))
return -EINVAL;
if (!dmabuf->ready && (val = prog_dmabuf(state, 0)) != 0)
@@ -1768,12 +1764,10 @@
#endif
case SNDCTL_DSP_NONBLOCK:
- /* P3 */ /* printk("ymfpci: ioctl SNDCTL_DSP_NONBLOCK\n"); */
file->f_flags |= O_NONBLOCK;
return 0;
case SNDCTL_DSP_GETCAPS:
- /* P3 */ /* printk("ymfpci: ioctl SNDCTL_DSP_GETCAPS\n"); */
/* return put_user(DSP_CAP_REALTIME|DSP_CAP_TRIGGER|DSP_CAP_MMAP,
(int *)arg); */
return put_user(0, (int *)arg);
@@ -1826,7 +1820,6 @@
#endif
case SNDCTL_DSP_GETOPTR:
- /* P3 */ /* printk("ymfpci: ioctl SNDCTL_DSP_GETOPTR\n"); */
if (!(file->f_mode & FMODE_WRITE))
return -EINVAL;
spin_lock_irqsave(&state->unit->reg_lock, flags);
@@ -1840,7 +1833,6 @@
return copy_to_user((void *)arg, &cinfo, sizeof(cinfo));
case SNDCTL_DSP_SETDUPLEX: /* XXX TODO */
- /* P3 */ /* printk("ymfpci: ioctl SNDCTL_DSP_SETDUPLEX\n"); */
return -EINVAL;
#if 0 /* old */
@@ -1871,7 +1863,6 @@
return -ENOTTY;
default:
- /* P3 */ printk(KERN_WARNING "ymfpci: unknown ioctl cmd 0x%x\n", cmd);
/*
* Some programs mix up audio devices and ioctls
* or perhaps they expect "universal" ioctls,
@@ -1886,7 +1877,7 @@
{
struct list_head *list;
ymfpci_t *unit;
- int minor, instance;
+ int minor;
struct ymf_state *state;
int nvirt;
int err;
@@ -1903,24 +1894,24 @@
} else {
return -ENXIO;
}
- instance = (minor >> 4) & 0x0F;
nvirt = 0; /* Such is the partitioning of minor */
- /* XXX Semaphore here! */
for (list = ymf_devs.next; list != &ymf_devs; list = list->next) {
unit = list_entry(list, ymfpci_t, ymf_devs);
- if (unit->inst == instance)
+ if (((unit->dev_audio ^ minor) & ~0x0F) == 0)
break;
}
if (list == &ymf_devs)
return -ENODEV;
+ down(&unit->open_sem);
if (unit->states[nvirt] != NULL) {
- /* P3 */ printk("ymfpci%d: busy\n", unit->inst);
+ up(&unit->open_sem);
return -EBUSY;
}
- if ((err = ymf_state_alloc(unit, nvirt, instance)) != 0) {
+ if ((err = ymf_state_alloc(unit, nvirt)) != 0) {
+ up(&unit->open_sem);
return err;
}
state = unit->states[nvirt];
@@ -1940,6 +1931,7 @@
unit->states[state->virt] = NULL;
kfree(state);
+ up(&unit->open_sem);
return err;
}
@@ -1948,6 +1940,8 @@
ymfpci_writeb(codec, YDSXGR_TIMERCTRL,
(YDSXGR_TIMERCTRL_TEN|YDSXGR_TIMERCTRL_TIEN));
#endif
+ up(&unit->open_sem);
+ /* XXX Is it correct to have MOD_INC_USE_COUNT outside of sem.? */
MOD_INC_USE_COUNT;
return 0;
@@ -1962,14 +1956,14 @@
ymfpci_writeb(codec, YDSXGR_TIMERCTRL, 0);
#endif
- /* XXX Use the semaphore to unrace us with opens */
-
if (state != codec->states[state->virt]) {
printk(KERN_ERR "ymfpci%d.%d: state mismatch\n",
- state->unit->inst, state->virt);
+ state->unit->dev_audio, state->virt);
return -EIO;
}
+ down(&codec->open_sem);
+
/*
* XXX Solve the case of O_NONBLOCK close - don't deallocate here.
* Deallocate when unloading the driver and we can wait.
@@ -1981,6 +1975,8 @@
codec->states[state->virt] = NULL;
kfree(state);
+ up(&codec->open_sem);
+
MOD_DEC_USE_COUNT;
return 0;
}
@@ -2235,7 +2231,6 @@
codec->codec_write = ymfpci_codec_write;
if (ac97_probe_codec(codec) == 0) {
- /* Alan does not have this printout. P3 */
printk("ymfpci: ac97_probe_codec failed\n");
goto out_kfree;
}
@@ -2264,7 +2259,6 @@
static int __devinit ymf_probe_one(struct pci_dev *pcidev, const struct pci_device_id *ent)
{
u16 ctrl;
- static int ymf_instance; /* = 0 */
ymfpci_t *codec;
int err;
@@ -2282,13 +2276,13 @@
spin_lock_init(&codec->reg_lock);
spin_lock_init(&codec->voice_lock);
+ init_MUTEX(&codec->open_sem);
codec->pci = pcidev;
- codec->inst = ymf_instance;
- pci_read_config_byte(pcidev, PCI_REVISION_ID, (u8 *)&codec->rev);
+ pci_read_config_byte(pcidev, PCI_REVISION_ID, &codec->rev);
codec->reg_area_virt = ioremap(pci_resource_start(pcidev, 0), 0x8000);
- printk(KERN_INFO "ymfpci%d: %s at 0x%lx IRQ %d\n", ymf_instance,
+ printk(KERN_INFO "ymfpci: %s at 0x%lx IRQ %d\n",
(char *)ent->driver_data, pci_resource_start(pcidev, 0), pcidev->irq);
ymfpci_aclink_reset(pcidev);
@@ -2306,13 +2300,14 @@
if (request_irq(pcidev->irq, ymf_interrupt, SA_SHIRQ, "ymfpci", codec) != 0) {
printk(KERN_ERR "ymfpci%d: unable to request IRQ %d\n",
- codec->inst, pcidev->irq);
+ codec->dev_audio, pcidev->irq);
goto out_memfree;
}
/* register /dev/dsp */
if ((codec->dev_audio = register_sound_dsp(&ymf_fops, -1)) < 0) {
- printk(KERN_ERR "ymfpci%d: unable to register dsp\n", codec->inst);
+ printk(KERN_ERR "ymfpci%d: unable to register dsp\n",
+ codec->dev_audio);
goto out_free_irq;
}
@@ -2325,7 +2320,6 @@
/* put it into driver list */
list_add_tail(&codec->ymf_devs, &ymf_devs);
pci_set_drvdata(pcidev, codec);
- ymf_instance++;
return 0;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
reply other threads:[~2000-12-10 21:55 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3A33F479.6E7B0C50@metabyte.com \
--to=zaitcev@metabyte.com \
--cc=linux-kernel@vger.kernel.org \
--cc=perex@suze.cz \
--cc=torvalds@transmeta.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.