Linux I2C development
 help / color / mirror / Atom feed
* [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes
@ 2026-07-21  7:52 Chen-Yu Tsai
  2026-07-21  7:52 ` [PATCH v3 1/9] regulator: core: Add "enable and wait" functions Chen-Yu Tsai
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-21  7:52 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
  Cc: Andy Shevchenko, Chen-Yu Tsai, linux-mediatek, devicetree,
	linux-arm-kernel, chrome-platform, linux-input, linux-i2c,
	linux-kernel

Hi everyone,

This is v3 of my Chromebook trackpad supply fixes and optimization
series.

Changes since v2:
- Added new regulator "enable and wait" functions that wait until at
  least the given amount of time has passed since the regulator was
  last enabled.
- Converted patches to use the new functions
- New header inclusion cleanup patch for elan_i2c driver

This series fixes the trackpad descriptions on some MediaTek-based
Chromebooks: either the trackpad's supply was set as always-on to
workaround missing delays in the driver, or the delay and supply
are missing from the trackpad's device node.

v1 was just the first patch [1]. It has since grown to cover multiple
drivers and devices.


Patch 1 adds new "enable and wait" functions for single and bulk
regulator enable.

Patch 2 cleans up header inclusion for elan_i2c driver.

Patch 3 adds the correct enable delay after enabling the supply regulator
for the Elan trackpad to initialize. This uses the new "enable and wait"
regulator enable function.

Patch 4 applies the same logic of skipping the power on delay to the
i2c-hid-of driver.

Patch 5 applies the same logic of skipping the power on delay to the
i2c OF component prober library.

Patch 6 adds a delay between when the device node found is enabled and
when regulator_disable() is called. This gives an asynchronously probing
driver some time to increment the enable count of their regulator
reference, thus keeping the device operational and allowing the driver
to skip the initialization delay.

Patch 7 adds the correct delay for probing trackpads for Hana devices
to the ChromeOS OF component prober.

Patch 8 removes the "always-on" setting from the trackpad supply for
Elm / Hana and adds the correct delay to the second source trackpad.
This corrects the hardware description.

Patch 9 adds the supply and power on delay properties to the Synaptics
trackpad on the Spherion device. Combined with previous driver changes
this should cause no actual functional changes or delays.


Please take a look. Patches 3, 4, and 5 have build time dependencies
on patch 1. I suggest placing patch 1 on a separate immutable branch
or tag for the other maintainers to pull in. The three patches all have
different maintainers.

The DT changes must go in after all the driver changes land, especially
patch 3 that adds delays to the Elan trackpad driver. Otherwise the user
could potentially end up with a non-functional trackpad on the device.


Thanks
ChenYu

[1] https://lore.kernel.org/all/20241001093815.2481899-1-wenst@chromium.org/


Chen-Yu Tsai (9):
  regulator: core: Add "enable and wait" functions
  Input: elan_i2c - sort include statements
  Input: elan_i2c - Wait for initialization after enabling regulator
    supply
  HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently
    long
  i2c: of-prober: skip post-power-on delay if powered on sufficiently
    long
  i2c: of-prober: Defer regulator_disable() on successful probe in
    simple helper
  platform/chrome: of_hw_prober: Add delay for hana trackpads
  arm64: dts: mediatek: mt8173-elm-hana: Unmark trackpad supply as
    always-on
  arm64: dts: mediatek: mt8192-asurada-spherion: Add Synaptics
    trackpad's supply

 .../boot/dts/mediatek/mt8173-elm-hana.dtsi    |  8 +--
 arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi  |  1 -
 .../mediatek/mt8192-asurada-spherion-r0.dts   |  2 +
 drivers/hid/i2c-hid/i2c-hid-of.c              |  9 ++--
 drivers/i2c/i2c-core-of-prober.c              | 25 ++++++---
 drivers/input/mouse/elan_i2c_core.c           | 23 ++++----
 .../platform/chrome/chromeos_of_hw_prober.c   |  4 +-
 drivers/regulator/core.c                      | 53 ++++++++++++++-----
 include/linux/regulator/consumer.h            | 21 +++++---
 include/linux/regulator/driver.h              |  2 +
 10 files changed, 96 insertions(+), 52 deletions(-)

-- 
2.55.0.229.g6434b31f56-goog


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 1/9] regulator: core: Add "enable and wait" functions
  2026-07-21  7:52 [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes Chen-Yu Tsai
@ 2026-07-21  7:52 ` Chen-Yu Tsai
  2026-07-21  9:54   ` Andy Shevchenko
  2026-07-21  7:52 ` [PATCH v3 2/9] Input: elan_i2c - sort include statements Chen-Yu Tsai
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-21  7:52 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
  Cc: Andy Shevchenko, Chen-Yu Tsai, linux-mediatek, devicetree,
	linux-arm-kernel, chrome-platform, linux-input, linux-i2c,
	linux-kernel

In device power sequencing and initialization use cases, it is common
for the driver to enable the regulator and then wait for a certain
period of time to pass before continuing.

In cases where the regulator supply is always on, or has been turned on
or left on by another consumer, the driver could shorten the delay or
skip it altogether, provided that enough time has already passed since
the regulator was _actually_ turned on.

Tracking this requires support from the regulator core. Introduce a
"last turned on" timestamp field to the regulator device, and "enable
and wait" functions to the single and bulk regulator consumer APIs.
The existing "enable without wait" functions are then converted to
macros that expand to the new functions.

One case in particular is not optimized yet: a regulator left on either
by hardware reset default or by the bootloader, but does not have the
"regulator-boot-on" property set. As the enable timestamp only gets
updated when enabled by a consumer or by the core, the first enablement
always needs to wait.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- New patch
---
 drivers/regulator/core.c           | 53 ++++++++++++++++++++++--------
 include/linux/regulator/consumer.h | 21 ++++++++----
 include/linux/regulator/driver.h   |  2 ++
 3 files changed, 57 insertions(+), 19 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 1797929dfe56..d14ce86d8f7b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -98,7 +98,8 @@ struct regulator_event_work {
 	unsigned long event;
 };
 
-static int _regulator_enable(struct regulator *regulator);
+static int _regulator_enable_and_wait(struct regulator *regulator, unsigned int wait_us);
+#define _regulator_enable(regulator)	_regulator_enable_and_wait(regulator, 0)
 static int _regulator_is_enabled(struct regulator_dev *rdev);
 static int _regulator_disable(struct regulator *regulator);
 static int _regulator_get_error_flags(struct regulator_dev *rdev, unsigned int *flags);
@@ -3043,6 +3044,8 @@ static int _regulator_do_enable(struct regulator_dev *rdev)
 		fsleep(delay);
 	}
 
+	rdev->last_on = ktime_get_boottime();
+
 	trace_regulator_enable_complete(rdev_get_name(rdev));
 
 	return 0;
@@ -3112,8 +3115,8 @@ static int _regulator_handle_consumer_disable(struct regulator *regulator)
 	return 0;
 }
 
