Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1468943 > unrolled thread

[PATCH V3] leds: trigger: Introduce an USB port trigger

Back to article view | Back to linux.kernel

Contents

  [PATCH V3] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-24 00:10 +0200
    Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Greg KH <gregkh@linuxfoundation.org> - 2016-08-24 11:30 +0200
      Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-24 11:40 +0200
        Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Greg KH <gregkh@linuxfoundation.org> - 2016-08-24 23:10 +0200
          Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-25 07:20 +0200
            Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Greg KH <gregkh@linuxfoundation.org> - 2016-08-25 15:00 +0200
              Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Bjørn Mork <bjorn@mork.no> - 2016-08-25 21:00 +0200
                Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Greg KH <gregkh@linuxfoundation.org> - 2016-08-25 23:10 +0200
    Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Greg KH <gregkh@linuxfoundation.org> - 2016-08-24 11:30 +0200
      Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-24 11:30 +0200
        Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Greg KH <gregkh@linuxfoundation.org> - 2016-08-24 23:20 +0200
    Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-24 13:10 +0200
      Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Matthias Brugger <mbrugger@suse.com> - 2016-08-24 14:00 +0200
    Re: [PATCH V3] leds: trigger: Introduce an USB port trigger Matthias Brugger <mbrugger@suse.com> - 2016-08-24 13:20 +0200
    [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-24 20:00 +0200
      Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Bjørn Mork <bjorn@mork.no> - 2016-08-24 21:20 +0200
        Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-24 22:50 +0200
      Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-08-25 10:20 +0200
        Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Matthias Brugger <mbrugger@suse.com> - 2016-08-25 10:30 +0200
        Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-25 10:40 +0200
          Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-08-25 11:10 +0200
            Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Alan Stern <stern@rowland.harvard.edu> - 2016-08-25 16:50 +0200
              Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-08-25 20:50 +0200
                Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Alan Stern <stern@rowland.harvard.edu> - 2016-08-25 21:40 +0200
                  Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Alan Stern <stern@rowland.harvard.edu> - 2016-08-25 22:10 +0200
                Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-26 18:00 +0200
                  Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-08-29 10:00 +0200
                    Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Pavel Machek <pavel@ucw.cz> - 2016-08-29 10:10 +0200
                      Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-29 10:30 +0200
                        Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Pavel Machek <pavel@ucw.cz> - 2016-08-29 10:50 +0200
                          Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-29 11:10 +0200
                Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Pavel Machek <pavel@ucw.cz> - 2016-08-26 22:00 +0200
                  Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-08-29 10:40 +0200
            Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Pavel Machek <pavel@ucw.cz> - 2016-08-26 21:50 +0200
        Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger Rafał Miłecki <zajec5@gmail.com> - 2016-08-25 10:50 +0200

Page 2 of 2 — ← Prev page 1 [2]

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

On 08/25/2016 10:29 AM, Rafał Miłecki wrote:
> On 25 August 2016 at 10:03, Jacek Anaszewski <j.anaszewski@samsung.com> wrote:
>> On 08/24/2016 07:52 PM, Rafał Miłecki wrote:
>>>
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> This commit adds a new trigger responsible for turning on LED when USB
>>> device gets connected to the specified USB port. This can can useful for
>>> various home routers that have USB port(s) and a proper LED telling user
>>> a device is connected.
>>>
>>> The trigger gets its documentation file but basically it just requires
>>> specifying USB port in a Linux format (e.g. echo 1-1 > new_port).
>>>
>>> During work on this trigger there was a plan to add DT bindings for it,
>>> but there wasn't an agreement on the format yet. This can be worked on
>>> later, a sysfs interface is needed anyway for platforms not using DT.
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>> V2: Trying to add DT support, idea postponed as it will take more time
>>>     to discuss the bindings.
>>> V3: Fix typos in commit and Documentation (thanks Jacek!)
>>>     Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
>>>     Check if there is USB device connected after adding new USB port
>>>     Fix memory leak or two
>>> V3.5: Fix e-mail address (thanks Matthias)
>>>       Simplify conditions in usbport_trig_notify (thx Matthias)
>>>       Make "ports" a subdirectory with file per port, to match one value
>>>       per file sysfs rule (thanks Greg)
>>>       As "ports" couldn't be used for adding and removing ports anymore,
>>>       there are now "new_port" and "remove_port". Having them makes this
>>>       API also common with e.g. pci and usb buses.
>>
>>
>> Now writing new_port with "1-1" produces a file named "1-1" in the
>> ports directory with 000 permissions. I think that what Greg had
>> on mind by referring to "one value per file" rule was a set of
>> files representing ports, like "1-1 1-2 2-1", and each file should be
>> readable/writeable.
>>
>> For instance "echo 1 > 1-1" would enable the trigger for the port 1-1
>> and "echo 0 > 1-1" would disable it. The problem is that we don't know
>> the number of required ports at compilation time and the sysfs
>> attributes would have to be dynamically created on driver instantiation.
>> What is more, as the USB ports can dynamically appear/disappear in the
>> system, the files would have to be created/removed accordingly during
>> LED class device lifetime, which is not the best design for the sysfs
>> interface I think.
>>
>> Therefore, maybe it would be good to follow the "triggers" sysfs
>> attribute pattern, where it lists the available LED triggers?
>>
>> The question is whether there is some mechanism available for
>> notifying addition/removal of a USB port?
>
> Every port is part of some hub and every hub (even root one) is a USB
> device. So I could just get struct usb_device and check its "maxchild"
> property. If e.g. I get USB device "1-1" with maxchild 4, I know there
> are:
> 1-1.1
> 1-1.2
> 1-1.3
> 1-1.4
>
> So the sysfs structure you suggested would be possible, I just don't
> know if it's preferred one or not.

It would require an ack from Greg.

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.

Cc Pavel.

> Confirmation: yes, right now I create simple files with chmod 000 for
> every port added to the usbport observable list.
>
>
>> Also a description of the device connected to the port would be a nice
>> feature, however I am not certain about the feasibility thereof.
>
> What kind of description do you mean? Where should it be used / where
> should it appear?
>

Product name/symbol. Actually it should be USB subsystem responsibility
to provide the means for querying the product name by port id, if it
is possible at all.

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]

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

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...

