2023-12-23 09:40:34

by Paul Menzel

[permalink] [raw]
Subject: Ideas for a generic solution to support accelerometer lis3lv02d in Dell laptops/notebooks?

Dear Linux folks,


Currently, on Dell systems with the accelerometer lis3lv02d, its I²C
address needs to be added to `dell_lis3lv02d_devices[]` in
`drivers/i2c/busses/i2c-i801.c`.

In Linux 6.7-rc6 that array has nine elements, so only a small fraction
of all Dell notebooks is listed. Searching the Linux logs uploaded to
the Linux hardware database from May 2023 [1], there are around 129
devices without support in the Linux kernel version the upload was done
with.

Do you know, how the Microsoft Windows driver is doing this? Is it
hard-coded there too, or can it be deduced somehow, for example from the
ACPI tables?

I added some Kai-Heng and Hans to Cc as they might have contact. Dell
offers or offered quite a few of the models with official Ubuntu
support, so I would have hoped to have a generic solution for this.
Maybe Mario can also forward it to the Dell team.


Kind regards,

Paul


[1]: https://github.com/linuxhw/Dmesg


PS: Dell devices in Linux hardware database with accelerometer:

linux-hardware-dmesg/Notebook/Dell (main)$ git grep -l ccelerome | cut
-d '/' -f 1,2 | sort -u
Inspiron/Inspiron 11 - 3147
Inspiron/Inspiron 5520
Inspiron/Inspiron 7547
Inspiron/Inspiron 7548
Latitude/Latitude 12 Rugged Extreme
Latitude/Latitude 2110
Latitude/Latitude 2120
Latitude/Latitude 3330
Latitude/Latitude 3380
Latitude/Latitude 3400
Latitude/Latitude 3470
Latitude/Latitude 3480
Latitude/Latitude 3490
Latitude/Latitude 3500
Latitude/Latitude 3570
Latitude/Latitude 3580
Latitude/Latitude 3590
Latitude/Latitude 5280
Latitude/Latitude 5290
Latitude/Latitude 5400
Latitude/Latitude 5401
Latitude/Latitude 5410
Latitude/Latitude 5411
Latitude/Latitude 5414
Latitude/Latitude 5420 Rugged
Latitude/Latitude 5424 Rugged
Latitude/Latitude 5480
Latitude/Latitude 5490
Latitude/Latitude 5491
Latitude/Latitude 5500
Latitude/Latitude 5501
Latitude/Latitude 5510
Latitude/Latitude 5511
Latitude/Latitude 5531
Latitude/Latitude 5580
Latitude/Latitude 5590
Latitude/Latitude 5591
Latitude/Latitude 7214
Latitude/Latitude 7414
Latitude/Latitude 7424 Rugged Extreme
Latitude/Latitude E4310
Latitude/Latitude E5270
Latitude/Latitude E5410
Latitude/Latitude E5420
Latitude/Latitude E5420m
Latitude/Latitude E5430 non-vPro
Latitude/Latitude E5430 vPro
Latitude/Latitude E5440
Latitude/Latitude E5470
Latitude/Latitude E5510
Latitude/Latitude E5520
Latitude/Latitude E5520m
Latitude/Latitude E5530 non-vPro
Latitude/Latitude E5530 vPro
Latitude/Latitude E5540
Latitude/Latitude E5570
Latitude/Latitude E6220
Latitude/Latitude E6230
Latitude/Latitude E6320
Latitude/Latitude E6330
Latitude/Latitude E6410
Latitude/Latitude E6420
Latitude/Latitude E6430
Latitude/Latitude E6430s
Latitude/Latitude E6440
Latitude/Latitude E64406342Q0286-
Latitude/Latitude E6510
Latitude/Latitude E6520
Latitude/Latitude E6530
Latitude/Latitude E6540
Latitude/Latitude E7440
Latitude/Latitude XT3
Precision/Precision 3510
Precision/Precision 3520
Precision/Precision 3530
Precision/Precision 3540
Precision/Precision 3541
Precision/Precision 3550
Precision/Precision 3551
Precision/Precision 3571
Precision/Precision 5510
Precision/Precision 5520
Precision/Precision 5530
Precision/Precision 5540
Precision/Precision 7510
Precision/Precision 7520
Precision/Precision 7530
Precision/Precision 7540
Precision/Precision 7710
Precision/Precision 7720
Precision/Precision 7730
Precision/Precision 7740
Precision/Precision M2800
Precision/Precision M3800
Precision/Precision M4500
Precision/Precision M4600
Precision/Precision M4700
Precision/Precision M4800
Precision/Precision M6600
Precision/Precision M6700
Precision/Precision M6800
Studio/Studio 1458
Studio/Studio 1557
Studio/Studio 1558
Studio/Studio 1569
Studio/Studio 1747
Studio/Studio 1749
Unidentified/Unidentified System
Vostro/Vostro 3300
Vostro/Vostro 3350
Vostro/Vostro 3400
Vostro/Vostro 3500
Vostro/Vostro 3550
Vostro/Vostro 3560
Vostro/Vostro 3700
Vostro/Vostro 5468
Vostro/Vostro 5471
Vostro/Vostro 5568
Vostro/Vostro 7580
Vostro/Vostro V130
Vostro/Vostro V131
XPS/XPS 15 7590
XPS/XPS 15 9530
XPS/XPS 15 9550
XPS/XPS 15 9560
XPS/XPS 15 9570
XPS/XPS L401X
XPS/XPS L412Z
XPS/XPS L421X
XPS/XPS L501X
XPS/XPS L521X
XPS/XPS L701X

