Linux Input/HID development
 help / color / mirror / Atom feed
* [dtor-input:for-linus] BUILD SUCCESS 2479191c30fe90c368fd6c8842e9846959466ccd
From: kernel test robot @ 2023-07-06 11:00 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: 2479191c30fe90c368fd6c8842e9846959466ccd  Input: cpcap-pwrbutton - replace GPLv2 boilerplate with SPDX

elapsed time: 721m

configs tested: 131
configs skipped: 4

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha                            allyesconfig   gcc  
alpha                               defconfig   gcc  
alpha                randconfig-r012-20230706   gcc  
arc                              allyesconfig   gcc  
arc                                 defconfig   gcc  
arc                  randconfig-r043-20230706   gcc  
arm                              allmodconfig   gcc  
arm                              allyesconfig   gcc  
arm                         at91_dt_defconfig   gcc  
arm                                 defconfig   clang
arm                                 defconfig   gcc  
arm                          exynos_defconfig   gcc  
arm                  randconfig-r046-20230706   clang
arm                         socfpga_defconfig   clang
arm                           tegra_defconfig   gcc  
arm                    vt8500_v6_v7_defconfig   clang
arm64                            allyesconfig   gcc  
arm64                               defconfig   gcc  
csky                                defconfig   gcc  
hexagon                          alldefconfig   clang
hexagon              randconfig-r002-20230706   clang
hexagon              randconfig-r013-20230706   clang
hexagon              randconfig-r041-20230706   clang
hexagon              randconfig-r045-20230706   clang
i386                             allyesconfig   gcc  
i386         buildonly-randconfig-r004-20230706   clang
i386         buildonly-randconfig-r005-20230706   clang
i386         buildonly-randconfig-r006-20230706   clang
i386                              debian-10.3   gcc  
i386                                defconfig   gcc  
i386                 randconfig-i001-20230706   clang
i386                 randconfig-i002-20230706   clang
i386                 randconfig-i003-20230706   clang
i386                 randconfig-i004-20230706   clang
i386                 randconfig-i005-20230706   clang
i386                 randconfig-i006-20230706   clang
i386                 randconfig-i011-20230706   gcc  
i386                 randconfig-i012-20230706   gcc  
i386                 randconfig-i013-20230706   gcc  
i386                 randconfig-i014-20230706   gcc  
i386                 randconfig-i015-20230706   gcc  
i386                 randconfig-i016-20230706   gcc  
loongarch                        allmodconfig   gcc  
loongarch                         allnoconfig   gcc  
loongarch                           defconfig   gcc  
loongarch            randconfig-r016-20230706   gcc  
m68k                             allmodconfig   gcc  
m68k                             allyesconfig   gcc  
m68k                                defconfig   gcc  
m68k                            q40_defconfig   gcc  
m68k                           sun3_defconfig   gcc  
microblaze           randconfig-r005-20230706   gcc  
mips                             allmodconfig   gcc  
mips                             allyesconfig   gcc  
mips                           ip32_defconfig   gcc  
mips                          malta_defconfig   clang
mips                 randconfig-r022-20230706   clang
mips                           rs90_defconfig   clang
nios2                               defconfig   gcc  
parisc                           allyesconfig   gcc  
parisc                              defconfig   gcc  
parisc               randconfig-r001-20230706   gcc  
parisc               randconfig-r021-20230706   gcc  
parisc               randconfig-r032-20230705   gcc  
parisc64                            defconfig   gcc  
powerpc                          allmodconfig   gcc  
powerpc                           allnoconfig   gcc  
powerpc                   currituck_defconfig   gcc  
powerpc                  iss476-smp_defconfig   gcc  
powerpc                   lite5200b_defconfig   clang
powerpc                     mpc83xx_defconfig   gcc  
powerpc              randconfig-r011-20230706   gcc  
powerpc              randconfig-r024-20230706   gcc  
powerpc              randconfig-r036-20230705   gcc  
powerpc                    sam440ep_defconfig   gcc  
powerpc                     tqm8541_defconfig   gcc  
riscv                            allmodconfig   gcc  
riscv                             allnoconfig   gcc  
riscv                            allyesconfig   gcc  
riscv                               defconfig   gcc  
riscv                    nommu_virt_defconfig   clang
riscv                randconfig-r014-20230706   gcc  
riscv                randconfig-r015-20230706   gcc  
riscv                randconfig-r042-20230706   gcc  
riscv                          rv32_defconfig   gcc  
s390                             allmodconfig   gcc  
s390                             allyesconfig   gcc  
s390                                defconfig   gcc  
s390                 randconfig-r004-20230706   clang
s390                 randconfig-r025-20230706   gcc  
s390                 randconfig-r031-20230705   gcc  
s390                 randconfig-r044-20230706   gcc  
sh                               allmodconfig   gcc  
sh                             espt_defconfig   gcc  
sh                           se7751_defconfig   gcc  
sh                             sh03_defconfig   gcc  
sh                   sh7724_generic_defconfig   gcc  
sh                        sh7785lcr_defconfig   gcc  
sparc                            allyesconfig   gcc  
sparc                               defconfig   gcc  
sparc64              randconfig-r035-20230705   gcc  
um                               allmodconfig   clang
um                                allnoconfig   clang
um                               allyesconfig   clang
um                                  defconfig   gcc  
um                             i386_defconfig   gcc  
um                   randconfig-r023-20230706   clang
um                   randconfig-r026-20230706   clang
um                           x86_64_defconfig   gcc  
x86_64                           allyesconfig   gcc  
x86_64       buildonly-randconfig-r001-20230706   clang
x86_64       buildonly-randconfig-r002-20230706   clang
x86_64       buildonly-randconfig-r003-20230706   clang
x86_64                              defconfig   gcc  
x86_64                                  kexec   gcc  
x86_64               randconfig-x001-20230706   gcc  
x86_64               randconfig-x002-20230706   gcc  
x86_64               randconfig-x003-20230706   gcc  
x86_64               randconfig-x004-20230706   gcc  
x86_64               randconfig-x005-20230706   gcc  
x86_64               randconfig-x006-20230706   gcc  
x86_64               randconfig-x011-20230706   clang
x86_64               randconfig-x012-20230706   clang
x86_64               randconfig-x013-20230706   clang
x86_64               randconfig-x014-20230706   clang
x86_64               randconfig-x015-20230706   clang
x86_64               randconfig-x016-20230706   clang
x86_64                          rhel-8.3-rust   clang
x86_64                               rhel-8.3   gcc  
xtensa                          iss_defconfig   gcc  
xtensa               randconfig-r006-20230706   gcc  
xtensa               randconfig-r034-20230705   gcc  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* [dtor-input:next] BUILD SUCCESS 650cda2ce25f08e8fae391b3ba6be27e7296c6a5
From: kernel test robot @ 2023-07-06 13:37 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: 650cda2ce25f08e8fae391b3ba6be27e7296c6a5  Input: gameport - add ISA and HAS_IOPORT dependencies

elapsed time: 725m

