Octavian Purdila schrieb am 24.11.2014 11:42:
> On Mon, Nov 24, 2014 at 1:06 AM, Hartmut Knaack <knaack.h@xxxxxx> wrote:
>>> +
>>> +static void bmc150_accel_unregister_triggers(struct bmc150_accel_data *data,
>>> + int from)
>>> +{
>>> + int i;
>>> +
>>> + for (i = from; i >= 0; i++) {
>>> + if (data->triggers[i].indio_trig) {
>>> + iio_trigger_unregister(data->triggers[i].indio_trig);
>>> + data->triggers[i].indio_trig = NULL;
>> Better use devm_iio_trigger_free()?
>
> Wouldn't that be called anyway a little bit later?
I thought it would be cleaner/safer to release it using the opposite of devm_iio_trigger_alloc(), instead of changing its reference to NULL.
>
>>> + }
>>> + i--;
>>> + }
>> i++ and i-- in the same loop looks like an infinite loop to me. Not to mention what happens on the second attempt to unregister the same trigger.
>
> Oops.
>
>>> +}
>>> +
>>> +static int bmc150_accel_triggers_setup(struct iio_dev *indio_dev,
>>> + struct bmc150_accel_data *data)
>>> +{
>>> + int i, ret;
>>> +
>>> + for (i = 0; i < BMC150_ACCEL_TRIGGERS; i++) {
>>> + struct bmc150_accel_trigger *t = &data->triggers[i];
>>> + const char *name = bmc150_accel_triggers[i].name;
>>> + int intr = bmc150_accel_triggers[i].intr;
>>> +
>>> + t->indio_trig = devm_iio_trigger_alloc(&data->client->dev, name,
>>> + indio_dev->name,
>>> + indio_dev->id);
>>> + if (!t->indio_trig) {
>>> + ret = -ENOMEM;
>>> + break;
>>> + }
>>> +
>>> + t->indio_trig->dev.parent = &data->client->dev;
>>> + t->indio_trig->ops = &bmc150_accel_trigger_ops;
>>> + t->intr = &data->interrupts[intr];
>>> + t->data = data;
>>> + iio_trigger_set_drvdata(t->indio_trig, t);
>>> +
>>> + ret = iio_trigger_register(t->indio_trig);
>>> + if (ret)
>>> + break;
>>> + }
>>> +
>>> + if (ret)
>>> + bmc150_accel_unregister_triggers(data, i);
>> I think this should be called with i - 1.
>
> You're right.
>
> Thanks for the reviews Hartmut, I will send a new version after we
> decide on the hardware buffer approach.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
