Re: 0.led_name 2.other.led.name in /sysfs Re: [PATCH/RFC v11 01/20] leds: flash: document sysfs interface

[Jacek Anaszewski <j.anaszewski@samsung.com>]

On 02/20/2015 04:36 PM, Greg KH wrote:
> On Fri, Feb 20, 2015 at 08:56:11AM +0100, Jacek Anaszewski wrote:
>> On 02/19/2015 10:40 PM, Greg KH wrote:
>>> On Thu, Feb 19, 2015 at 11:02:04AM +0200, Sakari Ailus wrote:
>>>> On Wed, Feb 18, 2015 at 11:47:47PM +0100, Pavel Machek wrote:
>>>>>
>>>>> On Wed 2015-02-18 17:20:22, Jacek Anaszewski wrote:
>>>>>> Add a documentation of LED Flash class specific sysfs attributes.
>>>>>>
>>>>>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>>>>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>>> Cc: Bryan Wu <cooloney@gmail.com>
>>>>>> Cc: Richard Purdie <rpurdie@rpsys.net>
>>>>>
>>>>> NAK-ed-by: Pavel Machek
>>>>>
>>>>>> +What:		/sys/class/leds/<led>/available_sync_leds
>>>>>> +Date:		February 2015
>>>>>> +KernelVersion:	3.20
>>>>>> +Contact:	Jacek Anaszewski <j.anaszewski@samsung.com>
>>>>>> +Description:	read/write
>>>>>> +		Space separated list of LEDs available for flash strobe
>>>>>> +		synchronization, displayed in the format:
>>>>>> +
>>>>>> +		led1_id.led1_name led2_id.led2_name led3_id.led3_name etc.
>>>>>
>>>>> Multiple values per file, with all the problems we had in /proc. I
>>>>> assume led_id is an integer? What prevents space or dot in led name?
>>>>
>>>> Very good point. How about using a newline instead? That'd be a little bit
>>>> easier to parse, too.
>>>
>>> No, please make it one value per-file, which is what sysfs requires.
>>
>> The purpose of this attribute is only to provide an information about
>> the range of valid identifiers that can be written to the
>> flash_sync_strobe attribute. Wouldn't splitting this to many attributes
>> be an unnecessary inflation of sysfs files?
>
> Ok a list of allowed values to write is acceptable, as long as it is not
> hard to parse and always is space separated.

Is a new line character also acceptable as a delimiter?

>> Apart from it, we have also flash_faults attribute, that currently
>> provides a space separated list of flash faults that have occurred.
>
> That's crazy, what's to keep it from growing and growing to be larger
> than is allowed to be read?

The number of possible faults is fixed to 9 currently. They are 
presented in the form of strings no longer currently than 40 characters.
There can be maximum 9 faults reported at a time, this is not a kind of
a log. This will allow to define roughly 100 types of faults, having
that PAGE_SIZE is 4096. I think this is far more than it is conceivable
for the simple LED flash device.

>> If we are to stick tightly to the one-value-per-file rule, then how
>> we should approach flash_faults case? Should the separate file be
>> dynamically created for each reported fault?
>
> I think you need to use something other than sysfs here, sorry.
>
> uevents for your faults?
>
> thanks,
>
> greg k-h
>

-- 
Best Regards,
Jacek Anaszewski