Hi Mark,
Thanks for reviewing this so quickly.
On Thu, 2018-03-22 at 09:54 +0800, Mark Brown wrote:
> On Wed, Mar 21, 2018 at 07:30:15PM -0300, Ezequiel Garcia wrote:
>
> > +- sdmode-delay : specify a delay time for SD_MODE pin. According
> > + to the DAC datasheet, if LRCLK is removed while BCLK is
> > present,
> > + the DAC output can cause loud pop/crack noises. This
> > property
> > + specifies a delay for the SD_MODE pin assert, required to
> > + eliminate the noise.
>
> Why is this configurable? This sounds like something entirely within
> the digital domain of the device rather than something that depends
> on
> board configuration and it's hard to see how someone would configure
> this.
>
The amount of delay needed seems specific to the CPU DAI,
not specific to the DAC. The CPU DAIs or the machine
could specify this as a parameter, but I can't don't see how.
Perhaps it could be a driver parameter?
> > +static void max98357a_enable_sdmode_work(struct work_struct *work)
> > +{
> > + struct max98357a_priv *max98357a = container_of(work,
> > + struct max98357a_priv, enable_sdmode_work.work);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&max98357a->sdmode_lock, flags);
> > + gpiod_set_value(max98357a->sdmode, max98357a-
> > >sdmode_enabled);
> > + spin_unlock_irqrestore(&max98357a->sdmode_lock, flags);
> > +}
>
> What is this lock supposed to accomplish? We perform a single action
> under the lock which itself has internal locking, it's not going to
> have
> any meaningful effect.
I am under the impression it removes the race condition
between the gpiod_set_value() happening in the different
contexts.
Thanks,
Eze
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
