mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Vojtech Pavlik <vojtech@suse.cz>
To: dtor_core@ameritech.net
Cc: Prarit Bhargava <prarit@sgi.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH][RFC]: Clean up resource allocation in i8042 driver
Date: Fri, 21 Jan 2005 17:35:40 +0100	[thread overview]
Message-ID: <20050121163540.GC4795@ucw.cz> (raw)
In-Reply-To: <d120d500050121074313788f99@mail.gmail.com>

On Fri, Jan 21, 2005 at 10:43:36AM -0500, Dmitry Torokhov wrote:
> Hi,
> 
> On Fri, 21 Jan 2005 10:14:46 -0500, Prarit Bhargava <prarit@sgi.com> wrote:
> > Hi,
> > 
> > The following patch cleans up resource allocations in the i8042 driver
> > when initialization fails.
> > 
> ...
> > 
> >                if (i8042_command(&param, I8042_CMD_CTL_TEST)) {
> > -                       printk(KERN_ERR "i8042.c: i8042 controller self test timeout.\n");
> > +                       if (i8042_read_status() != 0xFF)
> > +                               printk(KERN_ERR "i8042.c: i8042 controller self test timeout.\n");
> > +                       else
> > +                               printk(KERN_ERR "i8042.c: no i8042 controller found.\n");
> 
> Is this documented somewhere?

No. But vacant ports usually return 0xff. The problem here is that 0xff
is a valid value for the status register, too. Fortunately this patch
checks for 0xff only after the timeout failed.

Anyway, I suppose we could fail silently here on ia64 machines where
ACPI is present.

> >        if (i8042_platform_init())
> > +       {
> > +               del_timer_sync(&i8042_timer);
> >                return -EBUSY;
> > +       }
> > 
> 
> Couple of comments:
>  - i8042_timer has not been started yet so there is no need to delete
> it in either of the chinks.

Indeed.

> - opening brace placement does not follow Linux coding style.
> 
> I think I have some changes to i8042 in my tree, I will add
> i8042_platform_exit calls to the init routine. Thanks for noticing it!

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR

  reply	other threads:[~2005-01-21 16:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-01-21 15:14 Prarit Bhargava
2005-01-21 15:43 ` Dmitry Torokhov
2005-01-21 16:35   ` Vojtech Pavlik [this message]
2005-01-21 16:43     ` Dmitry Torokhov
2005-01-21 16:47     ` Jesse Barnes
2005-01-21 17:17       ` Prarit Bhargava
2005-02-14 16:32         ` Prarit Bhargava
2005-02-14 22:46           ` Dmitry Torokhov
2005-02-15  7:21           ` Vojtech Pavlik
2005-01-21 19:26 ` Dave Jones
2005-01-21 21:47   ` Kyle Moffett

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=20050121163540.GC4795@ucw.cz \
    --to=vojtech@suse.cz \
    --cc=dtor_core@ameritech.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=prarit@sgi.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

Powered by JetHome