Unsupported:

$ git grep ccelerome | grep "is present on SMBus" | cut -d '/' -f 1,2 |
sort -u | wc -l
129


2023-12-23 12:19:55

by Pali Rohár

[permalink] [raw]
Subject: Re: Ideas for a generic solution to support accelerometer lis3lv02d in Dell laptops/notebooks?

On Saturday 23 December 2023 10:39:18 Paul Menzel wrote:
> Dear Linux folks,
>
>
> Currently, on Dell systems with the accelerometer lis3lv02d, its I²C address
> needs to be added to `dell_lis3lv02d_devices[]` in
> `drivers/i2c/busses/i2c-i801.c`.

Hello. This is because smbus bus do not have smart discovery
capabilities like PCIe or USB buses. The device address has to be
somewhere stored. And at the moment it is in the parent bus driver.

> In Linux 6.7-rc6 that array has nine elements, so only a small fraction of
> all Dell notebooks is listed. Searching the Linux logs uploaded to the Linux
> hardware database from May 2023 [1], there are around 129 devices without
> support in the Linux kernel version the upload was done with.

In Dell ACPI device table is already stored information if the smbus
device is present in the laptop or not. And there is (or at least was
for older models) also interrupt number in ACPI device resource list.
Kernel driver already registers for interrupt events.

What is missing the ACPI device resource list is that smbus device
address on which is device listening. Or better, it was missing for
older models. I have not checked new models.

Why it is missing, I have no idea. As ACPI tables are passed to kernel
by laptop firmware (BIOS/UEFI) I bet that this is just a programming
error that somebody forgot to put this information there.

I think that it was Mario who is the past confirmed smbus address of few
models and confirmed also information that address is not really stored
in the ACPI tables.

Before taking any future steps it would be needed to confirm if the
smbus device address is still missing in the ACPI device resource list
in the new laptop models.

> Do you know, how the Microsoft Windows driver is doing this? Is it
> hard-coded there too, or can it be deduced somehow, for example from the
> ACPI tables?

In past on Windows laptops it was always needed to install correct and
specific version of drivers for every laptop, I would not be surprised
that smbus device address can be there hardcoded too. I really do not
remember details, it is years ago when I was looking at this particular
problem. But these practises do not (or at least should not) apply to
modern Windows machines, so it can be possible that new Windows drivers
can discover it somehow (read from BIOS, UEFI, EEPROM, ACPI methods,
etc.). Or there is another option: There is no Windows driver for it at
all (and so no problem).

If you have such new laptops then the best for you is to do inspection.
Look how is device driver registered, how the application communicate
with drivers, look at device resources, etc... Basically Linux kernel
driver should use same resources as the Windows kernel driver.

> I added some Kai-Heng and Hans to Cc as they might have contact. Dell offers
> or offered quite a few of the models with official Ubuntu support, so I
> would have hoped to have a generic solution for this. Maybe Mario can also
> forward it to the Dell team.

This is really question for Dell people.

We have done the best what we were able, but internal wiring of the
Dell laptop boards and how it is reported to the operating system is
really what only Dell firmware people or other Dell team can do it.

