[PATCH 0/3] Powerbutton driver and powerdown request for TPS65224 PMIC

[PATCH 0/3] Powerbutton driver and powerdown request for TPS65224 PMIC

[Job Sava]

Hello all,

The following patches were created to get the tps65224 PMIC powerbutton driver and power off request working on the MitySOM-AM62PX. The patches are as follows:

1-Powerbutton:
Full implementation
TPS659224 pmic push and release interrupts are now handled by the powerbutton driver. These events now issue a power off request that puts the PMIC into STANDBY mode. tps6594-pwrbutton.c is modeled off the tps65219-pwrbutton.c driver.

2-OFF-request:
The PMIC will now respond to a power off request. This is done by sending the PMIC a command to enter STANDBY mode. The PMIC will then turn off the power to the system.

3-Device tree bindings:
The device tree binding for the TPS65224 PMIC has been updated to include a proper description of the power button functionality.

4-Impact:
Button presses will now be detected reliably by the newly added powerbutton driver. The `tps6594-pfsm.c` file will no longer handle push and release pushbutton interrupts. This is because the powerbutton driver now manages these interrupts, as intended, which ensures the powerbutton functionality operates independently and correctly.

Regards,
Job Sava
jsava@criticallink.com
Critical Link LLC

Signed-off-by: Job Sava <jsava@criticallink.com>
---
Job Sava (3):
      dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC
      mfd: tps6594-pwrbutton: Add powerbutton functionality
      mfd: tps6594: Adds support for powering off the PMIC

 .../devicetree/bindings/mfd/ti,tps6594.yaml        |  15 +++
 drivers/input/misc/Kconfig                         |  10 ++
 drivers/input/misc/Makefile                        |   1 +
 drivers/input/misc/tps6594-pwrbutton.c             | 126 +++++++++++++++++++++
 drivers/mfd/tps6594-core.c                         |  49 +++++++-
 5 files changed, 199 insertions(+), 2 deletions(-)
---
base-commit: a5806cd506af5a7c19bcd596e4708b5c464bfd21
change-id: 20250403-linux-stable-tps6594-pwrbutton-e056dccc6be5

Best regards,
-- 
Job Sava <jsava@criticallink.com>

[PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Job Sava]

The TPS6594 power-button option permits users to enter STANDBY or
ACTIVE state by a push, release, or short push button request.

Signed-off-by: Job Sava <jsava@criticallink.com>
---
 Documentation/devicetree/bindings/mfd/ti,tps6594.yaml | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
index 6341b6070366..a40808fd2747 100644
--- a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
+++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
@@ -37,6 +37,21 @@ properties:
       device on the SPMI bus, and the secondary PMICs are the target devices
       on the SPMI bus.
 
+  ti,power-button:
+    type: boolean
+    description: |
+      Optional property that sets the EN/PB/VSENSE pin to be a
+      power-button.
+      TPS6594 has a multipurpose pin called EN/PB/VSENSE that can be either
+      1. EN in which case it functions as an enable pin.
+      2. VSENSE which compares the voltages and triggers an automatic
+      on/off request.
+      3. PB in which case it can be configured to trigger an interrupt
+      to the SoC.
+      ti,power-button reflects the last one of those options
+      where the board has a button wired to the pin and triggers
+      an interrupt on pressing it.
+
   system-power-controller: true
 
   gpio-controller: true

-- 
2.43.0

[PATCH 2/3] mfd: tps6594-pwrbutton: Add powerbutton functionality

[Job Sava]

TPS6594 defines two interrupts for the powerbutton one for push and
one for release.

This driver is very simple in that it maps the push interrupt to a key
input and the release interrupt to a key release.

Signed-off-by: Job Sava <jsava@criticallink.com>
---
 drivers/input/misc/Kconfig             |  10 +++
 drivers/input/misc/Makefile            |   1 +
 drivers/input/misc/tps6594-pwrbutton.c | 126 +++++++++++++++++++++++++++++++++
 drivers/mfd/tps6594-core.c             |  25 ++++++-
 4 files changed, 160 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index f5496ca0c0d2..20776651caf3 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -506,6 +506,16 @@ config INPUT_TPS65219_PWRBUTTON
 	  To compile this driver as a module, choose M here. The module will
 	  be called tps65219-pwrbutton.
 
+config INPUT_TPS6594_PWRBUTTON
+	tristate "TPS6594 Power button driver"
+	depends on MFD_TPS6594
+	help
+	  Say Y here if you want to enable power button reporting for
+	  TPS6594 Power Management IC devices.
+
+	  To compile this driver as a module, choose M here. The module will
+	  be called tps6594-pwrbutton.
+
 config INPUT_AXP20X_PEK
 	tristate "X-Powers AXP20X power button driver"
 	depends on MFD_AXP20X
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 6d91804d0a6f..a065b8889ccd 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_INPUT_SPARCSPKR)		+= sparcspkr.o
 obj-$(CONFIG_INPUT_STPMIC1_ONKEY)  	+= stpmic1_onkey.o
 obj-$(CONFIG_INPUT_TPS65218_PWRBUTTON)	+= tps65218-pwrbutton.o
 obj-$(CONFIG_INPUT_TPS65219_PWRBUTTON)	+= tps65219-pwrbutton.o
+obj-$(CONFIG_INPUT_TPS6594_PWRBUTTON)	+= tps6594-pwrbutton.o
 obj-$(CONFIG_INPUT_TWL4030_PWRBUTTON)	+= twl4030-pwrbutton.o
 obj-$(CONFIG_INPUT_TWL4030_VIBRA)	+= twl4030-vibra.o
 obj-$(CONFIG_INPUT_TWL6040_VIBRA)	+= twl6040-vibra.o
