* [PATCH] dt-bindings: mfd: twl: Convert trivial subdevices to json-schema
From: Andreas Kemnade @ 2024-03-18 12:40 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
alexandre.belloni, wim, linux, andreas, linux-input, devicetree,
linux-kernel, linux-rtc, linux-watchdog, linux-omap, sre
Convert subdevices with just an interrupt and compatbile to
json-schema and wire up already converted subdevices.
RTC is available in all variants, so allow it unconditionally
GPADC binding for TWL603X uses two different compatibles, so
specify just the compatible and not include it.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
Well, my name is in that yaml file, so I should take care of my sheep,
in case a step-by-step approach is acceptable this is at least a
checkpoint for me that I understand multi file binding mechanics
properly.
.../bindings/input/twl4030-pwrbutton.txt | 21 ------
.../devicetree/bindings/mfd/ti,twl.yaml | 68 +++++++++++++++++++
.../devicetree/bindings/rtc/twl-rtc.txt | 11 ---
.../bindings/watchdog/twl4030-wdt.txt | 10 ---
4 files changed, 68 insertions(+), 42 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
delete mode 100644 Documentation/devicetree/bindings/rtc/twl-rtc.txt
delete mode 100644 Documentation/devicetree/bindings/watchdog/twl4030-wdt.txt
diff --git a/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
deleted file mode 100644
index 6c201a2ba8acf..0000000000000
--- a/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
+++ /dev/null
@@ -1,21 +0,0 @@
-Texas Instruments TWL family (twl4030) pwrbutton module
-
-This module is part of the TWL4030. For more details about the whole
-chip see Documentation/devicetree/bindings/mfd/ti,twl.yaml.
-
-This module provides a simple power button event via an Interrupt.
-
-Required properties:
-- compatible: should be one of the following
- - "ti,twl4030-pwrbutton": For controllers compatible with twl4030
-- interrupts: should be one of the following
- - <8>: For controllers compatible with twl4030
-
-Example:
-
-&twl {
- twl_pwrbutton: pwrbutton {
- compatible = "ti,twl4030-pwrbutton";
- interrupts = <8>;
- };
-};
diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
index 52ed228fb1e7e..03d725d5294db 100644
--- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
+++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
@@ -15,6 +15,65 @@ description: |
USB transceiver or Audio amplifier.
These chips are connected to an i2c bus.
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ti,twl4030
+ then:
+ properties:
+ madc:
+ type: object
+ $ref: ../iio/adc/ti,twl4030-madc.yaml
+
+ unevaluatedProperties: false
+ bci:
+ type: object
+ $ref: ../power/supply/twl4030-charger.yaml
+
+ unevaluatedProperties: false
+ pwrbutton:
+ type: object
+ properties:
+ compatible:
+ const: ti,twl4030-pwrbutton
+ interrupts:
+ const: 8
+
+ additionalProperties: false
+ watchdog:
+ type: object
+ properties:
+ compatible:
+ const: ti,twl4030-wdt
+
+ additionalProperties: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ti,twl6030
+ then:
+ properties:
+ gpadc:
+ type: object
+ properties:
+ compatible:
+ const: ti,twl6030-gpadc
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ti,twl6032
+ then:
+ properties:
+ gpadc:
+ type: object
+ properties:
+ compatible:
+ const: ti,twl6032-gpadc
+
properties:
compatible:
description:
@@ -42,6 +101,15 @@ properties:
"#clock-cells":
const: 1
+ rtc:
+ type: object
+ properties:
+ compatible:
+ const: ti,twl4030-rtc
+ interrupts:
+ maxItems: 1
+ additionalProperties: false
+
additionalProperties: false
required:
diff --git a/Documentation/devicetree/bindings/rtc/twl-rtc.txt b/Documentation/devicetree/bindings/rtc/twl-rtc.txt
deleted file mode 100644
index 8f9a94f2f8969..0000000000000
--- a/Documentation/devicetree/bindings/rtc/twl-rtc.txt
+++ /dev/null
@@ -1,11 +0,0 @@
-* Texas Instruments TWL4030/6030 RTC
-
-Required properties:
-- compatible : Should be "ti,twl4030-rtc"
-- interrupts : Should be the interrupt number.
-
-Example:
- rtc {
- compatible = "ti,twl4030-rtc";
- interrupts = <11>;
- };
diff --git a/Documentation/devicetree/bindings/watchdog/twl4030-wdt.txt b/Documentation/devicetree/bindings/watchdog/twl4030-wdt.txt
deleted file mode 100644
index 80a37193c0b86..0000000000000
--- a/Documentation/devicetree/bindings/watchdog/twl4030-wdt.txt
+++ /dev/null
@@ -1,10 +0,0 @@
-Device tree bindings for twl4030-wdt driver (TWL4030 watchdog)
-
-Required properties:
- compatible = "ti,twl4030-wdt";
-
-Example:
-
-watchdog {
- compatible = "ti,twl4030-wdt";
-};
--
2.39.2
^ permalink raw reply related
* Re: Future handling of complex RGB devices on Linux v3
From: Hans de Goede @ 2024-03-18 11:11 UTC (permalink / raw)
To: Werner Sembach
Cc: Lee Jones, jikos, linux-kernel, Jelle van der Waa, Miguel Ojeda,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds,
Pavel Machek, Gregor Riepl
In-Reply-To: <adbfdf6c-fb59-4fae-a472-17b04dd8a3f6@tuxedocomputers.com>
Hi Werner,
Sorry for the late reply.
On 2/22/24 2:14 PM, Werner Sembach wrote:
> Hi,
>
> Thanks everyone for the exhaustive feedback. And at least this thread is a good comprehesive reference for the future ^^.
>
> To recap the hopefully final UAPI for complex RGB lighting devices:
>
> - By default there is a singular /sys/class/leds/* entry that treats the device as if it was a single zone RGB keyboard backlight with no special effects.
Ack this sounds good.
>
> - There is an accompanying misc device with the sysfs attributes "name", "device_type", "firmware_version_string", "serial_number" for device identification and "use_leds_uapi" that defaults to 1.
You don't need a char misc device here, you can just make this sysfs attributes on the LED class device's parent device by using device_driver.dev_groups. Please don't use a char misc device just to attach sysfs attributes to it.
Also I'm a bit unsure about most of these attributes, "use_leds_uapi" was discussed before
and makes sense. But at least for HID devices the rest of this info is already available
in sysfs attributes on the HID devices (things like vendor and product id) and since the
userspace code needs per device code to drive the kbd anyways it can also have device
specific code to retrieve all this info, so the other sysfs attributes just feel like
duplicating information. Also there already are a ton of existing hidraw userspace rgbkbd
drivers which already get this info from other places.
> - If set to 0 the /sys/class/leds/* entry disappears. The driver should keep the last state the backlight was in active if possible.
>
> - If set 1 it appears again. The driver should bring it back to a static 1 zone setting while avoiding flicker if possible.
Ack, if this finds it way into some documentation (which it should) please make it
clear that this is about the "use_leds_uapi" sysfs attribute.
> - If the device is not controllable by for example hidraw, the misc device might also implement additional ioctls or sysfs attributes to allow a more complex low level control for the keyboard backlight. This is will be a highly vendor specific UAPI.
IMHO this is the only case where actually using a misc device makes sense, so that
you have a chardev to do the ioctls on. misc-device-s should really only be used
when you need a chardev under /dev . Since you don't need the chardev for the e.g.
hidraw case you should not use a miscdev there IMHO.
>
> - The actual logic interacting with this low level UAPI is implemented by a userspace driver
>
> Implementation wise: For the creation of the misc device with the use_leds_uapi switch a helper function/macro might be useful? Wonder if it should go into leds.h, led-class-multicolor.h, or a new header file?
See above, I don't think we want the misc device for the hidraw case, at which
point I think the helper becomes unnecessary since just a single sysfs write
callback is necessary.
Also for adding new sysfs attributes it is strongly encouraged to use
device_driver.dev_groups so that the device core handled registering /
unregistering the sysfs attributes which fixes a bunch of races; and
using device_driver.dev_groups does not mix well with a helper as you've
suggested.
>
> - Out of my head it would look something like this:
>
> led_classdev_add_optional_misc_control(
> struct led_classdev *led_cdev,
> char* name,
> char* device_type,
> char* firmware_version_string,
> char* serial_number,
> void (*deregister_led)(struct led_classdev *led_cdev),
> void (*reregister_led)(struct led_classdev *led_cdev))
>
> Let me know your thoughts and hopefully I can start implementing it soon for one of our devices.
I think overall the plan sounds good, with my main suggested change
being to not use an unnecessary misc device for the hid-raw case.
Regards,
Hans
^ permalink raw reply
* [PATCH v3 RESEND] HID: nintendo: use ida for LED player id
From: Martino Fontana @ 2024-03-18 10:36 UTC (permalink / raw)
To: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel
Cc: Martino Fontana, Ryan McClelland
Previously, the leds pattern would just increment with every controller
connected. This wouldn't take into consideration when controllers are
disconnected. The same controller could be connected and disconnected
with the pattern increasing player count each time.
This patch changes it by using an ID allocator in order to assign the
player id, the same way hid-playstation does.
Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
Signed-off-by: Ryan McClelland <rymcclel@gmail.com>
---
Changes for v2:
ida_free now frees the correct id, instead of an id that got moduloed.
Changes for v3:
Destroy allocator when removing driver.
drivers/hid/hid-nintendo.c | 41 ++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 997c3a1adaca..8a8ae7881469 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -28,6 +28,7 @@
#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/hid.h>
+#include <linux/idr.h>
#include <linux/input.h>
#include <linux/jiffies.h>
#include <linux/leds.h>
@@ -427,6 +428,7 @@ static const enum led_brightness joycon_player_led_patterns[JC_NUM_LED_PATTERNS]
struct joycon_ctlr {
struct hid_device *hdev;
struct input_dev *input;
+ u32 player_id;
struct led_classdev leds[JC_NUM_LEDS]; /* player leds */
struct led_classdev home_led;
enum joycon_ctlr_state ctlr_state;
@@ -1930,7 +1932,8 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
return ret;
}
-static DEFINE_SPINLOCK(joycon_input_num_spinlock);
+static DEFINE_IDA(nintendo_player_id_allocator);
+
static int joycon_leds_create(struct joycon_ctlr *ctlr)
{
struct hid_device *hdev = ctlr->hdev;
@@ -1941,20 +1944,19 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
char *name;
int ret;
int i;
- unsigned long flags;
int player_led_pattern;
- static int input_num;
-
- /*
- * Set the player leds based on controller number
- * Because there is no standard concept of "player number", the pattern
- * number will simply increase by 1 every time a controller is connected.
- */
- spin_lock_irqsave(&joycon_input_num_spinlock, flags);
- player_led_pattern = input_num++ % JC_NUM_LED_PATTERNS;
- spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
/* configure the player LEDs */
+ ctlr->player_id = U32_MAX;
+ ret = ida_alloc(&nintendo_player_id_allocator, GFP_KERNEL);
+ if (ret < 0) {
+ hid_warn(hdev, "Failed to allocate player ID, skipping; ret=%d\n", ret);
+ goto home_led;
+ }
+ ctlr->player_id = ret;
+ player_led_pattern = ret % JC_NUM_LED_PATTERNS;
+ hid_info(ctlr->hdev, "assigned player %d led pattern", player_led_pattern + 1);
+
for (i = 0; i < JC_NUM_LEDS; i++) {
name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
d_name,
@@ -2423,6 +2425,7 @@ static void nintendo_hid_remove(struct hid_device *hdev)
spin_unlock_irqrestore(&ctlr->lock, flags);
destroy_workqueue(ctlr->rumble_queue);
+ ida_free(&nintendo_player_id_allocator, ctlr->player_id);
hid_hw_close(hdev);
hid_hw_stop(hdev);
@@ -2468,7 +2471,19 @@ static struct hid_driver nintendo_hid_driver = {
.resume = nintendo_hid_resume,
#endif
};
-module_hid_driver(nintendo_hid_driver);
+static int __init nintendo_init(void)
+{
+ return hid_register_driver(&nintendo_hid_driver);
+}
+
+static void __exit nintendo_exit(void)
+{
+ hid_unregister_driver(&nintendo_hid_driver);
+ ida_destroy(&nintendo_player_id_allocator);
+}
+
+module_init(nintendo_init);
+module_exit(nintendo_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
--
2.43.2
^ permalink raw reply related
* [PATCH] HID: i2c-hid: remove I2C_HID_READ_PENDING flag to prevent lock-up
From: Nam Cao @ 2024-03-18 10:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Douglas Anderson, Hans de Goede,
Maxime Ripard, Kai-Heng Feng, Nam Cao, Johan Hovold, linux-input,
linux-kernel
Cc: Eva Kurchatova, stable
The flag I2C_HID_READ_PENDING is used to serialize I2C operations.
However, this is not necessary, because I2C core already has its own
locking for that.
More importantly, this flag can cause a lock-up: if the flag is set in
i2c_hid_xfer() and an interrupt happens, the interrupt handler
(i2c_hid_irq) will check this flag and return immediately without doing
anything, then the interrupt handler will be invoked again in an
infinite loop.
Since interrupt handler is an RT task, it takes over the CPU and the
flag-clearing task never gets scheduled, thus we have a lock-up.
Delete this unnecessary flag.
Reported-and-tested-by: Eva Kurchatova <nyandarknessgirl@gmail.com>
Closes: https://lore.kernel.org/r/CA+eeCSPUDpUg76ZO8dszSbAGn+UHjcyv8F1J-CUPVARAzEtW9w@mail.gmail.com
Fixes: 4a200c3b9a40 ("HID: i2c-hid: introduce HID over i2c specification implementation")
Cc: <stable@vger.kernel.org>
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
drivers/hid/i2c-hid/i2c-hid-core.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 2df1ab3c31cc..1c86c97688e9 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -64,7 +64,6 @@
/* flags */
#define I2C_HID_STARTED 0
#define I2C_HID_RESET_PENDING 1
-#define I2C_HID_READ_PENDING 2
#define I2C_HID_PWR_ON 0x00
#define I2C_HID_PWR_SLEEP 0x01
@@ -190,15 +189,10 @@ static int i2c_hid_xfer(struct i2c_hid *ihid,
msgs[n].len = recv_len;
msgs[n].buf = recv_buf;
n++;
-
- set_bit(I2C_HID_READ_PENDING, &ihid->flags);
}
ret = i2c_transfer(client->adapter, msgs, n);
- if (recv_len)
- clear_bit(I2C_HID_READ_PENDING, &ihid->flags);
-
if (ret != n)
return ret < 0 ? ret : -EIO;
@@ -556,9 +550,6 @@ static irqreturn_t i2c_hid_irq(int irq, void *dev_id)
{
struct i2c_hid *ihid = dev_id;
- if (test_bit(I2C_HID_READ_PENDING, &ihid->flags))
- return IRQ_HANDLED;
-
i2c_hid_get_input(ihid);
return IRQ_HANDLED;
--
2.39.2
^ permalink raw reply related
* Re: Boot hang with SiFive PLIC when routing I2C-HID level-triggered interrupts
From: Eva Kurchatova @ 2024-03-18 10:24 UTC (permalink / raw)
To: Nam Cao
Cc: linux-riscv, bugs, linux-i2c, jikos, benjamin.tissoires, dianders,
mripard, johan+linaro, linux-input, linux-kernel
In-Reply-To: <20240318100108.4fafd72b@namcao>
On Mon, Mar 18, 2024 at 11:01 AM Nam Cao <namcao@linutronix.de> wrote:
> Nice! I assume I can add
> Reported-and-tested-by: Eva Kurchatova <nyandarknessgirl@gmail.com>
> to the patch?
>
Yes.
You can also add link to upstream RVVM repo if anyone is interested in
reproduction. This RVVM patch applied to 0.6 makes a keystroke storm:
(window_update() is called on each display redraw and has access to hid_kb)
diff --git a/src/devices/fb_window.c b/src/devices/fb_window.c
index f170e2d..17e2519 100644
--- a/src/devices/fb_window.c
+++ b/src/devices/fb_window.c
@@ -77,6 +77,11 @@ static const uint8_t rvvm_logo_pix[] = {
static void window_update(rvvm_mmio_dev_t* device)
{
+ fb_window_t* win = device->data;
+ for (size_t i=0; i<100000; ++i) {
+ hid_keyboard_press(win->keyboard, HID_KEY_LEFTCTRL);
+ hid_keyboard_release(win->keyboard, HID_KEY_LEFTCTRL);
+ }
fb_window_update((fb_window_t*)device->data);
}
> I am still confused why RT throttling doesn't unstuck the kernel in this
> case. I will consult some people and investigate more on this. But I think
> this patch is good on its own, so I will send a proper patch shortly.
>
> Best regards,
> Nam
RT throttling kicked in *very* rarely, in most cases where the unpatched
kernel was actually stuck RT throttling wasn't reported at all.
It didn't hang every time either, so it's possible that RT throttling
helped sometimes, but not enough to always recover from such a loop:
[incoming IRQ]->[IRQ claimed]->[no handling]->[IRQ completion (which
immediately triggers phase 1 again)]
Best regards,
Eva
^ permalink raw reply related
* Re: Boot hang with SiFive PLIC when routing I2C-HID level-triggered interrupts
From: Nam Cao @ 2024-03-18 9:01 UTC (permalink / raw)
To: Eva Kurchatova
Cc: linux-riscv, bugs, linux-i2c, jikos, benjamin.tissoires, dianders,
mripard, johan+linaro, linux-input, linux-kernel
In-Reply-To: <CA+eeCSMiqcvzrQcgRr7AZWJQTv-c9-uSX5jbPZPzmsDjy08z=A@mail.gmail.com>
On 18/Mar/2024 Eva Kurchatova wrote:
> On Sun, Mar 17, 2024 at 11:27 PM Nam Cao <namcao@linutronix.de> wrote:
> > It seems I2C HID's interrupt handler (i2c_hid_irq) returns immediately if
> > I2C_HID_READ_PENDING is set. This flag is supposed to be cleared in
> > i2c_hid_xfer(), but since the (threaded) interrupt handler runs at higher
> > priority, the flag is never cleared. So we have a lock-up: interrupt
> > handler won't do anything unless the flag is cleared, but the clearing of
> > this flag is done in a lower priority task which never gets scheduled while
> > the interrupt handler is active.
> >
> > There is RT throttling to prevent RT tasks from locking up the system like
> > this. I don't know much about scheduling stuffs, so I am not really sure
> > why RT throttling does not work. I think because RT throttling triggers
> > when RT tasks take too much CPU time, but in this case hard interrupt
> > handlers take lots of CPU time too (~50% according to my measurement), so
> > RT throttling doesn't trigger often enough (in this case, it triggers once
> > and never again). Again, I don't know much about scheduler so I may be
> > talking nonsense here.
> >
> > The flag I2C_HID_READ_PENDING seems to be used to make sure that only 1
> > I2C operation can happen at a time. But this seems pointless, because I2C
> > subsystem already takes care of this. So I think we can just remove it.
> >
> > Can you give the below patch a try?
> >
> > diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> > index 2735cd585af0..799ad0ef9c4a 100644
> > --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> > +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> > @@ -64,7 +64,6 @@
> > /* flags */
> > #define I2C_HID_STARTED 0
> > #define I2C_HID_RESET_PENDING 1
> > -#define I2C_HID_READ_PENDING 2
> >
> > #define I2C_HID_PWR_ON 0x00
> > #define I2C_HID_PWR_SLEEP 0x01
> > @@ -190,15 +189,10 @@ static int i2c_hid_xfer(struct i2c_hid *ihid,
> > msgs[n].len = recv_len;
> > msgs[n].buf = recv_buf;
> > n++;
> > -
> > - set_bit(I2C_HID_READ_PENDING, &ihid->flags);
> > }
> >
> > ret = i2c_transfer(client->adapter, msgs, n);
> >
> > - if (recv_len)
> > - clear_bit(I2C_HID_READ_PENDING, &ihid->flags);
> > -
> > if (ret != n)
> > return ret < 0 ? ret : -EIO;
> >
> > @@ -566,9 +560,6 @@ static irqreturn_t i2c_hid_irq(int irq, void *dev_id)
> > {
> > struct i2c_hid *ihid = dev_id;
> >
> > - if (test_bit(I2C_HID_READ_PENDING, &ihid->flags))
> > - return IRQ_HANDLED;
> > -
> > i2c_hid_get_input(ihid);
> >
> > return IRQ_HANDLED;
>
> Patch applied cleanly on top of 6.7.9, builds OK (No warns, etc).
>
> This indeed fixes the hang completely.
Nice! I assume I can add
Reported-and-tested-by: Eva Kurchatova <nyandarknessgirl@gmail.com>
to the patch?
> I modified RVVM to send millions of keystroke events per second,
> and put `reboot` as a service hook in the guest. It has been continuously
> rebooting without a hitch for the last 30 minutes or so (Full boot takes
> around 2 seconds), whereas unpatched 6.7.9 hangs almost immediately
> in such conditions (Reverted your patch & rebuilt to be sure).
>
> Thank you very much for this! Hope to see it upstreamed soon
Thank you for the report, your investigation helped a lot.
I am still confused why RT throttling doesn't unstuck the kernel in this
case. I will consult some people and investigate more on this. But I think
this patch is good on its own, so I will send a proper patch shortly.
Best regards,
Nam
^ permalink raw reply
* Re: Boot hang with SiFive PLIC when routing I2C-HID level-triggered interrupts
From: Eva Kurchatova @ 2024-03-18 8:48 UTC (permalink / raw)
To: Nam Cao
Cc: linux-riscv, bugs, linux-i2c, jikos, benjamin.tissoires, dianders,
mripard, johan+linaro, linux-input, linux-kernel
In-Reply-To: <20240317222718.3e03edb1@namcao>
On Sun, Mar 17, 2024 at 11:27 PM Nam Cao <namcao@linutronix.de> wrote:
>
> Cc: HID folks
>
> On 14/Mar/2024 Eva Kurchatova wrote:
> > If an I2C-HID controller level-triggered IRQ line is routed directly as
> > a PLIC IRQ, and we spam input early enough in kernel boot process
> > (Somewhere between initializing NET, ALSA subsystems and before
> > i2c-hid driver init), then there is a chance of kernel locking up
> > completely and not going any further.
> >
> > There are no kernel messages printed with all the IRQ, task hang
> > debugging enabled - other than (sometimes) it reports sched RT
> > throttling after a few seconds. Basic timer interrupt handling is
> > intact - fbdev tty cursor is still blinking.
> >
> > It appears that in such a case the I2C-HID IRQ line is raised; PLIC
> > notifies the (single) boot system hart, kernel claims the IRQ and
> > immediately completes it by writing to CLAIM/COMPLETE register.
> > No access to the I2C controller (OpenCores) or I2C-HID registers
> > is made, so the HID report is never consumed and IRQ line stays
> > raised forever. The kernel endlessly claims & completes IRQs
> > without doing any work with the device. It doesn't always end up this
> > way; sometimes boot process completes and there are no signs of
> > interrupt storm or stuck IRQ processing afterwards.
>
> It seems I2C HID's interrupt handler (i2c_hid_irq) returns immediately if
> I2C_HID_READ_PENDING is set. This flag is supposed to be cleared in
> i2c_hid_xfer(), but since the (threaded) interrupt handler runs at higher
> priority, the flag is never cleared. So we have a lock-up: interrupt
> handler won't do anything unless the flag is cleared, but the clearing of
> this flag is done in a lower priority task which never gets scheduled while
> the interrupt handler is active.
>
> There is RT throttling to prevent RT tasks from locking up the system like
> this. I don't know much about scheduling stuffs, so I am not really sure
> why RT throttling does not work. I think because RT throttling triggers
> when RT tasks take too much CPU time, but in this case hard interrupt
> handlers take lots of CPU time too (~50% according to my measurement), so
> RT throttling doesn't trigger often enough (in this case, it triggers once
> and never again). Again, I don't know much about scheduler so I may be
> talking nonsense here.
>
> The flag I2C_HID_READ_PENDING seems to be used to make sure that only 1
> I2C operation can happen at a time. But this seems pointless, because I2C
> subsystem already takes care of this. So I think we can just remove it.
>
> Can you give the below patch a try?
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 2735cd585af0..799ad0ef9c4a 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -64,7 +64,6 @@
> /* flags */
> #define I2C_HID_STARTED 0
> #define I2C_HID_RESET_PENDING 1
> -#define I2C_HID_READ_PENDING 2
>
> #define I2C_HID_PWR_ON 0x00
> #define I2C_HID_PWR_SLEEP 0x01
> @@ -190,15 +189,10 @@ static int i2c_hid_xfer(struct i2c_hid *ihid,
> msgs[n].len = recv_len;
> msgs[n].buf = recv_buf;
> n++;
> -
> - set_bit(I2C_HID_READ_PENDING, &ihid->flags);
> }
>
> ret = i2c_transfer(client->adapter, msgs, n);
>
> - if (recv_len)
> - clear_bit(I2C_HID_READ_PENDING, &ihid->flags);
> -
> if (ret != n)
> return ret < 0 ? ret : -EIO;
>
> @@ -566,9 +560,6 @@ static irqreturn_t i2c_hid_irq(int irq, void *dev_id)
> {
> struct i2c_hid *ihid = dev_id;
>
> - if (test_bit(I2C_HID_READ_PENDING, &ihid->flags))
> - return IRQ_HANDLED;
> -
> i2c_hid_get_input(ihid);
>
> return IRQ_HANDLED;
Patch applied cleanly on top of 6.7.9, builds OK (No warns, etc).
This indeed fixes the hang completely.
I modified RVVM to send millions of keystroke events per second,
and put `reboot` as a service hook in the guest. It has been continuously
rebooting without a hitch for the last 30 minutes or so (Full boot takes
around 2 seconds), whereas unpatched 6.7.9 hangs almost immediately
in such conditions (Reverted your patch & rebuilt to be sure).
Thank you very much for this! Hope to see it upstreamed soon
^ permalink raw reply
* Re: [PATCH v9 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: Kamel Bouhara @ 2024-03-18 8:05 UTC (permalink / raw)
To: Marco Felsch
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
Jeff LaBundy, catalin.popescu, mark.satterthwaite,
Thomas Petazzoni, Gregory Clement, bsp-development.geo
In-Reply-To: <20240313202135.2lwgtu2z67ksh2tz@pengutronix.de>
Le Wed, Mar 13, 2024 at 09:21:35PM +0100, Marco Felsch a écrit :
> Hi Kamel,
>
Hello Marco,
[...]
> > +static int axiom_i2c_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct input_dev *input_dev;
> > + struct axiom_data *ts;
> > + u32 poll_interval;
> > + int target;
> > + int error;
> > +
> > + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> > + if (!ts)
> > + return -ENOMEM;
> > +
> > + i2c_set_clientdata(client, ts);
> > + ts->client = client;
> > + ts->dev = dev;
> > +
> > + ts->regmap = devm_regmap_init_i2c(client, &axiom_i2c_regmap_config);
> > + error = PTR_ERR_OR_ZERO(ts->regmap);
> > + if (error) {
> > + dev_err(dev, "Failed to initialize regmap: %d\n", error);
> > + return error;
> > + }
> > +
> > + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> > + if (IS_ERR(ts->reset_gpio))
> > + return dev_err_probe(dev, PTR_ERR(ts->reset_gpio), "failed to get reset GPIO\n");
> > +
> > + if (ts->reset_gpio)
> > + axiom_reset(ts->reset_gpio);
>
> This seems useless, since you doing an reset without enabling the power
> supply (below). I know there are systems which do have the supply always
> connected or for ACPI the supply is managed via firmware, but the driver
> should implement the correct logic and for DT/OF case this is not
> correct.
>
> > +
> > + ts->vddi = devm_regulator_get_optional(dev, "vddi");
> > + if (!IS_ERR(ts->vddi)) {
> > + error = devm_regulator_get_enable(dev, "vddi");
>
> Regulators are ref counted and now you request the regulator twice. Also
> the regulator is not optional, it is required for the device to work.
> Same applies to the vdda below.
>
While it is true most of the time, it occurs that for x86 based boards,
adding a regulator entirely is not always possible.
In our particular case, the I2C controller for this touchscreen is
behind a CPLD (aka embedded controller) so I have no direct access to
the I2C controller and it isn't described in the ACPI table.
In a normal case, I would use ACPI override to pass regulator
properties, but here it's not possible.
Having a CPLD exposing this kind of controller is quite common on x86
based boards. So, we need a way to support the case when a regulator
can't be described. The optional regulator looked like a good option,
but if you have a better alternative, I am open to considering it.
--
Kamel Bouhara, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH v3 2/4] HID: hid-sensor-custom: Convert sprintf/snprintf to sysfs_emit
From: Li Zhijian @ 2024-03-18 1:28 UTC (permalink / raw)
To: linux-kernel
Cc: Li Zhijian, Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
Benjamin Tissoires, linux-input, linux-iio, Jonathan Cameron
In-Reply-To: <20240318012819.1405003-1-lizhijian@fujitsu.com>
Per filesystems/sysfs.rst, show() should only use sysfs_emit()
or sysfs_emit_at() when formatting the value to be returned to user space.
coccinelle complains that there are still a couple of functions that use
snprintf(). Convert them to sysfs_emit().
sprintf() will be converted as weel if they have.
Generally, this patch is generated by
make coccicheck M=<path/to/file> MODE=patch \
COCCI=scripts/coccinelle/api/device_attr_show.cocci
No functional change intended
CC: Jiri Kosina <jikos@kernel.org>
CC: Jonathan Cameron <jic23@kernel.org>
CC: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
CC: Benjamin Tissoires <benjamin.tissoires@redhat.com>
CC: linux-input@vger.kernel.org
CC: linux-iio@vger.kernel.org
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
V3:
rewrap the line as will be under 80 chars and add Reviewed-by # Jonathan
This is a part of the work "Fix coccicheck device_attr_show warnings"[1]
Split them per subsystem so that the maintainer can review it easily
[1] https://lore.kernel.org/lkml/20240116041129.3937800-1-lizhijian@fujitsu.com/
---
drivers/hid/hid-sensor-custom.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
index d85398721659..ac214777d7d9 100644
--- a/drivers/hid/hid-sensor-custom.c
+++ b/drivers/hid/hid-sensor-custom.c
@@ -155,7 +155,7 @@ static ssize_t enable_sensor_show(struct device *dev,
{
struct hid_sensor_custom *sensor_inst = dev_get_drvdata(dev);
- return sprintf(buf, "%d\n", sensor_inst->enable);
+ return sysfs_emit(buf, "%d\n", sensor_inst->enable);
}
static int set_power_report_state(struct hid_sensor_custom *sensor_inst,
@@ -372,14 +372,13 @@ static ssize_t show_value(struct device *dev, struct device_attribute *attr,
sizeof(struct hid_custom_usage_desc),
usage_id_cmp);
if (usage_desc)
- return snprintf(buf, PAGE_SIZE, "%s\n",
- usage_desc->desc);
+ return sysfs_emit(buf, "%s\n", usage_desc->desc);
else
- return sprintf(buf, "not-specified\n");
+ return sysfs_emit(buf, "not-specified\n");
} else
return -EINVAL;
- return sprintf(buf, "%d\n", value);
+ return sysfs_emit(buf, "%d\n", value);
}
static ssize_t store_value(struct device *dev, struct device_attribute *attr,
--
2.29.2
^ permalink raw reply related
* [PATCH v3 3/4] HID: roccat: Convert sprintf/snprintf to sysfs_emit
From: Li Zhijian @ 2024-03-18 1:28 UTC (permalink / raw)
To: linux-kernel
Cc: Li Zhijian, Stefan Achatz, Jiri Kosina, Benjamin Tissoires,
linux-input
In-Reply-To: <20240318012819.1405003-1-lizhijian@fujitsu.com>
Per filesystems/sysfs.rst, show() should only use sysfs_emit()
or sysfs_emit_at() when formatting the value to be returned to user space.
coccinelle complains that there are still a couple of functions that use
snprintf(). Convert them to sysfs_emit().
sprintf() will be converted as weel if they have.
Generally, this patch is generated by
make coccicheck M=<path/to/file> MODE=patch \
COCCI=scripts/coccinelle/api/device_attr_show.cocci
No functional change intended
CC: Stefan Achatz <erazor_de@users.sourceforge.net>
CC: Jiri Kosina <jikos@kernel.org>
CC: Benjamin Tissoires <benjamin.tissoires@redhat.com>
CC: linux-input@vger.kernel.org
Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
This is a part of the work "Fix coccicheck device_attr_show warnings"[1]
Split them per subsystem so that the maintainer can review it easily
[1] https://lore.kernel.org/lkml/20240116041129.3937800-1-lizhijian@fujitsu.com/
---
drivers/hid/hid-roccat-isku.c | 2 +-
drivers/hid/hid-roccat-kone.c | 12 ++++++------
drivers/hid/hid-roccat-koneplus.c | 4 ++--
drivers/hid/hid-roccat-kovaplus.c | 10 +++++-----
drivers/hid/hid-roccat-pyra.c | 6 +++---
5 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/hid-roccat-isku.c b/drivers/hid/hid-roccat-isku.c
index 458060403397..0cd6208fb371 100644
--- a/drivers/hid/hid-roccat-isku.c
+++ b/drivers/hid/hid-roccat-isku.c
@@ -61,7 +61,7 @@ static ssize_t isku_sysfs_show_actual_profile(struct device *dev,
{
struct isku_device *isku =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", isku->actual_profile);
+ return sysfs_emit(buf, "%d\n", isku->actual_profile);
}
static ssize_t isku_sysfs_set_actual_profile(struct device *dev,
diff --git a/drivers/hid/hid-roccat-kone.c b/drivers/hid/hid-roccat-kone.c
index 00a1abc7e839..3f8f459edcf3 100644
--- a/drivers/hid/hid-roccat-kone.c
+++ b/drivers/hid/hid-roccat-kone.c
@@ -400,7 +400,7 @@ static ssize_t kone_sysfs_show_actual_profile(struct device *dev,
{
struct kone_device *kone =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", kone->actual_profile);
+ return sysfs_emit(buf, "%d\n", kone->actual_profile);
}
static DEVICE_ATTR(actual_profile, 0440, kone_sysfs_show_actual_profile, NULL);
@@ -409,7 +409,7 @@ static ssize_t kone_sysfs_show_actual_dpi(struct device *dev,
{
struct kone_device *kone =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", kone->actual_dpi);
+ return sysfs_emit(buf, "%d\n", kone->actual_dpi);
}
static DEVICE_ATTR(actual_dpi, 0440, kone_sysfs_show_actual_dpi, NULL);
@@ -432,7 +432,7 @@ static ssize_t kone_sysfs_show_weight(struct device *dev,
if (retval)
return retval;
- return snprintf(buf, PAGE_SIZE, "%d\n", weight);
+ return sysfs_emit(buf, "%d\n", weight);
}
static DEVICE_ATTR(weight, 0440, kone_sysfs_show_weight, NULL);
@@ -441,7 +441,7 @@ static ssize_t kone_sysfs_show_firmware_version(struct device *dev,
{
struct kone_device *kone =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", kone->firmware_version);
+ return sysfs_emit(buf, "%d\n", kone->firmware_version);
}
static DEVICE_ATTR(firmware_version, 0440, kone_sysfs_show_firmware_version,
NULL);
@@ -451,7 +451,7 @@ static ssize_t kone_sysfs_show_tcu(struct device *dev,
{
struct kone_device *kone =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", kone->settings.tcu);
+ return sysfs_emit(buf, "%d\n", kone->settings.tcu);
}
static int kone_tcu_command(struct usb_device *usb_dev, int number)
@@ -553,7 +553,7 @@ static ssize_t kone_sysfs_show_startup_profile(struct device *dev,
{
struct kone_device *kone =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", kone->settings.startup_profile);
+ return sysfs_emit(buf, "%d\n", kone->settings.startup_profile);
}
static ssize_t kone_sysfs_set_startup_profile(struct device *dev,
diff --git a/drivers/hid/hid-roccat-koneplus.c b/drivers/hid/hid-roccat-koneplus.c
index 22b895436a7c..8ccb3b14a1a9 100644
--- a/drivers/hid/hid-roccat-koneplus.c
+++ b/drivers/hid/hid-roccat-koneplus.c
@@ -242,7 +242,7 @@ static ssize_t koneplus_sysfs_show_actual_profile(struct device *dev,
{
struct koneplus_device *koneplus =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", koneplus->actual_profile);
+ return sysfs_emit(buf, "%d\n", koneplus->actual_profile);
}
static ssize_t koneplus_sysfs_set_actual_profile(struct device *dev,
@@ -309,7 +309,7 @@ static ssize_t koneplus_sysfs_show_firmware_version(struct device *dev,
&info, KONEPLUS_SIZE_INFO);
mutex_unlock(&koneplus->koneplus_lock);
- return snprintf(buf, PAGE_SIZE, "%d\n", info.firmware_version);
+ return sysfs_emit(buf, "%d\n", info.firmware_version);
}
static DEVICE_ATTR(firmware_version, 0440,
koneplus_sysfs_show_firmware_version, NULL);
diff --git a/drivers/hid/hid-roccat-kovaplus.c b/drivers/hid/hid-roccat-kovaplus.c
index 86af538c10d6..748d4d7cb2fc 100644
--- a/drivers/hid/hid-roccat-kovaplus.c
+++ b/drivers/hid/hid-roccat-kovaplus.c
@@ -272,7 +272,7 @@ static ssize_t kovaplus_sysfs_show_actual_profile(struct device *dev,
{
struct kovaplus_device *kovaplus =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", kovaplus->actual_profile);
+ return sysfs_emit(buf, "%d\n", kovaplus->actual_profile);
}
static ssize_t kovaplus_sysfs_set_actual_profile(struct device *dev,
@@ -325,7 +325,7 @@ static ssize_t kovaplus_sysfs_show_actual_cpi(struct device *dev,
{
struct kovaplus_device *kovaplus =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", kovaplus->actual_cpi);
+ return sysfs_emit(buf, "%d\n", kovaplus->actual_cpi);
}
static DEVICE_ATTR(actual_cpi, 0440, kovaplus_sysfs_show_actual_cpi, NULL);
@@ -334,7 +334,7 @@ static ssize_t kovaplus_sysfs_show_actual_sensitivity_x(struct device *dev,
{
struct kovaplus_device *kovaplus =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", kovaplus->actual_x_sensitivity);
+ return sysfs_emit(buf, "%d\n", kovaplus->actual_x_sensitivity);
}
static DEVICE_ATTR(actual_sensitivity_x, 0440,
kovaplus_sysfs_show_actual_sensitivity_x, NULL);
@@ -344,7 +344,7 @@ static ssize_t kovaplus_sysfs_show_actual_sensitivity_y(struct device *dev,
{
struct kovaplus_device *kovaplus =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", kovaplus->actual_y_sensitivity);
+ return sysfs_emit(buf, "%d\n", kovaplus->actual_y_sensitivity);
}
static DEVICE_ATTR(actual_sensitivity_y, 0440,
kovaplus_sysfs_show_actual_sensitivity_y, NULL);
@@ -365,7 +365,7 @@ static ssize_t kovaplus_sysfs_show_firmware_version(struct device *dev,
&info, KOVAPLUS_SIZE_INFO);
mutex_unlock(&kovaplus->kovaplus_lock);
- return snprintf(buf, PAGE_SIZE, "%d\n", info.firmware_version);
+ return sysfs_emit(buf, "%d\n", info.firmware_version);
}
static DEVICE_ATTR(firmware_version, 0440,
kovaplus_sysfs_show_firmware_version, NULL);
diff --git a/drivers/hid/hid-roccat-pyra.c b/drivers/hid/hid-roccat-pyra.c
index 5663b9cd9c69..eeb3d38cd805 100644
--- a/drivers/hid/hid-roccat-pyra.c
+++ b/drivers/hid/hid-roccat-pyra.c
@@ -283,7 +283,7 @@ static ssize_t pyra_sysfs_show_actual_cpi(struct device *dev,
{
struct pyra_device *pyra =
hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
- return snprintf(buf, PAGE_SIZE, "%d\n", pyra->actual_cpi);
+ return sysfs_emit(buf, "%d\n", pyra->actual_cpi);
}
static DEVICE_ATTR(actual_cpi, 0440, pyra_sysfs_show_actual_cpi, NULL);
@@ -300,7 +300,7 @@ static ssize_t pyra_sysfs_show_actual_profile(struct device *dev,
&settings, PYRA_SIZE_SETTINGS);
mutex_unlock(&pyra->pyra_lock);
- return snprintf(buf, PAGE_SIZE, "%d\n", settings.startup_profile);
+ return sysfs_emit(buf, "%d\n", settings.startup_profile);
}
static DEVICE_ATTR(actual_profile, 0440, pyra_sysfs_show_actual_profile, NULL);
static DEVICE_ATTR(startup_profile, 0440, pyra_sysfs_show_actual_profile, NULL);
@@ -321,7 +321,7 @@ static ssize_t pyra_sysfs_show_firmware_version(struct device *dev,
&info, PYRA_SIZE_INFO);
mutex_unlock(&pyra->pyra_lock);
- return snprintf(buf, PAGE_SIZE, "%d\n", info.firmware_version);
+ return sysfs_emit(buf, "%d\n", info.firmware_version);
}
static DEVICE_ATTR(firmware_version, 0440, pyra_sysfs_show_firmware_version,
NULL);
--
2.29.2
^ permalink raw reply related
* [PATCH v3 4/4] HID: corsair,lenovo: Convert sprintf/snprintf to sysfs_emit
From: Li Zhijian @ 2024-03-18 1:28 UTC (permalink / raw)
To: linux-kernel; +Cc: Li Zhijian, Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20240318012819.1405003-1-lizhijian@fujitsu.com>
Per filesystems/sysfs.rst, show() should only use sysfs_emit()
or sysfs_emit_at() when formatting the value to be returned to user space.
coccinelle complains that there are still a couple of functions that use
snprintf(). Convert them to sysfs_emit().
sprintf() will be converted as weel if they have.
Generally, this patch is generated by
make coccicheck M=<path/to/file> MODE=patch \
COCCI=scripts/coccinelle/api/device_attr_show.cocci
No functional change intended
CC: Jiri Kosina <jikos@kernel.org>
CC: Benjamin Tissoires <benjamin.tissoires@redhat.com>
CC: linux-input@vger.kernel.org
Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
This is a part of the work "Fix coccicheck device_attr_show warnings"[1]
Split them per subsystem so that the maintainer can review it easily
[1] https://lore.kernel.org/lkml/20240116041129.3937800-1-lizhijian@fujitsu.com/
---
drivers/hid/hid-corsair.c | 4 ++--
drivers/hid/hid-lenovo.c | 19 ++++++++-----------
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
index 8c895c820b67..702f50e9841d 100644
--- a/drivers/hid/hid-corsair.c
+++ b/drivers/hid/hid-corsair.c
@@ -298,7 +298,7 @@ static ssize_t k90_show_macro_mode(struct device *dev,
goto out;
}
- ret = snprintf(buf, PAGE_SIZE, "%s\n", macro_mode);
+ ret = sysfs_emit(buf, "%s\n", macro_mode);
out:
kfree(data);
@@ -367,7 +367,7 @@ static ssize_t k90_show_current_profile(struct device *dev,
goto out;
}
- ret = snprintf(buf, PAGE_SIZE, "%d\n", current_profile);
+ ret = sysfs_emit(buf, "%d\n", current_profile);
out:
kfree(data);
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 149a3c74346b..26768c319361 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -555,7 +555,7 @@ static ssize_t attr_fn_lock_show(struct device *dev,
struct hid_device *hdev = to_hid_device(dev);
struct lenovo_drvdata *data = hid_get_drvdata(hdev);
- return snprintf(buf, PAGE_SIZE, "%u\n", data->fn_lock);
+ return sysfs_emit(buf, "%u\n", data->fn_lock);
}
static ssize_t attr_fn_lock_store(struct device *dev,
@@ -599,8 +599,7 @@ static ssize_t attr_sensitivity_show_cptkbd(struct device *dev,
struct hid_device *hdev = to_hid_device(dev);
struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
- return snprintf(buf, PAGE_SIZE, "%u\n",
- cptkbd_data->sensitivity);
+ return sysfs_emit(buf, "%u\n", cptkbd_data->sensitivity);
}
static ssize_t attr_sensitivity_store_cptkbd(struct device *dev,
@@ -789,7 +788,7 @@ static ssize_t attr_press_to_select_show_tpkbd(struct device *dev,
struct hid_device *hdev = to_hid_device(dev);
struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
- return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->press_to_select);
+ return sysfs_emit(buf, "%u\n", data_pointer->press_to_select);
}
static ssize_t attr_press_to_select_store_tpkbd(struct device *dev,
@@ -819,7 +818,7 @@ static ssize_t attr_dragging_show_tpkbd(struct device *dev,
struct hid_device *hdev = to_hid_device(dev);
struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
- return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->dragging);
+ return sysfs_emit(buf, "%u\n", data_pointer->dragging);
}
static ssize_t attr_dragging_store_tpkbd(struct device *dev,
@@ -849,7 +848,7 @@ static ssize_t attr_release_to_select_show_tpkbd(struct device *dev,
struct hid_device *hdev = to_hid_device(dev);
struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
- return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->release_to_select);
+ return sysfs_emit(buf, "%u\n", data_pointer->release_to_select);
}
static ssize_t attr_release_to_select_store_tpkbd(struct device *dev,
@@ -879,7 +878,7 @@ static ssize_t attr_select_right_show_tpkbd(struct device *dev,
struct hid_device *hdev = to_hid_device(dev);
struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
- return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->select_right);
+ return sysfs_emit(buf, "%u\n", data_pointer->select_right);
}
static ssize_t attr_select_right_store_tpkbd(struct device *dev,
@@ -909,8 +908,7 @@ static ssize_t attr_sensitivity_show_tpkbd(struct device *dev,
struct hid_device *hdev = to_hid_device(dev);
struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
- return snprintf(buf, PAGE_SIZE, "%u\n",
- data_pointer->sensitivity);
+ return sysfs_emit(buf, "%u\n", data_pointer->sensitivity);
}
static ssize_t attr_sensitivity_store_tpkbd(struct device *dev,
@@ -938,8 +936,7 @@ static ssize_t attr_press_speed_show_tpkbd(struct device *dev,
struct hid_device *hdev = to_hid_device(dev);
struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
- return snprintf(buf, PAGE_SIZE, "%u\n",
- data_pointer->press_speed);
+ return sysfs_emit(buf, "%u\n", data_pointer->press_speed);
}
static ssize_t attr_press_speed_store_tpkbd(struct device *dev,
--
2.29.2
^ permalink raw reply related
* [PATCH v3 1/4] HID: hid-picolcd*: Convert sprintf/scnprintf to sysfs_emit/sysfs_emit_at
From: Li Zhijian @ 2024-03-18 1:28 UTC (permalink / raw)
To: linux-kernel
Cc: Li Zhijian, Bruno Prémont, Jiri Kosina, Benjamin Tissoires,
linux-input
Per filesystems/sysfs.rst, show() should only use sysfs_emit()
or sysfs_emit_at() when formatting the value to be returned to user space.
coccinelle complains that there are still a couple of functions that use
snprintf(). Convert them to sysfs_emit().
scnprintf() will be converted as weel if they have.
Generally, this patch is generated by
make coccicheck M=<path/to/file> MODE=patch \
COCCI=scripts/coccinelle/api/device_attr_show.cocci
No functional change intended
CC: "Bruno Prémont" <bonbons@linux-vserver.org>
CC: Jiri Kosina <jikos@kernel.org>
CC: Benjamin Tissoires <benjamin.tissoires@redhat.com>
CC: linux-input@vger.kernel.org
Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
V3:
Covert more file(drivers/hid/hid-picolcd_fb.c) as suggested by Bruno
This is a part of the work "Fix coccicheck device_attr_show warnings"[1]
Split them per subsystem so that the maintainer can review it easily
[1] https://lore.kernel.org/lkml/20240116041129.3937800-1-lizhijian@fujitsu.com/
Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
drivers/hid/hid-picolcd_core.c | 6 +++---
drivers/hid/hid-picolcd_fb.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
index bbda231a7ce3..fa46fb6eab3f 100644
--- a/drivers/hid/hid-picolcd_core.c
+++ b/drivers/hid/hid-picolcd_core.c
@@ -256,9 +256,9 @@ static ssize_t picolcd_operation_mode_show(struct device *dev,
struct picolcd_data *data = dev_get_drvdata(dev);
if (data->status & PICOLCD_BOOTLOADER)
- return snprintf(buf, PAGE_SIZE, "[bootloader] lcd\n");
+ return sysfs_emit(buf, "[bootloader] lcd\n");
else
- return snprintf(buf, PAGE_SIZE, "bootloader [lcd]\n");
+ return sysfs_emit(buf, "bootloader [lcd]\n");
}
static ssize_t picolcd_operation_mode_store(struct device *dev,
@@ -301,7 +301,7 @@ static ssize_t picolcd_operation_mode_delay_show(struct device *dev,
{
struct picolcd_data *data = dev_get_drvdata(dev);
- return snprintf(buf, PAGE_SIZE, "%hu\n", data->opmode_delay);
+ return sysfs_emit(buf, "%hu\n", data->opmode_delay);
}
static ssize_t picolcd_operation_mode_delay_store(struct device *dev,
diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
index d7dddd99d325..369c78d70e66 100644
--- a/drivers/hid/hid-picolcd_fb.c
+++ b/drivers/hid/hid-picolcd_fb.c
@@ -424,9 +424,9 @@ static ssize_t picolcd_fb_update_rate_show(struct device *dev,
if (ret >= PAGE_SIZE)
break;
else if (i == fb_update_rate)
- ret += scnprintf(buf+ret, PAGE_SIZE-ret, "[%u] ", i);
+ ret += sysfs_emit_at(buf, ret, "[%u] ", i);
else
- ret += scnprintf(buf+ret, PAGE_SIZE-ret, "%u ", i);
+ ret += sysfs_emit_at(buf, ret, "%u ", i);
if (ret > 0)
buf[min(ret, (size_t)PAGE_SIZE)-1] = '\n';
return ret;
--
2.29.2
^ permalink raw reply related
* Re: Boot hang with SiFive PLIC when routing I2C-HID level-triggered interrupts
From: Nam Cao @ 2024-03-17 21:27 UTC (permalink / raw)
To: Eva Kurchatova
Cc: linux-riscv, bugs, linux-i2c, jikos, benjamin.tissoires, dianders,
mripard, johan+linaro, linux-input, linux-kernel
In-Reply-To: <CA+eeCSPUDpUg76ZO8dszSbAGn+UHjcyv8F1J-CUPVARAzEtW9w@mail.gmail.com>
Cc: HID folks
On 14/Mar/2024 Eva Kurchatova wrote:
> If an I2C-HID controller level-triggered IRQ line is routed directly as
> a PLIC IRQ, and we spam input early enough in kernel boot process
> (Somewhere between initializing NET, ALSA subsystems and before
> i2c-hid driver init), then there is a chance of kernel locking up
> completely and not going any further.
>
> There are no kernel messages printed with all the IRQ, task hang
> debugging enabled - other than (sometimes) it reports sched RT
> throttling after a few seconds. Basic timer interrupt handling is
> intact - fbdev tty cursor is still blinking.
>
> It appears that in such a case the I2C-HID IRQ line is raised; PLIC
> notifies the (single) boot system hart, kernel claims the IRQ and
> immediately completes it by writing to CLAIM/COMPLETE register.
> No access to the I2C controller (OpenCores) or I2C-HID registers
> is made, so the HID report is never consumed and IRQ line stays
> raised forever. The kernel endlessly claims & completes IRQs
> without doing any work with the device. It doesn't always end up this
> way; sometimes boot process completes and there are no signs of
> interrupt storm or stuck IRQ processing afterwards.
It seems I2C HID's interrupt handler (i2c_hid_irq) returns immediately if
I2C_HID_READ_PENDING is set. This flag is supposed to be cleared in
i2c_hid_xfer(), but since the (threaded) interrupt handler runs at higher
priority, the flag is never cleared. So we have a lock-up: interrupt
handler won't do anything unless the flag is cleared, but the clearing of
this flag is done in a lower priority task which never gets scheduled while
the interrupt handler is active.
There is RT throttling to prevent RT tasks from locking up the system like
this. I don't know much about scheduling stuffs, so I am not really sure
why RT throttling does not work. I think because RT throttling triggers
when RT tasks take too much CPU time, but in this case hard interrupt
handlers take lots of CPU time too (~50% according to my measurement), so
RT throttling doesn't trigger often enough (in this case, it triggers once
and never again). Again, I don't know much about scheduler so I may be
talking nonsense here.
The flag I2C_HID_READ_PENDING seems to be used to make sure that only 1
I2C operation can happen at a time. But this seems pointless, because I2C
subsystem already takes care of this. So I think we can just remove it.
Can you give the below patch a try?
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 2735cd585af0..799ad0ef9c4a 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -64,7 +64,6 @@
/* flags */
#define I2C_HID_STARTED 0
#define I2C_HID_RESET_PENDING 1
-#define I2C_HID_READ_PENDING 2
#define I2C_HID_PWR_ON 0x00
#define I2C_HID_PWR_SLEEP 0x01
@@ -190,15 +189,10 @@ static int i2c_hid_xfer(struct i2c_hid *ihid,
msgs[n].len = recv_len;
msgs[n].buf = recv_buf;
n++;
-
- set_bit(I2C_HID_READ_PENDING, &ihid->flags);
}
ret = i2c_transfer(client->adapter, msgs, n);
- if (recv_len)
- clear_bit(I2C_HID_READ_PENDING, &ihid->flags);
-
if (ret != n)
return ret < 0 ? ret : -EIO;
@@ -566,9 +560,6 @@ static irqreturn_t i2c_hid_irq(int irq, void *dev_id)
{
struct i2c_hid *ihid = dev_id;
- if (test_bit(I2C_HID_READ_PENDING, &ihid->flags))
- return IRQ_HANDLED;
-
i2c_hid_get_input(ihid);
return IRQ_HANDLED;
^ permalink raw reply related
* New driver for Corsair Void headset family
From: Stuart @ 2024-03-17 20:45 UTC (permalink / raw)
To: linux-input; +Cc: LKML, Jiri Kosina, Benjamin Tissoires
Hi, I'm writing a driver for the Corsair Void family of headsets, but
I have a few questions before I submit a patch for v1.
1. I can see there's already a hid-corsair driver, but my driver is
around the same size as it so far, and shares almost nothing in common
with it, as they're completely different devices. Should I still
attempt to merge my corsair-void driver with hid-corsair, or submit it
as a separate driver, such as hid-corsair-void?
2. The headsets can be queried for their firmware versions (separate
versions for the headset and the receiver). However, the wired version
of the headset doesn't have a receiver to report the version for, and
the wireless version isn't always connected to the receiver, so the
version can't always be retrieved. Should I just register and
unregister the attributes as the versions are made available /
unavailable, or always show them and just return something like
-ENODEV if there's no device to return a version for?
3. How should I expose the limits for some of the sysfs attributes?
For example, the headset can play a few alert tones, and I wired that
up by playing the tone whose ID is passed to the send_alert attribute.
If it only supports IDs 0 and 1, should I document this in the docs
for the driver, in the source code, or as another attribute to return
the highest supported ID? I have the same issue for sidetone, where
the headset supports values 0-55, but I'm not sure how to communicate
this.
4. The wireless variant of the headset reports a value for its charge
that's ~54 higher than reality while plugged in. The Windows driver
just conveniently reports 'Charging' instead of a value, I'm not sure
how to communicate that the charge value can't be trusted while
plugged in. Who do I need to ask about this? I'm assuming someone
that's responsible for the power_supply class, but I'm not sure who
that is.
Thanks,
Stuart
^ permalink raw reply
* Re: [git pull] Input updates for v6.9-rc0
From: pr-tracker-bot @ 2024-03-17 19:34 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <ZfaLw1CiHZDSXCLo@google.com>
The pull request you sent on Sat, 16 Mar 2024 23:20:51 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.9-rc0
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/6885d7263ecd8ce5ab06029078861fbeb06c4a0a
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* [dtor-input:for-linus] BUILD SUCCESS 57ed9567e63b59350c21ae026635ae051e247abb
From: kernel test robot @ 2024-03-17 18:52 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: 57ed9567e63b59350c21ae026635ae051e247abb Merge branch 'next' into for-linus
elapsed time: 726m
configs tested: 177
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc defconfig gcc
arc haps_hs_defconfig gcc
arc nsimosci_hs_smp_defconfig gcc
arc randconfig-001-20240317 gcc
arc randconfig-002-20240317 gcc
arm allmodconfig gcc
arm allnoconfig clang
arm allyesconfig gcc
arm clps711x_defconfig clang
arm defconfig clang
arm moxart_defconfig gcc
arm randconfig-001-20240317 clang
arm randconfig-002-20240317 clang
arm randconfig-003-20240317 gcc
arm randconfig-004-20240317 clang
arm64 allmodconfig clang
arm64 allnoconfig gcc
arm64 defconfig gcc
arm64 randconfig-001-20240317 clang
arm64 randconfig-002-20240317 gcc
arm64 randconfig-003-20240317 gcc
arm64 randconfig-004-20240317 clang
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
csky randconfig-001-20240317 gcc
csky randconfig-002-20240317 gcc
hexagon allmodconfig clang
hexagon allnoconfig clang
hexagon allyesconfig clang
hexagon defconfig clang
hexagon randconfig-001-20240317 clang
hexagon randconfig-002-20240317 clang
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 buildonly-randconfig-001-20240317 gcc
i386 buildonly-randconfig-002-20240317 clang
i386 buildonly-randconfig-003-20240317 gcc
i386 buildonly-randconfig-004-20240317 clang
i386 buildonly-randconfig-005-20240317 clang
i386 buildonly-randconfig-006-20240317 gcc
i386 defconfig clang
i386 randconfig-001-20240317 clang
i386 randconfig-002-20240317 clang
i386 randconfig-003-20240317 clang
i386 randconfig-004-20240317 clang
i386 randconfig-005-20240317 gcc
i386 randconfig-006-20240317 gcc
i386 randconfig-011-20240317 clang
i386 randconfig-012-20240317 clang
i386 randconfig-013-20240317 clang
i386 randconfig-014-20240317 gcc
i386 randconfig-015-20240317 gcc
i386 randconfig-016-20240317 clang
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20240317 gcc
loongarch randconfig-002-20240317 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
m68k m5208evb_defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
nios2 randconfig-001-20240317 gcc
nios2 randconfig-002-20240317 gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc randconfig-001-20240317 gcc
parisc randconfig-002-20240317 gcc
parisc64 defconfig gcc
powerpc adder875_defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig clang
powerpc ps3_defconfig gcc
powerpc randconfig-001-20240317 clang
powerpc randconfig-002-20240317 gcc
powerpc randconfig-003-20240317 clang
powerpc storcenter_defconfig gcc
powerpc64 randconfig-001-20240317 gcc
powerpc64 randconfig-002-20240317 clang
powerpc64 randconfig-003-20240317 clang
riscv allmodconfig clang
riscv allnoconfig gcc
riscv allyesconfig clang
riscv defconfig clang
riscv randconfig-001-20240317 clang
riscv randconfig-002-20240317 clang
riscv rv32_defconfig clang
s390 allmodconfig clang
s390 allnoconfig clang
s390 allyesconfig gcc
s390 defconfig clang
s390 randconfig-001-20240317 clang
s390 randconfig-002-20240317 gcc
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sh ecovec24_defconfig gcc
sh edosk7705_defconfig gcc
sh kfr2r09-romimage_defconfig gcc
sh randconfig-001-20240317 gcc
sh randconfig-002-20240317 gcc
sh se7343_defconfig gcc
sh sh7785lcr_32bit_defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc defconfig gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
sparc64 randconfig-001-20240317 gcc
sparc64 randconfig-002-20240317 gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig gcc
um defconfig clang
um i386_defconfig gcc
um randconfig-001-20240317 gcc
um randconfig-002-20240317 gcc
um x86_64_defconfig clang
x86_64 allnoconfig clang
x86_64 allyesconfig clang
x86_64 buildonly-randconfig-001-20240317 gcc
x86_64 buildonly-randconfig-002-20240317 gcc
x86_64 buildonly-randconfig-003-20240317 gcc
x86_64 buildonly-randconfig-004-20240317 clang
x86_64 buildonly-randconfig-005-20240317 gcc
x86_64 buildonly-randconfig-006-20240317 clang
x86_64 defconfig gcc
x86_64 randconfig-001-20240317 gcc
x86_64 randconfig-002-20240317 gcc
x86_64 randconfig-003-20240317 gcc
x86_64 randconfig-004-20240317 gcc
x86_64 randconfig-005-20240317 gcc
x86_64 randconfig-006-20240317 clang
x86_64 randconfig-011-20240317 clang
x86_64 randconfig-012-20240317 gcc
x86_64 randconfig-013-20240317 clang
x86_64 randconfig-014-20240317 clang
x86_64 randconfig-015-20240317 gcc
x86_64 randconfig-016-20240317 clang
x86_64 randconfig-071-20240317 gcc
x86_64 randconfig-072-20240317 clang
x86_64 randconfig-073-20240317 clang
x86_64 randconfig-074-20240317 gcc
x86_64 randconfig-075-20240317 gcc
x86_64 randconfig-076-20240317 clang
x86_64 rhel-8.3-rust clang
xtensa allnoconfig gcc
xtensa randconfig-001-20240317 gcc
xtensa randconfig-002-20240317 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v9 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: Marco Felsch @ 2024-03-17 16:32 UTC (permalink / raw)
To: Kamel Bouhara
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
Jeff LaBundy, catalin.popescu, mark.satterthwaite,
Thomas Petazzoni, Gregory Clement, bsp-development.geo
In-Reply-To: <20240314082842.GA6963@tpx1.home>
On 24-03-14, Kamel Bouhara wrote:
> Le Wed, Mar 13, 2024 at 09:21:35PM +0100, Marco Felsch a écrit :
> > Hi Kamel,
> >
> Hi Marco,
>
> > please see below, be aware that this is just an rough review.
> >
>
> [...]
>
> > > +
> > > +static int axiom_i2c_probe(struct i2c_client *client)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct input_dev *input_dev;
> > > + struct axiom_data *ts;
> > > + u32 poll_interval;
> > > + int target;
> > > + int error;
> > > +
> > > + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> > > + if (!ts)
> > > + return -ENOMEM;
> > > +
> > > + i2c_set_clientdata(client, ts);
> > > + ts->client = client;
> > > + ts->dev = dev;
> > > +
> > > + ts->regmap = devm_regmap_init_i2c(client, &axiom_i2c_regmap_config);
> > > + error = PTR_ERR_OR_ZERO(ts->regmap);
> > > + if (error) {
> > > + dev_err(dev, "Failed to initialize regmap: %d\n", error);
> > > + return error;
> > > + }
> > > +
> > > + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> > > + if (IS_ERR(ts->reset_gpio))
> > > + return dev_err_probe(dev, PTR_ERR(ts->reset_gpio), "failed to get reset GPIO\n");
> > > +
> > > + if (ts->reset_gpio)
> > > + axiom_reset(ts->reset_gpio);
> >
> > This seems useless, since you doing an reset without enabling the power
> > supply (below). I know there are systems which do have the supply always
> > connected or for ACPI the supply is managed via firmware, but the driver
> > should implement the correct logic and for DT/OF case this is not
> > correct.
> >
>
> Alright, this can be moved after enabling vdda regulator as this is
> still required in the power sequence.
>
> > > +
> > > + ts->vddi = devm_regulator_get_optional(dev, "vddi");
> > > + if (!IS_ERR(ts->vddi)) {
> > > + error = devm_regulator_get_enable(dev, "vddi");
> >
> > Regulators are ref counted and now you request the regulator twice. Also
> > the regulator is not optional, it is required for the device to work.
> > Same applies to the vdda below.
> >
>
> Ack, I wrongly took my use case (ACPI + fixed regulators) but this isn't
> a common use case.
>
> > > + if (error)
> > > + return dev_err_probe(&client->dev, error,
> > > + "Failed to enable vddi regulator\n");
> > > + }
> > > +
> > > + ts->vdda = devm_regulator_get_optional(dev, "vdda");
> > > + if (!IS_ERR(ts->vdda)) {
> > > + error = devm_regulator_get_enable(dev, "vdda");
> > > + if (error)
> > > + return dev_err_probe(&client->dev, error,
> > > + "Failed to enable vdda regulator\n");
> > > + msleep(AXIOM_STARTUP_TIME_MS);
> > > + }
> > > +
> > > + error = axiom_discover(ts);
> > > + if (error)
> > > + return dev_err_probe(dev, error, "Failed touchscreen discover\n");
> > > +
> > > + input_dev = devm_input_allocate_device(ts->dev);
> > > + if (!input_dev)
> > > + return -ENOMEM;
> > > +
> > > + input_dev->name = "TouchNetix axiom Touchscreen";
> > > + input_dev->phys = "input/axiom_ts";
> > > +
> > > + input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, 65535, 0, 0);
> > > + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, 65535, 0, 0);
> > > + input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
> > > + input_set_abs_params(input_dev, ABS_MT_DISTANCE, 0, 127, 0, 0);
> > > + input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 127, 0, 0);
> > > +
> > > + touchscreen_parse_properties(input_dev, true, &ts->prop);
> > > +
> > > + /* Registers the axiom device as a touchscreen instead of a mouse pointer */
> > > + error = input_mt_init_slots(input_dev, AXIOM_U41_MAX_TARGETS, INPUT_MT_DIRECT);
> > > + if (error)
> > > + return error;
> > > +
> > > + /* Enables the raw data for up to 4 force channels to be sent to the input subsystem */
> > > + set_bit(EV_REL, input_dev->evbit);
> > > + set_bit(EV_MSC, input_dev->evbit);
> > > + /* Declare that we support "RAW" Miscellaneous events */
> > > + set_bit(MSC_RAW, input_dev->mscbit);
> > > +
> > > + ts->input_dev = input_dev;
> > > + input_set_drvdata(ts->input_dev, ts);
> > > +
> > > + /* Ensure that all reports are initialised to not be present. */
> > > + for (target = 0; target < AXIOM_U41_MAX_TARGETS; target++)
> > > + ts->targets[target].state = AXIOM_TARGET_STATE_NOT_PRESENT;
> > > +
> > > + error = devm_request_threaded_irq(dev, client->irq, NULL,
> > > + axiom_irq, IRQF_ONESHOT, dev_name(dev), ts);
> > > + if (error) {
> > > + dev_info(dev, "Request irq failed, falling back to polling mode");
> > > +
> > > + error = input_setup_polling(input_dev, axiom_i2c_poll);
> > > + if (error)
> > > + return dev_err_probe(ts->dev, error, "Unable to set up polling mode\n");
> > > +
> > > + if (!device_property_read_u32(ts->dev, "poll-interval", &poll_interval))
> >
> > This is not wrong but can we move the "poll-intervall" parsing into
> > touchscreen_parse_properties() since it seems pretty common to me.
>
> Maybe too late to add it in this series :).
>
> >
> > > + input_set_poll_interval(input_dev, poll_interval);
> > > + else
> > > + input_set_poll_interval(input_dev, POLL_INTERVAL_DEFAULT_MS);
> > > + }
> > > +
> > > + error = input_register_device(input_dev);
> > > + if (error)
> > > + return dev_err_probe(ts->dev, error,
> > > + "Could not register with Input Sub-system.\n");
> >
> > return input_register_device(input_dev);
>
> Ack, thanks.
>
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct i2c_device_id axiom_i2c_id_table[] = {
> > > + { "ax54a" },
> > > + { },
> > > +};
> > > +MODULE_DEVICE_TABLE(i2c, axiom_i2c_id_table);
> >
> > Do we really need an i2c-id table here? Most platforms do either use OF
> > or ACPI.
>
> If not wrong this is used to enumarate the device from userspace
> and in my case it is required as there is no direct i2c controller
> exposed from ACPI pov.
Ah you're right, I forgot this use-case.
Regards,
Marco
>
> Thanks !
>
> --
> Kamel Bouhara, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com
>
^ permalink raw reply
* [git pull] Input updates for v6.9-rc0
From: Dmitry Torokhov @ 2024-03-17 6:20 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.9-rc0
to receive updates for the input subsystem. You will get:
- a new driver for Goodix Berlin I2C and SPI touch controllers
- support for IQS7222D v1.1 and v1.2 in iqs7222 driver
- support for IST3032C and IST3038B parts in Imagis touchscreen driver
- support for touch keys for Imagis touchscreen controllers
- support for Snakebyte GAMEPADs in xpad driver
- various cleanups and conversions to yaml for device tree bindings
- assorted fixes and cleanups
- old Synaptics navpoint driver has been removed since the only board
that used it (HP iPAQ hx4700) was removed a while ago.
Changelog:
---------
Bernhard Seibold (1):
Input: leds - set default-trigger for mute
Brenton Simpson (1):
Input: xpad - sort xpad_device by vendor and product ID
Christophe JAILLET (1):
Input: remove usage of the deprecated ida_simple_xx() API
Colin Ian King (1):
Input: ti_am335x_tsc - remove redundant assignment to variable config
Dharma Balasubiramani (1):
dt-bindings: input: atmel,captouch: convert bindings to YAML
Dmitry Torokhov (3):
Input: matrix_keypad - avoid repeatedly converting GPIO to IRQ
Input: matrix_keypad - consolidate handling of clustered interrupt
Input: matrix_keypad - switch to using managed resources
Duje Mihanović (5):
Input: navpoint - remove driver
Input: 88pm80x_onkey - add SPDX and drop GPL boilerplate
Input: imagis - use FIELD_GET where applicable
dt-bindings: input: imagis: Document touch keys
Input: imagis - add touch key support
Heiner Kallweit (1):
Input: leds - change config symbol dependency for audio mute trigger
Jeff LaBundy (1):
Input: iqs7222 - add support for IQS7222D v1.1 and v1.2
Karel Balej (2):
dt-bindings: input/touchscreen: imagis: add compatible for IST3032C
input/touchscreen: imagis: add support for IST3032C
Krzysztof Kozlowski (3):
dt-bindings: input: silead,gsl1680: do not override firmware-name $ref
dt-bindings: input: allwinner,sun4i-a10-lrad: drop redundant type from label
dt-bindings: input: samsung,s3c6410-keypad: convert to DT Schema
Kunwu Chan (1):
Input: synaptics-rmi4 - fail probing if memory allocation for "phys" fails
Luca Ceresoli (1):
dt-bindings: input: touchscreen: goodix: clarify irq-gpios misleading text
Luca Weiss (1):
dt-bindings: input: melfas,mms114: add MMS252 compatible
Markuss Broks (3):
input/touchscreen: imagis: Correct the maximum touch area value
dt-bindings: input/touchscreen: Add compatible for IST3038B
input/touchscreen: imagis: Add support for Imagis IST3038B
Matt Scialabba (1):
Input: xpad - add support for Snakebyte GAMEPADs
Neil Armstrong (4):
dt-bindings: input: document Goodix Berlin Touchscreen IC
Input: add core support for Goodix Berlin Touchscreen IC
Input: goodix-berlin - add I2C support for Goodix Berlin Touchscreen IC
Input: goodix-berlin - add SPI support for Goodix Berlin Touchscreen IC
Ricardo B. Marliere (4):
Input: gameport - make gameport_bus const
Input: synaptics-rmi4 - make rmi_bus_type const
Input: serio - make serio_bus const
Input: make input_class constant
Ruan Jinjie (1):
Input: bcm-keypad - remove redundant of_match_ptr()
Yang Li (1):
Input: xilinx_ps2 - fix kernel-doc for xps2_of_probe function
Diffstat:
--------
.../input/allwinner,sun4i-a10-lradc-keys.yaml | 1 -
.../devicetree/bindings/input/atmel,captouch.txt | 36 -
.../devicetree/bindings/input/atmel,captouch.yaml | 59 ++
.../bindings/input/samsung,s3c6410-keypad.yaml | 121 ++++
.../devicetree/bindings/input/samsung-keypad.txt | 77 ---
.../bindings/input/touchscreen/goodix,gt9916.yaml | 95 +++
.../bindings/input/touchscreen/goodix.yaml | 5 +-
.../input/touchscreen/imagis,ist3038c.yaml | 21 +-
.../bindings/input/touchscreen/melfas,mms114.yaml | 6 +-
.../bindings/input/touchscreen/silead,gsl1680.yaml | 2 +-
.../devicetree/bindings/power/wakeup-source.txt | 2 +-
drivers/input/gameport/gameport.c | 4 +-
drivers/input/input-leds.c | 8 +-
drivers/input/input.c | 16 +-
drivers/input/joystick/xpad.c | 14 +-
drivers/input/keyboard/bcm-keypad.c | 2 +-
drivers/input/keyboard/matrix_keypad.c | 170 ++---
drivers/input/misc/88pm80x_onkey.c | 14 +-
drivers/input/misc/iqs7222.c | 112 +++
drivers/input/mouse/Kconfig | 12 -
drivers/input/mouse/Makefile | 1 -
drivers/input/mouse/navpoint.c | 350 ----------
drivers/input/rmi4/rmi_bus.c | 2 +-
drivers/input/rmi4/rmi_bus.h | 2 +-
drivers/input/rmi4/rmi_driver.c | 6 +-
drivers/input/serio/serio.c | 2 +-
drivers/input/serio/xilinx_ps2.c | 3 +-
drivers/input/touchscreen/Kconfig | 31 +
drivers/input/touchscreen/Makefile | 3 +
drivers/input/touchscreen/goodix_berlin.h | 24 +
drivers/input/touchscreen/goodix_berlin_core.c | 755 +++++++++++++++++++++
drivers/input/touchscreen/goodix_berlin_i2c.c | 75 ++
drivers/input/touchscreen/goodix_berlin_spi.c | 178 +++++
drivers/input/touchscreen/imagis.c | 118 +++-
drivers/input/touchscreen/ti_am335x_tsc.c | 1 -
include/linux/input.h | 2 +-
include/linux/input/navpoint.h | 8 -
include/linux/serio.h | 2 +-
38 files changed, 1669 insertions(+), 671 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/atmel,captouch.txt
create mode 100644 Documentation/devicetree/bindings/input/atmel,captouch.yaml
create mode 100644 Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
delete mode 100644 Documentation/devicetree/bindings/input/samsung-keypad.txt
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml
delete mode 100644 drivers/input/mouse/navpoint.c
create mode 100644 drivers/input/touchscreen/goodix_berlin.h
create mode 100644 drivers/input/touchscreen/goodix_berlin_core.c
create mode 100644 drivers/input/touchscreen/goodix_berlin_i2c.c
create mode 100644 drivers/input/touchscreen/goodix_berlin_spi.c
delete mode 100644 include/linux/input/navpoint.h
Thanks.
--
Dmitry
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS 81c32343d04f8ca974681d5fb5d939d2e1f58851
From: kernel test robot @ 2024-03-16 7:27 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: 81c32343d04f8ca974681d5fb5d939d2e1f58851 Input: xpad - add support for Snakebyte GAMEPADs
elapsed time: 720m
configs tested: 170
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-001-20240316 gcc
arc randconfig-002-20240316 gcc
arc vdk_hs38_smp_defconfig gcc
arm alldefconfig gcc
arm allmodconfig gcc
arm allnoconfig clang
arm allyesconfig gcc
arm at91_dt_defconfig clang
arm axm55xx_defconfig clang
arm clps711x_defconfig clang
arm defconfig clang
arm exynos_defconfig clang
arm milbeaut_m10v_defconfig clang
arm moxart_defconfig gcc
arm mps2_defconfig clang
arm nhk8815_defconfig clang
arm omap1_defconfig gcc
arm qcom_defconfig clang
arm randconfig-001-20240316 gcc
arm randconfig-002-20240316 gcc
arm randconfig-003-20240316 gcc
arm randconfig-004-20240316 gcc
arm s5pv210_defconfig gcc
arm stm32_defconfig gcc
arm wpcm450_defconfig gcc
arm64 allmodconfig clang
arm64 allnoconfig gcc
arm64 defconfig gcc
arm64 randconfig-001-20240316 gcc
arm64 randconfig-002-20240316 clang
arm64 randconfig-003-20240316 clang
arm64 randconfig-004-20240316 gcc
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
csky randconfig-001-20240316 gcc
csky randconfig-002-20240316 gcc
hexagon allmodconfig clang
hexagon allnoconfig clang
hexagon allyesconfig clang
hexagon defconfig clang
hexagon randconfig-001-20240316 clang
hexagon randconfig-002-20240316 clang
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 buildonly-randconfig-001-20240316 clang
i386 buildonly-randconfig-002-20240316 gcc
i386 buildonly-randconfig-003-20240316 gcc
i386 buildonly-randconfig-004-20240316 gcc
i386 buildonly-randconfig-005-20240316 gcc
i386 buildonly-randconfig-006-20240316 gcc
i386 defconfig clang
i386 randconfig-001-20240316 gcc
i386 randconfig-002-20240316 gcc
i386 randconfig-003-20240316 gcc
i386 randconfig-004-20240316 gcc
i386 randconfig-005-20240316 clang
i386 randconfig-006-20240316 gcc
i386 randconfig-011-20240316 clang
i386 randconfig-012-20240316 clang
i386 randconfig-013-20240316 clang
i386 randconfig-014-20240316 clang
i386 randconfig-015-20240316 clang
i386 randconfig-016-20240316 clang
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20240316 gcc
loongarch randconfig-002-20240316 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
m68k m5249evb_defconfig gcc
m68k mvme16x_defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
mips cavium_octeon_defconfig gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
nios2 randconfig-001-20240316 gcc
nios2 randconfig-002-20240316 gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc randconfig-001-20240316 gcc
parisc randconfig-002-20240316 gcc
parisc64 defconfig gcc
powerpc acadia_defconfig clang
powerpc adder875_defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig clang
powerpc cm5200_defconfig clang
powerpc kilauea_defconfig clang
powerpc pcm030_defconfig clang
powerpc ppc40x_defconfig clang
powerpc randconfig-001-20240316 clang
powerpc randconfig-002-20240316 clang
powerpc randconfig-003-20240316 gcc
powerpc tqm8555_defconfig clang
powerpc64 randconfig-001-20240316 gcc
powerpc64 randconfig-002-20240316 gcc
powerpc64 randconfig-003-20240316 clang
riscv allmodconfig clang
riscv allnoconfig gcc
riscv allyesconfig clang
riscv defconfig clang
riscv randconfig-001-20240316 gcc
riscv randconfig-002-20240316 clang
riscv rv32_defconfig clang
s390 allmodconfig clang
s390 allnoconfig clang
s390 allyesconfig gcc
s390 defconfig clang
s390 randconfig-001-20240316 clang
s390 randconfig-002-20240316 clang
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh apsh4ad0a_defconfig gcc
sh defconfig gcc
sh ecovec24_defconfig gcc
sh polaris_defconfig gcc
sh randconfig-001-20240316 gcc
sh randconfig-002-20240316 gcc
sh se7750_defconfig gcc
sh sh7770_generic_defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc defconfig gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
sparc64 randconfig-001-20240316 gcc
sparc64 randconfig-002-20240316 gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig gcc
um defconfig clang
um i386_defconfig gcc
um randconfig-001-20240316 clang
um randconfig-002-20240316 gcc
um x86_64_defconfig clang
x86_64 allnoconfig clang
x86_64 allyesconfig clang
x86_64 defconfig gcc
x86_64 rhel-8.3-rust clang
xtensa allnoconfig gcc
xtensa randconfig-001-20240316 gcc
xtensa randconfig-002-20240316 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v9 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
From: James Ogletree @ 2024-03-15 20:23 UTC (permalink / raw)
To: Jeff LaBundy
Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones, Mark Brown,
open list:CIRRUS LOGIC HAPTIC DRIVERS,
open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <ZfMdmYM5cK7GKdbG@nixie71>
Hi Jeff,
> On Mar 14, 2024, at 10:54 AM, Jeff LaBundy <jeff@labundy.com> wrote:
>
> Hi James,
>
> On Fri, Mar 08, 2024 at 10:24:20PM +0000, James Ogletree wrote:
>>
>> +static void vibra_start_worker(struct work_struct *work)
>> +{
>> + struct vibra_work *work_data = container_of(work, struct vibra_work, work);
>> + struct vibra_info *info = work_data->info;
>> + struct vibra_effect *start_effect;
>> +
>> + if (pm_runtime_resume_and_get(info->dev) < 0)
>> + goto err_free;
>> +
>> + mutex_lock(&info->lock);
>> +
>> + start_effect = vibra_find_effect(work_data->effect->id, &info->effect_head);
>> + if (start_effect) {
>> + while (--work_data->count >= 0) {
>> + info->dsp.write(info->dev, info->regmap, start_effect->index);
>> + usleep_range(work_data->effect->replay.length,
>> + work_data->effect->replay.length + 100);
>> + }
>> + }
>> +
>> + mutex_unlock(&info->lock);
>> +
>> + if (!start_effect)
>> + dev_err(info->dev, "Effect to play not found\n");
>> +
>> + pm_runtime_mark_last_busy(info->dev);
>> + pm_runtime_put_autosuspend(info->dev);
>> +err_free:
>> + kfree(work_data);
>
> This business of allocating memory in one thread and freeing it in another
> got pretty hard to follow, which means it will be hard to maintain. I know
> there are some restrictions around writing into the HALO core, but I'd like
> to see something simpler; the da7280 driver is nowhere near as complex.
>
> How many s16 words do you realistically need for custom_data? Is it possible
> to simply include an array in the cs40l50_vibra_info struct, and bleat if
> user space tries to upload greater than the maximum size? This seems to be
> the approach of the much simpler da7280 driver as well.
>
> This array would go on the heap just the same, but you do not have to manage
> it so carefully. Opinions may vary, but mine is that memory allocation and
> freeing should be done in the same scope where possible.
The original issue was that the old implementation of playback effect had
a race condition which Dmitry pointed out. Consider when cs40l50_playback
is called: it populates effect data (including the index, say 1) and
schedules a trigger work to be run in the future. Before this work is
executed, a second call to cs40l50_playback is made, and again effect is
populated (say, index 2), and trigger work is scheduled again. If effect
is in shared data, then it will be overwritten by the second call and index
2 will be triggered twice.
So, I needed a separate work item for each playback to hold its individual
data so that effect cannot be overwritten by other playback calls. This
meant that I needed to take the playback work item and work-related data
out of shared memory (cs40l50_vibra_info), and create a structure
containing all the work data tied to that specific queue_work invocation,
as well as the work_struct itself so we can access that unique work data
via container_of() in the worker function.
Of course to have individuated work data, we need to allocate and populate
a new cs40l50_work with data in each input callback. Sometimes this
entails dynamic allocation. Why? Because sometimes we have no way of
knowing when we can let go of the work data. Contrast upload and erase:
these callbacks are not called in atomic context, so we can use flush_work
to guarantee the worker function returns, and therefore we can free the
work data in the callback. cs40l50_playback on the other hand is called
in atomic context, so we cannot use flush_work, and so we have to
"hold on" to the work data until such time known only by the worker
function itself. Hence we must free it there. So I think the complexity
here falls into the necessary bucket.
I don't think the above applies to the custom data memory, since that
particular memory is freed in the same function it is allocated in.
Looking at the possible use cases, the ceiling on the size of the data
is quite high if the customer wishes to upload a long sequence of
effects. So I prefer to keep that as it currently is.
It might be possible to use the "individuated work item" design for
playbacks only, and use the old "shared work item" for the other
callbacks. However, I decided that having one uniform implementation
would be better than mixing two schemes.
Best,
James
^ permalink raw reply
* Re: [PATCH] dt-bindings: input: samsung,s3c6410-keypad: convert to DT Schema
From: Dmitry Torokhov @ 2024-03-15 19:45 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
devicetree, linux-kernel
In-Reply-To: <20240312183001.714626-1-krzysztof.kozlowski@linaro.org>
On Tue, Mar 12, 2024 at 07:30:01PM +0100, Krzysztof Kozlowski wrote:
> Convert Samsung SoC Keypad bindings to DT schema with changes:
> 1. Rename "linux,keypad-no-autorepeat" property to
> "linux,input-no-autorepeat", because the latter was implemented in
> the Linux driver.
> 2. Add clocks and clock-names, already used by DTS and the Linux driver.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: Advise needed on stale patches
From: Dmitry Torokhov @ 2024-03-15 18:53 UTC (permalink / raw)
To: Francesco Dolcini; +Cc: linux-input, Stefan Eichenberger, Emanuele Ghidoli
In-Reply-To: <20240315093307.GA5060@francesco-nb>
Hi Francesco,
On Fri, Mar 15, 2024 at 10:33:07AM +0100, Francesco Dolcini wrote:
> Hello Dmitry and Linux Input people,
>
> In the last months I sent (or was involved in) a couple of small patches
> [1][2] (bug fixes or small addition to an existing driver) and they have
> been around in the mailing list for quite some time (months, 2023-09-20
> the oldest one).
>
> I am not sure on how to progress here, therefore looking for some advise.
>
> Maybe they did just fall through the cracks, in this case just take this
> as a kind head up (or maybe those email did not pass some aggressive
> spam filtering?).
>
> Or maybe the changes are not deemed correct or something wrong was done,
> and in this case it would be great to know what we could improve.
>
> Or, maybe you just had other priorities (life and work are challenging
> at times, I know), in this case, if possible, I would love to know if I
> can provide some small help in any way.
Sorry, I am swamped with other projects. I just responded on the
mailing list for the patch [2].
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH RESEND v3 2/2] Input: atmel_mxt_ts - support poweroff in suspend
From: Dmitry Torokhov @ 2024-03-15 18:51 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: nick, robh+dt, krzysztof.kozlowski+dt, conor+dt, nicolas.ferre,
alexandre.belloni, claudiu.beznea, linus.walleij, linux-input,
devicetree, linux-arm-kernel, linux-kernel, francesco.dolcini,
Stefan Eichenberger
In-Reply-To: <20240209105012.22470-3-eichest@gmail.com>
Hi Stefan,
On Fri, Feb 09, 2024 at 11:50:12AM +0100, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> Add a new device tree property to indicate that the device should be
> powered off in suspend mode. We have a shared regulator that powers the
> display, a USB hub and some other peripherals. The maXTouch controller
> doesn't normally disable the regulator in suspend mode, so our extra
> peripherals stay powered on. This is not desirable as it consumes more
> power. With this patch we add the option to disable the regulator in
> suspend mode for the maXTouch and accept the longer initialisation time.
>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 72 ++++++++++++++++++------
> 1 file changed, 55 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 542a31448c8f..2d5655385702 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -317,6 +317,7 @@ struct mxt_data {
> struct gpio_desc *reset_gpio;
> struct gpio_desc *wake_gpio;
> bool use_retrigen_workaround;
> + bool poweroff_sleep;
>
> /* Cached parameters from object table */
> u16 T5_address;
> @@ -2799,15 +2800,18 @@ static int mxt_configure_objects(struct mxt_data *data,
> dev_warn(dev, "Error %d updating config\n", error);
> }
>
> - if (data->multitouch) {
> - error = mxt_initialize_input_device(data);
> - if (error)
> - return error;
> - } else {
> - dev_warn(dev, "No touch object detected\n");
> - }
> + /* If input device is not already registered */
> + if (!data->input_dev) {
> + if (data->multitouch) {
> + error = mxt_initialize_input_device(data);
> + if (error)
> + return error;
> + } else {
> + dev_warn(dev, "No touch object detected\n");
> + }
>
> - mxt_debug_init(data);
> + mxt_debug_init(data);
> + }
>
> return 0;
> }
> @@ -3325,6 +3329,8 @@ static int mxt_probe(struct i2c_client *client)
> msleep(MXT_RESET_INVALID_CHG);
> }
>
> + data->poweroff_sleep = device_property_read_bool(&client->dev,
> + "atmel,poweroff-sleep");
> /*
> * Controllers like mXT1386 have a dedicated WAKE line that could be
> * connected to a GPIO or to I2C SCL pin, or permanently asserted low.
> @@ -3387,12 +3393,21 @@ static int mxt_suspend(struct device *dev)
> if (!input_dev)
> return 0;
>
> - mutex_lock(&input_dev->mutex);
> + if (!device_may_wakeup(dev) && data->poweroff_sleep) {
> + if (data->reset_gpio)
> + gpiod_set_value(data->reset_gpio, 1);
>
> - if (input_device_enabled(input_dev))
> - mxt_stop(data);
> + regulator_bulk_disable(ARRAY_SIZE(data->regulators),
> + data->regulators);
> + data->T44_address = 0;
> + } else {
> + mutex_lock(&input_dev->mutex);
> +
> + if (input_device_enabled(input_dev))
> + mxt_stop(data);
>
> - mutex_unlock(&input_dev->mutex);
> + mutex_unlock(&input_dev->mutex);
> + }
This all should go into mxt_stop(), so that if device is closed, or
inhibited, you power it down as well (if you can).
>
> disable_irq(data->irq);
>
> @@ -3408,14 +3423,37 @@ static int mxt_resume(struct device *dev)
> if (!input_dev)
> return 0;
>
> - enable_irq(data->irq);
> + if (!device_may_wakeup(dev) && data->poweroff_sleep) {
> + int ret;
>
> - mutex_lock(&input_dev->mutex);
> + ret = regulator_bulk_enable(ARRAY_SIZE(data->regulators),
> + data->regulators);
> + if (ret) {
> + dev_err(dev, "failed to enable regulators: %d\n",
> + ret);
> + return ret;
> + }
> + msleep(MXT_BACKUP_TIME);
>
> - if (input_device_enabled(input_dev))
> - mxt_start(data);
> + if (data->reset_gpio) {
> + /* Wait a while and then de-assert the RESET GPIO line */
> + msleep(MXT_RESET_GPIO_TIME);
> + gpiod_set_value(data->reset_gpio, 0);
> + msleep(MXT_RESET_INVALID_CHG);
> + }
>
> - mutex_unlock(&input_dev->mutex);
> + /* This also enables the irq again */
> + mxt_initialize(data);
And this needs to go into mxt_start(). Also, we should make sure that
once resume operation completes the device is fully operational. That
means you should not request the firmware asynchronously in
mxt_initialize() in case you are in the resume path. I think you should
also unwind mxt_initialize() and mxt_configure_objects() to make it
clear what is the part of initial initialization and what is part of
re-initializing during resume. The configuration that is exposed to
userspace (resolution, number of objects, other properties) should stay
the same, the configuration of the chip itself (power mode, etc) should
be restored.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2] HID: nintendo: Don't fail on setting baud rate
From: Max Staudt @ 2024-03-15 16:57 UTC (permalink / raw)
To: Daniel J . Ogorchock, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, max
Some third-party controllers can't change the baud rate.
We can still use the gamepad as-is, so let's do that.
Signed-off-by: Max Staudt <max@enpas.org>
---
Changes in v2:
- Made hid_err() a hid_warn() since it's not a fatal failure.
---
drivers/hid/hid-nintendo.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 997c3a1adaca..395ed6b5ef59 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -2164,8 +2164,11 @@ static int joycon_init(struct hid_device *hdev)
/* set baudrate for improved latency */
ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
if (ret) {
- hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
- goto out_unlock;
+ /*
+ * We can function with the default baudrate.
+ * Provide a warning, and continue on.
+ */
+ hid_warn(hdev, "Failed to set baudrate (ret=%d), continuing anyway\n", ret);
}
/* handshake */
ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
--
2.39.2
^ permalink raw reply related
* Re: [PATCH] dt-bindings: input: samsung,s3c6410-keypad: convert to DT Schema
From: Rob Herring @ 2024-03-15 16:10 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dmitry Torokhov, linux-kernel, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree
In-Reply-To: <20240312183001.714626-1-krzysztof.kozlowski@linaro.org>
On Tue, 12 Mar 2024 19:30:01 +0100, Krzysztof Kozlowski wrote:
> Convert Samsung SoC Keypad bindings to DT schema with changes:
> 1. Rename "linux,keypad-no-autorepeat" property to
> "linux,input-no-autorepeat", because the latter was implemented in
> the Linux driver.
> 2. Add clocks and clock-names, already used by DTS and the Linux driver.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> .../input/samsung,s3c6410-keypad.yaml | 121 ++++++++++++++++++
> .../bindings/input/samsung-keypad.txt | 77 -----------
> .../bindings/power/wakeup-source.txt | 2 +-
> 3 files changed, 122 insertions(+), 78 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> delete mode 100644 Documentation/devicetree/bindings/input/samsung-keypad.txt
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ 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