Re: [PATCH 07/07] ONKEY: OnKey module for DA9052/53 PMIC v1

Mark Brown
2012-01-17T19:55:33Z

On Tue, Jan 17, 2012 at 06:59:11PM +0530, Ashish Jangam wrote:

> + ret = da9052_reg_read(onkey->da9052, DA9052_EVENT_B_REG);
> + if (ret < 0) {
> + dev_err(onkey->da9052->dev,
> + "da9052_onkey_report_event da9052_reg_read error %d\n",
> + ret);
> + ret = 1;
> + } else {
> + ret = ret & DA9052_EVENTB_ENONKEY;
> + input_report_key(onkey->input, KEY_POWER, ret);
> + input_sync(onkey->input);
> + }
> +
> + if (ret)
> + schedule_delayed_work(&onkey->work, msecs_to_jiffies(50));

Why not just schedule the work directly? The use of ret took a bit of
thinking about to follow.

> +
> +static int __devinit da9052_onkey_probe(struct platform_device *pdev)
> +{
> + struct da9052_onkey *onkey;
> + int error;
> +
> + onkey = kzalloc(sizeof(*onkey), GFP_KERNEL);
> + if (!onkey) {
> + dev_err(&pdev->dev, "Failed to allocate memory\n");
> + return -ENOMEM;
> + }

devm_kzalloc()

> + onkey->irq = platform_get_irq_byname(pdev, "ONKEY");
> + if (onkey->irq < 0) {
> + error = -ENXIO;

platform_get_irq_byname() returned an error code, you should
normallypass it back.

> + error = request_threaded_irq(onkey->da9052->irq_base + onkey->irq, NULL,
> + da9052_onkey_irq,

This looks buggy, the resource should have the IRQ you need directly in
it. The MFD core can do this for the chip core driver when it registers
children.

> +static int __init da9052_onkey_init(void)
> +{
> + return platform_driver_register(&da9052_onkey_driver);
> +}
> +module_init(da9052_onkey_init);
> +
> +static void __exit da9052_onkey_exit(void)
> +{
> + platform_driver_unregister(&da9052_onkey_driver);
> +}
> +module_exit(da9052_onkey_exit);

Use module_platform_driver.