From patchwork Wed Aug 17 11:06:40 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Bryan Wu X-Patchwork-Id: 1073612 Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p7HB7N3r003189 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Wed, 17 Aug 2011 11:07:44 GMT Received: from canuck.infradead.org ([2001:4978:20e::1]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1Qtdxk-0008HS-Ob; Wed, 17 Aug 2011 11:07:09 +0000 Received: from localhost ([127.0.0.1] helo=canuck.infradead.org) by canuck.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1Qtdxk-0002Pp-9D; Wed, 17 Aug 2011 11:07:08 +0000 Received: from mail-iy0-f171.google.com ([209.85.210.171]) by canuck.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1Qtdxg-0002PH-6X for linux-arm-kernel@lists.infradead.org; Wed, 17 Aug 2011 11:07:05 +0000 Received: by iyf13 with SMTP id 13so1774670iyf.16 for ; Wed, 17 Aug 2011 04:07:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:from:date :x-google-sender-auth:message-id:subject:to:cc:content-type :content-transfer-encoding; bh=Nm0cTEDTxtuEC5fgh7N8XbBNf0GzalT3Jl1XfebIk7A=; b=ubVKtiREdANFOHnQwcaflrjio25o4VAUW3cp+FfhjDBatgwRTG/C5inEw+V5ppFooF hwSnSfpUE6/jTxpdEdf3rcedT4wA8+ysThS9Bd/kmDu095AqmUNFzSb+0eNvTgMnM8Jc JPK+M6yZ271N/tqHaOsL41ViRXZMASUY0hdk4= Received: by 10.231.0.96 with SMTP id 32mr1936099iba.36.1313579220368; Wed, 17 Aug 2011 04:07:00 -0700 (PDT) MIME-Version: 1.0 Received: by 10.231.206.145 with HTTP; Wed, 17 Aug 2011 04:06:40 -0700 (PDT) In-Reply-To: <20110803115918.GA23953@n2100.arm.linux.org.uk> References: <1312364089-32380-1-git-send-email-bryan.wu@canonical.com> <1312364089-32380-4-git-send-email-bryan.wu@canonical.com> <20110803115918.GA23953@n2100.arm.linux.org.uk> From: Bryan Wu Date: Wed, 17 Aug 2011 19:06:40 +0800 X-Google-Sender-Auth: _HtEzvRV2gabD0V9jEw4ve57qKE Message-ID: Subject: Re: [PATCH 03/17] mach-realview and mach-versatile: retire custom LED code To: rpurdie@rpsys.net, Russell King - ARM Linux X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20110817_070704_652799_2D118FEC X-CRM114-Status: GOOD ( 23.60 ) X-Spam-Score: -0.7 (/) X-Spam-Report: SpamAssassin version 3.3.1 on canuck.infradead.org summary: Content analysis details: (-0.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.210.171 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (cooloney[at]gmail.com) 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: nicolas.pitre@linaro.org, tony@atomide.com, linus.walleij@linaro.org, jochen@scram.de, linux@maxim.org.za, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Wed, 17 Aug 2011 11:07:44 +0000 (UTC) X-MIME-Autoconverted: from quoted-printable to 8bit by demeter2.kernel.org id p7HB7N3r003189 On Wed, Aug 3, 2011 at 7:59 PM, Russell King - ARM Linux wrote: > On Wed, Aug 03, 2011 at 05:34:35PM +0800, Bryan Wu wrote: >> From: Linus Walleij >> >> This replaces the custom LED trigger code in mach-realview with >> some overarching platform code for the plat-versatile family that >> will lock down LEDs 2 thru 5 for CPU activity indication. The >> day we have 8 core ARM systems the plat-versatile code will have >> to become more elaborate. >> >> Tested on RealView PB11MPCore by invoking four different CPU >> hogs (yes > /dev/null&) and see the LEDs go on one at a time. >> They all go off as the hogs are killed. Tested on the PB1176 >> as well - just one activity led (led 2) goes on and off with >> CPU activity. >> >> (bryan.wu@canonical.com: use ledtrig-cpu instead of ledtrig-arm-cpu) > > This is broken.  More than that, this LEDS stuff is broken. > > With CONFIG_NEW_LEDS=y and CONFIG_LEDS_CLASS=n, > Actually, I don't see any user of this config, either. And all the LEDS stuffs depend on LEDS_CLASS. Can we just simply select LEDS_CLASS when CONFIG_NEW_LEDS=y? --- --- Richard, could you please give some comments? -Bryan >  CC      arch/arm/plat-versatile/leds.o > arch/arm/plat-versatile/leds.c: In function 'versatile_leds_init': > arch/arm/plat-versatile/leds.c:91: error: 'struct led_classdev' has no member named 'trigger_data' > > I wasn't offered LEDS_TRIGGERS to select.  It looks like this leds > stuff really is broken: > > config LEDS_CLASS >        bool "LED Class Support" >        help >          This option enables the led sysfs class in /sys/class/leds.  You'll >          need this to do anything useful with LEDs.  If unsure, say N. > > Okay, this says its for sysfs support.  But then: > > config LEDS_TRIGGERS >        bool "LED Trigger support" >        depends on LEDS_CLASS >        help >          This option enables trigger support for the leds class. >          These triggers allow kernel events to drive the LEDs and can >          be configured via sysfs. If unsure, say Y. > > you can only have LED triggers if you have LED class support.  So what's > the point of NEW_LEDS=y and LEDS_CLASS=n?  From those descriptions, > LEDS_CLASS should control the sysfs interfaces, and LEDS_TRIGGERS should > control the kernel internal interfaces _independently_. > > As things stand, this looks completely crazy.  So no, I'm not acking these > patches until the LEDs layer gets a modicum of sanity. > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index b444cc3..5ae3ef7 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -7,6 +7,7 @@ config LEDS_GPIO_REGISTER menuconfig NEW_LEDS bool "LED Support" + select LEDS_CLASS help Say Y to enable Linux LED support. This allows control of supported LEDs from both userspace and optionally, by kernel events (triggers).