[permalink] [raw]

Subject: [PATCH 19/28] mfd: t7l66xb: Remove #ifdef guards for PM related functions

Use the new pm_sleep_ptr() macro to handle the .suspend/.resume
callbacks.

This macro allow the suspend and resume functions to be automatically
dropped by the compiler when CONFIG_SUSPEND is disabled, without having
to use #ifdef guards.

The advantage is then that these functions are now always compiled
independently of any Kconfig option, and thanks to that bugs and
regressions are easier to catch.

Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/mfd/t7l66xb.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/t7l66xb.c b/drivers/mfd/t7l66xb.c
index 5369c67e3280..74d26dce2003 100644
--- a/drivers/mfd/t7l66xb.c
+++ b/drivers/mfd/t7l66xb.c
@@ -257,7 +257,6 @@ static void t7l66xb_detach_irq(struct platform_device *dev)

/*--------------------------------------------------------------------------*/

-#ifdef CONFIG_PM
static int t7l66xb_suspend(struct platform_device *dev, pm_message_t state)
{
struct t7l66xb *t7l66xb = platform_get_drvdata(dev);
@@ -288,10 +287,6 @@ static int t7l66xb_resume(struct platform_device *dev)

return 0;
}
-#else
-#define t7l66xb_suspend NULL
-#define t7l66xb_resume NULL
-#endif

/*--------------------------------------------------------------------------*/

@@ -420,8 +415,8 @@ static struct platform_driver t7l66xb_platform_driver = {
.driver = {
.name = "t7l66xb",
},
- .suspend = t7l66xb_suspend,
- .resume = t7l66xb_resume,
+ .suspend = pm_sleep_ptr(t7l66xb_suspend),
+ .resume = pm_sleep_ptr(t7l66xb_resume),
.probe = t7l66xb_probe,
.remove = t7l66xb_remove,
};
--
2.35.1

2022-08-07 15:39:17

by Paul Cercueil

[permalink] [raw]

Subject: [PATCH 24/28] mfd: sprd-sc27xx: Remove #ifdef guards for PM related functions

2022-08-07 15:39:36

by Paul Cercueil

[permalink] [raw]

Subject: [PATCH 09/28] mfd: ucb1x00: Remove #ifdef guards for PM related functions

2022-08-07 15:39:37

by Paul Cercueil

[permalink] [raw]

Subject: [PATCH 14/28] mfd: sm501: Remove #ifdef guards for PM related functions

Use the new pm_sleep_ptr() macro to handle the .suspend/.resume
callbacks.

This macro allow the suspend and resume functions to be automatically
dropped by the compiler when CONFIG_SUSPEND is disabled, without having
to use #ifdef guards.

The advantage is then that these functions are now always compiled
independently of any Kconfig option, and thanks to that bugs and
regressions are easier to catch.

Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/mfd/sm501.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
index bc0a2c38653e..81e15e646e77 100644
--- a/drivers/mfd/sm501.c
+++ b/drivers/mfd/sm501.c
@@ -1432,8 +1432,6 @@ static int sm501_plat_probe(struct platform_device *dev)

}

-#ifdef CONFIG_PM
-
/* power management support */

static void sm501_set_power(struct sm501_devdata *sm, int on)
@@ -1509,10 +1507,6 @@ static int sm501_plat_resume(struct platform_device *pdev)

return 0;
}
-#else
-#define sm501_plat_suspend NULL
-#define sm501_plat_resume NULL
-#endif

/* Initialisation data for PCI devices */

@@ -1714,8 +1708,8 @@ static struct platform_driver sm501_plat_driver = {
},
.probe = sm501_plat_probe,
.remove = sm501_plat_remove,
- .suspend = sm501_plat_suspend,
- .resume = sm501_plat_resume,
+ .suspend = pm_sleep_ptr(sm501_plat_suspend),
+ .resume = pm_sleep_ptr(sm501_plat_resume),
};

static int __init sm501_base_init(void)
--
2.35.1

2022-08-07 15:40:26

by Paul Cercueil

[permalink] [raw]

Subject: [PATCH 13/28] mfd: sec: Remove #ifdef guards for PM related functions

2022-08-07 15:40:32

by Paul Cercueil

[permalink] [raw]

