linux-kernel - Re: [PATCH can-next v6] can: add tx/rx LED trigger support

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]

Message-ID: <20120906151158.GA37075@macbook.local>
Date:	Thu, 6 Sep 2012 17:11:58 +0200
From:	Kurt Van Dijck <kurt.van.dijck@....be>
To:	Fabio Baltieri <fabio.baltieri@...il.com>
Cc:	Oliver Hartkopp <socketcan@...tkopp.net>,
	Marc Kleine-Budde <mkl@...gutronix.de>,
	linux-can@...r.kernel.org, linux-kernel@...r.kernel.org,
	Wolfgang Grandegger <wg@...ndegger.com>
Subject: Re: [PATCH can-next v6] can: add tx/rx LED trigger support

On Thu, Sep 06, 2012 at 01:17:39PM +0200, Fabio Baltieri wrote:
> Hi Kurt,
> 
> On Thu, Sep 6, 2012 at 12:33 PM, Kurt Van Dijck <kurt.van.dijck@....be> wrote:
> > On Tue, Sep 04, 2012 at 10:15:53PM +0200, Fabio Baltieri wrote:
> >> On Tue, Sep 04, 2012 at 09:11:28AM +0200, Kurt Van Dijck wrote:
> > [...]
> >> > > The name of the device can only be changed when the interface is down.
> >> > > Is it possible to put some scripting around it to detach and attach the leds
> >> > > to the interfaces on ifup/ifdown triggers?
> >> >
> >> > Are the led triggers available for using while the netdev is down then?
> >>
> >> Sure!  On embedded systems triggers are usually attached to actual LEDs
> >> at probe time using default_trigger field of struct led_classdev, and
> >> that can be specified both in machine files or in device tree.
> >
> > I also think that led triggers should be available.
> 
> Right, that's why I think the only way is to use device name.

yes, but it has 2 disadvantages:
* inconvenient. I like 'can0-tx' much more than any device name
  (Sorry I can't get any real example, I'm not at home now).
  And for usecases where the actual device of the CAN iface is
  important, such setup requires udev to assign the proper iface names.
* extends existing function calls like sja1000

> 
> > I asked the question because detach & attach leds to interfaces would
> > indeed break that.
> 
> Sure? I think that the trigger would be set again on reattach, as
> default_trigger is checked both in led_cdev probe and
> trigger_register, see:
> 
> http://lxr.free-electrons.com/source/drivers/leds/led-triggers.c#L180
> 
> I'll try that tonight.

It looks even more inconvenient. It only works as expected when you don't
change the trigger afterwards, but still it is possible (as should be),
so the design of trigger is ... wrong.
example:
When you put another trigger to a led, and have a proper sequence of
'ip link set xxx up' and 'ip link set xxx down', you will end up
with the default_trigger again.
I realize I'm looking at unusual scenario's.

> 
> > btw, I tried to send a patch tuesday (my first $ git send-email) using
> > netdev notifiers: did you receive it, and what do you think of it?
> 
> Sure, I got it!
Ok, well done for me then :-)
> I was planning to try that this weekend but I can
> give you some comments earlier tonight... sorry for the dealy!
no hurry, I'm already satified that I got git setup right.

Kurt
--
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/