All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthias Kaehlcke <matthias@kaehlcke.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] smc911x driver: cleanup smc911x_initialize()
Date: Fri, 22 Jan 2010 10:02:51 +0100	[thread overview]
Message-ID: <20100122090251.GC18402@darwin> (raw)
In-Reply-To: <f8328f7c1001212336s4a1c88f0s2b5779519844a246@mail.gmail.com>

Hi Ben,

El Thu, Jan 21, 2010 at 11:36:41PM -0800 Ben Warren ha dit:

>    On Thu, Jan 21, 2010 at 10:18 PM, Matthias Kaehlcke
>    <matthias@kaehlcke.net> wrote:
> 
>      El Thu, Jan 21, 2010 at 06:01:47PM -0500 Mike Frysinger ha dit:
>      > On Thursday 21 January 2010 16:29:24 Matthias Kaehlcke wrote:
>      > > smc911x_initialize(): remove unecessary call to free() and
>      > > return 0 in case of failure instead of -1
>      > >
>      > > Signed-off-by: Matthias Kaehlcke <matthias@kaehlcke.net>
>      > > ---
>      > >  drivers/net/smc911x.c |    3 +--
>      > >  1 files changed, 1 insertions(+), 2 deletions(-)
>      > >
>      > > diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
>      > > index 5d51406..f2b5895 100644
>      > > --- a/drivers/net/smc911x.c
>      > > +++ b/drivers/net/smc911x.c
>      > > @@ -242,8 +242,7 @@ int smc911x_initialize(u8 dev_num, int
>      base_addr)
>      > >
>      > >     dev = malloc(sizeof(*dev));
>      > >     if (!dev) {
>      > > -           free(dev);
>      >
>      > OK
>      >
>      > > -           return -1;
>      > > +           return 0;
>      >
>      > this is an error path, so i think -1 is correct.  if you're out of
>      memory,
>      > increase your malloc region.
> 
>      that's what I thought in the first place, but Ben Warren told me that
>      in the initialize function the return value indicates the number of
>      devices that were initialized (see
>      http://lists.denx.de/pipermail/u-boot/2010-January/066859.html)
> 
>    I agree that this is confusing.  If the following xxx_eth_initialize()
>    return codes seem reasonable, I'll send a patch tomorrow that applies it
>    to the logic in net/eth.c:
>    -2: reserved (magic value, used to return from __def_eth_init())
>    -1: error
>    0: no devices added, no error
>    1+: number of devices added

to me it seems reasonable and less confusing than the current logic.

i can collaborate in the task of applying it to the driver code

-- 
Matthias Kaehlcke
Embedded Linux Developer
Barcelona

  Control over the use of one's ideas really constitutes control over other
  people's lives; and it is usually used to make their lives more difficult.
                          (Richard Stallman)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

      reply	other threads:[~2010-01-22  9:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-21 21:29 [U-Boot] [PATCH] smc911x driver: cleanup smc911x_initialize() Matthias Kaehlcke
2010-01-21 23:01 ` Mike Frysinger
2010-01-22  6:18   ` Matthias Kaehlcke
2010-01-22  7:36     ` Ben Warren
2010-01-22  9:02       ` Matthias Kaehlcke [this message]

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=20100122090251.GC18402@darwin \
    --to=matthias@kaehlcke.net \
    --cc=u-boot@lists.denx.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.