| Message ID | 1424772112-27399-5-git-send-email-robert.dolca@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
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 linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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),
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(+)