[alsa-devel] ESI Juli@ crash with external clock switch - patch

[Pavel Hofman]

Dne 12.1.2015 v 16:43 Takashi Iwai napsal(a):
> At Mon, 12 Jan 2015 09:34:52 +0100,
> Pavel Hofman wrote:
>>
>>
>> I wish I could help but unfortunately my practical knowledge of kernel
>> workqueues is close to zero :-( Of course I will test the patches and
>> will extend them for quartet with testing too.
>
> How about the patch below?  This is a quick fix for 3.19 (and
> stable).  More better fixes will follow later once after it's
> confirmed to work.

Hi,

Thanks a lot, the patch seems to run fine. Only ak4114_init_regs has to 
be called every samplerate change, otherwise the receiver does not 
re-run the samplerate detection and its register with detected 
samplerate does not update its value.

The following patch seems to run ok:

diff --git a/include/sound/ak4114.h b/include/sound/ak4114.h
index 52f02a6..796834b 100644
--- a/include/sound/ak4114.h
+++ b/include/sound/ak4114.h
@@ -169,6 +169,7 @@ struct ak4114 {
         ak4114_read_t * read;
         void * private_data;
         unsigned int init: 1;
+       bool in_workq;
         spinlock_t lock;
         unsigned char regmap[6];
         unsigned char txcsb[5];
diff --git a/sound/i2c/other/ak4114.c b/sound/i2c/other/ak4114.c
index c7f5633..6d643b7 100644
--- a/sound/i2c/other/ak4114.c
+++ b/sound/i2c/other/ak4114.c
@@ -152,10 +152,12 @@ static void ak4114_init_regs(struct ak4114 *chip)

  void snd_ak4114_reinit(struct ak4114 *chip)
  {
+       ak4114_init_regs(chip);
+       if (chip->in_workq)
+               return;
         chip->init = 1;
         mb();
-       flush_delayed_work(&chip->work);
-       ak4114_init_regs(chip);
+       cancel_delayed_work_sync(&chip->work);
         /* bring up statistics / event queing */
         chip->init = 0;
         if (chip->kctls[0])
@@ -612,10 +614,12 @@ static void ak4114_stats(struct work_struct *work)
  {
         struct ak4114 *chip = container_of(work, struct ak4114, work.work);

-       if (!chip->init)
+       chip->in_workq = true;
+       if (!chip->init) {
                 snd_ak4114_check_rate_and_errors(chip, chip->check_flags);
-
-       schedule_delayed_work(&chip->work, HZ / 10);
+               schedule_delayed_work(&chip->work, HZ / 10);
+       }
+       chip->in_workq = false;
  }

  EXPORT_SYMBOL(snd_ak4114_create);


>
>>> The HZ/10 isn't that bad, but the problem is that it's unconditionally
>>> running even if user doesn't need/want.
>>
>> It is useful only for the external clock mode. In fact the detection of
>> incoming SPDIF rate is not reliable for internal clock in Juli (while it
>> works just fine in Quartet, its FPGA pins configure the SPDIF receiver
>> differently). IMO the thread could be running only when clock is
>> switched to external.
>
> Yeah, we can do some smart task change in addition to manual on/off.
> Maybe it's good to have an enum control for that.

I am not sure users would want/need to disable a feature which detects 
incoming samplerate. IMO if the work thread is running only in the 
external clock mode, nothing more is needed.

Thanks a lot and regards,

Pavel.