* [PATCH 5/7] HID: amd_sfh: Correct the stop all command
From: Basavaraj Natikar @ 2023-04-11 16:10 UTC (permalink / raw)
To: jikos, benjamin.tissoires, linux-input; +Cc: Basavaraj Natikar
In-Reply-To: <20230411161030.909350-1-Basavaraj.Natikar@amd.com>
Misinterpreted the stop all command in SHF1.1 firmware. Therefore, it is
necessary to update the stop all command accordingly to disable all
sensors.
Fixes: 93ce5e0231d7 ("HID: amd_sfh: Implement SFH1.1 functionality")
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 6e19ccc12450..6f6047f7f12e 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -58,8 +58,10 @@ static void amd_stop_all_sensor(struct amd_mp2_dev *privdata)
struct sfh_cmd_base cmd_base;
cmd_base.ul = 0;
- cmd_base.cmd.cmd_id = STOP_ALL_SENSORS;
+ cmd_base.cmd.cmd_id = DISABLE_SENSOR;
cmd_base.cmd.intr_disable = 0;
+ /* 0xf indicates all sensors */
+ cmd_base.cmd.sensor_id = 0xf;
writel(cmd_base.ul, privdata->mmio + AMD_C2P_MSG(0));
}
--
2.25.1
^ permalink raw reply related
* [PATCH 6/7] HID: amd_sfh: Increase sensor command timeout for SFH1.1
From: Basavaraj Natikar @ 2023-04-11 16:10 UTC (permalink / raw)
To: jikos, benjamin.tissoires, linux-input; +Cc: Basavaraj Natikar
In-Reply-To: <20230411161030.909350-1-Basavaraj.Natikar@amd.com>
The initialization of SFH1.1 sensors may take some time. Hence, increase
sensor command timeouts in order to obtain status responses within a
maximum timeout.
Fixes: 93ce5e0231d7 ("HID: amd_sfh: Implement SFH1.1 functionality")
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 6f6047f7f12e..4f81ef2d4f56 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -16,11 +16,11 @@ static int amd_sfh_wait_response(struct amd_mp2_dev *mp2, u8 sid, u32 cmd_id)
{
struct sfh_cmd_response cmd_resp;
- /* Get response with status within a max of 1600 ms timeout */
+ /* Get response with status within a max of 10000 ms timeout */
if (!readl_poll_timeout(mp2->mmio + AMD_P2C_MSG(0), cmd_resp.resp,
(cmd_resp.response.response == 0 &&
cmd_resp.response.cmd_id == cmd_id && (sid == 0xff ||
- cmd_resp.response.sensor_id == sid)), 500, 1600000))
+ cmd_resp.response.sensor_id == sid)), 500, 10000000))
return cmd_resp.response.response;
return -1;
--
2.25.1
^ permalink raw reply related
* [PATCH 3/7] HID: amd_sfh: Fix illuminance value
From: Basavaraj Natikar @ 2023-04-11 16:10 UTC (permalink / raw)
To: jikos, benjamin.tissoires, linux-input; +Cc: Basavaraj Natikar
In-Reply-To: <20230411161030.909350-1-Basavaraj.Natikar@amd.com>
Illuminance value is actually 32 bits, but is incorrectly trancated to
16 bits. Hence convert to integer illuminace accordingly to reflect
correct values.
Fixes: 93ce5e0231d7 ("HID: amd_sfh: Implement SFH1.1 functionality")
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 2 +-
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 2 +-
2 files changed, 2 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 0609fea581c9..6f0d332ccf51 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
@@ -218,7 +218,7 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
OFFSET_SENSOR_DATA_DEFAULT;
memcpy_fromio(&als_data, sensoraddr, sizeof(struct sfh_als_data));
get_common_inputs(&als_input.common_property, report_id);
- als_input.illuminance_value = als_data.lux;
+ als_input.illuminance_value = float_to_int(als_data.lux);
report_size = sizeof(als_input);
memcpy(input_report, &als_input, sizeof(als_input));
break;
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index a3e0ec289e3f..9d31d5b510eb 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -133,7 +133,7 @@ struct sfh_mag_data {
struct sfh_als_data {
struct sfh_common_data commondata;
- u16 lux;
+ u32 lux;
};
struct hpd_status {
--
2.25.1
^ permalink raw reply related
* [PATCH 2/7] HID: amd_sfh: Correct the sensor enable and disable command
From: Basavaraj Natikar @ 2023-04-11 16:10 UTC (permalink / raw)
To: jikos, benjamin.tissoires, linux-input; +Cc: Basavaraj Natikar
In-Reply-To: <20230411161030.909350-1-Basavaraj.Natikar@amd.com>
In order to start or stop sensors, the firmware command needs to be
changed to add an additional default subcommand value. For this reason,
add a subcommand value to enable or disable sensors accordingly.
Fixes: 93ce5e0231d7 ("HID: amd_sfh: Implement SFH1.1 functionality")
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index c6df959ec725..6e19ccc12450 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -33,6 +33,7 @@ static void amd_start_sensor(struct amd_mp2_dev *privdata, struct amd_mp2_sensor
cmd_base.ul = 0;
cmd_base.cmd.cmd_id = ENABLE_SENSOR;
cmd_base.cmd.intr_disable = 0;
+ cmd_base.cmd.sub_cmd_value = 1;
cmd_base.cmd.sensor_id = info.sensor_idx;
writel(cmd_base.ul, privdata->mmio + AMD_C2P_MSG(0));
@@ -45,6 +46,7 @@ static void amd_stop_sensor(struct amd_mp2_dev *privdata, u16 sensor_idx)
cmd_base.ul = 0;
cmd_base.cmd.cmd_id = DISABLE_SENSOR;
cmd_base.cmd.intr_disable = 0;
+ cmd_base.cmd.sub_cmd_value = 1;
cmd_base.cmd.sensor_id = sensor_idx;
writeq(0x0, privdata->mmio + AMD_C2P_MSG(1));
--
2.25.1
^ permalink raw reply related
* [PATCH 1/7] HID: amd_sfh: Correct the structure fields
From: Basavaraj Natikar @ 2023-04-11 16:10 UTC (permalink / raw)
To: jikos, benjamin.tissoires, linux-input; +Cc: Basavaraj Natikar
In-Reply-To: <20230411161030.909350-1-Basavaraj.Natikar@amd.com>
Misinterpreted sfh_cmd_base structure member fields. Therefore, adjust
the structure member fields accordingly to reflect functionality.
Fixes: 93ce5e0231d7 ("HID: amd_sfh: Implement SFH1.1 functionality")
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index ae47a369dc05..a3e0ec289e3f 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -33,9 +33,9 @@ struct sfh_cmd_base {
struct {
u32 sensor_id : 4;
u32 cmd_id : 4;
- u32 sub_cmd_id : 6;
- u32 length : 12;
- u32 rsvd : 5;
+ u32 sub_cmd_id : 8;
+ u32 sub_cmd_value : 12;
+ u32 rsvd : 3;
u32 intr_disable : 1;
} cmd;
};
--
2.25.1
^ permalink raw reply related
* [PATCH 0/7] Fixes to amd_sfh
From: Basavaraj Natikar @ 2023-04-11 16:10 UTC (permalink / raw)
To: jikos, benjamin.tissoires, linux-input; +Cc: Basavaraj Natikar
Changes include correcting structure fields, illuminance values,
shutdown PM operations, stop all command, increasing sensor
command timeout and no sensor condition.
Basavaraj Natikar (7):
HID: amd_sfh: Correct the structure fields
HID: amd_sfh: Correct the sensor enable and disable command
HID: amd_sfh: Fix illuminance value
HID: amd_sfh: Add support for shutdown operation
HID: amd_sfh: Correct the stop all command
HID: amd_sfh: Increase sensor command timeout for SFH1.1
HID: amd_sfh: Handle "no sensors" enabled for SFH1.1
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 9 +++++++++
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 2 +-
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 11 +++++++++++
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 10 +++++++---
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 8 ++++----
5 files changed, 32 insertions(+), 8 deletions(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH 0/6] HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms + remove specialized drivers
From: Hans de Goede @ 2023-04-11 16:00 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, Douglas Anderson, linux-input
In-Reply-To: <20230411125036.3ojjdrjzxhasu5du@mail.corp.redhat.com>
Hi Benjamin,
On 4/11/23 14:50, Benjamin Tissoires wrote:
> On Apr 11 2023, Hans de Goede wrote:
>> Hi Benjamin,
>>
>> On 4/11/23 11:02, Benjamin Tissoires wrote:
>>> Hi Hans,
>>>
>>> On Apr 09 2023, Hans de Goede wrote:
>>>> Hi All,
>>>>
>>>> This series consist of 2 parts:
>>>>
>>>> 1. Patches 1-3. Allow using i2c-hid-of on non OF platforms to allow I2C-HID
>>>> devices which are not enumerated by ACPI to work on ACPI platforms
>>>> (by manual i2c_client instantiation using i2c_client_id matching).
>>>
>>> Patches 1 and 2 are looking good, but I wonder if you can not achieve the
>>> same result by relying on an ACPI SSDT override. I got something similar
>>> working on this thread[0].
>>
>> Yes this could be made to work with an ACPI override. But the goal is
>> to make things work OOTB for end users when they install Linux and
>> ACPI overrides are very far from something which works OOTB.
>
> Fair enough.
>
>>
>>> I understand the "post-reset-deassert-delay-ms" might be something hard
>>> to express with an SSDT, but we should already have all the bits in
>>> place, no?
>>
>> Actually if an ACPI override is used then the setting of the GPIO
>> can be done in _PS0 and _PS3 (power on / off) methods and those
>> can simply include a sleep after setting the GPIO.
>
> Though this is all conditional if we can make ACPI SSDT override
> something that can be shipped while installing the device...
>
>>
>>> Also, the problem of "post-reset-deassert-delay-ms" is that you are not
>>> documenting it, because the OF folks do not want this in device tree,
>>> and I tend to agree with them. So this basically creates a brand new
>>> undocumented property, which is less than ideal.
>>
>> I'm merely not documenting it because there are no devicetree users yet.
>
> AFAIU, the non devicetree properties should also be documented through
> DT bindings, no? So not documenting feels very much like "I want to slip
> this one in without having to deal with DT maintainers" (and before you
> take it personaly, I know this is definitively not the intent). So I'd
> rather much having a public API documented, even if there are no users.
Right, so as a hobby I have a tendency to work on these somewhat niche/weird
x86 devices, like x86 tablets which use Android as factory OS :)
As such I have encountered the need for device-properties to pass info
from drivers/platform/x86 code to more generic drivers a number of
times before.
Each time this happens, if I try to add them to bindings I get
asked for some example devicetree code, I then respond that these
are *device*-properties not of-properties and that there are no
current devicetree users after which the DT maintainers tell me
to then NOT document them in the DT bindings, at least not until
actually DT users show up. I fully expect any attempt do add
this to the DT bindings to go the same way.
And now I have you telling me you really want to see this
documented at the same time as it getting implemented. Which
I fully understand, but does leads to a bit of a catch 22.
Anyways lets just go with the alternative of treating this
similar as the existing specialized drivers, see below.
<snip>
>> Note if just the existence of the property is the main stumbling
>> block I can go the match_data route for the wacom digitizer on
>> the Yoga Book 1 too and add an extra i2c_device_id with match-data
>> setting the delay. This could then either be its own specialized
>> driver, or we could still go with the current patch-set
>> (minus the property) and add an i2c_device_id with match-data
>> to i2c-hid-of.c .
>
> I'd much rather have a i2c-hid-of.c internal API, yes. Whether it's a
> function call, a callback or a match-data (or a driver-data), this is
> something we are in control and we can change.
Ok.
So I see 2 options here:
1. Take the approach from patches 1-4 here, but drop the property and
use match data on a new "wacom0169" i2c_device_id instead.
This would also pave the way to merging patches 5 + 6 once tested
by google to reduce some code duplication. Although you write below
you would prefer to keep these around as example code for other
specialized drivers...
2. Add a new specialized i2c-hid-of-wacom driver for this.
Question: Since this will be using i2c_device_id binding not
DT/OF binding the -of- in the name is technically incorrect,
but it would be consistent with the other specialized drivers
and could be seen as preparation (avoiding a rename/confusion)
for when any DT enumerated devices which need special handling
show up (note only relevant if you prefer this approach).
Either way is fine with me really. So you get to chose. If you
let me know which route you prefer, I'll go and prepare either
a v2 of this series, or a whole new patch for the new specialized
driver.
Regards,
Hans
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS 483a14418661878d89216be0f02918892227833b
From: kernel test robot @ 2023-04-11 13:04 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: 483a14418661878d89216be0f02918892227833b Input: edt-ft5x06 - select REGMAP_I2C
elapsed time: 723m
configs tested: 163
configs skipped: 16
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allyesconfig gcc
alpha buildonly-randconfig-r006-20230409 gcc
alpha defconfig gcc
alpha randconfig-r014-20230409 gcc
alpha randconfig-r015-20230410 gcc
alpha randconfig-r023-20230410 gcc
alpha randconfig-r026-20230410 gcc
arc allyesconfig gcc
arc buildonly-randconfig-r003-20230409 gcc
arc defconfig gcc
arc randconfig-r012-20230409 gcc
arc randconfig-r012-20230410 gcc
arc randconfig-r043-20230409 gcc
arc randconfig-r043-20230410 gcc
arm allmodconfig gcc
arm allyesconfig gcc
arm buildonly-randconfig-r001-20230409 clang
arm buildonly-randconfig-r005-20230410 clang
arm defconfig gcc
arm randconfig-r005-20230411 clang
arm randconfig-r014-20230409 clang
arm randconfig-r016-20230409 clang
arm randconfig-r046-20230409 clang
arm randconfig-r046-20230410 clang
arm64 allyesconfig gcc
arm64 defconfig gcc
arm64 randconfig-r022-20230410 gcc
arm64 randconfig-r036-20230410 clang
csky defconfig gcc
csky randconfig-r003-20230411 gcc
csky randconfig-r021-20230410 gcc
hexagon buildonly-randconfig-r001-20230409 clang
hexagon buildonly-randconfig-r003-20230409 clang
hexagon randconfig-r001-20230411 clang
hexagon randconfig-r015-20230409 clang
hexagon randconfig-r026-20230409 clang
hexagon randconfig-r041-20230409 clang
hexagon randconfig-r041-20230410 clang
hexagon randconfig-r045-20230409 clang
hexagon randconfig-r045-20230410 clang
hexagon randconfig-r045-20230411 clang
i386 allyesconfig gcc
i386 buildonly-randconfig-r003-20230410 clang
i386 buildonly-randconfig-r004-20230410 clang
i386 buildonly-randconfig-r006-20230410 clang
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-a001-20230410 clang
i386 randconfig-a002-20230410 clang
i386 randconfig-a003-20230410 clang
i386 randconfig-a004-20230410 clang
i386 randconfig-a005-20230410 clang
i386 randconfig-a006-20230410 clang
i386 randconfig-a011-20230410 gcc
i386 randconfig-a012-20230410 gcc
i386 randconfig-a013-20230410 gcc
i386 randconfig-a014-20230410 gcc
i386 randconfig-a015-20230410 gcc
i386 randconfig-a016-20230410 gcc
i386 randconfig-r015-20230410 gcc
ia64 allmodconfig gcc
ia64 buildonly-randconfig-r005-20230409 gcc
ia64 defconfig gcc
ia64 randconfig-r005-20230411 gcc
ia64 randconfig-r011-20230409 gcc
ia64 randconfig-r022-20230409 gcc
ia64 randconfig-r024-20230410 gcc
ia64 randconfig-r031-20230409 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-r006-20230411 gcc
loongarch randconfig-r024-20230409 gcc
m68k allmodconfig gcc
m68k defconfig gcc
microblaze randconfig-r025-20230409 gcc
microblaze randconfig-r034-20230409 gcc
mips allmodconfig gcc
mips allyesconfig gcc
mips randconfig-r021-20230409 clang
nios2 defconfig gcc
nios2 randconfig-r002-20230411 gcc
nios2 randconfig-r006-20230411 gcc
nios2 randconfig-r024-20230409 gcc
openrisc buildonly-randconfig-r004-20230409 gcc
openrisc randconfig-r012-20230410 gcc
openrisc randconfig-r013-20230409 gcc
openrisc randconfig-r016-20230409 gcc
parisc defconfig gcc
parisc randconfig-r013-20230409 gcc
parisc randconfig-r023-20230409 gcc
parisc randconfig-r033-20230409 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc buildonly-randconfig-r002-20230410 gcc
powerpc buildonly-randconfig-r006-20230409 gcc
powerpc randconfig-r011-20230410 gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv buildonly-randconfig-r002-20230410 gcc
riscv defconfig gcc
riscv randconfig-r016-20230410 gcc
riscv randconfig-r024-20230410 gcc
riscv randconfig-r025-20230410 gcc
riscv randconfig-r031-20230410 clang
riscv randconfig-r042-20230409 gcc
riscv randconfig-r042-20230410 gcc
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 buildonly-randconfig-r002-20230409 gcc
s390 defconfig gcc
s390 randconfig-r001-20230411 gcc
s390 randconfig-r004-20230411 gcc
s390 randconfig-r015-20230409 gcc
s390 randconfig-r026-20230409 gcc
s390 randconfig-r035-20230410 clang
s390 randconfig-r044-20230409 gcc
s390 randconfig-r044-20230410 gcc
s390 randconfig-r044-20230411 clang
sh allmodconfig gcc
sh buildonly-randconfig-r001-20230410 gcc
sh randconfig-r021-20230409 gcc
sh randconfig-r022-20230409 gcc
sh randconfig-r025-20230409 gcc
sh randconfig-r032-20230410 gcc
sh randconfig-r034-20230410 gcc
sparc buildonly-randconfig-r004-20230410 gcc
sparc defconfig gcc
sparc randconfig-r011-20230409 gcc
sparc randconfig-r035-20230409 gcc
sparc randconfig-r036-20230409 gcc
sparc64 buildonly-randconfig-r003-20230410 gcc
sparc64 randconfig-r011-20230410 gcc
sparc64 randconfig-r012-20230409 gcc
sparc64 randconfig-r025-20230410 gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 defconfig gcc
x86_64 kexec gcc
x86_64 randconfig-a001-20230410 clang
x86_64 randconfig-a002-20230410 clang
x86_64 randconfig-a003-20230410 clang
x86_64 randconfig-a004-20230410 clang
x86_64 randconfig-a005-20230410 clang
x86_64 randconfig-a006-20230410 clang
x86_64 randconfig-a011-20230410 gcc
x86_64 randconfig-a012-20230410 gcc
x86_64 randconfig-a013-20230410 gcc
x86_64 randconfig-a014-20230410 gcc
x86_64 randconfig-a015-20230410 gcc
x86_64 randconfig-a016-20230410 gcc
x86_64 randconfig-k001 clang
x86_64 randconfig-r021-20230410 gcc
x86_64 rhel-8.3 gcc
xtensa buildonly-randconfig-r004-20230409 gcc
xtensa randconfig-r004-20230411 gcc
xtensa randconfig-r014-20230410 gcc
xtensa randconfig-r023-20230410 gcc
xtensa randconfig-r033-20230410 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply
* Re: [PATCH 0/6] HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms + remove specialized drivers
From: Benjamin Tissoires @ 2023-04-11 12:50 UTC (permalink / raw)
To: Hans de Goede; +Cc: Jiri Kosina, Douglas Anderson, linux-input
In-Reply-To: <c3e08839-b621-3e57-0d6b-f4cd14c897b9@redhat.com>
On Apr 11 2023, Hans de Goede wrote:
> Hi Benjamin,
>
> On 4/11/23 11:02, Benjamin Tissoires wrote:
> > Hi Hans,
> >
> > On Apr 09 2023, Hans de Goede wrote:
> >> Hi All,
> >>
> >> This series consist of 2 parts:
> >>
> >> 1. Patches 1-3. Allow using i2c-hid-of on non OF platforms to allow I2C-HID
> >> devices which are not enumerated by ACPI to work on ACPI platforms
> >> (by manual i2c_client instantiation using i2c_client_id matching).
> >
> > Patches 1 and 2 are looking good, but I wonder if you can not achieve the
> > same result by relying on an ACPI SSDT override. I got something similar
> > working on this thread[0].
>
> Yes this could be made to work with an ACPI override. But the goal is
> to make things work OOTB for end users when they install Linux and
> ACPI overrides are very far from something which works OOTB.
Fair enough.
>
> > I understand the "post-reset-deassert-delay-ms" might be something hard
> > to express with an SSDT, but we should already have all the bits in
> > place, no?
>
> Actually if an ACPI override is used then the setting of the GPIO
> can be done in _PS0 and _PS3 (power on / off) methods and those
> can simply include a sleep after setting the GPIO.
Though this is all conditional if we can make ACPI SSDT override
something that can be shipped while installing the device...
>
> > Also, the problem of "post-reset-deassert-delay-ms" is that you are not
> > documenting it, because the OF folks do not want this in device tree,
> > and I tend to agree with them. So this basically creates a brand new
> > undocumented property, which is less than ideal.
>
> I'm merely not documenting it because there are no devicetree users yet.
AFAIU, the non devicetree properties should also be documented through
DT bindings, no? So not documenting feels very much like "I want to slip
this one in without having to deal with DT maintainers" (and before you
take it personaly, I know this is definitively not the intent). So I'd
rather much having a public API documented, even if there are no users.
>
> Between the 2 currently supported of devices with a reset GPIO +
> the I2C-HID capable touchscreen + wacom digitizer on the x86 android
> Yoga Book 1 I'm trying to get to work that is 4 I2C-HID devices which
> all follow the pattern of: 1. They have a reset GPIO, 2. they need
> some delay after reset is deasserted.
>
> It seems silly to keep adding more and more device-ids + match-data
> with just the delays in there when it seems that many many I2C-HID
> capable controllers/chips follow this pattern.
The problem, AFAICT, is that I2C-HID is only described through ACPI, and
a lot of the HW specific is let to ACPI. So i2c-hid-of, is basically OEM
bringing components together on a DT platform, and hoping for the best.
This works well in the way that we don't need to add a new driver for
it, but they can not easily describe what they need (or even fail like
your tablet which is supposed to be working thorugh ACPI).
So if we had one big competitor, like Google for Chromebooks, who just
said: "this is what you need for OF i2c-hid devices, and you can not
rely on anything else", life would be much simpler.
>
> Also note that there already is a very similar "post-power-on-delay-ms"
> property. I really don't see what makes specifying a delay after
> enabling regulators through a property ok, but specifying the delay
> after reset-deassert not ok. Allowing one but not the other is not
> very consistent.
Agree :/
>
> The reason why I'm not documenting the property now is because of
> lack of current devicetree users. It can be documented once
> the first DT users show up and getting it accepted should really not
> be an issue given that "post-power-on-delay-ms" already exists.
Honestly this is not a good way of doing thing IMO. This is basically
what I did with i2c-hid, and it pissed the DT maintainers. (I did it
because at the time the maintainers were not reactive, and/or the ML
were not correctly set IIRC).
>
> Note if just the existence of the property is the main stumbling
> block I can go the match_data route for the wacom digitizer on
> the Yoga Book 1 too and add an extra i2c_device_id with match-data
> setting the delay. This could then either be its own specialized
> driver, or we could still go with the current patch-set
> (minus the property) and add an i2c_device_id with match-data
> to i2c-hid-of.c .
I'd much rather have a i2c-hid-of.c internal API, yes. Whether it's a
function call, a callback or a match-data (or a driver-data), this is
something we are in control and we can change. A blind undocumented
property is going to be a pain if we get to change it.
(unless of course you can get Rob's ack on the preperty itself).
>
> The only question then is how to name the i2c_device_id for the wacom
> digitizer. It has a vid:pid of 056A:0169 So maybe "wacom0169" ?
Seems reasonable to me :)
[few minutes later]
Well, maybe we don't want the PID to be used here, because we will end
up having to quirk every single device. But OTOH I do not see a
different solution...
>
>
> >> 2. Patches 4-6. Remove the special i2c-hid-of-elan and i2c-hid-of-goodix
> >> driver, folding the functionality into the generic i2c-hid-of driver.
> >> Since 1. requires adding reset-gpio support to i2c-hid-of there was
> >> very little difference left between the generic i2c-hid-of code and
> >> the specialized drivers. So I decided to merge them into the generic
> >> driver instead of having duplicate code.
> >
> > I understand the spirit, but I am not a big fan of this. The reason is
> > just detailed your following statements: getting tests on those is hard.
>
> Actually AFAIK the chromeos folks have an automated test lab where
> all supported models get tested and they regularly test the latest
> mainline kernels. So even without me asking for it any regressions
> here should have been caught in this case since support for both
> special-case i2c-hid-of drivers was added for chromeos.
>
> And the code is almost identical, the only difference is using
> the bulk-regulator API vs enabling the regulators 1 by 1, which
> should not make any difference.
Well, it's nice to know regressions, but it's nicer to know them before
we introduce them in linux-next :)
My point is if you don't manage to get tests on those devices, and we
can "guarantee" that the changes in i2c-hid-of.c will be a noop for
them, why bother merging them together? If the files have dedicated
maintainers, we should probably rely on them instead :)
>
> > So there is code duplication, yes, but OTOH this guarantees that we do
> > not break those devices while working on something else.
> >
> > I can always be convinced otherwise, but I still think the approach of
> > the devicetree-bindings maintainers works better: if you need a new
> > property that isn't available in the core of i2c-hid-of, and which is
> > device specific (even if it's just a msleep for a line to be ready),
> > make this a separate driver. Trying to parametrize everything with
> > properties will just end up in a situation where one "meaningless"
> > property will break another device, and it's going to be a pain to
> > trace, because those drivers are not tested every single kernel release.
>
> This is not trying to parametrize everything, this is trying to
> parametrize something which turns out to be necessary over 4 different
> chips/controller models. And I'm pretty sure that if I start looking
> into ACPI tables I will find many more controllers which use a reset
> GPIO + a delay after de-assert like this.
>
> IOW something which is clearly a very common pattern.
>
> You have been advocating to make HID code more generic allowing
> device-quirks in BPF format to avoid adding drivers for every
> tiny descriptor fixup.
Hehe, fair enough :) But my problem here is more who is responsible for
this code, and merging them together means more responsibility to the
i2c-hid-of.c maintainer. Having separate self-contained drivers for
handling device subtleties (when they are not generic) allows to not
break one device when fixing one other.
>
> Do you really want to go the route of a tiny driver for every
> i2c-hid chip variant used with devicetree, rather then having
> a single extra property ?
I'd like that property to be validated by Rob first. You raised the
inconsistency above, and I'd rather have an ack from him first.
Having a "this is how every i2c-hid device works" kind of argument might
make it enough for him to change his opinion (because I think that was
the argument for the post power delay).
>
> Note that if patches 1-3 had been in place when Douglas
> started adding support for the "elan,ekth6915" and
> "goodix,gt7375p" devices that the devicetree on
> the chromeos devices using those would then likely
> have simply used the "hid-descr-addr", "post-power-on-delay-ms"
> and "post-reset-deassert-delay-ms" properties and no
> separate drivers would have been necessary at all.
I think that's how the whole separate driver started. And the argument
of having separate drivers still stands, for devices that are not doing
the same things like others.
The compatible allows to store a set of device specific data that will
not change whatever board the device is placed in. So though technically
easier for the maintainers, having a dedicated property is putting the
burden on the user. While OTOH, if this property is internal API, we can
have a table with it, that says that Goodix needs X ms, when Elan Y and
Wacom Z ms.
But then, you're going to say that this requires a kernel bump, when a
device property is just on the board side, so much convenient from an
OEM point of view :(
>
> (We need patches 4-6 now only to keep compatibility with
> existing devicetree files which don't set these)
>
> So I really see patches 4-6 as a way to reduce future
> work reviewing specialized drivers for you and Jiri.
And I thank you for that :)
>
> Yes there may still be some special cases in the future
> which need a specialized driver which we have now, but
> I believe that covering the common reset-GPIO pattern
> will drastically reduce the need for those drivers and
> thus will lower the maintainer burden.
Just a small thinking here: if we keep the current compatible drivers
here, we have an example we can point people at if they need fancier
things. So maybe keeping them (or one at least) is a good thing, no?
Cheers,
Benjamin
>
> Regards,
>
> Hans
>
>
^ permalink raw reply
* Re: [PATCH 0/6] HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms + remove specialized drivers
From: Hans de Goede @ 2023-04-11 10:18 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, Douglas Anderson, linux-input
In-Reply-To: <20230411090209.gartwrkq56syerwk@mail.corp.redhat.com>
Hi Benjamin,
On 4/11/23 11:02, Benjamin Tissoires wrote:
> Hi Hans,
>
> On Apr 09 2023, Hans de Goede wrote:
>> Hi All,
>>
>> This series consist of 2 parts:
>>
>> 1. Patches 1-3. Allow using i2c-hid-of on non OF platforms to allow I2C-HID
>> devices which are not enumerated by ACPI to work on ACPI platforms
>> (by manual i2c_client instantiation using i2c_client_id matching).
>
> Patches 1 and 2 are looking good, but I wonder if you can not achieve the
> same result by relying on an ACPI SSDT override. I got something similar
> working on this thread[0].
Yes this could be made to work with an ACPI override. But the goal is
to make things work OOTB for end users when they install Linux and
ACPI overrides are very far from something which works OOTB.
> I understand the "post-reset-deassert-delay-ms" might be something hard
> to express with an SSDT, but we should already have all the bits in
> place, no?
Actually if an ACPI override is used then the setting of the GPIO
can be done in _PS0 and _PS3 (power on / off) methods and those
can simply include a sleep after setting the GPIO.
> Also, the problem of "post-reset-deassert-delay-ms" is that you are not
> documenting it, because the OF folks do not want this in device tree,
> and I tend to agree with them. So this basically creates a brand new
> undocumented property, which is less than ideal.
I'm merely not documenting it because there are no devicetree users yet.
Between the 2 currently supported of devices with a reset GPIO +
the I2C-HID capable touchscreen + wacom digitizer on the x86 android
Yoga Book 1 I'm trying to get to work that is 4 I2C-HID devices which
all follow the pattern of: 1. They have a reset GPIO, 2. they need
some delay after reset is deasserted.
It seems silly to keep adding more and more device-ids + match-data
with just the delays in there when it seems that many many I2C-HID
capable controllers/chips follow this pattern.
Also note that there already is a very similar "post-power-on-delay-ms"
property. I really don't see what makes specifying a delay after
enabling regulators through a property ok, but specifying the delay
after reset-deassert not ok. Allowing one but not the other is not
very consistent.
The reason why I'm not documenting the property now is because of
lack of current devicetree users. It can be documented once
the first DT users show up and getting it accepted should really not
be an issue given that "post-power-on-delay-ms" already exists.
Note if just the existence of the property is the main stumbling
block I can go the match_data route for the wacom digitizer on
the Yoga Book 1 too and add an extra i2c_device_id with match-data
setting the delay. This could then either be its own specialized
driver, or we could still go with the current patch-set
(minus the property) and add an i2c_device_id with match-data
to i2c-hid-of.c .
The only question then is how to name the i2c_device_id for the wacom
digitizer. It has a vid:pid of 056A:0169 So maybe "wacom0169" ?
>> 2. Patches 4-6. Remove the special i2c-hid-of-elan and i2c-hid-of-goodix
>> driver, folding the functionality into the generic i2c-hid-of driver.
>> Since 1. requires adding reset-gpio support to i2c-hid-of there was
>> very little difference left between the generic i2c-hid-of code and
>> the specialized drivers. So I decided to merge them into the generic
>> driver instead of having duplicate code.
>
> I understand the spirit, but I am not a big fan of this. The reason is
> just detailed your following statements: getting tests on those is hard.
Actually AFAIK the chromeos folks have an automated test lab where
all supported models get tested and they regularly test the latest
mainline kernels. So even without me asking for it any regressions
here should have been caught in this case since support for both
special-case i2c-hid-of drivers was added for chromeos.
And the code is almost identical, the only difference is using
the bulk-regulator API vs enabling the regulators 1 by 1, which
should not make any difference.
> So there is code duplication, yes, but OTOH this guarantees that we do
> not break those devices while working on something else.
>
> I can always be convinced otherwise, but I still think the approach of
> the devicetree-bindings maintainers works better: if you need a new
> property that isn't available in the core of i2c-hid-of, and which is
> device specific (even if it's just a msleep for a line to be ready),
> make this a separate driver. Trying to parametrize everything with
> properties will just end up in a situation where one "meaningless"
> property will break another device, and it's going to be a pain to
> trace, because those drivers are not tested every single kernel release.
This is not trying to parametrize everything, this is trying to
parametrize something which turns out to be necessary over 4 different
chips/controller models. And I'm pretty sure that if I start looking
into ACPI tables I will find many more controllers which use a reset
GPIO + a delay after de-assert like this.
IOW something which is clearly a very common pattern.
You have been advocating to make HID code more generic allowing
device-quirks in BPF format to avoid adding drivers for every
tiny descriptor fixup.
Do you really want to go the route of a tiny driver for every
i2c-hid chip variant used with devicetree, rather then having
a single extra property ?
Note that if patches 1-3 had been in place when Douglas
started adding support for the "elan,ekth6915" and
"goodix,gt7375p" devices that the devicetree on
the chromeos devices using those would then likely
have simply used the "hid-descr-addr", "post-power-on-delay-ms"
and "post-reset-deassert-delay-ms" properties and no
separate drivers would have been necessary at all.
(We need patches 4-6 now only to keep compatibility with
existing devicetree files which don't set these)
So I really see patches 4-6 as a way to reduce future
work reviewing specialized drivers for you and Jiri.
Yes there may still be some special cases in the future
which need a specialized driver which we have now, but
I believe that covering the common reset-GPIO pattern
will drastically reduce the need for those drivers and
thus will lower the maintainer burden.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 1/2] Input: cs40l26: Support for CS40L26 Boosted Haptic Amplifier
From: Charles Keepax @ 2023-04-11 9:27 UTC (permalink / raw)
To: Jeff LaBundy
Cc: Fred Treven, dmitry.torokhov, ben.bright, james.ogletree, lee,
jdelvare, joel, cy_huang, rdunlap, eajames, ping.bai, msp, arnd,
bartosz.golaszewski, linux-kernel, linux-input, patches
In-Reply-To: <ZDSqfHemG8pKj1k7@nixie71>
On Mon, Apr 10, 2023 at 07:31:56PM -0500, Jeff LaBundy wrote:
> On Mon, Apr 10, 2023 at 08:56:34AM +0000, Charles Keepax wrote:
> > On Sat, Apr 08, 2023 at 10:44:39PM -0500, Jeff LaBundy wrote:
> > I would far rather not have every single attempt to communicate
> > with the device wrapped in a retry if the communication failed
> > incase the device is hibernating. It seems much cleaner, and less
> > likely to risk odd behaviour, to know we have brought the device
> > out of hibernation.
> A common way to deal with this is that of [1], where the bus calls
> are simply wrapped with all retry logic limited to two places (read
> and write). These functions could also print the register address
> in case of failure, solving the problem of having dozens of custom
> error messages thorughout the driver.
I suspect this really comes down to a matter of taste, but my
thoughts would be that the code is shorter that way, but not
necessarily simpler. This feels far more error prone and likely
to encounter issues where the device hibernates at a time someone
hadn't properly thought through. I am far more comfortable with
the device is blocked from hibernating whilst the driver is
actively engaged with it and it keeps any special handling for
exiting hibernate in one place.
> Does the current implementation at least allow the device to hibernate
> while the system is otherwise active, as opposed to _only_ during
> runtime suspend? If so, that's still a marked improvement from L25
> era where customers rightfully pointed out that the downstream driver
> was not making efficient use of hibernation. ;)
I am not entirely sure I follow this one, yes the device can only
hibernate whilst it is runtime suspended. But I don't understand
why that is a problem being runtime resumed implies this device
is active, not the system is otherwise active. I am not sure if
I am missing your point or there is some confusion here between
runtime and system suspend. The device can only hibernate during
runtime suspend, but the only thing that determines being runtime
resumed is activity on this device so in general it shouldn't be
hibernating at that point anyway.
> I don't feel particularly strongly about it, so if the current
> implementation will stay, perhaps consider a few comments in this
> area to describe how the device's state is managed.
>
I certainly never object to adding some comments.
Thanks,
Charles
^ permalink raw reply
* Re: [PATCH 0/6] HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms + remove specialized drivers
From: Benjamin Tissoires @ 2023-04-11 9:02 UTC (permalink / raw)
To: Hans de Goede; +Cc: Jiri Kosina, Douglas Anderson, linux-input
In-Reply-To: <20230409144243.25360-1-hdegoede@redhat.com>
Hi Hans,
On Apr 09 2023, Hans de Goede wrote:
> Hi All,
>
> This series consist of 2 parts:
>
> 1. Patches 1-3. Allow using i2c-hid-of on non OF platforms to allow I2C-HID
> devices which are not enumerated by ACPI to work on ACPI platforms
> (by manual i2c_client instantiation using i2c_client_id matching).
Patches 1 and 2 are looking good, but I wonder if you can not achieve the
same result by relying on an ACPI SSDT override. I got something similar
working on this thread[0].
I understand the "post-reset-deassert-delay-ms" might be something hard
to express with an SSDT, but we should already have all the bits in
place, no?
Also, the problem of "post-reset-deassert-delay-ms" is that you are not
documenting it, because the OF folks do not want this in device tree,
and I tend to agree with them. So this basically creates a brand new
undocumented property, which is less than ideal.
>
> 2. Patches 4-6. Remove the special i2c-hid-of-elan and i2c-hid-of-goodix
> driver, folding the functionality into the generic i2c-hid-of driver.
> Since 1. requires adding reset-gpio support to i2c-hid-of there was
> very little difference left between the generic i2c-hid-of code and
> the specialized drivers. So I decided to merge them into the generic
> driver instead of having duplicate code.
I understand the spirit, but I am not a big fan of this. The reason is
just detailed your following statements: getting tests on those is hard.
So there is code duplication, yes, but OTOH this guarantees that we do
not break those devices while working on something else.
I can always be convinced otherwise, but I still think the approach of
the devicetree-bindings maintainers works better: if you need a new
property that isn't available in the core of i2c-hid-of, and which is
device specific (even if it's just a msleep for a line to be ready),
make this a separate driver. Trying to parametrize everything with
properties will just end up in a situation where one "meaningless"
property will break another device, and it's going to be a pain to
trace, because those drivers are not tested every single kernel release.
>
> Note patches 4-6 have not been actually tested with an "elan,ekth6915"
> touchscreen nor with a "goodix,gt7375p" touchscreen.
>
> Douglas, can you perhaps test this patch-set with an "elan,ekth6915"
> touchscreen and with a "goodix,gt7375p" touchscreen ?
>
> Regards,
>
> Hans
>
Cheers,
Benjamin
[0] https://lore.kernel.org/linux-input/20230308155527.jnrsowubvnk22ica@mail.corp.redhat.com/
>
> Hans de Goede (6):
> HID: i2c-hid-of: Consistenly use dev local variable in probe()
> HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms
> HID: i2c-hid-of: Add reset GPIO support to i2c-hid-of
> HID: i2c-hid-of: Add chip_data struct
> HID: i2c-hid-of: Consolidate Elan support into generic i2c-hid-of
> driver
> HID: i2c-hid-of: Consolidate Goodix support into generic i2c-hid-of
> driver
>
> drivers/hid/i2c-hid/Kconfig | 36 +------
> drivers/hid/i2c-hid/Makefile | 2 -
> drivers/hid/i2c-hid/i2c-hid-of-elan.c | 129 ------------------------
> drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 125 -----------------------
> drivers/hid/i2c-hid/i2c-hid-of.c | 124 +++++++++++++++++++----
> 5 files changed, 106 insertions(+), 310 deletions(-)
> delete mode 100644 drivers/hid/i2c-hid/i2c-hid-of-elan.c
> delete mode 100644 drivers/hid/i2c-hid/i2c-hid-of-goodix.c
>
> --
> 2.39.1
>
^ permalink raw reply
* Słowa kluczowe do wypozycjonowania
From: Adam Charachuta @ 2023-04-11 7:40 UTC (permalink / raw)
To: linux-input
Dzień dobry,
zapoznałem się z Państwa ofertą i z przyjemnością przyznaję, że przyciąga uwagę i zachęca do dalszych rozmów.
Pomyślałem, że może mógłbym mieć swój wkład w Państwa rozwój i pomóc dotrzeć z tą ofertą do większego grona odbiorców. Pozycjonuję strony www, dzięki czemu generują świetny ruch w sieci.
Możemy porozmawiać w najbliższym czasie?
Pozdrawiam
Adam Charachuta
^ permalink raw reply
* Re: [PATCH v2 3/5] arm64: dts: qcom: sdm845-xiaomi-beryllium-common: add touchscreen related nodes
From: Krzysztof Kozlowski @ 2023-04-11 5:56 UTC (permalink / raw)
To: Joel Selvaraj, Caleb Connolly, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Henrik Rydberg, Arnd Bergmann, Robert Jarzmik, Jeff LaBundy,
Markuss Broks, Jean Delvare, Job Noorman, Alistair Francis,
Chris Morgan, Hans de Goede
Cc: linux-input, devicetree, linux-kernel, linux-arm-msm,
~postmarketos/upstreaming, phone-devel
In-Reply-To: <20230410160200.57261-4-joelselvaraj.oss@gmail.com>
On 10/04/2023 18:01, Joel Selvaraj wrote:
> Enable qupv3_id_1 and gpi_dma1 as they are required for configuring
> touchscreen. Also add pinctrl configurations needed for touchscreen.
> These are common for both the tianma and ebbg touchscreen variant.
> In the subsequent patch, we will initially enable support for the focaltech
> touchscreen used in the EBBG variant. This is done in preparation for that.
>
> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
> ---
> .../qcom/sdm845-xiaomi-beryllium-common.dtsi | 39 +++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi
> index 5ed975cc6ecb..b580a32fdc3b 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi
> @@ -268,6 +268,10 @@ &gmu {
> status = "okay";
> };
>
> +&gpi_dma1 {
> + status = "okay";
> +};
> +
> &gpu {
> status = "okay";
>
> @@ -376,6 +380,10 @@ &qupv3_id_0 {
> status = "okay";
> };
>
> +&qupv3_id_1 {
> + status = "okay";
> +};
> +
> &sdhc_2 {
> status = "okay";
>
> @@ -481,6 +489,37 @@ sdc2_card_det_n: sd-card-det-n-state {
> function = "gpio";
> bias-pull-up;
> };
> +
> + ts_int_default: ts-int-default-state {
> + pins = "gpio31";
> + function = "gpio";
> + drive-strength = <16>;
> + bias-pull-down;
> + input-enable;
input-enable is not valid.
> + };
> +
> + ts_reset_default: ts-reset-default-state {
> + pins = "gpio32";
> + function = "gpio";
> + drive-strength = <16>;
> + output-high;
> + };
> +
> + ts_int_sleep: ts-int-sleep-state {
> + pins = "gpio31";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-pull-down;
> + input-enable;
input-enable is not valid.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 1/5] dt-bindings: input: touchscreen: add bindings for focaltech,fts5452
From: Krzysztof Kozlowski @ 2023-04-11 5:55 UTC (permalink / raw)
To: Joel Selvaraj, Caleb Connolly, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Henrik Rydberg, Arnd Bergmann, Robert Jarzmik, Jeff LaBundy,
Markuss Broks, Jean Delvare, Job Noorman, Alistair Francis,
Chris Morgan, Hans de Goede
Cc: linux-input, devicetree, linux-kernel, linux-arm-msm,
~postmarketos/upstreaming, phone-devel
In-Reply-To: <20230410160200.57261-2-joelselvaraj.oss@gmail.com>
On 10/04/2023 18:01, Joel Selvaraj wrote:
> Add devicetree bindings for the Focaltech FTS touchscreen drivers.
>
Subject: drop second/last, redundant "bindings for". The "dt-bindings"
prefix is already stating that these are bindings.
> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> ---
> .../input/touchscreen/focaltech,fts5452.yaml | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml b/Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml
> new file mode 100644
> index 000000000000..f42868293439
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/focaltech,fts5452.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Focaltech FTS I2C Touchscreen Controller
> +
> +maintainers:
> + - Joel Selvaraj <joelselvaraj.oss@gmail.com>
> + - Caleb Connolly <caleb@connolly.tech>
> +
> +allOf:
> + - $ref: touchscreen.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - focaltech,fts5452
> + - focaltech,fts8719
> +
> + reg:
> + const: 0x38
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> +
> + avdd-supply:
> + description: regulator supplying analog power (2.6V to 3.3V).
> +
> + vddio-supply:
> + description: regulator supplying IO power (1.8V).
> +
> +unevaluatedProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - touchscreen-size-x
> + - touchscreen-size-y
We always put required: before unevaluatedProperties. Base your schema
on example-schema.yaml.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v2 2/2] Input: xpad - fix PowerA EnWired Controller guide button
From: Vicki Pfau @ 2023-04-11 3:16 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input; +Cc: Vicki Pfau, Pavel Rojtberg
In-Reply-To: <20230411031650.960322-1-vi@endrift.com>
This commit explicitly disables the audio interface the same way the official
driver does. This is needed for some controllers, such as the PowerA Enhanced
Wired Controller for Series X|S (0x20d6:0x200e) to report the guide button.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/input/joystick/xpad.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 6ea9c10dfb8a..175fcba7f92d 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1396,6 +1396,16 @@ static int xpad_start_xbox_one(struct usb_xpad *xpad)
unsigned long flags;
int retval;
+ if (usb_ifnum_to_if(xpad->udev, GIP_WIRED_INTF_AUDIO)) {
+ /* Explicitly disable the audio interface. This is needed for some
+ * controllers, such as the PowerA Enhanced Wired Controller
+ * for Series X|S (0x20d6:0x200e) to report the guide button */
+ retval = usb_set_interface(xpad->udev, GIP_WIRED_INTF_AUDIO, 0);
+ if (retval)
+ dev_warn(&xpad->dev->dev,
+ "unable to disable audio interface: %d\n", retval);
+ }
+
spin_lock_irqsave(&xpad->odata_lock, flags);
/*
--
2.40.0
^ permalink raw reply related
* [PATCH v2 1/2] Input: xpad - Add constants for GIP interface numbers
From: Vicki Pfau @ 2023-04-11 3:16 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input; +Cc: Vicki Pfau, Pavel Rojtberg
In-Reply-To: <20230411031650.960322-1-vi@endrift.com>
Wired GIP devices present multiple interfaces with the same USB identification
other than the interface number. This adds constants for differentiating two of
them and uses them where appropriate
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/input/joystick/xpad.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 260f91fef427..6ea9c10dfb8a 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -561,6 +561,9 @@ struct xboxone_init_packet {
#define GIP_MOTOR_LT BIT(3)
#define GIP_MOTOR_ALL (GIP_MOTOR_R | GIP_MOTOR_L | GIP_MOTOR_RT | GIP_MOTOR_LT)
+#define GIP_WIRED_INTF_DATA 0
+#define GIP_WIRED_INTF_AUDIO 1
+
/*
* This packet is required for all Xbox One pads with 2015
* or later firmware installed (or present from the factory).
@@ -2004,7 +2007,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
}
if (xpad->xtype == XTYPE_XBOXONE &&
- intf->cur_altsetting->desc.bInterfaceNumber != 0) {
+ intf->cur_altsetting->desc.bInterfaceNumber != GIP_WIRED_INTF_DATA) {
/*
* The Xbox One controller lists three interfaces all with the
* same interface class, subclass and protocol. Differentiate by
--
2.40.0
^ permalink raw reply related
* [PATCH v2 0/2] Improve GIP support
From: Vicki Pfau @ 2023-04-11 3:16 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input; +Cc: Vicki Pfau, Pavel Rojtberg
This series contains a new version of the previously submitted "fix PowerA
EnWired Controller guide button" patch to make the failure soft instead of hard
as well as making sure the interface exists before trying to disable it, as
well as a further patch to add (and use) constants for the interface names,
based on information gleaned from the xone project.
Vicki Pfau (2):
Input: xpad - Add constants for GIP interface numbers
Input: xpad - fix PowerA EnWired Controller guide button
drivers/input/joystick/xpad.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
--
2.40.0
^ permalink raw reply
* [dtor-input:for-linus] BUILD SUCCESS b3d80fd27a3c2d8715a40cbf876139b56195f162
From: kernel test robot @ 2023-04-11 2:46 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: b3d80fd27a3c2d8715a40cbf876139b56195f162 Input: pegasus-notetaker - check pipe type when probing
elapsed time: 1445m
configs tested: 167
configs skipped: 8
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-r002-20230410 gcc
alpha randconfig-r013-20230410 gcc
alpha randconfig-r034-20230409 gcc
alpha randconfig-r034-20230410 gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-r015-20230409 gcc
arc randconfig-r043-20230409 gcc
arc randconfig-r043-20230410 gcc
arc tb10x_defconfig gcc
arm allmodconfig gcc
arm allyesconfig gcc
arm defconfig gcc
arm ep93xx_defconfig clang
arm mvebu_v7_defconfig gcc
arm omap2plus_defconfig gcc
arm pxa_defconfig gcc
arm randconfig-r012-20230409 clang
arm randconfig-r012-20230410 clang
arm randconfig-r021-20230409 clang
arm randconfig-r033-20230410 gcc
arm randconfig-r046-20230409 clang
arm randconfig-r046-20230410 clang
arm s3c6400_defconfig gcc
arm sunxi_defconfig gcc
arm64 alldefconfig gcc
arm64 allyesconfig gcc
arm64 defconfig gcc
arm64 randconfig-r031-20230409 clang
csky buildonly-randconfig-r006-20230410 gcc
csky defconfig gcc
csky randconfig-r002-20230409 gcc
csky randconfig-r006-20230409 gcc
csky randconfig-r022-20230410 gcc
csky randconfig-r025-20230410 gcc
hexagon randconfig-r011-20230409 clang
hexagon randconfig-r041-20230409 clang
hexagon randconfig-r041-20230410 clang
hexagon randconfig-r045-20230409 clang
hexagon randconfig-r045-20230410 clang
i386 allyesconfig gcc
i386 buildonly-randconfig-r004-20230410 clang
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-a001-20230410 clang
i386 randconfig-a002-20230410 clang
i386 randconfig-a003-20230410 clang
i386 randconfig-a004-20230410 clang
i386 randconfig-a005-20230410 clang
i386 randconfig-a006-20230410 clang
i386 randconfig-a011-20230410 gcc
i386 randconfig-a012-20230410 gcc
i386 randconfig-a013-20230410 gcc
i386 randconfig-a014-20230410 gcc
i386 randconfig-a015-20230410 gcc
i386 randconfig-a016-20230410 gcc
ia64 allmodconfig gcc
ia64 defconfig gcc
ia64 tiger_defconfig gcc
ia64 zx1_defconfig gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-r003-20230410 gcc
m68k allmodconfig gcc
m68k defconfig gcc
m68k randconfig-r012-20230409 gcc
m68k randconfig-r023-20230410 gcc
m68k randconfig-r032-20230409 gcc
microblaze defconfig gcc
microblaze randconfig-r032-20230410 gcc
microblaze randconfig-r033-20230410 gcc
microblaze randconfig-r035-20230409 gcc
microblaze randconfig-r036-20230410 gcc
mips allyesconfig gcc
mips buildonly-randconfig-r003-20230410 gcc
mips ip22_defconfig clang
mips ip27_defconfig clang
mips randconfig-r001-20230409 gcc
mips randconfig-r011-20230410 clang
mips randconfig-r031-20230410 gcc
mips rs90_defconfig clang
nios2 buildonly-randconfig-r001-20230410 gcc
nios2 buildonly-randconfig-r005-20230410 gcc
nios2 defconfig gcc
nios2 randconfig-r036-20230409 gcc
openrisc randconfig-r003-20230409 gcc
openrisc randconfig-r004-20230409 gcc
openrisc randconfig-r023-20230409 gcc
openrisc randconfig-r031-20230410 gcc
parisc buildonly-randconfig-r001-20230409 gcc
parisc buildonly-randconfig-r003-20230409 gcc
parisc defconfig gcc
parisc randconfig-r005-20230409 gcc
parisc randconfig-r014-20230409 gcc
parisc randconfig-r021-20230410 gcc
parisc randconfig-r022-20230409 gcc
parisc randconfig-r034-20230409 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc eiger_defconfig gcc
powerpc mpc836x_mds_defconfig clang
powerpc pasemi_defconfig gcc
powerpc randconfig-r016-20230409 gcc
powerpc randconfig-r016-20230410 gcc
powerpc randconfig-r033-20230409 clang
powerpc skiroot_defconfig clang
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv defconfig gcc
riscv randconfig-r014-20230410 gcc
riscv randconfig-r042-20230409 gcc
riscv randconfig-r042-20230410 gcc
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 buildonly-randconfig-r002-20230410 gcc
s390 buildonly-randconfig-r005-20230409 gcc
s390 defconfig gcc
s390 randconfig-r013-20230409 gcc
s390 randconfig-r024-20230409 gcc
s390 randconfig-r025-20230409 gcc
s390 randconfig-r044-20230409 gcc
s390 randconfig-r044-20230410 gcc
sh allmodconfig gcc
sh ap325rxa_defconfig gcc
sh edosk7760_defconfig gcc
sh randconfig-r011-20230409 gcc
sh rsk7264_defconfig gcc
sh se7750_defconfig gcc
sparc defconfig gcc
sparc randconfig-r001-20230410 gcc
sparc randconfig-r015-20230410 gcc
sparc randconfig-r032-20230409 gcc
sparc randconfig-r033-20230409 gcc
sparc randconfig-r035-20230409 gcc
sparc64 buildonly-randconfig-r006-20230409 gcc
sparc64 randconfig-r036-20230410 gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 defconfig gcc
x86_64 kexec gcc
x86_64 randconfig-a001-20230410 clang
x86_64 randconfig-a002-20230410 clang
x86_64 randconfig-a003-20230410 clang
x86_64 randconfig-a004-20230410 clang
x86_64 randconfig-a005-20230410 clang
x86_64 randconfig-a006-20230410 clang
x86_64 randconfig-a011-20230410 gcc
x86_64 randconfig-a012-20230410 gcc
x86_64 randconfig-a013-20230410 gcc
x86_64 randconfig-a014-20230410 gcc
x86_64 randconfig-a015-20230410 gcc
x86_64 randconfig-a016-20230410 gcc
x86_64 randconfig-k001 clang
x86_64 randconfig-r006-20230410 clang
x86_64 rhel-8.3 gcc
xtensa buildonly-randconfig-r002-20230409 gcc
xtensa randconfig-r005-20230410 gcc
xtensa randconfig-r024-20230410 gcc
xtensa randconfig-r034-20230410 gcc
xtensa randconfig-r035-20230410 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply
* Re: [PATCH 1/2] Input: cs40l26: Support for CS40L26 Boosted Haptic Amplifier
From: Jeff LaBundy @ 2023-04-11 0:31 UTC (permalink / raw)
To: Charles Keepax
Cc: Fred Treven, dmitry.torokhov, ben.bright, james.ogletree, lee,
jdelvare, joel, cy_huang, rdunlap, eajames, ping.bai, msp, arnd,
bartosz.golaszewski, linux-kernel, linux-input, patches
In-Reply-To: <20230410085634.GV68926@ediswmail.ad.cirrus.com>
Hi Charles,
Thank you for the additional follow-up.
On Mon, Apr 10, 2023 at 08:56:34AM +0000, Charles Keepax wrote:
> On Sat, Apr 08, 2023 at 10:44:39PM -0500, Jeff LaBundy wrote:
>
> Hi Jeff,
>
> Thanks for the detailed review, very much appreciated. I agree
> with most of your comments, save a couple of exceptions below,
> well and one that I just felt the need to agree with you
> explictly:
>
> > > +static bool cs40l26_readable_reg(struct device *dev, unsigned int reg)
> > > +{
> > > + switch (reg) {
> > > + case CS40L26_DEVID ... CS40L26_REVID:
> > > + case CS40L26_TEST_KEY_CTRL:
> > > + case CS40L26_GLOBAL_ENABLES:
> > > + case CS40L26_ERROR_RELEASE:
> > > + case CS40L26_PWRMGT_CTL ... CS40L26_PWRMGT_STS:
> > > + case CS40L26_REFCLK_INPUT:
> > > + case CS40L26_PLL_REFCLK_DETECT_0:
> > > + case CS40L26_VBST_CTL_1 ... CS40L26_BST_IPK_CTL:
> > > + case CS40L26_TEST_LBST:
> > > + case CS40L26_NGATE1_INPUT:
> > > + case CS40L26_DAC_MSM_CONFIG ... CS40L26_TST_DAC_MSM_CONFIG:
> > > + case CS40L26_IRQ1_STATUS:
> > > + case CS40L26_IRQ1_EINT_1 ... CS40L26_IRQ1_EINT_5:
> > > + case CS40L26_IRQ1_STS_1 ... CS40L26_IRQ1_STS_5:
> > > + case CS40L26_IRQ1_MASK_1 ... CS40L26_IRQ1_MASK_5:
> > > + case CS40L26_MIXER_NGATE_CH1_CFG:
> > > + case CS40L26_DSP_MBOX_1 ... CS40L26_DSP_VIRTUAL2_MBOX_8:
> > > + case CS40L26_OTP_MEM0 ... CS40L26_OTP_MEM31:
> > > + case CS40L26_DSP1_XMEM_PACKED_0 ... CS40L26_DSP1_XMEM_PACKED_6143:
> > > + case CS40L26_DSP1_XROM_PACKED_0 ... CS40L26_DSP1_XROM_PACKED_4604:
> > > + case CS40L26_DSP1_XMEM_UNPACKED32_0 ... CS40L26_DSP1_XROM_UNPACKED32_3070:
> > > + case CS40L26_DSP1_SYS_INFO_ID:
> > > + case CS40L26_DSP1_XMEM_UNPACKED24_0 ... CS40L26_DSP1_XMEM_UNPACKED24_8191:
> > > + case CS40L26_DSP1_XROM_UNPACKED24_0 ... CS40L26_DSP1_XROM_UNPACKED24_6141:
> > > + case CS40L26_DSP1_CCM_CORE_CONTROL:
> > > + case CS40L26_DSP1_YMEM_PACKED_0 ... CS40L26_DSP1_YMEM_PACKED_1532:
> > > + case CS40L26_DSP1_YMEM_UNPACKED32_0 ... CS40L26_DSP1_YMEM_UNPACKED32_1022:
> > > + case CS40L26_DSP1_YMEM_UNPACKED24_0 ... CS40L26_DSP1_YMEM_UNPACKED24_2045:
> > > + case CS40L26_DSP1_PMEM_0 ... CS40L26_DSP1_PMEM_5114:
> > > + case CS40L26_DSP1_PROM_0 ... CS40L26_DSP1_PROM_30714:
> > > + return true;
> > > + default:
> > > + return false;
> > > + }
> >
> > Is this function necessary? Are there cases throughout the driver that may
> > attempt to read an illegal register, and you depend on the regmap call to
> > fail? If not, I think you can just drop this function.
> >
>
> I would much rather we kept this one, at the very least such
> that the debugfs representation of the regmap shows the correct
> registers.
Ah, that's a good point; with that in mind, I agree.
>
> > > +static inline void cs40l26_pm_runtime_setup(struct cs40l26_private *cs40l26)
> > > +{
> > > + pm_runtime_mark_last_busy(cs40l26->dev);
> > > + pm_runtime_use_autosuspend(cs40l26->dev);
> > > + pm_runtime_set_autosuspend_delay(cs40l26->dev, CS40L26_AUTOSUSPEND_DELAY_MS);
> > > + pm_runtime_enable(cs40l26->dev);
> >
> > My first impression was that this driver is doing an uncommonly large
> > amount of PM-related housekeeping. In fact, not a single haptic driver
> > in mainline is calling any of these. What is unique about this device
> > that calls for this much overhead?
> >
> > Can the device wake up from hibernation (AoH?) from both control port
> > and GPIO triggers? If so, why not to simply allow the device to hibernate
> > at its own discretion and avoid all PM-related housekeeping? Stated
> > another way, it seems some of the DSP's job is unnecessarily duplicated
> > in the kernel.
> >
> > In case I have misunderstood, please let me know.
> >
>
> I would far rather not have every single attempt to communicate
> with the device wrapped in a retry if the communication failed
> incase the device is hibernating. It seems much cleaner, and less
> likely to risk odd behaviour, to know we have brought the device
> out of hibernation.
A common way to deal with this is that of [1], where the bus calls
are simply wrapped with all retry logic limited to two places (read
and write). These functions could also print the register address
in case of failure, solving the problem of having dozens of custom
error messages thorughout the driver.
In my mind that is simpler, reduces the number of instances that
keep track of the device's state from two (kernel and DSP FW) to
one (DSP FW), and removes the onslaught of PM-related housekeeping
that doesn't seem to be required by other input/misc devices.
Does the current implementation at least allow the device to hibernate
while the system is otherwise active, as opposed to _only_ during
runtime suspend? If so, that's still a marked improvement from L25
era where customers rightfully pointed out that the downstream driver
was not making efficient use of hibernation. ;)
I don't feel particularly strongly about it, so if the current
implementation will stay, perhaps consider a few comments in this
area to describe how the device's state is managed.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/iqs5xx.c#n158
>
> > > +static int cs40l26_irq_update_mask(struct cs40l26_private *cs40l26, u32 reg, u32 val, u32 bit_mask)
> > > +{
> > > + u32 eint_reg, cur_mask, new_mask;
> > > + int ret;
> > > +
> > > + if (reg == CS40L26_IRQ1_MASK_1) {
> > > + eint_reg = CS40L26_IRQ1_EINT_1;
> > > + } else if (reg == CS40L26_IRQ1_MASK_2) {
> > > + eint_reg = CS40L26_IRQ1_EINT_2;
> > > + } else {
> > > + dev_err(cs40l26->dev, "Invalid IRQ mask reg: 0x%08X\n", reg);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = regmap_read(cs40l26->regmap, reg, &cur_mask);
> > > + if (ret) {
> > > + dev_err(cs40l26->dev, "Failed to get IRQ mask\n");
> >
> > Having a custom error message for every possible failed register read
> > does not ultimately aid in debugging and unnecessarily grows the size
> > of the driver.
> >
> > If a specific message is absolutely necessary, then add wrappers around
> > regmap_read/write and print the failed address. However, this does not
> > seem necessary either. Simply propagate the error code all the way up
> > to the caller, whether it is probe or a sysfs attribute.
> >
> > Stated another way:
> >
> > error = regmap_...(...);
> > if (error)
> > return error;
> >
>
> Not sure I feel super strongly on this one, but I do find when
> debugging issues on drivers that do this that I usually end up
> adding the printks back in.
>
> > > +static int cs40l26_dsp_state_get(struct cs40l26_private *cs40l26, u8 *state)
> > > +{
> > > + bool mutex_available = !mutex_is_locked(&cs40l26->dsp.pwr_lock);
> >
> > This is dangerous and a sign that locks are not properly managed. What would
> > be a case where you do not know the state of the lock upon entering this function?
> > If you do not know whether the mutex is locked inside this function, it is not the
> > proper place to grab it.
> >
>
> Yes I whole heartedly agree here this should not be done this
> way.
>
> > > + cs40l26->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> > > + if (IS_ERR_OR_NULL(cs40l26->reset_gpio)) {
> > > + dev_err(dev, "Failed to get reset GPIO: %d\n", (int) PTR_ERR(cs40l26->reset_gpio));
> > > + ret = -ENOENT;
> > > + goto err;
> > > + }
> > > +
> > > + usleep_range(CS40L26_MIN_RESET_PULSE_WIDTH_US, CS40L26_MIN_RESET_PULSE_WIDTH_US + 100);
> > > +
> > > + gpiod_set_value_cansleep(cs40l26->reset_gpio, 1);
> >
> > This is backwards, which means your dts must be backwards as well. gpiod is
> > a logical API, so you should actually declare the GPIO as GPIOD_OUT_HIGH, i.e.
> > asserted, and then call gpiod_set_value_cansleep() with 0, i.e. de-asserted.
> > The definition of "asserted" then comes from the polarity defined in dts.
> >
> > By the way, did you test this driver without a reset GPIO defined in dts? It's
> > an optional GPIO, rightfully so, but you need to check whether reset_gpio is
> > NULL prior to acting upon it.
> >
>
> gpiod does the NULL check for you no need to duplicate it in the
> driver.
Great catch; you're right.
>
> Thanks,
> Charles
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [PATCH] input: touchscreen: edt-ft5x06: select REGMAP_I2C
From: Dmitry Torokhov @ 2023-04-10 23:55 UTC (permalink / raw)
To: Daniel Golle
Cc: linux-input, linux-kernel, Arnd Bergmann, Robert Jarzmik,
Jeff LaBundy, Maxime Ripard, Max Krummenacher, Alistair Francis,
Jean Delvare, Job Noorman, Chris Morgan, Hans de Goede,
Dario Binacchi
In-Reply-To: <ZDRBExF1xmxalMZc@makrotopia.org>
On Mon, Apr 10, 2023 at 06:02:11PM +0100, Daniel Golle wrote:
> After starting to use regmap API to access registers the edt-ft5x06
> driver depends on symbols provided by REGMAP_I2C:
>
> edt-ft5x06.o: in function `edt_ft5x06_ts_probe':
> edt-ft5x06.c:1154: undefined reference to `__regmap_init_i2c'
> edt-ft5x06.o: in function `edt_ft5x06_ts_identify':
> edt-ft5x06.c:897: undefined reference to `__regmap_init_i2c'
>
> Make sure support for I2C regmap is actually selected by adding this
> dependency to Kconfig.
>
> Fixes: 9dfd9708ffba ("Input: edt-ft5x06 - convert to use regmap API")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* [PATCH v2 1/1] HID: shield: Initial driver implementation with Thunderstrike support
From: Rahul Rameshbabu @ 2023-04-10 17:08 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Rahul Rameshbabu
In-Reply-To: <20230410170840.16119-1-rrameshbabu@nvidia.com>
Supports the Thunderstrike (SHIELD 2017) controller. Implements support for
the Thunderstrike HOSTCMD firmware interface. Adds sysfs attributes about a
SHIELD device and introduces haptics support for controllers.
Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
---
MAINTAINERS | 6 +
drivers/hid/Kconfig | 18 ++
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 3 +
drivers/hid/hid-shield.c | 587 +++++++++++++++++++++++++++++++++++++++
5 files changed, 615 insertions(+)
create mode 100644 drivers/hid/hid-shield.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2b073facf399..4fa673401bc7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9122,6 +9122,12 @@ F: drivers/hid/hid-sensor-*
F: drivers/iio/*/hid-*
F: include/linux/hid-sensor-*
+HID SHIELD DRIVER
+M: Rahul Rameshbabu <rrameshbabu@nvidia.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-shield.c
+
HID VRC-2 CAR CONTROLLER DRIVER
M: Marcus Folkesson <marcus.folkesson@gmail.com>
L: linux-input@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 82f64fb31fda..eb19debaa1f5 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -990,6 +990,24 @@ config HID_SEMITEK
- Woo-dy
- X-Bows Nature/Knight
+config HID_SHIELD
+ tristate "SHIELD HID Driver"
+ depends on USB_HID
+ depends on BT_HIDP
+ help
+ Support for NVIDIA SHIELD accessories.
+
+ Supported devices:
+ - Thunderstrike (NVIDIA SHIELD Controller 2017)
+
+config SHIELD_FF
+ bool "SHIELD force feedback support"
+ depends on HID_SHIELD
+ select INPUT_FF_MEMLESS
+ help
+ Say Y here if you would like to enable force feedback support for
+ NVIDIA SHIELD accessories with haptics capabilities.
+
config HID_SIGMAMICRO
tristate "SiGma Micro-based keyboards"
depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 5d37cacbde33..a2ed1db9ed9d 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_HID_RMI) += hid-rmi.o
obj-$(CONFIG_HID_SAITEK) += hid-saitek.o
obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
obj-$(CONFIG_HID_SEMITEK) += hid-semitek.o
+obj-$(CONFIG_HID_SHIELD) += hid-shield.o
obj-$(CONFIG_HID_SIGMAMICRO) += hid-sigmamicro.o
obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
obj-$(CONFIG_HID_SONY) += hid-sony.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 63545cd307e5..4000d53b1bac 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -993,6 +993,9 @@
#define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_18 0x0014
#define USB_DEVICE_ID_NTRIG_DUOSENSE 0x1500
+#define USB_VENDOR_ID_NVIDIA 0x0955
+#define USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER 0x7214
+
#define USB_VENDOR_ID_ONTRAK 0x0a07
#define USB_DEVICE_ID_ONTRAK_ADU100 0x0064
diff --git a/drivers/hid/hid-shield.c b/drivers/hid/hid-shield.c
new file mode 100644
index 000000000000..3befa8f133c7
--- /dev/null
+++ b/drivers/hid/hid-shield.c
@@ -0,0 +1,587 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * HID driver for NVIDIA SHIELD peripherals.
+ */
+
+#include <linux/hid.h>
+#include <linux/input-event-codes.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+
+#include "hid-ids.h"
+
+#define NOT_INIT_STR "NOT INITIALIZED"
+
+enum {
+ SHIELD_FW_VERSION_INITIALIZED = 0,
+ SHIELD_BOARD_INFO_INITIALIZED,
+};
+
+enum {
+ THUNDERSTRIKE_FW_VERSION_UPDATE = 0,
+ THUNDERSTRIKE_BOARD_INFO_UPDATE,
+ THUNDERSTRIKE_HAPTICS_UPDATE,
+};
+
+enum {
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE = 33,
+ THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID = 0x4,
+ THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID = 0x3,
+};
+
+enum {
+ THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION = 1,
+ THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO = 16,
+ THUNDERSTRIKE_HOSTCMD_ID_USB_INIT = 53,
+ THUNDERSTRIKE_HOSTCMD_ID_HAPTICS = 57,
+ THUNDERSTRIKE_HOSTCMD_ID_BLUETOOTH_INIT = 58,
+};
+
+struct thunderstrike_hostcmd_board_info {
+ __le16 revision;
+ __le16 serial[7];
+};
+
+struct thunderstrike_hostcmd_haptics {
+ u8 motor_left;
+ u8 motor_right;
+};
+
+struct thunderstrike_hostcmd_resp_report {
+ u8 report_id; /* THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID */
+ u8 cmd_id;
+ u8 reserved_at_10;
+
+ union {
+ struct thunderstrike_hostcmd_board_info board_info;
+ struct thunderstrike_hostcmd_haptics motors;
+ __le16 fw_version;
+ u8 payload[30];
+ };
+} __packed;
+static_assert(sizeof(struct thunderstrike_hostcmd_resp_report) ==
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE);
+
+struct thunderstrike_hostcmd_req_report {
+ u8 report_id; /* THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID */
+ u8 cmd_id;
+ u8 reserved_at_10;
+
+ struct {
+ u8 update;
+ struct thunderstrike_hostcmd_haptics motors;
+ } haptics;
+ u8 reserved_at_30[27];
+} __packed;
+static_assert(sizeof(struct thunderstrike_hostcmd_req_report) ==
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE);
+
+/* Common struct for shield accessories. */
+struct shield_device {
+ struct hid_device *hdev;
+
+ unsigned long initialized_flags;
+ const char *codename;
+ u16 fw_version;
+ struct {
+ u16 revision;
+ char serial_number[15];
+ } board_info;
+};
+
+struct thunderstrike {
+ struct shield_device base;
+
+ /* Sub-devices */
+ struct input_dev *haptics_dev;
+
+ /* Resources */
+ void *req_report_dmabuf;
+ unsigned long update_flags;
+ struct thunderstrike_hostcmd_haptics haptics_val;
+ spinlock_t haptics_update_lock;
+ struct work_struct hostcmd_req_work;
+};
+
+static inline void thunderstrike_hostcmd_req_report_init(
+ struct thunderstrike_hostcmd_req_report *report, u8 cmd_id)
+{
+ memset(report, 0, sizeof(*report));
+ report->report_id = THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID;
+ report->cmd_id = cmd_id;
+}
+
+static inline void shield_strrev(char *dest, size_t len, u16 rev)
+{
+ dest[0] = ('A' - 1) + (rev >> 8);
+ snprintf(&dest[1], len - 1, "%02X", 0xff & rev);
+}
+
+static struct input_dev *shield_allocate_input_dev(struct hid_device *hdev,
+ const char *name_suffix)
+{
+ struct input_dev *idev;
+
+ idev = input_allocate_device();
+ if (!idev)
+ goto err_device;
+
+ idev->id.bustype = hdev->bus;
+ idev->id.vendor = hdev->vendor;
+ idev->id.product = hdev->product;
+ idev->id.version = hdev->version;
+ idev->uniq = hdev->uniq;
+ idev->name = devm_kasprintf(&idev->dev, GFP_KERNEL, "%s %s", hdev->name,
+ name_suffix);
+ if (!idev->name)
+ goto err_name;
+
+ input_set_drvdata(idev, hdev);
+
+ return idev;
+
+err_name:
+ input_free_device(idev);
+err_device:
+ return ERR_PTR(-ENOMEM);
+}
+
+static struct input_dev *shield_haptics_create(
+ struct shield_device *dev,
+ int (*play_effect)(struct input_dev *, void *, struct ff_effect *))
+{
+ struct input_dev *haptics;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_SHIELD_FF))
+ return NULL;
+
+ haptics = shield_allocate_input_dev(dev->hdev, "Haptics");
+ if (IS_ERR(haptics))
+ return haptics;
+
+ input_set_capability(haptics, EV_FF, FF_RUMBLE);
+ input_ff_create_memless(haptics, NULL, play_effect);
+
+ ret = input_register_device(haptics);
+ if (ret)
+ goto err;
+
+ return haptics;
+
+ err:
+ input_free_device(haptics);
+ return ERR_PTR(ret);
+}
+
+static inline void thunderstrike_send_hostcmd_request(struct thunderstrike *ts)
+{
+ struct thunderstrike_hostcmd_req_report *report = ts->req_report_dmabuf;
+ struct shield_device *shield_dev = &ts->base;
+ int ret;
+
+ ret = hid_hw_raw_request(shield_dev->hdev, report->report_id,
+ ts->req_report_dmabuf,
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE,
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+
+ if (ret < 0) {
+ hid_err(shield_dev->hdev,
+ "Failed to output Thunderstrike HOSTCMD request HID report due to %pe\n",
+ ERR_PTR(ret));
+ }
+}
+
+static void thunderstrike_hostcmd_req_work_handler(struct work_struct *work)
+{
+ struct thunderstrike *ts =
+ container_of(work, struct thunderstrike, hostcmd_req_work);
+ struct thunderstrike_hostcmd_req_report *report;
+ unsigned long flags;
+
+ report = ts->req_report_dmabuf;
+
+ if (test_and_clear_bit(THUNDERSTRIKE_FW_VERSION_UPDATE, &ts->update_flags)) {
+ thunderstrike_hostcmd_req_report_init(
+ report, THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION);
+ thunderstrike_send_hostcmd_request(ts);
+ }
+
+ if (test_and_clear_bit(THUNDERSTRIKE_BOARD_INFO_UPDATE, &ts->update_flags)) {
+ thunderstrike_hostcmd_req_report_init(
+ report, THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO);
+ thunderstrike_send_hostcmd_request(ts);
+ }
+
+ if (test_and_clear_bit(THUNDERSTRIKE_HAPTICS_UPDATE, &ts->update_flags)) {
+ thunderstrike_hostcmd_req_report_init(
+ report, THUNDERSTRIKE_HOSTCMD_ID_HAPTICS);
+
+ report->haptics.update = 1;
+ spin_lock_irqsave(&ts->haptics_update_lock, flags);
+ report->haptics.motors = ts->haptics_val;
+ spin_unlock_irqrestore(&ts->haptics_update_lock, flags);
+
+ thunderstrike_send_hostcmd_request(ts);
+ }
+}
+
+static inline void thunderstrike_request_firmware_version(struct thunderstrike *ts)
+{
+ set_bit(THUNDERSTRIKE_FW_VERSION_UPDATE, &ts->update_flags);
+ schedule_work(&ts->hostcmd_req_work);
+}
+
+static inline void thunderstrike_request_board_info(struct thunderstrike *ts)
+{
+ set_bit(THUNDERSTRIKE_BOARD_INFO_UPDATE, &ts->update_flags);
+ schedule_work(&ts->hostcmd_req_work);
+}
+
+static inline int
+thunderstrike_update_haptics(struct thunderstrike *ts,
+ struct thunderstrike_hostcmd_haptics *motors)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&ts->haptics_update_lock, flags);
+ ts->haptics_val = *motors;
+ spin_unlock_irqrestore(&ts->haptics_update_lock, flags);
+
+ set_bit(THUNDERSTRIKE_HAPTICS_UPDATE, &ts->update_flags);
+ schedule_work(&ts->hostcmd_req_work);
+
+ return 0;
+}
+
+static int thunderstrike_play_effect(struct input_dev *idev, void *data,
+ struct ff_effect *effect)
+{
+ struct hid_device *hdev = input_get_drvdata(idev);
+ struct thunderstrike_hostcmd_haptics motors;
+ struct shield_device *shield_dev;
+ struct thunderstrike *ts;
+
+ if (effect->type != FF_RUMBLE)
+ return 0;
+
+ shield_dev = hid_get_drvdata(hdev);
+ ts = container_of(shield_dev, struct thunderstrike, base);
+
+ /* Thunderstrike motor values range from 0 to 32 inclusively */
+ motors.motor_left = effect->u.rumble.strong_magnitude / 2047;
+ motors.motor_right = effect->u.rumble.weak_magnitude / 2047;
+
+ hid_dbg(hdev, "Thunderstrike FF_RUMBLE request, left: %u right: %u\n",
+ motors.motor_left, motors.motor_right);
+
+ return thunderstrike_update_haptics(ts, &motors);
+}
+
+static void
+thunderstrike_parse_fw_version_payload(struct shield_device *shield_dev,
+ __le16 fw_version)
+{
+ shield_dev->fw_version = le16_to_cpu(fw_version);
+
+ set_bit(SHIELD_FW_VERSION_INITIALIZED, &shield_dev->initialized_flags);
+
+ hid_dbg(shield_dev->hdev, "Thunderstrike firmware version 0x%04X\n",
+ shield_dev->fw_version);
+}
+
+static void
+thunderstrike_parse_board_info_payload(struct shield_device *shield_dev,
+ struct thunderstrike_hostcmd_board_info *board_info)
+{
+ char board_revision_str[4];
+ int i;
+
+ shield_dev->board_info.revision = le16_to_cpu(board_info->revision);
+ for (i = 0; i < 7; ++i) {
+ u16 val = le16_to_cpu(board_info->serial[i]);
+
+ shield_dev->board_info.serial_number[2 * i] = val & 0xFF;
+ shield_dev->board_info.serial_number[2 * i + 1] = val >> 8;
+ }
+ shield_dev->board_info.serial_number[14] = '\0';
+
+ set_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags);
+
+ shield_strrev(board_revision_str, 4, shield_dev->board_info.revision);
+ hid_dbg(shield_dev->hdev,
+ "Thunderstrike BOARD_REVISION_%s (0x%04X) S/N: %s\n",
+ board_revision_str, shield_dev->board_info.revision,
+ shield_dev->board_info.serial_number);
+}
+
+static inline void
+thunderstrike_parse_haptics_payload(struct shield_device *shield_dev,
+ struct thunderstrike_hostcmd_haptics *haptics)
+{
+ hid_dbg(shield_dev->hdev,
+ "Thunderstrike haptics HOSTCMD response, left: %u right: %u\n",
+ haptics->motor_left, haptics->motor_right);
+}
+
+static int thunderstrike_parse_report(struct shield_device *shield_dev,
+ struct hid_report *report, u8 *data,
+ int size)
+{
+ struct thunderstrike_hostcmd_resp_report *hostcmd_resp_report;
+ struct thunderstrike *ts =
+ container_of(shield_dev, struct thunderstrike, base);
+ struct hid_device *hdev = shield_dev->hdev;
+
+ switch (report->id) {
+ case THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID:
+ if (size != THUNDERSTRIKE_HOSTCMD_REPORT_SIZE) {
+ hid_err(hdev,
+ "Encountered Thunderstrike HOSTCMD HID report with unexpected size %d\n",
+ size);
+ return -EINVAL;
+ }
+
+ hostcmd_resp_report =
+ (struct thunderstrike_hostcmd_resp_report *)data;
+
+ switch (hostcmd_resp_report->cmd_id) {
+ case THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION:
+ thunderstrike_parse_fw_version_payload(
+ shield_dev, hostcmd_resp_report->fw_version);
+ break;
+ case THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO:
+ thunderstrike_parse_board_info_payload(
+ shield_dev, &hostcmd_resp_report->board_info);
+ break;
+ case THUNDERSTRIKE_HOSTCMD_ID_HAPTICS:
+ thunderstrike_parse_haptics_payload(
+ shield_dev, &hostcmd_resp_report->motors);
+ break;
+
+ case THUNDERSTRIKE_HOSTCMD_ID_USB_INIT:
+ case THUNDERSTRIKE_HOSTCMD_ID_BLUETOOTH_INIT:
+ /* May block HOSTCMD requests till received initially */
+ thunderstrike_request_firmware_version(ts);
+ thunderstrike_request_board_info(ts);
+ /* Only HOSTCMD that can be triggered without a request */
+ return 0;
+ default:
+ hid_warn(hdev,
+ "Unhandled Thunderstrike HOSTCMD id %d\n",
+ hostcmd_resp_report->cmd_id);
+ return -ENOENT;
+ }
+
+ break;
+ default:
+ return 0;
+ }
+
+ return 0;
+}
+
+static struct shield_device *thunderstrike_create(struct hid_device *hdev)
+{
+ struct shield_device *shield_dev;
+ struct thunderstrike *ts;
+
+ ts = devm_kzalloc(&hdev->dev, sizeof(*ts), GFP_KERNEL);
+ if (!ts)
+ return ERR_PTR(-ENOMEM);
+
+ ts->req_report_dmabuf = devm_kzalloc(
+ &hdev->dev, THUNDERSTRIKE_HOSTCMD_REPORT_SIZE, GFP_KERNEL);
+ if (!ts->req_report_dmabuf)
+ return ERR_PTR(-ENOMEM);
+
+ shield_dev = &ts->base;
+ shield_dev->hdev = hdev;
+ shield_dev->codename = "Thunderstrike";
+
+ spin_lock_init(&ts->haptics_update_lock);
+ INIT_WORK(&ts->hostcmd_req_work, thunderstrike_hostcmd_req_work_handler);
+
+ hid_set_drvdata(hdev, shield_dev);
+
+ ts->haptics_dev = shield_haptics_create(shield_dev, thunderstrike_play_effect);
+ if (IS_ERR(ts->haptics_dev))
+ return ERR_CAST(ts->haptics_dev);
+
+ hid_info(hdev, "Registered Thunderstrike controller\n");
+ return shield_dev;
+}
+
+static ssize_t firmware_version_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct shield_device *shield_dev;
+ int ret;
+
+ shield_dev = hid_get_drvdata(hdev);
+
+ if (test_bit(SHIELD_FW_VERSION_INITIALIZED, &shield_dev->initialized_flags))
+ ret = sysfs_emit(buf, "0x%04X\n", shield_dev->fw_version);
+ else
+ ret = sysfs_emit(buf, NOT_INIT_STR "\n");
+
+ return ret;
+}
+
+static DEVICE_ATTR_RO(firmware_version);
+
+static ssize_t hardware_version_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct shield_device *shield_dev;
+ char board_revision_str[4];
+ int ret;
+
+ shield_dev = hid_get_drvdata(hdev);
+
+ if (test_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags)) {
+ shield_strrev(board_revision_str, 4, shield_dev->board_info.revision);
+ ret = sysfs_emit(buf, "%s BOARD_REVISION_%s (0x%04X)\n",
+ shield_dev->codename, board_revision_str,
+ shield_dev->board_info.revision);
+ } else
+ ret = sysfs_emit(buf, NOT_INIT_STR "\n");
+
+ return ret;
+}
+
+static DEVICE_ATTR_RO(hardware_version);
+
+static ssize_t serial_number_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct shield_device *shield_dev;
+ int ret;
+
+ shield_dev = hid_get_drvdata(hdev);
+
+ if (test_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags))
+ ret = sysfs_emit(buf, "%s\n", shield_dev->board_info.serial_number);
+ else
+ ret = sysfs_emit(buf, NOT_INIT_STR "\n");
+
+ return ret;
+}
+
+static DEVICE_ATTR_RO(serial_number);
+
+static struct attribute *shield_device_attrs[] = {
+ &dev_attr_firmware_version.attr,
+ &dev_attr_hardware_version.attr,
+ &dev_attr_serial_number.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(shield_device);
+
+static int shield_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct shield_device *dev = hid_get_drvdata(hdev);
+
+ return thunderstrike_parse_report(dev, report, data, size);
+}
+
+static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ struct shield_device *shield_dev = NULL;
+ struct thunderstrike *ts;
+ int ret;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "Parse failed\n");
+ return ret;
+ }
+
+ switch (id->product) {
+ case USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER:
+ shield_dev = thunderstrike_create(hdev);
+ break;
+ }
+
+ if (unlikely(!shield_dev)) {
+ hid_err(hdev, "Failed to identify SHIELD device\n");
+ return -ENODEV;
+ }
+ if (IS_ERR(shield_dev)) {
+ hid_err(hdev, "Failed to create SHIELD device\n");
+ return PTR_ERR(shield_dev);
+ }
+
+ ts = container_of(shield_dev, struct thunderstrike, base);
+
+ ret = hid_hw_start(hdev, HID_CONNECT_HIDINPUT);
+ if (ret) {
+ hid_err(hdev, "Failed to start HID device\n");
+ goto err_haptics;
+ }
+
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev, "Failed to open HID device\n");
+ goto err_stop;
+ }
+
+ thunderstrike_request_firmware_version(ts);
+ thunderstrike_request_board_info(ts);
+
+ return ret;
+
+err_stop:
+ hid_hw_stop(hdev);
+err_haptics:
+ if (ts->haptics_dev)
+ input_unregister_device(ts->haptics_dev);
+ return ret;
+}
+
+static void shield_remove(struct hid_device *hdev)
+{
+ struct shield_device *dev = hid_get_drvdata(hdev);
+ struct thunderstrike *ts;
+
+ ts = container_of(dev, struct thunderstrike, base);
+
+ hid_hw_close(hdev);
+ if (ts->haptics_dev)
+ input_unregister_device(ts->haptics_dev);
+ cancel_work_sync(&ts->hostcmd_req_work);
+ hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id shield_devices[] = {
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NVIDIA,
+ USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_NVIDIA,
+ USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER) },
+ {}
+};
+MODULE_DEVICE_TABLE(hid, shield_devices);
+
+static struct hid_driver shield_driver = {
+ .name = "shield",
+ .id_table = shield_devices,
+ .probe = shield_probe,
+ .remove = shield_remove,
+ .raw_event = shield_raw_event,
+ .driver = {
+ .dev_groups = shield_device_groups,
+ },
+};
+module_hid_driver(shield_driver);
+
+MODULE_AUTHOR("Rahul Rameshbabu <rrameshbabu@nvidia.com>");
+MODULE_DESCRIPTION("HID Driver for NVIDIA SHIELD peripherals.");
+MODULE_LICENSE("GPL");
--
2.38.4
^ permalink raw reply related
* [PATCH v2 0/1] HID: shield
From: Rahul Rameshbabu @ 2023-04-10 17:08 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Rahul Rameshbabu
Hi.
This is an initial driver implementation for supporting NVIDIA SHIELD
peripherals. Currently supports the following functionality for the
THUNDERSTRIKE (SHIELD 2017) controller.
- Haptics (ff_memless)
- Serial number query (sysfs)
- Hardware information query (sysfs)
- Firmware version query (sysfs)
Changes:
v1 -> v2:
* Declared thunderstrike_hostcmd_req_work_handler as static.
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/oe-kbuild-all/202304011342.6Bh3cWhA-lkp@intel.com/
Link: https://lore.kernel.org/oe-kbuild-all/202304020922.vsngJnBT-lkp@intel.com/
Rahul Rameshbabu (1):
HID: shield: Initial driver implementation with Thunderstrike support
MAINTAINERS | 6 +
drivers/hid/Kconfig | 18 ++
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 3 +
drivers/hid/hid-shield.c | 587 +++++++++++++++++++++++++++++++++++++++
5 files changed, 615 insertions(+)
create mode 100644 drivers/hid/hid-shield.c
Link: https://lore.kernel.org/linux-input/20230401032150.7424-1-rrameshbabu@nvidia.com/
--
2.38.4
^ permalink raw reply
* [PATCH] input: touchscreen: edt-ft5x06: select REGMAP_I2C
From: Daniel Golle @ 2023-04-10 17:02 UTC (permalink / raw)
To: linux-input, linux-kernel, Dmitry Torokhov, Arnd Bergmann,
Robert Jarzmik, Jeff LaBundy, Maxime Ripard, Max Krummenacher,
Alistair Francis, Jean Delvare, Job Noorman, Chris Morgan,
Hans de Goede, Dario Binacchi
After starting to use regmap API to access registers the edt-ft5x06
driver depends on symbols provided by REGMAP_I2C:
edt-ft5x06.o: in function `edt_ft5x06_ts_probe':
edt-ft5x06.c:1154: undefined reference to `__regmap_init_i2c'
edt-ft5x06.o: in function `edt_ft5x06_ts_identify':
edt-ft5x06.c:897: undefined reference to `__regmap_init_i2c'
Make sure support for I2C regmap is actually selected by adding this
dependency to Kconfig.
Fixes: 9dfd9708ffba ("Input: edt-ft5x06 - convert to use regmap API")
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
drivers/input/touchscreen/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 1feecd7ed3cb5..143ff43c67ae3 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -768,6 +768,7 @@ config TOUCHSCREEN_PENMOUNT
config TOUCHSCREEN_EDT_FT5X06
tristate "EDT FocalTech FT5x06 I2C Touchscreen support"
depends on I2C
+ select REGMAP_I2C
help
Say Y here if you have an EDT "Polytouch" touchscreen based
on the FocalTech FT5x06 family of controllers connected to
--
2.40.0
^ permalink raw reply related
* [dtor-input:next] BUILD SUCCESS cd7cd6f386df21725eeab5d803226d4f74177203
From: kernel test robot @ 2023-04-10 16:15 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: cd7cd6f386df21725eeab5d803226d4f74177203 Input: cma3000_d0x - remove unneeded code
elapsed time: 812m
configs tested: 192
configs skipped: 18
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allyesconfig gcc
alpha buildonly-randconfig-r005-20230409 gcc
alpha defconfig gcc
alpha randconfig-r005-20230409 gcc
arc allyesconfig gcc
arc buildonly-randconfig-r001-20230409 gcc
arc buildonly-randconfig-r002-20230410 gcc
arc buildonly-randconfig-r005-20230410 gcc
arc defconfig gcc
arc randconfig-r005-20230409 gcc
arc randconfig-r013-20230410 gcc
arc randconfig-r016-20230410 gcc
arc randconfig-r033-20230409 gcc
arc randconfig-r036-20230410 gcc
arc randconfig-r043-20230409 gcc
arc randconfig-r043-20230410 gcc
arc tb10x_defconfig gcc
arm allmodconfig gcc
arm allyesconfig gcc
arm defconfig gcc
arm mvebu_v7_defconfig gcc
arm omap2plus_defconfig gcc
arm pxa_defconfig gcc
arm randconfig-r001-20230409 gcc
arm randconfig-r046-20230409 clang
arm randconfig-r046-20230410 clang
arm64 alldefconfig gcc
arm64 allyesconfig gcc
arm64 buildonly-randconfig-r002-20230409 clang
arm64 defconfig gcc
arm64 randconfig-r002-20230410 clang
arm64 randconfig-r004-20230410 clang
arm64 randconfig-r031-20230410 clang
csky buildonly-randconfig-r003-20230410 gcc
csky defconfig gcc
csky randconfig-r032-20230409 gcc
hexagon buildonly-randconfig-r006-20230410 clang
hexagon randconfig-r011-20230410 clang
hexagon randconfig-r012-20230409 clang
hexagon randconfig-r013-20230410 clang
hexagon randconfig-r025-20230410 clang
hexagon randconfig-r041-20230409 clang
hexagon randconfig-r041-20230410 clang
hexagon randconfig-r045-20230409 clang
hexagon randconfig-r045-20230410 clang
i386 allyesconfig gcc
i386 buildonly-randconfig-r006-20230410 clang
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-a001-20230410 clang
i386 randconfig-a002-20230410 clang
i386 randconfig-a003-20230410 clang
i386 randconfig-a004-20230410 clang
i386 randconfig-a005-20230410 clang
i386 randconfig-a006-20230410 clang
i386 randconfig-a011-20230410 gcc
i386 randconfig-a012-20230410 gcc
i386 randconfig-a013-20230410 gcc
i386 randconfig-a014-20230410 gcc
i386 randconfig-a015-20230410 gcc
i386 randconfig-a016-20230410 gcc
i386 randconfig-r006-20230410 clang
i386 randconfig-r026-20230410 gcc
i386 randconfig-r033-20230410 clang
ia64 allmodconfig gcc
ia64 buildonly-randconfig-r003-20230410 gcc
ia64 defconfig gcc
ia64 randconfig-r001-20230410 gcc
ia64 randconfig-r013-20230409 gcc
ia64 randconfig-r015-20230410 gcc
ia64 randconfig-r025-20230409 gcc
ia64 randconfig-r035-20230409 gcc
ia64 tiger_defconfig gcc
ia64 zx1_defconfig gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch buildonly-randconfig-r003-20230409 gcc
loongarch buildonly-randconfig-r006-20230409 gcc
loongarch defconfig gcc
loongarch randconfig-r003-20230410 gcc
loongarch randconfig-r004-20230409 gcc
loongarch randconfig-r016-20230410 gcc
loongarch randconfig-r023-20230409 gcc
loongarch randconfig-r032-20230409 gcc
m68k allmodconfig gcc
m68k defconfig gcc
m68k randconfig-r002-20230409 gcc
m68k randconfig-r031-20230410 gcc
m68k randconfig-r035-20230409 gcc
m68k randconfig-r035-20230410 gcc
microblaze defconfig gcc
microblaze randconfig-r005-20230410 gcc
mips allmodconfig gcc
mips allyesconfig gcc
mips ip22_defconfig clang
mips ip27_defconfig clang
mips randconfig-r032-20230410 gcc
mips rs90_defconfig clang
nios2 buildonly-randconfig-r004-20230410 gcc
nios2 defconfig gcc
nios2 randconfig-r011-20230409 gcc
openrisc buildonly-randconfig-r006-20230409 gcc
openrisc randconfig-r003-20230409 gcc
openrisc randconfig-r006-20230409 gcc
openrisc randconfig-r006-20230410 gcc
openrisc randconfig-r024-20230409 gcc
openrisc randconfig-r031-20230409 gcc
openrisc randconfig-r033-20230409 gcc
parisc buildonly-randconfig-r002-20230410 gcc
parisc defconfig gcc
parisc randconfig-r002-20230410 gcc
parisc randconfig-r011-20230409 gcc
parisc randconfig-r011-20230410 gcc
parisc randconfig-r012-20230409 gcc
parisc randconfig-r022-20230409 gcc
parisc randconfig-r034-20230410 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc buildonly-randconfig-r004-20230409 gcc
powerpc eiger_defconfig gcc
powerpc mpc836x_mds_defconfig clang
powerpc pasemi_defconfig gcc
powerpc randconfig-r001-20230410 clang
powerpc randconfig-r003-20230410 clang
powerpc randconfig-r015-20230410 gcc
powerpc randconfig-r022-20230409 gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv defconfig gcc
riscv randconfig-r014-20230410 gcc
riscv randconfig-r026-20230409 gcc
riscv randconfig-r034-20230409 clang
riscv randconfig-r042-20230409 gcc
riscv randconfig-r042-20230410 gcc
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 buildonly-randconfig-r005-20230410 gcc
s390 defconfig gcc
s390 randconfig-r001-20230409 clang
s390 randconfig-r004-20230410 clang
s390 randconfig-r021-20230409 gcc
s390 randconfig-r023-20230409 gcc
s390 randconfig-r044-20230409 gcc
s390 randconfig-r044-20230410 gcc
sh allmodconfig gcc
sh ap325rxa_defconfig gcc
sh buildonly-randconfig-r001-20230410 gcc
sh buildonly-randconfig-r004-20230410 gcc
sh edosk7760_defconfig gcc
sh randconfig-r004-20230409 gcc
sh randconfig-r012-20230410 gcc
sh randconfig-r014-20230409 gcc
sh randconfig-r015-20230409 gcc
sh randconfig-r026-20230409 gcc
sh se7750_defconfig gcc
sparc defconfig gcc
sparc randconfig-r021-20230410 gcc
sparc randconfig-r031-20230409 gcc
sparc64 buildonly-randconfig-r001-20230409 gcc
sparc64 buildonly-randconfig-r003-20230409 gcc
sparc64 randconfig-r005-20230410 gcc
sparc64 randconfig-r006-20230409 gcc
sparc64 randconfig-r021-20230409 gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 defconfig gcc
x86_64 kexec gcc
x86_64 randconfig-a001-20230410 clang
x86_64 randconfig-a002-20230410 clang
x86_64 randconfig-a003-20230410 clang
x86_64 randconfig-a004-20230410 clang
x86_64 randconfig-a005-20230410 clang
x86_64 randconfig-a006-20230410 clang
x86_64 randconfig-a011-20230410 gcc
x86_64 randconfig-a012-20230410 gcc
x86_64 randconfig-a013-20230410 gcc
x86_64 randconfig-a014-20230410 gcc
x86_64 randconfig-a015-20230410 gcc
x86_64 randconfig-a016-20230410 gcc
x86_64 randconfig-k001 clang
x86_64 randconfig-r024-20230410 gcc
x86_64 randconfig-r033-20230410 clang
x86_64 rhel-8.3 gcc
xtensa randconfig-r014-20230410 gcc
xtensa randconfig-r015-20230409 gcc
xtensa randconfig-r016-20230409 gcc
xtensa randconfig-r034-20230410 gcc
xtensa randconfig-r036-20230410 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ 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