2014-01-01 20:11:15

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH] Add LED pattern trigger

On Tue, 31 Dec 2013 13:48:50 -0500
Joe Xue <[email protected]> wrote:

> >> + * Based on Richard Purdie's ledtrig-timer.c and Atsushi Nemoto's
> >> + * ledtrig-heartbeat.c and Shuah Khan's ledtrig-transient.c
> >
> > I stil think this belongs in user space except for platforms with hardware
> > acceleration for it.
>
> This can free the user space application from loop or thread.

Which is not a good reason for putting it in the kernel. I could make the
same argument for putting firefox in the kernel ...

> > This doesn't as far as I can see do what you think. If I have the file
> > currently open then device_remove_file will not remove my existing access
> > to it, but you just released the pattern data so I now write to free
> > memory.
>
> will add the mutex lock to avoid that

Ok


2014-01-01 22:54:23

by David Lang

[permalink] [raw]
Subject: Re: [PATCH] Add LED pattern trigger

On Wed, 1 Jan 2014, One Thousand Gnomes wrote:

> On Tue, 31 Dec 2013 13:48:50 -0500
> Joe Xue <[email protected]> wrote:
>
>>>> + * Based on Richard Purdie's ledtrig-timer.c and Atsushi Nemoto's
>>>> + * ledtrig-heartbeat.c and Shuah Khan's ledtrig-transient.c
>>>
>>> I stil think this belongs in user space except for platforms with hardware
>>> acceleration for it.
>>
>> This can free the user space application from loop or thread.
>
> Which is not a good reason for putting it in the kernel. I could make the
> same argument for putting firefox in the kernel ...
>

I agree with you that this isn't a good reason, but I think the performance
could be a reason.

whatever mechanism is created for toggling LEDs should be able to toggle
arbitrary GPIO pins, and there is a problem with the speed of the standard
access mechanisms in /sysfs. see this post on hackaday for an example

http://hackaday.com/2013/12/07/speeding-up-beaglebone-black-gpio-a-thousand-times/

now, it may be that telling people to access /dev/mem is deemed a better option,
but I would hope not.

Also, since there are a number of cases where this is hardware accelerated, it
seems like there should be an abstration that userspace can use that doesn't
care if or how it's accelerated, setup the output and tell the system to do it
without worrying about the specific hardware details. Isn't that a large part of
what the kernel is supposed to be doing?

David Lang

2014-01-01 23:02:08

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH] Add LED pattern trigger

> whatever mechanism is created for toggling LEDs should be able to toggle
> arbitrary GPIO pins, and there is a problem with the speed of the standard
> access mechanisms in /sysfs. see this post on hackaday for an example
>
> http://hackaday.com/2013/12/07/speeding-up-beaglebone-black-gpio-a-thousand-times/

The usage described is short human speed flashing patterns for things like
"my brain fell out" from devices, not trying to do 1KHz PWM dimming.
Dimming might actually be one case you want the kernel interface,
although it'll kill your power management.

> Also, since there are a number of cases where this is hardware accelerated, it
> seems like there should be an abstration that userspace can use that doesn't
> care if or how it's accelerated, setup the output and tell the system to do it
> without worrying about the specific hardware details. Isn't that a large part of
> what the kernel is supposed to be doing?

Not usually. The kernel is supposed to be providing a consistent interface
to hardware, not emulating bits you don't have. Now and then it does (Eg
FPU emulation) but in general the job it does is "make all the network
cards look the same" not "make pretend network cards out of string and
cups". It's not a hard and fast rule in either direction. There are cases
the kernel doesn't try and create a common interface for the hardware
because the abstraction that can be done at kernel level would be
nonsensical.

A library also allows a higher level of abstraction and better security,
and it allows consistency a kernel interface cannot provide as well as
not pinning down memory which at least in embedded space is valuable (and
may become more so in the 'internet of things' world of lower and lower
power and cheaper and cheaper widgets)

At best a kernel interface would mean people writing "post 3.15 do this,
pre 3.15 do the other". A library interface avoids that as it will work
with old kernels too, and can be taught to interface with acceleration
features, or even with other device types. A kernel interface cannot
drive X.10 for example, drive a remote bluetooth display, beep the code or
flash the LED patterns as an overlay on a monitor. Nor can it do things
like automatically routing the alert based upon stuff like "is the
display on", "is the management interface up" etc.

The library interface can also be made to do sensible things in virtual
environments, on other operating systems and so forth.

Alan

2014-01-01 23:52:41

by David Lang

[permalink] [raw]
Subject: Re: [PATCH] Add LED pattern trigger