diff --git a/drivers/input/misc/tps6594-pwrbutton.c b/drivers/input/misc/tps6594-pwrbutton.c
new file mode 100644
index 000000000000..cd039b3866dc
--- /dev/null
+++ b/drivers/input/misc/tps6594-pwrbutton.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * power button driver for TI TPS6594 PMICs
+ *
+ * Copyright (C) 2025 Critical Link LLC - https://www.criticallink.com/
+ */
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/tps6594.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+struct tps6594_pwrbutton {
+	struct device *dev;
+	struct input_dev *idev;
+	char phys[32];
+};
+
+static irqreturn_t tps6594_pb_push_irq(int irq, void *_pwr)
+{
+	struct tps6594_pwrbutton *pwr = _pwr;
+
+	input_report_key(pwr->idev, KEY_POWER, 1);
+	pm_wakeup_event(pwr->dev, 0);
+	input_sync(pwr->idev);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t tps6594_pb_release_irq(int irq, void *_pwr)
+{
+	struct tps6594_pwrbutton *pwr = _pwr;
+
+	input_report_key(pwr->idev, KEY_POWER, 0);
+	input_sync(pwr->idev);
+
+	return IRQ_HANDLED;
+}
+
+static int tps6594_pb_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct tps6594_pwrbutton *pwr;
+	struct input_dev *idev;
+	int error;
+	int push_irq;
+	int release_irq;
+
+	pwr = devm_kzalloc(dev, sizeof(*pwr), GFP_KERNEL);
+	if (!pwr)
+		return -ENOMEM;
+
+	idev = devm_input_allocate_device(dev);
+	if (!idev)
+		return -ENOMEM;
+
+	idev->name = pdev->name;
+	snprintf(pwr->phys, sizeof(pwr->phys), "%s/input0",
+		 pdev->name);
+	idev->phys = pwr->phys;
+	idev->id.bustype = BUS_I2C;
+
+	input_set_capability(idev, EV_KEY, KEY_POWER);
+
+	pwr->dev = dev;
+	pwr->idev = idev;
+	device_init_wakeup(dev, true);
+
+	push_irq = platform_get_irq(pdev, 0);
+	if (push_irq < 0)
+		return -EINVAL;
+
+	release_irq = platform_get_irq(pdev, 1);
+	if (release_irq < 0)
+		return -EINVAL;
+
+	error = devm_request_threaded_irq(dev, push_irq, NULL,
+					  tps6594_pb_push_irq,
+					  IRQF_ONESHOT,
+					  pdev->resource[0].name, pwr);
+	if (error) {
+		dev_err(dev, "failed to request push IRQ #%d: %d\n", push_irq,
+			error);
+		return error;
+	}
+
+	error = devm_request_threaded_irq(dev, release_irq, NULL,
+					  tps6594_pb_release_irq,
+					  IRQF_ONESHOT,
+					  pdev->resource[1].name, pwr);
+	if (error) {
+		dev_err(dev, "failed to request release IRQ #%d: %d\n",
+			release_irq, error);
+		return error;
+	}
+
+	error = input_register_device(idev);
+	if (error) {
+		dev_err(dev, "Can't register power button: %d\n", error);
+		return error;
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id tps6594_pwrbtn_id_table[] = {
+	{ "tps6594-pwrbutton", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, tps6594_pwrbtn_id_table);
+
+static struct platform_driver tps6594_pb_driver = {
+	.probe = tps6594_pb_probe,
+	.driver = {
+		.name = "tps6594_pwrbutton",
+	},
+	.id_table = tps6594_pwrbtn_id_table,
+};
+module_platform_driver(tps6594_pb_driver);
+
+MODULE_DESCRIPTION("TPS6594 Power Button");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/tps6594-core.c b/drivers/mfd/tps6594-core.c
index a7223e873cd1..1b0b3d1bf6c4 100644
--- a/drivers/mfd/tps6594-core.c
+++ b/drivers/mfd/tps6594-core.c
@@ -123,6 +123,13 @@ static const struct resource tps6594_rtc_resources[] = {
 	DEFINE_RES_IRQ_NAMED(TPS6594_IRQ_POWER_UP, TPS6594_IRQ_NAME_POWERUP),
 };
 
+
+static const struct resource tps6594_pwrbutton_resources[] = {
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_FALL, TPS65224_IRQ_NAME_PB_FALL),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_RISE, TPS65224_IRQ_NAME_PB_RISE),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_SHORT, TPS65224_IRQ_NAME_PB_SHORT),
+};
+
 static const struct mfd_cell tps6594_common_cells[] = {
 	MFD_CELL_RES("tps6594-regulator", tps6594_regulator_resources),
 	MFD_CELL_RES("tps6594-pinctrl", tps6594_pinctrl_resources),
@@ -313,8 +320,6 @@ static const struct resource tps65224_pfsm_resources[] = {
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_REG_UNLOCK, TPS65224_IRQ_NAME_REG_UNLOCK),
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_TWARN, TPS65224_IRQ_NAME_TWARN),
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_LONG, TPS65224_IRQ_NAME_PB_LONG),
-	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_FALL, TPS65224_IRQ_NAME_PB_FALL),
-	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_RISE, TPS65224_IRQ_NAME_PB_RISE),
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_TSD_ORD, TPS65224_IRQ_NAME_TSD_ORD),
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BIST_FAIL, TPS65224_IRQ_NAME_BIST_FAIL),
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_REG_CRC_ERR, TPS65224_IRQ_NAME_REG_CRC_ERR),
@@ -342,6 +347,12 @@ static const struct mfd_cell tps65224_common_cells[] = {
 	MFD_CELL_RES("tps6594-regulator", tps65224_regulator_resources),
 };
 
