Skip to content

Commit

Permalink
Input: max8925_onkey - avoid accessing input device too early
Browse files Browse the repository at this point in the history
Input device must be allocated (but not necessarily registered) before
requesting IRQs, otherwise there is a chance that IRQ handler fires and
tries to reference not yet allocated input device.

Also it makes sense to store relative IRQ numbers in max8925_onkey_info
structure as they are needed in suspend/resume which we expect to be
called more often than probe and remove.

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
  • Loading branch information
Dmitry Torokhov committed Mar 6, 2012
1 parent adab30d commit 104a5f3
Showing 1 changed file with 44 additions and 47 deletions.
91 changes: 44 additions & 47 deletions drivers/input/misc/max8925_onkey.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* max8925_onkey.c - MAX8925 ONKEY driver
* MAX8925 ONKEY driver
*
* Copyright (C) 2009 Marvell International Ltd.
* Haojian Zhuang <haojian.zhuang@marvell.com>
Expand Down Expand Up @@ -35,7 +35,7 @@ struct max8925_onkey_info {
struct input_dev *idev;
struct i2c_client *i2c;
struct device *dev;
int irq[2];
unsigned int irq[2];
};

/*
Expand All @@ -46,17 +46,14 @@ struct max8925_onkey_info {
static irqreturn_t max8925_onkey_handler(int irq, void *data)
{
struct max8925_onkey_info *info = data;
int ret, event;

ret = max8925_reg_read(info->i2c, MAX8925_ON_OFF_STATUS);
if (ret & SW_INPUT)
event = 1;
else
event = 0;
input_report_key(info->idev, KEY_POWER, event);
int state;

state = max8925_reg_read(info->i2c, MAX8925_ON_OFF_STATUS);

input_report_key(info->idev, KEY_POWER, state & SW_INPUT);
input_sync(info->idev);

dev_dbg(info->dev, "onkey event:%d\n", event);
dev_dbg(info->dev, "onkey state:%d\n", state);

/* Enable hardreset to halt if system isn't shutdown on time */
max8925_set_bits(info->i2c, MAX8925_SYSENSEL,
Expand All @@ -69,25 +66,40 @@ static int __devinit max8925_onkey_probe(struct platform_device *pdev)
{
struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
struct max8925_onkey_info *info;
struct input_dev *input;
int irq[2], error;

irq[0] = platform_get_irq(pdev, 0);
if (irq[0] < 0) {
dev_err(&pdev->dev, "No IRQ resource!\n");
return -EINVAL;
}

irq[1] = platform_get_irq(pdev, 1);
if (irq[1] < 0) {
dev_err(&pdev->dev, "No IRQ resource!\n");
return -EINVAL;
}

info = kzalloc(sizeof(struct max8925_onkey_info), GFP_KERNEL);
if (!info)
return -ENOMEM;
input = input_allocate_device();
if (!info || !input) {
error = -ENOMEM;
goto err_free_mem;
}

info->idev = input;
info->i2c = chip->i2c;
info->dev = &pdev->dev;
info->irq[0] = irq[0];
info->irq[1] = irq[1];

input->name = "max8925_on";
input->phys = "max8925_on/input0";
input->id.bustype = BUS_I2C;
input->dev.parent = &pdev->dev;
input_set_capability(input, EV_KEY, KEY_POWER);

irq[0] += chip->irq_base;
irq[1] += chip->irq_base;

Expand All @@ -96,61 +108,46 @@ static int __devinit max8925_onkey_probe(struct platform_device *pdev)
if (error < 0) {
dev_err(chip->dev, "Failed to request IRQ: #%d: %d\n",
irq[0], error);
goto out;
goto err_free_mem;
}

error = request_threaded_irq(irq[1], NULL, max8925_onkey_handler,
IRQF_ONESHOT, "onkey-up", info);
if (error < 0) {
dev_err(chip->dev, "Failed to request IRQ: #%d: %d\n",
irq[1], error);
goto out_irq;
goto err_free_irq0;
}

info->idev = input_allocate_device();
if (!info->idev) {
dev_err(chip->dev, "Failed to allocate input dev\n");
error = -ENOMEM;
goto out_input;
}

info->idev->name = "max8925_on";
info->idev->phys = "max8925_on/input0";
info->idev->id.bustype = BUS_I2C;
info->idev->dev.parent = &pdev->dev;
info->irq[0] = irq[0];
info->irq[1] = irq[1];
info->idev->evbit[0] = BIT_MASK(EV_KEY);
info->idev->keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER);

device_init_wakeup(&pdev->dev, 1);

error = input_register_device(info->idev);
if (error) {
dev_err(chip->dev, "Can't register input device: %d\n", error);
goto out_reg;
goto err_free_irq1;
}

platform_set_drvdata(pdev, info);
device_init_wakeup(&pdev->dev, 1);

return 0;

out_reg:
input_free_device(info->idev);
out_input:
free_irq(info->irq[1], info);
out_irq:
free_irq(info->irq[0], info);
out:
err_free_irq1:
free_irq(irq[1], info);
err_free_irq0:
free_irq(irq[0], info);
err_free_mem:
input_free_device(input);
kfree(info);

return error;
}

static int __devexit max8925_onkey_remove(struct platform_device *pdev)
{
struct max8925_onkey_info *info = platform_get_drvdata(pdev);
struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);

free_irq(info->irq[0], info);
free_irq(info->irq[1], info);
free_irq(info->irq[0] + chip->irq_base, info);
free_irq(info->irq[1] + chip->irq_base, info);
input_unregister_device(info->idev);
kfree(info);

Expand All @@ -167,8 +164,8 @@ static int max8925_onkey_suspend(struct device *dev)
struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);

if (device_may_wakeup(dev)) {
chip->wakeup_flag |= 1 << (info->irq[0] - chip->irq_base);
chip->wakeup_flag |= 1 << (info->irq[1] - chip->irq_base);
chip->wakeup_flag |= 1 << info->irq[0];
chip->wakeup_flag |= 1 << info->irq[1];
}

return 0;
Expand All @@ -181,8 +178,8 @@ static int max8925_onkey_resume(struct device *dev)
struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);

if (device_may_wakeup(dev)) {
chip->wakeup_flag &= ~(1 << (info->irq[0] - chip->irq_base));
chip->wakeup_flag &= ~(1 << (info->irq[1] - chip->irq_base));
chip->wakeup_flag &= ~(1 << info->irq[0]);
chip->wakeup_flag &= ~(1 << info->irq[1]);
}

return 0;
Expand Down

0 comments on commit 104a5f3

Please sign in to comment.