On Sat, 8 Feb 2020 18:12:03 -0800
Matt Ranostay <matt.ranostay@xxxxxxxxxxxx> wrote:
> > On Feb 8, 2020, at 08:39, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> >
> > On Wed, 5 Feb 2020 22:13:30 -0800
> > Matt Ranostay <matt.ranostay@xxxxxxxxxxxx> wrote:
> >
> >> Sensors don't actually need a interrupt line to give valid readings,
> >> and can triggered with CONFIG_IIO_HRTIMER_TRIGGER as well. Remove
> >> the required check for interrupt, and continue along in the probe
> >> function.
> >
> > Basic aim is good, but if you don't have an interrupt doesn't make
> > sense to register the trigger either.
> >
> > The interrupt enable / disable is also tied up with the buffer whereas
> > it should probably be done via the trigger enable callback or am I
> > missing something?
>
> It was for allowing sysfs and hrtimer triggers. But just remembered most these sensors have a low refresh rate. I can go either way on having it or not. Thoughts?
I'm a bit confused. With sysfs and hrtimer triggers why would we
need the interrupt enabled? As things stand it will be as it's done
in the buffer setup. I was suggesting moving it to the trigger
setup so we only deal with the interrupt if we are actually using
the data ready trigger.
So move it the atlas_set_interrupt call from pre / post enable to
the trigger state callback.
Jonathan
>
> - Matt
>
>
> >
> > Jonathan
> >
> >>
> >> Signed-off-by: Matt Ranostay <matt.ranostay@xxxxxxxxxxxx>
> >> ---
> >> drivers/iio/chemical/atlas-sensor.c | 27 ++++++++++++---------------
> >> 1 file changed, 12 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
> >> index 2f0a6fed2589..2e34c82cb65d 100644
> >> --- a/drivers/iio/chemical/atlas-sensor.c
> >> +++ b/drivers/iio/chemical/atlas-sensor.c
> >> @@ -572,11 +572,6 @@ static int atlas_probe(struct i2c_client *client,
> >> if (ret)
> >> return ret;
> >>
> >> - if (client->irq <= 0) {
> >> - dev_err(&client->dev, "no valid irq defined\n");
> >> - return -EINVAL;
> >> - }
> >> -
> >> ret = chip->calibration(data);
> >> if (ret)
> >> return ret;
> >> @@ -596,16 +591,18 @@ static int atlas_probe(struct i2c_client *client,
> >>
> >> init_irq_work(&data->work, atlas_work_handler);
> >>
> >> - /* interrupt pin toggles on new conversion */
> >> - ret = devm_request_threaded_irq(&client->dev, client->irq,
> >> - NULL, atlas_interrupt_handler,
> >> - IRQF_TRIGGER_RISING |
> >> - IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> >> - "atlas_irq",
> >> - indio_dev);
> >> - if (ret) {
> >> - dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
> >> - goto unregister_buffer;
> >> + if (client->irq <= 0) {
> >> + /* interrupt pin toggles on new conversion */
> >> + ret = devm_request_threaded_irq(&client->dev, client->irq,
> >> + NULL, atlas_interrupt_handler,
> >> + IRQF_TRIGGER_RISING |
> >> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> >> + "atlas_irq",
> >> + indio_dev);
> >> +
> >> + if (ret)
> >> + dev_warn(&client->dev,
> >> + "request irq (%d) failed\n", client->irq);
> >> }
> >>
> >> ret = atlas_set_powermode(data, 1);
> >
