Re: [PATCH v3 2/2] iio: adc: Add TI ADS1100 and ADS1000

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/2/23 05:16, Lars-Peter Clausen wrote:
On 3/1/23 23:49, Mike Looijmans wrote:


...
...

+static int ads1100_runtime_suspend(struct device *dev)
+{
+    struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+    struct ads1100_data *data = iio_priv(indio_dev);
+
+    ads1100_set_config_bits(data, ADS1100_CFG_SC, ADS1100_SINGLESHOT);
+    regulator_disable(data->reg_vdd);
Wrong devm / non-devm ordering.

Don't understand your remark, can you explain further please?

devm / non-devm ordering would be related to the "probe" function. As far as I can tell, I'm not allocating resources after the devm calls. And the "remove" is empty.

Strictly speaking we need to unregister the IIO device before disabling the regulator, otherwise there is a small window where the IIO device still exists, but doesn't work anymore. This is a very theoretical scenario though.

You are lucky :) There is a new function `devm_regulator_get_enable()`[1], which will manage the regulator_disable() for you. Using that will also reduce the boilerplate in `probe()` a bit

- Lars

[1] https://lwn.net/Articles/904383/

Sorry, just saw that Andy's comment was on the suspend() function, not remove(). In that case there is of course no need for any devm things. But still a good idea to use `devm_regulator_get_enable()` in probe for the boiler plate.




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux