RE: [[linux-nfc] PATCH v5 2/3] driver: nfc: Add ST95HF NFC Transceiver support

[Shikha SINGH <shikha.singh@st.com>]

Hello Samuel,
	Please see my answer below.
	
>This looks a lot better than the initial version.
>I only have one question:
>
>On Fri, Nov 20, 2015 at 06:40:20AM -0500, Shikha Singh wrote:
>> +/*
>> + * st95hf_send_recv_cmd() is for sending commands to ST95HF
>> + * that are described in the cmd_array[]. It can optionally
>> + * receive the response if the cmd request is of type
>> + * SYNC. For that to happen caller must pass true to recv_res.
>> + * For ASYNC request, recv_res is ignored and the
>> + * function will never try to receive the response on behalf
>> + * of the caller.
>> + */
>> +static int st95hf_send_recv_cmd(struct st95hf_context *st95context,
>> +				enum st95hf_cmd_list cmd,
>> +				int no_modif,
>> +				struct param_list *list_array,
>> +				bool recv_res)
>> +{
>> +	unsigned char spi_cmd_buffer[MAX_CMD_LEN];
>> +	int i, ret;
>> +	struct device *dev = &st95context->spicontext.spidev->dev;
>How do you know this driver is still valid at that point ?
>It seems to be a potential corner case against the driver's remove function, but
>it seems to be a race nevertheless.

st95hf_send_recv_cmd() is a static function that can be called only when a NFC digital request is submitted to our driver.
So in summary, it can be called either from an implemented NFC digital ops (such as st95hf_switch_rf()) or from the driver's threaded ISR.
Now if we see the remove function of the driver i.e. st95hf_remove(), it waits for all the outstanding NFC digital request to finish via nfc_digital_unregister_device(stcontext->ddev).

It also waits for any outstanding threaded ISR to finish using the semaphore mechanism:

        /* if last in_send_cmd's ISR is pending, wait for it to finish */
        result = down_killable(&stcontext->exchange_lock);

The spi device object is removed after st95hf_remove() finishes, but then we can't have any call to st95hf_send_recv_cmd() after st95hf_remove finishes !
Till st95hf_remove() finishes, there is no issue in calling spi functions/data structures (such as spi device) either in "parallel" from a different thread or called from st95hf_remove() itself.
So I don't think there is race condition.

Please let me know if you think there could still be a race condition here and also tell me the corresponding use-case (sequence of calls that could lead to such a race).

Thanks and BR,
Shikha