> >> Also a description of the device connected to the port would be a nice
> >> feature, however I am not certain about the feasibility thereof.
> >
> > What kind of description do you mean? Where should it be used / where
> > should it appear?
> >
> 
> Product name/symbol. Actually it should be USB subsystem responsibility
> to provide the means for querying the product name by port id, if it
> is possible at all.

	cat /sys/bus/usb/devices/PORT/product
	cat /sys/bus/usb/devices/PORT/manufacturer

These will work if there is a device registered under PORT.

Alan Stern

[toc] | [prev] | [next] | [standalone]

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

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.

>>>> Also a description of the device connected to the port would be a nice
>>>> feature, however I am not certain about the feasibility thereof.
>>>
>>> What kind of description do you mean? Where should it be used / where
>>> should it appear?
>>>
>>
>> Product name/symbol. Actually it should be USB subsystem responsibility
>> to provide the means for querying the product name by port id, if it
>> is possible at all.
>
> 	cat /sys/bus/usb/devices/PORT/product
> 	cat /sys/bus/usb/devices/PORT/manufacturer
>
> These will work if there is a device registered under PORT.

I've found only idProduct and idVendor files. They indeed uniquely
identify the device, but the numbers are not human readable.
Is there a way to retrieve the corresponding names in kernel?
Does the lsusb command do the mapping in the user space or maybe
it takes the names from kernel?

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]

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

On Thu, 25 Aug 2016, Jacek Anaszewski wrote:

