lkml.org 
[lkml]   [2023]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 2/2] iio: adc: Add TI ADS1100 and ADS1000
From
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.

\
 
 \ /
  Last update: 2023-03-27 00:41    [W:0.080 / U:0.548 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site