Messages in this thread | | | Date | Tue, 15 Dec 2009 17:50:29 -0700 | From | Jonathan Corbet <> | Subject | Re: [RFC][Patch] IBM Real-Time "SMI Free" mode drive -v2 |
| |
On Tue, 15 Dec 2009 16:38:05 -0800 Keith Mannthey <kmannth@us.ibm.com> wrote:
> > Do you need to create a new directory for a single .c file? > > Perhaps not. I have a single .h and a single .c file. > Do you think I should move them right into misc (with maybe a better > name the rtl.h)?
That's what I would do, but this is a detail.
> > For a driver which is intended to help reduce latencies, a 10ms delay with > > a spinlock held seems a little harsh. It seems like maybe you could drop > > the lock and use msleep()? > > Irony is that doing this special write actually triggers an SMI. > > Well I really don't want any other possible action to happen until after > the command finishes. I can definitely shorten the amount of time of > the mdelay but I don't want any possible way to start another write > until the command finishes.
Why not protect it with a mutex and use msleep()? I see no reason why you need a spinlock there.
> > > + if (count++ > 10000) { > > > > ...that's 100 seconds total - a *long* time... > > The is a "possible" error condition that I have given a large window > too. More than happy to shorten it up to say a small human amount of > time maybe 2 seconds?
Whatever makes sense for the hardware. Certainly there comes a point where you can conclude it's not going to get back to you. If you get rid of the busy wait, you can delay a lot longer here without creating trouble.
> > Again, you should use ioread32() here. > > I should never directly access the ioremap region?
No, you really shouldn't. Probably, on any machine where this driver is relevant you get away with it just fine, but it's not the way to write portable code.
jon
| |