configs tested: 163
configs skipped: 9

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha                            allyesconfig   gcc  
alpha                               defconfig   gcc  
arc                              allyesconfig   gcc  
arc                                 defconfig   gcc  
arc                  randconfig-r043-20230706   gcc  
arm                              allmodconfig   gcc  
arm                              allyesconfig   gcc  
arm                          collie_defconfig   clang
arm                                 defconfig   clang
arm                                 defconfig   gcc  
arm                          exynos_defconfig   gcc  
arm                          gemini_defconfig   gcc  
arm                       imx_v4_v5_defconfig   clang
arm                         lpc18xx_defconfig   gcc  
arm                   milbeaut_m10v_defconfig   clang
arm                            mps2_defconfig   gcc  
arm                         mv78xx0_defconfig   clang
arm                  randconfig-r002-20230706   gcc  
arm                  randconfig-r014-20230706   clang
arm                  randconfig-r046-20230706   clang
arm                         s5pv210_defconfig   clang
arm                           sama7_defconfig   clang
arm                         socfpga_defconfig   clang
arm                           tegra_defconfig   gcc  
arm                    vt8500_v6_v7_defconfig   clang
arm64                            allyesconfig   gcc  
arm64                               defconfig   gcc  
arm64                randconfig-r004-20230706   clang
arm64                randconfig-r024-20230706   gcc  
arm64                randconfig-r032-20230706   clang
csky                                defconfig   gcc  
csky                 randconfig-r033-20230706   gcc  
hexagon                          alldefconfig   clang
hexagon              randconfig-r031-20230706   clang
hexagon              randconfig-r041-20230706   clang
hexagon              randconfig-r045-20230706   clang
i386                             allyesconfig   gcc  
i386         buildonly-randconfig-r004-20230706   clang
i386         buildonly-randconfig-r005-20230706   clang
i386         buildonly-randconfig-r006-20230706   clang
i386                              debian-10.3   gcc  
i386                                defconfig   gcc  
i386                 randconfig-i001-20230706   clang
i386                 randconfig-i002-20230706   clang
i386                 randconfig-i003-20230706   clang
i386                 randconfig-i004-20230706   clang
i386                 randconfig-i005-20230706   clang
i386                 randconfig-i006-20230706   clang
i386                 randconfig-i011-20230706   gcc  
i386                 randconfig-i012-20230706   gcc  
i386                 randconfig-i013-20230706   gcc  
i386                 randconfig-i014-20230706   gcc  
i386                 randconfig-i015-20230706   gcc  
i386                 randconfig-i016-20230706   gcc  
i386                 randconfig-r005-20230706   clang
i386                 randconfig-r031-20230706   clang
i386                 randconfig-r035-20230706   clang
loongarch                        allmodconfig   gcc  
loongarch                         allnoconfig   gcc  
loongarch                           defconfig   gcc  
loongarch            randconfig-r001-20230706   gcc  
loongarch            randconfig-r025-20230706   gcc  
loongarch            randconfig-r034-20230706   gcc  
m68k                             allmodconfig   gcc  
m68k                             allyesconfig   gcc  
m68k                                defconfig   gcc  
m68k                       m5249evb_defconfig   gcc  
m68k                            q40_defconfig   gcc  
m68k                 randconfig-r023-20230706   gcc  
m68k                           sun3_defconfig   gcc  
mips                             allmodconfig   gcc  
mips                             allyesconfig   gcc  
mips                          malta_defconfig   clang
mips                 randconfig-r012-20230706   clang
mips                 randconfig-r032-20230706   gcc  
mips                           rs90_defconfig   clang
nios2                               defconfig   gcc  
nios2                randconfig-r035-20230706   gcc  
openrisc             randconfig-r022-20230706   gcc  
openrisc             randconfig-r024-20230706   gcc  
parisc                           allyesconfig   gcc  
parisc                              defconfig   gcc  
parisc               randconfig-r011-20230706   gcc  
parisc64                            defconfig   gcc  
powerpc                          allmodconfig   gcc  
powerpc                           allnoconfig   gcc  
powerpc                   currituck_defconfig   gcc  
powerpc                     ep8248e_defconfig   gcc  
powerpc                          g5_defconfig   clang
powerpc                  iss476-smp_defconfig   gcc  
powerpc                   lite5200b_defconfig   clang
powerpc                     mpc5200_defconfig   clang
powerpc               mpc834x_itxgp_defconfig   clang
powerpc                     mpc83xx_defconfig   gcc  
powerpc                      pcm030_defconfig   gcc  
powerpc                      ppc64e_defconfig   clang
powerpc                    sam440ep_defconfig   gcc  
powerpc                     tqm8541_defconfig   gcc  
powerpc                 xes_mpc85xx_defconfig   clang
riscv                            allmodconfig   gcc  
riscv                             allnoconfig   gcc  
riscv                            allyesconfig   gcc  
riscv                               defconfig   gcc  
riscv                    nommu_virt_defconfig   clang
riscv                randconfig-r016-20230706   gcc  
riscv                randconfig-r021-20230706   gcc  
riscv                randconfig-r023-20230706   gcc  
riscv                randconfig-r036-20230706   clang
riscv                randconfig-r042-20230706   gcc  
riscv                          rv32_defconfig   gcc  
s390                             allmodconfig   gcc  
s390                             allyesconfig   gcc  
s390                                defconfig   gcc  
s390                 randconfig-r044-20230706   gcc  
sh                               allmodconfig   gcc  
sh                         ecovec24_defconfig   gcc  
sh                   randconfig-r001-20230706   gcc  
sh                   randconfig-r026-20230706   gcc  
sh                           se7751_defconfig   gcc  
sh                             sh03_defconfig   gcc  
sh                   sh7724_generic_defconfig   gcc  
sparc                            allyesconfig   gcc  
sparc                               defconfig   gcc  
sparc                randconfig-r012-20230706   gcc  
sparc                randconfig-r016-20230706   gcc  
sparc64              randconfig-r011-20230706   gcc  
um                               allmodconfig   clang
um                                allnoconfig   clang
um                               allyesconfig   clang
um                                  defconfig   gcc  
um                             i386_defconfig   gcc  
um                   randconfig-r002-20230706   gcc  
um                   randconfig-r025-20230706   clang
um                           x86_64_defconfig   gcc  
x86_64                           allyesconfig   gcc  
x86_64       buildonly-randconfig-r001-20230706   clang
x86_64       buildonly-randconfig-r002-20230706   clang
x86_64       buildonly-randconfig-r003-20230706   clang
x86_64                              defconfig   gcc  
x86_64                                  kexec   gcc  
x86_64               randconfig-r026-20230706   gcc  
x86_64               randconfig-r033-20230706   clang
x86_64               randconfig-r034-20230706   clang
x86_64               randconfig-r036-20230706   clang
x86_64               randconfig-x001-20230706   gcc  
x86_64               randconfig-x002-20230706   gcc  
x86_64               randconfig-x003-20230706   gcc  
x86_64               randconfig-x004-20230706   gcc  
x86_64               randconfig-x005-20230706   gcc  
x86_64               randconfig-x006-20230706   gcc  
x86_64               randconfig-x011-20230706   clang
x86_64               randconfig-x012-20230706   clang
x86_64               randconfig-x013-20230706   clang
x86_64               randconfig-x014-20230706   clang
x86_64               randconfig-x015-20230706   clang
x86_64               randconfig-x016-20230706   clang
x86_64                          rhel-8.3-rust   clang
x86_64                               rhel-8.3   gcc  
xtensa                  cadence_csp_defconfig   gcc  
xtensa                          iss_defconfig   gcc  
xtensa               randconfig-r004-20230706   gcc  
xtensa               randconfig-r015-20230706   gcc  
xtensa               randconfig-r022-20230706   gcc  
xtensa                    xip_kc705_defconfig   gcc  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH 2/2] Input: exc3000 - Support power supply regulators
From: Mike Looijmans @ 2023-07-06 15:36 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: devicetree, linux-input, Uwe Kleine-König, linux-kernel
In-Reply-To: <ZKXlYbsk+6MPfeWy@google.com>

On 05-07-2023 23:49, Dmitry Torokhov wrote:
> Hi Mike,
>
> On Mon, Jul 03, 2023 at 10:45:36AM +0200, Mike Looijmans wrote:
>> Add power supply regulator support to the exc3000 devices.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>>
>> ---
>>
>>   drivers/input/touchscreen/exc3000.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/input/touchscreen/exc3000.c b/drivers/input/touchscreen/exc3000.c
>> index 4af4c1e5d0da..3e50af8a4a2d 100644
>> --- a/drivers/input/touchscreen/exc3000.c
>> +++ b/drivers/input/touchscreen/exc3000.c
>> @@ -18,6 +18,7 @@
>>   #include <linux/interrupt.h>
>>   #include <linux/module.h>
>>   #include <linux/of.h>
>> +#include <linux/regulator/consumer.h>
>>   #include <linux/sizes.h>
>>   #include <linux/timer.h>
>>   #include <asm/unaligned.h>
>> @@ -360,6 +361,12 @@ static int exc3000_probe(struct i2c_client *client)
>>   	if (IS_ERR(data->reset))
>>   		return PTR_ERR(data->reset);
>>   
>> +	/* For proper reset sequence, enable power while reset asserted */
>> +	error = devm_regulator_get_enable_optional(&client->dev, "vdd");
>> +	if (error && error != -ENODEV)
>> +		dev_err_probe(&client->dev, error,
>> +			      "failed to request vdd regulator\n");
>
> If there is a regulator described in the firmware we should not continue
> with initializing the device if we fail to grab/enable it. Think about
> what happens if you get -EPROBE_DEFER here. You should return here.

True, good catch, I forgot the "return" here (which in all honesty was 
the reason for using dev_err_probe in the first place...).

> Also, why are you using the _optional() variant? VDD is not an optional
> for the controller. regulator_get_optional() is needed when you need to
> alter the behavior of the device/driver depending on the presence of an
> optional supply, whereas here it should work fine with a sub supply that
> will be created if you simply call devm_regulator_get_enable() and there
> is not regulator mentioned in DT/ACPI for the board.

Indeed, "optional" isn't correct here (I probably had the clock variant 
in mind), a dummy regulator is fine.

BTW, I also have a patch in store that tries to "recover" the device 
(using its reset when possible) if it isn't responding or if it causes 
I2C errors. Took several weeks uptime for this to occur spontaneously on 
our hardware, so I'll wait for this series so I can put that patch on 
top of this one.

-- 
Mike Looijmans
System Expert

TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl




^ permalink raw reply

* [PATCH] Input: xpad - add GameSir T4 Kaleid Controller support
From: Jonathan Frederick @ 2023-07-07  3:45 UTC (permalink / raw)
  To: linux-input; +Cc: dmitry.torokhov

Add VID and PID to the xpad_device table to allow driver
to use the GameSir T4 Kaleid Controller, which is
XTYPE_XBOX360 compatible in xinput mode.

