* [PATCH] PM: runtime: Simplify __rpm_get_callback() @ 2022-12-15 18:18 Bjorn Helgaas 2023-01-13 20:01 ` Rafael J. Wysocki 2023-01-24 11:19 ` Geert Uytterhoeven 0 siblings, 2 replies; 8+ messages in thread From: Bjorn Helgaas @ 2022-12-15 18:18 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown Cc: Greg Kroah-Hartman, linux-pm, linux-kernel, Bjorn Helgaas 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) \ -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PM: runtime: Simplify __rpm_get_callback() 2022-12-15 18:18 [PATCH] PM: runtime: Simplify __rpm_get_callback() Bjorn Helgaas @ 2023-01-13 20:01 ` Rafael J. Wysocki 2023-01-24 11:19 ` Geert Uytterhoeven 1 sibling, 0 replies; 8+ messages in thread From: Rafael J. Wysocki @ 2023-01-13 20:01 UTC (permalink / raw) To: Bjorn Helgaas Cc: Rafael J . Wysocki, Len Brown, Greg Kroah-Hartman, linux-pm, linux-kernel, Bjorn Helgaas 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! ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PM: runtime: Simplify __rpm_get_callback() 2022-12-15 18:18 [PATCH] PM: runtime: Simplify __rpm_get_callback() Bjorn Helgaas 2023-01-13 20:01 ` Rafael J. Wysocki @ 2023-01-24 11:19 ` Geert Uytterhoeven 2023-01-24 14:07 ` Rafael J. Wysocki 1 sibling, 1 reply; 8+ messages in thread From: Geert Uytterhoeven @ 2023-01-24 11:19 UTC (permalink / raw) To: Bjorn Helgaas Cc: Rafael J . Wysocki, Len Brown, Greg Kroah-Hartman, linux-pm, linux-kernel, Bjorn Helgaas 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PM: runtime: Simplify __rpm_get_callback() 2023-01-24 11:19 ` Geert Uytterhoeven @ 2023-01-24 14:07 ` Rafael J. Wysocki 2023-01-24 14:37 ` Geert Uytterhoeven 0 siblings, 1 reply; 8+ messages in thread From: Rafael J. Wysocki @ 2023-01-24 14:07 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Bjorn Helgaas, Rafael J . Wysocki, Len Brown, Greg Kroah-Hartman, linux-pm, linux-kernel, Bjorn Helgaas 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); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PM: runtime: Simplify __rpm_get_callback() 2023-01-24 14:07 ` Rafael J. Wysocki @ 2023-01-24 14:37 ` Geert Uytterhoeven 2023-01-24 14:45 ` Rafael J. Wysocki 0 siblings, 1 reply; 8+ messages in thread From: Geert Uytterhoeven @ 2023-01-24 14:37 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Bjorn Helgaas, Len Brown, Greg Kroah-Hartman, linux-pm, linux-kernel, Bjorn Helgaas 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PM: runtime: Simplify __rpm_get_callback() 2023-01-24 14:37 ` Geert Uytterhoeven @ 2023-01-24 14:45 ` Rafael J. Wysocki 2023-01-24 15:05 ` Bjorn Helgaas 0 siblings, 1 reply; 8+ messages in thread From: Rafael J. Wysocki @ 2023-01-24 14:45 UTC (permalink / raw) To: Geert Uytterhoeven, Bjorn Helgaas Cc: Rafael J. Wysocki, Len Brown, Greg Kroah-Hartman, linux-pm, linux-kernel, Bjorn Helgaas 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PM: runtime: Simplify __rpm_get_callback() 2023-01-24 14:45 ` Rafael J. Wysocki @ 2023-01-24 15:05 ` Bjorn Helgaas 2023-01-24 16:14 ` Geert Uytterhoeven 0 siblings, 1 reply; 8+ messages in thread From: Bjorn Helgaas @ 2023-01-24 15:05 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Geert Uytterhoeven, Len Brown, Greg Kroah-Hartman, linux-pm, linux-kernel, Bjorn Helgaas 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PM: runtime: Simplify __rpm_get_callback() 2023-01-24 15:05 ` Bjorn Helgaas @ 2023-01-24 16:14 ` Geert Uytterhoeven 0 siblings, 0 replies; 8+ messages in thread From: Geert Uytterhoeven @ 2023-01-24 16:14 UTC (permalink / raw) To: Bjorn Helgaas Cc: Rafael J. Wysocki, Len Brown, Greg Kroah-Hartman, linux-pm, linux-kernel, Bjorn Helgaas 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-01-24 16:14 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-15 18:18 [PATCH] PM: runtime: Simplify __rpm_get_callback() Bjorn Helgaas 2023-01-13 20:01 ` Rafael J. Wysocki 2023-01-24 11:19 ` Geert Uytterhoeven 2023-01-24 14:07 ` Rafael J. Wysocki 2023-01-24 14:37 ` Geert Uytterhoeven 2023-01-24 14:45 ` Rafael J. Wysocki 2023-01-24 15:05 ` Bjorn Helgaas 2023-01-24 16:14 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®