| Message ID | 20231227190036.20725-2-code@stefan-gloor.ch (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: (sht3x) read out sensor serial number | expand |
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@stefan-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@sensirion.com>"); > MODULE_AUTHOR("Pascal Sachs <pascal.sachs@sensirion.com>");
On Sun, Dec 31, 2023 at 10:32:56AM -0800, Guenter Roeck wrote: > 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 > Hi Guenter, Thank you for your patience. As this is my first patch set for Linux I still need to learn a lot. You are right. I was confused about driver instantiation and driver loading/unloading. The i2cX-XX directory needs to be removed explicitly. If I understood correctly, the following changes should achieve this: +static void sht3x_remove(struct i2c_client *client) +{ + struct sht3x_data *data; + + data = dev_get_drvdata(&client->dev); + debugfs_remove_recursive(data->sensor_dir); +} + static struct i2c_driver sht3x_i2c_driver = { .driver.name = "sht3x", .probe = sht3x_probe, + .remove = sht3x_remove, .id_table = sht3x_ids, }; Of course data->sensor_dir needs to be set to the i2X-XX directory when it is created. If there is nothing obviously wrong with it I'll submit v4 shortly. Best, Stefan
On Mon, Jan 01, 2024 at 11:18:21PM +0100, Stefan Gloor wrote: > On Sun, Dec 31, 2023 at 10:32:56AM -0800, Guenter Roeck wrote: > > 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 > > > > Hi Guenter, > > Thank you for your patience. As this is my first patch set for Linux I still > need to learn a lot. > > You are right. I was confused about driver instantiation and driver > loading/unloading. The i2cX-XX directory needs to be removed explicitly. > > If I understood correctly, the following changes should achieve this: > > +static void sht3x_remove(struct i2c_client *client) > +{ > + struct sht3x_data *data; > + > + data = dev_get_drvdata(&client->dev); > + debugfs_remove_recursive(data->sensor_dir); > +} > + > static struct i2c_driver sht3x_i2c_driver = { > .driver.name = "sht3x", > .probe = sht3x_probe, > + .remove = sht3x_remove, > .id_table = sht3x_ids, > }; > > Of course data->sensor_dir needs to be set to the i2X-XX directory when it is > created. > > If there is nothing obviously wrong with it I'll submit v4 shortly. > This is correct. I personally would prefer the use of devm_add_action_or_reset() instead of a remove function, but the above works as well. Make sure though that debugfs_remove_recursive() is also called if hwmon registration fails. Thanks, Guenter
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); +} + +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@sensirion.com>"); MODULE_AUTHOR("Pascal Sachs <pascal.sachs@sensirion.com>");
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@stefan-gloor.ch> --- Documentation/hwmon/sht3x.rst | 11 +++++++ drivers/hwmon/sht3x.c | 56 ++++++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 1 deletion(-)