linux-kernel - Re: [PATCH] ASoC: Add max98088 CODEC driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100930005850.GB4517@opensource.wolfsonmicro.com>
Date:	Wed, 29 Sep 2010 17:58:51 -0700
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Peter Hsiang <Peter.Hsiang@...im-ic.com>
Cc:	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.de>,
	Liam Girdwood <lrg@...mlogic.co.uk>,
	Peter Ujfalusi <peter.ujfalusi@...ia.com>,
	"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Jesse Marroquin <Jesse.Marroquin@...im-ic.com>
Subject: Re: [PATCH] ASoC: Add max98088 CODEC driver

On Wed, Sep 29, 2010 at 05:52:08PM -0700, Peter Hsiang wrote:
> On Wed, Sep 29, 2010, Mark Brown wrote:

> > You're looking for the non-DAPM equivalent of SND_SOC_DAPM_VALUE_MUX().

> This is a simple table lookup of a register value from the index
> number given by SOC_ENUM, the same way it's been done in other drivers.

Yes, exactly.

> I found a "case snd_soc_dapm_value_mux:" in dapm_set_path_status()
> Is this what you are referring to?
> How is the code there relevant to this?

No, I'm referring to the fact that it provides an enum that allows
arbatrary values to be set for each enum value rather than just allowing
indexes as the standard SOC_ENUM does.  This is essentially what you've
implemented, it would factor out the code so that others can use it too
(and IIRC save you some code since IIRC you had more than one of these
in the driver).

> > > > > +       /* powering down headphone gracefully */
> > > > > +       status = snd_soc_read(codec, M98088_REG_4D_PWR_EN_OUT);
> > > > > +       if ((status & M98088_HPEN) == M98088_HPEN) {
> > > > > +               max98088_hw_write(codec, M98088_REG_4D_PWR_EN_OUT,
> > > > > +                       (status & ~M98088_HPEN));
> > > > > +       }
> > > > > +       schedule_timeout(msecs_to_jiffies(20));

> > > > This looks rather like it should just be a post event implementing a
> > > > timeout?

> > > This needs to work as a pre event.

> > Again, why is this?

> When powering down the headphone, the way that DAPM works is it
> likes to power off one item at a time, for example, the left channel,
> then right channel.  The headphone hardware likes to see the 

Have you tested with current Linux versions?  For quite a few kernel
releases now register writes are batched so that all the controls of a
given type in a single register will be set together.

> headphone bits L and R be powered down together, for optimum result.
> This works best with the pre method.  Powering up one channel at a 
> time later is fine, when DAPM resumes.

With current Linux versions you should see the left and right channels
powered down together (if they are both being powered down, of course).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/