> >>> What kind of description do you mean? Where should it be used / where
> >>> should it appear?
> >>>
> >>
> >> Product name/symbol. Actually it should be USB subsystem responsibility
> >> to provide the means for querying the product name by port id, if it
> >> is possible at all.
> >
> > 	cat /sys/bus/usb/devices/PORT/product
> > 	cat /sys/bus/usb/devices/PORT/manufacturer
> >
> > These will work if there is a device registered under PORT.
> 
> I've found only idProduct and idVendor files. They indeed uniquely
> identify the device, but the numbers are not human readable.
> Is there a way to retrieve the corresponding names in kernel?

If the device provides the string descriptors then the textual product
and manufacturer files are available in sysfs, otherwise they aren't.

> Does the lsusb command do the mapping in the user space or maybe
> it takes the names from kernel?

lsusb does the mapping in userspace, based on an ID database.  On my 
system (Fedora), the database is /etc/udev/hwdb.bin.

Alan Stern

[toc] | [prev] | [next] | [standalone]

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

On Thu, 25 Aug 2016, Alan Stern wrote:

> > Does the lsusb command do the mapping in the user space or maybe
> > it takes the names from kernel?
> 
> lsusb does the mapping in userspace, based on an ID database.  On my 
> system (Fedora), the database is /etc/udev/hwdb.bin.

There's also /usr/share/hwdata/usb.ids -- I don't know if or how the
two databases are related.  And there's /usr/lib/udev/hwdb.d/20-usb*.

Alan Stern

[toc] | [prev] | [next] | [standalone]

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

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.

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.

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

-- 
Rafał

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

Hi!

> >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.

sysfs already exposes current view of all usb devices. Just use it.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

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

On 29 August 2016 at 10:05, Pavel Machek <pavel@ucw.cz> wrote:
>> >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.
>
> sysfs already exposes current view of all usb devices. Just use it.

We're talking about USB ports not devices, but this is still true. You
can find them in
/sys/bus/usb/devices/*/*-port*

I can't see how we could use them. How could I develop sysfs interface
in /sys/class/leds/*/ to allow userspace assigning USB ports to the
LED trigger?

-- 
Rafał

[toc] | [prev] | [next] | [standalone]

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

On Mon 2016-08-29 10:21:48, Rafał Miłecki wrote:
> On 29 August 2016 at 10:05, Pavel Machek <pavel@ucw.cz> wrote:
> >> >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.
> >
> > sysfs already exposes current view of all usb devices. Just use it.
> 
> We're talking about USB ports not devices, but this is still true. You
> can find them in
> /sys/bus/usb/devices/*/*-port*
> 
> I can't see how we could use them. How could I develop sysfs interface
> in /sys/class/leds/*/ to allow userspace assigning USB ports to the
> LED trigger?

Create /sys/bus/usb/devices/*/*-port*/led_trigger file?

(Do you plan one USB trigger, or multiple ones?)
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

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

On 29 August 2016 at 10:41, Pavel Machek <pavel@ucw.cz> wrote:
> On Mon 2016-08-29 10:21:48, Rafał Miłecki wrote:
>> On 29 August 2016 at 10:05, Pavel Machek <pavel@ucw.cz> wrote:
>> >> >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.
>> >
>> > sysfs already exposes current view of all usb devices. Just use it.
>>
>> We're talking about USB ports not devices, but this is still true. You
>> can find them in
>> /sys/bus/usb/devices/*/*-port*
>>
>> I can't see how we could use them. How could I develop sysfs interface
>> in /sys/class/leds/*/ to allow userspace assigning USB ports to the
>> LED trigger?
>
> Create /sys/bus/usb/devices/*/*-port*/led_trigger file?
>
> (Do you plan one USB trigger, or multiple ones?)

I guess it means slightly more complex cross-subsystem interaction I
may need help to understand.

First of all: I will need multiple USB port triggers. It's because
there are many devices with multiple USB LEDs. Some complex (but
existing!) case:
USB 2.0 white LED - activated by USB 2.0 dev in USB 2.0 port
USB 3.0 white LED - activated by USB 2.0 dev in USB 3.0 port
USB 3.0 green LED - activated by USB 3.0 dev in USB 3.0 port
(This devices has separated EHCI and XHCI controllers, so we have 2
logical ports for 1 physical one)