-/* locks held by regulator_enable() */
-static int _regulator_enable(struct regulator *regulator)
+/* locks held by regulator_enable_and_wait() */
+static int _regulator_enable_and_wait(struct regulator *regulator, unsigned int wait_us)
 {
 	struct regulator_dev *rdev = regulator->rdev;
 	int ret;
@@ -3159,13 +3162,24 @@ static int _regulator_enable(struct regulator *regulator)
 		} else if (ret < 0) {
 			rdev_err(rdev, "is_enabled() failed: %pe\n", ERR_PTR(ret));
 			goto err_consumer_disable;
+		} else {
+			/* regulator already enabled somehow, but timestamp might be invalid */
+			if (!rdev->last_on)
+				rdev->last_on = ktime_get_boottime();
 		}
-		/* Fallthrough on positive return values - already enabled */
 	}
 
 	if (regulator->enable_count == 1)
 		rdev->use_count++;
 
+	if (wait_us) {
+		ktime_t end = ktime_add_us(rdev->last_on, wait_us);
+		s64 remaining = ktime_us_delta(end, ktime_get_boottime());
+
+		if (remaining > 0)
+			fsleep(remaining);
+	}
+
 	return 0;
 
 err_consumer_disable:
@@ -3179,31 +3193,36 @@ static int _regulator_enable(struct regulator *regulator)
 }
 
 /**
- * regulator_enable - enable regulator output
+ * regulator_enable_and_wait - enable regulator output and wait for time
+ *			       passed after regulator actually enabled
  * @regulator: regulator source
+ * @wait_us: time to wait after regulator actually turned on; 0 to not wait
  *
  * Request that the regulator be enabled with the regulator output at
  * the predefined voltage or current value.  Calls to regulator_enable()
  * must be balanced with calls to regulator_disable().
  *
+ * If wait_us is greater than zero, then check that wait_us has passed since
+ * the regulator is _actually_ enabled before returning.
+ *
  * NOTE: the output value can be set by other drivers, boot loader or may be
  * hardwired in the regulator.
  *
  * Return: 0 on success or a negative error number on failure.
  */
