Message ID | 20230420121615.967487-1-d-gole@ti.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp296149vqo; Thu, 20 Apr 2023 05:30:27 -0700 (PDT) X-Google-Smtp-Source: AKy350btTzgb2nA5s0w+c1F5QO5yTUWeoRUMbEoKMmEMWon3bsuv1Ozx0rCD7DCCL/Y0+ZaMG6if X-Received: by 2002:a17:90a:400f:b0:247:761d:a6af with SMTP id u15-20020a17090a400f00b00247761da6afmr1470139pjc.17.1681993827466; Thu, 20 Apr 2023 05:30:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1681993827; cv=none; d=google.com; s=arc-20160816; b=NCEE53NH/Qu1tStAc6L04T+jF3tBZ9nErRX584qk5bb3soGfUq94B9G0Nv+cNzZQwZ Luilo8/AIck2HeXsc2vzkDLKYe47lFwMU4aaoBZYDSfPwufqfLV147X6uPevQEyA7+qk zAgi6DZsEFQIeloinYpaCc+0ChVFBNA8P+H+U/qO1xgMjvRRU+QlaeIZMmTr22i/HWTk Ow6paoOvH7fmPE4L9tpuYFbRpVmZehKY5fW7eCNpWELKuqCOI98UmZF6MJJY4ixnrZ1a HL88uH3Tj0ecfnfdQEtFH9F9i6y9ZpLADqMBSgrc1P1dW7gkCPjKz1RNReU+AzCMXPd/ nnaA== 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 :message-id:date:subject:cc:to:from:dkim-signature; bh=fb0VdJBs/6V8Ibz0yc1qYJ+5jAg/m7Cy7/Q+zts3yyo=; b=QixnJjzWh4PGGgULYKlnDkImIkjJ6DiPWlyb79RCsk9KIWLYZztEbhN9gyA7e4RvT+ bemMPQVPh7jxW5azhT13zzhlbgI14wE35n+uAkEM+c9e35MOzVzlTfqsweWW/YiaI5V+ /E7eUwfzzTcv8D+qBBk+vpfkH6ShkpMTejInJoIG3VXyIEIUlOq00cxAVIm3yaoYVNfO 6VQ8Y44QnkIVGvz6kkhtY/upbjHZ8LTvQQmodpJfeNtp3GS/iXa07u2JMF4fCq+Yo/4k qNTdkt/KOQTvOreCV7s9Tz03BTVYAS/XPZ/iMVrocD8PyltzHmVKnx7DdMfITc6sek79 FIbQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ti.com header.s=ti-com-17Q1 header.b=TRqImYGX; 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=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id e7-20020a17090a9a8700b0024981e98175si1789379pjp.75.2023.04.20.05.30.09; Thu, 20 Apr 2023 05:30:27 -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=@ti.com header.s=ti-com-17Q1 header.b=TRqImYGX; 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=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234727AbjDTMQj (ORCPT <rfc822;cjcooper78@gmail.com> + 99 others); Thu, 20 Apr 2023 08:16:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40246 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234127AbjDTMQh (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 20 Apr 2023 08:16:37 -0400 Received: from fllv0016.ext.ti.com (fllv0016.ext.ti.com [198.47.19.142]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2EE4710B; Thu, 20 Apr 2023 05:16:36 -0700 (PDT) Received: from lelv0265.itg.ti.com ([10.180.67.224]) by fllv0016.ext.ti.com (8.15.2/8.15.2) with ESMTP id 33KCGPol015336; Thu, 20 Apr 2023 07:16:25 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1681992985; bh=fb0VdJBs/6V8Ibz0yc1qYJ+5jAg/m7Cy7/Q+zts3yyo=; h=From:To:CC:Subject:Date; b=TRqImYGXcpDAO8mQzMWSzD9OZx2uPBhGoHDieUJMx+9znZFksF9vXhLRunodUFW9V Btbn0A7T/ZusPnjmFUiknNjFin9hDxkw3f4SfOpTle1j7OMdVXh0XRMZtdkHocbt1a iwHmNpdkVj61MqLnGS/7cUZUtrFS20B0BfE2EUxY= Received: from DFLE112.ent.ti.com (dfle112.ent.ti.com [10.64.6.33]) by lelv0265.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 33KCGP7v008010 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 20 Apr 2023 07:16:25 -0500 Received: from DFLE103.ent.ti.com (10.64.6.24) by DFLE112.ent.ti.com (10.64.6.33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.16; Thu, 20 Apr 2023 07:16:25 -0500 Received: from fllv0040.itg.ti.com (10.64.41.20) by DFLE103.ent.ti.com (10.64.6.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.16 via Frontend Transport; Thu, 20 Apr 2023 07:16:25 -0500 Received: from localhost (ileaxei01-snat2.itg.ti.com [10.180.69.6]) by fllv0040.itg.ti.com (8.15.2/8.15.2) with ESMTP id 33KCGO9q089893; Thu, 20 Apr 2023 07:16:24 -0500 From: Dhruva Gole <d-gole@ti.com> To: Mark Brown <broonie@kernel.org> CC: Dhruva Gole <d-gole@ti.com>, Vaishnav Achath <vaishnav.a@ti.com>, Vignesh <vigneshr@ti.com>, Apurva Nandan <a-nandan@ti.com>, <linux-arm-kernel@lists.infradead.org>, <linux-spi@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Grant Likely <grant.likely@secretlab.ca>, Tanguy Bouzeloc <tanguy.bouzeloc@efixo.com> Subject: [PATCH] spi: bcm63xx: remove PM_SLEEP based conditional compilation Date: Thu, 20 Apr 2023 17:46:15 +0530 Message-ID: <20230420121615.967487-1-d-gole@ti.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, SPF_HELO_PASS,SPF_PASS,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: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1763698359578765939?= X-GMAIL-MSGID: =?utf-8?q?1763698359578765939?= |
Series |
spi: bcm63xx: remove PM_SLEEP based conditional compilation
|
|
Commit Message
Dhruva Gole
April 20, 2023, 12:16 p.m. UTC
Get rid of conditional compilation based on CONFIG_PM_SLEEP because
it may introduce build issues with certain configs where it maybe disabled
This is because if above config is not enabled the suspend-resume
functions are never part of the code but the bcm63xx_spi_pm_ops struct
still inits them to non-existent suspend-resume functions.
Fixes: b42dfed83d95 ("spi: add Broadcom BCM63xx SPI controller driver")
Signed-off-by: Dhruva Gole <d-gole@ti.com>
---
drivers/spi/spi-bcm63xx.c | 2 --
1 file changed, 2 deletions(-)
Comments
On Thu, 20 Apr 2023 17:46:15 +0530, Dhruva Gole wrote: > Get rid of conditional compilation based on CONFIG_PM_SLEEP because > it may introduce build issues with certain configs where it maybe disabled > This is because if above config is not enabled the suspend-resume > functions are never part of the code but the bcm63xx_spi_pm_ops struct > still inits them to non-existent suspend-resume functions. > > Fixes: b42dfed83d95 ("spi: add Broadcom BCM63xx SPI controller driver") > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: bcm63xx: remove PM_SLEEP based conditional compilation commit: 25f0617109496e1aff49594fbae5644286447a0f All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
On 4/20/23 05:16, Dhruva Gole wrote: > Get rid of conditional compilation based on CONFIG_PM_SLEEP because > it may introduce build issues with certain configs where it maybe disabled > This is because if above config is not enabled the suspend-resume > functions are never part of the code but the bcm63xx_spi_pm_ops struct > still inits them to non-existent suspend-resume functions. > > Fixes: b42dfed83d95 ("spi: add Broadcom BCM63xx SPI controller driver") > > Signed-off-by: Dhruva Gole <d-gole@ti.com> > --- > drivers/spi/spi-bcm63xx.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c > index 96633a0051b1..99395932074c 100644 > --- a/drivers/spi/spi-bcm63xx.c > +++ b/drivers/spi/spi-bcm63xx.c > @@ -617,7 +617,6 @@ static void bcm63xx_spi_remove(struct platform_device *pdev) > clk_disable_unprepare(bs->clk); > } > > -#ifdef CONFIG_PM_SLEEP > static int bcm63xx_spi_suspend(struct device *dev) Don't we need a __maybe_unused here?
On Fri, 21 Apr 2023 at 19:17, Florian Fainelli <f.fainelli@gmail.com> wrote: > > On 4/20/23 05:16, Dhruva Gole wrote: > > Get rid of conditional compilation based on CONFIG_PM_SLEEP because > > it may introduce build issues with certain configs where it maybe disabled > > This is because if above config is not enabled the suspend-resume > > functions are never part of the code but the bcm63xx_spi_pm_ops struct > > still inits them to non-existent suspend-resume functions. > > > > Fixes: b42dfed83d95 ("spi: add Broadcom BCM63xx SPI controller driver") > > > > Signed-off-by: Dhruva Gole <d-gole@ti.com> > > --- > > drivers/spi/spi-bcm63xx.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c > > index 96633a0051b1..99395932074c 100644 > > --- a/drivers/spi/spi-bcm63xx.c > > +++ b/drivers/spi/spi-bcm63xx.c > > @@ -617,7 +617,6 @@ static void bcm63xx_spi_remove(struct platform_device *pdev) > > clk_disable_unprepare(bs->clk); > > } > > > > -#ifdef CONFIG_PM_SLEEP > > static int bcm63xx_spi_suspend(struct device *dev) > > Don't we need a __maybe_unused here? Actually the premise of this patch is wrong, and should rather be reverted. The bcm63xx_spi_pm_ops struct is initialized with SET_SYSTEM_SLEEP_PM_OPS(), which is defined as #ifdef CONFIG_PM_SLEEP #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) #else #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) #endif so for !CONFIG_PM_SLEEP it won't initialize the struct at all (or reference non-existing functions), and therefore there will be no build issues. Regards, Jonas
Hi Jonas, On 24/04/23 13:50, Jonas Gorski wrote: > On Fri, 21 Apr 2023 at 19:17, Florian Fainelli <f.fainelli@gmail.com> wrote: >> >> On 4/20/23 05:16, Dhruva Gole wrote: >>> Get rid of conditional compilation based on CONFIG_PM_SLEEP because >>> it may introduce build issues with certain configs where it maybe disabled >>> This is because if above config is not enabled the suspend-resume >>> functions are never part of the code but the bcm63xx_spi_pm_ops struct >>> still inits them to non-existent suspend-resume functions. >>> >>> Fixes: b42dfed83d95 ("spi: add Broadcom BCM63xx SPI controller driver") >>> >>> Signed-off-by: Dhruva Gole <d-gole@ti.com> >>> --- >>> drivers/spi/spi-bcm63xx.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c >>> index 96633a0051b1..99395932074c 100644 >>> --- a/drivers/spi/spi-bcm63xx.c >>> +++ b/drivers/spi/spi-bcm63xx.c >>> @@ -617,7 +617,6 @@ static void bcm63xx_spi_remove(struct platform_device *pdev) >>> clk_disable_unprepare(bs->clk); >>> } >>> >>> -#ifdef CONFIG_PM_SLEEP >>> static int bcm63xx_spi_suspend(struct device *dev) >> >> Don't we need a __maybe_unused here? > > Actually the premise of this patch is wrong, and should rather be reverted. > > The bcm63xx_spi_pm_ops struct is initialized with > SET_SYSTEM_SLEEP_PM_OPS(), which is defined as > > #ifdef CONFIG_PM_SLEEP > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ > SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) > #else > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) > #endif > Thanks for pointing this out, I must have missed this. Anyway, I have sent another patch to migrate to using DEFINE_SIMPLE_DEV_PM_OPS as per Mark's suggestion [0]. There I think it would be necessary to remove the CONFIG_PM_SLEEP checks in the driver. So no need to revert this patch. > so for !CONFIG_PM_SLEEP it won't initialize the struct at all (or > reference non-existing functions), and therefore there will be no > build issues. > > Regards, > Jonas [0] https://lore.kernel.org/all/e65683c1-9f1b-4cfb-8e14-087ef7d69595@sirena.org.uk/
On Thu, Apr 20, 2023 at 05:46:15PM +0530, Dhruva Gole wrote: > Get rid of conditional compilation based on CONFIG_PM_SLEEP because > it may introduce build issues with certain configs where it maybe disabled > This is because if above config is not enabled the suspend-resume > functions are never part of the code but the bcm63xx_spi_pm_ops struct > still inits them to non-existent suspend-resume functions. > > Fixes: b42dfed83d95 ("spi: add Broadcom BCM63xx SPI controller driver") > > Signed-off-by: Dhruva Gole <d-gole@ti.com> This patch results in: drivers/spi/spi-bcm63xx.c:632:12: error: 'bcm63xx_spi_resume' defined but not used [-Werror=unused-function] 632 | static int bcm63xx_spi_resume(struct device *dev) | ^~~~~~~~~~~~~~~~~~ drivers/spi/spi-bcm63xx.c:620:12: error: 'bcm63xx_spi_suspend' defined but not used [-Werror=unused-function] 620 | static int bcm63xx_spi_suspend(struct device *dev) on architectures with no PM support (alpha, csky, m68k, openrisc, parisc, riscv, s390). Guenter
Hi, On 4/25/2023 8:38 PM, Guenter Roeck wrote: > On Thu, Apr 20, 2023 at 05:46:15PM +0530, Dhruva Gole wrote: >> Get rid of conditional compilation based on CONFIG_PM_SLEEP because >> it may introduce build issues with certain configs where it maybe disabled >> This is because if above config is not enabled the suspend-resume >> functions are never part of the code but the bcm63xx_spi_pm_ops struct >> still inits them to non-existent suspend-resume functions. >> >> Fixes: b42dfed83d95 ("spi: add Broadcom BCM63xx SPI controller driver") >> >> Signed-off-by: Dhruva Gole <d-gole@ti.com> > This patch results in: > > drivers/spi/spi-bcm63xx.c:632:12: error: 'bcm63xx_spi_resume' defined but not used [-Werror=unused-function] > 632 | static int bcm63xx_spi_resume(struct device *dev) > | ^~~~~~~~~~~~~~~~~~ > drivers/spi/spi-bcm63xx.c:620:12: error: 'bcm63xx_spi_suspend' defined but not used [-Werror=unused-function] > 620 | static int bcm63xx_spi_suspend(struct device *dev) > > on architectures with no PM support (alpha, csky, m68k, openrisc, parisc, > riscv, s390). Thanks for pointing this out. I could send a patch to address this as pointed here [0] by adding a __maybe_unused. However, do you think my other patch [1] could address this issue without the need for above? I think it would because DEFINE_SIMPLE_DEV_PM_OPS doesn't seem to be under any conditional CONFIG_PM. However, I may have missed something, please do let me know what's the best way to fix. [0] https://lore.kernel.org/all/24ec3728-9720-ae6a-9ff5-3e2e13a96f76@gmail.com/ [1] https://lore.kernel.org/all/20230424102546.1604484-1-d-gole@ti.com/ > > Guenter
On 4/25/23 10:18, Gole, Dhruva wrote: > Hi, > > On 4/25/2023 8:38 PM, Guenter Roeck wrote: >> On Thu, Apr 20, 2023 at 05:46:15PM +0530, Dhruva Gole wrote: >>> Get rid of conditional compilation based on CONFIG_PM_SLEEP because >>> it may introduce build issues with certain configs where it maybe disabled >>> This is because if above config is not enabled the suspend-resume >>> functions are never part of the code but the bcm63xx_spi_pm_ops struct >>> still inits them to non-existent suspend-resume functions. >>> >>> Fixes: b42dfed83d95 ("spi: add Broadcom BCM63xx SPI controller driver") >>> >>> Signed-off-by: Dhruva Gole <d-gole@ti.com> >> This patch results in: >> >> drivers/spi/spi-bcm63xx.c:632:12: error: 'bcm63xx_spi_resume' defined but not used [-Werror=unused-function] >> 632 | static int bcm63xx_spi_resume(struct device *dev) >> | ^~~~~~~~~~~~~~~~~~ >> drivers/spi/spi-bcm63xx.c:620:12: error: 'bcm63xx_spi_suspend' defined but not used [-Werror=unused-function] >> 620 | static int bcm63xx_spi_suspend(struct device *dev) >> >> on architectures with no PM support (alpha, csky, m68k, openrisc, parisc, >> riscv, s390). > > Thanks for pointing this out. > > I could send a patch to address this as pointed here [0] > > by adding a __maybe_unused. > > However, do you think my other patch [1] could address this issue without the need for above? > Personally I would go for [0] as the least invasive solution, but I really have no idea, sorry. I just hope that your (broken) patch doesn't make it as-is into the upstream kernel. Guenter > I think it would because DEFINE_SIMPLE_DEV_PM_OPS doesn't seem to be under any conditional CONFIG_PM. > > However, I may have missed something, please do let me know what's the best way to fix. > > [0] https://lore.kernel.org/all/24ec3728-9720-ae6a-9ff5-3e2e13a96f76@gmail.com/ > > [1] https://lore.kernel.org/all/20230424102546.1604484-1-d-gole@ti.com/ > >> >> Guenter >
On Tue, Apr 25, 2023 at 10:37:24AM -0700, Guenter Roeck wrote: > Personally I would go for [0] as the least invasive solution, but I really > have no idea, sorry. I just hope that your (broken) patch doesn't make it > as-is into the upstream kernel. I've applied the SIMPLE_DEV_PM_OPS patch which seems to fix the issue for riscv.
diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c index 96633a0051b1..99395932074c 100644 --- a/drivers/spi/spi-bcm63xx.c +++ b/drivers/spi/spi-bcm63xx.c @@ -617,7 +617,6 @@ static void bcm63xx_spi_remove(struct platform_device *pdev) clk_disable_unprepare(bs->clk); } -#ifdef CONFIG_PM_SLEEP static int bcm63xx_spi_suspend(struct device *dev) { struct spi_master *master = dev_get_drvdata(dev); @@ -644,7 +643,6 @@ static int bcm63xx_spi_resume(struct device *dev) return 0; } -#endif static const struct dev_pm_ops bcm63xx_spi_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(bcm63xx_spi_suspend, bcm63xx_spi_resume)