linux-kernel - Re: [PATCH v4] leds: add SGI IP30 led support

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] [day] [month] [year] [list]

Message-ID: <7bd606df-a5b6-6f05-43ef-b6691e16be94@gmail.com>
Date:   Sat, 22 Feb 2020 17:36:09 +0100
From:   Jacek Anaszewski <jacek.anaszewski@...il.com>
To:     Thomas Bogendoerfer <tbogendoerfer@...e.de>
Cc:     Pavel Machek <pavel@....cz>, Dan Murphy <dmurphy@...com>,
        linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org
Subject: Re: [PATCH v4] leds: add SGI IP30 led support

On 2/22/20 11:05 AM, Thomas Bogendoerfer wrote:
> On Fri, 21 Feb 2020 21:31:04 +0100
> Jacek Anaszewski <jacek.anaszewski@...il.com> wrote:
> 
> 
>>> +	if (num == 0)
>>> +		data->cdev.name = "white:indicator";
>>> +	else
>>> +		data->cdev.name = "red:indicator";
>>
>> Why indicator? Whereas it sounds quite generic it is used in the LED
>> subsystem specifically for naming indicator LEDs, that are often
>> found on flash LED controllers and are designed to indicate camera
>> sensor activity.
> 
> ok, ic.
> 
>> If it is on the front panel of SGI Octane workstations then its
>> function is perhaps well known?
> 
> the red LED is clearly a fault led.

We have fault in common LED names, so you can go for it.

> The white LED will be switch on
> after system diagnostic was successfull. Nothing from common.h really
> fits that, maybe status ?

I think that "system" would be OK. There are some LED drivers
and DT bindings that use "system" in the LED names. I even proposed
that also in the initial versions of my patch set standardizing LED
naming but dropped it afterwards, since it its use was not clear.

But now we have good use case for it.

And one more thing: please add definitions for LEDs in your
driver like:

#define IP30_LED_SYSTEM 0
#define IP30_LED_FAULT  1

and use them instead of bare 0 and 1 numbers.

-- 
Best regards,
Jacek Anaszewski