+static const struct mfd_cell tps6594_pwrbutton_cell = {
+	.name = "tps6594-pwrbutton",
+	.resources = tps6594_pwrbutton_resources,
+	.num_resources = ARRAY_SIZE(tps6594_pwrbutton_resources),
+};
+
 static const struct regmap_irq tps65224_irqs[] = {
 	/* INT_BUCK register */
 	REGMAP_IRQ_REG(TPS65224_IRQ_BUCK1_UVOV, 0, TPS65224_BIT_BUCK1_UVOV_INT),
@@ -611,6 +622,7 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 	struct regmap_irq_chip *irq_chip;
 	const struct mfd_cell *cells;
 	int n_cells;
+	bool pwr_button;
 
 	if (enable_crc) {
 		ret = tps6594_enable_crc(tps);
@@ -651,6 +663,15 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to add common child devices\n");
 
+	pwr_button = of_property_read_bool(dev->of_node, "ti,power-button");
+	if (pwr_button) {
+		ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
+						&tps6594_pwrbutton_cell, 1, NULL, 0,
+						regmap_irq_get_domain(tps->irq_data));
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to add power-button: %d\n", ret);
+	}
+
 	/* No RTC for LP8764 and TPS65224 */
 	if (tps->chip_id != LP8764 && tps->chip_id != TPS65224) {
 		ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, tps6594_rtc_cells,

-- 
2.43.0

[PATCH 3/3] mfd: tps6594: Adds support for powering off the PMIC

[Job Sava]

When the FSM_I2C_TRIGGER register's bit 0 is set it triggers TRIGGER_I2C_0
and the PMIC is transitioned to the STANDBY state
(table 6-18: SLVSGG7 – DECEMBER 2023).

An ON request is required to transition from STANDBY to ACTIVE.

Signed-off-by: Job Sava <jsava@criticallink.com>
---
 drivers/mfd/tps6594-core.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/mfd/tps6594-core.c b/drivers/mfd/tps6594-core.c
index 1b0b3d1bf6c4..f4c434c0d87a 100644
--- a/drivers/mfd/tps6594-core.c
+++ b/drivers/mfd/tps6594-core.c
@@ -10,6 +10,7 @@
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/reboot.h>
 
 #include <linux/mfd/core.h>
 #include <linux/mfd/tps6594.h>
@@ -615,6 +616,19 @@ static int tps6594_enable_crc(struct tps6594 *tps)
 	return ret;
 }
 
+static int tps6594_soft_shutdown(struct tps6594 *tps)
+{
+	return regmap_update_bits(tps->regmap, TPS6594_REG_FSM_I2C_TRIGGERS,
+				TPS6594_BIT_TRIGGER_I2C(0),
+				TPS6594_BIT_TRIGGER_I2C(0));
+}
+
+static int tps6594_power_off_handler(struct sys_off_data *data)
+{
+	tps6594_soft_shutdown(data->cb_data);
+	return NOTIFY_DONE;
+}
+
 int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 {
 	struct device *dev = tps->dev;
@@ -623,6 +637,7 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 	const struct mfd_cell *cells;
 	int n_cells;
 	bool pwr_button;
+	bool system_power_controller;
 
 	if (enable_crc) {
 		ret = tps6594_enable_crc(tps);
@@ -681,6 +696,15 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 			return dev_err_probe(dev, ret, "Failed to add RTC child device\n");
 	}
 
+	system_power_controller = of_property_read_bool(dev->of_node, "system-power-controller");
+	if (system_power_controller) {
+		ret = devm_register_power_off_handler(tps->dev,
+								tps6594_power_off_handler,
+								tps);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to register power-off handler\n");
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(tps6594_device_init);

-- 
2.43.0

Re: [PATCH 2/3] mfd: tps6594-pwrbutton: Add powerbutton functionality

[Dmitry Torokhov]

On Tue, May 20, 2025 at 01:43:37PM -0400, Job Sava wrote:
> TPS6594 defines two interrupts for the powerbutton one for push and
> one for release.
> 
> This driver is very simple in that it maps the push interrupt to a key
> input and the release interrupt to a key release.
> 
> Signed-off-by: Job Sava <jsava@criticallink.com>

For the input part:

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.

-- 
Dmitry

Re: [PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Krzysztof Kozlowski]

On Tue, May 20, 2025 at 01:43:36PM GMT, Job Sava wrote:
> The TPS6594 power-button option permits users to enter STANDBY or
> ACTIVE state by a push, release, or short push button request.
> 
> Signed-off-by: Job Sava <jsava@criticallink.com>
> ---
>  Documentation/devicetree/bindings/mfd/ti,tps6594.yaml | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> index 6341b6070366..a40808fd2747 100644
> --- a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> +++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> @@ -37,6 +37,21 @@ properties:
>        device on the SPMI bus, and the secondary PMICs are the target devices
>        on the SPMI bus.
>  
> +  ti,power-button:
> +    type: boolean
> +    description: |
> +      Optional property that sets the EN/PB/VSENSE pin to be a
> +      power-button.
> +      TPS6594 has a multipurpose pin called EN/PB/VSENSE that can be either
> +      1. EN in which case it functions as an enable pin.
> +      2. VSENSE which compares the voltages and triggers an automatic
> +      on/off request.
> +      3. PB in which case it can be configured to trigger an interrupt
> +      to the SoC.
> +      ti,power-button reflects the last one of those options
> +      where the board has a button wired to the pin and triggers
> +      an interrupt on pressing it.

Don't you need to handle two other cases as well? I assume you copied
this from the other binding, but all three options are valid?

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Job Sava]

On Wed, May 21, 2025 at 6:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Tue, May 20, 2025 at 01:43:36PM GMT, Job Sava wrote:
> > The TPS6594 power-button option permits users to enter STANDBY or
> > ACTIVE state by a push, release, or short push button request.
> >
> > Signed-off-by: Job Sava <jsava@criticallink.com>
> > ---
> >  Documentation/devicetree/bindings/mfd/ti,tps6594.yaml | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> > index 6341b6070366..a40808fd2747 100644
> > --- a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> > @@ -37,6 +37,21 @@ properties:
> >        device on the SPMI bus, and the secondary PMICs are the target devices
> >        on the SPMI bus.
> >
> > +  ti,power-button:
> > +    type: boolean
> > +    description: |
> > +      Optional property that sets the EN/PB/VSENSE pin to be a
> > +      power-button.
> > +      TPS6594 has a multipurpose pin called EN/PB/VSENSE that can be either
> > +      1. EN in which case it functions as an enable pin.
> > +      2. VSENSE which compares the voltages and triggers an automatic
> > +      on/off request.
> > +      3. PB in which case it can be configured to trigger an interrupt
> > +      to the SoC.
> > +      ti,power-button reflects the last one of those options
> > +      where the board has a button wired to the pin and triggers
> > +      an interrupt on pressing it.
>
> Don't you need to handle two other cases as well? I assume you copied
> this from the other binding, but all three options are valid?
>
> Best regards,
> Krzysztof
>
Hello Krzysztof,

Thank you for your response!

I agree that the other two cases are valid options. However, for this
particular patch series, they may be out of scope. The primary goal of
this patch is to enable push-button functionality, rather than
addressing the VSENSE or EN modes.

Thanks again for the feedback.

Best regards,
Job

Re: [PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Krzysztof Kozlowski]

On Fri, May 23, 2025 at 09:46:49AM GMT, Job Sava wrote:
> On Wed, May 21, 2025 at 6:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Tue, May 20, 2025 at 01:43:36PM GMT, Job Sava wrote:
> > > The TPS6594 power-button option permits users to enter STANDBY or
> > > ACTIVE state by a push, release, or short push button request.
> > >
> > > Signed-off-by: Job Sava <jsava@criticallink.com>
> > > ---
> > >  Documentation/devicetree/bindings/mfd/ti,tps6594.yaml | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> > > index 6341b6070366..a40808fd2747 100644
> > > --- a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> > > +++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> > > @@ -37,6 +37,21 @@ properties:
> > >        device on the SPMI bus, and the secondary PMICs are the target devices
> > >        on the SPMI bus.
> > >
> > > +  ti,power-button:
> > > +    type: boolean
> > > +    description: |
> > > +      Optional property that sets the EN/PB/VSENSE pin to be a
> > > +      power-button.
> > > +      TPS6594 has a multipurpose pin called EN/PB/VSENSE that can be either
> > > +      1. EN in which case it functions as an enable pin.
> > > +      2. VSENSE which compares the voltages and triggers an automatic
> > > +      on/off request.
> > > +      3. PB in which case it can be configured to trigger an interrupt
> > > +      to the SoC.
> > > +      ti,power-button reflects the last one of those options
> > > +      where the board has a button wired to the pin and triggers
> > > +      an interrupt on pressing it.
> >
> > Don't you need to handle two other cases as well? I assume you copied
> > this from the other binding, but all three options are valid?
> >
> > Best regards,
> > Krzysztof
> >
> Hello Krzysztof,
> 
> Thank you for your response!
> 
> I agree that the other two cases are valid options. However, for this
> particular patch series, they may be out of scope. The primary goal of
> this patch is to enable push-button functionality, rather than
> addressing the VSENSE or EN modes.

Binding should be complete, because if you design this as bool, it
cannot be later changed to three-state (enum).

I don't know if the EN and VSENSE modes are anyhow useful, maybe people
interested in this hardware should say.

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Job Sava]

On Thu, May 29, 2025 at 5:26 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Fri, May 23, 2025 at 09:46:49AM GMT, Job Sava wrote:
> > On Wed, May 21, 2025 at 6:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > On Tue, May 20, 2025 at 01:43:36PM GMT, Job Sava wrote:
> > > > The TPS6594 power-button option permits users to enter STANDBY or
> > > > ACTIVE state by a push, release, or short push button request.
> > > >
> > > > Signed-off-by: Job Sava <jsava@criticallink.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/mfd/ti,tps6594.yaml | 15 +++++++++++++++
> > > >  1 file changed, 15 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> > > > index 6341b6070366..a40808fd2747 100644
> > > > --- a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> > > > +++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> > > > @@ -37,6 +37,21 @@ properties:
> > > >        device on the SPMI bus, and the secondary PMICs are the target devices
> > > >        on the SPMI bus.
> > > >
> > > > +  ti,power-button:
> > > > +    type: boolean
> > > > +    description: |
> > > > +      Optional property that sets the EN/PB/VSENSE pin to be a
> > > > +      power-button.
> > > > +      TPS6594 has a multipurpose pin called EN/PB/VSENSE that can be either
> > > > +      1. EN in which case it functions as an enable pin.
> > > > +      2. VSENSE which compares the voltages and triggers an automatic
> > > > +      on/off request.
> > > > +      3. PB in which case it can be configured to trigger an interrupt
> > > > +      to the SoC.
> > > > +      ti,power-button reflects the last one of those options
> > > > +      where the board has a button wired to the pin and triggers
> > > > +      an interrupt on pressing it.
> > >
> > > Don't you need to handle two other cases as well? I assume you copied
> > > this from the other binding, but all three options are valid?
> > >
> > > Best regards,
> > > Krzysztof
> > >
> > Hello Krzysztof,
> >
> > Thank you for your response!
> >
> > I agree that the other two cases are valid options. However, for this
> > particular patch series, they may be out of scope. The primary goal of
> > this patch is to enable push-button functionality, rather than
> > addressing the VSENSE or EN modes.
>
> Binding should be complete, because if you design this as bool, it
> cannot be later changed to three-state (enum).
>
> I don't know if the EN and VSENSE modes are anyhow useful, maybe people
> interested in this hardware should say.
>
> Best regards,
> Krzysztof
>

Hi Krzysztof,

Thanks again for the feedback.

I modeled this binding after the TPS65219 PMIC, which uses a boolean
for ti,power-button, despite the same EN/PB/VSENSE options. Since this
patch only enables PB mode, I felt a boolean was appropriate and
consistent.

That said, if you think an enum is strongly preferred here, I’m happy
to rework it accordingly.

Best regards,
Job Sava

Re: [PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Krzysztof Kozlowski]

On 02/06/2025 15:07, Job Sava wrote:
> On Thu, May 29, 2025 at 5:26 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On Fri, May 23, 2025 at 09:46:49AM GMT, Job Sava wrote:
>>> On Wed, May 21, 2025 at 6:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On Tue, May 20, 2025 at 01:43:36PM GMT, Job Sava wrote:
>>>>> The TPS6594 power-button option permits users to enter STANDBY or
>>>>> ACTIVE state by a push, release, or short push button request.
>>>>>
>>>>> Signed-off-by: Job Sava <jsava@criticallink.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/mfd/ti,tps6594.yaml | 15 +++++++++++++++
>>>>>  1 file changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
>>>>> index 6341b6070366..a40808fd2747 100644
>>>>> --- a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
>>>>> +++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
>>>>> @@ -37,6 +37,21 @@ properties:
>>>>>        device on the SPMI bus, and the secondary PMICs are the target devices
>>>>>        on the SPMI bus.
>>>>>
>>>>> +  ti,power-button:
>>>>> +    type: boolean
>>>>> +    description: |
>>>>> +      Optional property that sets the EN/PB/VSENSE pin to be a
>>>>> +      power-button.
>>>>> +      TPS6594 has a multipurpose pin called EN/PB/VSENSE that can be either
>>>>> +      1. EN in which case it functions as an enable pin.
>>>>> +      2. VSENSE which compares the voltages and triggers an automatic
>>>>> +      on/off request.
>>>>> +      3. PB in which case it can be configured to trigger an interrupt
>>>>> +      to the SoC.
>>>>> +      ti,power-button reflects the last one of those options
>>>>> +      where the board has a button wired to the pin and triggers
>>>>> +      an interrupt on pressing it.
>>>>
>>>> Don't you need to handle two other cases as well? I assume you copied
>>>> this from the other binding, but all three options are valid?
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>> Hello Krzysztof,
>>>
>>> Thank you for your response!
>>>
>>> I agree that the other two cases are valid options. However, for this
>>> particular patch series, they may be out of scope. The primary goal of
>>> this patch is to enable push-button functionality, rather than
>>> addressing the VSENSE or EN modes.
>>
>> Binding should be complete, because if you design this as bool, it
>> cannot be later changed to three-state (enum).
>>
>> I don't know if the EN and VSENSE modes are anyhow useful, maybe people
>> interested in this hardware should say.
>>
>> Best regards,
>> Krzysztof
>>
> 
> Hi Krzysztof,
> 
> Thanks again for the feedback.
> 
> I modeled this binding after the TPS65219 PMIC, which uses a boolean

Yeah, that's what I meant in my first reply.

> for ti,power-button, despite the same EN/PB/VSENSE options. Since this
> patch only enables PB mode, I felt a boolean was appropriate and
> consistent.

Properties should have only one type, so that would be a different
property. Someone knowing the device should come with arguments whether
other states for this are useful at all. Or not useful and then argument
that in commit msg for example.


Best regards,
Krzysztof

Re: [PATCH 2/3] mfd: tps6594-pwrbutton: Add powerbutton functionality

[Lee Jones]

On Tue, 20 May 2025, Job Sava wrote:

> TPS6594 defines two interrupts for the powerbutton one for push and
> one for release.
> 
> This driver is very simple in that it maps the push interrupt to a key
> input and the release interrupt to a key release.
> 
> Signed-off-by: Job Sava <jsava@criticallink.com>
> ---
>  drivers/input/misc/Kconfig             |  10 +++
>  drivers/input/misc/Makefile            |   1 +
>  drivers/input/misc/tps6594-pwrbutton.c | 126 +++++++++++++++++++++++++++++++++
>  drivers/mfd/tps6594-core.c             |  25 ++++++-

This should be a separate patch.

>  4 files changed, 160 insertions(+), 2 deletions(-)

[...]

-- 
Lee Jones [李琼斯]

Re: [PATCH 3/3] mfd: tps6594: Adds support for powering off the PMIC

[Lee Jones]

On Tue, 20 May 2025, Job Sava wrote:

> When the FSM_I2C_TRIGGER register's bit 0 is set it triggers TRIGGER_I2C_0
> and the PMIC is transitioned to the STANDBY state
> (table 6-18: SLVSGG7 – DECEMBER 2023).
> 
> An ON request is required to transition from STANDBY to ACTIVE.
> 
> Signed-off-by: Job Sava <jsava@criticallink.com>
> ---
>  drivers/mfd/tps6594-core.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/mfd/tps6594-core.c b/drivers/mfd/tps6594-core.c
> index 1b0b3d1bf6c4..f4c434c0d87a 100644
> --- a/drivers/mfd/tps6594-core.c
> +++ b/drivers/mfd/tps6594-core.c
> @@ -10,6 +10,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/reboot.h>
>  
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/tps6594.h>
> @@ -615,6 +616,19 @@ static int tps6594_enable_crc(struct tps6594 *tps)
>  	return ret;
>  }
>  
> +static int tps6594_soft_shutdown(struct tps6594 *tps)

