From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C22E6C433EF for ; Fri, 15 Oct 2021 09:34:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 97C5561151 for ; Fri, 15 Oct 2021 09:34:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237425AbhJOJgy (ORCPT ); Fri, 15 Oct 2021 05:36:54 -0400 Received: from muru.com ([72.249.23.125]:44834 "EHLO muru.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236690AbhJOJgy (ORCPT ); Fri, 15 Oct 2021 05:36:54 -0400 Received: from localhost (localhost [127.0.0.1]) by muru.com (Postfix) with ESMTPS id D6ECF80F1; Fri, 15 Oct 2021 09:35:19 +0000 (UTC) Date: Fri, 15 Oct 2021 12:34:46 +0300 From: Tony Lindgren To: Ulf Hansson Cc: Adrian Hunter , Chunyan Zhang , Faiz Abbas , Kishon Vijay Abraham I , Santosh Shilimkar , linux-mmc , linux-omap , Rob Herring , DTML Subject: Re: [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions Message-ID: References: <20211012103750.38328-1-tony@atomide.com> <20211012103750.38328-5-tony@atomide.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org * Ulf Hansson [211012 15:17]: > On Tue, 12 Oct 2021 at 12:38, Tony Lindgren wrote: > > @@ -1350,15 +1355,21 @@ static int sdhci_omap_probe(struct platform_device *pdev) > > if (ret) > > goto err_cleanup_host; > > > > + sdhci_omap_context_save(omap_host); > > Calling sdhci_omap_context_save() here looks unnecessary. The device > is already runtime resumed at this point. > > In other words, sdhci_omap_context_save() will be called from the > ->runtime_suspend() callback, next time the device becomes runtime > suspended. That should be sufficient, right? Yup this can be now dropped with omap_host->con initialized to -EINVAL earlier. > > @@ -1371,8 +1382,12 @@ static int sdhci_omap_remove(struct platform_device *pdev) > > struct device *dev = &pdev->dev; > > struct sdhci_host *host = platform_get_drvdata(pdev); > > > > + pm_runtime_get_sync(dev); > > sdhci_remove_host(host, true); > > + pm_runtime_dont_use_autosuspend(dev); > > pm_runtime_put_sync(dev); > > + /* Ensure device gets idled despite userspace sysfs config */ > > + pm_runtime_force_suspend(dev); > > pm_runtime_disable(dev); > > The call to pm_runtime_disable() can be removed, as that is taken care > of in pm_runtime_force_suspend(). OK > > +static int __maybe_unused sdhci_omap_suspend(struct device *dev) > > +{ > > + struct sdhci_host *host = dev_get_drvdata(dev); > > + int err; > > + > > + /* Enable for configuring wakeups, paired in resume */ > > + err = pm_runtime_resume_and_get(dev); > > + if (err < 0) > > + return err; > > + > > + sdhci_suspend_host(host); > > As far as I can tell, sdhci_suspend_host() doesn't really make sense > for the omap variant. What you need, is to put the device into the > same low power state as "runtime suspend", that should be sufficient. > > The system wakeup will be armed (and later then disarmed) by the PM > core, when it calls device_wakeup_arm_wake_irqs() from the > dpm_suspend_noirq() phase. > > In other words, pointing the system suspend/resume callbacks to > pm_runtime_force_suspend|resume() should work fine, I think. OK sounds good to me. Regards, Tony