>
>
> Kind regards,
>
> Paul
>
>
> [1]: https://github.com/linuxhw/Dmesg
>
>
> PS: Dell devices in Linux hardware database with accelerometer:
>
> linux-hardware-dmesg/Notebook/Dell (main)$ git grep -l ccelerome | cut -d
> '/' -f 1,2 | sort -u
> Inspiron/Inspiron 11 - 3147
> Inspiron/Inspiron 5520
> Inspiron/Inspiron 7547
> Inspiron/Inspiron 7548
> Latitude/Latitude 12 Rugged Extreme
> Latitude/Latitude 2110
> Latitude/Latitude 2120
> Latitude/Latitude 3330
> Latitude/Latitude 3380
> Latitude/Latitude 3400
> Latitude/Latitude 3470
> Latitude/Latitude 3480
> Latitude/Latitude 3490
> Latitude/Latitude 3500
> Latitude/Latitude 3570
> Latitude/Latitude 3580
> Latitude/Latitude 3590
> Latitude/Latitude 5280
> Latitude/Latitude 5290
> Latitude/Latitude 5400
> Latitude/Latitude 5401
> Latitude/Latitude 5410
> Latitude/Latitude 5411
> Latitude/Latitude 5414
> Latitude/Latitude 5420 Rugged
> Latitude/Latitude 5424 Rugged
> Latitude/Latitude 5480
> Latitude/Latitude 5490
> Latitude/Latitude 5491
> Latitude/Latitude 5500
> Latitude/Latitude 5501
> Latitude/Latitude 5510
> Latitude/Latitude 5511
> Latitude/Latitude 5531
> Latitude/Latitude 5580
> Latitude/Latitude 5590
> Latitude/Latitude 5591
> Latitude/Latitude 7214
> Latitude/Latitude 7414
> Latitude/Latitude 7424 Rugged Extreme
> Latitude/Latitude E4310
> Latitude/Latitude E5270
> Latitude/Latitude E5410
> Latitude/Latitude E5420
> Latitude/Latitude E5420m
> Latitude/Latitude E5430 non-vPro
> Latitude/Latitude E5430 vPro
> Latitude/Latitude E5440
> Latitude/Latitude E5470
> Latitude/Latitude E5510
> Latitude/Latitude E5520
> Latitude/Latitude E5520m
> Latitude/Latitude E5530 non-vPro
> Latitude/Latitude E5530 vPro
> Latitude/Latitude E5540
> Latitude/Latitude E5570
> Latitude/Latitude E6220
> Latitude/Latitude E6230
> Latitude/Latitude E6320
> Latitude/Latitude E6330
> Latitude/Latitude E6410
> Latitude/Latitude E6420
> Latitude/Latitude E6430
> Latitude/Latitude E6430s
> Latitude/Latitude E6440
> Latitude/Latitude E64406342Q0286-
> Latitude/Latitude E6510
> Latitude/Latitude E6520
> Latitude/Latitude E6530
> Latitude/Latitude E6540
> Latitude/Latitude E7440
> Latitude/Latitude XT3
> Precision/Precision 3510
> Precision/Precision 3520
> Precision/Precision 3530
> Precision/Precision 3540
> Precision/Precision 3541
> Precision/Precision 3550
> Precision/Precision 3551
> Precision/Precision 3571
> Precision/Precision 5510
> Precision/Precision 5520
> Precision/Precision 5530
> Precision/Precision 5540
> Precision/Precision 7510
> Precision/Precision 7520
> Precision/Precision 7530
> Precision/Precision 7540
> Precision/Precision 7710
> Precision/Precision 7720
> Precision/Precision 7730
> Precision/Precision 7740
> Precision/Precision M2800
> Precision/Precision M3800
> Precision/Precision M4500
> Precision/Precision M4600
> Precision/Precision M4700
> Precision/Precision M4800
> Precision/Precision M6600
> Precision/Precision M6700
> Precision/Precision M6800
> Studio/Studio 1458
> Studio/Studio 1557
> Studio/Studio 1558
> Studio/Studio 1569
> Studio/Studio 1747
> Studio/Studio 1749
> Unidentified/Unidentified System
> Vostro/Vostro 3300
> Vostro/Vostro 3350
> Vostro/Vostro 3400
> Vostro/Vostro 3500
> Vostro/Vostro 3550
> Vostro/Vostro 3560
> Vostro/Vostro 3700
> Vostro/Vostro 5468
> Vostro/Vostro 5471
> Vostro/Vostro 5568
> Vostro/Vostro 7580
> Vostro/Vostro V130
> Vostro/Vostro V131
> XPS/XPS 15 7590
> XPS/XPS 15 9530
> XPS/XPS 15 9550
> XPS/XPS 15 9560
> XPS/XPS 15 9570
> XPS/XPS L401X
> XPS/XPS L412Z
> XPS/XPS L421X
> XPS/XPS L501X
> XPS/XPS L521X
> XPS/XPS L701X
>
> Unsupported:
>
> $ git grep ccelerome | grep "is present on SMBus" | cut -d '/' -f 1,2 | sort
> -u | wc -l
> 129

