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
| ||
|
Date: Tue, 8 Sep 2020 15:59:40 -0500 From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com> To: Radoslaw Biernacki <rad@...ihalf.com>, Liam Girdwood <liam.r.girdwood@...ux.intel.com>, Jie Yang <yang.jie@...ux.intel.com>, Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com> Cc: Lech Betlej <Lech.Betlej@...el.com>, alsa-devel@...a-project.org, Todd Broch <tbroch@...gle.com>, Harshapriya <harshapriya.n@...el.com>, John Hsu <KCHSU0@...oton.com>, linux-kernel@...r.kernel.org, michal.sienkiewicz@...el.com, Ben Zhang <benzh@...omium.org>, Mac Chiang <mac.chiang@...el.com>, Yong Zhi <yong.zhi@...el.com>, Marcin Wojtas <mw@...ihalf.com>, Vamshi Krishna <vamshi.krishna.gopal@...el.com>, Alex Levin <levinale@...gle.com> Subject: Re: [PATCH V3] ASoC: Intel: boards: Use FS as nau8825 sysclk in nau88125_* machine Sorry, I couldn't resist adding three more comments to improve further: > -static int skylake_nau8825_hw_params(struct snd_pcm_substream *substream, > - struct snd_pcm_hw_params *params) > +static int skylake_nau8825_trigger(struct snd_pcm_substream *substream, int cmd) > { > struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct snd_pcm_runtime *runtime = substream->runtime; > struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0); > - int ret; > - > - ret = snd_soc_dai_set_sysclk(codec_dai, > - NAU8825_CLK_MCLK, 24000000, SND_SOC_CLOCK_IN); > + int ret = 0; > > - if (ret < 0) > - dev_err(rtd->dev, "snd_soc_dai_set_sysclk err = %d\n", ret); > + switch (cmd) { > + case SNDRV_PCM_TRIGGER_START: > + ret = snd_soc_dai_set_sysclk(codec_dai, NAU8825_CLK_FLL_FS, 0, > + SND_SOC_CLOCK_IN); Maybe a simple comment to explain what this does? > + if (ret < 0) { > + dev_err(codec_dai->dev, "can't set FS clock %d\n", ret); > + break; > + } > + ret = snd_soc_dai_set_pll(codec_dai, 0, 0, runtime->rate, > + runtime->rate * 256); > + if (ret < 0) > + dev_err(codec_dai->dev, "can't set FLL: %d\n", ret); > + break; You could replace this by a /* fallthrough */ statement? > + case SNDRV_PCM_TRIGGER_RESUME: > + ret = snd_soc_dai_set_pll(codec_dai, 0, 0, runtime->rate, > + runtime->rate * 256); > + if (ret < 0) > + dev_err(codec_dai->dev, "can't set FLL: %d\n", ret); > + break; > + } > +static int __maybe_unused skylake_nau8825_resume_post(struct snd_soc_card *card) > +{ > + struct snd_soc_dai *codec_dai; > + > + codec_dai = snd_soc_card_get_codec_dai(card, SKL_NUVOTON_CODEC_DAI); > + if (!codec_dai) { > + dev_err(card->dev, "Codec dai not found\n"); > + return -EIO; > + } > + > + dev_dbg(codec_dai->dev, "playback_active:%d playback_widget->active:%d codec_dai->rate:%d\n", > + codec_dai->stream_active[SNDRV_PCM_STREAM_PLAYBACK], > + codec_dai->playback_widget->active, > + codec_dai->rate); > + > + if (codec_dai->stream_active[SNDRV_PCM_STREAM_PLAYBACK] && > + codec_dai->playback_widget->active) > + snd_soc_dai_set_sysclk(codec_dai, NAU8825_CLK_FLL_FS, 0, > + SND_SOC_CLOCK_IN); And that part is also worthy of a comment, e.g. why not do this as part of the TRIGGER_RESUME and why only for playback? > --- a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c > +++ b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c same comments for this other machine driver.
Powered by blists - more mailing lists