public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Richard Purdie <rpurdie@linux.intel.com>
To: me@felipebalbi.com
Cc: linux-kernel@vger.kernel.org,
	Felipe Balbi <felipe.balbi@nokia.com>,
	Anton Vorontsov <cbou@mail.ru>,
	David Woodhouse <dwmw2@infradead.org>, Greg KH <greg@kroah.com>,
	Pierre Ossman <drzeus@drzeus.cx>
Subject: Re: [PATCH] led: simplify led_trigger_register_simple
Date: Thu, 20 Nov 2008 13:33:28 +0000	[thread overview]
Message-ID: <1227188008.22263.9.camel@ted> (raw)
In-Reply-To: <20081113191046.GA25855@frodo>

On Thu, 2008-11-13 at 21:10 +0200, Felipe Balbi wrote:
> On Thu, Nov 13, 2008 at 08:14:16PM +0200, Felipe Balbi wrote:
> > On Thu, Nov 13, 2008 at 12:38:32PM +0000, Richard Purdie wrote:
> > > The simple triggers were designed to cause minimum interference to the
> > > usually external subsystem code they were added into. As an example this
> > > meant things like errors were just handled gracefully with a printk
> > > warning and did not take down the whole subsystem. I therefore don't
> > > regard this patch as a simplification, more a complication.
> > 
> > That's a matter of changing the return ERR_PTR(err); back to a printk.
> 
> And here you are. I still think we should at least kfree(trigger) in
> case of error, though.

This patch now just changes the calling convention of the function which
doesn't seem to serve much purpose.

In answer to your question about kfree, I agree it needs to be called
upon error. The callers should just be calling
led_trigger_unregister_simple() in their failure paths though which
should take care of all problems? I know we used to register the simple
triggers late in paths so no error handling was needed to keep the code
simple and minimise the LED triggers impact on those systems.

Cheers,

Richard


  parent reply	other threads:[~2008-11-20 13:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-13  3:09 [PATCH] led: simplify led_trigger_register_simple Felipe Balbi
2008-11-13 12:03 ` Anton Vorontsov
2008-11-13 12:38 ` Richard Purdie
2008-11-13 18:14   ` Felipe Balbi
2008-11-13 19:10     ` Felipe Balbi
2008-11-20 13:10       ` Felipe Balbi
2008-11-20 13:33       ` Richard Purdie [this message]
2008-11-20 14:14         ` Felipe Balbi
2008-11-20 14:45           ` Richard Purdie
2008-11-20 15:01             ` Felipe Balbi

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=1227188008.22263.9.camel@ted \
    --to=rpurdie@linux.intel.com \
    --cc=cbou@mail.ru \
    --cc=drzeus@drzeus.cx \
    --cc=dwmw2@infradead.org \
    --cc=felipe.balbi@nokia.com \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=me@felipebalbi.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