ESI Juli@ crash with external clock switch - patch

At Fri, 16 Jan 2015 18:13:09 +0100,
Takashi Iwai wrote:
> 
> At Thu, 15 Jan 2015 22:15:46 +0100,
> Pavel Hofman wrote:
> > 
> > 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.
> 
> OK, I'm going to send a fix series including the relevant correction.
> Give it a try later.
> 
> > 
> > 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.
> 
> Hm, but you can still see the other attributes of SPDIF input frames,
> right?  Or all these useless when the clock is set to internal?
> If so, it'd be easy to add the dynamic turn on/off per the clock
> mode.

The patch below can be applied on top of the patch series I've sent.

Takashi

---

ESI Juli@ crash with external clock switch - patch

Commit Message

Patch