On 10/25/2016 07:54 PM, Jason Gunthorpe wrote:
On Tue, Oct 25, 2016 at 06:50:01PM -0700, Guenter Roeck wrote:
+
+ const struct attribute_group *attr_groups[5];
There can be up to 5 active groups total, so this array needs to have
6 entries (it needs to be NULL_terminated).
Agh right, I forgot about that. I will send a v2
{
struct lm87_data *data = i2c_get_clientdata(client);
- hwmon_device_unregister(data->hwmon_dev);
- lm87_remove_files(client);
-
lm87_write_value(client, LM87_REG_CONFIG, data->config);
Strictly speaking this is now racy: The sysfs attributes still exist here,
meaning the chip can still be accessed. Consider using devm_add_action();
then you can drop the remove function entirely, and simplify error cleanup
in the probe function.
I guess, but that is very obscure. If you unload the driver while
monitoring it with sysfs and the boot firmware left the chip in the
low power state then a sysfs right might race and return garbage..
Exactly my point.
I can switch it to use hwmon_device_register_with_groups and put the
hwmon_device_unregister back - I'm not sure wrapping the write in devm
would be a simplification..
Sure, that is a possibility. I personally prefer devm_add_action().
Effectively you claim that devm_add_action() would not add value,
meaning there are - as of today - 67 calls in the kernel which don't add
value. I'll leave that up to you, though - not worth arguing about.
Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
