[PATCH v2 0/3] regulator pre-enable

[PATCH v2 0/3] regulator pre-enable

[Jianqun Xu]

Rockchip io-domain care about regulator pre-enable

Jianqun Xu (3):
  regulator: add PRE_ENABLE event define
  regulator: core: add pre-enable event notify to regulator
  soc: rockchip: io-domain: do more thing about regulator notify

 drivers/regulator/core.c           | 3 +++
 drivers/soc/rockchip/io-domain.c   | 6 ++++++
 include/linux/regulator/consumer.h | 2 ++
 3 files changed, 11 insertions(+)

-- 
v2:
 - remove patch "regulator: core: notify regulator enable with the voltage value"
 - use "uV = regulator_get_voltage(supply->reg);" but from notify data
v1:
 - first

2.25.1

[PATCH v2 0/3] regulator pre-enable

[Jianqun Xu]

Rockchip io-domain care about regulator pre-enable

Jianqun Xu (3):
  regulator: add PRE_ENABLE event define
  regulator: core: add pre-enable event notify to regulator
  soc: rockchip: io-domain: do more thing about regulator notify

 drivers/regulator/core.c           | 3 +++
 drivers/soc/rockchip/io-domain.c   | 6 ++++++
 include/linux/regulator/consumer.h | 2 ++
 3 files changed, 11 insertions(+)

-- 
v2:
 - remove patch "regulator: core: notify regulator enable with the voltage value"
 - use "uV = regulator_get_voltage(supply->reg);" but from notify data
v1:
 - first

2.25.1




_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 1/3] regulator: add PRE_ENABLE event define

[Jianqun Xu]

Add REGULATOR_EVENT_PRE_ENABLE to allow to notify driver that the
regulator is about to enabled.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
v2: none

 include/linux/regulator/consumer.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 20e84a84fb77..edd31f0dad17 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -102,6 +102,7 @@ struct regulator_dev;
  *                      Data passed is old voltage cast to (void *).
  * PRE_DISABLE    Regulator is about to be disabled
  * ABORT_DISABLE  Regulator disable failed for some reason
+ * PRE_ENABLE     Regulator is about to be enabled
  *
  * NOTE: These events can be OR'ed together when passed into handler.
  */
@@ -119,6 +120,7 @@ struct regulator_dev;
 #define REGULATOR_EVENT_PRE_DISABLE		0x400
 #define REGULATOR_EVENT_ABORT_DISABLE		0x800
 #define REGULATOR_EVENT_ENABLE			0x1000
+#define REGULATOR_EVENT_PRE_ENABLE		0x2000
 
 /*
  * Regulator errors that can be queried using regulator_get_error_flags
-- 
2.25.1

[PATCH v2 1/3] regulator: add PRE_ENABLE event define

[Jianqun Xu]

Add REGULATOR_EVENT_PRE_ENABLE to allow to notify driver that the
regulator is about to enabled.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
v2: none

 include/linux/regulator/consumer.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 20e84a84fb77..edd31f0dad17 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -102,6 +102,7 @@ struct regulator_dev;
  *                      Data passed is old voltage cast to (void *).
  * PRE_DISABLE    Regulator is about to be disabled
  * ABORT_DISABLE  Regulator disable failed for some reason
+ * PRE_ENABLE     Regulator is about to be enabled
  *
  * NOTE: These events can be OR'ed together when passed into handler.
  */
@@ -119,6 +120,7 @@ struct regulator_dev;
 #define REGULATOR_EVENT_PRE_DISABLE		0x400
 #define REGULATOR_EVENT_ABORT_DISABLE		0x800
 #define REGULATOR_EVENT_ENABLE			0x1000
