Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v2 5/6] ARM: dts: imx25: mbimxsd25: Add touchscreen support.
From: Denis Carikli @ 2014-06-13 15:21 UTC (permalink / raw)
  To: Shawn Guo, Samuel Ortiz, Dmitry Torokhov, Jonathan Cameron
  Cc: Eric Bénard, Sascha Hauer, linux-arm-kernel, Lee Jones,
	linux-input, linux-iio, Fabio Estevam, Lars-Peter Clausen,
	Markus Pargmann, Denis Carikli
In-Reply-To: <1402672899-6995-1-git-send-email-denis@eukrea.com>

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 .../imx25-eukrea-mbimxsd25-baseboard-cmo-qvga.dts  |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-cmo-qvga.dts b/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-cmo-qvga.dts
index 9797280..047918d 100644
--- a/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-cmo-qvga.dts
+++ b/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-cmo-qvga.dts
@@ -70,3 +70,16 @@
 	lcd-supply = <&reg_lcd_3v3>;
 	status = "okay";
 };
+
+&tscadc {
+	status = "okay";
+
+	tsc: tcq@50030400 {
+		compatible = "fsl,imx25-tcq";
+		reg = <0x50030400 0x60>;
+		interrupt-parent = <&tscadc>;
+		interrupts = <0>;
+		fsl,wires = <4>;
+		fsl,pen-debounce = <100>;
+	};
+};
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 6/6] ARM: imx_v4_v5_defconfig: Add I.MX25 Touchscreen controller support.
From: Denis Carikli @ 2014-06-13 15:21 UTC (permalink / raw)
  To: Shawn Guo, Samuel Ortiz, Dmitry Torokhov, Jonathan Cameron
  Cc: Eric Bénard, Sascha Hauer, linux-arm-kernel, Lee Jones,
	linux-input, linux-iio, Fabio Estevam, Lars-Peter Clausen,
	Markus Pargmann, Denis Carikli
In-Reply-To: <1402672899-6995-1-git-send-email-denis@eukrea.com>

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 arch/arm/configs/imx_v4_v5_defconfig |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/imx_v4_v5_defconfig b/arch/arm/configs/imx_v4_v5_defconfig
index 397f43c..b6b3674 100644
--- a/arch/arm/configs/imx_v4_v5_defconfig
+++ b/arch/arm/configs/imx_v4_v5_defconfig
@@ -98,6 +98,7 @@ CONFIG_KEYBOARD_IMX=y
 # CONFIG_INPUT_MOUSE is not set
 CONFIG_INPUT_TOUCHSCREEN=y
 CONFIG_TOUCHSCREEN_ADS7846=m
+CONFIG_TOUCHSCREEN_MX25=y
 CONFIG_TOUCHSCREEN_MC13783=y
 # CONFIG_LEGACY_PTYS is not set
 CONFIG_SERIAL_8250=m
@@ -118,6 +119,7 @@ CONFIG_HWMON=m
 CONFIG_SENSORS_MC13783_ADC=m
 CONFIG_WATCHDOG=y
 CONFIG_IMX2_WDT=y
+CONFIG_MFD_MX25_TSADC=y
 CONFIG_MFD_MC13XXX_SPI=y
 CONFIG_REGULATOR=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 1/6] mfd: fsl imx25 Touchscreen ADC driver
From: Denis Carikli @ 2014-06-13 15:21 UTC (permalink / raw)
  To: Shawn Guo, Samuel Ortiz, Dmitry Torokhov, Jonathan Cameron
  Cc: Eric Bénard, Sascha Hauer, linux-arm-kernel, Lee Jones,
	linux-input, linux-iio, Fabio Estevam, Lars-Peter Clausen,
	Markus Pargmann, Denis Carikli
In-Reply-To: <1402672899-6995-1-git-send-email-denis@eukrea.com>

From: Markus Pargmann <mpa@pengutronix.de>

This is the core driver for imx25 touchscreen/adc driver. The module
has one shared ADC and two different conversion queues which use the
ADC. The two queues are identical. Both can be used for general purpose
ADC but one is meant to be used for touchscreens.

This driver is the core which manages the central components and
registers of the TSC/ADC unit. It manages the IRQs and forwards them to
the correct components.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 .../devicetree/bindings/mfd/fsl-imx25-tsadc.txt    |   46 ++++
 drivers/mfd/Kconfig                                |    9 +
 drivers/mfd/Makefile                               |    2 +
 drivers/mfd/fsl-imx25-tsadc.c                      |  232 ++++++++++++++++++++
 include/linux/mfd/imx25-tsadc.h                    |  138 ++++++++++++
 5 files changed, 427 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
 create mode 100644 drivers/mfd/fsl-imx25-tsadc.c
 create mode 100644 include/linux/mfd/imx25-tsadc.h

diff --git a/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt b/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
new file mode 100644
index 0000000..a857af0e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
@@ -0,0 +1,46 @@
+Freescale mx25 ADC/TSC multifunction device
+
+This device combines two general purpose conversion queues one used for general
+ADC and the other used for touchscreens.
+
+Required properties:
+ - compatible: Should be "fsl,imx25-tsadc".
+ - reg: Memory range of the device.
+ - interrupts: Interrupt for this device as described in
+   interrupts/interrupts.txt
+ - clocks: An 'ipg' clock defined as described in clocks/clock.txt
+ - interrupt-controller: This device is an interrupt controller. It controls
+   the interrupts of both conversion queues.
+ - #interrupt-cells: Should be '<1>'.
+ - #address-cells: Should be '<1>'.
+ - #size-cells: Should be '<1>'.
+ - ranges
+
+This device includes two conversion queues which can be added as subnodes.
+The first queue is for the touchscreen, the second for general purpose ADC.
+
+Example:
+	tscadc: tscadc@50030000 {
+		compatible = "fsl,imx25-tsadc";
+		reg = <0x50030000 0xc>;
+		interrupts = <46>;
+		clocks = <&clks 119>;
+		clock-names = "ipg";
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		tsc: tcq@50030400 {
+			compatible = "fsl,imx25-tcq";
+			reg = <0x50030400 0x60>;
+			...
+		};
+
+		adc: gcq@50030800 {
+			compatible = "fsl,imx25-gcq";
+			reg = <0x50030800 0x60>;
+			...
+		};
+	};
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ee8204c..73f4e66 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -183,6 +183,15 @@ config MFD_DA9063
 	  Additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MX25_TSADC
+	tristate "Freescale i.MX25 integrated Touchscreen and ADC unit"
+	depends on ARCH_MXC
+	select REGMAP_MMIO
+	help
+	  Enable support for the integrated Touchscreen and ADC unit of the
+	  i.MX25 processors. They consist of a conversion queue for general
+	  purpose ADC and a queue for Touchscreens.
+
 config MFD_MC13XXX
 	tristate
 	depends on (SPI_MASTER || I2C)
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 8afedba..7ff1013 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -78,6 +78,8 @@ obj-$(CONFIG_TWL4030_POWER)    += twl4030-power.o
 obj-$(CONFIG_MFD_TWL4030_AUDIO)	+= twl4030-audio.o
 obj-$(CONFIG_TWL6040_CORE)	+= twl6040.o
 
+obj-$(CONFIG_MFD_MX25_TSADC)	+= fsl-imx25-tsadc.o
+
 obj-$(CONFIG_MFD_MC13XXX)	+= mc13xxx-core.o
 obj-$(CONFIG_MFD_MC13XXX_SPI)	+= mc13xxx-spi.o
 obj-$(CONFIG_MFD_MC13XXX_I2C)	+= mc13xxx-i2c.o
diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c
new file mode 100644
index 0000000..10332c2
--- /dev/null
+++ b/drivers/mfd/fsl-imx25-tsadc.c
@@ -0,0 +1,232 @@
+/*
+ * Copyright 2014 Markus Pargmann, Pengutronix <mpa@pengutronix.de>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
+#include <linux/irqdomain.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/mfd/imx25-tsadc.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+struct mx25_tsadc_priv {
+	struct regmap *regs;
+	struct irq_domain *domain;
+	struct clk *clk;
+};
+
+static struct regmap_config mx25_tsadc = {
+	.fast_io = true,
+	.max_register = 0x8,
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+struct regmap *mx25_tsadc_get_regmap(struct device *dev)
+{
+	struct platform_device *pdev;
+	struct mx25_tsadc_priv *priv;
+
+	if (!dev)
+		return NULL;
+
+	pdev = container_of(dev, struct platform_device, dev);
+	priv = platform_get_drvdata(pdev);
+	if (IS_ERR_OR_NULL(priv->regs))
+		return NULL;
+
+	return priv->regs;
+}
+EXPORT_SYMBOL(mx25_tsadc_get_regmap);
+
+struct clk *mx25_tsadc_get_ipg(struct device *dev)
+{
+	struct platform_device *pdev;
+	struct mx25_tsadc_priv *priv;
+
+	if (!dev)
+		return NULL;
+
+	pdev = container_of(dev, struct platform_device, dev);
+	priv = platform_get_drvdata(pdev);
+	if (IS_ERR_OR_NULL(priv->clk))
+		return NULL;
+
+	return priv->clk;
+}
+EXPORT_SYMBOL(mx25_tsadc_get_ipg);
+
+static void mx25_tsadc_irq_handler(u32 irq, struct irq_desc *desc)
+{
+	struct mx25_tsadc_priv *priv = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_get_chip(irq);
+	u32 status;
+
+	chained_irq_enter(chip, desc);
+
+	regmap_read(priv->regs, MX25_TSC_TGSR, &status);
+
+	if (status & MX25_TGSR_GCQ_INT)
+		generic_handle_irq(irq_find_mapping(priv->domain, 1));
+
+	if (status & MX25_TGSR_TCQ_INT)
+		generic_handle_irq(irq_find_mapping(priv->domain, 0));
+
+	chained_irq_exit(chip, desc);
+}
+
+static void mx25_tsadc_nop(struct irq_data *d)
+{
+}
+
+static int mx25_tsadc_set_wake_nop(struct irq_data *d, unsigned int state)
+{
+	return 0;
+}
+
+static struct irq_chip mx25_tsadc_irq_chip = {
+	.name = "mx25-tsadc",
+	.irq_ack = mx25_tsadc_nop,
+	.irq_mask = mx25_tsadc_nop,
+	.irq_unmask = mx25_tsadc_nop,
+	.irq_set_wake = mx25_tsadc_set_wake_nop,
+};
+
+static int mx25_tsadc_domain_map(struct irq_domain *d, unsigned int irq,
+			     irq_hw_number_t hwirq)
+{
+	struct mx25_tsadc_priv *priv = d->host_data;
+
+	irq_set_chip_data(irq, priv);
+	irq_set_chip_and_handler(irq, &mx25_tsadc_irq_chip,
+				 handle_level_irq);
+
+
+	set_irq_flags(irq, IRQF_VALID);
+
+	return 0;
+}
+
+static struct irq_domain_ops mx25_tsadc_domain_ops = {
+	.map = mx25_tsadc_domain_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static int mx25_tsadc_setup_irq(struct platform_device *pdev,
+		struct mx25_tsadc_priv *priv)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	int irq;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(dev, "Failed to get irq\n");
+		return irq;
+	}
+
+	priv->domain = irq_domain_add_simple(np, 2, 0, &mx25_tsadc_domain_ops,
+			priv);
+	if (!priv->domain) {
+		dev_err(dev, "Failed to add irq domain\n");
+		return -ENOMEM;
+	}
+
+	irq_set_chained_handler(irq, mx25_tsadc_irq_handler);
+	irq_set_handler_data(irq, priv);
+
+	return 0;
+}
+
+static int mx25_tsadc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct mx25_tsadc_priv *priv;
+	struct resource *iores;
+	int ret;
+	void __iomem *iomem;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	iomem = devm_ioremap_resource(dev, iores);
+	if (IS_ERR(iomem)) {
+		dev_err(dev, "Failed to remap iomem\n");
+		return PTR_ERR(iomem);
+	}
+
+	priv->regs = regmap_init_mmio(dev, iomem, &mx25_tsadc);
+	if (IS_ERR(priv->regs)) {
+		dev_err(dev, "Failed to initialize regmap\n");
+		return PTR_ERR(priv->regs);
+	}
+
+	priv->clk = devm_clk_get(dev, "ipg");
+	if (IS_ERR(priv->clk)) {
+		dev_err(dev, "Failed to get ipg clock\n");
+		return PTR_ERR(priv->clk);
+	}
+
+	/* Enable clock and reset the component */
+	regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_CLK_EN,
+			MX25_TGCR_CLK_EN);
+	regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_TSC_RST,
+			MX25_TGCR_TSC_RST);
+
+	/* Setup powersaving mode, but enable internal reference voltage */
+	regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_POWERMODE_MASK,
+			MX25_TGCR_POWERMODE_SAVE);
+	regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_INTREFEN,
+			MX25_TGCR_INTREFEN);
+
+	ret = mx25_tsadc_setup_irq(pdev, priv);
+	if (ret) {
+		dev_err(dev, "Failed to setup irqs\n");
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	of_platform_populate(np, NULL, NULL, dev);
+
+	dev_info(dev, "i.MX25 Touchscreen and ADC core driver loaded\n");
+
+	return 0;
+}
+
+static const struct of_device_id mx25_tsadc_ids[] = {
+	{ .compatible = "fsl,imx25-tsadc", },
+	{ /* Sentinel */ }
+};
+
+static struct platform_driver mx25_tsadc_driver = {
+	.driver		= {
+		.name	= "mx25-tsadc",
+		.owner	= THIS_MODULE,
+		.of_match_table = mx25_tsadc_ids,
+	},
+	.probe		= mx25_tsadc_probe,
+};
+module_platform_driver(mx25_tsadc_driver);
+
+MODULE_DESCRIPTION("MFD for ADC/TSC for Freescale mx25");
+MODULE_AUTHOR("Markus Pargmann <mpa@pengutronix.de>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:mx25-tsadc");
diff --git a/include/linux/mfd/imx25-tsadc.h b/include/linux/mfd/imx25-tsadc.h
new file mode 100644
index 0000000..6fba341
--- /dev/null
+++ b/include/linux/mfd/imx25-tsadc.h
@@ -0,0 +1,138 @@
+#ifndef _LINUX_INCLUDE_INPUT_IMX25_TSADC_H_
+#define _LINUX_INCLUDE_INPUT_IMX25_TSADC_H_
+
+struct regmap;
+struct device;
+struct clk;
+
+struct regmap *mx25_tsadc_get_regmap(struct device *dev);
+struct clk *mx25_tsadc_get_ipg(struct device *dev);
+
+#define MX25_TSC_TGCR 0x00
+#define MX25_TSC_TGSR 0x04
+#define MX25_TSC_TICR 0x08
+
+/* The same register layout for TC and GC queue */
+#define MX25_ADCQ_FIFO 0x00
+#define MX25_ADCQ_CR 0x04
+#define MX25_ADCQ_SR 0x08
+#define MX25_ADCQ_MR 0x0c
+#define MX25_ADCQ_ITEM_7_0 0x20
+#define MX25_ADCQ_ITEM_15_8 0x24
+#define MX25_ADCQ_CFG(n) (0x40 + ((n) * 0x4))
+
+/* Register values */
+/* Queue Config register */
+#define MX25_ADCQ_MR_MASK 0xffffffff
+
+/* TGCR */
+#define MX25_TGCR_PDBTIME(x) ((x) << 25)
+#define MX25_TGCR_PDBTIME_MASK MX25_TGCR_PDBTIME(0x7f)
+#define MX25_TGCR_PDBEN (1 << 24)
+#define MX25_TGCR_PDEN (1 << 23)
+#define MX25_TGCR_ADCCLKCFG(x) ((x) << 16)
+#define MX25_TGCR_GET_ADCCLK(x) (((x) >> 16) & 0x1f)
+#define MX25_TGCR_INTREFEN (1 << 10)
+#define MX25_TGCR_POWERMODE_MASK (3 << 8)
+#define MX25_TGCR_POWERMODE_SAVE (1 << 8)
+#define MX25_TGCR_POWERMODE_ON (2 << 8)
+#define MX25_TGCR_STLC (1 << 5)
+#define MX25_TGCR_SLPC (1 << 4)
+#define MX25_TGCR_FUNC_RST (1 << 2)
+#define MX25_TGCR_TSC_RST (1 << 1)
+#define MX25_TGCR_CLK_EN (1 << 0)
+
+/* TGSR */
+#define MX25_TGSR_SLP_INT (1 << 2)
+#define MX25_TGSR_GCQ_INT (1 << 1)
+#define MX25_TGSR_TCQ_INT (1 << 0)
+
+/* ADCQ_ITEM_* */
+#define _MX25_ADCQ_ITEM(item, x) ((x) << ((item) * 4))
+#define MX25_ADCQ_ITEM(item, x) ((item) >= 8 ? \
+		_MX25_ADCQ_ITEM((item) - 8, (x)) : _MX25_ADCQ_ITEM((item), (x)))
+
+/* ADCQ_FIFO (TCQFIFO and GCQFIFO) */
+#define MX25_ADCQ_FIFO_DATA(x) (((x) >> 4) & 0xfff)
+#define MX25_ADCQ_FIFO_ID(x) ((x) & 0xf)
+
+/* ADCQ_CR (TCQR and GCQR) */
+#define MX25_ADCQ_CR_PDCFG_LEVEL (1 << 19)
+#define MX25_ADCQ_CR_PDMSK (1 << 18)
+#define MX25_ADCQ_CR_FRST (1 << 17)
+#define MX25_ADCQ_CR_QRST (1 << 16)
+#define MX25_ADCQ_CR_RWAIT_MASK (0xf << 12)
+#define MX25_ADCQ_CR_RWAIT(x) ((x) << 12)
+#define MX25_ADCQ_CR_WMRK_MASK (0xf << 8)
+#define MX25_ADCQ_CR_WMRK(x) ((x) << 8)
+#define MX25_ADCQ_CR_LITEMID_MASK (0xf << 4)
+#define MX25_ADCQ_CR_LITEMID(x) ((x) << 4)
+#define MX25_ADCQ_CR_RPT (1 << 3)
+#define MX25_ADCQ_CR_FQS (1 << 2)
+#define MX25_ADCQ_CR_QSM_MASK 0x3
+#define MX25_ADCQ_CR_QSM_PD 0x1
+#define MX25_ADCQ_CR_QSM_FQS 0x2
+#define MX25_ADCQ_CR_QSM_FQS_PD 0x3
+
+/* ADCQ_SR (TCQSR and GCQSR) */
+#define MX25_ADCQ_SR_FDRY (1 << 15)
+#define MX25_ADCQ_SR_FULL (1 << 14)
+#define MX25_ADCQ_SR_EMPT (1 << 13)
+#define MX25_ADCQ_SR_FDN(x) (((x) >> 8) & 0x1f)
+#define MX25_ADCQ_SR_FRR (1 << 6)
+#define MX25_ADCQ_SR_FUR (1 << 5)
+#define MX25_ADCQ_SR_FOR (1 << 4)
+#define MX25_ADCQ_SR_EOQ (1 << 1)
+#define MX25_ADCQ_SR_PD (1 << 0)
+
+/* ADCQ_MR (TCQMR and GCQMR) */
+#define MX25_ADCQ_MR_FDRY_DMA (1 << 31)
+#define MX25_ADCQ_MR_FER_DMA (1 << 22)
+#define MX25_ADCQ_MR_FUR_DMA (1 << 21)
+#define MX25_ADCQ_MR_FOR_DMA (1 << 20)
+#define MX25_ADCQ_MR_EOQ_DMA (1 << 17)
+#define MX25_ADCQ_MR_PD_DMA (1 << 16)
+#define MX25_ADCQ_MR_FDRY_IRQ (1 << 15)
+#define MX25_ADCQ_MR_FER_IRQ (1 << 6)
+#define MX25_ADCQ_MR_FUR_IRQ (1 << 5)
+#define MX25_ADCQ_MR_FOR_IRQ (1 << 4)
+#define MX25_ADCQ_MR_EOQ_IRQ (1 << 1)
+#define MX25_ADCQ_MR_PD_IRQ (1 << 0)
+
+/* ADCQ_CFG (TICR, TCC0-7,GCC0-7) */
+#define MX25_ADCQ_CFG_SETTLING_TIME(x) ((x) << 24)
+#define MX25_ADCQ_CFG_IGS (1 << 20)
+#define MX25_ADCQ_CFG_NOS_MASK (0xf << 16)
+#define MX25_ADCQ_CFG_NOS(x) (((x) - 1) << 16)
+#define MX25_ADCQ_CFG_WIPER (1 << 15)
+#define MX25_ADCQ_CFG_YNLR (1 << 14)
+#define MX25_ADCQ_CFG_YPLL_HIGH 0
+#define MX25_ADCQ_CFG_YPLL_OFF (1 << 12)
+#define MX25_ADCQ_CFG_YPLL_LOW (3 << 12)
+#define MX25_ADCQ_CFG_XNUR_HIGH 0
+#define MX25_ADCQ_CFG_XNUR_OFF (1 << 10)
+#define MX25_ADCQ_CFG_XNUR_LOW (3 << 10)
+#define MX25_ADCQ_CFG_XPUL_OFF (1 << 9)
+#define MX25_ADCQ_CFG_XPUL_HIGH 0
+#define MX25_ADCQ_CFG_REFP_YP 0
+#define MX25_ADCQ_CFG_REFP_XP (1 << 7)
+#define MX25_ADCQ_CFG_REFP_EXT (2 << 7)
+#define MX25_ADCQ_CFG_REFP_INT (3 << 7)
+#define MX25_ADCQ_CFG_REFP_MASK (3 << 7)
+#define MX25_ADCQ_CFG_IN_XP 0
+#define MX25_ADCQ_CFG_IN_YP (1 << 4)
+#define MX25_ADCQ_CFG_IN_XN (2 << 4)
+#define MX25_ADCQ_CFG_IN_YN (3 << 4)
+#define MX25_ADCQ_CFG_IN_WIPER (4 << 4)
+#define MX25_ADCQ_CFG_IN_AUX0 (5 << 4)
+#define MX25_ADCQ_CFG_IN_AUX1 (6 << 4)
+#define MX25_ADCQ_CFG_IN_AUX2 (7 << 4)
+#define MX25_ADCQ_CFG_REFN_XN 0
+#define MX25_ADCQ_CFG_REFN_YN (1 << 2)
+#define MX25_ADCQ_CFG_REFN_NGND (2 << 2)
+#define MX25_ADCQ_CFG_REFN_NGND2 (3 << 2)
+#define MX25_ADCQ_CFG_REFN_MASK (3 << 2)
+#define MX25_ADCQ_CFG_PENIACK (1 << 1)
+
+
+#endif  /* _LINUX_INCLUDE_INPUT_IMX25_TSADC_H_ */
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 3/6] iio: adc: fsl,imx25-gcq driver
From: Denis Carikli @ 2014-06-13 15:21 UTC (permalink / raw)
  To: Shawn Guo, Samuel Ortiz, Dmitry Torokhov, Jonathan Cameron
  Cc: Eric Bénard, Sascha Hauer, linux-arm-kernel, Lee Jones,
	linux-input, linux-iio, Fabio Estevam, Lars-Peter Clausen,
	Markus Pargmann, Denis Carikli