2023-12-23 12:45:59

by Hans de Goede

[permalink] [raw]
Subject: Re: Ideas for a generic solution to support accelerometer lis3lv02d in Dell laptops/notebooks?

Hi Paul,

On 12/23/23 10:39, Paul Menzel wrote:
> Dear Linux folks,
>
>
> Currently, on Dell systems with the accelerometer lis3lv02d, its I²C address needs to be added to `dell_lis3lv02d_devices[]` in `drivers/i2c/busses/i2c-i801.c`.
>
> In Linux 6.7-rc6 that array has nine elements, so only a small fraction of all Dell notebooks is listed. Searching the Linux logs uploaded to the Linux hardware database from May 2023 [1], there are around 129 devices without support in the Linux kernel version the upload was done with.
>
> Do you know, how the Microsoft Windows driver is doing this? Is it hard-coded there too, or can it be deduced somehow, for example from the ACPI tables?
>
> I added some Kai-Heng and Hans to Cc as they might have contact. Dell offers or offered quite a few of the models with official Ubuntu support, so I would have hoped to have a generic solution for this. Maybe Mario can also forward it to the Dell team.

Interesting question.

So there are really 2 issues here:

a. The probe problem you are describing

b. The support for the lis3lv02d is using an old misc-char + input(evdev)
userspace API defined in:
drivers/platform/x86/dell/dell-smo8800.c
drivers/misc/lis3lv02d/lis3lv02d[_i2c].c

where as it really should be using the IIO interface like
almost all other accelerometer chips are doing. There even
already is a driver for this:
drivers/iio/accel/st_accel[_i2c].c

Here is what I believe should be done to fix this:

1. The handling of instantiating the i2c-client really does NOT
belong in drivers/i2c/busses/i2c-i801.c instead some code
should be added to drivers/platform/x86/dell/dell-smo8800.c
for finding the right i2c-bus and then the code to instantiate
the i2c_client for the lis3lv02d device should be moved to
drivers/platform/x86/dell/dell-smo8800.c .

2. Add a "probe_i2c_address" bool module option and when this
is set try to read the WHO_AM_I register, see
drivers/misc/lis3lv02d/lis3lv02d.c
and if this succeeds and gives a known model id then
continue with the found i2c_address. This should first
try address 0x29 which seems to be the most common and
then try 0x18 and then give up.

This should also modify the dmesg "Accelerometer lis3lv02d is
present on SMBus but its address is unknown, skipping registration\n"

message to hint at trying to use the probe_i2c_address option
with a remark that this could theoretically be dangerous for
the laptop.

And likewise when probe_i2c_address option is set and the
laptop model is not in the DMI list then on successful
probe print a message to please report the i2c-address upstream.

This should resolve (a) from above.

3. Once we have the i2c-client instantiation in dell-smo8800.c
we can add a "use_misc_lis3lv02d" boolean module option there
which defaults to false.

And then if we know the i2c-address and use_misc_lis3lv02d is false:
2.1 register the i2c_client with "lis3lv02dl_accel"
as type instead of "lis3lv02dl", note I think we may need
to use different type-s depending on the WHO_AM_I register
value, the st_accel.c code needs to be checked for this.
2.2 pass the interrupt to the i2c_client driver by setting
it in board_info
2.3 do not register the dell-smo8800.c IRQ handler
(the i2c_client will own the IRQ)
2.4 do not register the dell-smo8800.c misc char device

This solves (b) from above giving us a more standard accel
userspace interface. We do need to evaluate how this
impacts iio-sensor-proxy though, since this now may start
doing screen-rotation based on this!

If you plan to work on this please let me know. I think
the trickiest issue is going to be to find the right i2c-bus
in dell-smo8800.c.