On Wed, 1 Jan 2014, One Thousand Gnomes wrote:

>> whatever mechanism is created for toggling LEDs should be able to toggle
>> arbitrary GPIO pins, and there is a problem with the speed of the standard
>> access mechanisms in /sysfs. see this post on hackaday for an example
>>
>> http://hackaday.com/2013/12/07/speeding-up-beaglebone-black-gpio-a-thousand-times/
>
> The usage described is short human speed flashing patterns for things like
> "my brain fell out" from devices, not trying to do 1KHz PWM dimming.
> Dimming might actually be one case you want the kernel interface,
> although it'll kill your power management.

any high speed signalling would probably also need the kernel interface.

>> Also, since there are a number of cases where this is hardware accelerated, it
>> seems like there should be an abstration that userspace can use that doesn't
>> care if or how it's accelerated, setup the output and tell the system to do it
>> without worrying about the specific hardware details. Isn't that a large part of
>> what the kernel is supposed to be doing?
>
> Not usually. The kernel is supposed to be providing a consistent interface
> to hardware, not emulating bits you don't have. Now and then it does (Eg
> FPU emulation) but in general the job it does is "make all the network
> cards look the same" not "make pretend network cards out of string and
> cups". It's not a hard and fast rule in either direction. There are cases
> the kernel doesn't try and create a common interface for the hardware
> because the abstraction that can be done at kernel level would be
> nonsensical.

fair enough, would it make sense to redirect the discussion to focus on what a
good interface would be for the cases where there is special hardware
assistance? Then the discussion of if the same interface could/should be used to
emulate harsdware when it isn't there can be a seprate discussion.

And while this use case the original developer had in mind was the 'I've lost my
mind' notification to a human, I think it makes sense to consider other uses for
repeating pattern toggling of GPIO ports.

David Lang

2014-01-03 00:14:57

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH] Add LED pattern trigger

On Wed, Jan 1, 2014 at 3:51 PM, David Lang <[email protected]> wrote:
> On Wed, 1 Jan 2014, One Thousand Gnomes wrote:
>
>>> whatever mechanism is created for toggling LEDs should be able to toggle
>>> arbitrary GPIO pins, and there is a problem with the speed of the
>>> standard
>>> access mechanisms in /sysfs. see this post on hackaday for an example
>>>
>>>
>>> http://hackaday.com/2013/12/07/speeding-up-beaglebone-black-gpio-a-thousand-times/
>>
>>
>> The usage described is short human speed flashing patterns for things like
>> "my brain fell out" from devices, not trying to do 1KHz PWM dimming.
>> Dimming might actually be one case you want the kernel interface,
>> although it'll kill your power management.
>
>
> any high speed signalling would probably also need the kernel interface.
>
>
>>> Also, since there are a number of cases where this is hardware
>>> accelerated, it
>>> seems like there should be an abstration that userspace can use that
>>> doesn't
>>> care if or how it's accelerated, setup the output and tell the system to
>>> do it
>>> without worrying about the specific hardware details. Isn't that a large
>>> part of
>>> what the kernel is supposed to be doing?
>>
>>
>> Not usually. The kernel is supposed to be providing a consistent interface
>> to hardware, not emulating bits you don't have. Now and then it does (Eg
>> FPU emulation) but in general the job it does is "make all the network
>> cards look the same" not "make pretend network cards out of string and
>> cups". It's not a hard and fast rule in either direction. There are cases
>> the kernel doesn't try and create a common interface for the hardware
>> because the abstraction that can be done at kernel level would be
>> nonsensical.
>
>
> fair enough, would it make sense to redirect the discussion to focus on what
> a good interface would be for the cases where there is special hardware
> assistance? Then the discussion of if the same interface could/should be
> used to emulate harsdware when it isn't there can be a seprate discussion.
>
> And while this use case the original developer had in mind was the 'I've
> lost my mind' notification to a human, I think it makes sense to consider
> other uses for repeating pattern toggling of GPIO ports.
>
> David Lang

Hi folks,

I probably missed this hot discussion during the holiday in our
linux-leds community. After read all the emails of this topic, I think
this is a good idea to take this pattern drivers as a led trigger.

Actually we will see more new LED chips with hardware acceleration and
most of them support like data pattern operations. (see
drivers/leds/leds-lp55xx-common.c) Having a generic led trigger driver
will be good for those LED chip drivers.

Also for user space application, I think we don't have any user space
LED library, if I'm wrong please correct me. Why there is no such
library, since we don't need it.
led trigger driver can provide most generic led operations and people
don't need to know what's kind of hardware is under neath. If it has
hardware acceleration, LED framework will use it otherwise just use
normal operation. Take a look at other trigger drivers, heartbeat etc.
All of the operations can be done via sysfs.

