On Thu, Oct 09, 2014 at 11:27:12PM +0300, Octavian Purdila wrote: > On Thu, Oct 9, 2014 at 10:44 PM, Joe Perches <joe@xxxxxxxxxxx> wrote: > > On Thu, 2014-10-09 at 22:22 +0300, Octavian Purdila wrote: > >> diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c > > [] > > +struct dln2_mod_rx_slots { > > + /* RX slots bitmap */ > > + unsigned long bmap; > > > > Probably better as: > > DECLARE_BITMAP(bmap, DLN2_MAX_RX_SLOTS); > > > > Then a lot of &ptr->bmap uses can be ptr->bmap > > > > Originally I was using DECLARE_BITMAP, but during the review process > Johan suggested to use unsigned long. Now that I think about it, it > sounds better to use DECLARE_BITMAP and of couse keep using > find_first_bit, set_bit, etc. Johan do you see any issue with that? No, that's fine as long as you keep the bitops. > >> +struct dln2_dev { > >> + struct usb_device *usb_dev; > >> + struct usb_interface *interface; > >> + u8 ep_in; > >> + u8 ep_out; > >> + > >> + struct urb *rx_urb[DLN2_MAX_URBS]; > >> + void *rx_buf[DLN2_MAX_URBS]; > >> + > >> + struct dln2_mod_rx_slots mod_rx_slots[DLN2_HANDLES]; > >> + > >> + struct list_head event_cb_list; > >> + spinlock_t event_cb_lock; > >> + > >> + bool disconnect; > >> + int active_transfers; > >> + wait_queue_head_t disconnect_wq; > >> + spinlock_t disconnect_lock; > >> +}; > > > > Maybe reorder the bools and u8s to pack this a bit better? > > I prefer to keep it this way, it's not wasting a lot since you will > only have a handful of these devices, and it keeps the related data > together. I agree. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html