On Mon, Dec 11, 2023 at 06:31:27AM -0800, Guenter Roeck wrote:
> On Mon, Dec 04, 2023 at 05:50:04PM +0100, Stefan Gloor wrote:
> >
> > +#ifdef CONFIG_DEBUG_FS
> > +
> > +static void sht3x_debugfs_init(struct sht3x_data *data)
> > +{
> > + char name[32];
> > + struct dentry *sensor_dir;
> > +
> > + data->debugfs = debugfs_lookup("sht3x", NULL);
> > + if (IS_ERR_OR_NULL(data->debugfs))
> > + data->debugfs = debugfs_create_dir("sht3x", NULL);
> > +
> > + snprintf(name, sizeof(name), "i2c%u-%02x",
> > + data->client->adapter->nr, data->client->addr);
> > + sensor_dir = debugfs_create_dir(name, data->debugfs);
> > + debugfs_create_u32("serial_number", 0444,
> > + sensor_dir, &data->serial_number);
> > +}
> > +
> > +#else
> > +
> > +static void sht3x_debugfs_init(struct sht3x_data *data)
> > +{
> > +}
> > +
> > +#endif
>
> debugfs doesn't need if/else or error handling.
>
Do you mean the IS_ERR_OR_NULL? I included that to get rid of the
"debugfs directory already exists" message when using multiple sensors.
Will remove #ifdef in V3.
>
> > +
> > +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 +947,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");
> > + data->serial_number = 0;
> > + }
> > + sht3x_debugfs_init(data);
>
> The debugfs entry should not be created in the first place if the
> serial number can not be read. On top of that, the debugfs entries
> are never removed, meaning the system will crash if the driver or device
> is unloaded and the no longer referenced debugfs file is accessed.
Got it, will fix in V3.
--