Why do you have a whole separate function that itself is only called
once and only conducts a single one call to one other function?

> +{
> +	return regmap_update_bits(tps->regmap, TPS6594_REG_FSM_I2C_TRIGGERS,
> +				TPS6594_BIT_TRIGGER_I2C(0),
> +				TPS6594_BIT_TRIGGER_I2C(0));
> +}
> +
> +static int tps6594_power_off_handler(struct sys_off_data *data)
> +{
> +	tps6594_soft_shutdown(data->cb_data);
> +	return NOTIFY_DONE;
> +}
> +
>  int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
>  {
>  	struct device *dev = tps->dev;
> @@ -623,6 +637,7 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
>  	const struct mfd_cell *cells;
>  	int n_cells;
>  	bool pwr_button;
> +	bool system_power_controller;
>  
>  	if (enable_crc) {
>  		ret = tps6594_enable_crc(tps);
> @@ -681,6 +696,15 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
>  			return dev_err_probe(dev, ret, "Failed to add RTC child device\n");
>  	}
>  
> +	system_power_controller = of_property_read_bool(dev->of_node, "system-power-controller");
> +	if (system_power_controller) {
> +		ret = devm_register_power_off_handler(tps->dev,
> +								tps6594_power_off_handler,
> +								tps);

This alignment is odd.

> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to register power-off handler\n");
> +	}
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(tps6594_device_init);
> 
> -- 
> 2.43.0
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH 2/3] mfd: tps6594-pwrbutton: Add powerbutton functionality

