linux-kernel - Re: [RESEND PATCH] iio: light: add support for TI's opt3001 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:	Fri, 3 Oct 2014 22:17:21 -0500
From:	Michael Welling <mwelling@...e.org>
To:	Felipe Balbi <balbi@...com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Jonathan Cameron <jic23@...nel.org>, linux-iio@...r.kernel.org,
	pmeerw@...erw.net,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH] iio: light: add support for TI's opt3001 light
 sensor

On Thu, Oct 02, 2014 at 01:05:53PM -0500, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Sep 30, 2014 at 04:22:04PM -0500, Felipe Balbi wrote:
> > On Mon, Sep 29, 2014 at 06:41:59PM -0500, Michael Welling wrote:
> > > On Mon, Sep 29, 2014 at 05:46:38PM -0500, Felipe Balbi wrote:
> > > > Hi,
> > > > 
> > > > On Mon, Sep 29, 2014 at 05:38:33PM -0500, Michael Welling wrote:
> > > > > > > > Alright, this is already ridiculous. Andrew, if I resend the patch can
> > > > > > > > you apply it since maintainer has been ignoring this thread anyway ?
> > > > > > > > 
> > > > > > > 
> > > > > > > Do you reall think this is the best way to approach this?
> > > > > > 
> > > > > > when maintainer doesn't respond for weeks, yeah! Sure it is.
> > > > > > 
> > > > > > > Perhaps it would be better to explain what each field of the
> > > > > > > configuration register does and then we can move on.
> > > > > > 
> > > > > > perhaps Jonathan could tell me exactly what he wants because I can't
> > > > > > handle back-and-forth anymore. Specially when he complains about stuff
> > > > > > he asked me to modify himself.
> > > > > >
> > > > > > > In particular the OPT3001_CONFIGURATION_L field needs to be explained
> > > > > > > such that the ABI can be properly applied.
> > > > > > > 
> > > > > > > Looking at the docs for the Windows demo program the field is associated
> > > > > > > with a latch configuration. What does this bit field actually do?
> > > > > 
> > > > > Still no technical information. Without all the facts how can you expect
> > > > > him to tell you what he wants?
> > > > 
> > > > yeah, because clearly he doesn't know himself, right ?
> > > 
> > > Could you explain how it works to me then?
> > 
> > checking how much of the docs I can expose now, gimme a couple days.
> 
> alright, here's a snippet of what's on preliminary docs:
> 

Firstly, there is logical error in the latest code.
The hysteresis setting is 0 not 1.

+       if (opt->hysteresis)
+               reg |= OPT3001_CONFIGURATION_L;
+       else
+               reg &= ~OPT3001_CONFIGURATION_L;


> Latch field (read or write).
> The latch field controls the functionality of the interrupt reporting
> mechanisms: the INT pin, the flag high field (FH), and flag low field (FL).
> This bit selects the reporting style between a latched window-style comparison
> and a transparent hysteresis-style comparison.
> 
> 0 = The device functions in transparent hysteresis-style comparison operation,
> where the three interrupt reporting mechanisms directly reflect the comparison
> of the result register with the high- and low-limit registers with no
> user-controlled clearing event. See the Interrupt Operation, INT Pin, and
> Interrupt Reporting Mechanisms section for further details.
> 
> 1 = The device functions in latched window-style comparison operation, latching
> the interrupt reporting mechanisms until a user-controlled clearing event.
> 

How is the interrupt cleared in mode 1 in the latest version of your
code?

> [...]
> 
> 7.4.2.2 Transparent Hysteresis-Style Comparison Mode
> The transparent hysteresis-style comparison mode is typically used when a
> single digital signal is desired that indicates whether the input light is
> higher than or lower than a light level of interest. If the result register is
> higher than the high-limit register for a consecutive number of events set by
> the fault count field, the INT line is set to active, the flag high field is
> set to 1, and the flag low field is set to 0. If the result register is lower
> than the lowlimit register for a consecutive number of events set by the fault
> count field, the INT line is set to inactive, the flag low field is set to 1,
> and the flag high field is set to 0. The INT pin and flag high and flag low
> fields do not change state with configuration reads and writes. The INT pin and
> flag fields continually report the appropriate comparison of the light to the
> low-limit and high-limit registers. The device does not respond to the SMBus
> alert response protocol while in either of the two transparent comparison modes
> (configuration register, latch field = 0).
> 

The ABI confusion starts here.

You are dealing with a mode enable and IIO_EV_INFO_HYSTERESIS is associated
with a the hysteresis level of a threshold event.

http://lxr.free-electrons.com/source/Documentation/ABI/testing/sysfs-bus-iio#L605

You are going to have abstract the register access to conform to the ABI or add
to the ABI as Jonathan suggests.

> -- 
> balbi


--
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/