From: "Opensource [Adam Thomson]" <Adam.Thomson.Opensource@diasemi.com> To: Mark Brown <broonie@kernel.org>, "Opensource [Adam Thomson]" <Adam.Thomson.Opensource@diasemi.com> Cc: Liam Girdwood <lgirdwood@gmail.com>, Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>, Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>, Mark Rutland <mark.rutland@arm.com>, Ian Campbell <ijc+devicetree@hellion.org.uk>, Kumar Gala <galak@codeaurora.org>, "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "Support Opensource" <Support.Opensource@diasemi.com> Subject: RE: [PATCH 2/2] ASoC: codecs: Add da7218 codec driver Date: Fri, 6 Nov 2015 11:11:38 +0000 [thread overview] Message-ID: <2E89032DDAA8B9408CB92943514A0337D460CA0C@SW-EX-MBX01.diasemi.com> (raw) In-Reply-To: <20151105152736.GO18409@sirena.org.uk> [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1961 bytes --] On November 5, 2015 15:28, Mark Brown wrote: > > +/* ALC */ > > +static void da7218_alc_calib(struct snd_soc_codec *codec) > > +{ > > + struct da7218_priv *da7218 = snd_soc_codec_get_drvdata(codec); > > + u8 calib_ctrl; > > + int i = 0; > > + bool calibrated = false; > > + > > + /* Bypass cache so it saves current settings */ > > + regcache_cache_bypass(da7218->regmap, true); > > What ensures that nothing else is running at the same time this is? Is a fair point. Originally I was saving the state of registers then re-instating them at the end, which worked fine, but then was trying to be clever and tidy things up by bypassing the cache instead. Will revert back to the previous method. > > > +static int da7218_mic_lvl_det_sw_put(struct snd_kcontrol *kcontrol, > > + struct snd_ctl_elem_value *ucontrol) > > +{ > > Why is this a user visible control? I can envisage in a system you may want to choose which capture channels can trigger level detection (if any), and this may change depending on the use-case at the time, so having it as a control makes sense to me. > > > + /* Default all mixers off */ > > + snd_soc_write(codec, DA7218_DROUTING_OUTDAI_1L, 0); > > + snd_soc_write(codec, DA7218_DROUTING_OUTDAI_1R, 0); > > + snd_soc_write(codec, DA7218_DROUTING_OUTDAI_2L, 0); > > + snd_soc_write(codec, DA7218_DROUTING_OUTDAI_2R, 0); > > + snd_soc_write(codec, DA7218_DROUTING_OUTFILT_1L, 0); > > + snd_soc_write(codec, DA7218_DROUTING_OUTFILT_1R, 0); > > + snd_soc_write(codec, DA7218_DROUTING_ST_OUTFILT_1L, 0); > > + snd_soc_write(codec, DA7218_DROUTING_ST_OUTFILT_1R, 0); > > We generally just use the device defaults, why change them? I figured it made more sense to have the device start with audio routes disabled but I can remove this as it's really not essential. ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
WARNING: multiple messages have this Message-ID (diff)
From: "Opensource [Adam Thomson]" <Adam.Thomson.Opensource@diasemi.com> To: Mark Brown <broonie@kernel.org>, "Opensource [Adam Thomson]" <Adam.Thomson.Opensource@diasemi.com> Cc: Mark Rutland <mark.rutland@arm.com>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>, Support Opensource <Support.Opensource@diasemi.com>, Pawel Moll <pawel.moll@arm.com>, Ian Campbell <ijc+devicetree@hellion.org.uk>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Takashi Iwai <tiwai@suse.com>, Liam Girdwood <lgirdwood@gmail.com>, Rob Herring <robh+dt@kernel.org>, Kumar Gala <galak@codeaurora.org> Subject: Re: [PATCH 2/2] ASoC: codecs: Add da7218 codec driver Date: Fri, 6 Nov 2015 11:11:38 +0000 [thread overview] Message-ID: <2E89032DDAA8B9408CB92943514A0337D460CA0C@SW-EX-MBX01.diasemi.com> (raw) In-Reply-To: <20151105152736.GO18409@sirena.org.uk> On November 5, 2015 15:28, Mark Brown wrote: > > +/* ALC */ > > +static void da7218_alc_calib(struct snd_soc_codec *codec) > > +{ > > + struct da7218_priv *da7218 = snd_soc_codec_get_drvdata(codec); > > + u8 calib_ctrl; > > + int i = 0; > > + bool calibrated = false; > > + > > + /* Bypass cache so it saves current settings */ > > + regcache_cache_bypass(da7218->regmap, true); > > What ensures that nothing else is running at the same time this is? Is a fair point. Originally I was saving the state of registers then re-instating them at the end, which worked fine, but then was trying to be clever and tidy things up by bypassing the cache instead. Will revert back to the previous method. > > > +static int da7218_mic_lvl_det_sw_put(struct snd_kcontrol *kcontrol, > > + struct snd_ctl_elem_value *ucontrol) > > +{ > > Why is this a user visible control? I can envisage in a system you may want to choose which capture channels can trigger level detection (if any), and this may change depending on the use-case at the time, so having it as a control makes sense to me. > > > + /* Default all mixers off */ > > + snd_soc_write(codec, DA7218_DROUTING_OUTDAI_1L, 0); > > + snd_soc_write(codec, DA7218_DROUTING_OUTDAI_1R, 0); > > + snd_soc_write(codec, DA7218_DROUTING_OUTDAI_2L, 0); > > + snd_soc_write(codec, DA7218_DROUTING_OUTDAI_2R, 0); > > + snd_soc_write(codec, DA7218_DROUTING_OUTFILT_1L, 0); > > + snd_soc_write(codec, DA7218_DROUTING_OUTFILT_1R, 0); > > + snd_soc_write(codec, DA7218_DROUTING_ST_OUTFILT_1L, 0); > > + snd_soc_write(codec, DA7218_DROUTING_ST_OUTFILT_1R, 0); > > We generally just use the device defaults, why change them? I figured it made more sense to have the device start with audio routes disabled but I can remove this as it's really not essential.
next prev parent reply other threads:[~2015-11-06 11:11 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-11-05 10:43 [PATCH 0/2] ASoC: Add support for DA7217 and DA7218 audio codecs Adam Thomson 2015-11-05 10:43 ` Adam Thomson 2015-11-05 10:43 ` [PATCH 1/2] ASoC: da7218: Add bindings documentation for DA7218 audio codec Adam Thomson 2015-11-05 10:43 ` Adam Thomson 2015-11-05 14:58 ` Mark Brown 2015-11-05 17:59 ` Opensource [Adam Thomson] 2015-11-05 17:59 ` Opensource [Adam Thomson] 2015-11-06 10:30 ` Mark Brown 2015-11-05 10:43 ` [PATCH 2/2] ASoC: codecs: Add da7218 codec driver Adam Thomson 2015-11-05 10:43 ` Adam Thomson 2015-11-05 15:27 ` Mark Brown 2015-11-06 11:11 ` Opensource [Adam Thomson] [this message] 2015-11-06 11:11 ` Opensource [Adam Thomson] 2015-11-06 11:21 ` Mark Brown 2015-11-06 11:21 ` Mark Brown 2015-11-06 11:53 ` Opensource [Adam Thomson] 2015-11-06 11:53 ` Opensource [Adam Thomson] 2015-11-06 11:54 ` Mark Brown 2015-11-06 11:54 ` Mark Brown 2015-11-06 13:17 ` Opensource [Adam Thomson] 2015-11-06 13:17 ` Opensource [Adam Thomson] 2015-11-08 10:34 ` Mark Brown 2015-11-08 10:34 ` Mark Brown 2015-11-09 12:28 ` Opensource [Adam Thomson] 2015-11-09 12:28 ` Opensource [Adam Thomson] 2015-11-09 14:02 ` Mark Brown 2015-11-09 14:02 ` Mark Brown 2015-11-10 13:55 ` Opensource [Adam Thomson] 2015-11-10 13:55 ` Opensource [Adam Thomson] 2015-11-10 14:15 ` Mark Brown 2015-11-10 14:24 ` Opensource [Adam Thomson] 2015-11-10 14:24 ` Opensource [Adam Thomson] 2015-11-10 15:44 ` Mark Brown 2015-11-10 16:21 ` Opensource [Adam Thomson] 2015-11-10 16:21 ` Opensource [Adam Thomson] 2015-11-10 16:42 ` Mark Brown 2015-11-10 16:42 ` Mark Brown
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=2E89032DDAA8B9408CB92943514A0337D460CA0C@SW-EX-MBX01.diasemi.com \ --to=adam.thomson.opensource@diasemi.com \ --cc=Support.Opensource@diasemi.com \ --cc=alsa-devel@alsa-project.org \ --cc=broonie@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=galak@codeaurora.org \ --cc=ijc+devicetree@hellion.org.uk \ --cc=lgirdwood@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=pawel.moll@arm.com \ --cc=perex@perex.cz \ --cc=robh+dt@kernel.org \ --cc=tiwai@suse.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.