| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4389cd48-5d61-49bc-8cb5-b337afe40c23@roeck-us.net>
Date: Sun, 31 Dec 2023 10:32:56 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Stefan Gloor <code@...fan-gloor.ch>
Cc: jdelvare@...e.com, corbet@....net, linux-hwmon@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/1] hwmon: (sht3x) read out sensor serial number
On Wed, Dec 27, 2023 at 08:00:36PM +0100, Stefan Gloor wrote:
> The temperature/humidity sensors of the STS3x/SHT3x family are
> calibrated and factory-programmed with a unique serial number.
> For some sensors, this serial number can be used to obtain a calibration
> certificate via an API provided by the manufacturer (Sensirion).
> Expose the serial number via debugfs.
>
> Tested with: 2x STS31, 1x STS32, 1x SHT31
>
> Signed-off-by: Stefan Gloor <code@...fan-gloor.ch>
> ---
> Documentation/hwmon/sht3x.rst | 11 +++++++
> drivers/hwmon/sht3x.c | 56 ++++++++++++++++++++++++++++++++++-
> 2 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/hwmon/sht3x.rst b/Documentation/hwmon/sht3x.rst
> index 957c854f5d08..9585fa7c5a5d 100644
> --- a/Documentation/hwmon/sht3x.rst
> +++ b/Documentation/hwmon/sht3x.rst
> @@ -65,6 +65,10 @@ When the temperature and humidity readings move back between the hysteresis
> values, the alert bit is set to 0 and the alert pin on the sensor is set to
> low.
>
> +The serial number exposed to debugfs allows for unique identification of the
> +sensors. For sts32, sts33 and sht33, the manufacturer provides calibration
> +certificates through an API.
> +
> sysfs-Interface
> ---------------
>
> @@ -99,3 +103,10 @@ repeatability: write or read repeatability, higher repeatability means
> - 1: medium repeatability
> - 2: high repeatability
> =================== ============================================================
> +
> +debugfs-Interface
> +-----------------
> +
> +=================== ============================================================
> +serial_number: unique serial number of the sensor in decimal
> +=================== ============================================================
> diff --git a/drivers/hwmon/sht3x.c b/drivers/hwmon/sht3x.c
> index 79657910b79e..e016e0d9a6c4 100644
> --- a/drivers/hwmon/sht3x.c
> +++ b/drivers/hwmon/sht3x.c
> @@ -10,6 +10,7 @@
>
> #include <asm/page.h>
> #include <linux/crc8.h>
> +#include <linux/debugfs.h>
> #include <linux/delay.h>
> #include <linux/err.h>
> #include <linux/hwmon.h>
> @@ -41,6 +42,9 @@ static const unsigned char sht3x_cmd_heater_off[] = { 0x30, 0x66 };
> /* other commands */
> static const unsigned char sht3x_cmd_read_status_reg[] = { 0xf3, 0x2d };
> static const unsigned char sht3x_cmd_clear_status_reg[] = { 0x30, 0x41 };
> +static const unsigned char sht3x_cmd_read_serial_number[] = { 0x37, 0x80 };
> +
> +static struct dentry *debugfs;
>
> /* delays for single-shot mode i2c commands, both in us */
> #define SHT3X_SINGLE_WAIT_TIME_HPM 15000
> @@ -169,6 +173,7 @@ struct sht3x_data {
> u32 wait_time; /* in us*/
> unsigned long last_update; /* last update in periodic mode*/
> enum sht3x_repeatability repeatability;
> + u32 serial_number;
>
> /*
> * cached values for temperature and humidity and limits
> @@ -831,6 +836,36 @@ static int sht3x_write(struct device *dev, enum hwmon_sensor_types type,
> }
> }
>
> +static void sht3x_debugfs_init(struct sht3x_data *data)
> +{
> + char name[32];
> + struct dentry *sensor_dir;
> +
> + snprintf(name, sizeof(name), "i2c%u-%02x",
> + data->client->adapter->nr, data->client->addr);
> + sensor_dir = debugfs_create_dir(name, debugfs);
> + debugfs_create_u32("serial_number", 0444,
> + sensor_dir, &data->serial_number);
This creates i2c<bus>-<address>/serial_number when the device is instantiated.
That debugfs entry is not removed when the device is removed, only when the
driver is unloaded. This means that de-instantiating the device will leave
stray debugfs directories and files behind until the driver is unloaded.
We had this before, and I understand that you claimed that this doesn't happen.
To get me to believe you, you'll have to provide a log of
- instantiating the driver
- Showing the debufs tree
- de-instantiating the driver
- Showing the debugfs tree
... but even then I'll want to be able to test it myself. Not sure if I
have an eval board, but either case that will take some time. Frankly,
I don't understand why you refuse to remove
i2c<bus>-<address>/serial_number on device removal.
Guenter
> +}
> +
> +static int sht3x_serial_number_read(struct sht3x_data *data)
> +{
> + int ret;
> + char buffer[SHT3X_RESPONSE_LENGTH];
> + struct i2c_client *client = data->client;
> +
> + ret = sht3x_read_from_command(client, data,
> + sht3x_cmd_read_serial_number,
> + buffer,
> + SHT3X_RESPONSE_LENGTH, 0);
> + if (ret)
> + return ret;
> +
> + data->serial_number = (buffer[0] << 24) | (buffer[1] << 16) |
> + (buffer[3] << 8) | buffer[4];
> + return ret;
> +}
> +
> static const struct hwmon_ops sht3x_ops = {
> .is_visible = sht3x_is_visible,
> .read = sht3x_read,
> @@ -899,6 +934,13 @@ static int sht3x_probe(struct i2c_client *client)
> if (ret)
> return ret;
>
> + ret = sht3x_serial_number_read(data);
> + if (ret) {
> + dev_dbg(dev, "unable to read serial number\n");
> + } else {
> + sht3x_debugfs_init(data);
> + }
> +
> hwmon_dev = devm_hwmon_device_register_with_info(dev,
> client->name,
> data,
> @@ -917,7 +959,19 @@ static struct i2c_driver sht3x_i2c_driver = {
> .id_table = sht3x_ids,
> };
>
> -module_i2c_driver(sht3x_i2c_driver);
> +static int __init sht3x_init(void)
> +{
> + debugfs = debugfs_create_dir("sht3x", NULL);
> + return i2c_add_driver(&sht3x_i2c_driver);
> +}
> +module_init(sht3x_init);
> +
> +static void __exit sht3x_cleanup(void)
> +{
> + debugfs_remove_recursive(debugfs);
> + i2c_del_driver(&sht3x_i2c_driver);
> +}
> +module_exit(sht3x_cleanup);
>
> MODULE_AUTHOR("David Frey <david.frey@...sirion.com>");
> MODULE_AUTHOR("Pascal Sachs <pascal.sachs@...sirion.com>");
Powered by blists - more mailing lists