* Re: [PATCH v2] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
From: srinivas pandruvada @ 2026-07-20 20:59 UTC (permalink / raw)
To: Wahid7852, linux-input; +Cc: jikos, Lixu Zhang
In-Reply-To: <20260720175656.559351-1-wahidzk0091@gmail.com>
On Mon, 2026-07-20 at 23:26 +0530, Wahid7852 wrote:
> From: Wahid Khan <wahidzk0091@gmail.com>
>
> Some ISH firmware versions (observed on Tiger Lake LP, 8086:a0fc,
> GUID {33AECD58-B679-4E54-9BD9-A04D34F0C226}) periodically re-send an
> unsolicited HOSTIF_DM_ENUM_DEVICES response roughly every 79 seconds.
>
> The current guard collapses two distinct cases into one condition:
>
> if (!(response_flag) || init_done) -> bad packet + ish_hw_reset
>
> This incorrectly treats a valid post-init firmware announcement as a
> corrupted packet, triggering an ISH soft-reset and a full ISHTP
> re-initialisation cycle on each occurrence (~1100 times per day on
> affected hardware).
>
> Split the check: reject messages with no response flag as before, but
> simply discard valid ENUM_DEVICES messages that arrive after init is
> complete.
>
> Signed-off-by: Wahid Khan <wahidzk0091@gmail.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Lixu, please add your tested by. I think you already tested this.
Thanks,
Srinivas
> ---
> v2: added proper sign off
>
> drivers/hid/intel-ish-hid/ishtp-hid-client.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> index 6d64008f2..ec5e8afe7 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -113,8 +113,7 @@ static void process_recv(struct ishtp_cl
> *hid_ishtp_cl, void *recv_buf,
>
> switch (recv_msg->hdr.command & CMD_MASK) {
> case HOSTIF_DM_ENUM_DEVICES:
> - if ((!(recv_msg->hdr.command & ~CMD_MASK) ||
> - client_data->init_done)) {
> + if (!(recv_msg->hdr.command & ~CMD_MASK)) {
> ++client_data->bad_recv_cnt;
> report_bad_packet(hid_ishtp_cl,
> recv_msg,
> cur_pos,
> @@ -122,6 +121,8 @@ static void process_recv(struct ishtp_cl
> *hid_ishtp_cl, void *recv_buf,
> ish_hw_reset(ishtp_get_ishtp_device(
> hid_ishtp_cl));
> break;
> }
> + if (client_data->init_done)
> + break;
> client_data->hid_dev_count = (unsigned
> int)*payload;
> if (!client_data->hid_devices)
> client_data->hid_devices =
> devm_kcalloc(
> --
> 2.54.0
^ permalink raw reply
* Re: [PATCH v2] HID: bpf: Fix signedness bug in hid_bpf_hw_request
From: Emil Tsalapatis @ 2026-07-21 0:12 UTC (permalink / raw)
To: Guangshuo Li, Jiri Kosina, Benjamin Tissoires, Greg Kroah-Hartman,
Alexei Starovoitov, Johan Hovold, Puranjay Mohan, Kees Cook,
linux-input, linux-kernel, bpf
In-Reply-To: <20260713132608.1265541-1-lgs201920130244@gmail.com>
On Mon Jul 13, 2026 at 9:26 AM EDT, Guangshuo Li wrote:
> hid_bpf_hw_request() clamps the return value of hid_hw_raw_request() to
> the size of the caller-supplied buffer before copying data back to the
> BPF buffer.
>
> However, ret is signed while size is unsigned. If hid_hw_raw_request()
> returns a negative error code, the comparison promotes ret to size_t.
> This makes the negative value look like a very large positive value, so
> the error is clamped to size. The following memcpy() then treats the
> failed request as a successful transfer and copies stale data back to
> the caller.
>
> Handle negative return values before comparing ret with size and jump to
> the common cleanup path on error. This preserves negative error codes
> while still preventing oversized successful returns from overflowing
> the caller-supplied buffer.
>
> Fixes: 2b658c1c442e ("HID: bpf: prevent buffer overflow in hid_hw_request")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
> v2:
> - Handle negative return values before comparing the return length
> with the unsigned buffer size, as suggested by Emil Tsalapatis.
> - Use the common cleanup path for negative return values.
>
> drivers/hid/bpf/hid_bpf_dispatch.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index d0130658091b..520b8f56a514 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -445,12 +445,14 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
> reqtype,
> (u64)(long)ctx,
> true); /* prevent infinite recursions */
> -
> + if (ret < 0)
> + goto done;
> if (ret > size)
> ret = size;
> if (ret > 0)
> memcpy(buf, dma_data, ret);
>
> +done:
> kfree(dma_data);
> return ret;
> }
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: snvs_pwrkey: Improve probe error handling
From: Bui Duc Phuc @ 2026-07-21 1:59 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <al5VY95YC0Y40MJY@google.com>
Hi Dmitry,
Thank you for your feedback.
> > Use dev_err_probe() for devm_clk_get_optional_enabled() to handle
> > deferred probe correctly.
>
> In what way the does existing code handle deferred probe incorrectly?
>
With the current implementation, dev_err() is still called even when
devm_clk_get_optional_enabled() returns -EPROBE_DEFER.
Since the probe will be retried later anyway, logging an error via dev_err()
causes unnecessary log spam.
Using dev_err_probe() fixes this because under the hood (in
__dev_probe_failed()),
it handles -EPROBE_DEFER using dev_dbg() instead of dev_err():
-----------------------
case -EPROBE_DEFER:
device_set_deferred_probe_reason(dev, &vaf);
dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
break;
----------------------
This avoids log spam while maintaining deferred probe information.
Best regards,
Phuc
^ permalink raw reply
* Re: [PATCH v2 2/2] Input: st-keyscan: Improve probe error handling
From: Bui Duc Phuc @ 2026-07-21 2:20 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <al5V0Ym6zCGqPqJI@google.com>
Hi Dmitry,
Thank you for your feedback.
> > Use dev_err_probe() for devm_clk_get() so deferred probe errors are
> > handled correctly.
>
> Same here: in what way the does existing code handle deferred probe
> incorrectly?
>
This is the same case as the previous patch. The current code uses dev_err(),
which logs an error message even when devm_clk_get() returns -EPROBE_DEFER.
Since the probe will be retried later, this leads to unnecessary log spam.
Link to previous discussion for reference:
https://lore.kernel.org/all/CAABR9nHHe1AzWmqhV0d5p87qfSsBk2O7bYj-Weowyr=99wtNNA@mail.gmail.com/
Would you like me to send a v3 updating the commit message to be clearer,
such as: "Use dev_err_probe() for devm_clk_get() to prevent log spam
when probe returns -EPROBE_DEFER.""
or keep the current patch as is?
Best regards,
Phuc
^ permalink raw reply
* Re: [PATCH v2 2/2] Input: st-keyscan: Improve probe error handling
From: Bui Duc Phuc @ 2026-07-21 2:41 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <CAABR9nGCv3J0cZCb8MYVYXby6HZhoMbpfKdsNeTxFJS3dzuNwA@mail.gmail.com>
Hi Dmitry,
Additionally, using dev_err_probe() here also simplifies the error
handling path
by combining logging and returning into a single statement,
making the code shorter and cleaner.
Best regards,
Phuc
^ permalink raw reply
* RE: [PATCH v2] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
From: Zhang, Lixu @ 2026-07-21 5:04 UTC (permalink / raw)
To: srinivas pandruvada, Wahid7852, linux-input@vger.kernel.org
Cc: jikos@kernel.org
In-Reply-To: <efbf11ebb2faa0a60857408d3b820dffb890666c.camel@linux.intel.com>
>-----Original Message-----
>From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
>Sent: Tuesday, July 21, 2026 5:00 AM
>To: Wahid7852 <wahidzk0091@gmail.com>; linux-input@vger.kernel.org
>Cc: jikos@kernel.org; Zhang, Lixu <lixu.zhang@intel.com>
>Subject: Re: [PATCH v2] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from
>firmware
>
>On Mon, 2026-07-20 at 23:26 +0530, Wahid7852 wrote:
>> From: Wahid Khan <wahidzk0091@gmail.com>
>>
>> Some ISH firmware versions (observed on Tiger Lake LP, 8086:a0fc, GUID
>> {33AECD58-B679-4E54-9BD9-A04D34F0C226}) periodically re-send an
>> unsolicited HOSTIF_DM_ENUM_DEVICES response roughly every 79 seconds.
>>
>> The current guard collapses two distinct cases into one condition:
>>
>> if (!(response_flag) || init_done) -> bad packet + ish_hw_reset
>>
>> This incorrectly treats a valid post-init firmware announcement as a
>> corrupted packet, triggering an ISH soft-reset and a full ISHTP
>> re-initialisation cycle on each occurrence (~1100 times per day on
>> affected hardware).
>>
>> Split the check: reject messages with no response flag as before, but
>> simply discard valid ENUM_DEVICES messages that arrive after init is
>> complete.
>>
>> Signed-off-by: Wahid Khan <wahidzk0091@gmail.com>
>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Tested-by: Zhang Lixu <lixu.zhang@intel.com>
>
>
>Lixu, please add your tested by. I think you already tested this.
Yes, thanks.
Best regards,
Lixu
>
>Thanks,
>Srinivas
>
>> ---
>> v2: added proper sign off
>>
>> drivers/hid/intel-ish-hid/ishtp-hid-client.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
>> b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
>> index 6d64008f2..ec5e8afe7 100644
>> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
>> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
>> @@ -113,8 +113,7 @@ static void process_recv(struct ishtp_cl
>> *hid_ishtp_cl, void *recv_buf,
>>
>> switch (recv_msg->hdr.command & CMD_MASK) {
>> case HOSTIF_DM_ENUM_DEVICES:
>> - if ((!(recv_msg->hdr.command & ~CMD_MASK) ||
>> - client_data->init_done)) {
>> + if (!(recv_msg->hdr.command & ~CMD_MASK)) {
>> ++client_data->bad_recv_cnt;
>> report_bad_packet(hid_ishtp_cl,
>> recv_msg,
>> cur_pos,
>> @@ -122,6 +121,8 @@ static void process_recv(struct ishtp_cl
>> *hid_ishtp_cl, void *recv_buf,
>> ish_hw_reset(ishtp_get_ishtp_device(
>> hid_ishtp_cl));
>> break;
>> }
>> + if (client_data->init_done)
>> + break;
>> client_data->hid_dev_count = (unsigned int)*payload;
>> if (!client_data->hid_devices)
>> client_data->hid_devices =
>> devm_kcalloc(
>> --
>> 2.54.0
^ permalink raw reply
* [PATCH] Input: psxpad-spi - set driver data before use
From: Linmao Li @ 2026-07-21 5:55 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Haotian Zhang, Tomohiro Yoshidomi, David Rheinsberg, linux-input,
linux-kernel, Linmao Li, stable
psxpad_spi_suspend() retrieves the controller state with
spi_get_drvdata(), but probe never stores it, so suspend dereferences a
NULL pointer. Store it during probe.
Fixes: 8be193c7b1f4 ("Input: add support for PlayStation 1/2 joypads connected via SPI")
Cc: stable@vger.kernel.org
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
drivers/input/joystick/psxpad-spi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/joystick/psxpad-spi.c b/drivers/input/joystick/psxpad-spi.c
index f902a56d011f..b1ab2486bb0d 100644
--- a/drivers/input/joystick/psxpad-spi.c
+++ b/drivers/input/joystick/psxpad-spi.c
@@ -301,6 +301,7 @@ static int psxpad_spi_probe(struct spi_device *spi)
/* input poll device settings */
pad->idev = idev;
pad->spi = spi;
+ spi_set_drvdata(spi, pad);
/* input device settings */
input_set_drvdata(idev, pad);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] Input: psxpad-spi - set driver data before use
From: sashiko-bot @ 2026-07-21 6:05 UTC (permalink / raw)
To: Linmao Li; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260721055551.1714965-1-lilinmao@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Rumble motors are not physically stopped during system suspend, causing continuous vibration.
--
--- Patch [1]: [PATCH] Input: psxpad-spi - set driver data before use ---
commit 23ee40f181c679e252ac9e0908c0dcf15457e7b6
Author: Linmao Li <lilinmao@kylinos.cn>
Input: psxpad-spi - set driver data before use
This commit fixes a NULL pointer dereference in the suspend path by ensuring
the driver data is stored in the SPI device during probe. It prevents
psxpad_spi_suspend() from crashing when accessing the controller state.
> diff --git a/drivers/input/joystick/psxpad-spi.c b/drivers/input/joystick/psxpad-spi.c
> index f902a56d011f4..b1ab2486bb0d7 100644
> --- a/drivers/input/joystick/psxpad-spi.c
> +++ b/drivers/input/joystick/psxpad-spi.c
> @@ -301,6 +301,7 @@ static int psxpad_spi_probe(struct spi_device *spi)
> /* input poll device settings */
> pad->idev = idev;
> pad->spi = spi;
> + spi_set_drvdata(spi, pad);
>
> /* input device settings */
> input_set_drvdata(idev, pad);
[Severity: High]
This is a pre-existing issue, but will the rumble motors actually stop during
suspend?
While this patch fixes the crash when retrieving the driver data, looking at
psxpad_spi_suspend(), it calls psxpad_set_motor_level(pad, 0, 0):
static int psxpad_spi_suspend(struct device *dev)
{
struct spi_device *spi = to_spi_device(dev);
struct psxpad *pad = spi_get_drvdata(spi);
psxpad_set_motor_level(pad, 0, 0);
return 0;
}
This only updates the internal software state (pad->motor1level and
pad->motor2level). Since the driver relies entirely on its polling thread
to transmit this state via SPI to the hardware, and that poller is frozen
during suspend, is there a risk that the hardware never receives the stop
command?
Does the suspend routine need to dispatch a synchronous SPI command before
returning so the motors do not continue to vibrate indefinitely while the
system sleeps?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721055551.1714965-1-lilinmao@kylinos.cn?part=1
^ permalink raw reply
* [PATCH v3 0/9] arm64: mediatek: Chromebook trackpad supply fixes
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
* [PATCH v3 1/9] regulator: core: Add "enable and wait" functions
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-Reply-To: <20260721075226.2347933-1-wenst@chromium.org>
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
* [PATCH v3 2/9] Input: elan_i2c - sort include statements
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-Reply-To: <20260721075226.2347933-1-wenst@chromium.org>
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
* [PATCH v3 3/9] Input: elan_i2c - Wait for initialization after enabling regulator supply
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-Reply-To: <20260721075226.2347933-1-wenst@chromium.org>
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
* [PATCH v3 4/9] HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently long
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-Reply-To: <20260721075226.2347933-1-wenst@chromium.org>
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
* [PATCH v3 5/9] i2c: of-prober: skip post-power-on delay if powered on sufficiently long
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-Reply-To: <20260721075226.2347933-1-wenst@chromium.org>
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
* [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper
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-Reply-To: <20260721075226.2347933-1-wenst@chromium.org>
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
* [PATCH v3 7/9] platform/chrome: of_hw_prober: Add delay for hana trackpads
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-Reply-To: <20260721075226.2347933-1-wenst@chromium.org>
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
* [PATCH v3 8/9] arm64: dts: mediatek: mt8173-elm-hana: Unmark trackpad supply as always-on
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-Reply-To: <20260721075226.2347933-1-wenst@chromium.org>
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
* [PATCH v3 9/9] arm64: dts: mediatek: mt8192-asurada-spherion: Add Synaptics trackpad's supply
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
In-Reply-To: <20260721075226.2347933-1-wenst@chromium.org>
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
* Re: [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper
From: sashiko-bot @ 2026-07-21 8:06 UTC (permalink / raw)
To: Chen-Yu Tsai; +Cc: robh, devicetree, linux-input, conor+dt, dmitry.torokhov
In-Reply-To: <20260721075226.2347933-7-wenst@chromium.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unconditional 100ms boot delay during cleanup when no I2C component is found.
--
--- Patch [6]: [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper ---
commit ea6b3a85720b084309159275d4c96f6a15c76ff3
Author: Chen-Yu Tsai <wenst@chromium.org>
i2c: of-prober: Defer regulator_disable() on successful probe in simple helper
This commit introduces a 100ms delay before disabling the regulator during
cleanup. This allows async drivers time to probe and increment the
regulator enable count, avoiding hardware re-initialization delays.
> diff --git a/drivers/i2c/i2c-core-of-prober.c b/drivers/i2c/i2c-core-of-prober.c
> --- 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);
[ ... ]
> @@ -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);
[Severity: Medium]
Will this cause an unconditional 100ms boot delay if no I2C component is
found?
When i2c_of_probe_component() finishes its scan without finding a
responding component, it still unconditionally calls ops->cleanup():
drivers/i2c/i2c-core-of-prober.c:i2c_of_probe_component() {
...
if (ops->cleanup)
ops->cleanup(dev, ctx);
...
}
This invokes i2c_of_probe_simple_cleanup(), which now passes
defer_disable = true, triggering the msleep(100) even when nothing was
probed.
Could this delay be skipped if no component was actually found?
> i2c_of_probe_simple_put_supply(ctx);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721075226.2347933-1-wenst@chromium.org?part=6
^ permalink raw reply
* Re: [PATCH v3 4/9] HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently long
From: sashiko-bot @ 2026-07-21 8:06 UTC (permalink / raw)
To: Chen-Yu Tsai; +Cc: linux-input, dmitry.torokhov, robh, conor+dt, devicetree
In-Reply-To: <20260721075226.2347933-5-wenst@chromium.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Skipping the post-power-on delay when regulators are already enabled reduces the reset assertion pulse width to near zero, causing device initialization failures.
--
--- Patch [4]: [PATCH v3 4/9] HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently long ---
commit 77e6d660661b46c12652ca11459dae1f91a79af7
Author: Chen-Yu Tsai <wenst@chromium.org>
HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently long
This commit switches the post-power-on delay to use the new
regulator_bulk_enable_and_wait() API. This avoids unnecessary boot time
delays on devices where the power rail is always-on or left enabled by
the bootloader.
> diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
> index 59393d71ddb9d..fdaad451e7102 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-of.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-of.c
[ ... ]
> @@ -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);
[Severity: High]
Does skipping this delay cause a regression where the reset assertion pulse
becomes too short?
If the regulators are already enabled (e.g., from an always-on rail or
bootloader), regulator_bulk_enable_and_wait() will return immediately
without waiting.
Because the reset GPIO is asserted during probe just before calling
i2c_hid_of_power_up(), skipping the post-power-on delay here causes the
reset line to be de-asserted almost instantly. This reduces the reset
assertion pulse width to near zero, which might violate the minimum reset
assertion time required by the hardware.
> if (ihid_of->post_reset_delay_ms)
> msleep(ihid_of->post_reset_delay_ms);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721075226.2347933-1-wenst@chromium.org?part=4
^ permalink raw reply
* Re: [PATCH v3 3/9] Input: elan_i2c - Wait for initialization after enabling regulator supply
From: sashiko-bot @ 2026-07-21 8:07 UTC (permalink / raw)
To: Chen-Yu Tsai; +Cc: conor+dt, linux-input, devicetree, dmitry.torokhov, robh
In-Reply-To: <20260721075226.2347933-4-wenst@chromium.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] `regulator_enable_and_wait` sleeps for the specified duration while holding the WW mutexes for the regulator and all of its suppliers, degrading system concurrency.
--
--- Patch [3]: [PATCH v3 3/9] Input: elan_i2c - Wait for initialization after enabling regulator supply ---
commit 3e5bcc07a47c73771197519603fee461c8ac8685
Author: Chen-Yu Tsai <wenst@chromium.org>
Input: elan_i2c - Wait for initialization after enabling regulator supply
This commit switches the Elan trackpad driver to use the new
regulator_enable_and_wait() API. This ensures the required hardware and
firmware initialization delays are respected after the trackpad regulator
is enabled during probe and resume.
Link: https://lore.kernel.org/all/20241001093815.2481899-1-wenst@chromium.org/
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index 9f024a435dbf1..77885930bf9e6 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
[ ... ]
> @@ -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);
[Severity: High]
Will using regulator_enable_and_wait() here cause a 110ms sleep while holding
the regulator's WW mutexes?
Looking at _regulator_enable_and_wait() in drivers/regulator/core.c, the sleep
happens while the dependent locks for the regulator and all of its suppliers
are still held:
drivers/regulator/core.c:_regulator_enable_and_wait() {
...
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);
}
...
}
Could this serialize all operations on the shared PMIC/supply during probe
and resume?
If multiple consumers share a parent supply, wouldn't they block on the
parent's mutex and execute sequentially, potentially causing boot and resume
time regressions?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721075226.2347933-1-wenst@chromium.org?part=3
^ permalink raw reply
* Re: [PATCH v3 1/9] regulator: core: Add "enable and wait" functions
From: sashiko-bot @ 2026-07-21 8:08 UTC (permalink / raw)
To: Chen-Yu Tsai; +Cc: conor+dt, dmitry.torokhov, robh, linux-input, devicetree
In-Reply-To: <20260721075226.2347933-2-wenst@chromium.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The prototype for `regulator_enable_and_wait` in `consumer.h` incorrectly names the delay parameter `ms` instead of `wait_us`, inviting developers to pass millisecond values that will be incorrectly interpreted as microseconds.
- [High] Sleeping for the consumer delay while holding the recursive `ww_mutex` for the regulator and its entire parent supply chain severely degrades concurrency, blocking shared regulators and serializing async bulk enablement.
--
--- Patch [1]: [PATCH v3 1/9] regulator: core: Add "enable and wait" functions ---
commit 73793dc0bdc3050ff22074a860d438d5d9cc0b49
Author: Chen-Yu Tsai <wenst@chromium.org>
regulator: core: Add "enable and wait" functions
This commit introduces new "enable and wait" functions to the regulator core,
allowing consumers to ensure a minimum amount of time has passed since a
regulator was actually turned on. It adds a last_on timestamp to the
regulator device and updates single and bulk enable APIs to accept a wait time.
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 1797929dfe566..d14ce86d8f7bb 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
[ ... ]
> @@ -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);
> + }
> +
[Severity: High]
Does sleeping here while holding the recursive ww_mutex for the regulator and
its parent supply chain cause concurrency issues?
Since regulator_enable_and_wait() acquires these locks via
regulator_lock_dependent() before calling _regulator_enable_and_wait(), this
sleep happens while the locks are held.
In cases like regulator_bulk_enable_and_wait() where async workers are created
to enable regulators in parallel, won't this serialize the wait times for any
consumers sharing a parent supply, defeating the purpose of the async design?
> return 0;
>
> err_consumer_disable:
[ ... ]
> diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
> index 56fe2693d9b22..a69157c9b5b51 100644
> --- a/include/linux/regulator/consumer.h
> +++ b/include/linux/regulator/consumer.h
[ ... ]
> @@ -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);
[Severity: High]
Could the naming of this parameter cause consumers to inadvertently pass
millisecond values that get interpreted as microseconds?
The prototype here defines the parameter as ms, but the implementation and
the parameter passed to ktime_add_us() inside _regulator_enable_and_wait()
treat it strictly as microseconds.
If a driver developer passes 10 expecting a 10-millisecond delay based on this
header, the core will interpret it as 10 microseconds. Could this lead to
hardware initialization failures due to premature access to unstable power
supplies?
> int regulator_disable(struct regulator *regulator);
> int regulator_force_disable(struct regulator *regulator);
> int regulator_is_enabled(struct regulator *regulator);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721075226.2347933-1-wenst@chromium.org?part=1
^ permalink raw reply
* Re: [PATCH v3 1/9] regulator: core: Add "enable and wait" functions
From: Chen-Yu Tsai @ 2026-07-21 8:22 UTC (permalink / raw)
To: sashiko-reviews, Mark Brown
Cc: conor+dt, dmitry.torokhov, robh, linux-input, devicetree,
linux-arm-kernel
In-Reply-To: <20260721080829.4153C1F000E9@smtp.kernel.org>
On Tue, Jul 21, 2026 at 4:08 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The prototype for `regulator_enable_and_wait` in `consumer.h` incorrectly names the delay parameter `ms` instead of `wait_us`, inviting developers to pass millisecond values that will be incorrectly interpreted as microseconds.
> - [High] Sleeping for the consumer delay while holding the recursive `ww_mutex` for the regulator and its entire parent supply chain severely degrades concurrency, blocking shared regulators and serializing async bulk enablement.
> --
>
> --- Patch [1]: [PATCH v3 1/9] regulator: core: Add "enable and wait" functions ---
> commit 73793dc0bdc3050ff22074a860d438d5d9cc0b49
> Author: Chen-Yu Tsai <wenst@chromium.org>
>
> regulator: core: Add "enable and wait" functions
>
> This commit introduces new "enable and wait" functions to the regulator core,
> allowing consumers to ensure a minimum amount of time has passed since a
> regulator was actually turned on. It adds a last_on timestamp to the
> regulator device and updates single and bulk enable APIs to accept a wait time.
>
> > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> > index 1797929dfe566..d14ce86d8f7bb 100644
> > --- a/drivers/regulator/core.c
> > +++ b/drivers/regulator/core.c
>
> [ ... ]
>
> > @@ -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);
> > + }
> > +
>
> [Severity: High]
> Does sleeping here while holding the recursive ww_mutex for the regulator and
> its parent supply chain cause concurrency issues?
>
> Since regulator_enable_and_wait() acquires these locks via
> regulator_lock_dependent() before calling _regulator_enable_and_wait(), this
> sleep happens while the locks are held.
>
> In cases like regulator_bulk_enable_and_wait() where async workers are created
> to enable regulators in parallel, won't this serialize the wait times for any
> consumers sharing a parent supply, defeating the purpose of the async design?
I suppose the fsleep() needs to be moved outside the lock region.
So instead of changing _regulator_enable(), make it return the last_on
timestamp, and then act on it in regulator_enable_and_wait() after the
lock is released.
> > return 0;
> >
> > err_consumer_disable:
>
> [ ... ]
>
> > diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
> > index 56fe2693d9b22..a69157c9b5b51 100644
> > --- a/include/linux/regulator/consumer.h
> > +++ b/include/linux/regulator/consumer.h
>
> [ ... ]
>
> > @@ -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);
>
> [Severity: High]
> Could the naming of this parameter cause consumers to inadvertently pass
> millisecond values that get interpreted as microseconds?
Yes I missed this part when I changed the API to use microseconds
instead of milliseconds. Will fix in the next version.
ChenYu
> The prototype here defines the parameter as ms, but the implementation and
> the parameter passed to ktime_add_us() inside _regulator_enable_and_wait()
> treat it strictly as microseconds.
>
> If a driver developer passes 10 expecting a 10-millisecond delay based on this
> header, the core will interpret it as 10 microseconds. Could this lead to
> hardware initialization failures due to premature access to unstable power
> supplies?
>
> > int regulator_disable(struct regulator *regulator);
> > int regulator_force_disable(struct regulator *regulator);
> > int regulator_is_enabled(struct regulator *regulator);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260721075226.2347933-1-wenst@chromium.org?part=1
^ permalink raw reply
* Re: [PATCH v4 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Fenglin Wu @ 2026-07-21 9:19 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-arm-msm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Stephen Boyd, Bjorn Andersson, Konrad Dybcio,
David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <alpWEL1hJgQ18I7d@google.com>
On 7/18/2026 12:51 AM, Dmitry Torokhov wrote:
>> +
>> +static void qcom_haptics_remove(struct platform_device *pdev)
>> +{
>> + struct qcom_haptics *h = platform_get_drvdata(pdev);
>> +
>> + /*
>> + * Unregister the input device explicitly at the beginning
>> + * to avoid the input device being used after the resources
>> + * are freed.
>> + */
>> + input_unregister_device(h->input);
>> + disable_delayed_work_sync(&h->play_work);
>> + scoped_guard(mutex, &h->play_lock) {
>> + haptics_queue_flush(h);
>> + haptics_stop_locked(h, true);
>> + }
>> +
>> + haptics_enable_module(h, false);
>> +
>> + scoped_guard(mutex, &h->fifo_lock) {
>> + for (int i = 0; i < HAPTICS_MAX_EFFECTS; i++) {
>> + kvfree(h->effects[i].fifo_data);
>> + h->effects[i].fifo_data = NULL;
>> + }
>> + }
>
> Most of this is better suited to close() method of input device.
Hi Dmitry,
I tried moving this into the close() function of the input device, but
I’m running into an issue: 'disable_delayed_work_sync(&h->play_work)'
may be called multiple times when the input device is opened and closed,
and the delayed work can sometimes end up being disabled unexpectedly.
That would mean I need another flag to track whether the delayed work is
enabled. Do you see any concerns with that, or should I still keep it in
remove()?
Thanks
Fenglin
>
>> +}
>
^ permalink raw reply
* [PATCH v10 0/7] mfd: Add support for Asus Transformer embedded controller
From: Svyatoslav Ryhel @ 2026-07-21 9:52 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Lee Jones, Pavel Machek, Sebastian Reichel, Svyatoslav Ryhel,
Ion Agorria, Michał Mirosław
Cc: devicetree, linux-kernel, linux-input, linux-leds, linux-pm
Add support for embedded controller used in Asus Transformers for
managing power and input functions.
---
Changes in v2:
- converted sysfs debug exports into debugfs
- added kernel-doc comments for exposed functions
- fixed minor typos and inconsistencies
Changes in v3:
- dropped DockRAM commits (both schema and driver)
- integrated DockRAM functionality directly into the controller driver
- EC schema moved to embedded controllers folder
- removed all cell descriptions from the schema
- removed all compatibles from the cell drivers
- adjusted naming conventions to better align with the ASUS Transformers
- defined EC variant sets to provide coverage for all known devices
Changes in v4:
- grouped known programming models of EC chronologically (both schema
and driver)
- call debugfs init only if CONFIG_DEBUG_FS is enabled
Changes in v5:
- added asus,tf600t-ec-dock compatible to schema
- rebased on top of v7.0
- kzalloc > kzalloc_obj in keys and kbc
Changes in v6:
- removed asus_ec_handle_smi casting
- asus_dockram_access_ctl > asus_ec_get_ctl in control_reg_*
- fixed battery Kconfig description
Changes in v7:
- added status check of devm_kasprintf where missing
- devm_asus_ec_register_notifier dropped, added .remove where it was used
- removed cell_to_ec, asus_dockram_read, asus_dockram_write, asus_ec_* public API
asus_ec_i2c_command, devm_asus_ec_register_notifier, asus_ec_read, asus_ec_write
- renamed asusec_info > asusec_core
- ec-kbc: added packed size check
ret > error
improved key remap logic
- ec-keys: improve formatting and comments
ret > error
switched to dev_err_probe
- ec-leds: reworked to register both leds via loop
- ec-mfd: adjusted Kconfig description
fixed smbus operation sizes
fixed saving of EC fw model
adjusted IRQ flags
converted to use definer for set cell composition
added factory mode states enum and handling
defined some "magic" values
self > client, info > ec, ecreq > ecreq_gpio, priv > ddata
asus_ec_chip_data data > asus_ec_chip_info info
ec_data > ec_buf, ctl_data > ctl_buf
added and improved comments, added structure descriptions
asus_ec_magic_debug > asus_ec_susb_on_status
removed all dev_dbg and most of dev_info
pronts with model, fw behavior, factory and susb state preserved
switched to MFD_CELL_* macros
removed debugfs
- ec-battery: swithced to BIT macro
lock usage moved to asus_ec_battery_get_value
in asus_ec_battery_poll_work fixed possible rescheduling fail
in asus_ec_battery_poll_work fixed missing not charging
- ec-charger: POWER_SUPPLY_PROP_MODEL_NAME set as the first check
Changes in v8:
- added MODULE_ALIAS
- renamed DOCKRAM_* to ASUSEC_*
- ec-keys: input_handler moved into private structure
- ec-leds: added brightness_get
- ec-mdf: fixed i2c_smbus_* return checks ()
improved model storing
- ec-batt: added status check of devm_kasprintf
Changes in v9:
- fixed i2c_smbus_read_i2c_block_data return check
- blocking_notifier_chain_register moved before serio registration
- adjusted get_unaligned_le16 bounds check
- unsigned long long > u64
- iterator vars made scoped
- removed "magic" values from ec-mfd
- simplified logging, detect split into detect and init
- improved error logs formatting
- adjusted handler in media keys to connect strictly to dock keyboard
Changes in v10:
- ec-kbc: fixed possible out of buffer reads
- ec-kbc: set notifier_call before blocking_notifier_chain_register
- ec-keys: dropped input_unregister_device which should be handled by devm_input_allocate_device
---
Michał Mirosław (6):
mfd: Add driver for ASUS Transformer embedded controller
input: serio: Add driver for ASUS Transformer dock keyboard and
touchpad
input: keyboard: Add driver for ASUS Transformer dock multimedia keys
leds: Add driver for ASUS Transformer LEDs
power: supply: Add driver for ASUS Transformer battery
power: supply: Add charger driver for Asus Transformers
Svyatoslav Ryhel (1):
dt-bindings: embedded-controller: document ASUS Transformer EC
.../asus,tf201-ec-pad.yaml | 119 ++++
drivers/input/keyboard/Kconfig | 10 +
drivers/input/keyboard/Makefile | 1 +
.../input/keyboard/asus-transformer-ec-keys.c | 310 ++++++++++
drivers/input/serio/Kconfig | 15 +
drivers/input/serio/Makefile | 1 +
drivers/input/serio/asus-transformer-ec-kbc.c | 168 ++++++
drivers/leds/Kconfig | 11 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-asus-transformer-ec.c | 125 ++++
drivers/mfd/Kconfig | 16 +
drivers/mfd/Makefile | 1 +
drivers/mfd/asus-transformer-ec.c | 549 ++++++++++++++++++
drivers/power/supply/Kconfig | 22 +
drivers/power/supply/Makefile | 2 +
.../supply/asus-transformer-ec-battery.c | 289 +++++++++
.../supply/asus-transformer-ec-charger.c | 208 +++++++
include/linux/mfd/asus-transformer-ec.h | 92 +++
18 files changed, 1940 insertions(+)
create mode 100644 Documentation/devicetree/bindings/embedded-controller/asus,tf201-ec-pad.yaml
create mode 100644 drivers/input/keyboard/asus-transformer-ec-keys.c
create mode 100644 drivers/input/serio/asus-transformer-ec-kbc.c
create mode 100644 drivers/leds/leds-asus-transformer-ec.c
create mode 100644 drivers/mfd/asus-transformer-ec.c
create mode 100644 drivers/power/supply/asus-transformer-ec-battery.c
create mode 100644 drivers/power/supply/asus-transformer-ec-charger.c
create mode 100644 include/linux/mfd/asus-transformer-ec.h
--
2.53.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox