Re: [PATCH] input: Add keypad support for w90p910 evb

[Trilok Soni <soni.trilok@gmail.com>]

Hi Wan ZongShun,

>>> +static void w90p910_keypad_scan_matrix(struct w90p910_keypad *keypad,
>>> +                                                       unsigned int status)
>>> +{
>>> +       unsigned int row, col, val;
>>> +
>>> +       row = KGET_RAW(status);
>>> +       col = KGET_COLUMN(status);
>>> +
>>> +       val = lookup_matrix_keycode(keypad, row, col);
>>> +
>>> +       input_report_key(keypad->input_dev, val, 1);
>>> +
>>> +       input_sync(keypad->input_dev);
>>> +
>>> +       input_report_key(keypad->input_dev, val, 0);
>>
>> So, we don't get interrupt on key release?
>
> In w90p910,there is no way to identify whether the key was released or not,so I have to put above code here to report the key has been released.
>

Ok.

>>> +
>>> +static void w90p910_keypad_close(struct input_dev *dev)
>>> +{
>>> +       struct w90p910_keypad *keypad = input_get_drvdata(dev);
>>> +
>>> +       /* Disable clock unit */
>>> +       clk_disable(keypad->clk);
>>
>> So this controller doesn't support putting controller FSM into IDLE
>> state by writing some bits into it or is it doing auto-sleep?
>
> What you said not be supporting in w90p910,
> When to disable this clk,I think the controller will be power down.

Ok.

>>> +
>>> +       /* set multi-function pin for w90p910 kpi. */
>>> +       mfp_set_groupi(&pdev->dev);
>>> +
>>
>> Could you please point me to implementation of this API? I believe
>> this is pin-multiplexing code.
>
> Sure, it is a muti-function pin API, which was defined in /mach-w90x900/mfp-w90p910.c.
> What's wrong?

I prefer pin-multiplexing being done from board-xxx.c files, as
pin-multiplexing differs for every board design. I will look at this
API.

>
>>
>>> +       input_dev->name = pdev->name;
>>> +       input_dev->id.bustype = BUS_HOST;
>>> +       input_dev->open = w90p910_keypad_open;
>>> +       input_dev->close = w90p910_keypad_close;
>>> +       input_dev->dev.parent = &pdev->dev;
>>> +
>>> +       keypad->input_dev = input_dev;
>>> +       input_set_drvdata(input_dev, keypad);
>>> +
>>> +       input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
>>
>> EV_REP can be done as per platform data, but not a problem to enable
>> by default if keypad controller itself is not supporting it
>
>
> This is not suitable to w90p910?
> Maybe I have a mistake to understand EV_REP, how to do for me here?.

No problem, keep EV_REP as is.

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
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