2022-08-07 18:17:08

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 25/28] mfd: stmfx: Remove #ifdef guards for PM related functions

Hi Paul,

I love your patch! Yet something to improve:

[auto build test ERROR on v5.19]
[cannot apply to lee-mfd/for-mfd-next linus/master next-20220805]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/mfd-Remove-ifdef-guards-for-PM-functions/20220807-225947
base: 3d7cb6b04c3f3115719235cc6866b10326de34cd
config: hexagon-randconfig-r013-20220807 (https://download.01.org/0day-ci/archive/20220808/[email protected]/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/5a39bd52d549e3f3be743fb336dd710eac75055d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Paul-Cercueil/mfd-Remove-ifdef-guards-for-PM-functions/20220807-225947
git checkout 5a39bd52d549e3f3be743fb336dd710eac75055d
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/mfd/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/mfd/stmfx.c:485:18: error: no member named 'bkp_sysctrl' in 'struct stmfx'
&stmfx->bkp_sysctrl, sizeof(stmfx->bkp_sysctrl));
~~~~~ ^
drivers/mfd/stmfx.c:485:45: error: no member named 'bkp_sysctrl' in 'struct stmfx'
&stmfx->bkp_sysctrl, sizeof(stmfx->bkp_sysctrl));
~~~~~ ^
>> drivers/mfd/stmfx.c:490:18: error: no member named 'bkp_irqoutpin' in 'struct stmfx'
&stmfx->bkp_irqoutpin,
~~~~~ ^
drivers/mfd/stmfx.c:491:24: error: no member named 'bkp_irqoutpin' in 'struct stmfx'
sizeof(stmfx->bkp_irqoutpin));
~~~~~ ^
drivers/mfd/stmfx.c:525:19: error: no member named 'bkp_sysctrl' in 'struct stmfx'
&stmfx->bkp_sysctrl, sizeof(stmfx->bkp_sysctrl));
~~~~~ ^
drivers/mfd/stmfx.c:525:46: error: no member named 'bkp_sysctrl' in 'struct stmfx'
&stmfx->bkp_sysctrl, sizeof(stmfx->bkp_sysctrl));
~~~~~ ^
drivers/mfd/stmfx.c:530:19: error: no member named 'bkp_irqoutpin' in 'struct stmfx'
&stmfx->bkp_irqoutpin,
~~~~~ ^
drivers/mfd/stmfx.c:531:25: error: no member named 'bkp_irqoutpin' in 'struct stmfx'
sizeof(stmfx->bkp_irqoutpin));
~~~~~ ^
8 errors generated.

vim +485 drivers/mfd/stmfx.c

06252ade915665 Amelie Delaunay 2019-05-09 478
06252ade915665 Amelie Delaunay 2019-05-09 479 static int stmfx_suspend(struct device *dev)
06252ade915665 Amelie Delaunay 2019-05-09 480 {
06252ade915665 Amelie Delaunay 2019-05-09 481 struct stmfx *stmfx = dev_get_drvdata(dev);
06252ade915665 Amelie Delaunay 2019-05-09 482 int ret;
06252ade915665 Amelie Delaunay 2019-05-09 483
06252ade915665 Amelie Delaunay 2019-05-09 484 ret = regmap_raw_read(stmfx->map, STMFX_REG_SYS_CTRL,
06252ade915665 Amelie Delaunay 2019-05-09 @485 &stmfx->bkp_sysctrl, sizeof(stmfx->bkp_sysctrl));
06252ade915665 Amelie Delaunay 2019-05-09 486 if (ret)
06252ade915665 Amelie Delaunay 2019-05-09 487 return ret;
06252ade915665 Amelie Delaunay 2019-05-09 488
06252ade915665 Amelie Delaunay 2019-05-09 489 ret = regmap_raw_read(stmfx->map, STMFX_REG_IRQ_OUT_PIN,
06252ade915665 Amelie Delaunay 2019-05-09 @490 &stmfx->bkp_irqoutpin,
06252ade915665 Amelie Delaunay 2019-05-09 491 sizeof(stmfx->bkp_irqoutpin));
06252ade915665 Amelie Delaunay 2019-05-09 492 if (ret)
06252ade915665 Amelie Delaunay 2019-05-09 493 return ret;
06252ade915665 Amelie Delaunay 2019-05-09 494
97eda5dcc2cde5 Amelie Delaunay 2020-04-22 495 disable_irq(stmfx->irq);
97eda5dcc2cde5 Amelie Delaunay 2020-04-22 496
06252ade915665 Amelie Delaunay 2019-05-09 497 if (stmfx->vdd)
06252ade915665 Amelie Delaunay 2019-05-09 498 return regulator_disable(stmfx->vdd);
06252ade915665 Amelie Delaunay 2019-05-09 499
06252ade915665 Amelie Delaunay 2019-05-09 500 return 0;
06252ade915665 Amelie Delaunay 2019-05-09 501 }
06252ade915665 Amelie Delaunay 2019-05-09 502

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-08-08 07:59:33