In-Reply-To: <1402672899-6995-1-git-send-email-denis@eukrea.com>

From: Markus Pargmann <mpa@pengutronix.de>

This is a conversion queue driver for the mx25 SoC. It uses the central
ADC which is used by two seperate independent queues. This driver
prepares different conversion configurations for each possible input.
For a conversion it creates a conversionqueue of one item with the
correct configuration for the chosen channel. It then executes the queue
once and disables the conversion queue afterwards.

The reference voltages are configurable through devicetree subnodes,
depending on the connections of the ADC inputs.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 .../devicetree/bindings/iio/adc/fsl,imx25-gcq.txt  |   54 ++++
 drivers/iio/adc/Kconfig                            |    7 +
 drivers/iio/adc/Makefile                           |    1 +
 drivers/iio/adc/fsl-imx25-gcq.c                    |  338 ++++++++++++++++++++
 4 files changed, 400 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/fsl,imx25-gcq.txt
 create mode 100644 drivers/iio/adc/fsl-imx25-gcq.c

diff --git a/Documentation/devicetree/bindings/iio/adc/fsl,imx25-gcq.txt b/Documentation/devicetree/bindings/iio/adc/fsl,imx25-gcq.txt
new file mode 100644
index 0000000..333fc55
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/fsl,imx25-gcq.txt
@@ -0,0 +1,54 @@
+Freescale i.MX25 ADC GCQ device
+
+This is a generic conversion queue device that can convert any of the analog
+inputs using the ADC unit of the i.MX25.
+
+Required properties:
+ - compatible: Should be "fsl,imx25-gcq".
+ - reg: Should be the register range of the module.
+ - interrupts: Should be the interrupt number of the module. Typically this is <1>.
+ - interrupt-parent: phandle to the tsadc module of the i.MX25.
+ - #address-cells: Should be <1> (setting for the subnodes)
+ - #size-cells: Should be <0> (setting for the subnodes)
+
+Optionally you can define subnodes which define the positive and negative
+reference voltage for one of the analog inputs.
+
+Required properties for subnodes:
+ - reg: Should be the number of the analog input.
+     0: xp
+     1: yp
+     2: xn
+     3: yn
+     4: wiper
+     5: inaux0
+     6: inaux1
+     7: inaux2
+ - fsl,adc-refp: Positive reference input
+     0: yp
+     1: xp
+     2: External reference
+     3: Internal reference
+ - fsl,adc-refn: Negative reference input
+     0: xn
+     1: yn
+     2: ngnd_adc
+     3: ngnd_adc
+
+
+Example:
+
+	adc: adc@50030800 {
+		compatible = "fsl,imx25-gcq";
+		reg = <0x50030800 0x60>;
+		interrupt-parent = <&tscadc>;
+		interrupts = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		inaux@5 {
+			reg = <5>;
+			fsl,adc-refp = <3>;
+			fsl,adc-refn = <3>;
+		};
+	};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a80d236..58efb8d 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -125,6 +125,13 @@ config EXYNOS_ADC
 	  of SoCs for drivers such as the touchscreen and hwmon to use to share
 	  this resource.
 
+config FSL_MX25_ADC
+	tristate "Freescale MX25 ADC driver"
+	depends on MFD_MX25_TSADC
+	help
+	  Generic Conversion Queue driver used for general purpose ADC in the
+	  MX25. This driver supports single measurements using the MX25 ADC.
+
 config LP8788_ADC
 	tristate "LP8788 ADC driver"
 	depends on MFD_LP8788
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 9d60f2d..2767fd6 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_AD7887) += ad7887.o
 obj-$(CONFIG_AD799X) += ad799x.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
 obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
+obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_MCP320X) += mcp320x.o
diff --git a/drivers/iio/adc/fsl-imx25-gcq.c b/drivers/iio/adc/fsl-imx25-gcq.c
new file mode 100644
index 0000000..1ae697c
--- /dev/null
+++ b/drivers/iio/adc/fsl-imx25-gcq.c
@@ -0,0 +1,338 @@
+/*
+ * Copyright 2014 Markus Pargmann, Pengutronix <mpa@pengutronix.de>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * This is the driver for the imx25 GCQ (Generic Conversion Queue)
+ * connected to the imx25 ADC.
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/iio/iio.h>
+#include <linux/mfd/imx25-tsadc.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MX25_GCQ_TIMEOUT (msecs_to_jiffies(2000))
+
+enum mx25_gcq_cfgs {
+	MX25_CFG_XP = 0,
+	MX25_CFG_YP,
+	MX25_CFG_XN,
+	MX25_CFG_YN,
+	MX25_CFG_WIPER,
+	MX25_CFG_INAUX0,
+	MX25_CFG_INAUX1,
+	MX25_CFG_INAUX2,
+	MX25_NUM_CFGS,
+};
+
+struct mx25_gcq_priv {
+	struct regmap *regs;
+	struct completion completed;
+	unsigned int settling_time;
+	struct clk *clk;
+	int irq;
+};
+
+#define MX25_IIO_CHAN(chan, id) {\
+		.type = IIO_VOLTAGE,\
+		.indexed = 1,\
+		.channel = chan,\
+		.address = chan,\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
+		.datasheet_name = id,				\
+	}
+
+static const struct iio_chan_spec mx25_gcq_channels[MX25_NUM_CFGS] = {
+	MX25_IIO_CHAN(0, "xp"),
+	MX25_IIO_CHAN(1, "yp"),
+	MX25_IIO_CHAN(2, "xn"),
+	MX25_IIO_CHAN(3, "yn"),
+	MX25_IIO_CHAN(4, "wiper"),
+	MX25_IIO_CHAN(5, "inaux0"),
+	MX25_IIO_CHAN(6, "inaux1"),
+	MX25_IIO_CHAN(7, "inaux2"),
+};
+
+static void mx25_gcq_disable_eoq(struct mx25_gcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_MR, MX25_ADCQ_MR_EOQ_IRQ,
+			MX25_ADCQ_MR_EOQ_IRQ);
+}
+
+static void mx25_gcq_enable_eoq(struct mx25_gcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_MR, MX25_ADCQ_MR_EOQ_IRQ, 0);
+}
+
+static irqreturn_t mx25_gcq_irq(int irq, void *data)
+{
+	struct mx25_gcq_priv *priv = data;
+	u32 stats;
+
+	regmap_read(priv->regs, MX25_ADCQ_SR, &stats);
+
+	if (stats & MX25_ADCQ_SR_EOQ) {
+		mx25_gcq_disable_eoq(priv);
+		complete(&priv->completed);
+	}
+
+	/* Disable conversion queue run */
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS, 0);
+
+	/* Acknowledge all possible irqs */
+	regmap_write(priv->regs, MX25_ADCQ_SR, MX25_ADCQ_SR_FRR |
+			MX25_ADCQ_SR_FUR | MX25_ADCQ_SR_FOR | MX25_ADCQ_SR_EOQ |
+			MX25_ADCQ_SR_PD);
+
+	return IRQ_HANDLED;
+}
+
+static int mx25_gcq_read_raw(struct iio_dev *idev,
+		struct iio_chan_spec const *chan, int *val, int *val2,
+		long mask)
+{
+	struct mx25_gcq_priv *priv = iio_priv(idev);
+	unsigned long timeout;
+	u32 data;
+	int ret;
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	mutex_lock(&idev->mlock);
+
+	/* Setup the configuration we want to use */
+	regmap_write(priv->regs, MX25_ADCQ_ITEM_7_0,
+			MX25_ADCQ_ITEM(0, chan->address));
+
+	mx25_gcq_enable_eoq(priv);
+
+	/* Trigger queue for one run */
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS,
+			MX25_ADCQ_CR_FQS);
+
+	timeout = wait_for_completion_interruptible_timeout(&priv->completed,
+			MX25_GCQ_TIMEOUT);
+	if (timeout < 0) {
+		dev_err(&idev->dev, "ADC wait for measurement failed\n");
+		ret = timeout;
+		goto out;
+	} else if (timeout == 0) {
+		dev_err(&idev->dev, "ADC timed out\n");
+		ret = -ETIMEDOUT;
+		goto out;
+	}
+
+	regmap_read(priv->regs, MX25_ADCQ_FIFO, &data);
+	*val = MX25_ADCQ_FIFO_DATA(data);
+
+	ret = IIO_VAL_INT;
+
+out:
+	mutex_unlock(&idev->mlock);
+
+	return ret;
+}
+
+static const struct iio_info mx25_gcq_iio_info = {
+	.read_raw = mx25_gcq_read_raw,
+};
+
+static const struct regmap_config mx25_gcq_regconfig = {
+	.max_register = 0x5c,
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static int mx25_gcq_setup_cfgs(struct platform_device *pdev,
+		struct mx25_gcq_priv *priv)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+	struct device *dev = &pdev->dev;
+	int ret;
+	int i;
+
+	/* Setup all configurations registers with a default conversion
+	 * configuration for each input */
+	for (i = 0; i != MX25_NUM_CFGS; ++i)
+		regmap_write(priv->regs, MX25_ADCQ_CFG(i),
+				MX25_ADCQ_CFG_YPLL_OFF |
+				MX25_ADCQ_CFG_XNUR_OFF |
+				MX25_ADCQ_CFG_XPUL_OFF |
+				MX25_ADCQ_CFG_REFP_INT |
+				(i << 4) |
+				MX25_ADCQ_CFG_REFN_NGND2);
+
+	for_each_child_of_node(np, child) {
+		u32 reg;
+		u32 refn;
+		u32 refp;
+
+		ret = of_property_read_u32(child, "reg", &reg);
+		if (ret) {
+			dev_err(dev, "Failed to get reg property\n");
+			return ret;
+		}
+		if (reg > MX25_NUM_CFGS) {
+			dev_err(dev, "reg value is greater than the number of available configuration registers\n");
+			return -EINVAL;
+		}
+
+		ret = of_property_read_u32(child, "fsl,adc-refn", &refn);
+		if (ret) {
+			dev_err(dev, "Failed to get fsl,adc-refn property\n");
+			return ret;
+		}
+		if (refn < 0 || refn > 3) {
+			dev_err(dev, "Invalid fsl,adc-refn property value %d\n",
+							refn);
+			return -EINVAL
+		}
+
+		ret = of_property_read_u32(child, "fsl,adc-refp", &refp);
+		if (ret) {
+			dev_err(dev, "Failed to get fsl,adc-refp property\n");
+			return ret;
+		}
+		if (refp < 0 || refp > 3) {
+			dev_err(dev, "Invalid fsl,adc-refp property value %d\n",
+							refp);
+			return -EINVAL;
+		}
+
+		regmap_update_bits(priv->regs, MX25_ADCQ_CFG(reg),
+				MX25_ADCQ_CFG_REFP_MASK |
+				MX25_ADCQ_CFG_REFN_MASK,
+				(refp << 7) | (refn << 2));
+	}
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR,
+			MX25_ADCQ_CR_FRST | MX25_ADCQ_CR_QRST,
+			MX25_ADCQ_CR_FRST | MX25_ADCQ_CR_QRST);
+
+	regmap_write(priv->regs, MX25_ADCQ_CR,
+			MX25_ADCQ_CR_PDMSK |
+			MX25_ADCQ_CR_QSM_FQS);
+
+	return 0;
+}
+
+static int mx25_gcq_probe(struct platform_device *pdev)
+{
+	struct iio_dev *idev;
+	struct mx25_gcq_priv *priv;
+	struct resource *res;
+	struct device *dev = &pdev->dev;
+	int ret;
+	void __iomem *mem;
+
+	idev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+	if (!idev)
+		return -ENOMEM;
+
+	priv = iio_priv(idev);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mem = devm_ioremap_resource(dev, res);
+	if (!mem)
+		return -ENOMEM;
+
+	priv->regs = devm_regmap_init_mmio(dev, mem, &mx25_gcq_regconfig);
+	if (IS_ERR(priv->regs)) {
+		dev_err(dev, "Failed to initialize regmap\n");
+		return PTR_ERR(priv->regs);
+	}
+
+	init_completion(&priv->completed);
+
+	ret = mx25_gcq_setup_cfgs(pdev, priv);
+	if (ret)
+		goto err_irq_free;
+
+	priv->clk = mx25_tsadc_get_ipg(pdev->dev.parent);
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable clock\n");
+		return ret;
+	}
+
+	priv->irq = platform_get_irq(pdev, 0);
+	if (priv->irq <= 0) {
+		dev_err(dev, "Failed to get IRQ\n");
+		err = priv->irq;
+		goto err_clk_unprepare;
+	}
+
+	ret = request_irq(priv->irq, mx25_gcq_irq, NULL, pdev->name,
+			priv);
+	if (ret) {
+		dev_err(dev, "Failed requesting IRQ\n");
+		goto err_clk_unprepare;
+	}
+
+	idev->dev.parent = &pdev->dev;
+	idev->channels = mx25_gcq_channels;
+	idev->num_channels = ARRAY_SIZE(mx25_gcq_channels);
+	idev->info = &mx25_gcq_iio_info;
+
+	ret = iio_device_register(idev);
+	if (ret) {
+		dev_err(dev, "Failed to register iio device\n");
+		goto err_irq_free;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+
+err_irq_free:
+	free_irq(priv->irq, (void *)priv);
+
+err_clk_unprepare:
+	clk_disable_unprepare(priv->clk);
+
+	return ret;
+}
+
+static int mx25_gcq_remove(struct platform_device *pdev)
+{
+	struct mx25_gcq_priv *priv = platform_get_drvdata(pdev);
+	struct iio_dev *idev = iio_priv_to_dev(pdev);
+
+	iio_device_unregister(idev);
+	free_irq(priv->irq, (void *)priv);
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static struct of_device_id mx25_gcq_ids[] = {
+	{ .compatible = "fsl,imx25-gcq", },
+	{ /* Senitel */ }
+};
+
+static struct platform_driver mx25_gcq_driver = {
+	.driver		= {
+		.name	= "mx25-gcq",
+		.owner	= THIS_MODULE,
+		.of_match_table = mx25_gcq_ids,
+	},
+	.probe		= mx25_gcq_probe,
+	.remove		= mx25_gcq_remove,
+};
+module_platform_driver(mx25_gcq_driver);
+
+MODULE_DESCRIPTION("ADC driver for Freescale mx25");
+MODULE_AUTHOR("Markus Pargmann <mpa@pengutronix.de>");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 2/6] input: touchscreen: imx25 tcq driver
From: Denis Carikli @ 2014-06-13 15:21 UTC (permalink / raw)
  To: Shawn Guo, Samuel Ortiz, Dmitry Torokhov, Jonathan Cameron
  Cc: Eric Bénard, Sascha Hauer, linux-arm-kernel, Lee Jones,
	linux-input, linux-iio, Fabio Estevam, Lars-Peter Clausen,
	Markus Pargmann, Denis Carikli
In-Reply-To: <1402672899-6995-1-git-send-email-denis@eukrea.com>

From: Markus Pargmann <mpa@pengutronix.de>

This is a driver for the imx25 ADC/TSC module. It controls the
touchscreen conversion queue and creates a touchscreen input device.
The driver currently only supports 4 wire touchscreens. The driver uses
a simple conversion queue of precharge, touch detection, X measurement,
Y measurement, precharge and another touch detection.

This driver uses the regmap from the parent to setup some touch specific
settings in the core driver and setup a idle configuration with touch
detection.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 .../bindings/input/touchscreen/fsl-mx25-tcq.txt    |   29 +
 drivers/input/touchscreen/Kconfig                  |    6 +
 drivers/input/touchscreen/Makefile                 |    1 +
 drivers/input/touchscreen/fsl-imx25-tcq.c          |  576 ++++++++++++++++++++
 4 files changed, 612 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/fsl-mx25-tcq.txt
 create mode 100644 drivers/input/touchscreen/fsl-imx25-tcq.c

diff --git a/Documentation/devicetree/bindings/input/touchscreen/fsl-mx25-tcq.txt b/Documentation/devicetree/bindings/input/touchscreen/fsl-mx25-tcq.txt
new file mode 100644
index 0000000..4214a99
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/fsl-mx25-tcq.txt
@@ -0,0 +1,29 @@
+Freescale mx25 TS conversion queue module
+
+mx25 touchscreen conversion queue module which controls the ADC unit of the
+mx25 for attached touchscreens.
+
+Required properties:
+ - compatible: Should be "fsl,imx25-tcq".
+ - reg: Memory range of the device.
+ - interrupts: Should be the interrupt number associated with this module within
+   the tscadc unit (<0>).
+ - interrupt-parent: Should be a phandle to the tscadc unit.
+ - fsl,wires: Should be '<4>' or '<5>'
+
+Optional properties:
+ - fsl,pen-debounce: Pen debounce time.
+ - fsl,pen-threshold: Pen-down threshold for the touchscreen.
+ - fsl,settling-time: Settling time in nanoseconds.
+
+This device includes two conversion queues which can be added as subnodes.
+The first queue is for the touchscreen, the second for general purpose ADC.
+
+Example:
+	tsc: tcq@50030400 {
+		compatible = "fsl,imx25-tcq";
+		reg = <0x50030400 0x60>;
+		interrupt-parent = <&tscadc>;
+		interrupts = <0>;
+		fsl,wires = <4>;
+	};
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index a23a94b..a2290b9 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -699,6 +699,12 @@ config TOUCHSCREEN_USB_COMPOSITE
 	  To compile this driver as a module, choose M here: the
 	  module will be called usbtouchscreen.
 
+config TOUCHSCREEN_MX25
+	tristate "Freescale i.MX25 touchscreen input driver"
+	depends on MFD_MX25_TSADC
+	help
+	  Enable support for touchscreen connected to your i.MX25.
+
 config TOUCHSCREEN_MC13783
 	tristate "Freescale MC13783 touchscreen input driver"
 	depends on MFD_MC13XXX
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 126479d..537d591 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_TOUCHSCREEN_INEXIO)	+= inexio.o
 obj-$(CONFIG_TOUCHSCREEN_INTEL_MID)	+= intel-mid-touch.o
 obj-$(CONFIG_TOUCHSCREEN_LPC32XX)	+= lpc32xx_ts.o
 obj-$(CONFIG_TOUCHSCREEN_MAX11801)	+= max11801_ts.o
+obj-$(CONFIG_TOUCHSCREEN_MX25)		+= fsl-imx25-tcq.o
 obj-$(CONFIG_TOUCHSCREEN_MC13783)	+= mc13783_ts.o
 obj-$(CONFIG_TOUCHSCREEN_MCS5000)	+= mcs5000_ts.o
 obj-$(CONFIG_TOUCHSCREEN_MIGOR)		+= migor_ts.o