[Job Sava]

On Fri, Jun 13, 2025 at 10:09 AM Lee Jones <lee@kernel.org> wrote:
>
> On Tue, 20 May 2025, Job Sava wrote:
>
> > TPS6594 defines two interrupts for the powerbutton one for push and
> > one for release.
> >
> > This driver is very simple in that it maps the push interrupt to a key
> > input and the release interrupt to a key release.
> >
> > Signed-off-by: Job Sava <jsava@criticallink.com>
> > ---
> >  drivers/input/misc/Kconfig             |  10 +++
> >  drivers/input/misc/Makefile            |   1 +
> >  drivers/input/misc/tps6594-pwrbutton.c | 126 +++++++++++++++++++++++++++++++++
> >  drivers/mfd/tps6594-core.c             |  25 ++++++-
>
> This should be a separate patch.

Hello Lee,

Thank you for the response!

Sure thing I will convert this into a separate patch.

Best Regards,
- Job

>
> >  4 files changed, 160 insertions(+), 2 deletions(-)
>
> [...]
>
> --
> Lee Jones [李琼斯]

Re: [PATCH 3/3] mfd: tps6594: Adds support for powering off the PMIC

[Job Sava]

On Fri, Jun 13, 2025 at 10:11 AM Lee Jones <lee@kernel.org> wrote:
>
> On Tue, 20 May 2025, Job Sava wrote:
>
> > When the FSM_I2C_TRIGGER register's bit 0 is set it triggers TRIGGER_I2C_0
> > and the PMIC is transitioned to the STANDBY state
> > (table 6-18: SLVSGG7 – DECEMBER 2023).
> >
> > An ON request is required to transition from STANDBY to ACTIVE.
> >
> > Signed-off-by: Job Sava <jsava@criticallink.com>
> > ---
> >  drivers/mfd/tps6594-core.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/mfd/tps6594-core.c b/drivers/mfd/tps6594-core.c
> > index 1b0b3d1bf6c4..f4c434c0d87a 100644
> > --- a/drivers/mfd/tps6594-core.c
> > +++ b/drivers/mfd/tps6594-core.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/interrupt.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> > +#include <linux/reboot.h>
> >
> >  #include <linux/mfd/core.h>
> >  #include <linux/mfd/tps6594.h>
> > @@ -615,6 +616,19 @@ static int tps6594_enable_crc(struct tps6594 *tps)
> >       return ret;
> >  }
> >
> > +static int tps6594_soft_shutdown(struct tps6594 *tps)
>
> Why do you have a whole separate function that itself is only called
> once and only conducts a single one call to one other function?

