regulator: fixed: add suspend pm routines for pinctrl

PATCH Superseded Peter Chen 3 participants 6 messages Jan 3, 2017 – Jan 20, 2017 Voltage and current regulator framework

lore

Share & tools

lore

Peter Chen

author

2017-01-03 09:02 UTC #0 lore raw

PATCH 1/1 regulator: fixed: add suspend pm routines for pinctrl

At some systems, the pinctrl setting will be lost or needs to set as "sleep" state to save power consumption. So, we need to configure pinctrl as "sleep" state when system enters suspend, and as "default" state after system resumes. In this way, the pinctrl value can be recovered as "default" state after resuming.

Signed-off-by Peter Chen <[email protected]>

---
 drivers/regulator/fixed.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

drivers/regulator/fixed.c +21 -0

index a43b0e8..8f41015 100644
@@ -245,11 +245,32 @@ static const struct of_device_id fixed_of_match[] = {
 MODULE_DEVICE_TABLE(of, fixed_of_match);
 #endif
 
+#ifdef CONFIG_PM_SLEEP
+static int reg_fixed_voltage_suspend(struct device *dev)
+{
+	pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
+}
+static int reg_fixed_voltage_resume(struct device *dev)
+{
+	pinctrl_pm_select_default_state(dev);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops reg_fixed_voltage_pm_ops = {
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(reg_fixed_voltage_suspend,
+		reg_fixed_voltage_resume)
+};
+
 static struct platform_driver regulator_fixed_voltage_driver = {
 	.probe		= reg_fixed_voltage_probe,
 	.driver		= {
 		.name		= "reg-fixed-voltage",
 		.of_match_table = of_match_ptr(fixed_of_match),
+		.pm = &reg_fixed_voltage_pm_ops,
 	},
 };

Signature · 1 line

2.7.4

Mark Brown

maintainer

2017-01-03 11:15 UTC #1 lore raw

Quoted 5 lines · Peter Chen

At some systems, the pinctrl setting will be lost or needs to set as "sleep" state to save power consumption. So, we need to configure pinctrl as "sleep" state when system enters suspend, and as "default" state after system resumes. In this way, the pinctrl value can be recovered as "default" state after resuming.

I thought this was supposed to be done automatically by the driver core? If not it should be, every single driver is likely to end up with that.

Peter Chen

author

2017-01-04 06:51 UTC #2 lore raw

Quoted 9 lines · Peter Chen

Quoted 5 lines
At some systems, the pinctrl setting will be lost or needs to set as "sleep" state to save power consumption. So, we need to configure pinctrl as "sleep" state when system enters suspend, and as "default" state after system resumes. In this way, the pinctrl value can be recovered as "default" state after resuming.
I thought this was supposed to be done automatically by the driver core? If not it should be, every single driver is likely to end up with that.

Good idea, I will send a patch for that.

Peter

Peter Chen

author

2017-01-04 07:05 UTC #3 lore raw

In reply to Mark Brown

Quoted 8 lines

Quoted 8 lines · Peter Chen
Quoted 5 lines
At some systems, the pinctrl setting will be lost or needs to set as "sleep" state to save power consumption. So, we need to configure pinctrl as "sleep" state when system enters suspend, and as "default" state after system resumes. In this way, the pinctrl value can be recovered as "default" state after resuming.

Quoted 5 lines

Quoted 2 lines
I thought this was supposed to be done automatically by the driver core? If not it should be, every single driver is likely to end up with that.
Good idea, I will send a patch for that.

After checking more, it is not suitable for driver core do it since different driver has different usage for "sleep" pinctrl. Below are some examples:

- Some drivers set "idle" for pinctrl when system goes to suspend
- Some drivers has some conditions for setting "sleep" for pinctrl
eg: drivers/net/ethernet/freescale/fec_main.c
- Some drivers may set "sleep" for pinctrl as ->close interface, but not system suspend interface
- Some drivers can be woken up for "sleep" pinctrl, some may not.

Peter

Peter Chen

author

2017-01-20 08:13 UTC #4 lore raw

In reply to Mark Brown

Quoted 18 lines

Quoted 9 lines
Quoted 9 lines · Peter Chen
Quoted 5 lines
At some systems, the pinctrl setting will be lost or needs to set as "sleep" state to save power consumption. So, we need to configure pinctrl as "sleep" state when system enters suspend, and as "default" state after system resumes. In this way, the pinctrl value can be recovered as "default" state after resuming.
Quoted 5 lines
Quoted 2 lines
I thought this was supposed to be done automatically by the driver core? If not it should be, every single driver is likely to end up with that.
Good idea, I will send a patch for that.
After checking more, it is not suitable for driver core do it since different driver has different usage for "sleep" pinctrl. Below are some examples:

Quoted 7 lines

- Some drivers set "idle" for pinctrl when system goes to suspend
- Some drivers has some conditions for setting "sleep" for pinctrl
eg: drivers/net/ethernet/freescale/fec_main.c
- Some drivers may set "sleep" for pinctrl as ->close interface, but not system suspend interface
- Some drivers can be woken up for "sleep" pinctrl, some may not.

Hi Mark,

Any more comment for this patch?
Thanks.

Peter

kbuild test robot

build bot

2017-01-03 14:35 UTC #5 lore raw

In reply to Peter Chen

reproduce:

Hi Peter,

[auto build test ERROR on regulator/for-next]
[also build test ERROR on v4.10-rc2 next-20170103]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Peter-Chen/regulator-fixed-add-suspend-pm-routines-for-pinctrl/20170103-214456
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
config: i386-randconfig-a0-201701 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386

All errors (new ones prefixed by >>):

drivers/regulator/fixed.c: In function 'reg_fixed_voltage_suspend':

>> drivers/regulator/fixed.c:251:2: error: implicit declaration of function 'pinctrl_pm_select_sleep_state' [-Werror=implicit-function-declaration]
     pinctrl_pm_select_sleep_state(dev);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/regulator/fixed.c: In function 'reg_fixed_voltage_resume':
>> drivers/regulator/fixed.c:257:2: error: implicit declaration of function 'pinctrl_pm_select_default_state' [-Werror=implicit-function-declaration]
     pinctrl_pm_select_default_state(dev);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

cc1: some warnings being treated as errors

vim +/pinctrl_pm_select_sleep_state +251 drivers/regulator/fixed.c

245 MODULE_DEVICE_TABLE(of, fixed_of_match);
246 #endif
247
248 #ifdef CONFIG_PM_SLEEP
249 static int reg_fixed_voltage_suspend(struct device *dev)
250 {

 > 251		pinctrl_pm_select_sleep_state(dev);
   252	
   253		return 0;
   254	}
   255	static int reg_fixed_voltage_resume(struct device *dev)
   256	{
 > 257		pinctrl_pm_select_default_state(dev);

258
259 return 0;
260 }

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation