Re: [PATCH 2/4] leds: simatic-ipc-leds: add new driver for Siemens Industial PCs

[Henning Schild]

Am Wed, 3 Mar 2021 20:31:34 +0100
schrieb Pavel Machek <pavel@xxxxxx>:

> Hi!
> 
> > > > +static struct simatic_ipc_led simatic_ipc_leds_io[] = {
> > > > +	{1 << 15, "simatic-ipc:green:run-stop"},
> > > > +	{1 << 7,  "simatic-ipc:yellow:run-stop"},
> > > > +	{1 << 14, "simatic-ipc:red:error"},
> > > > +	{1 << 6,  "simatic-ipc:yellow:error"},
> > > > +	{1 << 13, "simatic-ipc:red:maint"},
> > > > +	{1 << 5,  "simatic-ipc:yellow:maint"},
> > > > +	{0, ""},
> > > > +};    
> > > 
> > > Please use names consistent with other systems, this is user
> > > visible. If you have two-color power led, it should be
> > > :green:power... See include/dt-bindings/leds/common.h .  
> > 
> > Well we wanted to pick names that are printed on the devices and
> > would like to stick to those. Has been a discussion ...
> > Can we have symlinks to have multiple names per LED?  
> 
> No symlinks. We plan to have command line tool to manipulate LEDs,
> aliases might be possible there.

Sounds like a future plan. sysfs and "cat" "echo" are mighty tools and
"everything is a file" is the best idea ever. So i would say any
aliasing should live in the kernel, but that is just me. Tools will
just get out of sync, be missing in busybox or a random yocto ... or
whichever distro you like.
On the other hand you have "complexity should be userland" ... i do not
have the answer.

> > How strong would you feel about us using our names?  
> 
> Strongly. :-)

OK, will try to find a match where possible. 

> Do you have a picture how the leds look like?

I could even find chassis photos in our internal review but that would
be too much.

Our idea is probably the same as yours. We want the same names across
all devices. But we struggle with colors because on some boxes we have
red+green, while other offer yellow ... implemented in HW and messing
with red+green in some cases.

But so far we only looked at Siemens devices and thought we could get
our own "namespace".

To be honest i could not even tell how our names map on the known ones,
but we will do our best to find a match. They all are "high-level" so
"power" and other basic things are not exposed.

regards,
Henning
 
> Best regards,
> 							Pavel