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, 1 Dec 2015 18:17:54 -0800 From: Simran Rai <ssimran@...adcom.com> To: Mark Brown <broonie@...nel.org> CC: Rob Herring <robh+dt@...nel.org>, Pawel Moll <pawel.moll@....com>, "Mark Rutland" <mark.rutland@....com>, Ian Campbell <ijc+devicetree@...lion.org.uk>, Kumar Gala <galak@...eaurora.org>, Ray Jui <rjui@...adcom.com>, Scott Branden <sbranden@...adcom.com>, Liam Girdwood <lgirdwood@...il.com>, Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, Lori Hikichi <lhikichi@...adcom.com>, <devicetree@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>, <bcm-kernel-feedback-list@...adcom.com>, <linux-kernel@...r.kernel.org>, Arun Parameswaran <arunp@...adcom.com>, <alsa-devel@...a-project.org> Subject: Re: [PATCH v4 2/3] ASoC: cygnus: Add Cygnus audio DAI driver On 11/22/2015 5:44 AM, Mark Brown wrote: > On Mon, Nov 09, 2015 at 04:17:43PM -0800, Simran Rai wrote: > >> +#ifdef CONFIG_PM_SLEEP >> +static int cygnus_ssp_suspend(struct snd_soc_dai *cpu_dai) >> +{ >> + struct cygnus_aio_port *aio = cygnus_dai_get_portinfo(cpu_dai); >> + struct cygnus_audio *cygaud = snd_soc_dai_get_drvdata(cpu_dai); >> + >> + audio_ssp_out_disable(aio); >> + audio_ssp_in_disable(aio); >> + if (cygaud->active_ports > 0) >> + cygaud->active_ports--; >> + >> + return 0; >> +} >> + >> +static int cygnus_ssp_resume(struct snd_soc_dai *cpu_dai) >> +{ >> + return 0; >> +} > I'm a bit confused here - why do we need to disable things on suspend > but not reenable them on resume? I'd expect that the core would have > quiesced any streams that need to be suspended before we get as far as > suspending the drivers. > > Please also remove empty functions. > > Now I check I see that I'm repeating the questions I had on my previous > review: > > | Blank line between functions and remove empty functions. Though I'm not > | clear why the result doesn't undo what the suspend did... > > Please don't ignore review comments. Let me investigate this further and debug why the core did not quiescence the streams. > >> + parent = clk_get_parent(cygaud->audio_clk[0]); >> + if (IS_ERR(parent)) { >> + error = PTR_ERR(parent); >> + goto err_get_parent; >> + } >> + >> + /* Set PLL VCO Frequency (Hz) to default */ >> + error = clk_set_rate(parent, DEFAULT_VCO); >> + if (error) { >> + dev_err(&pdev->dev, >> + "%s Set PLL VCO rate failed: %d\n", __func__, error); >> + goto err_get_parent; >> + } > I would expect any initialisationn of clocks beyond the ones that the > device directly interacts with to be handled within the clock API > configuration rather than in a specific driver, this avoids the driver > being dependent on a particular system integration. I will have to figure out a way to propagate the frequency from leaf clocks to the parent clock. Will send out another patch with the fix. Thanks, Simran -- 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/
Powered by blists - more mailing lists