Re: [PATCH v1 01/11] iio: light: tsl2563: Do not hardcode interrupt trigger type

Jonathan Cameron
2022-12-12T11:10:00Z

On Sun, 11 Dec 2022 18:14:01 +0100
Ferry Toth <fntoth@gmail.com> wrote:

> Hi,
>
> Op 11-12-2022 om 14:26 schreef Jonathan Cameron:
> > On Wed, 7 Dec 2022 21:03:38 +0200
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >
> >> From: Ferry Toth <ftoth@exalondelft.nl>
> >>
> >> Instead of hardcoding IRQ trigger type to IRQF_TRIGGER_RAISING,
> >> let's respect the settings specified in the firmware description.
> >> To be compatible with the older firmware descriptions, if trigger
> >> type is not set up there, we'll set it to default (raising edge).
> >>
> >> Fixes: 388be4883952 ("staging:iio: tsl2563 abi fixes and interrupt handling")
> >> Fixes: bdab1001738f ("staging:iio:light:tsl2563 remove old style event registration.")
> >> Signed-off-by: Ferry Toth <ftoth@exalondelft.nl>
> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > Andy, would have preferred a cover letter, so I had an obvious place
> > to reply to the whole series...
> >
> > Mostly I'm amazed anyone still has one of these devices (I have one but
> > it's on a break out board for the stargate2/imote2 pxa27x platform that we
> > dropped support for last year - I hadn't booted it for a few years)
> > - I can probably bodge it onto something else but I can't say it was
> > high on my todo list ;) So nice to know that someone still cares about
> > this.
> >
> > So I'm curious Ferry, what device has one of these?
>
> It's a breakout board too. I think it's something like GY-2561.
>
> I wanted to write up an example how to get connect iio sensors to work
> with linux. So I asked my colleague who is a great fan of aliexpress if
> he had any sensor on a breakout board with I2C. In the past I had it
> working with MRAA and UPM but that seems to be a dead end now.
>
> We have ACPI working on Intel Edison-Arduino with quite a few examples
> from Andy. And the "Arduino" header makes it very easy to wire up these
> kind of breakout boards, fantastic platform this type of developments.
>
> Just wiring up the I2C and get it to work was easy enough. And then the
> interrupt pin makes an interesting example (even though likely useless
> for most applications of the light sensor).
>
> Write-up here if you are interested:
> https://htot.github.io/meta-intel-edison/4.6-libiio.html

Thanks!

Jonathan

>
> > Whole series applied to the togreg branch of iio.git though note I'll only
> > push this out as testing for now because I'll want to rebase that tree
> > after rc1 is available.
> >
> > Thanks,
> >
> > Jonathan
> >
> >> ---
> >> drivers/iio/light/tsl2563.c | 8 +++++++-
> >> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
> >> index d0e42b73203a..71302ae864d9 100644
> >> --- a/drivers/iio/light/tsl2563.c
> >> +++ b/drivers/iio/light/tsl2563.c
> >> @@ -704,6 +704,7 @@ static int tsl2563_probe(struct i2c_client *client)
> >> struct iio_dev *indio_dev;
> >> struct tsl2563_chip *chip;
> >> struct tsl2563_platform_data *pdata = client->dev.platform_data;
> >> + unsigned long irq_flags;
> >> int err = 0;
> >> u8 id = 0;
> >>
> >> @@ -759,10 +760,15 @@ static int tsl2563_probe(struct i2c_client *client)
> >> indio_dev->info = &tsl2563_info_no_irq;
> >>
> >> if (client->irq) {
> >> + irq_flags = irq_get_trigger_type(client->irq);
> >> + if (irq_flags == IRQF_TRIGGER_NONE)
> >> + irq_flags = IRQF_TRIGGER_RISING;
> >> + irq_flags |= IRQF_ONESHOT;
> >> +
> >> err = devm_request_threaded_irq(&client->dev, client->irq,
> >> NULL,
> >> &tsl2563_event_handler,
> >> - IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> >> + irq_flags,
> >> "tsl2563_event",
> >> indio_dev);
> >> if (err) {
> >
>