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

[Nish Aravamudan <nish.aravamudan@xxxxxxxxx>]

On Mon, 10 Jan 2005 22:56:31 -0500, Jeff Garzik <jgarzik@xxxxxxxxx> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/