Re: [RFC/PATCH] iio: light: add support for TI's opt3001 ligth sensor

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 3108 bytes --]

Hi,

(your email setup is still messed up, I had to rewrap the whole email)

On Thu, Aug 07, 2014 at 05:58:57PM +0100, Jonathan Cameron wrote:
> >>> I can set low limit to 200 lux and high limit to 50 lux, then I'll
> >>> get a rising edge IRQ when I have more than 50 lux and a falling
> >>> edge when I get less than 200 lux.
> >>> 
> >>> If Latch bit is set, however, then it's almost like it ignores
> >>> low/high limit altogether and I get an IRQ ever $int_time ms or
> >>> so.
> >> That would be similar to the motion triggers we have for
> >> accelerometers. They capture every sample as long as the threshold
> >> is met (during motion). No idea why you'd really want to do that on
> >> a light sensor though.  Perhaps not worth capturing light info if
> >> the cover is closed on a phone?
> > 
> > Yeah, but that's something else altogether. This device doesn't
> > really care about a phone cover. Sounds like policy which should be
> > in userland, or part of an iio_gpio_trigger...
> Agreed.  Can you think of a use for the non latched version? :)

Sure, you might just want a notification when illuminance reaches a
certain level (rising) or drop below another (or the same) level
(falling).

> >>>>> +	ret = devm_iio_device_register(dev, iio);
> >>>>> +	if (ret) {
> >>>>> +		dev_err(dev, "failed to register IIO device\n");
> >>>>> +		return ret;
> >>>>> +	}
> >>>>> +
> >>>> Normally we'd expect the devm_iio_device_register (that provides
> >>>> the userspace interfaces) to be after the irq request.
> >>> 
> >>> that's wrong. By the time the IRQ is requested, an IRQ can
> >>> actually fire and you better have a valid e.g.  iio_dev by then.
> >> Its very unusual to bring up a device with the IRQ already in a
> >> position to fire.  Normally that would require some enabling from
> >> userspace after the driver is loaded?  How can it fire here before
> >> that point?
> > 
> > at the moment you call request_irq() (or any of its friends), that
> > IRQ line is unmasked and ready to fire. Remember that IRQ lines can
> > be shared and your IRQ handler will be called even if a separate
> > device triggered the IRQ. Heck, if you have a bad board design, even
> > noise can assert the IRQ line but let's not go there :-)
> IIRC Sharing is only possible if the device driver explicitly allows
> it?

not, necessarily. It can be set by the arch support code.

> Bad board design can get you in many many ways ;)
> > 
> > In any case, you better have valid pointers around so that by the
> > time your IRQ is triggered, you don't crash the kernel. Another way
> > would be to mask the IRQ at your device level, but that still
> > doesn't solve shared IRQs.
> > 
> > I usually request the IRQ as the last step for that very reason.
> Fair enough - easy way to be sure I guess.  Though by just before the
> register of the device all relevant pointers should be available.  It
> won't do anything particularly helpful but it should not crash. If it
> does then we have a bug we should probably close.

alright.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]