* [PATCH v2 0/3] regulator pre-enable
@ 2021-08-17 3:38 ` Jianqun Xu
0 siblings, 0 replies; 14+ messages in thread
From: Jianqun Xu @ 2021-08-17 3:38 UTC (permalink / raw)
To: lgirdwood, broonie, ulf.hansson, lee.jones, zhangchangzhong, heiko
Cc: linux-rockchip, linux-kernel, 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/3] regulator: add PRE_ENABLE event define
2021-08-17 3:38 ` Jianqun Xu
@ 2021-08-17 3:38 ` Jianqun Xu
-1 siblings, 0 replies; 14+ messages in thread
From: Jianqun Xu @ 2021-08-17 3:38 UTC (permalink / raw)
To: lgirdwood, broonie, ulf.hansson, lee.jones, zhangchangzhong, heiko
Cc: linux-rockchip, linux-kernel, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 1/3] regulator: add PRE_ENABLE event define
@ 2021-08-17 3:38 ` Jianqun Xu
0 siblings, 0 replies; 14+ messages in thread
From: Jianqun Xu @ 2021-08-17 3:38 UTC (permalink / raw)
To: lgirdwood, broonie, ulf.hansson, lee.jones, zhangchangzhong, heiko
Cc: linux-rockchip, linux-kernel, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] regulator: add PRE_ENABLE event define
2021-08-17 3:38 ` Jianqun Xu
@ 2021-08-18 12:56 ` Mark Brown
-1 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2021-08-18 12:56 UTC (permalink / raw)
To: Jianqun Xu
Cc: lgirdwood, ulf.hansson, lee.jones, zhangchangzhong, heiko,
linux-rockchip, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] regulator: add PRE_ENABLE event define
@ 2021-08-18 12:56 ` Mark Brown
0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2021-08-18 12:56 UTC (permalink / raw)
To: Jianqun Xu
Cc: lgirdwood, ulf.hansson, lee.jones, zhangchangzhong, heiko,
linux-rockchip, linux-kernel
[-- 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] regulator: core: add pre-enable event notify to regulator
2021-08-17 3:38 ` Jianqun Xu
@ 2021-08-17 3:38 ` Jianqun Xu
-1 siblings, 0 replies; 14+ messages in thread
From: Jianqun Xu @ 2021-08-17 3:38 UTC (permalink / raw)
To: lgirdwood, broonie, ulf.hansson, lee.jones, zhangchangzhong, heiko
Cc: linux-rockchip, linux-kernel, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] regulator: core: add pre-enable event notify to regulator
@ 2021-08-17 3:38 ` Jianqun Xu
0 siblings, 0 replies; 14+ messages in thread
From: Jianqun Xu @ 2021-08-17 3:38 UTC (permalink / raw)
To: lgirdwood, broonie, ulf.hansson, lee.jones, zhangchangzhong, heiko
Cc: linux-rockchip, linux-kernel, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify
2021-08-17 3:38 ` Jianqun Xu
@ 2021-08-17 3:38 ` Jianqun Xu
-1 siblings, 0 replies; 14+ messages in thread
From: Jianqun Xu @ 2021-08-17 3:38 UTC (permalink / raw)
To: lgirdwood, broonie, ulf.hansson, lee.jones, zhangchangzhong, heiko
Cc: linux-rockchip, linux-kernel, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify
@ 2021-08-17 3:38 ` Jianqun Xu
0 siblings, 0 replies; 14+ messages in thread
From: Jianqun Xu @ 2021-08-17 3:38 UTC (permalink / raw)
To: lgirdwood, broonie, ulf.hansson, lee.jones, zhangchangzhong, heiko
Cc: linux-rockchip, linux-kernel, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify
2021-08-17 3:38 ` Jianqun Xu
@ 2021-08-17 14:36 ` Mark Brown
-1 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2021-08-17 14:36 UTC (permalink / raw)
To: Jianqun Xu
Cc: lgirdwood, ulf.hansson, lee.jones, zhangchangzhong, heiko,
linux-rockchip, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify
@ 2021-08-17 14:36 ` Mark Brown
0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2021-08-17 14:36 UTC (permalink / raw)
To: Jianqun Xu
Cc: lgirdwood, ulf.hansson, lee.jones, zhangchangzhong, heiko,
linux-rockchip, linux-kernel
[-- 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify
2021-08-17 14:36 ` Mark Brown
(?)
@ 2021-08-18 1:16 ` jay.xu
2021-08-18 2:24 ` jay.xu
-1 siblings, 1 reply; 14+ messages in thread
From: jay.xu @ 2021-08-18 1:16 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, ulf.hansson, Lee Jones, Zhang Changzhong,
Heiko Stübner, open list:ARM/Rockchip SoC...,
Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH v2 3/3] soc: rockchip: io-domain: do more thing about regulator notify
2021-08-18 1:16 ` jay.xu
@ 2021-08-18 2:24 ` jay.xu
0 siblings, 0 replies; 14+ messages in thread
From: jay.xu @ 2021-08-18 2:24 UTC (permalink / raw)
To: 许剑群, Mark Brown
Cc: lgirdwood, ulf.hansson, Lee Jones, Zhang Changzhong,
Heiko Stübner, open list:ARM/Rockchip SoC...,
Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 14+ messages in thread