diff --git a/drivers/input/touchscreen/fsl-imx25-tcq.c b/drivers/input/touchscreen/fsl-imx25-tcq.c
new file mode 100644
index 0000000..4ce576e
--- /dev/null
+++ b/drivers/input/touchscreen/fsl-imx25-tcq.c
@@ -0,0 +1,576 @@
+/*
+ * Copyright 2014 Markus Pargmann, Pengutronix <mpa@pengutronix.de>
+ * Based on driver from 2011:
+ *   Juergen Beisert, Pengutronix <kernel@pengutronix.de>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * This is the driver for the imx25 TCQ (Touchscreen Conversion Queue)
+ * connected to the imx25 ADC.
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/input.h>
+#include <linux/mfd/imx25-tsadc.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+static const char mx25_tcq_name[] = "mx25-tcq";
+
+enum mx25_tcq_mode {
+	MX25_TS_4WIRE,
+};
+
+struct mx25_tcq_priv {
+	struct regmap *regs;
+	struct regmap *core_regs;
+	struct input_dev *idev;
+	enum mx25_tcq_mode mode;
+	unsigned int pen_threshold;
+	unsigned int sample_count;
+	unsigned int expected_samples;
+	unsigned int pen_debounce;
+	unsigned int settling_time;
+	struct clk *clk;
+};
+
+static struct regmap_config mx25_tcq_regconfig = {
+	.fast_io = true,
+	.max_register = 0x5c,
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static struct of_device_id mx25_tcq_ids[] = {
+	{ .compatible = "fsl,imx25-tcq", },
+	{ /* Senitel */ }
+};
+
+#define TSC_4WIRE_PRE_INDEX 0
+#define TSC_4WIRE_X_INDEX 1
+#define TSC_4WIRE_Y_INDEX 2
+#define TSC_4WIRE_POST_INDEX 3
+#define TSC_4WIRE_LEAVE 4
+
+#define MX25_TSC_DEF_THRESHOLD 80
+#define TSC_MAX_SAMPLES 16
+
+
+enum mx25_adc_configurations {
+	MX25_CFG_PRECHARGE = 0,
+	MX25_CFG_TOUCH_DETECT,
+	MX25_CFG_X_MEASUREMENT,
+	MX25_CFG_Y_MEASUREMENT,
+};
+
+#define MX25_PRECHARGE_VALUE (\
+			MX25_ADCQ_CFG_YPLL_OFF | \
+			MX25_ADCQ_CFG_XNUR_OFF | \
+			MX25_ADCQ_CFG_XPUL_HIGH | \
+			MX25_ADCQ_CFG_REFP_INT | \
+			MX25_ADCQ_CFG_IN_XP | \
+			MX25_ADCQ_CFG_REFN_NGND2 | \
+			MX25_ADCQ_CFG_IGS)
+
+#define MX25_TOUCH_DETECT_VALUE (\
+			MX25_ADCQ_CFG_YNLR | \
+			MX25_ADCQ_CFG_YPLL_OFF | \
+			MX25_ADCQ_CFG_XNUR_OFF | \
+			MX25_ADCQ_CFG_XPUL_OFF | \
+			MX25_ADCQ_CFG_REFP_INT | \
+			MX25_ADCQ_CFG_IN_XP | \
+			MX25_ADCQ_CFG_REFN_NGND2 | \
+			MX25_ADCQ_CFG_PENIACK)
+
+static void imx25_setup_queue_cfgs(struct mx25_tcq_priv *priv,
+		unsigned int settling_time)
+{
+	u32 precharge_cfg =
+			MX25_PRECHARGE_VALUE |
+			MX25_ADCQ_CFG_SETTLING_TIME(settling_time);
+	u32 touch_detect_cfg =
+			MX25_TOUCH_DETECT_VALUE |
+			MX25_ADCQ_CFG_NOS(1) |
+			MX25_ADCQ_CFG_SETTLING_TIME(settling_time);
+
+	regmap_write(priv->core_regs, MX25_TSC_TICR, precharge_cfg);
+
+	/* PRECHARGE */
+	regmap_write(priv->regs, MX25_ADCQ_CFG(MX25_CFG_PRECHARGE),
+			precharge_cfg);
+
+	/* TOUCH_DETECT */
+	regmap_write(priv->regs, MX25_ADCQ_CFG(MX25_CFG_TOUCH_DETECT),
+			touch_detect_cfg);
+
+	/* X Measurement */
+	regmap_write(priv->regs, MX25_ADCQ_CFG(MX25_CFG_X_MEASUREMENT),
+			MX25_ADCQ_CFG_YPLL_OFF |
+			MX25_ADCQ_CFG_XNUR_LOW |
+			MX25_ADCQ_CFG_XPUL_HIGH |
+			MX25_ADCQ_CFG_REFP_XP |
+			MX25_ADCQ_CFG_IN_YP |
+			MX25_ADCQ_CFG_REFN_XN |
+			MX25_ADCQ_CFG_NOS(priv->sample_count) |
+			MX25_ADCQ_CFG_SETTLING_TIME(settling_time));
+
+	/* Y Measurement */
+	regmap_write(priv->regs, MX25_ADCQ_CFG(MX25_CFG_Y_MEASUREMENT),
+			MX25_ADCQ_CFG_YNLR |
+			MX25_ADCQ_CFG_YPLL_HIGH |
+			MX25_ADCQ_CFG_XNUR_OFF |
+			MX25_ADCQ_CFG_XPUL_OFF |
+			MX25_ADCQ_CFG_REFP_YP |
+			MX25_ADCQ_CFG_IN_XP |
+			MX25_ADCQ_CFG_REFN_YN |
+			MX25_ADCQ_CFG_NOS(priv->sample_count) |
+			MX25_ADCQ_CFG_SETTLING_TIME(settling_time));
+
+	/* Enable the touch detection right now */
+	regmap_write(priv->core_regs, MX25_TSC_TICR, touch_detect_cfg |
+			MX25_ADCQ_CFG_IGS);
+}
+
+static int imx25_setup_queue_4wire(struct mx25_tcq_priv *priv,
+		unsigned settling_time, int *items)
+{
+	imx25_setup_queue_cfgs(priv, settling_time);
+
+	/* Setup the conversion queue */
+	regmap_write(priv->regs, MX25_ADCQ_ITEM_7_0,
+			MX25_ADCQ_ITEM(0, MX25_CFG_PRECHARGE) |
+			MX25_ADCQ_ITEM(1, MX25_CFG_TOUCH_DETECT) |
+			MX25_ADCQ_ITEM(2, MX25_CFG_X_MEASUREMENT) |
+			MX25_ADCQ_ITEM(3, MX25_CFG_Y_MEASUREMENT) |
+			MX25_ADCQ_ITEM(4, MX25_CFG_PRECHARGE) |
+			MX25_ADCQ_ITEM(5, MX25_CFG_TOUCH_DETECT));
+
+	/* We measure X/Y with 'sample_count' number of samples and execute a
+	 * touch detection twice, with 1 sample each */
+	priv->expected_samples = priv->sample_count * 2 + 2;
+	*items = 6;
+
+	return 0;
+}
+
+static void mx25_tcq_disable_touch_irq(struct mx25_tcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_PDMSK,
+			MX25_ADCQ_CR_PDMSK);
+}
+
+static void mx25_tcq_enable_touch_irq(struct mx25_tcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_PDMSK, 0);
+}
+
+static void mx25_tcq_disable_fifo_irq(struct mx25_tcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_MR, MX25_ADCQ_MR_FDRY_IRQ,
+			MX25_ADCQ_MR_FDRY_IRQ);
+}
+
+static void mx25_tcq_enable_fifo_irq(struct mx25_tcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_MR, MX25_ADCQ_MR_FDRY_IRQ, 0);
+}
+
+static void mx25_tcq_force_queue_start(struct mx25_tcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS,
+			MX25_ADCQ_CR_FQS);
+
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS, 0);
+}
+
+static void mx25_tcq_force_queue_stop(struct mx25_tcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS, 0);
+}
+
+static void mx25_tcq_fifo_reset(struct mx25_tcq_priv *priv)
+{
+	u32 tcqcr;
+
+	regmap_read(priv->regs, MX25_ADCQ_CR, &tcqcr);
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FRST,
+			MX25_ADCQ_CR_FRST);
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FRST,
+			0);
+	regmap_write(priv->regs, MX25_ADCQ_CR, tcqcr);
+}
+
+static void mx25_tcq_re_enable_touch_detection(struct mx25_tcq_priv *priv)
+{
+	/* stop the queue from looping */
+	mx25_tcq_force_queue_stop(priv);
+
+	/* for a clean touch detection, preload the X plane */
+	regmap_write(priv->core_regs, MX25_TSC_TICR, MX25_PRECHARGE_VALUE);
+
+	/* waste some time now to pre-load the X plate to high voltage */
+	mx25_tcq_fifo_reset(priv);
+
+	/* re-enable the detection right now */
+	regmap_write(priv->core_regs, MX25_TSC_TICR, MX25_TOUCH_DETECT_VALUE |
+			MX25_ADCQ_CFG_IGS);
+
+	regmap_update_bits(priv->regs, MX25_ADCQ_SR, MX25_ADCQ_SR_PD,
+			MX25_ADCQ_SR_PD);
+
+	/* enable the pen down event to be a source for the interrupt */
+	regmap_update_bits(priv->regs, MX25_ADCQ_MR, MX25_ADCQ_MR_PD_IRQ, 0);
+
+	/* lets fire the next IRQ if someone touches the touchscreen */
+	mx25_tcq_enable_touch_irq(priv);
+}
+
+static int mx25_tcq_create_event_for_4wire(struct mx25_tcq_priv *priv,
+		u32 *sample_buf, int samples)
+{
+	unsigned int x_pos = 0;
+	unsigned int y_pos = 0;
+	unsigned int touch_pre = 0;
+	unsigned int touch_post = 0;
+	unsigned i;
+	int ret = 0;
+
+	for (i = 0; i < samples; i++) {
+		unsigned int index = MX25_ADCQ_FIFO_ID(sample_buf[i]);
+		unsigned int val = MX25_ADCQ_FIFO_DATA(sample_buf[i]);
+
+		switch (index) {
+		case 1:
+			touch_pre = val;
+			break;
+		case 2:
+			x_pos = val;
+			break;
+		case 3:
+			y_pos = val;
+			break;
+		case 5:
+			touch_post = val;
+			break;
+		default:
+			ret = -EINVAL;
+			break;
+		}
+	}
+
+	if (ret == 0 && samples != 0) {
+		/*
+		 * only if both touch measures are below a treshold,
+		 * the position is valid
+		 */
+		if (touch_pre < priv->pen_threshold &&
+					touch_post < priv->pen_threshold) {
+			/* valid samples, generate a report */
+			x_pos /= priv->sample_count;
+			y_pos /= priv->sample_count;
+			input_report_abs(priv->idev, ABS_X, x_pos);
+			input_report_abs(priv->idev, ABS_Y, y_pos);
+			input_report_key(priv->idev, BTN_TOUCH,
+					0xfff - ((touch_pre + touch_post) / 2));
+			input_sync(priv->idev);
+
+			/* get next sample */
+			mx25_tcq_force_queue_start(priv);
+			mx25_tcq_enable_fifo_irq(priv);
+		} else if (touch_pre >= priv->pen_threshold &&
+				touch_post >= priv->pen_threshold) {
+			/*
+			 * if both samples are invalid,
+			 * generate a release report
+			 */
+			input_report_key(priv->idev, BTN_TOUCH, 0);
+			input_sync(priv->idev);
+			mx25_tcq_re_enable_touch_detection(priv);
+		} else {
+			/*
+			 * if only one of both touch measurements are
+			 * below the threshold, still some bouncing
+			 * happens. Take additional samples in this
+			 * case to be sure
+			 */
+			mx25_tcq_force_queue_start(priv);
+			mx25_tcq_enable_fifo_irq(priv);
+		}
+	}
+
+	return ret;
+}
+
+static irqreturn_t mx25_tcq_irq_thread(int irq, void *dev_id)
+{
+	struct mx25_tcq_priv *priv = (struct mx25_tcq_priv *) dev_id;
+	u32 sample_buf[TSC_MAX_SAMPLES];
+	int samples = 0;
+
+	/* read all samples */
+	while (1) {
+		u32 stats;
+
+		regmap_read(priv->regs, MX25_ADCQ_SR, &stats);
+		if (stats & MX25_ADCQ_SR_EMPT)
+			break;
+
+		if (samples < TSC_MAX_SAMPLES) {
+			regmap_read(priv->regs, MX25_ADCQ_FIFO,
+							&sample_buf[samples]);
+			++samples;
+		} else {
+			u32 discarded;
+			/* discard samples */
+			regmap_read(priv->regs, MX25_ADCQ_FIFO, &discarded);
+		}
+	}
+
+	mx25_tcq_create_event_for_4wire(priv, sample_buf, samples);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t mx25_tcq_irq(int irq, void *dev_id)
+{
+	struct mx25_tcq_priv *priv = (struct mx25_tcq_priv *)dev_id;
+	u32 stat;
+	int ret = IRQ_HANDLED;
+
+	regmap_read(priv->regs, MX25_ADCQ_SR, &stat);
+
+	if (stat & (MX25_ADCQ_SR_FRR | MX25_ADCQ_SR_FUR | MX25_ADCQ_SR_FOR))
+		mx25_tcq_fifo_reset(priv);
+
+	if (stat & MX25_ADCQ_SR_PD) {
+		mx25_tcq_disable_touch_irq(priv);
+		mx25_tcq_force_queue_start(priv);
+		mx25_tcq_enable_fifo_irq(priv);
+	}
+
+	if (stat & MX25_ADCQ_SR_FDRY) {
+		mx25_tcq_disable_fifo_irq(priv);
+		ret = IRQ_WAKE_THREAD;
+	}
+
+	regmap_update_bits(priv->regs, MX25_ADCQ_SR, MX25_ADCQ_SR_FRR |
+			MX25_ADCQ_SR_FUR | MX25_ADCQ_SR_FOR | MX25_ADCQ_SR_PD |
+			MX25_ADCQ_SR_EOQ,
+			MX25_ADCQ_SR_FRR |
+			MX25_ADCQ_SR_FUR | MX25_ADCQ_SR_FOR | MX25_ADCQ_SR_PD |
+			MX25_ADCQ_SR_EOQ);
+
+	return ret;
+}
+
+/* configure the statemachine for a 4-wire touchscreen */
+static int mx25_tcq_init(struct mx25_tcq_priv *priv)
+{
+	u32 tgcr;
+	unsigned int ipg_div;
+	unsigned int adc_period;
+	unsigned int debounce_cnt;
+	unsigned int settling_time;
+	int itemct;
+	int ret;
+
+	regmap_read(priv->core_regs, MX25_TSC_TGCR, &tgcr);
+	ipg_div = max_t(unsigned int, 4, MX25_TGCR_GET_ADCCLK(tgcr));
+	adc_period = clk_get_rate(priv->clk) / (ipg_div * 2 + 2);
+	debounce_cnt = DIV_ROUND_UP(priv->pen_debounce, adc_period * 8) - 1;
+	settling_time = DIV_ROUND_UP(priv->settling_time, adc_period);
+
+
+	/* Reset */
+	regmap_write(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_QRST |
+			MX25_ADCQ_CR_FRST);
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_QRST |
+			MX25_ADCQ_CR_FRST, 0);
+
+	/* up to 128 * 8 ADC clocks are possible */
+	if (debounce_cnt > 127)
+		debounce_cnt = 127;
+
+	ret = imx25_setup_queue_4wire(priv, 0x0, &itemct);
+	if (ret)
+		return ret;
+
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_LITEMID_MASK |
+			MX25_ADCQ_CR_WMRK_MASK,
+			MX25_ADCQ_CR_LITEMID(itemct - 1) |
+			MX25_ADCQ_CR_WMRK(priv->expected_samples - 1));
+
+	/* setup debounce count */
+	regmap_update_bits(priv->core_regs, MX25_TSC_TGCR,
+			MX25_TGCR_PDBTIME_MASK,
+			MX25_TGCR_PDBTIME(debounce_cnt));
+
+	/* enable debounce */
+	regmap_update_bits(priv->core_regs, MX25_TSC_TGCR, MX25_TGCR_PDBEN,
+			MX25_TGCR_PDBEN);
+	regmap_update_bits(priv->core_regs, MX25_TSC_TGCR, MX25_TGCR_PDEN,
+			MX25_TGCR_PDEN);
+
+	/* enable the engine on demand */
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_QSM_FQS,
+			MX25_ADCQ_CR_QSM_FQS);
+
+	mx25_tcq_re_enable_touch_detection(priv);
+
+	return 0;
+}
+
+static int mx25_tcq_parse_dt(struct platform_device *pdev,
+		struct mx25_tcq_priv *priv)
+{
+	struct device_node *np = pdev->dev.of_node;
+	u32 wires;
+	int ret;
+
+	/* Setup defaults */
+	priv->pen_threshold = 500;
+	priv->sample_count = 3;
+	priv->pen_debounce = 1000000;
+	priv->settling_time = 250000;
+
+	ret = of_property_read_u32(np, "fsl,wires", &wires);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to find fsl,wires properties\n");
+		return ret;
+	}
+
+	if (wires == 4) {
+		priv->mode = MX25_TS_4WIRE;
+	} else {
+		dev_err(&pdev->dev, "%u-wire mode not supported\n", wires);
+		return -EINVAL;
+	}
+
+	/* These are optional, we don't care about the return values */
+	of_property_read_u32(np, "fsl,pen-threshold", &priv->pen_threshold);
+	of_property_read_u32(np, "fsl,settling-time", &priv->settling_time);
+	of_property_read_u32(np, "fsl,pen-debounce", &priv->pen_debounce);
+
+	return 0;
+}
+
+static int mx25_tcq_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct input_dev *idev;
+	struct mx25_tcq_priv *priv;
+	struct resource *res;
+	void __iomem *mem;
+	int ret;
+	int irq;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mem = devm_ioremap_resource(dev, res);
+	if (!mem)
+		return -ENOMEM;
+
+	ret = mx25_tcq_parse_dt(pdev, priv);
+	if (ret)
+		return ret;
+
+	priv->regs = devm_regmap_init_mmio(dev, mem, &mx25_tcq_regconfig);
+	if (IS_ERR(priv->regs)) {
+		dev_err(dev, "Failed to initialize regmap\n");
+		return PTR_ERR(priv->regs);
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq <= 0) {
+		dev_err(dev, "Failed to get IRQ\n");
+		return irq;
+	}
+
+	idev = devm_input_allocate_device(dev);
+	if (!idev) {
+		dev_err(dev, "Failed to allocate input device\n");
+		return -ENOMEM;
+	}
+
+	idev->name = mx25_tcq_name;
+	idev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	idev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+	input_set_abs_params(idev, ABS_X, 0, 0xfff, 0, 0);
+	input_set_abs_params(idev, ABS_Y, 0, 0xfff, 0, 0);
+
+	idev->id.bustype = BUS_HOST;
+
+	ret = input_register_device(idev);
+	if (ret) {
+		dev_err(dev, "Failed to register input device\n");
+		return ret;
+	}
+
+	priv->idev = idev;
+
+	priv->core_regs = mx25_tsadc_get_regmap(pdev->dev.parent);
+	priv->clk = mx25_tsadc_get_ipg(pdev->dev.parent);
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable ipg clock\n");
+		return ret;
+	}
+
+	ret = devm_request_threaded_irq(dev, irq, mx25_tcq_irq,
+			mx25_tcq_irq_thread, IRQF_ONESHOT, pdev->name, priv);
+	if (ret) {
+		dev_err(dev, "Failed requesting IRQ\n");
+		return ret;
+	}
+
+	ret = mx25_tcq_init(priv);
+	if (ret) {
+		dev_err(dev, "Failed to init tcq\n");
+		goto error_tcq_init;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+
+error_tcq_init:
+	clk_disable_unprepare(priv->clk);
+	return ret;
+}
+
+static int mx25_tcq_remove(struct platform_device *pdev)
+{
+	struct mx25_tcq_priv *priv = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static struct platform_driver mx25_tcq_driver = {
+	.driver		= {
+		.name	= "mx25-tcq",
+		.owner	= THIS_MODULE,
+		.of_match_table = mx25_tcq_ids,
+	},
+	.probe		= mx25_tcq_probe,
+	.remove		= mx25_tcq_remove,
+};
+module_platform_driver(mx25_tcq_driver);
+
+MODULE_DESCRIPTION("TS input driver for Freescale mx25");
+MODULE_AUTHOR("Markus Pargmann <mpa@pengutronix.de>");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 4/6] ARM: dts: imx25: Add touchscreen support
From: Denis Carikli @ 2014-06-13 15:21 UTC (permalink / raw)
  To: Shawn Guo, Samuel Ortiz, Dmitry Torokhov, Jonathan Cameron
  Cc: Eric Bénard, Sascha Hauer, linux-arm-kernel, Lee Jones,
	linux-input, linux-iio, Fabio Estevam, Lars-Peter Clausen,
	Markus Pargmann, Denis Carikli
In-Reply-To: <1402672899-6995-1-git-send-email-denis@eukrea.com>

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 arch/arm/boot/dts/imx25.dtsi |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
index 98783f5..5564167 100644
--- a/arch/arm/boot/dts/imx25.dtsi
+++ b/arch/arm/boot/dts/imx25.dtsi
@@ -264,12 +264,17 @@
 				status = "disabled";
 			};
 
