From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933140AbcHYJEq (ORCPT ); Thu, 25 Aug 2016 05:04:46 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:32197 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932283AbcHYJEn (ORCPT ); Thu, 25 Aug 2016 05:04:43 -0400 MIME-version: 1.0 Content-type: text/plain; charset=utf-8; format=flowed X-AuditID: cbfec7f4-f79cb6d000001359-51-57beb4a76738 Subject: Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger To: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= References: <20160823220404.9887-1-zajec5@gmail.com> <20160824175345.7033-1-zajec5@gmail.com> Cc: Richard Purdie , Felipe Balbi , Greg KH , Peter Chen , "linux-usb@vger.kernel.org" , =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , Jonathan Corbet , Ezequiel Garcia , Stephan Linz , Matthias Brugger , Boris Brezillon , Geert Uytterhoeven , "open list:DOCUMENTATION" , open list , "open list:LED SUBSYSTEM" , Pavel Machek From: Jacek Anaszewski Message-id: Date: Thu, 25 Aug 2016 11:04:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 In-reply-to: Content-transfer-encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrMIsWRmVeSWpSXmKPExsVy+t/xa7rLt+wLN1h4QdPiWNsTdosDLxay WDw50M5osfHFZxaLZ7f2Mlk0L17PZtF1dRObxcK2JSwWl3fNYbPY+mYdo8WiZa3MFldfL2Kx WDD5CavF3VNH2Sx2r13EZLF711NWizUnUx0EPZ5susjosXPWXXaPTas62Tx2v031OHS4g9Fj /9w17B6L+yazepyf6umxZ/4PVo/1W66yeKxY/Z3do+vIdTaPz5vkAnijuGxSUnMyy1KL9O0S uDJurzvJXDBRseLVuhssDYx9Ul2MnBwSAiYSE3+0s0LYYhIX7q1nA7GFBJYySmzYUAJi8woI SvyYfI8FxGYWMJP48vIwK0TNM6CarjwQW1jAS+LkvUVMILaIgLnEk75HQDYXUM1tRokp23ex gDjMAsdZJb4un8kOUsUmYCjx88VrJogNdhKvmhaCTWURUJVYdv0rWFxUIELi1qqPjCA2p0Cw xO37N5ggrpCXOHjlOcsERoFZSA6cheTAWUjKFjAyr2IUTS1NLihOSs811CtOzC0uzUvXS87P 3cQIicovOxgXH7M6xCjAwajEw7tj1d5wIdbEsuLK3EOMEhzMSiK839bvCxfiTUmsrEotyo8v Ks1JLT7EKM3BoiTOO3fX+xAhgfTEktTs1NSC1CKYLBMHp1QD46rGhYnuX7xF1E6fu37tqF6H 2c7VJnHJWvnxf6MXm7RlifsFOJU2Gcxf4/TN9lOhTM7G4N5968OZbkgzpU1a47NZe9fty9NO qK9MSBFX/XsqhNXuaMzT5oqiw0t+nPs2s39xvbhSXcDGVU/apk/pXvmf9+mT3hVvthy8wBSx c3pAfuEiDreFS5VYijMSDbWYi4oTAWwILmTGAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/25/2016 10:29 AM, Rafał Miłecki wrote: > On 25 August 2016 at 10:03, Jacek Anaszewski wrote: >> On 08/24/2016 07:52 PM, Rafał Miłecki wrote: >>> >>> From: Rafał Miłecki >>> >>> 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 >>> --- >>> 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