Re: [PATCH 1/3] Input: DaVinci Key Scan Driver

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Fri, Sep 25, 2009 at 08:19:37AM -0600, Miguel Aguilar wrote:
> Dmitry Torokhov wrote:
>>
>>> +
>>> +	davinci_ks = kzalloc(sizeof(struct davinci_ks) +
>>> +		sizeof(unsigned short) * pdata->keymapsize, GFP_KERNEL);
>>> +	if(!davinci_ks) {
>>> +		dev_dbg(dev, "could not allocate memory for private data\n");
>>> +		return -ENOMEM;
>>> +	}
>>> +
>>> +	if (!pdata->keymap) {
>>> +		dev_dbg(dev, "no keymap from pdata\n");
>>
>> You are leaking davinci_ks here. Maybe you should check pdata->keymap
>> first.
> [MA] Do you mean check pdata->keymap before davinci_ks allocation?

Yes.

>>> +
>>> +	ret = input_register_device(davinci_ks->input);
>>> +	if (ret < 0) {
>>> +		dev_err(dev, "unable to register DaVinci keyscan device\n");
>>> +		goto fail4;
>>> +	}
>>> +
>>> +	ret = request_irq(davinci_ks->irq, davinci_ks_interrupt, IRQF_DISABLED,
>>> +                         "davinci_keyscan", davinci_ks);
>>> +	if (ret < 0) {
>>> +		dev_err(dev, "unable to register DaVinci keyscan Interrupt\n");
>>> +		goto fail5;
>>> +	}
>>
>> FWIW you may request IRQ before registering the device - as soon as it
>> is alloctaed it can survive events going through it.
> [MA] Ok. request_irq before input_register_device.
>>

You don't have to change it, it was more of FYI item because sometimes
registering IRQ first simplifies error unwinding.

-- 
Dmitry