-			tsc: tsc@50030000 {
-				compatible = "fsl,imx25-adc", "fsl,imx21-tsc";
-				reg = <0x50030000 0x4000>;
+			tscadc: tscadc@50030000 {
+				compatible = "fsl,imx25-tsadc";
+				reg = <0x50030000 0xc>;
 				interrupts = <46>;
 				clocks = <&clks 119>;
 				clock-names = "ipg";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+				ranges;
 				status = "disabled";
 			};
 
-- 
1.7.9.5


^ permalink raw reply related

* Dust off the "arm imx25 touchscreen/ADC drivers" patch serie.
From: Denis Carikli @ 2014-06-13 15:21 UTC (permalink / raw)
  To: Shawn Guo, Samuel Ortiz, Dmitry Torokhov, Jonathan Cameron
  Cc: Eric Bénard, Sascha Hauer, linux-arm-kernel, Lee Jones,
	linux-input, linux-iio, Fabio Estevam, Lars-Peter Clausen,
	Markus Pargmann
In-Reply-To: <1392913312-9030-1-git-send-email-mpa@pengutronix.de>


Hi,

I haven't seen any new comments on that serie since february,
so I picked it again and I tried to address all remaining comments.

To be able to test that patch, the following patch will need to be
cherry-picked.

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/263494.html

Denis.

^ permalink raw reply

* Re: [PATCH v5] leds: USB: HID: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-13  7:54 UTC (permalink / raw)
  To: Janne Kanniainen
  Cc: jkosina, johan, cooloney, linux-kernel, linux-leds, linux-usb,
	linux-input
In-Reply-To: <1402605252-9620-1-git-send-email-janne.kanniainen@gmail.com>

On Thu, Jun 12, 2014 at 11:34:12PM +0300, Janne Kanniainen wrote:
> This driver adds support for USB controlled led panels that exists in
> MSI GT683R laptop
> 
> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
> ---
> Changes in v2:
> 	- sorted headers to alphabetic order
> 	- using devm_kzalloc
> 	- using BIT(n)
> 	- using usb_control_msg instead of usb_submit_urb
> 	- removing unneeded code
> 
> Changes in v3:
> 	- implemented as HID device
> 	- some cleanups and bug fixes
> 
> Changes in v4:
> 	- more cleanups
> 	- support for selecting leds
> 	- suppport for selecting status
> 
> Changes in v5:
> 	- mode attribute documented under Documentation/ABI
> 	- made array for led_classdev
> 	- led devices uses now recommended naming scheme
> 
>  .../ABI/testing/sysfs-class-hid-driver-gt683r      |  10 +
>  drivers/hid/Kconfig                                |  11 +
>  drivers/hid/Makefile                               |   1 +
>  drivers/hid/hid-core.c                             |   1 +
>  drivers/hid/hid-gt683r.c                           | 294 +++++++++++++++++++++
>  drivers/hid/hid-ids.h                              |   2 +-
>  drivers/hid/usbhid/hid-quirks.c                    |   2 +-
>  7 files changed, 319 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
>  create mode 100644 drivers/hid/hid-gt683r.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> new file mode 100644
> index 0000000..c4d604e
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> @@ -0,0 +1,10 @@
> +What:		/sys/class/hidraw/<hidraw>/device/state

You should probably stick to "mode" (rather than "state") throughout (it
seems you just forgot to update a few uses).

> +Date:		Jun 2014
> +KernelVersion:	3.15

This should be 3.17.

> +Contact:	Janne Kanniainen <janne.kanniainen@gmail.com>
> +Description:
> +		Set the mode of LEDs
> +
> +		0 - normal
> +		1 - audio
> +		2 - breathing

Perhaps expand this with a short paragraph describing the different
modes.

> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 7af9d0b..d93e0ae 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -261,6 +261,17 @@ config HOLTEK_FF
>  	  Say Y here if you have a Holtek On Line Grip based game controller
>  	  and want to have force feedback support for it.
>  
> +config HID_GT683R
> +       tristate "MSI GT68xR LED support"
> +       depends on LEDS_CLASS && USB_HID
> +       ---help---
> +       Say Y here if you want to enable support for the MSI GT68xR LEDS
> +
> +       This driver support following states normal, breathing and audio.

You could also use "modes" and expand this with the same description.

> +       You can also select which leds you want to enable.
> +       Currently the following devices are know to be supported:
> +       - MSI GT683R
> +
>  config HID_HUION
>  	tristate "Huion tablets"
>  	depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index fc712dd..7129311 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_HID_EMS_FF)	+= hid-emsff.o
>  obj-$(CONFIG_HID_ELECOM)	+= hid-elecom.o
>  obj-$(CONFIG_HID_ELO)		+= hid-elo.o
>  obj-$(CONFIG_HID_EZKEY)		+= hid-ezkey.o
> +obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
>  obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
>  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-kbd.o
>  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-mouse.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index da52279..ec88fdb 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1827,6 +1827,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2) },
> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
> new file mode 100644
> index 0000000..6dffb76
> --- /dev/null
> +++ b/drivers/hid/hid-gt683r.c
> @@ -0,0 +1,294 @@
> +/*
> + * MSI GT683R led driver
> + *
> + * Copyright (c) 2014 Janne Kanniainen <janne.kanniainen@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +#define GT683R_LED_BACK				BIT(0)
> +#define GT683R_LED_SIDE				BIT(1)
> +#define GT683R_LED_FRONT			BIT(2)
> +
> +#define GT683R_BUFFER_SIZE			8
> +
> +/*
> + * GT683R_LED_OFF: all LEDs are off
> + * GT683R_LED_AUDIO: the status of LEDs depends
> + * on sound level
> + * GT683R_LED_BREATHING: LEDs brightness varies
> + * at human breathing rate
> + * GT683R_LED_NORMAL: LEDs are on
> + */
> +enum gt683r_led_mode {
> +	GT683R_LED_OFF = 0,
> +	GT683R_LED_AUDIO = 2,
> +	GT683R_LED_BREATHING = 3,
> +	GT683R_LED_NORMAL = 5
> +};
> +
> +enum gt683r_panels {
> +	back,
> +	side,
> +	front,
> +};

How about calling these GT683R_LED_BACK, etc, and simply do
BIT(GT683R_LED_BACK) in gt683r_led_setset_led below (and drop the
bitmask defines above).

You should also add a count enum/define rather than hard-code 3
throughout the driver, for example:

enum gt683r_panels {
	GT683R_LED_BACK = 0,
	GT683R_LED_SIDE = 1,
	GT683R_LED_FRONT = 2,
	GT683R_LED_COUNT
};

> +
> +const char *gt683r_panel_names[] = {
> +	"gt683r::back",
> +	"gt683r::side",
> +	"gt683r::front",
> +};

Your forgot to declare this static.

I don't think you should hard-code the device-part (i.e. "gt683r") but
rather derive it from the hid device (as in the driver I referred to as
an example). Just store the description here (e.g. "back"). More details
below.  

> +
> +struct gt683r_led {
> +	struct hid_device *hdev;
> +	struct led_classdev *led_devs[3];

You could keep these as embedded structs (an array of structs) rather
than do individual allocations in probe.

> +	struct mutex lock;
> +	struct work_struct work;
> +	enum led_brightness brightnesses[3];
> +	enum gt683r_led_mode mode;
> +};
> +
> +static const struct hid_device_id gt683r_led_id[] = {
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
> +	{ }
> +};
> +
> +static void gt683r_brightness_set(struct led_classdev *led_cdev,
> +				enum led_brightness brightness)
> +{
> +	int i;
> +	struct device *dev = led_cdev->dev->parent;
> +	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> +	struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> +	for (i = 0; i < 3; i++) {
> +		if (led->led_devs[i] != led_cdev)
> +			continue;
> +
> +		led->brightnesses[i] = brightness;
> +		schedule_work(&led->work);
> +		break;

I'd move the assignment and work scheduling out of the loop (and add an
explicit check for LED-not-found, if at all needed).

> +	}
> +}
> +
> +static ssize_t gt683r_show_mode(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct hid_device *hdev =
> +		container_of(dev, struct hid_device, dev);
> +	struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> +	if (led->mode == GT683R_LED_NORMAL)
> +		return scnprintf(buf, PAGE_SIZE, "0\n");
> +	else if (led->mode == GT683R_LED_AUDIO)
> +		return scnprintf(buf, PAGE_SIZE, "1\n");
> +	else
> +		return scnprintf(buf, PAGE_SIZE, "2\n");

How about mapping led->mode to sysfs mode in a temporary variable and
the just do one:

	return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);

at the end?