by Hennerich, Michael

[permalink] [raw]

Subject: RE: [PATCH 03/28] mfd: adp5520: Remove #ifdef guards for PM related functions

> -----Original Message-----
> From: Paul Cercueil <[email protected]>
> Sent: Sonntag, 7. August 2022 16:52
> To: Lee Jones <[email protected]>
> Cc: [email protected]; Paul Cercueil <[email protected]>;
> Hennerich, Michael <[email protected]>
> Subject: [PATCH 03/28] mfd: adp5520: Remove #ifdef guards for PM related
> functions
>
>
> Use the new DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr() macros
> to handle the .suspend/.resume callbacks.
>
> These macros allow the suspend and resume functions to be automatically
> dropped by the compiler when CONFIG_SUSPEND is disabled, without
> having to use #ifdef guards.
>
> The advantage is then that these functions are now always compiled
> independently of any Kconfig option, and thanks to that bugs and
> regressions are easier to catch.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> Cc: Michael Hennerich <[email protected]>

Acked-by: Michael Hennerich <[email protected]>

> ---
> drivers/mfd/adp5520.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mfd/adp5520.c b/drivers/mfd/adp5520.c index
> 8db15f5a7179..882eddc32f8e 100644
> --- a/drivers/mfd/adp5520.c
> +++ b/drivers/mfd/adp5520.c
> @@ -305,7 +305,6 @@ static int adp5520_probe(struct i2c_client *client,
> return ret;
> }
>
> -#ifdef CONFIG_PM_SLEEP
> static int adp5520_suspend(struct device *dev) {
> struct i2c_client *client = to_i2c_client(dev); @@ -326,9 +325,8 @@
> static int adp5520_resume(struct device *dev)
> adp5520_write(chip->dev, ADP5520_MODE_STATUS, chip->mode);
> return 0;
> }
> -#endif
>
> -static SIMPLE_DEV_PM_OPS(adp5520_pm, adp5520_suspend,
> adp5520_resume);
> +static DEFINE_SIMPLE_DEV_PM_OPS(adp5520_pm, adp5520_suspend,
> +adp5520_resume);
>
> static const struct i2c_device_id adp5520_id[] = {
> { "pmic-adp5520", ID_ADP5520 },
> @@ -339,7 +337,7 @@ static const struct i2c_device_id adp5520_id[] = {
> static struct i2c_driver adp5520_driver = {
> .driver = {
> .name = "adp5520",
> - .pm = &adp5520_pm,
> + .pm = pm_sleep_ptr(&adp5520_pm),
> .suppress_bind_attrs = true,
> },
> .probe = adp5520_probe,
> --
> 2.35.1

2022-08-08 09:24:29

by Krzysztof Kozlowski

[permalink] [raw]

Subject: Re: [PATCH 13/28] mfd: sec: Remove #ifdef guards for PM related functions

On 07/08/2022 17:52, Paul Cercueil wrote:
> Use the new DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr() macros
> to handle the .suspend/.resume callbacks.
>
> These macros allow the suspend and resume functions to be automatically
> dropped by the compiler when CONFIG_SUSPEND is disabled, without having
> to use #ifdef guards.
>
> The advantage is then that these functions are now always compiled
> independently of any Kconfig option, and thanks to that bugs and
> regressions are easier to catch.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Bartlomiej Zolnierkiewicz <[email protected]>

The address does not work. Please don't add it to commit log.

> Cc: [email protected]

This is also not really needed in commit log... it's just a mailing list...

I actually never understood why people want to add to commit log, so to
something which will last 10 years, Cc-ing other folks, instead of
adding such tags after '---'. Imagine 10 years from now:

1. What's the point to be cced on this patch after 10 years instead of
using maintainers file (the one in 10 years)? Why Cc-ing me in 10 years?
If I am a maintainer of this driver in that time, I will be C-ced based
on maintainers file. If I am not a maintainer in 10 years, why the heck
cc-ing me based on some 10-year old commit? Just because I was a
maintainer once, like 10 years ago?

2. Or why cc-ing such people when backporting to stable?

It's quite a lot of unnecessary emails which many of us won't actually
handle later...

I sincerely admit I was once also adding such Cc-tags. But that time my
employer was counting lines-of-patch (including commit log)... crazy, right?

> ---
> drivers/mfd/sec-core.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index 1fb29c45f5cf..a467de2b2fea 100644
> --- a/drivers/mfd/sec-core.c
> +++ b/drivers/mfd/sec-core.c
> @@ -455,7 +455,6 @@ static void sec_pmic_shutdown(struct i2c_client *i2c)
> regmap_update_bits(sec_pmic->regmap_pmic, reg, mask, 0);
> }
>
> -#ifdef CONFIG_PM_SLEEP
> static int sec_pmic_suspend(struct device *dev)

Did you test W=1 with !CONFIG_PM_SLEEP? No warnings?

Best regards,
Krzysztof

2022-08-08 09:41:50

by Paul Cercueil

[permalink] [raw]

Subject: Re: [PATCH 13/28] mfd: sec: Remove #ifdef guards for PM related functions

Hi Krzysztof,

Le lun., ao?t 8 2022 at 12:11:02 +0300, Krzysztof Kozlowski
<[email protected]> a ?crit :
> On 07/08/2022 17:52, Paul Cercueil wrote:
>> Use the new DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr() macros
>> to handle the .suspend/.resume callbacks.
>>
>> These macros allow the suspend and resume functions to be
>> automatically
>> dropped by the compiler when CONFIG_SUSPEND is disabled, without
>> having
>> to use #ifdef guards.
>>
>> The advantage is then that these functions are now always compiled
>> independently of any Kconfig option, and thanks to that bugs and
>> regressions are easier to catch.
>>
>> Signed-off-by: Paul Cercueil <[email protected]>
>> Cc: Krzysztof Kozlowski <[email protected]>
>> Cc: Bartlomiej Zolnierkiewicz <[email protected]>
>
> The address does not work. Please don't add it to commit log.

That's what get-maintainers gave me, and I didn't get any error sending
at that address. But I'll take your word.

>> Cc: [email protected]
>
> This is also not really needed in commit log... it's just a mailing
> list...
>
> I actually never understood why people want to add to commit log, so
> to
> something which will last 10 years, Cc-ing other folks, instead of
> adding such tags after '---'. Imagine 10 years from now:
>
> 1. What's the point to be cced on this patch after 10 years instead of
> using maintainers file (the one in 10 years)? Why Cc-ing me in 10
> years?
> If I am a maintainer of this driver in that time, I will be C-ced
> based
> on maintainers file. If I am not a maintainer in 10 years, why the
> heck
> cc-ing me based on some 10-year old commit? Just because I was a
> maintainer once, like 10 years ago?
>
> 2. Or why cc-ing such people when backporting to stable?
>
> It's quite a lot of unnecessary emails which many of us won't actually
> handle later...
>
> I sincerely admit I was once also adding such Cc-tags. But that time
> my
> employer was counting lines-of-patch (including commit log)... crazy,
> right?

Yeah, well, I can add these tags after the '---' line. Nobody ever told
me that I was doing it wrong, and I see Cc: tags quite often in commit
messages, so I thought it was common practice.

>> ---
>> drivers/mfd/sec-core.c | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
>> index 1fb29c45f5cf..a467de2b2fea 100644
>> --- a/drivers/mfd/sec-core.c
>> +++ b/drivers/mfd/sec-core.c
>> @@ -455,7 +455,6 @@ static void sec_pmic_shutdown(struct i2c_client
>> *i2c)
>> regmap_update_bits(sec_pmic->regmap_pmic, reg, mask, 0);
>> }
>>
>> -#ifdef CONFIG_PM_SLEEP
>> static int sec_pmic_suspend(struct device *dev)
>
> Did you test W=1 with !CONFIG_PM_SLEEP? No warnings?

I tested the PR with !CONFIG_PM_SLEEP, correct. sec-core.o compiles
fine. No warnings with W=1.
>
Cheers,
-Paul

2022-08-08 09:46:15

by Krzysztof Kozlowski

[permalink] [raw]

2022-08-09 17:56:19

by Lee Jones

[permalink] [raw]

Subject: Re: [PATCH 13/28] mfd: sec: Remove #ifdef guards for PM related functions

On Tue, 09 Aug 2022, Krzysztof Kozlowski wrote:

> On 09/08/2022 18:33, Lee Jones wrote:
> > On Mon, 08 Aug 2022, Krzysztof Kozlowski wrote:
> >
> >> On 07/08/2022 17:52, Paul Cercueil wrote:
> >>> Use the new DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr() macros
> >>> to handle the .suspend/.resume callbacks.
> >>>
> >>> These macros allow the suspend and resume functions to be automatically
> >>> dropped by the compiler when CONFIG_SUSPEND is disabled, without having
> >>> to use #ifdef guards.
> >>>
> >>> The advantage is then that these functions are now always compiled
> >>> independently of any Kconfig option, and thanks to that bugs and
> >>> regressions are easier to catch.
> >>>
> >>> Signed-off-by: Paul Cercueil <[email protected]>
> >>> Cc: Krzysztof Kozlowski <[email protected]>
> >>> Cc: Bartlomiej Zolnierkiewicz <[email protected]>
> >>
> >> The address does not work. Please don't add it to commit log.
> >>
> >>> Cc: [email protected]
> >>
> >> This is also not really needed in commit log... it's just a mailing list...
> >>
> >> I actually never understood why people want to add to commit log, so to
> >> something which will last 10 years, Cc-ing other folks, instead of
> >> adding such tags after '---'. Imagine 10 years from now:
> >>
> >> 1. What's the point to be cced on this patch after 10 years instead of
> >> using maintainers file (the one in 10 years)? Why Cc-ing me in 10 years?
> >> If I am a maintainer of this driver in that time, I will be C-ced based
> >> on maintainers file. If I am not a maintainer in 10 years, why the heck
> >> cc-ing me based on some 10-year old commit? Just because I was a
> >> maintainer once, like 10 years ago?
> >
> > Why would that happen?
> >
> > These tags are only used during initial submission.
>
> No, the tags are used in any other resends, backports etc while
> traveling through different trees. I think only stable-backports do not
> use them, but all other gfp+git-send will follow the tags.
>
> >
> >> 2. Or why cc-ing such people when backporting to stable?
> >
> > That doesn't happen either.
>
> Indeed, stable does not use these Cc.
>
> >> It's quite a lot of unnecessary emails which many of us won't actually
> >> handle later...
> >>
> >> I sincerely admit I was once also adding such Cc-tags. But that time my
> >> employer was counting lines-of-patch (including commit log)... crazy, right?
> >
> > Nothing wrong with adding these tags IMHO. It's what they're for.
> >
> > I use them when I'm maintaining a large amount of out-of-tree, but
> > to-be-upstreamed patches over several versions. Re-applying the
> > recipients list can become pretty labour-some after several
> > iterations.
>
> You can do it still while keeping the tags after ---. Only patch-related
> workflows strip such tags. If you cherry-pick, rebase, merge, you always
> get the content of ---.
>
> The same as typical changelog (not cover letter but one in the patch) -
> you keep it after --- and it does not disappear.

I'll have to try this next time.

> > Adding them under the '---' doesn't work when the purpose of them is
> > to keep the recipients list in Git history.
>
> This I understand, what I did not understand (and you did not explain)
> is what would be the purpose to keep them in Git history. What is the
> point to have them in commit log of 10 year old commit?

Here is a documented use for the tags:

"If a person has had the opportunity to comment on a patch, but has not
provided such comments, you may optionally add a ``Cc:`` tag to the patch."

Thus, when a recipient replies with a *-by tag, I strip out the
corresponding Cc: line.

Obviously this does not apply to mailing lists.

--
DEPRECATED: Please use [email protected]