Regards,

Hans






> [1]: https://github.com/linuxhw/Dmesg
>
>
> PS: Dell devices in Linux hardware database with accelerometer:
>
> linux-hardware-dmesg/Notebook/Dell (main)$ git grep -l ccelerome | cut -d '/' -f 1,2 | sort -u
> Inspiron/Inspiron 11 - 3147
> Inspiron/Inspiron 5520
> Inspiron/Inspiron 7547
> Inspiron/Inspiron 7548
> Latitude/Latitude 12 Rugged Extreme
> Latitude/Latitude 2110
> Latitude/Latitude 2120
> Latitude/Latitude 3330
> Latitude/Latitude 3380
> Latitude/Latitude 3400
> Latitude/Latitude 3470
> Latitude/Latitude 3480
> Latitude/Latitude 3490
> Latitude/Latitude 3500
> Latitude/Latitude 3570
> Latitude/Latitude 3580
> Latitude/Latitude 3590
> Latitude/Latitude 5280
> Latitude/Latitude 5290
> Latitude/Latitude 5400
> Latitude/Latitude 5401
> Latitude/Latitude 5410
> Latitude/Latitude 5411
> Latitude/Latitude 5414
> Latitude/Latitude 5420 Rugged
> Latitude/Latitude 5424 Rugged
> Latitude/Latitude 5480
> Latitude/Latitude 5490
> Latitude/Latitude 5491
> Latitude/Latitude 5500
> Latitude/Latitude 5501
> Latitude/Latitude 5510
> Latitude/Latitude 5511
> Latitude/Latitude 5531
> Latitude/Latitude 5580
> Latitude/Latitude 5590
> Latitude/Latitude 5591
> Latitude/Latitude 7214
> Latitude/Latitude 7414
> Latitude/Latitude 7424 Rugged Extreme
> Latitude/Latitude E4310
> Latitude/Latitude E5270
> Latitude/Latitude E5410
> Latitude/Latitude E5420
> Latitude/Latitude E5420m
> Latitude/Latitude E5430 non-vPro
> Latitude/Latitude E5430 vPro
> Latitude/Latitude E5440
> Latitude/Latitude E5470
> Latitude/Latitude E5510
> Latitude/Latitude E5520
> Latitude/Latitude E5520m
> Latitude/Latitude E5530 non-vPro
> Latitude/Latitude E5530 vPro
> Latitude/Latitude E5540
> Latitude/Latitude E5570
> Latitude/Latitude E6220
> Latitude/Latitude E6230
> Latitude/Latitude E6320
> Latitude/Latitude E6330
> Latitude/Latitude E6410
> Latitude/Latitude E6420
> Latitude/Latitude E6430
> Latitude/Latitude E6430s
> Latitude/Latitude E6440
> Latitude/Latitude E64406342Q0286-
> Latitude/Latitude E6510
> Latitude/Latitude E6520
> Latitude/Latitude E6530
> Latitude/Latitude E6540
> Latitude/Latitude E7440
> Latitude/Latitude XT3
> Precision/Precision 3510
> Precision/Precision 3520
> Precision/Precision 3530
> Precision/Precision 3540
> Precision/Precision 3541
> Precision/Precision 3550
> Precision/Precision 3551
> Precision/Precision 3571
> Precision/Precision 5510
> Precision/Precision 5520
> Precision/Precision 5530
> Precision/Precision 5540
> Precision/Precision 7510
> Precision/Precision 7520
> Precision/Precision 7530
> Precision/Precision 7540
> Precision/Precision 7710
> Precision/Precision 7720
> Precision/Precision 7730
> Precision/Precision 7740
> Precision/Precision M2800
> Precision/Precision M3800
> Precision/Precision M4500
> Precision/Precision M4600
> Precision/Precision M4700
> Precision/Precision M4800
> Precision/Precision M6600
> Precision/Precision M6700
> Precision/Precision M6800
> Studio/Studio 1458
> Studio/Studio 1557
> Studio/Studio 1558
> Studio/Studio 1569
> Studio/Studio 1747
> Studio/Studio 1749
> Unidentified/Unidentified System
> Vostro/Vostro 3300
> Vostro/Vostro 3350
> Vostro/Vostro 3400
> Vostro/Vostro 3500
> Vostro/Vostro 3550
> Vostro/Vostro 3560
> Vostro/Vostro 3700
> Vostro/Vostro 5468
> Vostro/Vostro 5471
> Vostro/Vostro 5568
> Vostro/Vostro 7580
> Vostro/Vostro V130
> Vostro/Vostro V131
> XPS/XPS 15 7590
> XPS/XPS 15 9530
> XPS/XPS 15 9550
> XPS/XPS 15 9560
> XPS/XPS 15 9570
> XPS/XPS L401X
> XPS/XPS L412Z
> XPS/XPS L421X
> XPS/XPS L501X
> XPS/XPS L521X
> XPS/XPS L701X
>
> Unsupported:
>
> $ git grep ccelerome | grep "is present on SMBus" | cut -d '/' -f 1,2 | sort -u | wc -l
> 129
>


