Re: [PATCH v2] input/misc: new ADXL345/346 driver

["Hennerich, Michael" <Michael.Hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>]

Hi Dmitry,

>> +	if (int_stat & SINGLE_TAP) {
>> +		if (tap_stat & TAP_X_SRC)
>> +			adxl34x_report_key_single(ac->input,
>> +						  pdata->ev_code_tap_x);
>> +		if (tap_stat & TAP_Y_SRC)
>> +			adxl34x_report_key_single(ac->input,
>> +						  pdata->ev_code_tap_y);
>> +		if (tap_stat & TAP_Z_SRC)
>> +			adxl34x_report_key_single(ac->input,
>> +						  pdata->ev_code_tap_z);
>> +	}
>> +
>> +	if (int_stat & DOUBLE_TAP) {
>> +		if (tap_stat & TAP_X_SRC)
>> +			adxl34x_report_key_double(ac->input,
>> +						  pdata->ev_code_tap_x);
>> +		if (tap_stat & TAP_Y_SRC)
>> +			adxl34x_report_key_double(ac->input,
>> +						  pdata->ev_code_tap_y);
>> +		if (tap_stat & TAP_Z_SRC)
>> +			adxl34x_report_key_double(ac->input,
>> +						  pdata->ev_code_tap_z);
>> +	}
>
>Hmm.. this generates loads of syncs... how about having tap codes in an
>array and do:
>
>void send_key_events(ac, pdata, status, press)
>{
>	for (i = 0; i < num, i++) {
>		if (status & (1 << i))
>			input_report_key(ac->input,
>					 pdata->ev_code_tap[i], press);
>	}
>}
>
>void do_tap(ac, pdata, status)
>{
>	send_key_events(ac, pdata, int_stat, true);
>	input_sync(ac->input);
>	send_key_events(ac, pdata, int_stat, false);
>}
>
>And you'd do:
>
>	if (int_stat & (SINGLE_TAP|DOUBLE_TAP) {
>		do_tap(ac, pdata, status);
>
>		if (int_stat & DOUBLE_TAP)
>			do_tap(ac, pdata, status);
>	}
>
>?

That's a great idea - Thanks!

>> +void adxl34x_disable(struct adxl34x *ac)
>> +{
>> +	mutex_lock(&ac->mutex);
>> +	if (!ac->disabled && ac->opened) {
>> +		ac->disabled = 1;
>> +		cancel_work_sync(&ac->work);
>
>If you actually cancel the work before it runs you will leave interrupt
>disabled for good. You need to check the return core and re-enable
>interrupt if needed.

Good catch - will fix it.

-Michael