Re: [PATCH] NFC: Driver for Inside Secure MicroRead NFC chip

[Arnd Bergmann]

On Friday 18 March 2011, Waldemar.Rymarkiewicz@xxxxxxxxx wrote:
> >
> >Ermm nope.. why do we have do nothing ioctls ?
> >
> 
> onfc stack requires those ones, but they are only valid for a specific test enviroment.
> This should not be a case for driver and the stack should care about it if it needs
> this. Then will remove it.

The way this normally works is to figure out the correct
way to implement the kernel driver first and then write
a user space stack around it, not the other way round...
  
> >> +
> >> +    mutex_lock(&info->rx_mutex);
> >> +    info->irq_state = 1;
> >> +    mutex_unlock(&info->rx_mutex);
> >
> >Would it not be lighter to use atomic bit ops ?
> 
> Do you mean  in order to remove rx_mutex? 
> 
> mutex_lock(&info->rx_mutex);
> atomic_set(info->irq_state ,1);
> mutex_unlock(&info->rx_mutex);
> 
> looks a bit strange. I still need the rx_mutex to protect irq_state while reading i2c.
> 
>         mutex_lock(&info->rx_mutex);
>         ret = i2c_master_recv(client, info->buf, info->buflen);
>         info->irq_state = 0;
>         mutex_unlock(&info->rx_mutex);

What Alan meant was set_bit/test_and_clear_bit here, not atomic_*().
As I mentioned in the other mail, the way you check the flag is
probably still racy with the current mutex use.

Note that the interrupt handler doesn't do much at all, so it's probably
a good idea to use a regular (non-threaded) handler here to reduce
the overhead, once you have removed the mutex.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/