I copied this code from the tps65219 pmic driver, which setup the
functions in this way. I will merge the two functions.

>
> > +{
> > +     return regmap_update_bits(tps->regmap, TPS6594_REG_FSM_I2C_TRIGGERS,
> > +                             TPS6594_BIT_TRIGGER_I2C(0),
> > +                             TPS6594_BIT_TRIGGER_I2C(0));
> > +}
> > +
> > +static int tps6594_power_off_handler(struct sys_off_data *data)
> > +{
> > +     tps6594_soft_shutdown(data->cb_data);
> > +     return NOTIFY_DONE;
> > +}
> > +
> >  int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
> >  {
> >       struct device *dev = tps->dev;
> > @@ -623,6 +637,7 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
> >       const struct mfd_cell *cells;
> >       int n_cells;
> >       bool pwr_button;
> > +     bool system_power_controller;
> >
> >       if (enable_crc) {
> >               ret = tps6594_enable_crc(tps);
> > @@ -681,6 +696,15 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
> >                       return dev_err_probe(dev, ret, "Failed to add RTC child device\n");
> >       }
> >
> > +     system_power_controller = of_property_read_bool(dev->of_node, "system-power-controller");
> > +     if (system_power_controller) {
> > +             ret = devm_register_power_off_handler(tps->dev,
> > +                                                             tps6594_power_off_handler,
> > +                                                             tps);
>
> This alignment is odd.

Will have this fixed for the next patch.

>
> > +             if (ret)
> > +                     return dev_err_probe(dev, ret, "Failed to register power-off handler\n");
> > +     }
> > +
> >       return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(tps6594_device_init);
> >
> > --
> > 2.43.0
> >
>
> --
> Lee Jones [李琼斯]

Thank you
-Job

Re: [PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Job Sava]

On Tue, Jun 3, 2025 at 2:52 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 02/06/2025 15:07, Job Sava wrote:
> > On Thu, May 29, 2025 at 5:26 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On Fri, May 23, 2025 at 09:46:49AM GMT, Job Sava wrote:
> >>> On Wed, May 21, 2025 at 6:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>>>
> >>>> On Tue, May 20, 2025 at 01:43:36PM GMT, Job Sava wrote:
> >>>>> The TPS6594 power-button option permits users to enter STANDBY or
> >>>>> ACTIVE state by a push, release, or short push button request.
> >>>>>
> >>>>> Signed-off-by: Job Sava <jsava@criticallink.com>
> >>>>> ---
> >>>>>  Documentation/devicetree/bindings/mfd/ti,tps6594.yaml | 15 +++++++++++++++
> >>>>>  1 file changed, 15 insertions(+)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> >>>>> index 6341b6070366..a40808fd2747 100644
> >>>>> --- a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
> >>>>> @@ -37,6 +37,21 @@ properties:
> >>>>>        device on the SPMI bus, and the secondary PMICs are the target devices
> >>>>>        on the SPMI bus.
> >>>>>
> >>>>> +  ti,power-button:
> >>>>> +    type: boolean
> >>>>> +    description: |
> >>>>> +      Optional property that sets the EN/PB/VSENSE pin to be a
> >>>>> +      power-button.
> >>>>> +      TPS6594 has a multipurpose pin called EN/PB/VSENSE that can be either
> >>>>> +      1. EN in which case it functions as an enable pin.
> >>>>> +      2. VSENSE which compares the voltages and triggers an automatic
> >>>>> +      on/off request.
> >>>>> +      3. PB in which case it can be configured to trigger an interrupt
> >>>>> +      to the SoC.
> >>>>> +      ti,power-button reflects the last one of those options
> >>>>> +      where the board has a button wired to the pin and triggers
> >>>>> +      an interrupt on pressing it.
> >>>>
> >>>> Don't you need to handle two other cases as well? I assume you copied
> >>>> this from the other binding, but all three options are valid?
> >>>>
> >>>> Best regards,
> >>>> Krzysztof
> >>>>
> >>> Hello Krzysztof,
> >>>
> >>> Thank you for your response!
> >>>
> >>> I agree that the other two cases are valid options. However, for this
> >>> particular patch series, they may be out of scope. The primary goal of
> >>> this patch is to enable push-button functionality, rather than
> >>> addressing the VSENSE or EN modes.
> >>
> >> Binding should be complete, because if you design this as bool, it
> >> cannot be later changed to three-state (enum).
> >>
> >> I don't know if the EN and VSENSE modes are anyhow useful, maybe people
> >> interested in this hardware should say.
> >>
> >> Best regards,
> >> Krzysztof
> >>
> >
> > Hi Krzysztof,
> >
> > Thanks again for the feedback.
> >
> > I modeled this binding after the TPS65219 PMIC, which uses a boolean
>
> Yeah, that's what I meant in my first reply.
>
> > for ti,power-button, despite the same EN/PB/VSENSE options. Since this
> > patch only enables PB mode, I felt a boolean was appropriate and
> > consistent.
>
> Properties should have only one type, so that would be a different
> property.
Yes, the type is boolean.
> Someone knowing the device should come with arguments whether
> other states for this are useful at all. Or not useful and then argument
> that in commit msg for example.
The other states are not useful for the kernel. Only the push button
has a need for an interrupt handler. The other states the PMIC handles
on its own.

What exactly do you want me to change?

Best regards,
-Job
>
>
> Best regards,
> Krzysztof

Re: [PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Michael Walle]

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

Hi,

> > Someone knowing the device should come with arguments whether
> > other states for this are useful at all. Or not useful and then argument
> > that in commit msg for example.
> The other states are not useful for the kernel. Only the push button
> has a need for an interrupt handler. The other states the PMIC handles
> on its own.
>
> What exactly do you want me to change?

Because the driver isn't setting the configuration anyway, wouldn't
it be possible to read the config bits (Register 0x3c, bits 7-6) to
figure out whether the pin is configured as power-button instead of
having this property?

I mean, the correct config is likely stored in the NVM anyway, and
reconfiguring it to another value seems unlikely.

-michael

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

Re: [PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Jon Cormier]

On Thu, Jul 17, 2025 at 9:58 AM Michael Walle <michael@walle.cc> wrote:
>
> Hi,
>
> > > Someone knowing the device should come with arguments whether
> > > other states for this are useful at all. Or not useful and then argument
> > > that in commit msg for example.
> > The other states are not useful for the kernel. Only the push button
> > has a need for an interrupt handler. The other states the PMIC handles
> > on its own.
> >
> > What exactly do you want me to change?
>
> Because the driver isn't setting the configuration anyway, wouldn't
> it be possible to read the config bits (Register 0x3c, bits 7-6) to
> figure out whether the pin is configured as power-button instead of
> having this property?
>
> I mean, the correct config is likely stored in the NVM anyway, and
> reconfiguring it to another value seems unlikely.
Currently, the TPS MFD driver only loads the power button driver if
the flag is set.  We could put that discovery code in the MFD driver,
but what if the system designer doesn't want the power button driver?
I'm not sure auto detecting it makes sense.

We are basing this on the other TI PMIC drivers and how they are
configured. I'm not sure I want to reinvent the wheel, so to speak.
>
> -michael




--
Jonathan Cormier
Senior Software Engineer

Voice:  315.425.4045 x222

http://www.CriticalLink.com
6712 Brooklawn Parkway, Syracuse, NY 13211

Re: [PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Michael Walle]

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

[+ Jerome and Markus ]

Hi,

> > > > Someone knowing the device should come with arguments whether
> > > > other states for this are useful at all. Or not useful and then argument
> > > > that in commit msg for example.
> > > The other states are not useful for the kernel. Only the push button
> > > has a need for an interrupt handler. The other states the PMIC handles
> > > on its own.
> > >
> > > What exactly do you want me to change?
> >
> > Because the driver isn't setting the configuration anyway, wouldn't
> > it be possible to read the config bits (Register 0x3c, bits 7-6) to
> > figure out whether the pin is configured as power-button instead of
> > having this property?
> >
> > I mean, the correct config is likely stored in the NVM anyway, and
> > reconfiguring it to another value seems unlikely.
> Currently, the TPS MFD driver only loads the power button driver if
> the flag is set.  We could put that discovery code in the MFD driver,
> but what if the system designer doesn't want the power button driver?

The device tree is not for configuration. The designer can just
ignore the input event in that case.

> I'm not sure auto detecting it makes sense.

Why?

> We are basing this on the other TI PMIC drivers and how they are
> configured. I'm not sure I want to reinvent the wheel, so to speak.

That was never a good reason. Maybe there was a reason for the
TPS65219. Markus? Jerome? I haven't found anything in the commit
messages or cover letters. Only that it is "optional". Not sure what
that means. According to the TPS65219 datasheet, that pin if not
used shall be configured as EN and be connected to VSYS.

-michael

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

Re: [PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Markus Schneider-Pargmann]

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

Hi,

I think my mail wasn't sent properly, so here we go again:

On Mon Jul 21, 2025 at 8:42 AM CEST, Michael Walle wrote:
> [+ Jerome and Markus ]
>
> Hi,
>
>> > > > Someone knowing the device should come with arguments whether
>> > > > other states for this are useful at all. Or not useful and then argument
>> > > > that in commit msg for example.
>> > > The other states are not useful for the kernel. Only the push button
>> > > has a need for an interrupt handler. The other states the PMIC handles
>> > > on its own.
>> > >
>> > > What exactly do you want me to change?
>> >
>> > Because the driver isn't setting the configuration anyway, wouldn't
>> > it be possible to read the config bits (Register 0x3c, bits 7-6) to
>> > figure out whether the pin is configured as power-button instead of
>> > having this property?
>> >
>> > I mean, the correct config is likely stored in the NVM anyway, and
>> > reconfiguring it to another value seems unlikely.
>> Currently, the TPS MFD driver only loads the power button driver if
>> the flag is set.  We could put that discovery code in the MFD driver,
>> but what if the system designer doesn't want the power button driver?
>
> The device tree is not for configuration. The designer can just
> ignore the input event in that case.
>
>> I'm not sure auto detecting it makes sense.
>
> Why?
>
>> We are basing this on the other TI PMIC drivers and how they are
>> configured. I'm not sure I want to reinvent the wheel, so to speak.
>
> That was never a good reason. Maybe there was a reason for the
> TPS65219. Markus? Jerome? I haven't found anything in the commit
> messages or cover letters. Only that it is "optional". Not sure what
> that means. According to the TPS65219 datasheet, that pin if not
> used shall be configured as EN and be connected to VSYS.

I don't think the TPS65219 has a config register to detect if the pin is
a power-button that's why a devicetree description was necessary.
Looking at it now, it should probably have been an enum for TPS65219. It
is not relevant to any software but it is not describing the
configuration fully.

Best
Markus

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

Re: [PATCH 0/3] Powerbutton driver and powerdown request for TPS65224 PMIC

[Michael Walle]

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

Hi Job,

> The following patches were created to get the tps65224 PMIC
> powerbutton driver and power off request working on the
> MitySOM-AM62PX. The patches are as follows:

Are there any news on this series? Do you plan to post a new
version soon, or do you want me to take over? I'd like to get
support for this into the kernel :)

