[PATCH 1/1] regulator: fixed: add system pm routines for pinctrl

[PATCH 1/1] regulator: fixed: add system pm routines for pinctrl

[Peter Chen]

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

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
This patch at NXP local tree serveral years, and fixed the
pinctrl value lost issue at some boards.

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

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index bc0bbd99e98d..29bb93c04b03 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -27,7 +27,7 @@
 #include <linux/regulator/of_regulator.h>
 #include <linux/regulator/machine.h>
 #include <linux/clk.h>
-
+#include <linux/pinctrl/consumer.h>
 
 struct fixed_voltage_data {
 	struct regulator_desc desc;
@@ -275,11 +275,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,
 	},
 };
 
-- 
2.17.1

Re: [PATCH 1/1] regulator: fixed: add system pm routines for pinctrl

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 588 bytes --]

On Thu, Mar 12, 2020 at 06:38:04PM +0800, Peter Chen wrote:
> At some systems, the pinctrl setting will be lost and needs to
> set as "sleep" state to save power consumption after system
> enters suspend. So, we need to configure pinctrl as "sleep" state
> when system enters suspend, and set it as "default" state after
> system resume. In this way, the pinctrl value can be recovered
> as "default" state after resuming.

Which pins exactly is this controlling?  I would not expect a fixed
voltage regulator to have pinctrl support, this feels like it's papering
over some other issue.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/1] regulator: fixed: add system pm routines for pinctrl

[Peter Chen]

On 20-03-12 11:47:12, Mark Brown wrote:
> On Thu, Mar 12, 2020 at 06:38:04PM +0800, Peter Chen wrote:
> > At some systems, the pinctrl setting will be lost and needs to
> > set as "sleep" state to save power consumption after system
> > enters suspend. So, we need to configure pinctrl as "sleep" state
> > when system enters suspend, and set it as "default" state after
> > system resume. In this way, the pinctrl value can be recovered
> > as "default" state after resuming.
> 
> Which pins exactly is this controlling?  I would not expect a fixed
> voltage regulator to have pinctrl support, this feels like it's papering
> over some other issue.

Sorry, I forget sending dts patch. We use fixed gpio regulator to control
USB VBus.

