On 14/01/17 20:00, Brian Masney wrote: > On Sun, Dec 04, 2016 at 11:16:04AM +0000, Jonathan Cameron wrote: >> On 04/12/16 02:19, Brian Masney wrote: >>> isl29028_enable_proximity() has a boolean argument named enable. This >>> function is only called once and the enable flag is set to true in that >>> call. This patch removes the enable parameter from that function. >>> >>> Signed-off-by: Brian Masney <masneyb@xxxxxxxxxxxxx> >> >> The first thing that strikes me about this, is why do we have an enable >> only function? >> >> I think the intention was probably that we also disable the proximity >> sensing after the >> reading was done... Ideally we'd do this a little more cleverly, >> perhaps using runtime >> pm so that if someone is requesting a stream of proximity measurements, >> we won't end up >> powering up and down each time. >> >> It's a little 'interesting' as we would want to power this element down >> even if we do >> have a continuous stream of reads on the ALS. As such we may need to >> roll our own >> equivalent of runtime pm. >> >> In the first instance, I'd just put a disable after the reading is >> taken. This will >> make a bit of a mockery of the faster sampling frequencies but there we >> are! >> >> --------------------- >> >> On second thoughts (stupid email is hiding somewhere to be sent when I >> have wifi so can't reply to it) perhaps this is a coarse way of only >> turning proximity on if the LED is present? Not sure... > > Hi Jonathan, > > I chained your two replies together above. I am probably stating the > obvious here, but I've verified with an oscilloscope that the IRDR pin > that drives the external LED is off when the chip is first initialized > and ALS readings are taken. The IRDR pin fluctuates between high and low > every 100us (if memory serves me right) once the first proximity reading > is taken until the chip is suspended. > > What do you think about enabling runtime auto suspend after say 2 > seconds for the whole device? There is the situation that you describe > where if someone is continuously polling the ALS but asks for a single > proximity reading. The external LED will stay on in that case. Once the > chip is suspended, and later resumes, the IRDR pin that drives the > external LED will be off until the user asks for another proximity > reading. That would allow for the faster sampling frequency. > > If you still prefer, I'll go the route of shutting down the IRDR pin > after a proximity reading is taken. Perhaps runtime auto suspend for the whole thing is the simplest option. I don't mind that much either way. Jonathan > > Brian > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