+#define REGULATOR_EVENT_PRE_ENABLE		0x2000
 
 /*
  * Regulator errors that can be queried using regulator_get_error_flags
-- 
2.25.1




_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 1/3] regulator: add PRE_ENABLE event define

[Mark Brown]

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

On Tue, Aug 17, 2021 at 11:38:46AM +0800, Jianqun Xu wrote:
> Add REGULATOR_EVENT_PRE_ENABLE to allow to notify driver that the
> regulator is about to enabled.

This doesn't apply against current code, please check and resend.

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

Re: [PATCH v2 1/3] regulator: add PRE_ENABLE event define

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 228 bytes --]

On Tue, Aug 17, 2021 at 11:38:46AM +0800, Jianqun Xu wrote:
> Add REGULATOR_EVENT_PRE_ENABLE to allow to notify driver that the
> regulator is about to enabled.

This doesn't apply against current code, please check and resend.

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

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 2/3] regulator: core: add pre-enable event notify to regulator

[Jianqun Xu]

Notify the event about regulator to be enabled to driver.

The IO-DOMAIN driver on Rockchip SoCs will take this event and then to
configure the io-domain mode before regualtor_enable.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
v2: none

 drivers/regulator/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f192bf19492e..d7dacc608509 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2654,6 +2654,9 @@ static int _regulator_enable(struct regulator *regulator)
 				goto err_consumer_disable;
 			}
 
+			_notifier_call_chain(rdev, REGULATOR_EVENT_PRE_ENABLE,
+					     NULL);
+
 			ret = _regulator_do_enable(rdev);
 			if (ret < 0)
 				goto err_consumer_disable;
-- 
2.25.1

[PATCH v2 2/3] regulator: core: add pre-enable event notify to regulator

[Jianqun Xu]

Notify the event about regulator to be enabled to driver.

The IO-DOMAIN driver on Rockchip SoCs will take this event and then to
configure the io-domain mode before regualtor_enable.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
v2: none

 drivers/regulator/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f192bf19492e..d7dacc608509 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2654,6 +2654,9 @@ static int _regulator_enable(struct regulator *regulator)
 				goto err_consumer_disable;
 			}
 
+			_notifier_call_chain(rdev, REGULATOR_EVENT_PRE_ENABLE,
+					     NULL);
+
 			ret = _regulator_do_enable(rdev);
 			if (ret < 0)
 				goto err_consumer_disable;
-- 
2.25.1




_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify

[Jianqun Xu]

Do a fix to rockchip io-domain, follow this orders:

* system running state
  -> io-domain vsel to 3.3V
    -> regulator_enable
      -> vsel change according to regulator voltage

* system running state
  -> io-domain vsel to 3.3V
    -> regulator_disable

Found on some Rockchip SoCs, the regulator enable or disable without
care about the io-domain maybe caused soc damaged.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
v2:
 - use "uV = regulator_get_voltage(supply->reg);" but from notify data
v1:
 - first version

 drivers/soc/rockchip/io-domain.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/soc/rockchip/io-domain.c b/drivers/soc/rockchip/io-domain.c
index cf8182fc3642..3c59077fafb1 100644
--- a/drivers/soc/rockchip/io-domain.c
+++ b/drivers/soc/rockchip/io-domain.c
@@ -123,6 +123,12 @@ static int rockchip_iodomain_notify(struct notifier_block *nb,
 	} else if (event & (REGULATOR_EVENT_VOLTAGE_CHANGE |
 			    REGULATOR_EVENT_ABORT_VOLTAGE_CHANGE)) {
 		uV = (unsigned long)data;
+	} else if (event & REGULATOR_EVENT_PRE_ENABLE) {
+		uV = MAX_VOLTAGE_3_3;
+	} else if (event & REGULATOR_EVENT_PRE_DISABLE) {
+		uV = MAX_VOLTAGE_3_3;
+	} else if (event & REGULATOR_EVENT_ENABLE) {
+		uV = regulator_get_voltage(supply->reg);
 	} else {
 		return NOTIFY_OK;
 	}
-- 
2.25.1

[PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify

[Jianqun Xu]

Do a fix to rockchip io-domain, follow this orders:

* system running state
  -> io-domain vsel to 3.3V
    -> regulator_enable
      -> vsel change according to regulator voltage

* system running state
  -> io-domain vsel to 3.3V
    -> regulator_disable

Found on some Rockchip SoCs, the regulator enable or disable without
care about the io-domain maybe caused soc damaged.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
v2:
 - use "uV = regulator_get_voltage(supply->reg);" but from notify data
v1:
 - first version

 drivers/soc/rockchip/io-domain.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/soc/rockchip/io-domain.c b/drivers/soc/rockchip/io-domain.c
index cf8182fc3642..3c59077fafb1 100644
--- a/drivers/soc/rockchip/io-domain.c
+++ b/drivers/soc/rockchip/io-domain.c
@@ -123,6 +123,12 @@ static int rockchip_iodomain_notify(struct notifier_block *nb,
 	} else if (event & (REGULATOR_EVENT_VOLTAGE_CHANGE |
 			    REGULATOR_EVENT_ABORT_VOLTAGE_CHANGE)) {
 		uV = (unsigned long)data;
+	} else if (event & REGULATOR_EVENT_PRE_ENABLE) {
+		uV = MAX_VOLTAGE_3_3;
+	} else if (event & REGULATOR_EVENT_PRE_DISABLE) {
+		uV = MAX_VOLTAGE_3_3;
+	} else if (event & REGULATOR_EVENT_ENABLE) {
+		uV = regulator_get_voltage(supply->reg);
 	} else {
 		return NOTIFY_OK;
 	}
-- 
2.25.1




_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify

[Mark Brown]

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

On Tue, Aug 17, 2021 at 11:38:48AM +0800, Jianqun Xu wrote:

> +	} else if (event & REGULATOR_EVENT_ENABLE) {
> +		uV = regulator_get_voltage(supply->reg);
>  	} else {

I am very surprised this doesn't cause locking issues given that we
might call notifiers with the regulator API's locks held.  Have you
tested this with lockdep on?

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

Re: [PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 335 bytes --]

On Tue, Aug 17, 2021 at 11:38:48AM +0800, Jianqun Xu wrote:

> +	} else if (event & REGULATOR_EVENT_ENABLE) {
> +		uV = regulator_get_voltage(supply->reg);
>  	} else {

I am very surprised this doesn't cause locking issues given that we
might call notifiers with the regulator API's locks held.  Have you
tested this with lockdep on?

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

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: Re: [PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify

[jay.xu]

Hi Mark
--------------
jay.xu@rock-chips.com
>On Tue, Aug 17, 2021 at 11:38:48AM +0800, Jianqun Xu wrote:
>
>> +	} else if (event & REGULATOR_EVENT_ENABLE) {
>> +	uV = regulator_get_voltage(supply->reg);
>>  } else {
>
>I am very surprised this doesn't cause locking issues given that we
>might call notifiers with the regulator API's locks held.  Have you
>tested this with lockdep on? 
Thanks for your reply, there really has locking issue, our test team fail to find out it
but get a pass result.

So, if the voltage cannot get here by driver, can the notify  pass it, like pre-version patch?
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: Re: [PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify

[jay.xu]

Hi Mark
--------------
jay.xu@rock-chips.com
>Hi Mark
>--------------
>jay.xu@rock-chips.com
>>On Tue, Aug 17, 2021 at 11:38:48AM +0800, Jianqun Xu wrote:
>>
>>> +	} else if (event & REGULATOR_EVENT_ENABLE) {
>>> +	uV = regulator_get_voltage(supply->reg);
>>>  } else {
>>
>>I am very surprised this doesn't cause locking issues given that we
>>might call notifiers with the regulator API's locks held.  Have you
>>tested this with lockdep on?
>Thanks for your reply, there really has locking issue, our test team fail to find out it
>but get a pass result.
>
>So, if the voltage cannot get here by driver, can the notify  pass it, like pre-version patch? 

I think it's possilbe to store the voltage in driver side, and do the io-domain config after EVENT_ENABLE
notify witout voltage, I will push it to io-domain maintainer to review, can I add yours' review in that patch ?
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip