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 1474EC282C4 for ; Mon, 4 Feb 2019 10:13:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DDFDA2081B for ; Mon, 4 Feb 2019 10:13:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728464AbfBDKNx (ORCPT ); Mon, 4 Feb 2019 05:13:53 -0500 Received: from mx2.suse.de ([195.135.220.15]:48186 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726928AbfBDKNx (ORCPT ); Mon, 4 Feb 2019 05:13:53 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 5099EABDC; Mon, 4 Feb 2019 10:13:50 +0000 (UTC) Date: Mon, 04 Feb 2019 11:13:49 +0100 Message-ID: From: Takashi Iwai To: Jon Hunter Cc: Sameer Pujar , "Rafael J. Wysocki" , Thierry Reding , "Rafael J. Wysocki" , Pierre-Louis Bossart , Jaroslav Kysela , "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." , , , , Linux Kernel Mailing List , , Linux PM Subject: Re: [PATCH v2] ALSA: hda/tegra: enable clock during probe In-Reply-To: <80a45aa3-8a22-959c-0c73-03ad2d6b1f8c@nvidia.com> References: <1548414418-5785-1-git-send-email-spujar@nvidia.com> <20190131143024.GO23438@ulmo> <2034694.JE9CgBysmF@aspire.rjw.lan> <80a45aa3-8a22-959c-0c73-03ad2d6b1f8c@nvidia.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 04 Feb 2019 11:04:50 +0100, Jon Hunter wrote: > > > On 04/02/2019 08:16, Sameer Pujar wrote: > > ... > > > Objective is to have things working with or without CONFIG_PM enabled. > > From previous comments and discussions it appears that there is mixed > > response > > for calling hda_tegra_runtime_resume() or runtime PM APIs in probe() > > call. Need > > to have consensus regarding the best practice to be followed, which > > would eventually > > can be used in other drivers too. > > > > Rafael is suggesting to use CONFIG_PM check to do manual setup or > > runtime PM setup in probe, > > which would bring back the earlier above mentioned concern. > > > > if (IS_ENABLED(CONFIG_PM)) { > > do setup based on pm-runtime > > } else { > >     do manual setup > > } > > Both if/else might end up doing the same here. > > Do we really need CONFIG_PM check here? > > > > Instead does below proposal appear fine? > > > > probe() { > >     hda_tegra_enable_clock(); > > } > > > > probe_work() { > >     /* hda setup */ > >     . . . > >     pm_runtime_set_active(); /* initial state as active */ > >     pm_runtime_enable(); > >     return; > > } > > I believe that this still does not work, because if there is a > power-domain that needs to be turned on, this does not guarantee this. > So I think that you need to call pm_runtime_get ... > > probe() { > if (!IS_ENABLED(CONFIG_PM)) > hda_tegra_enable_clock(); > } > > > probe_work() { > /* hda setup */ > . . . > pm_runtime_enable(); > pm_runtime_get_sync(); > return; > } > > The alternative here could just be ... > > probe() { > pm_runtime_enable(); > if (!pm_runtime_enabled()) > ret = hda_tegra_enable_clock(); > else > ret = pm_runtime_get_sync(); > > if (ret < 0) { > ... > } > } > > Very similar to what I was saying to begin with but not call the > pm_runtime_resume handler directly. Which I believe was Iwai-san's dislike. Yes, exactly, what bothered me was really a nuance: calling hda_tegra_runtime_resume() there makes the code misleading (or confusing) as if the runtime PM were mandatory. I hoped there could be some standard idiom for this expression, but apparently there isn't any, so far... Obviously the easiest option is to enforce the dependency on CONFIG_PM. Would there be any platform that needs to run without PM, practically seen...? But, now after lengthy discussions and the clarification of the current situation, I have no strong opinion on this any longer. So I leave the decision to you guys, and I'll apply it as-is. thanks, Takashi