So I guess I'll need something more complex like
/sys/bus/usb/devices/*/*-port*/bcm53xx:white:usb2_led_trigger
/sys/bus/usb/devices/*/*-port*/bcm53xx:white:usb3_led_trigger
/sys/bus/usb/devices/*/*-port*/bcm53xx:green:usb3_led_trigger
in case every USB led has "usbport" trigger assigned.

How can I create above sysfs files? Should I write code for this in
ledtrig-usbport.c? That would require accessing struct usb_hub and
then struct usb_port, but both of them are internal structs defined in
drivers/usb/core/hub.h. How could I work with that?

-- 
Rafał

[toc] | [prev] | [next] | [standalone]

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

On Thu 2016-08-25 20:48:04, Jacek Anaszewski 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.
> 
> >>>>Also a description of the device connected to the port would be a nice
> >>>>feature, however I am not certain about the feasibility thereof.
> >>>
> >>>What kind of description do you mean? Where should it be used / where
> >>>should it appear?
> >>>
> >>
> >>Product name/symbol. Actually it should be USB subsystem responsibility
> >>to provide the means for querying the product name by port id, if it
> >>is possible at all.
> >
> >	cat /sys/bus/usb/devices/PORT/product
> >	cat /sys/bus/usb/devices/PORT/manufacturer
> >
> >These will work if there is a device registered under PORT.
> 
> I've found only idProduct and idVendor files. They indeed uniquely
> identify the device, but the numbers are not human readable.

Actually, they don't. They identify device _type_. If you have two
mice of the same type connected, they'll have same idProduct /
idVendor values.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

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

On 08/26/2016 09:50 PM, Pavel Machek wrote:
> On Thu 2016-08-25 20:48:04, Jacek Anaszewski 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.
>>
>>>>>> Also a description of the device connected to the port would be a nice
>>>>>> feature, however I am not certain about the feasibility thereof.
>>>>>
>>>>> What kind of description do you mean? Where should it be used / where
>>>>> should it appear?
>>>>>
>>>>
>>>> Product name/symbol. Actually it should be USB subsystem responsibility
>>>> to provide the means for querying the product name by port id, if it
>>>> is possible at all.
>>>
>>> 	cat /sys/bus/usb/devices/PORT/product
>>> 	cat /sys/bus/usb/devices/PORT/manufacturer
>>>
>>> These will work if there is a device registered under PORT.
>>
>> I've found only idProduct and idVendor files. They indeed uniquely
>> identify the device, but the numbers are not human readable.
>
> Actually, they don't. They identify device _type_. If you have two
> mice of the same type connected, they'll have same idProduct /
> idVendor values.

That's true. We'd have to be able to distinguish between the devices
of the same type. The only way seems to be the port id, whereas
the initial intention was to give a hint on what device is represented
by given port id :-)

Regardless of that, having a device name instead of port id would allow
for unique identification of a device in most of cases.

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]

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

