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

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

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) {  
> >   
>