LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Kevin Tsai <ktsai@capellamicro.com>,
	"'Daniel Baluta'" <daniel.baluta@intel.com>
Cc: "'Hartmut Knaack'" <knaack.h@gmx.de>,
	"'Lars-Peter Clausen'" <lars@metafoo.de>,
	"'Peter Meerwald'" <pmeerw@pmeerw.net>,
	linux-iio@vger.kernel.org,
	"'Linux Kernel Mailing List'" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] iio: light: Add support for Capella CM3323 color/light sensor
Date: Mon, 09 Mar 2015 15:45:09 +0000	[thread overview]
Message-ID: <54FDC005.2000905@kernel.org> (raw)
In-Reply-To: <000d01d0551b$9d424bb0$d7c6e310$@capellamicro.com>

On 02/03/15 19:03, Kevin Tsai wrote:
> Hi Daniel,
> 
> Ambient light sensor is trying to match the brightness sensitivity of human visual system.  Please see the following links:
> http://en.wikipedia.org/wiki/Color_vision#mediaviewer/File:Eyesensitivity.svg
> http://en.wikipedia.org/wiki/Color_vision
> 
> You can compare the spectrum with the datasheet.  The green channel is matched with ALS spectrum.
It's close (ish) to the right measurement, but it isn't the right measurement.  As such
I'd argue we leave it up to userspace to make the jump and use Green if a true estimate
if illuminance isn't available, but don't suggest it is available from the kernel.
 
> 
> Kevin Tsai
> 03/02/15
> 
> -----Original Message-----
> From: daniel.baluta@gmail.com [mailto:daniel.baluta@gmail.com] On Behalf Of Daniel Baluta
> Sent: Monday, March 02, 2015 1:57 AM
> To: Daniel Baluta
> Cc: Jonathan Cameron; Kevin Tsai; Hartmut Knaack; Lars-Peter Clausen; Peter Meerwald; linux-iio@vger.kernel.org; Linux Kernel Mailing List
> Subject: Re: [PATCH] iio: light: Add support for Capella CM3323 color/light sensor
> 
> On Sun, Jan 25, 2015 at 12:50 PM, Daniel Baluta <daniel.baluta@intel.com> wrote:
>> On Sun, Jan 25, 2015 at 12:27 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>>> On 22/01/15 10:10, Daniel Baluta wrote:
>>>> Minimal implementation providing raw light intensity and illuminance 
>>>> readings. For illuminance user can compute lux values using raw 
>>>> readings and scale.
>>>>
>>>> This driver also supports CM3323E sensor chip.
>>>>
>>>> Cc: Kevin Tsai <ktsai@capellamicro.com>
>>>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>>>
>>> Hi Daniel,
>>>
>>> My only real question on this one is whether using the 'green' 
>>> channel and pretending it is a measure of illuminance is a good idea 
>>> or whether we are better leaving that decision to userspace...
>>>
>>> I'm guessing the reason it is green rather than clear is that the 
>>> clear is letting infrared through and we don't have an additional 
>>> infrared sensor available to allow that component to be removed?
>>
>> Hi Jonathan,
>>
>> <snip>
>>>> +#define CM3323_COLOR_CHANNEL(_color, _addr) {\
>>>> +     .type = IIO_INTENSITY, \
>>>> +     .modified = 1, \
>>>> +     .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>>>> +     .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME), \
>>>> +     .channel2 = IIO_MOD_LIGHT_##_color, \
>>>> +     .address = _addr, \
>>>> +}
>>>> +
>>>> +/*
>>>> + * CM3323's GREEN channel is used for reading illuminance
>>> That's impressively random and unlikely to be anywhere near correct 
>>> pretty much all the time!
>>>
>>> I'd personally prefer just not providing an illuminance channel and 
>>> leaving it up to userspace to make the decision on whether the green 
>>> channel is good enough...
>>
>> Datasheet says that the GREEN channel should be used for getting 
>> illuminance readings. I'm not sure if this is totally random.
>>
>> I understand your worries, perhaps Kevin could 'enlighten' us here.
>>
>> I would  prefer to have the illuminance channel exported from IIO 
>> because there are already userspace applications/frameworks  written 
>> and tested for ALS that use IIO illuminance channel.
>> We want to use this driver without modifying user space.
>>
>> Of course, one option for the moment would be to only export the 
>> INTENSITY channels as per your recommendation and to decide later what 
>> to do for the LIGHT channel.
> 
> Hi Kevin,
> 
> Could you tell us how reliable is using the GREEN channel for getting illuminance readings?
> 
> If no objection, I will resend this driver exposing only INTENSITY channels for the moment.
> 
> thanks,
> Daniel.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


  reply	other threads:[~2015-03-09 15:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-22 10:10 Daniel Baluta
2015-01-22 10:48 ` Peter Meerwald
2015-01-25 10:27 ` Jonathan Cameron
2015-01-25 10:50   ` Daniel Baluta
2015-03-02  9:57     ` Daniel Baluta
2015-03-02 19:03       ` Kevin Tsai
2015-03-09 15:45         ` Jonathan Cameron [this message]
2015-03-09 20:38           ` Daniel Baluta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54FDC005.2000905@kernel.org \
    --to=jic23@kernel.org \
    --cc=daniel.baluta@intel.com \
    --cc=knaack.h@gmx.de \
    --cc=ktsai@capellamicro.com \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmeerw@pmeerw.net \
    --subject='Re: [PATCH] iio: light: Add support for Capella CM3323 color/light sensor' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).