On Fri, Sep 09, 2005 at 01:49:55PM -0700, Dale Farnsworth wrote: > On Fri, Sep 09, 2005 at 08:20:20PM +0000, Walter L. Wimer III wrote: > > On Fri, 2005-09-09 at 12:27 -0700, Dale Farnsworth wrote: > > > > > > No additional locking is necessary. In fact, it seems to me that the > > > 32-bit > > > register reads and writes are already atomic and all of the locking using > > > mv64x60_lock is superfluous. > > > > Ah ha. mv64x60.h also defines an mv64x60_modify() function that isn't > > intrinsically atomic, so it needs the spinlock. That in turn requires > > mv64x60_read() and mv64x60_write() to play along too. > > Yes, the lock is needed for mv64x60_modify(), mv64x60_write(). I still > don't think it's needed for mv64x60_read().
IMHO, the mv64x60_read/write/modify routines should be deleted and the locking + I/O done explicitly. That makes it more obvious, more efficient in places where there are multiple writes, etc. in a row (not as much grabbing/releasing of the spinlock), and is the preferred way to do things in linux. A few times now, I almost started to do that...looked at it and went off to do something else. :) Yes, I'm lazy. Yes, I should do it. Eventually, I will (however, if you want to, I won't complain ;). Mark
