new locomo driver set

LoCoMo is a GA used on Sharp Zaurus SL-5x00. Current driver does has several design issues (special bus instead of platform bus, doesn't use mfd-core, etc).

Implement 'core' parts of locomo support as an mfd driver.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---
 drivers/mfd/Kconfig        |  10 ++
 drivers/mfd/Makefile       |   1 +
 drivers/mfd/locomo.c       | 356 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/locomo.h | 173 ++++++++++++++++++++++
 4 files changed, 540 insertions(+)
 create mode 100644 drivers/mfd/locomo.c
 create mode 100644 include/linux/mfd/locomo.h

drivers/mfd/Kconfig +0 -0

index d5ad04d..8c33940 100644
@@ -1430,6 +1430,16 @@ config MFD_STW481X
 	  in various ST Microelectronics and ST-Ericsson embedded
 	  Nomadik series.

drivers/mfd/Kconfig +0 -0

 menu "Multimedia Capabilities Port drivers"
 	depends on ARCH_SA1100
 

drivers/mfd/Makefile +1 -0

index 0e5cfeb..6c23b73 100644
@@ -181,6 +181,7 @@ obj-$(CONFIG_MFD_HI6421_PMIC)	+= hi6421-pmic-core.o
 obj-$(CONFIG_MFD_DLN2)		+= dln2.o
 obj-$(CONFIG_MFD_RT5033)	+= rt5033.o
 obj-$(CONFIG_MFD_SKY81452)	+= sky81452.o
+obj-$(CONFIG_MFD_LOCOMO)	+= locomo.o
 
 intel-soc-pmic-objs		:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o

drivers/mfd/locomo.c new file +4 -0

new file mode 100644
index 0000000..f981c94
@@ -0,0 +1,356 @@
+/*
+ * Sharp LoCoMo support
+ *
+ * Based on old driver at arch/arm/common/locomo.c

drivers/mfd/locomo.c new file +10 -0

+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>

drivers/mfd/locomo.c new file +9 -0

+/* the following is the overall data for the locomo chip */
+struct locomo {
+	struct device *dev;
+	unsigned int irq;
+	spinlock_t lock;
+	struct irq_domain *domain;
+	struct regmap *regmap;
+};
+

drivers/mfd/locomo.c new file +30 -0

+static struct resource locomo_lt_resources[] = {
+	DEFINE_RES_IRQ(IRQ_LOCOMO_LT),
+};
+
+static struct resource locomo_spi_resources[] = {
+	DEFINE_RES_IRQ(IRQ_LOCOMO_SPI),
+};
+
+/* Filled in locomo_probe() function. */
+static struct locomo_lcd_platform_data locomo_lcd_pdata;
+
+static struct mfd_cell locomo_cells[] = {
+	{
+		.name = "locomo-kbd",
+		.resources = locomo_kbd_resources,
+		.num_resources = ARRAY_SIZE(locomo_kbd_resources),
+	},
+	{
+		.name = "locomo-gpio",
+		.resources = locomo_gpio_resources,
+		.num_resources = ARRAY_SIZE(locomo_gpio_resources),
+		.platform_data = &locomo_gpio_pdata,
+		.pdata_size = sizeof(locomo_gpio_pdata),
+	},
+	{
+		.name = "locomo-lt", /* Long time timer */
+		.resources = locomo_lt_resources,
+		.num_resources = ARRAY_SIZE(locomo_lt_resources),
+	},
+	{

drivers/mfd/locomo.c new file +10 -0

+	{
+		.name = "locomo-lcd",
+		.platform_data = &locomo_lcd_pdata,
+		.pdata_size = sizeof(locomo_lcd_pdata),
+	},
+	{
+		.name = "locomo-i2c",
+	},
+};
+

drivers/mfd/locomo.c new file +25 -0

+	.irq_ack	= locomo_ack_irq,
+	.irq_mask	= locomo_mask_irq,
+	.irq_unmask	= locomo_unmask_irq,
+};
+
+static int locomo_irq_map(struct irq_domain *d, unsigned int virq,
+				irq_hw_number_t hwirq)
+{
+	struct locomo *locomo = d->host_data;
+
+	irq_set_chip_data(virq, locomo);
+	irq_set_chip_and_handler(virq, &locomo_chip,
+				handle_level_irq);
+	set_irq_flags(virq, IRQF_VALID);
+
+	return 0;
+}
+
+static void locomo_irq_unmap(struct irq_domain *d, unsigned int virq)
+{
+	set_irq_flags(virq, 0);
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+

drivers/mfd/locomo.c new file +9 -0

+		return -ENOMEM;
+	}
+
+	/*
+	 * Install handler for IRQ_LOCOMO_HW.
+	 */
+	irq_set_irq_type(lchip->irq, IRQ_TYPE_EDGE_FALLING);
+	irq_set_handler_data(lchip->irq, lchip);
+	irq_set_chained_handler(lchip->irq, locomo_handler);

drivers/mfd/locomo.c new file +9 -0

+	regmap_write(lchip->regmap, LOCOMO_PAIF, 0x00);
+
+	/*
+	 * Original code disabled the clock depending on leds settings
+	 * However we disable leds before suspend, thus it's safe
+	 * to just assume this setting.
+	 */
+	/* CLK32 off */
+	regmap_write(lchip->regmap, LOCOMO_C32K, 0x00);

drivers/mfd/locomo.c new file +4 -0

+	irq_set_chained_handler(lchip->irq, NULL);
+	irq_set_handler_data(lchip->irq, NULL);
+	if (lchip->domain)
+		irq_domain_remove(lchip->domain);

drivers/mfd/locomo.c new file +4 -0

+	if (lchip->domain)
+		irq_domain_remove(lchip->domain);
+
+	return 0;

include/linux/mfd/locomo.h new file +4 -0

+/* Pin status */
+#define LOCOMO_ST	0x04
+
+/* Pin status */

include/linux/mfd/locomo.h new file +20 -0

+/* Touch panel controller */
+#define LOCOMO_ASD	0x20		/* AD start delay */
+#define LOCOMO_HSD	0x28		/* HSYS delay */
+#define LOCOMO_HSC	0x2c		/* HSYS period */
+#define LOCOMO_TADC	0x30		/* tablet ADC clock */
+
+/* Backlight controller: TFT signal */
+#define LOCOMO_TC	0x38		/* TFT control signal */
+#define LOCOMO_CPSD	0x3c		/* CPS delay */
+
+/* Keyboard controller */
+#define LOCOMO_KIB	0x40	/* KIB level */
+#define LOCOMO_KSC	0x44	/* KSTRB control */
+#define LOCOMO_KCMD	0x48	/* KSTRB command */
+#define LOCOMO_KIC	0x4c	/* Key interrupt */
+
+/* Audio clock */
+#define LOCOMO_ACC	0x54	/* Audio clock */
+#define	LOCOMO_ACC_XON		0x80
+#define	LOCOMO_ACC_XEN		0x40

include/linux/mfd/locomo.h new file +4 -0

+#define LOCOMO_SPIMD_RDINV    (1 << 8)	/* invert receive line */
+#define LOCOMO_SPIMD_XON      (1 << 7)	/* enable spi controller clock */
+#define LOCOMO_SPIMD_XEN      (1 << 6)	/* clock bit write enable */
+#define LOCOMO_SPIMD_XSEL     0x0018	/* clock select */

include/linux/mfd/locomo.h new file +65 -0

+#define LOCOMO_SPICT	0x64		/* SPI mode control */
+#define LOCOMO_SPICT_CRC16_7_B	(1 << 15)	/* 0: crc16 1: crc7 */
+#define LOCOMO_SPICT_CRCRX_TX_B	(1 << 14)
+#define LOCOMO_SPICT_CRCRESET_B	(1 << 13)
+#define LOCOMO_SPICT_CEN	(1 << 7)	/* ?? enable */
+#define LOCOMO_SPICT_CS		(1 << 6)	/* chip select */
+#define LOCOMO_SPICT_UNIT16	(1 << 5)	/* 0: 8 bit, 1: 16 bit*/
+#define LOCOMO_SPICT_ALIGNEN	(1 << 2)	/* align transfer enable */
+#define LOCOMO_SPICT_RXWEN	(1 << 1)	/* continuous receive */
+#define LOCOMO_SPICT_RXUEN	(1 << 0)	/* aligned receive */
+
+#define LOCOMO_SPIST	0x68		/* SPI status */
+#define	LOCOMO_SPI_TEND	(1 << 3)	/* Transfer end bit */
+#define	LOCOMO_SPI_REND	(1 << 2)	/* Receive end bit */
+#define	LOCOMO_SPI_RFW	(1 << 1)	/* write buffer bit */
+#define	LOCOMO_SPI_RFR	(1)		/* read buffer bit */
+
+#define LOCOMO_SPIIS	0x70		/* SPI interrupt status */
+#define LOCOMO_SPIWE	0x74		/* SPI interrupt status write enable */
+#define LOCOMO_SPIIE	0x78		/* SPI interrupt enable */
+#define LOCOMO_SPIIR	0x7c		/* SPI interrupt request */
+#define LOCOMO_SPITD	0x80		/* SPI transfer data write */
+#define LOCOMO_SPIRD	0x84		/* SPI receive data read */
+#define LOCOMO_SPITS	0x88		/* SPI transfer data shift */
+#define LOCOMO_SPIRS	0x8C		/* SPI receive data shift */
+
+/* GPIO */
+#define LOCOMO_GPD	0x90	/* GPIO direction */
+#define LOCOMO_GPE	0x94	/* GPIO input enable */
+#define LOCOMO_GPL	0x98	/* GPIO level */
+#define LOCOMO_GPO	0x9c	/* GPIO out data setting */
+#define LOCOMO_GRIE	0xa0	/* GPIO rise detection */
+#define LOCOMO_GFIE	0xa4	/* GPIO fall detection */
+#define LOCOMO_GIS	0xa8	/* GPIO edge detection status */
+#define LOCOMO_GWE	0xac	/* GPIO status write enable */
+#define LOCOMO_GIE	0xb0	/* GPIO interrupt enable */
+#define LOCOMO_GIR	0xb4	/* GPIO interrupt request */
+
+/* Front light adjustment controller */
+#define LOCOMO_ALS	0xc8	/* Adjust light cycle */
+#define LOCOMO_ALS_EN		0x8000
+#define LOCOMO_ALD	0xcc	/* Adjust light duty */
+
+/* PCM audio interface */
+#define LOCOMO_PAIF	0xd0	/* PCM audio interface */
+#define	LOCOMO_PAIF_SCINV	0x20
+#define	LOCOMO_PAIF_SCEN	0x10
+#define	LOCOMO_PAIF_LRCRST	0x08
+#define	LOCOMO_PAIF_LRCEVE	0x04
+#define	LOCOMO_PAIF_LRCINV	0x02
+#define	LOCOMO_PAIF_LRCEN	0x01
+
+/* Long time timer */
+#define LOCOMO_LTC	0xd8		/* LTC interrupt setting */
+#define LOCOMO_LTINT	0xdc		/* LTC interrupt */
+
+/* DAC control signal for LCD (COMADJ ) */
+#define LOCOMO_DAC	0xe0
+/* DAC control */
+#define	LOCOMO_DAC_SCLOEB	0x08	/* SCL pin output data       */
+#define	LOCOMO_DAC_TEST		0x04	/* Test bit                  */
+#define	LOCOMO_DAC_SDA		0x02	/* SDA pin level (read-only) */
+#define	LOCOMO_DAC_SDAOEB	0x01	/* SDA pin output data       */
+
+/* LED controller */

include/linux/mfd/locomo.h new file +1 -0

+#endif

Quoted 9 lines

LoCoMo is a GA used on Sharp Zaurus SL-5x00. Current driver does has several design issues (special bus instead of platform bus, doesn't use mfd-core, etc).

Implement 'core' parts of locomo support as an mfd driver.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---

Quoted 14 lines

drivers/mfd/locomo.c new file +13 -0

+/* the following is the overall data for the locomo chip */
+struct locomo {
+     struct device *dev;
+     unsigned int irq;
+     spinlock_t lock;
+     struct irq_domain *domain;
+     struct regmap *regmap;
+};
+
+static struct resource locomo_kbd_resources[] = {
+     DEFINE_RES_IRQ(IRQ_LOCOMO_KEY),
+};
+

drivers/mfd/locomo.c line 54

Quoted 6 lines

drivers/mfd/locomo.c new file +6 -0

+static struct resource locomo_gpio_resources[] = {
+     DEFINE_RES_IRQ(IRQ_LOCOMO_GPIO),
+};
+
+/* Filled in locomo_probe() function. */
+static struct locomo_gpio_platform_data locomo_gpio_pdata;

I'd prefer you didn't use globals for this.

Quoted 30 lines

drivers/mfd/locomo.c new file +30 -0

+static struct resource locomo_lt_resources[] = {
+     DEFINE_RES_IRQ(IRQ_LOCOMO_LT),
+};
+
+static struct resource locomo_spi_resources[] = {
+     DEFINE_RES_IRQ(IRQ_LOCOMO_SPI),
+};
+
+/* Filled in locomo_probe() function. */
+static struct locomo_lcd_platform_data locomo_lcd_pdata;
+
+static struct mfd_cell locomo_cells[] = {
+     {
+             .name = "locomo-kbd",
+             .resources = locomo_kbd_resources,
+             .num_resources = ARRAY_SIZE(locomo_kbd_resources),
+     },
+     {
+             .name = "locomo-gpio",
+             .resources = locomo_gpio_resources,
+             .num_resources = ARRAY_SIZE(locomo_gpio_resources),
+             .platform_data = &locomo_gpio_pdata,
+             .pdata_size = sizeof(locomo_gpio_pdata),
+     },
+     {
+             .name = "locomo-lt", /* Long time timer */
+             .resources = locomo_lt_resources,
+             .num_resources = ARRAY_SIZE(locomo_lt_resources),
+     },
+     {

drivers/mfd/locomo.c line 95

Quoted 10 lines

drivers/mfd/locomo.c new file +10 -0

+             .name = "locomo-spi",
+             .resources = locomo_spi_resources,
+             .num_resources = ARRAY_SIZE(locomo_spi_resources),
+     },
+     {
+             .name = "locomo-led",
+     },
+     {
+             .name = "locomo-backlight",
+     },

Please make these:

Review

Quoted 2 lines

(quoted diff) +2 -0

+     { .name = "locomo-led" },
+     { .name = "locomo-backlight" },

... and put them at the bottom.

drivers/mfd/locomo.c line 118

Quoted 3 lines

drivers/mfd/locomo.c new file +3 -0

+     while (1) {
+             regmap_read(lchip->regmap, LOCOMO_ICR, &req);
+             req &= 0x0f00;

What is this magic number? Please #define it.

drivers/mfd/locomo.c line 123

Quoted 5 lines

drivers/mfd/locomo.c new file +4 -0

+             if (!req)
+                     break;
+
+             irq = ffs(req) - 9;

Minus another random number? Either define it or enter a comment.

drivers/mfd/locomo.c line 209

Quoted 6 lines

drivers/mfd/locomo.c new file +6 -0

+#ifdef CONFIG_PM_SLEEP
+static int locomo_suspend(struct device *dev)
+{
+     struct locomo *lchip = dev_get_drvdata(dev);
+
+     /* AUDIO */

WHY ARE YOU SHOUTING? Ironic eh? ;)

Quoted 13 lines

drivers/mfd/locomo.c new file +13 -0

+     regmap_write(lchip->regmap, LOCOMO_PAIF, 0x00);
+
+     /*
+      * Original code disabled the clock depending on leds settings
+      * However we disable leds before suspend, thus it's safe
+      * to just assume this setting.
+      */
+     /* CLK32 off */
+     regmap_write(lchip->regmap, LOCOMO_C32K, 0x00);
+
+     /* 22MHz/24MHz clock off */
+     regmap_write(lchip->regmap, LOCOMO_ACC, 0x00);
+

drivers/mfd/locomo.c line 228

Quoted 6 lines

drivers/mfd/locomo.c new file +6 -0

+     return 0;
+}
+
+static int locomo_resume(struct device *dev)
+{
+     struct locomo *lchip = dev_get_drvdata(dev);

Do audio and clk sort themselves out?

Quoted 5 lines

drivers/mfd/locomo.c new file +5 -0

+     regmap_write(lchip->regmap, LOCOMO_C32K, 0x00);
+
+     return 0;
+}
+

drivers/mfd/locomo.c line 287

Quoted 8 lines

drivers/mfd/locomo.c new file +8 -0

+
+     if (pdata) {
+             locomo_gpio_pdata.gpio_base = pdata->gpio_base;
+             locomo_lcd_pdata.comadj = pdata->comadj;
+     } else {
+             locomo_gpio_pdata.gpio_base = -1;
+             locomo_lcd_pdata.comadj = 128;
+     }

struct locomo_gpio_platform_data locomo_gpio_pdata;

locomo_gpio_pdata = devm_kzalloc(<blah>);

locomo_cells[GPIO].platform_data = locomo_gpio_pdata;

Quoted 5 lines

include/linux/mfd/locomo.h new file +0 -0

new file mode 100644
index 0000000..6729767

include/linux/mfd/locomo.h line 32

Quoted 5 lines

include/linux/mfd/locomo.h new file +5 -0

+/* MCS decoder for boot selecting */
+#define LOCOMO_MCSX0 0x10
+#define LOCOMO_MCSX1 0x14
+#define LOCOMO_MCSX2 0x18
+#define LOCOMO_MCSX3 0x1c

These are pretty cryptic. Any way of making them easier to identify.

include/linux/mfd/locomo.h line 162

Quoted 3 lines

include/linux/mfd/locomo.h new file +3 -0

+struct locomo_gpio_platform_data {
+     unsigned int gpio_base;
+};

A struct for a single int seems overkill.

include/linux/mfd/locomo.h line 171

Quoted 8 lines

include/linux/mfd/locomo.h new file +8 -0

+struct locomo_lcd_platform_data {
+     u8 comadj;
+};
+
+struct locomo_platform_data {
+     unsigned int gpio_base;
+     u8 comadj;
+};

Why do you need to pass gpio_base twice?

2015-04-28 21:45 GMT+03:00 Lee Jones <[email protected]>:

Quoted 10 lines · Dmitry Eremin-Solenikov

Quoted 9 lines

LoCoMo is a GA used on Sharp Zaurus SL-5x00. Current driver does has several design issues (special bus instead of platform bus, doesn't use mfd-core, etc).

Implement 'core' parts of locomo support as an mfd driver.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---

Thanks for the review. I agree (and have implemented) with most of your comments.
However I have few questions. See below.

Quoted 14 lines

Quoted 14 lines

drivers/mfd/locomo.c new file +13 -0

+/* the following is the overall data for the locomo chip */
+struct locomo {
+     struct device *dev;
+     unsigned int irq;
+     spinlock_t lock;
+     struct irq_domain *domain;
+     struct regmap *regmap;
+};
+
+static struct resource locomo_kbd_resources[] = {
+     DEFINE_RES_IRQ(IRQ_LOCOMO_KEY),
+};
+

Quoted 8 lines

drivers/mfd/locomo.c line 54

Quoted 6 lines

drivers/mfd/locomo.c new file +6 -0

+static struct resource locomo_gpio_resources[] = {
+     DEFINE_RES_IRQ(IRQ_LOCOMO_GPIO),
+};
+
+/* Filled in locomo_probe() function. */
+static struct locomo_gpio_platform_data locomo_gpio_pdata;

I'd prefer you didn't use globals for this.

Just for platform data, or for all the structures?

Quoted 43 lines

Quoted 30 lines

drivers/mfd/locomo.c new file +30 -0

+static struct resource locomo_lt_resources[] = {
+     DEFINE_RES_IRQ(IRQ_LOCOMO_LT),
+};
+
+static struct resource locomo_spi_resources[] = {
+     DEFINE_RES_IRQ(IRQ_LOCOMO_SPI),
+};
+
+/* Filled in locomo_probe() function. */
+static struct locomo_lcd_platform_data locomo_lcd_pdata;
+
+static struct mfd_cell locomo_cells[] = {
+     {
+             .name = "locomo-kbd",
+             .resources = locomo_kbd_resources,
+             .num_resources = ARRAY_SIZE(locomo_kbd_resources),
+     },
+     {
+             .name = "locomo-gpio",
+             .resources = locomo_gpio_resources,
+             .num_resources = ARRAY_SIZE(locomo_gpio_resources),
+             .platform_data = &locomo_gpio_pdata,
+             .pdata_size = sizeof(locomo_gpio_pdata),
+     },
+     {
+             .name = "locomo-lt", /* Long time timer */
+             .resources = locomo_lt_resources,
+             .num_resources = ARRAY_SIZE(locomo_lt_resources),
+     },
+     {

drivers/mfd/locomo.c line 95

Quoted 10 lines

drivers/mfd/locomo.c new file +10 -0

+             .name = "locomo-spi",
+             .resources = locomo_spi_resources,
+             .num_resources = ARRAY_SIZE(locomo_spi_resources),
+     },
+     {
+             .name = "locomo-led",
+     },
+     {
+             .name = "locomo-backlight",
+     },

Please make these:

Quoted 4 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+     { .name = "locomo-led" },
+     { .name = "locomo-backlight" },

... and put them at the bottom.

They will be populated by of_compatible lines, so it makes little sense to me. What about adding of compatibility lines to this patch?

Quoted 5 lines

drivers/mfd/locomo.c line 118

Quoted 3 lines

drivers/mfd/locomo.c new file +3 -0

+     while (1) {
+             regmap_read(lchip->regmap, LOCOMO_ICR, &req);
+             req &= 0x0f00;

What is this magic number? Please #define it.

Adding comments to this function instead.

Quoted 29 lines

drivers/mfd/locomo.c line 123

Quoted 4 lines

drivers/mfd/locomo.c new file +4 -0

+             if (!req)
+                     break;
+
+             irq = ffs(req) - 9;

Minus another random number? Either define it or enter a comment.

drivers/mfd/locomo.c line 209

Quoted 6 lines

drivers/mfd/locomo.c new file +6 -0

+#ifdef CONFIG_PM_SLEEP
+static int locomo_suspend(struct device *dev)
+{
+     struct locomo *lchip = dev_get_drvdata(dev);
+
+     /* AUDIO */

WHY ARE YOU SHOUTING? Ironic eh? ;)

Quoted 13 lines

drivers/mfd/locomo.c new file +13 -0

+     regmap_write(lchip->regmap, LOCOMO_PAIF, 0x00);
+
+     /*
+      * Original code disabled the clock depending on leds settings
+      * However we disable leds before suspend, thus it's safe
+      * to just assume this setting.
+      */
+     /* CLK32 off */
+     regmap_write(lchip->regmap, LOCOMO_C32K, 0x00);
+
+     /* 22MHz/24MHz clock off */
+     regmap_write(lchip->regmap, LOCOMO_ACC, 0x00);
+

Quoted 8 lines

drivers/mfd/locomo.c line 228

Quoted 6 lines

drivers/mfd/locomo.c new file +6 -0

+     return 0;
+}
+
+static int locomo_resume(struct device *dev)
+{
+     struct locomo *lchip = dev_get_drvdata(dev);

Do audio and clk sort themselves out?

PAIF and ACC registers are used only by audio parts of the device. However there is no current Linux driver for those parts. The registers are cleared in case the firmware has set something in them, but in future it will be the task
of the audio driver to properly clear and restore them.

Quoted 5 lines

Quoted 5 lines

drivers/mfd/locomo.c new file +5 -0

+     regmap_write(lchip->regmap, LOCOMO_C32K, 0x00);
+
+     return 0;
+}
+

[skipped]

Quoted 13 lines

drivers/mfd/locomo.c line 287

Quoted 8 lines

drivers/mfd/locomo.c new file +8 -0

+
+     if (pdata) {
+             locomo_gpio_pdata.gpio_base = pdata->gpio_base;
+             locomo_lcd_pdata.comadj = pdata->comadj;
+     } else {
+             locomo_gpio_pdata.gpio_base = -1;
+             locomo_lcd_pdata.comadj = 128;
+     }

struct locomo_gpio_platform_data locomo_gpio_pdata;

locomo_gpio_pdata = devm_kzalloc(<blah>);

Quoted 1 line

locomo_cells[GPIO].platform_data = locomo_gpio_pdata;

I do not quite agree with you at this place. The passed platform_data will be kmemdup()'ed inside platform core. So the whole struct will be duplicated twice inside kmallocate'd memory. Ideally I'd like to drop the whole platform_data busyness, but that requires switching to DTS first.

Quoted 6 lines

Quoted 5 lines

include/linux/mfd/locomo.h new file +0 -0

new file mode 100644
index 0000000..6729767

Quoted 7 lines

include/linux/mfd/locomo.h line 32

Quoted 5 lines

include/linux/mfd/locomo.h new file +5 -0

+/* MCS decoder for boot selecting */
+#define LOCOMO_MCSX0 0x10
+#define LOCOMO_MCSX1 0x14
+#define LOCOMO_MCSX2 0x18
+#define LOCOMO_MCSX3 0x1c

These are pretty cryptic. Any way of making them easier to identify.

No way. The names are based on old Sharp code. The drivers do not use them, but I'd like to still keep the registers for the reference purposes.

Quoted 6 lines

include/linux/mfd/locomo.h line 162

Quoted 3 lines

include/linux/mfd/locomo.h new file +3 -0

+struct locomo_gpio_platform_data {
+     unsigned int gpio_base;
+};

A struct for a single int seems overkill.

Quoted 10 lines

include/linux/mfd/locomo.h line 171

Quoted 8 lines

include/linux/mfd/locomo.h new file +8 -0

+struct locomo_lcd_platform_data {
+     u8 comadj;
+};
+
+struct locomo_platform_data {
+     unsigned int gpio_base;
+     u8 comadj;
+};

Why do you need to pass gpio_base twice?

First: machine file -> core driver
Second: core driver -> gpio driver

The other way to do the same would be:

struct locomo_gpio_platform_data {
     unsigned int gpio_base;
};

struct locomo_lcd_platform_data {
     u8 comadj;
};

struct locomo_platform_data {
     struct locomo_gpio_platform_data gpio_pdata;
     struct locomo_lcd_platform_data lcd_pdata;
};

And to assign pointers to the passed data in the mfd_cells during locomo_probe. Does that look better to you?

Adapt locomo leds driver to new locomo core setup.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---
  drivers/leds/Kconfig       |   1 -
  drivers/leds/leds-locomo.c | 119 +++++++++++++++++++++++----------------------
  2 files changed, 61 insertions(+), 59 deletions(-)

drivers/leds/Kconfig +0 -0

index 966b960..4b4650b 100644

drivers/leds/Kconfig +0 -0

  	help
  	  This option enables support for the LEDs on Sharp Locomo.
  	  Zaurus models SL-5500 and SL-5600.

drivers/leds/leds-locomo.c +0 -1

index 80ba048..7fde812 100644
@@ -9,89 +9,92 @@
   */
 
  #include <linux/kernel.h>
-#include <linux/init.h>

drivers/leds/leds-locomo.c +8 -3

+
+struct locomo_led {
+	struct led_classdev led;
+	struct regmap *regmap;
+	unsigned int reg;
+};
 
  static void locomoled_brightness_set(struct led_classdev *led_cdev,
-				enum led_brightness value, int offset)
+				enum led_brightness value)
  {
-	struct locomo_dev *locomo_dev = LOCOMO_DEV(led_cdev->dev->parent);
-	unsigned long flags;
+	struct locomo_led *led = container_of(led_cdev, struct locomo_led, led);

drivers/leds/leds-locomo.c +47 -43

-static void locomoled_brightness_set0(struct led_classdev *led_cdev,
-				enum led_brightness value)
+static int locomo_led_register(
+		struct device *dev,
+		struct locomo_led *led,
+		const char *name,
+		const char *trigger,
+		struct regmap *regmap,
+		unsigned int reg)
  {
-	locomoled_brightness_set(led_cdev, value, LOCOMO_LPT0);
+	led->led.name = name;
+	led->led.flags = LED_CORE_SUSPENDRESUME;
+	led->led.default_trigger = trigger;
+	led->led.brightness_set = locomoled_brightness_set;
+	led->regmap = regmap;
+	led->reg = reg;
+
+	return devm_led_classdev_register(dev, &led->led);
  }
 
-static void locomoled_brightness_set1(struct led_classdev *led_cdev,
-				enum led_brightness value)
-{
-	locomoled_brightness_set(led_cdev, value, LOCOMO_LPT1);
-}
-
-static struct led_classdev locomo_led0 = {
-	.name			= "locomo:amber:charge",
-	.default_trigger	= "main-battery-charging",
-	.brightness_set		= locomoled_brightness_set0,
-};
-
-static struct led_classdev locomo_led1 = {
-	.name			= "locomo:green:mail",
-	.default_trigger	= "nand-disk",
-	.brightness_set		= locomoled_brightness_set1,
-};
-
-static int locomoled_probe(struct locomo_dev *ldev)
+static int locomoled_probe(struct platform_device *pdev)
  {
  	int ret;
-
-	ret = led_classdev_register(&ldev->dev, &locomo_led0);
+	struct locomo_led *leds;
+	struct regmap *regmap;
+
+	leds = devm_kzalloc(&pdev->dev, 2 * sizeof(*leds), GFP_KERNEL);
+	if (!leds)
+		return -ENOMEM;
+
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!regmap)
+		return -ENODEV;
+
+	ret = locomo_led_register(
+			&pdev->dev,
+			leds,
+			"locomo:amber:charge",
+			"main-battery-charging",
+			regmap,
+			LOCOMO_LPT0);
  	if (ret < 0)
  		return ret;
 
-	ret = led_classdev_register(&ldev->dev, &locomo_led1);
+	ret = locomo_led_register(
+			&pdev->dev,
+			leds + 1,
+			"locomo:green:mail",
+			"mmc0",
+			regmap,
+			LOCOMO_LPT1);
  	if (ret < 0)
-		led_classdev_unregister(&locomo_led0);
-
-	return ret;
-}
+		return ret;
 
-static int locomoled_remove(struct locomo_dev *dev)
-{
-	led_classdev_unregister(&locomo_led0);
-	led_classdev_unregister(&locomo_led1);
  	return 0;
  }
 
-static struct locomo_driver locomoled_driver = {
-	.drv = {
-		.name = "locomoled"
+static struct platform_driver locomoled_driver = {
+	.driver = {
+		.name = "locomo-led"
  	},
-	.devid	= LOCOMO_DEVID_LED,
  	.probe	= locomoled_probe,
-	.remove	= locomoled_remove,
  };
 
-static int __init locomoled_init(void)
-{
-	return locomo_driver_register(&locomoled_driver);
-}
-module_init(locomoled_init);
+module_platform_driver(locomoled_driver);
 
  MODULE_AUTHOR("John Lenz <[email protected]>");
  MODULE_DESCRIPTION("Locomo LED driver");
  MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:locomo-led");
 

Quoted 10 lines

Adapt locomo leds driver to new locomo core setup.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---
  drivers/leds/Kconfig       |   1 -
  drivers/leds/leds-locomo.c | 119
+++++++++++++++++++++++----------------------
  2 files changed, 61 insertions(+), 59 deletions(-)

drivers/leds/Kconfig config LEDS_LM3642 · line 82

Quoted 9 lines

drivers/leds/Kconfig +0 -1

index 966b960..4b4650b 100644
@@ -79,7 +79,6 @@ config LEDS_LM3642
  config LEDS_LOCOMO
        tristate "LED Support for Locomo device"
        depends on LEDS_CLASS
-       depends on SHARP_LOCOMO

Why do you remove this dependency?

Quoted 11 lines

drivers/leds/Kconfig +0 -0

        help
          This option enables support for the LEDs on Sharp Locomo.
          Zaurus models SL-5500 and SL-5600.

drivers/leds/leds-locomo.c +0 -0

index 80ba048..7fde812 100644
@@ -9,89 +9,92 @@
   */
 

drivers/leds/leds-locomo.c line 16

Quoted 12 lines

drivers/leds/leds-locomo.c +4 -6

  #include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/device.h>
  #include <linux/leds.h>
-
-#include <mach/hardware.h>
-#include <asm/hardware/locomo.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/locomo.h>

Please keep alphabetical order.

Quoted 17 lines

drivers/leds/leds-locomo.c +8 -3

+
+struct locomo_led {
+       struct led_classdev led;
+       struct regmap *regmap;
+       unsigned int reg;
+};
 
  static void locomoled_brightness_set(struct led_classdev *led_cdev,
-                               enum led_brightness value, int offset)
+                               enum led_brightness value)
  {
-       struct locomo_dev *locomo_dev = LOCOMO_DEV(led_cdev->dev->parent);
-       unsigned long flags;
+       struct locomo_led *led = container_of(led_cdev, struct locomo_led,
 led);
 

drivers/leds/leds-locomo.c line 31

Quoted 11 lines

drivers/leds/leds-locomo.c +2 -6

-       local_irq_save(flags);
-       if (value)
-               locomo_writel(LOCOMO_LPT_TOFH, locomo_dev->mapbase +
 offset);
-       else
-               locomo_writel(LOCOMO_LPT_TOFL, locomo_dev->mapbase +
 offset);
-       local_irq_restore(flags);
+       regmap_write(led->regmap, led->reg,
+                       value ? LOCOMO_LPT_TOFH : LOCOMO_LPT_TOFL);
  }

Please use work queue for setting brightness. This is required for the driver to be compatible with led triggers. You can refer to the existing LED drivers on how to implement this.

2015-05-06 18:05 GMT+03:00 Jacek Anaszewski <[email protected]>:

Quoted 11 lines · Dmitry Eremin-Solenikov

Quoted 10 lines

Adapt locomo leds driver to new locomo core setup.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---
   drivers/leds/Kconfig       |   1 -
   drivers/leds/leds-locomo.c | 119
+++++++++++++++++++++++----------------------
   2 files changed, 61 insertions(+), 59 deletions(-)

Quoted 12 lines

drivers/leds/Kconfig config LEDS_LM3642 · line 82

Quoted 9 lines

drivers/leds/Kconfig +0 -1

index 966b960..4b4650b 100644
@@ -79,7 +79,6 @@ config LEDS_LM3642
   config LEDS_LOCOMO
         tristate "LED Support for Locomo device"
         depends on LEDS_CLASS
-       depends on SHARP_LOCOMO

Why do you remove this dependency?

Because SHARP_LOCOMO is a Kconfig symbol for the old driver. New driver uses MFD_LOCOMO Kconfig entry. Also the driver now uses generic platform device and regmap interfaces, so there is no direct dependency on main LoCoMo driver. And the policy (IIRC) was not to have such dependencies.

Quoted 11 lines

Quoted 11 lines

drivers/leds/Kconfig +0 -0

         help
           This option enables support for the LEDs on Sharp Locomo.
           Zaurus models SL-5500 and SL-5600.

drivers/leds/leds-locomo.c +0 -0

index 80ba048..7fde812 100644
@@ -9,89 +9,92 @@
    */
 

Quoted 15 lines

drivers/leds/leds-locomo.c line 16

Quoted 12 lines

drivers/leds/leds-locomo.c +4 -6

   #include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/device.h>
   #include <linux/leds.h>
-
-#include <mach/hardware.h>
-#include <asm/hardware/locomo.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/locomo.h>

Please keep alphabetical order.

Ack

Quoted 17 lines

Quoted 17 lines

drivers/leds/leds-locomo.c +8 -3

+
+struct locomo_led {
+       struct led_classdev led;
+       struct regmap *regmap;
+       unsigned int reg;
+};
 
   static void locomoled_brightness_set(struct led_classdev *led_cdev,
-                               enum led_brightness value, int offset)
+                               enum led_brightness value)
   {
-       struct locomo_dev *locomo_dev = LOCOMO_DEV(led_cdev->dev->parent);
-       unsigned long flags;
+       struct locomo_led *led = container_of(led_cdev, struct locomo_led,
 led);
 

Quoted 16 lines

drivers/leds/leds-locomo.c line 31

Quoted 11 lines

drivers/leds/leds-locomo.c +2 -6

-       local_irq_save(flags);
-       if (value)
-               locomo_writel(LOCOMO_LPT_TOFH, locomo_dev->mapbase +
 offset);
-       else
-               locomo_writel(LOCOMO_LPT_TOFL, locomo_dev->mapbase +
 offset);
-       local_irq_restore(flags);
+       regmap_write(led->regmap, led->reg,
+                       value ? LOCOMO_LPT_TOFH : LOCOMO_LPT_TOFL);
   }

Please use work queue for setting brightness. This is required for the driver to be compatible with led triggers. You can refer to the existing LED drivers on how to implement this.

Hmm. Why? The regmap here uses MMIO access, so it is atomic operation and doesn't need to be wrapped into work queue, does it?

Quoted 30 lines

2015-05-06 18:05 GMT+03:00 Jacek Anaszewski <[email protected]>:

Quoted 26 lines · Dmitry Eremin-Solenikov

Quoted 16 lines

Adapt locomo leds driver to new locomo core setup.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---
   drivers/leds/Kconfig       |   1 -
   drivers/leds/leds-locomo.c | 119
+++++++++++++++++++++++----------------------
   2 files changed, 61 insertions(+), 59 deletions(-)

drivers/leds/Kconfig +0 -0

index 966b960..4b4650b 100644

drivers/leds/Kconfig config LEDS_LM3642 · line 82

Quoted 4 lines

drivers/leds/Kconfig +0 -1

@@ -79,7 +79,6 @@ config LEDS_LM3642
   config LEDS_LOCOMO
         tristate "LED Support for Locomo device"
         depends on LEDS_CLASS
-       depends on SHARP_LOCOMO

Why do you remove this dependency?

Quoted 4 lines

Because SHARP_LOCOMO is a Kconfig symbol for the old driver. New driver uses MFD_LOCOMO Kconfig entry. Also the driver now uses generic platform device and regmap interfaces, so there is no direct dependency on main LoCoMo driver. And the policy (IIRC) was not to have such dependencies.

Ack. Shouldn't you also need "select REGMAP_MMIO" ?

Quoted 25 lines

Quoted 25 lines

Quoted 22 lines

drivers/leds/Kconfig +1 -2

  static void locomoled_brightness_set(struct led_classdev *led_cdev,
-                               enum led_brightness value, int offset)
+                               enum led_brightness value)
   {
-       struct locomo_dev *locomo_dev =

LOCOMO_DEV(led_cdev->dev->parent);

drivers/leds/Kconfig +1 -1

-       unsigned long flags;
+       struct locomo_led *led = container_of(led_cdev, struct

locomo_led,
led);

drivers/leds/Kconfig +0 -3

-       local_irq_save(flags);
-       if (value)
-               locomo_writel(LOCOMO_LPT_TOFH, locomo_dev->mapbase +

offset);

drivers/leds/Kconfig +0 -2

-       else
-               locomo_writel(LOCOMO_LPT_TOFL, locomo_dev->mapbase +

offset);

drivers/leds/Kconfig +2 -1

-       local_irq_restore(flags);
+       regmap_write(led->regmap, led->reg,
+                       value ? LOCOMO_LPT_TOFH : LOCOMO_LPT_TOFL);
   }

Quoted 7 lines

Quoted 3 lines

Please use work queue for setting brightness. This is required for the driver to be compatible with led triggers. You can refer to the existing LED drivers on how to implement this.

Hmm. Why? The regmap here uses MMIO access, so it is atomic operation and doesn't need to be wrapped into work queue, does it?

Triggers can call brightness_set op in the interrupt context, so it cannot sleep. I see "map->lock(map->lock_arg)" in regmap_write, thus I am inferring that it can sleep.

I am not sure if regmap implements its 'lock' op when using MMIO.

The best way to figure this out is turning "LED Timer Trigger" on in the config and execute:

echo "timer" > trigger

2015-05-13 13:31 GMT+03:00 Jacek Anaszewski <[email protected]>:

Quoted 37 lines · Dmitry Eremin-Solenikov

Quoted 34 lines

2015-05-06 18:05 GMT+03:00 Jacek Anaszewski <[email protected]>:

Quoted 13 lines · Dmitry Eremin-Solenikov

Quoted 11 lines

Adapt locomo leds driver to new locomo core setup.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---
    drivers/leds/Kconfig       |   1 -
    drivers/leds/leds-locomo.c | 119
+++++++++++++++++++++++----------------------
    2 files changed, 61 insertions(+), 59 deletions(-)

Quoted 13 lines

drivers/leds/Kconfig config LEDS_LM3642 · line 82

Quoted 9 lines

drivers/leds/Kconfig +0 -1

index 966b960..4b4650b 100644
@@ -79,7 +79,6 @@ config LEDS_LM3642
    config LEDS_LOCOMO
          tristate "LED Support for Locomo device"
          depends on LEDS_CLASS
-       depends on SHARP_LOCOMO

Why do you remove this dependency?

Because SHARP_LOCOMO is a Kconfig symbol for the old driver. New driver uses MFD_LOCOMO Kconfig entry. Also the driver now uses generic platform device and regmap interfaces, so there is no direct dependency on main LoCoMo driver. And the policy (IIRC) was not to have such dependencies.

Quoted 1 line

Ack. Shouldn't you also need "select REGMAP_MMIO" ?

No. Maybe I should add "select REGMAP" instead.

Quoted 38 lines

Quoted 25 lines

Quoted 25 lines

Quoted 22 lines

drivers/leds/Kconfig +1 -2

   static void locomoled_brightness_set(struct led_classdev *led_cdev,
-                               enum led_brightness value, int offset)
+                               enum led_brightness value)
    {
-       struct locomo_dev *locomo_dev =

LOCOMO_DEV(led_cdev->dev->parent);

drivers/leds/Kconfig +1 -1

-       unsigned long flags;
+       struct locomo_led *led = container_of(led_cdev, struct

locomo_led,
led);

drivers/leds/Kconfig +0 -3

-       local_irq_save(flags);
-       if (value)
-               locomo_writel(LOCOMO_LPT_TOFH, locomo_dev->mapbase +

offset);

drivers/leds/Kconfig +0 -2

-       else
-               locomo_writel(LOCOMO_LPT_TOFL, locomo_dev->mapbase +

offset);

drivers/leds/Kconfig +2 -1

-       local_irq_restore(flags);
+       regmap_write(led->regmap, led->reg,
+                       value ? LOCOMO_LPT_TOFH : LOCOMO_LPT_TOFL);
    }

Quoted 7 lines

Quoted 3 lines

Please use work queue for setting brightness. This is required for the driver to be compatible with led triggers. You can refer to the existing LED drivers on how to implement this.

Hmm. Why? The regmap here uses MMIO access, so it is atomic operation and doesn't need to be wrapped into work queue, does it?

Triggers can call brightness_set op in the interrupt context, so it cannot sleep. I see "map->lock(map->lock_arg)" in regmap_write, thus I am inferring that it can sleep.

Quoted 6 lines

I am not sure if regmap implements its 'lock' op when using MMIO.

The best way to figure this out is turning "LED Timer Trigger" on in the config and execute:

echo "timer" > trigger

It works without any "might sleep/sleeping in atomic context" warnings.

Technically I'd agree with you. If I'm using regmaps, ideally I should not depend on the exact backend and put working with it to the work queue. However as this is a driver for quite old IP block, it is not used with regmap backends other than MMIO, I'd deduce, it's ok to do things in a more relaxed way and to call regmap_write even from atomic contexts.

Quoted 38 lines · Dmitry Eremin-Solenikov

2015-05-13 13:31 GMT+03:00 Jacek Anaszewski <[email protected]>:

Quoted 37 lines · Dmitry Eremin-Solenikov

Quoted 34 lines

2015-05-06 18:05 GMT+03:00 Jacek Anaszewski <[email protected]>:

Quoted 13 lines · Dmitry Eremin-Solenikov

Quoted 11 lines

Adapt locomo leds driver to new locomo core setup.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---
   drivers/leds/Kconfig       |   1 -
   drivers/leds/leds-locomo.c | 119
+++++++++++++++++++++++----------------------
   2 files changed, 61 insertions(+), 59 deletions(-)

Quoted 13 lines

drivers/leds/Kconfig config LEDS_LM3642 · line 82

Quoted 9 lines

drivers/leds/Kconfig +0 -1

index 966b960..4b4650b 100644
@@ -79,7 +79,6 @@ config LEDS_LM3642
   config LEDS_LOCOMO
         tristate "LED Support for Locomo device"
         depends on LEDS_CLASS
-       depends on SHARP_LOCOMO

Why do you remove this dependency?

Because SHARP_LOCOMO is a Kconfig symbol for the old driver. New driver uses MFD_LOCOMO Kconfig entry. Also the driver now uses generic platform device and regmap interfaces, so there is no direct dependency on main LoCoMo driver. And the policy (IIRC) was not to have such dependencies.

Quoted 3 lines

Quoted 1 line

Ack. Shouldn't you also need "select REGMAP_MMIO" ?

No. Maybe I should add "select REGMAP" instead.

REGMAP is enabled by default if REGMAP_MMIO is enabled. Having

Quoted 39 lines · Jacek Anaszewski

Quoted 38 lines · Dmitry Eremin-Solenikov

2015-05-13 13:31 GMT+03:00 Jacek Anaszewski <[email protected]>:

Quoted 37 lines · Dmitry Eremin-Solenikov

Quoted 34 lines

2015-05-06 18:05 GMT+03:00 Jacek Anaszewski <[email protected]>:

Quoted 13 lines · Dmitry Eremin-Solenikov

Quoted 11 lines

Adapt locomo leds driver to new locomo core setup.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---
    drivers/leds/Kconfig       |   1 -
    drivers/leds/leds-locomo.c | 119
+++++++++++++++++++++++----------------------
    2 files changed, 61 insertions(+), 59 deletions(-)

Quoted 13 lines

drivers/leds/Kconfig config LEDS_LM3642 · line 82

Quoted 9 lines

drivers/leds/Kconfig +0 -1

index 966b960..4b4650b 100644
@@ -79,7 +79,6 @@ config LEDS_LM3642
    config LEDS_LOCOMO
          tristate "LED Support for Locomo device"
          depends on LEDS_CLASS
-       depends on SHARP_LOCOMO

Why do you remove this dependency?

Because SHARP_LOCOMO is a Kconfig symbol for the old driver. New driver uses MFD_LOCOMO Kconfig entry. Also the driver now uses generic platform device and regmap interfaces, so there is no direct dependency on main LoCoMo driver. And the policy (IIRC) was not to have such dependencies.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Ack. Shouldn't you also need "select REGMAP_MMIO" ?

No. Maybe I should add "select REGMAP" instead.

REGMAP is enabled by default if REGMAP_MMIO is enabled. Having

That is unfortunately not how select works: it does not automatically select parents for the selected symbol.

As LoCoMo is switching to new device model, adapt keyboard driver to support new locomo core driver.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---
 drivers/input/keyboard/Kconfig     |   1 -
 drivers/input/keyboard/locomokbd.c | 271 +++++++++++++++++++------------------
 2 files changed, 143 insertions(+), 129 deletions(-)

drivers/input/keyboard/Kconfig +0 -1

index 106fbac..0a3d875 100644
@@ -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
 

drivers/input/keyboard/locomokbd.c +17 -23

index c94d610..eed0a94 100644
@@ -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 <[email protected]>");
-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 {
 };
 

drivers/input/keyboard/locomokbd.c +19 -32

 {
-	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 */

drivers/input/keyboard/locomokbd.c +22 -32

 	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);
 

drivers/input/keyboard/locomokbd.c +4 -1

+	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);

drivers/input/keyboard/locomokbd.c +3 -2

+
 	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);