2023-12-23 12:54:04

by Pali Rohár

[permalink] [raw]
Subject: Re: Ideas for a generic solution to support accelerometer lis3lv02d in Dell laptops/notebooks?

On Saturday 23 December 2023 13:45:32 Hans de Goede wrote:
> 2. Add a "probe_i2c_address" bool module option and when this
> is set try to read the WHO_AM_I register, see
> drivers/misc/lis3lv02d/lis3lv02d.c
> and if this succeeds and gives a known model id then
> continue with the found i2c_address. This should first
> try address 0x29 which seems to be the most common and
> then try 0x18 and then give up.

This is the main problem of the whole email thread. How to figure out
the correct smbus device address.

And we really must not poke random address during kernel boot time.
I think in the past was there enough problems linux kernel broke some HW
or made system unbootable just because it tried to read something from
some random undocumented address.

Please do not try random unverified address on all machines.

smbus is not really bus which provides discovering and identifying
devices on the bus.

2023-12-23 13:40:26

by Hans de Goede

[permalink] [raw]
Subject: Re: Ideas for a generic solution to support accelerometer lis3lv02d in Dell laptops/notebooks?

Hi,

On 12/23/23 13:53, Pali Rohár wrote:
> On Saturday 23 December 2023 13:45:32 Hans de Goede wrote:
>> 2. Add a "probe_i2c_address" bool module option and when this
>> is set try to read the WHO_AM_I register, see
>> drivers/misc/lis3lv02d/lis3lv02d.c
>> and if this succeeds and gives a known model id then
>> continue with the found i2c_address. This should first
>> try address 0x29 which seems to be the most common and
>> then try 0x18 and then give up.
>
> This is the main problem of the whole email thread. How to figure out
> the correct smbus device address.

Ack.

> And we really must not poke random address during kernel boot time.
> I think in the past was there enough problems linux kernel broke some HW
> or made system unbootable just because it tried to read something from
> some random undocumented address.
>
> Please do not try random unverified address on all machines.

Right, that is why this sits behind a module option. Also note
that the 0x29 / 0x18 addresses are typically used by some
sensor ic, which are typically safe to probe.

> smbus is not really bus which provides discovering and identifying
> devices on the bus.

I know I have worked on the lm_sensors project and the
sensors-detect script in the past. Generally speaking
though i2c probing is not that dangerous. But one can
get unlucky ...

We should probably first do 2 single byte i2c-reads
(not smbus byte reads but plain i2c reads) if there
is a i2c device there with the standard smbus register
model where there is a 8 bit register address pointer
then reading 2 times in a row will read 2 different
registers (the internal register address pointer will
increment) so we should get 2 different values.

If we get the same value twice then whatever is
present on address 0x29 or 0x18 does not follow
the standard smbus register addressing and we should
refrain from doing a smbus-byte-read, which first
sends the register-address to read so involves
an actual i2c-*write*.

The combination of determining that normal smbus
register addressing is used + only doing reads
should make probing pretty safe. And the probing
will only happen when the module option is set
in the first place.

Regards,

Hans





2023-12-23 14:22:00

by Pali Rohár

[permalink] [raw]
Subject: Re: Ideas for a generic solution to support accelerometer lis3lv02d in Dell laptops/notebooks?

