On Mon, Jan 20, 2020 at 09:30:43AM +0000, Vladimir Stankovic wrote: > +int mausb_enqueue_event_from_user(uint8_t madev_addr, uint32_t all_events) > +{ > + unsigned long flags; > + uint16_t num_of_completed, > + num_of_events; > + struct mausb_device *dev; > + > + spin_lock_irqsave(&mss.lock, flags); > + dev = mausb_get_dev_from_addr_unsafe(madev_addr); > + > + if (!dev) { > + spin_unlock_irqrestore(&mss.lock, flags); > + return 0; > + } > + > + spin_lock_irqsave(&dev->num_of_user_events_lock, flags); > + num_of_completed = (uint16_t)all_events + > + (uint16_t)dev->num_of_user_events; > + num_of_events = (all_events >> (8 * sizeof(num_of_events))) + > + (dev->num_of_user_events >> (8 * sizeof(num_of_events))); > + dev->num_of_user_events = num_of_completed; > + dev->num_of_user_events |= (uint32_t)num_of_events << > + (8 * sizeof(num_of_events)); I might be missing something. Why can't we just declare two struct members instead of doing these bit shifts to fit two values into dev->num_of_user_events? > + spin_unlock_irqrestore(&dev->num_of_user_events_lock, flags); > + queue_work(dev->workq, &dev->work); > + spin_unlock_irqrestore(&mss.lock, flags); > + > + return 0; > +} regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel