Message ID | 20221215181848.129326-1-helgaas@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:e747:0:0:0:0:0 with SMTP id c7csp528903wrn; Thu, 15 Dec 2022 10:24:34 -0800 (PST) X-Google-Smtp-Source: AA0mqf74LVARcWlj8rDZCuKRvV4W90jurkzLlOmrJsYxK2X2GSNB9MGG/Xq4g3PM4LKjAmsK20dy X-Received: by 2002:a17:90a:194b:b0:220:bbe1:1fde with SMTP id 11-20020a17090a194b00b00220bbe11fdemr23402934pjh.14.1671128674423; Thu, 15 Dec 2022 10:24:34 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1671128674; cv=none; d=google.com; s=arc-20160816; b=HimIydKq00y9ab+kDyGVTi8zeDyCpduPfAKIXFjASSnYttVHkPUDIpZhNPo7aBEETZ N/pjPuZkUMmKiPFpN5ysGqo5obw2It8Vbre7Q1FzGrE3Z14K6CaNoqMfLDb8DtowFnQK D8WvvFJVyLMyjWt31wA2+9qJwlxWbS79hKGLXG2X5UVkwVGwpMsBP0YPmHPcBJAJErXe DfPqaCP1N5v466lb5DwhsNP8j6XtcB51RULDrpbpRRq+mP4YZO8FYXIv/WOorayRRqwc yp5JXH+4dmm8pugXA/DU93MTNig5a4+/GQN7o9E6ooqkdoQyx3oja3fB1RiU0dVGEGz4 SHWA== 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=GR//KiUU2wxZg4lkaOii48CdLwuCRvaNwqh8E9dLjRA=; b=sFa5qIfeUCvUKhlz21FvximhCtAnNStp/aF7isDK/H+GeFygPOaHJ4Q+5T/y5RHifV lFfD2T2GjtFSCnxTm10xmwmi7XsljFSVlJDIhf63yo7rv5tL6PNw7AIFRwojHD918Yni VB5tixnU9C48Q1CaTzSDK+9Ib9h/Ag0Lj90UbyRjJurwD0QlacqLC8poASm62KO7wfcx oQMQfm9PYB2kyp4LAsHx5EpwNMEFhU5aDNj4KUPoK/2z6vIefJcbL8jDphxWuWD+IFCZ JvrPFgNmEQqnP+rBmNGj4GdPUmQRRQ7VjlfxvqwhfmxRZOYvEHSznPiSlRJtTQaqQVpC gnhQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=FskV9TVh; 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=kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id v38-20020a631526000000b004402f48dee0si43807pgl.629.2022.12.15.10.24.19; Thu, 15 Dec 2022 10:24:34 -0800 (PST) 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=@kernel.org header.s=k20201202 header.b=FskV9TVh; 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=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230171AbiLOSTO (ORCPT <rfc822;jeantsuru.cumc.mandola@gmail.com> + 99 others); Thu, 15 Dec 2022 13:19:14 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47636 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230289AbiLOSTE (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 15 Dec 2022 13:19:04 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3EC1248746; Thu, 15 Dec 2022 10:19:02 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id E2A23B81C1F; Thu, 15 Dec 2022 18:19:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 67BEDC433D2; Thu, 15 Dec 2022 18:18:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1671128339; bh=78tX3t6kUkY6BMXVJo5OYZXMdsWwwpj5lrEjbZ2nP5k=; h=From:To:Cc:Subject:Date:From; b=FskV9TVhDGw/J3zgBg0l/b33IrEveHitlVioGB/zPpBgsy+Rqi5GUybZXmQB+EPT/ MGo8+xWqRtpyI5EJpJmAtwbcP+WWKjKJ5cSKfRNS/R0OWeVhnOcscgK5+W9/ge/k8c FKckAFwV/A/vWObAeBKHTZy9+oOuvoKKM9+Hj9SV4KyvMdkLC0q+hJVKVupUS0ghpO 8SYIt57hOCe7GOq141BQiwSW7Qe4IHyTnga7w5Jw4L4dvfUyKqR+gOMUFhC1njhh07 eZachBoK9k5ZPm+Xzt2DuLkOqMv3HluGy7fcPsPtb3q+kiMG+Nuy4OJDH5Im01BQOU zYj2RdwEuXUyQ== From: Bjorn Helgaas <helgaas@kernel.org> To: "Rafael J . Wysocki" <rafael@kernel.org>, Len Brown <len.brown@intel.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com> Subject: [PATCH] PM: runtime: Simplify __rpm_get_callback() Date: Thu, 15 Dec 2022 12:18:48 -0600 Message-Id: <20221215181848.129326-1-helgaas@kernel.org> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS 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?1752305421025944818?= X-GMAIL-MSGID: =?utf-8?q?1752305421025944818?= |
Series |
PM: runtime: Simplify __rpm_get_callback()
|
|
Commit Message
Bjorn Helgaas
Dec. 15, 2022, 6:18 p.m. UTC
From: Bjorn Helgaas <bhelgaas@google.com> Simplify __rpm_get_callback() slightly by returning as soon as the return value is known. No functional change intended. Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> --- drivers/base/power/runtime.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
Comments
On Thu, Dec 15, 2022 at 7:19 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > From: Bjorn Helgaas <bhelgaas@google.com> > > Simplify __rpm_get_callback() slightly by returning as soon as the return > value is known. No functional change intended. > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/base/power/runtime.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c > index 50e726b6c2cf..7171ed0668f3 100644 > --- a/drivers/base/power/runtime.c > +++ b/drivers/base/power/runtime.c > @@ -20,8 +20,7 @@ typedef int (*pm_callback_t)(struct device *); > > static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > { > - pm_callback_t cb; > - const struct dev_pm_ops *ops; > + const struct dev_pm_ops *ops = NULL; > > if (dev->pm_domain) > ops = &dev->pm_domain->ops; > @@ -31,18 +30,14 @@ static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > ops = dev->class->pm; > else if (dev->bus && dev->bus->pm) > ops = dev->bus->pm; > - else > - ops = NULL; > > if (ops) > - cb = *(pm_callback_t *)((void *)ops + cb_offset); > - else > - cb = NULL; > + return *(pm_callback_t *)((void *)ops + cb_offset); > > - if (!cb && dev->driver && dev->driver->pm) > - cb = *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > + if (dev->driver && dev->driver->pm) > + return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > - return cb; > + return NULL; > } > > #define RPM_GET_CALLBACK(dev, callback) \ > -- Applied as 6.3 material, thanks!
Hi Bjorn, On Thu, Dec 15, 2022 at 7:23 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > From: Bjorn Helgaas <bhelgaas@google.com> > > Simplify __rpm_get_callback() slightly by returning as soon as the return > value is known. No functional change intended. > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Thanks for your patch, which is now commit 650bdddb6b311705 ("PM: runtime: Simplify __rpm_get_callback()") in pm/linux-next. > --- a/drivers/base/power/runtime.c > +++ b/drivers/base/power/runtime.c > @@ -20,8 +20,7 @@ typedef int (*pm_callback_t)(struct device *); > > static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > { > - pm_callback_t cb; > - const struct dev_pm_ops *ops; > + const struct dev_pm_ops *ops = NULL; > > if (dev->pm_domain) > ops = &dev->pm_domain->ops; > @@ -31,18 +30,14 @@ static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > ops = dev->class->pm; > else if (dev->bus && dev->bus->pm) > ops = dev->bus->pm; > - else > - ops = NULL; > > if (ops) > - cb = *(pm_callback_t *)((void *)ops + cb_offset); > - else > - cb = NULL; > + return *(pm_callback_t *)((void *)ops + cb_offset); This is a change in behavior in case the callback turns out to be NULL: - before, it would fall back to the driver-specific callback below, - after, it always returns NULL. > > - if (!cb && dev->driver && dev->driver->pm) > - cb = *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > + if (dev->driver && dev->driver->pm) > + return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > - return cb; > + return NULL; > } Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Jan 24, 2023 at 12:20 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Bjorn, > > On Thu, Dec 15, 2022 at 7:23 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > Simplify __rpm_get_callback() slightly by returning as soon as the return > > value is known. No functional change intended. > > > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > > Thanks for your patch, which is now commit 650bdddb6b311705 ("PM: > runtime: Simplify __rpm_get_callback()") in pm/linux-next. > > > --- a/drivers/base/power/runtime.c > > +++ b/drivers/base/power/runtime.c > > @@ -20,8 +20,7 @@ typedef int (*pm_callback_t)(struct device *); > > > > static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > > { > > - pm_callback_t cb; > > - const struct dev_pm_ops *ops; > > + const struct dev_pm_ops *ops = NULL; > > > > if (dev->pm_domain) > > ops = &dev->pm_domain->ops; > > @@ -31,18 +30,14 @@ static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > > ops = dev->class->pm; > > else if (dev->bus && dev->bus->pm) > > ops = dev->bus->pm; > > - else > > - ops = NULL; > > > > if (ops) > > - cb = *(pm_callback_t *)((void *)ops + cb_offset); > > - else > > - cb = NULL; > > + return *(pm_callback_t *)((void *)ops + cb_offset); > > This is a change in behavior in case the callback turns out to be NULL: > - before, it would fall back to the driver-specific callback below, > - after, it always returns NULL. Good point and sorry for missing this! > > > > - if (!cb && dev->driver && dev->driver->pm) > > - cb = *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > + if (dev->driver && dev->driver->pm) > > + return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > > > - return cb; > > + return NULL; > > } > Something like the patch below (modulo gmail-induced whitespace breakage) should restore the previous behavior if I'm not mistaken: --- drivers/base/power/runtime.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) Index: linux-pm/drivers/base/power/runtime.c =================================================================== --- linux-pm.orig/drivers/base/power/runtime.c +++ linux-pm/drivers/base/power/runtime.c @@ -31,8 +31,13 @@ static pm_callback_t __rpm_get_callback( else if (dev->bus && dev->bus->pm) ops = dev->bus->pm; - if (ops) - return *(pm_callback_t *)((void *)ops + cb_offset); + if (ops) { + pm_callback_t cb; + + cb = *(pm_callback_t *)((void *)ops + cb_offset); + if (cb) + return cb; + } if (dev->driver && dev->driver->pm) return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset);
Hi Rafael, On Tue, Jan 24, 2023 at 3:18 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > On Tue, Jan 24, 2023 at 12:20 PM Geert Uytterhoeven > <geert@linux-m68k.org> wrote: > > > > Hi Bjorn, > > > > On Thu, Dec 15, 2022 at 7:23 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > > > Simplify __rpm_get_callback() slightly by returning as soon as the return > > > value is known. No functional change intended. > > > > > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > > > > Thanks for your patch, which is now commit 650bdddb6b311705 ("PM: > > runtime: Simplify __rpm_get_callback()") in pm/linux-next. > > > > > --- a/drivers/base/power/runtime.c > > > +++ b/drivers/base/power/runtime.c > > > @@ -20,8 +20,7 @@ typedef int (*pm_callback_t)(struct device *); > > > > > > static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > > > { > > > - pm_callback_t cb; > > > - const struct dev_pm_ops *ops; > > > + const struct dev_pm_ops *ops = NULL; > > > > > > if (dev->pm_domain) > > > ops = &dev->pm_domain->ops; > > > @@ -31,18 +30,14 @@ static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > > > ops = dev->class->pm; > > > else if (dev->bus && dev->bus->pm) > > > ops = dev->bus->pm; > > > - else > > > - ops = NULL; > > > > > > if (ops) > > > - cb = *(pm_callback_t *)((void *)ops + cb_offset); > > > - else > > > - cb = NULL; > > > + return *(pm_callback_t *)((void *)ops + cb_offset); > > > > This is a change in behavior in case the callback turns out to be NULL: > > - before, it would fall back to the driver-specific callback below, > > - after, it always returns NULL. > > Good point and sorry for missing this! > > > > > > > - if (!cb && dev->driver && dev->driver->pm) > > > - cb = *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > > + if (dev->driver && dev->driver->pm) > > > + return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > > > > > - return cb; > > > + return NULL; > > > } > > > > Something like the patch below (modulo gmail-induced whitespace > breakage) should restore the previous behavior if I'm not mistaken: > > --- > drivers/base/power/runtime.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > Index: linux-pm/drivers/base/power/runtime.c > =================================================================== > --- linux-pm.orig/drivers/base/power/runtime.c > +++ linux-pm/drivers/base/power/runtime.c > @@ -31,8 +31,13 @@ static pm_callback_t __rpm_get_callback( > else if (dev->bus && dev->bus->pm) > ops = dev->bus->pm; > > - if (ops) > - return *(pm_callback_t *)((void *)ops + cb_offset); > + if (ops) { > + pm_callback_t cb; > + > + cb = *(pm_callback_t *)((void *)ops + cb_offset); > + if (cb) > + return cb; > + } > > if (dev->driver && dev->driver->pm) > return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); Which is now more complex than the original? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Jan 24, 2023 at 3:37 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Rafael, > > On Tue, Jan 24, 2023 at 3:18 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Tue, Jan 24, 2023 at 12:20 PM Geert Uytterhoeven > > <geert@linux-m68k.org> wrote: > > > > > > Hi Bjorn, > > > > > > On Thu, Dec 15, 2022 at 7:23 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > > > > > Simplify __rpm_get_callback() slightly by returning as soon as the return > > > > value is known. No functional change intended. > > > > > > > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > > > > > > Thanks for your patch, which is now commit 650bdddb6b311705 ("PM: > > > runtime: Simplify __rpm_get_callback()") in pm/linux-next. > > > > > > > --- a/drivers/base/power/runtime.c > > > > +++ b/drivers/base/power/runtime.c > > > > @@ -20,8 +20,7 @@ typedef int (*pm_callback_t)(struct device *); > > > > > > > > static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > > > > { > > > > - pm_callback_t cb; > > > > - const struct dev_pm_ops *ops; > > > > + const struct dev_pm_ops *ops = NULL; > > > > > > > > if (dev->pm_domain) > > > > ops = &dev->pm_domain->ops; > > > > @@ -31,18 +30,14 @@ static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > > > > ops = dev->class->pm; > > > > else if (dev->bus && dev->bus->pm) > > > > ops = dev->bus->pm; > > > > - else > > > > - ops = NULL; > > > > > > > > if (ops) > > > > - cb = *(pm_callback_t *)((void *)ops + cb_offset); > > > > - else > > > > - cb = NULL; > > > > + return *(pm_callback_t *)((void *)ops + cb_offset); > > > > > > This is a change in behavior in case the callback turns out to be NULL: > > > - before, it would fall back to the driver-specific callback below, > > > - after, it always returns NULL. > > > > Good point and sorry for missing this! > > > > > > > > > > - if (!cb && dev->driver && dev->driver->pm) > > > > - cb = *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > > > + if (dev->driver && dev->driver->pm) > > > > + return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > > > > > > > - return cb; > > > > + return NULL; > > > > } > > > > > > > Something like the patch below (modulo gmail-induced whitespace > > breakage) should restore the previous behavior if I'm not mistaken: > > > > --- > > drivers/base/power/runtime.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > Index: linux-pm/drivers/base/power/runtime.c > > =================================================================== > > --- linux-pm.orig/drivers/base/power/runtime.c > > +++ linux-pm/drivers/base/power/runtime.c > > @@ -31,8 +31,13 @@ static pm_callback_t __rpm_get_callback( > > else if (dev->bus && dev->bus->pm) > > ops = dev->bus->pm; > > > > - if (ops) > > - return *(pm_callback_t *)((void *)ops + cb_offset); > > + if (ops) { > > + pm_callback_t cb; > > + > > + cb = *(pm_callback_t *)((void *)ops + cb_offset); > > + if (cb) > > + return cb; > > + } > > > > if (dev->driver && dev->driver->pm) > > return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > Which is now more complex than the original? Arguably so. OK, I'll drop the commit in question then, sorry Bjorn.
On Tue, Jan 24, 2023 at 03:45:50PM +0100, Rafael J. Wysocki wrote: > On Tue, Jan 24, 2023 at 3:37 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Tue, Jan 24, 2023 at 3:18 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > > On Tue, Jan 24, 2023 at 12:20 PM Geert Uytterhoeven > > > <geert@linux-m68k.org> wrote: > > > > On Thu, Dec 15, 2022 at 7:23 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > > > > > > > Simplify __rpm_get_callback() slightly by returning as soon as the return > > > > > value is known. No functional change intended. > > > > > > > > > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > > > > > > > > Thanks for your patch, which is now commit 650bdddb6b311705 ("PM: > > > > runtime: Simplify __rpm_get_callback()") in pm/linux-next. > > > > > > > > > --- a/drivers/base/power/runtime.c > > > > > +++ b/drivers/base/power/runtime.c > > > > > @@ -20,8 +20,7 @@ typedef int (*pm_callback_t)(struct device *); > > > > > > > > > > static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > > > > > { > > > > > - pm_callback_t cb; > > > > > - const struct dev_pm_ops *ops; > > > > > + const struct dev_pm_ops *ops = NULL; > > > > > > > > > > if (dev->pm_domain) > > > > > ops = &dev->pm_domain->ops; > > > > > @@ -31,18 +30,14 @@ static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > > > > > ops = dev->class->pm; > > > > > else if (dev->bus && dev->bus->pm) > > > > > ops = dev->bus->pm; > > > > > - else > > > > > - ops = NULL; > > > > > > > > > > if (ops) > > > > > - cb = *(pm_callback_t *)((void *)ops + cb_offset); > > > > > - else > > > > > - cb = NULL; > > > > > + return *(pm_callback_t *)((void *)ops + cb_offset); > > > > > > > > This is a change in behavior in case the callback turns out to be NULL: > > > > - before, it would fall back to the driver-specific callback below, > > > > - after, it always returns NULL. > > > > > > Good point and sorry for missing this! > > > > > > > > > > > > > - if (!cb && dev->driver && dev->driver->pm) > > > > > - cb = *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > > > > + if (dev->driver && dev->driver->pm) > > > > > + return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > > > > > > > > > - return cb; > > > > > + return NULL; > > > > > } > > > > > > > > > > Something like the patch below (modulo gmail-induced whitespace > > > breakage) should restore the previous behavior if I'm not mistaken: > > > > > > --- > > > drivers/base/power/runtime.c | 9 +++++++-- > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > Index: linux-pm/drivers/base/power/runtime.c > > > =================================================================== > > > --- linux-pm.orig/drivers/base/power/runtime.c > > > +++ linux-pm/drivers/base/power/runtime.c > > > @@ -31,8 +31,13 @@ static pm_callback_t __rpm_get_callback( > > > else if (dev->bus && dev->bus->pm) > > > ops = dev->bus->pm; > > > > > > - if (ops) > > > - return *(pm_callback_t *)((void *)ops + cb_offset); > > > + if (ops) { > > > + pm_callback_t cb; > > > + > > > + cb = *(pm_callback_t *)((void *)ops + cb_offset); > > > + if (cb) > > > + return cb; > > > + } > > > > > > if (dev->driver && dev->driver->pm) > > > return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > > > Which is now more complex than the original? > > Arguably so. > > OK, I'll drop the commit in question then, sorry Bjorn. Really sorry about this. Think I'm all out of brown paper bags :( Thanks for catching this, Geert, and sorry for all the time you had to spend debugging it. Bjorn
Hi Bjorn, On Tue, Jan 24, 2023 at 4:06 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > On Tue, Jan 24, 2023 at 03:45:50PM +0100, Rafael J. Wysocki wrote: > > On Tue, Jan 24, 2023 at 3:37 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Tue, Jan 24, 2023 at 3:18 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > On Tue, Jan 24, 2023 at 12:20 PM Geert Uytterhoeven > > > > <geert@linux-m68k.org> wrote: > > > > > On Thu, Dec 15, 2022 at 7:23 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > > > > > > > > > Simplify __rpm_get_callback() slightly by returning as soon as the return > > > > > > value is known. No functional change intended. > > > > > > > > > > > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > > > > > > > > > > Thanks for your patch, which is now commit 650bdddb6b311705 ("PM: > > > > > runtime: Simplify __rpm_get_callback()") in pm/linux-next. > > > > > > > > > > > --- a/drivers/base/power/runtime.c > > > > > > +++ b/drivers/base/power/runtime.c > > > > > > @@ -20,8 +20,7 @@ typedef int (*pm_callback_t)(struct device *); > > > > > > > > > > > > static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > > > > > > { > > > > > > - pm_callback_t cb; > > > > > > - const struct dev_pm_ops *ops; > > > > > > + const struct dev_pm_ops *ops = NULL; > > > > > > > > > > > > if (dev->pm_domain) > > > > > > ops = &dev->pm_domain->ops; > > > > > > @@ -31,18 +30,14 @@ static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) > > > > > > ops = dev->class->pm; > > > > > > else if (dev->bus && dev->bus->pm) > > > > > > ops = dev->bus->pm; > > > > > > - else > > > > > > - ops = NULL; > > > > > > > > > > > > if (ops) > > > > > > - cb = *(pm_callback_t *)((void *)ops + cb_offset); > > > > > > - else > > > > > > - cb = NULL; > > > > > > + return *(pm_callback_t *)((void *)ops + cb_offset); > > > > > > > > > > This is a change in behavior in case the callback turns out to be NULL: > > > > > - before, it would fall back to the driver-specific callback below, > > > > > - after, it always returns NULL. > > > > > > > > Good point and sorry for missing this! > > > > > > > > > > > > > > > > - if (!cb && dev->driver && dev->driver->pm) > > > > > > - cb = *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > > > > > + if (dev->driver && dev->driver->pm) > > > > > > + return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > > > > > > > > > > > - return cb; > > > > > > + return NULL; > > > > > > } > > > > > > > > > > > > > Something like the patch below (modulo gmail-induced whitespace > > > > breakage) should restore the previous behavior if I'm not mistaken: > > > > > > > > --- > > > > drivers/base/power/runtime.c | 9 +++++++-- > > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > > > Index: linux-pm/drivers/base/power/runtime.c > > > > =================================================================== > > > > --- linux-pm.orig/drivers/base/power/runtime.c > > > > +++ linux-pm/drivers/base/power/runtime.c > > > > @@ -31,8 +31,13 @@ static pm_callback_t __rpm_get_callback( > > > > else if (dev->bus && dev->bus->pm) > > > > ops = dev->bus->pm; > > > > > > > > - if (ops) > > > > - return *(pm_callback_t *)((void *)ops + cb_offset); > > > > + if (ops) { > > > > + pm_callback_t cb; > > > > + > > > > + cb = *(pm_callback_t *)((void *)ops + cb_offset); > > > > + if (cb) > > > > + return cb; > > > > + } > > > > > > > > if (dev->driver && dev->driver->pm) > > > > return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); > > > > > > Which is now more complex than the original? > > > > Arguably so. > > > > OK, I'll drop the commit in question then, sorry Bjorn. > > Really sorry about this. Think I'm all out of brown paper bags :( > Thanks for catching this, Geert, and sorry for all the time you had to Np, no time was wasted on debugging. I just noticed because I have some local debug code on top, which no longer applied 8^) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 50e726b6c2cf..7171ed0668f3 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -20,8 +20,7 @@ typedef int (*pm_callback_t)(struct device *); static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) { - pm_callback_t cb; - const struct dev_pm_ops *ops; + const struct dev_pm_ops *ops = NULL; if (dev->pm_domain) ops = &dev->pm_domain->ops; @@ -31,18 +30,14 @@ static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) ops = dev->class->pm; else if (dev->bus && dev->bus->pm) ops = dev->bus->pm; - else - ops = NULL; if (ops) - cb = *(pm_callback_t *)((void *)ops + cb_offset); - else - cb = NULL; + return *(pm_callback_t *)((void *)ops + cb_offset); - if (!cb && dev->driver && dev->driver->pm) - cb = *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); + if (dev->driver && dev->driver->pm) + return *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); - return cb; + return NULL; } #define RPM_GET_CALLBACK(dev, callback) \