Re: [PATCH 5/5] max44000: Initial triggered buffer support

[Jonathan Cameron <jic23@kernel.org>]

On 11/04/16 17:11, Crestez Dan Leonard wrote:
> On 04/07/2016 10:59 PM, Peter Meerwald-Stadler wrote:
>>>   static int max44000_probe(struct i2c_client *client,
>>>                 const struct i2c_device_id *id)
>>>   {
>>> @@ -513,6 +569,12 @@ static int max44000_probe(struct i2c_client *client,
>>>           return ret;
>>>       }
>>>
>>> +    ret = iio_triggered_buffer_setup(indio_dev, NULL, max44000_trigger_handler, NULL);
>>> +    if (ret < 0) {
>>> +        dev_err(&client->dev, "iio triggered buffer setup failed\n");
>>> +        return ret;
>>> +    }
>>> +
>>>       return iio_device_register(indio_dev);
>>
>> no devm_ possible anymore :-)
> 
> It's not clear to me why explicit calls to
> iio_triggered_buffer_cleanup are done in every driver. Wouldn't it be
> possible for iio_dev_release to call iio_triggered_buffer_cleanup?
> That would require triggered_buffer_cleanup to explictly NULL the
> fields it deallocates so that duplicate cleanups don't crash.
Because not all devices are using the triggered buffer setup function
thus they can't use the cleanup one.  There are different buffers for
starters and the core has no way to identify them (very deliberately
as it should never care what they are).

There is also the general obvious correctness of code question.
If you do one thing in probe, it should be reverse in remove...

We could in theory do a devm version of the iio_triggered_buffer_cleanup
function though as then the tear down would occur in the write order and
as it is a devm_ call in the probe we'd know how the unwind was being
done...


> -- 
> Regards,
> Leonard