LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Grant Likely" <grant.likely@secretlab.ca>
To: "Trent Piepho" <tpiepho@freescale.com>
Cc: linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	"Anton Vorontsov" <avorontsov@ru.mvista.com>,
	"Richard Purdie" <rpurdie@rpsys.net>,
	"Sean MacLennan" <smaclennan@pikatech.com>,
	"Wolfram Sang" <w.sang@pengutronix.de>
Subject: Re: [PATCH 2/4] leds: Support OpenFirmware led bindings
Date: Fri, 24 Oct 2008 17:50:18 -0600	[thread overview]
Message-ID: <fa686aa40810241650q4f195249t44a9276b20190a5d@mail.gmail.com> (raw)
In-Reply-To: <1224889741-4167-2-git-send-email-tpiepho@freescale.com>

On Fri, Oct 24, 2008 at 5:08 PM, Trent Piepho <tpiepho@freescale.com> wrote:
> Add bindings to support LEDs defined as of_platform devices in addition to
> the existing bindings for platform devices.
>
> New options in Kconfig allow the platform binding code and/or the
> of_platform code to be turned on.  The of_platform code is of course only
> available on archs that have OF support.
>
> The existing probe and remove methods are refactored to use new functions
> create_gpio_led(), to create and register one led, and delete_gpio_led(),
> to unregister and free one led.  The new probe and remove methods for the
> of_platform driver can then share most of the common probe and remove code
> with the platform driver.
>
> The suspend and resume methods aren't shared, but they are very short.  The
> actual led driving code is the same for LEDs created by either binding.
>
> The OF bindings are based on patch by Anton Vorontsov
> <avorontsov@ru.mvista.com>.  They have been extended to allow multiple LEDs
> per device.
>
> Signed-off-by: Trent Piepho <tpiepho@freescale.com>

This also looks sane.  However, since this modifies a binding, can you
please repost to the devicetree-discuss@ozlabs.org mailing list?
Also, one comment below.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt b/Documentation/powerpc/dts-bindings/gpio/led.txt
> index ff51f4c..9f969c2 100644
> --- a/Documentation/powerpc/dts-bindings/gpio/led.txt
> +++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
> @@ -1,15 +1,43 @@
> -LED connected to GPIO
> +LEDs connected to GPIO lines
>
>  Required properties:
> -- compatible : should be "gpio-led".
> -- label : (optional) the label for this LED. If omitted, the label is
> +- compatible : should be "gpio-leds".
> +
> +Each LED is represented as a sub-node of the gpio-leds device.  Each
> +node's name represents the name of the corresponding LED.
> +
> +LED sub-node properties:
> +- gpios :  Should specify the LED's GPIO, see "Specifying GPIO information
> +  for devices" in Documentation/powerpc/booting-without-of.txt.  Active
> +  low LEDs should be indicated using flags in the GPIO specifier.
> +- label :  (optional) The label for this LED.  If omitted, the label is
>   taken from the node name (excluding the unit address).
> -- gpios : should specify LED GPIO.
> +- linux,default-trigger :  (optional) This parameter, if present, is a
> +  string defining the trigger assigned to the LED.  Current triggers are:
> +    "backlight" - LED will act as a back-light, controlled by the framebuffer
> +                 system
> +    "default-on" - LED will turn on
> +    "heartbeat" - LED "double" flashes at a load average based rate
> +    "ide-disk" - LED indicates disk activity
> +    "timer" - LED flashes at a fixed, configurable rate

As I'm sure you can predict, I'm not thrilled with this; but it *is* a
very linux-specific sort of thing, it *is* a necessary bit of data.
:-)  My biggest worry is that the Linux internal strings will change
in the future which will force the gpio driver to insert a translation
between these names and some future internal Linux name...

... I suppose another option is to just codify them here right now
(maybe named something like "led-usage") and if Linux changes then the
driver just needs to keep up with the new trigger names (translating
the old).

Either way, I'm not going to oppose this patch over this issue.

Nice patch.
g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

  reply	other threads:[~2008-10-24 23:50 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-24 23:04 OpenFirmware GPIO LED driver Trent Piepho
2008-10-24 23:08 ` [PATCH 1/4] of_gpio: Return GPIO flags from of_get_gpio() Trent Piepho
2008-10-24 23:32   ` Grant Likely
2008-10-28 14:39   ` Anton Vorontsov
2008-10-28 14:53     ` Grant Likely
2008-10-28 15:16       ` Anton Vorontsov
2008-10-28 15:42         ` Grant Likely
2008-10-28 16:56           ` Anton Vorontsov
2008-10-28 17:40             ` Grant Likely
2008-10-30  2:21     ` [PATCH 1/4] of_gpio: Return GPIO flags from of_get_gpio()A Trent Piepho
2008-10-30 11:15       ` Anton Vorontsov
2008-10-31  2:03         ` [PATCH v2] of_gpio: Return GPIO flags from of_get_gpio() Trent Piepho
2008-11-26 16:20           ` Anton Vorontsov
2008-11-26 21:38             ` Paul Mackerras
2008-11-26 22:31               ` Anton Vorontsov
2008-11-26 22:35               ` Trent Piepho
2008-11-26 22:58                 ` Anton Vorontsov
2008-11-26 23:32                 ` Paul Mackerras
2008-10-24 23:08 ` [PATCH 2/4] leds: Support OpenFirmware led bindings Trent Piepho
2008-10-24 23:50   ` Grant Likely [this message]
2008-10-24 23:09 ` [PATCH 3/4] leds: Add option to have GPIO LEDs start on Trent Piepho
2008-10-24 23:59   ` Grant Likely
2008-10-24 23:09 ` [PATCH 4/4] leds: Let GPIO LEDs keep their current state Trent Piepho
2008-10-25  0:04   ` Grant Likely
2008-11-17 14:50   ` Richard Purdie
2008-11-21  1:05     ` Trent Piepho
2008-11-23 12:31       ` Pavel Machek
2008-12-03 10:04         ` Richard Purdie
2008-12-10  4:33           ` Trent Piepho

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fa686aa40810241650q4f195249t44a9276b20190a5d@mail.gmail.com \
    --to=grant.likely@secretlab.ca \
    --cc=avorontsov@ru.mvista.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=rpurdie@rpsys.net \
    --cc=smaclennan@pikatech.com \
    --cc=tpiepho@freescale.com \
    --cc=w.sang@pengutronix.de \
    --subject='Re: [PATCH 2/4] leds: Support OpenFirmware led bindings' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).