-int regulator_enable(struct regulator *regulator)
+int regulator_enable_and_wait(struct regulator *regulator, unsigned int wait_us)
 {
 	struct regulator_dev *rdev = regulator->rdev;
 	struct ww_acquire_ctx ww_ctx;
 	int ret;
 
 	regulator_lock_dependent(rdev, &ww_ctx);
-	ret = _regulator_enable(regulator);
+	ret = _regulator_enable_and_wait(regulator, wait_us);
 	regulator_unlock_dependent(rdev, &ww_ctx);
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(regulator_enable);
+EXPORT_SYMBOL_GPL(regulator_enable_and_wait);
 
 static int _regulator_do_disable(struct regulator_dev *rdev)
 {
@@ -5372,30 +5391,38 @@ static void regulator_bulk_enable_async(void *data, async_cookie_t cookie)
 {
 	struct regulator_bulk_data *bulk = data;
 
-	bulk->ret = regulator_enable(bulk->consumer);
+	bulk->ret = regulator_enable_and_wait(bulk->consumer, bulk->wait_us);
 }
 
 /**
- * regulator_bulk_enable - enable multiple regulator consumers
+ * regulator_bulk_enable_and_wait - enable multiple regulator consumers and
+ *				    wait for time passed after regulators are
+ *				    actually enabled
  *
  * @num_consumers: Number of consumers
  * @consumers:     Consumer data; clients are stored here.
+ * @wait_us: time to wait after regulators actually turned on; 0 to not wait
  *
  * This convenience API allows consumers to enable multiple regulator
  * clients in a single API call.  If any consumers cannot be enabled
  * then any others that were enabled will be disabled again prior to
  * return.
  *
+ * If wait_us is greater than zero, then check that wait_us has passed since
+ * the regulators are _actually_ enabled before returning.
+ *
  * Return: 0 on success or a negative error number on failure.
  */
-int regulator_bulk_enable(int num_consumers,
-			  struct regulator_bulk_data *consumers)
+int regulator_bulk_enable_and_wait(int num_consumers,
+				   struct regulator_bulk_data *consumers,
+				   unsigned int wait_us)
 {
 	ASYNC_DOMAIN_EXCLUSIVE(async_domain);
 	int i;
 	int ret = 0;
 
 	for (i = 0; i < num_consumers; i++) {
+		consumers[i].wait_us = wait_us;
 		async_schedule_domain(regulator_bulk_enable_async,
 				      &consumers[i], &async_domain);
 	}
@@ -5423,7 +5450,7 @@ int regulator_bulk_enable(int num_consumers,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(regulator_bulk_enable);
+EXPORT_SYMBOL_GPL(regulator_bulk_enable_and_wait);
 
 /**
  * regulator_bulk_disable - disable multiple regulator consumers
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 56fe2693d9b2..a69157c9b5b5 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -145,6 +145,7 @@ struct regulator_bulk_data {
 
 	/* private: Internal use */
 	int ret;
+	unsigned int wait_us;
 };
 
 #if defined(CONFIG_REGULATOR)
@@ -192,7 +193,7 @@ int devm_regulator_bulk_register_supply_alias(struct device *dev,
 					      int num_id);
 
 /* regulator output control and status */
-int __must_check regulator_enable(struct regulator *regulator);
+int __must_check regulator_enable_and_wait(struct regulator *regulator, unsigned int ms);
 int regulator_disable(struct regulator *regulator);
 int regulator_force_disable(struct regulator *regulator);
 int regulator_is_enabled(struct regulator *regulator);
@@ -209,8 +210,9 @@ int __must_check devm_regulator_bulk_get_const(
 	struct device *dev, int num_consumers,
 	const struct regulator_bulk_data *in_consumers,
 	struct regulator_bulk_data **out_consumers);
-int __must_check regulator_bulk_enable(int num_consumers,
-				       struct regulator_bulk_data *consumers);
+int __must_check regulator_bulk_enable_and_wait(int num_consumers,
+						struct regulator_bulk_data *consumers,
+						unsigned int wait_us);
 int devm_regulator_bulk_get_enable(struct device *dev, int num_consumers,
 				   const char * const *id);
 int regulator_bulk_disable(int num_consumers,
@@ -410,7 +412,8 @@ static inline int devm_regulator_bulk_register_supply_alias(struct device *dev,
 	return 0;
 }
 
-static inline int regulator_enable(struct regulator *regulator)
+static inline int regulator_enable_and_wait(struct regulator *regulator,
+					    unsigned int wait_us)
 {
 	return 0;
 }
@@ -457,8 +460,9 @@ static inline int devm_regulator_bulk_get_const(
 	return 0;
 }
 
-static inline int regulator_bulk_enable(int num_consumers,
-					struct regulator_bulk_data *consumers)
+static inline int regulator_bulk_enable_and_wait(int num_consumers,
+						 struct regulator_bulk_data *consumers,
+						 unsigned int wait_us)
 {
 	return 0;
 }
@@ -676,6 +680,11 @@ regulator_is_equal(struct regulator *reg1, struct regulator *reg2)
 }
 #endif
 
+#define regulator_enable(regulator)	regulator_enable_and_wait(regulator, 0)
+
+#define regulator_bulk_enable(num_consumers, consumers)	\
+		regulator_bulk_enable_and_wait(num_consumers, consumers, 0)
+
 #if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_REGULATOR)
 struct regulator *__must_check of_regulator_get(struct device *dev,
 						struct device_node *node,
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index cc6ce709ec86..8a74aa681df3 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -658,6 +658,8 @@ struct regulator_dev {
 	unsigned int constraints_pending:1;
 	unsigned int is_switch:1;
 
+	/* time when this regulator was enabled last time */
+	ktime_t last_on;
 	/* time when this regulator was disabled last time */
 	ktime_t last_off;
 	int cached_err;
-- 
2.55.0.229.g6434b31f56-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 2/9] Input: elan_i2c - sort include statements
  2026-07-21  7:52 [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes Chen-Yu Tsai
  2026-07-21  7:52 ` [PATCH v3 1/9] regulator: core: Add "enable and wait" functions Chen-Yu Tsai
@ 2026-07-21  7:52 ` Chen-Yu Tsai
  2026-07-21 10:20   ` Andy Shevchenko
  2026-07-21  7:52 ` [PATCH v3 3/9] Input: elan_i2c - Wait for initialization after enabling regulator supply Chen-Yu Tsai
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-21  7:52 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
  Cc: Andy Shevchenko, Chen-Yu Tsai, linux-mediatek, devicetree,
	linux-arm-kernel, chrome-platform, linux-input, linux-i2c,
	linux-kernel

Sort the include statements before adding new ones in the next change.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- New patch
---
 drivers/input/mouse/elan_i2c_core.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index f93dd545d66b..9f024a435dbf 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -16,27 +16,27 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/firmware.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
+#include <linux/input.h>
 #include <linux/input/mt.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/kernel.h>
-#include <linux/sched.h>
-#include <linux/string_choices.h>
-#include <linux/input.h>
-#include <linux/uaccess.h>
 #include <linux/jiffies.h>
-#include <linux/completion.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/pm_wakeirq.h>
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/string_choices.h>
+#include <linux/uaccess.h>
 #include <linux/unaligned.h>
 
 #include "elan_i2c.h"
-- 
2.55.0.229.g6434b31f56-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 3/9] Input: elan_i2c - Wait for initialization after enabling regulator supply
  2026-07-21  7:52 [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes Chen-Yu Tsai
  2026-07-21  7:52 ` [PATCH v3 1/9] regulator: core: Add "enable and wait" functions Chen-Yu Tsai
  2026-07-21  7:52 ` [PATCH v3 2/9] Input: elan_i2c - sort include statements Chen-Yu Tsai
@ 2026-07-21  7:52 ` Chen-Yu Tsai
  2026-07-21  7:52 ` [PATCH v3 4/9] HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently long Chen-Yu Tsai
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-21  7:52 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
  Cc: Andy Shevchenko, Chen-Yu Tsai, linux-mediatek, devicetree,
	linux-arm-kernel, chrome-platform, linux-input, linux-i2c,
	linux-kernel

Elan trackpad controllers require some delay after enabling power to
the controller for the hardware and firmware to initialize:

  - 2ms for hardware initialization
  - 100ms for firmware initialization

Until then, the hardware will not respond to I2C transfers. This was
observed on the MT8173 Chromebooks after the regulator supply for the
trackpad was changed to "not always on".

Switch to the new regulator_enable_and_wait(). This makes sure that
enough time has passed since the regulator was first enabled, satisfying
the power sequencing delay requirement. This allows the delay to be
skipped if the regulator supply was already enabled by some other part
of the kernel, such as the I2C OF component prober.

Fixes: 6696777c6506 ("Input: add driver for Elan I2C/SMbus touchpad")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Switched to new regulator_enable_and_wait() API

Changes since v1:
- Delay only if the regulator was previously disabled / turned off
- Link to v1
  https://lore.kernel.org/all/20241001093815.2481899-1-wenst@chromium.org/
---
 drivers/input/mouse/elan_i2c_core.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 9f024a435dbf..77885930bf9e 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -36,6 +36,7 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/string_choices.h>
+#include <linux/time.h>
 #include <linux/uaccess.h>
 #include <linux/unaligned.h>
 
@@ -47,6 +48,8 @@
 #define ETP_FWIDTH_REDUCE	90
 #define ETP_FINGER_WIDTH	15
 #define ETP_RETRY_COUNT		3
+/* H/W init 2 ms + F/W init 100 ms w/ round up */
+#define ETP_POWER_ON_DELAY_US	(110 * USEC_PER_MSEC)
 
 /* quirks to control the device */
 #define ETP_QUIRK_QUICK_WAKEUP	BIT(0)
@@ -1250,7 +1253,7 @@ static int elan_probe(struct i2c_client *client)
 	if (IS_ERR(data->vcc))
 		return dev_err_probe(dev, PTR_ERR(data->vcc), "Failed to get 'vcc' regulator\n");
 
-	error = regulator_enable(data->vcc);
+	error = regulator_enable_and_wait(data->vcc, ETP_POWER_ON_DELAY_US);
 	if (error) {
 		dev_err(dev, "Failed to enable regulator: %d\n", error);
 		return error;
@@ -1406,7 +1409,7 @@ static int elan_resume(struct device *dev)
 	int error;
 
 	if (!device_may_wakeup(dev)) {
-		error = regulator_enable(data->vcc);
+		error = regulator_enable_and_wait(data->vcc, ETP_POWER_ON_DELAY_US);
 		if (error) {
 			dev_err(dev, "error %d enabling regulator\n", error);
 			goto err;
-- 
2.55.0.229.g6434b31f56-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 4/9] HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently long
  2026-07-21  7:52 [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes Chen-Yu Tsai
                   ` (2 preceding siblings ...)
  2026-07-21  7:52 ` [PATCH v3 3/9] Input: elan_i2c - Wait for initialization after enabling regulator supply Chen-Yu Tsai
@ 2026-07-21  7:52 ` Chen-Yu Tsai
  2026-07-21  7:52 ` [PATCH v3 5/9] i2c: of-prober: " Chen-Yu Tsai
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-21  7:52 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
  Cc: Andy Shevchenko, Chen-Yu Tsai, linux-mediatek, devicetree,
	linux-arm-kernel, chrome-platform, linux-input, linux-i2c,
	linux-kernel

On some devices the HID device is powered from an always-on power rail,
or the power rail has been left on by either POR defaults or the
bootloader. By the time the driver probes, the device most certainly
has finished initializing. There is no need for the delay.

In such designs, the system integrators tend to work around the delay
to avoid the boot time penalty by simply omitting it from the device
tree. This is undesired, as the device tree is not fully describing
the hardware.

Switch to the new regulator_bulk_enable_and_wait() function that makes
sure a certain amount of time has passed since the regulator supplies
were actually enabled.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Switched to new regulator_bulk_enable_and_wait() API
---
 drivers/hid/i2c-hid/i2c-hid-of.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
index 59393d71ddb9..fdaad451e710 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of.c
@@ -29,6 +29,7 @@
 #include <linux/of.h>
 #include <linux/pm.h>
 #include <linux/regulator/consumer.h>
+#include <linux/time.h>
 
 #include "i2c-hid.h"
 
@@ -48,16 +49,14 @@ static int i2c_hid_of_power_up(struct i2chid_ops *ops)
 	struct device *dev = &ihid_of->client->dev;
 	int ret;
 
-	ret = regulator_bulk_enable(ARRAY_SIZE(ihid_of->supplies),
-				    ihid_of->supplies);
+	ret = regulator_bulk_enable_and_wait(ARRAY_SIZE(ihid_of->supplies),
+					     ihid_of->supplies,
+					     ihid_of->post_power_delay_ms * USEC_PER_MSEC);
 	if (ret) {
 		dev_warn(dev, "Failed to enable supplies: %d\n", ret);
 		return ret;
 	}
 
-	if (ihid_of->post_power_delay_ms)
-		msleep(ihid_of->post_power_delay_ms);
-
 	gpiod_set_value_cansleep(ihid_of->reset_gpio, 0);
 	if (ihid_of->post_reset_delay_ms)
 		msleep(ihid_of->post_reset_delay_ms);
-- 
2.55.0.229.g6434b31f56-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 5/9] i2c: of-prober: skip post-power-on delay if powered on sufficiently long
  2026-07-21  7:52 [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes Chen-Yu Tsai
                   ` (3 preceding siblings ...)
  2026-07-21  7:52 ` [PATCH v3 4/9] HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently long Chen-Yu Tsai
@ 2026-07-21  7:52 ` Chen-Yu Tsai
  2026-07-21  7:52 ` [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper Chen-Yu Tsai
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-21  7:52 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
  Cc: Andy Shevchenko, Chen-Yu Tsai, linux-mediatek, devicetree,
	linux-arm-kernel, chrome-platform, linux-input, linux-i2c,
	linux-kernel

On some devices the I2C component is powered from an always-on power
rail, or the power rail has been left on by either POR defaults or
the bootloader. By the time the prober probes the device, the device
most certainly has finished initializing and can respond. There is no
need for the delay.

In such designs, the system integrators tend to work around the delay
to avoid the boot time penalty by simply omitting it from the device
tree and the component prober. This is undesired, as the device tree
is not fully describing the hardware.

Switch to the new regulator_enable_and_wait() function that makes sure
a certain amount of time has passed since the regulator supply was
actually enabled.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Switched to new regulator_enable_and_wait() API
---
 drivers/i2c/i2c-core-of-prober.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core-of-prober.c b/drivers/i2c/i2c-core-of-prober.c
index 6a82b03809d4..f9f3c0ef93ff 100644
--- a/drivers/i2c/i2c-core-of-prober.c
+++ b/drivers/i2c/i2c-core-of-prober.c
@@ -18,6 +18,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/stddef.h>
+#include <linux/time.h>
 
 /*
  * Some devices, such as Google Hana Chromebooks, are produced by multiple
@@ -226,13 +227,11 @@ static int i2c_of_probe_simple_enable_regulator(struct device *dev, struct i2c_o
 
 	dev_dbg(dev, "Enabling regulator supply \"%s\"\n", ctx->opts->supply_name);
 
-	ret = regulator_enable(ctx->supply);
+	ret = regulator_enable_and_wait(ctx->supply,
+					ctx->opts->post_power_on_delay_ms * USEC_PER_MSEC);
 	if (ret)
 		return ret;
 
-	if (ctx->opts->post_power_on_delay_ms)
-		msleep(ctx->opts->post_power_on_delay_ms);
-
 	return 0;
 }
 
-- 
2.55.0.229.g6434b31f56-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper
  2026-07-21  7:52 [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes Chen-Yu Tsai
                   ` (4 preceding siblings ...)
  2026-07-21  7:52 ` [PATCH v3 5/9] i2c: of-prober: " Chen-Yu Tsai
@ 2026-07-21  7:52 ` Chen-Yu Tsai
  2026-07-21 10:24   ` Andy Shevchenko
  2026-07-21  7:52 ` [PATCH v3 7/9] platform/chrome: of_hw_prober: Add delay for hana trackpads Chen-Yu Tsai
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-21  7:52 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
  Cc: Andy Shevchenko, Chen-Yu Tsai, linux-mediatek, devicetree,
	linux-arm-kernel, chrome-platform, linux-input, linux-i2c,
	linux-kernel

When a I2C component is found, it's device node is immediately enabled.
This triggers device creation and driver binding. The prober will hold
the regulator enable reference across this part. If the driver probes
synchronously, then it happens within this window. On the other hand,
if the driver probes asynchronously, there is high chance that it
happens after the prober's cleanup function was called, in which case
the regulator would have been disabled when the driver's probe function
is called. This would then require the driver to wait 100 ms for the
hardware to reinitialize, even if the probe function was just a split
second late and the regulator was disabled a few milliseconds ago.

Recently, some of the drivers for the component that are targeted by the
I2C OF component prober gained the ability to skip waiting for hardware
initialization if the regulator was left enabled. This happens when the
PMIC has them on by default, or if the component prober left them on
after probing the component.

Wait a bit of time before dropping the enable refcount on our end so
that the actual driver has the opportunity to catch and increase the
refcount on their end. The 100 ms delay was arbitrarily chosen.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/i2c/i2c-core-of-prober.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/i2c-core-of-prober.c b/drivers/i2c/i2c-core-of-prober.c
index f9f3c0ef93ff..68c929b16b06 100644
--- a/drivers/i2c/i2c-core-of-prober.c
+++ b/drivers/i2c/i2c-core-of-prober.c
@@ -235,11 +235,23 @@ static int i2c_of_probe_simple_enable_regulator(struct device *dev, struct i2c_o
 	return 0;
 }
 
-static void i2c_of_probe_simple_disable_regulator(struct device *dev, struct i2c_of_probe_simple_ctx *ctx)
+static void i2c_of_probe_simple_disable_regulator(struct device *dev,
+						  struct i2c_of_probe_simple_ctx *ctx,
+						  bool defer_disable)
 {
 	if (!ctx->supply)
 		return;
 
+	/*
+	 * Wait a bit of time for async drivers to probe and increase the
+	 * regulator enable count. This allows the drivers to check and
+	 * skip waiting for re-initialization.
+	 */
+	if (defer_disable) {
+		dev_dbg(dev, "Deferring regulator disable\n");
+		msleep(100);
+	}
+
 	dev_dbg(dev, "Disabling regulator supply \"%s\"\n", ctx->opts->supply_name);
 
 	regulator_disable(ctx->supply);
@@ -356,7 +368,7 @@ int i2c_of_probe_simple_enable(struct device *dev, struct device_node *bus_node,
 	return 0;
 
 out_disable_regulator:
-	i2c_of_probe_simple_disable_regulator(dev, ctx);
+	i2c_of_probe_simple_disable_regulator(dev, ctx, false);
 out_put_gpiod:
 	i2c_of_probe_simple_put_gpiod(ctx);
 out_put_supply:
@@ -401,7 +413,7 @@ void i2c_of_probe_simple_cleanup(struct device *dev, void *data)
 	i2c_of_probe_simple_disable_gpio(dev, ctx);
 	i2c_of_probe_simple_put_gpiod(ctx);
 
-	i2c_of_probe_simple_disable_regulator(dev, ctx);
+	i2c_of_probe_simple_disable_regulator(dev, ctx, true);
 	i2c_of_probe_simple_put_supply(ctx);
 }
 EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_cleanup, "I2C_OF_PROBER");
-- 
2.55.0.229.g6434b31f56-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 7/9] platform/chrome: of_hw_prober: Add delay for hana trackpads
  2026-07-21  7:52 [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes Chen-Yu Tsai
                   ` (5 preceding siblings ...)
  2026-07-21  7:52 ` [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper Chen-Yu Tsai
@ 2026-07-21  7:52 ` Chen-Yu Tsai
  2026-07-21 10:27   ` Andy Shevchenko
  2026-07-21  7:52 ` [PATCH v3 8/9] arm64: dts: mediatek: mt8173-elm-hana: Unmark trackpad supply as always-on Chen-Yu Tsai
  2026-07-21  7:52 ` [PATCH v3 9/9] arm64: dts: mediatek: mt8192-asurada-spherion: Add Synaptics trackpad's supply Chen-Yu Tsai
  8 siblings, 1 reply; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-21  7:52 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
  Cc: Andy Shevchenko, Chen-Yu Tsai, linux-mediatek, devicetree,
	linux-arm-kernel, chrome-platform, linux-input, linux-i2c,
	linux-kernel

Up until now, the MT8173 elm/hana device tree has set the dedicated
regulator supplying the trackpad as always-on, simply because the Elan
driver was missing proper delays. As a result the delay for the
Synaptics trackpad was also omitted, as it was not strictly required
under such a model and delayed the availability of the trackpad to the
user.

The Elan driver recently gained proper delays after power-up, with
adaptive skipping of the delay if the regulator was originally
on. The I2C HID driver and I2C OF component prober library gained
similar adaptive delay skipping. The device tree will be fixed to have
the regulator not be always on, and proper post-power-on delay time
added to the I2C HID device.

Also add the post-power-on delay to the ChromeOS OF component prober,
so that if the regulator is off at the time of probing, the prober knows
to wait for the hardware to initialize.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/platform/chrome/chromeos_of_hw_prober.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/platform/chrome/chromeos_of_hw_prober.c b/drivers/platform/chrome/chromeos_of_hw_prober.c
index 8562a0e89dc6..54d8941617e2 100644
--- a/drivers/platform/chrome/chromeos_of_hw_prober.c
+++ b/drivers/platform/chrome/chromeos_of_hw_prober.c
@@ -70,10 +70,8 @@ static const struct chromeos_i2c_probe_data chromeos_i2c_probe_hana_trackpad = {
 		/*
 		 * ELAN trackpad needs 2 ms for H/W init and 100 ms for F/W init.
 		 * Synaptics trackpad needs 100 ms.
-		 * However, the regulator is set to "always-on", presumably to
-		 * avoid this delay. The ELAN driver is also missing delays.
 		 */
-		.post_power_on_delay_ms = 0,
+		.post_power_on_delay_ms = 110,
 	},
 };
 
-- 
2.55.0.229.g6434b31f56-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 8/9] arm64: dts: mediatek: mt8173-elm-hana: Unmark trackpad supply as always-on
  2026-07-21  7:52 [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes Chen-Yu Tsai
                   ` (6 preceding siblings ...)
  2026-07-21  7:52 ` [PATCH v3 7/9] platform/chrome: of_hw_prober: Add delay for hana trackpads Chen-Yu Tsai
@ 2026-07-21  7:52 ` Chen-Yu Tsai
  2026-07-21  7:52 ` [PATCH v3 9/9] arm64: dts: mediatek: mt8192-asurada-spherion: Add Synaptics trackpad's supply Chen-Yu Tsai
  8 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-21  7:52 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
  Cc: Andy Shevchenko, Chen-Yu Tsai, linux-mediatek, devicetree,
	linux-arm-kernel, chrome-platform, linux-input, linux-i2c,
	linux-kernel

Up until now, the MT8173 elm/hana device tree has set the dedicated
regulator supplying the trackpad as always-on, simply because the Elan
driver was missing proper delays. As a result the delay for the
Synaptics trackpad was also omitted, as it was not strictly required
under such a model and delayed the availability of the trackpad to the
user.

The Elan driver recently gained proper delays after power up, with
opportunistic skipping of the delay when the regulator was originally
on. The I2C HID driver gained similar opportunistic delay skipping.
So has the I2C OF component prober library.

Now fix the device tree to have the regulator not be always on, and
let the I2C HID device have the correct post-power-on delay time.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi | 8 +-------
 arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi      | 1 -
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi b/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi
index 1004eb8ea52c..b9e311fcd9a0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi
@@ -62,13 +62,7 @@ trackpad2: trackpad@2c {
 		pinctrl-0 = <&trackpad_irq>;
 		reg = <0x2c>;
 		hid-descr-addr = <0x0020>;
-		/*
-		 * The trackpad needs a post-power-on delay of 100ms,
-		 * but at time of writing, the power supply for it on
-		 * this board is always on. The delay is therefore not
-		 * added to avoid impacting the readiness of the
-		 * trackpad.
-		 */
+		post-power-on-delay-ms = <100>;
 		vdd-supply = <&mt6397_vgp6_reg>;
 		wakeup-source;
 		status = "fail-needs-probe";
diff --git a/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi b/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi
index a0573bc359fb..6b9f47f515c7 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi
@@ -1093,7 +1093,6 @@ mt6397_vgp6_reg: ldo_vgp6 {
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-enable-ramp-delay = <218>;
-				regulator-always-on;
 			};
 
 			mt6397_vibr_reg: ldo_vibr {
-- 
2.55.0.229.g6434b31f56-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 9/9] arm64: dts: mediatek: mt8192-asurada-spherion: Add Synaptics trackpad's supply
  2026-07-21  7:52 [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes Chen-Yu Tsai
                   ` (7 preceding siblings ...)
  2026-07-21  7:52 ` [PATCH v3 8/9] arm64: dts: mediatek: mt8173-elm-hana: Unmark trackpad supply as always-on Chen-Yu Tsai
@ 2026-07-21  7:52 ` Chen-Yu Tsai
  8 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-21  7:52 UTC (permalink / raw)
  To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
  Cc: Andy Shevchenko, Chen-Yu Tsai, linux-mediatek, devicetree,
	linux-arm-kernel, chrome-platform, linux-input, linux-i2c,
	linux-kernel, stable+noautosel

The Synaptics trackpad, like the Elan trackpad option, is fed from the
system 3.3V power rail. Add it to the trackpad device node.

Also add the correct post-power-on delay, even though in practice it is
not required. The Synaptics trackpad requires 100ms after power-on (or
deasserting the reset, whichever comes later) to fully initialize. The
power is always on and the reset pin is not routed out, so the
implementation could try skipping the delay.

Cc: <stable+noautosel@kernel.org> # Without driver changes only lengthens probe time
Fixes: 925ebc0cd55c ("arm64: dts: mt8192-asurada-spherion: Add Synaptics trackpad support")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
I think this shouldn't be backported, as backporting it without the
driver enhancements just delays the trackpad probing with no real
gains.
---
 arch/arm64/boot/dts/mediatek/mt8192-asurada-spherion-r0.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada-spherion-r0.dts b/arch/arm64/boot/dts/mediatek/mt8192-asurada-spherion-r0.dts
index 68caf4c58cfe..8adbfc307fca 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192-asurada-spherion-r0.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada-spherion-r0.dts
@@ -94,6 +94,8 @@ trackpad@2c {
 		hid-descr-addr = <0x20>;
 		interrupts-extended = <&pio 15 IRQ_TYPE_LEVEL_LOW>;
 		wakeup-source;
+		vdd-supply = <&pp3300_u>;
+		post-power-on-delay-ms = <100>;
 		status = "fail-needs-probe";
 	};
 };
-- 
2.55.0.229.g6434b31f56-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/9] regulator: core: Add "enable and wait" functions
  2026-07-21  7:52 ` [PATCH v3 1/9] regulator: core: Add "enable and wait" functions Chen-Yu Tsai
@ 2026-07-21  9:54   ` Andy Shevchenko
  2026-07-22  9:06     ` Chen-Yu Tsai
  0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2026-07-21  9:54 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti,
	linux-mediatek, devicetree, linux-arm-kernel, chrome-platform,
	linux-input, linux-i2c, linux-kernel

On Tue, Jul 21, 2026 at 03:52:15PM +0800, Chen-Yu Tsai wrote:
> In device power sequencing and initialization use cases, it is common
> for the driver to enable the regulator and then wait for a certain
> period of time to pass before continuing.
> 
> In cases where the regulator supply is always on, or has been turned on
> or left on by another consumer, the driver could shorten the delay or
> skip it altogether, provided that enough time has already passed since
> the regulator was _actually_ turned on.
> 
> Tracking this requires support from the regulator core. Introduce a
> "last turned on" timestamp field to the regulator device, and "enable
> and wait" functions to the single and bulk regulator consumer APIs.
> The existing "enable without wait" functions are then converted to
> macros that expand to the new functions.
> 
> One case in particular is not optimized yet: a regulator left on either
> by hardware reset default or by the bootloader, but does not have the
> "regulator-boot-on" property set. As the enable timestamp only gets
> updated when enabled by a consumer or by the core, the first enablement
> always needs to wait.

...

> -/* locks held by regulator_enable() */
> -static int _regulator_enable(struct regulator *regulator)
> +/* locks held by regulator_enable_and_wait() */
> +static int _regulator_enable_and_wait(struct regulator *regulator, unsigned int wait_us)

Hmm...
This comment a bit confusing, perhaps adding some lockdep annotations help?

...

> +	if (wait_us) {
> +		ktime_t end = ktime_add_us(rdev->last_on, wait_us);
> +		s64 remaining = ktime_us_delta(end, ktime_get_boottime());
> +
> +		if (remaining > 0)
> +			fsleep(remaining);

This style is discouraged as it makes maintenance harder. Better

		s64 remaining;

		remaining = ktime_us_delta(end, ktime_get_boottime());
		if (remaining > 0)
			fsleep(remaining);

> +	}

...

>  	/* private: Internal use */
>  	int ret;
> +	unsigned int wait_us;

If you want to make it more private (the above is only for kernel-doc) add
__private annotation that will affect how sparse will check this.

...

> -int __must_check regulator_enable(struct regulator *regulator);
> +int __must_check regulator_enable_and_wait(struct regulator *regulator, unsigned int ms);

ms or us? Please, double check all units.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/9] Input: elan_i2c - sort include statements
  2026-07-21  7:52 ` [PATCH v3 2/9] Input: elan_i2c - sort include statements Chen-Yu Tsai
@ 2026-07-21 10:20   ` Andy Shevchenko
  0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2026-07-21 10:20 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti,
	linux-mediatek, devicetree, linux-arm-kernel, chrome-platform,
	linux-input, linux-i2c, linux-kernel

On Tue, Jul 21, 2026 at 03:52:16PM +0800, Chen-Yu Tsai wrote:
> Sort the include statements before adding new ones in the next change.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper
  2026-07-21  7:52 ` [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper Chen-Yu Tsai
@ 2026-07-21 10:24   ` Andy Shevchenko
  2026-07-22  8:34     ` Chen-Yu Tsai
  0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2026-07-21 10:24 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti,
	linux-mediatek, devicetree, linux-arm-kernel, chrome-platform,
	linux-input, linux-i2c, linux-kernel

On Tue, Jul 21, 2026 at 03:52:20PM +0800, Chen-Yu Tsai wrote:
> When a I2C component is found, it's device node is immediately enabled.
> This triggers device creation and driver binding. The prober will hold
> the regulator enable reference across this part. If the driver probes
> synchronously, then it happens within this window. On the other hand,
> if the driver probes asynchronously, there is high chance that it
> happens after the prober's cleanup function was called, in which case
> the regulator would have been disabled when the driver's probe function
> is called. This would then require the driver to wait 100 ms for the
> hardware to reinitialize, even if the probe function was just a split
> second late and the regulator was disabled a few milliseconds ago.
> 
> Recently, some of the drivers for the component that are targeted by the
> I2C OF component prober gained the ability to skip waiting for hardware
> initialization if the regulator was left enabled. This happens when the
> PMIC has them on by default, or if the component prober left them on
> after probing the component.
> 
> Wait a bit of time before dropping the enable refcount on our end so
> that the actual driver has the opportunity to catch and increase the
> refcount on their end. The 100 ms delay was arbitrarily chosen.

...

> +	/*
> +	 * Wait a bit of time for async drivers to probe and increase the
> +	 * regulator enable count. This allows the drivers to check and
> +	 * skip waiting for re-initialization.
> +	 */
> +	if (defer_disable) {
> +		dev_dbg(dev, "Deferring regulator disable\n");
> +		msleep(100);

How was this value chosen?

> +	}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 7/9] platform/chrome: of_hw_prober: Add delay for hana trackpads
  2026-07-21  7:52 ` [PATCH v3 7/9] platform/chrome: of_hw_prober: Add delay for hana trackpads Chen-Yu Tsai
@ 2026-07-21 10:27   ` Andy Shevchenko
  2026-07-22  3:33     ` Chen-Yu Tsai
  0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2026-07-21 10:27 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti,
	linux-mediatek, devicetree, linux-arm-kernel, chrome-platform,
	linux-input, linux-i2c, linux-kernel

On Tue, Jul 21, 2026 at 03:52:21PM +0800, Chen-Yu Tsai wrote:
> Up until now, the MT8173 elm/hana device tree has set the dedicated
> regulator supplying the trackpad as always-on, simply because the Elan
> driver was missing proper delays. As a result the delay for the
> Synaptics trackpad was also omitted, as it was not strictly required
> under such a model and delayed the availability of the trackpad to the
> user.
> 
> The Elan driver recently gained proper delays after power-up, with
> adaptive skipping of the delay if the regulator was originally
> on. The I2C HID driver and I2C OF component prober library gained
> similar adaptive delay skipping. The device tree will be fixed to have
> the regulator not be always on, and proper post-power-on delay time
> added to the I2C HID device.
> 
> Also add the post-power-on delay to the ChromeOS OF component prober,
> so that if the regulator is off at the time of probing, the prober knows
> to wait for the hardware to initialize.

...

> +++ b/drivers/platform/chrome/chromeos_of_hw_prober.c

>  		/*
>  		 * ELAN trackpad needs 2 ms for H/W init and 100 ms for F/W init.
>  		 * Synaptics trackpad needs 100 ms.
> -		 * However, the regulator is set to "always-on", presumably to
> -		 * avoid this delay. The ELAN driver is also missing delays.
>  		 */
> -		.post_power_on_delay_ms = 0,
> +		.post_power_on_delay_ms = 110,

This and the other delay in the previous patches are all HW-related. What if
another touchpad gets connected? I mean that the delays for the certain HW
should not affect other possible devices connected to the platform.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 7/9] platform/chrome: of_hw_prober: Add delay for hana trackpads
  2026-07-21 10:27   ` Andy Shevchenko
@ 2026-07-22  3:33     ` Chen-Yu Tsai
  0 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-22  3:33 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti,
	linux-mediatek, devicetree, linux-arm-kernel, chrome-platform,
	linux-input, linux-i2c, linux-kernel

On Tue, Jul 21, 2026 at 6:27 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Jul 21, 2026 at 03:52:21PM +0800, Chen-Yu Tsai wrote:
> > Up until now, the MT8173 elm/hana device tree has set the dedicated
> > regulator supplying the trackpad as always-on, simply because the Elan
> > driver was missing proper delays. As a result the delay for the
> > Synaptics trackpad was also omitted, as it was not strictly required
> > under such a model and delayed the availability of the trackpad to the
> > user.
> >
> > The Elan driver recently gained proper delays after power-up, with
> > adaptive skipping of the delay if the regulator was originally
> > on. The I2C HID driver and I2C OF component prober library gained
> > similar adaptive delay skipping. The device tree will be fixed to have
> > the regulator not be always on, and proper post-power-on delay time
> > added to the I2C HID device.
> >
> > Also add the post-power-on delay to the ChromeOS OF component prober,
> > so that if the regulator is off at the time of probing, the prober knows
> > to wait for the hardware to initialize.
>
> ...
>
> > +++ b/drivers/platform/chrome/chromeos_of_hw_prober.c
>
> >               /*
> >                * ELAN trackpad needs 2 ms for H/W init and 100 ms for F/W init.
> >                * Synaptics trackpad needs 100 ms.
> > -              * However, the regulator is set to "always-on", presumably to
> > -              * avoid this delay. The ELAN driver is also missing delays.
> >                */
> > -             .post_power_on_delay_ms = 0,
> > +             .post_power_on_delay_ms = 110,
>
> This and the other delay in the previous patches are all HW-related. What if
> another touchpad gets connected? I mean that the delays for the certain HW
> should not affect other possible devices connected to the platform.

This is a complete laptop, not a development board. You can't just randomly
swap out the trackpad. The model AFAIK is no longer in production, and we
have already added all possible configurations to the device tree. And
there's no extra connector or pin header to attach additional peripherals.
If someone hacks their own device to swap out a trackpad, then handling
the additional bring-up should fall on them.

If you are referring to the delay for the other trackpad option, that is
100 ms, and is described in the device tree. See patch 8 of this series.

Any other device with different delay requirements should have its own
entry, not reuse this one. The information in the entry is tightly
coupled to the device tree anyway. I already corrected the other device
that incorrectly reused this entry [1]. We have some additional offenders
downstream, but nothing that can't be corrected in the same manner.


ChenYu

[1] https://lore.kernel.org/all/20260625060859.1020483-1-wenst@chromium.org/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper
  2026-07-21 10:24   ` Andy Shevchenko
@ 2026-07-22  8:34     ` Chen-Yu Tsai
  2026-08-06 20:43       ` Andy Shevchenko
  0 siblings, 1 reply; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-22  8:34 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti,
	linux-mediatek, devicetree, linux-arm-kernel, chrome-platform,
	linux-input, linux-i2c, linux-kernel

On Tue, Jul 21, 2026 at 6:24 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Jul 21, 2026 at 03:52:20PM +0800, Chen-Yu Tsai wrote:
> > When a I2C component is found, it's device node is immediately enabled.
> > This triggers device creation and driver binding. The prober will hold
> > the regulator enable reference across this part. If the driver probes
> > synchronously, then it happens within this window. On the other hand,
> > if the driver probes asynchronously, there is high chance that it
> > happens after the prober's cleanup function was called, in which case
> > the regulator would have been disabled when the driver's probe function
> > is called. This would then require the driver to wait 100 ms for the
> > hardware to reinitialize, even if the probe function was just a split
> > second late and the regulator was disabled a few milliseconds ago.
> >
> > Recently, some of the drivers for the component that are targeted by the
> > I2C OF component prober gained the ability to skip waiting for hardware
> > initialization if the regulator was left enabled. This happens when the
> > PMIC has them on by default, or if the component prober left them on
> > after probing the component.
> >
> > Wait a bit of time before dropping the enable refcount on our end so
> > that the actual driver has the opportunity to catch and increase the
> > refcount on their end. The 100 ms delay was arbitrarily chosen.
>
> ...
>
> > +     /*
> > +      * Wait a bit of time for async drivers to probe and increase the
> > +      * regulator enable count. This allows the drivers to check and
> > +      * skip waiting for re-initialization.
> > +      */
> > +     if (defer_disable) {
> > +             dev_dbg(dev, "Deferring regulator disable\n");
> > +             msleep(100);
>
> How was this value chosen?

I just chose an arbitrary round value.

If both the prober and the driver for the probed device (trackpad in
this example) are builtin, the time between enabling the node and the
driver asynchronously probing is between 5 ms and 30 ms, but could
also see outliers exceeding 100 ms.

If you're worried the value won't suit every user, I could make it a
parameter of the simple helpers? And the user itself could be made to
probe asynchronously to not block the main code path.


ChenYu

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/9] regulator: core: Add "enable and wait" functions
  2026-07-21  9:54   ` Andy Shevchenko
@ 2026-07-22  9:06     ` Chen-Yu Tsai
  0 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2026-07-22  9:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti,
	linux-mediatek, devicetree, linux-arm-kernel, chrome-platform,
	linux-input, linux-i2c, linux-kernel

On Tue, Jul 21, 2026 at 5:54 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Jul 21, 2026 at 03:52:15PM +0800, Chen-Yu Tsai wrote:
> > In device power sequencing and initialization use cases, it is common
> > for the driver to enable the regulator and then wait for a certain
> > period of time to pass before continuing.
> >
> > In cases where the regulator supply is always on, or has been turned on
> > or left on by another consumer, the driver could shorten the delay or
> > skip it altogether, provided that enough time has already passed since
> > the regulator was _actually_ turned on.
> >
> > Tracking this requires support from the regulator core. Introduce a
> > "last turned on" timestamp field to the regulator device, and "enable
> > and wait" functions to the single and bulk regulator consumer APIs.
> > The existing "enable without wait" functions are then converted to
> > macros that expand to the new functions.
> >
> > One case in particular is not optimized yet: a regulator left on either
> > by hardware reset default or by the bootloader, but does not have the
> > "regulator-boot-on" property set. As the enable timestamp only gets
> > updated when enabled by a consumer or by the core, the first enablement
> > always needs to wait.
>
> ...
>
> > -/* locks held by regulator_enable() */
> > -static int _regulator_enable(struct regulator *regulator)
> > +/* locks held by regulator_enable_and_wait() */
> > +static int _regulator_enable_and_wait(struct regulator *regulator, unsigned int wait_us)
>
> Hmm...
> This comment a bit confusing, perhaps adding some lockdep annotations help?
>
> ...

It's already there, just outside the diff context, the first line after
the variable declarations:

        lockdep_assert_held_once(&rdev->mutex.base);

> > +     if (wait_us) {
> > +             ktime_t end = ktime_add_us(rdev->last_on, wait_us);
> > +             s64 remaining = ktime_us_delta(end, ktime_get_boottime());
> > +
> > +             if (remaining > 0)
> > +                     fsleep(remaining);
>
> This style is discouraged as it makes maintenance harder. Better
>
>                 s64 remaining;
>
>                 remaining = ktime_us_delta(end, ktime_get_boottime());
>                 if (remaining > 0)
>                         fsleep(remaining);

Ack. FYI this will be rewritten based on concerns from Sashiko. The
wait will be pushed over to regulator_enable(), i.e. outside the scope
of the lock.

> > +     }
>
> ...
>
> >       /* private: Internal use */
> >       int ret;
> > +     unsigned int wait_us;
>
> If you want to make it more private (the above is only for kernel-doc) add
> __private annotation that will affect how sparse will check this.

Nice. Will add that.

> ...
>
> > -int __must_check regulator_enable(struct regulator *regulator);
> > +int __must_check regulator_enable_and_wait(struct regulator *regulator, unsigned int ms);
>
> ms or us? Please, double check all units.

Yeah this should be us.


Thanks
ChenYu

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper
  2026-07-22  8:34     ` Chen-Yu Tsai
@ 2026-08-06 20:43       ` Andy Shevchenko
  0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2026-08-06 20:43 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti,
	linux-mediatek, devicetree, linux-arm-kernel, chrome-platform,
	linux-input, linux-i2c, linux-kernel

On Wed, Jul 22, 2026 at 04:34:15PM +0800, Chen-Yu Tsai wrote:
> On Tue, Jul 21, 2026 at 6:24 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Jul 21, 2026 at 03:52:20PM +0800, Chen-Yu Tsai wrote:

...

> > > +     /*
> > > +      * Wait a bit of time for async drivers to probe and increase the
> > > +      * regulator enable count. This allows the drivers to check and
> > > +      * skip waiting for re-initialization.
> > > +      */
> > > +     if (defer_disable) {
> > > +             dev_dbg(dev, "Deferring regulator disable\n");
> > > +             msleep(100);
> >
> > How was this value chosen?
> 
> I just chose an arbitrary round value.
> 
> If both the prober and the driver for the probed device (trackpad in
> this example) are builtin, the time between enabling the node and the
> driver asynchronously probing is between 5 ms and 30 ms, but could
> also see outliers exceeding 100 ms.
> 
> If you're worried the value won't suit every user, I could make it a
> parameter of the simple helpers? And the user itself could be made to
> probe asynchronously to not block the main code path.

I think the comment above should be updated to summarize the above instead of
"a bit of time".

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2026-08-06 20:43 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-21  7:52 [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes Chen-Yu Tsai
2026-07-21  7:52 ` [PATCH v3 1/9] regulator: core: Add "enable and wait" functions Chen-Yu Tsai
2026-07-21  9:54   ` Andy Shevchenko
2026-07-22  9:06     ` Chen-Yu Tsai
2026-07-21  7:52 ` [PATCH v3 2/9] Input: elan_i2c - sort include statements Chen-Yu Tsai
2026-07-21 10:20   ` Andy Shevchenko
2026-07-21  7:52 ` [PATCH v3 3/9] Input: elan_i2c - Wait for initialization after enabling regulator supply Chen-Yu Tsai
2026-07-21  7:52 ` [PATCH v3 4/9] HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently long Chen-Yu Tsai
2026-07-21  7:52 ` [PATCH v3 5/9] i2c: of-prober: " Chen-Yu Tsai
2026-07-21  7:52 ` [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper Chen-Yu Tsai
2026-07-21 10:24   ` Andy Shevchenko
2026-07-22  8:34     ` Chen-Yu Tsai
2026-08-06 20:43       ` Andy Shevchenko
2026-07-21  7:52 ` [PATCH v3 7/9] platform/chrome: of_hw_prober: Add delay for hana trackpads Chen-Yu Tsai
2026-07-21 10:27   ` Andy Shevchenko
2026-07-22  3:33     ` Chen-Yu Tsai
2026-07-21  7:52 ` [PATCH v3 8/9] arm64: dts: mediatek: mt8173-elm-hana: Unmark trackpad supply as always-on Chen-Yu Tsai
2026-07-21  7:52 ` [PATCH v3 9/9] arm64: dts: mediatek: mt8192-asurada-spherion: Add Synaptics trackpad's supply Chen-Yu Tsai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox