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 X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5B61BC282CB for ; Tue, 5 Feb 2019 11:53:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 333362083B for ; Tue, 5 Feb 2019 11:53:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728796AbfBELxa (ORCPT ); Tue, 5 Feb 2019 06:53:30 -0500 Received: from cloudserver094114.home.pl ([79.96.170.134]:48140 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727496AbfBELxa (ORCPT ); Tue, 5 Feb 2019 06:53:30 -0500 Received: from 79.184.254.36.ipv4.supernova.orange.pl (79.184.254.36) (HELO aspire.rjw.lan) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.183) id 5e38e35669498778; Tue, 5 Feb 2019 12:53:28 +0100 From: "Rafael J. Wysocki" To: Thierry Reding Cc: Jon Hunter , "Rafael J. Wysocki" , Takashi Iwai , Pierre-Louis Bossart , Sameer Pujar , Jaroslav Kysela , "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." , mkumard@nvidia.com, rlokhande@nvidia.com, sharadg@nvidia.com, Linux Kernel Mailing List , linux-tegra@vger.kernel.org, Linux PM Subject: Re: [PATCH v2] ALSA: hda/tegra: enable clock during probe Date: Tue, 05 Feb 2019 12:52:17 +0100 Message-ID: <2224002.YA0vJHJrjz@aspire.rjw.lan> In-Reply-To: <20190204110510.GA10412@ulmo> References: <1548414418-5785-1-git-send-email-spujar@nvidia.com> <07d58962-4c49-0575-2f95-4c885998bb52@nvidia.com> <20190204110510.GA10412@ulmo> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday, February 4, 2019 12:05:10 PM CET Thierry Reding wrote: > > --FCuugMFkClbJLl1L > Content-Type: text/plain; charset=us-ascii > Content-Disposition: inline > Content-Transfer-Encoding: quoted-printable > > On Mon, Feb 04, 2019 at 09:53:32AM +0000, Jon Hunter wrote: > >=20 > >=20 > > On 04/02/2019 08:45, Thierry Reding wrote: > >=20 > > ... > >=20 > > > The idea was, as I was saying below, to reuse dev_pm_ops even if > > > !CONFIG_PM. So pm_runtime_enable() could be something like this: > > >=20 > > > pm_runtime_enable(dev) > > > { > > > if (!CONFIG_PM) > > > if (dev->pm_ops->resume) > > > dev->pm_ops->resume(dev); > > >=20 > > > ... > > > } > > >=20 > > > But that's admittedly somewhat of a stretch. This could of course be > > > made somewhat nicer by adding an explicit variant, say: > > >=20 > > > pm_runtime_enable_foo(dev) > > > { > > > if (!CONFIG_PM && dev->pm_ops->resume) > > > return dev->pm_ops->resume(dev); > > >=20 > > > return 0; > > > } > > >=20 > > > Maybe the fact that I couldn't come up with a good name is a good > > > indication that this is a bad idea... > >=20 > > How about some new APIs called ... > >=20 > > pm_runtime_enable_get() > > pm_runtime_enable_get_sync() > > pm_runtime_put_disable() (implies a put_sync) > >=20 > > ... and in these APIs we add ... > >=20 > > pm_runtime_enable_get(dev) > > { > > if (!CONFIG_PM && dev->pm_ops->resume) > > return dev->pm_ops->resume(dev); No, we're not adding this to the core. While in principle you could provide a set of pointers to the routines you want to be called when CONFIG_PM is unset, IMO it is just cleaner and more straightforward (and fewer lines of code for that matter) to add a simple conditional to each ->probe() and ->remove(). [cut] > > None, but seems overkill just for this case. > > But that's precisely the point. It's not just about this case. We've > already got some drivers where we do a similar dance just to be able to > support the, let's admit it, exotic case where somebody turns off PM. I > think supporting !PM might have made sense at a point where we had no > support for power management at all. But I think we've come a long way > since then, and while we may still have some ways to go in some areas, > we do fairly decent runtime PM most of the time, to the point where I no > longer see any benefits in !PM. This IMO is an excellent point. Trying to handle the !CONFIG_PM case only really makes sense if you know what to do to turn the device on without relying on things like PM domains etc which are not even available if CONFIG_PM is not set. IOW, if any of the platforms your driver is expected to work with require something like genpd to even make the device functional, I wouldn't bother. Thanks, Rafael