grep -rn reg_usb_otg_vbus arch/arm/boot/dts/*

arch/arm/boot/dts/imx53-m53evk.dts:82:		reg_usb_otg_vbus: regulator@4 {
arch/arm/boot/dts/imx53-m53evk.dts:367:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx53-ppd.dts:95:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx53-ppd.dts:650:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6dl-riotboard.dts:69:	reg_usb_otg_vbus: regulator-usbotgvbus {
arch/arm/boot/dts/imx6dl-riotboard.dts:339:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6dl-yapp4-common.dtsi:83:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6dl-yapp4-common.dtsi:571:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi:72:	reg_usb_otg_vbus: regulator-otg-vbus {
arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi:351:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-apalis-eval.dts:237:&reg_usb_otg_vbus {
arch/arm/boot/dts/imx6q-apalis-eval.dts:279:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-apalis-ixora.dts:240:&reg_usb_otg_vbus {
arch/arm/boot/dts/imx6q-apalis-ixora.dts:282:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts:236:&reg_usb_otg_vbus {
arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts:278:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-arm2.dts:34:		reg_usb_otg_vbus: regulator@1 {
arch/arm/boot/dts/imx6q-arm2.dts:188:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-ba16.dtsi:122:	reg_usb_otg_vbus: regulator-usbotgvbus {
arch/arm/boot/dts/imx6q-ba16.dtsi:378:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-cm-fx6.dts:93:	reg_usb_otg_vbus: usb_otg_vbus {
arch/arm/boot/dts/imx6q-cm-fx6.dts:471:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-dhcom-som.dtsi:26:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6q-dhcom-som.dtsi:442:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-apalis.dtsi:81:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-apf6dev.dtsi:102:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-apf6dev.dtsi:250:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-dfi-fs700-m60.dtsi:16:		reg_usb_otg_vbus: regulator@1 {
arch/arm/boot/dts/imx6qdl-dfi-fs700-m60.dtsi:175:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-gw51xx.dtsi:70:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-gw51xx.dtsi:312:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-gw52xx.dtsi:93:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-gw52xx.dtsi:392:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-gw53xx.dtsi:93:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-gw53xx.dtsi:383:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-gw54xx.dtsi:102:		reg_usb_otg_vbus: regulator@3 {
arch/arm/boot/dts/imx6qdl-gw54xx.dtsi:451:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-gw551x.dtsi:98:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-gw551x.dtsi:402:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-gw553x.dtsi:104:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-gw553x.dtsi:352:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-gw560x.dtsi:177:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-gw560x.dtsi:485:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-gw5903.dtsi:124:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-gw5903.dtsi:390:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-gw5904.dtsi:132:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-gw5904.dtsi:423:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-gw5907.dtsi:70:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-gw5907.dtsi:238:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-icore.dtsi:64:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-icore.dtsi:271:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi:71:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi:246:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi:41:		reg_usb_otg_vbus: regulator@2 {
arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi:545:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi:50:		reg_usb_otg_vbus: regulator@3 {
arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi:790:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-nitrogen6_som2.dtsi:193:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-nitrogen6_som2.dtsi:688:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi:43:		reg_usb_otg_vbus: regulator@2 {
arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi:638:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi:22:		reg_usb_otg_vbus: regulator@0 {
arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi:423:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-rex.dtsi:41:		reg_usb_otg_vbus: regulator@2 {
arch/arm/boot/dts/imx6qdl-rex.dtsi:345:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-sabreauto.dtsi:96:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-sabreauto.dtsi:830:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-sabrelite.dtsi:80:		reg_usb_otg_vbus: regulator@2 {
arch/arm/boot/dts/imx6qdl-sabrelite.dtsi:729:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-sabresd.dtsi:20:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-sabresd.dtsi:780:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-ts4900.dtsi:75:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-ts4900.dtsi:454:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-ts7970.dtsi:116:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6qdl-ts7970.dtsi:549:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-var-dart.dtsi:457:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6qdl-wandboard.dtsi:76:	reg_usb_otg_vbus: regulator-usbotgvbus {
arch/arm/boot/dts/imx6qdl-wandboard.dtsi:338:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-dms-ba16.dts:12:	reg_usb_otg_vbus: regulator-usbotgvbus {
arch/arm/boot/dts/imx6q-dms-ba16.dts:122:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-evi.dts:70:	reg_usb_otg_vbus: regulator-usbotgvbus {
arch/arm/boot/dts/imx6q-evi.dts:226:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-gw5400-a.dts:102:		reg_usb_otg_vbus: regulator@3 {
arch/arm/boot/dts/imx6q-gw5400-a.dts:369:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-marsboard.dts:62:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6q-marsboard.dts:205:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-novena.dts:180:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6q-novena.dts:492:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-pistachio.dts:88:	reg_usb_otg_vbus: regulator-usb_vbus {
arch/arm/boot/dts/imx6q-pistachio.dts:623:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6q-var-dt6customboard.dts:109:	reg_usb_otg_vbus: regulator-usbotgvbus {
arch/arm/boot/dts/imx6q-var-dt6customboard.dts:223:	vbus-supply = <&reg_usb_otg_vbus>;
arch/arm/boot/dts/imx6ul-pico.dtsi:54:	reg_usb_otg_vbus: regulator-usb-otg-vbus {
arch/arm/boot/dts/imx6ul-pico.dtsi:224:	vbus-supply = <&reg_usb_otg_vbus>;

-- 

Thanks,
Peter Chen

Re: [PATCH 1/1] regulator: fixed: add system pm routines for pinctrl

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 434 bytes --]

On Thu, Mar 12, 2020 at 01:00:33PM +0000, Peter Chen wrote:
> On 20-03-12 11:47:12, Mark Brown wrote:

> > Which pins exactly is this controlling?  I would not expect a fixed
> > voltage regulator to have pinctrl support, this feels like it's papering
> > over some other issue.

> Sorry, I forget sending dts patch. We use fixed gpio regulator to control
> USB VBus.

Surely it's the GPIO controller that needs pinctrl support then?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/1] regulator: fixed: add system pm routines for pinctrl

[Peter Chen]

On 20-03-12 14:37:23, Mark Brown wrote:
> On Thu, Mar 12, 2020 at 01:00:33PM +0000, Peter Chen wrote:
> > On 20-03-12 11:47:12, Mark Brown wrote:
> 
> > > Which pins exactly is this controlling?  I would not expect a fixed
> > > voltage regulator to have pinctrl support, this feels like it's papering
> > > over some other issue.
> 
> > Sorry, I forget sending dts patch. We use fixed gpio regulator to control
> > USB VBus.
> 
> Surely it's the GPIO controller that needs pinctrl support then?

But the pinctrl register value for this gpio will be cleared after
suspend, we need to restore it, otherwise, the function will be not
gpio. See below patch:

From 355de404fecd29a330e5b636ce1ffd1adce2f230 Mon Sep 17 00:00:00 2001
From: Peter Chen <peter.chen@nxp.com>
Date: Mon, 20 Jan 2020 10:04:43 +0800
Subject: [PATCH 1/1] ARM: dts: imx7ulp-evk: add pinctrl for suspend

For imx7ulp, the power of pinctrl is lost during the system
susupend, so we need to restore the pinctrl value after resume.
Add one group pinctrl for "sleep" for both id and vbus pinctrl.

Reviewed-by: Jun Li <jun.li@nxp.com>
Acked-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Peter Chen <peter.chen@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 arch/arm/boot/dts/imx7ulp-evk.dts | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx7ulp-evk.dts b/arch/arm/boot/dts/imx7ulp-evk.dts
index d67ca52042eb..3bf6bb071fa0 100644
--- a/arch/arm/boot/dts/imx7ulp-evk.dts
+++ b/arch/arm/boot/dts/imx7ulp-evk.dts
@@ -125,8 +125,9 @@
 
 		reg_usb_otg1_vbus: regulator-usb-otg1-vbus {
 			compatible = "regulator-fixed";
-			pinctrl-names = "default";
+			pinctrl-names = "default", "sleep";
 			pinctrl-0 = <&pinctrl_usbotg1_vbus>;
+			pinctrl-1 = <&pinctrl_usbotg1_vbus>;
 			regulator-name = "usb_otg1_vbus";
 			regulator-min-microvolt = <5000000>;
 			regulator-max-microvolt = <5000000>;
@@ -402,8 +403,9 @@
 
 &usbotg1 {
 	vbus-supply = <&reg_usb_otg1_vbus>;
-	pinctrl-names = "default";
+	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&pinctrl_usbotg1_id>;
+	pinctrl-1 = <&pinctrl_usbotg1_id>;
 	srp-disable;
 	hnp-disable;
 	adp-disable;
-- 
2.17.1


-- 

Thanks,
Peter Chen

Re: [PATCH 1/1] regulator: fixed: add system pm routines for pinctrl

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 429 bytes --]

On Thu, Mar 12, 2020 at 03:03:26PM +0000, Peter Chen wrote:
> On 20-03-12 14:37:23, Mark Brown wrote:

> > Surely it's the GPIO controller that needs pinctrl support then?

> But the pinctrl register value for this gpio will be cleared after
> suspend, we need to restore it, otherwise, the function will be not
> gpio. See below patch:

I'd expect that this would be handled by the GPIO driver, the user
shouldn't need to care.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/1] regulator: fixed: add system pm routines for pinctrl

[Peter Chen]

On 20-03-12 15:07:10, Mark Brown wrote:
> On Thu, Mar 12, 2020 at 03:03:26PM +0000, Peter Chen wrote:
> > On 20-03-12 14:37:23, Mark Brown wrote:
> 
> > > Surely it's the GPIO controller that needs pinctrl support then?
> 
> > But the pinctrl register value for this gpio will be cleared after
> > suspend, we need to restore it, otherwise, the function will be not
> > gpio. See below patch:
> 
> I'd expect that this would be handled by the GPIO driver, the user
> shouldn't need to care.

GPIO function is just our case for this fixed regulator, other users for
this fixed regulator may set pinctrl as other functions.

Here, it is just save and restore pinctrl value for fixed regulator
driver, not related to GPIO.

-- 

Thanks,
Peter Chen

Re: [PATCH 1/1] regulator: fixed: add system pm routines for pinctrl

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 687 bytes --]

On Fri, Mar 13, 2020 at 03:08:48AM +0000, Peter Chen wrote:
> On 20-03-12 15:07:10, Mark Brown wrote:

> > I'd expect that this would be handled by the GPIO driver, the user
> > shouldn't need to care.

> GPIO function is just our case for this fixed regulator, other users for
> this fixed regulator may set pinctrl as other functions.

> Here, it is just save and restore pinctrl value for fixed regulator
> driver, not related to GPIO.

My point is that the fixed regulator doesn't have pins in pinctrl,
whatever is providing the control signal to the fixed voltage regulator
(if there is one) does.  I'd expect this to be being handled on the
producer side rather than the consumer.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/1] regulator: fixed: add system pm routines for pinctrl

[Peter Chen]

On 20-03-13 12:11:03, Mark Brown wrote:
> On Fri, Mar 13, 2020 at 03:08:48AM +0000, Peter Chen wrote:
> > On 20-03-12 15:07:10, Mark Brown wrote:
> 
> > > I'd expect that this would be handled by the GPIO driver, the user
> > > shouldn't need to care.
> 
> > GPIO function is just our case for this fixed regulator, other users for
> > this fixed regulator may set pinctrl as other functions.
> 
> > Here, it is just save and restore pinctrl value for fixed regulator
> > driver, not related to GPIO.
> 
> My point is that the fixed regulator doesn't have pins in pinctrl,
> whatever is providing the control signal to the fixed voltage regulator
> (if there is one) does.  I'd expect this to be being handled on the
> producer side rather than the consumer.

I am sorry I have different points.

Most of pins for controlling fixed regulator on or off is GPIO, but how
GPIO driver handles this? We usually configure pin as GPIO function at
its user's node (Eg, reset pin for most drivers), but not GPIO node,
GPIO node is usually per SoC, not per board level.

So, I am wondering why fixed regulator can't have a pin in pinctrl.
If you grep the dts, there are already several fixed regulator has
pinctrl.

-- 

Thanks,
Peter Chen

Re: [PATCH 1/1] regulator: fixed: add system pm routines for pinctrl

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

On Fri, Mar 13, 2020 at 01:16:31PM +0000, Peter Chen wrote:

> Most of pins for controlling fixed regulator on or off is GPIO, but how
> GPIO driver handles this? We usually configure pin as GPIO function at
> its user's node (Eg, reset pin for most drivers), but not GPIO node,

The GPIO driver knows what signals it's supposed to be generating, it
should be able to configure pins for this.  Setting the pinctrl stuff on
the consumer seems like for example setting the pinctrl for a SPI
controller on the SPI device rather than the controller device which
would be a bit weird.

> GPIO node is usually per SoC, not per board level.

Sure, and it's totally normal for boards to add configuration to nodes
that are part of the SoC.  Again I'm a bit confused about why GPIOs are
expected to be different here.

> So, I am wondering why fixed regulator can't have a pin in pinctrl.
> If you grep the dts, there are already several fixed regulator has
> pinctrl.

Obviously those pinctrl settings currently don't actually do anything...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]