Re: [PATCH 2/2] add ad714x input driver for button/scrollwhell/slider/touchpad

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

Hi,

On Tue, Sep 01, 2009 at 11:55:30AM +0800, Barry Song wrote:
> ad7142/ad7147 are Programmable Controllers for Capacitance Touch Sensors.
> The chips don't restrict the specific usage, and it can be used as button/
> slider/scrollwheel/touchpad etc. depending on the hardware connection.
> One special target board can include one or several these components.
> 
> The driver is independent of special boards. It gets the components
> layout information from the platform_data, registers related devices,
> fullfills the algorithms and state machines for these components and
> report related input events to up level.
> 

There were already quite a few comments, I will just add:

> +			if (h_state) {
> +				slider_cal_sensor_val(ad714x, idx);
> +				slider_cal_highest_stage(ad714x, idx);
> +				slider_cal_abs_pos(ad714x, idx);
> +				slider_cal_flt_pos(ad714x, idx);
> +
> +				input_report_abs(sw->input, ABS_X, sw->flt_pos);
> +				input_report_abs(sw->input, ABS_PRESSURE, 1);

The driver does not seem to report true pressure so please don't fake
ABS_PRESSURE. You probably want to return BTN_TOUCH here,

Also, what about vertical sliders? I also wonder if slider 

> +fail_irq:
> +fail_alloc_reg:
> +	for (i = 0; i < reg_idx; i++)
> +		input_unregister_device(input[i]);
> +	for (i = 0; i < alloc_idx; i++)
> +		input_free_device(input[i]);

No callls to input_free_device after input_unregister_device.

-- 
Dmitry