Re: [PATCH] i810_audio fix for version 0.11

Andris Pavenis
2001-12-08T09:25:50Z

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

On Saturday 08 December 2001 10:39, Andris Pavenis wrote:
> Sorry, but this patch is still not OK. It still causes system
> locking up for me.
>
> In some cases I have (I added printk in __start_dac):
> dmabuf->count = 0
> dmabuf->ready = 1
> dmabuf->enable = 1
> PCM_ENABLE_OUTPUT set in dmabuf->triger
>
> in __start_dac(). As result nothing was done in this procedure
> and I got system locking in __i810_update_lvi() immediatelly after
> that. This was reason why I added return code to __start_dac,
> __start_adc to skip the loop if there is nothing to wait for.
> Perhaps checking return code is more efficient and less error prone
> that repeating all conditions in __i810_update_lvi.
>
> Maybe really we should use wait_event_interruptible() instead
> of plain loop in __i810_update_lvi(). This happens not so often,
> so I don't think it could cause too big delays. At least we could
> avoid kernel freezing in this way.
>

Here is my updated patch against v0.12 (I changed version to 0.12a to point
a version against which is the patch)

Andris

[-- Attachment #2: i810_audio.c.diff3 --]
[-- Type: text/x-diff, Size: 2724 bytes --]

--- i810_audio.c-0.12 Sat Dec 8 10:24:24 2001
+++ i810_audio.c Sat Dec 8 11:14:16 2001
@@ -198,7 +198,7 @@
#define INT_MASK (INT_SEC|INT_PRI|INT_MC|INT_PO|INT_PI|INT_MO|INT_NI|INT_GPI)

-#define DRIVER_VERSION "0.12"
+#define DRIVER_VERSION "0.12a"

/* magic numbers to protect our data structures */
#define I810_CARD_MAGIC 0x5072696E /* "Prin" */
@@ -690,25 +690,30 @@
spin_unlock_irqrestore(&card->lock, flags);
}

-static inline void __start_adc(struct i810_state *state)
+static inline int __start_adc(struct i810_state *state)
{
+ int ret = 0;
struct dmabuf *dmabuf = &state->dmabuf;

if (dmabuf->count < dmabuf->dmasize && dmabuf->ready && !dmabuf->enable &&
(dmabuf->trigger & PCM_ENABLE_INPUT)) {
+ ret = 1;
dmabuf->enable |= ADC_RUNNING;
outb((1<<4) | (1<<2) | 1, state->card->iobase + PI_CR);
}
+ return ret;
}

-static void start_adc(struct i810_state *state)
+static int start_adc(struct i810_state *state)
{
+ int ret;
struct i810_card *card = state->card;
unsigned long flags;

spin_lock_irqsave(&card->lock, flags);
- __start_adc(state);
+ ret = __start_adc(state);
spin_unlock_irqrestore(&card->lock, flags);
+ return ret;
}

/* stop playback (lock held) */
@@ -736,24 +741,29 @@
spin_unlock_irqrestore(&card->lock, flags);
}

-static inline void __start_dac(struct i810_state *state)
+static inline int __start_dac(struct i810_state *state)
{
+ int ret = 0;
struct dmabuf *dmabuf = &state->dmabuf;

if (dmabuf->count > 0 && dmabuf->ready && !dmabuf->enable &&
(dmabuf->trigger & PCM_ENABLE_OUTPUT)) {
+ ret = 1;
dmabuf->enable |= DAC_RUNNING;
outb((1<<4) | (1<<2) | 1, state->card->iobase + PO_CR);
}
+ return ret;
}
-static void start_dac(struct i810_state *state)
+static int start_dac(struct i810_state *state)
{
+ int ret;
struct i810_card *card = state->card;
unsigned long flags;

spin_lock_irqsave(&card->lock, flags);
- __start_dac(state);
+ ret = __start_dac(state);
spin_unlock_irqrestore(&card->lock, flags);
+ return ret;
}

#define DMABUF_DEFAULTORDER (16-PAGE_SHIFT)
@@ -936,7 +946,7 @@
static void __i810_update_lvi(struct i810_state *state, int rec)
{
struct dmabuf *dmabuf = &state->dmabuf;
- int x, port;
+ int x, port, ok;

port = state->card->iobase;
if(rec)
@@ -955,11 +965,12 @@
if (!dmabuf->enable && dmabuf->trigger) {
outb((inb(port+OFF_CIV)+1)&31, port+OFF_LVI);
if(rec) {
- __start_adc(state);
+ ok = __start_adc(state);
} else {
- __start_dac(state);
+ ok = __start_dac(state);
}
- while( !(inb(port + OFF_CR) & ((1<<4) | (1<<2))) ) ;
+
+ if (ok) while( !(inb(port + OFF_CR) & ((1<<4) | (1<<2))) ) ;
}

/* swptr - 1 is the tail of our transfer */