[alsa-devel] Correct use of ak4114.c?
Takashi Iwai
tiwai at suse.de
Fri Mar 30 15:44:00 CEST 2007
At Fri, 30 Mar 2007 12:24:39 +0200 (CEST),
dustin at seznam.cz wrote:
>
> Hello,
>
> while working on support for AK4114 in Audiotrak MI/ODI/O card, I
> used corresponding code from revo.c (and similarly from juli.c),
> calling function snd_ak4114_create. However, this function does not
> initialize array ak4114->kctls[idx], leading to consequent kernel
> oops after the periodically called snd_ak4114_check_rate_and_errors,
> specifically in function snd_ctl_notify called by
> snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE,
> &ak4114->kctls[0]->id)
>
> Temporarily I resolved the problem by commenting out the
> snd_ctl_notify calls. However, it seems author of ak4114.c intended
> to create the device with snd_ak4114_build instead which defines all
> the controls. I did not hit the problem until fixing the 4-wire
> communication in revo.c - that explaing why nobody had this problem
> with revo cards. I do not understand how the code can work in juli.c
> - perhaps I have overlooked something there.
I don't know how well the juli code works over all as I've never had
this hardware and had no chance for tests. But my guess is that the
driver is more or less borken now.
> My question: Is the correct way to add null pointer checks before
> snd_ctl_notify calls only, or to use snd_ak4114_build insted of
> snd_ak4114_create? If using snd_ak4114_build, where do I get the
> parameters ply_substream and cap_substream?
I fixed this on HG tree now, so at least the driver should work even
without calling snd_ak4114_build(). The patch is below.
For working properly with the parameter change notifications, you'd
need to call snd_ak4114_build() with PCM substreams that you made.
That is, first create ak4114 instance via snd_ak4114_create(), create
PCMs, then attach PCMs with snd_ak4114_build() later.
Thanks,
Takashi
# HG changeset patch
# User tiwai
# Date 1175261919 -7200
# Node ID 1df47bdd721aed75cedc4c6d9456c7cb72b92c8e
# Parent c88499e08ee15f55c0adf5807331d4b15c8cd2d8
ak4114 - Fix possible Oops with callbacks
ak4114 code may trigger Oops when the parameters are changed without
call of snd_ak4114_build(). Now it checks the existence of kctl
element, and the workq is triggered after building the necessary
kcontrols.
Also, did some code clean up.
diff -r c88499e08ee1 -r 1df47bdd721a i2c/other/ak4114.c
--- a/i2c/other/ak4114.c Thu Mar 29 17:02:45 2007 +0200
+++ b/i2c/other/ak4114.c Fri Mar 30 15:38:39 2007 +0200
@@ -36,6 +36,7 @@ MODULE_LICENSE("GPL");
#define AK4114_ADDR 0x00 /* fixed address */
static void ak4114_stats(struct work_struct *work);
+static void ak4114_init_regs(struct ak4114 *chip);
static void reg_write(struct ak4114 *ak4114, unsigned char reg, unsigned char val)
{
@@ -105,7 +106,7 @@ int snd_ak4114_create(struct snd_card *c
for (reg = 0; reg < 5; reg++)
chip->txcsb[reg] = txcsb[reg];
- snd_ak4114_reinit(chip);
+ ak4114_init_regs(chip);
chip->rcs0 = reg_read(chip, AK4114_REG_RCS0) & ~(AK4114_QINT | AK4114_CINT);
chip->rcs1 = reg_read(chip, AK4114_REG_RCS1);
@@ -131,13 +132,10 @@ void snd_ak4114_reg_write(struct ak4114
(chip->txcsb[reg-AK4114_REG_TXCSB0] & ~mask) | val);
}
-void snd_ak4114_reinit(struct ak4114 *chip)
+static void ak4114_init_regs(struct ak4114 *chip)
{
unsigned char old = chip->regmap[AK4114_REG_PWRDN], reg;
- chip->init = 1;
- mb();
- flush_scheduled_work();
/* bring the chip to reset state and powerdown state */
reg_write(chip, AK4114_REG_PWRDN, old & ~(AK4114_RST|AK4114_PWN));
udelay(200);
@@ -150,9 +148,18 @@ void snd_ak4114_reinit(struct ak4114 *ch
reg_write(chip, reg + AK4114_REG_TXCSB0, chip->txcsb[reg]);
/* release powerdown, everything is initialized now */
reg_write(chip, AK4114_REG_PWRDN, old | AK4114_RST | AK4114_PWN);
+}
+
+void snd_ak4114_reinit(struct ak4114 *chip)
+{
+ chip->init = 1;
+ mb();
+ flush_scheduled_work();
+ ak4114_init_regs(chip);
/* bring up statistics / event queing */
chip->init = 0;
- schedule_delayed_work(&chip->work, HZ / 10);
+ if (chip->kctls[0])
+ schedule_delayed_work(&chip->work, HZ / 10);
}
static unsigned int external_rate(unsigned char rcs1)
@@ -472,7 +479,53 @@ int snd_ak4114_build(struct ak4114 *ak41
return err;
ak4114->kctls[idx] = kctl;
}
- return 0;
+ /* trigger workq */
+ schedule_delayed_work(&ak4114->work, HZ / 10);
+ return 0;
+}
+
+/* notify kcontrols if any parameters are changed */
+static void ak4114_notify(struct ak4114 *ak4114,
+ unsigned char rcs0, unsigned char rcs1,
+ unsigned char c0, unsigned char c1)
+{
+ if (!ak4114->kctls[0])
+ return;
+
+ if (rcs0 & AK4114_PAR)
+ snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE,
+ &ak4114->kctls[0]->id);
+ if (rcs0 & AK4114_V)
+ snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE,
+ &ak4114->kctls[1]->id);
+ if (rcs1 & AK4114_CCRC)
+ snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE,
+ &ak4114->kctls[2]->id);
+ if (rcs1 & AK4114_QCRC)
+ snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE,
+ &ak4114->kctls[3]->id);
+
+ /* rate change */
+ if (c1 & 0xf0)
+ snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE,
+ &ak4114->kctls[4]->id);
+
+ if ((c0 & AK4114_PEM) | (c0 & AK4114_CINT))
+ snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE,
+ &ak4114->kctls[9]->id);
+ if (c0 & AK4114_QINT)
+ snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE,
+ &ak4114->kctls[10]->id);
+
+ if (c0 & AK4114_AUDION)
+ snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE,
+ &ak4114->kctls[11]->id);
+ if (c0 & AK4114_AUTO)
+ snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE,
+ &ak4114->kctls[12]->id);
+ if (c0 & AK4114_DTSCD)
+ snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE,
+ &ak4114->kctls[13]->id);
}
int snd_ak4114_external_rate(struct ak4114 *ak4114)
@@ -511,31 +564,7 @@ int snd_ak4114_check_rate_and_errors(str
ak4114->rcs1 = rcs1;
spin_unlock_irqrestore(&ak4114->lock, _flags);
- if (rcs0 & AK4114_PAR)
- snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE, &ak4114->kctls[0]->id);
- if (rcs0 & AK4114_V)
- snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE, &ak4114->kctls[1]->id);
- if (rcs1 & AK4114_CCRC)
- snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE, &ak4114->kctls[2]->id);
- if (rcs1 & AK4114_QCRC)
- snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE, &ak4114->kctls[3]->id);
-
- /* rate change */
- if (c1 & 0xf0)
- snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE, &ak4114->kctls[4]->id);
-
- if ((c0 & AK4114_PEM) | (c0 & AK4114_CINT))
- snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE, &ak4114->kctls[9]->id);
- if (c0 & AK4114_QINT)
- snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE, &ak4114->kctls[10]->id);
-
- if (c0 & AK4114_AUDION)
- snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE, &ak4114->kctls[11]->id);
- if (c0 & AK4114_AUTO)
- snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE, &ak4114->kctls[12]->id);
- if (c0 & AK4114_DTSCD)
- snd_ctl_notify(ak4114->card, SNDRV_CTL_EVENT_MASK_VALUE, &ak4114->kctls[13]->id);
-
+ ak4114_notify(ak4114, rcs0, rcs1, c0, c1);
if (ak4114->change_callback && (c0 | c1) != 0)
ak4114->change_callback(ak4114, c0, c1);
@@ -559,8 +588,8 @@ static void ak4114_stats(struct work_str
struct ak4114 *chip = container_of(work, struct ak4114, work.work);
if (chip->init)
- return;
- snd_ak4114_check_rate_and_errors(chip, 0);
+ snd_ak4114_check_rate_and_errors(chip, 0);
+
schedule_delayed_work(&chip->work, HZ / 10);
}
diff -r c88499e08ee1 -r 1df47bdd721a pci/ice1712/juli.c
--- a/pci/ice1712/juli.c Thu Mar 29 17:02:45 2007 +0200
+++ b/pci/ice1712/juli.c Fri Mar 30 15:38:39 2007 +0200
@@ -160,13 +160,6 @@ static int __devinit juli_init(struct sn
int err;
struct snd_akm4xxx *ak;
-#if 0
- for (err = 0; err < 0x20; err++)
- juli_ak4114_read(ice, err);
- juli_ak4114_write(ice, 0, 0x0f);
- juli_ak4114_read(ice, 0);
- juli_ak4114_read(ice, 1);
-#endif
err = snd_ak4114_create(ice->card,
juli_ak4114_read,
juli_ak4114_write,
More information about the Alsa-devel
mailing list