On Saturday 23 December 2023 14:40:09 Hans de Goede wrote:
> Hi,
>
> On 12/23/23 13:53, Pali Rohár wrote:
> > On Saturday 23 December 2023 13:45:32 Hans de Goede wrote:
> >> 2. Add a "probe_i2c_address" bool module option and when this
> >> is set try to read the WHO_AM_I register, see
> >> drivers/misc/lis3lv02d/lis3lv02d.c
> >> and if this succeeds and gives a known model id then
> >> continue with the found i2c_address. This should first
> >> try address 0x29 which seems to be the most common and
> >> then try 0x18 and then give up.
> >
> > This is the main problem of the whole email thread. How to figure out
> > the correct smbus device address.
>
> Ack.
>
> > And we really must not poke random address during kernel boot time.
> > I think in the past was there enough problems linux kernel broke some HW
> > or made system unbootable just because it tried to read something from
> > some random undocumented address.
> >
> > Please do not try random unverified address on all machines.
>
> Right, that is why this sits behind a module option. Also note
> that the 0x29 / 0x18 addresses are typically used by some
> sensor ic, which are typically safe to probe.
>
> > smbus is not really bus which provides discovering and identifying
> > devices on the bus.
>
> I know I have worked on the lm_sensors project and the
> sensors-detect script in the past. Generally speaking
> though i2c probing is not that dangerous. But one can
> get unlucky ...

I think that manual probing after user confirmation is acceptable. But
automatic one without any user confirmation after kernel upgrade for all
existing and also all future machines is not something which I can say
that is safe.

I have experience when broken bytes in EDID eeprom and monitor
completely stopped working. It was after manual probing of some eeprom
driver (so driver of the correct class). If kernel/X11 logs did not
print warnings about EDID checksum I would not be able to debug & repair
it.

So one can be really unlucky if something happen in the future, like
changing laptop board wiring, changing accelerator chipset or whatever.

I have also experience with i2c device which have broken first i2c
single byte read (first after wakeup from low power state) and product
errata was to put data line to some position for at least some period of
time before doing the real data transfer.

I rather do not want to image what can happen if similar hw bug is in
the i2c multiplexer after which are connected more i2c devices...

After those experiences I want to be really safe about what is kernel
going to do automatically after the boot.

> We should probably first do 2 single byte i2c-reads
> (not smbus byte reads but plain i2c reads) if there
> is a i2c device there with the standard smbus register
> model where there is a 8 bit register address pointer
> then reading 2 times in a row will read 2 different
> registers (the internal register address pointer will
> increment) so we should get 2 different values.
>
> If we get the same value twice then whatever is
> present on address 0x29 or 0x18 does not follow
> the standard smbus register addressing and we should
> refrain from doing a smbus-byte-read, which first
> sends the register-address to read so involves
> an actual i2c-*write*.
>
> The combination of determining that normal smbus
> register addressing is used + only doing reads
> should make probing pretty safe. And the probing
> will only happen when the module option is set
> in the first place.

Hiding all above logic behind module parameter which is disabled by
default sounds safe. I think that this is something against which should
not be too much disagreements (expecting that module parameter will have
correct description with warning, just to be safe).

But well, my guess is that people would like to see accelerometer to
work out-of-the-box without configuring anything.

And this is possible only by communication with Dell. Dell designers
should have some ideas how it is suppose to work. And reinventing the
wheel from scratch in Linux kernel is not the best option.

> Regards,
>
> Hans
>
>
>
>

2024-02-13 14:07:49

by Jean Delvare

[permalink] [raw]
Subject: Re: Ideas for a generic solution to support accelerometer lis3lv02d in Dell laptops/notebooks?

On Sat, 23 Dec 2023 13:53:50 +0100, Pali Rohár wrote:
> smbus is not really bus which provides discovering and identifying
> devices on the bus.

For completeness, SMBus version 2.0 actually added support for device
discovery and even dynamic slave address allocation. This is explained
in chapter 5, section 5.6 (SMBus Address resolution protocol).

Unfortunately, this is an optional feature which requires active
cooperation from each device connected to the bus. If any device on the
bus supports SMBus ARP then you should get an answer when probing
(7-bit) I2C address 0x61.

Long ago I had a plan to add support for SMBus ARP to the kernel, but
gave up because I couldn't find any system implementing it. If the
accelerometer device in Dell laptops supported ARP then we could use it
to figure out the device's address, unfortunately this doesn't seem to
be the case.

--
Jean Delvare
SUSE L3 Support

2024-02-15 18:16:53

by Pali Rohár

[permalink] [raw]
Subject: Re: Ideas for a generic solution to support accelerometer lis3lv02d in Dell laptops/notebooks?