On Thu 2016-08-25 11:04:38, Jacek Anaszewski wrote:
> On 08/25/2016 10:29 AM, Rafał Miłecki wrote:
> >On 25 August 2016 at 10:03, Jacek Anaszewski <j.anaszewski@samsung.com> wrote:
> >>On 08/24/2016 07:52 PM, Rafał Miłecki wrote:
> >>>
> >>>From: Rafał Miłecki <rafal@milecki.pl>
> >>>
> >>>This commit adds a new trigger responsible for turning on LED when USB
> >>>device gets connected to the specified USB port. This can can useful for
> >>>various home routers that have USB port(s) and a proper LED telling user
> >>>a device is connected.
> >>>
> >>>The trigger gets its documentation file but basically it just requires
> >>>specifying USB port in a Linux format (e.g. echo 1-1 > new_port).
> >>>
> >>>During work on this trigger there was a plan to add DT bindings for it,
> >>>but there wasn't an agreement on the format yet. This can be worked on
> >>>later, a sysfs interface is needed anyway for platforms not using DT.
> >>>
> >>>Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> >>>---
> >>>V2: Trying to add DT support, idea postponed as it will take more time
> >>>    to discuss the bindings.
> >>>V3: Fix typos in commit and Documentation (thanks Jacek!)
> >>>    Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
> >>>    Check if there is USB device connected after adding new USB port
> >>>    Fix memory leak or two
> >>>V3.5: Fix e-mail address (thanks Matthias)
> >>>      Simplify conditions in usbport_trig_notify (thx Matthias)
> >>>      Make "ports" a subdirectory with file per port, to match one value
> >>>      per file sysfs rule (thanks Greg)
> >>>      As "ports" couldn't be used for adding and removing ports anymore,
> >>>      there are now "new_port" and "remove_port". Having them makes this
> >>>      API also common with e.g. pci and usb buses.
> >>
> >>
> >>Now writing new_port with "1-1" produces a file named "1-1" in the
> >>ports directory with 000 permissions. I think that what Greg had
> >>on mind by referring to "one value per file" rule was a set of
> >>files representing ports, like "1-1 1-2 2-1", and each file should be
> >>readable/writeable.
> >>
> >>For instance "echo 1 > 1-1" would enable the trigger for the port 1-1
> >>and "echo 0 > 1-1" would disable it. The problem is that we don't know
> >>the number of required ports at compilation time and the sysfs
> >>attributes would have to be dynamically created on driver instantiation.
> >>What is more, as the USB ports can dynamically appear/disappear in the
> >>system, the files would have to be created/removed accordingly during
> >>LED class device lifetime, which is not the best design for the sysfs
> >>interface I think.

Could we add a flag to the USB port, instead? That way, USB code would
take care of creating the enable file in its own directory.

Is per-port control needed? Would just global control be sufficient?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

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

On 25 August 2016 at 10:03, Jacek Anaszewski <j.anaszewski@samsung.com> wrote:
> On 08/24/2016 07:52 PM, Rafał Miłecki wrote:
>>
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> This commit adds a new trigger responsible for turning on LED when USB
>> device gets connected to the specified USB port. This can can useful for
>> various home routers that have USB port(s) and a proper LED telling user
>> a device is connected.
>>
>> The trigger gets its documentation file but basically it just requires
>> specifying USB port in a Linux format (e.g. echo 1-1 > new_port).
>>
>> During work on this trigger there was a plan to add DT bindings for it,
>> but there wasn't an agreement on the format yet. This can be worked on
>> later, a sysfs interface is needed anyway for platforms not using DT.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> V2: Trying to add DT support, idea postponed as it will take more time
>>     to discuss the bindings.
>> V3: Fix typos in commit and Documentation (thanks Jacek!)
>>     Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
>>     Check if there is USB device connected after adding new USB port
>>     Fix memory leak or two
>> V3.5: Fix e-mail address (thanks Matthias)
>>       Simplify conditions in usbport_trig_notify (thx Matthias)
>>       Make "ports" a subdirectory with file per port, to match one value
>>       per file sysfs rule (thanks Greg)
>>       As "ports" couldn't be used for adding and removing ports anymore,
>>       there are now "new_port" and "remove_port". Having them makes this
>>       API also common with e.g. pci and usb buses.
>
>
> Now writing new_port with "1-1" produces a file named "1-1" in the
> ports directory with 000 permissions. I think that what Greg had
> on mind by referring to "one value per file" rule was a set of
> files representing ports, like "1-1 1-2 2-1", and each file should be
> readable/writeable.

On the other hand, when I described my idea with "new_port" and
"remove_port", Greg replied with "Maybe", so I'm not sure if he really
got a design you described in his mind.

Greg: could you comment on this sysfs interface, please? :)

-- 
Rafał

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web