Messages in this thread | | | From | Andy Shevchenko <> | Date | Tue, 21 Apr 2020 14:24:30 +0300 | Subject | Re: [PATCH v4 2/4] iio: vcnl4000: Add event support for VCNL4010/20. |
| |
On Tue, Apr 21, 2020 at 10:57 AM Mathieu Othacehe <m.othacehe@gmail.com> wrote: > > The VCNL4010 and VCNL4020 chips are able to raise interrupts on proximity > threshold events. Add support for threshold rising and falling events for > those two chips.
Some nitpicks below (up to you and maintainer to address)
...
> +static bool vcnl4010_in_periodic_mode(struct vcnl4000_data *data)
Since it's boolean I would name it ..._is_in_prediodic_mode().
> { > + int ret; > > + ret = i2c_smbus_read_byte_data(data->client, VCNL4000_COMMAND); > + if (ret < 0) > + return false; >
> + return (ret & VCNL4000_SELF_TIMED_EN) > 0;
This > 0 for bitmasked values looks slightly strange. And actually if a sign bit is included, potentially wrong.
I would rather go without any comparison, or do !!(foo & BAR).
> +}
...
> +static bool vcnl4010_thr_enabled(struct vcnl4000_data *data)
_is_thr_enabled() ?
> +{ > + int ret; > + > + ret = i2c_smbus_read_byte_data(data->client, VCNL4010_INT_CTRL); > + if (ret < 0) > + return false; > +
> + return (ret & VCNL4010_INT_THR_EN) > 0;
Ditto.
> +}
...
> + ret = devm_request_threaded_irq(&client->dev, client->irq, > + NULL, vcnl4010_irq_thread, > + IRQF_TRIGGER_FALLING |
> + IRQF_ONESHOT,
Isn't it by default when threaded IRQ is asked with NULL for hw handler?
> + "vcnl4010_irq", > + indio_dev);
-- With Best Regards, Andy Shevchenko
| |