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

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

On 2/22/20 11:05 AM, Thomas Bogendoerfer wrote:
> On Fri, 21 Feb 2020 21:31:04 +0100
> Jacek Anaszewski <jacek.anaszewski@gmail.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