Eric Miao <eric.y.miao@xxxxxxxxx> writes:
Hi Eric, some minor cosmetic points ...
<snip>
> +static void activate_all_cols(struct matrix_keypad_platform_data
> *pdata, int on)
That function declaration spacing looks suspicious, looks like my mailer (or
yours) ate a bit out of it, and patch doesn't accept to apply it.
<snip>
+static void activate_col(struct matrix_keypad_platform_data *pdata,
+ int col, int on)
+{
+ __activate_col(pdata, col, on);
+
+ if (on && pdata->col_scan_delay_us)
+ udelay(pdata->col_scan_delay_us);
+}
That function is called is the worst case 16 times in a row (if I understood
correctly matrix_keypad_scan()). If delay is 100us, that makes 1.6ms for a
keystroke with all kernel blocked (udelay).
I didn't follow the discussion before, so maybe that's the only way. Still,
that's too bad ...
<snip>
> +/*
> + * This gets the keys from keyboard and reports it to input subsystem
> + */
> +static void matrix_keypad_scan(struct work_struct *work)
> +{
> + struct matrix_keypad *keypad =
> + container_of(work, struct matrix_keypad, work.work);
> + struct matrix_keypad_platform_data *pdata = keypad->pdata;
> + uint32_t new_state[MATRIX_MAX_COLS];
> + int row, col;
> +
> + /* de-activate all columns for scanning */
> + activate_all_cols(pdata, 0);
> +
> + memset(new_state, 0, sizeof(new_state));
> +
> + /* assert each column and read the row status out */
> + for (col = 0; col < pdata->num_col_gpios; col++) {
> +
> + activate_col(pdata, col, 1);
> +
> + for (row = 0; row < pdata->num_row_gpios; row++)
> + new_state[col] |= row_asserted(pdata, row) ?
> + (1 << row) : 0;
> + activate_col(pdata, col, 0);
> + }
> +
> + for (col = 0; col < pdata->num_col_gpios; col++) {
> + uint32_t bits_changed;
> +
Nitpicking: extra space on that line.
<snip>
Cheers.
--
Robert
--
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
