Re: [PATCH 1/4] [INPUT][KEYBOARD] Samsung keypad driver support

[Jae hoon Chung <jh80.chung@gmail.com>]

Hi...

I have some question to you.

> +static int s3c_keypad_scan(struct s3c_keypad *keypad, u32 *keymask_low,
> +                               u32 *keymask_high)
> +{
> +       struct s3c_platform_keypad *pdata = keypad->pdata;
> +       int i, j = 0;
> +       u32 cval, rval, cfg;
> +
> +       for (i = 0; i < pdata->nr_cols; i++) {
> +               cval = readl(keypad->regs + S3C_KEYIFCOL);
> +               cval |= S3C_KEYIF_COL_DMASK;
> +               cval &= ~(1 << i);
> +               writel(cval, keypad->regs + S3C_KEYIFCOL);
> +               udelay(pdata->delay);
> +
> +               rval = ~(readl(keypad->regs + S3C_KEYIFROW)) &
> +                       S3C_KEYIF_ROW_DMASK;
> +
> +               if ((i * pdata->nr_rows) < pdata->max_masks)
> +                       *keymask_low |= (rval << (i * pdata->nr_rows));
> +               else {
> +                       *keymask_high |= (rval << (j * pdata->nr_rows));
> +                       j++;
> +               }
> +       }
> +
> +       cfg = readl(keypad->regs + S3C_KEYIFCOL);
> +       cfg &= ~S3C_KEYIF_COL_MASK_ALL;
> +       writel(cfg, keypad->regs + S3C_KEYIFCOL);
> +
> +       return 0;
> +}
> +
> +static void s3c_keypad_timer_handler(unsigned long data)
> +{
> +       struct s3c_keypad *keypad = (struct s3c_keypad *)data;
> +       struct s3c_platform_keypad *pdata = keypad->pdata;
> +       struct input_dev *input = keypad->dev;
> +       u32 keymask_low = 0, keymask_high = 0;
> +       u32 press_mask_low, press_mask_high;
> +       u32 release_mask_low, release_mask_high, code, cfg;
> +       int i;
> +
> +       s3c_keypad_scan(keypad, &keymask_low, &keymask_high);
> +
> +       if (keymask_low != keypad->prevmask_low) {
> +               press_mask_low = ((keymask_low ^ keypad->prevmask_low) &
> +                                       keymask_low);
> +               release_mask_low = ((keymask_low ^ keypad->prevmask_low) &
> +                                       keypad->prevmask_low);
> +
> +               i = 0;
> +               while (press_mask_low) {
> +                       if (press_mask_low & 1) {
> +                               code = keypad->keycodes[i];
> +                               input_report_key(input, code, 1);
> +                               dev_dbg(&input->dev, "low pressed: %d\n", i);
> +                       }
> +                       press_mask_low >>= 1;
> +                       i++;
> +               }
> +
> +               i = 0;
> +               while (release_mask_low) {
> +                       if (release_mask_low & 1) {
> +                               code = keypad->keycodes[i];
> +                               input_report_key(input, code, 0);
> +                               dev_dbg(&input->dev, "low released: %d\n", i);
> +                       }
> +                       release_mask_low >>= 1;
> +                       i++;
> +               }
> +               keypad->prevmask_low = keymask_low;
> +       }
> +
> +       if (keymask_high != keypad->prevmask_high) {
> +               press_mask_high = ((keymask_high ^ keypad->prevmask_high) &
> +                                       keymask_high);
> +               release_mask_high = ((keymask_high ^ keypad->prevmask_high) &
> +                                       keypad->prevmask_high);
> +
> +               i = 0;
> +               while (press_mask_high) {
> +                       if (press_mask_high & 1) {
> +                               code = keypad->keycodes[i + pdata->max_masks];
> +                               input_report_key(input, code, 1);
> +                               dev_dbg(&input->dev, "high pressed: %d %d\n",
> +                                       keypad->keycodes[i + pdata->max_masks],
> +                                       i);
> +                       }
> +                       press_mask_high >>= 1;
> +                       i++;
> +               }
> +
> +               i = 0;
> +               while (release_mask_high) {
> +                       if (release_mask_high & 1) {
> +                               code = keypad->keycodes[i + pdata->max_masks];
> +                               input_report_key(input, code, 0);
> +                               dev_dbg(&input->dev, "high released: %d\n",
> +                                       keypad->keycodes[i + pdata->max_masks]);
> +                       }
> +                       release_mask_high >>= 1;
> +                       i++;
> +               }
> +               keypad->prevmask_high = keymask_high;
> +       }
> +
> +       if (keymask_low | keymask_high) {
> +               mod_timer(&keypad->timer, jiffies + HZ / 10);
> +       } else {
> +               cfg = readl(keypad->regs + S3C_KEYIFCON);
> +               cfg &= ~S3C_KEYIF_CON_MASK_ALL;
> +               cfg |= (S3C_KEYIF_INT_F_EN | S3C_KEYIF_INT_R_EN |
> +                               S3C_KEYIF_DF_EN | S3C_KEYIF_FC_EN);
> +               writel(cfg, keypad->regs + S3C_KEYIFCON);
> +       }
> +}

> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


I understood  your code that used the keymask_low & keymask_high.
why did you use to distinguish  a keymask_low & keymask_high?
In your code, that variable saved all value of row_val , then you are
searching the value of pressed row_val...
i think it is unnecessary.