linux-kernel - Re: [PATCH] iio: light: Add support for Capella CM3323 color/light sensor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 25 Jan 2015 12:50:02 +0200
From:	Daniel Baluta <daniel.baluta@...el.com>
To:	Jonathan Cameron <jic23@...nel.org>
Cc:	Daniel Baluta <daniel.baluta@...el.com>,
	Kevin Tsai <ktsai@...ellamicro.com>,
	Hartmut Knaack <knaack.h@....de>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Peter Meerwald <pmeerw@...erw.net>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] iio: light: Add support for Capella CM3323 color/light sensor

On Sun, Jan 25, 2015 at 12:27 PM, Jonathan Cameron <jic23@...nel.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@...ellamicro.com>
>> Signed-off-by: Daniel Baluta <daniel.baluta@...el.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.


Thanks for review!

Daniel.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/