linux-kernel - Re: [PATCH 2/2] leds: Support OpenFirmware led bindings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 28 Jul 2008 22:02:04 +0400
From:	Anton Vorontsov <avorontsov@...mvista.com>
To:	Grant Likely <grant.likely@...retlab.ca>
Cc:	Trent Piepho <tpiepho@...escale.com>, linux-kernel@...r.kernel.org,
	Richard Purdie <rpurdie@...ys.net>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	Kumar Gala <galak@...nel.crashing.org>, linuxppc-dev@...abs.org
Subject: Re: [PATCH 2/2] leds: Support OpenFirmware led bindings

On Mon, Jul 28, 2008 at 11:09:14AM -0600, Grant Likely wrote:
[...]
> > >> +- function :  (optional) This parameter, if present, is a string
> > >> +  defining the function of the LED.  It can be used to put the LED
> > >> +  under software control, e.g. Linux LED triggers like "heartbeat",
> > >> +  "ide-disk", and "timer".  Or it could be used to attach a hardware
> > >> +  signal to the LED, e.g. a SoC that can configured to put a SATA
> > >> +  activity signal on a GPIO line.
> > >
> > > This makes me nervous.  It exposes Linux internal implementation details
> > > into the device tree data.  If you want to have a property that
> > > describes the LED usage, then the possible values and meanings should be
> > > documented here.
> > 
> > Should it be a linux specific property then?  I could list all the current
> > linux triggers, but enumerating every possible function someone might want
> > to assign to an LED seems hopeless.
> 
> I don't like adding Linux specific properties to the device tree if at
> all possible, and I really don't like encoding Linux internal details
> (like trigger names).  They can change between kernel versions and
> breaking compatibility with older device trees is strongly avoided.
> That's why so much effort goes into getting bindings correct the first
> time.
> 
> I'd rather see the device tree provide 'hints' toward the expected usage
> and if a platform needs something specific, then the platform specific
> code should setup the trigger.
> 
> Regardless, any hints provided by the binding must be documented.  In
> most cases the gpio-leds driver should be able to figure out which trigger
> to bind without platform code intervention.

Maybe we can encode leds into devices themselves, via phandles?

E.g.

sata@101 {
	compatible = "fsl,sata";
	leds = <&red_led>;
};

And then the OF GPIO LEDs driver could do something like:

char *ide_disk_trigger_compatibles[] = {
	"fsl,sata",
	"ide-generic",
	...
};

for_each_node_with_leds_property(node, led_phandle) {
	if (if_ide_disk_compatible(node)) {
		struct gpio_led *led = phandle_to_led(led_phandle);

		led->default_trigger = "ide-disk";
	}
}

-- 
Anton Vorontsov
email: cbouatmailru@...il.com
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/