From: Jean Delvare on
On Wed, 3 Mar 2010 02:29:09 -0800, Dima Zavin wrote:
> Jean,
>
> Thanks for the prompt reply.
>
> >> I definitely see the need for what you guys are trying to accomplish.
> >> For example, currently, we use an input device for reporting events,
> >> and a separate misc device node for control
> >> (enable/disable/configure). It's definitely suboptimal, but there
> >> currently isn't anything there would let us do things cleanly.
> >>
> >> What I would love to see is a more generic sensors framework that
> >> handles different kinds of sensor devices, and different data
> >> acquisition schemes (sampled vs. change notifications).
> >>
> >> I would love to work with you to design something more generic.
> >
> > This can happen later, I see no reason to block the creation of the ALS
> > subsystem. Having a common framework for all ambient light sensor
> > drivers will already be a step forward compared to the current
>
> Following the logic of putting the ALS subsystem under drivers/als, we
> would then put the proximity subsystem under drivers/proximity, and
> then an accelerometer subsystem under drivers/accelerometer, etc. Each
> with their own implementation of very similar set of interfaces. Is
> that what you envision? I just figured that instead of creating
> one-off interfaces for some subset of environmental sensors such as
> als, we can add a sensors subsystem of which als is just an instance.

We need ALS support yesterday, so it can't build on top of a more
generic framework that doesn't exist today. If you want to design such
a generic framework, feel free, go ahead, and when it's ready, merge
everything you want in it. But we will not wait for you.

> > situation. If improvements are needed on top of this, this can happen
> > later.
>
> I'm just concerned that instead of solving the actual problem, you are
> adding what is essentially a temporary solution.

This is certainly true. But you have to realize that _all_ the kernel
code is a temporary solution. We change over 2000 lines of code every
day on average.

> This will only make
> it harder to solve the real issue by introducing new interfaces which
> will need to be obsoleted unless they are designed with care. What you
> are proposing already needs improvements since there are plenty of
> drivers floating out there from many OEMs/vendors that are not ALSs,
> but essentially need a similar interface (e.g. proximity sensor).

I don't care at all. I want ALS support, and I want it now. If you or
others want something else, go work on it. And if several subsystems
should share something, it can happen later. The point is, you won't
know what they need to share exactly until they are all already there
each with a decent set of drivers. So there is no real benefit in
designing the big thing first, because you'd have to adjust it later
anyway.

> Furthermore, are there more patches coming for this subsystem? Based
> on the above tree, it just seems to be a class device (without any
> standard attributes) and a register/unregister function. It doesn't
> seem to actually be doing anything. Registering with the als subsystem
> at the moment buys the driver nothing. So, in its current state, I'm
> not sure I see what this new common framework actually provides us,
> and thus I'm not sure that it's actually a step forward.

There are two major benefits:
1* The set of available ALS drivers become visible: just look into
drivers/als and you get most of them, search for callers of the
registration function and you get them all. This will avoid redundant
code development, and will make adding new drivers easier.
2* All ALS devices will be reachable at the same location in sysfs.
There is no way user-space can consume whatever the drivers provide
without this common entry point. And we need user-space consumers,
otherwise we just don't know what the real needs are.

> The drivers
> are still responsible to provide all their own non-standard,
> incompatible sysfs interfaces for exporting the sensor values.

Apparently you did not read the patches all that carefully. One of them
creates a documentation file describing the standard sysfs files for ALS
drivers. All ALS driver must comply with it, so they are neither
non-standard nor incompatible. Note that the sysfs files are defined as
a "testing ABI", because we are well aware that it might change in a
near feature. We will move it to "stable ABI" when ready.

> If
> there are other patches for the als subsys that are then used by the
> two drivers that got moved into drivers/als, I'd love to take a look
> at them.

Not that I am aware of. But again, it doesn't mean this won't happen
later, if the need arises. How do you expect us to know what code
should be common as long as we don't have a central place to store all
drivers and a common interface to their devices?

