Hi Christophe,
On Fri, May 01, 2015 at 10:19:34PM +0200, Christophe Ricard wrote:
> +static int st21nfcb_nci_prop_request(struct nci_dev *ndev,
> + void (*req)(struct nci_dev *ndev,
> + unsigned long opt),
> + unsigned long opt, __u32 timeout)
> +{
> + int r = 0;
> + long completion_r;
> +
> + ndev->req_status = NCI_REQ_PEND;
> +
> + reinit_completion(&ndev->req_completion);
> + req(ndev, opt);
> + completion_r =
> + wait_for_completion_interruptible_timeout(&ndev->req_completion,
> + timeout);
> +
> + pr_debug("wait_for_completion return %ld\n", completion_r);
> +
> + if (completion_r > 0) {
> + switch (ndev->req_status) {
> + case NCI_REQ_DONE:
> + r = nci_to_errno(ndev->req_result);
> + break;
> +
> + case NCI_REQ_CANCELED:
> + r = -ndev->req_result;
> + break;
> +
> + default:
> + r = -ETIMEDOUT;
> + break;
> + }
> + } else {
> + pr_err("wait_for_completion_interruptible_timeout failed %ld\n",
> + completion_r);
> +
> + r = ((completion_r == 0) ? (-ETIMEDOUT) : (completion_r));
> + }
> +
> + ndev->req_status = ndev->req_result = 0;
> + return r;
> +}
At that point, I would be ok for you to export nci_request() and use it
here instead of reimplementing most of it.
> +static void st21nfcb_nci_set_mode_req(struct nci_dev *ndev, unsigned long opt)
> +{
> + struct nci_mode_set_cmd cmd;
> + __u8 mode = opt;
> +
> + cmd.cmd_type = ST21NFCB_SET_NFC_MODE;
> + cmd.mode = mode;
> +
> + nci_send_cmd(ndev, NCI_OP_PROP_CMD,
> + sizeof(struct nci_mode_set_cmd), &cmd);
> +}
> +
> +int st21nfcb_nci_set_mode(struct nci_dev *ndev, u8 mode)
> +{
> + atomic_set(&ndev->cmd_cnt, 1);
> + set_bit(NCI_INIT, &ndev->flags);
Why is that needed ?
> + return st21nfcb_nci_prop_request(ndev, st21nfcb_nci_set_mode_req,
> + mode, msecs_to_jiffies(NCI_CMD_TIMEOUT));
That is racy, as you are not taking the req_lock. You may be woken up by
a previously handled command. Another reason for using an exported
nci_request().
Cheers,
Samuel.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
