On Tue, Jul 14, 2009 at 14:03, Dmitry Torokhov wrote:
> On Tue, Jul 14, 2009 at 01:33:46PM -0400, Mike Frysinger wrote:
>> +static int ad7142_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>
> __devinit
fixed
>> + rc = request_irq(client->irq, ad7142_interrupt,
>> + IRQF_TRIGGER_LOW, "ad7142_joystick", data);
>
> Let's use threaded IRQs, they are so nice for devices like this one.
i'll open a tracker item for someone to look at this ... Bryan has left our team
>> + disable_irq_nosync(client->irq);
>
> Why nosync here?
no idea -- changed to normal version
>> + /* Allocate and register AD7142 input device */
>> + data->input = input_allocate_device();
>> + if (!data->input) {
>> + dev_err(&client->dev, "Can't allocate input device\n");
>> + rc = -ENOMEM;
>> + goto fail_allocate;
>> + }
>
> Allocate the device before you request IRQ.. Even if you disabled it
> there is still a window where it can be raised without input device
> allocated.
fixed ... sure would be nice to have a IRQF_xxx flag to request an IRQ
in the disabled state
>> + input->evbit[0] = BIT_MASK(EV_KEY);
>> + input->keybit[BIT_WORD(BTN_BASE)] = BIT_MASK(BTN_BASE) |
>> + BIT_MASK(BTN_BASE2) |
>> + BIT_MASK(BTN_BASE3) |
>> + BIT_MASK(BTN_BASE4);
>> + input->keybit[BIT_WORD(KEY_UP)] |= BIT_MASK(KEY_UP) |
>> + BIT_MASK(KEY_DOWN) |
>> + BIT_MASK(KEY_LEFT) |
>> + BIT_MASK(KEY_RIGHT);
>> +
>
> I am really not sure why you call it a joystick since it does not report
> relative axes... Let's put it in misc and just another button device.
if the hardware doesnt actually support relative events, sure ...
otherwise we'll add support for it
>> +static int __exit ad7142_remove(struct i2c_client *client)
>
> __devexit()
fixed
>> + .remove = __exit_p(ad7142_remove),
>
> __devexit_p()
fixed
> Any PM support?
guess we'll add it
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
