2023-04-28 11:53:56

by Sean Nyekjaer

[permalink] [raw]
Subject: [PATCH 1/2] mfd: stpmic1: fixup main control register and bits naming

Fixup main control register and bits naming so the match the naming from
the datasheet.

https://www.st.com/resource/en/datasheet/stpmic1.pdf

Signed-off-by: Sean Nyekjaer <[email protected]>
---
drivers/mfd/stpmic1.c | 4 ++--
include/linux/mfd/stpmic1.h | 12 ++++++------
2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/stpmic1.c b/drivers/mfd/stpmic1.c
index 8db1530d9bac..4c9b18d9dec8 100644
--- a/drivers/mfd/stpmic1.c
+++ b/drivers/mfd/stpmic1.c
@@ -19,7 +19,7 @@

static const struct regmap_range stpmic1_readable_ranges[] = {
regmap_reg_range(TURN_ON_SR, VERSION_SR),
- regmap_reg_range(SWOFF_PWRCTRL_CR, LDO6_STDBY_CR),
+ regmap_reg_range(MAIN_CR, LDO6_STDBY_CR),
regmap_reg_range(BST_SW_CR, BST_SW_CR),
regmap_reg_range(INT_PENDING_R1, INT_PENDING_R4),
regmap_reg_range(INT_CLEAR_R1, INT_CLEAR_R4),
@@ -30,7 +30,7 @@ static const struct regmap_range stpmic1_readable_ranges[] = {
};

static const struct regmap_range stpmic1_writeable_ranges[] = {
- regmap_reg_range(SWOFF_PWRCTRL_CR, LDO6_STDBY_CR),
+ regmap_reg_range(MAIN_CR, LDO6_STDBY_CR),
regmap_reg_range(BST_SW_CR, BST_SW_CR),
regmap_reg_range(INT_CLEAR_R1, INT_CLEAR_R4),
regmap_reg_range(INT_SET_MASK_R1, INT_SET_MASK_R4),
diff --git a/include/linux/mfd/stpmic1.h b/include/linux/mfd/stpmic1.h
index fa3f99f7e9a1..dc00bac24f5a 100644
--- a/include/linux/mfd/stpmic1.h
+++ b/include/linux/mfd/stpmic1.h
@@ -15,7 +15,7 @@
#define RREQ_STATE_SR 0x5
#define VERSION_SR 0x6

-#define SWOFF_PWRCTRL_CR 0x10
+#define MAIN_CR 0x10
#define PADS_PULL_CR 0x11
#define BUCKS_PD_CR 0x12
#define LDO14_PD_CR 0x13
@@ -148,14 +148,14 @@
#define LDO_BYPASS_MASK BIT(7)

/* Main PMIC Control Register
- * SWOFF_PWRCTRL_CR
+ * MAIN_CR
* Address : 0x10
*/
-#define ICC_EVENT_ENABLED BIT(4)
+#define OCP_OFF_DBG BIT(4)
#define PWRCTRL_POLARITY_HIGH BIT(3)
-#define PWRCTRL_PIN_VALID BIT(2)
-#define RESTART_REQUEST_ENABLED BIT(1)
-#define SOFTWARE_SWITCH_OFF_ENABLED BIT(0)
+#define PWRCTRL_ENABLE BIT(2)
+#define RESTART_REQUEST_ENABLE BIT(1)
+#define SOFTWARE_SWITCH_OFF BIT(0)

/* Main PMIC PADS Control Register
* PADS_PULL_CR
--
2.40.0


2023-04-28 12:01:48

by Sean Nyekjaer

[permalink] [raw]
Subject: [PATCH 2/2] mfd: stpmic1: add pmic poweroff via sys-off handler

Use devm_register_sys_off_handler() that allows to register multiple
power-off handlers.

This can be enabled by adding "st,pmic-poweroff" to device-tree.

Signed-off-by: Sean Nyekjaer <[email protected]>
---
drivers/mfd/stpmic1.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)

diff --git a/drivers/mfd/stpmic1.c b/drivers/mfd/stpmic1.c
index 4c9b18d9dec8..c628141eb03c 100644
--- a/drivers/mfd/stpmic1.c
+++ b/drivers/mfd/stpmic1.c
@@ -7,6 +7,7 @@
#include <linux/mfd/core.h>
#include <linux/mfd/stpmic1.h>
#include <linux/module.h>
+#include <linux/reboot.h>
#include <linux/of.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
@@ -117,6 +118,33 @@ static const struct regmap_irq_chip stpmic1_regmap_irq_chip = {
.num_irqs = ARRAY_SIZE(stpmic1_irqs),
};

+static int stpmic1_power_off(struct sys_off_data *data)
+{
+ struct stpmic1 *ddata = data->cb_data;
+
+ regmap_update_bits(ddata->regmap, MAIN_CR,
+ SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
+
+ return NOTIFY_DONE;
+}
+
+static int stpmic1_power_off_prepare_init(struct stpmic1 *ddata)
+{
+ int ret;
+
+ ret = devm_register_sys_off_handler(ddata->dev,
+ SYS_OFF_MODE_POWER_OFF,
+ SYS_OFF_PRIO_DEFAULT,
+ stpmic1_power_off,
+ ddata);
+ if (ret) {
+ dev_err(ddata->dev, "failed to register sys-off handler: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
static int stpmic1_probe(struct i2c_client *i2c)
{
struct stpmic1 *ddata;
@@ -159,6 +187,12 @@ static int stpmic1_probe(struct i2c_client *i2c)
return ret;
}

+ if (of_property_read_bool(i2c->dev.of_node, "st,pmic-poweroff")) {
+ ret = stpmic1_power_off_prepare_init(ddata);
+ if (ret)
+ return ret;
+ }
+
return devm_of_platform_populate(dev);
}

--
2.40.0

2023-05-02 06:22:43

by Sean Nyekjaer

[permalink] [raw]
Subject: Re: [PATCH 2/2] mfd: stpmic1: add pmic poweroff via sys-off handler



> On 28 Apr 2023, at 13.28, Sean Nyekjaer <[email protected]> wrote:
>
> Use devm_register_sys_off_handler() that allows to register multiple
> power-off handlers.
>
> This can be enabled by adding "st,pmic-poweroff" to device-tree.
>
> Signed-off-by: Sean Nyekjaer <[email protected]>
> ---

Oh, I forgot the device-tree documentation.
Will add that after review…

/Sean

> drivers/mfd/stpmic1.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/mfd/stpmic1.c b/drivers/mfd/stpmic1.c
> index 4c9b18d9dec8..c628141eb03c 100644
> --- a/drivers/mfd/stpmic1.c
> +++ b/drivers/mfd/stpmic1.c
> @@ -7,6 +7,7 @@
> #include <linux/mfd/core.h>
> #include <linux/mfd/stpmic1.h>
> #include <linux/module.h>
> +#include <linux/reboot.h>
> #include <linux/of.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> @@ -117,6 +118,33 @@ static const struct regmap_irq_chip stpmic1_regmap_irq_chip = {
> .num_irqs = ARRAY_SIZE(stpmic1_irqs),
> };
>
> +static int stpmic1_power_off(struct sys_off_data *data)
> +{
> + struct stpmic1 *ddata = data->cb_data;
> +
> + regmap_update_bits(ddata->regmap, MAIN_CR,
> + SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int stpmic1_power_off_prepare_init(struct stpmic1 *ddata)
> +{
> + int ret;
> +
> + ret = devm_register_sys_off_handler(ddata->dev,
> + SYS_OFF_MODE_POWER_OFF,
> + SYS_OFF_PRIO_DEFAULT,
> + stpmic1_power_off,
> + ddata);
> + if (ret) {
> + dev_err(ddata->dev, "failed to register sys-off handler: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int stpmic1_probe(struct i2c_client *i2c)
> {
> struct stpmic1 *ddata;
> @@ -159,6 +187,12 @@ static int stpmic1_probe(struct i2c_client *i2c)
> return ret;
> }
>
> + if (of_property_read_bool(i2c->dev.of_node, "st,pmic-poweroff")) {
> + ret = stpmic1_power_off_prepare_init(ddata);
> + if (ret)
> + return ret;
> + }
> +
> return devm_of_platform_populate(dev);
> }
>
> --
> 2.40.0
>


2023-05-15 12:46:46

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 1/2] mfd: stpmic1: fixup main control register and bits naming

On Fri, 28 Apr 2023, Sean Nyekjaer wrote:

> Fixup main control register and bits naming so the match the naming from
> the datasheet.

I'm in two minds about this.

On one hand using the names from the datasheet in the right thing to
do, however, on the other hand, the current nomenclature appears clearer.

> https://www.st.com/resource/en/datasheet/stpmic1.pdf
>
> Signed-off-by: Sean Nyekjaer <[email protected]>
> ---
> drivers/mfd/stpmic1.c | 4 ++--
> include/linux/mfd/stpmic1.h | 12 ++++++------
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mfd/stpmic1.c b/drivers/mfd/stpmic1.c
> index 8db1530d9bac..4c9b18d9dec8 100644
> --- a/drivers/mfd/stpmic1.c
> +++ b/drivers/mfd/stpmic1.c
> @@ -19,7 +19,7 @@
>
> static const struct regmap_range stpmic1_readable_ranges[] = {
> regmap_reg_range(TURN_ON_SR, VERSION_SR),
> - regmap_reg_range(SWOFF_PWRCTRL_CR, LDO6_STDBY_CR),
> + regmap_reg_range(MAIN_CR, LDO6_STDBY_CR),
> regmap_reg_range(BST_SW_CR, BST_SW_CR),
> regmap_reg_range(INT_PENDING_R1, INT_PENDING_R4),
> regmap_reg_range(INT_CLEAR_R1, INT_CLEAR_R4),
> @@ -30,7 +30,7 @@ static const struct regmap_range stpmic1_readable_ranges[] = {
> };
>
> static const struct regmap_range stpmic1_writeable_ranges[] = {
> - regmap_reg_range(SWOFF_PWRCTRL_CR, LDO6_STDBY_CR),
> + regmap_reg_range(MAIN_CR, LDO6_STDBY_CR),
> regmap_reg_range(BST_SW_CR, BST_SW_CR),
> regmap_reg_range(INT_CLEAR_R1, INT_CLEAR_R4),
> regmap_reg_range(INT_SET_MASK_R1, INT_SET_MASK_R4),
> diff --git a/include/linux/mfd/stpmic1.h b/include/linux/mfd/stpmic1.h
> index fa3f99f7e9a1..dc00bac24f5a 100644
> --- a/include/linux/mfd/stpmic1.h
> +++ b/include/linux/mfd/stpmic1.h
> @@ -15,7 +15,7 @@
> #define RREQ_STATE_SR 0x5
> #define VERSION_SR 0x6
>
> -#define SWOFF_PWRCTRL_CR 0x10
> +#define MAIN_CR 0x10
> #define PADS_PULL_CR 0x11
> #define BUCKS_PD_CR 0x12
> #define LDO14_PD_CR 0x13
> @@ -148,14 +148,14 @@
> #define LDO_BYPASS_MASK BIT(7)
>
> /* Main PMIC Control Register
> - * SWOFF_PWRCTRL_CR
> + * MAIN_CR
> * Address : 0x10
> */
> -#define ICC_EVENT_ENABLED BIT(4)
> +#define OCP_OFF_DBG BIT(4)
> #define PWRCTRL_POLARITY_HIGH BIT(3)
> -#define PWRCTRL_PIN_VALID BIT(2)
> -#define RESTART_REQUEST_ENABLED BIT(1)
> -#define SOFTWARE_SWITCH_OFF_ENABLED BIT(0)
> +#define PWRCTRL_ENABLE BIT(2)
> +#define RESTART_REQUEST_ENABLE BIT(1)
> +#define SOFTWARE_SWITCH_OFF BIT(0)
>
> /* Main PMIC PADS Control Register
> * PADS_PULL_CR
> --
> 2.40.0
>

--
Lee Jones [李琼斯]

2023-05-15 13:12:37

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 2/2] mfd: stpmic1: add pmic poweroff via sys-off handler

On Fri, 28 Apr 2023, Sean Nyekjaer wrote:

> Use devm_register_sys_off_handler() that allows to register multiple
> power-off handlers.
>
> This can be enabled by adding "st,pmic-poweroff" to device-tree.
>
> Signed-off-by: Sean Nyekjaer <[email protected]>
> ---
> drivers/mfd/stpmic1.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)

You need to submit the Device Tree binding with this patch.

> diff --git a/drivers/mfd/stpmic1.c b/drivers/mfd/stpmic1.c
> index 4c9b18d9dec8..c628141eb03c 100644
> --- a/drivers/mfd/stpmic1.c
> +++ b/drivers/mfd/stpmic1.c
> @@ -7,6 +7,7 @@
> #include <linux/mfd/core.h>
> #include <linux/mfd/stpmic1.h>
> #include <linux/module.h>
> +#include <linux/reboot.h>
> #include <linux/of.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> @@ -117,6 +118,33 @@ static const struct regmap_irq_chip stpmic1_regmap_irq_chip = {
> .num_irqs = ARRAY_SIZE(stpmic1_irqs),
> };
>
> +static int stpmic1_power_off(struct sys_off_data *data)
> +{
> + struct stpmic1 *ddata = data->cb_data;
> +
> + regmap_update_bits(ddata->regmap, MAIN_CR,
> + SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int stpmic1_power_off_prepare_init(struct stpmic1 *ddata)

This function looks superfluous.

Why not just call devm_register_sys_off_handler() inside the if?

> +{
> + int ret;
> +
> + ret = devm_register_sys_off_handler(ddata->dev,
> + SYS_OFF_MODE_POWER_OFF,
> + SYS_OFF_PRIO_DEFAULT,
> + stpmic1_power_off,
> + ddata);
> + if (ret) {
> + dev_err(ddata->dev, "failed to register sys-off handler: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int stpmic1_probe(struct i2c_client *i2c)
> {
> struct stpmic1 *ddata;
> @@ -159,6 +187,12 @@ static int stpmic1_probe(struct i2c_client *i2c)
> return ret;
> }
>
> + if (of_property_read_bool(i2c->dev.of_node, "st,pmic-poweroff")) {
> + ret = stpmic1_power_off_prepare_init(ddata);
> + if (ret)
> + return ret;
> + }
> +
> return devm_of_platform_populate(dev);
> }
>
> --
> 2.40.0
>

--
Lee Jones [李琼斯]

2023-05-16 06:59:11

by Sean Nyekjaer

[permalink] [raw]
Subject: Re: [PATCH 1/2] mfd: stpmic1: fixup main control register and bits naming

Hi Lee,

> On 15 May 2023, at 14.43, Lee Jones <[email protected]> wrote:
>
> On Fri, 28 Apr 2023, Sean Nyekjaer wrote:
>
>> Fixup main control register and bits naming so the match the naming from
>> the datasheet.
>
> I'm in two minds about this.
>
> On one hand using the names from the datasheet in the right thing to
> do, however, on the other hand, the current nomenclature appears clearer.

If you like me, tries to understand what’s happening inside the PMIC. It’s really nice to have the same naming in the software as in the datasheet.

It looks like the naming was updated in the datasheet version 9.

>
>> https://www.st.com/resource/en/datasheet/stpmic1.pdf
>>
>>

[ … ]

>>
>
> --
> Lee Jones [李琼斯]


/Sean