On Tuesday 13 February 2024 15:07:08 Jean Delvare wrote:
> On Sat, 23 Dec 2023 13:53:50 +0100, Pali Rohár wrote:
> > smbus is not really bus which provides discovering and identifying
> > devices on the bus.
>
> For completeness, SMBus version 2.0 actually added support for device
> discovery and even dynamic slave address allocation. This is explained
> in chapter 5, section 5.6 (SMBus Address resolution protocol).
>
> Unfortunately, this is an optional feature which requires active
> cooperation from each device connected to the bus. If any device on the
> bus supports SMBus ARP then you should get an answer when probing
> (7-bit) I2C address 0x61.
>
> Long ago I had a plan to add support for SMBus ARP to the kernel, but
> gave up because I couldn't find any system implementing it. If the
> accelerometer device in Dell laptops supported ARP then we could use it
> to figure out the device's address, unfortunately this doesn't seem to
> be the case.
>
> --
> Jean Delvare
> SUSE L3 Support

According to my notes, accelerometer in Dell laptops should use
LNG3DMTR-LGA16-3x3 chipset. From what I found it should be
pin-compatible with LIS302DL, just in different package.

ST LIS302DL datasheet is on the website:
https://www.st.com/resource/en/datasheet/lis302dl.pdf

It is dual i2c and SPI bus support chipset. But in the datasheet there
is nothing about SMBus, looks like this is designed for i2c usage. So I
highly doubt that chipset supports SMBus version 2.0 with ARP extension.

Anyway, SMBus ARP is new thing to me, I have never heard about it or its
usage before. Has anybody else found some device which supports it?
Would be interesting to know if this is not just another standard which
was not publicly deployed yet.

2024-02-15 19:08:32

by Wolfram Sang

[permalink] [raw]
Subject: Re: Ideas for a generic solution to support accelerometer lis3lv02d in Dell laptops/notebooks?


> Anyway, SMBus ARP is new thing to me, I have never heard about it or its
> usage before. Has anybody else found some device which supports it?
> Would be interesting to know if this is not just another standard which
> was not publicly deployed yet.

SMBus ARP was introduced with spec 2.0 in 2000. I personally have never
seen it used in the wild. I am biased because I am way more familiar
with embedded than, say, servers. But it tells something that we don't
have support for it in the Linux Kernel.


Attachments:
(No filename) (519.00 B)
signature.asc (849.00 B)
Download all attachments

2024-02-27 20:52:45

by Pali Rohár

[permalink] [raw]
Subject: Re: Ideas for a generic solution to support accelerometer lis3lv02d in Dell laptops/notebooks?

On Thursday 15 February 2024 19:16:33 Pali Rohár wrote:
> On Tuesday 13 February 2024 15:07:08 Jean Delvare wrote:
> > On Sat, 23 Dec 2023 13:53:50 +0100, Pali Rohár wrote:
> > > smbus is not really bus which provides discovering and identifying
> > > devices on the bus.
> >
> > For completeness, SMBus version 2.0 actually added support for device
> > discovery and even dynamic slave address allocation. This is explained
> > in chapter 5, section 5.6 (SMBus Address resolution protocol).
> >
> > Unfortunately, this is an optional feature which requires active
> > cooperation from each device connected to the bus. If any device on the
> > bus supports SMBus ARP then you should get an answer when probing
> > (7-bit) I2C address 0x61.
> >
> > Long ago I had a plan to add support for SMBus ARP to the kernel, but
> > gave up because I couldn't find any system implementing it. If the
> > accelerometer device in Dell laptops supported ARP then we could use it
> > to figure out the device's address, unfortunately this doesn't seem to
> > be the case.
> >
> > --
> > Jean Delvare
> > SUSE L3 Support
>
> According to my notes, accelerometer in Dell laptops should use
> LNG3DMTR-LGA16-3x3 chipset. From what I found it should be
> pin-compatible with LIS302DL, just in different package.
>
> ST LIS302DL datasheet is on the website:
> https://www.st.com/resource/en/datasheet/lis302dl.pdf
>
> It is dual i2c and SPI bus support chipset. But in the datasheet there
> is nothing about SMBus, looks like this is designed for i2c usage. So I
> highly doubt that chipset supports SMBus version 2.0 with ARP extension.

Now I checked i2c address 0x61 and nothing responds to it.

> Anyway, SMBus ARP is new thing to me, I have never heard about it or its
> usage before. Has anybody else found some device which supports it?
> Would be interesting to know if this is not just another standard which
> was not publicly deployed yet.