NFC: pn533: change order operations in dev registation
authorAndrey Rusalin <arusalin@dev.rtsoft.ru>
Wed, 28 Dec 2016 17:10:59 +0000 (20:10 +0300)
committerSamuel Ortiz <sameo@linux.intel.com>
Sat, 1 Apr 2017 23:06:23 +0000 (01:06 +0200)
Sometimes during probing and registration of pn533_i2c
NULL pointer dereference happens.
Reproduced in cycle of inserting and removing pn533_i2c
and pn533 modules.

Backtrace:
[<8004205c>] (__queue_work) from [<80042324>] (queue_work_on+0x50/0x5c)
r10:acdc7c80 r9:8006b330 r8:ac0dfb40 r7:ac50c600 r6:00000004 r5:acbbee40 r4:600f0113
[<800422d4>] (queue_work_on) from [<7f7d5b6c>] (pn533_recv_frame+0x158/0x1fc [pn533])
r7:ffffff87 r6:00000000 r5:acbbee40 r4:acbbee00
[<7f7d5a14>] (pn533_recv_frame [pn533]) from [<7f7df4b8>] (pn533_i2c_irq_thread_fn+0x184/0x)
r6:acb2a000 r5:00000000 r4:acdc7b90
[<7f7df334>] (pn533_i2c_irq_thread_fn [pn533_i2c]) from [<8006b354>] (irq_thread_fn+0x24/0x)
r7:00000000 r6:accde000 r5:ac0dfb40 r4:acdc7c80
...

Seems there is some race condition due registration of
irq handler until all data stuctures that could be needed
are ready. So I re-ordered some ops. After this, problem has gone.

Changes in USB part was not tested, but it should not break
anything.

Signed-off-by: Andrey Rusalin <arusalin@dev.rtsoft.ru>
Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
drivers/nfc/pn533/i2c.c
drivers/nfc/pn533/pn533.c
drivers/nfc/pn533/pn533.h
drivers/nfc/pn533/usb.c

index 11d78b43cf76c658989504d0fad328aca89851f7..2c2fb9cfe10ae8160ca767df545ddd374cb7795d 100644 (file)
@@ -206,14 +206,6 @@ static int pn533_i2c_probe(struct i2c_client *client,
        phy->i2c_dev = client;
        i2c_set_clientdata(client, phy);
 
-       r = request_threaded_irq(client->irq, NULL, pn533_i2c_irq_thread_fn,
-                                IRQF_TRIGGER_FALLING |
-                                IRQF_SHARED | IRQF_ONESHOT,
-                                PN533_I2C_DRIVER_NAME, phy);
-
-       if (r < 0)
-               nfc_err(&client->dev, "Unable to register IRQ handler\n");
-
        priv = pn533_register_device(PN533_DEVICE_PN532,
                                     PN533_NO_TYPE_B_PROTOCOLS,
                                     PN533_PROTO_REQ_ACK_RESP,
@@ -223,16 +215,32 @@ static int pn533_i2c_probe(struct i2c_client *client,
 
        if (IS_ERR(priv)) {
                r = PTR_ERR(priv);
-               goto err_register;
+               return r;
        }
 
        phy->priv = priv;
 
+       r = request_threaded_irq(client->irq, NULL, pn533_i2c_irq_thread_fn,
+                               IRQF_TRIGGER_FALLING |
+                               IRQF_SHARED | IRQF_ONESHOT,
+                               PN533_I2C_DRIVER_NAME, phy);
+       if (r < 0) {
+               nfc_err(&client->dev, "Unable to register IRQ handler\n");
+               goto irq_rqst_err;
+       }
+
+       r = pn533_finalize_setup(priv);
+       if (r)
+               goto fn_setup_err;
+
        return 0;
 
-err_register:
+fn_setup_err:
        free_irq(client->irq, phy);
 
+irq_rqst_err:
+       pn533_unregister_device(phy->priv);
+
        return r;
 }
 
index 712aa67e1770b5b2d867b7ad965bbbeae2e835ad..65bbaa5fcdda6c2a1dae0e415b4cc2c7b2029eac 100644 (file)
@@ -2570,6 +2570,31 @@ static int pn533_setup(struct pn533 *dev)
        return 0;
 }
 
+int pn533_finalize_setup(struct pn533 *dev)
+{
+
+       struct pn533_fw_version fw_ver;
+       int rc;
+
+       memset(&fw_ver, 0, sizeof(fw_ver));
+
+       rc = pn533_get_firmware_version(dev, &fw_ver);
+       if (rc) {
+               nfc_err(dev->dev, "Unable to get FW version\n");
+               return rc;
+       }
+
+       nfc_info(dev->dev, "NXP PN5%02X firmware ver %d.%d now attached\n",
+               fw_ver.ic, fw_ver.ver, fw_ver.rev);
+
+       rc = pn533_setup(dev);
+       if (rc)
+               return rc;
+
+       return 0;
+}
+EXPORT_SYMBOL_GPL(pn533_finalize_setup);
+
 struct pn533 *pn533_register_device(u32 device_type,
                                u32 protocols,
                                enum pn533_protocol_type protocol_type,
@@ -2579,7 +2604,6 @@ struct pn533 *pn533_register_device(u32 device_type,
                                struct device *dev,
                                struct device *parent)
 {
-       struct pn533_fw_version fw_ver;
        struct pn533 *priv;
        int rc = -ENOMEM;
 
@@ -2622,15 +2646,6 @@ struct pn533 *pn533_register_device(u32 device_type,
 
        INIT_LIST_HEAD(&priv->cmd_queue);
 
-       memset(&fw_ver, 0, sizeof(fw_ver));
-       rc = pn533_get_firmware_version(priv, &fw_ver);
-       if (rc < 0)
-               goto destroy_wq;
-
-       nfc_info(dev, "NXP PN5%02X firmware ver %d.%d now attached\n",
-                fw_ver.ic, fw_ver.ver, fw_ver.rev);
-
-
        priv->nfc_dev = nfc_allocate_device(&pn533_nfc_ops, protocols,
                                           priv->ops->tx_header_len +
                                           PN533_CMD_DATAEXCH_HEAD_LEN,
@@ -2647,15 +2662,8 @@ struct pn533 *pn533_register_device(u32 device_type,
        if (rc)
                goto free_nfc_dev;
 
-       rc = pn533_setup(priv);
-       if (rc)
-               goto unregister_nfc_dev;
-
        return priv;
 
-unregister_nfc_dev:
-       nfc_unregister_device(priv->nfc_dev);
-
 free_nfc_dev:
        nfc_free_device(priv->nfc_dev);
 
index 553c7d171fd1112b6c4906b566aa66a760bb81dc..88d569666c51fd3ce6fff3b800900bd26d303eb5 100644 (file)
@@ -231,6 +231,7 @@ struct pn533 *pn533_register_device(u32 device_type,
                                struct device *dev,
                                struct device *parent);
 
+int pn533_finalize_setup(struct pn533 *dev);
 void pn533_unregister_device(struct pn533 *priv);
 void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status);
 
index 33ed78be2750455c34fd51d3b69611f177b52374..000159ea9c5f1dee18e92a41c78f77cf4f836fb4 100644 (file)
@@ -543,6 +543,10 @@ static int pn533_usb_probe(struct usb_interface *interface,
 
        phy->priv = priv;
 
+       rc = pn533_finalize_setup(priv);
+       if (rc)
+               goto error;
+
        usb_set_intfdata(interface, phy);
 
        return 0;