| Message ID | 1424772112-27399-2-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:45PM +0200, Robert Dolca wrote: > By calling __nci_request instead of nci_request allows the driver to use > the function while initializing the device (setup stage) > > Signed-off-by: Robert Dolca <robert.dolca@intel.com> > --- > net/nfc/nci/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c > index 9575a18..c4dd5d8 100644 > --- a/net/nfc/nci/core.c > +++ b/net/nfc/nci/core.c > @@ -558,7 +558,7 @@ static void nci_core_conn_close_req(struct nci_dev *ndev, unsigned long opt) > > int nci_core_conn_close(struct nci_dev *ndev, u8 conn_id) > { > - return nci_request(ndev, nci_core_conn_close_req, conn_id, > + return __nci_request(ndev, nci_core_conn_close_req, conn_id, > msecs_to_jiffies(NCI_CMD_TIMEOUT)); You're fixing your problem by removing the NCI request serialization and removing the check for your device being UP. I assume you need to open and close a proprietary connection from your setup hook ? Then please extend nci_request() to check for both NCI_UP and NCI_INIT. 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:45PM +0200, Robert Dolca wrote: >> By calling __nci_request instead of nci_request allows the driver to use >> the function while initializing the device (setup stage) >> >> Signed-off-by: Robert Dolca <robert.dolca@intel.com> >> --- >> net/nfc/nci/core.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c >> index 9575a18..c4dd5d8 100644 >> --- a/net/nfc/nci/core.c >> +++ b/net/nfc/nci/core.c >> @@ -558,7 +558,7 @@ static void nci_core_conn_close_req(struct nci_dev *ndev, unsigned long opt) >> >> int nci_core_conn_close(struct nci_dev *ndev, u8 conn_id) >> { >> - return nci_request(ndev, nci_core_conn_close_req, conn_id, >> + return __nci_request(ndev, nci_core_conn_close_req, conn_id, >> msecs_to_jiffies(NCI_CMD_TIMEOUT)); > You're fixing your problem by removing the NCI request serialization and > removing the check for your device being UP. > I assume you need to open and close a proprietary connection from your > setup hook ? Then please extend nci_request() to check for both NCI_UP > and NCI_INIT. You are right, I am opening and closing a connection from the setup function. The setup is called by nci_open_device. At the beginning of nci_open_device, req_lock is being acquired and it is release at the end of the function. That means that when setup is being called req_lock is acuired. As you said I can modify nci_request to check for NCI_INIT but it tries to acquire req_lock and it can not succeed. We could use another mutex for nci_request but I am not sure if that is a good idea. Regards, 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
Hi Robert, On Tue, Mar 31, 2015 at 05:03:42PM +0300, Robert Dolca wrote: > 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:45PM +0200, Robert Dolca wrote: > >> By calling __nci_request instead of nci_request allows the driver to use > >> the function while initializing the device (setup stage) > >> > >> Signed-off-by: Robert Dolca <robert.dolca@intel.com> > >> --- > >> net/nfc/nci/core.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c > >> index 9575a18..c4dd5d8 100644 > >> --- a/net/nfc/nci/core.c > >> +++ b/net/nfc/nci/core.c > >> @@ -558,7 +558,7 @@ static void nci_core_conn_close_req(struct nci_dev *ndev, unsigned long opt) > >> > >> int nci_core_conn_close(struct nci_dev *ndev, u8 conn_id) > >> { > >> - return nci_request(ndev, nci_core_conn_close_req, conn_id, > >> + return __nci_request(ndev, nci_core_conn_close_req, conn_id, > >> msecs_to_jiffies(NCI_CMD_TIMEOUT)); > > You're fixing your problem by removing the NCI request serialization and > > removing the check for your device being UP. > > I assume you need to open and close a proprietary connection from your > > setup hook ? Then please extend nci_request() to check for both NCI_UP > > and NCI_INIT. > > You are right, I am opening and closing a connection from the setup > function. The setup is called by nci_open_device. At the beginning of > nci_open_device, req_lock is being acquired and it is release at the > end of the function. That means that when setup is being called > req_lock is acuired. As you said I can modify nci_request to check for > NCI_INIT but it tries to acquire req_lock and it can not succeed. I see, I thought the issue was only about checking the NCI_* flags. As a short term solution, I propose you do the following: a) Export nci_core_conn_create_req, nci_core_conn_close_req and __nci_request. b) Call __nci_request() directly from your fdp_nci_close_conn() and fdp_nci_create_conn() routines. The long term, scalable fix would be to implement and export an __nci_send_cmd_sync() routine, that would transparently build an NCI request and tail it to the ndev req skb queue, and put the caller on a wait queue. The created request's response callback would then wake the caller up. 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 Sun, May 24, 2015 at 8:07 PM, Samuel Ortiz <sameo@linux.intel.com> wrote: > Hi Robert, > > On Tue, Mar 31, 2015 at 05:03:42PM +0300, Robert Dolca wrote: >> 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:45PM +0200, Robert Dolca wrote: >> >> By calling __nci_request instead of nci_request allows the driver to use >> >> the function while initializing the device (setup stage) >> >> >> >> Signed-off-by: Robert Dolca <robert.dolca@intel.com> >> >> --- >> >> net/nfc/nci/core.c | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c >> >> index 9575a18..c4dd5d8 100644 >> >> --- a/net/nfc/nci/core.c >> >> +++ b/net/nfc/nci/core.c >> >> @@ -558,7 +558,7 @@ static void nci_core_conn_close_req(struct nci_dev *ndev, unsigned long opt) >> >> >> >> int nci_core_conn_close(struct nci_dev *ndev, u8 conn_id) >> >> { >> >> - return nci_request(ndev, nci_core_conn_close_req, conn_id, >> >> + return __nci_request(ndev, nci_core_conn_close_req, conn_id, >> >> msecs_to_jiffies(NCI_CMD_TIMEOUT)); >> > You're fixing your problem by removing the NCI request serialization and >> > removing the check for your device being UP. >> > I assume you need to open and close a proprietary connection from your >> > setup hook ? Then please extend nci_request() to check for both NCI_UP >> > and NCI_INIT. >> >> You are right, I am opening and closing a connection from the setup >> function. The setup is called by nci_open_device. At the beginning of >> nci_open_device, req_lock is being acquired and it is release at the >> end of the function. That means that when setup is being called >> req_lock is acuired. As you said I can modify nci_request to check for >> NCI_INIT but it tries to acquire req_lock and it can not succeed. > I see, I thought the issue was only about checking the NCI_* flags. > > As a short term solution, I propose you do the following: > > a) Export nci_core_conn_create_req, nci_core_conn_close_req and > __nci_request. > b) Call __nci_request() directly from your fdp_nci_close_conn() and > fdp_nci_create_conn() routines. > > The long term, scalable fix would be to implement and export an > __nci_send_cmd_sync() routine, that would transparently build an NCI > request and tail it to the ndev req skb queue, and put the caller on a > wait queue. The created request's response callback would then wake the > caller up. If nci_open_device would use another mutex instead of req_lock this wouldn't be necessary. I don't see any reason why nci_open_device should block the send queue. Of course, in nci_open_device all calls to __nci_request would have to be replaced with nci_request. Samuel, would that be an acceptable solution? Regards, 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/net/nfc/nci/core.c b/net/nfc/nci/core.c index 9575a18..c4dd5d8 100644 --- a/net/nfc/nci/core.c +++ b/net/nfc/nci/core.c @@ -558,7 +558,7 @@ static void nci_core_conn_close_req(struct nci_dev *ndev, unsigned long opt) int nci_core_conn_close(struct nci_dev *ndev, u8 conn_id) { - return nci_request(ndev, nci_core_conn_close_req, conn_id, + return __nci_request(ndev, nci_core_conn_close_req, conn_id, msecs_to_jiffies(NCI_CMD_TIMEOUT)); } EXPORT_SYMBOL(nci_core_conn_close);
By calling __nci_request instead of nci_request allows the driver to use the function while initializing the device (setup stage) Signed-off-by: Robert Dolca <robert.dolca@intel.com> --- net/nfc/nci/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)