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