> +
> +}
> +
> +static ssize_t gt683r_store_mode(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	struct hid_device *hdev =
> +		container_of(dev, struct hid_device, dev);
> +	struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> +	mutex_lock(&led->lock);
> +
> +	if (!strncmp("0", buf, strlen("0"))) {
> +		led->mode = GT683R_LED_NORMAL;
> +	} else if (!strncmp("1", buf, strlen("1"))) {
> +		led->mode = GT683R_LED_AUDIO;
> +	} else if (!strncmp("2", buf, strlen("2"))) {
> +		led->mode = GT683R_LED_BREATHING;
> +	} else {
> +		count = -EINVAL;
> +		goto fail;
> +	}

Here you should use snprintf to parse buf as I already mentioned. With
the current implementation "22" would be accepted as a valid mode.

> +
> +	schedule_work(&led->work);
> +fail:
> +	mutex_unlock(&led->lock);

You should unlock before scheduling.

> +
> +	return count;
> +}
> +
> +static int gt683r_led_snd_msg(struct gt683r_led *led, u8 *msg)
> +{
> +	int ret;
> +
> +	ret = hid_hw_raw_request(led->hdev, msg[0], msg, GT683R_BUFFER_SIZE,
> +				HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> +	if (ret != GT683R_BUFFER_SIZE) {
> +		hid_err(led->hdev,
> +			"failed to send set report request: %i\n", ret);
> +		if (ret < 0)
> +			return ret;
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static void gt683r_led_set(struct gt683r_led *led, u8 leds, u8 mode)
> +{
> +	u8 *buffer;
> +
> +	buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
> +	if (!buffer)
> +		return;
> +
> +	buffer[0] = 0x01;
> +	buffer[1] = 0x02;
> +	buffer[2] = 0x30;
> +	buffer[3] = leds;
> +	if (gt683r_led_snd_msg(led, buffer))
> +		goto fail;
> +
> +	buffer[2] = 0x20;
> +	buffer[3] = mode;
> +	buffer[4] = 0x01;
> +	gt683r_led_snd_msg(led, buffer);

Ok, so you decided to continue setting mode on every LED brightness
update. That should be fine, but you never answered my question about
whether it is necessary?

> +
> +fail:
> +	kfree(buffer);
> +}
> +
> +static void gt683r_led_work(struct work_struct *work)
> +{
> +	struct gt683r_led *led =
> +			container_of(work, struct gt683r_led, work);
> +	u8 leds = 0;
> +
> +	mutex_lock(&led->lock);
> +
> +	if (led->brightnesses[back])
> +		leds |= GT683R_LED_BACK;
> +
> +	if (led->brightnesses[side])
> +		leds |= GT683R_LED_SIDE;
> +
> +	if (led->brightnesses[front])
> +		leds |= GT683R_LED_FRONT;
> +
> +	if (leds)
> +		gt683r_led_set(led, leds, led->mode);
> +	else
> +		gt683r_led_set(led, leds, GT683R_LED_OFF);
> +
> +	mutex_unlock(&led->lock);
> +}
> +
> +static DEVICE_ATTR(mode, 0644, gt683r_show_mode, gt683r_store_mode);

It is even recommended to use the new DEVICE_ATTR_RW macro here rather
(forgot about that).

> +
> +static int gt683r_led_probe(struct hid_device *hdev,
> +			const struct hid_device_id *id)
> +{
> +	int ret, i;
> +	struct gt683r_led *led;
> +
> +	led = devm_kzalloc(&hdev->dev, sizeof(struct gt683r_led), GFP_KERNEL);
> +	if (!led)
> +		return -ENOMEM;
> +
> +	led->mode = GT683R_LED_NORMAL;
> +	led->hdev = hdev;
> +	hid_set_drvdata(hdev, led);
> +
> +	ret = hid_parse(hdev);
> +	if (ret) {
> +		hid_err(hdev, "hid parsing failed\n");
> +		goto fail;
> +	}
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> +	if (ret) {
> +		hid_err(hdev, "hw start failed\n");
> +		goto fail;
> +	}
> +
> +	for (i = 0; i < 3; i++) {
> +		led->led_devs[i] = kzalloc(sizeof(struct led_classdev) +
> +					strlen(gt683r_panel_names[i]) + 1,

Why are you allocating space for the panel names here? Oh, now I see,
that's how it's done in the example I referred to. :)

Just embed the struct led_classdevs directly in struct gt683r_led, and
use devm_kzalloc(&hdev->dev, ...) to allocate the name buffers.

> +					GFP_KERNEL);
> +		if (!led->led_devs[i])
> +			goto fail2;
> +		led->led_devs[i]->name = gt683r_panel_names[i];

The other hid leds use dev_name(&hdev->dev) for the device name part of
the name. So determine the buffer length:

	strlen(dev_name(&hdev->dev)) + strlen(gt683r_panel_names[i]) + 3

and use snprintf with the following format "%s::%s".

> +		led->led_devs[i]->max_brightness = 1;
> +		led->led_devs[i]->brightness_set = gt683r_brightness_set;
> +		ret = led_classdev_register(&hdev->dev, led->led_devs[i]);
> +		if (ret) {
> +			hid_err(hdev, "could not register led device\n");
> +			goto fail2;
> +		}
> +	}
> +
> +	ret = device_create_file(&led->hdev->dev,
> +				&dev_attr_mode);
> +	if (ret) {
> +		hid_err(hdev, "could not make mode attribute file\n");
> +		goto fail2;
> +	}
> +
> +	mutex_init(&led->lock);
> +	INIT_WORK(&led->work, gt683r_led_work);
> +
> +	return 0;
> +
> +fail2:
> +	while (i-- > 0) {

Please use a for-loop instead (for readability).

> +		led_classdev_unregister(led->led_devs[i]);
> +		kfree(led->led_devs[i]);

This would currently leak memory if led_classdev_register fails, but
will work if you use devm_kzalloc as suggested above.

> +	}
> +	hid_hw_stop(hdev);
> +fail:
> +	return ret;
> +}
> +
> +static void gt683r_led_remove(struct hid_device *hdev)
> +{
> +	int i;
> +	struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> +	for (i = 0; i < 3; i++) {
> +		led_classdev_unregister(led->led_devs[i]);
> +		kfree(led->led_devs[i]);
> +	}
> +	cancel_work_sync(&led->work);
> +	device_remove_file(&hdev->dev,
> +			&dev_attr_mode);
> +	hid_hw_stop(hdev);
> +}
> +
> +static struct hid_driver gt683r_led_driver = {
> +	.probe = gt683r_led_probe,
> +	.remove = gt683r_led_remove,
> +	.name = "gt683r_led",
> +	.id_table = gt683r_led_id,
> +};
> +
> +module_hid_driver(gt683r_led_driver);
> +
> +MODULE_AUTHOR("Janne Kanniainen");
> +MODULE_DESCRIPTION("MSI GT683R led driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 34bb220..3692d37 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -641,7 +641,7 @@
>  #define USB_DEVICE_ID_GENIUS_KB29E	0x3004
>  
>  #define USB_VENDOR_ID_MSI		0x1770
> -#define USB_DEVICE_ID_MSI_GX680R_LED_PANEL	0xff00
> +#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
>  
>  #define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
>  #define USB_DEVICE_ID_N_S_HARMONY	0xc359
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 8e4ddb3..c640e1d 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -73,7 +73,7 @@ static const struct hid_blacklist {
>  	{ USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, HID_QUIRK_NO_INIT_REPORTS },
>  	{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
>  	{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
> -	{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
> +	{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
>  	{ USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },
>  	{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
>  	{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, HID_QUIRK_NO_INIT_REPORTS },

You're almost done. One last update? :)

Thanks,
Johan

^ permalink raw reply

* Re: [PATCH v2 1/1] elantech: Add support for trackpoint found on some v3 models
From: David Herrmann @ 2014-06-13  7:39 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Ulrik De Bie, Dmitry Torokhov, stable, open list:HID CORE LAYER
In-Reply-To: <539AA71D.2040808@redhat.com>

Hi

On Fri, Jun 13, 2014 at 9:24 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> You rely on external data here, so please check for truncation. If
>> anyone changes psmouse->ps2dev.serio->phys, they would not notice that
>> you rely on it here. So I'd do sth like:
>>
>> size_t n = snprintf(etd->tp_phys, sizeof(etd->tp_phys), "%s/input1",
>> psmouse->ps2dev.serio->phys);
>> if (n >= sizeof(etd->tp_phys))
>>         goto init_fail_tp_reg;
>
> Ugh no, we've this snprintf pattern for foo->phys everywhere in the kernel
> and we don't do a truncation check anywhere. No-one in its right mind will
> make phys smaller, and even if phys gets truncated things will likely
> still work fine.
>
>> Or at least force a terminating-zero on phys by decreasing the length
>> by 1 (phys is initialized to zero):
>
> snprintf already forces a terminating zero, no need for weird tricks.
>
>>
>> snprintf(etd->tp_phys, sizeof(etd->tp_phys) - 1, "%s/input1",
>> psmouse->ps2dev.serio->phys);
>
> So this is wrong, no need for the - 1.

Oh, right, snprintf() always writes null. Fair enough.

>>
>>> +               tp_dev->phys = etd->tp_phys;
>>> +               tp_dev->name = "Elantech PS/2 TrackPoint";
>>> +               tp_dev->id.bustype = BUS_I8042;
>>> +               tp_dev->id.vendor  = 0x0002;
>>> +               tp_dev->id.product = PSMOUSE_ELANTECH;
>>> +               tp_dev->id.version = 0x0000;
>>> +               tp_dev->dev.parent = &psmouse->ps2dev.serio->dev;
>>> +               tp_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
>>> +               tp_dev->relbit[BIT_WORD(REL_X)] = BIT_MASK(REL_X) | BIT_MASK(REL_Y);
>>> +               tp_dev->keybit[BIT_WORD(BTN_LEFT)] =
>>> +                       BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);
>>
>> Ugh, why not use __set_bit()? But ok, seems to be normal to use these
>> shortcuts for mice.
>>
>>> +               if (input_register_device(etd->tp_dev))
>>> +                       goto init_fail_tp_reg;
>>
>> Please forward the error code:
>>
>> int error = -EINVAL;
>>
>> ...
>>
>> error = input_register_device(tp_dev);
>> if (error < 0)
>>         goto init_fail_tp_reg;
>>
>> ...
>>
>> init_fail:
>> kfree(etd);
>> return error;
>>
>>> +       }
>>>
>>>         psmouse->protocol_handler = elantech_process_byte;
>>>         psmouse->disconnect = elantech_disconnect;
>>> @@ -1504,8 +1597,13 @@ int elantech_init(struct psmouse *psmouse)
>>>         psmouse->pktsize = etd->hw_version > 1 ? 6 : 4;
>>>
>>>         return 0;
>>> -
>>> + init_fail_tp_reg:
>>> +       input_free_device(tp_dev);
>>> + init_fail_tp_alloc:
>>> +       sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj,
>>> +                          &elantech_attr_group);
>>>   init_fail:
>>> +        psmouse_reset(psmouse);
>>
>> elantech_disconnect() does not call psmouse_reset, so why add it here?
>> What am I missing?
>
> AFAIK elantech disconnect will only get called if the ps2 mouse driver gets
> unloaded / the mouse gets hot-unplugged. Where as this gets called on
> probe failure, after which we want to turn the ps/2 mouse emulation
> back on so that it will work as a regular mouse.

But this sounds like a bugfix to me, which is unrelated to this
comment? If it is, please consider sending it separately.

Thanks
David

^ permalink raw reply

* Re: [PATCH v2 1/1] elantech: Add support for trackpoint found on some v3 models
From: David Herrmann @ 2014-06-13  6:24 UTC (permalink / raw)
  To: Ulrik De Bie
  Cc: Dmitry Torokhov, stable, open list:HID CORE LAYER, Hans de Goede
In-Reply-To: <1402596901-28784-2-git-send-email-ulrik.debie-os@e2big.org>

Hi

On Thu, Jun 12, 2014 at 8:15 PM, Ulrik De Bie <ulrik.debie-os@e2big.org> wrote:
> Some elantech v3 touchpad equipped laptops also have a trackpoint, before
> this commit, these give sync errors. With this patch, the trackpoint is
> provided as another input device: 'Elantech PS/2 TrackPoint'
>
> The patch will also output messages that do not follow the expected pattern.
> In the mean time I've seen 2 unknown packets occasionally:
> 0x04 , 0x00 , 0x00 , 0x00 , 0x00 , 0x00
> 0x00 , 0x00 , 0x00 , 0x02 , 0x00 , 0x00
> I don't know what those are for, but they can be safely ignored.
>
> Currently all packets that are not known to v3 touchpad and where
> packet[3] (the fourth byte) lowest nibble is 6 are now recognized as
> PACKET_TRACKPOINT and processed by the new elantech_report_trackpoint.
>
> This has been verified to work on a laptop Lenovo L530 where the
> touchpad/trackpoint combined identify themselves as:
> psmouse serio1: elantech: assuming hardware version 3 (with firmware version 0x350f02)
> psmouse serio1: elantech: Synaptics capabilities query result 0xb9, 0x15, 0x0c.

Thanks for pushing on this. Patch looks good to me, mostly minor nitpicks below.

> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
> ---
>  drivers/input/mouse/elantech.c | 104 +++++++++++++++++++++++++++++++++++++++--
>  drivers/input/mouse/elantech.h |   4 ++
>  2 files changed, 105 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index ee2a04d..7faec12 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -403,6 +403,63 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
>         input_sync(dev);
>  }
>
> +static void elantech_report_trackpoint(struct psmouse *psmouse,
> +                                      int packet_type)
> +{
> +       /* byte 0:  0   0 ~sx ~sy   0   M   R   L */
> +       /* byte 1: sx   0   0   0   0   0   0   0 */
> +       /* byte 2: sy   0   0   0   0   0   0   0 */
> +       /* byte 3:  0   0  sy  sx   0   1   1   0 */
> +       /* byte 4: x7  x6  x5  x4  x3  x2  x1  x0 */
> +       /* byte 5: y7  y6  y5  y4  y3  y2  y1  y0 */

Please make this a proper multi-byte comment instead of 'n'-comments.

> +
> +       /*
> +        * x and y are written in two's complement spread
> +        * over 9 bits with sx/sy the relative top bit and
> +        * x7..x0 and y7..y0 the lower bits.
> +        * The sign of y is opposite to what the input driver
> +        * expects for a relative movement
> +        */
> +
> +       struct elantech_data *etd = psmouse->private;
> +       struct input_dev *tp_dev = etd->tp_dev;
> +       unsigned char *packet = psmouse->packet;
> +       int x, y;
> +
> +       if (!etd->trackpoint_present) {

Why not drop "etd->trackpoint_present" entirely and rely on "etd->tp_dev"?

> +               psmouse_err(psmouse, "Unexpected trackpoint message\n");

Ugh, please use some protection here. I mean _if_ this message occurs,
it is very likely that it will occur thousands of times. So something
like:

static bool __section(.data.unlikely) __warned;

if (!etd->tp_dev) {
        if (!__warned) {
                __warned = true;
                psmouse_err(psmouse, "...");
        }

        return;
}

> +               if (etd->debug == 1)
> +                       elantech_packet_dump(psmouse);
> +               return;
> +       }
> +
> +       input_report_key(tp_dev, BTN_LEFT, packet[0] & 0x01);
> +       input_report_key(tp_dev, BTN_RIGHT, packet[0] & 0x02);
> +       input_report_key(tp_dev, BTN_MIDDLE, packet[0] & 0x04);
> +       x = (s32) ((u32) ((packet[1] & 0x80) ? 0UL : 0xFFFFFF00UL) | (u32)
> +                  packet[4]);
> +       y = -(s32) ((u32) ((packet[2] & 0x80) ? 0UL : 0xFFFFFF00UL) | (u32)
> +                   packet[5]);

nitpick: No need for "UL" here, you can use just "U". In the kernel we
rely on "unsigned int" to be 32bit, always! But the compiler should
optimize that, anyway.

Both (u32)-casts seem redundant. packet[] is "unsigned char" so it's
properly up-casted to "unsigned int". And "unsigned int" is
automatically casted to "int", so you can also drop the (s32) cast.
This should work:
    x = ((packet[1] & 0x80) ? 0U : 0xFFFFFF00U) | packet[4];
    y = -(int)(((packet[2] & 0x80) ? 0U : 0xFFFFFF00U) | packet[5]);
But please verify with tests.

> +       input_report_rel(tp_dev, REL_X, x);
> +       input_report_rel(tp_dev, REL_Y, y);

How about some new-lines between input_report_key() and the variable
assignments?

> +
> +       switch ((((u32) packet[0] & 0xF8) << 24) | ((u32) packet[1] << 16)
> +               | (u32) packet[2] << 8 | (u32) packet[3]) {

Please use a temporary variable for that. And we don't use spaces in
front of casts:

u32 t = ...;
switch(t) {
}

> +       case 0x00808036UL:
> +       case 0x10008026UL:
> +       case 0x20800016UL:
> +       case 0x30000006UL:
> +               break;
> +       default:
> +               /* Dump unexpected packet sequences if debug=1 (default) */
> +               if (etd->debug == 1)
> +                       elantech_packet_dump(psmouse);
> +               break;
> +       }
> +
> +       input_sync(tp_dev);
> +}
> +
>  /*
>   * Interpret complete data packets and report absolute mode input events for
>   * hardware version 3. (12 byte packets for two fingers)
> @@ -715,6 +772,8 @@ static int elantech_packet_check_v3(struct psmouse *psmouse)
>
>                 if ((packet[0] & 0x0c) == 0x0c && (packet[3] & 0xce) == 0x0c)
>                         return PACKET_V3_TAIL;
> +               if ((packet[3]&0x0f) == 0x06)

coding-stlye: please add spaces around binary-operators (a & b) like above.

> +                       return PACKET_TRACKPOINT;
>         }
>
>         return PACKET_UNKNOWN;
> @@ -798,7 +857,10 @@ static psmouse_ret_t elantech_process_byte(struct psmouse *psmouse)
>                 if (packet_type == PACKET_UNKNOWN)
>                         return PSMOUSE_BAD_DATA;
>
> -               elantech_report_absolute_v3(psmouse, packet_type);
> +               if (packet_type == PACKET_TRACKPOINT)
> +                       elantech_report_trackpoint(psmouse, packet_type);
> +               else
> +                       elantech_report_absolute_v3(psmouse, packet_type);
>                 break;
>
>         case 4:
> @@ -1018,8 +1080,10 @@ static int elantech_get_resolution_v4(struct psmouse *psmouse,
>   * Asus UX31               0x361f00        20, 15, 0e      clickpad
>   * Asus UX32VD             0x361f02        00, 15, 0e      clickpad
>   * Avatar AVIU-145A2       0x361f00        ?               clickpad
> + * Fujitsu H730            0x570f00        c0, 14, 0c      3 hw buttons (**)
>   * Gigabyte U2442          0x450f01        58, 17, 0c      2 hw buttons
>   * Lenovo L430             0x350f02        b9, 15, 0c      2 hw buttons (*)
> + * Lenovo L530             0x350f02        b9, 15, 0c      2 hw buttons (*)
>   * Samsung NF210           0x150b00        78, 14, 0a      2 hw buttons
>   * Samsung NP770Z5E        0x575f01        10, 15, 0f      clickpad
>   * Samsung NP700Z5B        0x361f06        21, 15, 0f      clickpad
> @@ -1029,6 +1093,8 @@ static int elantech_get_resolution_v4(struct psmouse *psmouse,
>   * Samsung RF710           0x450f00        ?               2 hw buttons
>   * System76 Pangolin       0x250f01        ?               2 hw buttons
>   * (*) + 3 trackpoint buttons
> + * (**) + 0 trackpoint buttons
> + * Note: Lenovo L430 and Lenovo L430 have the same fw_version/caps
>   */
>  static void elantech_set_buttonpad_prop(struct psmouse *psmouse)
>  {
> @@ -1324,6 +1390,11 @@ int elantech_detect(struct psmouse *psmouse, bool set_properties)
>   */
>  static void elantech_disconnect(struct psmouse *psmouse)
>  {
> +       struct elantech_data *etd = psmouse->private;
> +       struct input_dev *tp_dev = etd->tp_dev;

I think there's no need for "tp_dev" here. There's no cast involved,
so you can access it via "etd" safely.

> +
> +       if (etd->trackpoint_present && tp_dev)
> +               input_unregister_device(tp_dev);

Please change this to:

if (etd->tp_dev)
        input_unregister_device(etd->tp_dev);

There is no reason to test for trackpoint_present. tp_dev must be NULL
if not allocated.

>         sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj,
>                            &elantech_attr_group);
>         kfree(psmouse->private);
> @@ -1440,11 +1511,11 @@ int elantech_init(struct psmouse *psmouse)
>         struct elantech_data *etd;
>         int i, error;
>         unsigned char param[3];
> +       struct input_dev *tp_dev;
>
>         psmouse->private = etd = kzalloc(sizeof(struct elantech_data), GFP_KERNEL);
>         if (!etd)
>                 return -ENOMEM;
> -
>         psmouse_reset(psmouse);
>
>         etd->parity[0] = 1;
> @@ -1497,6 +1568,28 @@ int elantech_init(struct psmouse *psmouse)
>                             error);
>                 goto init_fail;
>         }

Please add a new-line here.

> +       etd->trackpoint_present = ((etd->capabilities[0] & 0x80) == 0x80);
> +       if (etd->trackpoint_present) {
> +               tp_dev = input_allocate_device();
> +               if (!tp_dev)
> +                       goto init_fail_tp_alloc;

Again, a new-line here please.

> +               etd->tp_dev = tp_dev;
> +               snprintf(etd->tp_phys, sizeof(etd->tp_phys), "%s/input1",
> +                       psmouse->ps2dev.serio->phys);

You rely on external data here, so please check for truncation. If
anyone changes psmouse->ps2dev.serio->phys, they would not notice that
you rely on it here. So I'd do sth like:

size_t n = snprintf(etd->tp_phys, sizeof(etd->tp_phys), "%s/input1",
psmouse->ps2dev.serio->phys);
if (n >= sizeof(etd->tp_phys))
        goto init_fail_tp_reg;

Or at least force a terminating-zero on phys by decreasing the length
by 1 (phys is initialized to zero):

snprintf(etd->tp_phys, sizeof(etd->tp_phys) - 1, "%s/input1",
psmouse->ps2dev.serio->phys);

> +               tp_dev->phys = etd->tp_phys;
> +               tp_dev->name = "Elantech PS/2 TrackPoint";
> +               tp_dev->id.bustype = BUS_I8042;
> +               tp_dev->id.vendor  = 0x0002;
> +               tp_dev->id.product = PSMOUSE_ELANTECH;
> +               tp_dev->id.version = 0x0000;
> +               tp_dev->dev.parent = &psmouse->ps2dev.serio->dev;
> +               tp_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
> +               tp_dev->relbit[BIT_WORD(REL_X)] = BIT_MASK(REL_X) | BIT_MASK(REL_Y);
> +               tp_dev->keybit[BIT_WORD(BTN_LEFT)] =
> +                       BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);

Ugh, why not use __set_bit()? But ok, seems to be normal to use these
shortcuts for mice.

> +               if (input_register_device(etd->tp_dev))
> +                       goto init_fail_tp_reg;

Please forward the error code:

int error = -EINVAL;

...

error = input_register_device(tp_dev);
if (error < 0)
        goto init_fail_tp_reg;

...

init_fail:
kfree(etd);
return error;

> +       }
>
>         psmouse->protocol_handler = elantech_process_byte;
>         psmouse->disconnect = elantech_disconnect;
> @@ -1504,8 +1597,13 @@ int elantech_init(struct psmouse *psmouse)
>         psmouse->pktsize = etd->hw_version > 1 ? 6 : 4;
>
>         return 0;
> -
> + init_fail_tp_reg:
> +       input_free_device(tp_dev);
> + init_fail_tp_alloc:
> +       sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj,
> +                          &elantech_attr_group);
>   init_fail:
> +        psmouse_reset(psmouse);

elantech_disconnect() does not call psmouse_reset, so why add it here?
What am I missing?

>         kfree(etd);
>         return -1;
>  }
> diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
> index 9e0e2a1..25cbc8c 100644
> --- a/drivers/input/mouse/elantech.h
> +++ b/drivers/input/mouse/elantech.h
> @@ -94,6 +94,7 @@
>  #define PACKET_V4_HEAD                 0x05
>  #define PACKET_V4_MOTION               0x06
>  #define PACKET_V4_STATUS               0x07
> +#define PACKET_TRACKPOINT              0x08
>
>  /*
>   * track up to 5 fingers for v4 hardware
> @@ -114,6 +115,8 @@ struct finger_pos {
>  };
>
>  struct elantech_data {
> +       struct input_dev *tp_dev;               /* Relative device */

This comment is really misleading, I'd just drop it or change it to
"trackpoint device".

> +       char    tp_phys[32];
>         unsigned char reg_07;
>         unsigned char reg_10;
>         unsigned char reg_11;
> @@ -130,6 +133,7 @@ struct elantech_data {
>         bool jumpy_cursor;
>         bool reports_pressure;
>         bool crc_enabled;
> +       bool trackpoint_present;

Why do you use "tp_*" prefix above, but change it to "trackpoint_*"
here? "tp_present" seems more consistent to me.

Thanks
David

>         bool set_hw_resolution;
>         unsigned char hw_version;
>         unsigned int fw_version;
> --
> 2.0.0.rc2
>

^ permalink raw reply

* Re: [PATCH v2 1/1] elantech: Add support for trackpoint found on some v3 models
From: Greg KH @ 2014-06-12 22:13 UTC (permalink / raw)
  To: ulrik.debie-os
  Cc: Dmitry Torokhov, stable, linux-input, Hans de Goede,
	David Herrmann
In-Reply-To: <20140612190344.GA20952@lantern>

On Thu, Jun 12, 2014 at 09:03:45PM +0200, ulrik.debie-os@e2big.org wrote:
> Hi Greg,
> 
> Without this patch the elantech.c driver will still bind to the
> touchpad, and then get out of sync each time the user touches
> the trackpoint or the 3 mouse buttons associated with the trackpoint.
> 
> This out of sync will result in a few dmesg lines and a few lost
> touchpad input events. The fact that the trackpoint + buttons itself
> do not work at all is a minor annoyance compared to the sync loss
> effect on the touchpad events.
> 
> This resulted in multiple bug reports with patches:
> http://tojaj.com/fedora-20-howto-fix-elantech-trackpoint-on-lenovo-thinkpad-l430/
> http://permalink.gmane.org/gmane.linux.kernel.input/27763
> https://bugzilla.kernel.org/show_bug.cgi?id=48161
> 
> Hans suggested me to consider a cc:stable.

Broken hardware is never nice, but as this isn't a regression, and has
never worked, asking to update to a new kernel seems reasonable.

thanks,

greg k-h

^ permalink raw reply

* [PATCH v5] leds: USB: HID: Add support for MSI GT683R led panels
From: Janne Kanniainen @ 2014-06-12 20:34 UTC (permalink / raw)
  To: jkosina
  Cc: johan, cooloney, linux-kernel, linux-leds, linux-usb, linux-input,
	Janne Kanniainen
In-Reply-To: <20140612090636.GK10256@localhost>

This driver adds support for USB controlled led panels that exists in
MSI GT683R laptop

Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
---
Changes in v2:
	- sorted headers to alphabetic order
	- using devm_kzalloc
	- using BIT(n)
	- using usb_control_msg instead of usb_submit_urb
	- removing unneeded code

Changes in v3:
	- implemented as HID device
	- some cleanups and bug fixes

Changes in v4:
	- more cleanups
	- support for selecting leds
	- suppport for selecting status

Changes in v5:
	- mode attribute documented under Documentation/ABI
	- made array for led_classdev
	- led devices uses now recommended naming scheme

 .../ABI/testing/sysfs-class-hid-driver-gt683r      |  10 +
 drivers/hid/Kconfig                                |  11 +
 drivers/hid/Makefile                               |   1 +
 drivers/hid/hid-core.c                             |   1 +
 drivers/hid/hid-gt683r.c                           | 294 +++++++++++++++++++++
 drivers/hid/hid-ids.h                              |   2 +-
 drivers/hid/usbhid/hid-quirks.c                    |   2 +-
 7 files changed, 319 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
 create mode 100644 drivers/hid/hid-gt683r.c

diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
new file mode 100644
index 0000000..c4d604e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
@@ -0,0 +1,10 @@
+What:		/sys/class/hidraw/<hidraw>/device/state
+Date:		Jun 2014
+KernelVersion:	3.15
+Contact:	Janne Kanniainen <janne.kanniainen@gmail.com>
+Description:
+		Set the mode of LEDs
+
+		0 - normal
+		1 - audio
+		2 - breathing
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 7af9d0b..d93e0ae 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -261,6 +261,17 @@ config HOLTEK_FF
 	  Say Y here if you have a Holtek On Line Grip based game controller
 	  and want to have force feedback support for it.
 
+config HID_GT683R
+       tristate "MSI GT68xR LED support"
+       depends on LEDS_CLASS && USB_HID
+       ---help---
+       Say Y here if you want to enable support for the MSI GT68xR LEDS
+
+       This driver support following states normal, breathing and audio.
+       You can also select which leds you want to enable.
+       Currently the following devices are know to be supported:
+       - MSI GT683R
+
 config HID_HUION
 	tristate "Huion tablets"
 	depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index fc712dd..7129311 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_HID_EMS_FF)	+= hid-emsff.o
 obj-$(CONFIG_HID_ELECOM)	+= hid-elecom.o
 obj-$(CONFIG_HID_ELO)		+= hid-elo.o
 obj-$(CONFIG_HID_EZKEY)		+= hid-ezkey.o
+obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
 obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-kbd.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-mouse.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index da52279..ec88fdb 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1827,6 +1827,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2) },
diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
new file mode 100644
index 0000000..6dffb76
--- /dev/null
+++ b/drivers/hid/hid-gt683r.c
@@ -0,0 +1,294 @@
+/*
+ * MSI GT683R led driver
+ *
+ * Copyright (c) 2014 Janne Kanniainen <janne.kanniainen@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+#define GT683R_LED_BACK				BIT(0)
+#define GT683R_LED_SIDE				BIT(1)
+#define GT683R_LED_FRONT			BIT(2)
+
+#define GT683R_BUFFER_SIZE			8
+
+/*
+ * GT683R_LED_OFF: all LEDs are off
+ * GT683R_LED_AUDIO: the status of LEDs depends
+ * on sound level
+ * GT683R_LED_BREATHING: LEDs brightness varies
+ * at human breathing rate
+ * GT683R_LED_NORMAL: LEDs are on
+ */
+enum gt683r_led_mode {
+	GT683R_LED_OFF = 0,
+	GT683R_LED_AUDIO = 2,
+	GT683R_LED_BREATHING = 3,
+	GT683R_LED_NORMAL = 5
+};
+
+enum gt683r_panels {
+	back,
+	side,
+	front,
+};
+
+const char *gt683r_panel_names[] = {
+	"gt683r::back",
+	"gt683r::side",
+	"gt683r::front",
+};
+
+struct gt683r_led {
+	struct hid_device *hdev;
+	struct led_classdev *led_devs[3];
+	struct mutex lock;
+	struct work_struct work;
+	enum led_brightness brightnesses[3];
+	enum gt683r_led_mode mode;
+};
+
+static const struct hid_device_id gt683r_led_id[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
+	{ }
+};
+
+static void gt683r_brightness_set(struct led_classdev *led_cdev,
+				enum led_brightness brightness)
+{
+	int i;
+	struct device *dev = led_cdev->dev->parent;
+	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+	struct gt683r_led *led = hid_get_drvdata(hdev);
+
+	for (i = 0; i < 3; i++) {
+		if (led->led_devs[i] != led_cdev)
+			continue;
+
+		led->brightnesses[i] = brightness;
+		schedule_work(&led->work);
+		break;
+	}
+}
+
+static ssize_t gt683r_show_mode(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct hid_device *hdev =
+		container_of(dev, struct hid_device, dev);
+	struct gt683r_led *led = hid_get_drvdata(hdev);
+
+	if (led->mode == GT683R_LED_NORMAL)
+		return scnprintf(buf, PAGE_SIZE, "0\n");
+	else if (led->mode == GT683R_LED_AUDIO)
+		return scnprintf(buf, PAGE_SIZE, "1\n");
+	else
+		return scnprintf(buf, PAGE_SIZE, "2\n");
+
+}
+
+static ssize_t gt683r_store_mode(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct hid_device *hdev =
+		container_of(dev, struct hid_device, dev);
+	struct gt683r_led *led = hid_get_drvdata(hdev);
+
+	mutex_lock(&led->lock);
+
+	if (!strncmp("0", buf, strlen("0"))) {
+		led->mode = GT683R_LED_NORMAL;
+	} else if (!strncmp("1", buf, strlen("1"))) {
+		led->mode = GT683R_LED_AUDIO;
+	} else if (!strncmp("2", buf, strlen("2"))) {
+		led->mode = GT683R_LED_BREATHING;
+	} else {
+		count = -EINVAL;
+		goto fail;
+	}
+
+	schedule_work(&led->work);
+fail:
+	mutex_unlock(&led->lock);
+
+	return count;
+}
+
+static int gt683r_led_snd_msg(struct gt683r_led *led, u8 *msg)
+{
+	int ret;
+
+	ret = hid_hw_raw_request(led->hdev, msg[0], msg, GT683R_BUFFER_SIZE,
+				HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+	if (ret != GT683R_BUFFER_SIZE) {
+		hid_err(led->hdev,
+			"failed to send set report request: %i\n", ret);
+		if (ret < 0)
+			return ret;
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static void gt683r_led_set(struct gt683r_led *led, u8 leds, u8 mode)
+{
+	u8 *buffer;
+
+	buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
+	if (!buffer)
+		return;
+
+	buffer[0] = 0x01;
+	buffer[1] = 0x02;
+	buffer[2] = 0x30;
+	buffer[3] = leds;
+	if (gt683r_led_snd_msg(led, buffer))
+		goto fail;
+
+	buffer[2] = 0x20;
+	buffer[3] = mode;
+	buffer[4] = 0x01;
+	gt683r_led_snd_msg(led, buffer);
+
+fail:
+	kfree(buffer);
+}
+
+static void gt683r_led_work(struct work_struct *work)
+{
+	struct gt683r_led *led =
+			container_of(work, struct gt683r_led, work);
+	u8 leds = 0;
+
+	mutex_lock(&led->lock);
+
+	if (led->brightnesses[back])
+		leds |= GT683R_LED_BACK;
+
+	if (led->brightnesses[side])
+		leds |= GT683R_LED_SIDE;
+
+	if (led->brightnesses[front])
+		leds |= GT683R_LED_FRONT;
+
+	if (leds)
+		gt683r_led_set(led, leds, led->mode);
+	else
+		gt683r_led_set(led, leds, GT683R_LED_OFF);
+
+	mutex_unlock(&led->lock);
+}
+
+static DEVICE_ATTR(mode, 0644, gt683r_show_mode, gt683r_store_mode);
+
+static int gt683r_led_probe(struct hid_device *hdev,
+			const struct hid_device_id *id)
+{
+	int ret, i;
+	struct gt683r_led *led;
+
+	led = devm_kzalloc(&hdev->dev, sizeof(struct gt683r_led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	led->mode = GT683R_LED_NORMAL;
+	led->hdev = hdev;
+	hid_set_drvdata(hdev, led);
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		hid_err(hdev, "hid parsing failed\n");
+		goto fail;
+	}
+
+	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+	if (ret) {
+		hid_err(hdev, "hw start failed\n");
+		goto fail;
+	}
+
+	for (i = 0; i < 3; i++) {
+		led->led_devs[i] = kzalloc(sizeof(struct led_classdev) +
+					strlen(gt683r_panel_names[i]) + 1,
+					GFP_KERNEL);
+		if (!led->led_devs[i])
+			goto fail2;
+		led->led_devs[i]->name = gt683r_panel_names[i];
+		led->led_devs[i]->max_brightness = 1;
+		led->led_devs[i]->brightness_set = gt683r_brightness_set;
+		ret = led_classdev_register(&hdev->dev, led->led_devs[i]);
+		if (ret) {
+			hid_err(hdev, "could not register led device\n");
+			goto fail2;
+		}
+	}
+
+	ret = device_create_file(&led->hdev->dev,
+				&dev_attr_mode);
+	if (ret) {
+		hid_err(hdev, "could not make mode attribute file\n");
+		goto fail2;
+	}
+
+	mutex_init(&led->lock);
+	INIT_WORK(&led->work, gt683r_led_work);
+
+	return 0;
+
+fail2:
+	while (i-- > 0) {
+		led_classdev_unregister(led->led_devs[i]);
+		kfree(led->led_devs[i]);
+	}
+	hid_hw_stop(hdev);
+fail:
+	return ret;
+}
+
+static void gt683r_led_remove(struct hid_device *hdev)
+{
+	int i;
+	struct gt683r_led *led = hid_get_drvdata(hdev);
+
+	for (i = 0; i < 3; i++) {
+		led_classdev_unregister(led->led_devs[i]);
+		kfree(led->led_devs[i]);
+	}
+	cancel_work_sync(&led->work);
+	device_remove_file(&hdev->dev,
+			&dev_attr_mode);
+	hid_hw_stop(hdev);
+}
+
+static struct hid_driver gt683r_led_driver = {
+	.probe = gt683r_led_probe,
+	.remove = gt683r_led_remove,
+	.name = "gt683r_led",
+	.id_table = gt683r_led_id,
+};
+
+module_hid_driver(gt683r_led_driver);
+
+MODULE_AUTHOR("Janne Kanniainen");
+MODULE_DESCRIPTION("MSI GT683R led driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 34bb220..3692d37 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -641,7 +641,7 @@
 #define USB_DEVICE_ID_GENIUS_KB29E	0x3004
 
 #define USB_VENDOR_ID_MSI		0x1770
-#define USB_DEVICE_ID_MSI_GX680R_LED_PANEL	0xff00
+#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
 
 #define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
 #define USB_DEVICE_ID_N_S_HARMONY	0xc359
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 8e4ddb3..c640e1d 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -73,7 +73,7 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, HID_QUIRK_NO_INIT_REPORTS },
 	{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
-	{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
+	{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
 	{ USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },
 	{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
 	{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, HID_QUIRK_NO_INIT_REPORTS },
-- 
1.9.2

^ permalink raw reply related

* Re: [PATCH v2 1/1] elantech: Add support for trackpoint found on some v3 models
From: ulrik.debie-os @ 2014-06-12 19:03 UTC (permalink / raw)
  To: Greg KH; +Cc: Dmitry Torokhov, stable, linux-input, Hans de Goede,
	David Herrmann
In-Reply-To: <20140612184844.GA25788@kroah.com>

Hi Greg,

Without this patch the elantech.c driver will still bind to the
touchpad, and then get out of sync each time the user touches
the trackpoint or the 3 mouse buttons associated with the trackpoint.

This out of sync will result in a few dmesg lines and a few lost
touchpad input events. The fact that the trackpoint + buttons itself
do not work at all is a minor annoyance compared to the sync loss
effect on the touchpad events.

This resulted in multiple bug reports with patches:
http://tojaj.com/fedora-20-howto-fix-elantech-trackpoint-on-lenovo-thinkpad-l430/
http://permalink.gmane.org/gmane.linux.kernel.input/27763
https://bugzilla.kernel.org/show_bug.cgi?id=48161

Hans suggested me to consider a cc:stable.

Thank you,
Greetings,
Ulrik


On Thu, Jun 12, 2014 at 11:48:44AM -0700, Greg KH wrote:
> On Thu, Jun 12, 2014 at 08:15:01PM +0200, Ulrik De Bie wrote:
> > Some elantech v3 touchpad equipped laptops also have a trackpoint, before
> > this commit, these give sync errors. With this patch, the trackpoint is
> > provided as another input device: 'Elantech PS/2 TrackPoint'
> > 
> > The patch will also output messages that do not follow the expected pattern.
> > In the mean time I've seen 2 unknown packets occasionally:
> > 0x04 , 0x00 , 0x00 , 0x00 , 0x00 , 0x00
> > 0x00 , 0x00 , 0x00 , 0x02 , 0x00 , 0x00
> > I don't know what those are for, but they can be safely ignored.
> > 
> > Currently all packets that are not known to v3 touchpad and where
> > packet[3] (the fourth byte) lowest nibble is 6 are now recognized as
> > PACKET_TRACKPOINT and processed by the new elantech_report_trackpoint.
> > 
> > This has been verified to work on a laptop Lenovo L530 where the
> > touchpad/trackpoint combined identify themselves as:
> > psmouse serio1: elantech: assuming hardware version 3 (with firmware version 0x350f02)
> > psmouse serio1: elantech: Synaptics capabilities query result 0xb9, 0x15, 0x0c.
> > 
> > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
> > ---
> >  drivers/input/mouse/elantech.c | 104 +++++++++++++++++++++++++++++++++++++++--
> >  drivers/input/mouse/elantech.h |   4 ++
> >  2 files changed, 105 insertions(+), 3 deletions(-)
> 
> This seems like a new feature to me and not a stable kernel patch.  How
> does this fit in with the Documentation/stable_kernel_rules.txt
> requirements?
> 
> thanks,
> 
> greg k-h

^ permalink raw reply

* Re: [PATCH v2 1/1] elantech: Add support for trackpoint found on some v3 models
From: Greg KH @ 2014-06-12 18:48 UTC (permalink / raw)
  To: Ulrik De Bie
  Cc: Dmitry Torokhov, stable, linux-input, Hans de Goede,
	David Herrmann
In-Reply-To: <1402596901-28784-2-git-send-email-ulrik.debie-os@e2big.org>

On Thu, Jun 12, 2014 at 08:15:01PM +0200, Ulrik De Bie wrote:
> Some elantech v3 touchpad equipped laptops also have a trackpoint, before
> this commit, these give sync errors. With this patch, the trackpoint is
> provided as another input device: 'Elantech PS/2 TrackPoint'
> 
> The patch will also output messages that do not follow the expected pattern.
> In the mean time I've seen 2 unknown packets occasionally:
> 0x04 , 0x00 , 0x00 , 0x00 , 0x00 , 0x00
> 0x00 , 0x00 , 0x00 , 0x02 , 0x00 , 0x00
> I don't know what those are for, but they can be safely ignored.
> 
> Currently all packets that are not known to v3 touchpad and where
> packet[3] (the fourth byte) lowest nibble is 6 are now recognized as
> PACKET_TRACKPOINT and processed by the new elantech_report_trackpoint.
> 
> This has been verified to work on a laptop Lenovo L530 where the
> touchpad/trackpoint combined identify themselves as:
> psmouse serio1: elantech: assuming hardware version 3 (with firmware version 0x350f02)
> psmouse serio1: elantech: Synaptics capabilities query result 0xb9, 0x15, 0x0c.
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
> ---
>  drivers/input/mouse/elantech.c | 104 +++++++++++++++++++++++++++++++++++++++--
>  drivers/input/mouse/elantech.h |   4 ++
>  2 files changed, 105 insertions(+), 3 deletions(-)

This seems like a new feature to me and not a stable kernel patch.  How
does this fit in with the Documentation/stable_kernel_rules.txt
requirements?

thanks,

greg k-h

^ permalink raw reply

* [PATCH v2 1/1] elantech: Add support for trackpoint found on some v3 models
From: Ulrik De Bie @ 2014-06-12 18:15 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: stable, linux-input, Hans de Goede, David Herrmann,
	ulrik.debie-os
In-Reply-To: <1402596901-28784-1-git-send-email-ulrik.debie-os@e2big.org>

Some elantech v3 touchpad equipped laptops also have a trackpoint, before
this commit, these give sync errors. With this patch, the trackpoint is
provided as another input device: 'Elantech PS/2 TrackPoint'

The patch will also output messages that do not follow the expected pattern.
In the mean time I've seen 2 unknown packets occasionally:
0x04 , 0x00 , 0x00 , 0x00 , 0x00 , 0x00
0x00 , 0x00 , 0x00 , 0x02 , 0x00 , 0x00
I don't know what those are for, but they can be safely ignored.

Currently all packets that are not known to v3 touchpad and where
packet[3] (the fourth byte) lowest nibble is 6 are now recognized as
PACKET_TRACKPOINT and processed by the new elantech_report_trackpoint.

This has been verified to work on a laptop Lenovo L530 where the
touchpad/trackpoint combined identify themselves as:
psmouse serio1: elantech: assuming hardware version 3 (with firmware version 0x350f02)
psmouse serio1: elantech: Synaptics capabilities query result 0xb9, 0x15, 0x0c.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
---
 drivers/input/mouse/elantech.c | 104 +++++++++++++++++++++++++++++++++++++++--
 drivers/input/mouse/elantech.h |   4 ++
 2 files changed, 105 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index ee2a04d..7faec12 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -403,6 +403,63 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 	input_sync(dev);
 }
 
+static void elantech_report_trackpoint(struct psmouse *psmouse,
+				       int packet_type)
+{
+	/* byte 0:  0   0 ~sx ~sy   0   M   R   L */
+	/* byte 1: sx   0   0   0   0   0   0   0 */
+	/* byte 2: sy   0   0   0   0   0   0   0 */
+	/* byte 3:  0   0  sy  sx   0   1   1   0 */
+	/* byte 4: x7  x6  x5  x4  x3  x2  x1  x0 */
+	/* byte 5: y7  y6  y5  y4  y3  y2  y1  y0 */
+
+	/*
+	 * x and y are written in two's complement spread
+	 * over 9 bits with sx/sy the relative top bit and
+	 * x7..x0 and y7..y0 the lower bits.
+	 * The sign of y is opposite to what the input driver
+	 * expects for a relative movement
+	 */
+
+	struct elantech_data *etd = psmouse->private;
+	struct input_dev *tp_dev = etd->tp_dev;
+	unsigned char *packet = psmouse->packet;
+	int x, y;
+
+	if (!etd->trackpoint_present) {
+		psmouse_err(psmouse, "Unexpected trackpoint message\n");
+		if (etd->debug == 1)
+			elantech_packet_dump(psmouse);
+		return;
+	}
+
+	input_report_key(tp_dev, BTN_LEFT, packet[0] & 0x01);
+	input_report_key(tp_dev, BTN_RIGHT, packet[0] & 0x02);
+	input_report_key(tp_dev, BTN_MIDDLE, packet[0] & 0x04);
+	x = (s32) ((u32) ((packet[1] & 0x80) ? 0UL : 0xFFFFFF00UL) | (u32)
+		   packet[4]);
+	y = -(s32) ((u32) ((packet[2] & 0x80) ? 0UL : 0xFFFFFF00UL) | (u32)
+		    packet[5]);
+	input_report_rel(tp_dev, REL_X, x);
+	input_report_rel(tp_dev, REL_Y, y);
+
+	switch ((((u32) packet[0] & 0xF8) << 24) | ((u32) packet[1] << 16)
+		| (u32) packet[2] << 8 | (u32) packet[3]) {
+	case 0x00808036UL:
+	case 0x10008026UL:
+	case 0x20800016UL:
+	case 0x30000006UL:
+		break;
+	default:
+		/* Dump unexpected packet sequences if debug=1 (default) */
+		if (etd->debug == 1)
+			elantech_packet_dump(psmouse);
+		break;
+	}
+
+	input_sync(tp_dev);
+}
+
 /*
  * Interpret complete data packets and report absolute mode input events for
  * hardware version 3. (12 byte packets for two fingers)
@@ -715,6 +772,8 @@ static int elantech_packet_check_v3(struct psmouse *psmouse)
 
 		if ((packet[0] & 0x0c) == 0x0c && (packet[3] & 0xce) == 0x0c)
 			return PACKET_V3_TAIL;
+		if ((packet[3]&0x0f) == 0x06)
+			return PACKET_TRACKPOINT;
 	}
 
 	return PACKET_UNKNOWN;
@@ -798,7 +857,10 @@ static psmouse_ret_t elantech_process_byte(struct psmouse *psmouse)
 		if (packet_type == PACKET_UNKNOWN)
 			return PSMOUSE_BAD_DATA;
 
-		elantech_report_absolute_v3(psmouse, packet_type);
+		if (packet_type == PACKET_TRACKPOINT)
+			elantech_report_trackpoint(psmouse, packet_type);
+		else
+			elantech_report_absolute_v3(psmouse, packet_type);
 		break;
 
 	case 4:
@@ -1018,8 +1080,10 @@ static int elantech_get_resolution_v4(struct psmouse *psmouse,
  * Asus UX31               0x361f00        20, 15, 0e      clickpad
  * Asus UX32VD             0x361f02        00, 15, 0e      clickpad
  * Avatar AVIU-145A2       0x361f00        ?               clickpad
+ * Fujitsu H730            0x570f00        c0, 14, 0c      3 hw buttons (**)
  * Gigabyte U2442          0x450f01        58, 17, 0c      2 hw buttons
  * Lenovo L430             0x350f02        b9, 15, 0c      2 hw buttons (*)
+ * Lenovo L530             0x350f02        b9, 15, 0c      2 hw buttons (*) 
  * Samsung NF210           0x150b00        78, 14, 0a      2 hw buttons
  * Samsung NP770Z5E        0x575f01        10, 15, 0f      clickpad
  * Samsung NP700Z5B        0x361f06        21, 15, 0f      clickpad
@@ -1029,6 +1093,8 @@ static int elantech_get_resolution_v4(struct psmouse *psmouse,
  * Samsung RF710           0x450f00        ?               2 hw buttons
  * System76 Pangolin       0x250f01        ?               2 hw buttons
  * (*) + 3 trackpoint buttons
+ * (**) + 0 trackpoint buttons
+ * Note: Lenovo L430 and Lenovo L430 have the same fw_version/caps
  */
 static void elantech_set_buttonpad_prop(struct psmouse *psmouse)
 {
@@ -1324,6 +1390,11 @@ int elantech_detect(struct psmouse *psmouse, bool set_properties)
  */
 static void elantech_disconnect(struct psmouse *psmouse)
 {
+	struct elantech_data *etd = psmouse->private;
+	struct input_dev *tp_dev = etd->tp_dev;
+
+	if (etd->trackpoint_present && tp_dev) 
+		input_unregister_device(tp_dev);
 	sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj,
 			   &elantech_attr_group);
 	kfree(psmouse->private);
@@ -1440,11 +1511,11 @@ int elantech_init(struct psmouse *psmouse)
 	struct elantech_data *etd;
 	int i, error;
 	unsigned char param[3];
+	struct input_dev *tp_dev;
 
 	psmouse->private = etd = kzalloc(sizeof(struct elantech_data), GFP_KERNEL);
 	if (!etd)
 		return -ENOMEM;
-
 	psmouse_reset(psmouse);
 
 	etd->parity[0] = 1;
@@ -1497,6 +1568,28 @@ int elantech_init(struct psmouse *psmouse)
 			    error);
 		goto init_fail;
 	}
+	etd->trackpoint_present = ((etd->capabilities[0] & 0x80) == 0x80);
+	if (etd->trackpoint_present) {
+		tp_dev = input_allocate_device();
+		if (!tp_dev)
+			goto init_fail_tp_alloc;
+		etd->tp_dev = tp_dev;
+		snprintf(etd->tp_phys, sizeof(etd->tp_phys), "%s/input1",
+			psmouse->ps2dev.serio->phys);
+		tp_dev->phys = etd->tp_phys;
+		tp_dev->name = "Elantech PS/2 TrackPoint";
+		tp_dev->id.bustype = BUS_I8042;
+		tp_dev->id.vendor  = 0x0002;
+		tp_dev->id.product = PSMOUSE_ELANTECH;
+		tp_dev->id.version = 0x0000;
+		tp_dev->dev.parent = &psmouse->ps2dev.serio->dev;
+		tp_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
+		tp_dev->relbit[BIT_WORD(REL_X)] = BIT_MASK(REL_X) | BIT_MASK(REL_Y);
+		tp_dev->keybit[BIT_WORD(BTN_LEFT)] =
+			BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);
+		if (input_register_device(etd->tp_dev))
+			goto init_fail_tp_reg;
+	}
 
 	psmouse->protocol_handler = elantech_process_byte;
 	psmouse->disconnect = elantech_disconnect;
@@ -1504,8 +1597,13 @@ int elantech_init(struct psmouse *psmouse)
 	psmouse->pktsize = etd->hw_version > 1 ? 6 : 4;
 
 	return 0;
-
+ init_fail_tp_reg:
+	input_free_device(tp_dev);
+ init_fail_tp_alloc:
+	sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj,
+			   &elantech_attr_group);
  init_fail:
+        psmouse_reset(psmouse);
 	kfree(etd);
 	return -1;
 }
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index 9e0e2a1..25cbc8c 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -94,6 +94,7 @@
 #define PACKET_V4_HEAD			0x05
 #define PACKET_V4_MOTION		0x06
 #define PACKET_V4_STATUS		0x07
+#define PACKET_TRACKPOINT		0x08
 
 /*
  * track up to 5 fingers for v4 hardware
@@ -114,6 +115,8 @@ struct finger_pos {
 };
 
 struct elantech_data {
+	struct input_dev *tp_dev;		/* Relative device */
+	char	tp_phys[32];
 	unsigned char reg_07;
 	unsigned char reg_10;
 	unsigned char reg_11;
@@ -130,6 +133,7 @@ struct elantech_data {
 	bool jumpy_cursor;
 	bool reports_pressure;
 	bool crc_enabled;
+	bool trackpoint_present;
 	bool set_hw_resolution;
 	unsigned char hw_version;
 	unsigned int fw_version;
-- 
2.0.0.rc2

^ permalink raw reply related

* [PATCH v2 0/1] Input: Support in the elantech driver of the trackpoint present on for instance Lenovo L530
From: Ulrik De Bie @ 2014-06-12 18:15 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: stable, linux-input, Hans de Goede, David Herrmann,
	ulrik.debie-os
In-Reply-To: <20140214205126.GB15526@lantern>

This patch adds support for trackpoint on elantech driver for v3 models

Changes since v1:
* New patch now with reference to 3.14rc1
* Added etd->trackpoint_present to indicate presence of trackpoint (based
  on MSB of etd->capabilities[0])
* trackpoint will only be registered now when MSB of etd->capabilities[0] is
  set; got confirmation that this is the indicator of trackpoint
* Added input_unregister_device/input_free_device in elantech_disconnect()
* Fixed a bug in cleaning up when elantech_init fails
* Rename commit to be more specific (now also applicable to future elantech
  v3 models with trackpoint)
* input device name 'TPPS/2 IBM TrackPoint' changed to
  'Elantech PS/2 TrackPoint', this patch is not ibm/lenovo specific!
* dev2 renamed to tp_dev to indicate that this is the trackpoint device
* etd->phys renamed to etd->tp_phys
* Added Lenovo 530 and Fujitsu H730 to the laptop list because those are now
  also known.
* Added psmouse_reset at the end of elantech_init when it fails
* Added warning when trackpoint packets are received with no trackpoint detected

The patch is also available from:
https://github.com/ulrikdb/linux/commit/74f8d3a9307c109ae40c02072dc9c16d3557c3d4

Ulrik De Bie (1):
  elantech: Add support for trackpoint found on some v3 models

 drivers/input/mouse/elantech.c | 104 +++++++++++++++++++++++++++++++++++++++--
 drivers/input/mouse/elantech.h |   4 ++
 2 files changed, 105 insertions(+), 3 deletions(-)

-- 
2.0.0.rc2

^ permalink raw reply

* Re: [PATCH 0/4] Input: atmel_mxt_ts - make it work on Tegra
From: Stephen Warren @ 2014-06-12 17:37 UTC (permalink / raw)
  To: Nick Dyer
  Cc: Dmitry Torokhov, Benson Leung, Yufeng Shen, Daniel Kurtz,
	linux-input@vger.kernel.org, Stephen Warren, Bowens, Alan
In-Reply-To: <5399DF76.8020108@wwwdotorg.org>

On 06/12/2014 11:12 AM, Stephen Warren wrote:
...
> One bug I see is that the mouse doesn't seem to release when I drag it
> (either through double clicking and holding on the second click, or by a
> two-finger drag). Is that a known issue?

Let me refine the issue: It's nothing to do with drags, but rather any
time I push the touchpad hard to physically press the mouse button,
there's no release event.

"Soft" taps of the touchpad work fine, for single, double, or even
triple clicks:

(single soft click)
> Event: time 1402594144.964326, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
> Event: time 1402594144.964326, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
...
...
> Event: time 1402594145.026351, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
> Event: time 1402594145.026351, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0

However, "hard" pushes that physically depress the touchpad and activate
the physical button don't generate release events, so clicks and drags
never end.

Single hard click:

> Event: time 1402594196.593745, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
> Event: time 1402594196.593745, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
...
> Event: time 1402594196.614456, type 1 (EV_KEY), code 272 (BTN_LEFT), value 1
...
> Event: time 1402594196.946386, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
> Event: time 1402594196.946386, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0

(BTN_LEFT value 0 not sent)

Perhaps commit "Input: atmel_mxt_ts - Set pointer emulation on
touchpads" is incomplete? However, without that applied, neither soft
nor hard click generate any kind of mouse button events, just BTN_TOUCH
which X doesn't seem to interpret as a mouse click.

^ permalink raw reply

* Re: [PATCH 0/4] Input: atmel_mxt_ts - make it work on Tegra
From: Stephen Warren @ 2014-06-12 17:12 UTC (permalink / raw)
  To: Nick Dyer
  Cc: Dmitry Torokhov, Benson Leung, Yufeng Shen, Daniel Kurtz,
	linux-input@vger.kernel.org, Stephen Warren, Bowens, Alan
In-Reply-To: <53998E12.8050408@itdev.co.uk>

On 06/12/2014 05:25 AM, Nick Dyer wrote:
> Stephen Warren wrote:
>>> I will test this on my setup and see if I can figure out what is causing
>>> the problem.
>>
>> Nick, did you get anywhere reproducing my problem? Unfortunately with
>> your recent patches merged, I can't get the touchpad working at all,
>> whereas before I had a few simple patches that made it work:-(
> 
> I haven't been able to reproduce your issue, I have a devicetree based
> system that will successfully probe without a config file.
> 
> Could you send me the dmesg output with all of the driver debug enabled? It
> would be good to see how far it is getting.

My apologies; I should have retested before asking about this again. I
guess something must have changed in linux-next since the last time I
tested this.

With next-20140611 plus the following patches on top:

d3168ccd7d57 ARM: tegra: enable Atmel touchpad in defconfig
fc1d7c4aefe0 ARM: tegra: add touchpad to Venice2 DT
da5f283a52ea Input: atmel_mxt_ts: implement device tree parsing
22911ed92e76 Input: atmel_mxt_ts: define a device tree binding
57f79f65a8f9 Input: atmel_mxt_ts - Set pointer emulation on touchpads

... the driver built into the kernel, and no firmware available in
/lib/firmware, the touchpad now works.

Are you still expecting to take my DT binding patches above and resend them?

One bug I see is that the mouse doesn't seem to release when I drag it
(either through double clicking and holding on the second click, or by a
two-finger drag). Is that a known issue?

^ permalink raw reply

* re: Input: psmouse - switch to using dev_*() for messages
From: Dan Carpenter @ 2014-06-12 14:29 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input

Hello Dmitry Torokhov,

This is a semi-automatic email about new static checker warnings.

The patch b5d21704361e: "Input: psmouse - switch to using dev_*() for
messages" from Oct 10, 2011, leads to the following Smatch complaint:

drivers/input/mouse/psmouse-base.c:1512 psmouse_reconnect()
	 error: we previously assumed 'psmouse' could be null (see line 1511)

drivers/input/mouse/psmouse-base.c
  1510	
  1511		if (!drv || !psmouse) {
                             ^^^^^^^
Old check.

  1512			psmouse_dbg(psmouse,
                                    ^^^^^^^
New dereference.

  1513				    "reconnect request, but serio is disconnected, ignoring...\n");
  1514			return -1;

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH 0/4] Input: atmel_mxt_ts - make it work on Tegra
From: Nick Dyer @ 2014-06-12 11:25 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Dmitry Torokhov, Benson Leung, Yufeng Shen, Daniel Kurtz,
	linux-input@vger.kernel.org, Stephen Warren, Bowens, Alan
In-Reply-To: <53989D30.1000605@wwwdotorg.org>

Stephen Warren wrote:
>> I will test this on my setup and see if I can figure out what is causing
>> the problem.
> 
> Nick, did you get anywhere reproducing my problem? Unfortunately with
> your recent patches merged, I can't get the touchpad working at all,
> whereas before I had a few simple patches that made it work:-(

I haven't been able to reproduce your issue, I have a devicetree based
system that will successfully probe without a config file.

Could you send me the dmesg output with all of the driver debug enabled? It
would be good to see how far it is getting.

^ permalink raw reply

* Re: [PATCH v2 2/2] Add support for Compact (Bluetooth|USB) keyboard with Trackpoint
From: Jamie Lentin @ 2014-06-12  8:56 UTC (permalink / raw)
  To: Antonio Ospite; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20140611103616.79b40e7b0bbf3447837921fc@ao2.it>

On Wed, 11 Jun 2014, Antonio Ospite wrote:

> On Tue, 10 Jun 2014 23:24:54 +0100
> Jamie Lentin <jm@lentin.co.uk> wrote:
>
>> Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
>
> Some minor comments here too.

Thankyou for taking the time over both sets! All comments make sense.

>> ---
>>  drivers/hid/hid-core.c         |   2 +
>>  drivers/hid/hid-ids.h          |   2 +
>>  drivers/hid/hid-lenovo-tpkbd.c | 203 +++++++++++++++++++++++++++++++++++++++++
>>  include/linux/hid.h            |   1 +
>>  4 files changed, 208 insertions(+)
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 8a5384c..6591f4e 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -1738,6 +1738,8 @@ static const struct hid_device_id hid_have_special_driver[] = {
>>  	{ HID_USB_DEVICE(USB_VENDOR_ID_LCPOWER, USB_DEVICE_ID_LCPOWER_LC1000 ) },
>>  #if IS_ENABLED(CONFIG_HID_LENOVO_TPKBD)
>>  	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
>> +	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CUSBKBD) },
>> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CBTKBD) },
>>  #endif
>>  	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) },
>>  	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) },
>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>> index 6e12cd0..1763a07 100644
>> --- a/drivers/hid/hid-ids.h
>> +++ b/drivers/hid/hid-ids.h
>> @@ -551,6 +551,8 @@
>>
>>  #define USB_VENDOR_ID_LENOVO		0x17ef
>>  #define USB_DEVICE_ID_LENOVO_TPKBD	0x6009
>> +#define USB_DEVICE_ID_LENOVO_CUSBKBD	0x6047
>> +#define USB_DEVICE_ID_LENOVO_CBTKBD		0x6048
>>
>>  #define USB_VENDOR_ID_LG		0x1fd2
>>  #define USB_DEVICE_ID_LG_MULTITOUCH	0x0064
>> diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo-tpkbd.c
>> index 3bec9f5..1671e7a 100644
>> --- a/drivers/hid/hid-lenovo-tpkbd.c
>> +++ b/drivers/hid/hid-lenovo-tpkbd.c
>> @@ -1,8 +1,11 @@
>>  /*
>>   *  HID driver for Lenovo:-
>>   *  * ThinkPad USB Keyboard with TrackPoint
>> + *  * ThinkPad Compact Bluetooth Keyboard with TrackPoint
>> + *  * ThinkPad Compact USB Keyboard with TrackPoint
>
> Use - as the bullet.
>
>>   *
>>   *  Copyright (c) 2012 Bernhard Seibold
>> + *  Copyright (c) 2014 Jamie Lentin <jm@lentin.co.uk>
>>   */
>>
>>  /*
>> @@ -34,6 +37,10 @@ struct tpkbd_data_pointer {
>>  	int press_speed;
>>  };
>>
>> +struct tpcompactkbd_sc {
>> +	unsigned int fn_lock;
>> +};
>> +
>>  #define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
>>
>>  static int tpkbd_input_mapping_tp(struct hid_device *hdev,
>> @@ -49,17 +56,154 @@ static int tpkbd_input_mapping_tp(struct hid_device *hdev,
>>  	return 0;
>>  }
>>
>> +static int tpcompactkbd_input_mapping(struct hid_device *hdev,
>
> Maybe name these functions like tpkbd_input_mapping_compact()?
>
> This way the namespace is more consistent, and follows the logic of
> patch 1/2 more closely.
>
> Use this scheme at least for functions which have a _tp() counterpart.

Previously the tpkbd driver had various functions marked "_tp" to indicate 
that it's for the "mouse" half of the keyboard as the kernel sees it, 
however it does nothing special with the keyboard half. I was intending 
(somewhat sloppily) to repurpose this into having versions of each 
function for each keyboard, and a common function to switch between them. 
Should make it fairly easy to add extra keyboards in the future.

The problem, as ever, is choosing decent names for them. It should 
probably be either:-

* tpkbd_input_mapping_usbkbd
* tpkbd_input_mapping_compactkbd
...and tpkbd_input_mapping switches between them

or rename the driver to hid-lenovo and do:-

* lenovo_input_mapping_tpkbd
* lenovo_input_mapping_compacttp
...and lenovo_input_mapping switches between them

The latter seems a bit too invasive, but I'm not sure how obvious with the 
former that it'd be that the "Compact USB keyboard" is in-fact a 
compactkbd not a usbkbd. The former is probably what I'll go for unless 
you have any thoughts.

>> +		struct hid_input *hi, struct hid_field *field,
>> +		struct hid_usage *usage, unsigned long **bit, int *max)
>> +{
>> +	/* HID_UP_LNVENDOR = USB, HID_UP_MSVENDOR = BT */
>> +	if ((usage->hid & HID_USAGE_PAGE) == HID_UP_MSVENDOR ||
>> +	    (usage->hid & HID_USAGE_PAGE) == HID_UP_LNVENDOR) {
>> +		set_bit(EV_REP, hi->input->evbit);
>> +		switch (usage->hid & HID_USAGE) {
>> +		case 0x00f1: /* Fn-F4: Mic mute */
>> +			map_key_clear(KEY_MICMUTE);
>> +			return 1;
>> +		case 0x00f2: /* Fn-F5: Brightness down */
>> +			map_key_clear(KEY_BRIGHTNESSDOWN);
>> +			return 1;
>> +		case 0x00f3: /* Fn-F6: Brightness up */
>> +			map_key_clear(KEY_BRIGHTNESSUP);
>> +			return 1;
>> +		case 0x00f4: /* Fn-F7: External display (projector) */
>> +			map_key_clear(KEY_SWITCHVIDEOMODE);
>> +			return 1;
>> +		case 0x00f5: /* Fn-F8: Wireless */
>> +			map_key_clear(KEY_WLAN);
>> +			return 1;
>> +		case 0x00f6: /* Fn-F9: Control panel */
>> +			map_key_clear(KEY_CONFIG);
>> +			return 1;
>> +		case 0x00f8: /* Fn-F11: View open applications (3 boxes) */
>> +			map_key_clear(KEY_FN_F11);
>> +			return 1;
>> +		case 0x00fa: /* Fn-Esc: Fn-lock toggle */
>> +			map_key_clear(KEY_FN_ESC);
>> +			return 1;
>> +		case 0x00fb: /* Fn-F12: Open My computer (6 boxes) USB-only */
>> +			/* NB: This mapping is invented in raw_event below */
>> +			map_key_clear(KEY_FILE);
>> +			return 1;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int tpkbd_input_mapping(struct hid_device *hdev,
>>  		struct hid_input *hi, struct hid_field *field,
>>  		struct hid_usage *usage, unsigned long **bit, int *max)
>>  {
>>  	if (hdev->product == USB_DEVICE_ID_LENOVO_TPKBD)
>>  		return tpkbd_input_mapping_tp(hdev, hi, field, usage, bit, max);
>> +	if (hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD)
>> +		return tpcompactkbd_input_mapping(hdev, hi, field, usage, bit, max);
>> +	if (hdev->product == USB_DEVICE_ID_LENOVO_CBTKBD)
>
> What about combining the checks?
>
> 	if (hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD ||
> 	    hdev->product == USB_DEVICE_ID_LENOVO_CBTKBD)

A switch statement might be neater, but either way yes could combine them.

>> +		return tpcompactkbd_input_mapping(hdev, hi, field, usage, bit, max);
>>  	return 0;
>>  }
>>
>>  #undef map_key_clear
>>
>> +/* Send a config command to the keyboard */
>> +static int tpcompactkbd_send_cmd(struct hid_device *hdev,
>> +			unsigned char byte2, unsigned char byte3)
>> +{
>> +	int ret;
>> +	unsigned char buf[] = {0x18, byte2, byte3};
>> +	unsigned char report_type = HID_OUTPUT_REPORT;
>> +
>> +	/* The USB keyboard accepts commands via SET_FEATURE */
>> +	if (hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD) {
>> +		buf[0] = 0x13;
>> +		report_type = HID_FEATURE_REPORT;
>> +	}
>> +
>> +	ret = hdev->hid_output_raw_report(hdev, buf, sizeof(buf), report_type);
>> +	return ret < 0 ? ret : 0; /* BT returns 0, USB returns sizeof(buf) */
>> +}
>> +
>> +static void tpcompactkbd_features_set(struct hid_device *hdev)
>> +{
>> +	struct tpcompactkbd_sc *tpcsc = hid_get_drvdata(hdev);
>> +
>> +	if (tpcompactkbd_send_cmd(hdev, 0x05, tpcsc->fn_lock ? 0x01 : 0x00))
>> +		hid_err(hdev, "Fn-lock setting failed\n");
>> +}
>> +
>> +static ssize_t tpcompactkbd_fn_lock_show(struct device *dev,
>> +		struct device_attribute *attr,
>> +		char *buf)
>> +{
>> +	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
>> +	struct tpcompactkbd_sc *tpcsc = hid_get_drvdata(hdev);
>> +
>> +	return snprintf(buf, PAGE_SIZE, "%u\n", tpcsc->fn_lock);
>> +}
>> +
>> +static ssize_t tpcompactkbd_fn_lock_store(struct device *dev,
>> +		struct device_attribute *attr,
>> +		const char *buf,
>> +		size_t count)
>> +{
>> +	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
>> +	struct tpcompactkbd_sc *tpcsc = hid_get_drvdata(hdev);
>> +	int value;
>> +
>> +	if (kstrtoint(buf, 10, &value))
>> +		return -EINVAL;
>> +	if (value < 0 || value > 1)
>> +		return -EINVAL;
>> +
>> +	tpcsc->fn_lock = value;
>> +	tpcompactkbd_features_set(hdev);
>> +
>> +	return count;
>> +}
>> +
>> +static struct device_attribute dev_attr_pointer_fn_lock =
>> +	__ATTR(fn_lock, S_IWUSR | S_IRUGO,
>> +			tpcompactkbd_fn_lock_show,
>> +			tpcompactkbd_fn_lock_store);
>> +
>> +static struct attribute *tpcompactkbd_attributes[] = {
>> +	&dev_attr_pointer_fn_lock.attr,
>> +	NULL
>> +};
>> +
>> +static const struct attribute_group tpcompactkbd_attr_group = {
>> +	.attrs = tpcompactkbd_attributes,
>> +};
>> +
>> +static int tpkbd_raw_event(struct hid_device *hdev,
>> +			struct hid_report *report, u8 *data, int size)
>> +{
>> +	/*
>> +	 * USB keyboard's Fn-F12 report holds down many other keys, and it's
>> +	 * own key is outside the usage page range. Remove extra keypresses and
>> +	 * remap to inside usage page.
>> +	 */
>> +	if (unlikely(hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD
>> +			&& size == 3
>> +			&& data[0] == 0x15
>> +			&& data[1] == 0x94
>> +			&& data[2] == 0x01)) {
>> +		data[1] = 0x0;
>> +		data[2] = 0x4;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int tpkbd_features_set(struct hid_device *hdev)
>>  {
>>  	struct hid_report *report;
>> @@ -406,6 +550,46 @@ static int tpkbd_probe_tp(struct hid_device *hdev)
>>  	return 0;
>>  }
>>
>> +static int tpcompactkbd_probe(struct hid_device *hdev,
>> +			const struct hid_device_id *id)
>> +{
>> +	int ret;
>> +	struct tpcompactkbd_sc *tpcsc;
>> +
>> +	tpcsc = devm_kzalloc(&hdev->dev, sizeof(*tpcsc), GFP_KERNEL);
>> +	if (tpcsc == NULL) {
>> +		hid_err(hdev, "can't alloc keyboard descriptor\n");
>> +		return -ENOMEM;
>> +	}
>> +	hid_set_drvdata(hdev, tpcsc);
>> +
>> +	/* All the custom action happens on the mouse device for USB */
>> +	if (hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD
>> +			&& hdev->type != HID_TYPE_USBMOUSE) {
>> +		pr_debug("Ignoring keyboard half of device\n");
>> +		return 0;
>> +	}
>> +
>> +	/*
>> +	 * Tell the keyboard a driver understands it, and turn F7, F9, F11 into
>> +	 * regular keys
>> +	 */
>> +	ret = tpcompactkbd_send_cmd(hdev, 0x01, 0x03);
>> +	if (ret)
>> +		hid_warn(hdev, "Failed to switch F7/9/11 into regular keys\n");
>> +
>> +	/* Turn Fn-Lock on by default */
>> +	tpcsc->fn_lock = 1;
>> +	tpcompactkbd_features_set(hdev);
>> +
>> +	if (sysfs_create_group(&hdev->dev.kobj,
>> +				&tpcompactkbd_attr_group)) {
>
> Use:
> 	ret = sysfs_create_group()
> 	if (ret) {
>
>> +		hid_warn(hdev, "Could not create sysfs group\n");
>> +	}
>
> with only one statement in the if body curly braces are not needed.
>
>> +
>> +	return 0;
>> +}
>> +
>>  static int tpkbd_probe(struct hid_device *hdev,
>>  		const struct hid_device_id *id)
>>  {
>> @@ -426,6 +610,12 @@ static int tpkbd_probe(struct hid_device *hdev,
>>  	if (hdev->product == USB_DEVICE_ID_LENOVO_TPKBD
>>  			&& tpkbd_probe_tp(hdev))
>>  		goto err_hid;
>> +	if (hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD
>> +			&& tpcompactkbd_probe(hdev, id))
>> +		goto err_hid;
>> +	if (hdev->product == USB_DEVICE_ID_LENOVO_CBTKBD
>> +			&& tpcompactkbd_probe(hdev, id))
>
> Again, what about combining the checks?
> Calling the function in the if body (and checking the return value of
> course :P).
>
>> +		goto err_hid;
>>
>>  	return 0;
>>  err_hid:
>> @@ -447,6 +637,12 @@ static void tpkbd_remove_tp(struct hid_device *hdev)
>>  	hid_set_drvdata(hdev, NULL);
>>  }
>>
>> +static void tpcompactkbd_remove(struct hid_device *hdev)
>> +{
>> +	sysfs_remove_group(&hdev->dev.kobj,
>> +			&tpcompactkbd_attr_group);
>> +}
>> +
>>  static void tpkbd_remove(struct hid_device *hdev)
>>  {
>>  	if (!hid_get_drvdata(hdev))
>> @@ -454,12 +650,18 @@ static void tpkbd_remove(struct hid_device *hdev)
>>
>>  	if (hdev->product == USB_DEVICE_ID_LENOVO_TPKBD)
>>  		tpkbd_remove_tp(hdev);
>> +	if (hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD)
>> +		tpcompactkbd_remove(hdev);
>> +	if (hdev->product == USB_DEVICE_ID_LENOVO_CBTKBD)
>
> Here too.
>
>> +		tpcompactkbd_remove(hdev);
>>
>>  	hid_hw_stop(hdev);
>>  }
>>
>>  static const struct hid_device_id tpkbd_devices[] = {
>>  	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
>> +	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CUSBKBD) },
>> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CBTKBD) },
>>  	{ }
>>  };
>>
>> @@ -471,6 +673,7 @@ static struct hid_driver tpkbd_driver = {
>>  	.input_mapping = tpkbd_input_mapping,
>>  	.probe = tpkbd_probe,
>>  	.remove = tpkbd_remove,
>> +	.raw_event = tpkbd_raw_event,
>>  };
>>  module_hid_driver(tpkbd_driver);
>>
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index 31b9d29..ed23d6a 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -167,6 +167,7 @@ struct hid_item {
>>  #define HID_UP_MSVENDOR		0xff000000
>>  #define HID_UP_CUSTOM		0x00ff0000
>>  #define HID_UP_LOGIVENDOR	0xffbc0000
>> +#define HID_UP_LNVENDOR		0xffa00000
>>  #define HID_UP_SENSOR		0x00200000
>>
>>  #define HID_USAGE		0x0000ffff
>> --
>> 2.0.0.rc2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> Ciao ciao,
>   Antonio
>
>

-- 
Jamie Lentin

^ permalink raw reply

* Re: [PATCH v4] leds: USB: HID: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-12  9:06 UTC (permalink / raw)
  To: Janne Kanniainen
  Cc: jkosina, cooloney, johan, linux-kernel, linux-leds, linux-usb,
	linux-input
In-Reply-To: <1402526921-1903-1-git-send-email-janne.kanniainen@gmail.com>

On Thu, Jun 12, 2014 at 01:48:41AM +0300, Janne Kanniainen wrote:
> This driver adds support for USB controlled led panels that exists in MSI GT683R laptop

You forgot to break this line.

> 
> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
> ---
> Changes in v2:
>         - sorted headers to alphabetic order
>         - using devm_kzalloc
>         - using BIT(n)
>         - using usb_control_msg instead of usb_submit_urb
>         - removing unneeded code
> 
> Changes in v3:
>         - implemented as HID device
>         - some cleanups and bug fixes
> 
> Changes in v4:
> 	- more cleanups
> 	- support for selecting leds
> 	- support for selecting status

That was fast. :)

> 
>  drivers/hid/Kconfig             |  11 ++
>  drivers/hid/Makefile            |   1 +
>  drivers/hid/hid-core.c          |   1 +
>  drivers/hid/hid-gt683r.c        | 320 ++++++++++++++++++++++++++++++++++++++++
>  drivers/hid/hid-ids.h           |   2 +-
>  drivers/hid/usbhid/hid-quirks.c |   2 +-
>  6 files changed, 335 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/hid/hid-gt683r.c
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 7af9d0b..d93e0ae 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -261,6 +261,17 @@ config HOLTEK_FF
>  	  Say Y here if you have a Holtek On Line Grip based game controller
>  	  and want to have force feedback support for it.
>  
> +config HID_GT683R
> +       tristate "MSI GT68xR LED support"
> +       depends on LEDS_CLASS && USB_HID
> +       ---help---
> +       Say Y here if you want to enable support for the MSI GT68xR LEDS
> +
> +       This driver support following states normal, breathing and audio.
> +       You can also select which leds you want to enable.
> +       Currently the following devices are know to be supported:
> +       - MSI GT683R
> +
>  config HID_HUION
>  	tristate "Huion tablets"
>  	depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index fc712dd..7129311 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_HID_EMS_FF)	+= hid-emsff.o
>  obj-$(CONFIG_HID_ELECOM)	+= hid-elecom.o
>  obj-$(CONFIG_HID_ELO)		+= hid-elo.o
>  obj-$(CONFIG_HID_EZKEY)		+= hid-ezkey.o
> +obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
>  obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
>  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-kbd.o
>  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-mouse.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index da52279..ec88fdb 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1827,6 +1827,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2) },
> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
> new file mode 100644
> index 0000000..04e4cc2
> --- /dev/null
> +++ b/drivers/hid/hid-gt683r.c
> @@ -0,0 +1,320 @@
> +/*
> + * MSI GT683R led driver
> + *
> + * Copyright (c) 2014 Janne Kanniainen <janne.kanniainen@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +#define GT683R_LED_BACK				BIT(0)
> +#define GT683R_LED_SIDE				BIT(1)
> +#define GT683R_LED_FRONT			BIT(2)
> +
> +#define GT683R_BUFFER_SIZE			8
> +
> +/*
> + * GT683R_LED_OFF: all LEDs are off
> + * GT683R_LED_AUDIO: the status of LEDs depends
> + * on sound level
> + * GT683R_LED_BREATHING: LEDs brightness varies
> + * at human breathing rate
> + * GT683R_LED_NORMAL: LEDs are on
> + */
> +enum gt683r_led_state {
> +	GT683R_LED_OFF = 0,
> +	GT683R_LED_AUDIO = 2,
> +	GT683R_LED_BREATHING = 3,
> +	GT683R_LED_NORMAL = 5
> +};
> +
> +struct gt683r_led {
> +	struct hid_device *hdev;
> +	struct led_classdev led_dev_back;
> +	struct led_classdev led_dev_side;
> +	struct led_classdev led_dev_front;

You could store these as an array, and add an enum for back, side, and
front (more below).

> +	struct mutex lock;
> +	struct mutex state_lock;

You should be able to use only one lock.

> +	struct work_struct work;
> +	enum led_brightness brightness_back;
> +	enum led_brightness brightness_side;
> +	enum led_brightness brightness_front;

These could then also be in an array.

> +	enum gt683r_led_state state;
> +};
> +
> +static const struct hid_device_id gt683r_led_id[] = {
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
> +	{ }
> +};
> +
> +#define GT683R_BRIGHTNESS_SET(name)					\
> +static void gt683r_brightness_set_##name(struct led_classdev *led_cdev, \
> +					enum led_brightness brightness)	\
> +{									\
> +	struct gt683r_led *led =					\
> +			container_of(led_cdev, struct gt683r_led,	\
> +				    led_dev_##name);			\
> +									\
> +	led->brightness_##name = brightness;				\
> +									\
> +	schedule_work(&led->work);					\
> +}
> +
> +GT683R_BRIGHTNESS_SET(back);
> +GT683R_BRIGHTNESS_SET(side);
> +GT683R_BRIGHTNESS_SET(front);

This is one way of solving this, but it's better to have one
set_brightness function and determine which led it is called for by
accessing the driver data of the hid device (via the parent device) and
iterate over the available leds. See drivers/hid/hid-lg4ff.c for an
example of how this can be implemented.

> +
> +static ssize_t gt683r_show_state(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct hid_device *hdev =
> +		container_of(dev, struct hid_device, dev);
> +	struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> +	if (led->state == GT683R_LED_OFF)
> +		return sprintf(buf, "off\n");

You should always use scnprintf (with a size of PAGE_SIZE) in show
callbacks.

> +	else if (led->state == GT683R_LED_AUDIO)
> +		return sprintf(buf, "audio\n");
> +	else if (led->state == GT683R_LED_BREATHING)
> +		return sprintf(buf, "breathing\n");
> +	else
> +		return sprintf(buf, "normal\n");
> +}

Ok, so the blink mode (or type rather than state, I understand now --
sorry for the confusion) is common for all three LEDs. 

You shouldn't allow the LEDs to be disabled through this attribute, but
rather set the mode that will be used for any enabled LEDs (e.g. the
attribute should only have three possible values).

Would it even be sufficient to only set the blink mode (type, state) to
NORMAL at probe and then only update it if it changes (in store_state
below)? In particular, will all three LEDs stay off if they are masked
out in gt683r_let_set? (Otherwise, you could only set OFF mode when all
three LEDs are disabled.)

Note that any new attribute has to be documented under
Documentation/ABI.

> +
> +static ssize_t gt683r_store_state(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	struct hid_device *hdev =
> +		container_of(dev, struct hid_device, dev);
> +	struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> +	mutex_lock(&led->state_lock);
> +	if (!strncmp("off", buf, strlen("off"))) {
> +		led->state = GT683R_LED_OFF;
> +	} else if (!strncmp("audio", buf, strlen("audio"))) {
> +		led->state = GT683R_LED_AUDIO;
> +	} else if (!strncmp("breathing", buf, strlen("breathing"))) {
> +		led->state = GT683R_LED_BREATHING;
> +	} else if (!strncmp("normal", buf, strlen("normal"))) {
> +		led->state = GT683R_LED_NORMAL;
> +	} else {
> +		count = -EINVAL;
> +		goto fail;
> +	}
> +
> +	schedule_work(&led->work);
> +
> +fail:
> +	mutex_unlock(&led->state_lock);
> +
> +	return count;
> +}

I think you should use an integer value (and snprintf) for the three
modes (e.g.  0: normal, 1: audio, 2: breathing) and just document that
in the ABI files mentioned above.

> +
> +static int gt683r_led_snd_msg(struct gt683r_led *led, char *msg)

I didn't notice before, but you should be using the unsigned u8 type for
msg (and leds and state below).

> +{
> +	int ret;
> +
> +	ret = hid_hw_raw_request(led->hdev, msg[0], msg, GT683R_BUFFER_SIZE,
> +				HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> +	if (ret != GT683R_BUFFER_SIZE) {
> +		hid_err(led->hdev,
> +			"failed to send set report request: %i\n", ret);
> +		return ret < 0 ? ret : -EIO;

I try to avoid ?: constructs.

> +	}
> +
> +	return 0;
> +}
> +
> +static void gt683r_led_set(struct gt683r_led *led, char leds, char state)
> +{
> +	char *buffer;
> +
> +	buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
> +	if (!buffer)
> +		return;
> +
> +	buffer[0] = 0x01;
> +	buffer[1] = 0x02;
> +	buffer[2] = 0x30;
> +	buffer[3] = leds;
> +	if (gt683r_led_snd_msg(led, buffer))
> +		goto fail;
> +
> +	buffer[2] = 0x20;
> +	buffer[3] = state;
> +	buffer[4] = 0x01;
> +	gt683r_led_snd_msg(led, buffer);
> +
> +fail:
> +	kfree(buffer);
> +}
> +
> +static void gt683r_led_work(struct work_struct *work)
> +{
> +	struct gt683r_led *led =
> +			container_of(work, struct gt683r_led, work);
> +	char leds = 0;
> +
> +	mutex_lock(&led->lock);
> +
> +	if (led->brightness_back)
> +		leds |= GT683R_LED_BACK;
> +
> +	if (led->brightness_side)
> +		leds |= GT683R_LED_SIDE;
> +
> +	if (led->brightness_front)
> +		leds |= GT683R_LED_FRONT;
> +
> +	if (leds)
> +		gt683r_led_set(led, leds, led->state);
> +	else
> +		gt683r_led_set(led, leds, GT683R_LED_OFF);
> +
> +	mutex_unlock(&led->lock);
> +}
> +
> +static struct led_classdev gt683r_led_dev_back = {
> +	.name = "gt683r-led:back",
> +	.brightness_set = gt683r_brightness_set_back,
> +	.max_brightness = 1,
> +	.flags = LED_CORE_SUSPENDRESUME,
> +};
> +
> +static struct led_classdev gt683r_led_dev_side = {
> +	.name = "gt683r-led:side",
> +	.brightness_set = gt683r_brightness_set_side,
> +	.max_brightness = 1,
> +	.flags = LED_CORE_SUSPENDRESUME,
> +};
> +
> +static struct led_classdev gt683r_led_dev_front = {
> +	.name = "gt683r-led:front",
> +	.brightness_set = gt683r_brightness_set_front,
> +	.max_brightness = 1,
> +	.flags = LED_CORE_SUSPENDRESUME,
> +};

You should remove these three as you only use them for initialising the
dynamically allocated instances, which could be initialised directly in
probe instead.

You can find examples of this under drivers/hid (e.g.
drivers/hid/hid-lg4ff.c) including how to initialise the name field.
Note that the recommended naming scheme is "devicename:colour:function",
but you can leave the colour out (but still keep the two ':').

> +
> +static struct device_attribute gt683r_led_state_attribute = {
> +	.attr = {
> +		.name = "state",
> +		.mode = 0644,
> +	},
> +	.show = gt683r_show_state,
> +	.store = gt683r_store_state,
> +};

You should use the DEVICE_ATTR macro instead.

> +
> +static int gt683r_led_probe(struct hid_device *hdev,
> +			const struct hid_device_id *id)
> +{
> +	int ret;
> +	struct gt683r_led *led;
> +
> +	led = devm_kzalloc(&hdev->dev, sizeof(struct gt683r_led), GFP_KERNEL);
> +	if (!led)
> +		return -ENOMEM;
> +
> +	led->hdev = hdev;
> +	hid_set_drvdata(hdev, led);
> +
> +	ret = hid_parse(hdev);
> +	if (ret) {
> +		hid_err(hdev, "hid parsing failed\n");
> +		goto fail;
> +	}
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> +	if (ret) {
> +		hid_err(hdev, "hw start failed\n");
> +		goto fail;
> +	}
> +
> +	led->led_dev_back = gt683r_led_dev_back;
> +	ret = led_classdev_register(&hdev->dev, &led->led_dev_back);
> +	if (ret) {
> +		hid_err(hdev, "could not register led device\n");
> +		goto fail_back;
> +	}
> +
> +	led->led_dev_side = gt683r_led_dev_side;
> +	ret = led_classdev_register(&hdev->dev, &led->led_dev_side);
> +	if (ret) {
> +		hid_err(hdev, "could not register led device\n");
> +		goto fail_side;
> +	}
> +
> +	led->led_dev_front = gt683r_led_dev_front;
> +	ret = led_classdev_register(&hdev->dev, &led->led_dev_front);
> +	if (ret) {
> +		hid_err(hdev, "could not register led device\n");
> +		goto fail_front;
> +	}

With the enum an arrays mentioned above this could be implemented as a
loop.

> +
> +	ret = device_create_file(&led->hdev->dev,
> +				&gt683r_led_state_attribute);
> +	if (ret) {
> +		hid_err(hdev, "could not make state attribute file\n");
> +		goto fail_create_file;
> +	}
> +
> +	mutex_init(&led->lock);
> +	mutex_init(&led->state_lock);
> +	INIT_WORK(&led->work, gt683r_led_work);
> +
> +	return 0;
> +
> +fail_create_file:
> +	led_classdev_unregister(&led->led_dev_front);
> +fail_front:
> +	led_classdev_unregister(&led->led_dev_side);
> +fail_side:
> +	led_classdev_unregister(&led->led_dev_back);

...with a roll-back loop here.

> +fail_back:
> +	hid_hw_stop(hdev);
> +fail:
> +	return ret;
> +}
> +
> +static void gt683r_led_remove(struct hid_device *hdev)
> +{
> +	struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> +	led_classdev_unregister(&led->led_dev_side);
> +	led_classdev_unregister(&led->led_dev_back);
> +	led_classdev_unregister(&led->led_dev_front);
> +	cancel_work_sync(&led->work);
> +	device_remove_file(&hdev->dev,
> +			&gt683r_led_state_attribute);
> +	hid_hw_stop(hdev);
> +}
> +
> +static struct hid_driver gt683r_led_driver = {
> +	.probe = gt683r_led_probe,
> +	.remove = gt683r_led_remove,
> +	.name = "gt683r_led",
> +	.id_table = gt683r_led_id,
> +};
> +
> +module_hid_driver(gt683r_led_driver);
> +
> +MODULE_AUTHOR("Janne Kanniainen");
> +MODULE_DESCRIPTION("MSI GT683R led driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 34bb220..3692d37 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -641,7 +641,7 @@
>  #define USB_DEVICE_ID_GENIUS_KB29E	0x3004
>  
>  #define USB_VENDOR_ID_MSI		0x1770
> -#define USB_DEVICE_ID_MSI_GX680R_LED_PANEL	0xff00
> +#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
>  
>  #define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
>  #define USB_DEVICE_ID_N_S_HARMONY	0xc359
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 8e4ddb3..c640e1d 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -73,7 +73,7 @@ static const struct hid_blacklist {
>  	{ USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, HID_QUIRK_NO_INIT_REPORTS },
>  	{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
>  	{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
> -	{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
> +	{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
>  	{ USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },
>  	{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
>  	{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, HID_QUIRK_NO_INIT_REPORTS },

Thanks,
Johan

^ permalink raw reply

* Re: [PATCH] Input: st-keyscan - Protect PM-only functions by #ifdef CONFIG_PM_SLEEP
From: Gabriel Fernandez @ 2014-06-12  8:36 UTC (permalink / raw)
  To: Geert Uytterhoeven, Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <1402513493-17613-1-git-send-email-geert@linux-m68k.org>

Thanks Geert

Acked-by : Gabriel Fernandez <gabriel.fernandez@linaro.org>

Best Regards


On 06/11/2014 09:04 PM, Geert Uytterhoeven wrote:
> If CONFIG_PM_SLEEP=n:
>
> drivers/input/keyboard/st-keyscan.c:219: warning: ‘keyscan_suspend’ defined but not used
> drivers/input/keyboard/st-keyscan.c:236: warning: ‘keyscan_resume’ defined but not used
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>   drivers/input/keyboard/st-keyscan.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
> index 758b48731415..668e3cf4b491 100644
> --- a/drivers/input/keyboard/st-keyscan.c
> +++ b/drivers/input/keyboard/st-keyscan.c
> @@ -215,6 +215,7 @@ static int keyscan_probe(struct platform_device *pdev)
>   	return 0;
>   }
>   
> +#ifdef CONFIG_PM_SLEEP
>   static int keyscan_suspend(struct device *dev)
>   {
>   	struct platform_device *pdev = to_platform_device(dev);
> @@ -249,6 +250,7 @@ static int keyscan_resume(struct device *dev)
>   	mutex_unlock(&input->mutex);
>   	return retval;
>   }
> +#endif /* CONFIG_PM_SLEEP */
>   
>   static SIMPLE_DEV_PM_OPS(keyscan_dev_pm_ops, keyscan_suspend, keyscan_resume);
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] HID: rmi: Protect PM-only functions by #ifdef CONFIG_PM
From: Jiri Kosina @ 2014-06-12  8:29 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linux-input, linux-kernel
In-Reply-To: <1402513398-17546-1-git-send-email-geert@linux-m68k.org>

On Wed, 11 Jun 2014, Geert Uytterhoeven wrote:

> If CONFIG_PM=n:
> 
> drivers/hid/hid-rmi.c:432: warning: ‘rmi_post_reset’ defined but not used
> drivers/hid/hid-rmi.c:437: warning: ‘rmi_post_resume’ defined but not used
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  drivers/hid/hid-rmi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
> index 2451c7e5febd..578bbe65902b 100644
> --- a/drivers/hid/hid-rmi.c
> +++ b/drivers/hid/hid-rmi.c
> @@ -428,6 +428,7 @@ static int rmi_raw_event(struct hid_device *hdev,
>  	return 0;
>  }
>  
> +#ifdef CONFIG_PM
>  static int rmi_post_reset(struct hid_device *hdev)
>  {
>  	return rmi_set_mode(hdev, RMI_MODE_ATTN_REPORTS);
> @@ -437,6 +438,7 @@ static int rmi_post_resume(struct hid_device *hdev)
>  {
>  	return rmi_set_mode(hdev, RMI_MODE_ATTN_REPORTS);
>  }
> +#endif /* CONFIG_PM */
>  
>  #define RMI4_MAX_PAGE 0xff
>  #define RMI4_PAGE_SIZE 0x0100

Applied, thanks for spotting it.

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] Input: evdev - Fix incorrect kfree of err_free_client after vzalloc
From: David Rientjes @ 2014-06-12  6:49 UTC (permalink / raw)
  To: Yongtaek Lee; +Cc: rydberg, dmitry.torokhov, daniels, linux-input, linux-kernel
In-Reply-To: <1402537537-17945-1-git-send-email-ytk.lee@samsung.com>

On Thu, 12 Jun 2014, Yongtaek Lee wrote:

> This bug was introduced by commit 92eb77d ("Input: evdev - fall back
> to vmalloc for client event buffer").
> 
> vzalloc is used to alloc memory as fallback in case of failure
> of kzalloc. But err_free_client was not considered on below case.
> 1. kzalloc fail
> 2. vzalloc success
> 3. evdev_open_device fail
> 4. kfree
> 
> So that address checking is needed to call correct free function.
> 
> Signed-off-by: Yongtaek Lee <ytk.lee@samsung.com>
> Reviewed-by: Daniel Stone <daniels@collabora.com>

Acked-by: David Rientjes <rientjes@google.com>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox