linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Fabio Baltieri <fabio.baltieri@gmail.com>
To: Oliver Hartkopp <socketcan@hartkopp.net>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	linux-can@vger.kernel.org, linux-kernel@vger.kernel.org,
	Wolfgang Grandegger <wg@grandegger.com>
Subject: Re: [PATCH can-next v6] can: add tx/rx LED trigger support
Date: Thu, 6 Sep 2012 22:57:28 +0200	[thread overview]
Message-ID: <20120906205728.GC4043@gmail.com> (raw)
In-Reply-To: <20120906151158.GA37075@macbook.local>

On Thu, Sep 06, 2012 at 05:11:58PM +0200, Kurt Van Dijck wrote:
> > > 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.

Can't argue with that... I'm trying to see how it comes but names like
"can-3-2:1.0-tx" doesn't looks that friendly to me...

> * extends existing function calls like sja1000

Indeed.

> > > 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.

That's not correct, default trigger is going to be set only if there are
no other trigger assigned to the LED, and only on led probe and trigger
probe.

So, the LED framework is not going to change the trigger if you manually
changed it to something else, and in any case default_trigger assignment
only happens at probe/exit, not when interface is set up/down.

> > > 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.

Way to go! :-)

Fabio

  reply	other threads:[~2012-09-06 20:56 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-30 19:20 [PATCH can-next v3 1/2] can: add tx/rx LED trigger support Fabio Baltieri
2012-07-30 19:20 ` [PATCH can-next v3 2/2] can: flexcan: add " Fabio Baltieri
2012-07-30 21:17 ` [PATCH can-next v3 1/2] can: add tx/rx " Marc Kleine-Budde
2012-07-31  6:57   ` Fabio Baltieri
2012-07-31  7:10     ` Marc Kleine-Budde
2012-07-31 11:57       ` Fabio Baltieri
2012-07-31 12:00         ` Marc Kleine-Budde
2012-07-31 22:05           ` [PATCH can-next v4] " Fabio Baltieri
2012-08-01  9:36             ` Marc Kleine-Budde
2012-08-01 10:07               ` Marc Kleine-Budde
2012-08-01 10:30               ` Fabio Baltieri
2012-08-01 11:37                 ` Marc Kleine-Budde
2012-08-01 11:49               ` [PATCH can-next v5 1/2] " Fabio Baltieri
2012-08-01 11:49                 ` [PATCH can-next v5 2/2] can: flexcan: add " Fabio Baltieri
2012-08-01 11:53                   ` Marc Kleine-Budde
2012-08-01 12:24                     ` Fabio Baltieri
2012-08-01 12:30                       ` Marc Kleine-Budde
2012-08-01 21:02                   ` Marc Kleine-Budde
2012-08-01 11:59                 ` [PATCH can-next v5 1/2] can: add tx/rx " Marc Kleine-Budde
2012-08-01 12:06                 ` Oliver Hartkopp
2012-08-01 12:18                   ` Marc Kleine-Budde
2012-08-01 18:21                     ` [PATCH can-next v6] " Fabio Baltieri
2012-08-01 21:00                       ` Marc Kleine-Budde
2012-08-01 22:38                         ` Fabio Baltieri
2012-08-01 21:05                       ` Oliver Hartkopp
2012-08-24  5:10                       ` Kurt Van Dijck
2012-08-24 11:28                         ` Marc Kleine-Budde
2012-08-24 12:42                           ` Kurt Van Dijck
2012-08-24 22:01                             ` Fabio Baltieri
2012-08-25 20:25                               ` Kurt Van Dijck
2012-09-03 12:40                               ` Marc Kleine-Budde
2012-09-03 18:13                                 ` Kurt Van Dijck
2012-09-03 18:29                                   ` Fabio Baltieri
2012-09-03 20:54                                     ` Oliver Hartkopp
2012-09-04  7:11                                       ` Kurt Van Dijck
2012-09-04  9:29                                         ` [PATCH] can: rename LED trigger name on netdev renames Kurt Van Dijck
2012-09-06 18:59                                           ` Fabio Baltieri
2012-09-06 19:31                                             ` Oliver Hartkopp
2012-09-06 20:46                                               ` Fabio Baltieri
2012-09-07  7:19                                                 ` Kurt Van Dijck
2012-09-09 16:17                                                   ` [PATCH v2] " Fabio Baltieri
2012-09-10 14:25                                                     ` [PATCH] can: export a safe netdev_priv wrapper for candev Kurt Van Dijck
2012-09-10 18:22                                                       ` Oliver Hartkopp
2012-09-10 18:29                                                       ` Fabio Baltieri
2012-09-10 19:55                                                         ` [PATCH v2] " Kurt Van Dijck
2012-09-10 14:28                                                     ` [PATCH v3] can: rename LED trigger name on netdev renames Kurt Van Dijck
2012-09-10 18:25                                                       ` Oliver Hartkopp
2012-09-10 18:40                                                         ` Fabio Baltieri
2012-09-10 19:01                                                           ` Oliver Hartkopp
2012-09-10 20:08                                                             ` Kurt Van Dijck
2012-09-11  5:42                                                               ` Oliver Hartkopp
2012-09-11  7:13                                                                 ` Fabio Baltieri
2012-09-12  7:22                                                                   ` Kurt Van Dijck
2012-09-11  8:05                                                               ` Kurt Van Dijck
2012-09-10 20:06                                                           ` [PATCH v4] " Kurt Van Dijck
2012-09-11 21:04                                                             ` Fabio Baltieri
2012-09-04 20:15                                         ` [PATCH can-next v6] can: add tx/rx LED trigger support Fabio Baltieri
2012-09-06 10:33                                           ` Kurt Van Dijck
2012-09-06 11:17                                             ` Fabio Baltieri
2012-09-06 15:11                                               ` Kurt Van Dijck
2012-09-06 20:57                                                 ` Fabio Baltieri [this message]
2012-09-07  7:04                                                   ` Kurt Van Dijck
2012-09-07 18:59                                                     ` Fabio Baltieri
2012-07-31  8:46 ` [PATCH can-next v3 1/2] " Wolfgang Grandegger
2012-07-31 10:12   ` Marc Kleine-Budde
2012-07-31 11:55     ` Fabio Baltieri
2012-07-31 12:14   ` Fabio Baltieri

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=20120906205728.GC4043@gmail.com \
    --to=fabio.baltieri@gmail.com \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=socketcan@hartkopp.net \
    --cc=wg@grandegger.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).