Hi Holger,
I've got your keypad driver running on my i.MX21 board - I had a few
problems mainly when trying to use it as a module -comments below.
> --- linux.orig/arch/arm/mach-mx2/devices.c
> +++ linux/arch/arm/mach-mx2/devices.c
...
> +
> +#ifdef CONFIG_KEYBOARD_MXC
For module needs to be
#if defined(CONFIG_KEYBOARD_MXC) || defined(CONFIG_KEYBOARD_MXC_MODULE)
But I notice the other resource definitions in this file SDHC etc
aren't conditionally compiled at all - what is the policy on this?
> +static struct resource mxc_resource_keypad[] = {
> + [0] = {
> + .start = 0x10008000,
> + .end = 0x10008014,
Why not use KPP_BASE_ADDR?
> +++ linux/drivers/input/keyboard/mxc_keypad.c
> +static void mxc_keypad_scan(unsigned long data)
> +{
> + struct mxc_keypad *kp = (struct mxc_keypad *)data;
I changed this to directly take a struct mxc_keypad * rather than a
long with a cast
> + /* Quick-discharge by setting to totem-pole, */
> + /* then turn back to open-drain */
> + __raw_writew(kp->pdata->output_pins, kp->base + KPCR);
> + wmb();
> + udelay(2);
> + __raw_writew(kp->pdata->output_pins |
> + kp->pdata->input_pins, kp->base + KPCR);
> +
I think the first write should be input_pins (need to set the output
pins bits to zero in KPCR to switch to totem pole mode.
Where does the udelay(2) come from?
Also (more for my information - I'm a bit sketchy on this) why
__raw_writel + wmb() rather than plain writel ?
> + if (col_bit & kp->pdata->output_pins) {
> + __raw_writew(~col_bit, kp->base + KPDR);
> + wmb();
> + udelay(2);
> + reg = ~__raw_readw(kp->base + KPDR);
> + keystate_cur[snum] = reg & kp->pdata->input_pins;
> +
> + if (snum++ >= MAX_INPUTS)
I was getting bounces with this - increasing the udelay(2) to 10 fixed
it but I preferred to do :
if (col_bit & kp->pdata->output_pins) {
u16 last_reg = 0;
int debounce = 0;
__raw_writew(~col_bit, kp->base + KPDR);
wmb();
while (debounce < 3) {
udelay(1);
reg = ~__raw_readw(kp->base + KPDR);
if (reg == last_reg)
debounce++;
else
debounce = 0;
last_reg = reg;
}
keystate_cur[snum] = reg & kp->pdata->input_pins;
if (snum++ >= MAX_INPUTS)
On my keyboard this exits after 4-6 loops.
> +static irqreturn_t mxc_keypad_irq_handler(int irq, void *data)
> +{
> + struct mxc_keypad *kp = data;
> + u16 stat;
> +
...
> +
> + mxc_keypad_scan((unsigned long)kp);
> +
mxc_keypad_scan(kp); as mentionned above?
> +static int __devinit mxc_keypad_probe(struct platform_device *pdev)
> +{
...
> + error = request_irq(irq, mxc_keypad_irq_handler,
> + IRQF_DISABLED, pdev->name, kp);
>
..
> +failed_free_irq:
> + free_irq(irq, pdev);
This needs to be free_irq(irq, kp) to match the request_irq
> +static int __devexit mxc_keypad_remove(struct platform_device *pdev)
> +{
..
> + free_irq(kp->irq, pdev);
Idem - this caused module ulooad / reload to fail since the IRQ was
not freed and was seen as busy the second time;
Regards,
Martin
--
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
