Re: [PATCH 07/11] NFC: pn533: improve cmd queue handling
Hi Michael, On Thu, Apr 21, 2016 at 04:43:55PM +0200, Michael Thalmeier wrote: > Make sure cmd is set before a frame is passed to the transport layer for > sending. In addition pn533_send_async_complete checks if cmd is set before > accessing its members. > > Signed-off-by: Michael Thalmeier> --- > drivers/nfc/pn533/pn533.c | 54 > +-- > 1 file changed, 34 insertions(+), 20 deletions(-) > > diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c > index d9c5583..d1cc70a 100644 > --- a/drivers/nfc/pn533/pn533.c > +++ b/drivers/nfc/pn533/pn533.c > @@ -383,26 +383,36 @@ static void pn533_build_cmd_frame(struct pn533 *dev, u8 > cmd_code, > static int pn533_send_async_complete(struct pn533 *dev) > { > struct pn533_cmd *cmd = dev->cmd; > - int status = cmd->status; > + int rc = 0; > > - struct sk_buff *req = cmd->req; > - struct sk_buff *resp = cmd->resp; > + if (cmd) { if (!cmd) { dev_dbg(dev->dev, "%s: cmd not set\n", __func__); return rc; } would make the rest of the code more readable. Cheers, Samuel.
Re: [PATCH 07/11] NFC: pn533: improve cmd queue handling
Hi Michael, On Thu, Apr 21, 2016 at 04:43:55PM +0200, Michael Thalmeier wrote: > Make sure cmd is set before a frame is passed to the transport layer for > sending. In addition pn533_send_async_complete checks if cmd is set before > accessing its members. > > Signed-off-by: Michael Thalmeier > --- > drivers/nfc/pn533/pn533.c | 54 > +-- > 1 file changed, 34 insertions(+), 20 deletions(-) > > diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c > index d9c5583..d1cc70a 100644 > --- a/drivers/nfc/pn533/pn533.c > +++ b/drivers/nfc/pn533/pn533.c > @@ -383,26 +383,36 @@ static void pn533_build_cmd_frame(struct pn533 *dev, u8 > cmd_code, > static int pn533_send_async_complete(struct pn533 *dev) > { > struct pn533_cmd *cmd = dev->cmd; > - int status = cmd->status; > + int rc = 0; > > - struct sk_buff *req = cmd->req; > - struct sk_buff *resp = cmd->resp; > + if (cmd) { if (!cmd) { dev_dbg(dev->dev, "%s: cmd not set\n", __func__); return rc; } would make the rest of the code more readable. Cheers, Samuel.
[PATCH 07/11] NFC: pn533: improve cmd queue handling
Make sure cmd is set before a frame is passed to the transport layer for sending. In addition pn533_send_async_complete checks if cmd is set before accessing its members. Signed-off-by: Michael Thalmeier--- drivers/nfc/pn533/pn533.c | 54 +-- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c index d9c5583..d1cc70a 100644 --- a/drivers/nfc/pn533/pn533.c +++ b/drivers/nfc/pn533/pn533.c @@ -383,26 +383,36 @@ static void pn533_build_cmd_frame(struct pn533 *dev, u8 cmd_code, static int pn533_send_async_complete(struct pn533 *dev) { struct pn533_cmd *cmd = dev->cmd; - int status = cmd->status; + int rc = 0; - struct sk_buff *req = cmd->req; - struct sk_buff *resp = cmd->resp; + if (cmd) { + int status = cmd->status; - int rc; + struct sk_buff *req = cmd->req; + struct sk_buff *resp = cmd->resp; - dev_kfree_skb(req); + dev_kfree_skb(req); - if (status < 0) { - rc = cmd->complete_cb(dev, cmd->complete_cb_context, - ERR_PTR(status)); - dev_kfree_skb(resp); - goto done; - } + if (status < 0) { + rc = cmd->complete_cb(dev, cmd->complete_cb_context, + ERR_PTR(status)); + dev_kfree_skb(resp); + goto done; + } + + /* when no response is set we got interrupted */ + if (!resp) + resp = ERR_PTR(-EINTR); - skb_pull(resp, dev->ops->rx_header_len); - skb_trim(resp, resp->len - dev->ops->rx_tail_len); + if (!IS_ERR(resp)) { + skb_pull(resp, dev->ops->rx_header_len); + skb_trim(resp, resp->len - dev->ops->rx_tail_len); + } - rc = cmd->complete_cb(dev, cmd->complete_cb_context, resp); + rc = cmd->complete_cb(dev, cmd->complete_cb_context, resp); + } else { + dev_dbg(dev->dev, "%s: cmd not set\n", __func__); + } done: kfree(cmd); @@ -434,12 +444,14 @@ static int __pn533_send_async(struct pn533 *dev, u8 cmd_code, mutex_lock(>cmd_lock); if (!dev->cmd_pending) { + dev->cmd = cmd; rc = dev->phy_ops->send_frame(dev, req); - if (rc) + if (rc) { + dev->cmd = NULL; goto error; + } dev->cmd_pending = 1; - dev->cmd = cmd; goto unlock; } @@ -511,11 +523,12 @@ static int pn533_send_cmd_direct_async(struct pn533 *dev, u8 cmd_code, pn533_build_cmd_frame(dev, cmd_code, req); + dev->cmd = cmd; rc = dev->phy_ops->send_frame(dev, req); - if (rc < 0) + if (rc < 0) { + dev->cmd = NULL; kfree(cmd); - else - dev->cmd = cmd; + } return rc; } @@ -550,14 +563,15 @@ static void pn533_wq_cmd(struct work_struct *work) mutex_unlock(>cmd_lock); + dev->cmd = cmd; rc = dev->phy_ops->send_frame(dev, cmd->req); if (rc < 0) { + dev->cmd = NULL; dev_kfree_skb(cmd->req); kfree(cmd); return; } - dev->cmd = cmd; } struct pn533_sync_cmd_response { -- 2.5.5
[PATCH 07/11] NFC: pn533: improve cmd queue handling
Make sure cmd is set before a frame is passed to the transport layer for sending. In addition pn533_send_async_complete checks if cmd is set before accessing its members. Signed-off-by: Michael Thalmeier --- drivers/nfc/pn533/pn533.c | 54 +-- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c index d9c5583..d1cc70a 100644 --- a/drivers/nfc/pn533/pn533.c +++ b/drivers/nfc/pn533/pn533.c @@ -383,26 +383,36 @@ static void pn533_build_cmd_frame(struct pn533 *dev, u8 cmd_code, static int pn533_send_async_complete(struct pn533 *dev) { struct pn533_cmd *cmd = dev->cmd; - int status = cmd->status; + int rc = 0; - struct sk_buff *req = cmd->req; - struct sk_buff *resp = cmd->resp; + if (cmd) { + int status = cmd->status; - int rc; + struct sk_buff *req = cmd->req; + struct sk_buff *resp = cmd->resp; - dev_kfree_skb(req); + dev_kfree_skb(req); - if (status < 0) { - rc = cmd->complete_cb(dev, cmd->complete_cb_context, - ERR_PTR(status)); - dev_kfree_skb(resp); - goto done; - } + if (status < 0) { + rc = cmd->complete_cb(dev, cmd->complete_cb_context, + ERR_PTR(status)); + dev_kfree_skb(resp); + goto done; + } + + /* when no response is set we got interrupted */ + if (!resp) + resp = ERR_PTR(-EINTR); - skb_pull(resp, dev->ops->rx_header_len); - skb_trim(resp, resp->len - dev->ops->rx_tail_len); + if (!IS_ERR(resp)) { + skb_pull(resp, dev->ops->rx_header_len); + skb_trim(resp, resp->len - dev->ops->rx_tail_len); + } - rc = cmd->complete_cb(dev, cmd->complete_cb_context, resp); + rc = cmd->complete_cb(dev, cmd->complete_cb_context, resp); + } else { + dev_dbg(dev->dev, "%s: cmd not set\n", __func__); + } done: kfree(cmd); @@ -434,12 +444,14 @@ static int __pn533_send_async(struct pn533 *dev, u8 cmd_code, mutex_lock(>cmd_lock); if (!dev->cmd_pending) { + dev->cmd = cmd; rc = dev->phy_ops->send_frame(dev, req); - if (rc) + if (rc) { + dev->cmd = NULL; goto error; + } dev->cmd_pending = 1; - dev->cmd = cmd; goto unlock; } @@ -511,11 +523,12 @@ static int pn533_send_cmd_direct_async(struct pn533 *dev, u8 cmd_code, pn533_build_cmd_frame(dev, cmd_code, req); + dev->cmd = cmd; rc = dev->phy_ops->send_frame(dev, req); - if (rc < 0) + if (rc < 0) { + dev->cmd = NULL; kfree(cmd); - else - dev->cmd = cmd; + } return rc; } @@ -550,14 +563,15 @@ static void pn533_wq_cmd(struct work_struct *work) mutex_unlock(>cmd_lock); + dev->cmd = cmd; rc = dev->phy_ops->send_frame(dev, cmd->req); if (rc < 0) { + dev->cmd = NULL; dev_kfree_skb(cmd->req); kfree(cmd); return; } - dev->cmd = cmd; } struct pn533_sync_cmd_response { -- 2.5.5