NFC: pn533: change order operations in dev registation

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>
This commit is contained in:
Andrey Rusalin 2016-12-28 20:10:59 +03:00 committed by Samuel Ortiz
parent 5dd9c1bd61
commit 32ecc75ded
4 changed files with 48 additions and 27 deletions

View File

@ -206,14 +206,6 @@ static int pn533_i2c_probe(struct i2c_client *client,
phy->i2c_dev = client; phy->i2c_dev = client;
i2c_set_clientdata(client, phy); 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, priv = pn533_register_device(PN533_DEVICE_PN532,
PN533_NO_TYPE_B_PROTOCOLS, PN533_NO_TYPE_B_PROTOCOLS,
PN533_PROTO_REQ_ACK_RESP, PN533_PROTO_REQ_ACK_RESP,
@ -223,16 +215,32 @@ static int pn533_i2c_probe(struct i2c_client *client,
if (IS_ERR(priv)) { if (IS_ERR(priv)) {
r = PTR_ERR(priv); r = PTR_ERR(priv);
goto err_register; return r;
} }
phy->priv = priv; 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; return 0;
err_register: fn_setup_err:
free_irq(client->irq, phy); free_irq(client->irq, phy);
irq_rqst_err:
pn533_unregister_device(phy->priv);
return r; return r;
} }

View File

@ -2570,6 +2570,31 @@ static int pn533_setup(struct pn533 *dev)
return 0; 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, struct pn533 *pn533_register_device(u32 device_type,
u32 protocols, u32 protocols,
enum pn533_protocol_type protocol_type, enum pn533_protocol_type protocol_type,
@ -2579,7 +2604,6 @@ struct pn533 *pn533_register_device(u32 device_type,
struct device *dev, struct device *dev,
struct device *parent) struct device *parent)
{ {
struct pn533_fw_version fw_ver;
struct pn533 *priv; struct pn533 *priv;
int rc = -ENOMEM; int rc = -ENOMEM;
@ -2622,15 +2646,6 @@ struct pn533 *pn533_register_device(u32 device_type,
INIT_LIST_HEAD(&priv->cmd_queue); 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->nfc_dev = nfc_allocate_device(&pn533_nfc_ops, protocols,
priv->ops->tx_header_len + priv->ops->tx_header_len +
PN533_CMD_DATAEXCH_HEAD_LEN, PN533_CMD_DATAEXCH_HEAD_LEN,
@ -2647,15 +2662,8 @@ struct pn533 *pn533_register_device(u32 device_type,
if (rc) if (rc)
goto free_nfc_dev; goto free_nfc_dev;
rc = pn533_setup(priv);
if (rc)
goto unregister_nfc_dev;
return priv; return priv;
unregister_nfc_dev:
nfc_unregister_device(priv->nfc_dev);
free_nfc_dev: free_nfc_dev:
nfc_free_device(priv->nfc_dev); nfc_free_device(priv->nfc_dev);

View File

@ -231,6 +231,7 @@ struct pn533 *pn533_register_device(u32 device_type,
struct device *dev, struct device *dev,
struct device *parent); struct device *parent);
int pn533_finalize_setup(struct pn533 *dev);
void pn533_unregister_device(struct pn533 *priv); void pn533_unregister_device(struct pn533 *priv);
void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status); void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status);

View File

@ -543,6 +543,10 @@ static int pn533_usb_probe(struct usb_interface *interface,
phy->priv = priv; phy->priv = priv;
rc = pn533_finalize_setup(priv);
if (rc)
goto error;
usb_set_intfdata(interface, phy); usb_set_intfdata(interface, phy);
return 0; return 0;