drivers/input/keyboard/locomokbd.c +5 -9

 	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;

drivers/input/keyboard/locomokbd.c +4 -0

+{
+	struct locomokbd *locomokbd = dev_get_drvdata(dev);
+
+	regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x10, 0x0);
 

drivers/input/keyboard/locomokbd.c +8 -0

+{
+	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);
+

drivers/input/keyboard/locomokbd.c +5 -12

 	},
-	.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 <[email protected]>");
+MODULE_DESCRIPTION("LoCoMo keyboard driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:locomo-kbd");

Signature · 1 line

2.1.4

Hi Dmitry,

Quoted 5 lines · Dmitry Eremin-Solenikov

As LoCoMo is switching to new device model, adapt keyboard driver to support new locomo core driver.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---

Quoted 8 lines

/* helper functions for reading the keyboard matrix */

drivers/input/keyboard/locomokbd.c +2 -2

-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);
 }

drivers/input/keyboard/locomokbd.c struct locomokbd { · line 86

Quoted 2 lines

drivers/input/keyboard/locomokbd.c +1 -1

-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.

Quoted 12 lines

{

drivers/input/keyboard/locomokbd.c +2 -6

-     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);
 }

Quoted 4 lines

drivers/input/keyboard/locomokbd.c +0 -0

@@ -291,16 +275,30 @@ static int locomokbd_probe(struct locomo_dev *dev)
 
      input_set_drvdata(input_dev, locomokbd);
 

drivers/input/keyboard/locomokbd.c static int locomokbd_probe(struct locomo_dev *dev) · line 285

Quoted 9 lines

drivers/input/keyboard/locomokbd.c +8 -1

-     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.

drivers/input/keyboard/locomokbd.c static int locomokbd_probe(struct locomo_dev *dev) · line 298

Quoted 4 lines

drivers/input/keyboard/locomokbd.c +2 -1

      /* 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()?

drivers/input/keyboard/locomokbd.c static int locomokbd_probe(struct locomo_dev *dev) · line 322

Quoted 8 lines

drivers/input/keyboard/locomokbd.c +3 -3

-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.

Quoted 3 lines

      del_timer_sync(&locomokbd->timer);

Should likely to go into close().

drivers/input/keyboard/locomokbd.c static int locomokbd_probe(struct locomo_dev *dev) · line 332

Quoted 3 lines

drivers/input/keyboard/locomokbd.c +3 -0

+
+#ifdef CONFIG_PM_SLEEP
+static int locomokbd_suspend(struct device *dev)

Mark as __maybe_unused instead of giarding with CONFIG_PM_SLEEP.

Hello,

2015-05-12 23:21 GMT+03:00 Dmitry Torokhov <[email protected]>:

Quoted 8 lines

Hi Dmitry,

Quoted 5 lines · Dmitry Eremin-Solenikov

As LoCoMo is switching to new device model, adapt keyboard driver to support new locomo core driver.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---

Thanks for the review.

Quoted 8 lines

Quoted 8 lines

/* helper functions for reading the keyboard matrix */

