* [PATCH 21/74] ST SPEAr : Added keyboard support [not found] <cover.1283161023.git.viresh.kumar@st.com> @ 2010-08-30 10:43 ` Viresh KUMAR 2010-08-30 16:48 ` Dmitry Torokhov 0 siblings, 1 reply; 6+ messages in thread From: Viresh KUMAR @ 2010-08-30 10:43 UTC (permalink / raw) To: linux-arm-kernel, dmitry.torokhov, linux-input Cc: pratyush.anand, Viresh Kumar, Rajeev Kumar, bhupesh.sharma, armando.visconti, vipin.kumar, shiraz.hashim, vipulkumar.samar, deepak.sikri From: Rajeev Kumar <rajeev-dlh.kumar@st.com> Signed-off-by: Rajeev Kumar <rajeev-dlh.kumar@st.com> Signed-off-by: shiraz hashim <shiraz.hashim@st.com> Signed-off-by: Viresh Kumar <viresh.kumar@st.com> --- arch/arm/mach-spear13xx/clock.c | 2 +- arch/arm/mach-spear13xx/include/mach/generic.h | 1 + arch/arm/mach-spear13xx/spear1300_evb.c | 14 + arch/arm/mach-spear13xx/spear13xx.c | 19 ++ arch/arm/mach-spear3xx/clock.c | 12 + arch/arm/mach-spear3xx/include/mach/generic.h | 1 + arch/arm/mach-spear3xx/spear300.c | 19 ++ arch/arm/mach-spear3xx/spear300_evb.c | 14 + arch/arm/plat-spear/include/plat/keyboard.h | 154 +++++++++++ drivers/input/keyboard/Kconfig | 8 + drivers/input/keyboard/Makefile | 1 + drivers/input/keyboard/spear-keyboard.c | 335 ++++++++++++++++++++++++ 12 files changed, 579 insertions(+), 1 deletions(-) create mode 100644 arch/arm/plat-spear/include/plat/keyboard.h create mode 100644 drivers/input/keyboard/spear-keyboard.c diff --git a/arch/arm/mach-spear13xx/clock.c b/arch/arm/mach-spear13xx/clock.c index 9252940..c48b779 100644 --- a/arch/arm/mach-spear13xx/clock.c +++ b/arch/arm/mach-spear13xx/clock.c @@ -801,7 +801,7 @@ static struct clk_lookup spear_clk_lookups[] = { {.dev_id = "ssp", .clk = &ssp_clk}, {.dev_id = "gpio0", .clk = &gpio0_clk}, {.dev_id = "gpio1", .clk = &gpio1_clk}, - {.dev_id = "kbd", .clk = &kbd_clk}, + {.dev_id = "keyboard", .clk = &kbd_clk}, {.dev_id = "wdt", .clk = &wdt_clk}, }; diff --git a/arch/arm/mach-spear13xx/include/mach/generic.h b/arch/arm/mach-spear13xx/include/mach/generic.h index 7e5d7e1..19c5de0 100644 --- a/arch/arm/mach-spear13xx/include/mach/generic.h +++ b/arch/arm/mach-spear13xx/include/mach/generic.h @@ -33,6 +33,7 @@ extern struct amba_device uart_device; extern struct platform_device ehci0_device; extern struct platform_device ehci1_device; extern struct platform_device i2c_device; +extern struct platform_device kbd_device; extern struct platform_device ohci0_device; extern struct platform_device ohci1_device; extern struct platform_device rtc_device; diff --git a/arch/arm/mach-spear13xx/spear1300_evb.c b/arch/arm/mach-spear13xx/spear1300_evb.c index 0a6d26f..0cdad7a 100644 --- a/arch/arm/mach-spear13xx/spear1300_evb.c +++ b/arch/arm/mach-spear13xx/spear1300_evb.c @@ -16,6 +16,7 @@ #include <asm/mach-types.h> #include <mach/generic.h> #include <mach/spear.h> +#include <plat/keyboard.h> static struct amba_device *amba_devs[] __initdata = { &uart_device, @@ -25,15 +26,28 @@ static struct platform_device *plat_devs[] __initdata = { &ehci0_device, &ehci1_device, &i2c_device, + &kbd_device, &ohci0_device, &ohci1_device, &rtc_device, }; +/* keyboard specific platform data */ +static DECLARE_KEYMAP(spear_keymap); + +static struct kbd_platform_data kbd_data = { + .keymap = spear_keymap, + .keymapsize = ARRAY_SIZE(spear_keymap), + .rep = 1, +}; + static void __init spear1300_evb_init(void) { unsigned int i; + /* set keyboard plat data */ + kbd_set_plat_data(&kbd_device, &kbd_data); + /* call spear1300 machine init function */ spear1300_init(); diff --git a/arch/arm/mach-spear13xx/spear13xx.c b/arch/arm/mach-spear13xx/spear13xx.c index c8f1aff..342fcd9 100644 --- a/arch/arm/mach-spear13xx/spear13xx.c +++ b/arch/arm/mach-spear13xx/spear13xx.c @@ -166,6 +166,25 @@ struct platform_device ohci1_device = { .resource = ohci1_resources, }; +/* keyboard device registration */ +static struct resource kbd_resources[] = { + { + .start = SPEAR13XX_KBD_BASE, + .end = SPEAR13XX_KBD_BASE + SZ_1K - 1, + .flags = IORESOURCE_MEM, + }, { + .start = IRQ_KBD, + .flags = IORESOURCE_IRQ, + }, +}; + +struct platform_device kbd_device = { + .name = "keyboard", + .id = -1, + .num_resources = ARRAY_SIZE(kbd_resources), + .resource = kbd_resources, +}; + /* rtc device registration */ static struct resource rtc_resources[] = { { diff --git a/arch/arm/mach-spear3xx/clock.c b/arch/arm/mach-spear3xx/clock.c index 79f0159..3a26622 100644 --- a/arch/arm/mach-spear3xx/clock.c +++ b/arch/arm/mach-spear3xx/clock.c @@ -470,6 +470,15 @@ static struct clk i2c1_clk = { }; #endif +#ifdef CONFIG_MACH_SPEAR300 +/* keyboard clock */ +static struct clk kbd_clk = { + .flags = ALWAYS_ENABLED, + .pclk = &apb_clk, + .recalc = &follow_parent, +}; +#endif + /* array of all spear 3xx clock lookups */ static struct clk_lookup spear_clk_lookups[] = { { .con_id = "apb_pclk", .clk = &dummy_apb_pclk}, @@ -511,6 +520,9 @@ static struct clk_lookup spear_clk_lookups[] = { { .dev_id = "adc", .clk = &adc_clk}, { .dev_id = "ssp", .clk = &ssp_clk}, { .dev_id = "gpio", .clk = &gpio_clk}, +#ifdef CONFIG_MACH_SPEAR300 + { .dev_id = "keyboard", .clk = &kbd_clk}, +#endif #ifdef CONFIG_MACH_SPEAR320 { .dev_id = "i2c_designware.1", .clk = &i2c1_clk}, #endif diff --git a/arch/arm/mach-spear3xx/include/mach/generic.h b/arch/arm/mach-spear3xx/include/mach/generic.h index 3eb2737..70f7ee2 100644 --- a/arch/arm/mach-spear3xx/include/mach/generic.h +++ b/arch/arm/mach-spear3xx/include/mach/generic.h @@ -108,6 +108,7 @@ extern struct pmx_driver pmx_driver; /* Add spear300 machine device structure declarations here */ extern struct amba_device clcd_device; extern struct amba_device gpio1_device; +extern struct platform_device kbd_device; /* pad mux modes */ extern struct pmx_mode nand_mode; diff --git a/arch/arm/mach-spear3xx/spear300.c b/arch/arm/mach-spear3xx/spear300.c index 5d8df00..fa314e9 100644 --- a/arch/arm/mach-spear3xx/spear300.c +++ b/arch/arm/mach-spear3xx/spear300.c @@ -407,6 +407,25 @@ struct amba_device gpio1_device = { .irq = {VIRQ_GPIO1, NO_IRQ}, }; +/* keyboard device registration */ +static struct resource kbd_resources[] = { + { + .start = SPEAR300_KEYBOARD_BASE, + .end = SPEAR300_KEYBOARD_BASE + SZ_1K - 1, + .flags = IORESOURCE_MEM, + }, { + .start = VIRQ_KEYBOARD, + .flags = IORESOURCE_IRQ, + }, +}; + +struct platform_device kbd_device = { + .name = "keyboard", + .id = -1, + .num_resources = ARRAY_SIZE(kbd_resources), + .resource = kbd_resources, +}; + /* spear3xx shared irq */ struct shirq_dev_config shirq_ras1_config[] = { { diff --git a/arch/arm/mach-spear3xx/spear300_evb.c b/arch/arm/mach-spear3xx/spear300_evb.c index 3b3c6ce..afb773e 100644 --- a/arch/arm/mach-spear3xx/spear300_evb.c +++ b/arch/arm/mach-spear3xx/spear300_evb.c @@ -15,6 +15,7 @@ #include <asm/mach-types.h> #include <mach/generic.h> #include <mach/spear.h> +#include <plat/keyboard.h> /* padmux devices to enable */ static struct pmx_dev *pmx_devs[] = { @@ -51,6 +52,16 @@ static struct platform_device *plat_devs[] __initdata = { &rtc_device, /* spear300 specific devices */ + &kbd_device, +}; + +/* keyboard specific platform data */ +static DECLARE_KEYMAP(spear_keymap); + +static struct kbd_platform_data kbd_data = { + .keymap = spear_keymap, + .keymapsize = ARRAY_SIZE(spear_keymap), + .rep = 1, }; static void __init spear300_evb_init(void) @@ -62,6 +73,9 @@ static void __init spear300_evb_init(void) pmx_driver.devs = pmx_devs; pmx_driver.devs_count = ARRAY_SIZE(pmx_devs); + /* set keyboard plat data */ + kbd_set_plat_data(&kbd_device, &kbd_data); + /* call spear300 machine init function */ spear300_init(); diff --git a/arch/arm/plat-spear/include/plat/keyboard.h b/arch/arm/plat-spear/include/plat/keyboard.h new file mode 100644 index 0000000..8345770 --- /dev/null +++ b/arch/arm/plat-spear/include/plat/keyboard.h @@ -0,0 +1,154 @@ +/* + * arch/arm/plat-spear/include/plat/keyboard.h + * + * Copyright (C) 2010 ST Microelectronics + * Rajeev Kumar<rajeev-dlh.kumar@st.com> + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#ifndef __PLAT_KEYBOARD_H +#define __PLAT_KEYBOARD_H + +#include <linux/bitops.h> +#include <linux/input.h> +#include <mach/misc_regs.h> + +#define KEY(row, col, val) (((row) << 28 | ((col) << 24) | (val))) + +#define DECLARE_KEYMAP(name) \ +int name[] = {\ + KEY(0, 0, KEY_ESC), \ + KEY(0, 1, KEY_1), \ + KEY(0, 2, KEY_2), \ + KEY(0, 3, KEY_3), \ + KEY(0, 4, KEY_4), \ + KEY(0, 5, KEY_5), \ + KEY(0, 6, KEY_6), \ + KEY(0, 7, KEY_7), \ + KEY(0, 8, KEY_8), \ + KEY(1, 0, KEY_9), \ + KEY(1, 1, KEY_MINUS), \ + KEY(1, 2, KEY_EQUAL), \ + KEY(1, 3, KEY_BACKSPACE), \ + KEY(1, 4, KEY_TAB), \ + KEY(1, 5, KEY_Q), \ + KEY(1, 6, KEY_W), \ + KEY(1, 7, KEY_E), \ + KEY(1, 8, KEY_R), \ + KEY(2, 0, KEY_T), \ + KEY(2, 1, KEY_Y), \ + KEY(2, 2, KEY_U), \ + KEY(2, 3, KEY_I), \ + KEY(2, 4, KEY_O), \ + KEY(2, 5, KEY_P), \ + KEY(2, 6, KEY_LEFTBRACE), \ + KEY(2, 7, KEY_RIGHTBRACE), \ + KEY(2, 8, KEY_ENTER), \ + KEY(3, 0, KEY_LEFTCTRL), \ + KEY(3, 1, KEY_A), \ + KEY(3, 2, KEY_S), \ + KEY(3, 3, KEY_D), \ + KEY(3, 4, KEY_F), \ + KEY(3, 5, KEY_G), \ + KEY(3, 6, KEY_H), \ + KEY(3, 7, KEY_J), \ + KEY(3, 8, KEY_K), \ + KEY(4, 0, KEY_L), \ + KEY(4, 1, KEY_SEMICOLON), \ + KEY(4, 2, KEY_APOSTROPHE), \ + KEY(4, 3, KEY_GRAVE), \ + KEY(4, 4, KEY_LEFTSHIFT), \ + KEY(4, 5, KEY_BACKSLASH), \ + KEY(4, 6, KEY_Z), \ + KEY(4, 7, KEY_X), \ + KEY(4, 8, KEY_C), \ + KEY(4, 0, KEY_L), \ + KEY(4, 1, KEY_SEMICOLON), \ + KEY(4, 2, KEY_APOSTROPHE), \ + KEY(4, 3, KEY_GRAVE), \ + KEY(4, 4, KEY_LEFTSHIFT), \ + KEY(4, 5, KEY_BACKSLASH), \ + KEY(4, 6, KEY_Z), \ + KEY(4, 7, KEY_X), \ + KEY(4, 8, KEY_C), \ + KEY(4, 0, KEY_L), \ + KEY(4, 1, KEY_SEMICOLON), \ + KEY(4, 2, KEY_APOSTROPHE), \ + KEY(4, 3, KEY_GRAVE), \ + KEY(4, 4, KEY_LEFTSHIFT), \ + KEY(4, 5, KEY_BACKSLASH), \ + KEY(4, 6, KEY_Z), \ + KEY(4, 7, KEY_X), \ + KEY(4, 8, KEY_C), \ + KEY(5, 0, KEY_V), \ + KEY(5, 1, KEY_B), \ + KEY(5, 2, KEY_N), \ + KEY(5, 3, KEY_M), \ + KEY(5, 4, KEY_COMMA), \ + KEY(5, 5, KEY_DOT), \ + KEY(5, 6, KEY_SLASH), \ + KEY(5, 7, KEY_RIGHTSHIFT), \ + KEY(5, 8, KEY_KPASTERISK), \ + KEY(6, 0, KEY_LEFTALT), \ + KEY(6, 1, KEY_SPACE), \ + KEY(6, 2, KEY_CAPSLOCK), \ + KEY(6, 3, KEY_F1), \ + KEY(6, 4, KEY_F2), \ + KEY(6, 5, KEY_F3), \ + KEY(6, 6, KEY_F4), \ + KEY(6, 7, KEY_F5), \ + KEY(6, 8, KEY_F6), \ + KEY(7, 0, KEY_F7), \ + KEY(7, 1, KEY_F8), \ + KEY(7, 2, KEY_F9), \ + KEY(7, 3, KEY_F10), \ + KEY(7, 4, KEY_NUMLOCK), \ + KEY(7, 5, KEY_SCROLLLOCK), \ + KEY(7, 6, KEY_KP7), \ + KEY(7, 7, KEY_KP8), \ + KEY(7, 8, KEY_KP9), \ + KEY(8, 0, KEY_KPMINUS), \ + KEY(8, 1, KEY_KP4), \ + KEY(8, 2, KEY_KP5), \ + KEY(8, 3, KEY_KP6), \ + KEY(8, 4, KEY_KPPLUS), \ + KEY(8, 5, KEY_KP1), \ + KEY(8, 6, KEY_KP2), \ + KEY(8, 7, KEY_KP3), \ + KEY(8, 8, KEY_KP0), \ +}; +/** + * struct kbd_platform_data - keymap for spear keyboards + * keymap: pointer to array of values encoded with KEY() macro representing + * keymap + * keymapsize: number of entries (initialized) in this keymap + * rep: current values for autorepeat parameters + * + * This structure is supposed to be used by platform code to supply + * keymaps to drivers that implement keyboards. + */ +struct kbd_platform_data { + int *keymap; + unsigned int keymapsize; + unsigned int rep:1; +}; + +/* This function is used to set platform data field of pdev->dev */ +static inline void +kbd_set_plat_data(struct platform_device *pdev, struct kbd_platform_data *data) +{ +#ifdef CONFIG_ARCH_SPEAR13XX +#define KBD_PAD_SEL (BIT(18) | BIT(20) | BIT(22) | BIT(24) | BIT(26)) + + u32 val; + /* Workaround:Setting bit for routing it to the IP */ + val = readl(PAD_FUNCTION_EN_2); + val &= ~KBD_PAD_SEL; + writel(val, PAD_FUNCTION_EN_2); +#endif + pdev->dev.platform_data = data; +} +#endif /* __PLAT_KEYBOARD_H */ diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig index 9cc488d..2d7e3a8 100644 --- a/drivers/input/keyboard/Kconfig +++ b/drivers/input/keyboard/Kconfig @@ -424,6 +424,14 @@ config KEYBOARD_OMAP To compile this driver as a module, choose M here: the module will be called omap-keypad. +config KEYBOARD_SPEAR + tristate "ST SPEAR keyboard support" + help + Say Y here if you want to use the SPEAR keyboard. + + To compile this driver as a module, choose M here: the + module will be called spear-keboard. + config KEYBOARD_TWL4030 tristate "TI TWL4030/TWL5030/TPS659x0 keypad support" depends on TWL4030_CORE diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile index 504b591..b21c54d 100644 --- a/drivers/input/keyboard/Makefile +++ b/drivers/input/keyboard/Makefile @@ -35,6 +35,7 @@ obj-$(CONFIG_KEYBOARD_PXA930_ROTARY) += pxa930_rotary.o obj-$(CONFIG_KEYBOARD_QT2160) += qt2160.o obj-$(CONFIG_KEYBOARD_SAMSUNG) += samsung-keypad.o obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o +obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c new file mode 100644 index 0000000..4f5c734 --- /dev/null +++ b/drivers/input/keyboard/spear-keyboard.c @@ -0,0 +1,335 @@ +/* + * drivers/input/keyboard/keyboard-spear.c + * + * SPEAr Keyboard Driver + * Based on omap-keypad driver + * + * Copyright (C) 2010 ST Microelectronics + * Rajeev Kumar<rajeev-dlh.kumar@st.com> + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include <linux/clk.h> +#include <linux/errno.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/input.h> +#include <linux/io.h> +#include <linux/irq.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/pm_wakeup.h> +#include <linux/slab.h> +#include <linux/types.h> +#include <plat/keyboard.h> + +/* Keyboard Regsiters */ +#define MODE_REG 0x00 /* 16 bit reg */ +#define STATUS_REG 0x0C /* 2 bit reg */ +#define DATA_REG 0x10 /* 8 bit reg */ +#define INTR_MASK 0x54 + +/* Register Values */ +/* + * pclk freq mask = (APB FEQ -1)= 82 MHZ.Programme bit 15-9 in mode + * control register as 1010010(82MHZ) + */ +#define PCLK_FREQ_MSK 0xA400 /* 82 MHz */ +#define START_SCAN 0x0100 +#define SCAN_RATE_10 0x0000 +#define SCAN_RATE_20 0x0004 +#define SCAN_RATE_40 0x0008 +#define SCAN_RATE_80 0x000C +#define MODE_KEYBOARD 0x0002 +#define DATA_AVAIL 0x2 + +#define KEY_MASK 0xFF000000 +#define KEY_VALUE 0x00FFFFFF +#define ROW_MASK 0xF0 +#define COLUMN_MASK 0x0F +#define ROW_SHIFT 4 + +struct spear_kbd { + struct input_dev *input; + void __iomem *io_base; /* Keyboard Base Address */ + struct clk *clk; + int *keymap; +}; +/* TODO: Need to optimize this function */ +static inline int get_key_value(struct spear_kbd *dev, int row, int col) +{ + int i, key; + int *keymap = dev->keymap; + + key = KEY(row, col, 0); + for (i = 0; keymap[i] != 0; i++) + if ((keymap[i] & KEY_MASK) == key) + return keymap[i] & KEY_VALUE; + return -ENOKEY; +} + +static irqreturn_t spear_kbd_interrupt(int irq, void *dev_id) +{ + struct spear_kbd *dev = dev_id; + static u8 last_key ; + static u8 last_event; + int key; + u8 sts, val = 0; + + if (dev == NULL) { + pr_err("Keyboard: Invalid dev_id in irq handler\n"); + return IRQ_NONE; + } + + sts = readb(dev->io_base + STATUS_REG); + if (sts & DATA_AVAIL) { + /* following reads active (row, col) pair */ + val = readb(dev->io_base + DATA_REG); + key = get_key_value(dev, (val & ROW_MASK)>>ROW_SHIFT, (val + & COLUMN_MASK)); + + /* valid key press event */ + if (key >= 0) { + if (last_event == 1) { + /* check if we missed a release event */ + input_report_key(dev->input, last_key, + !last_event); + } + /* notify key press */ + last_event = 1; + last_key = key; + input_report_key(dev->input, key, last_event); + } else { + /* notify key release */ + last_event = 0; + input_report_key(dev->input, last_key, last_event); + } + } else + return IRQ_NONE; + + /* clear interrupt */ + writeb(0, dev->io_base + STATUS_REG); + + return IRQ_HANDLED; +} + +static int __init spear_kbd_probe(struct platform_device *pdev) +{ + struct spear_kbd *kbd; + struct kbd_platform_data *pdata = pdev->dev.platform_data; + struct resource *res; + int i, ret, irq; + u16 val = 0; + + if (!pdata) { + dev_err(&pdev->dev, "Invalid platform data\n"); + return -EINVAL; + } + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { + dev_err(&pdev->dev, "no keyboard resource defined\n"); + return -EBUSY; + } + + if (!request_mem_region(res->start, resource_size(res), pdev->name)) { + dev_err(&pdev->dev, "keyboard region already claimed\n"); + return -EBUSY; + } + + kbd = kzalloc(sizeof(*kbd), GFP_KERNEL); + if (!kbd) { + dev_err(&pdev->dev, "out of memory\n"); + ret = -ENOMEM; + goto err_release_mem_region; + } + + kbd->clk = clk_get(&pdev->dev, NULL); + if (IS_ERR(kbd->clk)) { + ret = PTR_ERR(kbd->clk); + goto err_kfree; + } + + ret = clk_enable(kbd->clk); + if (ret < 0) + goto err_clk_put; + + platform_set_drvdata(pdev, kbd); + kbd->keymap = pdata->keymap; /* key mappings */ + + kbd->io_base = ioremap(res->start, resource_size(res)); + if (!kbd->io_base) { + dev_err(&pdev->dev, "ioremap fail for kbd_region\n"); + ret = -ENOMEM; + goto err_clear_plat_data; + } + + irq = platform_get_irq(pdev, 0); + if (irq < 0) { + dev_err(&pdev->dev, "not able to get irq for the device\n"); + ret = irq; + goto err_iounmap; + } + + ret = request_irq(irq, spear_kbd_interrupt, 0, "keyboard", + kbd); + if (ret) { + dev_err(&pdev->dev, "request_irq fail\n"); + goto err_iounmap; + } + + kbd->input = input_allocate_device(); + if (!kbd->input) { + ret = -ENOMEM; + dev_err(&pdev->dev, "input device allocation fail\n"); + goto err_free_irq; + } + + if (pdata->rep) + __set_bit(EV_REP, kbd->input->evbit); + + /* setup input device */ + __set_bit(EV_KEY, kbd->input->evbit); + + for (i = 0; kbd->keymap[i] != 0; i++) + __set_bit(kbd->keymap[i] & KEY_MAX, kbd->input->keybit); + + kbd->input->name = "keyboard"; + kbd->input->phys = "keyboard/input0"; + kbd->input->dev.parent = &pdev->dev; + kbd->input->id.bustype = BUS_HOST; + kbd->input->id.vendor = 0x0001; + kbd->input->id.product = 0x0001; + kbd->input->id.version = 0x0100; + + ret = input_register_device(kbd->input); + if (ret < 0) { + dev_err(&pdev->dev, "Unable to register keyboard device\n"); + goto err_free_dev; + } + + /* program keyboard */ + val |= SCAN_RATE_80 | MODE_KEYBOARD | PCLK_FREQ_MSK; + writew(val, kbd->io_base + MODE_REG); + + writeb(1, kbd->io_base + STATUS_REG); + + /* start key scan */ + val |= START_SCAN; + writew(val, kbd->io_base + MODE_REG); + + device_init_wakeup(&pdev->dev, 1); + + return 0; + +err_free_dev: + input_free_device(kbd->input); +err_free_irq: + free_irq(irq, pdev); +err_iounmap: + iounmap(kbd->io_base); +err_clear_plat_data: + platform_set_drvdata(pdev, NULL); + clk_disable(kbd->clk); +err_clk_put: + clk_put(kbd->clk); +err_kfree: + kfree(kbd); +err_release_mem_region: + release_mem_region(res->start, resource_size(res)); + + return ret; +} + +static int spear_kbd_remove(struct platform_device *pdev) +{ + struct spear_kbd *kbd = platform_get_drvdata(pdev); + struct resource *res; + u16 val; + int irq; + + val = readw(kbd->io_base + MODE_REG); + val &= ~START_SCAN; + writew(val, kbd->io_base + MODE_REG); + + /* unregister input device */ + input_unregister_device(kbd->input); + input_free_device(kbd->input); + + irq = platform_get_irq(pdev, 0); + if (irq) + free_irq(irq, pdev); + + iounmap(kbd->io_base); + platform_set_drvdata(pdev, NULL); + clk_disable(kbd->clk); + clk_put(kbd->clk); + kfree(kbd); + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (res) + release_mem_region(res->start, resource_size(res)); + + return 0; +} + +#ifdef CONFIG_PM +static int spear_kbd_suspend(struct platform_device *pdev, pm_message_t state) +{ + struct spear_kbd *kbd = platform_get_drvdata(pdev); + int irq; + + irq = platform_get_irq(pdev, 0); + clk_disable(kbd->clk); + if (device_may_wakeup(&pdev->dev)) + enable_irq_wake(irq); + + return 0; +} + +static int spear_kbd_resume(struct platform_device *pdev) +{ + struct spear_kbd *kbd = platform_get_drvdata(pdev); + int irq; + + irq = platform_get_irq(pdev, 0); + if (device_may_wakeup(&pdev->dev)) + disable_irq_wake(irq); + clk_enable(kbd->clk); + + return 0; +} +#else +#define spear_kbd_suspend NULL +#define spear_kbd_resume NULL +#endif + +static struct platform_driver spear_kbd_driver = { + .probe = spear_kbd_probe, + .remove = spear_kbd_remove, + .suspend = spear_kbd_suspend, + .resume = spear_kbd_resume, + .driver = { + .name = "keyboard", + .owner = THIS_MODULE, + }, +}; + +static int __devinit spear_kbd_init(void) +{ + return platform_driver_register(&spear_kbd_driver); +} +module_init(spear_kbd_init); + +static void __exit spear_kbd_exit(void) +{ + platform_driver_unregister(&spear_kbd_driver); +} +module_exit(spear_kbd_exit); + +MODULE_AUTHOR("Rajeev Kumar"); +MODULE_DESCRIPTION("SPEAr Keyboard Driver"); +MODULE_LICENSE("GPL"); -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 21/74] ST SPEAr : Added keyboard support 2010-08-30 10:43 ` [PATCH 21/74] ST SPEAr : Added keyboard support Viresh KUMAR @ 2010-08-30 16:48 ` Dmitry Torokhov 2010-09-01 5:23 ` rajeev 0 siblings, 1 reply; 6+ messages in thread From: Dmitry Torokhov @ 2010-08-30 16:48 UTC (permalink / raw) To: Viresh KUMAR Cc: linux-arm-kernel, linux-input, Rajeev Kumar, shiraz.hashim, vipin.kumar, deepak.sikri, armando.visconti, vipulkumar.samar, pratyush.anand, bhupesh.sharma Hi Rajeev, On Mon, Aug 30, 2010 at 04:13:01PM +0530, Viresh KUMAR wrote: > From: Rajeev Kumar <rajeev-dlh.kumar@st.com> > > Signed-off-by: Rajeev Kumar <rajeev-dlh.kumar@st.com> > Signed-off-by: shiraz hashim <shiraz.hashim@st.com> > Signed-off-by: Viresh Kumar <viresh.kumar@st.com> > --- > arch/arm/mach-spear13xx/clock.c | 2 +- > arch/arm/mach-spear13xx/include/mach/generic.h | 1 + > arch/arm/mach-spear13xx/spear1300_evb.c | 14 + > arch/arm/mach-spear13xx/spear13xx.c | 19 ++ > arch/arm/mach-spear3xx/clock.c | 12 + > arch/arm/mach-spear3xx/include/mach/generic.h | 1 + > arch/arm/mach-spear3xx/spear300.c | 19 ++ > arch/arm/mach-spear3xx/spear300_evb.c | 14 + > arch/arm/plat-spear/include/plat/keyboard.h | 154 +++++++++++ > drivers/input/keyboard/Kconfig | 8 + > drivers/input/keyboard/Makefile | 1 + > drivers/input/keyboard/spear-keyboard.c | 335 ++++++++++++++++++++++++ > 12 files changed, 579 insertions(+), 1 deletions(-) > create mode 100644 arch/arm/plat-spear/include/plat/keyboard.h > create mode 100644 drivers/input/keyboard/spear-keyboard.c > First of all, please split platform modifications from the driver itself (as I care about the latter but less about the former). > diff --git a/arch/arm/mach-spear13xx/clock.c b/arch/arm/mach-spear13xx/clock.c > index 9252940..c48b779 100644 > --- a/arch/arm/mach-spear13xx/clock.c > +++ b/arch/arm/mach-spear13xx/clock.c > @@ -801,7 +801,7 @@ static struct clk_lookup spear_clk_lookups[] = { > {.dev_id = "ssp", .clk = &ssp_clk}, > {.dev_id = "gpio0", .clk = &gpio0_clk}, > {.dev_id = "gpio1", .clk = &gpio1_clk}, > - {.dev_id = "kbd", .clk = &kbd_clk}, > + {.dev_id = "keyboard", .clk = &kbd_clk}, > {.dev_id = "wdt", .clk = &wdt_clk}, > }; > > diff --git a/arch/arm/mach-spear13xx/include/mach/generic.h b/arch/arm/mach-spear13xx/include/mach/generic.h > index 7e5d7e1..19c5de0 100644 > --- a/arch/arm/mach-spear13xx/include/mach/generic.h > +++ b/arch/arm/mach-spear13xx/include/mach/generic.h > @@ -33,6 +33,7 @@ extern struct amba_device uart_device; > extern struct platform_device ehci0_device; > extern struct platform_device ehci1_device; > extern struct platform_device i2c_device; > +extern struct platform_device kbd_device; > extern struct platform_device ohci0_device; > extern struct platform_device ohci1_device; > extern struct platform_device rtc_device; > diff --git a/arch/arm/mach-spear13xx/spear1300_evb.c b/arch/arm/mach-spear13xx/spear1300_evb.c > index 0a6d26f..0cdad7a 100644 > --- a/arch/arm/mach-spear13xx/spear1300_evb.c > +++ b/arch/arm/mach-spear13xx/spear1300_evb.c > @@ -16,6 +16,7 @@ > #include <asm/mach-types.h> > #include <mach/generic.h> > #include <mach/spear.h> > +#include <plat/keyboard.h> > > static struct amba_device *amba_devs[] __initdata = { > &uart_device, > @@ -25,15 +26,28 @@ static struct platform_device *plat_devs[] __initdata = { > &ehci0_device, > &ehci1_device, > &i2c_device, > + &kbd_device, > &ohci0_device, > &ohci1_device, > &rtc_device, > }; > > +/* keyboard specific platform data */ > +static DECLARE_KEYMAP(spear_keymap); > + > +static struct kbd_platform_data kbd_data = { > + .keymap = spear_keymap, > + .keymapsize = ARRAY_SIZE(spear_keymap), > + .rep = 1, > +}; > + > static void __init spear1300_evb_init(void) > { > unsigned int i; > > + /* set keyboard plat data */ > + kbd_set_plat_data(&kbd_device, &kbd_data); > + > /* call spear1300 machine init function */ > spear1300_init(); > > diff --git a/arch/arm/mach-spear13xx/spear13xx.c b/arch/arm/mach-spear13xx/spear13xx.c > index c8f1aff..342fcd9 100644 > --- a/arch/arm/mach-spear13xx/spear13xx.c > +++ b/arch/arm/mach-spear13xx/spear13xx.c > @@ -166,6 +166,25 @@ struct platform_device ohci1_device = { > .resource = ohci1_resources, > }; > > +/* keyboard device registration */ > +static struct resource kbd_resources[] = { > + { > + .start = SPEAR13XX_KBD_BASE, > + .end = SPEAR13XX_KBD_BASE + SZ_1K - 1, > + .flags = IORESOURCE_MEM, > + }, { > + .start = IRQ_KBD, > + .flags = IORESOURCE_IRQ, > + }, > +}; > + > +struct platform_device kbd_device = { > + .name = "keyboard", > + .id = -1, > + .num_resources = ARRAY_SIZE(kbd_resources), > + .resource = kbd_resources, > +}; > + > /* rtc device registration */ > static struct resource rtc_resources[] = { > { > diff --git a/arch/arm/mach-spear3xx/clock.c b/arch/arm/mach-spear3xx/clock.c > index 79f0159..3a26622 100644 > --- a/arch/arm/mach-spear3xx/clock.c > +++ b/arch/arm/mach-spear3xx/clock.c > @@ -470,6 +470,15 @@ static struct clk i2c1_clk = { > }; > #endif > > +#ifdef CONFIG_MACH_SPEAR300 > +/* keyboard clock */ > +static struct clk kbd_clk = { > + .flags = ALWAYS_ENABLED, > + .pclk = &apb_clk, > + .recalc = &follow_parent, > +}; > +#endif > + > /* array of all spear 3xx clock lookups */ > static struct clk_lookup spear_clk_lookups[] = { > { .con_id = "apb_pclk", .clk = &dummy_apb_pclk}, > @@ -511,6 +520,9 @@ static struct clk_lookup spear_clk_lookups[] = { > { .dev_id = "adc", .clk = &adc_clk}, > { .dev_id = "ssp", .clk = &ssp_clk}, > { .dev_id = "gpio", .clk = &gpio_clk}, > +#ifdef CONFIG_MACH_SPEAR300 > + { .dev_id = "keyboard", .clk = &kbd_clk}, > +#endif > #ifdef CONFIG_MACH_SPEAR320 > { .dev_id = "i2c_designware.1", .clk = &i2c1_clk}, > #endif > diff --git a/arch/arm/mach-spear3xx/include/mach/generic.h b/arch/arm/mach-spear3xx/include/mach/generic.h > index 3eb2737..70f7ee2 100644 > --- a/arch/arm/mach-spear3xx/include/mach/generic.h > +++ b/arch/arm/mach-spear3xx/include/mach/generic.h > @@ -108,6 +108,7 @@ extern struct pmx_driver pmx_driver; > /* Add spear300 machine device structure declarations here */ > extern struct amba_device clcd_device; > extern struct amba_device gpio1_device; > +extern struct platform_device kbd_device; > > /* pad mux modes */ > extern struct pmx_mode nand_mode; > diff --git a/arch/arm/mach-spear3xx/spear300.c b/arch/arm/mach-spear3xx/spear300.c > index 5d8df00..fa314e9 100644 > --- a/arch/arm/mach-spear3xx/spear300.c > +++ b/arch/arm/mach-spear3xx/spear300.c > @@ -407,6 +407,25 @@ struct amba_device gpio1_device = { > .irq = {VIRQ_GPIO1, NO_IRQ}, > }; > > +/* keyboard device registration */ > +static struct resource kbd_resources[] = { > + { > + .start = SPEAR300_KEYBOARD_BASE, > + .end = SPEAR300_KEYBOARD_BASE + SZ_1K - 1, > + .flags = IORESOURCE_MEM, > + }, { > + .start = VIRQ_KEYBOARD, > + .flags = IORESOURCE_IRQ, > + }, > +}; > + > +struct platform_device kbd_device = { > + .name = "keyboard", > + .id = -1, > + .num_resources = ARRAY_SIZE(kbd_resources), > + .resource = kbd_resources, > +}; > + > /* spear3xx shared irq */ > struct shirq_dev_config shirq_ras1_config[] = { > { > diff --git a/arch/arm/mach-spear3xx/spear300_evb.c b/arch/arm/mach-spear3xx/spear300_evb.c > index 3b3c6ce..afb773e 100644 > --- a/arch/arm/mach-spear3xx/spear300_evb.c > +++ b/arch/arm/mach-spear3xx/spear300_evb.c > @@ -15,6 +15,7 @@ > #include <asm/mach-types.h> > #include <mach/generic.h> > #include <mach/spear.h> > +#include <plat/keyboard.h> > > /* padmux devices to enable */ > static struct pmx_dev *pmx_devs[] = { > @@ -51,6 +52,16 @@ static struct platform_device *plat_devs[] __initdata = { > &rtc_device, > > /* spear300 specific devices */ > + &kbd_device, > +}; > + > +/* keyboard specific platform data */ > +static DECLARE_KEYMAP(spear_keymap); > + > +static struct kbd_platform_data kbd_data = { > + .keymap = spear_keymap, > + .keymapsize = ARRAY_SIZE(spear_keymap), > + .rep = 1, > }; > > static void __init spear300_evb_init(void) > @@ -62,6 +73,9 @@ static void __init spear300_evb_init(void) > pmx_driver.devs = pmx_devs; > pmx_driver.devs_count = ARRAY_SIZE(pmx_devs); > > + /* set keyboard plat data */ > + kbd_set_plat_data(&kbd_device, &kbd_data); > + > /* call spear300 machine init function */ > spear300_init(); > > diff --git a/arch/arm/plat-spear/include/plat/keyboard.h b/arch/arm/plat-spear/include/plat/keyboard.h > new file mode 100644 > index 0000000..8345770 > --- /dev/null > +++ b/arch/arm/plat-spear/include/plat/keyboard.h > @@ -0,0 +1,154 @@ > +/* > + * arch/arm/plat-spear/include/plat/keyboard.h > + * > + * Copyright (C) 2010 ST Microelectronics > + * Rajeev Kumar<rajeev-dlh.kumar@st.com> > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#ifndef __PLAT_KEYBOARD_H > +#define __PLAT_KEYBOARD_H > + > +#include <linux/bitops.h> > +#include <linux/input.h> > +#include <mach/misc_regs.h> > + > +#define KEY(row, col, val) (((row) << 28 | ((col) << 24) | (val))) > + Please use definitions from include/linux/input/matrix_keypad.h > +#define DECLARE_KEYMAP(name) \ > +int name[] = {\ > + KEY(0, 0, KEY_ESC), \ > + KEY(0, 1, KEY_1), \ > + KEY(0, 2, KEY_2), \ > + KEY(0, 3, KEY_3), \ > + KEY(0, 4, KEY_4), \ > + KEY(0, 5, KEY_5), \ > + KEY(0, 6, KEY_6), \ > + KEY(0, 7, KEY_7), \ > + KEY(0, 8, KEY_8), \ > + KEY(1, 0, KEY_9), \ > + KEY(1, 1, KEY_MINUS), \ > + KEY(1, 2, KEY_EQUAL), \ > + KEY(1, 3, KEY_BACKSPACE), \ > + KEY(1, 4, KEY_TAB), \ > + KEY(1, 5, KEY_Q), \ > + KEY(1, 6, KEY_W), \ > + KEY(1, 7, KEY_E), \ > + KEY(1, 8, KEY_R), \ > + KEY(2, 0, KEY_T), \ > + KEY(2, 1, KEY_Y), \ > + KEY(2, 2, KEY_U), \ > + KEY(2, 3, KEY_I), \ > + KEY(2, 4, KEY_O), \ > + KEY(2, 5, KEY_P), \ > + KEY(2, 6, KEY_LEFTBRACE), \ > + KEY(2, 7, KEY_RIGHTBRACE), \ > + KEY(2, 8, KEY_ENTER), \ > + KEY(3, 0, KEY_LEFTCTRL), \ > + KEY(3, 1, KEY_A), \ > + KEY(3, 2, KEY_S), \ > + KEY(3, 3, KEY_D), \ > + KEY(3, 4, KEY_F), \ > + KEY(3, 5, KEY_G), \ > + KEY(3, 6, KEY_H), \ > + KEY(3, 7, KEY_J), \ > + KEY(3, 8, KEY_K), \ > + KEY(4, 0, KEY_L), \ > + KEY(4, 1, KEY_SEMICOLON), \ > + KEY(4, 2, KEY_APOSTROPHE), \ > + KEY(4, 3, KEY_GRAVE), \ > + KEY(4, 4, KEY_LEFTSHIFT), \ > + KEY(4, 5, KEY_BACKSLASH), \ > + KEY(4, 6, KEY_Z), \ > + KEY(4, 7, KEY_X), \ > + KEY(4, 8, KEY_C), \ > + KEY(4, 0, KEY_L), \ > + KEY(4, 1, KEY_SEMICOLON), \ > + KEY(4, 2, KEY_APOSTROPHE), \ > + KEY(4, 3, KEY_GRAVE), \ > + KEY(4, 4, KEY_LEFTSHIFT), \ > + KEY(4, 5, KEY_BACKSLASH), \ > + KEY(4, 6, KEY_Z), \ > + KEY(4, 7, KEY_X), \ > + KEY(4, 8, KEY_C), \ > + KEY(4, 0, KEY_L), \ > + KEY(4, 1, KEY_SEMICOLON), \ > + KEY(4, 2, KEY_APOSTROPHE), \ > + KEY(4, 3, KEY_GRAVE), \ > + KEY(4, 4, KEY_LEFTSHIFT), \ > + KEY(4, 5, KEY_BACKSLASH), \ > + KEY(4, 6, KEY_Z), \ > + KEY(4, 7, KEY_X), \ > + KEY(4, 8, KEY_C), \ > + KEY(5, 0, KEY_V), \ > + KEY(5, 1, KEY_B), \ > + KEY(5, 2, KEY_N), \ > + KEY(5, 3, KEY_M), \ > + KEY(5, 4, KEY_COMMA), \ > + KEY(5, 5, KEY_DOT), \ > + KEY(5, 6, KEY_SLASH), \ > + KEY(5, 7, KEY_RIGHTSHIFT), \ > + KEY(5, 8, KEY_KPASTERISK), \ > + KEY(6, 0, KEY_LEFTALT), \ > + KEY(6, 1, KEY_SPACE), \ > + KEY(6, 2, KEY_CAPSLOCK), \ > + KEY(6, 3, KEY_F1), \ > + KEY(6, 4, KEY_F2), \ > + KEY(6, 5, KEY_F3), \ > + KEY(6, 6, KEY_F4), \ > + KEY(6, 7, KEY_F5), \ > + KEY(6, 8, KEY_F6), \ > + KEY(7, 0, KEY_F7), \ > + KEY(7, 1, KEY_F8), \ > + KEY(7, 2, KEY_F9), \ > + KEY(7, 3, KEY_F10), \ > + KEY(7, 4, KEY_NUMLOCK), \ > + KEY(7, 5, KEY_SCROLLLOCK), \ > + KEY(7, 6, KEY_KP7), \ > + KEY(7, 7, KEY_KP8), \ > + KEY(7, 8, KEY_KP9), \ > + KEY(8, 0, KEY_KPMINUS), \ > + KEY(8, 1, KEY_KP4), \ > + KEY(8, 2, KEY_KP5), \ > + KEY(8, 3, KEY_KP6), \ > + KEY(8, 4, KEY_KPPLUS), \ > + KEY(8, 5, KEY_KP1), \ > + KEY(8, 6, KEY_KP2), \ > + KEY(8, 7, KEY_KP3), \ > + KEY(8, 8, KEY_KP0), \ > +}; Hm, I'd expect this to be in particular board code, not in the header file. > +/** > + * struct kbd_platform_data - keymap for spear keyboards > + * keymap: pointer to array of values encoded with KEY() macro representing > + * keymap > + * keymapsize: number of entries (initialized) in this keymap > + * rep: current values for autorepeat parameters > + * > + * This structure is supposed to be used by platform code to supply > + * keymaps to drivers that implement keyboards. > + */ > +struct kbd_platform_data { > + int *keymap; > + unsigned int keymapsize; > + unsigned int rep:1; Bool please. > +}; > + > +/* This function is used to set platform data field of pdev->dev */ > +static inline void > +kbd_set_plat_data(struct platform_device *pdev, struct kbd_platform_data *data) > +{ > +#ifdef CONFIG_ARCH_SPEAR13XX > +#define KBD_PAD_SEL (BIT(18) | BIT(20) | BIT(22) | BIT(24) | BIT(26)) > + > + u32 val; > + /* Workaround:Setting bit for routing it to the IP */ > + val = readl(PAD_FUNCTION_EN_2); > + val &= ~KBD_PAD_SEL; > + writel(val, PAD_FUNCTION_EN_2); Why does it belong here? > +#endif > + pdev->dev.platform_data = data; > +} > +#endif /* __PLAT_KEYBOARD_H */ > diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig > index 9cc488d..2d7e3a8 100644 > --- a/drivers/input/keyboard/Kconfig > +++ b/drivers/input/keyboard/Kconfig > @@ -424,6 +424,14 @@ config KEYBOARD_OMAP > To compile this driver as a module, choose M here: the > module will be called omap-keypad. > > +config KEYBOARD_SPEAR > + tristate "ST SPEAR keyboard support" No arch/platform dependencies? > + help > + Say Y here if you want to use the SPEAR keyboard. > + > + To compile this driver as a module, choose M here: the > + module will be called spear-keboard. > + > config KEYBOARD_TWL4030 > tristate "TI TWL4030/TWL5030/TPS659x0 keypad support" > depends on TWL4030_CORE > diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile > index 504b591..b21c54d 100644 > --- a/drivers/input/keyboard/Makefile > +++ b/drivers/input/keyboard/Makefile > @@ -35,6 +35,7 @@ obj-$(CONFIG_KEYBOARD_PXA930_ROTARY) += pxa930_rotary.o > obj-$(CONFIG_KEYBOARD_QT2160) += qt2160.o > obj-$(CONFIG_KEYBOARD_SAMSUNG) += samsung-keypad.o > obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o > +obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o > obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o > obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o > obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o > diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c > new file mode 100644 > index 0000000..4f5c734 > --- /dev/null > +++ b/drivers/input/keyboard/spear-keyboard.c > @@ -0,0 +1,335 @@ > +/* > + * drivers/input/keyboard/keyboard-spear.c > + * > + * SPEAr Keyboard Driver > + * Based on omap-keypad driver > + * > + * Copyright (C) 2010 ST Microelectronics > + * Rajeev Kumar<rajeev-dlh.kumar@st.com> > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#include <linux/clk.h> > +#include <linux/errno.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/input.h> > +#include <linux/io.h> > +#include <linux/irq.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/pm_wakeup.h> > +#include <linux/slab.h> > +#include <linux/types.h> > +#include <plat/keyboard.h> > + > +/* Keyboard Regsiters */ > +#define MODE_REG 0x00 /* 16 bit reg */ > +#define STATUS_REG 0x0C /* 2 bit reg */ > +#define DATA_REG 0x10 /* 8 bit reg */ > +#define INTR_MASK 0x54 > + > +/* Register Values */ > +/* > + * pclk freq mask = (APB FEQ -1)= 82 MHZ.Programme bit 15-9 in mode > + * control register as 1010010(82MHZ) > + */ > +#define PCLK_FREQ_MSK 0xA400 /* 82 MHz */ > +#define START_SCAN 0x0100 > +#define SCAN_RATE_10 0x0000 > +#define SCAN_RATE_20 0x0004 > +#define SCAN_RATE_40 0x0008 > +#define SCAN_RATE_80 0x000C > +#define MODE_KEYBOARD 0x0002 > +#define DATA_AVAIL 0x2 > + > +#define KEY_MASK 0xFF000000 > +#define KEY_VALUE 0x00FFFFFF > +#define ROW_MASK 0xF0 > +#define COLUMN_MASK 0x0F > +#define ROW_SHIFT 4 > + > +struct spear_kbd { > + struct input_dev *input; > + void __iomem *io_base; /* Keyboard Base Address */ > + struct clk *clk; > + int *keymap; You need a copy of keymap here so that userspace can modify it safely via EVIOCSKEYCODE. > +}; > +/* TODO: Need to optimize this function */ > +static inline int get_key_value(struct spear_kbd *dev, int row, int col) > +{ > + int i, key; > + int *keymap = dev->keymap; > + > + key = KEY(row, col, 0); > + for (i = 0; keymap[i] != 0; i++) > + if ((keymap[i] & KEY_MASK) == key) > + return keymap[i] & KEY_VALUE; > + return -ENOKEY; > +} > + > +static irqreturn_t spear_kbd_interrupt(int irq, void *dev_id) > +{ > + struct spear_kbd *dev = dev_id; > + static u8 last_key ; > + static u8 last_event; No statics please, pull it into spear_kbd structure. > + int key; > + u8 sts, val = 0; > + > + if (dev == NULL) { How can it be? > + pr_err("Keyboard: Invalid dev_id in irq handler\n"); > + return IRQ_NONE; > + } > + > + sts = readb(dev->io_base + STATUS_REG); > + if (sts & DATA_AVAIL) { > + /* following reads active (row, col) pair */ > + val = readb(dev->io_base + DATA_REG); > + key = get_key_value(dev, (val & ROW_MASK)>>ROW_SHIFT, (val > + & COLUMN_MASK)); > + > + /* valid key press event */ > + if (key >= 0) { > + if (last_event == 1) { > + /* check if we missed a release event */ > + input_report_key(dev->input, last_key, > + !last_event); > + } > + /* notify key press */ > + last_event = 1; > + last_key = key; > + input_report_key(dev->input, key, last_event); > + } else { > + /* notify key release */ > + last_event = 0; > + input_report_key(dev->input, last_key, last_event); > + } > + } else Don't you need to clear interrupt here? You got it somehow... > + return IRQ_NONE; > + > + /* clear interrupt */ > + writeb(0, dev->io_base + STATUS_REG); > + > + return IRQ_HANDLED; > +} > + > +static int __init spear_kbd_probe(struct platform_device *pdev) > +{ > + struct spear_kbd *kbd; > + struct kbd_platform_data *pdata = pdev->dev.platform_data; > + struct resource *res; > + int i, ret, irq; > + u16 val = 0; > + > + if (!pdata) { > + dev_err(&pdev->dev, "Invalid platform data\n"); > + return -EINVAL; > + } > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&pdev->dev, "no keyboard resource defined\n"); > + return -EBUSY; > + } > + > + if (!request_mem_region(res->start, resource_size(res), pdev->name)) { > + dev_err(&pdev->dev, "keyboard region already claimed\n"); > + return -EBUSY; > + } > + > + kbd = kzalloc(sizeof(*kbd), GFP_KERNEL); > + if (!kbd) { > + dev_err(&pdev->dev, "out of memory\n"); > + ret = -ENOMEM; > + goto err_release_mem_region; > + } > + > + kbd->clk = clk_get(&pdev->dev, NULL); > + if (IS_ERR(kbd->clk)) { > + ret = PTR_ERR(kbd->clk); > + goto err_kfree; > + } > + > + ret = clk_enable(kbd->clk); > + if (ret < 0) > + goto err_clk_put; > + > + platform_set_drvdata(pdev, kbd); > + kbd->keymap = pdata->keymap; /* key mappings */ > + > + kbd->io_base = ioremap(res->start, resource_size(res)); > + if (!kbd->io_base) { > + dev_err(&pdev->dev, "ioremap fail for kbd_region\n"); > + ret = -ENOMEM; > + goto err_clear_plat_data; > + } > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) { > + dev_err(&pdev->dev, "not able to get irq for the device\n"); > + ret = irq; > + goto err_iounmap; > + } > + > + ret = request_irq(irq, spear_kbd_interrupt, 0, "keyboard", > + kbd); > + if (ret) { > + dev_err(&pdev->dev, "request_irq fail\n"); > + goto err_iounmap; > + } > + Are you 100% sure the device is shut off at this point? Because if it isn't you risk to get interrupt before you allocated your input device. > + kbd->input = input_allocate_device(); > + if (!kbd->input) { > + ret = -ENOMEM; > + dev_err(&pdev->dev, "input device allocation fail\n"); > + goto err_free_irq; > + } > + > + if (pdata->rep) > + __set_bit(EV_REP, kbd->input->evbit); > + > + /* setup input device */ > + __set_bit(EV_KEY, kbd->input->evbit); > + > + for (i = 0; kbd->keymap[i] != 0; i++) > + __set_bit(kbd->keymap[i] & KEY_MAX, kbd->input->keybit); > + > + kbd->input->name = "keyboard"; > + kbd->input->phys = "keyboard/input0"; > + kbd->input->dev.parent = &pdev->dev; > + kbd->input->id.bustype = BUS_HOST; > + kbd->input->id.vendor = 0x0001; > + kbd->input->id.product = 0x0001; > + kbd->input->id.version = 0x0100; > + > + ret = input_register_device(kbd->input); > + if (ret < 0) { > + dev_err(&pdev->dev, "Unable to register keyboard device\n"); > + goto err_free_dev; > + } > + > + /* program keyboard */ > + val |= SCAN_RATE_80 | MODE_KEYBOARD | PCLK_FREQ_MSK; > + writew(val, kbd->io_base + MODE_REG); > + > + writeb(1, kbd->io_base + STATUS_REG); > + > + /* start key scan */ > + val |= START_SCAN; > + writew(val, kbd->io_base + MODE_REG); This should go into open() method. > + > + device_init_wakeup(&pdev->dev, 1); > + > + return 0; > + > +err_free_dev: > + input_free_device(kbd->input); > +err_free_irq: > + free_irq(irq, pdev); > +err_iounmap: > + iounmap(kbd->io_base); > +err_clear_plat_data: > + platform_set_drvdata(pdev, NULL); > + clk_disable(kbd->clk); > +err_clk_put: > + clk_put(kbd->clk); > +err_kfree: > + kfree(kbd); > +err_release_mem_region: > + release_mem_region(res->start, resource_size(res)); > + > + return ret; > +} > + > +static int spear_kbd_remove(struct platform_device *pdev) > +{ > + struct spear_kbd *kbd = platform_get_drvdata(pdev); > + struct resource *res; > + u16 val; > + int irq; > + > + val = readw(kbd->io_base + MODE_REG); > + val &= ~START_SCAN; > + writew(val, kbd->io_base + MODE_REG); Need to go into close() method. > + > + /* unregister input device */ > + input_unregister_device(kbd->input); > + input_free_device(kbd->input); No free after unregister. > + > + irq = platform_get_irq(pdev, 0); > + if (irq) > + free_irq(irq, pdev); Can it ever be 0 here? > + > + iounmap(kbd->io_base); > + platform_set_drvdata(pdev, NULL); > + clk_disable(kbd->clk); > + clk_put(kbd->clk); > + kfree(kbd); > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (res) > + release_mem_region(res->start, resource_size(res)); > + > + return 0; > +} > + > +#ifdef CONFIG_PM > +static int spear_kbd_suspend(struct platform_device *pdev, pm_message_t state) > +{ > + struct spear_kbd *kbd = platform_get_drvdata(pdev); > + int irq; > + > + irq = platform_get_irq(pdev, 0); > + clk_disable(kbd->clk); > + if (device_may_wakeup(&pdev->dev)) > + enable_irq_wake(irq); > + > + return 0; > +} > + > +static int spear_kbd_resume(struct platform_device *pdev) > +{ > + struct spear_kbd *kbd = platform_get_drvdata(pdev); > + int irq; > + > + irq = platform_get_irq(pdev, 0); > + if (device_may_wakeup(&pdev->dev)) > + disable_irq_wake(irq); > + clk_enable(kbd->clk); > + > + return 0; > +} > +#else > +#define spear_kbd_suspend NULL > +#define spear_kbd_resume NULL > +#endif > + > +static struct platform_driver spear_kbd_driver = { > + .probe = spear_kbd_probe, > + .remove = spear_kbd_remove, > + .suspend = spear_kbd_suspend, > + .resume = spear_kbd_resume, Switch to dev_pm_ops please. > + .driver = { > + .name = "keyboard", > + .owner = THIS_MODULE, > + }, > +}; > + > +static int __devinit spear_kbd_init(void) > +{ > + return platform_driver_register(&spear_kbd_driver); > +} > +module_init(spear_kbd_init); > + > +static void __exit spear_kbd_exit(void) > +{ > + platform_driver_unregister(&spear_kbd_driver); > +} > +module_exit(spear_kbd_exit); > + > +MODULE_AUTHOR("Rajeev Kumar"); > +MODULE_DESCRIPTION("SPEAr Keyboard Driver"); > +MODULE_LICENSE("GPL"); > -- > 1.7.2.2 > Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 21/74] ST SPEAr : Added keyboard support 2010-08-30 16:48 ` Dmitry Torokhov @ 2010-09-01 5:23 ` rajeev 2010-09-01 5:41 ` rajeev 2010-09-01 6:31 ` Dmitry Torokhov 0 siblings, 2 replies; 6+ messages in thread From: rajeev @ 2010-09-01 5:23 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pratyush ANAND, Viresh KUMAR, Vipul Kumar SAMAR, Bhupesh SHARMA, Armando VISCONTI, linux-input@vger.kernel.org, Vipin KUMAR, Shiraz HASHIM, Deepak SIKRI, linux-arm-kernel@lists.infradead.org Hi Dmitry Please find my answers embedded below. On 8/30/2010 10:18 PM, Dmitry Torokhov wrote: > Hi Rajeev, > [snip...] > > First of all, please split platform modifications from the driver itself > (as I care about the latter but less about the former). > Will be done. >> diff --git a/arch/arm/mach-spear13xx/clock.c b/arch/arm/mach-spear13xx/clock.c >> index 9252940..c48b779 100644 [snip...] >> +++ b/arch/arm/plat-spear/include/plat/keyboard.h >> @@ -0,0 +1,154 @@ >> +/* >> + * arch/arm/plat-spear/include/plat/keyboard.h >> + * >> + * Copyright (C) 2010 ST Microelectronics >> + * Rajeev Kumar<rajeev-dlh.kumar@st.com> >> + * >> + * This file is licensed under the terms of the GNU General Public >> + * License version 2. This program is licensed "as is" without any >> + * warranty of any kind, whether express or implied. >> + */ >> + >> +#ifndef __PLAT_KEYBOARD_H >> +#define __PLAT_KEYBOARD_H >> + >> +#include <linux/bitops.h> >> +#include <linux/input.h> >> +#include <mach/misc_regs.h> >> + >> +#define KEY(row, col, val) (((row) << 28 | ((col) << 24) | (val))) >> + > > Please use definitions from include/linux/input/matrix_keypad.h > Ok. I will check it and modify. >> +#define DECLARE_KEYMAP(name) \ >> +int name[] = {\ >> + KEY(0, 0, KEY_ESC), \ >> + KEY(0, 1, KEY_1), \ >> + KEY(0, 2, KEY_2), \ >> + KEY(0, 3, KEY_3), \ >> + KEY(0, 4, KEY_4), \ >> + KEY(0, 5, KEY_5), \ >> + KEY(0, 6, KEY_6), \ >> + KEY(0, 7, KEY_7), \ >> + KEY(0, 8, KEY_8), \ >> + KEY(1, 0, KEY_9), \ >> + KEY(1, 1, KEY_MINUS), \ >> + KEY(1, 2, KEY_EQUAL), \ >> + KEY(1, 3, KEY_BACKSPACE), \ >> + KEY(1, 4, KEY_TAB), \ >> + KEY(1, 5, KEY_Q), \ >> + KEY(1, 6, KEY_W), \ >> + KEY(1, 7, KEY_E), \ >> + KEY(1, 8, KEY_R), \ >> + KEY(2, 0, KEY_T), \ >> + KEY(2, 1, KEY_Y), \ >> + KEY(2, 2, KEY_U), \ >> + KEY(2, 3, KEY_I), \ >> + KEY(2, 4, KEY_O), \ >> + KEY(2, 5, KEY_P), \ >> + KEY(2, 6, KEY_LEFTBRACE), \ >> + KEY(2, 7, KEY_RIGHTBRACE), \ >> + KEY(2, 8, KEY_ENTER), \ >> + KEY(3, 0, KEY_LEFTCTRL), \ >> + KEY(3, 1, KEY_A), \ >> + KEY(3, 2, KEY_S), \ >> + KEY(3, 3, KEY_D), \ >> + KEY(3, 4, KEY_F), \ >> + KEY(3, 5, KEY_G), \ >> + KEY(3, 6, KEY_H), \ >> + KEY(3, 7, KEY_J), \ >> + KEY(3, 8, KEY_K), \ >> + KEY(4, 0, KEY_L), \ >> + KEY(4, 1, KEY_SEMICOLON), \ >> + KEY(4, 2, KEY_APOSTROPHE), \ >> + KEY(4, 3, KEY_GRAVE), \ >> + KEY(4, 4, KEY_LEFTSHIFT), \ >> + KEY(4, 5, KEY_BACKSLASH), \ >> + KEY(4, 6, KEY_Z), \ >> + KEY(4, 7, KEY_X), \ >> + KEY(4, 8, KEY_C), \ >> + KEY(4, 0, KEY_L), \ >> + KEY(4, 1, KEY_SEMICOLON), \ >> + KEY(4, 2, KEY_APOSTROPHE), \ >> + KEY(4, 3, KEY_GRAVE), \ >> + KEY(4, 4, KEY_LEFTSHIFT), \ >> + KEY(4, 5, KEY_BACKSLASH), \ >> + KEY(4, 6, KEY_Z), \ >> + KEY(4, 7, KEY_X), \ >> + KEY(4, 8, KEY_C), \ >> + KEY(4, 0, KEY_L), \ >> + KEY(4, 1, KEY_SEMICOLON), \ >> + KEY(4, 2, KEY_APOSTROPHE), \ >> + KEY(4, 3, KEY_GRAVE), \ >> + KEY(4, 4, KEY_LEFTSHIFT), \ >> + KEY(4, 5, KEY_BACKSLASH), \ >> + KEY(4, 6, KEY_Z), \ >> + KEY(4, 7, KEY_X), \ >> + KEY(4, 8, KEY_C), \ >> + KEY(5, 0, KEY_V), \ >> + KEY(5, 1, KEY_B), \ >> + KEY(5, 2, KEY_N), \ >> + KEY(5, 3, KEY_M), \ >> + KEY(5, 4, KEY_COMMA), \ >> + KEY(5, 5, KEY_DOT), \ >> + KEY(5, 6, KEY_SLASH), \ >> + KEY(5, 7, KEY_RIGHTSHIFT), \ >> + KEY(5, 8, KEY_KPASTERISK), \ >> + KEY(6, 0, KEY_LEFTALT), \ >> + KEY(6, 1, KEY_SPACE), \ >> + KEY(6, 2, KEY_CAPSLOCK), \ >> + KEY(6, 3, KEY_F1), \ >> + KEY(6, 4, KEY_F2), \ >> + KEY(6, 5, KEY_F3), \ >> + KEY(6, 6, KEY_F4), \ >> + KEY(6, 7, KEY_F5), \ >> + KEY(6, 8, KEY_F6), \ >> + KEY(7, 0, KEY_F7), \ >> + KEY(7, 1, KEY_F8), \ >> + KEY(7, 2, KEY_F9), \ >> + KEY(7, 3, KEY_F10), \ >> + KEY(7, 4, KEY_NUMLOCK), \ >> + KEY(7, 5, KEY_SCROLLLOCK), \ >> + KEY(7, 6, KEY_KP7), \ >> + KEY(7, 7, KEY_KP8), \ >> + KEY(7, 8, KEY_KP9), \ >> + KEY(8, 0, KEY_KPMINUS), \ >> + KEY(8, 1, KEY_KP4), \ >> + KEY(8, 2, KEY_KP5), \ >> + KEY(8, 3, KEY_KP6), \ >> + KEY(8, 4, KEY_KPPLUS), \ >> + KEY(8, 5, KEY_KP1), \ >> + KEY(8, 6, KEY_KP2), \ >> + KEY(8, 7, KEY_KP3), \ >> + KEY(8, 8, KEY_KP0), \ >> +}; > > Hm, I'd expect this to be in particular board code, not in the header > file. > Currently we have support for evaluation boards only and all of them will have this structure in their board source files. In order to remove redundant code we kept it in plat/keyboard.h. >> +/** >> + * struct kbd_platform_data - keymap for spear keyboards >> + * keymap: pointer to array of values encoded with KEY() macro representing >> + * keymap >> + * keymapsize: number of entries (initialized) in this keymap >> + * rep: current values for autorepeat parameters >> + * >> + * This structure is supposed to be used by platform code to supply >> + * keymaps to drivers that implement keyboards. >> + */ >> +struct kbd_platform_data { >> + int *keymap; >> + unsigned int keymapsize; >> + unsigned int rep:1; > > Bool please. > OK >> +}; >> + >> +/* This function is used to set platform data field of pdev->dev */ >> +static inline void >> +kbd_set_plat_data(struct platform_device *pdev, struct kbd_platform_data *data) >> +{ >> +#ifdef CONFIG_ARCH_SPEAR13XX >> +#define KBD_PAD_SEL (BIT(18) | BIT(20) | BIT(22) | BIT(24) | BIT(26)) >> + >> + u32 val; >> + /* Workaround:Setting bit for routing it to the IP */ >> + val = readl(PAD_FUNCTION_EN_2); >> + val &= ~KBD_PAD_SEL; >> + writel(val, PAD_FUNCTION_EN_2); > > Why does it belong here? > Sorry. It will be handled in padmux framework. >> +#endif >> + pdev->dev.platform_data = data; >> +} >> +#endif /* __PLAT_KEYBOARD_H */ >> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig >> index 9cc488d..2d7e3a8 100644 >> --- a/drivers/input/keyboard/Kconfig >> +++ b/drivers/input/keyboard/Kconfig >> @@ -424,6 +424,14 @@ config KEYBOARD_OMAP >> To compile this driver as a module, choose M here: the >> module will be called omap-keypad. >> >> +config KEYBOARD_SPEAR >> + tristate "ST SPEAR keyboard support" > > No arch/platform dependencies? > Will add them. >> + help [snip...] >> +struct spear_kbd { >> + struct input_dev *input; >> + void __iomem *io_base; /* Keyboard Base Address */ >> + struct clk *clk; >> + int *keymap; > > You need a copy of keymap here so that userspace can modify it safely > via EVIOCSKEYCODE. > We have one copy of struct spear_kbd per device structure. I think it will be fine if we change this structure only. And so don't need to copy another structure in driver. >> +}; >> +/* TODO: Need to optimize this function */ >> +static inline int get_key_value(struct spear_kbd *dev, int row, int col) >> +{ >> + int i, key; >> + int *keymap = dev->keymap; >> + >> + key = KEY(row, col, 0); >> + for (i = 0; keymap[i] != 0; i++) >> + if ((keymap[i] & KEY_MASK) == key) >> + return keymap[i] & KEY_VALUE; >> + return -ENOKEY; >> +} >> + >> +static irqreturn_t spear_kbd_interrupt(int irq, void *dev_id) >> +{ >> + struct spear_kbd *dev = dev_id; >> + static u8 last_key ; >> + static u8 last_event; > > No statics please, pull it into spear_kbd structure. > Ok >> + int key; >> + u8 sts, val = 0; >> + >> + if (dev == NULL) { > > How can it be? > Not needed, will be fixed. >> + pr_err("Keyboard: Invalid dev_id in irq handler\n"); >> + return IRQ_NONE; >> + } >> + >> + sts = readb(dev->io_base + STATUS_REG); >> + if (sts & DATA_AVAIL) { >> + /* following reads active (row, col) pair */ >> + val = readb(dev->io_base + DATA_REG); >> + key = get_key_value(dev, (val & ROW_MASK)>>ROW_SHIFT, (val >> + & COLUMN_MASK)); >> + >> + /* valid key press event */ >> + if (key >= 0) { >> + if (last_event == 1) { >> + /* check if we missed a release event */ >> + input_report_key(dev->input, last_key, >> + !last_event); >> + } >> + /* notify key press */ >> + last_event = 1; >> + last_key = key; >> + input_report_key(dev->input, key, last_event); >> + } else { >> + /* notify key release */ >> + last_event = 0; >> + input_report_key(dev->input, last_key, last_event); >> + } >> + } else > > Don't you need to clear interrupt here? You got it somehow... > I can think of only one way in which this handler is called for some other device interrupt, that is when this irq line is shared between peripherals. In that case if interrupt is not meant for kbd then kbd shouldn't clear it. Isn't it fine??? >> + return IRQ_NONE; >> + >> + /* clear interrupt */ >> + writeb(0, dev->io_base + STATUS_REG); >> + >> + return IRQ_HANDLED; >> +} >> + >> +static int __init spear_kbd_probe(struct platform_device *pdev) >> +{ [snip...] >> + ret = request_irq(irq, spear_kbd_interrupt, 0, "keyboard", >> + kbd); >> + if (ret) { >> + dev_err(&pdev->dev, "request_irq fail\n"); >> + goto err_iounmap; >> + } >> + > > Are you 100% sure the device is shut off at this point? Because if it > isn't you risk to get interrupt before you allocated your input device. > I will check and place it at correct place. >> + >> + /* start key scan */ >> + val |= START_SCAN; >> + writew(val, kbd->io_base + MODE_REG); > > This should go into open() method. > OK. >> +static int spear_kbd_remove(struct platform_device *pdev) >> +{ >> + struct spear_kbd *kbd = platform_get_drvdata(pdev); >> + struct resource *res; >> + u16 val; >> + int irq; >> + >> + val = readw(kbd->io_base + MODE_REG); >> + val &= ~START_SCAN; >> + writew(val, kbd->io_base + MODE_REG); > > Need to go into close() method. > OK. >> + >> + /* unregister input device */ >> + input_unregister_device(kbd->input); >> + input_free_device(kbd->input); > > No free after unregister. > OK. >> + >> + irq = platform_get_irq(pdev, 0); >> + if (irq) >> + free_irq(irq, pdev); > > Can it ever be 0 here? > No. Will correct. >> + >> +static struct platform_driver spear_kbd_driver = { >> + .probe = spear_kbd_probe, >> + .remove = spear_kbd_remove, >> + .suspend = spear_kbd_suspend, >> + .resume = spear_kbd_resume, > > Switch to dev_pm_ops please. > Ok. Thanks, rajeev. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 21/74] ST SPEAr : Added keyboard support 2010-09-01 5:23 ` rajeev @ 2010-09-01 5:41 ` rajeev 2010-09-01 6:31 ` Dmitry Torokhov 1 sibling, 0 replies; 6+ messages in thread From: rajeev @ 2010-09-01 5:41 UTC (permalink / raw) To: Dmitry Torokhov Cc: Viresh KUMAR, linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org, Shiraz HASHIM, Vipin KUMAR, Deepak SIKRI, Armando VISCONTI, Vipul Kumar SAMAR, Pratyush ANAND, Bhupesh SHARMA On 9/1/2010 10:53 AM, rajeev wrote: >>> +struct spear_kbd { >>> >> + struct input_dev *input; >>> >> + void __iomem *io_base; /* Keyboard Base Address */ >>> >> + struct clk *clk; >>> >> + int *keymap; >> > >> > You need a copy of keymap here so that userspace can modify it safely >> > via EVIOCSKEYCODE. >> > > We have one copy of struct spear_kbd per device structure. I think it > will be fine if we change this structure only. And so don't need to copy > another structure in driver. > Sorry!! I wanted to say "keymap array" not "struct spear_kbd" in above explanation. rajeev. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 21/74] ST SPEAr : Added keyboard support 2010-09-01 5:23 ` rajeev 2010-09-01 5:41 ` rajeev @ 2010-09-01 6:31 ` Dmitry Torokhov 2010-09-01 7:01 ` viresh kumar 1 sibling, 1 reply; 6+ messages in thread From: Dmitry Torokhov @ 2010-09-01 6:31 UTC (permalink / raw) To: rajeev Cc: Viresh KUMAR, linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org, Shiraz HASHIM, Vipin KUMAR, Deepak SIKRI, Armando VISCONTI, Vipul Kumar SAMAR, Pratyush ANAND, Bhupesh SHARMA On Wed, Sep 01, 2010 at 10:53:43AM +0530, rajeev wrote: > Hi Dmitry > > Please find my answers embedded below. > > On 8/30/2010 10:18 PM, Dmitry Torokhov wrote: > > Hi Rajeev, > > > > [snip...] > > > > > First of all, please split platform modifications from the driver itself > > (as I care about the latter but less about the former). > > > > Will be done. > > >> diff --git a/arch/arm/mach-spear13xx/clock.c b/arch/arm/mach-spear13xx/clock.c > >> index 9252940..c48b779 100644 > > [snip...] > > >> +++ b/arch/arm/plat-spear/include/plat/keyboard.h > >> @@ -0,0 +1,154 @@ > >> +/* > >> + * arch/arm/plat-spear/include/plat/keyboard.h > >> + * > >> + * Copyright (C) 2010 ST Microelectronics > >> + * Rajeev Kumar<rajeev-dlh.kumar@st.com> > >> + * > >> + * This file is licensed under the terms of the GNU General Public > >> + * License version 2. This program is licensed "as is" without any > >> + * warranty of any kind, whether express or implied. > >> + */ > >> + > >> +#ifndef __PLAT_KEYBOARD_H > >> +#define __PLAT_KEYBOARD_H > >> + > >> +#include <linux/bitops.h> > >> +#include <linux/input.h> > >> +#include <mach/misc_regs.h> > >> + > >> +#define KEY(row, col, val) (((row) << 28 | ((col) << 24) | (val))) > >> + > > > > Please use definitions from include/linux/input/matrix_keypad.h > > > > Ok. I will check it and modify. > > >> +#define DECLARE_KEYMAP(name) \ > >> +int name[] = {\ > >> + KEY(0, 0, KEY_ESC), \ > >> + KEY(0, 1, KEY_1), \ > >> + KEY(0, 2, KEY_2), \ > >> + KEY(0, 3, KEY_3), \ > >> + KEY(0, 4, KEY_4), \ > >> + KEY(0, 5, KEY_5), \ > >> + KEY(0, 6, KEY_6), \ > >> + KEY(0, 7, KEY_7), \ > >> + KEY(0, 8, KEY_8), \ > >> + KEY(1, 0, KEY_9), \ > >> + KEY(1, 1, KEY_MINUS), \ > >> + KEY(1, 2, KEY_EQUAL), \ > >> + KEY(1, 3, KEY_BACKSPACE), \ > >> + KEY(1, 4, KEY_TAB), \ > >> + KEY(1, 5, KEY_Q), \ > >> + KEY(1, 6, KEY_W), \ > >> + KEY(1, 7, KEY_E), \ > >> + KEY(1, 8, KEY_R), \ > >> + KEY(2, 0, KEY_T), \ > >> + KEY(2, 1, KEY_Y), \ > >> + KEY(2, 2, KEY_U), \ > >> + KEY(2, 3, KEY_I), \ > >> + KEY(2, 4, KEY_O), \ > >> + KEY(2, 5, KEY_P), \ > >> + KEY(2, 6, KEY_LEFTBRACE), \ > >> + KEY(2, 7, KEY_RIGHTBRACE), \ > >> + KEY(2, 8, KEY_ENTER), \ > >> + KEY(3, 0, KEY_LEFTCTRL), \ > >> + KEY(3, 1, KEY_A), \ > >> + KEY(3, 2, KEY_S), \ > >> + KEY(3, 3, KEY_D), \ > >> + KEY(3, 4, KEY_F), \ > >> + KEY(3, 5, KEY_G), \ > >> + KEY(3, 6, KEY_H), \ > >> + KEY(3, 7, KEY_J), \ > >> + KEY(3, 8, KEY_K), \ > >> + KEY(4, 0, KEY_L), \ > >> + KEY(4, 1, KEY_SEMICOLON), \ > >> + KEY(4, 2, KEY_APOSTROPHE), \ > >> + KEY(4, 3, KEY_GRAVE), \ > >> + KEY(4, 4, KEY_LEFTSHIFT), \ > >> + KEY(4, 5, KEY_BACKSLASH), \ > >> + KEY(4, 6, KEY_Z), \ > >> + KEY(4, 7, KEY_X), \ > >> + KEY(4, 8, KEY_C), \ > >> + KEY(4, 0, KEY_L), \ > >> + KEY(4, 1, KEY_SEMICOLON), \ > >> + KEY(4, 2, KEY_APOSTROPHE), \ > >> + KEY(4, 3, KEY_GRAVE), \ > >> + KEY(4, 4, KEY_LEFTSHIFT), \ > >> + KEY(4, 5, KEY_BACKSLASH), \ > >> + KEY(4, 6, KEY_Z), \ > >> + KEY(4, 7, KEY_X), \ > >> + KEY(4, 8, KEY_C), \ > >> + KEY(4, 0, KEY_L), \ > >> + KEY(4, 1, KEY_SEMICOLON), \ > >> + KEY(4, 2, KEY_APOSTROPHE), \ > >> + KEY(4, 3, KEY_GRAVE), \ > >> + KEY(4, 4, KEY_LEFTSHIFT), \ > >> + KEY(4, 5, KEY_BACKSLASH), \ > >> + KEY(4, 6, KEY_Z), \ > >> + KEY(4, 7, KEY_X), \ > >> + KEY(4, 8, KEY_C), \ > >> + KEY(5, 0, KEY_V), \ > >> + KEY(5, 1, KEY_B), \ > >> + KEY(5, 2, KEY_N), \ > >> + KEY(5, 3, KEY_M), \ > >> + KEY(5, 4, KEY_COMMA), \ > >> + KEY(5, 5, KEY_DOT), \ > >> + KEY(5, 6, KEY_SLASH), \ > >> + KEY(5, 7, KEY_RIGHTSHIFT), \ > >> + KEY(5, 8, KEY_KPASTERISK), \ > >> + KEY(6, 0, KEY_LEFTALT), \ > >> + KEY(6, 1, KEY_SPACE), \ > >> + KEY(6, 2, KEY_CAPSLOCK), \ > >> + KEY(6, 3, KEY_F1), \ > >> + KEY(6, 4, KEY_F2), \ > >> + KEY(6, 5, KEY_F3), \ > >> + KEY(6, 6, KEY_F4), \ > >> + KEY(6, 7, KEY_F5), \ > >> + KEY(6, 8, KEY_F6), \ > >> + KEY(7, 0, KEY_F7), \ > >> + KEY(7, 1, KEY_F8), \ > >> + KEY(7, 2, KEY_F9), \ > >> + KEY(7, 3, KEY_F10), \ > >> + KEY(7, 4, KEY_NUMLOCK), \ > >> + KEY(7, 5, KEY_SCROLLLOCK), \ > >> + KEY(7, 6, KEY_KP7), \ > >> + KEY(7, 7, KEY_KP8), \ > >> + KEY(7, 8, KEY_KP9), \ > >> + KEY(8, 0, KEY_KPMINUS), \ > >> + KEY(8, 1, KEY_KP4), \ > >> + KEY(8, 2, KEY_KP5), \ > >> + KEY(8, 3, KEY_KP6), \ > >> + KEY(8, 4, KEY_KPPLUS), \ > >> + KEY(8, 5, KEY_KP1), \ > >> + KEY(8, 6, KEY_KP2), \ > >> + KEY(8, 7, KEY_KP3), \ > >> + KEY(8, 8, KEY_KP0), \ > >> +}; > > > > Hm, I'd expect this to be in particular board code, not in the header > > file. > > > > Currently we have support for evaluation boards only and all of them will have > this structure in their board source files. In order to remove redundant code we > kept it in plat/keyboard.h. > So call it "spear_default_keymap" and put it right into the driver? And then allow platform code override it. > >> +/** > >> + * struct kbd_platform_data - keymap for spear keyboards > >> + * keymap: pointer to array of values encoded with KEY() macro representing > >> + * keymap > >> + * keymapsize: number of entries (initialized) in this keymap > >> + * rep: current values for autorepeat parameters > >> + * > >> + * This structure is supposed to be used by platform code to supply > >> + * keymaps to drivers that implement keyboards. > >> + */ > >> +struct kbd_platform_data { > >> + int *keymap; > >> + unsigned int keymapsize; > >> + unsigned int rep:1; > > > > Bool please. > > > > OK > > >> +}; > >> + > >> +/* This function is used to set platform data field of pdev->dev */ > >> +static inline void > >> +kbd_set_plat_data(struct platform_device *pdev, struct kbd_platform_data *data) > >> +{ > >> +#ifdef CONFIG_ARCH_SPEAR13XX > >> +#define KBD_PAD_SEL (BIT(18) | BIT(20) | BIT(22) | BIT(24) | BIT(26)) > >> + > >> + u32 val; > >> + /* Workaround:Setting bit for routing it to the IP */ > >> + val = readl(PAD_FUNCTION_EN_2); > >> + val &= ~KBD_PAD_SEL; > >> + writel(val, PAD_FUNCTION_EN_2); > > > > Why does it belong here? > > > > Sorry. It will be handled in padmux framework. > > >> +#endif > >> + pdev->dev.platform_data = data; > >> +} > >> +#endif /* __PLAT_KEYBOARD_H */ > >> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig > >> index 9cc488d..2d7e3a8 100644 > >> --- a/drivers/input/keyboard/Kconfig > >> +++ b/drivers/input/keyboard/Kconfig > >> @@ -424,6 +424,14 @@ config KEYBOARD_OMAP > >> To compile this driver as a module, choose M here: the > >> module will be called omap-keypad. > >> > >> +config KEYBOARD_SPEAR > >> + tristate "ST SPEAR keyboard support" > > > > No arch/platform dependencies? > > > > Will add them. > > >> + help > > [snip...] > > >> +struct spear_kbd { > >> + struct input_dev *input; > >> + void __iomem *io_base; /* Keyboard Base Address */ > >> + struct clk *clk; > >> + int *keymap; > > > > You need a copy of keymap here so that userspace can modify it safely > > via EVIOCSKEYCODE. > > > > We have one copy of struct spear_kbd per device structure. I think it > will be fine if we change this structure only. And so don't need to copy > another structure in driver. Normally such platform data should be declared as 'const' and drivers should not modify such data. Also unbinding device from driver and rebinding later should restore the device into pristine state. Having a copy of keymap in spear_kbd allows to achieve such behavior. > > >> +}; > >> +/* TODO: Need to optimize this function */ > >> +static inline int get_key_value(struct spear_kbd *dev, int row, int col) > >> +{ > >> + int i, key; > >> + int *keymap = dev->keymap; > >> + > >> + key = KEY(row, col, 0); > >> + for (i = 0; keymap[i] != 0; i++) > >> + if ((keymap[i] & KEY_MASK) == key) > >> + return keymap[i] & KEY_VALUE; > >> + return -ENOKEY; > >> +} > >> + > >> +static irqreturn_t spear_kbd_interrupt(int irq, void *dev_id) > >> +{ > >> + struct spear_kbd *dev = dev_id; > >> + static u8 last_key ; > >> + static u8 last_event; > > > > No statics please, pull it into spear_kbd structure. > > > > Ok > > >> + int key; > >> + u8 sts, val = 0; > >> + > >> + if (dev == NULL) { > > > > How can it be? > > > > Not needed, will be fixed. > > >> + pr_err("Keyboard: Invalid dev_id in irq handler\n"); > >> + return IRQ_NONE; > >> + } > >> + > >> + sts = readb(dev->io_base + STATUS_REG); > >> + if (sts & DATA_AVAIL) { > >> + /* following reads active (row, col) pair */ > >> + val = readb(dev->io_base + DATA_REG); > >> + key = get_key_value(dev, (val & ROW_MASK)>>ROW_SHIFT, (val > >> + & COLUMN_MASK)); > >> + > >> + /* valid key press event */ > >> + if (key >= 0) { > >> + if (last_event == 1) { > >> + /* check if we missed a release event */ > >> + input_report_key(dev->input, last_key, > >> + !last_event); > >> + } > >> + /* notify key press */ > >> + last_event = 1; > >> + last_key = key; > >> + input_report_key(dev->input, key, last_event); > >> + } else { > >> + /* notify key release */ > >> + last_event = 0; > >> + input_report_key(dev->input, last_key, last_event); > >> + } > >> + } else > > > > Don't you need to clear interrupt here? You got it somehow... > > > > I can think of only one way in which this handler is called for some other > device interrupt, that is when this irq line is shared between peripherals. > In that case if interrupt is not meant for kbd then kbd shouldn't clear it. You would be clearing IRQ condition _in your device_ so that sould not affect the other device in any way. > Isn't it fine??? I guess it should be OK. > > >> + return IRQ_NONE; > >> + > >> + /* clear interrupt */ > >> + writeb(0, dev->io_base + STATUS_REG); > >> + > >> + return IRQ_HANDLED; > >> +} > >> + > >> +static int __init spear_kbd_probe(struct platform_device *pdev) > >> +{ > > [snip...] > > >> + ret = request_irq(irq, spear_kbd_interrupt, 0, "keyboard", > >> + kbd); > >> + if (ret) { > >> + dev_err(&pdev->dev, "request_irq fail\n"); > >> + goto err_iounmap; > >> + } > >> + > > > > Are you 100% sure the device is shut off at this point? Because if it > > isn't you risk to get interrupt before you allocated your input device. > > > > I will check and place it at correct place. > > >> + > >> + /* start key scan */ > >> + val |= START_SCAN; > >> + writew(val, kbd->io_base + MODE_REG); > > > > This should go into open() method. > > > > OK. > > >> +static int spear_kbd_remove(struct platform_device *pdev) > >> +{ > >> + struct spear_kbd *kbd = platform_get_drvdata(pdev); > >> + struct resource *res; > >> + u16 val; > >> + int irq; > >> + > >> + val = readw(kbd->io_base + MODE_REG); > >> + val &= ~START_SCAN; > >> + writew(val, kbd->io_base + MODE_REG); > > > > Need to go into close() method. > > > > OK. > > >> + > >> + /* unregister input device */ > >> + input_unregister_device(kbd->input); > >> + input_free_device(kbd->input); > > > > No free after unregister. > > > > OK. > > >> + > >> + irq = platform_get_irq(pdev, 0); > >> + if (irq) > >> + free_irq(irq, pdev); > > > > Can it ever be 0 here? > > > > No. Will correct. > > > >> + > >> +static struct platform_driver spear_kbd_driver = { > >> + .probe = spear_kbd_probe, > >> + .remove = spear_kbd_remove, > >> + .suspend = spear_kbd_suspend, > >> + .resume = spear_kbd_resume, > > > > Switch to dev_pm_ops please. > > > > Ok. > > > Thanks, > rajeev. > -- Dmitry ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 21/74] ST SPEAr : Added keyboard support 2010-09-01 6:31 ` Dmitry Torokhov @ 2010-09-01 7:01 ` viresh kumar 0 siblings, 0 replies; 6+ messages in thread From: viresh kumar @ 2010-09-01 7:01 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pratyush ANAND, Rajeev KUMAR, Bhupesh SHARMA, Armando VISCONTI, linux-input@vger.kernel.org, Vipin KUMAR, Shiraz HASHIM, Vipul Kumar SAMAR, Deepak SIKRI, linux-arm-kernel@lists.infradead.org On 9/1/2010 12:01 PM, Dmitry Torokhov wrote: >>>> +#define DECLARE_KEYMAP(name) \ >>>> > >> +int name[] = {\ >>>> > >> + KEY(0, 0, KEY_ESC), \ [snip...] >>>> > >> + KEY(8, 6, KEY_KP2), \ >>>> > >> + KEY(8, 7, KEY_KP3), \ >>>> > >> + KEY(8, 8, KEY_KP0), \ >>>> > >> +}; >>> > > >>> > > Hm, I'd expect this to be in particular board code, not in the header >>> > > file. >>> > > >> > >> > Currently we have support for evaluation boards only and all of them will have >> > this structure in their board source files. In order to remove redundant code we >> > kept it in plat/keyboard.h. >> > > So call it "spear_default_keymap" and put it right into the driver? And > then allow platform code override it. > Renaming is fine but I would still like to keep spear_default_keymap in platform files instead of driver. >>>> > >> +struct spear_kbd { >>>> > >> + struct input_dev *input; >>>> > >> + void __iomem *io_base; /* Keyboard Base Address */ >>>> > >> + struct clk *clk; >>>> > >> + int *keymap; >>> > > >>> > > You need a copy of keymap here so that userspace can modify it safely >>> > > via EVIOCSKEYCODE. >>> > > >> > >> > We have one copy of struct spear_kbd per device structure. I think it >> > will be fine if we change this structure only. And so don't need to copy >> > another structure in driver. > Normally such platform data should be declared as 'const' and drivers > should not modify such data. Also unbinding device from driver and > rebinding later should restore the device into pristine state. Having a > copy of keymap in spear_kbd allows to achieve such behavior. > OK. viresh. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-01 7:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1283161023.git.viresh.kumar@st.com>
2010-08-30 10:43 ` [PATCH 21/74] ST SPEAr : Added keyboard support Viresh KUMAR
2010-08-30 16:48 ` Dmitry Torokhov
2010-09-01 5:23 ` rajeev
2010-09-01 5:41 ` rajeev
2010-09-01 6:31 ` Dmitry Torokhov
2010-09-01 7:01 ` viresh kumar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).