So for the user space application, we just simply need shell scripts.
1. load LED pattern trigger module
2. setup pattern trigger to LED /sys interface
3. then use it.

IMHO, firstly we should take this trigger into kernel, most time it
works as a module. But we need to define a good interface between
kernel and user space.

Thanks,
-Bryan

2014-01-03 09:33:56

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] Add LED pattern trigger

On Fri, Jan 3, 2014 at 1:14 AM, Bryan Wu <[email protected]> wrote:
> IMHO, firstly we should take this trigger into kernel, most time it
> works as a module. But we need to define a good interface between
> kernel and user space.

Put it in first place into the kernel such that it becomes ABI and
nobody is allowed
to remove it later?

Better design a sane interface such that such complex LED operations
can be achieved
in userspace using an helper library.

--
Thanks,
//richard

2014-01-03 15:23:43

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH] Add LED pattern trigger

> Also for user space application, I think we don't have any user space
> LED library, if I'm wrong please correct me. Why there is no such
> library, since we don't need it.

No - rght now it is a case of "we don't have a kernel driver because we
don't need one"

> IMHO, firstly we should take this trigger into kernel, most time it
> works as a module. But we need to define a good interface between
> kernel and user space.

You need the interface defined first. To do that it needs to reflect the
actual hardware accelerated devices, and also to deal with resource
management for those devices if necessary (eg if they can only manage one
led of a set at a time).

Your API can't handle things like brightness level, cross-fades
(which require multiple LEDs handled as one unit) and the like.

So the starting point has to be the hardware accelerated devices, whether
you then support software emulation in kernel or user space is a follow
on discussion. What the kernel/user API is also has to be a follow on
discussion from understanding what the hardware accelerated devices can
do and what their limits are.

Alan

2014-01-05 22:23:52

by Pavel Machek

[permalink] [raw]
Subject: n900 led compiler (was Re: [PATCH] Add LED pattern trigger)

Hi!

> > IMHO, firstly we should take this trigger into kernel, most time it
> > works as a module. But we need to define a good interface between
> > kernel and user space.
>
> You need the interface defined first. To do that it needs to reflect the
> actual hardware accelerated devices, and also to deal with resource
> management for those devices if necessary (eg if they can only manage one
> led of a set at a time).

Hardware can do quite a lot:

http://wiki.maemo.org/LED_patterns#Lysti_Format_Engine_Patterns_and_Commands

(and more).

I implemented compiler for it (should we put it into tools/ somewhere?)

https://gitorious.org/tui/tui/source/5b3f5cacf8e208d3ea50d6066e549940d85e55be:maemo/notcc.py

It can do quite a lot, including prime number computation. This uses
33% of program memory and only one of three execution units; but it
does not work, maybe I made mistake somewhere or maybe our kernel
can't take program this long. It only has 3 writable variables, which
is quite limiting.

start()
a = 1
next_number: a += 1
b = 1
next_divisor: b += 1
br = b
if (b==a) goto is_prime
c = 0
c = c+b
test_prime: if (c==a) goto not_prime
if (a<c) goto not_divisor
c = c+b
goto test_prime
not_divisor: goto next_divisor
not_prime: goto next_number
is_prime: b = 0
show_prime: c = 255
br = c
ramp_up_long(30,0)
c = 0
br = c
ramp_up_long(30,0)
b += 1
if (b == a) goto next_number2
goto show_prime
next_number2: goto next_number

> So the starting point has to be the hardware accelerated devices, whether
> you then support software emulation in kernel or user space is a follow
> on discussion. What the kernel/user API is also has to be a follow on

Well... this one is turing complete, and I have a compiler, but I
don't think it is good interface, either for library or kernel.

I believe series of RGB values might be good interface, maybe with
additional "want interpolation between these".
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-01-07 15:40:24

by Linus Walleij

[permalink] [raw]
Subject: Re: n900 led compiler (was Re: [PATCH] Add LED pattern trigger)

On Sun, Jan 5, 2014 at 11:23 PM, Pavel Machek <[email protected]> wrote:

> I implemented compiler for it (should we put it into tools/ somewhere?)

We have a precedent for putting firmware compilers into the kernel
tree:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/aic7xxx/aicasm

But that one used FLEX. And I think the assumption is that then
you store the firmware source with the driver, and the sysfs (or similar)
interface would just load and trigger one of the pre-defined firmware
programs, not have it be sent in from userspace.

Yours,
Linus Walleij