Re: [KJ] Re: [UPDATE PATCH] net/sb1000: replace nicedelay() with

From: Nish Aravamudan <nish.aravamudan@gmail.com>
To: Jeff Garzik <jgarzik@pobox.com>
Cc: Nishanth Aravamudan <nacc@us.ibm.com>,
	kj <kernel-janitors@lists.osdl.org>,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [KJ] Re: [UPDATE PATCH] net/sb1000: replace nicedelay() with
Date: Tue, 11 Jan 2005 04:10:23 +0000	[thread overview]
Message-ID: <29495f1d0501102010531fa775@mail.gmail.com> (raw)
In-Reply-To: <41E34E6F.70002@pobox.com>

[-- Attachment #1: Type: text/plain, Size: 2263 bytes --]

On Mon, 10 Jan 2005 22:56:31 -0500, Jeff Garzik <jgarzik@pobox.com> wrote:
> Nishanth Aravamudan wrote:
> > @@ -475,7 +467,7 @@ sb1000_reset(const int ioaddr[], const c
> >       udelay(1000);
> >       outb(0x0, port);
> >       inb(port);
> > -     nicedelay(60000);
> > +     ssleep(1);
> >       outb(0x4, port);
> >       inb(port);
> >       udelay(1000);
> > @@ -537,7 +529,7 @@ sb1000_activate(const int ioaddr[], cons
> >       const unsigned char Command0[6] = {0x80, 0x11, 0x00, 0x00, 0x00, 0x00};
> >       const unsigned char Command1[6] = {0x80, 0x16, 0x00, 0x00, 0x00, 0x00};
> >
> > -     nicedelay(50000);
> > +     ssleep(1);
> >       if ((status = card_send_command(ioaddr, name, Command0, st)))
> >               return status;
> >       if ((status = card_send_command(ioaddr, name, Command1, st)))
> > @@ -944,7 +936,7 @@ sb1000_open(struct net_device *dev)
> >       /* initialize sb1000 */
> >       if ((status = sb1000_reset(ioaddr, name)))
> >               return status;
> > -     nicedelay(200000);
> > +     ssleep(1);
> >       if ((status = sb1000_check_CRC(ioaddr, name)))
> >               return status;
> 
> 
> Your conversion of nicedelay() -> ssleep() values is imprecise.

True, but this is what I attempted to allude to in the description of the patch:

> > Remove the prototype and
> > definition of nicedelay(). This is a very weird function, because it is
> > called to sleep in terms of usecs, but always sleeps for 1 second,
> > completely ignoring the parameter. I have gone ahead and followed suit,
> > just sleeping for a second in all cases, but maybe someone with the
> > hardware could tell me if perhaps the paramter *should* matter.

> The author clearly intended the values to be different, right?

Since I'm not the author, I'm not certain whether you are right or
not. I was honestly very confused by nicedelay(). It is called with
various 600000, 500000, 200000 usecs, but the function currently
always requests a 1000000 usec delay (a full second of interruptible
sleep). It just doesn't make any sense... I have sent messages
regarding this function several times (and patches have existed for a
while), but no one has had any comment. I appreciate your input
greatly. What do you think?

Thanks,
Nish

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors