* Re: [PATCH] Input: xilinx_ps2 - replace in_be32/out_be32 with ioread32be/iowrite32be
From: Michal Simek @ 2026-06-02 8:34 UTC (permalink / raw)
To: Rosen Penev
Cc: linux-input, Dmitry Torokhov, chleroy, open list,
moderated list:ARM/ZYNQ ARCHITECTURE
In-Reply-To: <CAKxU2N-M+vDeDKtYa6=M-4kazOUrGTrwJfHoF6Z+od1OZ-VBpw@mail.gmail.com>
On 6/2/26 10:27, Rosen Penev wrote:
> On Mon, Jun 1, 2026 at 11:23 PM Michal Simek <michal.simek@amd.com> wrote:
>>
>>
>>
>> On 6/2/26 05:59, Rosen Penev wrote:
>>> Mechanical conversion of the ppc4xx-specific accessors to the generic
>>> portable helpers.
>>>
>>> Allows enabling COMPILE_TEST for extra compile coverage.
>>>
>>> Assisted-by: opencode:big-pickle
>>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>>> ---
>>> drivers/input/serio/Kconfig | 2 +-
>>> drivers/input/serio/xilinx_ps2.c | 24 ++++++++++++------------
>>
>> I don't think that anybody is really using this driver and would be better to
>> just remove it.
>> If not, no issue with that changes.
> I have no problem with removing.
Can you please send a patch for it?
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH v2] HID: Input: Add battery list cleanup with devm action
From: Benjamin Tissoires @ 2026-06-02 8:30 UTC (permalink / raw)
To: linux-input, lcasmz54, Rafael Passos
Cc: hadess, jikos, linux-kernel, linux-pm, sre, kernel
In-Reply-To: <20260602030519.3097058-1-rafael@rcpassos.me>
On Tue, 02 Jun 2026 00:05:19 -0300, Rafael Passos wrote:
> The batteries list (hdev->batteries) is not cleaned up during
> hidinput_disconnect(), but struct hid_battery entries are allocated
> with devm_kzalloc.
> When a driver is unbound (e.g. during devicereprobe), devm frees those
> entries while their list_head nodesremain dangling in hdev->batteries,
> which persists across rebinds.
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.1/upstream-fixes), thanks!
[1/1] HID: Input: Add battery list cleanup with devm action
https://git.kernel.org/hid/hid/c/426e5846eba7
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* Re: [PATCH] Input: xilinx_ps2 - replace in_be32/out_be32 with ioread32be/iowrite32be
From: Rosen Penev @ 2026-06-02 8:27 UTC (permalink / raw)
To: Michal Simek
Cc: linux-input, Dmitry Torokhov, chleroy, open list,
moderated list:ARM/ZYNQ ARCHITECTURE
In-Reply-To: <79ccbd62-9aa3-41a6-a8db-1737672a8aab@amd.com>
On Mon, Jun 1, 2026 at 11:23 PM Michal Simek <michal.simek@amd.com> wrote:
>
>
>
> On 6/2/26 05:59, Rosen Penev wrote:
> > Mechanical conversion of the ppc4xx-specific accessors to the generic
> > portable helpers.
> >
> > Allows enabling COMPILE_TEST for extra compile coverage.
> >
> > Assisted-by: opencode:big-pickle
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> > drivers/input/serio/Kconfig | 2 +-
> > drivers/input/serio/xilinx_ps2.c | 24 ++++++++++++------------
>
> I don't think that anybody is really using this driver and would be better to
> just remove it.
> If not, no issue with that changes.
I have no problem with removing.
>
> Thanks,
> Michal
^ permalink raw reply
* Re: [PATCH v14 2/2] HID: cp2112: Configure I2C bus speed from firmware
From: Benjamin Tissoires @ 2026-06-02 7:59 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Danny Kaehn, dmitry.torokhov, linux-input
In-Reply-To: <20260520174401.BDE571F00893@smtp.kernel.org>
On May 20 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [Medium] The driver silently overwrites custom hardware I2C clock speeds with a 100kHz default for devices without firmware nodes.
>
> Pre-existing issues:
> - [High] Firmware nodes are attached to device structures without holding a reference, leading to a dangling pointer and use-after-free.
> - [High] The delayed work `gpio_poll_worker` is not canceled in the probe error path, leading to a use-after-free if the probe fails.
> --
>
> commit 07ff467e4d8df706a7a27476591cd82570fb8287
> Author: Danny Kaehn <danny.kaehn@plexus.com>
>
> HID: cp2112: Configure I2C bus speed from firmware
>
> Now that the I2C adapter on the CP2112 can have an associated firmware
> node, set the bus speed based on firmware configuration.
>
> > diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> > index e960fc988058f..04379db93571b 100644
> > --- a/drivers/hid/hid-cp2112.c
> > +++ b/drivers/hid/hid-cp2112.c
> [ ... ]
> > @@ -1308,6 +1309,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > goto err_power_normal;
> > }
> >
> > + i2c_parse_fw_timings(&dev->adap.dev, &timings, true);
> > +
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this new call dereference
> a dangling pointer?
Cleaning up the refcounting in the previous patch would solve this
(false positive, admitedly).
>
> Earlier in cp2112_probe(), the reference to the child firmware node is
> dropped either explicitly via fwnode_handle_put(child) in the OF path, or
> implicitly when the fwnode_for_each_child_node loop iterates or completes
> in the ACPI path:
>
> } else if (is_of_node(cp2112_fwnode)) {
> child = fwnode_get_named_child_node(cp2112_fwnode, "i2c");
> device_set_node(&dev->adap.dev, child);
> fwnode_handle_put(child);
> }
>
> device_set_node(&dev->adap.dev, child) assigns the pointer without taking
> its own reference, leaving dev->adap.dev.fwnode as a dangling pointer.
>
> This newly added i2c_parse_fw_timings() call dereferences this released
> fwnode via device_property_read_u32().
>
> Could this lead to a use-after-free?
>
> > + config.clock_speed = cpu_to_be32(timings.bus_freq_hz);
>
> [Severity: Medium]
> Since use_defaults is set to true, does this overwrite custom hardware I2C
> clock speeds with a 100kHz default for devices without firmware nodes?
>
> Before this patch, it looks like cp2112_probe() read the current SMBus
> configuration from the device, only modified retry_time, and wrote it back.
> This preserved the device's original configured clock speed (which might be
> set to 400kHz in the EEPROM).
>
> By calling i2c_parse_fw_timings() with an uninitialized local timings
> struct and use_defaults = true, timings.bus_freq_hz will be
> unconditionally set to 100kHz if the device lacks a firmware property.
>
> Is it intended to silently downgrade the speed and override user-configured
> EEPROM settings?
I think we should fix this:
i2c_parse_fw_timings(&dev->adap.dev, &timings, false);
if (timings.bus_freq_hz)
config.clock_speed = cpu_to_be32(timings.bus_freq_hz);
>
> [Severity: High]
> This is a pre-existing issue, but is there a missing cancellation of the
> delayed work in the probe error path?
This one would be nice to fix in a follow up patch.
>
> If sysfs_create_group() fails during probe, the code jumps to the
> err_gpiochip_remove label, which calls gpiochip_remove(&dev->gc) and
> returns:
>
> ret = sysfs_create_group(&hdev->dev.kobj, &cp2112_attr_group);
> if (ret < 0) {
> ...
> goto err_gpiochip_remove;
> }
> ...
> err_gpiochip_remove:
> gpiochip_remove(&dev->gc);
> err_free_i2c:
>
> Because gpiochip_add_data() was already called, an IRQ consumer could have
> concurrently requested a GPIO line, triggering cp2112_gpio_irq_startup()
> and queuing dev->gpio_poll_worker.
>
> Unlike cp2112_remove(), the error path lacks a call to
> cancel_delayed_work_sync(&dev->gpio_poll_worker).
>
> Could this mean the queued work will execute on freed memory?
>
> > config.retry_time = cpu_to_be16(1);
> >
> > ret = cp2112_hid_output(hdev, (u8 *)&config, sizeof(config),
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260520-cp2112-dt-v14-0-b1b4b6734b6f@plexus.com?part=2
>
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH v14 1/2] HID: cp2112: Add fwnode support
From: Benjamin Tissoires @ 2026-06-02 7:55 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Danny Kaehn, dmitry.torokhov, linux-input
In-Reply-To: <20260520170229.CEFA31F000E9@smtp.kernel.org>
On May 20 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Premature release of firmware node references results in use-after-free vulnerabilities when assigning the I2C adapter and GPIO chip firmware nodes.
> --
>
> commit 3dfaf368e855835bf23009205c4b93a2a070aae1
> Author: Danny Kaehn <danny.kaehn@plexus.com>
>
> HID: cp2112: Add fwnode support
>
> This commit adds firmware node support to describe the CP2112's I2C and
> GPIO interfaces. It differentiates between ACPI and DeviceTree bindings,
> mapping the child nodes to the respective adapter and gpio_chip devices.
>
> > diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> > index 803b883ae8750..e960fc988058f 100644
> > --- a/drivers/hid/hid-cp2112.c
> > +++ b/drivers/hid/hid-cp2112.c
> [ ... ]
> > @@ -1226,6 +1241,28 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > return ret;
> > }
> >
> > + cp2112_fwnode = dev_fwnode(&hdev->dev);
> > + if (is_acpi_device_node(cp2112_fwnode)) {
> > + fwnode_for_each_child_node(cp2112_fwnode, child) {
> > + ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> > + if (ret)
> > + continue;
> > +
> > + switch (addr) {
> > + case CP2112_I2C_ADR:
> > + device_set_node(&dev->adap.dev, child);
> > + break;
> > + case CP2112_GPIO_ADR:
> > + dev->gc.fwnode = child;
> > + break;
> > + }
>
> [Severity: High]
> Does this code leave dangling pointers in the device structures?
>
> The fwnode_for_each_child_node() macro automatically drops the reference to
> the child node when advancing to the next node or exiting the loop.
>
> Because device_set_node() and direct pointer assignments do not increment
> the reference count, the nodes assigned here could be prematurely freed
> resulting in a use-after-free.
Sashiko is overly cautious here. ACPI handles are not refcounted, so we
are fine.
That being said, we could unify the OF and ACPI behaviour by calling the
noops fwnode_handle_get(child) for ACPI as well and clean them up in
cp2112_remove(). This would also shut up the next warning from Sashiko
below.
>
> > + }
> > + } else if (is_of_node(cp2112_fwnode)) {
> > + child = fwnode_get_named_child_node(cp2112_fwnode, "i2c");
> > + device_set_node(&dev->adap.dev, child);
> > + fwnode_handle_put(child);
>
> [Severity: High]
> Are we releasing the firmware node prematurely here?
>
> fwnode_get_named_child_node() returns a node with an incremented reference
> count. However, since device_set_node() does not take its own reference on
> the child node, dropping the reference immediately after could leave
> dev->adap.dev with a dangling pointer if the node is later removed.
OF children are tied to the lifespan of the OF device, so we are fine,
but technically incorrect, as mentioned by Sashiko.
If you regroup the 2 behaviours and clean them in cp2112_remove(), this
would be cleaner IMO.
Cheers,
Benjamin
>
> > + }
> > +
> > ret = hid_parse(hdev);
> > if (ret) {
> > hid_err(hdev, "parse failed\n");
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260520-cp2112-dt-v14-0-b1b4b6734b6f@plexus.com?part=1
>
^ permalink raw reply
* Re: [PATCH] HID: usbhid: switch HIDBP devices to boot protocol
From: Xu Rao @ 2026-06-02 7:26 UTC (permalink / raw)
To: gregkh; +Cc: bentiss, jikos, linux-input, linux-kernel, linux-usb, raoxu
In-Reply-To: <2026060216-freezable-donated-99e5@gregkh>
Hi Greg,
> > Do not depend on firmware or a previous driver to leave the
> > interface in boot protocol. Send SET_PROTOCOL(BOOT) during probe
> > before the interrupt URB can be opened.
> >
> > Keep failures non-fatal. Some devices may reject the request while
> > still sending boot-compatible reports. For those devices, the
> > existing driver behavior is preferable.
>
> Do you really use these drivers for devices? If so, why?
>
> And shouldn't the failure be fatal? If not, why even attempt to send
> it?
After checking the actual device path again, you are right: the devices
I had in mind use the normal usbhid/hid-generic path, not usbkbd or
usbmouse.
This patch came from reading the simple HID Boot Protocol drivers and
noticing that they decode fixed boot reports without explicitly selecting
boot protocol first. So this was a code-inspection cleanup rather than a
fix for a device that I can show is bound to these drivers.
Given that I do not have a real user of usbkbd/usbmouse to justify the
change and test coverage properly, please ignore/drop this patch.
Sorry for the noise, and thanks for pointing this out.
thanks,
Xu Rao
^ permalink raw reply
* Re: [PATCH] HID: usbhid: switch HIDBP devices to boot protocol
From: Greg KH @ 2026-06-02 6:41 UTC (permalink / raw)
To: Xu Rao; +Cc: jikos, bentiss, linux-usb, linux-input, linux-kernel
In-Reply-To: <A49C63267F00CDA8+20260602014835.2624940-1-raoxu@uniontech.com>
On Tue, Jun 02, 2026 at 09:48:35AM +0800, Xu Rao wrote:
> usbkbd and usbmouse are HID Boot Protocol drivers. They do not
> parse report descriptors before decoding interrupt reports; instead,
> they interpret input data as the fixed boot keyboard and boot mouse
> formats.
>
> The HID specification defines SET_PROTOCOL as the class request
> to select report or boot protocol. Boot subclass devices may power
> up or return from reset in report protocol, and hosts are expected
> to select the protocol they intend to use.
>
> Do not depend on firmware or a previous driver to leave the
> interface in boot protocol. Send SET_PROTOCOL(BOOT) during probe
> before the interrupt URB can be opened.
>
> Keep failures non-fatal. Some devices may reject the request while
> still sending boot-compatible reports. For those devices, the
> existing driver behavior is preferable.
Do you really use these drivers for devices? If so, why?
And shouldn't the failure be fatal? If not, why even attempt to send
it?
How was this tested? On what devices?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] Input: xilinx_ps2 - replace in_be32/out_be32 with ioread32be/iowrite32be
From: Michal Simek @ 2026-06-02 6:23 UTC (permalink / raw)
To: Rosen Penev, linux-input
Cc: Dmitry Torokhov, chleroy, open list,
moderated list:ARM/ZYNQ ARCHITECTURE
In-Reply-To: <20260602035907.623599-1-rosenp@gmail.com>
On 6/2/26 05:59, Rosen Penev wrote:
> Mechanical conversion of the ppc4xx-specific accessors to the generic
> portable helpers.
>
> Allows enabling COMPILE_TEST for extra compile coverage.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> drivers/input/serio/Kconfig | 2 +-
> drivers/input/serio/xilinx_ps2.c | 24 ++++++++++++------------
I don't think that anybody is really using this driver and would be better to
just remove it.
If not, no issue with that changes.
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH] Input: synaptics - add quirk for Daffodil DC253D touchpad
From: dmitrytorokhov @ 2026-06-02 4:56 UTC (permalink / raw)
To: Ashwin Gundarapu; +Cc: linux-input, linux-kernel, ubuntu-kernel, hasan6l
In-Reply-To: <19e5047ead1.ce62bdb39017.6022546617731203250@zohomail.in>
Hi Ashwin,
On Fri, May 22, 2026 at 08:52:30PM +0530, Ashwin Gundarapu wrote:
> From ee16efae3c4371d37467c49c0cae6af808e1b625 Mon Sep 17 00:00:00 2001
> From: Ashwin Gundarapu <linuxuser509@zohomail.in>
> Date: Fri, 22 May 2026 20:48:25 +0530
> Subject: [PATCH] Input: synaptics - add quirk for Daffodil DC253D touchpad
>
> On the Daffodil DC253D laptop, the touchpad is incorrectly detected as
> a clickpad, causing the right button to be missing. Add a DMI quirk
> to clear the CLICKPAD bit for this device.
I wonder if the device can work on SMbus/RMI4 mode and if it behaves
better in RMI configuration?
>
> Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2153983
> Signed-off-by: Ashwin Gundarapu <linuxuser509@zohomail.in>
> ---
> drivers/input/mouse/synaptics.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 26071128f43a..4ca85fb4ab70 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -534,9 +534,16 @@ static int synaptics_invert_y(int y)
> * Apply quirk(s) if the hardware matches
> */
> static void synaptics_apply_quirks(struct psmouse *psmouse,
> - struct synaptics_device_info *info)
> + struct synaptics_device_info *info)
> {
> - int i;
> + int i;
> +
> + /* Daffodil DC253D has a physical right button */
> + if (dmi_match(DMI_SYS_VENDOR, "Daffodil Computers Ltd.") &&
> + dmi_match(DMI_PRODUCT_NAME, "DC253D")) {
> + info->ext_cap_0c &= ~BIT(20); /* Clear CLICKPAD bit */
> + psmouse_info(psmouse, "Force right button on Daffodil DC253D\n");
> + }
The patch is whitespace-damaged.
>
> for (i = 0; min_max_pnpid_table[i].pnp_ids; i++) {
> if (!psmouse_matches_pnp_id(psmouse,
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: xilinx_ps2 - replace in_be32/out_be32 with ioread32be/iowrite32be
From: sashiko-bot @ 2026-06-02 4:10 UTC (permalink / raw)
To: Rosen Penev; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260602035907.623599-1-rosenp@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Adding COMPILE_TEST exposes the driver to architectures without HAS_IOMEM, leading to build failures.
--
commit fa18055bb5a7f7464dd14a35a0c7198e7099cd71
Author: Rosen Penev <rosenp@gmail.com>
Input: xilinx_ps2 - replace in_be32/out_be32 with ioread32be/iowrite32be
This commit mechanically converts ppc4xx-specific accessors to generic portable
helpers. It also enables COMPILE_TEST for extra compile coverage.
> diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
> index 5f15a6462056b..60d886631c429 100644
> --- a/drivers/input/serio/Kconfig
> +++ b/drivers/input/serio/Kconfig
> @@ -190,7 +190,7 @@ config SERIO_RAW
>
> config SERIO_XILINX_XPS_PS2
> tristate "Xilinx XPS PS/2 Controller Support"
> - depends on PPC || MICROBLAZE
> + depends on PPC || MICROBLAZE || COMPILE_TEST
[Severity: Low]
Does adding COMPILE_TEST here expose this driver to architectures without
HAS_IOMEM?
Since the driver relies heavily on memory-mapped I/O functions like ioremap(),
ioread32be(), and iowrite32be(), allowing it to build on architectures that do
not support MMIO (such as ARCH=um) could result in build or linker errors.
Would it be safer to add a dependency on HAS_IOMEM?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260602035907.623599-1-rosenp@gmail.com?part=1
^ permalink raw reply
* [PATCH] Input: xilinx_ps2 - replace in_be32/out_be32 with ioread32be/iowrite32be
From: Rosen Penev @ 2026-06-02 3:59 UTC (permalink / raw)
To: linux-input
Cc: Dmitry Torokhov, chleroy, Michal Simek, open list,
moderated list:ARM/ZYNQ ARCHITECTURE
Mechanical conversion of the ppc4xx-specific accessors to the generic
portable helpers.
Allows enabling COMPILE_TEST for extra compile coverage.
Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/input/serio/Kconfig | 2 +-
drivers/input/serio/xilinx_ps2.c | 24 ++++++++++++------------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 5f15a6462056..60d886631c42 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -190,7 +190,7 @@ config SERIO_RAW
config SERIO_XILINX_XPS_PS2
tristate "Xilinx XPS PS/2 Controller Support"
- depends on PPC || MICROBLAZE
+ depends on PPC || MICROBLAZE || COMPILE_TEST
help
This driver supports XPS PS/2 IP from the Xilinx EDK on
PowerPC platform.
diff --git a/drivers/input/serio/xilinx_ps2.c b/drivers/input/serio/xilinx_ps2.c
index 411d55ca1a66..7eb96375b515 100644
--- a/drivers/input/serio/xilinx_ps2.c
+++ b/drivers/input/serio/xilinx_ps2.c
@@ -89,9 +89,9 @@ static int xps2_recv(struct xps2data *drvdata, u8 *byte)
int status = -1;
/* If there is data available in the PS/2 receiver, read it */
- sr = in_be32(drvdata->base_address + XPS2_STATUS_OFFSET);
+ sr = ioread32be(drvdata->base_address + XPS2_STATUS_OFFSET);
if (sr & XPS2_STATUS_RX_FULL) {
- *byte = in_be32(drvdata->base_address + XPS2_RX_DATA_OFFSET);
+ *byte = ioread32be(drvdata->base_address + XPS2_RX_DATA_OFFSET);
status = 0;
}
@@ -109,8 +109,8 @@ static irqreturn_t xps2_interrupt(int irq, void *dev_id)
int status;
/* Get the PS/2 interrupts and clear them */
- intr_sr = in_be32(drvdata->base_address + XPS2_IPISR_OFFSET);
- out_be32(drvdata->base_address + XPS2_IPISR_OFFSET, intr_sr);
+ intr_sr = ioread32be(drvdata->base_address + XPS2_IPISR_OFFSET);
+ iowrite32be(intr_sr, drvdata->base_address + XPS2_IPISR_OFFSET);
/* Check which interrupt is active */
if (intr_sr & XPS2_IPIXR_RX_OVF)
@@ -160,11 +160,11 @@ static int sxps2_write(struct serio *pserio, unsigned char c)
guard(spinlock_irqsave)(&drvdata->lock);
/* If the PS/2 transmitter is empty send a byte of data */
- sr = in_be32(drvdata->base_address + XPS2_STATUS_OFFSET);
+ sr = ioread32be(drvdata->base_address + XPS2_STATUS_OFFSET);
if (sr & XPS2_STATUS_TX_FULL)
return -EAGAIN;
- out_be32(drvdata->base_address + XPS2_TX_DATA_OFFSET, c);
+ iowrite32be(c, drvdata->base_address + XPS2_TX_DATA_OFFSET);
return 0;
}
@@ -189,8 +189,8 @@ static int sxps2_open(struct serio *pserio)
}
/* start reception by enabling the interrupts */
- out_be32(drvdata->base_address + XPS2_GIER_OFFSET, XPS2_GIER_GIE_MASK);
- out_be32(drvdata->base_address + XPS2_IPIER_OFFSET, XPS2_IPIXR_RX_ALL);
+ iowrite32be(XPS2_GIER_GIE_MASK, drvdata->base_address + XPS2_GIER_OFFSET);
+ iowrite32be(XPS2_IPIXR_RX_ALL, drvdata->base_address + XPS2_IPIER_OFFSET);
(void)xps2_recv(drvdata, &c);
return 0; /* success */
@@ -207,8 +207,8 @@ static void sxps2_close(struct serio *pserio)
struct xps2data *drvdata = pserio->port_data;
/* Disable the PS2 interrupts */
- out_be32(drvdata->base_address + XPS2_GIER_OFFSET, 0x00);
- out_be32(drvdata->base_address + XPS2_IPIER_OFFSET, 0x00);
+ iowrite32be(0x00, drvdata->base_address + XPS2_GIER_OFFSET);
+ iowrite32be(0x00, drvdata->base_address + XPS2_IPIER_OFFSET);
free_irq(drvdata->irq, drvdata);
}
@@ -278,13 +278,13 @@ static int xps2_of_probe(struct platform_device *ofdev)
}
/* Disable all the interrupts, just in case */
- out_be32(drvdata->base_address + XPS2_IPIER_OFFSET, 0);
+ iowrite32be(0, drvdata->base_address + XPS2_IPIER_OFFSET);
/*
* Reset the PS2 device and abort any current transaction,
* to make sure we have the PS2 in a good state.
*/
- out_be32(drvdata->base_address + XPS2_SRST_OFFSET, XPS2_SRST_RESET);
+ iowrite32be(XPS2_SRST_RESET, drvdata->base_address + XPS2_SRST_OFFSET);
dev_info(dev, "Xilinx PS2 at 0x%08llX mapped to 0x%p, irq=%d\n",
(unsigned long long)phys_addr, drvdata->base_address,
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2] HID: Input: Add battery list cleanup with devm action
From: Lucas Zampieri @ 2026-06-02 3:44 UTC (permalink / raw)
To: Rafael Passos
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-kernel, linux-pm, kernel
In-Reply-To: <20260602030519.3097058-1-rafael@rcpassos.me>
Hi Rafael,
Thanks for catching this and the quick fix. Turns out this is the
hidinput_cleanup_battery() hunk we dropped in the devm conversion
(patch 1/3), figuring devm would handle the cleanup. It frees the
hid_battery structs fine, but never unlinks them from hdev->batteries,
which outlives the devm scope, so the list dangles into freed memory on
the next probe. My bad.
Acked-by: Lucas Zampieri <lcasmz54@gmail.com>
^ permalink raw reply
* [PATCH v2] HID: Input: Add battery list cleanup with devm action
From: Rafael Passos @ 2026-06-02 3:05 UTC (permalink / raw)
To: linux-input, lcasmz54, bentiss
Cc: hadess, jikos, linux-kernel, linux-pm, sre, kernel, Rafael Passos
The batteries list (hdev->batteries) is not cleaned up during
hidinput_disconnect(), but struct hid_battery entries are allocated
with devm_kzalloc.
When a driver is unbound (e.g. during devicereprobe), devm frees those
entries while their list_head nodesremain dangling in hdev->batteries,
which persists across rebinds.
Fixes: 4a58ae85c3f9 ("HID: input: Add support for multiple batteries per device")
Signed-off-by: Rafael Passos <rafael@rcpassos.me>
---
Hi, I hope there is time to land this in this cycle.
Tell me if I should make any changes.
I built and tested it againts the mainline.
I dont have devices with multiple batteries to test.
Bug report link:
https://lore.kernel.org/all/20260602011949.2825852-1-rafael@rcpassos.me/
I received a sashiko review, and looked at wacom drivers for inspiration.
Changes in v2:
- removed list deletion from hidinput_disconnect
- new hidinput_cleanup_battery method
- register the cleanup call using devm action
in hidinput_setup_battery
If my searching was corect, this shoul be called when devres releases its group
The commit message changed, because it referenced the
hidinput_disconnect method.
Original patch: https://lore.kernel.org/linux-input/20260602020352.4256D1F00893@smtp.kernel.org/T/#t
The sashiko review: https://sashiko.dev/#/patchset/20260602015029.2838058-1-rafael@rcpassos.me?part=1
drivers/hid/hid-input.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index d73cfa2e73d3..c7b8c4ff7a33 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -519,6 +519,13 @@ static struct hid_battery *hidinput_find_battery(struct hid_device *dev,
return NULL;
}
+static void hidinput_cleanup_battery(void *res)
+{
+ struct hid_battery *bat = res;
+
+ list_del(&bat->list);
+}
+
static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
struct hid_field *field, bool is_percentage)
{
@@ -610,6 +617,12 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
power_supply_powers(bat->ps, &dev->dev);
list_add_tail(&bat->list, &dev->batteries);
+
+ error = devm_add_action_or_reset(&dev->dev,
+ hidinput_cleanup_battery, bat);
+ if (error)
+ return error;
+
return 0;
err_free_name:
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] HID: Input: Add battery list cleanup in hidinput_disconnect()
From: sashiko-bot @ 2026-06-02 2:03 UTC (permalink / raw)
To: Rafael Passos; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260602015029.2838058-1-rafael@rcpassos.me>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch introduces a Use-After-Free (UAF) and a soft-lockup race condition by manually unlinking `devm`-managed battery structures from `hid->batteries` during `hidinput_disconnect()`.
--
commit f0760b781af9f4b5e629ebfa0c88970cbaaea8fe
Author: Rafael Passos <rafael@rcpassos.me>
HID: Input: Add battery list cleanup in hidinput_disconnect()
The batteries list (hdev->batteries) is not cleaned up during
hidinput_disconnect(), but struct hid_battery entries are allocated
with devm_kzalloc. When a driver is unbound, devm frees those entries
while their list_head nodes remain dangling in hdev->batteries.
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index d73cfa2e73d33..95815e140fdf2 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -2408,6 +2408,15 @@ EXPORT_SYMBOL_GPL(hidinput_connect);
> void hidinput_disconnect(struct hid_device *hid)
> {
> struct hid_input *hidinput, *next;
> +#ifdef CONFIG_HID_BATTERY_STRENGTH
> + {
> + struct hid_battery *bat, *tmp;
> +
> + list_for_each_entry_safe(bat, tmp, &hid->batteries, list) {
> + list_del_init(&bat->list);
[Severity: High]
Does this introduce a use-after-free and a race condition?
If a driver releases its devres group before calling hid_hw_stop(), the
battery structures allocated with devm_kzalloc() will be freed early. For
example, wacom_mode_change_work() in drivers/hid/wacom_sys.c acts like
this:
wacom_mode_change_work()
wacom_release_resources(wacom1);
hid_hw_stop(wacom1->hdev);
Since wacom_release_resources() releases the devres group, the battery
memory is freed before hid_hw_stop() calls hidinput_disconnect(). When we
reach this loop, we will iterate over and modify already-freed memory.
Additionally, hid_hw_stop() can be called asynchronously without holding
hdev->driver_input_lock. Modifying hid->batteries here can race with
concurrent readers like hidinput_update_battery(), which iterates over
the list, potentially causing a crash or infinite loop.
Could the cleanup be handled by adding a custom action during battery setup
via devm_add_action_or_reset(), so it is guaranteed to happen exactly when
the devm resource is freed?
> + }
> + }
> +#endif
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260602015029.2838058-1-rafael@rcpassos.me?part=1
^ permalink raw reply
* Re: [bug report] Potential atomicity bug in drivers/input/joydev.c, between joydev_0x_read() and joydev_ioctl_common()
From: Ginger @ 2026-06-02 1:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
In-Reply-To: <ah2-X1_innZZTAMZ@google.com>
Hi Dmitry,
Many thanks for the clarification.
In that case, may I kindly ask if it is necessary to fix joydev with locks,
or we can leave it this way and let it just tolerate some inconsistencies?
Regards,
Ginger
On Tue, Jun 2, 2026 at 1:22 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Ginger,
>
> On Mon, Jun 01, 2026 at 03:07:05PM +0800, Ginger wrote:
> > Dear Linux kernel maintainers,
> >
> > My research-based static analyzer found a potential atomicity bug
> > within the 'drivers/input' subsystem, more specifically, in
> > 'drivers/input/joydev.c'.
> >
> > This potential issue is present as of git commit
> > eb3f4b7426cfd2b79d65b7d37155480b32259a11 of the mainline kernel.
> >
> > Potential concurrent triggering executions:
> > T0:
> > joydev_0x_read
> > --> spin_lock_irq(&input->event_lock);
> > --> read from joydev->abs
> > --> spin_unlock_irq(&input->event_lock);
> >
> > T1:
> > joydev_ioctl_common
> > --> case JSIOCSCORR:
> > --> write to joydev->abs[i] (no unlocked)
> >
> > The above trace is meant to demonstrate an illustrative example of the issue:
> > IMHO, in 'joydev_0x_read', the 'input->event_lock' is adopted to
> > serialize the read
> > accesses to joydev's fields like 'abs' and 'keypam' or input's fields
> > like 'input->key.
> > However, in either case, the write-side accesses to these fields are
> > not similarly
> > serialized.
>
> Yes, there is lack of locking in joydev. Some of this might be OK (if
> we prevent tearing on reads/writes) since the data may actually be
> obsolete immediately after we read it, while in many places we
> actually do need consistency, especially when we adjust key and axis
> maps.
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* [PATCH] HID: Input: Add battery list cleanup in hidinput_disconnect()
From: Rafael Passos @ 2026-06-02 1:50 UTC (permalink / raw)
To: linux-input, lcasmz54, bentiss
Cc: hadess, jikos, linux-kernel, linux-pm, sre, kernel, Rafael Passos
The batteries list (hdev->batteries) is not cleaned up during
hidinput_disconnect(), but struct hid_battery entries are allocated
with devm_kzalloc.
When a driver is unbound (e.g. during devicereprobe), devm frees those
entries while their list_head nodesremain dangling in hdev->batteries,
which persists across rebinds.
Fixes: 4a58ae85c3f9 ("HID: input: Add support for multiple batteries per device")
Signed-off-by: Rafael Passos <rafael@rcpassos.me>
---
Hi, I hope there is time to land this in this cycle.
Tell me if I should make any changes.
I built and tested it againts the mainline.
Bug report link:
https://lore.kernel.org/all/20260602011949.2825852-1-rafael@rcpassos.me/
drivers/hid/hid-input.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index d73cfa2e73d3..95815e140fdf 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -2408,6 +2408,15 @@ EXPORT_SYMBOL_GPL(hidinput_connect);
void hidinput_disconnect(struct hid_device *hid)
{
struct hid_input *hidinput, *next;
+#ifdef CONFIG_HID_BATTERY_STRENGTH
+ {
+ struct hid_battery *bat, *tmp;
+
+ list_for_each_entry_safe(bat, tmp, &hid->batteries, list) {
+ list_del_init(&bat->list);
+ }
+ }
+#endif
list_for_each_entry_safe(hidinput, next, &hid->inputs, list) {
list_del(&hidinput->list);
--
2.53.0
^ permalink raw reply related
* [PATCH] HID: usbhid: switch HIDBP devices to boot protocol
From: Xu Rao @ 2026-06-02 1:48 UTC (permalink / raw)
To: jikos; +Cc: bentiss, linux-usb, linux-input, linux-kernel, raoxu
usbkbd and usbmouse are HID Boot Protocol drivers. They do not
parse report descriptors before decoding interrupt reports; instead,
they interpret input data as the fixed boot keyboard and boot mouse
formats.
The HID specification defines SET_PROTOCOL as the class request
to select report or boot protocol. Boot subclass devices may power
up or return from reset in report protocol, and hosts are expected
to select the protocol they intend to use.
Do not depend on firmware or a previous driver to leave the
interface in boot protocol. Send SET_PROTOCOL(BOOT) during probe
before the interrupt URB can be opened.
Keep failures non-fatal. Some devices may reject the request while
still sending boot-compatible reports. For those devices, the
existing driver behavior is preferable.
Signed-off-by: Xu Rao <raoxu@uniontech.com>
---
drivers/hid/usbhid/usbkbd.c | 22 ++++++++++++++++++++++
drivers/hid/usbhid/usbmouse.c | 22 ++++++++++++++++++++++
2 files changed, 44 insertions(+)
diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
index 6b33e6ad0846..f3b83f0396c8 100644
--- a/drivers/hid/usbhid/usbkbd.c
+++ b/drivers/hid/usbhid/usbkbd.c
@@ -233,6 +233,26 @@ static void usb_kbd_close(struct input_dev *dev)
usb_kill_urb(kbd->irq);
}
+static void usb_kbd_set_boot_protocol(struct usb_device *dev,
+ struct usb_interface *iface)
+{
+ struct usb_host_interface *interface = iface->cur_altsetting;
+ int error;
+
+ /*
+ * usbkbd does not parse report descriptors. Make the device produce
+ * the fixed boot report format that the driver decodes.
+ */
+ error = usb_control_msg_send(dev, 0, HID_REQ_SET_PROTOCOL,
+ USB_DIR_OUT | USB_TYPE_CLASS |
+ USB_RECIP_INTERFACE, HID_BOOT_PROTOCOL,
+ interface->desc.bInterfaceNumber, NULL, 0,
+ USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
+ if (error < 0)
+ dev_dbg(&iface->dev, "failed to set boot protocol: %d\n",
+ error);
+}
+
static int usb_kbd_alloc_mem(struct usb_device *dev, struct usb_kbd *kbd)
{
if (!(kbd->irq = usb_alloc_urb(0, GFP_KERNEL)))
@@ -289,6 +309,8 @@ static int usb_kbd_probe(struct usb_interface *iface,
if (usb_kbd_alloc_mem(dev, kbd))
goto fail2;
+ usb_kbd_set_boot_protocol(dev, iface);
+
kbd->usbdev = dev;
kbd->dev = input_dev;
spin_lock_init(&kbd->leds_lock);
diff --git a/drivers/hid/usbhid/usbmouse.c b/drivers/hid/usbhid/usbmouse.c
index 7cc4f9558e5f..c1b9d95f957e 100644
--- a/drivers/hid/usbhid/usbmouse.c
+++ b/drivers/hid/usbhid/usbmouse.c
@@ -103,6 +103,26 @@ static void usb_mouse_close(struct input_dev *dev)
usb_kill_urb(mouse->irq);
}
+static void usb_mouse_set_boot_protocol(struct usb_device *dev,
+ struct usb_interface *intf)
+{
+ struct usb_host_interface *interface = intf->cur_altsetting;
+ int error;
+
+ /*
+ * usbmouse does not parse report descriptors. Make the device produce
+ * the fixed boot report format that the driver decodes.
+ */
+ error = usb_control_msg_send(dev, 0, HID_REQ_SET_PROTOCOL,
+ USB_DIR_OUT | USB_TYPE_CLASS |
+ USB_RECIP_INTERFACE, HID_BOOT_PROTOCOL,
+ interface->desc.bInterfaceNumber, NULL, 0,
+ USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
+ if (error < 0)
+ dev_dbg(&intf->dev, "failed to set boot protocol: %d\n",
+ error);
+}
+
static int usb_mouse_probe(struct usb_interface *intf, const struct usb_device_id *id)
{
struct usb_device *dev = interface_to_usbdev(intf);
@@ -138,6 +158,8 @@ static int usb_mouse_probe(struct usb_interface *intf, const struct usb_device_i
if (!mouse->irq)
goto fail2;
+ usb_mouse_set_boot_protocol(dev, intf);
+
mouse->usbdev = dev;
mouse->dev = input_dev;
--
2.50.1
^ permalink raw reply related
* Bug report in HID: input: Add support for multiple batteries per device
From: Rafael Passos @ 2026-06-02 1:19 UTC (permalink / raw)
To: lcasmz54; +Cc: bentiss, hadess, jikos, linux-input, linux-kernel, linux-pm, sre
In-Reply-To: <20260314010533.110278-4-lcasmz54@gmail.com>
Hi, I found a bug preventing preventing my laptop from booting
the mainline, and my bisection led me to this patch.
I believe I found the issue already, and will send a patch
following this report.
Decoded trace:
```
jun 01 20:03:15 debbook kernel: Oops: Oops: 0000 [#1] SMP NOPTI
jun 01 20:03:15 debbook kernel: CPU: 4 UID: 0 PID: 257 Comm: (udev-worker) Not tainted 7.1.0-rc6rcpassos #85 PREEMPT(lazy)
jun 01 20:03:15 debbook kernel: Hardware name: HP HP ProBook x360 435 G7/8735, BIOS S80 Ver. 01.17.02 06/07/2024
jun 01 20:03:15 debbook kernel: RIP: 0010:hidinput_setup_battery.isra.0 (/media/wd/code/kernel-dev/linux/drivers/hid/hid-input.c:412 /media/wd/code/kernel-dev/linux/drivers/hid/hid-input.c:523) hid
jun 01 20:03:15 debbook kernel: Code: f3 48 ab 48 8b 85 18 1c 00 00 48 39 c3 74 4b 48 8b 52 68 48 83 e8 38 8b 4a 30 eb 0e 90 48 8b 50 38 48 8d 42 c8 48 39 d3 74 30 <3b> >
All code
========
0: f3 48 ab rep stos %rax,(%rdi)
3: 48 8b 85 18 1c 00 00 mov 0x1c18(%rbp),%rax
a: 48 39 c3 cmp %rax,%rbx
d: 74 4b je 0x5a
f: 48 8b 52 68 mov 0x68(%rdx),%rdx
13: 48 83 e8 38 sub $0x38,%rax
17: 8b 4a 30 mov 0x30(%rdx),%ecx
1a: eb 0e jmp 0x2a
1c: 90 nop
1d: 48 8b 50 38 mov 0x38(%rax),%rdx
21: 48 8d 42 c8 lea -0x38(%rdx),%rax
25: 48 39 d3 cmp %rdx,%rbx
28: 74 30 je 0x5a
2a:* 3b 00 cmp (%rax),%eax <-- trapping instruction
Code starting with the faulting instruction
===========================================
0: 3b 00 cmp (%rax),%eax
jun 01 20:03:15 debbook kernel: RSP: 0018:ffffce87407c76c0 EFLAGS: 00010286
jun 01 20:03:15 debbook kernel: RAX: ffffffffffffffc8 RBX: ffff8db823bb7c18 RCX: 0000000000000007
jun 01 20:03:15 debbook kernel: RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffce87407c7718
jun 01 20:03:15 debbook kernel: RBP: ffff8db823bb6000 R08: ffffce87407c77c0 R09: ffffce87407c77bc
jun 01 20:03:15 debbook kernel: R10: ffff8db80df756c0 R11: ffff8db81f7b3000 R12: ffffce87407c76e8
jun 01 20:03:15 debbook kernel: R13: ffff8db80df756c0 R14: ffff8db823bb6000 R15: ffff8db823119600
jun 01 20:03:15 debbook kernel: FS: 00007fe5afebfd80(0000) GS:ffff8dbf6807b000(0000) knlGS:0000000000000000
jun 01 20:03:15 debbook kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
jun 01 20:03:15 debbook kernel: CR2: ffffffffffffffe4 CR3: 000000011ffe4000 CR4: 0000000000350ef0
jun 01 20:03:15 debbook kernel: Call Trace:
jun 01 20:03:15 debbook kernel: <TASK>
jun 01 20:03:15 debbook kernel: ? input_alloc_absinfo (./include/linux/slab.h:950 ./include/linux/slab.h:1188 drivers/input/input.c:442)
jun 01 20:03:15 debbook kernel: hidinput_connect (/media/wd/code/kernel-dev/linux/drivers/hid/hid-input.c:1294 /media/wd/code/kernel-dev/linux/drivers/hid/hid-input.c:2278 /media/wd/code/kernel-dev/linux/drivers/hid/hid-input.c:2345) hid
jun 01 20:03:15 debbook kernel: hid_connect (/media/wd/code/kernel-dev/linux/drivers/hid/hid-core.c:2308 (discriminator 1)) hid
jun 01 20:03:15 debbook kernel: ? vfree.part.0 (mm/vmalloc.c:3475)
jun 01 20:03:15 debbook kernel: hid_hw_start (/media/wd/code/kernel-dev/linux/drivers/hid/hid-core.c:2426 /media/wd/code/kernel-dev/linux/drivers/hid/hid-core.c:2417) hid
jun 01 20:03:15 debbook kernel: mt_probe (/media/wd/code/kernel-dev/linux/drivers/hid/hid-multitouch.c:2028) hid_multitouch
jun 01 20:03:15 debbook kernel: ? devres_open_group (./include/linux/spinlock.h:408 ./include/linux/spinlock.h:619 drivers/base/devres.c:263 drivers/base/devres.c:617)
jun 01 20:03:15 debbook kernel: hid_device_probe (/media/wd/code/kernel-dev/linux/drivers/hid/hid-core.c:2822 /media/wd/code/kernel-dev/linux/drivers/hid/hid-core.c:2859) hid
jun 01 20:03:15 debbook kernel: really_probe (drivers/base/dd.c:631 drivers/base/dd.c:709)
jun 01 20:03:15 debbook kernel: ? __pfx___device_attach_driver (drivers/base/dd.c:908 (discriminator 1))
jun 01 20:03:15 debbook kernel: __driver_probe_device (drivers/base/dd.c:871)
jun 01 20:03:15 debbook kernel: driver_probe_device (drivers/base/dd.c:901)
jun 01 20:03:15 debbook kernel: __device_attach_driver (drivers/base/dd.c:1029)
jun 01 20:03:15 debbook kernel: bus_for_each_drv (drivers/base/bus.c:500)
jun 01 20:03:15 debbook kernel: __device_attach (drivers/base/dd.c:1101)
jun 01 20:03:15 debbook kernel: ? __pfx___hid_bus_reprobe_drivers (/media/wd/code/kernel-dev/linux/drivers/hid/hid-core.c:3111) hid
jun 01 20:03:15 debbook kernel: device_reprobe (drivers/base/bus.c:847 drivers/base/bus.c:881)
jun 01 20:03:15 debbook kernel: ? __hid_bus_reprobe_drivers (/media/wd/code/kernel-dev/linux/drivers/hid/hid-core.c:3094) hid
jun 01 20:03:15 debbook kernel: bus_for_each_dev (drivers/base/bus.c:383)
jun 01 20:03:15 debbook kernel: ? __pfx___hid_bus_driver_added (/media/wd/code/kernel-dev/linux/drivers/hid/hid-core.c:2900) hid
jun 01 20:03:15 debbook kernel: __hid_bus_driver_added (/media/wd/code/kernel-dev/linux/drivers/hid/hid-core.c:3106) hid
jun 01 20:03:15 debbook kernel: bus_for_each_drv (drivers/base/bus.c:500)
jun 01 20:03:15 debbook kernel: ? __pfx_sensor_hub_driver_init (hid-sensor-hub.mod.c:?) hid_sensor_hub
jun 01 20:03:15 debbook kernel: __hid_register_driver (/media/wd/code/kernel-dev/linux/drivers/hid/hid-core.c:3134 /media/wd/code/kernel-dev/linux/drivers/hid/hid-core.c:3118) hid
jun 01 20:03:15 debbook kernel: do_one_initcall (init/main.c:1392)
jun 01 20:03:15 debbook kernel: do_init_module (kernel/module/main.c:3106)
jun 01 20:03:15 debbook kernel: init_module_from_file (kernel/module/main.c:3777)
jun 01 20:03:15 debbook kernel: idempotent_init_module (kernel/module/main.c:3789)
jun 01 20:03:15 debbook kernel: __x64_sys_finit_module (kernel/module/main.c:3815 kernel/module/main.c:3799 kernel/module/main.c:3799)
jun 01 20:03:15 debbook kernel: do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
jun 01 20:03:15 debbook kernel: ? do_sys_openat2 (./include/linux/fs.h:2553 fs/open.c:1363)
jun 01 20:03:15 debbook kernel: ? __x64_sys_openat (fs/open.c:1370 fs/open.c:1386 fs/open.c:1381 fs/open.c:1381)
jun 01 20:03:15 debbook kernel: ? do_syscall_64 (./arch/x86/include/asm/entry-common.h:63 ./include/linux/irq-entry-common.h:210 ./include/linux/irq-entry-common.h:230 ./include/linux/entry-common.h:318 arch/x86/entry/syscall_64.c:100)
jun 01 20:03:15 debbook kernel: ? do_syscall_64 (./arch/x86/include/asm/entry-common.h:63 ./include/linux/irq-entry-common.h:210 ./include/linux/irq-entry-common.h:230 ./include/linux/entry-common.h:318 arch/x86/entry/syscall_64.c:100)
jun 01 20:03:15 debbook kernel: ? do_syscall_64 (./arch/x86/include/asm/entry-common.h:63 ./include/linux/irq-entry-common.h:210 ./include/linux/irq-entry-common.h:230 ./include/linux/entry-common.h:318 arch/x86/entry/syscall_64.c:100)
jun 01 20:03:15 debbook kernel: ? __x64_sys_openat (fs/open.c:1370 fs/open.c:1386 fs/open.c:1381 fs/open.c:1381)
jun 01 20:03:15 debbook kernel: ? __fput (./include/linux/percpu_counter.h:71 ./include/linux/percpu_counter.h:272 fs/file_table.c:99 fs/file_table.c:523)
jun 01 20:03:15 debbook kernel: ? __x64_sys_close (fs/open.c:1507 fs/open.c:1492 fs/open.c:1492)
jun 01 20:03:15 debbook kernel: ? do_syscall_64 (./arch/x86/include/asm/entry-common.h:63 ./include/linux/irq-entry-common.h:210 ./include/linux/irq-entry-common.h:230 ./include/linux/entry-common.h:318 arch/x86/entry/syscall_64.c:100)
jun 01 20:03:15 debbook kernel: ? from_kgid_munged (kernel/user_namespace.c:406 kernel/user_namespace.c:512 kernel/user_namespace.c:536)
jun 01 20:03:15 debbook kernel: ? __do_sys_getgid (kernel/sys.c:1042 (discriminator 2))
jun 01 20:03:15 debbook kernel: ? do_syscall_64 (./arch/x86/include/asm/entry-common.h:63 ./include/linux/irq-entry-common.h:210 ./include/linux/irq-entry-common.h:230 ./include/linux/entry-common.h:318 arch/x86/entry/syscall_64.c:100)
jun 01 20:03:15 debbook kernel: ? do_syscall_64 (./include/linux/randomize_kstack.h:57 arch/x86/entry/syscall_64.c:92)
jun 01 20:03:15 debbook kernel: entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
jun 01 20:03:15 debbook kernel: RIP: 0033:0x7fe5affd3c69
jun 01 20:03:15 debbook kernel: Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> >
All code
========
0: ff c3 inc %ebx
2: 66 2e 0f 1f 84 00 00 cs nopw 0x0(%rax,%rax,1)
9: 00 00 00
c: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
11: 48 89 f8 mov %rdi,%rax
14: 48 89 f7 mov %rsi,%rdi
17: 48 89 d6 mov %rdx,%rsi
1a: 48 89 ca mov %rcx,%rdx
1d: 4d 89 c2 mov %r8,%r10
20: 4d 89 c8 mov %r9,%r8
23: 4c 8b 4c 24 08 mov 0x8(%rsp),%r9
28: 0f 05 syscall
2a:* 48 rex.W <-- trapping instruction
...
Code starting with the faulting instruction
===========================================
0: 48 rex.W
...
jun 01 20:03:15 debbook kernel: RSP: 002b:00007ffc4716a3d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
jun 01 20:03:15 debbook kernel: RAX: ffffffffffffffda RBX: 0000555df96b8430 RCX: 00007fe5affd3c69
jun 01 20:03:15 debbook kernel: RDX: 0000000000000004 RSI: 00007fe5afd7344d RDI: 0000000000000050
jun 01 20:03:15 debbook kernel: RBP: 0000000000000004 R08: 0000000000000000 R09: 0000555df962b170
jun 01 20:03:15 debbook kernel: R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000020000
jun 01 20:03:15 debbook kernel: R13: 00007fe5afd7344d R14: 0000555df96bb2e0 R15: 0000000000000000
jun 01 20:03:15 debbook kernel: </TASK>
jun 01 20:03:15 debbook kernel: Modules linked in: hid_sensor_hub(+) drm_buddy amdxcp drm_panel_backlight_quirks gpu_sched drm_ttm_helper ttm drm_exec i2c_algo_bit drm_s>
```
Thanks,
Rafael Passos
^ permalink raw reply
* Re: [PATCH v2] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
From: sashiko-bot @ 2026-06-01 21:31 UTC (permalink / raw)
To: d3z; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260601211828.112626-1-d3z.the.dev@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Missing cleanup on error paths in `quickspi_resume()` leaves hardware interrupts enabled.
Pre-existing issues:
- [High] Potential Use-After-Free in the IRQ handler during device removal.
- [High] Concurrent execution of hardware initialization (`reset_tic`) between PM callbacks and the IRQ thread.
--
commit 4380dc624b155997446c58c62fab2b9cced12a58
Author: Danny D <d3z.the.dev@gmail.com>
HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
This commit updates quickspi_resume() to handle s2idle suspend by restoring
the THC context and resetting the touch IC when the device is not a wake
source. It reconfigures the THC SPI/LTR settings and runs reset_tic() to
re-enumerate the device.
Closes: https://github.com/linux-surface/linux-surface/issues/1799
> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> index f669235f18839..4ae2e1718b30c 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -784,20 +784,72 @@ static int quickspi_resume(struct device *device)
[ ... ]
> + thc_interrupt_config(qsdev->thc_hw);
>
> + thc_interrupt_enable(qsdev->thc_hw, true);
>
> - ret = thc_dma_configure(qsdev->thc_hw);
> + /* The touch IC may have lost power, reset it to recover */
> + ret = reset_tic(qsdev);
> + if (ret)
> + return ret;
[Severity: High]
If reset_tic() or the subsequent thc_dma_configure() fails, does this
function return without disabling the hardware interrupts?
It appears thc_interrupt_enable() is called before reset_tic(), but
the error paths directly return without undoing the interrupt enable
or quiescing the hardware. Could this leave the device in an active
state capable of generating unhandled interrupts?
[Severity: High]
This is a pre-existing issue, but the patch newly introduces this
pattern into the resume path. Is it safe to call reset_tic() here
while hardware interrupts are actively enabled?
If a fatal error interrupt occurs during the reset window, the
threaded IRQ handler could catch it and invoke try_recover():
drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c:try_recover() {
...
ret = reset_tic(qsdev);
...
}
Since there doesn't appear to be mutual exclusion protecting the
device state recovery path, could both the PM callback and the IRQ
thread execute acpi_tic_reset() concurrently and corrupt the hardware
state?
> +
> + ret = thc_dma_configure(qsdev->thc_hw);
> + if (ret)
> + return ret;
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but does the device
removal process have a use-after-free window in the IRQ handler?
In quickspi_remove(), the DMA buffers are freed before hardware
interrupts are disabled:
drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c:quickspi_remove() {
...
quickspi_dma_deinit(qsdev);
pm_runtime_get_noresume(qsdev->dev);
quickspi_dev_deinit(qsdev);
...
}
Because the IRQ is managed by devm_request_threaded_irq(), it isn't
freed until after quickspi_remove() returns. If an interrupt fires
during this window, could the threaded handler access freed memory?
drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:read_dma_buffer() {
...
prd_tbl = &read_config->prd_tbls[prd_table_index];
mes_len = calc_message_len(prd_tbl, &nent);
...
}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260601211828.112626-1-d3z.the.dev@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
From: d3z @ 2026-06-01 21:30 UTC (permalink / raw)
To: even.xu, xinpeng.sun, jikos, bentiss
Cc: Danny D ., linux-input, linux-kernel, abhishektamboli9,
sakari.ailus
In-Reply-To: <IA1PR11MB60985B856D77963E2DBE47F9F4152@IA1PR11MB6098.namprd11.prod.outlook.com>
From: Danny D. <d3z.the.dev@gmail.com>
Hi Even,
Thanks for the quick reply and for sharing your patch.
First, the missing piece from your mail: my name is Danny D.
(d3z.the.dev@gmail.com) - please use it for any
Signed-off-by/Reported-by/Tested-by.
One note on the starting assumption, though: in its current configuration
this Surface Pro 10 doesn't go to S3 at all. /sys/power/mem_sleep only
offers s2idle:
$ cat /sys/power/mem_sleep
[s2idle]
There's no "deep" entry to select here, so every suspend goes through
s2idle and never reaches PM_SUSPEND_MEM. The touch IC still loses power
across that s2idle suspend - the same as over hibernation - which is what
gives the "recv failed: -11" on resume.
That's the catch with the attached patch: it gates the THC reconfigure on
if (last_suspend_state == PM_SUSPEND_MEM)
i.e. S3 only. On this machine that branch never runs, so it wouldn't bring
the touchscreen back here. Happy to apply it and confirm on the device, but
from the code path alone it can't help while we're s2idle-only.
What I did instead was key off whether the device actually kept power
rather than the suspend type: run the full reset only when the device isn't
a wake source (device_may_wakeup() == false), mirroring quickspi_restore().
A wake-enabled device stays on the light path so wake-on-touch is preserved;
one that lost power goes through reset_tic() re-enumeration. That fires on
both s2idle and S3, which is what makes it work on the SP10.
I've tested this on the Surface Pro 10 across many s2idle suspend/resume
cycles - touch and pen both keep working - and just posted a v2 that adds
the device_may_wakeup() gate.
Could we converge on the wakeup-based condition? I'm glad to test any
revision of your patch on the hardware, and equally glad for you to carry
the fix with my Signed-off-by/Tested-by once it handles the s2idle case.
Thanks,
Danny
^ permalink raw reply
* [PATCH v2] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
From: d3z @ 2026-06-01 21:18 UTC (permalink / raw)
To: even.xu, jikos, bentiss
Cc: xinpeng.sun, linux-input, linux-kernel, sakari.ailus,
abhishektamboli9, Danny D .
In-Reply-To: <20260529222210.1532876-1-d3z.the.dev@gmail.com>
From: Danny D. <d3z.the.dev@gmail.com>
On the Surface Pro 10 (Meteor Lake) the touchscreen stops working after a
suspend/resume cycle and only recovers after a reboot. The driver logs
"GET_DEVICE_INFO: recv failed: -11" on resume.
This platform suspends through s2idle: /sys/power/mem_sleep exposes
"[s2idle]" as the only state, there is no "deep"/S3 entry at all. The
touch IC nonetheless loses power across that s2idle suspend, the same
way it does across hibernation. quickspi_resume() only re-selects the
THC port, restores interrupts and DMA and sends a HIDSPI_ON command,
assuming the touch IC kept its power and state. When it has actually
lost power the HIDSPI_ON command is never acknowledged and the
descriptor read fails, leaving the touchscreen dead until the module is
reloaded.
quickspi_restore() already handles this for hibernation by
reconfiguring the THC SPI/LTR settings and running reset_tic() to
re-enumerate the device. Make quickspi_resume() do the same when the
device is not a wake source. A wake-enabled device keeps its power and
state across suspend, so it stays on the light restore path: resetting
it would discard a pending wake touch event and break wake-on-touch.
The non-wake path mirrors the existing quickspi_restore() sequence,
including enabling interrupts before reset_tic(), so it introduces no
new ordering relative to code already in the driver.
This change has been validated on a Surface Pro 10 running the
linux-surface kernel across multiple s2idle suspend/resume cycles; it
has not been tested on a mainline build.
Closes: https://github.com/linux-surface/linux-surface/issues/1799
Signed-off-by: Danny D. <d3z.the.dev@gmail.com>
---
v1 -> v2:
- Only run the full reset when the device is not a wake source
(device_may_wakeup()), so wake-on-touch is no longer disturbed.
- Reword the changelog around s2idle: the SP10 has no "deep"/S3 state, the
touch IC loses power across s2idle.
.../hid/intel-thc-hid/intel-quickspi/pci-quickspi.c | 60 +++++++++++++++++--
1 file changed, 56 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -784,20 +784,72 @@ static int quickspi_resume(struct device
if (ret)
return ret;
+ /*
+ * A wake-enabled device keeps its power and state across suspend, so
+ * only restore the THC context. Resetting it here would discard a
+ * pending wake touch event and break wake-on-touch.
+ */
+ if (device_may_wakeup(qsdev->dev)) {
+ thc_interrupt_config(qsdev->thc_hw);
+
+ thc_interrupt_enable(qsdev->thc_hw, true);
+
+ ret = thc_dma_configure(qsdev->thc_hw);
+ if (ret)
+ return ret;
+
+ return thc_interrupt_quiesce(qsdev->thc_hw, false);
+ }
+
+ /*
+ * Otherwise the touch IC may have lost power across suspend. On
+ * platforms that suspend through s2idle (for example the Surface Pro
+ * 10, whose firmware exposes s2idle as the only mem_sleep state) the
+ * IC loses power the same way it does across hibernation. A plain
+ * HIDSPI_ON is then not acknowledged and the descriptor read fails, so
+ * re-enumerate the device through the full reset flow already used by
+ * quickspi_restore().
+ */
+ thc_spi_input_output_address_config(qsdev->thc_hw,
+ qsdev->input_report_hdr_addr,
+ qsdev->input_report_bdy_addr,
+ qsdev->output_report_addr);
+
+ ret = thc_spi_read_config(qsdev->thc_hw, qsdev->spi_freq_val,
+ qsdev->spi_read_io_mode,
+ qsdev->spi_read_opcode,
+ qsdev->spi_packet_size);
+ if (ret)
+ return ret;
+
+ ret = thc_spi_write_config(qsdev->thc_hw, qsdev->spi_freq_val,
+ qsdev->spi_write_io_mode,
+ qsdev->spi_write_opcode,
+ qsdev->spi_packet_size,
+ qsdev->performance_limit);
+ if (ret)
+ return ret;
+
thc_interrupt_config(qsdev->thc_hw);
thc_interrupt_enable(qsdev->thc_hw, true);
- ret = thc_dma_configure(qsdev->thc_hw);
+ /* The touch IC may have lost power, reset it to recover */
+ ret = reset_tic(qsdev);
if (ret)
return ret;
- ret = thc_interrupt_quiesce(qsdev->thc_hw, false);
+ ret = thc_dma_configure(qsdev->thc_hw);
if (ret)
return ret;
- if (!device_may_wakeup(qsdev->dev))
- return quickspi_set_power(qsdev, HIDSPI_ON);
+ thc_ltr_config(qsdev->thc_hw,
+ qsdev->active_ltr_val,
+ qsdev->low_power_ltr_val);
+
+ thc_change_ltr_mode(qsdev->thc_hw, THC_LTR_MODE_ACTIVE);
+
+ qsdev->state = QUICKSPI_ENABLED;
return 0;
}
--
2.43.0
^ permalink raw reply
* Re: [PATCH v14 1/2] HID: cp2112: Add fwnode support
From: Andy Shevchenko @ 2026-06-01 19:18 UTC (permalink / raw)
To: Danny Kaehn
Cc: Benjamin Tissoires, Andi Shyti, Conor Dooley, Jiri Kosina,
linux-input, Dmitry Torokhov, Ethan Twardy, linux-i2c,
linux-kernel, Leo Huang, Arun D Patil, Willie Thai, Ting-Kai Chen,
Bartosz Golaszewski
In-Reply-To: <20260520-cp2112-dt-v14-1-b1b4b6734b6f@plexus.com>
On Wed, May 20, 2026 at 11:13:06AM -0500, Danny Kaehn wrote:
> Support describing the CP2112's I2C and GPIO interfaces in firmware.
>
> Bindings between the firmware nodes and the functions of the device
> are distinct between ACPI and DeviceTree.
>
> For ACPI, the i2c_adapter will use the child with _ADR equal to Zero
> and the gpio_chip will use the child with _ADR equal to One.
>
> For DeviceTree, the i2c_adapter will use the child with name "i2c",
> but the gpio_chip will share a firmware node with the CP2112.
As we seems settled on the approaches in both cases, I think we are fine
to go
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 3/3] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing
From: sashiko-bot @ 2026-06-01 18:47 UTC (permalink / raw)
Cc: linux-input, dmitry.torokhov
In-Reply-To: <tencent_0EE402E968E17B8EACEDFF0E9EBDDBF17E09@qq.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The i2c-hid-of driver uses the ACPI _DSM fallback to obtain the HID descriptor address but does not provide a restore_sequence callback to re-evaluate the _DSM during hibernation (S4) restore.
--
commit 3ebee1e22f96c1e99e3c6adbd49d92242f2497f9
Author: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing
This commit updates the i2c-hid-of driver to fall back to the ACPI _DSM
method for obtaining the HID descriptor address when the "hid-descr-addr"
property is missing. It specifically targets devices using the "PRP0001"
_HID with a "hid-over-i2c" compatible string. Safe default power and reset
delays are also configured for these devices.
> diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
> index 57379b77e9778..e925e2d2cfe0d 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-of.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-of.c
> @@ -92,6 +92,36 @@ static int i2c_hid_of_probe(struct i2c_client *client)
> ihid_of->ops.power_down = i2c_hid_of_power_down;
>
> ret = device_property_read_u32(dev, "hid-descr-addr", &val);
> + if (ret) {
> + /*
> + * Some devices, for example the Lenovo KaiTian N60d and Inspur
> + * CP300L3, declare their I2C HID touchpad with _HID "PRP0001"
> + * and _DSD compatible "hid-over-i2c" but lack the
> + * "hid-descr-addr" property. Fall back to _DSM to obtain the
> + * HID descriptor address.
> + */
> + int dsm_ret = i2c_hid_core_acpi_get_descriptor(dev);
[Severity: High]
Does this missing PM callback break S4 hibernation restore?
The native i2c-hid-acpi driver re-evaluates the _DSM method during hibernation
restore via its restore_sequence callback to reinitialize the hardware state,
such as switching the hardware back from PS/2 emulation to I2C-HID mode.
Because this ACPI _DSM fallback is added in i2c_hid_of_probe() without also
wiring up ops.restore_sequence in the OF driver, will these touchpads fail
to resume correctly and become unresponsive after waking from S4?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260601181510.38705-1-Yeking@Red54.com?part=3
^ permalink raw reply
* Re: [PATCH v2 3/4] regulator: Add driver for MediaTek MT6328 PMIC regulators
From: Mark Brown @ 2026-06-01 18:41 UTC (permalink / raw)
To: y.oudjana
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Flora Fu, Alexandre Mergnat,
Liam Girdwood, Dmitry Torokhov, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, linux-input
In-Reply-To: <20260531-mt6735-pwrap-mt6328-pmic-v2-3-dac37bf92894@protonmail.com>
[-- Attachment #1: Type: text/plain, Size: 1887 bytes --]
On Sun, May 31, 2026 at 11:10:44AM +0200, Yassine Oudjana via B4 Relay wrote:
> @@ -0,0 +1,500 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * MediaTek MT6328 regulator driver
> + * Based on MT6323 driver.
> + *
> + * Copyright (c) 2016 MediaTek Inc.
> + * Copyright (c) 2022 Yassine Oudjana <y.oudjana@protonmail.com>
> + */
Please make the entire comment a C++ one so things look more
intentional.
> +static const unsigned int ldo_volt_table3[] = {
> + 0, 0, 0, 1800000, 1900000, 2000000, 2100000, 2200000
> +};
Use linear_min_sel for the first three values.
> +static const struct linear_range ldo_volt_range1[] = {
> + REGULATOR_LINEAR_RANGE(700000, 0, 0x7f, 6250)
> +};
If there's just one range there's no need for a lookup table, use
regulator_map_voltage_linar().
> +static int mt6328_get_status(struct regulator_dev *rdev)
> +{
> + int ret;
> + u32 regval;
> + struct mt6328_regulator_info *info = rdev_get_drvdata(rdev);
> +
> + ret = regmap_read(rdev->regmap, info->desc.enable_reg, ®val);
> + if (ret != 0) {
> + dev_err(&rdev->dev, "Failed to get enable reg: %d\n", ret);
> + return ret;
> + }
> +
> + return (regval & info->qi) ? REGULATOR_STATUS_ON : REGULATOR_STATUS_OFF;
> +}
get_status() should report the actual status of the regulator, not what
was configured. If the device can't report this just omit the
operation.
> + if (mt6328_regulators[i].vselctrl_reg) {
> + if (regmap_read(mt6328->regmap,
> + mt6328_regulators[i].vselctrl_reg,
> + ®val) < 0) {
> + dev_err(&pdev->dev,
> + "Failed to read buck ctrl\n");
> + return -EIO;
Better to return the actual error.
> + rdev = devm_regulator_register(&pdev->dev,
> + &mt6328_regulators[i].desc, &config);
> + if (IS_ERR(rdev)) {
> + dev_err(&pdev->dev, "failed to register %s\n",
> + mt6328_regulators[i].desc.name);
> + return PTR_ERR(rdev);
dev_err_probe().
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 02/10] [v3] input: gpio-keys: make legacy gpiolib optional
From: Arnd Bergmann @ 2026-06-01 18:32 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-input, Dmitry Torokhov, Hans de Goede, Jin Yao
In-Reply-To: <20260530004519.1C8E81F00893@smtp.kernel.org>
On Sat, May 30, 2026, at 02:45, sashiko-bot@kernel.org wrote:
> [Severity: High]
> Does this change break legacy devices that legitimately provide both a valid
> GPIO and a valid IRQ?
>
> If a legacy platform provides both a valid GPIO and an IRQ, this condition
> now evaluates to false.
>
> By skipping the GPIO request entirely, the driver falls back to treating the
> device as a pure IRQ button. This forces the driver to lose hardware
> debouncing and rely on a simulated release timer instead of checking the
> physical GPIO line.
There are only three drivers that provide the ->irq field. Two are the
ones that I'm changing in this patch, the soc_button_array driver
is the third and none of them set ->irq and ->gpio at the same time.
>> */
>> struct gpio_keys_button {
>> unsigned int code;
>> +#ifdef CONFIG_GPIOLIB_LEGACY
>> int gpio;
>> +#endif
>> int active_low;
>
> [Severity: Medium]
> Will this cause build failures for drivers that assign to this field when
> CONFIG_GPIOLIB_LEGACY is disabled?
Yes, that is the intention of the patch: nothing should set the
->gpio field unless GPIOLIB_LEGACY is set.
> Other modern drivers dynamically create the platform device and pass legacy
> GPIO numbers by directly assigning to this field:
>
> drivers/input/misc/soc_button_array.c:soc_button_device_create() {
> ...
> gpio_keys[n_buttons].gpio = gpio;
> ...
> }
>
> When CONFIG_GPIOLIB_LEGACY is disabled, this results in a compilation failure
> since struct gpio_keys_button no longer has the gpio member.
I previously included a patch force-enable GPIOLIB_LEGACY in this
patch series, see
https://lore.kernel.org/all/npijagtgyad33xxlq46b7kwzydhcgt5tkgd5ihsjl6t4czbqyf@umovipwh73i2/
I made a mistake during a rebase, so my older patch was still
included in my randconfig testing but not in the series I
sent. All the other changes in it are now redundant, bit
the soc_button_array portion indeed still remains.
Hans, Dmitry, do you already have plans to deal with the
soc_button_array driver to move it away from legacy GPIOs?
So far I can see four possible ways we can deal with it,
but none that I actually like:
a) delay the problem, apply my original oneline change
to 'select GPIOLIB_LEGACY' and fix it later, so we
can make GPIOLIB_LEGACY default-disabled in 7.3.
b) add a gpiod member to struct gpio_keys_button and skip
the intermediate gpio number here. Clean it up later.
c) always pass the gpio as an interrupt, as the driver
already does for some machines
d) add dynamic device properties that duplicate the
information from ACPI/DMI, so the driver can
stop using platform data
e) disconnect gpio_keys_button from gpio-keys.c and
register the buttons to the input subsystem
directly from soc_button_device_create().
Any suggestions?
Arnd
^ 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