drivers/input/keyboard/locomokbd.c +2 -2

-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);
 }

Quoted 4 lines

drivers/input/keyboard/locomokbd.c struct locomokbd { · line 86

Quoted 2 lines

drivers/input/keyboard/locomokbd.c +1 -1

-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.

Why?

Quoted 13 lines

Quoted 13 lines

{

drivers/input/keyboard/locomokbd.c +2 -6

-     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);
 }

[skipped]

Quoted 4 lines

Quoted 4 lines

drivers/input/keyboard/locomokbd.c +0 -0

@@ -291,16 +275,30 @@ static int locomokbd_probe(struct locomo_dev *dev)
 
      input_set_drvdata(input_dev, locomokbd);
 

Quoted 12 lines

drivers/input/keyboard/locomokbd.c static int locomokbd_probe(struct locomo_dev *dev) · line 285

Quoted 9 lines

drivers/input/keyboard/locomokbd.c +8 -1

-     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.

This fixes a hardware issue. Home key is wired differently on two devices using this driver.
I'd prefer to have such setting in board file or in DTS in future, however that looks like an
overkill. What would be your suggestion?

Quoted 8 lines

drivers/input/keyboard/locomokbd.c static int locomokbd_probe(struct locomo_dev *dev) · line 298

Quoted 5 lines