-michael

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

Re: [PATCH 0/3] Powerbutton driver and powerdown request for TPS65224 PMIC

[Michael Walle]

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

On Tue Aug 19, 2025 at 1:27 PM CEST, Michael Walle wrote:
> Hi Job,
>
> > The following patches were created to get the tps65224 PMIC
> > powerbutton driver and power off request working on the
> > MitySOM-AM62PX. The patches are as follows:
>
> Are there any news on this series? Do you plan to post a new
> version soon, or do you want me to take over? I'd like to get
> support for this into the kernel :)

FWIW, jsava@criticallink.com is bouncing.

|| <jsava@criticallink.com>: host aspmx.l.google.com[2607:f8b0:4023:1004::1b]
||     said: 550-5.1.1 The email account that you tried to reach does not exist.

-michael

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

Re: [PATCH 0/3] Powerbutton driver and powerdown request for TPS65224 PMIC

[Jon Cormier]

On Tue, Aug 19, 2025 at 7:30 AM Michael Walle <mwalle@kernel.org> wrote:
>
> On Tue Aug 19, 2025 at 1:27 PM CEST, Michael Walle wrote:
> > Hi Job,
> >
> > > The following patches were created to get the tps65224 PMIC
> > > powerbutton driver and power off request working on the
> > > MitySOM-AM62PX. The patches are as follows:
> >
> > Are there any news on this series? Do you plan to post a new
> > version soon, or do you want me to take over? I'd like to get
> > support for this into the kernel :)
>
> FWIW, jsava@criticallink.com is bouncing.
Yeah sorry, Job has gone back to school, he was on an internship.

I'm not entirely sure how best to move forward with this change.
There have been several suggestions thrown out and I'm a little lost
on what's best/easiest.

If you want to take over and add us as co authors that would be
greatly appreciated.  I can get his personal email if that makes
sense.  I'm not sure how the kernel normally deals with short term
emails (interns).

Note we are currently using the driver as is, on an Android prototype
and for some reason Android isn't able to detect the power button
hold, and open the power menu. Single push works fine to turn off the
screen.  I haven't looked into it yet, to see if this is an Android
issue or if we are using the POWER key events wrong. Just thought I'd
mention it.


-- 
Jonathan Cormier
Senior Software Engineer

Voice:  315.425.4045 x222

http://www.CriticalLink.com
6712 Brooklawn Parkway, Syracuse, NY 13211

Re: [PATCH 0/3] Powerbutton driver and powerdown request for TPS65224 PMIC

[Michael Walle]

Hi,

On Tue Aug 19, 2025 at 6:22 PM CEST, Jon Cormier wrote:
> On Tue, Aug 19, 2025 at 7:30 AM Michael Walle <mwalle@kernel.org> wrote:
>>
>> On Tue Aug 19, 2025 at 1:27 PM CEST, Michael Walle wrote:
>> > Hi Job,
>> >
>> > > The following patches were created to get the tps65224 PMIC
>> > > powerbutton driver and power off request working on the
>> > > MitySOM-AM62PX. The patches are as follows:
>> >
>> > Are there any news on this series? Do you plan to post a new
>> > version soon, or do you want me to take over? I'd like to get
>> > support for this into the kernel :)
>>
>> FWIW, jsava@criticallink.com is bouncing.
> Yeah sorry, Job has gone back to school, he was on an internship.
>
> I'm not entirely sure how best to move forward with this change.
> There have been several suggestions thrown out and I'm a little lost
> on what's best/easiest.
>
> If you want to take over and add us as co authors that would be
> greatly appreciated.  I can get his personal email if that makes
> sense.  I'm not sure how the kernel normally deals with short term
> emails (interns).

I've asked around and Krzysztof said I should replace the mail with
his new address - or if I can't find it, use the old one. So yes,
you could ask him for his new (or private) address and if he's fine
with having that mentioned in the commit message.

> Note we are currently using the driver as is, on an Android prototype
> and for some reason Android isn't able to detect the power button
> hold, and open the power menu. Single push works fine to turn off the
> screen.  I haven't looked into it yet, to see if this is an Android
> issue or if we are using the POWER key events wrong. Just thought I'd
> mention it.

Thanks, FWIW I'm using it with (normal) linux and it's working fine.
I'll double check if the press and release event is actually working.

-michael