Message ID | 20230717172821.62827-11-andriy.shevchenko@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c923:0:b0:3e4:2afc:c1 with SMTP id j3csp1248722vqt; Mon, 17 Jul 2023 10:31:05 -0700 (PDT) X-Google-Smtp-Source: APBJJlGMMLZsuzjb6oc8AfSMyd9mTncsOmkO4dvRJQ9AeIiisui4z649L+Tb1YvBTSiOSin9dinS X-Received: by 2002:a05:6a20:510:b0:133:d15c:b661 with SMTP id 16-20020a056a20051000b00133d15cb661mr8429980pzp.60.1689615065418; Mon, 17 Jul 2023 10:31:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689615065; cv=none; d=google.com; s=arc-20160816; b=vN71xdRuzCE4pxYfGMjUEQb1UwDPoxKaJApJ5Jrb+NIwQBUC9I2jNszDOpuJhmzfVi LgTGyZogqhq+o5oCvS0vGj2FVHNjPoznxlh81G5l101mEk4qoK2V9d/tkU4W6j29pDvl /eo+/jZO5bRbzj7YMM20LDKVMAcXOkmkij8QlzwjCm+YE5RUumdat9GiOvD/NwsHHHmE 7cbl6R6suTMVb9j4sQkr5pBP+UA1m1AD6+MVI2JhXRrK3PdsDLasxNR0DgJuUvh+sxId 9zvcttclm/34fxuIZQZRkpOrEAS/8EOKyBHcph4lgO73aNC6so2fMqNSdBwL0s8oOOn2 yhdg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=igEMZaE3uKgGBJB5YGFwcMB6QbUkVIjXqgXW6Rn/KoY=; fh=BB0GY8bRtFrF5EQ7S2W7nc4ojj3WvWGP74FS8Jyyj9E=; b=ZDISz+LYTJ/K6DuhoyGt4855p6zkZ82soOQADz52VgyAX+IBK5Iq/tEtl2MSQOtwy/ YsF6zC+/gfqramkU1vSyua0p1ttR84ApwMwC+q3GUFblrrAWpM0AHCMtfWxlK50Rj5yn i4nRpYisHCHcnzHv6NsF1sBiHRdnO7yPLr3Sq+k3F9CxQWTBv6lfsgG8O0F/JY27+DDe y1ckV0XL0o1C4Nt2FIQkorx74UsaMWw4NkvazkxpfXvc3+0KvaV7JHcT9wSS2rs93fEi /S9h6KKcFU4Fz8gEtt4qMuQw47CGPpXT2WhekLWau+DxcLJtVp+XCI10qUps6IeolB5I Pixw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=WuRzze1Y; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id i3-20020a170902e48300b001aaea258805si155549ple.134.2023.07.17.10.30.51; Mon, 17 Jul 2023 10:31:05 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=WuRzze1Y; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231435AbjGQRaI (ORCPT <rfc822;hadasmailinglist@gmail.com> + 99 others); Mon, 17 Jul 2023 13:30:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45896 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231303AbjGQR3x (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 17 Jul 2023 13:29:53 -0400 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 572C41720; Mon, 17 Jul 2023 10:29:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1689614970; x=1721150970; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=B7c/2IAOq+QysQrRwOIUK5uC4eESX4PCd4P3LMdk97c=; b=WuRzze1Y+/L64c2n/xwY175f+SEXpd4tlEJNKcYs3U99ggHUtsQMhNfC Cr26z/NJLVw/LC8402ErDLEy/AKaF+mM/IULQ/amcK2OfVK4FgaG/oqz+ n4+wli124n9/35DOH8MymHBhSTVMaXlxFLq1xmaz9JXBhWPgFAA63BkY7 CIBbSR+iLM0YyQ1xac3D/LTEzDjvmq6SjX3zIY3N06o8U5pAF9d3YzA46 qOMA3Vt3KtlQUDaRuqkWI8GgI3uxPt7cpKPUn5H7kDc0t297BHpz21hVO KORRGhm+kCMVDKuvU/Lywsd7ZHb0JjmLVLU+CNehGc2jI4WJASj5IGM0s A==; X-IronPort-AV: E=McAfee;i="6600,9927,10774"; a="355927005" X-IronPort-AV: E=Sophos;i="6.01,211,1684825200"; d="scan'208";a="355927005" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Jul 2023 10:28:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10774"; a="673604218" X-IronPort-AV: E=Sophos;i="6.01,211,1684825200"; d="scan'208";a="673604218" Received: from black.fi.intel.com ([10.237.72.28]) by orsmga003.jf.intel.com with ESMTP; 17 Jul 2023 10:28:40 -0700 Received: by black.fi.intel.com (Postfix, from userid 1003) id 51B8F748; Mon, 17 Jul 2023 20:28:40 +0300 (EEST) From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Mika Westerberg <mika.westerberg@linux.intel.com>, Linus Walleij <linus.walleij@linaro.org>, Balsam CHIHI <bchihi@baylibre.com>, Claudiu Beznea <claudiu.beznea@microchip.com>, Geert Uytterhoeven <geert+renesas@glider.be>, Wolfram Sang <wsa+renesas@sang-engineering.com>, Thierry Reding <thierry.reding@gmail.com>, Paul Cercueil <paul@crapouillou.net>, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-renesas-soc@vger.kernel.org, linux-tegra@vger.kernel.org, linux-pm@vger.kernel.org Cc: Andy Shevchenko <andy@kernel.org>, Sean Wang <sean.wang@kernel.org>, Matthias Brugger <matthias.bgg@gmail.com>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, Andrew Lunn <andrew@lunn.ch>, Gregory Clement <gregory.clement@bootlin.com>, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>, Ludovic Desroches <ludovic.desroches@microchip.com>, Nicolas Ferre <nicolas.ferre@microchip.com>, Alexandre Belloni <alexandre.belloni@bootlin.com>, Jonathan Hunter <jonathanh@nvidia.com>, "Rafael J. Wysocki" <rafael@kernel.org>, Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz> Subject: [PATCH v2 10/10] pinctrl: tegra: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper Date: Mon, 17 Jul 2023 20:28:21 +0300 Message-Id: <20230717172821.62827-11-andriy.shevchenko@linux.intel.com> X-Mailer: git-send-email 2.40.0.1.gaa8946217a0b In-Reply-To: <20230717172821.62827-1-andriy.shevchenko@linux.intel.com> References: <20230717172821.62827-1-andriy.shevchenko@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1771689806644471987 X-GMAIL-MSGID: 1771689806644471987 |
Series |
pinctrl: Provide NOIRQ PM helper and use it
|
|
Commit Message
Andy Shevchenko
July 17, 2023, 5:28 p.m. UTC
Since pm.h provides a helper for system no-IRQ PM callbacks,
switch the driver to use it instead of open coded variant.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
Comments
Hi Andy, Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit : > Since pm.h provides a helper for system no-IRQ PM callbacks, > switch the driver to use it instead of open coded variant. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c > b/drivers/pinctrl/tegra/pinctrl-tegra.c > index 4547cf66d03b..734c71ef005b 100644 > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c > @@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct device > *dev) > return 0; > } > > -const struct dev_pm_ops tegra_pinctrl_pm = { > - .suspend_noirq = &tegra_pinctrl_suspend, > - .resume_noirq = &tegra_pinctrl_resume > -}; > +DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, tegra_pinctrl_suspend, > tegra_pinctrl_resume); > > static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx *pmx) > { Another driver where using EXPORT_GPL_DEV_PM_OPS() would make more sense. Cheers, -Paul
On Mon, Jul 17, 2023 at 10:14 PM Paul Cercueil <paul@crapouillou.net> wrote: > Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit : ... > Another driver where using EXPORT_GPL_DEV_PM_OPS() would make more > sense. OK. ... Thank you for the review of the series!
On Mon, Jul 17, 2023 at 09:14:12PM +0200, Paul Cercueil wrote: > Hi Andy, > > Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit : > > Since pm.h provides a helper for system no-IRQ PM callbacks, > > switch the driver to use it instead of open coded variant. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c > > b/drivers/pinctrl/tegra/pinctrl-tegra.c > > index 4547cf66d03b..734c71ef005b 100644 > > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c > > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c > > @@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct device > > *dev) > > return 0; > > } > > > > -const struct dev_pm_ops tegra_pinctrl_pm = { > > - .suspend_noirq = &tegra_pinctrl_suspend, > > - .resume_noirq = &tegra_pinctrl_resume > > -}; > > +DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, tegra_pinctrl_suspend, > > tegra_pinctrl_resume); > > > > static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx *pmx) > > { > > Another driver where using EXPORT_GPL_DEV_PM_OPS() would make more > sense. We don't currently export these PM ops because none of the Tegra pinctrl drivers can be built as a module. Thierry
On Mon, Jul 17, 2023 at 08:28:21PM +0300, Andy Shevchenko wrote: > Since pm.h provides a helper for system no-IRQ PM callbacks, > switch the driver to use it instead of open coded variant. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) Acked-by: Thierry Reding <treding@nvidia.com>
Hi Thierry, Le mardi 18 juillet 2023 à 09:45 +0200, Thierry Reding a écrit : > On Mon, Jul 17, 2023 at 09:14:12PM +0200, Paul Cercueil wrote: > > Hi Andy, > > > > Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit : > > > Since pm.h provides a helper for system no-IRQ PM callbacks, > > > switch the driver to use it instead of open coded variant. > > > > > > Signed-off-by: Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> > > > --- > > > drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +---- > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c > > > b/drivers/pinctrl/tegra/pinctrl-tegra.c > > > index 4547cf66d03b..734c71ef005b 100644 > > > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c > > > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c > > > @@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct > > > device > > > *dev) > > > return 0; > > > } > > > > > > -const struct dev_pm_ops tegra_pinctrl_pm = { > > > - .suspend_noirq = &tegra_pinctrl_suspend, > > > - .resume_noirq = &tegra_pinctrl_resume > > > -}; > > > +DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, tegra_pinctrl_suspend, > > > tegra_pinctrl_resume); > > > > > > static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx > > > *pmx) > > > { > > > > Another driver where using EXPORT_GPL_DEV_PM_OPS() would make more > > sense. > > We don't currently export these PM ops because none of the Tegra > pinctrl > drivers can be built as a module. This doesn't change anything. You'd want to use EXPORT_GPL_DEV_PM_OPS (or better, the namespaced version) so that the PM ops can be defined in one file and referenced in another, while still having them garbage- collected when CONFIG_PM is disabled. Cheers, -Paul
On Tue, Jul 18, 2023 at 11:11:43AM +0100, Jonathan Cameron wrote: > On Mon, 17 Jul 2023 20:28:21 +0300 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > Since pm.h provides a helper for system no-IRQ PM callbacks, > > switch the driver to use it instead of open coded variant. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > No pm_sleep_ptr()? pm_sleep_ptr() is pointless on this driver. This driver is selected by ARCH_TEGRA and ARCH_TEGRA also always selects PM. Thierry
On Tue, Jul 18, 2023 at 10:42:47AM +0200, Paul Cercueil wrote: > Hi Thierry, > > Le mardi 18 juillet 2023 à 09:45 +0200, Thierry Reding a écrit : > > On Mon, Jul 17, 2023 at 09:14:12PM +0200, Paul Cercueil wrote: > > > Hi Andy, > > > > > > Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit : > > > > Since pm.h provides a helper for system no-IRQ PM callbacks, > > > > switch the driver to use it instead of open coded variant. > > > > > > > > Signed-off-by: Andy Shevchenko > > > > <andriy.shevchenko@linux.intel.com> > > > > --- > > > > drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +---- > > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > b/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > index 4547cf66d03b..734c71ef005b 100644 > > > > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > @@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct > > > > device > > > > *dev) > > > > return 0; > > > > } > > > > > > > > -const struct dev_pm_ops tegra_pinctrl_pm = { > > > > - .suspend_noirq = &tegra_pinctrl_suspend, > > > > - .resume_noirq = &tegra_pinctrl_resume > > > > -}; > > > > +DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, tegra_pinctrl_suspend, > > > > tegra_pinctrl_resume); > > > > > > > > static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx > > > > *pmx) > > > > { > > > > > > Another driver where using EXPORT_GPL_DEV_PM_OPS() would make more > > > sense. > > > > We don't currently export these PM ops because none of the Tegra > > pinctrl > > drivers can be built as a module. > > This doesn't change anything. You'd want to use EXPORT_GPL_DEV_PM_OPS > (or better, the namespaced version) so that the PM ops can be defined > in one file and referenced in another, while still having them garbage- > collected when CONFIG_PM is disabled. Looking at the definition of EXPORT_GPL_DEV_PM_OPS(), it will cause an EXPORT_SYMBOL_GPL() to be added. So there very well is a change. And it's a completely bogus change because no module is ever going to use that symbol. If we were to ever support building the pinctrl driver as a module, then this would perhaps make sense, but we don't. Thierry
Le mardi 18 juillet 2023 à 13:41 +0200, Thierry Reding a écrit : > On Tue, Jul 18, 2023 at 10:42:47AM +0200, Paul Cercueil wrote: > > Hi Thierry, > > > > Le mardi 18 juillet 2023 à 09:45 +0200, Thierry Reding a écrit : > > > On Mon, Jul 17, 2023 at 09:14:12PM +0200, Paul Cercueil wrote: > > > > Hi Andy, > > > > > > > > Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a > > > > écrit : > > > > > Since pm.h provides a helper for system no-IRQ PM callbacks, > > > > > switch the driver to use it instead of open coded variant. > > > > > > > > > > Signed-off-by: Andy Shevchenko > > > > > <andriy.shevchenko@linux.intel.com> > > > > > --- > > > > > drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +---- > > > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > > > > > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > b/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > index 4547cf66d03b..734c71ef005b 100644 > > > > > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > @@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct > > > > > device > > > > > *dev) > > > > > return 0; > > > > > } > > > > > > > > > > -const struct dev_pm_ops tegra_pinctrl_pm = { > > > > > - .suspend_noirq = &tegra_pinctrl_suspend, > > > > > - .resume_noirq = &tegra_pinctrl_resume > > > > > -}; > > > > > +DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, > > > > > tegra_pinctrl_suspend, > > > > > tegra_pinctrl_resume); > > > > > > > > > > static bool tegra_pinctrl_gpio_node_has_range(struct > > > > > tegra_pmx > > > > > *pmx) > > > > > { > > > > > > > > Another driver where using EXPORT_GPL_DEV_PM_OPS() would make > > > > more > > > > sense. > > > > > > We don't currently export these PM ops because none of the Tegra > > > pinctrl > > > drivers can be built as a module. > > > > This doesn't change anything. You'd want to use > > EXPORT_GPL_DEV_PM_OPS > > (or better, the namespaced version) so that the PM ops can be > > defined > > in one file and referenced in another, while still having them > > garbage- > > collected when CONFIG_PM is disabled. > > Looking at the definition of EXPORT_GPL_DEV_PM_OPS(), it will cause > an > EXPORT_SYMBOL_GPL() to be added. So there very well is a change. And > it's a completely bogus change because no module is ever going to use > that symbol. If we were to ever support building the pinctrl driver > as > a module, then this would perhaps make sense, but we don't. In this particular case the EXPORT_SYMBOL_GPL() isn't really important, the rest of EXPORT_GPL_DEV_PM_OPS() is. I don't think having a symbol exported it is a big deal, TBH, if you use the namespaced version. If you really don't want that, we need a version of EXPORT_GPL_DEV_PM_OPS() that doesn't export the symbol. -Paul
Hi Thierry, Le mardi 18 juillet 2023 à 13:38 +0200, Thierry Reding a écrit : > On Tue, Jul 18, 2023 at 11:11:43AM +0100, Jonathan Cameron wrote: > > On Mon, 17 Jul 2023 20:28:21 +0300 > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > > > Since pm.h provides a helper for system no-IRQ PM callbacks, > > > switch the driver to use it instead of open coded variant. > > > > > > Signed-off-by: Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> > > > > No pm_sleep_ptr()? > > pm_sleep_ptr() is pointless on this driver. This driver is selected > by > ARCH_TEGRA and ARCH_TEGRA also always selects PM. If I'm not mistaken, ARCH_TEGRA selects CONFIG_PM, not CONFIG_PM_SLEEP. Cheers, -Paul
On Tue, Jul 18, 2023 at 2:55 PM Paul Cercueil <paul@crapouillou.net> wrote: > Le mardi 18 juillet 2023 à 13:41 +0200, Thierry Reding a écrit : > > On Tue, Jul 18, 2023 at 10:42:47AM +0200, Paul Cercueil wrote: ... > > Looking at the definition of EXPORT_GPL_DEV_PM_OPS(), it will cause > > an > > EXPORT_SYMBOL_GPL() to be added. So there very well is a change. And > > it's a completely bogus change because no module is ever going to use > > that symbol. If we were to ever support building the pinctrl driver > > as > > a module, then this would perhaps make sense, but we don't. > > In this particular case the EXPORT_SYMBOL_GPL() isn't really important, > the rest of EXPORT_GPL_DEV_PM_OPS() is. > > I don't think having a symbol exported it is a big deal, TBH, if you > use the namespaced version. If you really don't want that, we need a > version of EXPORT_GPL_DEV_PM_OPS() that doesn't export the symbol. Ah, I agree with Thierry and it is another point why I do not like those EXPORT*PM_OPS() macros. Polluting an exported space (even namespaced) is a big deal, so, definitely no from my side.
On Tue, Jul 18, 2023 at 11:43 AM Paul Cercueil <paul@crapouillou.net> wrote: > Le mardi 18 juillet 2023 à 09:45 +0200, Thierry Reding a écrit : ... > (or better, the namespaced version) I am all for consistency, I agree on this whenever the driver is _already_ using namespaces. Having one macro with namespace and disrupting tons of the drivers (MediaTek case?) is not an option in my opinion.
On Tue, Jul 18, 2023 at 02:01:27PM +0200, Paul Cercueil wrote: > Hi Thierry, > > Le mardi 18 juillet 2023 à 13:38 +0200, Thierry Reding a écrit : > > On Tue, Jul 18, 2023 at 11:11:43AM +0100, Jonathan Cameron wrote: > > > On Mon, 17 Jul 2023 20:28:21 +0300 > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > Since pm.h provides a helper for system no-IRQ PM callbacks, > > > > switch the driver to use it instead of open coded variant. > > > > > > > > Signed-off-by: Andy Shevchenko > > > > <andriy.shevchenko@linux.intel.com> > > > > > > No pm_sleep_ptr()? > > > > pm_sleep_ptr() is pointless on this driver. This driver is selected > > by > > ARCH_TEGRA and ARCH_TEGRA also always selects PM. > > If I'm not mistaken, ARCH_TEGRA selects CONFIG_PM, not CONFIG_PM_SLEEP. Indeed. I suppose pm_sleep_ptr() would make sense, then. Thierry
On Tue, Jul 18, 2023 at 01:55:05PM +0200, Paul Cercueil wrote: > Le mardi 18 juillet 2023 à 13:41 +0200, Thierry Reding a écrit : > > On Tue, Jul 18, 2023 at 10:42:47AM +0200, Paul Cercueil wrote: > > > Hi Thierry, > > > > > > Le mardi 18 juillet 2023 à 09:45 +0200, Thierry Reding a écrit : > > > > On Mon, Jul 17, 2023 at 09:14:12PM +0200, Paul Cercueil wrote: > > > > > Hi Andy, > > > > > > > > > > Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a > > > > > écrit : > > > > > > Since pm.h provides a helper for system no-IRQ PM callbacks, > > > > > > switch the driver to use it instead of open coded variant. > > > > > > > > > > > > Signed-off-by: Andy Shevchenko > > > > > > <andriy.shevchenko@linux.intel.com> > > > > > > --- > > > > > > drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +---- > > > > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > > > > > > > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > > b/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > > index 4547cf66d03b..734c71ef005b 100644 > > > > > > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > > @@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct > > > > > > device > > > > > > *dev) > > > > > > return 0; > > > > > > } > > > > > > > > > > > > -const struct dev_pm_ops tegra_pinctrl_pm = { > > > > > > - .suspend_noirq = &tegra_pinctrl_suspend, > > > > > > - .resume_noirq = &tegra_pinctrl_resume > > > > > > -}; > > > > > > +DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, > > > > > > tegra_pinctrl_suspend, > > > > > > tegra_pinctrl_resume); > > > > > > > > > > > > static bool tegra_pinctrl_gpio_node_has_range(struct > > > > > > tegra_pmx > > > > > > *pmx) > > > > > > { > > > > > > > > > > Another driver where using EXPORT_GPL_DEV_PM_OPS() would make > > > > > more > > > > > sense. > > > > > > > > We don't currently export these PM ops because none of the Tegra > > > > pinctrl > > > > drivers can be built as a module. > > > > > > This doesn't change anything. You'd want to use > > > EXPORT_GPL_DEV_PM_OPS > > > (or better, the namespaced version) so that the PM ops can be > > > defined > > > in one file and referenced in another, while still having them > > > garbage- > > > collected when CONFIG_PM is disabled. > > > > Looking at the definition of EXPORT_GPL_DEV_PM_OPS(), it will cause > > an > > EXPORT_SYMBOL_GPL() to be added. So there very well is a change. And > > it's a completely bogus change because no module is ever going to use > > that symbol. If we were to ever support building the pinctrl driver > > as > > a module, then this would perhaps make sense, but we don't. > > In this particular case the EXPORT_SYMBOL_GPL() isn't really important, > the rest of EXPORT_GPL_DEV_PM_OPS() is. > > I don't think having a symbol exported it is a big deal, TBH, if you > use the namespaced version. If you really don't want that, we need a > version of EXPORT_GPL_DEV_PM_OPS() that doesn't export the symbol. I do think it's a big deal to export a symbol if there's no reason to do so. And please, can we stop adding these macros for every possible scenario? Maybe I'm just getting old, but I find it increasingly difficult to understand what all of these are supposed to be. I get that people want to get rid of boilerplate, but I think we need to more carefully balance boilerplate vs. simplicity. I'm seeing the same thing with stuff like those mass conversions to atrocities like devm_platform_ioremap_resource() and devm_platform_get_and_ioremap_resource(). There's so much churn involved in getting those merged for usually saving a single line of code. And it's not even mass conversions, but people tend to send these as one patch per driver, which doesn't exactly help (except perhaps for patch statistics). Thierry
Hi Thierry, Le mardi 18 juillet 2023 à 15:20 +0200, Thierry Reding a écrit : > On Tue, Jul 18, 2023 at 01:55:05PM +0200, Paul Cercueil wrote: > > Le mardi 18 juillet 2023 à 13:41 +0200, Thierry Reding a écrit : > > > On Tue, Jul 18, 2023 at 10:42:47AM +0200, Paul Cercueil wrote: > > > > Hi Thierry, > > > > > > > > Le mardi 18 juillet 2023 à 09:45 +0200, Thierry Reding a > > > > écrit : > > > > > On Mon, Jul 17, 2023 at 09:14:12PM +0200, Paul Cercueil > > > > > wrote: > > > > > > Hi Andy, > > > > > > > > > > > > Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a > > > > > > écrit : > > > > > > > Since pm.h provides a helper for system no-IRQ PM > > > > > > > callbacks, > > > > > > > switch the driver to use it instead of open coded > > > > > > > variant. > > > > > > > > > > > > > > Signed-off-by: Andy Shevchenko > > > > > > > <andriy.shevchenko@linux.intel.com> > > > > > > > --- > > > > > > > drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +---- > > > > > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > > > b/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > > > index 4547cf66d03b..734c71ef005b 100644 > > > > > > > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > > > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c > > > > > > > @@ -747,10 +747,7 @@ static int > > > > > > > tegra_pinctrl_resume(struct > > > > > > > device > > > > > > > *dev) > > > > > > > return 0; > > > > > > > } > > > > > > > > > > > > > > -const struct dev_pm_ops tegra_pinctrl_pm = { > > > > > > > - .suspend_noirq = &tegra_pinctrl_suspend, > > > > > > > - .resume_noirq = &tegra_pinctrl_resume > > > > > > > -}; > > > > > > > +DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, > > > > > > > tegra_pinctrl_suspend, > > > > > > > tegra_pinctrl_resume); > > > > > > > > > > > > > > static bool tegra_pinctrl_gpio_node_has_range(struct > > > > > > > tegra_pmx > > > > > > > *pmx) > > > > > > > { > > > > > > > > > > > > Another driver where using EXPORT_GPL_DEV_PM_OPS() would > > > > > > make > > > > > > more > > > > > > sense. > > > > > > > > > > We don't currently export these PM ops because none of the > > > > > Tegra > > > > > pinctrl > > > > > drivers can be built as a module. > > > > > > > > This doesn't change anything. You'd want to use > > > > EXPORT_GPL_DEV_PM_OPS > > > > (or better, the namespaced version) so that the PM ops can be > > > > defined > > > > in one file and referenced in another, while still having them > > > > garbage- > > > > collected when CONFIG_PM is disabled. > > > > > > Looking at the definition of EXPORT_GPL_DEV_PM_OPS(), it will > > > cause > > > an > > > EXPORT_SYMBOL_GPL() to be added. So there very well is a change. > > > And > > > it's a completely bogus change because no module is ever going to > > > use > > > that symbol. If we were to ever support building the pinctrl > > > driver > > > as > > > a module, then this would perhaps make sense, but we don't. > > > > In this particular case the EXPORT_SYMBOL_GPL() isn't really > > important, > > the rest of EXPORT_GPL_DEV_PM_OPS() is. > > > > I don't think having a symbol exported it is a big deal, TBH, if > > you > > use the namespaced version. If you really don't want that, we need > > a > > version of EXPORT_GPL_DEV_PM_OPS() that doesn't export the symbol. > > I do think it's a big deal to export a symbol if there's no reason to > do > so. > > And please, can we stop adding these macros for every possible > scenario? Yes, as you can read from my other responses, I am not really keen on having a multiplication of these macros. > Maybe I'm just getting old, but I find it increasingly difficult to > understand what all of these are supposed to be. I get that people > want > to get rid of boilerplate, but I think we need to more carefully > balance > boilerplate vs. simplicity. The EXPORT_GPL_DEV_PM_OPS() macro does more than get rid of boilerplate, it gets rid of dead code. If we take this driver as an example, before the patch the "tegra_pinctrl_pm" struct, as well as the "tegra_pinctrl_suspend" and "tegra_pinctrl_resume" functions were always compiled in, even if CONFIG_PM_SLEEP is disabled in the config. The status-quo before the introduction of the new PM macros was to just wrap the dev_pm_ops struct + callbacks with a #ifdef CONFIG_PM_SLEEP. This was pretty bad as the code was then conditionally compiled. With the new PM macros this code is always compiled, independently of any Kconfig option; and thanks to that, bugs and regressions are subsequently easier to catch. Cheers, -Paul > I'm seeing the same thing with stuff like those mass conversions to > atrocities like devm_platform_ioremap_resource() and > devm_platform_get_and_ioremap_resource(). There's so much churn > involved > in getting those merged for usually saving a single line of code. And > it's not even mass conversions, but people tend to send these as one > patch per driver, which doesn't exactly help (except perhaps for > patch > statistics). > > Thierry
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c index 4547cf66d03b..734c71ef005b 100644 --- a/drivers/pinctrl/tegra/pinctrl-tegra.c +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c @@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct device *dev) return 0; } -const struct dev_pm_ops tegra_pinctrl_pm = { - .suspend_noirq = &tegra_pinctrl_suspend, - .resume_noirq = &tegra_pinctrl_resume -}; +DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, tegra_pinctrl_suspend, tegra_pinctrl_resume); static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx *pmx) {