| Message ID | 20231224213629.395741-1-hdegoede@redhat.com |
|---|---|
| Headers | show |
| Series | i2c-i801 / dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800 | expand |
On Sunday 24 December 2023 22:36:17 Hans de Goede wrote: > The SMO8xxx ACPI HIDs are generic accelerometer ids which are also used > by other vendors. Add a sys_vendor check to ensure that the dell-smo8800 > driver only loads on Dell laptops. I saw that this driver was used also on some Acer laptops. As you wrote above, and now after years I have also feeling that these ACPI HIDs are generic, not Dell specific. So I would propose to not restrict smo8800 driver just for Dell laptops like this patch is introducing. Maybe better option would be to move this driver out of the dell namespace. Are we able to collect dmesg outputs and verify this information on how many non-dell laptops is this driver loaded? > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/platform/x86/dell/dell-smo8800.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/platform/x86/dell/dell-smo8800.c b/drivers/platform/x86/dell/dell-smo8800.c > index f7ec17c56833..4d5f778fb599 100644 > --- a/drivers/platform/x86/dell/dell-smo8800.c > +++ b/drivers/platform/x86/dell/dell-smo8800.c > @@ -10,6 +10,7 @@ > > #define DRIVER_NAME "smo8800" > > +#include <linux/dmi.h> > #include <linux/fs.h> > #include <linux/interrupt.h> > #include <linux/kernel.h> > @@ -108,6 +109,9 @@ static int smo8800_probe(struct platform_device *device) > int err; > struct smo8800_device *smo8800; > > + if (!dmi_match(DMI_SYS_VENDOR, "Dell Inc.")) > + return false; > + > smo8800 = devm_kzalloc(&device->dev, sizeof(*smo8800), GFP_KERNEL); > if (!smo8800) { > dev_err(&device->dev, "failed to allocate device data\n"); > -- > 2.43.0 >
On Sunday 24 December 2023 22:36:21 Hans de Goede wrote: > Instead of instantiating an i2c_client for the old misc joystick emulation > and freefall driver: drivers/misc/lis3lv02d/lis3lv02d.c use > i2c_client_id-s from the IIO st_accel driver so that the accelerometer > gets presented to userspace as an IIO device like all modern accelerometer > drivers do. > > Add a new use_misc_lis3lv02d module-parameter which can be set to restore > the old behavior in case someone has a use-case depending on this. > > When the st_accel IIO driver is used, also pass the IRQ to the i2c_client > and disable the /dev/freefall chardev. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/platform/x86/dell/dell-smo8800.c | 82 ++++++++++++++++++++++-- > 1 file changed, 78 insertions(+), 4 deletions(-) Sorry for the stupid question there, but what is the replacement for the /dev/freefall when using new st_accel IIO driver?
On Sun, Dec 24, 2023 at 11:36 PM Hans de Goede <hdegoede@redhat.com> wrote: > > It is not necessary to handle the Dell specific instantiation of > i2c_client-s for SMO8xxx ACPI devices without an ACPI I2cResource > inside the generic i801 I2C adapter driver. > > The kernel already instantiates platform_device-s for these ACPI devices > and the drivers/platform/x86/dell/dell-smo8800.c driver binds to these > platform drivers. > > Move the i2c_client instantiation from the generic i2c-i801 driver to > the Dell specific dell-smo8800 driver. ... > + static const u16 i801_idf_pci_device_ids[] = { > + 0x1d70, /* Patsburg (PCH) */ > + 0x1d71, /* Patsburg (PCH) */ > + 0x1d72, /* Patsburg (PCH) */ > + 0x8d7d, /* Wellsburg (PCH) */ > + 0x8d7e, /* Wellsburg (PCH) */ > + 0x8d7f, /* Wellsburg (PCH) */ > + }; I prefer to see this as a PCI ID table (yes, I know the downsides). ... > +/* Ensure the i2c-801 driver is loaded for i2c_client instantiation */ > +MODULE_SOFTDEP("pre: i2c-i801"); JFYI: software module dependencies are not supported by all kmod implementations in the user space. I don't expect people to complain, but just let you know that this kind of change needs to be done with care.
Hi Pali, Thank you for your feedback on this series. On 12/24/23 22:48, Pali Rohár wrote: > On Sunday 24 December 2023 22:36:17 Hans de Goede wrote: >> The SMO8xxx ACPI HIDs are generic accelerometer ids which are also used >> by other vendors. Add a sys_vendor check to ensure that the dell-smo8800 >> driver only loads on Dell laptops. > > I saw that this driver was used also on some Acer laptops. As you wrote > above, and now after years I have also feeling that these ACPI HIDs are > generic, not Dell specific. > > So I would propose to not restrict smo8800 driver just for Dell laptops > like this patch is introducing. Ok, I was not aware that some Acer laptops were also using this. I was just trying to honor the vendor check done in the i2c-i801 driver after the move of the i2c-client instantiation to dell-smo8800. Given that we are keeping the existing DMI matching I don't think keeping the sys-vendor check is necessary. So I'll drop the patch from v2 of the series. Regards, Hans > Maybe better option would be to move this driver out of the dell > namespace. > > Are we able to collect dmesg outputs and verify this information on how > many non-dell laptops is this driver loaded? > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/platform/x86/dell/dell-smo8800.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/platform/x86/dell/dell-smo8800.c b/drivers/platform/x86/dell/dell-smo8800.c >> index f7ec17c56833..4d5f778fb599 100644 >> --- a/drivers/platform/x86/dell/dell-smo8800.c >> +++ b/drivers/platform/x86/dell/dell-smo8800.c >> @@ -10,6 +10,7 @@ >> >> #define DRIVER_NAME "smo8800" >> >> +#include <linux/dmi.h> >> #include <linux/fs.h> >> #include <linux/interrupt.h> >> #include <linux/kernel.h> >> @@ -108,6 +109,9 @@ static int smo8800_probe(struct platform_device *device) >> int err; >> struct smo8800_device *smo8800; >> >> + if (!dmi_match(DMI_SYS_VENDOR, "Dell Inc.")) >> + return false; >> + >> smo8800 = devm_kzalloc(&device->dev, sizeof(*smo8800), GFP_KERNEL); >> if (!smo8800) { >> dev_err(&device->dev, "failed to allocate device data\n"); >> -- >> 2.43.0 >> >
Hi, On 12/24/23 23:03, Pali Rohár wrote: > On Sunday 24 December 2023 22:36:21 Hans de Goede wrote: >> Instead of instantiating an i2c_client for the old misc joystick emulation >> and freefall driver: drivers/misc/lis3lv02d/lis3lv02d.c use >> i2c_client_id-s from the IIO st_accel driver so that the accelerometer >> gets presented to userspace as an IIO device like all modern accelerometer >> drivers do. >> >> Add a new use_misc_lis3lv02d module-parameter which can be set to restore >> the old behavior in case someone has a use-case depending on this. >> >> When the st_accel IIO driver is used, also pass the IRQ to the i2c_client >> and disable the /dev/freefall chardev. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/platform/x86/dell/dell-smo8800.c | 82 ++++++++++++++++++++++-- >> 1 file changed, 78 insertions(+), 4 deletions(-) > > Sorry for the stupid question there, but what is the replacement for the > /dev/freefall when using new st_accel IIO driver? There is no replacement for /dev/freefall. I realize this is not ideal and if this turns out to be a problem the default of the module option can be reverted. But AFAIK / AFAICT there are no actual userspace consumers of /dev/freefall so removing it should not be an issue. Specifically I checked smartmontools which ships smartd which is the only daemon which I know of for hdd monitoring and that does not have /dev/freefall support. So /dev/freefall appears to be unused to me ? For completeness I also checked libatasmart which also does not access /dev/freefall. Regards, Hans
On Friday 05 January 2024 17:34:07 Hans de Goede wrote: > Hi, > > On 12/24/23 23:03, Pali Rohár wrote: > > On Sunday 24 December 2023 22:36:21 Hans de Goede wrote: > >> Instead of instantiating an i2c_client for the old misc joystick emulation > >> and freefall driver: drivers/misc/lis3lv02d/lis3lv02d.c use > >> i2c_client_id-s from the IIO st_accel driver so that the accelerometer > >> gets presented to userspace as an IIO device like all modern accelerometer > >> drivers do. > >> > >> Add a new use_misc_lis3lv02d module-parameter which can be set to restore > >> the old behavior in case someone has a use-case depending on this. > >> > >> When the st_accel IIO driver is used, also pass the IRQ to the i2c_client > >> and disable the /dev/freefall chardev. > >> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> --- > >> drivers/platform/x86/dell/dell-smo8800.c | 82 ++++++++++++++++++++++-- > >> 1 file changed, 78 insertions(+), 4 deletions(-) > > > > Sorry for the stupid question there, but what is the replacement for the > > /dev/freefall when using new st_accel IIO driver? > > There is no replacement for /dev/freefall. That is a big problem if there is no replacement. The primary usage of that hardware and all these drivers is that freefall ability. It was originally written for HP laptops and later extended for Dell. Access to accelerometer axes was just a secondary functions for linux fans. > I realize this is not ideal > and if this turns out to be a problem the default of the module option > can be reverted. > > But AFAIK / AFAICT there are no actual userspace consumers of > /dev/freefall so removing it should not be an issue. Userspace tool is directly in the kernel tree. Somewhere in tools dir now as it was moved (if it was not removed). > Specifically I checked smartmontools which ships smartd which > is the only daemon which I know of for hdd monitoring and > that does not have /dev/freefall support. So /dev/freefall > appears to be unused to me ? > > For completeness I also checked libatasmart which also does > not access /dev/freefall. I guess nobody ported it to these tools. IIRC the freefall design comes from the Suse the tool was also used on preloaded HP laptops. So maybe Suse have custom tool? I do not know. > Regards, > > Hans > >
On Fri, Jan 5, 2024 at 6:34 PM Hans de Goede <hdegoede@redhat.com> wrote: > On 12/24/23 23:03, Pali Rohár wrote: > > On Sunday 24 December 2023 22:36:21 Hans de Goede wrote: ... > But AFAIK / AFAICT there are no actual userspace consumers of > /dev/freefall so removing it should not be an issue. IIRC/AFAIK there is at least one (simple) computer game using it as a joystick.
On Fri, Jan 5, 2024 at 8:37 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Fri, Jan 5, 2024 at 6:34 PM Hans de Goede <hdegoede@redhat.com> wrote: > > On 12/24/23 23:03, Pali Rohár wrote: > > > On Sunday 24 December 2023 22:36:21 Hans de Goede wrote: ... > > But AFAIK / AFAICT there are no actual userspace consumers of > > /dev/freefall so removing it should not be an issue. > > IIRC/AFAIK there is at least one (simple) computer game using it as a joystick. Okay, I can't google for it and now I realised that it was my x60s, which has no freefall, but another interface to it. In any case the side effect of that googling is this (maybe more, I just took this one as example): https://github.com/linux-thinkpad/hdapsd/blob/master/README.md So, dropping it will break at least this tool.
On Friday 05 January 2024 21:04:59 Andy Shevchenko wrote: > On Fri, Jan 5, 2024 at 8:37 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Fri, Jan 5, 2024 at 6:34 PM Hans de Goede <hdegoede@redhat.com> wrote: > > > On 12/24/23 23:03, Pali Rohár wrote: > > > > On Sunday 24 December 2023 22:36:21 Hans de Goede wrote: > > ... > > > > But AFAIK / AFAICT there are no actual userspace consumers of > > > /dev/freefall so removing it should not be an issue. > > > > IIRC/AFAIK there is at least one (simple) computer game using it as a joystick. > > Okay, I can't google for it and now I realised that it was my x60s, > which has no freefall, but another interface to it. In any case the > side effect of that googling is this (maybe more, I just took this one > as example): > https://github.com/linux-thinkpad/hdapsd/blob/master/README.md > > So, dropping it will break at least this tool. > > -- > With Best Regards, > Andy Shevchenko Yes, this is that correct one. I forget the name of this daemon. Just to note /dev/freefall does not provide axes state, it just send signal to process when interrupt is triggered. Process than park disk heads. Axes state are/were exported throw /dev/js* interface and those games uses just js interface. I remember Tux Racer. Interrupt on HP and Dell is triggered only when laptop fall is detected, so games did not used it (hopefully!)
Hi, On 1/5/24 20:20, Pali Rohár wrote: > On Friday 05 January 2024 21:04:59 Andy Shevchenko wrote: >> On Fri, Jan 5, 2024 at 8:37 PM Andy Shevchenko >> <andy.shevchenko@gmail.com> wrote: >>> On Fri, Jan 5, 2024 at 6:34 PM Hans de Goede <hdegoede@redhat.com> wrote: >>>> On 12/24/23 23:03, Pali Rohár wrote: >>>>> On Sunday 24 December 2023 22:36:21 Hans de Goede wrote: >> >> ... >> >>>> But AFAIK / AFAICT there are no actual userspace consumers of >>>> /dev/freefall so removing it should not be an issue. >>> >>> IIRC/AFAIK there is at least one (simple) computer game using it as a joystick. >> >> Okay, I can't google for it and now I realised that it was my x60s, >> which has no freefall, but another interface to it. In any case the >> side effect of that googling is this (maybe more, I just took this one >> as example): >> https://github.com/linux-thinkpad/hdapsd/blob/master/README.md >> >> So, dropping it will break at least this tool. >> >> -- >> With Best Regards, >> Andy Shevchenko > > Yes, this is that correct one. I forget the name of this daemon. > > Just to note /dev/freefall does not provide axes state, it just send > signal to process when interrupt is triggered. Process than park disk > heads. > > Axes state are/were exported throw /dev/js* interface and those games > uses just js interface. I remember Tux Racer. > > Interrupt on HP and Dell is triggered only when laptop fall is detected, > so games did not used it (hopefully!) Ok, so I clearly need to change the module parameter so that we stick with the drivers/char/misc/lis3lv02d driver as default and offer using the i2c-client-id which results in loading the iio driver instead as an option enabled by a module parameter. I'll fix this for v2. Regards, Hans
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 > ``` >
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(-)