| Message ID | 20231224213629.395741-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | i2c-i801 / dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800 | expand |
Dear Hans, Thank you very much for working on this issue and even sending patches so quickly. Am 24.12.23 um 22:36 schrieb Hans de Goede: > Here is a patch series which implements my suggestions from: > https://lore.kernel.org/linux-i2c/4820e280-9ca4-4d97-9d21-059626161bfc@molgen.mpg.de/ > to improve the lis3lv02d accel support on Dell laptops. > > Jean, Andi the actual move is in patch 3/6 after some small prep patches > on the dell-smo8800 side. My plan for merging this is to create > an immutable branch based on 6.8-rc1 (once it is out) + these 6 patches and > then send a pull-request for this. Can I have your Ack for the i2c-i801 > changes in patch 3/6? I think you'll like the changes there since they only > remove code :) > Hans de Goede (6): > platform/x86: dell-smo8800: Only load on Dell laptops > platform/x86: dell-smo8800: Change probe() ordering a bit > platform/x86: dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800 > platform/x86: dell-smo8800: Pass the IRQ to the lis3lv02d i2c_client > platform/x86: dell-smo8800: Instantiate an i2c_client for the IIO st_accel driver > platform/x86: dell-smo8800: Add support for probing for the accelerometer i2c address > > drivers/i2c/busses/i2c-i801.c | 122 -------- > drivers/platform/x86/dell/dell-smo8800.c | 337 +++++++++++++++++++++-- > 2 files changed, 316 insertions(+), 143 deletions(-) This Thursday, I tested this on the Dell Inc. XPS 15 7590/0VYV0G, BIOS 1.24.0 09/11/2023. First just with your patch-set and trying the parameter: [ 0.000000] Command line: BOOT_IMAGE=/vmlinuz-6.7.0-rc8+ root=UUID=9fa41e21-7a5f-479e-afdc-9a5503368d8e ro quiet rd.luks=1 rd.auto=1 dell-smo8800.probe_i2c_addr=0x29 […] [ 28.826356] smo8800 SMO8810:00: Accelerometer lis3lv02d is present on SMBus but its address is unknown, skipping registration [ 28.826359] smo8800 SMO8810:00: Pass dell_smo8800.probe_i2c_addr=1 on the kernel commandline to probe, this may be dangerous! I misread the parameter documentation, but didn’t see the message back then, and just saved the log files. So, I added an entry for the device, and got: [ 19.197838] smo8800 SMO8810:00: Registered lis2de12 accelerometer on address 0x29 Kind regards, Paul PS: I still seem to miss some config option in my custom Linux kernel configuration, as with my self-built Linux kernel, the accelerometer is not detected as an input device. ``` $ sudo dmesg | grep input [ 0.648449] input: AT Translated Set 2 keyboard as /devices/platform/i8042/serio0/input/input0 [ 19.164633] input: Intel HID events as /devices/platform/INT33D5:00/input/input2 [ 19.176109] input: Intel HID 5 button array as /devices/platform/INT33D5:00/input/input3 [ 19.200645] input: Lid Switch as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0D:00/input/input4 [ 19.230941] input: Power Button as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0C:00/input/input5 [ 19.231434] input: Sleep Button as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0E:00/input/input6 [ 19.350390] input: PC Speaker as /devices/platform/pcspkr/input/input7 [ 19.996196] input: Dell WMI hotkeys as /devices/platform/PNP0C14:05/wmi_bus/wmi_bus-PNP0C14:05/9DBB5994-A997-11DA-B012-B622A1EF5492/input/input9 [ 20.014546] input: SYNA2393:00 06CB:7A13 Mouse as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-2/i2c-SYNA2393:00/0018:06CB:7A13.0001/input/input10 [ 20.047534] input: SYNA2393:00 06CB:7A13 Touchpad as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-2/i2c-SYNA2393:00/0018:06CB:7A13.0001/input/input11 [ 20.047667] hid-generic 0018:06CB:7A13.0001: input,hidraw0: I2C HID v1.00 Mouse [SYNA2393:00 06CB:7A13] on i2c-SYNA2393:00 [ 20.048874] input: WCOM490B:00 056A:490B Touchscreen as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input13 [ 20.049014] input: WCOM490B:00 056A:490B as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input14 [ 20.049089] input: WCOM490B:00 056A:490B Stylus as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input15 [ 20.049186] input: WCOM490B:00 056A:490B as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input16 [ 20.065066] input: WCOM490B:00 056A:490B Mouse as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input17 [ 20.065360] hid-generic 0018:056A:490B.0002: input,hidraw1: I2C HID v1.00 Mouse [WCOM490B:00 056A:490B] on i2c-WCOM490B:00 [ 20.760879] input: SYNA2393:00 06CB:7A13 Mouse as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-2/i2c-SYNA2393:00/0018:06CB:7A13.0001/input/input18 [ 20.760979] input: SYNA2393:00 06CB:7A13 Touchpad as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-2/i2c-SYNA2393:00/0018:06CB:7A13.0001/input/input19 [ 20.761032] hid-multitouch 0018:06CB:7A13.0001: input,hidraw0: I2C HID v1.00 Mouse [SYNA2393:00 06CB:7A13] on i2c-SYNA2393:00 [ 21.083016] input: Wacom HID 490B Pen as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input21 [ 21.083149] input: Wacom HID 490B Finger as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input22 [ 25.850198] input: Video Bus as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:00/input/input24 [ 25.850344] input: Video Bus as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:0b/LNXVIDEO:01/input/input25 [ 26.027649] snd_hda_codec_realtek hdaudioC0D0: inputs: [ 26.132076] input: HDA Intel PCH Headphone Mic as /devices/pci0000:00/0000:00:1f.3/sound/card0/input26 [ 26.132148] input: HDA Intel PCH HDMI/DP,pcm=3 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input27 [ 26.132192] input: HDA Intel PCH HDMI/DP,pcm=7 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input28 [ 26.132233] input: HDA Intel PCH HDMI/DP,pcm=8 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input29 [ 28.659169] rfkill: input handler disabled [ 47.283492] rfkill: input handler enabled [ 52.883611] rfkill: input handler disabled ```
Hi Paul, On 1/6/24 15:23, Paul Menzel wrote: > Dear Hans, > > > Thank you very much for working on this issue and even sending patches so quickly. You're welcome this was a nice little project to work on during the holidays. > Am 24.12.23 um 22:36 schrieb Hans de Goede: > >> Here is a patch series which implements my suggestions from: >> https://lore.kernel.org/linux-i2c/4820e280-9ca4-4d97-9d21-059626161bfc@molgen.mpg.de/ >> to improve the lis3lv02d accel support on Dell laptops. >> >> Jean, Andi the actual move is in patch 3/6 after some small prep patches >> on the dell-smo8800 side. My plan for merging this is to create >> an immutable branch based on 6.8-rc1 (once it is out) + these 6 patches and >> then send a pull-request for this. Can I have your Ack for the i2c-i801 >> changes in patch 3/6? I think you'll like the changes there since they only >> remove code :) > >> Hans de Goede (6): >> platform/x86: dell-smo8800: Only load on Dell laptops >> platform/x86: dell-smo8800: Change probe() ordering a bit >> platform/x86: dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800 >> platform/x86: dell-smo8800: Pass the IRQ to the lis3lv02d i2c_client >> platform/x86: dell-smo8800: Instantiate an i2c_client for the IIO st_accel driver >> platform/x86: dell-smo8800: Add support for probing for the accelerometer i2c address >> >> drivers/i2c/busses/i2c-i801.c | 122 -------- >> drivers/platform/x86/dell/dell-smo8800.c | 337 +++++++++++++++++++++-- >> 2 files changed, 316 insertions(+), 143 deletions(-) > > This Thursday, I tested this on the Dell Inc. XPS 15 7590/0VYV0G, BIOS 1.24.0 09/11/2023. Interesting, can you run: sudo acpidump -o acpidump.txt and then send me a private email with the generated acpidump.txt attached ? > > First just with your patch-set and trying the parameter: > > [ 0.000000] Command line: BOOT_IMAGE=/vmlinuz-6.7.0-rc8+ root=UUID=9fa41e21-7a5f-479e-afdc-9a5503368d8e ro quiet rd.luks=1 rd.auto=1 dell-smo8800.probe_i2c_addr=0x29 > […] > [ 28.826356] smo8800 SMO8810:00: Accelerometer lis3lv02d is present on SMBus but its address is unknown, skipping registration > [ 28.826359] smo8800 SMO8810:00: Pass dell_smo8800.probe_i2c_addr=1 on the kernel commandline to probe, this may be dangerous! > > I misread the parameter documentation, but didn’t see the message back then, and just saved the log files. > > So, I added an entry for the device, and got: > > [ 19.197838] smo8800 SMO8810:00: Registered lis2de12 accelerometer on address 0x29 Ok, that looks good. Can you provide the output of: cat /sys/class/dmi/id/product_name So that we can also add an entry for this upstream ? > PS: I still seem to miss some config option in my custom Linux kernel configuration, as with my self-built Linux kernel, the accelerometer is not detected as an input device. Right, v1 of my patches changed the code to by default instantiate an i2c_client to which the st_accel IIO driver will bind. Using the IIO framework is how accelerometers are handled normally and the handling of these "freefall" sensors so far has been a bit different, so I tried to make them more like normal accelerometers which don't do the joystick emulation. But Pali and Andy pointed out to me that there is userspace code out there relying on /dev/freefall, so for v2 of the patches I've kept the old behavior by default. I've just posted v2 of the patches. Note with v1 you can also get the old behavior by adding dell_smo8800.use_misc_lis3lv02d=1 to the kernel commandline. Adding that (or switching to the v2 patches) should give you an input device. Regards, Hans > > ``` > $ sudo dmesg | grep input > [ 0.648449] input: AT Translated Set 2 keyboard as /devices/platform/i8042/serio0/input/input0 > [ 19.164633] input: Intel HID events as /devices/platform/INT33D5:00/input/input2 > [ 19.176109] input: Intel HID 5 button array as /devices/platform/INT33D5:00/input/input3 > [ 19.200645] input: Lid Switch as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0D:00/input/input4 > [ 19.230941] input: Power Button as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0C:00/input/input5 > [ 19.231434] input: Sleep Button as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0E:00/input/input6 > [ 19.350390] input: PC Speaker as /devices/platform/pcspkr/input/input7 > [ 19.996196] input: Dell WMI hotkeys as /devices/platform/PNP0C14:05/wmi_bus/wmi_bus-PNP0C14:05/9DBB5994-A997-11DA-B012-B622A1EF5492/input/input9 > [ 20.014546] input: SYNA2393:00 06CB:7A13 Mouse as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-2/i2c-SYNA2393:00/0018:06CB:7A13.0001/input/input10 > [ 20.047534] input: SYNA2393:00 06CB:7A13 Touchpad as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-2/i2c-SYNA2393:00/0018:06CB:7A13.0001/input/input11 > [ 20.047667] hid-generic 0018:06CB:7A13.0001: input,hidraw0: I2C HID v1.00 Mouse [SYNA2393:00 06CB:7A13] on i2c-SYNA2393:00 > [ 20.048874] input: WCOM490B:00 056A:490B Touchscreen as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input13 > [ 20.049014] input: WCOM490B:00 056A:490B as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input14 > [ 20.049089] input: WCOM490B:00 056A:490B Stylus as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input15 > [ 20.049186] input: WCOM490B:00 056A:490B as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input16 > [ 20.065066] input: WCOM490B:00 056A:490B Mouse as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input17 > [ 20.065360] hid-generic 0018:056A:490B.0002: input,hidraw1: I2C HID v1.00 Mouse [WCOM490B:00 056A:490B] on i2c-WCOM490B:00 > [ 20.760879] input: SYNA2393:00 06CB:7A13 Mouse as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-2/i2c-SYNA2393:00/0018:06CB:7A13.0001/input/input18 > [ 20.760979] input: SYNA2393:00 06CB:7A13 Touchpad as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-2/i2c-SYNA2393:00/0018:06CB:7A13.0001/input/input19 > [ 20.761032] hid-multitouch 0018:06CB:7A13.0001: input,hidraw0: I2C HID v1.00 Mouse [SYNA2393:00 06CB:7A13] on i2c-SYNA2393:00 > [ 21.083016] input: Wacom HID 490B Pen as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input21 > [ 21.083149] input: Wacom HID 490B Finger as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-WCOM490B:00/0018:056A:490B.0002/input/input22 > [ 25.850198] input: Video Bus as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:00/input/input24 > [ 25.850344] input: Video Bus as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:0b/LNXVIDEO:01/input/input25 > [ 26.027649] snd_hda_codec_realtek hdaudioC0D0: inputs: > [ 26.132076] input: HDA Intel PCH Headphone Mic as /devices/pci0000:00/0000:00:1f.3/sound/card0/input26 > [ 26.132148] input: HDA Intel PCH HDMI/DP,pcm=3 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input27 > [ 26.132192] input: HDA Intel PCH HDMI/DP,pcm=7 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input28 > [ 26.132233] input: HDA Intel PCH HDMI/DP,pcm=8 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input29 > [ 28.659169] rfkill: input handler disabled > [ 47.283492] rfkill: input handler enabled > [ 52.883611] rfkill: input handler disabled > ``` >
Dear Hans, Again, thank you for your reply. Am 06.01.24 um 17:15 schrieb Hans de Goede: > On 1/6/24 15:23, Paul Menzel wrote: […] >> Am 24.12.23 um 22:36 schrieb Hans de Goede: >> >>> Here is a patch series which implements my suggestions from: >>> https://lore.kernel.org/linux-i2c/4820e280-9ca4-4d97-9d21-059626161bfc@molgen.mpg.de/ >>> to improve the lis3lv02d accel support on Dell laptops. >>> >>> Jean, Andi the actual move is in patch 3/6 after some small prep patches >>> on the dell-smo8800 side. My plan for merging this is to create >>> an immutable branch based on 6.8-rc1 (once it is out) + these 6 patches and >>> then send a pull-request for this. Can I have your Ack for the i2c-i801 >>> changes in patch 3/6? I think you'll like the changes there since they only >>> remove code :) >> >>> Hans de Goede (6): >>> platform/x86: dell-smo8800: Only load on Dell laptops >>> platform/x86: dell-smo8800: Change probe() ordering a bit >>> platform/x86: dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800 >>> platform/x86: dell-smo8800: Pass the IRQ to the lis3lv02d i2c_client >>> platform/x86: dell-smo8800: Instantiate an i2c_client for the IIO st_accel driver >>> platform/x86: dell-smo8800: Add support for probing for the accelerometer i2c address >>> >>> drivers/i2c/busses/i2c-i801.c | 122 -------- >>> drivers/platform/x86/dell/dell-smo8800.c | 337 +++++++++++++++++++++-- >>> 2 files changed, 316 insertions(+), 143 deletions(-) >> >> This Thursday, I tested this on the Dell Inc. XPS 15 7590/0VYV0G, BIOS 1.24.0 09/11/2023. > > Interesting, can you run: > > sudo acpidump -o acpidump.txt and then send me a private email > with the generated acpidump.txt attached ? Please find it in the Linux Kernel Bugzilla [1], where I attached it to another issue. >> First just with your patch-set and trying the parameter: >> >> [ 0.000000] Command line: BOOT_IMAGE=/vmlinuz-6.7.0-rc8+ root=UUID=9fa41e21-7a5f-479e-afdc-9a5503368d8e ro quiet rd.luks=1 rd.auto=1 dell-smo8800.probe_i2c_addr=0x29 >> […] >> [ 28.826356] smo8800 SMO8810:00: Accelerometer lis3lv02d is present on SMBus but its address is unknown, skipping registration >> [ 28.826359] smo8800 SMO8810:00: Pass dell_smo8800.probe_i2c_addr=1 on the kernel commandline to probe, this may be dangerous! >> >> I misread the parameter documentation, but didn’t see the message back then, and just saved the log files. >> >> So, I added an entry for the device, and got: >> >> [ 19.197838] smo8800 SMO8810:00: Registered lis2de12 accelerometer on address 0x29 > > Ok, that looks good. Can you provide the output of: > > cat /sys/class/dmi/id/product_name From my upload to Hardware for Linux [2]: XPS 15 7590 > So that we can also add an entry for this upstream ? I already sent a patch, that got applied [3]. >> PS: I still seem to miss some config option in my custom Linux >> kernel configuration, as with my self-built Linux kernel, the >> accelerometer is not detected as an input device. > > Right, v1 of my patches changed the code to by default instantiate an i2c_client > to which the st_accel IIO driver will bind. Using the IIO framework is > how accelerometers are handled normally and the handling of these "freefall" > sensors so far has been a bit different, so I tried to make them more like > normal accelerometers which don't do the joystick emulation. > > But Pali and Andy pointed out to me that there is userspace code out > there relying on /dev/freefall, so for v2 of the patches I've kept > the old behavior by default. > > I've just posted v2 of the patches. > > Note with v1 you can also get the old behavior by adding > dell_smo8800.use_misc_lis3lv02d=1 to the kernel commandline. > > Adding that (or switching to the v2 patches) should give you > an input device. Thank you very much. I am going to test them as soon as possible. Kind regards, Paul [1]: https://bugzilla.kernel.org/show_bug.cgi?id=218287 "`ACPI Error: AE_ERROR, Returned by Handler for [PCI_Config] (20230628/evregion-300)`" [2]: https://linux-hardware.org/?probe=74136911a0&log=dmidecode [3]: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git/commit/?h=i2c/for-next&id=dc3293b460db70e3b5b76175d1a158dc802b9740
Hi All, Here is a patch series which implements my suggestions from: https://lore.kernel.org/linux-i2c/4820e280-9ca4-4d97-9d21-059626161bfc@molgen.mpg.de/ to improve the lis3lv02d accel support on Dell laptops. Jean, Andi the actual move is in patch 3/6 after some small prep patches on the dell-smo8800 side. My plan for merging this is to create an immutable branch based on 6.8-rc1 (once it is out) + these 6 patches and then send a pull-request for this. Can I have your Ack for the i2c-i801 changes in patch 3/6? I think you'll like the changes there since they only remove code :) Regards, Hans Hans de Goede (6): platform/x86: dell-smo8800: Only load on Dell laptops platform/x86: dell-smo8800: Change probe() ordering a bit platform/x86: dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800 platform/x86: dell-smo8800: Pass the IRQ to the lis3lv02d i2c_client platform/x86: dell-smo8800: Instantiate an i2c_client for the IIO st_accel driver platform/x86: dell-smo8800: Add support for probing for the accelerometer i2c address drivers/i2c/busses/i2c-i801.c | 122 -------- drivers/platform/x86/dell/dell-smo8800.c | 337 +++++++++++++++++++++-- 2 files changed, 316 insertions(+), 143 deletions(-)