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--;
> > + }
> 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
