* [PATCH] HID: steelseries: Fix signedness bug in steelseries_headset_arctis_1_fetch_battery()
From: Dan Carpenter @ 2023-09-07 9:55 UTC (permalink / raw)
To: Bastien Nocera
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, kernel-janitors
The hid_hw_raw_request() function returns negative error codes or the
number bytes transferred. If it returns negative error codes, then the
problem is that when we check if "ret < sizeof(arctis_1_battery_request)",
negative values are type promoted to high unsigned values and treated as
success. Add an explicit check for negatives to address this issue.
Fixes: a0c76896c3fb ("HID: steelseries: Add support for Arctis 1 XBox")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
drivers/hid/hid-steelseries.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index 43d2cf7153d7..485d2287d58a 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -390,7 +390,7 @@ static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev)
ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0],
write_buf, sizeof(arctis_1_battery_request),
HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
- if (ret < sizeof(arctis_1_battery_request)) {
+ if (ret < 0 || ret < sizeof(arctis_1_battery_request)) {
hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
ret = -ENODATA;
}
--
2.39.2
^ permalink raw reply related
* [PATCH v4] HID: nintendo: cleanup LED code
From: Martino Fontana @ 2023-09-07 9:49 UTC (permalink / raw)
To: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel
Cc: Martino Fontana
- Only turn on the nth LED, instead of all the LEDs up to n. This better
matches Nintendo consoles' behaviour, as they never turn on more than
one LED.
(Note that the behavior still consinsts in increasing the player number
every time a controller is connected, never decreasing it. It should be
as is described in https://bugzilla.kernel.org/show_bug.cgi?id=216225.
However, any implementation here would stop making sense as soon as a
non-Nintendo controller is connected, which is why I'm not bothering.)
- Split part of `joycon_home_led_brightness_set` (which is called by hid)
into `joycon_set_home_led` (which is what actually sets the LEDs), for
consistency with player LEDs.
- `joycon_player_led_brightness_set` won't try it to "determine which player
led this is" anymore: it's already looking at every LED brightness value.
- Instead of first registering the `led_classdev`, then attempting to set
the LED and unregistering the `led_classdev` if it fails, first attempt
to set the LED, then register the `led_classdev` only if it succeeds
(the class is still filled up in either case).
- If setting the player LEDs fails, still attempt setting the home LED.
(I don't know there's a third party controller where this may actually
happen, but who knows...)
- Use `JC_NUM_LEDS` where appropriate instead of 4.
- Print return codes in more places.
- Use spinlock instead of mutex for `input_num`. Copy its value to a local
variable, so that it can be unlocked immediately.
- `input_num` starts counting from 0
- Less holding of mutexes in general.
Changes for v2:
Applied suggestions, commit message explains more stuff, restored `return ret`
when `devm_led_classdev_register` fails (since all other hid drivers do this).
If setting LED fails, `hid_warn` now explicitly says "skipping registration".
Changes for v3 and v4:
Fixed errors and warnings from test robot.
Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
---
drivers/hid/hid-nintendo.c | 113 ++++++++++++++++++-------------------
1 file changed, 56 insertions(+), 57 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index a5ebe857a..543098a8c 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -699,6 +699,25 @@ static int joycon_set_player_leds(struct joycon_ctlr *ctlr, u8 flash, u8 on)
return joycon_send_subcmd(ctlr, req, 1, HZ/4);
}
+static int joycon_set_home_led(struct joycon_ctlr *ctlr, enum led_brightness brightness)
+{
+ struct joycon_subcmd_request *req;
+ u8 buffer[sizeof(*req) + 5] = { 0 };
+ u8 *data;
+
+ req = (struct joycon_subcmd_request *)buffer;
+ req->subcmd_id = JC_SUBCMD_SET_HOME_LIGHT;
+ data = req->data;
+ data[0] = 0x01;
+ data[1] = brightness << 4;
+ data[2] = brightness | (brightness << 4);
+ data[3] = 0x11;
+ data[4] = 0x11;
+
+ hid_dbg(ctlr->hdev, "setting home led brightness\n");
+ return joycon_send_subcmd(ctlr, req, 5, HZ/4);
+}
+
static int joycon_request_spi_flash_read(struct joycon_ctlr *ctlr,
u32 start_addr, u8 size, u8 **reply)
{
@@ -1849,7 +1868,6 @@ static int joycon_player_led_brightness_set(struct led_classdev *led,
int val = 0;
int i;
int ret;
- int num;
ctlr = hid_get_drvdata(hdev);
if (!ctlr) {
@@ -1857,21 +1875,10 @@ static int joycon_player_led_brightness_set(struct led_classdev *led,
return -ENODEV;
}
- /* determine which player led this is */
- for (num = 0; num < JC_NUM_LEDS; num++) {
- if (&ctlr->leds[num] == led)
- break;
- }
- if (num >= JC_NUM_LEDS)
- return -EINVAL;
+ for (i = 0; i < JC_NUM_LEDS; i++)
+ val |= ctlr->leds[i].brightness << i;
mutex_lock(&ctlr->output_mutex);
- for (i = 0; i < JC_NUM_LEDS; i++) {
- if (i == num)
- val |= brightness << i;
- else
- val |= ctlr->leds[i].brightness << i;
- }
ret = joycon_set_player_leds(ctlr, 0, val);
mutex_unlock(&ctlr->output_mutex);
@@ -1884,9 +1891,6 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
struct device *dev = led->dev->parent;
struct hid_device *hdev = to_hid_device(dev);
struct joycon_ctlr *ctlr;
- struct joycon_subcmd_request *req;
- u8 buffer[sizeof(*req) + 5] = { 0 };
- u8 *data;
int ret;
ctlr = hid_get_drvdata(hdev);
@@ -1894,25 +1898,13 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
hid_err(hdev, "No controller data\n");
return -ENODEV;
}
-
- req = (struct joycon_subcmd_request *)buffer;
- req->subcmd_id = JC_SUBCMD_SET_HOME_LIGHT;
- data = req->data;
- data[0] = 0x01;
- data[1] = brightness << 4;
- data[2] = brightness | (brightness << 4);
- data[3] = 0x11;
- data[4] = 0x11;
-
- hid_dbg(hdev, "setting home led brightness\n");
mutex_lock(&ctlr->output_mutex);
- ret = joycon_send_subcmd(ctlr, req, 5, HZ/4);
+ ret = joycon_set_home_led(ctlr, brightness);
mutex_unlock(&ctlr->output_mutex);
-
return ret;
}
-static DEFINE_MUTEX(joycon_input_num_mutex);
+static DEFINE_SPINLOCK(joycon_input_num_spinlock);
static int joycon_leds_create(struct joycon_ctlr *ctlr)
{
struct hid_device *hdev = ctlr->hdev;
@@ -1920,17 +1912,16 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
const char *d_name = dev_name(dev);
struct led_classdev *led;
char *name;
- int ret = 0;
+ int ret;
int i;
- static int input_num = 1;
+ unsigned long flags;
+ int player_led_number;
+ static int input_num;
- /* Set the default controller player leds based on controller number */
- mutex_lock(&joycon_input_num_mutex);
- mutex_lock(&ctlr->output_mutex);
- ret = joycon_set_player_leds(ctlr, 0, 0xF >> (4 - input_num));
- if (ret)
- hid_warn(ctlr->hdev, "Failed to set leds; ret=%d\n", ret);
- mutex_unlock(&ctlr->output_mutex);
+ /* Set the player leds based on controller number */
+ spin_lock_irqsave(&joycon_input_num_spinlock, flags);
+ player_led_number = input_num++ % JC_NUM_LEDS;
+ spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
/* configure the player LEDs */
for (i = 0; i < JC_NUM_LEDS; i++) {
@@ -1938,31 +1929,35 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
d_name,
"green",
joycon_player_led_names[i]);
- if (!name) {
- mutex_unlock(&joycon_input_num_mutex);
+ if (!name)
return -ENOMEM;
- }
led = &ctlr->leds[i];
led->name = name;
- led->brightness = ((i + 1) <= input_num) ? 1 : 0;
+ led->brightness = (i == player_led_number) ? 1 : 0;
led->max_brightness = 1;
led->brightness_set_blocking =
joycon_player_led_brightness_set;
led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
+ }
+ mutex_lock(&ctlr->output_mutex);
+ ret = joycon_set_player_leds(ctlr, 0, 0x1 << player_led_number);
+ mutex_unlock(&ctlr->output_mutex);
+ if (ret) {
+ hid_warn(hdev, "Failed to set players LEDs, skipping registration; ret=%d\n", ret);
+ goto home_led;
+ }
+ for (i = 0; i < JC_NUM_LEDS; i++) {
+ led = &ctlr->leds[i];
ret = devm_led_classdev_register(&hdev->dev, led);
if (ret) {
- hid_err(hdev, "Failed registering %s LED\n", led->name);
- mutex_unlock(&joycon_input_num_mutex);
+ hid_err(hdev, "Failed to register player %d LED; ret=%d\n", i + 1, ret);
return ret;
}
}
- if (++input_num > 4)
- input_num = 1;
- mutex_unlock(&joycon_input_num_mutex);
-
+home_led:
/* configure the home LED */
if (jc_type_has_right(ctlr)) {
name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
@@ -1978,16 +1973,20 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
led->max_brightness = 0xF;
led->brightness_set_blocking = joycon_home_led_brightness_set;
led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
- ret = devm_led_classdev_register(&hdev->dev, led);
+
+ /* Set the home LED to 0 as default state */
+ mutex_lock(&ctlr->output_mutex);
+ ret = joycon_set_home_led(ctlr, 0);
+ mutex_unlock(&ctlr->output_mutex);
if (ret) {
- hid_err(hdev, "Failed registering home led\n");
- return ret;
+ hid_warn(hdev, "Failed to set home LED, skipping registration; ret=%d\n", ret);
+ return 0;
}
- /* Set the home LED to 0 as default state */
- ret = joycon_home_led_brightness_set(led, 0);
+
+ ret = devm_led_classdev_register(&hdev->dev, led);
if (ret) {
- hid_warn(hdev, "Failed to set home LED default, unregistering home LED");
- devm_led_classdev_unregister(&hdev->dev, led);
+ hid_err(hdev, "Failed to register home LED; ret=%d\n", ret);
+ return ret;
}
}
--
2.41.0
^ permalink raw reply related
* [dtor-input:next] BUILD SUCCESS 305dd76455038f3b2338bd0560387cf829c7567c
From: kernel test robot @ 2023-09-07 12:24 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: 305dd76455038f3b2338bd0560387cf829c7567c Input: wdt87xx_i2c - use device core to create driver-specific device attributes
elapsed time: 726m
configs tested: 208
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
alpha randconfig-r016-20230907 gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc defconfig gcc
arc nsim_700_defconfig gcc
arc randconfig-r003-20230907 gcc
arc randconfig-r024-20230907 gcc
arc randconfig-r033-20230907 gcc
arm allmodconfig gcc
arm allnoconfig gcc
arm allyesconfig gcc
arm defconfig gcc
arm ep93xx_defconfig clang
arm footbridge_defconfig gcc
arm randconfig-r002-20230907 gcc
arm stm32_defconfig gcc
arm64 allmodconfig gcc
arm64 allnoconfig gcc
arm64 allyesconfig gcc
arm64 defconfig gcc
arm64 randconfig-r001-20230907 clang
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
csky randconfig-r011-20230907 gcc
hexagon randconfig-r034-20230907 clang
hexagon randconfig-r035-20230907 clang
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 buildonly-randconfig-001-20230907 clang
i386 buildonly-randconfig-002-20230907 clang
i386 buildonly-randconfig-003-20230907 clang
i386 buildonly-randconfig-004-20230907 clang
i386 buildonly-randconfig-005-20230907 clang
i386 buildonly-randconfig-006-20230907 clang
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-001-20230907 clang
i386 randconfig-002-20230907 clang
i386 randconfig-003-20230907 clang
i386 randconfig-004-20230907 clang
i386 randconfig-005-20230907 clang
i386 randconfig-006-20230907 clang
i386 randconfig-011-20230907 gcc
i386 randconfig-012-20230907 gcc
i386 randconfig-013-20230907 gcc
i386 randconfig-014-20230907 gcc
i386 randconfig-015-20230907 gcc
i386 randconfig-016-20230907 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch allyesconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20230907 gcc
loongarch randconfig-r023-20230907 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
m68k hp300_defconfig gcc
m68k m5275evb_defconfig gcc
m68k m5307c3_defconfig gcc
m68k randconfig-r004-20230907 gcc
m68k randconfig-r006-20230907 gcc
m68k randconfig-r024-20230907 gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
microblaze randconfig-r003-20230907 gcc
microblaze randconfig-r022-20230907 gcc
microblaze randconfig-r023-20230907 gcc
mips allmodconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
mips randconfig-r014-20230907 clang
mips randconfig-r036-20230907 gcc
mips xway_defconfig gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
nios2 randconfig-r012-20230907 gcc
nios2 randconfig-r013-20230907 gcc
openrisc alldefconfig gcc
openrisc allmodconfig gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
openrisc or1ksim_defconfig gcc
openrisc randconfig-r004-20230907 gcc
openrisc randconfig-r011-20230907 gcc
openrisc randconfig-r015-20230907 gcc
openrisc randconfig-r031-20230907 gcc
openrisc randconfig-r035-20230907 gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc randconfig-r032-20230907 gcc
parisc randconfig-r033-20230907 gcc
parisc randconfig-r034-20230907 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig gcc
powerpc cell_defconfig gcc
powerpc mpc834x_itx_defconfig gcc
powerpc ppa8548_defconfig clang
powerpc randconfig-r014-20230907 gcc
powerpc randconfig-r026-20230907 gcc
powerpc skiroot_defconfig clang
powerpc64 randconfig-r005-20230907 clang
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv allyesconfig gcc
riscv defconfig gcc
riscv nommu_k210_defconfig gcc
riscv randconfig-r011-20230907 gcc
riscv randconfig-r025-20230907 gcc
riscv randconfig-r031-20230907 clang
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allnoconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
s390 randconfig-001-20230907 gcc
s390 randconfig-r002-20230907 clang
s390 randconfig-r012-20230907 gcc
sh alldefconfig gcc
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh ap325rxa_defconfig gcc
sh apsh4a3a_defconfig gcc
sh defconfig gcc
sh randconfig-r001-20230907 gcc
sh randconfig-r022-20230907 gcc
sh se7206_defconfig gcc
sh sh7785lcr_defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc allyesconfig gcc
sparc defconfig gcc
sparc randconfig-r025-20230907 gcc
sparc randconfig-r031-20230907 gcc
sparc sparc32_defconfig gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
sparc64 randconfig-r032-20230907 gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig clang
um defconfig gcc
um i386_defconfig gcc
um randconfig-r005-20230907 gcc
um randconfig-r006-20230907 gcc
um randconfig-r026-20230907 clang
um randconfig-r036-20230907 gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 buildonly-randconfig-001-20230907 clang
x86_64 buildonly-randconfig-002-20230907 clang
x86_64 buildonly-randconfig-003-20230907 clang
x86_64 buildonly-randconfig-004-20230907 clang
x86_64 buildonly-randconfig-005-20230907 clang
x86_64 buildonly-randconfig-006-20230907 clang
x86_64 defconfig gcc
x86_64 kexec gcc
x86_64 randconfig-001-20230907 gcc
x86_64 randconfig-002-20230907 gcc
x86_64 randconfig-003-20230907 gcc
x86_64 randconfig-004-20230907 gcc
x86_64 randconfig-005-20230907 gcc
x86_64 randconfig-006-20230907 gcc
x86_64 randconfig-011-20230907 clang
x86_64 randconfig-012-20230907 clang
x86_64 randconfig-013-20230907 clang
x86_64 randconfig-014-20230907 clang
x86_64 randconfig-015-20230907 clang
x86_64 randconfig-016-20230907 clang
x86_64 randconfig-071-20230907 clang
x86_64 randconfig-072-20230907 clang
x86_64 randconfig-073-20230907 clang
x86_64 randconfig-074-20230907 clang
x86_64 randconfig-075-20230907 clang
x86_64 randconfig-076-20230907 clang
x86_64 randconfig-r036-20230907 clang
x86_64 rhel-8.3-bpf gcc
x86_64 rhel-8.3-func gcc
x86_64 rhel-8.3-kselftests gcc
x86_64 rhel-8.3-kunit gcc
x86_64 rhel-8.3-ltp gcc
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
xtensa allnoconfig gcc
xtensa allyesconfig gcc
xtensa randconfig-r013-20230907 gcc
xtensa randconfig-r021-20230907 gcc
xtensa virt_defconfig gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH RESEND] Input: xpad - Add HyperX Clutch Gladiate Support
From: HP Dev @ 2023-09-06 23:15 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, HP Dev, Chris Toledanes, Carl Ng, Max Nguyen
Add HyperX controller support to xpad_device and xpad_table.
Suggested-by: Chris Toledanes <chris.toledanes@hp.com>
Reviewed-by: Carl Ng <carl.ng@hp.com>
Signed-off-by: Max Nguyen <maxwell.nguyen@hp.com>
---
drivers/input/joystick/xpad.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index cdb193317c3b..1e377d040c43 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -130,6 +130,7 @@ static const struct xpad_device {
{ 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
{ 0x03eb, 0xff01, "Wooting One (Legacy)", 0, XTYPE_XBOX360 },
{ 0x03eb, 0xff02, "Wooting Two (Legacy)", 0, XTYPE_XBOX360 },
+ { 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },
{ 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
{ 0x044f, 0x0f03, "Thrustmaster Wheel", 0, XTYPE_XBOX },
{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
@@ -457,6 +458,7 @@ static const struct usb_device_id xpad_table[] = {
{ USB_INTERFACE_INFO('X', 'B', 0) }, /* Xbox USB-IF not-approved class */
XPAD_XBOX360_VENDOR(0x0079), /* GPD Win 2 controller */
XPAD_XBOX360_VENDOR(0x03eb), /* Wooting Keyboards (Legacy) */
+ XPAD_XBOXONE_VENDOR(0x03f0), /* HP HyperX Xbox One controllers */
XPAD_XBOX360_VENDOR(0x044f), /* Thrustmaster Xbox 360 controllers */
XPAD_XBOX360_VENDOR(0x045e), /* Microsoft Xbox 360 controllers */
XPAD_XBOXONE_VENDOR(0x045e), /* Microsoft Xbox One controllers */
--
2.39.3
^ permalink raw reply related
* Re: [PATCH v3] HID: nintendo: cleanup LED code
From: kernel test robot @ 2023-09-06 22:47 UTC (permalink / raw)
To: Martino Fontana, djogorchock, jikos, benjamin.tissoires,
linux-input, linux-kernel
Cc: oe-kbuild-all, Martino Fontana
In-Reply-To: <20230906141533.36921-1-tinozzo123@gmail.com>
Hi Martino,
kernel test robot noticed the following build warnings:
[auto build test WARNING on hid/for-next]
[also build test WARNING on linus/master v6.5 next-20230906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Martino-Fontana/HID-nintendo-cleanup-LED-code/20230906-221818
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230906141533.36921-1-tinozzo123%40gmail.com
patch subject: [PATCH v3] HID: nintendo: cleanup LED code
config: arc-randconfig-r033-20230907 (https://download.01.org/0day-ci/archive/20230907/202309070615.yjnCY1YM-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230907/202309070615.yjnCY1YM-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309070615.yjnCY1YM-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from arch/arc/include/asm/ptrace.h:11,
from arch/arc/include/asm/unaligned.h:12,
from drivers/hid/hid-nintendo.c:26:
drivers/hid/hid-nintendo.c: In function 'joycon_leds_create':
>> include/linux/compiler.h:55:23: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^~
drivers/hid/hid-nintendo.c:1954:17: note: in expansion of macro 'if'
1954 | if (ret)
| ^~
drivers/hid/hid-nintendo.c:1956:25: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
1956 | return ret;
| ^~~~~~
>> include/linux/compiler.h:55:23: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^~
drivers/hid/hid-nintendo.c:1986:17: note: in expansion of macro 'if'
1986 | if (ret)
| ^~
drivers/hid/hid-nintendo.c:1988:25: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
1988 | return ret;
| ^~~~~~
vim +/if +55 include/linux/compiler.h
2bcd521a684cc9 Steven Rostedt 2008-11-21 49
2bcd521a684cc9 Steven Rostedt 2008-11-21 50 #ifdef CONFIG_PROFILE_ALL_BRANCHES
2bcd521a684cc9 Steven Rostedt 2008-11-21 51 /*
2bcd521a684cc9 Steven Rostedt 2008-11-21 52 * "Define 'is'", Bill Clinton
2bcd521a684cc9 Steven Rostedt 2008-11-21 53 * "Define 'if'", Steven Rostedt
2bcd521a684cc9 Steven Rostedt 2008-11-21 54 */
a15fd609ad53a6 Linus Torvalds 2019-03-20 @55 #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
a15fd609ad53a6 Linus Torvalds 2019-03-20 56
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [git pull] Input updates for v6.6-rc0
From: pr-tracker-bot @ 2023-09-06 16:27 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <ZPeTYcsVQ7/M4Bue@google.com>
The pull request you sent on Tue, 5 Sep 2023 13:47:54 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.6-rc0
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/744a759492b5c57ff24a6e8aabe47b17ad8ee964
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH 2/2] hid-sony: DS3: Report analog buttons for Sixaxis
From: Roderick Colenbrander @ 2023-09-06 15:45 UTC (permalink / raw)
To: Max Staudt
Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, Vicki Pfau,
Pavel Rojtberg, Roderick Colenbrander, linux-input, linux-kernel
In-Reply-To: <ec4d07de-4944-a7ea-2b74-c4162af75b16@enpas.org>
On Tue, Aug 29, 2023 at 12:12 PM Max Staudt <max@enpas.org> wrote:
>
> On 8/27/23 00:21, Max Staudt wrote:
> > This change exposes these buttons as axes in a way that is as backwards
> > compatible and as close to the Linux gamepad spec as possible.
> >
> > [...]
> >
> > - The D-Pad as ABS_HAT0X/ABS_HAT0Y, -255 to 255
>
>
> One further idea:
>
> The DualShock 3 reports all 4 D-pad buttons separately, and hid-sony currently reports them as discrete digital buttons to userspace.
>
>
> Would it be better to do the same with the analog buttons, i.e. to report the 4 measurements as discrete axes, rather than the current patch's approach of merging them into two logical axes?
>
> Of course, this would require 4 more axes, this would not fit into any existing scheme, and since we've run out of ABS_MISC+n at this point, this could be a further reason for officially reserving a range of axes for analog buttons. Something like:
>
>
> #define ABS_BTN_SOUTH 0x40
> #define ABS_BTN_A ABS_BTN_SOUTH
> #define ABS_BTN_EAST 0x41
> #define ABS_BTN_B ABS_BTN_EAST
> #define ABS_BTN_C 0x42
> #define ABS_BTN_NORTH 0x43
> #define ABS_BTN_X ABS_BTN_NORTH
> #define ABS_BTN_WEST 0x44
> #define ABS_BTN_Y ABS_BTN_WEST
> #define ABS_BTN_Z 0x45
>
> #define ABS_BTN_DPAD_UP 0x46
> #define ABS_BTN_DPAD_DOWN 0x47
> #define ABS_BTN_DPAD_LEFT 0x48
> #define ABS_BTN_DPAD_RIGHT 0x49
>
> #define ABS_MAX 0x4f
> #define ABS_CNT (ABS_MAX+1)
>
>
>
> Max
>
Hi Max,
Sorry for the late response, but I have been on vacation and just got back.
Analog buttons are as you know, fairly common on game controllers. For
this reason, I was working on this about 5 years ago as my company had
a need for it, but the need died out. I did send a proposal at the
time to linux-input, which I encourage you to look at ('Proposal to
support pressure sensitive "analog" buttons in evdev' on linux-input).
There are some good comments in there.
The summary of the discussion and also my thoughts is not to simply
reuse existing axes, but think of things in a bigger picture. In
particular I brought the example of analog keyboards into the
discussion at the time (Wooting made one) in which ALL buttons are
analog. The landscape has probably changed and I haven't caught up.
Quickly looking it looks like Razor now has one too and there are
probably more.
The key question is what are the similarities between these analog
devices. It feels it are not 'just' axes. There might be some level of
configurability (though not all of that) for example some keyboards
seem to use it as a way to trigger digital key presses at configurable
thresholds among others. Please look at the old discussion as there
were some good suggestions there if I recall from Peter Hutterer among
others.
Thanks,
Roderick
^ permalink raw reply
* Re: [PATCH v3 1/1] backlight: hid_bl: Add VESA VCP HID backlight driver
From: Hans de Goede @ 2023-09-06 15:19 UTC (permalink / raw)
To: Julius Zint, Thomas Weißschuh
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Jiri Kosina,
Benjamin Tissoires, Helge Deller, linux-kernel, dri-devel,
linux-input, linux-fbdev
In-Reply-To: <9a5364de-28e1-1d4a-1d3a-d6dcedb7e659@zint.sh>
Hi Julius,
On 9/4/23 21:02, Julius Zint wrote:
>
>
> On Mon, 4 Sep 2023, Thomas Weißschuh wrote:
>
>> +Cc Hans who ins involved with the backlight subsystem
>>
>> Hi Julius,
>>
>> today I stumbled upon a mail from Hans [0], which explains that the
>> backlight subsystem is not actually a good fit (yet?) for external
>> displays.
>>
>> It seems a new API is in the works that would better fit, but I'm not
>> sure about the state of this API. Maybe Hans can clarify.
>>
>> This also ties back to my review question how userspace can figure out
>> to which display a backlight devices applies. So far it can not.
>>
>> [0] https://lore.kernel.org/lkml/7f2d88de-60c5-e2ff-9b22-acba35cfdfb6@redhat.com/
>>
>
> Hi Thomas,
>
> thanks for the hint. I will make sure to give this a proper read and
> see, if it fits my use case better then the current backlight subsystem.
Note the actual proposal for the new usespace API for display brightness
control is here:
https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86d2e@redhat.com/
I have finished / stabilized the backlight code refactor which I landed
in 6.1, which is a prerequisite for the above proposal. But I have not
been able to make time to actually implement the above proposal; and
I don't know when I will be able to make time for this.
> Especially since I wasnt able to properly address your other review
> comments for now. You are right that the name should align better with
> the kernel module and also, that it is possible for multiple displays to
> be attached.
>
> In its current state, this would mean that you could only control the
> backlight for the first HID device (enough for me :-).
>
> The systemd-backlight@.service uses not only the file name, but also the
> full bus path for storing/restoring backlights. I did not yet get around
> to see how the desktops handle brightness control, but since the
> systemd-backlight@.service already uses the name, its important to stay
> the same over multiple boots.
>
> I would be able to get a handle on the underlying USB device and use the
> serial to uniquely (and persistently) name the backlight. But it does
> feel hacky doing it this way.
So mutter (gnome-shell compositor library) has a similar issue when saving
monitor layouts and I can tell you beforehand that monitor serial numbers
by themselves are not unique enough. Some models just report 123456789
as serial and if you have a dual-monitor setup with 2 such monitors
and name the backlight class device <serial>-vcp-hid or something like that
you will still end up with 2 identical names.
To avoid this when saving monitor layouts mutter saves both the port
to which the monitor is attached (e.g. DP-1 DP-2) and the serialnumber
and on startup / monitor hotplug when it checks to see if it has saved
layout info for the monitor it checks the port+serialnr combination.
So what I think you should do is figure out a way to map which
VCP HID device maps to which drm-connector and then use
the connector-name + serial-nr to generate the backlight device name.
We will need the mapping the a drm-connector object anyway for
the new brightness API proposal from above.
Note this does NOT solve the fact that registering a new backlight
class device for an external monitor on a laptop will hopelessly
confuse userspace, see:
https://lore.kernel.org/lkml/7f2d88de-60c5-e2ff-9b22-acba35cfdfb6@redhat.com/
Regards,
Hans
^ permalink raw reply
* [PATCH v3] HID: nintendo: cleanup LED code
From: Martino Fontana @ 2023-09-06 14:15 UTC (permalink / raw)
To: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel
Cc: Martino Fontana
- Only turn on the nth LED, instead of all the LEDs up to n. This better
matches Nintendo consoles' behaviour, as they never turn on more than
one LED.
(Note that the behavior still consinsts in increasing the player number
every time a controller is connected, never decreasing it. It should be
as is described in https://bugzilla.kernel.org/show_bug.cgi?id=216225.
However, any implementation here would stop making sense as soon as a
non-Nintendo controller is connected, which is why I'm not bothering.)
- Split part of `joycon_home_led_brightness_set` (which is called by hid)
into `joycon_set_home_led` (which is what actually sets the LEDs), for
consistency with player LEDs.
- `joycon_player_led_brightness_set` won't try it to "determine which player
led this is" anymore: it's already looking at every LED brightness value.
- Instead of first registering the `led_classdev`, then attempting to set
the LED and unregistering the `led_classdev` if it fails, first attempt
to set the LED, then register the `led_classdev` only if it succeeds
(the class is still filled up in either case).
- If setting the player LEDs fails, still attempt setting the home LED.
(I don't know there's a third party controller where this may actually
happen, but who knows...)
- Use `JC_NUM_LEDS` where appropriate instead of 4.
- Print return codes in more places.
- Use spinlock instead of mutex for `input_num`. Copy its value to a local
variable, so that it can be unlocked immediately.
- `input_num` starts counting from 0
- Less holding of mutexes in general.
Changes for v2:
Applied suggestions, commit message explains more stuff, restored `return ret`
when `devm_led_classdev_register` fails (since all other hid drivers do this).
If setting LED fails, `hid_warn` now explicitly says "skipping registration".
Changes for v3:
Fixed missing semicolon from the resend (I have no idea why it disappeared).
Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
---
drivers/hid/hid-nintendo.c | 117 ++++++++++++++++++-------------------
1 file changed, 57 insertions(+), 60 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index a5ebe857a..ad3c1b74c 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -699,6 +699,25 @@ static int joycon_set_player_leds(struct joycon_ctlr *ctlr, u8 flash, u8 on)
return joycon_send_subcmd(ctlr, req, 1, HZ/4);
}
+static int joycon_set_home_led(struct joycon_ctlr *ctlr, enum led_brightness brightness)
+{
+ struct joycon_subcmd_request *req;
+ u8 buffer[sizeof(*req) + 5] = { 0 };
+ u8 *data;
+
+ req = (struct joycon_subcmd_request *)buffer;
+ req->subcmd_id = JC_SUBCMD_SET_HOME_LIGHT;
+ data = req->data;
+ data[0] = 0x01;
+ data[1] = brightness << 4;
+ data[2] = brightness | (brightness << 4);
+ data[3] = 0x11;
+ data[4] = 0x11;
+
+ hid_dbg(ctlr->hdev, "setting home led brightness\n");
+ return joycon_send_subcmd(ctlr, req, 5, HZ/4);
+}
+
static int joycon_request_spi_flash_read(struct joycon_ctlr *ctlr,
u32 start_addr, u8 size, u8 **reply)
{
@@ -1849,7 +1868,6 @@ static int joycon_player_led_brightness_set(struct led_classdev *led,
int val = 0;
int i;
int ret;
- int num;
ctlr = hid_get_drvdata(hdev);
if (!ctlr) {
@@ -1857,21 +1875,10 @@ static int joycon_player_led_brightness_set(struct led_classdev *led,
return -ENODEV;
}
- /* determine which player led this is */
- for (num = 0; num < JC_NUM_LEDS; num++) {
- if (&ctlr->leds[num] == led)
- break;
- }
- if (num >= JC_NUM_LEDS)
- return -EINVAL;
+ for (i = 0; i < JC_NUM_LEDS; i++)
+ val |= ctlr->leds[i].brightness << i;
mutex_lock(&ctlr->output_mutex);
- for (i = 0; i < JC_NUM_LEDS; i++) {
- if (i == num)
- val |= brightness << i;
- else
- val |= ctlr->leds[i].brightness << i;
- }
ret = joycon_set_player_leds(ctlr, 0, val);
mutex_unlock(&ctlr->output_mutex);
@@ -1884,9 +1891,6 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
struct device *dev = led->dev->parent;
struct hid_device *hdev = to_hid_device(dev);
struct joycon_ctlr *ctlr;
- struct joycon_subcmd_request *req;
- u8 buffer[sizeof(*req) + 5] = { 0 };
- u8 *data;
int ret;
ctlr = hid_get_drvdata(hdev);
@@ -1894,25 +1898,13 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
hid_err(hdev, "No controller data\n");
return -ENODEV;
}
-
- req = (struct joycon_subcmd_request *)buffer;
- req->subcmd_id = JC_SUBCMD_SET_HOME_LIGHT;
- data = req->data;
- data[0] = 0x01;
- data[1] = brightness << 4;
- data[2] = brightness | (brightness << 4);
- data[3] = 0x11;
- data[4] = 0x11;
-
- hid_dbg(hdev, "setting home led brightness\n");
mutex_lock(&ctlr->output_mutex);
- ret = joycon_send_subcmd(ctlr, req, 5, HZ/4);
+ ret = joycon_set_home_led(ctlr, brightness);
mutex_unlock(&ctlr->output_mutex);
-
return ret;
}
-static DEFINE_MUTEX(joycon_input_num_mutex);
+static DEFINE_SPINLOCK(joycon_input_num_spinlock);
static int joycon_leds_create(struct joycon_ctlr *ctlr)
{
struct hid_device *hdev = ctlr->hdev;
@@ -1920,17 +1912,16 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
const char *d_name = dev_name(dev);
struct led_classdev *led;
char *name;
- int ret = 0;
+ int ret;
int i;
- static int input_num = 1;
+ unsigned long flags;
+ int player_led_number;
+ static int input_num;
- /* Set the default controller player leds based on controller number */
- mutex_lock(&joycon_input_num_mutex);
- mutex_lock(&ctlr->output_mutex);
- ret = joycon_set_player_leds(ctlr, 0, 0xF >> (4 - input_num));
- if (ret)
- hid_warn(ctlr->hdev, "Failed to set leds; ret=%d\n", ret);
- mutex_unlock(&ctlr->output_mutex);
+ /* Set the player leds based on controller number */
+ spin_lock_irqsave(&joycon_input_num_spinlock, flags);
+ player_led_number = input_num++ % JC_NUM_LEDS;
+ spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
/* configure the player LEDs */
for (i = 0; i < JC_NUM_LEDS; i++) {
@@ -1938,31 +1929,34 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
d_name,
"green",
joycon_player_led_names[i]);
- if (!name) {
- mutex_unlock(&joycon_input_num_mutex);
+ if (!name)
return -ENOMEM;
- }
led = &ctlr->leds[i];
led->name = name;
- led->brightness = ((i + 1) <= input_num) ? 1 : 0;
+ led->brightness = (i == player_led_number) ? 1 : 0;
led->max_brightness = 1;
led->brightness_set_blocking =
joycon_player_led_brightness_set;
led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
+ }
+ mutex_lock(&ctlr->output_mutex);
+ ret = joycon_set_player_leds(ctlr, 0, 0x1 << player_led_number);
+ mutex_unlock(&ctlr->output_mutex);
+ if (ret) {
+ hid_warn(hdev, "Failed to set players LEDs, skipping registration; ret=%d\n", ret);
+ goto home_led;
+ }
+ for (i = 0; i < JC_NUM_LEDS; i++) {
+ led = &ctlr->leds[i];
ret = devm_led_classdev_register(&hdev->dev, led);
- if (ret) {
- hid_err(hdev, "Failed registering %s LED\n", led->name);
- mutex_unlock(&joycon_input_num_mutex);
+ if (ret)
+ hid_err(hdev, "Failed to register player %d LED; ret=%d\n", i + 1, ret);
return ret;
- }
}
- if (++input_num > 4)
- input_num = 1;
- mutex_unlock(&joycon_input_num_mutex);
-
+home_led:
/* configure the home LED */
if (jc_type_has_right(ctlr)) {
name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
@@ -1978,17 +1972,20 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
led->max_brightness = 0xF;
led->brightness_set_blocking = joycon_home_led_brightness_set;
led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
- ret = devm_led_classdev_register(&hdev->dev, led);
- if (ret) {
- hid_err(hdev, "Failed registering home led\n");
- return ret;
- }
+
/* Set the home LED to 0 as default state */
- ret = joycon_home_led_brightness_set(led, 0);
+ mutex_lock(&ctlr->output_mutex);
+ ret = joycon_set_home_led(ctlr, 0);
+ mutex_unlock(&ctlr->output_mutex);
if (ret) {
- hid_warn(hdev, "Failed to set home LED default, unregistering home LED");
- devm_led_classdev_unregister(&hdev->dev, led);
+ hid_warn(hdev, "Failed to set home LED, skipping registration; ret=%d\n", ret);
+ return 0;
}
+
+ ret = devm_led_classdev_register(&hdev->dev, led);
+ if (ret)
+ hid_err(hdev, "Failed to register home LED; ret=%d\n", ret);
+ return ret;
}
return 0;
--
2.41.0
^ permalink raw reply related
* Re: [PATCH v2 RESEND] HID: nintendo: cleanup LED code
From: kernel test robot @ 2023-09-06 13:48 UTC (permalink / raw)
To: Martino Fontana, djogorchock, jikos, benjamin.tissoires,
linux-input, linux-kernel
Cc: oe-kbuild-all, Martino Fontana
In-Reply-To: <20230906102831.16734-2-tinozzo123@gmail.com>
Hi Martino,
kernel test robot noticed the following build errors:
[auto build test ERROR on hid/for-next]
[also build test ERROR on linus/master v6.5 next-20230906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Martino-Fontana/HID-nintendo-cleanup-LED-code/20230906-183111
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230906102831.16734-2-tinozzo123%40gmail.com
patch subject: [PATCH v2 RESEND] HID: nintendo: cleanup LED code
config: parisc-randconfig-r011-20230906 (https://download.01.org/0day-ci/archive/20230906/202309062140.CiSKWeEO-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230906/202309062140.CiSKWeEO-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309062140.CiSKWeEO-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
drivers/hid/hid-nintendo.c: In function 'joycon_leds_create':
>> drivers/hid/hid-nintendo.c:1954:17: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
1954 | if (ret)
| ^~
drivers/hid/hid-nintendo.c:1956:25: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
1956 | return ret;
| ^~~~~~
drivers/hid/hid-nintendo.c:1986:17: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
1986 | if (ret)
| ^~
drivers/hid/hid-nintendo.c:1988:25: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
1988 | return ret
| ^~~~~~
>> drivers/hid/hid-nintendo.c:1988:35: error: expected ';' before '}' token
1988 | return ret
| ^
| ;
1989 | }
| ~
vim +1988 drivers/hid/hid-nintendo.c
1906
1907 static DEFINE_SPINLOCK(joycon_input_num_spinlock);
1908 static int joycon_leds_create(struct joycon_ctlr *ctlr)
1909 {
1910 struct hid_device *hdev = ctlr->hdev;
1911 struct device *dev = &hdev->dev;
1912 const char *d_name = dev_name(dev);
1913 struct led_classdev *led;
1914 char *name;
1915 int ret;
1916 int i;
1917 unsigned long flags;
1918 int player_led_number;
1919 static int input_num;
1920
1921 /* Set the player leds based on controller number */
1922 spin_lock_irqsave(&joycon_input_num_spinlock, flags);
1923 player_led_number = input_num++ % JC_NUM_LEDS;
1924 spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
1925
1926 /* configure the player LEDs */
1927 for (i = 0; i < JC_NUM_LEDS; i++) {
1928 name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
1929 d_name,
1930 "green",
1931 joycon_player_led_names[i]);
1932 if (!name)
1933 return -ENOMEM;
1934
1935 led = &ctlr->leds[i];
1936 led->name = name;
1937 led->brightness = (i == player_led_number) ? 1 : 0;
1938 led->max_brightness = 1;
1939 led->brightness_set_blocking =
1940 joycon_player_led_brightness_set;
1941 led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
1942 }
1943 mutex_lock(&ctlr->output_mutex);
1944 ret = joycon_set_player_leds(ctlr, 0, 0x1 << player_led_number);
1945 mutex_unlock(&ctlr->output_mutex);
1946 if (ret) {
1947 hid_warn(hdev, "Failed to set players LEDs, skipping registration; ret=%d\n", ret);
1948 goto home_led;
1949 }
1950
1951 for (i = 0; i < JC_NUM_LEDS; i++) {
1952 led = &ctlr->leds[i];
1953 ret = devm_led_classdev_register(&hdev->dev, led);
> 1954 if (ret)
1955 hid_err(hdev, "Failed to register player %d LED; ret=%d\n", i + 1, ret);
> 1956 return ret;
1957 }
1958
1959 home_led:
1960 /* configure the home LED */
1961 if (jc_type_has_right(ctlr)) {
1962 name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
1963 d_name,
1964 "blue",
1965 LED_FUNCTION_PLAYER5);
1966 if (!name)
1967 return -ENOMEM;
1968
1969 led = &ctlr->home_led;
1970 led->name = name;
1971 led->brightness = 0;
1972 led->max_brightness = 0xF;
1973 led->brightness_set_blocking = joycon_home_led_brightness_set;
1974 led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
1975
1976 /* Set the home LED to 0 as default state */
1977 mutex_lock(&ctlr->output_mutex);
1978 ret = joycon_set_home_led(ctlr, 0);
1979 mutex_unlock(&ctlr->output_mutex);
1980 if (ret) {
1981 hid_warn(hdev, "Failed to set home LED, skipping registration; ret=%d\n", ret);
1982 return 0;
1983 }
1984
1985 ret = devm_led_classdev_register(&hdev->dev, led);
1986 if (ret)
1987 hid_err(hdev, "Failed to register home LED; ret=%d\n", ret);
> 1988 return ret
1989 }
1990
1991 return 0;
1992 }
1993
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v2 RESEND] HID: nintendo: cleanup LED code
From: Martino Fontana @ 2023-09-06 10:26 UTC (permalink / raw)
To: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel
Cc: Martino Fontana
- Only turn on the nth LED, instead of all the LEDs up to n. This better
matches Nintendo consoles' behaviour, as they never turn on more than
one LED.
(Note that the behavior still consinsts in increasing the player number
every time a controller is connected, never decreasing it. It should be
as is described in https://bugzilla.kernel.org/show_bug.cgi?id=216225.
However, any implementation here would stop making sense as soon as a
non-Nintendo controller is connected, which is why I'm not bothering.)
- Split part of `joycon_home_led_brightness_set` (which is called by hid)
into `joycon_set_home_led` (which is what actually sets the LEDs), for
consistency with player LEDs.
- `joycon_player_led_brightness_set` won't try it to "determine which player
led this is" anymore: it's already looking at every LED brightness value.
- Instead of first registering the `led_classdev`, then attempting to set
the LED and unregistering the `led_classdev` if it fails, first attempt
to set the LED, then register the `led_classdev` only if it succeeds
(the class is still filled up in either case).
- If setting the player LEDs fails, still attempt setting the home LED.
(I don't know there's a third party controller where this may actually
happen, but who knows...)
- Use `JC_NUM_LEDS` where appropriate instead of 4.
- Print return codes in more places.
- Use spinlock instead of mutex for `input_num`. Copy its value to a local
variable, so that it can be unlocked immediately.
- `input_num` starts counting from 0
- Less holding of mutexes in general.
Changes for v2:
Applied suggestions, commit message explains more stuff, restored `return ret`
when `devm_led_classdev_register` fails (since all other hid drivers do this).
If setting LED fails, `hid_warn` now explicitly says "skipping registration".
Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
---
drivers/hid/hid-nintendo.c | 117 ++++++++++++++++++-------------------
1 file changed, 57 insertions(+), 60 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index a5ebe857a..dd7c7fce3 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -699,6 +699,25 @@ static int joycon_set_player_leds(struct joycon_ctlr *ctlr, u8 flash, u8 on)
return joycon_send_subcmd(ctlr, req, 1, HZ/4);
}
+static int joycon_set_home_led(struct joycon_ctlr *ctlr, enum led_brightness brightness)
+{
+ struct joycon_subcmd_request *req;
+ u8 buffer[sizeof(*req) + 5] = { 0 };
+ u8 *data;
+
+ req = (struct joycon_subcmd_request *)buffer;
+ req->subcmd_id = JC_SUBCMD_SET_HOME_LIGHT;
+ data = req->data;
+ data[0] = 0x01;
+ data[1] = brightness << 4;
+ data[2] = brightness | (brightness << 4);
+ data[3] = 0x11;
+ data[4] = 0x11;
+
+ hid_dbg(ctlr->hdev, "setting home led brightness\n");
+ return joycon_send_subcmd(ctlr, req, 5, HZ/4);
+}
+
static int joycon_request_spi_flash_read(struct joycon_ctlr *ctlr,
u32 start_addr, u8 size, u8 **reply)
{
@@ -1849,7 +1868,6 @@ static int joycon_player_led_brightness_set(struct led_classdev *led,
int val = 0;
int i;
int ret;
- int num;
ctlr = hid_get_drvdata(hdev);
if (!ctlr) {
@@ -1857,21 +1875,10 @@ static int joycon_player_led_brightness_set(struct led_classdev *led,
return -ENODEV;
}
- /* determine which player led this is */
- for (num = 0; num < JC_NUM_LEDS; num++) {
- if (&ctlr->leds[num] == led)
- break;
- }
- if (num >= JC_NUM_LEDS)
- return -EINVAL;
+ for (i = 0; i < JC_NUM_LEDS; i++)
+ val |= ctlr->leds[i].brightness << i;
mutex_lock(&ctlr->output_mutex);
- for (i = 0; i < JC_NUM_LEDS; i++) {
- if (i == num)
- val |= brightness << i;
- else
- val |= ctlr->leds[i].brightness << i;
- }
ret = joycon_set_player_leds(ctlr, 0, val);
mutex_unlock(&ctlr->output_mutex);
@@ -1884,9 +1891,6 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
struct device *dev = led->dev->parent;
struct hid_device *hdev = to_hid_device(dev);
struct joycon_ctlr *ctlr;
- struct joycon_subcmd_request *req;
- u8 buffer[sizeof(*req) + 5] = { 0 };
- u8 *data;
int ret;
ctlr = hid_get_drvdata(hdev);
@@ -1894,25 +1898,13 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
hid_err(hdev, "No controller data\n");
return -ENODEV;
}
-
- req = (struct joycon_subcmd_request *)buffer;
- req->subcmd_id = JC_SUBCMD_SET_HOME_LIGHT;
- data = req->data;
- data[0] = 0x01;
- data[1] = brightness << 4;
- data[2] = brightness | (brightness << 4);
- data[3] = 0x11;
- data[4] = 0x11;
-
- hid_dbg(hdev, "setting home led brightness\n");
mutex_lock(&ctlr->output_mutex);
- ret = joycon_send_subcmd(ctlr, req, 5, HZ/4);
+ ret = joycon_set_home_led(ctlr, brightness);
mutex_unlock(&ctlr->output_mutex);
-
return ret;
}
-static DEFINE_MUTEX(joycon_input_num_mutex);
+static DEFINE_SPINLOCK(joycon_input_num_spinlock);
static int joycon_leds_create(struct joycon_ctlr *ctlr)
{
struct hid_device *hdev = ctlr->hdev;
@@ -1920,17 +1912,16 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
const char *d_name = dev_name(dev);
struct led_classdev *led;
char *name;
- int ret = 0;
+ int ret;
int i;
- static int input_num = 1;
+ unsigned long flags;
+ int player_led_number;
+ static int input_num;
- /* Set the default controller player leds based on controller number */
- mutex_lock(&joycon_input_num_mutex);
- mutex_lock(&ctlr->output_mutex);
- ret = joycon_set_player_leds(ctlr, 0, 0xF >> (4 - input_num));
- if (ret)
- hid_warn(ctlr->hdev, "Failed to set leds; ret=%d\n", ret);
- mutex_unlock(&ctlr->output_mutex);
+ /* Set the player leds based on controller number */
+ spin_lock_irqsave(&joycon_input_num_spinlock, flags);
+ player_led_number = input_num++ % JC_NUM_LEDS;
+ spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
/* configure the player LEDs */
for (i = 0; i < JC_NUM_LEDS; i++) {
@@ -1938,31 +1929,34 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
d_name,
"green",
joycon_player_led_names[i]);
- if (!name) {
- mutex_unlock(&joycon_input_num_mutex);
+ if (!name)
return -ENOMEM;
- }
led = &ctlr->leds[i];
led->name = name;
- led->brightness = ((i + 1) <= input_num) ? 1 : 0;
+ led->brightness = (i == player_led_number) ? 1 : 0;
led->max_brightness = 1;
led->brightness_set_blocking =
joycon_player_led_brightness_set;
led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
+ }
+ mutex_lock(&ctlr->output_mutex);
+ ret = joycon_set_player_leds(ctlr, 0, 0x1 << player_led_number);
+ mutex_unlock(&ctlr->output_mutex);
+ if (ret) {
+ hid_warn(hdev, "Failed to set players LEDs, skipping registration; ret=%d\n", ret);
+ goto home_led;
+ }
+ for (i = 0; i < JC_NUM_LEDS; i++) {
+ led = &ctlr->leds[i];
ret = devm_led_classdev_register(&hdev->dev, led);
- if (ret) {
- hid_err(hdev, "Failed registering %s LED\n", led->name);
- mutex_unlock(&joycon_input_num_mutex);
+ if (ret)
+ hid_err(hdev, "Failed to register player %d LED; ret=%d\n", i + 1, ret);
return ret;
- }
}
- if (++input_num > 4)
- input_num = 1;
- mutex_unlock(&joycon_input_num_mutex);
-
+home_led:
/* configure the home LED */
if (jc_type_has_right(ctlr)) {
name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
@@ -1978,17 +1972,20 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
led->max_brightness = 0xF;
led->brightness_set_blocking = joycon_home_led_brightness_set;
led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
- ret = devm_led_classdev_register(&hdev->dev, led);
- if (ret) {
- hid_err(hdev, "Failed registering home led\n");
- return ret;
- }
+
/* Set the home LED to 0 as default state */
- ret = joycon_home_led_brightness_set(led, 0);
+ mutex_lock(&ctlr->output_mutex);
+ ret = joycon_set_home_led(ctlr, 0);
+ mutex_unlock(&ctlr->output_mutex);
if (ret) {
- hid_warn(hdev, "Failed to set home LED default, unregistering home LED");
- devm_led_classdev_unregister(&hdev->dev, led);
+ hid_warn(hdev, "Failed to set home LED, skipping registration; ret=%d\n", ret);
+ return 0;
}
+
+ ret = devm_led_classdev_register(&hdev->dev, led);
+ if (ret)
+ hid_err(hdev, "Failed to register home LED; ret=%d\n", ret);
+ return ret
}
return 0;
--
2.41.0
^ permalink raw reply related
* [PATCH v2 RESEND] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend
From: Martino Fontana @ 2023-09-06 10:20 UTC (permalink / raw)
To: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel
Cc: Martino Fontana
When suspending the computer, a Switch Pro Controller connected via USB will
lose its internal status. However, because the USB connection was technically
never lost, when resuming the computer, the driver will attempt to communicate
with the controller as if nothing happened (and fail).
Because of this, the user was forced to manually disconnect the controller
(or to press the sync button on the controller to power it off), so that it
can be re-initialized.
With this patch, the controller will be automatically re-initialized after
resuming from suspend.
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=216233
Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
---
drivers/hid/hid-nintendo.c | 178 ++++++++++++++++++++++---------------
1 file changed, 106 insertions(+), 72 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 250f5d2f8..a5ebe857a 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -2088,7 +2088,9 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
struct joycon_input_report *report;
req.subcmd_id = JC_SUBCMD_REQ_DEV_INFO;
+ mutex_lock(&ctlr->output_mutex);
ret = joycon_send_subcmd(ctlr, &req, 0, HZ);
+ mutex_unlock(&ctlr->output_mutex);
if (ret) {
hid_err(ctlr->hdev, "Failed to get joycon info; ret=%d\n", ret);
return ret;
@@ -2117,6 +2119,88 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
return 0;
}
+static int joycon_init(struct hid_device *hdev)
+{
+ struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
+ int ret = 0;
+
+ mutex_lock(&ctlr->output_mutex);
+ /* if handshake command fails, assume ble pro controller */
+ if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
+ !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
+ hid_dbg(hdev, "detected USB controller\n");
+ /* 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 err_mutex;
+ }
+ /* handshake */
+ ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
+ if (ret) {
+ hid_err(hdev, "Failed handshake; ret=%d\n", ret);
+ goto err_mutex;
+ }
+ /*
+ * Set no timeout (to keep controller in USB mode).
+ * This doesn't send a response, so ignore the timeout.
+ */
+ joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
+ } else if (jc_type_is_chrggrip(ctlr)) {
+ hid_err(hdev, "Failed charging grip handshake\n");
+ ret = -ETIMEDOUT;
+ goto err_mutex;
+ }
+
+ /* get controller calibration data, and parse it */
+ ret = joycon_request_calibration(ctlr);
+ if (ret) {
+ /*
+ * We can function with default calibration, but it may be
+ * inaccurate. Provide a warning, and continue on.
+ */
+ hid_warn(hdev, "Analog stick positions may be inaccurate\n");
+ }
+
+ /* get IMU calibration data, and parse it */
+ ret = joycon_request_imu_calibration(ctlr);
+ if (ret) {
+ /*
+ * We can function with default calibration, but it may be
+ * inaccurate. Provide a warning, and continue on.
+ */
+ hid_warn(hdev, "Unable to read IMU calibration data\n");
+ }
+
+ /* Set the reporting mode to 0x30, which is the full report mode */
+ ret = joycon_set_report_mode(ctlr);
+ if (ret) {
+ hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
+ goto err_mutex;
+ }
+
+ /* Enable rumble */
+ ret = joycon_enable_rumble(ctlr);
+ if (ret) {
+ hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
+ goto err_mutex;
+ }
+
+ /* Enable the IMU */
+ ret = joycon_enable_imu(ctlr);
+ if (ret) {
+ hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
+ goto err_mutex;
+ }
+
+ mutex_unlock(&ctlr->output_mutex);
+ return 0;
+
+err_mutex:
+ mutex_unlock(&ctlr->output_mutex);
+ return ret;
+}
+
/* Common handler for parsing inputs */
static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
int size)
@@ -2248,85 +2332,19 @@ static int nintendo_hid_probe(struct hid_device *hdev,
hid_device_io_start(hdev);
- /* Initialize the controller */
- mutex_lock(&ctlr->output_mutex);
- /* if handshake command fails, assume ble pro controller */
- if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
- !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
- hid_dbg(hdev, "detected USB controller\n");
- /* 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 err_mutex;
- }
- /* handshake */
- ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
- if (ret) {
- hid_err(hdev, "Failed handshake; ret=%d\n", ret);
- goto err_mutex;
- }
- /*
- * Set no timeout (to keep controller in USB mode).
- * This doesn't send a response, so ignore the timeout.
- */
- joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
- } else if (jc_type_is_chrggrip(ctlr)) {
- hid_err(hdev, "Failed charging grip handshake\n");
- ret = -ETIMEDOUT;
- goto err_mutex;
- }
-
- /* get controller calibration data, and parse it */
- ret = joycon_request_calibration(ctlr);
- if (ret) {
- /*
- * We can function with default calibration, but it may be
- * inaccurate. Provide a warning, and continue on.
- */
- hid_warn(hdev, "Analog stick positions may be inaccurate\n");
- }
-
- /* get IMU calibration data, and parse it */
- ret = joycon_request_imu_calibration(ctlr);
- if (ret) {
- /*
- * We can function with default calibration, but it may be
- * inaccurate. Provide a warning, and continue on.
- */
- hid_warn(hdev, "Unable to read IMU calibration data\n");
- }
-
- /* Set the reporting mode to 0x30, which is the full report mode */
- ret = joycon_set_report_mode(ctlr);
- if (ret) {
- hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
- goto err_mutex;
- }
-
- /* Enable rumble */
- ret = joycon_enable_rumble(ctlr);
- if (ret) {
- hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
- goto err_mutex;
- }
-
- /* Enable the IMU */
- ret = joycon_enable_imu(ctlr);
+ ret = joycon_init(hdev);
if (ret) {
- hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
- goto err_mutex;
+ hid_err(hdev, "Failed to initialize controller; ret=%d\n", ret);
+ goto err_close;
}
ret = joycon_read_info(ctlr);
if (ret) {
hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
ret);
- goto err_mutex;
+ goto err_close;
}
- mutex_unlock(&ctlr->output_mutex);
-
/* Initialize the leds */
ret = joycon_leds_create(ctlr);
if (ret) {
@@ -2352,8 +2370,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
hid_dbg(hdev, "probe - success\n");
return 0;
-err_mutex:
- mutex_unlock(&ctlr->output_mutex);
err_close:
hid_hw_close(hdev);
err_stop:
@@ -2383,6 +2399,20 @@ static void nintendo_hid_remove(struct hid_device *hdev)
hid_hw_stop(hdev);
}
+#ifdef CONFIG_PM
+
+static int nintendo_hid_resume(struct hid_device *hdev)
+{
+ int ret = joycon_init(hdev);
+
+ if (ret)
+ hid_err(hdev, "Failed to restore controller after resume");
+
+ return ret;
+}
+
+#endif
+
static const struct hid_device_id nintendo_hid_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
USB_DEVICE_ID_NINTENDO_PROCON) },
@@ -2404,6 +2434,10 @@ static struct hid_driver nintendo_hid_driver = {
.probe = nintendo_hid_probe,
.remove = nintendo_hid_remove,
.raw_event = nintendo_hid_event,
+
+#ifdef CONFIG_PM
+ .resume = nintendo_hid_resume,
+#endif
};
module_hid_driver(nintendo_hid_driver);
--
2.41.0
^ permalink raw reply related
* [git pull] Input updates for v6.6-rc0
From: Dmitry Torokhov @ 2023-09-05 20:47 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.6-rc0
to receive updates for the input subsystem. You will get:
- a new driver for Azoteq IQS7210A/7211A/E touch controllers
- support for Azoteq IQS7222D variant added to iqs7222 driver
- support for touch keys functionality added to Melfas MMS114 driver
- new hardware IDs added to exc3000 and Goodix drivers
- xpad driver gained support for GameSir T4 Kaleid Controller
- a fix for xpad driver to properly support some third-party
controllers that need a magic packet to start properly
- a fix for psmouse driver to more reliably switch to RMI4 mode
on devices that use native RMI4/SMbus protocol
- a quirk for i8042 for TUXEDO Gemini 17 Gen1/Clevo PD70PN laptops
Also a number of drivers have been updated to make use of devm APIs
as well as other newer APIs such as dev_err_probe(),
devm_regulator_get_enable(), and others.
Changelog:
---------
Andreas Helbech Kleist (1):
Input: exc3000 - add ACPI support for EXC80H60
Artur Weber (2):
dt-bindings: mms114: Add linux,keycodes property for touch keys
Input: mms114 - add support for touch keys
Christophe JAILLET (2):
Input: pinephone-keyboard - use devm_regulator_get_enable()
Input: adp5588-keys - use devm_regulator_get_enable()
Dmitry Torokhov (9):
Input: gameport - use IS_REACHABLE() instead of open-coding it
Input: qt2160 - tweak check for i2c adapter functionality
Input: qt2160 - switch to using threaded interrupt handler
Input: qt2160 - do not hard code interrupt trigger
Input: lm8323 - rely on device core to create kp_disable attribute
Input: tca6416-keypad - always expect proper IRQ number in i2c client
Input: tca6416-keypad - rely on I2C core to set up suspend/resume
Input: tca6416-keypad - fix interrupt enable disbalance
Input: tca6416-keypad - switch to using input core's polling features
Felix Engelhardt (1):
Input: goodix - add support for ACPI ID GDX9110
Geert Uytterhoeven (1):
Input: gpio-keys - convert to dev_err_probe()
Hans de Goede (2):
Input: novatek-nvt-ts - fix input_register_device() failure error message
Input: novatek-nvt-ts - add touchscreen model number to description
Jeff LaBundy (7):
Input: iqs7222 - configure power mode before triggering ATI
dt-bindings: input: iqs7222: Define units for slider properties
dt-bindings: input: iqs7222: Add properties for Azoteq IQS7222D
Input: iqs7222 - add support for Azoteq IQS7222D
dt-bindings: input: Add bindings for Azoteq IQS7210A/7211A/E
Input: add support for Azoteq IQS7210A/7211A/E
Input: iqs7211 - point to match data directly
Jeffery Miller (1):
Input: psmouse - add delay when deactivating for SMBus mode
Jonathan Frederick (1):
Input: xpad - add GameSir T4 Kaleid Controller support
Krzysztof Kozlowski (25):
Input: gpio_keys_polled - simplify with dev_err_probe()
Input: gpio-vibra - simplify with dev_err_probe()
Input: pwm-vibra - simplify with dev_err_probe()
Input: rotary_encoder - simplify with dev_err_probe()
Input: elan_i2c - simplify with dev_err_probe()
Input: bu21013_ts - simplify with dev_err_probe()
Input: bu21029_ts - simplify with dev_err_probe()
Input: chipone_icn8318 - simplify with dev_err_probe()
Input: cy8ctma140 - simplify with dev_err_probe()
Input: edf-ft5x06 - simplify with dev_err_probe()
Input: ektf2127 - simplify with dev_err_probe()
Input: elants_i2c - simplify with dev_err_probe()
Input: goodix - simplify with dev_err_probe()
Input: melfas_mip4 - simplify with dev_err_probe()
Input: pixcir_i2c_ts - simplify with dev_err_probe()
Input: raydium_i2c_ts - simplify with dev_err_probe()
Input: resistive-adc-touch - simplify with dev_err_probe()
rInputrrrrrrr - simplify with dev_err_probe()
Input: sis_i2c - simplify with dev_err_probe()
Input: surface3_spi - simplify with dev_err_probe()
Input: sx8643 - simplify with dev_err_probe()
Input: bcm-keypad - simplify with dev_err_probe()
Input: bu21013_ts - use local 'client->dev' variable in probe()
Input: bu21029_ts - use local 'client->dev' variable in probe()
Input: bcm-keypad - correct dev_err_probe() error
Martin Kaiser (1):
Input: tegra-kbc - use devm_platform_ioremap_resource
Mike Looijmans (2):
dt-bindings: input: exc3000: support power supply regulators
Input: exc3000 - support power supply regulators
Nathan Chancellor (1):
Input: mcs-touchkey - fix uninitialized use of error in mcs_touchkey_probe()
Niklas Schnelle (1):
Input: gameport - add ISA and HAS_IOPORT dependencies
Oleksij Rempel (1):
dt-bindings: input: touchscreen: edt-ft5x06: Add 'threshold' property
Randy Dunlap (1):
Input: cpcap-pwrbutton - remove initial kernel-doc notation
Rob Herring (1):
Input: Explicitly include correct DT includes
Roi L (1):
Input: rotary_encoder - don't double assign input->dev.parent
Ruan Jinjie (1):
Input: rpckbd - fix the return value handle for platform_get_irq()
Sam Lantinga (1):
Input: xpad - add GameSir VID for Xbox One controllers
Samuel Holland (1):
Input: da9063 - add wakeup support
Sebastian Reichel (1):
Input: cpcap-pwrbutton - replace GPLv2 boilerplate with SPDX
Vicki Pfau (1):
Input: xpad - fix support for some third-party controllers
Werner Sembach (1):
Input: i8042 - add quirk for TUXEDO Gemini 17 Gen1/Clevo PD70PN
Yangtao Li (16):
Input: bcm-keypad - convert to devm_platform_ioremap_resource()
Input: lpc32xx-keys - convert to devm_platform_ioremap_resource()
Input: nspire-keypad - use devm_platform_get_and_ioremap_resource()
Input: omap4-keyad - convert to devm_platform_ioremap_resource()
Input: opencores-kbd - convert to devm_platform_ioremap_resource()
Input: pxa27x_keypad - convert to devm_platform_ioremap_resource()
Input: sun4i-lradc-keys - convert to devm_platform_ioremap_resource()
Input: nomadik-ske-keypad - convert to use devm_* api
Input: lpc32xx_ts - convert to use devm_* api
Input: lm8333 - convert to use devm_* api
Input: amikbd - convert to use devm_* api
Input: mcs-touchkey - convert to use devm_* api
Input: qt1070 - convert to use devm_* api
Input: qt2160 - convert to use devm_* api
Input: lm8323 - convert to use devm_* api
Input: tca6416-keypad - convert to use devm_* api
Diffstat:
--------
.../devicetree/bindings/input/azoteq,iqs7222.yaml | 248 +-
.../bindings/input/touchscreen/azoteq,iqs7211.yaml | 769 ++++++
.../bindings/input/touchscreen/edt-ft5x06.yaml | 6 +
.../bindings/input/touchscreen/eeti,exc3000.yaml | 2 +
.../bindings/input/touchscreen/melfas,mms114.yaml | 5 +
drivers/input/gameport/Kconfig | 4 +-
drivers/input/gameport/gameport.c | 26 +-
drivers/input/joystick/xpad.c | 25 +
drivers/input/keyboard/adp5588-keys.c | 17 +-
drivers/input/keyboard/amikbd.c | 25 +-
drivers/input/keyboard/bcm-keypad.c | 24 +-
drivers/input/keyboard/gpio_keys.c | 21 +-
drivers/input/keyboard/gpio_keys_polled.c | 8 +-
drivers/input/keyboard/lm8323.c | 95 +-
drivers/input/keyboard/lm8333.c | 44 +-
drivers/input/keyboard/lpc32xx-keys.c | 9 +-
drivers/input/keyboard/mcs_touchkey.c | 65 +-
drivers/input/keyboard/nomadik-ske-keypad.c | 127 +-
drivers/input/keyboard/nspire-keypad.c | 3 +-
drivers/input/keyboard/omap4-keypad.c | 9 +-
drivers/input/keyboard/opencores-kbd.c | 9 +-
drivers/input/keyboard/pinephone-keyboard.c | 20 +-
drivers/input/keyboard/pxa27x_keypad.c | 9 +-
drivers/input/keyboard/qt1070.c | 46 +-
drivers/input/keyboard/qt2160.c | 130 +-
drivers/input/keyboard/sun4i-lradc-keys.c | 6 +-
drivers/input/keyboard/tca6416-keypad.c | 141 +-
drivers/input/keyboard/tegra-kbc.c | 2 +-
drivers/input/keyboard/tm2-touchkey.c | 1 -
drivers/input/misc/Kconfig | 4 +-
drivers/input/misc/cpcap-pwrbutton.c | 12 +-
drivers/input/misc/da9063_onkey.c | 9 +
drivers/input/misc/gpio-vibra.c | 22 +-
drivers/input/misc/iqs269a.c | 2 +-
drivers/input/misc/iqs626a.c | 2 +-
drivers/input/misc/iqs7222.c | 478 +++-
drivers/input/misc/mma8450.c | 2 +-
drivers/input/misc/pm8941-pwrkey.c | 1 -
drivers/input/misc/pm8xxx-vibrator.c | 1 -
drivers/input/misc/pmic8xxx-pwrkey.c | 1 -
drivers/input/misc/pwm-beeper.c | 19 +-
drivers/input/misc/pwm-vibra.c | 32 +-
drivers/input/misc/rotary_encoder.c | 9 +-
drivers/input/misc/sparcspkr.c | 3 +-
drivers/input/mouse/elan_i2c_core.c | 9 +-
drivers/input/mouse/psmouse-smbus.c | 19 +-
drivers/input/serio/apbps2.c | 2 +-
drivers/input/serio/i8042-acpipnpio.h | 7 +
drivers/input/serio/i8042-sparcio.h | 4 +-
drivers/input/serio/rpckbd.c | 8 +-
drivers/input/serio/xilinx_ps2.c | 4 +-
drivers/input/touchscreen/Kconfig | 14 +-
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/bu21013_ts.c | 72 +-
drivers/input/touchscreen/bu21029_ts.c | 51 +-
drivers/input/touchscreen/chipone_icn8318.c | 8 +-
drivers/input/touchscreen/cy8ctma140.c | 8 +-
drivers/input/touchscreen/cyttsp5.c | 2 +-
drivers/input/touchscreen/edt-ft5x06.c | 10 +-
drivers/input/touchscreen/ektf2127.c | 8 +-
drivers/input/touchscreen/elants_i2c.c | 22 +-
drivers/input/touchscreen/exc3000.c | 17 +
drivers/input/touchscreen/goodix.c | 41 +-
drivers/input/touchscreen/ili210x.c | 2 +-
drivers/input/touchscreen/iqs5xx.c | 2 +-
drivers/input/touchscreen/iqs7211.c | 2557 ++++++++++++++++++++
drivers/input/touchscreen/lpc32xx_ts.c | 98 +-
drivers/input/touchscreen/melfas_mip4.c | 9 +-
drivers/input/touchscreen/mms114.c | 89 +-
drivers/input/touchscreen/novatek-nvt-ts.c | 10 +-
drivers/input/touchscreen/pixcir_i2c_ts.c | 40 +-
drivers/input/touchscreen/raydium_i2c_ts.c | 30 +-
drivers/input/touchscreen/resistive-adc-touch.c | 8 +-
drivers/input/touchscreen/silead.c | 8 +-
drivers/input/touchscreen/sis_i2c.c | 20 +-
drivers/input/touchscreen/surface3_spi.c | 13 +-
drivers/input/touchscreen/sx8654.c | 10 +-
drivers/input/touchscreen/ti_am335x_tsc.c | 1 -
include/linux/gameport.h | 2 +-
include/linux/tca6416_keypad.h | 1 -
80 files changed, 4644 insertions(+), 1056 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/azoteq,iqs7211.yaml
create mode 100644 drivers/input/touchscreen/iqs7211.c
Thanks.
--
Dmitry
^ permalink raw reply
* Prezentacja
From: Mateusz Talaga @ 2023-09-04 7:40 UTC (permalink / raw)
To: linux-input
Dzień dobry!
Czy mógłbym przedstawić rozwiązanie, które umożliwia monitoring każdego auta w czasie rzeczywistym w tym jego pozycję, zużycie paliwa i przebieg?
Dodatkowo nasze narzędzie minimalizuje koszty utrzymania samochodów, skraca czas przejazdów, a także tworzenie planu tras czy dostaw.
Z naszej wiedzy i doświadczenia korzysta już ponad 49 tys. Klientów. Monitorujemy 809 000 pojazdów na całym świecie, co jest naszą najlepszą wizytówką.
Bardzo proszę o e-maila zwrotnego, jeśli moglibyśmy wspólnie omówić potencjał wykorzystania takiego rozwiązania w Państwa firmie.
Pozdrawiam
Mateusz Talaga
^ permalink raw reply
* Re: [PATCH v3 1/1] backlight: hid_bl: Add VESA VCP HID backlight driver
From: Julius Zint @ 2023-09-04 19:02 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Hans de Goede, Lee Jones, Daniel Thompson, Jingoo Han,
Jiri Kosina, Benjamin Tissoires, Helge Deller, linux-kernel,
dri-devel, linux-input, linux-fbdev
In-Reply-To: <f2e1ab9e-e691-42e1-a600-42744f692922@t-8ch.de>
[-- Attachment #1: Type: text/plain, Size: 1755 bytes --]
On Mon, 4 Sep 2023, Thomas Weißschuh wrote:
> +Cc Hans who ins involved with the backlight subsystem
>
> Hi Julius,
>
> today I stumbled upon a mail from Hans [0], which explains that the
> backlight subsystem is not actually a good fit (yet?) for external
> displays.
>
> It seems a new API is in the works that would better fit, but I'm not
> sure about the state of this API. Maybe Hans can clarify.
>
> This also ties back to my review question how userspace can figure out
> to which display a backlight devices applies. So far it can not.
>
> [0] https://lore.kernel.org/lkml/7f2d88de-60c5-e2ff-9b22-acba35cfdfb6@redhat.com/
>
Hi Thomas,
thanks for the hint. I will make sure to give this a proper read and
see, if it fits my use case better then the current backlight subsystem.
Especially since I wasnt able to properly address your other review
comments for now. You are right that the name should align better with
the kernel module and also, that it is possible for multiple displays to
be attached.
In its current state, this would mean that you could only control the
backlight for the first HID device (enough for me :-).
The systemd-backlight@.service uses not only the file name, but also the
full bus path for storing/restoring backlights. I did not yet get around
to see how the desktops handle brightness control, but since the
systemd-backlight@.service already uses the name, its important to stay
the same over multiple boots.
I would be able to get a handle on the underlying USB device and use the
serial to uniquely (and persistently) name the backlight. But it does
feel hacky doing it this way.
Anyways, this is where am at. Thanks again for the support and I will
try my best to come up with something better.
Julius
^ permalink raw reply
* Re: [PATCH v3 1/1] backlight: hid_bl: Add VESA VCP HID backlight driver
From: Thomas Weißschuh @ 2023-09-04 15:59 UTC (permalink / raw)
To: Julius Zint, Hans de Goede
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Jiri Kosina,
Benjamin Tissoires, Helge Deller, linux-kernel, dri-devel,
linux-input, linux-fbdev
In-Reply-To: <20230820094118.20521-2-julius@zint.sh>
+Cc Hans who ins involved with the backlight subsystem
Hi Julius,
today I stumbled upon a mail from Hans [0], which explains that the
backlight subsystem is not actually a good fit (yet?) for external
displays.
It seems a new API is in the works that would better fit, but I'm not
sure about the state of this API. Maybe Hans can clarify.
This also ties back to my review question how userspace can figure out
to which display a backlight devices applies. So far it can not.
[0] https://lore.kernel.org/lkml/7f2d88de-60c5-e2ff-9b22-acba35cfdfb6@redhat.com/
Below the original PATCH for Hans' reference.
On 2023-08-20 11:41:18+0200, Julius Zint wrote:
> The HID spec defines the following Usage IDs (p. 345 ff):
>
> - Monitor Page (0x80) -> Monitor Control (0x01)
> - VESA Virtual Controls Page (0x82) -> Brightness (0x10)
>
> Apple made use of them in their Apple Studio Display and most likely on
> other external displays (LG UltraFine 5k, Pro Display XDR).
>
> The driver will work for any HID device with a report, where the
> application matches the Monitor Control Usage ID and:
>
> 1. An Input field in this report with the Brightness Usage ID (to get
> the current brightness)
> 2. A Feature field in this report with the Brightness Usage ID (to
> set the current brightness)
>
> This driver has been developed and tested with the Apple Studio Display.
> Here is a small excerpt from the decoded HID descriptor showing the
> feature field for setting the brightness:
>
> Usage Page (Monitor VESA VCP), ; Monitor VESA VPC (82h, monitor page)
> Usage (10h, Brightness),
> Logical Minimum (400),
> Logical Maximum (60000),
> Unit (Centimeter^-2 * Candela),
> Unit Exponent (14),
> Report Size (32),
> Report Count (1),
> Feature (Variable, Null State),
>
> The full HID descriptor dump is available as a comment in the source
> code.
>
> Signed-off-by: Julius Zint <julius@zint.sh>
> ---
> drivers/video/backlight/Kconfig | 8 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/hid_bl.c | 269 +++++++++++++++++++++++++++++++
> 3 files changed, 278 insertions(+)
> create mode 100644 drivers/video/backlight/hid_bl.c
>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 51387b1ef012..b964a820956d 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -472,6 +472,14 @@ config BACKLIGHT_LED
> If you have a LCD backlight adjustable by LED class driver, say Y
> to enable this driver.
>
> +config BACKLIGHT_HID
> + tristate "VESA VCP HID Backlight Driver"
> + depends on HID
> + help
> + If you have an external display with VESA compliant HID brightness
> + controls then say Y to enable this backlight driver. Currently the
> + only supported device is the Apple Studio Display.
> +
> endif # BACKLIGHT_CLASS_DEVICE
>
> endmenu
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index f72e1c3c59e9..835f9b8772c7 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -58,3 +58,4 @@ obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
> obj-$(CONFIG_BACKLIGHT_ARCXCNN) += arcxcnn_bl.o
> obj-$(CONFIG_BACKLIGHT_RAVE_SP) += rave-sp-backlight.o
> obj-$(CONFIG_BACKLIGHT_LED) += led_bl.o
> +obj-$(CONFIG_BACKLIGHT_HID) += hid_bl.o
> diff --git a/drivers/video/backlight/hid_bl.c b/drivers/video/backlight/hid_bl.c
> new file mode 100644
> index 000000000000..b40f8f412ee2
> --- /dev/null
> +++ b/drivers/video/backlight/hid_bl.c
> @@ -0,0 +1,269 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include <linux/backlight.h>
> +
> +#define APPLE_STUDIO_DISPLAY_VENDOR_ID 0x05ac
> +#define APPLE_STUDIO_DISPLAY_PRODUCT_ID 0x1114
> +
> +#define HID_USAGE_MONITOR_CTRL 0x800001
> +#define HID_USAGE_VESA_VCP_BRIGHTNESS 0x820010
> +
> +/*
> + * Apple Studio Display HID report descriptor
> + *
> + * Usage Page (Monitor), ; USB monitor (80h, monitor page)
> + * Usage (01h),
> + * Collection (Application),
> + * Report ID (1),
> + *
> + * Usage Page (Monitor VESA VCP), ; Monitor VESA virtual control panel (82h, monitor page)
> + * Usage (10h, Brightness),
> + * Logical Minimum (400),
> + * Logical Maximum (60000),
> + * Unit (Centimeter^-2 * Candela),
> + * Unit Exponent (14),
> + * Report Size (32),
> + * Report Count (1),
> + * Feature (Variable, Null State),
> + *
> + * Usage Page (PID), ; Physical interface device (0Fh)
> + * Usage (50h),
> + * Logical Minimum (0),
> + * Logical Maximum (20000),
> + * Unit (1001h),
> + * Unit Exponent (13),
> + * Report Size (16),
> + * Feature (Variable, Null State),
> + *
> + * Usage Page (Monitor VESA VCP), ; Monitor VESA virtual control panel (82h, monitor page)
> + * Usage (10h, Brightness),
> + * Logical Minimum (400),
> + * Logical Maximum (60000),
> + * Unit (Centimeter^-2 * Candela),
> + * Unit Exponent (14),
> + * Report Size (32),
> + * Report Count (1),
> + * Input (Variable),
> + * End Collection
> + */
> +
> +struct hid_bl_data {
> + struct hid_device *hdev;
> + unsigned int min_brightness;
> + unsigned int max_brightness;
> + struct hid_field *input_field;
> + struct hid_field *feature_field;
> +};
> +
> +static struct hid_field *hid_get_usage_field(struct hid_device *hdev,
> + unsigned int report_type,
> + unsigned int application, unsigned int usage)
> +{
> + struct hid_report_enum *re = &hdev->report_enum[report_type];
> + struct hid_report *report;
> + int i, j;
> +
> + list_for_each_entry(report, &re->report_list, list) {
> + if (report->application != application)
> + continue;
> +
> + for (i = 0; i < report->maxfield; i++) {
> + struct hid_field *field = report->field[i];
> +
> + for (j = 0; j < field->maxusage; j++)
> + if (field->usage[j].hid == usage)
> + return field;
> + }
> + }
> + return NULL;
> +}
> +
> +static void hid_bl_remove(struct hid_device *hdev)
> +{
> + struct backlight_device *bl;
> + struct hid_bl_data *data;
> +
> + hid_dbg(hdev, "remove\n");
> + bl = hid_get_drvdata(hdev);
> + data = bl_get_data(bl);
> +
> + devm_backlight_device_unregister(&hdev->dev, bl);
> + hid_hw_close(hdev);
> + hid_hw_stop(hdev);
> + hid_set_drvdata(hdev, NULL);
> + devm_kfree(&hdev->dev, data);
> +}
> +
> +static int hid_bl_get_brightness_raw(struct hid_bl_data *data)
> +{
> + struct hid_field *field;
> + int result;
> +
> + field = data->input_field;
> + hid_hw_request(data->hdev, field->report, HID_REQ_GET_REPORT);
> + hid_hw_wait(data->hdev);
> + result = *field->new_value;
> + hid_dbg(data->hdev, "get brightness: %d\n", result);
> +
> + return result;
> +}
> +
> +static int hid_bl_get_brightness(struct backlight_device *bl)
> +{
> + struct hid_bl_data *data;
> + int brightness;
> +
> + data = bl_get_data(bl);
> + brightness = hid_bl_get_brightness_raw(data);
> + return brightness - data->min_brightness;
> +}
> +
> +static void hid_bl_set_brightness_raw(struct hid_bl_data *data, int brightness)
> +{
> + struct hid_field *field;
> +
> + field = data->feature_field;
> + *field->value = brightness;
> + hid_hw_request(data->hdev, field->report, HID_REQ_SET_REPORT);
> + hid_hw_wait(data->hdev);
> + hid_dbg(data->hdev, "set brightness: %d\n", brightness);
> +}
> +
> +static int hid_bl_update_status(struct backlight_device *bl)
> +{
> + struct hid_bl_data *data;
> + int brightness;
> +
> + data = bl_get_data(bl);
> + brightness = backlight_get_brightness(bl);
> + brightness += data->min_brightness;
> + hid_bl_set_brightness_raw(data, brightness);
> + return 0;
> +}
> +
> +static const struct backlight_ops hid_bl_ops = {
> + .update_status = hid_bl_update_status,
> + .get_brightness = hid_bl_get_brightness,
> +};
> +
> +static int hid_bl_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> + int ret;
> + struct hid_field *input_field;
> + struct hid_field *feature_field;
> + struct hid_bl_data *data;
> + struct backlight_properties props;
> + struct backlight_device *bl;
> +
> + hid_dbg(hdev, "probe\n");
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "parse failed: %d\n", ret);
> + return ret;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_DRIVER);
> + if (ret) {
> + hid_err(hdev, "hw start failed: %d\n", ret);
> + return ret;
> + }
> +
> + input_field = hid_get_usage_field(hdev, HID_INPUT_REPORT,
> + HID_USAGE_MONITOR_CTRL,
> + HID_USAGE_VESA_VCP_BRIGHTNESS);
> + if (input_field == NULL) {
> + ret = -ENODEV;
> + goto exit_stop;
> + }
> +
> + feature_field = hid_get_usage_field(hdev, HID_FEATURE_REPORT,
> + HID_USAGE_MONITOR_CTRL,
> + HID_USAGE_VESA_VCP_BRIGHTNESS);
> + if (feature_field == NULL) {
> + ret = -ENODEV;
> + goto exit_stop;
> + }
> +
> + if (input_field->logical_minimum != feature_field->logical_minimum) {
> + hid_warn(hdev, "minimums do not match: %d / %d\n",
> + input_field->logical_minimum,
> + feature_field->logical_minimum);
> + ret = -ENODEV;
> + goto exit_stop;
> + }
> +
> + if (input_field->logical_maximum != feature_field->logical_maximum) {
> + hid_warn(hdev, "maximums do not match: %d / %d\n",
> + input_field->logical_maximum,
> + feature_field->logical_maximum);
> + ret = -ENODEV;
> + goto exit_stop;
> + }
> +
> + hid_dbg(hdev, "Monitor VESA VCP with brightness control\n");
> +
> + ret = hid_hw_open(hdev);
> + if (ret) {
> + hid_err(hdev, "hw open failed: %d\n", ret);
> + goto exit_stop;
> + }
> +
> + data = devm_kzalloc(&hdev->dev, sizeof(*data), GFP_KERNEL);
> + if (data == NULL) {
> + ret = -ENOMEM;
> + goto exit_stop;
> + }
> + data->hdev = hdev;
> + data->min_brightness = input_field->logical_minimum;
> + data->max_brightness = input_field->logical_maximum;
> + data->input_field = input_field;
> + data->feature_field = feature_field;
> +
> + memset(&props, 0, sizeof(props));
> + props.type = BACKLIGHT_RAW;
> + props.max_brightness = data->max_brightness - data->min_brightness;
> +
> + bl = devm_backlight_device_register(&hdev->dev, "vesa_vcp",
> + &hdev->dev, data,
> + &hid_bl_ops,
> + &props);
> + if (IS_ERR(bl)) {
> + ret = PTR_ERR(bl);
> + hid_err(hdev, "failed to register backlight: %d\n", ret);
> + goto exit_free;
> + }
> +
> + hid_set_drvdata(hdev, bl);
> +
> + return 0;
> +
> +exit_free:
> + hid_hw_close(hdev);
> + devm_kfree(&hdev->dev, data);
> +
> +exit_stop:
> + hid_hw_stop(hdev);
> + return ret;
> +}
> +
> +static const struct hid_device_id hid_bl_devices[] = {
> + { HID_USB_DEVICE(APPLE_STUDIO_DISPLAY_VENDOR_ID,
> + APPLE_STUDIO_DISPLAY_PRODUCT_ID) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, hid_bl_devices);
> +
> +static struct hid_driver hid_bl_driver = {
> + .name = "hid_backlight",
> + .id_table = hid_bl_devices,
> + .probe = hid_bl_probe,
> + .remove = hid_bl_remove,
> +};
> +module_hid_driver(hid_bl_driver);
> +
> +MODULE_AUTHOR("Julius Zint <julius@zint.sh>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Backlight driver for HID devices");
> --
> 2.41.0
>
^ permalink raw reply
* Re: [PATCH 2/8] fbdev/udlfb: Use fb_ops helpers for deferred I/O
From: Javier Martinez Canillas @ 2023-09-04 15:28 UTC (permalink / raw)
To: Thomas Zimmermann, deller, daniel, sam
Cc: linux-fbdev, linux-staging, linux-hyperv, dri-devel,
Bernie Thompson, linux-input
In-Reply-To: <37906737-5ca3-7f46-af30-85a117875eea@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> Am 04.09.23 um 15:05 schrieb Javier Martinez Canillas:
>> Thomas Zimmermann <tzimmermann@suse.de> writes:
>>
>>> Generate callback functions for struct fb_ops with the fbdev macro
>>> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to
>>> the generated functions with fbdev initializer macros.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: Bernie Thompson <bernie@plugable.com>
>>> ---
>>
>> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
>>
>> [...]
>>
>>> +static void dlfb_ops_damage_range(struct fb_info *info, off_t off, size_t len)
>>> +{
>>> + struct dlfb_data *dlfb = info->par;
>>> + int start = max((int)(off / info->fix.line_length), 0);
>>> + int lines = min((u32)((len / info->fix.line_length) + 1), (u32)info->var.yres);
>>> +
>>> + dlfb_handle_damage(dlfb, 0, start, info->var.xres, lines);
>>> +}
>>> +
>>> +static void dlfb_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height)
>>> +{
>>> + struct dlfb_data *dlfb = info->par;
>>> +
>>> + dlfb_offload_damage(dlfb, x, y, width, height);
>>> +}
>>> +
>>
>> These two are very similar to the helpers you added for the smscufx driver
>> in patch #1. I guess there's room for further consolidation as follow-up ?
>
> Maybe. I had patches that take the rectangle computation from [1] and
> turn it into a helper for these USB drivers. But it's an unrelated
> change, so I dropped them from this patchset.
>
Great and yes, I meant as separate patch-set, not as a part of this one.
> Best regards
> Thomas
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH 1/8] fbdev/smscufx: Use fb_ops helpers for deferred I/O
From: Javier Martinez Canillas @ 2023-09-04 15:26 UTC (permalink / raw)
To: Thomas Zimmermann, deller, daniel, sam
Cc: Steve Glendinning, linux-fbdev, linux-staging, linux-hyperv,
dri-devel, linux-input
In-Reply-To: <b9b985e7-4f60-7c59-3121-b26b07b13b03@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> Hi Javier
>
> Am 04.09.23 um 14:59 schrieb Javier Martinez Canillas:
>> Thomas Zimmermann <tzimmermann@suse.de> writes:
>>
>> Hello Thomas,
>>
>>> Generate callback functions for struct fb_ops with the fbdev macro
>>> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to
>>> the generated functions with fbdev initializer macros.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: Steve Glendinning <steve.glendinning@shawell.net>
>>> ---
>>
>> The patch looks good to me, but I've a question below.
>>
>> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
>>
>>> drivers/video/fbdev/smscufx.c | 85 +++++++++--------------------------
>>> 1 file changed, 22 insertions(+), 63 deletions(-)
>>>
>>> diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
>>
>> [...]
>>
>>> static const struct fb_ops ufx_ops = {
>>> .owner = THIS_MODULE,
>>> - .fb_read = fb_sys_read,
>>> - .fb_write = ufx_ops_write,
>>> + __FB_DEFAULT_DEFERRED_OPS_RDWR(ufx_ops),
>>> .fb_setcolreg = ufx_ops_setcolreg,
>>> - .fb_fillrect = ufx_ops_fillrect,
>>> - .fb_copyarea = ufx_ops_copyarea,
>>> - .fb_imageblit = ufx_ops_imageblit,
>>> + __FB_DEFAULT_DEFERRED_OPS_DRAW(ufx_ops),
>>> .fb_mmap = ufx_ops_mmap,
>>
>> There are no generated functions for .fb_mmap, I wonder what's the value
>> of __FB_DEFAULT_DEFERRED_OPS_MMAP() ? Maybe just removing that macro and
>> setting .fb_mmap = fb_deferred_io_mmap instead if there's no custom mmap
>> handler would be easier to read ?
>
> At least two drivers could use __FB_DEFAULT_DEFERRED_OPS_MMAP:
> picolcd-fb and hyperv_fb. At some point, we might want to set/clear
> fb_mmap depending on some Kconfig value. Having
> __FB_DEFAULT_DEFERRED_OPS_MMAP might be helpful then.
>
Got it, thanks for the explanation.
>>
>> Alternatively, __FB_DEFAULT_DEFERRED_OPS_MMAP() could still be left but
>> not taking a __prefix argument since that is not used anyways ?
>
> The driver optionally provides mmap without deferred I/O, hence the mmap
> function. That makes no sense, as these writes to the buffer would never
> make it to the device memory. But I didn't want to remove the code
> either. So I just left the existing function as-is. Usually, the
> deferred-I/O mmap is called immediately. [1]
>
Makes sense.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH 1/8] fbdev/smscufx: Use fb_ops helpers for deferred I/O
From: Thomas Zimmermann @ 2023-09-04 14:45 UTC (permalink / raw)
To: Javier Martinez Canillas, deller, daniel, sam
Cc: Steve Glendinning, linux-fbdev, linux-staging, linux-hyperv,
dri-devel, linux-input
In-Reply-To: <b9b985e7-4f60-7c59-3121-b26b07b13b03@suse.de>
[-- Attachment #1.1: Type: text/plain, Size: 1219 bytes --]
Am 04.09.23 um 16:39 schrieb Thomas Zimmermann:
[...]
> At least two drivers could use __FB_DEFAULT_DEFERRED_OPS_MMAP:
> picolcd-fb and hyperv_fb. At some point, we might want to set/clear
Both drivers are already in this patchset.
> fb_mmap depending on some Kconfig value. Having
> __FB_DEFAULT_DEFERRED_OPS_MMAP might be helpful then.
>
>>
>> Alternatively, __FB_DEFAULT_DEFERRED_OPS_MMAP() could still be left but
>> not taking a __prefix argument since that is not used anyways ?
>
> The driver optionally provides mmap without deferred I/O, hence the mmap
> function. That makes no sense, as these writes to the buffer would never
> make it to the device memory. But I didn't want to remove the code
> either. So I just left the existing function as-is. Usually, the
> deferred-I/O mmap is called immediately. [1]
>
> Best regards
> Thomas
>
> [1]
> https://elixir.bootlin.com/linux/v6.5.1/source/drivers/video/fbdev/smscufx.c#L784
>
>>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply
* Re: [PATCH 2/8] fbdev/udlfb: Use fb_ops helpers for deferred I/O
From: Thomas Zimmermann @ 2023-09-04 14:43 UTC (permalink / raw)
To: Javier Martinez Canillas, deller, daniel, sam
Cc: linux-fbdev, linux-staging, linux-hyperv, dri-devel,
Bernie Thompson, linux-input
In-Reply-To: <874jka6qd7.fsf@minerva.mail-host-address-is-not-set>
[-- Attachment #1.1: Type: text/plain, Size: 1795 bytes --]
Am 04.09.23 um 15:05 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
>> Generate callback functions for struct fb_ops with the fbdev macro
>> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to
>> the generated functions with fbdev initializer macros.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Bernie Thompson <bernie@plugable.com>
>> ---
>
> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
>
> [...]
>
>> +static void dlfb_ops_damage_range(struct fb_info *info, off_t off, size_t len)
>> +{
>> + struct dlfb_data *dlfb = info->par;
>> + int start = max((int)(off / info->fix.line_length), 0);
>> + int lines = min((u32)((len / info->fix.line_length) + 1), (u32)info->var.yres);
>> +
>> + dlfb_handle_damage(dlfb, 0, start, info->var.xres, lines);
>> +}
>> +
>> +static void dlfb_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height)
>> +{
>> + struct dlfb_data *dlfb = info->par;
>> +
>> + dlfb_offload_damage(dlfb, x, y, width, height);
>> +}
>> +
>
> These two are very similar to the helpers you added for the smscufx driver
> in patch #1. I guess there's room for further consolidation as follow-up ?
Maybe. I had patches that take the rectangle computation from [1] and
turn it into a helper for these USB drivers. But it's an unrelated
change, so I dropped them from this patchset.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v6.5.1/source/drivers/gpu/drm/drm_fb_helper.c#L641
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply
* Re: [PATCH 1/8] fbdev/smscufx: Use fb_ops helpers for deferred I/O
From: Thomas Zimmermann @ 2023-09-04 14:39 UTC (permalink / raw)
To: Javier Martinez Canillas, deller, daniel, sam
Cc: Steve Glendinning, linux-fbdev, linux-staging, linux-hyperv,
dri-devel, linux-input
In-Reply-To: <877cp66qmp.fsf@minerva.mail-host-address-is-not-set>
[-- Attachment #1.1: Type: text/plain, Size: 2600 bytes --]
Hi Javier
Am 04.09.23 um 14:59 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
> Hello Thomas,
>
>> Generate callback functions for struct fb_ops with the fbdev macro
>> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to
>> the generated functions with fbdev initializer macros.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Steve Glendinning <steve.glendinning@shawell.net>
>> ---
>
> The patch looks good to me, but I've a question below.
>
> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
>
>> drivers/video/fbdev/smscufx.c | 85 +++++++++--------------------------
>> 1 file changed, 22 insertions(+), 63 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
>
> [...]
>
>> static const struct fb_ops ufx_ops = {
>> .owner = THIS_MODULE,
>> - .fb_read = fb_sys_read,
>> - .fb_write = ufx_ops_write,
>> + __FB_DEFAULT_DEFERRED_OPS_RDWR(ufx_ops),
>> .fb_setcolreg = ufx_ops_setcolreg,
>> - .fb_fillrect = ufx_ops_fillrect,
>> - .fb_copyarea = ufx_ops_copyarea,
>> - .fb_imageblit = ufx_ops_imageblit,
>> + __FB_DEFAULT_DEFERRED_OPS_DRAW(ufx_ops),
>> .fb_mmap = ufx_ops_mmap,
>
> There are no generated functions for .fb_mmap, I wonder what's the value
> of __FB_DEFAULT_DEFERRED_OPS_MMAP() ? Maybe just removing that macro and
> setting .fb_mmap = fb_deferred_io_mmap instead if there's no custom mmap
> handler would be easier to read ?
At least two drivers could use __FB_DEFAULT_DEFERRED_OPS_MMAP:
picolcd-fb and hyperv_fb. At some point, we might want to set/clear
fb_mmap depending on some Kconfig value. Having
__FB_DEFAULT_DEFERRED_OPS_MMAP might be helpful then.
>
> Alternatively, __FB_DEFAULT_DEFERRED_OPS_MMAP() could still be left but
> not taking a __prefix argument since that is not used anyways ?
The driver optionally provides mmap without deferred I/O, hence the mmap
function. That makes no sense, as these writes to the buffer would never
make it to the device memory. But I didn't want to remove the code
either. So I just left the existing function as-is. Usually, the
deferred-I/O mmap is called immediately. [1]
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v6.5.1/source/drivers/video/fbdev/smscufx.c#L784
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply
* Re: [PATCH 8/8] staging/fbtft: Use fb_ops helpers for deferred I/O
From: Javier Martinez Canillas @ 2023-09-04 13:28 UTC (permalink / raw)
To: Thomas Zimmermann, deller, daniel, sam
Cc: linux-fbdev, dri-devel, linux-staging, linux-hyperv, linux-input,
Thomas Zimmermann
In-Reply-To: <20230828132131.29295-9-tzimmermann@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> Generate callback functions for struct fb_ops with the fbdev macro
> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to
> the generated functions with an fbdev initializer macro.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH 7/8] staging/fbtft: Initialize fb_op struct as static const
From: Javier Martinez Canillas @ 2023-09-04 13:28 UTC (permalink / raw)
To: Thomas Zimmermann, deller, daniel, sam
Cc: linux-fbdev, dri-devel, linux-staging, linux-hyperv, linux-input,
Thomas Zimmermann
In-Reply-To: <20230828132131.29295-8-tzimmermann@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> Replace dynamic allocation of the fb_ops instance with static
> allocation. Initialize the fields at module-load time. The owner
> field changes to THIS_MODULE, as in all other fbdev drivers.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH 6/8] hid/picolcd: Use fb_ops helpers for deferred I/O
From: Javier Martinez Canillas @ 2023-09-04 13:27 UTC (permalink / raw)
To: Thomas Zimmermann, deller, daniel, sam
Cc: linux-fbdev, dri-devel, linux-staging, linux-hyperv, linux-input,
Thomas Zimmermann, Jiri Kosina, Benjamin Tissoires,
Bruno Prémont
In-Reply-To: <20230828132131.29295-7-tzimmermann@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> Generate callback functions for struct fb_ops with the fbdev macro
> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to
> the generated functions with an fbdev initializer macro.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
> ---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH 5/8] hid: Remove trailing whitespace
From: Javier Martinez Canillas @ 2023-09-04 13:24 UTC (permalink / raw)
To: Thomas Zimmermann, deller, daniel, sam
Cc: linux-fbdev, dri-devel, linux-staging, linux-hyperv, linux-input,
Thomas Zimmermann, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <20230828132131.29295-6-tzimmermann@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> Fix coding style in Kconfig. No functional changes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ 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