Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger

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

On 08/26/2016 05:58 PM, Rafał Miłecki wrote:
> On 25 August 2016 at 20:48, Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
>> On 08/25/2016 04:30 PM, Alan Stern wrote:
>>>
>>> On Thu, 25 Aug 2016, Jacek Anaszewski wrote:
>>>
>>>> I'd see it as follows:
>>>>
>>>> #cat available_ports
>>>> #1-1 1-2 2-1
>>>>
>>>> #echo "1-1" > new_port
>>>>
>>>> #cat observed_ports
>>>> #1-1
>>>>
>>>> #echo "2-1" > new_port
>>>>
>>>> #cat observed_ports
>>>> #1-1 2-1
>>>>
>>>> We've already had few discussions about the sysfs designs trying
>>>> to break the one-value-per-file rule for LED class device, and
>>>> there was always strong resistance against.
>>>
>>>
>>> This scheme has multiple values in both the available_ports and
>>> observed_ports files.  :-(  Not that I have any better suggestions...
>>
>>
>> Right, I forgot to add a note here, that this follows space
>> separated list pattern similarly as in case of triggers attribute.
>> Of course other suggestions are welcome.
>
> So ppl have doubts about multiple values in a single sysfs file
> (whatever we call it: "ports" or "observed_ports"). Greg clearly said:
>> sysfs is "one value per file", here you are listing a bunch of things in
>> one sysfs file.  Please don't do that.
>
> What about my idea of using "ports" subdirectory and having each port
> as separated file inside that subdir? I think there are two ways of
> doing this:
>
> 1) Having "ports" subdir with 0x0000 chmod files, one per each port
> specified as observable
> In this solution we need "new_port" and "remove_port" that can be used
> for management of observable ports.
> I think Jacek wasn't happy with this chmod and he believes Greg meant R/W files.

It looks odd to me. In this case it would also abuse "one value per
file" rule - the files would have no value, and only their names would
carry an information.

> 2) Having "ports" subdir with RW files, one per each existing physical port
> In this situation we don't need "new_port" or "remove_port". If we
> want port to be observable we just do:
> echo 1 > 1-1
> Implementing this solution needs reading more details from USB subsystem.

The situation here is clear IMO - the number of USB ports in the system
can change dynamically. I'm not sure if this can be handled easily with
sysfs, where we usually expose an interface for known set of settings.
struct attribute arrays are usually defined statically at the compile
time and filled with the variables, that are created with DEVICE_ATTR
macro.

> Do you find any of solutions with "ports" subdir better than dealing
> with new-line/space separated values in a single sysfs file?
>


-- 
Best regards,
Jacek Anaszewski