Signed-off-by: Jonathan Frederick <doublej472@gmail.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 8f65f41a1fd7..9b12fa909cf5 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -366,6 +366,7 @@ static const struct xpad_device {
 	{ 0x31e3, 0x1300, "Wooting 60HE (AVR)", 0, XTYPE_XBOX360 },
 	{ 0x31e3, 0x1310, "Wooting 60HE (ARM)", 0, XTYPE_XBOX360 },
 	{ 0x3285, 0x0607, "Nacon GC-100", 0, XTYPE_XBOX360 },
+	{ 0x3537, 0x1004, "GameSir T4 Kaleid", 0, XTYPE_XBOX360 },
 	{ 0x3767, 0x0101, "Fanatec Speedster 3 Forceshock Wheel", 0, XTYPE_XBOX },
 	{ 0xffff, 0xffff, "Chinese-made Xbox Controller", 0, XTYPE_XBOX },
 	{ 0x0000, 0x0000, "Generic X-Box pad", 0, XTYPE_UNKNOWN }
@@ -498,6 +499,7 @@ static const struct usb_device_id xpad_table[] = {
 	XPAD_XBOX360_VENDOR(0x2f24),		/* GameSir Controllers */
 	XPAD_XBOX360_VENDOR(0x31e3),		/* Wooting Keyboards */
 	XPAD_XBOX360_VENDOR(0x3285),		/* Nacon GC-100 */
+	XPAD_XBOX360_VENDOR(0x3537),		/* GameSir Controllers */
 	{ }
 };
 
-- 
2.41.0


^ permalink raw reply related

* RE: [PATCH]  Input: elan_i2c - Implement inhibit/uninhibit functions.
From: Jingle.Wu @ 2023-07-07  5:56 UTC (permalink / raw)
  To: 'Dmitry Torokhov'
  Cc: linux-kernel, linux-input, phoenix, josh.chen, dave.wang
In-Reply-To: <ZJ4MVbTnBT5sM5XR@google.com>

HI Dmitry:

...
module_i2c_driver(elan_driver);

static int __init elan_init(void)
{
	return elan_create_lid_handler();
}

static void __exit elan_exit(void)
{
	if (elan_mode_wq)
		elan_destroy_lid_handler();
}

module_init(elan_init);
module_exit(elan_exit);
...

If I change to elan_create_lid_handler() at module_init, complie error as
follow.

<pre><b>./include/linux/module.h:130:49:</b> <font color="#C01C28"><b>error:
</b></font>redefinition of '<b>__inittest</b>'
  130 |         static inline initcall_t __maybe_unused <font
color="#C01C28"><b>__inittest</b></font>(void)                \
      |                                                 <font
color="#C01C28"><b>^~~~~~~~~~</b></font>
<b>drivers/input/mouse/elan_i2c_core.c:1666:1:</b> <font
color="#2AA1B3"><b>note: </b></font>in expansion of macro
'<b>module_init</b>'
 1666 | <font color="#2AA1B3"><b>module_init</b></font>(elan_init);
      | <font color="#2AA1B3"><b>^~~~~~~~~~~</b></font>
<b>./include/linux/module.h:130:49:</b> <font color="#2AA1B3"><b>note:
</b></font>previous definition of '<b>__inittest</b>' with type '<b>int
(*(void))(void)</b>'
  130 |         static inline initcall_t __maybe_unused <font
color="#2AA1B3"><b>__inittest</b></font>(void)                \
      |                                                 <font
color="#2AA1B3"><b>^~~~~~~~~~</b></font>
<b>./include/linux/device/driver.h:266:1:</b> <font color="#2AA1B3"><b>note:
</b></font>in expansion of macro '<b>module_init</b>'
  266 | <font color="#2AA1B3"><b>module_init</b></font>(__driver##_init); \
      | <font color="#2AA1B3"><b>^~~~~~~~~~~</b></font>
<b>./include/linux/i2c.h:956:9:</b> <font color="#2AA1B3"><b>note:
</b></font>in expansion of macro '<b>module_driver</b>'
  956 |         <font
color="#2AA1B3"><b>module_driver</b></font>(__i2c_driver, i2c_add_driver, \
      |         <font color="#2AA1B3"><b>^~~~~~~~~~~~~</b></font>
<b>drivers/input/mouse/elan_i2c_core.c:1653:1:</b> <font
color="#2AA1B3"><b>note: </b></font>in expansion of macro
'<b>module_i2c_driver</b>'
 1653 | <font color="#2AA1B3"><b>module_i2c_driver</b></font>(elan_driver);
      | <font color="#2AA1B3"><b>^~~~~~~~~~~~~~~~~</b></font>
<b>./include/linux/module.h:132:13:</b> <font color="#C01C28"><b>error:
</b></font>redefinition of '<b>init_module</b>'
  132 |         int <font color="#C01C28"><b>init_module</b></font>(void)
__copy(initfn)                    \
      |             <font color="#C01C28"><b>^~~~~~~~~~~</b></font>
<b>drivers/input/mouse/elan_i2c_core.c:1666:1:</b> <font
color="#2AA1B3"><b>note: </b></font>in expansion of macro
'<b>module_init</b>'
 1666 | <font color="#2AA1B3"><b>module_init</b></font>(elan_init);
      | <font color="#2AA1B3"><b>^~~~~~~~~~~</b></font>
<b>./include/linux/module.h:132:13:</b> <font color="#2AA1B3"><b>note:
</b></font>previous definition of '<b>init_module</b>' with type
'<b>int(void)</b>'
  132 |         int <font color="#2AA1B3"><b>init_module</b></font>(void)
__copy(initfn)                    \
      |             <font color="#2AA1B3"><b>^~~~~~~~~~~</b></font>
<b>./include/linux/device/driver.h:266:1:</b> <font color="#2AA1B3"><b>note:
</b></font>in expansion of macro '<b>module_init</b>'
  266 | <font color="#2AA1B3"><b>module_init</b></font>(__driver##_init); \
      | <font color="#2AA1B3"><b>^~~~~~~~~~~</b></font>
<b>./include/linux/i2c.h:956:9:</b> <font color="#2AA1B3"><b>note:
</b></font>in expansion of macro '<b>module_driver</b>'
  956 |         <font
color="#2AA1B3"><b>module_driver</b></font>(__i2c_driver, i2c_add_driver, \
      |         <font color="#2AA1B3"><b>^~~~~~~~~~~~~</b></font>
<b>drivers/input/mouse/elan_i2c_core.c:1653:1:</b> <font
color="#2AA1B3"><b>note: </b></font>in expansion of macro
'<b>module_i2c_driver</b>'
 1653 | <font color="#2AA1B3"><b>module_i2c_driver</b></font>(elan_driver);
      | <font color="#2AA1B3"><b>^~~~~~~~~~~~~~~~~</b></font>
<b>./include/linux/module.h:138:49:</b> <font color="#C01C28"><b>error:
</b></font>redefinition of '<b>__exittest</b>'
  138 |         static inline exitcall_t __maybe_unused <font
color="#C01C28"><b>__exittest</b></font>(void)                \
      |                                                 <font
color="#C01C28"><b>^~~~~~~~~~</b></font>
<b>drivers/input/mouse/elan_i2c_core.c:1667:1:</b> <font
color="#2AA1B3"><b>note: </b></font>in expansion of macro
'<b>module_exit</b>'
 1667 | <font color="#2AA1B3"><b>module_exit</b></font>(elan_exit);
      | <font color="#2AA1B3"><b>^~~~~~~~~~~</b></font>
<b>./include/linux/module.h:138:49:</b> <font color="#2AA1B3"><b>note:
</b></font>previous definition of '<b>__exittest</b>' with type '<b>void
(*(void))(void)</b>'
  138 |         static inline exitcall_t __maybe_unused <font
color="#2AA1B3"><b>__exittest</b></font>(void)                \
      |                                                 <font
color="#2AA1B3"><b>^~~~~~~~~~</b></font>
<b>./include/linux/device/driver.h:271:1:</b> <font color="#2AA1B3"><b>note:
</b></font>in expansion of macro '<b>module_exit</b>'
  271 | <font color="#2AA1B3"><b>module_exit</b></font>(__driver##_exit);
      | <font color="#2AA1B3"><b>^~~~~~~~~~~</b></font>
<b>./include/linux/i2c.h:956:9:</b> <font color="#2AA1B3"><b>note:
</b></font>in expansion of macro '<b>module_driver</b>'
  956 |         <font
color="#2AA1B3"><b>module_driver</b></font>(__i2c_driver, i2c_add_driver, \
      |         <font color="#2AA1B3"><b>^~~~~~~~~~~~~</b></font>
<b>drivers/input/mouse/elan_i2c_core.c:1653:1:</b> <font
color="#2AA1B3"><b>note: </b></font>in expansion of macro
'<b>module_i2c_driver</b>'
 1653 | <font color="#2AA1B3"><b>module_i2c_driver</b></font>(elan_driver);
      | <font color="#2AA1B3"><b>^~~~~~~~~~~~~~~~~</b></font>
<b>./include/linux/module.h:140:14:</b> <font color="#C01C28"><b>error:
</b></font>redefinition of '<b>cleanup_module</b>'
  140 |         void <font
color="#C01C28"><b>cleanup_module</b></font>(void) __copy(exitfn)
\
      |              <font color="#C01C28"><b>^~~~~~~~~~~~~~</b></font>
<b>drivers/input/mouse/elan_i2c_core.c:1667:1:</b> <font
color="#2AA1B3"><b>note: </b></font>in expansion of macro
'<b>module_exit</b>'
 1667 | <font color="#2AA1B3"><b>module_exit</b></font>(elan_exit);
      | <font color="#2AA1B3"><b>^~~~~~~~~~~</b></font>
<b>./include/linux/module.h:140:14:</b> <font color="#2AA1B3"><b>note:
</b></font>previous definition of '<b>cleanup_module</b>' with type
'<b>void(void)</b>'
  140 |         void <font
color="#2AA1B3"><b>cleanup_module</b></font>(void) __copy(exitfn)
\
      |              <font color="#2AA1B3"><b>^~~~~~~~~~~~~~</b></font>
<b>./include/linux/device/driver.h:271:1:</b> <font color="#2AA1B3"><b>note:
</b></font>in expansion of macro '<b>module_exit</b>'
  271 | <font color="#2AA1B3"><b>module_exit</b></font>(__driver##_exit);
      | <font color="#2AA1B3"><b>^~~~~~~~~~~</b></font>
<b>./include/linux/i2c.h:956:9:</b> <font color="#2AA1B3"><b>note:
</b></font>in expansion of macro '<b>module_driver</b>'
  956 |         <font
color="#2AA1B3"><b>module_driver</b></font>(__i2c_driver, i2c_add_driver, \
      |         <font color="#2AA1B3"><b>^~~~~~~~~~~~~</b></font>
<b>drivers/input/mouse/elan_i2c_core.c:1653:1:</b> <font
color="#2AA1B3"><b>note: </b></font>in expansion of macro
'<b>module_i2c_driver</b>'
 1653 | <font color="#2AA1B3"><b>module_i2c_driver</b></font>(elan_driver);
      | <font color="#2AA1B3"><b>^~~~~~~~~~~~~~~~~</b></font>
</pre>

THANJS
JINGLE
-----Original Message-----
From: Jingle.Wu [mailto:jingle.wu@emc.com.tw] 
Sent: Wednesday, July 5, 2023 2:09 PM
To: 'Dmitry Torokhov' <dmitry.torokhov@gmail.com>
Cc: 'linux-kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>;
'linux-input@vger.kernel.org' <linux-input@vger.kernel.org>;
'phoenix@emc.com.tw' <phoenix@emc.com.tw>; 'josh.chen@emc.com.tw'
<josh.chen@emc.com.tw>; 'dave.wang@emc.com.tw' <dave.wang@emc.com.tw>
Subject: RE: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit
functions.

HI Dmitry:

1.
> +static void elan_input_lid_event(struct input_handle *handle, 
> +unsigned
int type,
> +			     unsigned int code, int value) {
> +	struct elan_tp_data *data, *n;
> +
> +	if (type == EV_SW && code == SW_LID) {
> +		list_for_each_entry_safe(data, n,
&elan_devices_with_lid_handler, 
> +list) {

Why do you need the "_safe()" variant here?
-> Because I took the above link as reference.  
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3
578852/7/drivers/hid/hid-multitouch.c#394

2. 
> +	data->lid_switch = true;
> +	INIT_LIST_HEAD(&data->list);
> +	INIT_WORK(&data->lid_work, lid_work_handler);
> +	list_add_tail(&data->list, &elan_devices_with_lid_handler);

It looks like you call elan_create_lid_handler() from elan_probe() which
means it can be called several times (we should not assume there is only one
controller), I do not see it being destroyed in remove() either, so it will
break if you bind/unbind the driver.

I also not sure why you need the list of you have a handler per device.

-> If the elan_create_lid_handler() function not be put into probe(), 
-> the lid
handler would be invalid in elan private data("struct elan_tp_data *data").
Or do you have any suggestions for it? Thanks.

3.
> +	error = elan_create_lid_handler(data);
> +	if (error)
> +		dev_err(dev, "failed to create lid handler: %d\n", error);

Do we need this on _ALL_ devices with ELan controllers, or just certain
ones? If we need this on all devices how did it work before?

-> Yes, we need this on all device.
In Chromeos kernel v5.4 before, the elan_inhibit()/uninhibit function was
built and worked successfully in elan_i2c_driver.
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads
/chromeos-5.4/drivers/input/mouse/elan_i2c_core.c#353


Thanks
JINGLE

-----Original Message-----
From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
Sent: Friday, June 30, 2023 6:57 AM
To: jingle.wu <jingle.wu@emc.com.tw>
Cc: linux-kernel@vger.kernel.org; linux-input@vger.kernel.org;
phoenix@emc.com.tw; josh.chen@emc.com.tw; dave.wang@emc.com.tw
Subject: Re: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit
functions.

Hi Jingle,

On Wed, May 31, 2023 at 05:03:40PM +0800, jingle.wu wrote:
>  Add inhibit/uninhibit functions.

You need to provide justification for this change, i.e. explain why you need
this/what is currently not working or working sub-optimally.

> 
>  Signed-off-by: Jingle.wu <jingle.wu@emc.com.tw>
> ---
>  drivers/input/mouse/elan_i2c_core.c | 207
> ++++++++++++++++++++++++++++
>  1 file changed, 207 insertions(+)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c
> b/drivers/input/mouse/elan_i2c_core.c
> index 5f0d75a45c80..4ea57f4c7bd4 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -56,6 +56,7 @@ struct elan_tp_data {
>  	struct input_dev	*input;
>  	struct input_dev	*tp_input; /* trackpoint input node */
>  	struct regulator	*vcc;
> +	struct list_head list;	/* for list of devices needing input handler
*/
>  
>  	const struct elan_transport_ops *ops;
>  
> @@ -63,6 +64,11 @@ struct elan_tp_data {
>  	struct completion	fw_completion;
>  	bool			in_fw_update;
>  
> +	struct work_struct	lid_work;
> +	bool			lid_switch;
> +	int			lid_value;
> +	bool			in_inhibit;
> +
>  	struct mutex		sysfs_mutex;
>  
>  	unsigned int		max_x;
> @@ -96,6 +102,9 @@ struct elan_tp_data {
>  	u32			quirks;		/* Various quirks */
>  };
>  
> +static struct workqueue_struct *elan_mode_wq; static 
> +LIST_HEAD(elan_devices_with_lid_handler);
> +
>  static u32 elan_i2c_lookup_quirks(u16 ic_type, u16 product_id)  {
>  	static const struct {
> @@ -329,6 +338,74 @@ static int elan_initialize(struct elan_tp_data *data,
bool skip_reset)
>  	return error;
>  }
>  
> +static int elan_reactivate(struct elan_tp_data *data) {
> +	struct device *dev = &data->client->dev;
> +	int error;
> +
> +	error = elan_set_power(data, true);
> +	if (error)
> +		dev_err(dev, "failed to restore power: %d\n", error);
> +
> +	error = data->ops->sleep_control(data->client, false);
> +	if (error) {
> +		dev_err(dev,
> +			"failed to wake device up: %d\n", error);
> +		return error;
> +	}
> +
> +	return error;
> +}
> +
> +static int elan_inhibit(struct input_dev *input_dev)

I would rather you did not call it inhibit/uninhibit because this is not
what it does.

Please split the logic for recalibration from logic of powering on and off
the device. I also hope in the future firmware revisions the distinction
will be more clear (i.e. have a separate method/command to recalibrate
baseline on demand).

> +{
> +	struct elan_tp_data *data = input_get_drvdata(input_dev);
> +	struct i2c_client *client = data->client;
> +	int error;
> +
> +	dev_dbg(&client->dev, "inhibiting\n");
> +	/*
> +	 * We are taking the mutex to make sure sysfs operations are
> +	 * complete before we attempt to bring the device into low[er]
> +	 * power mode.
> +	 */
> +	error = mutex_lock_interruptible(&data->sysfs_mutex);
> +	if (error)
> +		return error;
> +
> +	disable_irq(client->irq);
> +
> +	error = elan_set_power(data, false);
> +	if (error)
> +		enable_irq(client->irq);
> +
> +	data->in_inhibit = true;
> +	mutex_unlock(&data->sysfs_mutex);
> +
> +	return error;
> +}
> +
> +static int elan_uninhibit(struct input_dev *input_dev) {
> +	struct elan_tp_data *data = input_get_drvdata(input_dev);
> +	struct i2c_client *client = data->client;
> +	int error;
> +
> +	dev_dbg(&client->dev, "uninhibiting\n");
> +	error = mutex_lock_interruptible(&data->sysfs_mutex);
> +	if (error)
> +		return error;
> +
> +	error = elan_reactivate(data);
> +	if (error == 0)
> +		enable_irq(client->irq);
> +
> +	data->in_inhibit = false;
> +	mutex_unlock(&data->sysfs_mutex);
> +
> +	return error;
> +}
> +
>  static int elan_query_device_info(struct elan_tp_data *data)  {
>  	int error;
> @@ -1187,6 +1264,124 @@ static void elan_disable_regulator(void *_data)
>  	regulator_disable(data->vcc);
>  }
>  
> +static void lid_work_handler(struct work_struct *work) {
> +	struct elan_tp_data *data = container_of(work, struct elan_tp_data,
> +					    lid_work);
> +
> +	if (data->lid_value)
> +		elan_inhibit(data->input);
> +	else
> +		elan_uninhibit(data->input);
> +
> +}
> +
> +static void elan_input_lid_event(struct input_handle *handle, unsigned
int type,
> +			     unsigned int code, int value) {
> +	struct elan_tp_data *data, *n;
> +
> +	if (type == EV_SW && code == SW_LID) {
> +		list_for_each_entry_safe(data, n,
&elan_devices_with_lid_handler,
> +list) {

Why do you need the "_safe()" variant here?

> +			data->lid_value = value;
> +			queue_work(elan_mode_wq, &data->lid_work);
> +		}
> +	}
> +
> +}
> +
> +struct elan_input_lid {
> +	struct input_handle handle;
> +};
> +
> +static int elan_input_lid_connect(struct input_handler *handler,
> +				struct input_dev *dev,
> +				const struct input_device_id *id) {
> +	struct elan_input_lid *lid;
> +	char *name;
> +	int error;
> +
> +	lid = kzalloc(sizeof(*lid), GFP_KERNEL);
> +	if (!lid)
> +		return -ENOMEM;
> +	name = kasprintf(GFP_KERNEL, "elan-i2c-lid-%s",
dev_name(&dev->dev));
> +	if (!name) {
> +		error = -ENOMEM;
> +		goto err_free_lid;
> +	}
> +	lid->handle.dev = dev;
> +	lid->handle.handler = handler;
> +	lid->handle.name = name;
> +	lid->handle.private = lid;
> +	error = input_register_handle(&lid->handle);
> +	if (error)
> +		goto err_free_name;
> +	error = input_open_device(&lid->handle);
> +	if (error)
> +		goto err_unregister_handle;
> +	return 0;
> +err_unregister_handle:
> +	input_unregister_handle(&lid->handle);
> +err_free_name:
> +	kfree(name);
> +err_free_lid:
> +	kfree(lid);
> +	return error;
> +}
> +
> +static void elan_input_lid_disconnect(struct input_handle *handle) {
> +	struct elan_input_lid *lid = handle->private;
> +
> +	input_close_device(handle);
> +	input_unregister_handle(handle);
> +	kfree(handle->name);
> +	kfree(lid);
> +}
> +
> +static const struct input_device_id elan_input_lid_ids[] = {
> +	{
> +		.flags = INPUT_DEVICE_ID_MATCH_EVBIT |
INPUT_DEVICE_ID_MATCH_SWBIT,
> +		.evbit = { BIT_MASK(EV_SW) },
> +		.swbit = { [BIT_WORD(SW_LID)] = BIT_MASK(SW_LID) },
> +	},
> +	{ },
> +};
> +
> +static struct input_handler elan_input_lid_handler = {
> +	.event =	elan_input_lid_event,
> +	.connect =	elan_input_lid_connect,
> +	.disconnect =	elan_input_lid_disconnect,
> +	.name =		"elan-i2c-lid",
> +	.id_table =	elan_input_lid_ids,
> +};
> +
> +static int elan_create_lid_handler(struct elan_tp_data *data) {
> +	int error = 0;
> +
> +	elan_mode_wq = create_singlethread_workqueue("elan-i2c-lid");
> +	if (elan_mode_wq == NULL)
> +		return -ENOMEM;
> +	error = input_register_handler(&elan_input_lid_handler);
> +	if (error)
> +		goto remove_wq;
> +
> +	data->lid_switch = true;
> +	INIT_LIST_HEAD(&data->list);
> +	INIT_WORK(&data->lid_work, lid_work_handler);
> +	list_add_tail(&data->list, &elan_devices_with_lid_handler);

It looks like you call elan_create_lid_handler() from elan_probe() which
means it can be called several times (we should not assume there is only one
controller), I do not see it being destroyed in remove() either, so it will
break if you bind/unbind the driver.

I also not sure why you need the list of you have a handler per device.

> +
> +	return 0;
> +
> +remove_wq:
> +	data->lid_switch = false;
> +	destroy_workqueue(elan_mode_wq);
> +	elan_mode_wq = NULL;
> +	return error;
> +}
> +
>  static int elan_probe(struct i2c_client *client)  {
>  	const struct elan_transport_ops *transport_ops; @@ -1325,6 +1520,10 
> @@ static int elan_probe(struct i2c_client *client)
>  		}
>  	}
>  
> +	error = elan_create_lid_handler(data);
> +	if (error)
> +		dev_err(dev, "failed to create lid handler: %d\n", error);

Do we need this on _ALL_ devices with ELan controllers, or just certain
ones? If we need this on all devices how did it work before?

> +
>  	return 0;
>  }
>  
> @@ -1334,6 +1533,10 @@ static int elan_suspend(struct device *dev)
>  	struct elan_tp_data *data = i2c_get_clientdata(client);
>  	int ret;
>  
> +	/* Wait for switch on completion */
> +	if (data->lid_switch)
> +		flush_workqueue(elan_mode_wq);
> +
>  	/*
>  	 * We are taking the mutex to make sure sysfs operations are
>  	 * complete before we attempt to bring the device into low[er] @@
> -1371,6 +1574,10 @@ static int elan_resume(struct device *dev)
>  	struct elan_tp_data *data = i2c_get_clientdata(client);
>  	int error;
>  
> +	/* Wait for switch on completion */
> +	if (data->lid_switch)
> +		flush_workqueue(elan_mode_wq);
> +
>  	if (!device_may_wakeup(dev)) {
>  		error = regulator_enable(data->vcc);
>  		if (error) {
> --
> 2.34.1
> 

Thanks.

--
Dmitry


^ permalink raw reply

* [PATCH 0/2] Updates to amd_sfh
From: Basavaraj Natikar @ 2023-07-07  6:57 UTC (permalink / raw)
  To: jikos, benjamin.tissoires, linux-input, akshata.mukundshetty
  Cc: Basavaraj Natikar

This patch series include changes for renaming the float32 variable
including the fix for shift-out-of-bounds issue.

Basavaraj Natikar (2):
  HID: amd_sfh: Rename the float32 variable
  HID: amd_sfh: Fix for shift-out-of-bounds

 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 30 ++++++++++++++-----
 1 file changed, 23 insertions(+), 7 deletions(-)

-- 
2.25.1


^ permalink raw reply

* [PATCH 1/2] HID: amd_sfh: Rename the float32 variable
From: Basavaraj Natikar @ 2023-07-07  6:57 UTC (permalink / raw)
  To: jikos, benjamin.tissoires, linux-input, akshata.mukundshetty
  Cc: Basavaraj Natikar
In-Reply-To: <20230707065722.9036-1-Basavaraj.Natikar@amd.com>

As float32 is also used in other places as a data type, it is necessary
to rename the float32 variable in order to avoid confusion.

Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
---
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
index 6f0d332ccf51..c81d20cd3081 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
@@ -132,13 +132,13 @@ static void get_common_inputs(struct common_input_property *common, int report_i
 	common->event_type = HID_USAGE_SENSOR_EVENT_DATA_UPDATED_ENUM;
 }
 
-static int float_to_int(u32 float32)
+static int float_to_int(u32 flt32_val)
 {
 	int fraction, shift, mantissa, sign, exp, zeropre;
 
-	mantissa = float32 & GENMASK(22, 0);
-	sign = (float32 & BIT(31)) ? -1 : 1;
-	exp = (float32 & ~BIT(31)) >> 23;
+	mantissa = flt32_val & GENMASK(22, 0);
+	sign = (flt32_val & BIT(31)) ? -1 : 1;
+	exp = (flt32_val & ~BIT(31)) >> 23;
 
 	if (!exp && !mantissa)
 		return 0;
@@ -151,10 +151,10 @@ static int float_to_int(u32 float32)
 	}
 
 	shift = 23 - exp;
-	float32 = BIT(exp) + (mantissa >> shift);
+	flt32_val = BIT(exp) + (mantissa >> shift);
 	fraction = mantissa & GENMASK(shift - 1, 0);
 
-	return (((fraction * 100) >> shift) >= 50) ? sign * (float32 + 1) : sign * float32;
+	return (((fraction * 100) >> shift) >= 50) ? sign * (flt32_val + 1) : sign * flt32_val;
 }
 
 static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
-- 
2.25.1


^ permalink raw reply related

* [PATCH 2/2] HID: amd_sfh: Fix for shift-out-of-bounds
From: Basavaraj Natikar @ 2023-07-07  6:57 UTC (permalink / raw)
  To: jikos, benjamin.tissoires, linux-input, akshata.mukundshetty
  Cc: Basavaraj Natikar
In-Reply-To: <20230707065722.9036-1-Basavaraj.Natikar@amd.com>

Shift operation of 'exp' and 'shift' variables exceeds the maximum number
of shift values in the u32 range leading to UBSAN shift-out-of-bounds.

...
[    6.120512] UBSAN: shift-out-of-bounds in drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c:149:50
[    6.120598] shift exponent 104 is too large for 64-bit type 'long unsigned int'
[    6.120659] CPU: 4 PID: 96 Comm: kworker/4:1 Not tainted 6.4.0amd_1-next-20230519-dirty #10
[    6.120665] Hardware name: AMD Birman-PHX/Birman-PHX, BIOS SFH_with_HPD_SEN.FD 04/05/2023
[    6.120667] Workqueue: events amd_sfh_work_buffer [amd_sfh]
[    6.120687] Call Trace:
[    6.120690]  <TASK>
[    6.120694]  dump_stack_lvl+0x48/0x70
[    6.120704]  dump_stack+0x10/0x20
[    6.120707]  ubsan_epilogue+0x9/0x40
[    6.120716]  __ubsan_handle_shift_out_of_bounds+0x10f/0x170
[    6.120720]  ? psi_group_change+0x25f/0x4b0
[    6.120729]  float_to_int.cold+0x18/0xba [amd_sfh]
[    6.120739]  get_input_rep+0x57/0x340 [amd_sfh]
[    6.120748]  ? __schedule+0xba7/0x1b60
[    6.120756]  ? __pfx_get_input_rep+0x10/0x10 [amd_sfh]
[    6.120764]  amd_sfh_work_buffer+0x91/0x180 [amd_sfh]
[    6.120772]  process_one_work+0x229/0x430
[    6.120780]  worker_thread+0x4a/0x3c0
[    6.120784]  ? __pfx_worker_thread+0x10/0x10
[    6.120788]  kthread+0xf7/0x130
[    6.120792]  ? __pfx_kthread+0x10/0x10
[    6.120795]  ret_from_fork+0x29/0x50
[    6.120804]  </TASK>
...

Fix this by adding the condition to validate shift ranges.

Fixes: 93ce5e0231d7 ("HID: amd_sfh: Implement SFH1.1 functionality")
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
---
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
index c81d20cd3081..06bdcf072d10 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
@@ -143,16 +143,32 @@ static int float_to_int(u32 flt32_val)
 	if (!exp && !mantissa)
 		return 0;
 
+	/*
+	 * Calculate the exponent and fraction part of floating
+	 * point representation.
+	 */
 	exp -= 127;
 	if (exp < 0) {
 		exp = -exp;
+		if (exp >= BITS_PER_TYPE(u32))
+			return 0;
 		zeropre = (((BIT(23) + mantissa) * 100) >> 23) >> exp;
 		return zeropre >= 50 ? sign : 0;
 	}
 
 	shift = 23 - exp;
-	flt32_val = BIT(exp) + (mantissa >> shift);
-	fraction = mantissa & GENMASK(shift - 1, 0);
+	if (abs(shift) >= BITS_PER_TYPE(u32))
+		return 0;
+
+	if (shift < 0) {
+		shift = -shift;
+		flt32_val = BIT(exp) + (mantissa << shift);
+		shift = 0;
+	} else {
+		flt32_val = BIT(exp) + (mantissa >> shift);
+	}
+
+	fraction = (shift == 0) ? 0 : mantissa & GENMASK(shift - 1, 0);
 
 	return (((fraction * 100) >> shift) >= 50) ? sign * (flt32_val + 1) : sign * flt32_val;
 }
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH] HID: amd_sfh: Increase bitwidth to avoid shift-out-of-bounds
From: Basavaraj Natikar @ 2023-07-07  7:36 UTC (permalink / raw)
  To: Kai-Heng Feng, basavaraj.natikar
  Cc: mario.limonciello, Jiri Kosina, Benjamin Tissoires, linux-input,
	linux-kernel
In-Reply-To: <20230704081724.2308195-1-kai.heng.feng@canonical.com>


On 7/4/2023 1:47 PM, Kai-Heng Feng wrote:
> UBSAN complains shift-out-of-bounds on amd_sfh:
> [    7.593412] input: SYNA3105:00 06CB:CEA3 Mouse as /devices/platform/AMDI0010:00/i2c-0/i2c-SYNA3105:00/0018:06CB:CEA3.0001/input/input11
> [    7.593541] input: SYNA3105:00 06CB:CEA3 Touchpad as /devices/platform/AMDI0010:00/i2c-0/i2c-SYNA3105:00/0018:06CB:CEA3.0001/input/input12
> [    7.593625] hid-multitouch 0018:06CB:CEA3.0001: input,hidraw0: I2C HID v1.00 Mouse [SYNA3105:00 06CB:CEA3] on i2c-SYNA3105:00
> [    7.664537] ================================================================================
> [    7.664540] UBSAN: shift-out-of-bounds in drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c:149:50
> [    7.664544] shift exponent 103 is too large for 64-bit type 'long unsigned int'
> [    7.664547] CPU: 5 PID: 124 Comm: kworker/5:1 Tainted: G        W          6.4.0-next-20230703 #1
> [    7.664549] Hardware name: HP HP EliteBook 835 13 inch G10 Notebook PC/8C10, BIOS V83 Ver. 01.01.09 06/05/2023
> [    7.664551] Workqueue: events amd_sfh_work_buffer [amd_sfh]
> [    7.664562] Call Trace:
> [    7.664564]  <TASK>
> [    7.664567]  dump_stack_lvl+0x48/0x70
> [    7.664576]  dump_stack+0x10/0x20
> [    7.664578]  __ubsan_handle_shift_out_of_bounds+0x156/0x310
> [    7.664584]  ? sched_clock_noinstr+0x9/0x10
> [    7.664589]  ? sched_clock_cpu+0x12/0x1d0
> [    7.664594]  ? raw_spin_rq_lock_nested+0x1e/0xa0
> [    7.664597]  ? psi_group_change+0x237/0x520
> [    7.664600]  float_to_int.cold+0x18/0xcf [amd_sfh]
> [    7.664606]  ? __pfx_get_input_rep+0x10/0x10 [amd_sfh]
> [    7.664611]  get_input_rep+0x241/0x340 [amd_sfh]
> [    7.664617]  amd_sfh_work_buffer+0x91/0x190 [amd_sfh]
> [    7.664622]  process_one_work+0x229/0x450
> [    7.664627]  worker_thread+0x50/0x3f0
> [    7.664629]  ? __pfx_worker_thread+0x10/0x10
> [    7.664632]  kthread+0xf4/0x130
> [    7.664635]  ? __pfx_kthread+0x10/0x10
> [    7.664638]  ret_from_fork+0x29/0x50
> [    7.664644]  </TASK>
> [    7.664652] ================================================================================
>
> So increase the variable bitwidth to solve the issue.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> index 6f0d332ccf51..550639ef8bfe 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> @@ -134,7 +134,8 @@ static void get_common_inputs(struct common_input_property *common, int report_i
>  
>  static int float_to_int(u32 float32)
>  {
> -	int fraction, shift, mantissa, sign, exp, zeropre;
> +	int fraction, shift, sign, exp, zeropre;
> +	s128 mantissa;
>  
>  	mantissa = float32 & GENMASK(22, 0);
>  	sign = (float32 & BIT(31)) ? -1 : 1;

Could you please check below patch which covers more scenario's and corner cases
for UBSAN: shift-out-of-bounds.
https://lore.kernel.org/all/20230707065722.9036-1-Basavaraj.Natikar@amd.com/T/#m67e0b7aa6a288ea074b3bfa61e7f085f7e036172

Thanks,
--
Basavaraj


^ permalink raw reply

* [syzbot] Monthly input report (Jul 2023)
From: syzbot @ 2023-07-07  8:54 UTC (permalink / raw)
  To: linux-input, linux-kernel, syzkaller-bugs

Hello input maintainers/developers,

This is a 31-day syzbot report for the input subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/input

During the period, 2 new issues were detected and 0 were fixed.
In total, 11 issues are still open and 48 have been fixed so far.

Some of the still happening issues:

Ref Crashes Repro Title
<1> 2737    Yes   WARNING in input_mt_init_slots
                  https://syzkaller.appspot.com/bug?extid=0122fa359a69694395d5
<2> 936     Yes   WARNING in implement
                  https://syzkaller.appspot.com/bug?extid=38e7237add3712479d65
<3> 102     Yes   general protection fault in hidraw_release
                  https://syzkaller.appspot.com/bug?extid=953a33deaf38c66a915e
<4> 5       Yes   INFO: task hung in uhid_char_release
                  https://syzkaller.appspot.com/bug?extid=8fe2d362af0e1cba8735
<5> 3       No    KASAN: slab-use-after-free Read in remove_wait_queue
                  https://syzkaller.appspot.com/bug?extid=e0b9084463edf54b83c4

---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders

To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem

You may send multiple commands in a single email message.

^ permalink raw reply

* Re: amd_sfh driver causes kernel oops during boot
From: Linux regression tracking #update (Thorsten Leemhuis) @ 2023-07-07  9:37 UTC (permalink / raw)
  To: Haochen Tong, stable
  Cc: regressions, linux-input, Basavaraj Natikar, Mario Limonciello
In-Reply-To: <f40e3897-76f1-2cd0-2d83-e48d87130eab@hexchain.org>

[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]

On 23.05.23 19:27, Haochen Tong wrote:
> 
> Since kernel 6.3.0 (and also 6.4rc3), on a ThinkPad Z13 system with Arch
> Linux, I've noticed that the amd_sfh driver spews a lot of stack traces
> during boot. Sometimes it is an oops:

For the record:

#regzbot resolve: fixed in newer firmware and mainline post-6.4;
backport not planned, as bug unlikely to repeat, but possible when needed
#regzbot ignore-activity

For details see Mario's explanation here (thx for it, btw):
https://lore.kernel.org/all/89ea9fb7-9026-ccb6-ad88-50e1c28b4474@amd.com/

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.


^ permalink raw reply

* [PATCH v2 RESEND] HID: hid-google-stadiaff: add support for Stadia force feedback
From: Fabio Baltieri @ 2023-07-07 10:40 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, linux-input, linux-kernel, Fabio Baltieri

Add a hid-stadiaff module to support rumble based force feedback on the
Google Stadia controller. This works using the HID output endpoint
exposed on both the USB and BLE interface.

Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
---

Hi, this adds rumble support to the stadia controller using the input
interface. Up to now this has only been implemented at application level
using hidraw:

https://source.chromium.org/chromium/chromium/src/+/main:device/gamepad/hid_haptic_gamepad.cc

Tested with fftest, works both over USB and BLE.

Changes from v1:
- renamed the module to hid-google-stadiaff.c
- added locking for passing the state to the worker code
- added a module removed check to prevent the work from rescheduling

 drivers/hid/Kconfig               |   7 ++
 drivers/hid/Makefile              |   1 +
 drivers/hid/hid-google-stadiaff.c | 153 ++++++++++++++++++++++++++++++
 drivers/hid/hid-ids.h             |   1 +
 4 files changed, 162 insertions(+)
 create mode 100644 drivers/hid/hid-google-stadiaff.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 82f64fb31fda..f4f75d8a28ac 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -412,6 +412,13 @@ config HID_GOOGLE_HAMMER
 	help
 	Say Y here if you have a Google Hammer device.
 
+config HID_GOOGLE_STADIA_FF
+	tristate "Google Stadia force feedback"
+	select INPUT_FF_MEMLESS
+	help
+	Say Y here if you want to enable force feedback support for the Google
+	Stadia controller.
+
 config HID_VIVALDI
 	tristate "Vivaldi Keyboard"
 	select HID_VIVALDI_COMMON
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 5d37cacbde33..18e9a3afecab 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_HID_GFRM)		+= hid-gfrm.o
 obj-$(CONFIG_HID_GLORIOUS)  += hid-glorious.o
 obj-$(CONFIG_HID_VIVALDI_COMMON) += hid-vivaldi-common.o
 obj-$(CONFIG_HID_GOOGLE_HAMMER)	+= hid-google-hammer.o
+obj-$(CONFIG_HID_GOOGLE_STADIA_FF)	+= hid-google-stadiaff.o
 obj-$(CONFIG_HID_VIVALDI)	+= hid-vivaldi.o
 obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
 obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
diff --git a/drivers/hid/hid-google-stadiaff.c b/drivers/hid/hid-google-stadiaff.c
new file mode 100644
index 000000000000..2628099e802c
--- /dev/null
+++ b/drivers/hid/hid-google-stadiaff.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Stadia controller rumble support.
+ *
+ * Copyright 2023 Google LLC
+ */
+
+#include <linux/hid.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+#define STADIA_FF_REPORT_ID 5
+
+struct stadiaff_device {
+	struct hid_device *hid;
+	struct hid_report *report;
+	spinlock_t lock;
+	bool removed;
+	uint16_t strong_magnitude;
+	uint16_t weak_magnitude;
+	struct work_struct work;
+};
+
+static void stadiaff_work(struct work_struct *work)
+{
+	struct stadiaff_device *stadiaff =
+		container_of(work, struct stadiaff_device, work);
+	struct hid_field *rumble_field = stadiaff->report->field[0];
+	unsigned long flags;
+
+	spin_lock_irqsave(&stadiaff->lock, flags);
+	rumble_field->value[0] = stadiaff->strong_magnitude;
+	rumble_field->value[1] = stadiaff->weak_magnitude;
+	spin_unlock_irqrestore(&stadiaff->lock, flags);
+
+	hid_hw_request(stadiaff->hid, stadiaff->report, HID_REQ_SET_REPORT);
+}
+
+static int stadiaff_play(struct input_dev *dev, void *data,
+			 struct ff_effect *effect)
+{
+	struct hid_device *hid = input_get_drvdata(dev);
+	struct stadiaff_device *stadiaff = hid_get_drvdata(hid);
+	unsigned long flags;
+
+	spin_lock_irqsave(&stadiaff->lock, flags);
+	if (!stadiaff->removed) {
+		stadiaff->strong_magnitude = effect->u.rumble.strong_magnitude;
+		stadiaff->weak_magnitude = effect->u.rumble.weak_magnitude;
+		schedule_work(&stadiaff->work);
+	}
+	spin_unlock_irqrestore(&stadiaff->lock, flags);
+
+	return 0;
+}
+
+static int stadiaff_init(struct hid_device *hid)
+{
+	struct stadiaff_device *stadiaff;
+	struct hid_report *report;
+	struct hid_input *hidinput;
+	struct input_dev *dev;
+	int error;
+
+	if (list_empty(&hid->inputs)) {
+		hid_err(hid, "no inputs found\n");
+		return -ENODEV;
+	}
+	hidinput = list_entry(hid->inputs.next, struct hid_input, list);
+	dev = hidinput->input;
+
+	report = hid_validate_values(hid, HID_OUTPUT_REPORT,
+				     STADIA_FF_REPORT_ID, 0, 2);
+	if (!report)
+		return -ENODEV;
+
+	stadiaff = devm_kzalloc(&hid->dev, sizeof(struct stadiaff_device),
+				GFP_KERNEL);
+	if (!stadiaff)
+		return -ENOMEM;
+
+	hid_set_drvdata(hid, stadiaff);
+
+	input_set_capability(dev, EV_FF, FF_RUMBLE);
+
+	error = input_ff_create_memless(dev, NULL, stadiaff_play);
+	if (error)
+		return error;
+
+	stadiaff->removed = false;
+	stadiaff->hid = hid;
+	stadiaff->report = report;
+	INIT_WORK(&stadiaff->work, stadiaff_work);
+	spin_lock_init(&stadiaff->lock);
+
+	hid_info(hid, "Force Feedback for Google Stadia controller\n");
+
+	return 0;
+}
+
+static int stadia_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+	int ret;
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		hid_err(hdev, "parse failed\n");
+		return ret;
+	}
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
+	if (ret) {
+		hid_err(hdev, "hw start failed\n");
+		return ret;
+	}
+
+	stadiaff_init(hdev);
+
+	return 0;
+}
+
+static void stadia_remove(struct hid_device *hid)
+{
+	struct stadiaff_device *stadiaff = hid_get_drvdata(hid);
+	unsigned long flags;
+
+	spin_lock_irqsave(&stadiaff->lock, flags);
+	stadiaff->removed = true;
+	spin_unlock_irqrestore(&stadiaff->lock, flags);
+
+	cancel_work_sync(&stadiaff->work);
+	hid_hw_stop(hid);
+}
+
+static const struct hid_device_id stadia_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_STADIA) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_STADIA) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, stadia_devices);
+
+static struct hid_driver stadia_driver = {
+	.name = "stadia",
+	.id_table = stadia_devices,
+	.probe = stadia_probe,
+	.remove = stadia_remove,
+};
+module_hid_driver(stadia_driver);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 63545cd307e5..cffd4ac609a0 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -525,6 +525,7 @@
 #define USB_DEVICE_ID_GOOGLE_MOONBALL	0x5044
 #define USB_DEVICE_ID_GOOGLE_DON	0x5050
 #define USB_DEVICE_ID_GOOGLE_EEL	0x5057
+#define USB_DEVICE_ID_GOOGLE_STADIA	0x9400
 
 #define USB_VENDOR_ID_GOTOP		0x08f2
 #define USB_DEVICE_ID_SUPER_Q2		0x007f
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related

* [PATCH v2 1/2] dt-bindings: Input: exc3000 - Support power supply regulators
From: Mike Looijmans @ 2023-07-07 13:10 UTC (permalink / raw)
  To: devicetree, linux-input
  Cc: Mike Looijmans, Conor Dooley, Dmitry Torokhov,
	Krzysztof Kozlowski, Rob Herring, linux-kernel
In-Reply-To: <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.949ef384-8293-46b8-903f-40a477c056ae.067a6add-28c9-4499-a235-882581bddc85@emailsignatures365.codetwo.com>

Add power supply regulator support to the exc3000 devices.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

(no changes since v1)

 .../devicetree/bindings/input/touchscreen/eeti,exc3000.yaml     | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/eeti,exc3000.yaml b/Documentation/devicetree/bindings/input/touchscreen/eeti,exc3000.yaml
index 007adbc89c14..9dc25d30a0a8 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/eeti,exc3000.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/eeti,exc3000.yaml
@@ -24,6 +24,8 @@ properties:
     maxItems: 1
   reset-gpios:
     maxItems: 1
+  vdd-supply:
+    description: Power supply regulator for the chip
   touchscreen-size-x: true
   touchscreen-size-y: true
   touchscreen-inverted-x: true
-- 
2.17.1


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topicproducts.com
W: www.topic.nl

Please consider the environment before printing this e-mail

^ permalink raw reply related

* [PATCH v2 2/2] Input: exc3000 - Support power supply regulators
From: Mike Looijmans @ 2023-07-07 13:10 UTC (permalink / raw)
  To: devicetree, linux-input
  Cc: Mike Looijmans, Dmitry Torokhov, Uwe Kleine-König,
	linux-kernel
In-Reply-To: <20230707131042.10795-1-mike.looijmans@topic.nl>

Add power supply regulator support to the exc3000 devices.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>

---

Changes in v2:
Add missing "return" statement
Use devm_regulator_get_enable without _optional

 drivers/input/touchscreen/exc3000.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/input/touchscreen/exc3000.c b/drivers/input/touchscreen/exc3000.c
index 4af4c1e5d0da..e3f6d21b3c1b 100644
--- a/drivers/input/touchscreen/exc3000.c
+++ b/drivers/input/touchscreen/exc3000.c
@@ -18,6 +18,7 @@
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/regulator/consumer.h>
 #include <linux/sizes.h>
 #include <linux/timer.h>
 #include <asm/unaligned.h>
@@ -360,6 +361,12 @@ static int exc3000_probe(struct i2c_client *client)
 	if (IS_ERR(data->reset))
 		return PTR_ERR(data->reset);
 
+	/* For proper reset sequence, enable power while reset asserted */
+	error = devm_regulator_get_enable(&client->dev, "vdd");
+	if (error && error != -ENODEV)
+		return dev_err_probe(&client->dev, error,
+				     "failed to request vdd regulator\n");
+
 	if (data->reset) {
 		msleep(EXC3000_RESET_MS);
 		gpiod_set_value_cansleep(data->reset, 0);
-- 
2.17.1


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topicproducts.com
W: www.topic.nl

Please consider the environment before printing this e-mail

^ permalink raw reply related

* Re: [PATCH v2 RESEND] HID: hid-google-stadiaff: add support for Stadia force feedback
From: Rahul Rameshbabu @ 2023-07-07 17:51 UTC (permalink / raw)
  To: Fabio Baltieri; +Cc: Benjamin Tissoires, Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20230707104035.1697204-1-fabiobaltieri@chromium.org>

On Fri, 07 Jul, 2023 10:40:35 +0000 Fabio Baltieri <fabiobaltieri@chromium.org> wrote:
> Add a hid-stadiaff module to support rumble based force feedback on the
> Google Stadia controller. This works using the HID output endpoint
> exposed on both the USB and BLE interface.
>
> Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> ---
> +static int stadiaff_init(struct hid_device *hid)
> +{
> +	struct stadiaff_device *stadiaff;
> +	struct hid_report *report;
> +	struct hid_input *hidinput;
> +	struct input_dev *dev;
> +	int error;
> +
> +	if (list_empty(&hid->inputs)) {
> +		hid_err(hid, "no inputs found\n");
> +		return -ENODEV;
> +	}
> +	hidinput = list_entry(hid->inputs.next, struct hid_input, list);
> +	dev = hidinput->input;
> +
> +	report = hid_validate_values(hid, HID_OUTPUT_REPORT,
> +				     STADIA_FF_REPORT_ID, 0, 2);
> +	if (!report)
> +		return -ENODEV;
> +
> +	stadiaff = devm_kzalloc(&hid->dev, sizeof(struct stadiaff_device),
> +				GFP_KERNEL);
> +	if (!stadiaff)
> +		return -ENOMEM;

If we fail to allocate stadiaff, we abort init without ever initializing
the spinlock and work struct.

> +
> +	hid_set_drvdata(hid, stadiaff);
> +
> +	input_set_capability(dev, EV_FF, FF_RUMBLE);
> +
> +	error = input_ff_create_memless(dev, NULL, stadiaff_play);
> +	if (error)
> +		return error;

Lets say input_ff_create_memless fails. The spinlock and work struct are
not properly initialized.

> +
> +	stadiaff->removed = false;
> +	stadiaff->hid = hid;
> +	stadiaff->report = report;
> +	INIT_WORK(&stadiaff->work, stadiaff_work);
> +	spin_lock_init(&stadiaff->lock);
> +
> +	hid_info(hid, "Force Feedback for Google Stadia controller\n");
> +
> +	return 0;
> +}
> +
> +static int stadia_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> +	int ret;
> +
> +	ret = hid_parse(hdev);
> +	if (ret) {
> +		hid_err(hdev, "parse failed\n");
> +		return ret;
> +	}
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
> +	if (ret) {
> +		hid_err(hdev, "hw start failed\n");
> +		return ret;
> +	}
> +
> +	stadiaff_init(hdev);

Is the intention for not error handling stadiaff_init to be able to use
the HID device even if haptics are not enabled? I think that's fine but
there are some design considerations that need to be made in order for
this code to be safe in the context of stadia_remove.

> +
> +	return 0;
> +}
> +
> +static void stadia_remove(struct hid_device *hid)
> +{
> +	struct stadiaff_device *stadiaff = hid_get_drvdata(hid);

stadiaff is unsafe to use if we failed to allocate memory for it and do
not set the hid device driver data. We would be dereferencing a
driver_data pointer never set/initialized by this driver in this error
path in stadiaff_init.

> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&stadiaff->lock, flags);
> +	stadiaff->removed = true;
> +	spin_unlock_irqrestore(&stadiaff->lock, flags);

Attempting to lock/unlock a spinlock that may not have been initialized
if an error occurred in the stadiaff_init path.

> +
> +	cancel_work_sync(&stadiaff->work);

Attempting to cancel work on a work_struct that may not be properly
initialized if an error occurred in stadiaff_init.

> +	hid_hw_stop(hid);
> +}

Thanks,

-- Rahul Rameshbabu

^ permalink raw reply

* Need information to implement correct handling of the GSP 370 wireless headset
From: Werner Sembach @ 2023-07-07 21:12 UTC (permalink / raw)
  To: linux-input

Hi,

The EPOS GSP 370 wireless headset has a volume control knob on the 
headset. Turning this has 2 effects on Linux:

- The volume gets adjusted in firmware

- A volume up/down key event is sent to the OS and the volume is 
adjustet on the OS level (the dongle registers both as a audio device 
and an usb keyboard to do this)

This double volume adjust is ofc not the intended behavior and induces 
all kinds of wiredness, like the os is already displaying 100% volume 
but you can still turn it higher because the firmware has not yet 
reached max volume.

Running Wireshark on Windows and Linux I recognized that Windows is 
sending "URB_CONTROL out" events to the dongle on volume adjust, which 
Linux doesn't, these events have a paylode that seems to encode a volume 
value in 2 byte with little endianess.

I'm unsure if I can attach pcap file or if it will get blocked in the 
list so i will send them as a reply.

So my 2 questions are:

- What are these USB "URB_CONTROL out" events? (Does not seem to be HID 
reports?)

- In which subsystem would I want to implement them? Or should this be 
handled in userspace?

Bast regards,

Werner


^ permalink raw reply

* Re: Need information to implement correct handling of the GSP 370 wireless headset
From: Werner Sembach @ 2023-07-07 21:22 UTC (permalink / raw)
  To: linux-input
In-Reply-To: <e01dbcd2-3ce3-ebaf-e685-3b89601ae55a@rdorf.de>

Am 07.07.23 um 23:12 schrieb Werner Sembach:
> Hi,
>
> The EPOS GSP 370 wireless headset has a volume control knob on the 
> headset. Turning this has 2 effects on Linux:
>
> - The volume gets adjusted in firmware
>
> - A volume up/down key event is sent to the OS and the volume is 
> adjustet on the OS level (the dongle registers both as a audio device 
> and an usb keyboard to do this)
>
> This double volume adjust is ofc not the intended behavior and induces 
> all kinds of wiredness, like the os is already displaying 100% volume 
> but you can still turn it higher because the firmware has not yet 
> reached max volume.
>
> Running Wireshark on Windows and Linux I recognized that Windows is 
> sending "URB_CONTROL out" events to the dongle on volume adjust, which 
> Linux doesn't, these events have a paylode that seems to encode a 
> volume value in 2 byte with little endianess.
>
> I'm unsure if I can attach pcap file or if it will get blocked in the 
> list so i will send them as a reply.
As I thought: The pcapng files are to big for the mailing list. When 
someone is interested let me know how I can send them (should I open a 
Bugzilla Ticket for it?).
>
> So my 2 questions are:
>
> - What are these USB "URB_CONTROL out" events? (Does not seem to be 
> HID reports?)
>
> - In which subsystem would I want to implement them? Or should this be 
> handled in userspace?
>
> Bast regards,
>
> Werner
>

^ permalink raw reply

* Re: [PATCH v2 RESEND] HID: hid-google-stadiaff: add support for Stadia force feedback
From: Fabio Baltieri @ 2023-07-07 21:35 UTC (permalink / raw)
  To: Rahul Rameshbabu
  Cc: Benjamin Tissoires, Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <87fs5zboej.fsf@nvidia.com>

Hi Rahul,

On Fri, Jul 07, 2023 at 10:51:48AM -0700, Rahul Rameshbabu wrote:
> On Fri, 07 Jul, 2023 10:40:35 +0000 Fabio Baltieri <fabiobaltieri@chromium.org> wrote:
> > Add a hid-stadiaff module to support rumble based force feedback on the
> > Google Stadia controller. This works using the HID output endpoint
> > exposed on both the USB and BLE interface.
> >
> > Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> > ---
> > +static int stadiaff_init(struct hid_device *hid)
> > +{
> > +	struct stadiaff_device *stadiaff;
> > +	struct hid_report *report;
> > +	struct hid_input *hidinput;
> > +	struct input_dev *dev;
> > +	int error;
> > +
> > +	if (list_empty(&hid->inputs)) {
> > +		hid_err(hid, "no inputs found\n");
> > +		return -ENODEV;
> > +	}
> > +	hidinput = list_entry(hid->inputs.next, struct hid_input, list);
> > +	dev = hidinput->input;
> > +
> > +	report = hid_validate_values(hid, HID_OUTPUT_REPORT,
> > +				     STADIA_FF_REPORT_ID, 0, 2);
> > +	if (!report)
> > +		return -ENODEV;
> > +
> > +	stadiaff = devm_kzalloc(&hid->dev, sizeof(struct stadiaff_device),
> > +				GFP_KERNEL);
> > +	if (!stadiaff)
> > +		return -ENOMEM;
> 
> If we fail to allocate stadiaff, we abort init without ever initializing
> the spinlock and work struct.
> 
> > +
> > +	hid_set_drvdata(hid, stadiaff);
> > +
> > +	input_set_capability(dev, EV_FF, FF_RUMBLE);
> > +
> > +	error = input_ff_create_memless(dev, NULL, stadiaff_play);
> > +	if (error)
> > +		return error;
> 
> Lets say input_ff_create_memless fails. The spinlock and work struct are
> not properly initialized.
> 
> > +
> > +	stadiaff->removed = false;
> > +	stadiaff->hid = hid;
> > +	stadiaff->report = report;
> > +	INIT_WORK(&stadiaff->work, stadiaff_work);
> > +	spin_lock_init(&stadiaff->lock);
> > +
> > +	hid_info(hid, "Force Feedback for Google Stadia controller\n");
> > +
> > +	return 0;
> > +}
> > +
> > +static int stadia_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > +{
> > +	int ret;
> > +
> > +	ret = hid_parse(hdev);
> > +	if (ret) {
> > +		hid_err(hdev, "parse failed\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
> > +	if (ret) {
> > +		hid_err(hdev, "hw start failed\n");
> > +		return ret;
> > +	}
> > +
> > +	stadiaff_init(hdev);
> 
> Is the intention for not error handling stadiaff_init to be able to use
> the HID device even if haptics are not enabled? I think that's fine but
> there are some design considerations that need to be made in order for
> this code to be safe in the context of stadia_remove.

Sorry, no, the intention was to catch the error here and fail the probe,
that's the pattern on other hid haptic drivers as well, would not really
want to get too creative about it here. I shuffled the code around a bit
and somehow missed this check, I think it addresses all the potential
unallocated dereferecing you pointed out.

I'll send a v3 with the check, thanks for spotting this.

> 
> > +
> > +	return 0;
> > +}
> > +
> > +static void stadia_remove(struct hid_device *hid)
> > +{
> > +	struct stadiaff_device *stadiaff = hid_get_drvdata(hid);
> 
> stadiaff is unsafe to use if we failed to allocate memory for it and do
> not set the hid device driver data. We would be dereferencing a
> driver_data pointer never set/initialized by this driver in this error
> path in stadiaff_init.
> 
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&stadiaff->lock, flags);
> > +	stadiaff->removed = true;
> > +	spin_unlock_irqrestore(&stadiaff->lock, flags);
> 
> Attempting to lock/unlock a spinlock that may not have been initialized
> if an error occurred in the stadiaff_init path.
> 
> > +
> > +	cancel_work_sync(&stadiaff->work);
> 
> Attempting to cancel work on a work_struct that may not be properly
> initialized if an error occurred in stadiaff_init.
> 
> > +	hid_hw_stop(hid);
> > +}
> 
> Thanks,
> 
> -- Rahul Rameshbabu

^ permalink raw reply

* Re: [PATCH v4 00/24] Input: Simplify with dev_err_probe()
From: Dmitry Torokhov @ 2023-07-07 23:58 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Hans de Goede, Linus Walleij, Bastien Nocera, Sangwon Jee,
	Eugen Hristev, Mika Penttilä, linux-input, linux-kernel,
	platform-driver-x86, Andi Shyti, Andy Shevchenko
In-Reply-To: <20230625162817.100397-1-krzysztof.kozlowski@linaro.org>

On Sun, Jun 25, 2023 at 06:27:53PM +0200, Krzysztof Kozlowski wrote:
> Hi,
> 
> Three years ago I sent v3 of this series. There was never an anwser from Dmitry
> - no comment at all. Maybe after three years this can go in? It makes
> the code nicely smaller.

OK, I give up. I still think that this API is wrong, but I will not get
around to making producers note the reasons for deferral, so be it.

Applied the lot, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: xpad - add GameSir T4 Kaleid Controller support
From: Dmitry Torokhov @ 2023-07-07 23:59 UTC (permalink / raw)
  To: Jonathan Frederick; +Cc: linux-input
In-Reply-To: <ZKeKSbP3faIPv5jB@dbj-hp-flip>

On Thu, Jul 06, 2023 at 10:45:18PM -0500, Jonathan Frederick wrote:
> Add VID and PID to the xpad_device table to allow driver
> to use the GameSir T4 Kaleid Controller, which is
> XTYPE_XBOX360 compatible in xinput mode.
> 
> Signed-off-by: Jonathan Frederick <doublej472@gmail.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 1/2] Input: xpad - fix support for some third-party controllers
From: Dmitry Torokhov @ 2023-07-07 23:59 UTC (permalink / raw)
  To: Vicki Pfau; +Cc: linux-input, Pavel Rojtberg, Andrey Smirnov
In-Reply-To: <20230607012812.379640-2-vi@endrift.com>

On Tue, Jun 06, 2023 at 06:28:11PM -0700, Vicki Pfau wrote:
> Some third-party controllers, such as the HORPIAD FPS for Nintendo Switch and
> Gamesir-G3w, require a specific packet that the first-party XInput driver sends
> before it will start sending reports. It's not currently known what this packet
> does, but since the first-party driver always sends it's unlikely that this
> could cause issues with existing controllers.
> 
> Co-authored-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Signed-off-by: Vicki Pfau <vi@endrift.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 2/2] Input: xpad - Add GameSir VID for Xbox One controllers
From: Dmitry Torokhov @ 2023-07-08  0:00 UTC (permalink / raw)
  To: Vicki Pfau; +Cc: linux-input, Sam Lantinga, Pavel Rojtberg
In-Reply-To: <20230607012812.379640-3-vi@endrift.com>

On Tue, Jun 06, 2023 at 06:28:12PM -0700, Vicki Pfau wrote:
> From: Sam Lantinga <slouken@libsdl.org>
> 
> Signed-off-by: Sam Lantinga <slouken@libsdl.org>
> Signed-off-by: Vicki Pfau <vi@endrift.com>
> ---
>  drivers/input/joystick/xpad.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 745047215e42..459b5c507399 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -499,6 +499,7 @@ static const struct usb_device_id xpad_table[] = {
>  	XPAD_XBOXONE_VENDOR(0x2dc8),		/* 8BitDo Pro 2 Wired Controller for Xbox */
>  	XPAD_XBOXONE_VENDOR(0x2e24),		/* Hyperkin Duke Xbox One pad */
>  	XPAD_XBOX360_VENDOR(0x2f24),		/* GameSir controllers */
> +	XPAD_XBOXONE_VENDOR(0x3537),		/* GameSir Controllers */

We try to keep this list sorted by vendor ID, so I moved this down and
applied.

>  	XPAD_XBOX360_VENDOR(0x31e3),		/* Wooting Keyboards */
>  	XPAD_XBOX360_VENDOR(0x3285),		/* Nacon GC-100 */
>  	{ }
> -- 
> 2.40.1
> 

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: Input: exc3000 - Support power supply regulators
From: Dmitry Torokhov @ 2023-07-08  0:00 UTC (permalink / raw)
  To: Mike Looijmans
  Cc: devicetree, linux-input, Conor Dooley, Krzysztof Kozlowski,
	Rob Herring, linux-kernel
In-Reply-To: <20230707131042.10795-1-mike.looijmans@topic.nl>

On Fri, Jul 07, 2023 at 03:10:41PM +0200, Mike Looijmans wrote:
> Add power supply regulator support to the exc3000 devices.
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 2/2] Input: exc3000 - Support power supply regulators
From: Dmitry Torokhov @ 2023-07-08  0:00 UTC (permalink / raw)
  To: Mike Looijmans
  Cc: devicetree, linux-input, Uwe Kleine-König, linux-kernel
In-Reply-To: <20230707131042.10795-2-mike.looijmans@topic.nl>

On Fri, Jul 07, 2023 at 03:10:42PM +0200, Mike Looijmans wrote:
> Add power supply regulator support to the exc3000 devices.
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> 

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: pinephone-keyboard - Use devm_regulator_get_enable()
From: Dmitry Torokhov @ 2023-07-08  0:04 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: Samuel Holland, linux-kernel, kernel-janitors, linux-input
In-Reply-To: <78d7a4719ed7c372a7aa62afb66d4f1561799b5f.1686985515.git.christophe.jaillet@wanadoo.fr>

On Sat, Jun 17, 2023 at 09:05:42AM +0200, Christophe JAILLET wrote:
> Use devm_regulator_get_enable() instead of hand writing it. It saves some
> line of code.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply


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