drivers/input/keyboard/locomokbd.c +2 -1

      /* 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()?

[skipped]

Quoted 6 lines

Quoted 6 lines

drivers/input/keyboard/locomokbd.c +2 -2

-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);

Quoted 4 lines

Review

Quoted 2 lines

(quoted diff) +1 -1

-     free_irq(dev->irq[0], locomokbd);
+     free_irq(locomokbd->irq, locomokbd);

Is not needed with devm.

Not quite. There will be a possibility for the IRQ to happen after deleting a timer in locomokbd_remove() and before freeing the IRQ through the devres core. Oops.

Quoted 6 lines

Quoted 4 lines

      del_timer_sync(&locomokbd->timer);

Should likely to go into close().

Hmm. I will rethink this part, thank you.

Quoted 5 lines

drivers/input/keyboard/locomokbd.c static int locomokbd_probe(struct locomo_dev *dev) · line 332

Quoted 3 lines

drivers/input/keyboard/locomokbd.c +3 -0

+
+#ifdef CONFIG_PM_SLEEP
+static int locomokbd_suspend(struct device *dev)

Mark as __maybe_unused instead of giarding with CONFIG_PM_SLEEP.

Fine, however I thought that #ifdef's here are a typical pattern.

It is usefull sometimes to have an Alt-SysRQ combo on the keyboard to be able to trigger sysrq functions directly. Add an option providing sysrq mapping for Contact-Home keys.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>

---
 drivers/input/keyboard/Kconfig     |  7 +++++++
 drivers/input/keyboard/locomokbd.c | 14 +++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

drivers/input/keyboard/Kconfig +7 -0

index 0a3d875..eb8b11b 100644
@@ -343,6 +343,13 @@ config KEYBOARD_LOCOMO
 	  To compile this driver as a module, choose M here: the
 	  module will be called locomokbd.
 
+config KEYBOARD_LOCOMO_SYSRQ
+	bool "Provide SysRQ key on LoCoMo keyboard"
+	depends on KEYBOARD_LOCOMO
+	help
+	  Say Y here to be able to use Contact-Home as Alt-Sysrq combo.
+	  Say N if you want to use them as usual keys.
+
 config KEYBOARD_LPC32XX
 	tristate "LPC32XX matrix key scanner support"
 	depends on ARCH_LPC32XX && OF

drivers/input/keyboard/locomokbd.c +11 -3

index eed0a94..eb8dbe2 100644
@@ -40,6 +40,14 @@
 #define KEY_CONTACT		KEY_F18
 #define KEY_CENTER		KEY_F15
 
+#ifdef CONFIG_KEYBOARD_LOCOMOKBD_SYSRQ
+#define LOCOMO_KEY_CONTACT	KEY_LEFTALT
+#define LOCOMO_KEY_HOME		KEY_SYSRQ
+#else
+#define LOCOMO_KEY_CONTACT	KEY_CONTACT
+#define LOCOMO_KEY_HOME		KEY_HOME
+#endif
+
 #define KB_ROWS			16
 #define KB_COLS			8
 #define LOCOMOKBD_NUMKEYS	(KB_ROWS * KB_COLS)

@@ -48,7 +56,7 @@
 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, 0, KEY_CONTACT,				/* 10 - 19 */
+	0, 0, 0, 0, 0, 0, 0, KEY_MENU, 0, LOCOMO_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 */

@@ -280,9 +288,9 @@ static int locomokbd_probe(struct platform_device *dev)
 			sizeof(locomokbd->keycode));
 
 	if (machine_is_collie())
-		locomokbd->keycode[18] = KEY_HOME;
+		locomokbd->keycode[18] = LOCOMO_KEY_HOME;
 	else
-		locomokbd->keycode[3] = KEY_HOME;
+		locomokbd->keycode[3] = LOCOMO_KEY_HOME;
 
 	for (i = 0; i < LOCOMOKBD_NUMKEYS; i++)
 		input_set_capability(input_dev, EV_KEY, locomokbd->keycode[i]);

Signature · 1 line

2.1.4