Received-SPF: pass (google.com: domain of linux-kernel+bounces-10421-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <79134dc1-c71c-4f9e-b721-52385df62cf5@redhat.com>
Date: Sat, 23 Dec 2023 14:40:09 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Ideas for a generic solution to support accelerometer lis3lv02d
 in Dell laptops/notebooks?
Content-Language: en-US, nl
To: =?UTF-8?Q?Pali_Roh=C3=A1r?= <pali@kernel.org>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>, Jean Delvare <jdelvare@suse.com>,
 Andi Shyti <andi.shyti@kernel.org>, Wolfram Sang <wsa@kernel.org>,
 linux-i2c@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
 Kai-Heng Feng <kai.heng.feng@canonical.com>, Marius Hoch
 <mail@mariushoch.de>, Mario Limonciello <mario.limonciello@amd.com>,
 Dell.Client.Kernel@dell.com, Greg KH <gregkh@linuxfoundation.org>
References: <4820e280-9ca4-4d97-9d21-059626161bfc@molgen.mpg.de>
 <a1128471-bbff-4124-a7e5-44de4b1730b7@redhat.com>
 <20231223125350.xqggx3nyzyjjmnut@pali>
From: Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <20231223125350.xqggx3nyzyjjmnut@pali>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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