--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jonathan Cameron on
On 03/03/10 10:30, Linus Walleij wrote:
> 2010/3/3 Dima Zavin <dmitriyz(a)google.com>:
>
>> how does this deal with hybrid devices?
>
> As any hybrid device, a drivers/mfd spawns multiple sensors, als,
> accelerometer, voltage level, you name it.
>
>> I definitely see the need for what you guys are trying to accomplish.
>> For example, currently, we use an input device for reporting events,
>> and a separate misc device node for control
>> (enable/disable/configure). It's definitely suboptimal, but there
>> currently isn't anything there would let us do things cleanly.
>
> Are you registering your misc node from an mfd device then?
>
>> I would love to work with you to design something more generic.
>
> You can design forever, people need this now. (But we'd love
> to see the patches!) It's better to refactor the day something
> better is in place IMHO. Also I see no real clash. The userspace
> interface will likely be the same (input subsystem) so what's the
> problem?
>
> In the drivers/staging/iio in the -next tree you can find something
> more generic for industrial I/O including ADCs, triggers and
> some sensors. I pointed out sometime last month that this
> has the problem of exposing only userland interfaces and no
> kernel-internal interfaces for the actual devices (just sysfs entries),
> so the current ALS subsystem cannot fit into it, for example.

We welcome patches for that as well ;)

(It is definitely on the todo list, but I'm afraid not terribly
near the top from my point of view as I don't personally need it!)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Linus Walleij on
2010/3/3 Dima Zavin <dmitriyz(a)google.com>:

> there are plenty of
> drivers floating out there from many OEMs/vendors

Bad for them that they didn't work on integrating these to the
mainline kernel and creating proper subsystems for them
earlier.

> that are not ALSs,
> but essentially need a similar interface (e.g. proximity sensor).

I have a proximity sensor driver pending here on top of GPIO,
creating an input-on-top-of-GPIO driver framework. Too much
to do but here is the userspace interface:

diff --git a/include/linux/input.h b/include/linux/input.h
index a5802c9..cbe8a98 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -644,6 +644,7 @@ struct input_absinfo {
#define SW_RADIO SW_RFKILL_ALL /* deprecated */
#define SW_MICROPHONE_INSERT 0x04 /* set = inserted */
#define SW_DOCK 0x05 /* set = plugged into dock */
+#define SW_PROXIMITY 0x06 /* set = prox. sensor detects object */
#define SW_MAX 0x0f
#define SW_CNT (SW_MAX+1)

--

Comes in through /dev/input/event* something, simple.

I have heard about proximity sensors returning more than a binary
value, do you have one of those?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Linus Torvalds on


On Tue, 2 Mar 2010, Dima Zavin wrote:
>
> I definitely see the need for what you guys are trying to accomplish.
> For example, currently, we use an input device for reporting events,
> and a separate misc device node for control
> (enable/disable/configure). It's definitely suboptimal, but there
> currently isn't anything there would let us do things cleanly.

I have to say, I personally don't see why something like an ambient light
sensor _isn't_ just an input device.

What's the difference between a physical "increase screen brightness" key,
and a "ambient light sensor"? Absolutely none as far as I can tell.

And for something like an X server, it sounds a lot more natural to just
have another input device than to have yet abother event reporting
interface.

And quite frankly, the "explanations" I see in this thread for why it
needs to be a subsystem of its own don't actually explain anything or make
sense. They seem to boil down to "we just did it this way" without
actually answering any of the issues brought up.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jonathan Cameron on
Cc'ing Input list and maintainer.
>
> On Tue, 2 Mar 2010, Dima Zavin wrote:
>>
>> I definitely see the need for what you guys are trying to accomplish.
>> For example, currently, we use an input device for reporting events,
>> and a separate misc device node for control
>> (enable/disable/configure). It's definitely suboptimal, but there
>> currently isn't anything there would let us do things cleanly.
>
> I have to say, I personally don't see why something like an ambient light
> sensor _isn't_ just an input device.
>
> What's the difference between a physical "increase screen brightness" key,
> and a "ambient light sensor"? Absolutely none as far as I can tell.
>
> And for something like an X server, it sounds a lot more natural to just
> have another input device than to have yet abother event reporting
> interface.
>
> And quite frankly, the "explanations" I see in this thread for why it
> needs to be a subsystem of its own don't actually explain anything or make
> sense. They seem to boil down to "we just did it this way" without
> actually answering any of the issues brought up.
>
> Linus
>

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