On Fri, Apr 04, 2014 at 12:26:27PM +0300, Kristina Martšenko wrote: > On 03/04/14 13:13, Dan Carpenter wrote: > > On Thu, Apr 03, 2014 at 01:00:53PM +0300, Kristina Martšenko wrote: > >> On 03/04/14 11:32, Dan Carpenter wrote: > >>> On Tue, Mar 25, 2014 at 01:45:09AM +0200, Kristina Martšenko wrote: > >>>> Use a mutex instead of a spinlock in goldfish_nand.c, as suggested by > >>>> the TODO list. > >>>> > >>>> Signed-off-by: Kristina Martšenko <kristina.martsenko@xxxxxxxxx> > >>> > >>> Have you tested this change? > >> > >> Nope, just compile-tested. After a day of trying to get the emulator to > >> work I finally gave up and decided that it looked okay enough... I > >> should have mentioned under the patch description that it wasn't tested, > >> sorry. > > > > It's not a wrong thing to submit patches that you can't test, but in > > this case the irq save/restores make me nervous. I can't see that they > > served any purpose and it's certainly not unheard of for staging code to > > do pointless things for unexplainable reasons. But on the other hand, I > > would feel a lot more comfortable if this change were tested or if there > > were more comments about how the change is safe. > > I'm not sure I understand. A mutex doesn't disable interrupts, so the > cpu irq flags should be the same after the mutex-protected code as they > were before. I.e. it would have the same effect as the save/restore. Or > am I missing something? A mutex doesn't disable IRQs but the original code used spin_lock_irqsave() so the original code did disable IRQs. Looking at it now, I'm not sure there was a reason to disable IRQs... This driver doesn't have an IRQ handler. Your change is probably fine. regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
