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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 7 Oct 2017 11:25:12 +0100 From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org> To: Jonathan Neuschäfer <j.neuschaefer@....net> Cc: gregkh@...uxfoundation.org, broonie@...nel.org, alsa-devel@...a-project.org, sdharia@...eaurora.org, bp@...e.de, poeschel@...onage.de, treding@...dia.com, gong.chen@...ux.intel.com, andreas.noever@...il.com, alan@...ux.intel.com, mathieu.poirier@...aro.org, daniel@...ll.ch, jkosina@...e.cz, sharon.dvir1@...l.huji.ac.il, joe@...ches.com, davem@...emloft.net, james.hogan@...tec.com, michael.opdenacker@...e-electrons.com, robh+dt@...nel.org, pawel.moll@....com, mark.rutland@....com, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, kheitke@...ience.com, linux-arm-msm@...r.kernel.org, arnd@...db.de Subject: Re: [Patch v6 5/7] slimbus: qcom: Add runtime-pm support using clock-pause feature Thanks for the review comments On 07/10/17 09:22, Jonathan Neuschäfer wrote: > Hi, some more trivial comments below. > > On Fri, Oct 06, 2017 at 05:51:34PM +0200, srinivas.kandagatla@...aro.org wrote: >> From: Sagar Dharia <sdharia@...eaurora.org> >> >> Slimbus HW mandates that clock-pause sequence has to be executed >> before disabling relevant interface and core clocks. >> Runtime-PM's autosuspend feature is used here to enter/exit low >> power mode for Qualcomm's Slimbus controller. Autosuspend feature >> enables driver to avoid changing power-modes too frequently since >> entering clock-pause is an expensive sequence >> >> Signed-off-by: Sagar Dharia <sdharia@...eaurora.org> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org> >> --- > [...] >> +static int msm_clk_pause_wakeup(struct slim_controller *ctrl) >> +{ >> + struct msm_slim_ctrl *dev = slim_get_ctrldata(ctrl); >> + >> + clk_prepare_enable(dev->hclk); >> + clk_prepare_enable(dev->rclk); >> + enable_irq(dev->irq); >> + >> + writel_relaxed(1, dev->base + FRM_WAKEUP); >> + /* Make sure framer wakeup write goes through before ISR fires */ >> + mb(); >> + /** > > This isn't really a kerneldoc comment. Yep I agree will fix all such instances. > >> + * HW Workaround: Currently, slave is reporting lost-sync messages >> + * after slimbus comes out of clock pause. >> + * Transaction with slave fail before slave reports that message >> + * Give some time for that report to come >> + * Slimbus wakes up in clock gear 10 at 24.576MHz. With each superframe >> + * being 250 usecs, we wait for 5-10 superframes here to ensure >> + * we get the message >> + */ >> + usleep_range(1250, 2500); >> + return 0; >> +} > [...] >> +#ifdef CONFIG_PM_SLEEP >> +static int msm_slim_suspend(struct device *dev) >> +{ >> + int ret = 0; >> + >> + if (!pm_runtime_enabled(dev) || >> + (!pm_runtime_suspended(dev))) { >> + dev_dbg(dev, "system suspend"); >> + ret = msm_slim_runtime_suspend(dev); >> + } >> + if (ret == -EISCONN) { >> + /** > > ditto. > Also, it looks misindented. Yep. will fix this too. > >> + * If the clock pause failed due to active channels, there is >> + * a possibility that some audio stream is active during suspend. >> + * (e.g. modem usecase during suspend) >> + * We dont want to return suspend failure in that case so that >> + * display and relevant components can still go to suspend. >> + * If there is some other error, then it should prevent >> + * system level suspend >> + */ >> + ret = 0; >> + } >> + return ret; >> +} > > > Thanks, > Jonathan Neuschäfer >
Powered by blists - more mailing lists