Re: ublink(4), led(4) and ledctl(1)
On 19/12/19(Thu) 18:37, Stuart Henderson wrote: > While it's nice to have basic support in the kernel, for people using > these devices for sequences / controlling a chain of neopixels / etc > they're going to need a custom kernel with the driver disabled in order > to access it from userland. Which features do you need that are currently missing in led(4)?
Re: ublink(4), led(4) and ledctl(1)
Stuart Henderson wrote: > While it's nice to have basic support in the kernel, for people using > these devices for sequences / controlling a chain of neopixels / etc > they're going to need a custom kernel with the driver disabled in order > to access it from userland. > > For release users this either locks them out of using syspatch > completely (if they do this by building -stable with custom config - > syspatch won't run at all with -stable), or at least from using syspatch > for kernel fixes (if using config -ef) - so it would be really nice to > have a better way around this before adding more drivers taking over > from existing device support. I disagree. This unbridled ugen/uhid/usb direct-access via libusb has got to stop. "taking over existing device support" is backwards. We don't expose /dev/pci for X anymore, and we should not do it here either.
Re: ublink(4), led(4) and ledctl(1)
While it's nice to have basic support in the kernel, for people using these devices for sequences / controlling a chain of neopixels / etc they're going to need a custom kernel with the driver disabled in order to access it from userland. For release users this either locks them out of using syspatch completely (if they do this by building -stable with custom config - syspatch won't run at all with -stable), or at least from using syspatch for kernel fixes (if using config -ef) - so it would be really nice to have a better way around this before adding more drivers taking over from existing device support.
Re: ublink(4), led(4) and ledctl(1)
On Fri, Dec 13, 2019 at 10:34:59PM +0100, Patrick Wildt wrote:
> Hi,
>
> I have a ThingM blink(1) USB RGB device that shows up as uhid(4).
> The tooling is "interesting", especially with all those libusb and
> HID libraries doing the abstraction. This introduces ublink(4), a
> dedicated kernel driver for that device. There are two LEDs on the
> device, which can be modified seperately. The firmware is impress-
> ive and provides features like playing sequences and adjusting how
> long it should take to fade from one colour to another. Obviously
> this also increases the complexity of the tools involved to toggle
> those RGB LEDs. Thus, the driver is kept simple (for now).
>
> In addition to providing a way to set RGB LEDs, we can also use it
> as a watchdog. If we don't "tickle" it in a specific timeframe it
> will play a (unless otherwise programmed) random sequence, which for
> instance can be used to see that the machine has paniced. This has
> been quite useful while debugging the USB stack, because once the
> magic sequence started you know that you're in deep trouble. All
> other features are unimplemented. Gamma correction would be nice
> to have.
>
> Since there is no abstraction layer for LEDs yet, this also intro-
> duces led(4), which attaches to ublink(4), and provides /dev/ledX.
>
> uhidev1 at uhub0 port 3 configuration 1 interface 0 "ThingM blink(1) mk2" rev
> 2.00/0.02 addr 2
> uhidev1: iclass 3/0, 1 report id
> ublink0 at uhidev1 reportid 1
> led0 at ublink0: 2 LEDs
>
> led(4) can be improved even further. We can attach the ThinkPad
> LEDs to it to control it. There are also plenty of mouses that
> have controllable RGBs. We could add "human readable descrip-
> tions", for instance "upper side LED" or "docking station LED",
> so that users can find out more easily which LED they have to
> toggle. A fading or blinking mechanism, including hardware off-
> loading, can probably be added as well.
>
> To be able to control those devices, there's ledctl(1), a simple
> program that can turn on/off /dev/ledX devices and also set RGB
> colours.
>
> I only did the MAKEDEV stuff for amd64, and also there are no
> manpages yet. Also I haven't run it through make build yet,
> sorry. And I don't often do userland tools, so feel free to
> let me know how to improve it.
>
> Thanks,
> Patrick
Updated diff, changes:
* All the manpages!
* /dev/led{0,1,2,...} are now by default 600.
* Doesn't panic when you remove the stick, I think I
might have already had that fixed in the first diff.
* Some small improvements here and there, nothing special.
Thanks to jan for ledctl(1) manpage and review. And thanks to
cheloha@!
Patrick
diff --git a/etc/MAKEDEV.common b/etc/MAKEDEV.common
index c187df748d6..dc67b54ec5a 100644
--- a/etc/MAKEDEV.common
+++ b/etc/MAKEDEV.common
@@ -521,6 +521,8 @@ __devitem(ipmi, ipmi*, IPMI BMC access)dnl
_mkdev(ipmi, ipmi*, {-M ipmi$U c major_ipmi_c $U 600-})dnl
__devitem(gpio, gpio*, General Purpose Input/Output)dnl
_mcdev(gpio, gpio*, gpio, {-major_gpio_c-}, 600)dnl
+__devitem(led, led*, Generic LED devices)dnl
+_mcdev({-led-}, led*, {-led-}, {-major_led_c-}, 600)dnl
__devitem(vmm, vmm, Virtual Machine Monitor)dnl
_mkdev(vmm, vmm, {-M vmm c major_vmm_c 0 600-})dnl
__devitem(pvbus, pvbus*, paravirtual device tree root)dnl
diff --git a/etc/etc.amd64/MAKEDEV b/etc/etc.amd64/MAKEDEV
index 543b0ec731b..50f7ac53acb 100644
--- a/etc/etc.amd64/MAKEDEV
+++ b/etc/etc.amd64/MAKEDEV
@@ -78,6 +78,7 @@
# gpio* General Purpose Input/Output
# hotplug devices hot plugging
# ipmi* IPMI BMC access
+# led*Generic LED devices
# nvram NVRAM access
# kcovKernel code coverage tracing
# pci*PCI bus devices
@@ -329,6 +330,10 @@ nvram)
M nvram c 85 0 440 kmem
;;
+led*)
+ M led$U c 55 $U 600
+ ;;
+
ipmi*)
M ipmi$U c 96 $U 600
;;
diff --git a/etc/etc.amd64/MAKEDEV.md b/etc/etc.amd64/MAKEDEV.md
index f46b52bd7d6..66e85a16da8 100644
--- a/etc/etc.amd64/MAKEDEV.md
+++ b/etc/etc.amd64/MAKEDEV.md
@@ -76,6 +76,7 @@ _DEV(gpio, 88)
_DEV(hotplug, 82)
_DEV(ipmi, 96)
dnl _DEV(joy, 26)
+_DEV(led, 55)
_DEV(nvram, 85)
_DEV(kcov, 19)
_DEV(pci, 72)
diff --git a/share/man/man4/Makefile b/share/man/man4/Makefile
index 307bd44b66b..45ab7539c60 100644
--- a/share/man/man4/Makefile
+++ b/share/man/man4/Makefile
@@ -41,8 +41,8 @@ MAN= aac.4 abcrtc.4 ac97.4 acphy.4 acrtc.4 \
ip.4 ip6.4 ipcomp.4 ipgphy.4 ipmi.4 ips.4 ipsec.4 ipw.4 \
isa.4 isagpio.4 isapnp.4 islrtc.4 it.4 itherm.4 iwi.4 iwn.4 iwm.4 \
ix.4 ixgb.4 ixl.4 jmb.4 jme.4 jmphy.4 \
- kate.4 kcov.4 km.4 ksmn.4 ksyms.4 kubsan.4 kue.4 lc.4 lge.4 lii.4 \
- lisa.4 lm.4 lmenv.4 lmn.4 lmtemp.4 lo.4 lpt.4 lxtphy.4 luphy.4 \
+ kate.4 kcov.4 km.4 ksmn.4 ksyms.4 kubsan.4 kue.4 lc.4 led.4 lge.4 \
+ lii.4 lisa.4 lm.4 lmenv.4 lmn.4 lmtemp.4 lo.4 lpt.4 lxtphy.4 luphy.4 \
maestro.4 mainbus.4
Re: ublink(4), led(4) and ledctl(1)
On Mon, Dec 16, 2019 at 01:57:41PM +, Stuart Henderson wrote: > On 2019/12/13 22:34, Patrick Wildt wrote: > > uhidev1 at uhub0 port 3 configuration 1 interface 0 "ThingM blink(1) mk2" > > rev 2.00/0.02 addr 2 > > uhidev1: iclass 3/0, 1 report id > > ublink0 at uhidev1 reportid 1 > > led0 at ublink0: 2 LEDs > > Does this attachment break the normal software used with these? > (in ports as misc/blink1) Of course it does! This means that you can use the device only with led(4), and that you can use all of the features provided by led(4), and nothing else. If you want to run it insecurely, with some external tool accessing your USB devices, you will *have to* explicitly disable the kernel driver.
Re: ublink(4), led(4) and ledctl(1)
On 2019/12/13 22:34, Patrick Wildt wrote: > uhidev1 at uhub0 port 3 configuration 1 interface 0 "ThingM blink(1) mk2" rev > 2.00/0.02 addr 2 > uhidev1: iclass 3/0, 1 report id > ublink0 at uhidev1 reportid 1 > led0 at ublink0: 2 LEDs Does this attachment break the normal software used with these? (in ports as misc/blink1)
Re: ublink(4), led(4) and ledctl(1)
On Fri, Dec 13, 2019 at 10:34:59PM +0100, Patrick Wildt wrote: > > Hi, > > I have a ThingM blink(1) USB RGB device that shows up as uhid(4). > The tooling is "interesting", especially with all those libusb and > HID libraries doing the abstraction. This introduces ublink(4), a > dedicated kernel driver for that device. There are two LEDs on the > device, which can be modified seperately. The firmware is impress- > ive and provides features like playing sequences and adjusting how > long it should take to fade from one colour to another. Obviously > this also increases the complexity of the tools involved to toggle > those RGB LEDs. Thus, the driver is kept simple (for now). > > In addition to providing a way to set RGB LEDs, we can also use it > as a watchdog. If we don't "tickle" it in a specific timeframe it > will play a (unless otherwise programmed) random sequence, which for > instance can be used to see that the machine has paniced. This has > been quite useful while debugging the USB stack, because once the > magic sequence started you know that you're in deep trouble. All > other features are unimplemented. Gamma correction would be nice > to have. > > Since there is no abstraction layer for LEDs yet, this also intro- > duces led(4), which attaches to ublink(4), and provides /dev/ledX. > > uhidev1 at uhub0 port 3 configuration 1 interface 0 "ThingM blink(1) mk2" rev > 2.00/0.02 addr 2 > uhidev1: iclass 3/0, 1 report id > ublink0 at uhidev1 reportid 1 > led0 at ublink0: 2 LEDs > > led(4) can be improved even further. We can attach the ThinkPad > LEDs to it to control it. There are also plenty of mouses that > have controllable RGBs. We could add "human readable descrip- > tions", for instance "upper side LED" or "docking station LED", > so that users can find out more easily which LED they have to > toggle. A fading or blinking mechanism, including hardware off- > loading, can probably be added as well. > > To be able to control those devices, there's ledctl(1), a simple > program that can turn on/off /dev/ledX devices and also set RGB > colours. > > I only did the MAKEDEV stuff for amd64, and also there are no > manpages yet. Also I haven't run it through make build yet, > sorry. And I don't often do userland tools, so feel free to > let me know how to improve it. I tested with my ThingM blink(1) mk2: uhidev2 at uhub3 port 4 configuration 1 interface 0 "ThingM blink(1) mk2" rev 2.00/0.02 addr 4 uhidev2: iclass 3/0, 1 report id ublink0 at uhidev2 reportid 1 led0 at ublink0: 2 LEDs Turning the LED on or off, and set it to blue (ledctl ff), all work for me. > Thanks, > Patrick Kevin
Re: ublink(4), led(4) and ledctl(1)
> Date: Sat, 14 Dec 2019 10:20:52 +1100
> From: Jonathan Gray
>
> On Fri, Dec 13, 2019 at 10:34:59PM +0100, Patrick Wildt wrote:
> > Hi,
> >
> > I have a ThingM blink(1) USB RGB device that shows up as uhid(4).
> > The tooling is "interesting", especially with all those libusb and
> > HID libraries doing the abstraction. This introduces ublink(4), a
> > dedicated kernel driver for that device. There are two LEDs on the
> > device, which can be modified seperately. The firmware is impress-
> > ive and provides features like playing sequences and adjusting how
> > long it should take to fade from one colour to another. Obviously
> > this also increases the complexity of the tools involved to toggle
> > those RGB LEDs. Thus, the driver is kept simple (for now).
> >
> > In addition to providing a way to set RGB LEDs, we can also use it
> > as a watchdog. If we don't "tickle" it in a specific timeframe it
> > will play a (unless otherwise programmed) random sequence, which for
> > instance can be used to see that the machine has paniced. This has
> > been quite useful while debugging the USB stack, because once the
> > magic sequence started you know that you're in deep trouble. All
> > other features are unimplemented. Gamma correction would be nice
> > to have.
> >
> > Since there is no abstraction layer for LEDs yet, this also intro-
> > duces led(4), which attaches to ublink(4), and provides /dev/ledX.
>
> There is the machdep.led_blink sysctl CPU_LED_BLINK implemented by
> multiple architectures to blink based on load average.
>
> See
> man4.hppa/lcd.4:.Ar machdep.led_blink
> man4.sparc64/auxio.4:.Ar machdep.led_blink
> man4.sparc64/clkbrd.4:.Ar machdep.led_blink
> man4.sparc64/fhc.4:.Ar machdep.led_blink
> man4.sparc64/led.4:.Ar machdep.led_blink
> man4.sparc64/ppm.4:.Ar machdep.led_blink
>
> sys/arch/alpha/include/cpu.h: { "led_blink", CTLTYPE_INT }
> sys/arch/hppa/include/cpu.h: { "led_blink", CTLTYPE_INT },
> sys/arch/landisk/include/cpu.h: { "led_blink", CTLTYPE_INT }
> sys/arch/sparc64/include/cpu.h: { "led_blink", CTLTYPE_INT },
>
Those machines all have a more-or-less dedicated LED for this.
What could be done is to make it possible to assign this function to
arbitrary LEDs through ledctl(8) such that we can have this
functionality on other architectures as well.
As patrick@ is probably aware of, the device trees on various
armv7/arm64 machines have LED devices in them as well. These are
mostly controlled through GPIOs. It'd make sense if the framework
allowed to control these LEDs as well at some point.
Re: ublink(4), led(4) and ledctl(1)
On Fri, Dec 13, 2019 at 10:34:59PM +0100, Patrick Wildt wrote:
Nice project. If this makes it into the tree then I could rewrite [1].
[1]: https://github.com/younix/g403led
> And I don't often do userland tools, so feel free to
> let me know how to improve it.
> ...
> +int
> +main(int argc, char **argv)
> +{
> ...
> + exit(0);
> +}
I think its better to return 0 in main() instead of calling the libc
function exit(3).
> ... and also there are no manpages yet.
Below is an example for ledctl.8.
Thanks,
Jan
.\" $OpenBSD$
.\"
.\" Copyright (c) 2019 Jan Klemkow
.\"
.\" Permission to use, copy, modify, and distribute this software for any
.\" purpose with or without fee is hereby granted, provided that the above
.\" copyright notice and this permission notice appear in all copies.
.\"
.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
.\"
.Dd $Mdocdate: December 14 2019 $
.Dt LEDCTL 8
.Os
.Sh NAME
.Nm ledctl
.Nd sets the colors of LEDs
.Sh SYNOPSIS
.Nm
.Op Fl f Ar device
.Op Fl n Ar led
.Op Ar on|off|rgb
.Sh DESCRIPTION
The
.Nm
utility turns LEDs
.Ar on ,
.Ar off
or set their color state.
The color value
.Ar rgb
has to be in hex format.
The parameters
.Ar on
and
.Ar off
are shortcuts for the color values 0xff and 0x00.
.Pp
The options are as follows:
.Bl -tag -width Ds
.It Fl f Ar device
.Nm
uses
the device file
.Ar device .
Default is
.Pa /dev/led0 .
.It Fl n Ar led
A
.Xr led 4
device may consists of multiple LEDs.
Sets
.Ar led
to the index of the right LED.
Default is 0.
.El
.Sh FILES
.Pa /dev/led0
.Sh EXIT STATUS
.Ex -std
.Sh EXAMPLES
Sets the first LED of
.Pa /dev/led0
to red:
.Bd -literal -offset indent
$ ledctl 0xff
.Ed
.Pp
Sets the second LED of
.Pa /dev/led0
to green:
.Bd -literal -offset indent
$ ledctl -n 1 0x00ff00
.Ed
.Pp
Sets the first LED of
.Pa /dev/led2
to blue:
.Bd -literal -offset indent
$ ledctl -f /dev/led2 0xff
.Ed
.\".Sh SEE ALSO
.\".Xr led 4
.Sh AUTHORS
The
.Nm
program was written by
.An Patrick Wildt Aq Mt patr...@blueri.se .
signature.asc
Description: PGP signature
Re: ublink(4), led(4) and ledctl(1)
On Fri, Dec 13, 2019 at 10:34:59PM +0100, Patrick Wildt wrote:
> Hi,
>
> I have a ThingM blink(1) USB RGB device that shows up as uhid(4).
> The tooling is "interesting", especially with all those libusb and
> HID libraries doing the abstraction. This introduces ublink(4), a
> dedicated kernel driver for that device. There are two LEDs on the
> device, which can be modified seperately. The firmware is impress-
> ive and provides features like playing sequences and adjusting how
> long it should take to fade from one colour to another. Obviously
> this also increases the complexity of the tools involved to toggle
> those RGB LEDs. Thus, the driver is kept simple (for now).
>
> In addition to providing a way to set RGB LEDs, we can also use it
> as a watchdog. If we don't "tickle" it in a specific timeframe it
> will play a (unless otherwise programmed) random sequence, which for
> instance can be used to see that the machine has paniced. This has
> been quite useful while debugging the USB stack, because once the
> magic sequence started you know that you're in deep trouble. All
> other features are unimplemented. Gamma correction would be nice
> to have.
>
> Since there is no abstraction layer for LEDs yet, this also intro-
> duces led(4), which attaches to ublink(4), and provides /dev/ledX.
There is the machdep.led_blink sysctl CPU_LED_BLINK implemented by
multiple architectures to blink based on load average.
See
man4.hppa/lcd.4:.Ar machdep.led_blink
man4.sparc64/auxio.4:.Ar machdep.led_blink
man4.sparc64/clkbrd.4:.Ar machdep.led_blink
man4.sparc64/fhc.4:.Ar machdep.led_blink
man4.sparc64/led.4:.Ar machdep.led_blink
man4.sparc64/ppm.4:.Ar machdep.led_blink
sys/arch/alpha/include/cpu.h: { "led_blink", CTLTYPE_INT } \
sys/arch/hppa/include/cpu.h:{ "led_blink", CTLTYPE_INT }, \
sys/arch/landisk/include/cpu.h: { "led_blink", CTLTYPE_INT }
\
sys/arch/sparc64/include/cpu.h: { "led_blink", CTLTYPE_INT }, \
Re: ublink(4), led(4) and ledctl(1)
Patrick Wildt wrote:
> On Fri, Dec 13, 2019 at 02:43:25PM -0700, Theo de Raadt wrote:
> > +__devitem(led, led*, Generic LED devices)dnl
> > +_mcdev({-led-}, led*, {-led-}, {-major_led_c-}, 660)dnl
> >
> > Mode 660 leads me to ask -- what is the group?
>
> I don't know, I figured you could help me there. I guess the
> default can be 600 and then, like you wrote in the other mail,
> people will have to chmod/chown it after every upgrade.
Yeah. Or utilize it in a wrapper.
Tough shit :)
Re: ublink(4), led(4) and ledctl(1)
On Fri, Dec 13, 2019 at 02:43:25PM -0700, Theo de Raadt wrote:
> +__devitem(led, led*, Generic LED devices)dnl
> +_mcdev({-led-}, led*, {-led-}, {-major_led_c-}, 660)dnl
>
> Mode 660 leads me to ask -- what is the group?
I don't know, I figured you could help me there. I guess the
default can be 600 and then, like you wrote in the other mail,
people will have to chmod/chown it after every upgrade.
Re: ublink(4), led(4) and ledctl(1)
+__devitem(led, led*, Generic LED devices)dnl
+_mcdev({-led-}, led*, {-led-}, {-major_led_c-}, 660)dnl
Mode 660 leads me to ask -- what is the group?
If you say wheel, that is related to the diff I sent recently.
wheel marks who can su to root. Somehow it has accidentally
gained additional powers over time, which is a disaster because
we have people running JIT in browsers who gain those powers.
I believe we should start removing those powers one by one.
