Re: [PATCH v3 6/6] platform/x86: dell-smo8800: Add support for probing for the accelerometer i2c address

[Hans de Goede <hdegoede@redhat.com>]

Hi Pali,

On 6/22/24 3:32 PM, Pali Rohár wrote:
> On Friday 21 June 2024 14:25:01 Hans de Goede wrote:
>> Unfortunately the SMOxxxx ACPI device does not contain the i2c-address
>> of the accelerometer. So a DMI product-name to address mapping table
>> is used.
> 
> This is statement which I got from Dell for 10 years old Dell models.
> 
> I have already stated that poking of address in kernel is a big risk
> specially for all current and any future dell hardware. Hiding it just
> under the kernel parameter is still a risk, specially as neither its
> name, nor description say that it is dangerous:

I think you are overstating how dangerous this is. lm-sensors detect
scripts has been poking i2c addresses for years without problems (1) and
still does so till today.

Besides the kernel message telling users about this option does mention that
it is dangerous:

>> @@ -370,6 +511,7 @@ static int smo8800_probe(struct platform_device *device)
>>  	} else {
>>  		dev_warn(&device->dev,
>>  			 "lis3lv02d accelerometer is present on SMBus but its address is unknown, skipping registration\n");
>> +		dev_info(&device->dev, "Pass dell_smo8800.probe_i2c_addr=1 on the kernel commandline to probe, this may be dangerous!\n");
>>  		if (!smo8800->irq)
>>  			return -ENODEV;
>>  	}

> But anyway, why this code is being introduced?

Because users have been asking about an easier way to find the address for
not yet supported Dell models:

https://lore.kernel.org/linux-i2c/4820e280-9ca4-4d97-9d21-059626161bfc@molgen.mpg.de/

This is the whole reason why I started working on this patch-set in
the first place.

> Have you communicated
> with Dell about this problem?

Dell is on the Cc of this thread, as well as the previous v2 posting:

Cc: Dell.Client.Kernel@dell.com

Regards,

Hans


1) There were some problems more then a decade ago, but those were only
at specific addresses on some really old (by now) ThinkPads and for
the other case the i2c_safety_check() function was added.