On Mon, Jun 06, 2005 at 11:19:50PM -0600, Zwane Mwaikambo wrote: > On Mon, 6 Jun 2005, Matt Porter wrote: > > > +spinlock_t rio_global_list_lock = SPIN_LOCK_UNLOCKED; > > spin_lock_init?
How about DEFINE_SPINLOCK() as they do the same thing (except the difference in amount of babble)? This is what PCI is doing, for example. I use the same init method in the static locks found in rio-access.c, FWIW. > > +extern struct rio_route_ops __start_rio_route_ops[]; > > +extern struct rio_route_ops __end_rio_route_ops[]; > > rio.h? Yep, will move. > > +static void > > +rio_set_device_id(struct rio_mport *port, u16 destid, u8 hopcount, u16 did) > > Shouldn't those be on the same line? Yes, will fix. > > +static int rio_device_has_destid(struct rio_mport *port, int src_ops, > > + int dst_ops) > > +{ > > + if (((src_ops & RIO_SRC_OPS_READ) || > > + (src_ops & RIO_SRC_OPS_WRITE) || > > + (src_ops & RIO_SRC_OPS_ATOMIC_TST_SWP) || > > + (src_ops & RIO_SRC_OPS_ATOMIC_INC) || > > + (src_ops & RIO_SRC_OPS_ATOMIC_DEC) || > > + (src_ops & RIO_SRC_OPS_ATOMIC_SET) || > > + (src_ops & RIO_SRC_OPS_ATOMIC_CLR)) && > > + ((dst_ops & RIO_DST_OPS_READ) || > > + (dst_ops & RIO_DST_OPS_WRITE) || > > + (dst_ops & RIO_DST_OPS_ATOMIC_TST_SWP) || > > + (dst_ops & RIO_DST_OPS_ATOMIC_INC) || > > + (dst_ops & RIO_DST_OPS_ATOMIC_DEC) || > > + (dst_ops & RIO_DST_OPS_ATOMIC_SET) || > > + (dst_ops & RIO_DST_OPS_ATOMIC_CLR))) { > > + return 1; > > Why not just; > > mask = (RIO_DST_OPS_READ | RIO_DST_OPS_WRITE....) > return !!((dst_ops & mask) && (src_ops & mask)) Yes, that's good. I already had that comment from an IRC discussion and neglected to address it in the last updates. Will do. > > + rdev->dev.dma_mask = (u64 *) 0xffffffff; > > + rdev->dev.coherent_dma_mask = 0xffffffffULL; > > Shouldn't that be dma_set_mask? There is a problem there, yes, but it shouldn't use dma_set_mask(). dma_set_mask() needs [struct device]->dma_mask to be non-zero (initialized to something) to do anything in all the copied implementations I've seen. I need to do something like the following to initialize the struct device dma_mask properly: rdev->dma_mask = 0xffffffffULL; rdev->dev.dma_mask = &rdev->dma_mask; -Matt