diff mbox

[4/8] NFC: NCI: Add a special nci_request for driver

Message ID 1424772112-27399-5-git-send-email-robert.dolca@intel.com
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Robert Dolca Feb. 24, 2015, 10:01 a.m. UTC
This patch adds nci_request_driver and nci_req_complete_driver
as a wrapper for __nci_request. When nci_req_complete_driver is
called it also sets cmd_cnt to 1. This is done because the response is not
sent to the NFC subsystem so cmd_cnt is not decremented there.

nci_send_cmd was previously exported in order to send commands to
the device from the driver. It shouldn't be used without
nci_req_complete_driver because cmd_cnt will have the wrong value.

Signed-off-by: Robert Dolca <robert.dolca@intel.com>
---
 include/net/nfc/nci_core.h |  4 ++++
 net/nfc/nci/core.c         | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+)

Comments

Samuel Ortiz March 26, 2015, 12:29 a.m. UTC | #1
Hi Robert,

On Tue, Feb 24, 2015 at 12:01:48PM +0200, Robert Dolca wrote:
> This patch adds nci_request_driver and nci_req_complete_driver
> as a wrapper for __nci_request. When nci_req_complete_driver is
> called it also sets cmd_cnt to 1. This is done because the response is not
> sent to the NFC subsystem so cmd_cnt is not decremented there.
> 
> nci_send_cmd was previously exported in order to send commands to
> the device from the driver. It shouldn't be used without
> nci_req_complete_driver because cmd_cnt will have the wrong value.
Any NCI packet should make it to the NCI core first, because the logic
and synchornization between Tx and Rx, commands and response is implemented
there. When exporting this kind of symbols, you're opening a can of worms by
potentially allowing each driver to implement the same kind of logic
that's already implemented in the NCI core.

The solution I'd like to see implemented is the following one: Add 1
additional nci_ops entry for handling proprietary packets on the Rx
path. From your driver perspective, you call nci_recv_frame for each and
every packet that you receive: No intercept, no trap. The core will call
your proprietary packets handler for each packet (NTF or RSP) with a
proprietary GID.
You should then be able to remove all the rx work, rx queue and
intercept logic from your driver.

Cheers,
Samuel.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Robert Dolca March 31, 2015, 2:07 p.m. UTC | #2
On Thu, Mar 26, 2015 at 2:29 AM, Samuel Ortiz <sameo@linux.intel.com> wrote:
> Hi Robert,
>
> On Tue, Feb 24, 2015 at 12:01:48PM +0200, Robert Dolca wrote:
>> This patch adds nci_request_driver and nci_req_complete_driver
>> as a wrapper for __nci_request. When nci_req_complete_driver is
>> called it also sets cmd_cnt to 1. This is done because the response is not
>> sent to the NFC subsystem so cmd_cnt is not decremented there.
>>
>> nci_send_cmd was previously exported in order to send commands to
>> the device from the driver. It shouldn't be used without
>> nci_req_complete_driver because cmd_cnt will have the wrong value.
> Any NCI packet should make it to the NCI core first, because the logic
> and synchornization between Tx and Rx, commands and response is implemented
> there. When exporting this kind of symbols, you're opening a can of worms by
> potentially allowing each driver to implement the same kind of logic
> that's already implemented in the NCI core.
>
> The solution I'd like to see implemented is the following one: Add 1
> additional nci_ops entry for handling proprietary packets on the Rx
> path. From your driver perspective, you call nci_recv_frame for each and
> every packet that you receive: No intercept, no trap. The core will call
> your proprietary packets handler for each packet (NTF or RSP) with a
> proprietary GID.
> You should then be able to remove all the rx work, rx queue and
> intercept logic from your driver.

Thanks for the suggestion. Will do.

Robert
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/net/nfc/nci_core.h b/include/net/nfc/nci_core.h
index 4358d0a..42ec342 100644
--- a/include/net/nfc/nci_core.h
+++ b/include/net/nfc/nci_core.h
@@ -333,6 +333,10 @@  void nci_clear_target_list(struct nci_dev *ndev);
 #define NCI_REQ_CANCELED	2
 
 void nci_req_complete(struct nci_dev *ndev, int result);
+int nci_request_driver(struct nci_dev *ndev,
+		       void (*req)(struct nci_dev *ndev, unsigned long opt),
+		       unsigned long opt, __u32 timeout);
+void nci_req_complete_driver(struct nci_dev *ndev, int result);
 struct nci_conn_info *nci_get_conn_info_by_conn_id(struct nci_dev *ndev,
 						   int conn_id);
 
diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 317b94b..1a449ac 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -74,6 +74,17 @@  void nci_req_complete(struct nci_dev *ndev, int result)
 	}
 }
 
+void nci_req_complete_driver(struct nci_dev *ndev, int result)
+{
+	nci_req_complete(ndev, result);
+
+	/* trigger the next cmd */
+	atomic_set(&ndev->cmd_cnt, 1);
+	if (!skb_queue_empty(&ndev->cmd_q))
+		queue_work(ndev->cmd_wq, &ndev->cmd_work);
+}
+EXPORT_SYMBOL(nci_req_complete_driver);
+
 static void nci_req_cancel(struct nci_dev *ndev, int err)
 {
 	if (ndev->req_status == NCI_REQ_PEND) {
@@ -127,6 +138,14 @@  static int __nci_request(struct nci_dev *ndev,
 	return rc;
 }
 
+int nci_request_driver(struct nci_dev *ndev,
+		       void (*req)(struct nci_dev *ndev, unsigned long opt),
+		       unsigned long opt, __u32 timeout)
+{
+	return __nci_request(ndev, req, opt, timeout);
+}
+EXPORT_SYMBOL(nci_request_driver);
+
 inline int nci_request(struct nci_dev *ndev,
 		       void (*req)(struct nci_dev *ndev,
 				   unsigned long opt),