From: Hans de Goede <hdegoede@redhat.com>
To: "Pali Rohár" <pali@kernel.org>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
"Andy Shevchenko" <andy@kernel.org>,
"Paul Menzel" <pmenzel@molgen.mpg.de>,
"Wolfram Sang" <wsa@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>,
eric.piel@tremplin-utc.net, Marius Hoch <mail@mariushoch.de>,
Dell.Client.Kernel@dell.com,
Kai Heng Feng <kai.heng.feng@canonical.com>,
platform-driver-x86@vger.kernel.org,
Jean Delvare <jdelvare@suse.com>,
Andi Shyti <andi.shyti@kernel.org>,
linux-i2c@vger.kernel.org
Subject: [PATCH v3 6/6] platform/x86: dell-smo8800: Add support for probing for the accelerometer i2c address
Date: Fri, 21 Jun 2024 14:25:01 +0200 [thread overview]
Message-ID: <20240621122503.10034-7-hdegoede@redhat.com> (raw)
In-Reply-To: <20240621122503.10034-1-hdegoede@redhat.com>
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.
At support to have the kernel probe for the i2c-address for modesl
which are not on the list.
The new probing code sits behind a new probe_i2c_addr module parameter,
which is disabled by default because probing might be dangerous.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/dell/dell-smo8800.c | 152 ++++++++++++++++++++++-
1 file changed, 147 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/x86/dell/dell-smo8800.c b/drivers/platform/x86/dell/dell-smo8800.c
index 4c79b2599d96..d64d200e927a 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"
+#define LIS3_WHO_AM_I 0x0f
#include <linux/device/bus.h>
#include <linux/dmi.h>
@@ -25,6 +26,10 @@
#include <linux/uaccess.h>
#include <linux/workqueue.h>
+static bool probe_i2c_addr;
+module_param(probe_i2c_addr, bool, 0444);
+MODULE_PARM_DESC(probe_i2c_addr, "Probe the i801 I2C bus for the accelerometer on models where the address is unknown");
+
struct smo8800_device {
u32 irq; /* acpi device irq */
atomic_t counter; /* count after last read */
@@ -225,6 +230,130 @@ static const struct dmi_system_id smo8800_lis3lv02d_devices[] = {
{ }
};
+/*
+ * This is the kernel version of the single register device sanity checks from
+ * the i2c_safety_check function from lm_sensors sensor-detect script:
+ * This is meant to prevent access to 1-register-only devices,
+ * which are designed to be accessed with SMBus receive byte and SMBus send
+ * byte transactions (i.e. short reads and short writes) and treat SMBus
+ * read byte as a real write followed by a read. The device detection
+ * routines would write random values to the chip with possibly very nasty
+ * results for the hardware. Note that this function won't catch all such
+ * chips, as it assumes that reads and writes relate to the same register,
+ * but that's the best we can do.
+ */
+static int i2c_safety_check(struct device *dev, struct i2c_adapter *adap, u8 addr)
+{
+ union i2c_smbus_data smbus_data;
+ int err;
+ u8 data;
+
+ /*
+ * First receive a byte from the chip, and remember it. This
+ * also checks if there is a device at the address at all.
+ */
+ err = i2c_smbus_xfer(adap, addr, 0, I2C_SMBUS_READ, 0,
+ I2C_SMBUS_BYTE, &smbus_data);
+ if (err < 0)
+ return err;
+
+ data = smbus_data.byte;
+
+ /*
+ * Receive a byte again; very likely to be the same for
+ * 1-register-only devices.
+ */
+ err = i2c_smbus_xfer(adap, addr, 0, I2C_SMBUS_READ, 0,
+ I2C_SMBUS_BYTE, &smbus_data);
+ if (err < 0)
+ return err;
+
+ if (smbus_data.byte != data)
+ return 0; /* Not a 1-register-only device. */
+
+ /*
+ * Then try a standard byte read, with a register offset equal to
+ * the read byte; for 1-register-only device this should read
+ * the same byte value in return.
+ */
+ err = i2c_smbus_xfer(adap, addr, 0, I2C_SMBUS_READ, data,
+ I2C_SMBUS_BYTE_DATA, &smbus_data);
+ if (err < 0)
+ return err;
+
+ if (smbus_data.byte != data)
+ return 0; /* Not a 1-register-only device. */
+
+ /*
+ * Then try a standard byte read, with a slightly different register
+ * offset; this should again read the register offset in return.
+ */
+ err = i2c_smbus_xfer(adap, addr, 0, I2C_SMBUS_READ, data ^ 0x01,
+ I2C_SMBUS_BYTE_DATA, &smbus_data);
+ if (err < 0)
+ return err;
+
+ if (smbus_data.byte != (data ^ 0x01))
+ return 0; /* Not a 1-register-only device. */
+
+ /*
+ * Apparently this is a 1-register-only device, restore the original
+ * register value and leave it alone.
+ */
+ i2c_smbus_xfer(adap, addr, 0, I2C_SMBUS_WRITE, data,
+ I2C_SMBUS_BYTE, NULL);
+ dev_warn(dev, "I2C safety check for address 0x%02x failed, skipping\n", addr);
+ return -ENODEV;
+}
+
+static int smo8800_detect_accel(struct smo8800_device *smo8800,
+ struct i2c_adapter *adap, u8 addr,
+ struct i2c_board_info *info, bool probe)
+{
+ union i2c_smbus_data smbus_data;
+ const char *type;
+ int err;
+
+ if (probe) {
+ dev_info(smo8800->dev, "Probing for accelerometer on address 0x%02x\n", addr);
+ err = i2c_safety_check(smo8800->dev, adap, addr);
+ if (err < 0)
+ return err;
+ }
+
+ err = i2c_smbus_xfer(adap, addr, 0, I2C_SMBUS_READ, LIS3_WHO_AM_I,
+ I2C_SMBUS_BYTE_DATA, &smbus_data);
+ if (err < 0) {
+ dev_warn(smo8800->dev, "Failed to read who-am-i register: %d\n", err);
+ return err;
+ }
+
+ /* who-am-i register mappings from drivers/misc/lis3lv02d/lis3lv02d.c */
+ switch (smbus_data.byte) {
+ case 0x32:
+ type = "lis331dlh";
+ break;
+ case 0x33:
+ type = "lis2de12"; /* LIS3DC / HP3DC in drivers/misc/lis3lv02d/lis3lv02d.c */
+ break;
+ case 0x3a:
+ type = "lis3lv02dl_accel";
+ break;
+ case 0x3b:
+ type = "lis302dl";
+ break;
+ default:
+ dev_warn(smo8800->dev, "Unknown who-am-i register value 0x%02x\n",
+ smbus_data.byte);
+ return -ENODEV;
+ }
+
+ dev_dbg(smo8800->dev, "Detected %s accelerometer on address 0x%02x\n", type, addr);
+ strscpy(info->type, "lis3lv02d", I2C_NAME_SIZE);
+ info->addr = addr;
+ return 0;
+}
+
static int smo8800_find_i801(struct device *dev, void *data)
{
struct i2c_adapter *adap, **adap_ret = data;
@@ -247,6 +376,7 @@ static void smo8800_instantiate_i2c_client(struct work_struct *work)
const struct dmi_system_id *lis3lv02d_dmi_id;
struct i2c_board_info info = { };
struct i2c_adapter *adap = NULL;
+ int err;
if (smo8800->i2c_dev)
return;
@@ -256,11 +386,22 @@ static void smo8800_instantiate_i2c_client(struct work_struct *work)
return;
lis3lv02d_dmi_id = dmi_first_match(smo8800_lis3lv02d_devices);
- if (!lis3lv02d_dmi_id)
+ if (lis3lv02d_dmi_id) {
+ info.addr = (long)lis3lv02d_dmi_id->driver_data;
+ /* Always detect the accel-type, this also checks the accel is actually there */
+ err = smo8800_detect_accel(smo8800, adap, info.addr, &info, false);
+ if (err)
+ goto out_put_adapter;
+ } else if (probe_i2c_addr) {
+ /* First try address 0x29 (most used) and then try 0x1d */
+ if (smo8800_detect_accel(smo8800, adap, 0x29, &info, true) != 0 &&
+ smo8800_detect_accel(smo8800, adap, 0x1d, &info, true) != 0) {
+ dev_warn(smo8800->dev, "failed to probe for lis3lv02d I2C address\n");
+ goto out_put_adapter;
+ }
+ } else {
goto out_put_adapter;
-
- info.addr = (long)lis3lv02d_dmi_id->driver_data;
- strscpy(info.type, "lis3lv02d", I2C_NAME_SIZE);
+ }
smo8800->i2c_dev = i2c_new_client_device(adap, &info);
if (IS_ERR(smo8800->i2c_dev)) {
@@ -357,7 +498,7 @@ static int smo8800_probe(struct platform_device *device)
smo8800->irq);
}
- if (dmi_check_system(smo8800_lis3lv02d_devices)) {
+ if (dmi_check_system(smo8800_lis3lv02d_devices) || probe_i2c_addr) {
/*
* Register i2c-bus notifier + queue initial scan for lis3lv02d
* i2c_client instantiation.
@@ -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;
}
--
2.45.1
next prev parent reply other threads:[~2024-06-21 12:26 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-21 12:24 [PATCH v3 0/6] i2c-i801 / dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800 Hans de Goede
2024-06-21 12:24 ` [PATCH v3 1/6] i2c: core: Setup i2c_adapter runtime-pm before calling device_add() Hans de Goede
2024-06-21 15:08 ` Andy Shevchenko
2024-06-21 12:24 ` [PATCH v3 2/6] i2c: i801: Use a different adapter-name for IDF adapters Hans de Goede
2024-06-21 15:13 ` Andy Shevchenko
2024-06-22 12:46 ` Pali Rohár
2024-06-22 13:56 ` Hans de Goede
2024-06-22 14:08 ` Pali Rohár
2024-06-22 14:14 ` Hans de Goede
2024-06-22 14:23 ` Pali Rohár
2024-06-22 14:29 ` Hans de Goede
2024-06-22 15:07 ` Pali Rohár
2024-06-23 13:58 ` Hans de Goede
2024-06-21 12:24 ` [PATCH v3 3/6] platform/x86: dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800 Hans de Goede
2024-06-21 15:24 ` Andy Shevchenko
2024-06-22 13:59 ` Hans de Goede
2024-06-22 13:16 ` Pali Rohár
2024-06-22 14:06 ` Hans de Goede
2024-06-22 14:20 ` Pali Rohár
2024-06-22 14:26 ` Hans de Goede
2024-06-22 15:12 ` Pali Rohár
2024-06-22 16:35 ` Pali Rohár
2024-06-23 13:56 ` Hans de Goede
2024-06-23 14:09 ` Hans de Goede
2024-06-22 22:36 ` Andy Shevchenko
2024-06-22 22:41 ` Pali Rohár
2024-06-22 16:26 ` Pali Rohár
2024-06-23 13:46 ` Hans de Goede
2024-06-22 16:43 ` Pali Rohár
2024-06-22 22:43 ` Andy Shevchenko
2024-06-22 22:50 ` Pali Rohár
2024-06-22 22:53 ` Andy Shevchenko
2024-06-23 14:00 ` Hans de Goede
2024-06-22 15:35 ` Pali Rohár
2024-06-23 13:45 ` Hans de Goede
2024-06-23 14:30 ` Pali Rohár
2024-06-21 12:24 ` [PATCH v3 4/6] platform/x86: dell-smo8800: Allow lis3lv02d i2c_client instantiation without IRQ Hans de Goede
2024-06-21 15:30 ` Andy Shevchenko
2024-06-22 13:20 ` Pali Rohár
2024-06-22 14:07 ` Hans de Goede
2024-06-22 15:14 ` Pali Rohár
2024-06-21 12:25 ` [PATCH v3 5/6] platform/x86: dell-smo8800: Add a couple more models to dell_lis3lv02d_devices[] Hans de Goede
2024-06-21 12:25 ` Hans de Goede [this message]
2024-06-21 15:37 ` [PATCH v3 6/6] platform/x86: dell-smo8800: Add support for probing for the accelerometer i2c address Andy Shevchenko
2024-06-22 13:32 ` Pali Rohár
2024-06-22 14:21 ` Hans de Goede
2024-06-22 14:50 ` Pali Rohár
2024-06-22 22:50 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240621122503.10034-7-hdegoede@redhat.com \
--to=hdegoede@redhat.com \
--cc=Dell.Client.Kernel@dell.com \
--cc=andi.shyti@kernel.org \
--cc=andy@kernel.org \
--cc=eric.piel@tremplin-utc.net \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jdelvare@suse.com \
--cc=kai.heng.feng@canonical.com \
--cc=linux-i2c@vger.kernel.org \
--cc=mail@mariushoch.de \
--cc=pali@kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=pmenzel@molgen.mpg.de \
--cc=wsa@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.