[PATCH] staging: goldfish: switch from spinlock to mutex

[Kristina Martšenko]

On 04/04/14 14:04, Dan Carpenter wrote:
> 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 at gmail.com>
>>>>>
>>>>> 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.

Yes, I didn't find any interrupt handlers either, which is partially why
I thought it was (probably) safe.

Kristina