Hi Dmitry,
On Tue, Apr 28, 2015 at 02:55:40AM +0300, Dmitry Eremin-Solenikov wrote:
> As LoCoMo is switching to new device model, adapt keyboard driver to
> support new locomo core driver.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@xxxxxxxxx>
> ---
> drivers/input/keyboard/Kconfig | 1 -
> drivers/input/keyboard/locomokbd.c | 271 +++++++++++++++++++------------------
> 2 files changed, 143 insertions(+), 129 deletions(-)
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 106fbac..0a3d875 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -337,7 +337,6 @@ config KEYBOARD_LM8333
>
> config KEYBOARD_LOCOMO
> tristate "LoCoMo Keyboard Support"
> - depends on SHARP_LOCOMO
> help
> Say Y here if you are running Linux on a Sharp Zaurus Collie or Poodle based PDA
>
> diff --git a/drivers/input/keyboard/locomokbd.c b/drivers/input/keyboard/locomokbd.c
> index c94d610..eed0a94 100644
> --- a/drivers/input/keyboard/locomokbd.c
> +++ b/drivers/input/keyboard/locomokbd.c
> @@ -23,37 +23,37 @@
> *
> */
>
> -#include <linux/slab.h>
> -#include <linux/module.h>
> +#include <linux/delay.h>
> #include <linux/init.h>
> #include <linux/input.h>
> -#include <linux/delay.h>
> -#include <linux/device.h>
> #include <linux/interrupt.h>
> -#include <linux/ioport.h>
> -
> -#include <asm/hardware/locomo.h>
> -#include <asm/irq.h>
> -
> -MODULE_AUTHOR("John Lenz <lenz@xxxxxxxxxxx>");
> -MODULE_DESCRIPTION("LoCoMo keyboard driver");
> -MODULE_LICENSE("GPL");
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/mfd/locomo.h>
>
> -#define LOCOMOKBD_NUMKEYS 128
> +/* There is one minor difference between mappings on poodle and collie */
> +#include <asm/mach-types.h>
>
> #define KEY_ACTIVITY KEY_F16
> #define KEY_CONTACT KEY_F18
> #define KEY_CENTER KEY_F15
>
> +#define KB_ROWS 16
> +#define KB_COLS 8
> +#define LOCOMOKBD_NUMKEYS (KB_ROWS * KB_COLS)
> +#define SCANCODE(c, r) (((c)<<4) + (r) + 1)
> +
> static const unsigned char
> locomokbd_keycode[LOCOMOKBD_NUMKEYS] = {
> 0, KEY_ESC, KEY_ACTIVITY, 0, 0, 0, 0, 0, 0, 0, /* 0 - 9 */
> - 0, 0, 0, 0, 0, 0, 0, KEY_MENU, KEY_HOME, KEY_CONTACT, /* 10 - 19 */
> + 0, 0, 0, 0, 0, 0, 0, KEY_MENU, 0, KEY_CONTACT, /* 10 - 19 */
> 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 20 - 29 */
> 0, 0, 0, KEY_CENTER, 0, KEY_MAIL, 0, 0, 0, 0, /* 30 - 39 */
> 0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_RIGHT, /* 40 - 49 */
> KEY_UP, KEY_LEFT, 0, 0, KEY_P, 0, KEY_O, KEY_I, KEY_Y, KEY_T, /* 50 - 59 */
> - KEY_E, KEY_W, 0, 0, 0, 0, KEY_DOWN, KEY_ENTER, 0, 0, /* 60 - 69 */
> + KEY_E, KEY_W, 0, 0, 0, 0, KEY_DOWN, KEY_KPENTER, 0, 0, /* 60 - 69 */
> KEY_BACKSPACE, 0, KEY_L, KEY_U, KEY_H, KEY_R, KEY_D, KEY_Q, 0, 0, /* 70 - 79 */
> 0, 0, 0, 0, 0, 0, KEY_ENTER, KEY_RIGHTSHIFT, KEY_K, KEY_J, /* 80 - 89 */
> KEY_G, KEY_F, KEY_X, KEY_S, 0, 0, 0, 0, 0, 0, /* 90 - 99 */
> @@ -62,20 +62,14 @@ locomokbd_keycode[LOCOMOKBD_NUMKEYS] = {
> KEY_M, KEY_SPACE, KEY_V, KEY_APOSTROPHE, KEY_SLASH, 0, 0, 0 /* 120 - 128 */
> };
>
> -#define KB_ROWS 16
> -#define KB_COLS 8
> -#define KB_ROWMASK(r) (1 << (r))
> -#define SCANCODE(c,r) ( ((c)<<4) + (r) + 1 )
> -
> #define KB_DELAY 8
> -#define SCAN_INTERVAL (HZ/10)
>
> struct locomokbd {
> unsigned char keycode[LOCOMOKBD_NUMKEYS];
> struct input_dev *input;
> - char phys[32];
>
> - unsigned long base;
> + struct regmap *regmap;
> + int irq;
> spinlock_t lock;
>
> struct timer_list timer;
> @@ -84,37 +78,33 @@ struct locomokbd {
> };
>
> /* helper functions for reading the keyboard matrix */
> -static inline void locomokbd_charge_all(unsigned long membase)
> +static inline void locomokbd_charge_all(struct locomokbd *locomokbd)
> {
> - locomo_writel(0x00FF, membase + LOCOMO_KSC);
> + regmap_write(locomokbd->regmap, LOCOMO_KSC, 0x00ff);
> }
>
> -static inline void locomokbd_activate_all(unsigned long membase)
> +static inline void locomokbd_activate_all(struct locomokbd *locomokbd)
Drop "inline"s from the .c file please.
> {
> - unsigned long r;
> -
> - locomo_writel(0, membase + LOCOMO_KSC);
> - r = locomo_readl(membase + LOCOMO_KIC);
> - r &= 0xFEFF;
> - locomo_writel(r, membase + LOCOMO_KIC);
> + regmap_write(locomokbd->regmap, LOCOMO_KSC, 0);
> + regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x100, 0);
> }
>
> -static inline void locomokbd_activate_col(unsigned long membase, int col)
> +static inline void locomokbd_activate_col(struct locomokbd *locomokbd, int col)
> {
> unsigned short nset;
> unsigned short nbset;
>
> - nset = 0xFF & ~(1 << col);
> + nset = 0xFF & ~BIT(col);
> nbset = (nset << 8) + nset;
> - locomo_writel(nbset, membase + LOCOMO_KSC);
> + regmap_write(locomokbd->regmap, LOCOMO_KSC, nbset);
> }
>
> -static inline void locomokbd_reset_col(unsigned long membase, int col)
> +static inline void locomokbd_reset_col(struct locomokbd *locomokbd, int col)
> {
> unsigned short nbset;
>
> - nbset = ((0xFF & ~(1 << col)) << 8) + 0xFF;
> - locomo_writel(nbset, membase + LOCOMO_KSC);
> + nbset = ((0xFF & ~BIT(col)) << 8) + 0xFF;
> + regmap_write(locomokbd->regmap, LOCOMO_KSC, nbset);
> }
>
> /*
> @@ -129,24 +119,25 @@ static void locomokbd_scankeyboard(struct locomokbd *locomokbd)
> unsigned int row, col, rowd;
> unsigned long flags;
> unsigned int num_pressed;
> - unsigned long membase = locomokbd->base;
> + bool esc_pressed = false;
>
> spin_lock_irqsave(&locomokbd->lock, flags);
>
> - locomokbd_charge_all(membase);
> + locomokbd_charge_all(locomokbd);
>
> num_pressed = 0;
> for (col = 0; col < KB_COLS; col++) {
> -
> - locomokbd_activate_col(membase, col);
> + udelay(KB_DELAY);
> + locomokbd_activate_col(locomokbd, col);
> udelay(KB_DELAY);
>
> - rowd = ~locomo_readl(membase + LOCOMO_KIB);
> + regmap_read(locomokbd->regmap, LOCOMO_KIB, &rowd);
> + rowd = ~rowd;
> for (row = 0; row < KB_ROWS; row++) {
> unsigned int scancode, pressed, key;
>
> scancode = SCANCODE(col, row);
> - pressed = rowd & KB_ROWMASK(row);
> + pressed = rowd & BIT(row);
> key = locomokbd->keycode[scancode];
>
> input_report_key(locomokbd->input, key, pressed);
> @@ -158,29 +149,30 @@ static void locomokbd_scankeyboard(struct locomokbd *locomokbd)
> /* The "Cancel/ESC" key is labeled "On/Off" on
> * Collie and Poodle and should suspend the device
> * if it was pressed for more than a second. */
> - if (unlikely(key == KEY_ESC)) {
> - if (!time_after(jiffies,
> - locomokbd->suspend_jiffies + HZ))
> - continue;
> - if (locomokbd->count_cancel++
> - != (HZ/SCAN_INTERVAL + 1))
> - continue;
> - input_event(locomokbd->input, EV_PWR,
> - KEY_SUSPEND, 1);
> - locomokbd->suspend_jiffies = jiffies;
> - } else
> - locomokbd->count_cancel = 0;
> + if (unlikely(key == KEY_ESC))
> + esc_pressed = true;
> }
> - locomokbd_reset_col(membase, col);
> + locomokbd_reset_col(locomokbd, col);
> }
> - locomokbd_activate_all(membase);
> + locomokbd_activate_all(locomokbd);
>
> input_sync(locomokbd->input);
>
> /* if any keys are pressed, enable the timer */
> if (num_pressed)
> - mod_timer(&locomokbd->timer, jiffies + SCAN_INTERVAL);
> + mod_timer(&locomokbd->timer, jiffies + msecs_to_jiffies(100));
> else
> + regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x10, 0x10);
> +
> +
> + if (esc_pressed && time_after(jiffies,
> + locomokbd->suspend_jiffies + msecs_to_jiffies(1000))) {
> + if (locomokbd->count_cancel++ > (20)) {
Why parentheses around 20?
> + input_event(locomokbd->input, EV_PWR,
> + KEY_SUSPEND, 1);
> + locomokbd->suspend_jiffies = jiffies;
> + }
> + } else
> locomokbd->count_cancel = 0;
If one branch has curly braces the other should have them too.
>
> spin_unlock_irqrestore(&locomokbd->lock, flags);
> @@ -192,18 +184,18 @@ static void locomokbd_scankeyboard(struct locomokbd *locomokbd)
> static irqreturn_t locomokbd_interrupt(int irq, void *dev_id)
> {
> struct locomokbd *locomokbd = dev_id;
> - u16 r;
> + unsigned int r;
>
> - r = locomo_readl(locomokbd->base + LOCOMO_KIC);
> +
> + regmap_read(locomokbd->regmap, LOCOMO_KIC, &r);
> if ((r & 0x0001) == 0)
> return IRQ_HANDLED;
>
> - locomo_writel(r & ~0x0100, locomokbd->base + LOCOMO_KIC); /* Ack */
> + /* Mask and Ack */
> + regmap_write(locomokbd->regmap, LOCOMO_KIC, r & ~0x110);
>
> - /** wait chattering delay **/
> - udelay(100);
> + mod_timer(&locomokbd->timer, jiffies + msecs_to_jiffies(1));
>
> - locomokbd_scankeyboard(locomokbd);
> return IRQ_HANDLED;
> }
>
> @@ -220,47 +212,37 @@ static void locomokbd_timer_callback(unsigned long data)
> static int locomokbd_open(struct input_dev *dev)
> {
> struct locomokbd *locomokbd = input_get_drvdata(dev);
> - u16 r;
> -
> - r = locomo_readl(locomokbd->base + LOCOMO_KIC) | 0x0010;
> - locomo_writel(r, locomokbd->base + LOCOMO_KIC);
> - return 0;
> +
> + return regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x10, 0x10);
> }
>
> static void locomokbd_close(struct input_dev *dev)
> {
> struct locomokbd *locomokbd = input_get_drvdata(dev);
> - u16 r;
> -
> - r = locomo_readl(locomokbd->base + LOCOMO_KIC) & ~0x0010;
> - locomo_writel(r, locomokbd->base + LOCOMO_KIC);
> +
> + regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x10, 0x0);
> }
>
> -static int locomokbd_probe(struct locomo_dev *dev)
> +static int locomokbd_probe(struct platform_device *dev)
> {
> struct locomokbd *locomokbd;
> struct input_dev *input_dev;
> int i, err;
>
> - locomokbd = kzalloc(sizeof(struct locomokbd), GFP_KERNEL);
> - input_dev = input_allocate_device();
> - if (!locomokbd || !input_dev) {
> - err = -ENOMEM;
> - goto err_free_mem;
> - }
> + locomokbd = devm_kzalloc(&dev->dev, sizeof(struct locomokbd),
> + GFP_KERNEL);
> + if (!locomokbd)
> + return -ENOMEM;
>
> - /* try and claim memory region */
> - if (!request_mem_region((unsigned long) dev->mapbase,
> - dev->length,
> - LOCOMO_DRIVER_NAME(dev))) {
> - err = -EBUSY;
> - printk(KERN_ERR "locomokbd: Can't acquire access to io memory for keyboard\n");
> - goto err_free_mem;
> - }
> + locomokbd->regmap = dev_get_regmap(dev->dev.parent, NULL);
> + if (!locomokbd->regmap)
> + return -EINVAL;
>
> - locomo_set_drvdata(dev, locomokbd);
> + locomokbd->irq = platform_get_irq(dev, 0);
> + if (locomokbd->irq < 0)
> + return -ENXIO;
>
> - locomokbd->base = (unsigned long) dev->mapbase;
> + platform_set_drvdata(dev, locomokbd);
>
> spin_lock_init(&locomokbd->lock);
>
> @@ -270,11 +252,13 @@ static int locomokbd_probe(struct locomo_dev *dev)
>
> locomokbd->suspend_jiffies = jiffies;
>
> - locomokbd->input = input_dev;
> - strcpy(locomokbd->phys, "locomokbd/input0");
> + input_dev = input_allocate_device();
devm_input_allocate_device()?
> + if (!input_dev)
> + return -ENOMEM;
>
> + locomokbd->input = input_dev;
> input_dev->name = "LoCoMo keyboard";
> - input_dev->phys = locomokbd->phys;
> + input_dev->phys = "locomokbd/input0";
> input_dev->id.bustype = BUS_HOST;
> input_dev->id.vendor = 0x0001;
> input_dev->id.product = 0x0001;
> @@ -291,16 +275,30 @@ static int locomokbd_probe(struct locomo_dev *dev)
>
> input_set_drvdata(input_dev, locomokbd);
>
> - memcpy(locomokbd->keycode, locomokbd_keycode, sizeof(locomokbd->keycode));
> + memcpy(locomokbd->keycode,
> + locomokbd_keycode,
> + sizeof(locomokbd->keycode));
> +
> + if (machine_is_collie())
> + locomokbd->keycode[18] = KEY_HOME;
> + else
> + locomokbd->keycode[3] = KEY_HOME;
This seems like a new addition. Ideally keymap twiddling shoudl be done
from userspace.
> +
> for (i = 0; i < LOCOMOKBD_NUMKEYS; i++)
> - set_bit(locomokbd->keycode[i], input_dev->keybit);
> - clear_bit(0, input_dev->keybit);
> + input_set_capability(input_dev, EV_KEY, locomokbd->keycode[i]);
> + input_set_capability(input_dev, EV_PWR, KEY_SUSPEND);
> + __set_bit(EV_REP, input_dev->evbit);
> +
> + regmap_write(locomokbd->regmap, LOCOMO_KCMD, 1);
> + regmap_write(locomokbd->regmap, LOCOMO_KSC, 0x0);
> + regmap_write(locomokbd->regmap, LOCOMO_KIC, 0x0);
>
> /* attempt to get the interrupt */
> - err = request_irq(dev->irq[0], locomokbd_interrupt, 0, "locomokbd", locomokbd);
> + err = request_irq(locomokbd->irq, locomokbd_interrupt, 0,
> + "locomokbd", locomokbd);
devm_request_irq()?
> if (err) {
> - printk(KERN_ERR "locomokbd: Can't get irq for keyboard\n");
> - goto err_release_region;
> + dev_err(&dev->dev, "locomokbd: Can't get irq for keyboard\n");
> + goto err_free_mem;
> }
>
> err = input_register_device(locomokbd->input);
> @@ -309,54 +307,71 @@ static int locomokbd_probe(struct locomo_dev *dev)
>
> return 0;
>
> - err_free_irq:
> - free_irq(dev->irq[0], locomokbd);
> - err_release_region:
> - release_mem_region((unsigned long) dev->mapbase, dev->length);
> - locomo_set_drvdata(dev, NULL);
> - err_free_mem:
> +err_free_irq:
> + free_irq(locomokbd->irq, locomokbd);
> +err_free_mem:
> input_free_device(input_dev);
> - kfree(locomokbd);
>
> return err;
> }
>
> -static int locomokbd_remove(struct locomo_dev *dev)
> +static int locomokbd_remove(struct platform_device *dev)
> {
> - struct locomokbd *locomokbd = locomo_get_drvdata(dev);
> + struct locomokbd *locomokbd = platform_get_drvdata(dev);
>
> - free_irq(dev->irq[0], locomokbd);
> + free_irq(locomokbd->irq, locomokbd);
Is not needed with devm.
>
> del_timer_sync(&locomokbd->timer);
Should likely to go into close().
>
> input_unregister_device(locomokbd->input);
Is not needed with devm.
> - locomo_set_drvdata(dev, NULL);
>
> - release_mem_region((unsigned long) dev->mapbase, dev->length);
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int locomokbd_suspend(struct device *dev)
Mark as __maybe_unused instead of giarding with CONFIG_PM_SLEEP.
> +{
> + struct locomokbd *locomokbd = dev_get_drvdata(dev);
> +
> + regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x10, 0x0);
>
> - kfree(locomokbd);
> + del_timer_sync(&locomokbd->timer);
>
> return 0;
> }
>
> -static struct locomo_driver keyboard_driver = {
> - .drv = {
> - .name = "locomokbd"
> +static int locomokbd_resume(struct device *dev)
__maybe_unused as well.
> +{
> + struct locomokbd *locomokbd = dev_get_drvdata(dev);
> +
> + regmap_write(locomokbd->regmap, LOCOMO_KCMD, 1);
> + regmap_write(locomokbd->regmap, LOCOMO_KSC, 0);
> + regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x100, 0);
> + regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x10, 0x10);
> +
> + locomokbd_scankeyboard(locomokbd);
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(locomo_kbd_pm, locomokbd_suspend, locomokbd_resume);
> +#define LOCOMO_KBD_PM (&locomo_kbd_pm)
> +#else
> +#define LOCOMO_KBD_PM NULL
> +#endif
Just do
static SIMPLE_DEV_PM_OPS(locomo_kbd_pm,
locomokbd_suspend, locomokbd_resume);
outside of #ifdef, it will produce the right thing (an empty structure).
> +
> +static struct platform_driver locomokbd_driver = {
> + .driver = {
> + .name = "locomo-kbd",
> + .pm = LOCOMO_KBD_PM,
.pm = &locomo_kbd_pm;
> },
> - .devid = LOCOMO_DEVID_KEYBOARD,
> .probe = locomokbd_probe,
> .remove = locomokbd_remove,
> };
>
> -static int __init locomokbd_init(void)
> -{
> - return locomo_driver_register(&keyboard_driver);
> -}
> -
> -static void __exit locomokbd_exit(void)
> -{
> - locomo_driver_unregister(&keyboard_driver);
> -}
> +module_platform_driver(locomokbd_driver);
>
> -module_init(locomokbd_init);
> -module_exit(locomokbd_exit);
> +MODULE_AUTHOR("John Lenz <lenz@xxxxxxxxxxx>");
> +MODULE_DESCRIPTION("LoCoMo keyboard driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:locomo-kbd");
> --
> 2.1.4
>
Thanks.
--
Dmitry
--
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
