[PATCH v3 0/5] Introduce the Atmel PTC subsystem

[PATCH v3 0/5] Introduce the Atmel PTC subsystem

[Ludovic Desroches]

Hi,

The Atmel Peripheral touch controller subsystem offers built-in hardware for
capacitive touch measurement on sensors that function as buttons, sliders and
wheels. It is available on SAMA5D2. Public documentation will be available
soon.

A firmware and a configuration file describing the topology and the parameters
of the sensor are loaded when probing the driver.

Changes:
- v3:
  - fix MAINTAINERS entry
- v2:
  - reorder patches to get the bindings documentation in first
  - remove the header from the uapi since it may change in the future. Declare
  only the few structures needed in the driver.
  - add this driver to the sama5_defconfig

Ludovic Desroches (5):
  dt-bindings: input: Add Atmel PTC subsystem bindings
  input: misc: introduce Atmel PTC driver
  MAINTAINERS: add Atmel PTC entries
  ARM: dts: at91: sama5d2: add PTC subsystem device
  ARM: configs: at91: add PTC driver to sama5_defconfig

 .../devicetree/bindings/input/atmel,ptc.txt        |  67 ++
 MAINTAINERS                                        |   7 +
 arch/arm/boot/dts/sama5d2.dtsi                     |  16 +
 arch/arm/configs/sama5_defconfig                   |   2 +
 drivers/input/misc/Kconfig                         |  12 +
 drivers/input/misc/Makefile                        |   1 +
 drivers/input/misc/atmel_ptc.c                     | 723 +++++++++++++++++++++
 7 files changed, 828 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/atmel,ptc.txt
 create mode 100644 drivers/input/misc/atmel_ptc.c

-- 
2.12.2

[PATCH v3 1/5] dt-bindings: input: Add Atmel PTC subsystem bindings

[Ludovic Desroches]

Add description of the Atmel PTC subsystem bindings.

Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/input/atmel,ptc.txt        | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/atmel,ptc.txt

diff --git a/Documentation/devicetree/bindings/input/atmel,ptc.txt b/Documentation/devicetree/bindings/input/atmel,ptc.txt
new file mode 100644
index 000000000000..a183fd511e04
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/atmel,ptc.txt
@@ -0,0 +1,67 @@
+Atmel PTC Subsystem
+
+The Atmel Peripheral Touch Controller subsystem offers built-in hardware
+for capacitive touch measurement on sensors that function as buttons, sliders
+and wheels.
+
+1) PTC Subsystem node
+
+Required properties:
+- compatible: 		Must be "atmel,sama5d2-ptc"
+- reg: 			Address, length of the shared memory and ppp registers location
+			and length.
+- clocks: 		Phandlers to the clocks.
+- clock-names: 		Must be "ptc_clk", "ptc_int_osc", "slow_clk".
+- #address-cells:	Must be one. The cell is the button or scroller id.
+- #size-cells: 		Must be zero.
+
+Example:
+	ptc@fc060000 {
+		compatible = "atmel,sama5d2-ptc";
+		reg = <0x00800000 0x10000
+		       0xfc060000 0xcf>;
+		interrupts = <58 IRQ_TYPE_LEVEL_HIGH 7>;
+		clocks = <&ptc_clk>, <&main>, <&clk32k>;
+		clock-names = "ptc_clk", "ptc_int_osc", "slow_clk";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		[ child node definitions... ]
+	};
+
+2) Scroller / buttons subnodes
+
+Subnodes describe the kind of sensors the customer want to use. They have to be
+named according to their function: button, slider or wheel.
+
+2.1) Scroller subnodes
+
+Required properties:
+- reg:	Id of the scroller, each id must be different.
+
+Example:
+	slider@0 {
+		reg = <0>;
+	};
+
+	wheel@1 {
+		reg = <1>;
+	};
+
+2.2) Button subnodes
+
+Required properties:
+- reg:			Id of node used for the button, each id must be
+			different.
+- linux,keycode: 	Key code of the button.
+
+Example:
+		button@8 {
+			reg = <8>;
+			linux,keycode = <2>;
+		};
+
+		button@9 {
+			reg = <9>;
+			linux,keycode = <3>;
+		};
-- 
2.12.2

[PATCH v3 2/5] input: misc: introduce Atmel PTC driver

[Ludovic Desroches]

The Atmel Peripheral Touch Controller subsystem offers built-in hardware
for capacitive touch measurement on sensors that function as buttons,
sliders and wheels.

Two files are loaded when probing the driver:
- a firmware for the Pico Power Processor that computes raw data from
  the ADC front end to provide high level information as button touch or
  untouch, slider position, etc.
- a configuration file that describe the topology and the parameters of
  the sensors.

Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 drivers/input/misc/Kconfig     |  12 +
 drivers/input/misc/Makefile    |   1 +
 drivers/input/misc/atmel_ptc.c | 723 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 736 insertions(+)
 create mode 100644 drivers/input/misc/atmel_ptc.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 9f082a388388..76616b0425b5 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -96,6 +96,18 @@ config INPUT_ATMEL_CAPTOUCH
 	  To compile this driver as a module, choose M here: the
 	  module will be called atmel_captouch.
 
+config INPUT_ATMEL_PTC
+	tristate "Atmel PTC Driver"
+	depends on OF || COMPILE_TEST
+	depends on SOC_SAMA5D2
+	help
+	  Say Y to enable support for the Atmel PTC Subsystem.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called atmel_ptc.
+	  If you compile it as a built-in driver, you have to build the
+	  firmware into the kernel or to use an initrd.
+
 config INPUT_BMA150
 	tristate "BMA150/SMB380 acceleration sensor support"
 	depends on I2C
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 03fd4262ada9..f71972adc002 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_INPUT_ARIZONA_HAPTICS)	+= arizona-haptics.o
 obj-$(CONFIG_INPUT_ATI_REMOTE2)		+= ati_remote2.o
 obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
 obj-$(CONFIG_INPUT_ATMEL_CAPTOUCH)	+= atmel_captouch.o
+obj-$(CONFIG_INPUT_ATMEL_PTC)		+= atmel_ptc.o
 obj-$(CONFIG_INPUT_BFIN_ROTARY)		+= bfin_rotary.o
 obj-$(CONFIG_INPUT_BMA150)		+= bma150.o
 obj-$(CONFIG_INPUT_CM109)		+= cm109.o
diff --git a/drivers/input/misc/atmel_ptc.c b/drivers/input/misc/atmel_ptc.c
new file mode 100644
index 000000000000..612eaede6072
--- /dev/null
+++ b/drivers/input/misc/atmel_ptc.c
@@ -0,0 +1,723 @@
+/*
+ * Atmel PTC subsystem driver for SAMA5D2 devices and compatible.
+ *
+ * Copyright (C) 2017 Microchip,
+ *               2017 Ludovic Desroches <ludovic.desroches@microchip.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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/cdev.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/firmware.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+
+#define ATMEL_PTC_MAX_NODES	64
+#define ATMEL_PTC_MAX_SCROLLERS	4
+
+/* ----- PPP ----- */
+#define ATMEL_PPP_FIRMWARE_NAME	"atmel_ptc.bin"
+
+#define ATMEL_PPP_CONFIG	0x20
+#define ATMEL_PPP_CTRL		0x24
+#define ATMEL_PPP_CMD		0x28
+#define		ATMEL_PPP_CMD_STOP		0x1
+#define		ATMEL_PPP_CMD_RESET		0x2
+#define		ATMEL_PPP_CMD_RESTART		0x3
+#define		ATMEL_PPP_CMD_ABORT		0x4
+#define		ATMEL_PPP_CMD_RUN		0x5
+#define		ATMEL_PPP_CMD_RUN_LOCKED	0x6
+#define		ATMEL_PPP_CMD_RUN_OCD		0x7
+#define		ATMEL_PPP_CMD_UNLOCK		0x8
+#define		ATMEL_PPP_CMD_NMI		0x9
+#define		ATMEL_PPP_CMD_HOST_OCD_RESUME	0xB
+#define ATMEL_PPP_ISR		0x33
+#define		ATMEL_PPP_IRQ_MASK	GENMASK(7, 4)
+#define		ATMEL_PPP_IRQ0		BIT(4)
+#define		ATMEL_PPP_IRQ1		BIT(5)
+#define		ATMEL_PPP_IRQ2		BIT(6)
+#define		ATMEL_PPP_IRQ3		BIT(7)
+#define		ATMEL_PPP_NOTIFY_MASK	GENMASK(3, 0)
+#define		ATMEL_PPP_NOTIFY0	BIT(0)
+#define		ATMEL_PPP_NOTIFY1	BIT(1)
+#define		ATMEL_PPP_NOTIFY2	BIT(2)
+#define		ATMEL_PPP_NOTIFY3	BIT(3)
+#define ATMEL_PPP_IDR		0x34
+#define ATMEL_PPP_IER		0x35
+
+#define atmel_ppp_readb(ptc, reg)	readb_relaxed(ptc->ppp_regs + reg)
+#define atmel_ppp_writeb(ptc, reg, val)	writeb_relaxed(val, ptc->ppp_regs + reg)
+#define atmel_ppp_readl(ptc, reg)	readl_relaxed(ptc->ppp_regs + reg)
+#define atmel_ppp_writel(ptc, reg, val)	writel_relaxed(val, ptc->ppp_regs + reg)
+
+/* ----- QTM ----- */
+#define ATMEL_QTM_CONF_NAME		"atmel_ptc.conf"
+
+#define ATMEL_QTM_MB_OFFSET			0x4000
+#define ATMEL_QTM_MB_SIZE			0x1000
+
+#define ATMEL_QTM_MB_CMD_OFFSET			0x0
+#define		ATMEL_QTM_CMD_FIRM_VERSION		8
+#define		ATMEL_QTM_CMD_INIT			18
+#define		ATMEL_QTM_CMD_RUN			19
+#define		ATMEL_QTM_CMD_STOP			21
+#define		ATMEL_QTM_CMD_SET_ACQ_MODE_TIMER	24
+#define ATMEL_QTM_MB_NODE_GROUP_CONFIG_OFFSET	0x100
+#define ATMEL_QTM_MB_SCROLLER_CONFIG_OFFSET	0x81a
+#define		ATMEL_QTM_SCROLLER_TYPE_SLIDER		0x0
+#define		ATMEL_QTM_SCROLLER_TYPE_WHEEL		0x1
+#define ATMEL_QTM_MB_SCROLLER_DATA_OFFSET	0x842
+#define ATMEL_QTM_MB_TOUCH_EVENTS_OFFSET	0x880
+
+#define atmel_qtm_get_scroller_config(buf, id) \
+	memcpy(buf, \
+	       ptc->qtm_mb + ATMEL_QTM_MB_SCROLLER_CONFIG_OFFSET \
+	       + (id) * sizeof(struct atmel_qtm_scroller_config), \
+	       sizeof(struct atmel_qtm_scroller_config))
+
+#define atmel_qtm_get_scroller_data(buf, id) \
+	memcpy(buf, \
+	       ptc->qtm_mb + ATMEL_QTM_MB_SCROLLER_DATA_OFFSET \
+	       + (id) * sizeof(struct atmel_qtm_scroller_data), \
+	       sizeof(struct atmel_qtm_scroller_data))
+
+#define get_scroller_resolution(scroller_config) \
+	(1 << (scroller_config.resol_deadband >> 4))
+
+struct atmel_qtm_cmd {
+	u16	id;
+	u16	addr;
+	u32	data;
+} __packed;
+
+struct atmel_qtm_node_group_config {
+	u16	count;
+	u8	ptc_type;
+	u8	freq_option;
+	u8	calib_option;
+	u8	unused;
+} __packed;
+
+struct atmel_qtm_scroller_config {
+	u8	type;
+	u8	unused;
+	u16	key_start;
+	u8	key_count;
+	u8	resol_deadband;
+	u8	position_hysteresis;
+	u8	unused2;
+	u16	contact_min_threshold;
+} __packed;
+
+struct atmel_qtm_scroller_data {
+	u8	status;
+	u8	right_hyst;
+	u8	left_hyst;
+	u8	unused;
+	u16	raw_position;
+	u16	position;
+	u16	contact_size;
+} __packed;
+
+struct atmel_qtm_touch_events {
+	u32	key_event_id[2];
+	u32	key_enable_state[2];
+	u32	scroller_event_id;
+	u32	scroller_event_state;
+} __packed;
+
+struct atmel_ptc {
+	void __iomem		*ppp_regs;
+	void __iomem		*firmware;
+	int			irq;
+	u8			imr;
+	void __iomem		*qtm_mb;
+	struct clk		*clk_per;
+	struct clk		*clk_int_osc;
+	struct clk		*clk_slow;
+	struct device		*dev;
+	struct completion	ppp_ack;
+	unsigned int		button_keycode[ATMEL_PTC_MAX_NODES];
+	struct input_dev	*buttons_input;
+	struct input_dev	*scroller_input[ATMEL_PTC_MAX_SCROLLERS];
+	bool			buttons_registered;
+	bool			scroller_registered[ATMEL_PTC_MAX_SCROLLERS];
+	u32			button_event[ATMEL_PTC_MAX_NODES / 32];
+	u32			button_state[ATMEL_PTC_MAX_NODES / 32];
+	u32			scroller_event;
+	u32			scroller_state;
+};
+
+static void atmel_ppp_irq_enable(struct atmel_ptc *ptc, u8 mask)
+{
+	ptc->imr |= mask;
+	atmel_ppp_writeb(ptc, ATMEL_PPP_IER, mask & ATMEL_PPP_IRQ_MASK);
+}
+
+static void atmel_ppp_irq_disable(struct atmel_ptc *ptc, u8 mask)
+{
+	ptc->imr &= ~mask;
+	atmel_ppp_writeb(ptc, ATMEL_PPP_IDR, mask & ATMEL_PPP_IRQ_MASK);
+}
+
+static void atmel_ppp_notify(struct atmel_ptc *ptc, u8 mask)
+{
+	if (mask & ATMEL_PPP_NOTIFY_MASK) {
+		u8 notify = atmel_ppp_readb(ptc, ATMEL_PPP_ISR)
+			| (mask & ATMEL_PPP_NOTIFY_MASK);
+
+		atmel_ppp_writeb(ptc, ATMEL_PPP_ISR, notify);
+	}
+}
+
+static void atmel_ppp_irq_pending_clr(struct atmel_ptc *ptc, u8 mask)
+{
+	if (mask & ATMEL_PPP_IRQ_MASK) {
+		u8 irq = atmel_ppp_readb(ptc, ATMEL_PPP_ISR) & ~mask;
+
+		atmel_ppp_writeb(ptc, ATMEL_PPP_ISR, irq);
+	}
+}
+
+static void atmel_ppp_cmd_send(struct atmel_ptc *ptc, u32 cmd)
+{
+	atmel_ppp_writel(ptc, ATMEL_PPP_CMD, cmd);
+}
+
+static void atmel_ppp_irq_scroller_event(struct atmel_ptc *ptc)
+{
+	int i;
+
+	if (!ptc->scroller_event)
+		return;
+
+	for (i = 0; i < ATMEL_PTC_MAX_SCROLLERS; i++) {
+		u32 mask = 1 << i;
+		struct atmel_qtm_scroller_data scroller_data;
+		struct atmel_qtm_scroller_config scroller_config;
+
+		if (!(ptc->scroller_event & mask))
+			continue;
+
+		atmel_qtm_get_scroller_data(&scroller_data, i);
+		atmel_qtm_get_scroller_config(&scroller_config, i);
+
+		if (scroller_config.type == ATMEL_QTM_SCROLLER_TYPE_WHEEL)
+			input_report_abs(ptc->scroller_input[i],
+					 ABS_WHEEL, scroller_data.position);
+		else
+			input_report_abs(ptc->scroller_input[i],
+					 ABS_X, scroller_data.position);
+
+		input_report_key(ptc->scroller_input[i], BTN_TOUCH,
+				 scroller_data.status & 0x1);
+		input_sync(ptc->scroller_input[i]);
+	}
+}
+
+static void atmel_ppp_irq_button_event(struct atmel_ptc *ptc)
+{
+	int i, j;
+
+	for (i = 0; i < ATMEL_PTC_MAX_NODES / 32; i++) {
+		if (!ptc->button_event[i])
+			continue;
+
+		for (j = 0; j < 32; j++) {
+			u32 mask = 1 << j;
+			u32 state = ptc->button_state[i] & mask;
+			unsigned int key_id = i * 32 + j;
+
+			if (!(ptc->button_event[i] & mask))
+				continue;
+
+			input_report_key(ptc->buttons_input,
+					 ptc->button_keycode[key_id], !!state);
+			input_sync(ptc->buttons_input);
+		}
+	}
+}
+
+static void atmel_ppp_irq_touch_event(struct atmel_ptc *ptc)
+{
+	atmel_ppp_irq_scroller_event(ptc);
+	atmel_ppp_irq_button_event(ptc);
+}
+
+static irqreturn_t atmel_ppp_irq_handler(int irq, void *data)
+{
+	struct atmel_ptc *ptc = data;
+	u32 isr = atmel_ppp_readb(ptc, ATMEL_PPP_ISR) & ptc->imr;
+
+	/* QTM CMD acknowledgment */
+	if (isr & ATMEL_PPP_IRQ0) {
+		atmel_ppp_irq_disable(ptc, ATMEL_PPP_IRQ0);
+		atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ0);
+		complete(&ptc->ppp_ack);
+	}
+	/* QTM touch event */
+	if (isr & ATMEL_PPP_IRQ1) {
+		struct atmel_qtm_touch_events touch_events;
+		int i;
+
+		memcpy(&touch_events,
+		       ptc->qtm_mb + ATMEL_QTM_MB_TOUCH_EVENTS_OFFSET,
+		       sizeof(touch_events));
+
+		for (i = 0; i < ATMEL_PTC_MAX_NODES / 32; i++) {
+			ptc->button_event[i] = touch_events.key_event_id[i];
+			ptc->button_state[i] = touch_events.key_enable_state[i];
+		}
+		ptc->scroller_event = touch_events.scroller_event_id;
+		ptc->scroller_state = touch_events.scroller_event_state;
+
+		atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ1);
+
+		atmel_ppp_irq_touch_event(ptc);
+	}
+	/* Debug event */
+	if (isr & ATMEL_PPP_IRQ2)
+		atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ2);
+
+	return IRQ_HANDLED;
+}
+
+void atmel_qtm_cmd_send(struct atmel_ptc *ptc, struct atmel_qtm_cmd *cmd)
+{
+	int i, ret;
+
+	dev_dbg(ptc->dev, "%s: cmd=0x%x, addr=0x%x, data=0x%x\n",
+		__func__, cmd->id, cmd->addr, cmd->data);
+
+	memcpy(ptc->qtm_mb, cmd, sizeof(*cmd));
+
+	/* Once command performed, we'll get an IRQ. */
+	atmel_ppp_irq_enable(ptc, ATMEL_PPP_IRQ0);
+	/* Notify PPP that we have sent a command. */
+	atmel_ppp_notify(ptc, ATMEL_PPP_NOTIFY0);
+	/* Wait for IRQ from PPP. */
+	wait_for_completion(&ptc->ppp_ack);
+
+	/*
+	 * Register input devices only when QTM is started since we need some
+	 * information from the QTM configuration.
+	 */
+	if (cmd->id == ATMEL_QTM_CMD_RUN) {
+		if (ptc->buttons_input && !ptc->buttons_registered) {
+			ret = input_register_device(ptc->buttons_input);
+			if (ret)
+				dev_err(ptc->dev, "can't register input button device.\n");
+			else
+				ptc->buttons_registered = true;
+		}
+
+		for (i = 0; i < ATMEL_PTC_MAX_SCROLLERS; i++) {
+			struct input_dev *scroller = ptc->scroller_input[i];
+			struct atmel_qtm_scroller_config scroller_config;
+
+			if (!scroller || ptc->scroller_registered[i])
+				continue;
+
+			atmel_qtm_get_scroller_config(&scroller_config, i);
+
+			if (scroller_config.type ==
+			    ATMEL_QTM_SCROLLER_TYPE_SLIDER) {
+				unsigned int max = get_scroller_resolution(scroller_config);
+
+				input_set_abs_params(scroller, 0, 0, max, 0, 0);
+			}
+			ret = input_register_device(scroller);
+			if (ret)
+				dev_err(ptc->dev, "can't register input scroller device.\n");
+			else
+				ptc->scroller_registered[i] = true;
+		}
+	}
+
+	memcpy(cmd, ptc->qtm_mb, sizeof(*cmd));
+}
+
+static inline struct atmel_ptc *kobj_to_atmel_ptc(struct kobject *kobj)
+{
+	struct device *dev = kobj_to_dev(kobj);
+
+	return dev->driver_data;
+}
+
+static ssize_t atmel_qtm_mb_read(struct file *filp, struct kobject *kobj,
+				 struct bin_attribute *attr,
+				 char *buf, loff_t off, size_t count)
+{
+	struct atmel_ptc *ptc = kobj_to_atmel_ptc(kobj);
+	char *qtm_mb = (char *)ptc->qtm_mb;
+
+	dev_dbg(ptc->dev, "%s: off=0x%llx, count=%zu\n", __func__, off, count);
+
+	memcpy(buf, qtm_mb + off, count);
+
+	return count;
+}
+
+static ssize_t atmel_qtm_mb_write(struct file *filp, struct kobject *kobj,
+				  struct bin_attribute *attr,
+				  char *buf, loff_t off, size_t count)
+{
+	struct atmel_ptc *ptc = kobj_to_atmel_ptc(kobj);
+	char *qtm_mb = (char *)ptc->qtm_mb;
+
+	dev_dbg(ptc->dev, "%s: off=0x%llx, count=%zu\n", __func__, off, count);
+
+	if (off == 0 && count == sizeof(struct atmel_qtm_cmd))
+		atmel_qtm_cmd_send(ptc, (struct atmel_qtm_cmd *)buf);
+	else
+		memcpy(qtm_mb + off, buf, count);
+
+	return count;
+}
+
+static struct bin_attribute atmel_ptc_qtm_mb_attr = {
+	.attr = {
+		.name = "qtm_mb",
+		.mode = 0644,
+	},
+	.size = ATMEL_QTM_MB_SIZE,
+	.read = atmel_qtm_mb_read,
+	.write = atmel_qtm_mb_write,
+};
+
+/*
+ * From QTM MB configuration, we can't retrieve all the information needed
+ * to setup correctly input devices: buttons key codes and slider axis are
+ * missing.
+ */
+static int atmel_ptc_of_parse(struct atmel_ptc *ptc)
+{
+	struct device_node *sensor;
+	bool first_button = true;
+
+	/* Parse sensors. */
+	for_each_child_of_node(ptc->dev->of_node, sensor) {
+		if (!strcmp(sensor->name, "button")) {
+			u32 key_id, keycode;
+			struct input_dev *buttons = ptc->buttons_input;
+
+			if (of_property_read_u32(sensor, "reg", &key_id)) {
+				dev_err(ptc->dev, "reg is missing (%s)\n",
+					sensor->full_name);
+				return -EINVAL;
+			}
+
+			if (of_property_read_u32(sensor, "linux,keycode", &keycode)) {
+				dev_err(ptc->dev, "linux,keycode is missing (%s)\n",
+					sensor->full_name);
+				return -EINVAL;
+			}
+			ptc->button_keycode[key_id] = keycode;
+
+			/* All buttons are put together in a keyboard device. */
+			if (first_button) {
+				buttons = devm_input_allocate_device(ptc->dev);
+				if (!buttons)
+					return -ENOMEM;
+				buttons->name = "atmel_ptc_buttons";
+				buttons->dev.parent = ptc->dev;
+				buttons->keycode = ptc->button_keycode;
+				buttons->keycodesize = sizeof(ptc->button_keycode[0]);
+				buttons->keycodemax = ATMEL_PTC_MAX_NODES;
+				ptc->buttons_input = buttons;
+				first_button = false;
+			}
+
+			input_set_capability(buttons, EV_KEY, keycode);
+		} else if (!strcmp(sensor->name, "slider") ||
+			   !strcmp(sensor->name, "wheel")) {
+			u32 scroller_id;
+			struct input_dev *scroller;
+
+			if (of_property_read_u32(sensor, "reg", &scroller_id)) {
+				dev_err(ptc->dev, "reg is missing (%s)\n",
+					sensor->full_name);
+				return -EINVAL;
+			}
+
+			if (scroller_id > ATMEL_PTC_MAX_SCROLLERS - 1) {
+				dev_err(ptc->dev, "wrong scroller id (%s)\n",
+					sensor->full_name);
+				return -EINVAL;
+			}
+
+			scroller = devm_input_allocate_device(ptc->dev);
+			if (!scroller)
+				return -ENOMEM;
+
+			scroller->dev.parent = ptc->dev;
+			ptc->scroller_input[scroller_id] = scroller;
+
+			if (!strcmp(sensor->name, "slider")) {
+				scroller->name = "atmel_ptc_slider";
+				input_set_capability(scroller, EV_ABS, ABS_X);
+				input_set_capability(scroller, EV_KEY, BTN_TOUCH);
+			} else {
+				scroller->name = "atmel_ptc_wheel";
+				input_set_capability(scroller, EV_ABS, ABS_WHEEL);
+				input_set_capability(scroller, EV_KEY, BTN_TOUCH);
+			}
+		} else {
+			dev_err(ptc->dev, "%s is not supported\n", sensor->name);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static void atmel_qtm_conf_callback(const struct firmware *conf, void *context)
+{
+	struct atmel_ptc *ptc = context;
+	struct atmel_qtm_cmd qtm_cmd;
+	char *dst;
+	struct atmel_qtm_node_group_config node_group_config;
+
+	if (!conf) {
+		dev_err(ptc->dev, "cannot load QTM configuration, it has to be set manually.\n");
+		return;
+	}
+
+	atmel_ppp_irq_enable(ptc, ATMEL_PPP_IRQ1);
+	atmel_ppp_irq_disable(ptc, ATMEL_PPP_IRQ2 | ATMEL_PPP_IRQ3);
+
+	qtm_cmd.id = ATMEL_QTM_CMD_STOP;
+	atmel_qtm_cmd_send(ptc, &qtm_cmd);
+
+	/* Load QTM configuration. */
+	dst = (char *)ptc->qtm_mb + ATMEL_QTM_MB_NODE_GROUP_CONFIG_OFFSET;
+	/* memcpy doesn't work for an unknown reason. */
+	_memcpy_toio(dst, conf->data, conf->size);
+	release_firmware(conf);
+
+	if (atmel_ptc_of_parse(ptc))
+		dev_err(ptc->dev, "ptc_of_parse failed\n");
+
+	memcpy(&node_group_config,
+	       ptc->qtm_mb + ATMEL_QTM_MB_NODE_GROUP_CONFIG_OFFSET,
+	       sizeof(node_group_config));
+
+	/* Start QTM. */
+	qtm_cmd.id = ATMEL_QTM_CMD_INIT;
+	qtm_cmd.data = node_group_config.count;
+	atmel_qtm_cmd_send(ptc, &qtm_cmd);
+	qtm_cmd.id = ATMEL_QTM_CMD_SET_ACQ_MODE_TIMER;
+	qtm_cmd.data = 20;
+	atmel_qtm_cmd_send(ptc, &qtm_cmd);
+	qtm_cmd.id = ATMEL_QTM_CMD_RUN;
+	qtm_cmd.data = node_group_config.count;
+	atmel_qtm_cmd_send(ptc, &qtm_cmd);
+}
+
+static void atmel_ppp_fw_callback(const struct firmware *fw, void *context)
+{
+	struct atmel_ptc *ptc = context;
+	int ret;
+	struct atmel_qtm_cmd cmd;
+
+	if (!fw || !fw->size) {
+		dev_err(ptc->dev, "cannot load firmware.\n");
+		release_firmware(fw);
+		device_release_driver(ptc->dev);
+		return;
+	}
+
+	/* Command sequence to start from a clean state. */
+	atmel_ppp_cmd_send(ptc, ATMEL_PPP_CMD_ABORT);
+	atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ_MASK);
+	atmel_ppp_cmd_send(ptc, ATMEL_PPP_CMD_RESET);
+
+	memcpy(ptc->firmware, fw->data, fw->size);
+	release_firmware(fw);
+
+	atmel_ppp_cmd_send(ptc, ATMEL_PPP_CMD_RUN);
+
+	cmd.id = ATMEL_QTM_CMD_FIRM_VERSION;
+	atmel_qtm_cmd_send(ptc, &cmd);
+	dev_info(ptc->dev, "firmware version: %u\n", cmd.data);
+
+	/* PPP is running, it's time to load the QTM configuration. */
+	ret = request_firmware_nowait(THIS_MODULE, 1, ATMEL_QTM_CONF_NAME, ptc->dev,
+				      GFP_KERNEL, ptc, atmel_qtm_conf_callback);
+	if (ret)
+		dev_err(ptc->dev, "QTM configuration loading failed.\n");
+}
+
+static int atmel_ptc_probe(struct platform_device *pdev)
+{
+	struct atmel_ptc *ptc;
+	struct resource	*res;
+	void *shared_memory;
+	int ret;
+
+	ptc = devm_kzalloc(&pdev->dev, sizeof(*ptc), GFP_KERNEL);
+	if (!ptc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, ptc);
+	ptc->dev = &pdev->dev;
+	ptc->dev->driver_data = ptc;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	shared_memory = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(shared_memory))
+		return PTR_ERR(shared_memory);
+
+	ptc->firmware = shared_memory;
+	ptc->qtm_mb = shared_memory + ATMEL_QTM_MB_OFFSET;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res)
+		return -EINVAL;
+
+	ptc->ppp_regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ptc->ppp_regs))
+		return PTR_ERR(ptc->ppp_regs);
+
+	ptc->irq = platform_get_irq(pdev, 0);
+	if (ptc->irq <= 0) {
+		if (!ptc->irq)
+			ptc->irq = -ENXIO;
+
+		return ptc->irq;
+	}
+
+	ptc->clk_per = devm_clk_get(&pdev->dev, "ptc_clk");
+	if (IS_ERR(ptc->clk_per))
+		return PTR_ERR(ptc->clk_per);
+
+	ptc->clk_int_osc = devm_clk_get(&pdev->dev, "ptc_int_osc");
+	if (IS_ERR(ptc->clk_int_osc))
+		return PTR_ERR(ptc->clk_int_osc);
+
+	ptc->clk_slow = devm_clk_get(&pdev->dev, "slow_clk");
+	if (IS_ERR(ptc->clk_slow))
+		return PTR_ERR(ptc->clk_slow);
+
+	ret = devm_request_irq(&pdev->dev, ptc->irq, atmel_ppp_irq_handler, 0,
+			       pdev->dev.driver->name, ptc);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(ptc->clk_int_osc);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(ptc->clk_per);
+	if (ret)
+		goto disable_clk_int_osc;
+
+	ret = clk_prepare_enable(ptc->clk_slow);
+	if (ret)
+		goto disable_clk_per;
+
+	/* Needed to avoid unexpected behaviors. */
+	memset(ptc->firmware, 0, ATMEL_QTM_MB_OFFSET + sizeof(*ptc->qtm_mb));
+	ptc->imr = 0;
+	init_completion(&ptc->ppp_ack);
+
+	/*
+	 * Expose a file to give an access to the QTM mailbox to a user space
+	 * application in order to configure it or to send commands.
+	 */
+	ret = sysfs_create_bin_file(&pdev->dev.kobj, &atmel_ptc_qtm_mb_attr);
+	if (ret)
+		goto disable_clk_slow;
+
+	ret = request_firmware_nowait(THIS_MODULE, 1, ATMEL_PPP_FIRMWARE_NAME,
+				      ptc->dev, GFP_KERNEL, ptc,
+				      atmel_ppp_fw_callback);
+	if (ret) {
+		dev_err(&pdev->dev, "firmware loading failed\n");
+		ret = -EPROBE_DEFER;
+		goto remove_qtm_mb;
+	}
+
+	return 0;
+
+remove_qtm_mb:
+	sysfs_remove_bin_file(&pdev->dev.kobj, &atmel_ptc_qtm_mb_attr);
+disable_clk_slow:
+	clk_disable_unprepare(ptc->clk_slow);
+disable_clk_per:
+	clk_disable_unprepare(ptc->clk_per);
+disable_clk_int_osc:
+	clk_disable_unprepare(ptc->clk_int_osc);
+
+	return ret;
+}
+
+static int atmel_ptc_remove(struct platform_device *pdev)
+{
+	struct atmel_ptc *ptc = platform_get_drvdata(pdev);
+	int i;
+
+	if (ptc->buttons_registered)
+		input_unregister_device(ptc->buttons_input);
+
+	for (i = 0; i < ATMEL_PTC_MAX_SCROLLERS; i++) {
+		struct input_dev *scroller = ptc->scroller_input[i];
+
+		if (!scroller || !ptc->scroller_registered[i])
+			continue;
+		input_unregister_device(scroller);
+	}
+
+	sysfs_remove_bin_file(&pdev->dev.kobj, &atmel_ptc_qtm_mb_attr);
+	clk_disable_unprepare(ptc->clk_slow);
+	clk_disable_unprepare(ptc->clk_per);
+	clk_disable_unprepare(ptc->clk_int_osc);
+
+	return 0;
+}
+
+static const struct of_device_id atmel_ptc_dt_match[] = {
+	{
+		.compatible = "atmel,sama5d2-ptc",
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, atmel_ptc_dt_match);
+
+static struct platform_driver atmel_ptc_driver = {
+	.probe = atmel_ptc_probe,
+	.remove = atmel_ptc_remove,
+	.driver = {
+		.name = "atmel_ptc",
+		.of_match_table = atmel_ptc_dt_match,
+	},
+};
+module_platform_driver(atmel_ptc_driver)
+
+MODULE_AUTHOR("Ludovic Desroches <ludovic.desroches@microchip.com>");
+MODULE_DESCRIPTION("Atmel PTC subsystem");
+MODULE_LICENSE("GPL v2");
+MODULE_FIRMWARE(ATMEL_PPP_FIRMWARE_NAME);
+MODULE_FIRMWARE(ATMEL_QTM_CONF_NAME);
-- 
2.12.2

[PATCH v3 3/5] MAINTAINERS: add Atmel PTC entries

[Ludovic Desroches]

Add entries for the Atmel PTC Subsystem.

Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0630482e701b..e1bf4c9d7696 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2371,6 +2371,13 @@ F:	Documentation/devicetree/bindings/input/atmel,maxtouch.txt
 F:	drivers/input/touchscreen/atmel_mxt_ts.c
 F:	include/linux/platform_data/atmel_mxt_ts.h
 
+ATMEL PTC SUBSYSTEM DRIVER
+M:	Ludovic Desroches <ludovic.desroches@microchip.com>
+L:	linux-input@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/input/atmel,ptc.txt
+F:	drivers/input/misc/atmel_ptc.c
+
 ATMEL NAND DRIVER
 M:	Wenyou Yang <wenyou.yang@atmel.com>
 M:	Josh Wu <rainyfeeling@outlook.com>
-- 
2.12.2

[PATCH v3 4/5] ARM: dts: at91: sama5d2: add PTC subsystem device

[Ludovic Desroches]

Add the Atmel Peripheral Touch Controller subsystem.

Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 arch/arm/boot/dts/sama5d2.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index b1a26b42d190..04e74013defb 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -840,6 +840,12 @@
 						atmel,clk-output-range = <0 83000000>;
 					};
 
+					ptc_clk: ptc_clk {
+						#clock-cells = <0>;
+						reg = <58>;
+						atmel,clk-output-range = <0 83000000>;
+					};
+
 					classd_clk: classd_clk {
 						#clock-cells = <0>;
 						reg = <59>;
@@ -1503,6 +1509,16 @@
 				reg = <0xfc05c000 0x20>;
 			};
 
+			ptc@fc060000 {
+				compatible = "atmel,sama5d2-ptc";
+				reg = <0x00800000 0x10000
+				       0xfc060000 0xcf>;
+				interrupts = <58 IRQ_TYPE_LEVEL_HIGH 7>;
+				clocks = <&ptc_clk>, <&main>, <&clk32k>;
+				clock-names = "ptc_clk", "ptc_int_osc", "slow_clk";
+				status = "disabled";
+			};
+
 			chipid@fc069000 {
 				compatible = "atmel,sama5d2-chipid";
 				reg = <0xfc069000 0x8>;
-- 
2.12.2

[PATCH v3 5/5] ARM: configs: at91: add PTC driver to sama5_defconfig

[Ludovic Desroches]

Add Peripheral Touch Controller driver to sama5_defconfig.

Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 arch/arm/configs/sama5_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/sama5_defconfig b/arch/arm/configs/sama5_defconfig
index 6529cb43e0fd..0422345ff3a3 100644
--- a/arch/arm/configs/sama5_defconfig
+++ b/arch/arm/configs/sama5_defconfig
@@ -118,6 +118,8 @@ CONFIG_KEYBOARD_GPIO=y
 # CONFIG_INPUT_MOUSE is not set
 CONFIG_INPUT_TOUCHSCREEN=y
 CONFIG_TOUCHSCREEN_ATMEL_MXT=y
+CONFIG_INPUT_MISC=y
+CONFIG_INPUT_ATMEL_PTC=m
 # CONFIG_SERIO is not set
 CONFIG_LEGACY_PTY_COUNT=4
 CONFIG_SERIAL_ATMEL=y
-- 
2.12.2

Re: [PATCH v3 1/5] dt-bindings: input: Add Atmel PTC subsystem bindings

[Dmitry Torokhov]

Hi Ludovic,

On Fri, Oct 20, 2017 at 03:31:17PM +0200, Ludovic Desroches wrote:
> Add description of the Atmel PTC subsystem bindings.
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>  .../devicetree/bindings/input/atmel,ptc.txt        | 67 ++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/atmel,ptc.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/atmel,ptc.txt b/Documentation/devicetree/bindings/input/atmel,ptc.txt
> new file mode 100644
> index 000000000000..a183fd511e04
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/atmel,ptc.txt
> @@ -0,0 +1,67 @@
> +Atmel PTC Subsystem
> +
> +The Atmel Peripheral Touch Controller subsystem offers built-in hardware
> +for capacitive touch measurement on sensors that function as buttons, sliders
> +and wheels.
> +
> +1) PTC Subsystem node
> +
> +Required properties:
> +- compatible: 		Must be "atmel,sama5d2-ptc"
> +- reg: 			Address, length of the shared memory and ppp registers location
> +			and length.
> +- clocks: 		Phandlers to the clocks.
> +- clock-names: 		Must be "ptc_clk", "ptc_int_osc", "slow_clk".
> +- #address-cells:	Must be one. The cell is the button or scroller id.
> +- #size-cells: 		Must be zero.
> +
> +Example:
> +	ptc@fc060000 {
> +		compatible = "atmel,sama5d2-ptc";
> +		reg = <0x00800000 0x10000
> +		       0xfc060000 0xcf>;
> +		interrupts = <58 IRQ_TYPE_LEVEL_HIGH 7>;
> +		clocks = <&ptc_clk>, <&main>, <&clk32k>;
> +		clock-names = "ptc_clk", "ptc_int_osc", "slow_clk";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		[ child node definitions... ]
> +	};
> +
> +2) Scroller / buttons subnodes
> +
> +Subnodes describe the kind of sensors the customer want to use. They have to be
> +named according to their function: button, slider or wheel.

I wonder do we really need this? Or we can have a generic:

	slider@0 {
		reg = <0>;
		linux,type = <EV_ABS>;
		linux,code = <ABS_X>;
	},
	vertical-slider@1 {
		reg = <1>;
		linux,type = <EV_ABS>;
		linux,code = <ABS_Y>;
	},
	wheel@2 {
		reg = <2>;
		linux,type = <EV_ABS>;
		linux,code = <ABS_WHEEL>;
	},
	button@3 {
		reg = <3>;
		linux,type = <EV_KEY>;
		linux,code = <KEY_A>;
	},
	...

I.e. you specify type/code in a generic way.

Thanks.

-- 
Dmitry

Re: [PATCH v3 2/5] input: misc: introduce Atmel PTC driver

[Dmitry Torokhov]

Hi Ludovic,

On Fri, Oct 20, 2017 at 03:31:18PM +0200, Ludovic Desroches wrote:
> The Atmel Peripheral Touch Controller subsystem offers built-in hardware
> for capacitive touch measurement on sensors that function as buttons,
> sliders and wheels.
> 
> Two files are loaded when probing the driver:
> - a firmware for the Pico Power Processor that computes raw data from
>   the ADC front end to provide high level information as button touch or
>   untouch, slider position, etc.
> - a configuration file that describe the topology and the parameters of
>   the sensors.

If I understand it correctly the configuration is loaded via a cha

> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
>  drivers/input/misc/Kconfig     |  12 +
>  drivers/input/misc/Makefile    |   1 +
>  drivers/input/misc/atmel_ptc.c | 723 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 736 insertions(+)
>  create mode 100644 drivers/input/misc/atmel_ptc.c
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 9f082a388388..76616b0425b5 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -96,6 +96,18 @@ config INPUT_ATMEL_CAPTOUCH
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called atmel_captouch.
>  
> +config INPUT_ATMEL_PTC
> +	tristate "Atmel PTC Driver"
> +	depends on OF || COMPILE_TEST
> +	depends on SOC_SAMA5D2

Maybe "default SOC_SAMA5D2"?


> +	help
> +	  Say Y to enable support for the Atmel PTC Subsystem.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called atmel_ptc.
> +	  If you compile it as a built-in driver, you have to build the
> +	  firmware into the kernel or to use an initrd.
> +
>  config INPUT_BMA150
>  	tristate "BMA150/SMB380 acceleration sensor support"
>  	depends on I2C
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 03fd4262ada9..f71972adc002 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_INPUT_ARIZONA_HAPTICS)	+= arizona-haptics.o
>  obj-$(CONFIG_INPUT_ATI_REMOTE2)		+= ati_remote2.o
>  obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
>  obj-$(CONFIG_INPUT_ATMEL_CAPTOUCH)	+= atmel_captouch.o
> +obj-$(CONFIG_INPUT_ATMEL_PTC)		+= atmel_ptc.o
>  obj-$(CONFIG_INPUT_BFIN_ROTARY)		+= bfin_rotary.o
>  obj-$(CONFIG_INPUT_BMA150)		+= bma150.o
>  obj-$(CONFIG_INPUT_CM109)		+= cm109.o
> diff --git a/drivers/input/misc/atmel_ptc.c b/drivers/input/misc/atmel_ptc.c
> new file mode 100644
> index 000000000000..612eaede6072
> --- /dev/null
> +++ b/drivers/input/misc/atmel_ptc.c
> @@ -0,0 +1,723 @@
> +/*
> + * Atmel PTC subsystem driver for SAMA5D2 devices and compatible.
> + *
> + * Copyright (C) 2017 Microchip,
> + *               2017 Ludovic Desroches <ludovic.desroches@microchip.com>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * 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/cdev.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/firmware.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +
> +#define ATMEL_PTC_MAX_NODES	64
> +#define ATMEL_PTC_MAX_SCROLLERS	4
> +
> +/* ----- PPP ----- */
> +#define ATMEL_PPP_FIRMWARE_NAME	"atmel_ptc.bin"
> +
> +#define ATMEL_PPP_CONFIG	0x20
> +#define ATMEL_PPP_CTRL		0x24
> +#define ATMEL_PPP_CMD		0x28
> +#define		ATMEL_PPP_CMD_STOP		0x1
> +#define		ATMEL_PPP_CMD_RESET		0x2
> +#define		ATMEL_PPP_CMD_RESTART		0x3
> +#define		ATMEL_PPP_CMD_ABORT		0x4
> +#define		ATMEL_PPP_CMD_RUN		0x5
> +#define		ATMEL_PPP_CMD_RUN_LOCKED	0x6
> +#define		ATMEL_PPP_CMD_RUN_OCD		0x7
> +#define		ATMEL_PPP_CMD_UNLOCK		0x8
> +#define		ATMEL_PPP_CMD_NMI		0x9
> +#define		ATMEL_PPP_CMD_HOST_OCD_RESUME	0xB
> +#define ATMEL_PPP_ISR		0x33
> +#define		ATMEL_PPP_IRQ_MASK	GENMASK(7, 4)
> +#define		ATMEL_PPP_IRQ0		BIT(4)
> +#define		ATMEL_PPP_IRQ1		BIT(5)
> +#define		ATMEL_PPP_IRQ2		BIT(6)
> +#define		ATMEL_PPP_IRQ3		BIT(7)
> +#define		ATMEL_PPP_NOTIFY_MASK	GENMASK(3, 0)
> +#define		ATMEL_PPP_NOTIFY0	BIT(0)
> +#define		ATMEL_PPP_NOTIFY1	BIT(1)
> +#define		ATMEL_PPP_NOTIFY2	BIT(2)
> +#define		ATMEL_PPP_NOTIFY3	BIT(3)
> +#define ATMEL_PPP_IDR		0x34
> +#define ATMEL_PPP_IER		0x35
> +
> +#define atmel_ppp_readb(ptc, reg)	readb_relaxed(ptc->ppp_regs + reg)
> +#define atmel_ppp_writeb(ptc, reg, val)	writeb_relaxed(val, ptc->ppp_regs + reg)
> +#define atmel_ppp_readl(ptc, reg)	readl_relaxed(ptc->ppp_regs + reg)
> +#define atmel_ppp_writel(ptc, reg, val)	writel_relaxed(val, ptc->ppp_regs + reg)
> +
> +/* ----- QTM ----- */
> +#define ATMEL_QTM_CONF_NAME		"atmel_ptc.conf"
> +
> +#define ATMEL_QTM_MB_OFFSET			0x4000
> +#define ATMEL_QTM_MB_SIZE			0x1000
> +
> +#define ATMEL_QTM_MB_CMD_OFFSET			0x0
> +#define		ATMEL_QTM_CMD_FIRM_VERSION		8
> +#define		ATMEL_QTM_CMD_INIT			18
> +#define		ATMEL_QTM_CMD_RUN			19
> +#define		ATMEL_QTM_CMD_STOP			21
> +#define		ATMEL_QTM_CMD_SET_ACQ_MODE_TIMER	24
> +#define ATMEL_QTM_MB_NODE_GROUP_CONFIG_OFFSET	0x100
> +#define ATMEL_QTM_MB_SCROLLER_CONFIG_OFFSET	0x81a
> +#define		ATMEL_QTM_SCROLLER_TYPE_SLIDER		0x0
> +#define		ATMEL_QTM_SCROLLER_TYPE_WHEEL		0x1
> +#define ATMEL_QTM_MB_SCROLLER_DATA_OFFSET	0x842
> +#define ATMEL_QTM_MB_TOUCH_EVENTS_OFFSET	0x880
> +
> +#define atmel_qtm_get_scroller_config(buf, id) \
> +	memcpy(buf, \
> +	       ptc->qtm_mb + ATMEL_QTM_MB_SCROLLER_CONFIG_OFFSET \
> +	       + (id) * sizeof(struct atmel_qtm_scroller_config), \
> +	       sizeof(struct atmel_qtm_scroller_config))

Macros are usually capitals. This also looks like obfuscation...

> +
> +
> +#define atmel_qtm_get_scroller_data(buf, id) \
> +	memcpy(buf, \
> +	       ptc->qtm_mb + ATMEL_QTM_MB_SCROLLER_DATA_OFFSET \
> +	       + (id) * sizeof(struct atmel_qtm_scroller_data), \
> +	       sizeof(struct atmel_qtm_scroller_data))
> +
> +#define get_scroller_resolution(scroller_config) \
> +	(1 << (scroller_config.resol_deadband >> 4))
> +
> +struct atmel_qtm_cmd {
> +	u16	id;
> +	u16	addr;
> +	u32	data;

I think these (and others where you talk to the hardware) need
endianness annotation (__le16/__be16, __le32/__be32).

> +} __packed;
> +
> +struct atmel_qtm_node_group_config {
> +	u16	count;
> +	u8	ptc_type;
> +	u8	freq_option;
> +	u8	calib_option;
> +	u8	unused;
> +} __packed;
> +
> +struct atmel_qtm_scroller_config {
> +	u8	type;
> +	u8	unused;
> +	u16	key_start;
> +	u8	key_count;
> +	u8	resol_deadband;
> +	u8	position_hysteresis;
> +	u8	unused2;
> +	u16	contact_min_threshold;
> +} __packed;
> +
> +struct atmel_qtm_scroller_data {
> +	u8	status;
> +	u8	right_hyst;
> +	u8	left_hyst;
> +	u8	unused;
> +	u16	raw_position;
> +	u16	position;
> +	u16	contact_size;
> +} __packed;
> +
> +struct atmel_qtm_touch_events {
> +	u32	key_event_id[2];
> +	u32	key_enable_state[2];
> +	u32	scroller_event_id;
> +	u32	scroller_event_state;
> +} __packed;
> +
> +struct atmel_ptc {
> +	void __iomem		*ppp_regs;
> +	void __iomem		*firmware;
> +	int			irq;
> +	u8			imr;
> +	void __iomem		*qtm_mb;
> +	struct clk		*clk_per;
> +	struct clk		*clk_int_osc;
> +	struct clk		*clk_slow;
> +	struct device		*dev;
> +	struct completion	ppp_ack;
> +	unsigned int		button_keycode[ATMEL_PTC_MAX_NODES];
> +	struct input_dev	*buttons_input;
> +	struct input_dev	*scroller_input[ATMEL_PTC_MAX_SCROLLERS];
> +	bool			buttons_registered;
> +	bool			scroller_registered[ATMEL_PTC_MAX_SCROLLERS];
> +	u32			button_event[ATMEL_PTC_MAX_NODES / 32];
> +	u32			button_state[ATMEL_PTC_MAX_NODES / 32];
> +	u32			scroller_event;
> +	u32			scroller_state;
> +};
> +
> +static void atmel_ppp_irq_enable(struct atmel_ptc *ptc, u8 mask)
> +{
> +	ptc->imr |= mask;
> +	atmel_ppp_writeb(ptc, ATMEL_PPP_IER, mask & ATMEL_PPP_IRQ_MASK);
> +}
> +
> +static void atmel_ppp_irq_disable(struct atmel_ptc *ptc, u8 mask)
> +{
> +	ptc->imr &= ~mask;
> +	atmel_ppp_writeb(ptc, ATMEL_PPP_IDR, mask & ATMEL_PPP_IRQ_MASK);
> +}
> +
> +static void atmel_ppp_notify(struct atmel_ptc *ptc, u8 mask)
> +{
> +	if (mask & ATMEL_PPP_NOTIFY_MASK) {
> +		u8 notify = atmel_ppp_readb(ptc, ATMEL_PPP_ISR)
> +			| (mask & ATMEL_PPP_NOTIFY_MASK);
> +
> +		atmel_ppp_writeb(ptc, ATMEL_PPP_ISR, notify);
> +	}
> +}
> +
> +static void atmel_ppp_irq_pending_clr(struct atmel_ptc *ptc, u8 mask)
> +{
> +	if (mask & ATMEL_PPP_IRQ_MASK) {
> +		u8 irq = atmel_ppp_readb(ptc, ATMEL_PPP_ISR) & ~mask;
> +
> +		atmel_ppp_writeb(ptc, ATMEL_PPP_ISR, irq);
> +	}
> +}
> +
> +static void atmel_ppp_cmd_send(struct atmel_ptc *ptc, u32 cmd)
> +{
> +	atmel_ppp_writel(ptc, ATMEL_PPP_CMD, cmd);
> +}
> +
> +static void atmel_ppp_irq_scroller_event(struct atmel_ptc *ptc)
> +{
> +	int i;
> +
> +	if (!ptc->scroller_event)
> +		return;
> +
> +	for (i = 0; i < ATMEL_PTC_MAX_SCROLLERS; i++) {
> +		u32 mask = 1 << i;

BIT(i)

But I wonder if you can't use for_each_set_bit() here.

> +		struct atmel_qtm_scroller_data scroller_data;
> +		struct atmel_qtm_scroller_config scroller_config;
> +
> +		if (!(ptc->scroller_event & mask))
> +			continue;
> +
> +		atmel_qtm_get_scroller_data(&scroller_data, i);
> +		atmel_qtm_get_scroller_config(&scroller_config, i);

This ends up copying memory on each interrupt. Can we look up in the
original buffer?

> +
> +		if (scroller_config.type == ATMEL_QTM_SCROLLER_TYPE_WHEEL)
> +			input_report_abs(ptc->scroller_input[i],
> +					 ABS_WHEEL, scroller_data.position);

What endianness "position" is in?

> +		else
> +			input_report_abs(ptc->scroller_input[i],
> +					 ABS_X, scroller_data.position);
> +
> +		input_report_key(ptc->scroller_input[i], BTN_TOUCH,
> +				 scroller_data.status & 0x1);
> +		input_sync(ptc->scroller_input[i]);
> +	}
> +}
> +
> +static void atmel_ppp_irq_button_event(struct atmel_ptc *ptc)
> +{
> +	int i, j;
> +
> +	for (i = 0; i < ATMEL_PTC_MAX_NODES / 32; i++) {
> +		if (!ptc->button_event[i])
> +			continue;
> +
> +		for (j = 0; j < 32; j++) {
> +			u32 mask = 1 << j;
> +			u32 state = ptc->button_state[i] & mask;
> +			unsigned int key_id = i * 32 + j;

I think you want to flatten these 2 loops with for_each_set_bit() as
well.

> +
> +			if (!(ptc->button_event[i] & mask))
> +				continue;
> +
> +			input_report_key(ptc->buttons_input,
> +					 ptc->button_keycode[key_id], !!state);
> +			input_sync(ptc->buttons_input);
> +		}
> +	}
> +}
> +
> +static void atmel_ppp_irq_touch_event(struct atmel_ptc *ptc)
> +{
> +	atmel_ppp_irq_scroller_event(ptc);
> +	atmel_ppp_irq_button_event(ptc);
> +}
> +
> +static irqreturn_t atmel_ppp_irq_handler(int irq, void *data)
> +{
> +	struct atmel_ptc *ptc = data;
> +	u32 isr = atmel_ppp_readb(ptc, ATMEL_PPP_ISR) & ptc->imr;
> +
> +	/* QTM CMD acknowledgment */
> +	if (isr & ATMEL_PPP_IRQ0) {
> +		atmel_ppp_irq_disable(ptc, ATMEL_PPP_IRQ0);
> +		atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ0);
> +		complete(&ptc->ppp_ack);
> +	}
> +	/* QTM touch event */
> +	if (isr & ATMEL_PPP_IRQ1) {
> +		struct atmel_qtm_touch_events touch_events;
> +		int i;
> +
> +		memcpy(&touch_events,
> +		       ptc->qtm_mb + ATMEL_QTM_MB_TOUCH_EVENTS_OFFSET,
> +		       sizeof(touch_events));

Did you mean memcpy_fromio() here?

> +
> +		for (i = 0; i < ATMEL_PTC_MAX_NODES / 32; i++) {
> +			ptc->button_event[i] = touch_events.key_event_id[i];
> +			ptc->button_state[i] = touch_events.key_enable_state[i];
> +		}
> +		ptc->scroller_event = touch_events.scroller_event_id;
> +		ptc->scroller_state = touch_events.scroller_event_state;
> +
> +		atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ1);
> +
> +		atmel_ppp_irq_touch_event(ptc);
> +	}
> +	/* Debug event */
> +	if (isr & ATMEL_PPP_IRQ2)
> +		atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ2);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +void atmel_qtm_cmd_send(struct atmel_ptc *ptc, struct atmel_qtm_cmd *cmd)
> +{
> +	int i, ret;
> +
> +	dev_dbg(ptc->dev, "%s: cmd=0x%x, addr=0x%x, data=0x%x\n",
> +		__func__, cmd->id, cmd->addr, cmd->data);
> +
> +	memcpy(ptc->qtm_mb, cmd, sizeof(*cmd));

memcpy_toio()? It looks like it is needed elsewhere as well.

> +
> +	/* Once command performed, we'll get an IRQ. */
> +	atmel_ppp_irq_enable(ptc, ATMEL_PPP_IRQ0);
> +	/* Notify PPP that we have sent a command. */
> +	atmel_ppp_notify(ptc, ATMEL_PPP_NOTIFY0);
> +	/* Wait for IRQ from PPP. */
> +	wait_for_completion(&ptc->ppp_ack);
> +
> +	/*
> +	 * Register input devices only when QTM is started since we need some
> +	 * information from the QTM configuration.
> +	 */

This is a bit weird... If we have configuration in device tree, what
else do we need to do here? And if we need dynamic config anyway, then
maybe we should not be using device tree?

> +	if (cmd->id == ATMEL_QTM_CMD_RUN) {
> +		if (ptc->buttons_input && !ptc->buttons_registered) {
> +			ret = input_register_device(ptc->buttons_input);
> +			if (ret)
> +				dev_err(ptc->dev, "can't register input button device.\n");
> +			else
> +				ptc->buttons_registered = true;
> +		}
> +
> +		for (i = 0; i < ATMEL_PTC_MAX_SCROLLERS; i++) {
> +			struct input_dev *scroller = ptc->scroller_input[i];
> +			struct atmel_qtm_scroller_config scroller_config;
> +
> +			if (!scroller || ptc->scroller_registered[i])
> +				continue;
> +
> +			atmel_qtm_get_scroller_config(&scroller_config, i);
> +
> +			if (scroller_config.type ==
> +			    ATMEL_QTM_SCROLLER_TYPE_SLIDER) {
> +				unsigned int max = get_scroller_resolution(scroller_config);
> +
> +				input_set_abs_params(scroller, 0, 0, max, 0, 0);
> +			}
> +			ret = input_register_device(scroller);
> +			if (ret)
> +				dev_err(ptc->dev, "can't register input scroller device.\n");
> +			else
> +				ptc->scroller_registered[i] = true;
> +		}
> +	}
> +
> +	memcpy(cmd, ptc->qtm_mb, sizeof(*cmd));
> +}
> +
> +static inline struct atmel_ptc *kobj_to_atmel_ptc(struct kobject *kobj)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +
> +	return dev->driver_data;
> +}
> +
> +static ssize_t atmel_qtm_mb_read(struct file *filp, struct kobject *kobj,
> +				 struct bin_attribute *attr,
> +				 char *buf, loff_t off, size_t count)
> +{
> +	struct atmel_ptc *ptc = kobj_to_atmel_ptc(kobj);
> +	char *qtm_mb = (char *)ptc->qtm_mb;
> +
> +	dev_dbg(ptc->dev, "%s: off=0x%llx, count=%zu\n", __func__, off, count);
> +
> +	memcpy(buf, qtm_mb + off, count);
> +
> +	return count;
> +}
> +
> +static ssize_t atmel_qtm_mb_write(struct file *filp, struct kobject *kobj,
> +				  struct bin_attribute *attr,
> +				  char *buf, loff_t off, size_t count)
> +{
> +	struct atmel_ptc *ptc = kobj_to_atmel_ptc(kobj);
> +	char *qtm_mb = (char *)ptc->qtm_mb;
> +
> +	dev_dbg(ptc->dev, "%s: off=0x%llx, count=%zu\n", __func__, off, count);
> +
> +	if (off == 0 && count == sizeof(struct atmel_qtm_cmd))
> +		atmel_qtm_cmd_send(ptc, (struct atmel_qtm_cmd *)buf);
> +	else
> +		memcpy(qtm_mb + off, buf, count);
> +
> +	return count;
> +}
> +
> +static struct bin_attribute atmel_ptc_qtm_mb_attr = {
> +	.attr = {
> +		.name = "qtm_mb",
> +		.mode = 0644,
> +	},
> +	.size = ATMEL_QTM_MB_SIZE,
> +	.read = atmel_qtm_mb_read,
> +	.write = atmel_qtm_mb_write,
> +};
> +
> +/*
> + * From QTM MB configuration, we can't retrieve all the information needed
> + * to setup correctly input devices: buttons key codes and slider axis are
> + * missing.
> + */
> +static int atmel_ptc_of_parse(struct atmel_ptc *ptc)
> +{
> +	struct device_node *sensor;
> +	bool first_button = true;
> +
> +	/* Parse sensors. */
> +	for_each_child_of_node(ptc->dev->of_node, sensor) {
> +		if (!strcmp(sensor->name, "button")) {
> +			u32 key_id, keycode;
> +			struct input_dev *buttons = ptc->buttons_input;
> +
> +			if (of_property_read_u32(sensor, "reg", &key_id)) {
> +				dev_err(ptc->dev, "reg is missing (%s)\n",
> +					sensor->full_name);
> +				return -EINVAL;
> +			}
> +
> +			if (of_property_read_u32(sensor, "linux,keycode", &keycode)) {
> +				dev_err(ptc->dev, "linux,keycode is missing (%s)\n",
> +					sensor->full_name);
> +				return -EINVAL;
> +			}
> +			ptc->button_keycode[key_id] = keycode;
> +
> +			/* All buttons are put together in a keyboard device. */
> +			if (first_button) {
> +				buttons = devm_input_allocate_device(ptc->dev);
> +				if (!buttons)
> +					return -ENOMEM;
> +				buttons->name = "atmel_ptc_buttons";
> +				buttons->dev.parent = ptc->dev;
> +				buttons->keycode = ptc->button_keycode;
> +				buttons->keycodesize = sizeof(ptc->button_keycode[0]);
> +				buttons->keycodemax = ATMEL_PTC_MAX_NODES;
> +				ptc->buttons_input = buttons;
> +				first_button = false;
> +			}
> +
> +			input_set_capability(buttons, EV_KEY, keycode);
> +		} else if (!strcmp(sensor->name, "slider") ||
> +			   !strcmp(sensor->name, "wheel")) {
> +			u32 scroller_id;
> +			struct input_dev *scroller;
> +
> +			if (of_property_read_u32(sensor, "reg", &scroller_id)) {
> +				dev_err(ptc->dev, "reg is missing (%s)\n",
> +					sensor->full_name);
> +				return -EINVAL;
> +			}
> +
> +			if (scroller_id > ATMEL_PTC_MAX_SCROLLERS - 1) {
> +				dev_err(ptc->dev, "wrong scroller id (%s)\n",
> +					sensor->full_name);
> +				return -EINVAL;
> +			}
> +
> +			scroller = devm_input_allocate_device(ptc->dev);
> +			if (!scroller)
> +				return -ENOMEM;
> +
> +			scroller->dev.parent = ptc->dev;
> +			ptc->scroller_input[scroller_id] = scroller;
> +
> +			if (!strcmp(sensor->name, "slider")) {
> +				scroller->name = "atmel_ptc_slider";
> +				input_set_capability(scroller, EV_ABS, ABS_X);
> +				input_set_capability(scroller, EV_KEY, BTN_TOUCH);
> +			} else {
> +				scroller->name = "atmel_ptc_wheel";
> +				input_set_capability(scroller, EV_ABS, ABS_WHEEL);
> +				input_set_capability(scroller, EV_KEY, BTN_TOUCH);
> +			}
> +		} else {
> +			dev_err(ptc->dev, "%s is not supported\n", sensor->name);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void atmel_qtm_conf_callback(const struct firmware *conf, void *context)
> +{
> +	struct atmel_ptc *ptc = context;
> +	struct atmel_qtm_cmd qtm_cmd;
> +	char *dst;
> +	struct atmel_qtm_node_group_config node_group_config;
> +
> +	if (!conf) {
> +		dev_err(ptc->dev, "cannot load QTM configuration, it has to be set manually.\n");
> +		return;
> +	}
> +
> +	atmel_ppp_irq_enable(ptc, ATMEL_PPP_IRQ1);
> +	atmel_ppp_irq_disable(ptc, ATMEL_PPP_IRQ2 | ATMEL_PPP_IRQ3);
> +
> +	qtm_cmd.id = ATMEL_QTM_CMD_STOP;
> +	atmel_qtm_cmd_send(ptc, &qtm_cmd);
> +
> +	/* Load QTM configuration. */
> +	dst = (char *)ptc->qtm_mb + ATMEL_QTM_MB_NODE_GROUP_CONFIG_OFFSET;
> +	/* memcpy doesn't work for an unknown reason. */
> +	_memcpy_toio(dst, conf->data, conf->size);
> +	release_firmware(conf);
> +
> +	if (atmel_ptc_of_parse(ptc))
> +		dev_err(ptc->dev, "ptc_of_parse failed\n");
> +
> +	memcpy(&node_group_config,
> +	       ptc->qtm_mb + ATMEL_QTM_MB_NODE_GROUP_CONFIG_OFFSET,
> +	       sizeof(node_group_config));
> +
> +	/* Start QTM. */
> +	qtm_cmd.id = ATMEL_QTM_CMD_INIT;
> +	qtm_cmd.data = node_group_config.count;
> +	atmel_qtm_cmd_send(ptc, &qtm_cmd);
> +	qtm_cmd.id = ATMEL_QTM_CMD_SET_ACQ_MODE_TIMER;
> +	qtm_cmd.data = 20;
> +	atmel_qtm_cmd_send(ptc, &qtm_cmd);
> +	qtm_cmd.id = ATMEL_QTM_CMD_RUN;
> +	qtm_cmd.data = node_group_config.count;
> +	atmel_qtm_cmd_send(ptc, &qtm_cmd);
> +}
> +
> +static void atmel_ppp_fw_callback(const struct firmware *fw, void *context)
> +{
> +	struct atmel_ptc *ptc = context;
> +	int ret;
> +	struct atmel_qtm_cmd cmd;
> +
> +	if (!fw || !fw->size) {
> +		dev_err(ptc->dev, "cannot load firmware.\n");
> +		release_firmware(fw);
> +		device_release_driver(ptc->dev);
> +		return;
> +	}
> +
> +	/* Command sequence to start from a clean state. */
> +	atmel_ppp_cmd_send(ptc, ATMEL_PPP_CMD_ABORT);
> +	atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ_MASK);
> +	atmel_ppp_cmd_send(ptc, ATMEL_PPP_CMD_RESET);
> +
> +	memcpy(ptc->firmware, fw->data, fw->size);
> +	release_firmware(fw);
> +
> +	atmel_ppp_cmd_send(ptc, ATMEL_PPP_CMD_RUN);
> +
> +	cmd.id = ATMEL_QTM_CMD_FIRM_VERSION;
> +	atmel_qtm_cmd_send(ptc, &cmd);
> +	dev_info(ptc->dev, "firmware version: %u\n", cmd.data);

dev_dbg().

> +
> +	/* PPP is running, it's time to load the QTM configuration. */
> +	ret = request_firmware_nowait(THIS_MODULE, 1, ATMEL_QTM_CONF_NAME, ptc->dev,
> +				      GFP_KERNEL, ptc, atmel_qtm_conf_callback);
> +	if (ret)
> +		dev_err(ptc->dev, "QTM configuration loading failed.\n");
> +}
> +
> +static int atmel_ptc_probe(struct platform_device *pdev)
> +{
> +	struct atmel_ptc *ptc;
> +	struct resource	*res;
> +	void *shared_memory;
> +	int ret;
> +
> +	ptc = devm_kzalloc(&pdev->dev, sizeof(*ptc), GFP_KERNEL);
> +	if (!ptc)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, ptc);
> +	ptc->dev = &pdev->dev;
> +	ptc->dev->driver_data = ptc;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +
> +	shared_memory = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(shared_memory))
> +		return PTR_ERR(shared_memory);
> +
> +	ptc->firmware = shared_memory;
> +	ptc->qtm_mb = shared_memory + ATMEL_QTM_MB_OFFSET;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (!res)
> +		return -EINVAL;
> +
> +	ptc->ppp_regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(ptc->ppp_regs))
> +		return PTR_ERR(ptc->ppp_regs);
> +
> +	ptc->irq = platform_get_irq(pdev, 0);
> +	if (ptc->irq <= 0) {
> +		if (!ptc->irq)
> +			ptc->irq = -ENXIO;
> +
> +		return ptc->irq;
> +	}
> +
> +	ptc->clk_per = devm_clk_get(&pdev->dev, "ptc_clk");
> +	if (IS_ERR(ptc->clk_per))
> +		return PTR_ERR(ptc->clk_per);
> +
> +	ptc->clk_int_osc = devm_clk_get(&pdev->dev, "ptc_int_osc");
> +	if (IS_ERR(ptc->clk_int_osc))
> +		return PTR_ERR(ptc->clk_int_osc);
> +
> +	ptc->clk_slow = devm_clk_get(&pdev->dev, "slow_clk");
> +	if (IS_ERR(ptc->clk_slow))
> +		return PTR_ERR(ptc->clk_slow);
> +
> +	ret = devm_request_irq(&pdev->dev, ptc->irq, atmel_ppp_irq_handler, 0,
> +			       pdev->dev.driver->name, ptc);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(ptc->clk_int_osc);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(ptc->clk_per);
> +	if (ret)
> +		goto disable_clk_int_osc;
> +
> +	ret = clk_prepare_enable(ptc->clk_slow);
> +	if (ret)
> +		goto disable_clk_per;

You really want to enable clocks before you request IRQ. And since you
mixing devm and non-devm APIs you need to be extra careful on teardown
path to preserve order of releasing resources.
devm_add_action_or_reset() is your friend here.

Is your IRQ routine ready to be fired even though not all device
structures are allocated/initialized?

> +
> +	/* Needed to avoid unexpected behaviors. */
> +	memset(ptc->firmware, 0, ATMEL_QTM_MB_OFFSET + sizeof(*ptc->qtm_mb));
> +	ptc->imr = 0;
> +	init_completion(&ptc->ppp_ack);
> +
> +	/*
> +	 * Expose a file to give an access to the QTM mailbox to a user space
> +	 * application in order to configure it or to send commands.
> +	 */
> +	ret = sysfs_create_bin_file(&pdev->dev.kobj, &atmel_ptc_qtm_mb_attr);
> +	if (ret)
> +		goto disable_clk_slow;

devm_device_add_group()?

> +
> +	ret = request_firmware_nowait(THIS_MODULE, 1, ATMEL_PPP_FIRMWARE_NAME,
> +				      ptc->dev, GFP_KERNEL, ptc,
> +				      atmel_ppp_fw_callback);
> +	if (ret) {
> +		dev_err(&pdev->dev, "firmware loading failed\n");
> +		ret = -EPROBE_DEFER;

Always defer? That is optimistic.

> +		goto remove_qtm_mb;
> +	}
> +
> +	return 0;
> +
> +remove_qtm_mb:
> +	sysfs_remove_bin_file(&pdev->dev.kobj, &atmel_ptc_qtm_mb_attr);
> +disable_clk_slow:
> +	clk_disable_unprepare(ptc->clk_slow);
> +disable_clk_per:
> +	clk_disable_unprepare(ptc->clk_per);
> +disable_clk_int_osc:
> +	clk_disable_unprepare(ptc->clk_int_osc);
> +
> +	return ret;
> +}
> +
> +static int atmel_ptc_remove(struct platform_device *pdev)
> +{
> +	struct atmel_ptc *ptc = platform_get_drvdata(pdev);
> +	int i;
> +

You need a completion in your fimrware loading callback and wait for it
here, otherwise quick bind/unbind will crash the kernel.


> +	if (ptc->buttons_registered)
> +		input_unregister_device(ptc->buttons_input);
> +
> +	for (i = 0; i < ATMEL_PTC_MAX_SCROLLERS; i++) {
> +		struct input_dev *scroller = ptc->scroller_input[i];
> +
> +		if (!scroller || !ptc->scroller_registered[i])
> +			continue;
> +		input_unregister_device(scroller);
> +	}
> +
> +	sysfs_remove_bin_file(&pdev->dev.kobj, &atmel_ptc_qtm_mb_attr);
> +	clk_disable_unprepare(ptc->clk_slow);
> +	clk_disable_unprepare(ptc->clk_per);
> +	clk_disable_unprepare(ptc->clk_int_osc);
> +
> +	return 0;
> +}
> +

#ifdef CONFIG_OF
> +static const struct of_device_id atmel_ptc_dt_match[] = {
> +	{
> +		.compatible = "atmel,sama5d2-ptc",
> +	}, {
> +		/* sentinel */
> +	}
> +};
> +MODULE_DEVICE_TABLE(of, atmel_ptc_dt_match);
#endif

> +
> +static struct platform_driver atmel_ptc_driver = {
> +	.probe = atmel_ptc_probe,
> +	.remove = atmel_ptc_remove,
> +	.driver = {
> +		.name = "atmel_ptc",
> +		.of_match_table = atmel_ptc_dt_match,

of_match_ptr() - you are allowing it to be compiled without OF.

> +	},
> +};
> +module_platform_driver(atmel_ptc_driver)
> +
> +MODULE_AUTHOR("Ludovic Desroches <ludovic.desroches@microchip.com>");
> +MODULE_DESCRIPTION("Atmel PTC subsystem");
> +MODULE_LICENSE("GPL v2");
> +MODULE_FIRMWARE(ATMEL_PPP_FIRMWARE_NAME);
> +MODULE_FIRMWARE(ATMEL_QTM_CONF_NAME);
> -- 
> 2.12.2
> 

Thanks.

-- 
Dmitry

Re: [PATCH v3 1/5] dt-bindings: input: Add Atmel PTC subsystem bindings

[Ludovic Desroches]

Hi Dmitry,

On Mon, Oct 30, 2017 at 04:06:58PM -0700, Dmitry Torokhov wrote:
> Hi Ludovic,
> 
> On Fri, Oct 20, 2017 at 03:31:17PM +0200, Ludovic Desroches wrote:
> > Add description of the Atmel PTC subsystem bindings.
> > 
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> > Acked-by: Rob Herring <robh@kernel.org>
> > ---
> >  .../devicetree/bindings/input/atmel,ptc.txt        | 67 ++++++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/atmel,ptc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/input/atmel,ptc.txt b/Documentation/devicetree/bindings/input/atmel,ptc.txt
> > new file mode 100644
> > index 000000000000..a183fd511e04
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/atmel,ptc.txt
> > @@ -0,0 +1,67 @@
> > +Atmel PTC Subsystem
> > +
> > +The Atmel Peripheral Touch Controller subsystem offers built-in hardware
> > +for capacitive touch measurement on sensors that function as buttons, sliders
> > +and wheels.
> > +
> > +1) PTC Subsystem node
> > +
> > +Required properties:
> > +- compatible: 		Must be "atmel,sama5d2-ptc"
> > +- reg: 			Address, length of the shared memory and ppp registers location
> > +			and length.
> > +- clocks: 		Phandlers to the clocks.
> > +- clock-names: 		Must be "ptc_clk", "ptc_int_osc", "slow_clk".
> > +- #address-cells:	Must be one. The cell is the button or scroller id.
> > +- #size-cells: 		Must be zero.
> > +
> > +Example:
> > +	ptc@fc060000 {
> > +		compatible = "atmel,sama5d2-ptc";
> > +		reg = <0x00800000 0x10000
> > +		       0xfc060000 0xcf>;
> > +		interrupts = <58 IRQ_TYPE_LEVEL_HIGH 7>;
> > +		clocks = <&ptc_clk>, <&main>, <&clk32k>;
> > +		clock-names = "ptc_clk", "ptc_int_osc", "slow_clk";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		[ child node definitions... ]
> > +	};
> > +
> > +2) Scroller / buttons subnodes
> > +
> > +Subnodes describe the kind of sensors the customer want to use. They have to be
> > +named according to their function: button, slider or wheel.
> 
> I wonder do we really need this? Or we can have a generic:
> 
> 	slider@0 {
> 		reg = <0>;
> 		linux,type = <EV_ABS>;
> 		linux,code = <ABS_X>;
> 	},
> 	vertical-slider@1 {
> 		reg = <1>;
> 		linux,type = <EV_ABS>;
> 		linux,code = <ABS_Y>;
> 	},
> 	wheel@2 {
> 		reg = <2>;
> 		linux,type = <EV_ABS>;
> 		linux,code = <ABS_WHEEL>;
> 	},
> 	button@3 {
> 		reg = <3>;
> 		linux,type = <EV_KEY>;
> 		linux,code = <KEY_A>;
> 	},
> 	...
> 
> I.e. you specify type/code in a generic way.

No problem with this approach, I can retrieve the kind of sensors from
the linux,type property.

Can I set both ABS_X and ABS_Y? In the future, we may have a "surface"
object reporting coordinates for these axes. It seems there are no
generic helpers to parse the 'linux,code' property, so I assume it won't be a
problem.

Regards

Ludovic

Re: [PATCH v3 2/5] input: misc: introduce Atmel PTC driver

[Ludovic Desroches]

Hi Dmitry,

On Mon, Oct 30, 2017 at 04:30:41PM -0700, Dmitry Torokhov wrote:
> Hi Ludovic,
> 
> On Fri, Oct 20, 2017 at 03:31:18PM +0200, Ludovic Desroches wrote:
> > The Atmel Peripheral Touch Controller subsystem offers built-in hardware
> > for capacitive touch measurement on sensors that function as buttons,
> > sliders and wheels.
> > 
> > Two files are loaded when probing the driver:
> > - a firmware for the Pico Power Processor that computes raw data from
> >   the ADC front end to provide high level information as button touch or
> >   untouch, slider position, etc.
> > - a configuration file that describe the topology and the parameters of
> >   the sensors.
> 
> If I understand it correctly the configuration is loaded via a cha
> 

Sorry, what's a cha?

> > 
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> > ---
> >  drivers/input/misc/Kconfig     |  12 +
> >  drivers/input/misc/Makefile    |   1 +
> >  drivers/input/misc/atmel_ptc.c | 723 +++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 736 insertions(+)
> >  create mode 100644 drivers/input/misc/atmel_ptc.c
> > 
> > diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> > index 9f082a388388..76616b0425b5 100644
> > --- a/drivers/input/misc/Kconfig
> > +++ b/drivers/input/misc/Kconfig
> > @@ -96,6 +96,18 @@ config INPUT_ATMEL_CAPTOUCH
> >  	  To compile this driver as a module, choose M here: the
> >  	  module will be called atmel_captouch.
> >  
> > +config INPUT_ATMEL_PTC
> > +	tristate "Atmel PTC Driver"
> > +	depends on OF || COMPILE_TEST
> > +	depends on SOC_SAMA5D2
> 
> Maybe "default SOC_SAMA5D2"?
> 
> 

I don't think it's a peripheral which will be used often.

> > +	help
> > +	  Say Y to enable support for the Atmel PTC Subsystem.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called atmel_ptc.
> > +	  If you compile it as a built-in driver, you have to build the
> > +	  firmware into the kernel or to use an initrd.
> > +
> >  config INPUT_BMA150
> >  	tristate "BMA150/SMB380 acceleration sensor support"
> >  	depends on I2C
> > diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> > index 03fd4262ada9..f71972adc002 100644
> > --- a/drivers/input/misc/Makefile
> > +++ b/drivers/input/misc/Makefile
> > @@ -18,6 +18,7 @@ obj-$(CONFIG_INPUT_ARIZONA_HAPTICS)	+= arizona-haptics.o
> >  obj-$(CONFIG_INPUT_ATI_REMOTE2)		+= ati_remote2.o
> >  obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
> >  obj-$(CONFIG_INPUT_ATMEL_CAPTOUCH)	+= atmel_captouch.o
> > +obj-$(CONFIG_INPUT_ATMEL_PTC)		+= atmel_ptc.o
> >  obj-$(CONFIG_INPUT_BFIN_ROTARY)		+= bfin_rotary.o
> >  obj-$(CONFIG_INPUT_BMA150)		+= bma150.o
> >  obj-$(CONFIG_INPUT_CM109)		+= cm109.o
> > diff --git a/drivers/input/misc/atmel_ptc.c b/drivers/input/misc/atmel_ptc.c
> > new file mode 100644
> > index 000000000000..612eaede6072
> > --- /dev/null
> > +++ b/drivers/input/misc/atmel_ptc.c
> > @@ -0,0 +1,723 @@
> > +/*
> > + * Atmel PTC subsystem driver for SAMA5D2 devices and compatible.
> > + *
> > + * Copyright (C) 2017 Microchip,
> > + *               2017 Ludovic Desroches <ludovic.desroches@microchip.com>
> > + *
> > + * This software is licensed under the terms of the GNU General Public
> > + * License version 2, as published by the Free Software Foundation, and
> > + * may be copied, distributed, and modified under those terms.
> > + *
> > + * 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/cdev.h>
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/firmware.h>
> > +#include <linux/input.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +#include <linux/uaccess.h>
> > +
> > +#define ATMEL_PTC_MAX_NODES	64
> > +#define ATMEL_PTC_MAX_SCROLLERS	4
> > +
> > +/* ----- PPP ----- */
> > +#define ATMEL_PPP_FIRMWARE_NAME	"atmel_ptc.bin"
> > +
> > +#define ATMEL_PPP_CONFIG	0x20
> > +#define ATMEL_PPP_CTRL		0x24
> > +#define ATMEL_PPP_CMD		0x28
> > +#define		ATMEL_PPP_CMD_STOP		0x1
> > +#define		ATMEL_PPP_CMD_RESET		0x2
> > +#define		ATMEL_PPP_CMD_RESTART		0x3
> > +#define		ATMEL_PPP_CMD_ABORT		0x4
> > +#define		ATMEL_PPP_CMD_RUN		0x5
> > +#define		ATMEL_PPP_CMD_RUN_LOCKED	0x6
> > +#define		ATMEL_PPP_CMD_RUN_OCD		0x7
> > +#define		ATMEL_PPP_CMD_UNLOCK		0x8
> > +#define		ATMEL_PPP_CMD_NMI		0x9
> > +#define		ATMEL_PPP_CMD_HOST_OCD_RESUME	0xB
> > +#define ATMEL_PPP_ISR		0x33
> > +#define		ATMEL_PPP_IRQ_MASK	GENMASK(7, 4)
> > +#define		ATMEL_PPP_IRQ0		BIT(4)
> > +#define		ATMEL_PPP_IRQ1		BIT(5)
> > +#define		ATMEL_PPP_IRQ2		BIT(6)
> > +#define		ATMEL_PPP_IRQ3		BIT(7)
> > +#define		ATMEL_PPP_NOTIFY_MASK	GENMASK(3, 0)
> > +#define		ATMEL_PPP_NOTIFY0	BIT(0)
> > +#define		ATMEL_PPP_NOTIFY1	BIT(1)
> > +#define		ATMEL_PPP_NOTIFY2	BIT(2)
> > +#define		ATMEL_PPP_NOTIFY3	BIT(3)
> > +#define ATMEL_PPP_IDR		0x34
> > +#define ATMEL_PPP_IER		0x35
> > +
> > +#define atmel_ppp_readb(ptc, reg)	readb_relaxed(ptc->ppp_regs + reg)
> > +#define atmel_ppp_writeb(ptc, reg, val)	writeb_relaxed(val, ptc->ppp_regs + reg)
> > +#define atmel_ppp_readl(ptc, reg)	readl_relaxed(ptc->ppp_regs + reg)
> > +#define atmel_ppp_writel(ptc, reg, val)	writel_relaxed(val, ptc->ppp_regs + reg)
> > +
> > +/* ----- QTM ----- */
> > +#define ATMEL_QTM_CONF_NAME		"atmel_ptc.conf"
> > +
> > +#define ATMEL_QTM_MB_OFFSET			0x4000
> > +#define ATMEL_QTM_MB_SIZE			0x1000
> > +
> > +#define ATMEL_QTM_MB_CMD_OFFSET			0x0
> > +#define		ATMEL_QTM_CMD_FIRM_VERSION		8
> > +#define		ATMEL_QTM_CMD_INIT			18
> > +#define		ATMEL_QTM_CMD_RUN			19
> > +#define		ATMEL_QTM_CMD_STOP			21
> > +#define		ATMEL_QTM_CMD_SET_ACQ_MODE_TIMER	24
> > +#define ATMEL_QTM_MB_NODE_GROUP_CONFIG_OFFSET	0x100
> > +#define ATMEL_QTM_MB_SCROLLER_CONFIG_OFFSET	0x81a
> > +#define		ATMEL_QTM_SCROLLER_TYPE_SLIDER		0x0
> > +#define		ATMEL_QTM_SCROLLER_TYPE_WHEEL		0x1
> > +#define ATMEL_QTM_MB_SCROLLER_DATA_OFFSET	0x842
> > +#define ATMEL_QTM_MB_TOUCH_EVENTS_OFFSET	0x880
> > +
> > +#define atmel_qtm_get_scroller_config(buf, id) \
> > +	memcpy(buf, \
> > +	       ptc->qtm_mb + ATMEL_QTM_MB_SCROLLER_CONFIG_OFFSET \
> > +	       + (id) * sizeof(struct atmel_qtm_scroller_config), \
> > +	       sizeof(struct atmel_qtm_scroller_config))
> 
> Macros are usually capitals. This also looks like obfuscation...
> 

I'll capitalize this macro and next ones.

> > +
> > +
> > +#define atmel_qtm_get_scroller_data(buf, id) \
> > +	memcpy(buf, \
> > +	       ptc->qtm_mb + ATMEL_QTM_MB_SCROLLER_DATA_OFFSET \
> > +	       + (id) * sizeof(struct atmel_qtm_scroller_data), \
> > +	       sizeof(struct atmel_qtm_scroller_data))
> > +
> > +#define get_scroller_resolution(scroller_config) \
> > +	(1 << (scroller_config.resol_deadband >> 4))
> > +
> > +struct atmel_qtm_cmd {
> > +	u16	id;
> > +	u16	addr;
> > +	u32	data;
> 
> I think these (and others where you talk to the hardware) need
> endianness annotation (__le16/__be16, __le32/__be32).
> 

Ok.

> > +} __packed;
> > +
> > +struct atmel_qtm_node_group_config {
> > +	u16	count;
> > +	u8	ptc_type;
> > +	u8	freq_option;
> > +	u8	calib_option;
> > +	u8	unused;
> > +} __packed;
> > +
> > +struct atmel_qtm_scroller_config {
> > +	u8	type;
> > +	u8	unused;
> > +	u16	key_start;
> > +	u8	key_count;
> > +	u8	resol_deadband;
> > +	u8	position_hysteresis;
> > +	u8	unused2;
> > +	u16	contact_min_threshold;
> > +} __packed;
> > +
> > +struct atmel_qtm_scroller_data {
> > +	u8	status;
> > +	u8	right_hyst;
> > +	u8	left_hyst;
> > +	u8	unused;
> > +	u16	raw_position;
> > +	u16	position;
> > +	u16	contact_size;
> > +} __packed;
> > +
> > +struct atmel_qtm_touch_events {
> > +	u32	key_event_id[2];
> > +	u32	key_enable_state[2];
> > +	u32	scroller_event_id;
> > +	u32	scroller_event_state;
> > +} __packed;
> > +
> > +struct atmel_ptc {
> > +	void __iomem		*ppp_regs;
> > +	void __iomem		*firmware;
> > +	int			irq;
> > +	u8			imr;
> > +	void __iomem		*qtm_mb;
> > +	struct clk		*clk_per;
> > +	struct clk		*clk_int_osc;
> > +	struct clk		*clk_slow;
> > +	struct device		*dev;
> > +	struct completion	ppp_ack;
> > +	unsigned int		button_keycode[ATMEL_PTC_MAX_NODES];
> > +	struct input_dev	*buttons_input;
> > +	struct input_dev	*scroller_input[ATMEL_PTC_MAX_SCROLLERS];
> > +	bool			buttons_registered;
> > +	bool			scroller_registered[ATMEL_PTC_MAX_SCROLLERS];
> > +	u32			button_event[ATMEL_PTC_MAX_NODES / 32];
> > +	u32			button_state[ATMEL_PTC_MAX_NODES / 32];
> > +	u32			scroller_event;
> > +	u32			scroller_state;
> > +};
> > +
> > +static void atmel_ppp_irq_enable(struct atmel_ptc *ptc, u8 mask)
> > +{
> > +	ptc->imr |= mask;
> > +	atmel_ppp_writeb(ptc, ATMEL_PPP_IER, mask & ATMEL_PPP_IRQ_MASK);
> > +}
> > +
> > +static void atmel_ppp_irq_disable(struct atmel_ptc *ptc, u8 mask)
> > +{
> > +	ptc->imr &= ~mask;
> > +	atmel_ppp_writeb(ptc, ATMEL_PPP_IDR, mask & ATMEL_PPP_IRQ_MASK);
> > +}
> > +
> > +static void atmel_ppp_notify(struct atmel_ptc *ptc, u8 mask)
> > +{
> > +	if (mask & ATMEL_PPP_NOTIFY_MASK) {
> > +		u8 notify = atmel_ppp_readb(ptc, ATMEL_PPP_ISR)
> > +			| (mask & ATMEL_PPP_NOTIFY_MASK);
> > +
> > +		atmel_ppp_writeb(ptc, ATMEL_PPP_ISR, notify);
> > +	}
> > +}
> > +
> > +static void atmel_ppp_irq_pending_clr(struct atmel_ptc *ptc, u8 mask)
> > +{
> > +	if (mask & ATMEL_PPP_IRQ_MASK) {
> > +		u8 irq = atmel_ppp_readb(ptc, ATMEL_PPP_ISR) & ~mask;
> > +
> > +		atmel_ppp_writeb(ptc, ATMEL_PPP_ISR, irq);
> > +	}
> > +}
> > +
> > +static void atmel_ppp_cmd_send(struct atmel_ptc *ptc, u32 cmd)
> > +{
> > +	atmel_ppp_writel(ptc, ATMEL_PPP_CMD, cmd);
> > +}
> > +
> > +static void atmel_ppp_irq_scroller_event(struct atmel_ptc *ptc)
> > +{
> > +	int i;
> > +
> > +	if (!ptc->scroller_event)
> > +		return;
> > +
> > +	for (i = 0; i < ATMEL_PTC_MAX_SCROLLERS; i++) {
> > +		u32 mask = 1 << i;
> 
> BIT(i)
> 
> But I wonder if you can't use for_each_set_bit() here.
> 

Probably, I'll have a look.

> > +		struct atmel_qtm_scroller_data scroller_data;
> > +		struct atmel_qtm_scroller_config scroller_config;
> > +
> > +		if (!(ptc->scroller_event & mask))
> > +			continue;
> > +
> > +		atmel_qtm_get_scroller_data(&scroller_data, i);
> > +		atmel_qtm_get_scroller_config(&scroller_config, i);
> 
> This ends up copying memory on each interrupt. Can we look up in the
> original buffer?
> 

Yes I can. I did this to get a snapshot and so consistent values within
those structures which may change in a continuous way.

> > +
> > +		if (scroller_config.type == ATMEL_QTM_SCROLLER_TYPE_WHEEL)
> > +			input_report_abs(ptc->scroller_input[i],
> > +					 ABS_WHEEL, scroller_data.position);
> 
> What endianness "position" is in?
> 

LE.

> > +		else
> > +			input_report_abs(ptc->scroller_input[i],
> > +					 ABS_X, scroller_data.position);
> > +
> > +		input_report_key(ptc->scroller_input[i], BTN_TOUCH,
> > +				 scroller_data.status & 0x1);
> > +		input_sync(ptc->scroller_input[i]);
> > +	}
> > +}
> > +
> > +static void atmel_ppp_irq_button_event(struct atmel_ptc *ptc)
> > +{
> > +	int i, j;
> > +
> > +	for (i = 0; i < ATMEL_PTC_MAX_NODES / 32; i++) {
> > +		if (!ptc->button_event[i])
> > +			continue;
> > +
> > +		for (j = 0; j < 32; j++) {
> > +			u32 mask = 1 << j;
> > +			u32 state = ptc->button_state[i] & mask;
> > +			unsigned int key_id = i * 32 + j;
> 
> I think you want to flatten these 2 loops with for_each_set_bit() as
> well.
> 

Yes.

> > +
> > +			if (!(ptc->button_event[i] & mask))
> > +				continue;
> > +
> > +			input_report_key(ptc->buttons_input,
> > +					 ptc->button_keycode[key_id], !!state);
> > +			input_sync(ptc->buttons_input);
> > +		}
> > +	}
> > +}
> > +
> > +static void atmel_ppp_irq_touch_event(struct atmel_ptc *ptc)
> > +{
> > +	atmel_ppp_irq_scroller_event(ptc);
> > +	atmel_ppp_irq_button_event(ptc);
> > +}
> > +
> > +static irqreturn_t atmel_ppp_irq_handler(int irq, void *data)
> > +{
> > +	struct atmel_ptc *ptc = data;
> > +	u32 isr = atmel_ppp_readb(ptc, ATMEL_PPP_ISR) & ptc->imr;
> > +
> > +	/* QTM CMD acknowledgment */
> > +	if (isr & ATMEL_PPP_IRQ0) {
> > +		atmel_ppp_irq_disable(ptc, ATMEL_PPP_IRQ0);
> > +		atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ0);
> > +		complete(&ptc->ppp_ack);
> > +	}
> > +	/* QTM touch event */
> > +	if (isr & ATMEL_PPP_IRQ1) {
> > +		struct atmel_qtm_touch_events touch_events;
> > +		int i;
> > +
> > +		memcpy(&touch_events,
> > +		       ptc->qtm_mb + ATMEL_QTM_MB_TOUCH_EVENTS_OFFSET,
> > +		       sizeof(touch_events));
> 
> Did you mean memcpy_fromio() here?
>

Probably, I have to checked. I remember having some weird behaviors with
first firmwares and memcpy, I tried several variants.

> > +
> > +		for (i = 0; i < ATMEL_PTC_MAX_NODES / 32; i++) {
> > +			ptc->button_event[i] = touch_events.key_event_id[i];
> > +			ptc->button_state[i] = touch_events.key_enable_state[i];
> > +		}
> > +		ptc->scroller_event = touch_events.scroller_event_id;
> > +		ptc->scroller_state = touch_events.scroller_event_state;
> > +
> > +		atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ1);
> > +
> > +		atmel_ppp_irq_touch_event(ptc);
> > +	}
> > +	/* Debug event */
> > +	if (isr & ATMEL_PPP_IRQ2)
> > +		atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ2);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +void atmel_qtm_cmd_send(struct atmel_ptc *ptc, struct atmel_qtm_cmd *cmd)
> > +{
> > +	int i, ret;
> > +
> > +	dev_dbg(ptc->dev, "%s: cmd=0x%x, addr=0x%x, data=0x%x\n",
> > +		__func__, cmd->id, cmd->addr, cmd->data);
> > +
> > +	memcpy(ptc->qtm_mb, cmd, sizeof(*cmd));
> 
> memcpy_toio()? It looks like it is needed elsewhere as well.
> 
> > +
> > +	/* Once command performed, we'll get an IRQ. */
> > +	atmel_ppp_irq_enable(ptc, ATMEL_PPP_IRQ0);
> > +	/* Notify PPP that we have sent a command. */
> > +	atmel_ppp_notify(ptc, ATMEL_PPP_NOTIFY0);
> > +	/* Wait for IRQ from PPP. */
> > +	wait_for_completion(&ptc->ppp_ack);
> > +
> > +	/*
> > +	 * Register input devices only when QTM is started since we need some
> > +	 * information from the QTM configuration.
> > +	 */
> 
> This is a bit weird... If we have configuration in device tree, what
> else do we need to do here? And if we need dynamic config anyway, then
> maybe we should not be using device tree?
> 

I agree. The resolution is not in the device tree, it will be redundant
with the configuration loaded.

I kept the DT part to get a link between the sensors and the code. If
there is another way to manage this, then I can rely only on the
configuration loaded.

> > +	if (cmd->id == ATMEL_QTM_CMD_RUN) {
> > +		if (ptc->buttons_input && !ptc->buttons_registered) {
> > +			ret = input_register_device(ptc->buttons_input);
> > +			if (ret)
> > +				dev_err(ptc->dev, "can't register input button device.\n");
> > +			else
> > +				ptc->buttons_registered = true;
> > +		}
> > +
> > +		for (i = 0; i < ATMEL_PTC_MAX_SCROLLERS; i++) {
> > +			struct input_dev *scroller = ptc->scroller_input[i];
> > +			struct atmel_qtm_scroller_config scroller_config;
> > +
> > +			if (!scroller || ptc->scroller_registered[i])
> > +				continue;
> > +
> > +			atmel_qtm_get_scroller_config(&scroller_config, i);
> > +
> > +			if (scroller_config.type ==
> > +			    ATMEL_QTM_SCROLLER_TYPE_SLIDER) {
> > +				unsigned int max = get_scroller_resolution(scroller_config);
> > +
> > +				input_set_abs_params(scroller, 0, 0, max, 0, 0);
> > +			}
> > +			ret = input_register_device(scroller);
> > +			if (ret)
> > +				dev_err(ptc->dev, "can't register input scroller device.\n");
> > +			else
> > +				ptc->scroller_registered[i] = true;
> > +		}
> > +	}
> > +
> > +	memcpy(cmd, ptc->qtm_mb, sizeof(*cmd));
> > +}
> > +
> > +static inline struct atmel_ptc *kobj_to_atmel_ptc(struct kobject *kobj)
> > +{
> > +	struct device *dev = kobj_to_dev(kobj);
> > +
> > +	return dev->driver_data;
> > +}
> > +
> > +static ssize_t atmel_qtm_mb_read(struct file *filp, struct kobject *kobj,
> > +				 struct bin_attribute *attr,
> > +				 char *buf, loff_t off, size_t count)
> > +{
> > +	struct atmel_ptc *ptc = kobj_to_atmel_ptc(kobj);
> > +	char *qtm_mb = (char *)ptc->qtm_mb;
> > +
> > +	dev_dbg(ptc->dev, "%s: off=0x%llx, count=%zu\n", __func__, off, count);
> > +
> > +	memcpy(buf, qtm_mb + off, count);
> > +
> > +	return count;
> > +}
> > +
> > +static ssize_t atmel_qtm_mb_write(struct file *filp, struct kobject *kobj,
> > +				  struct bin_attribute *attr,
> > +				  char *buf, loff_t off, size_t count)
> > +{
> > +	struct atmel_ptc *ptc = kobj_to_atmel_ptc(kobj);
> > +	char *qtm_mb = (char *)ptc->qtm_mb;
> > +
> > +	dev_dbg(ptc->dev, "%s: off=0x%llx, count=%zu\n", __func__, off, count);
> > +
> > +	if (off == 0 && count == sizeof(struct atmel_qtm_cmd))
> > +		atmel_qtm_cmd_send(ptc, (struct atmel_qtm_cmd *)buf);
> > +	else
> > +		memcpy(qtm_mb + off, buf, count);
> > +
> > +	return count;
> > +}
> > +
> > +static struct bin_attribute atmel_ptc_qtm_mb_attr = {
> > +	.attr = {
> > +		.name = "qtm_mb",
> > +		.mode = 0644,
> > +	},
> > +	.size = ATMEL_QTM_MB_SIZE,
> > +	.read = atmel_qtm_mb_read,
> > +	.write = atmel_qtm_mb_write,
> > +};
> > +
> > +/*
> > + * From QTM MB configuration, we can't retrieve all the information needed
> > + * to setup correctly input devices: buttons key codes and slider axis are
> > + * missing.
> > + */
> > +static int atmel_ptc_of_parse(struct atmel_ptc *ptc)
> > +{
> > +	struct device_node *sensor;
> > +	bool first_button = true;
> > +
> > +	/* Parse sensors. */
> > +	for_each_child_of_node(ptc->dev->of_node, sensor) {
> > +		if (!strcmp(sensor->name, "button")) {
> > +			u32 key_id, keycode;
> > +			struct input_dev *buttons = ptc->buttons_input;
> > +
> > +			if (of_property_read_u32(sensor, "reg", &key_id)) {
> > +				dev_err(ptc->dev, "reg is missing (%s)\n",
> > +					sensor->full_name);
> > +				return -EINVAL;
> > +			}
> > +
> > +			if (of_property_read_u32(sensor, "linux,keycode", &keycode)) {
> > +				dev_err(ptc->dev, "linux,keycode is missing (%s)\n",
> > +					sensor->full_name);
> > +				return -EINVAL;
> > +			}
> > +			ptc->button_keycode[key_id] = keycode;
> > +
> > +			/* All buttons are put together in a keyboard device. */
> > +			if (first_button) {
> > +				buttons = devm_input_allocate_device(ptc->dev);
> > +				if (!buttons)
> > +					return -ENOMEM;
> > +				buttons->name = "atmel_ptc_buttons";
> > +				buttons->dev.parent = ptc->dev;
> > +				buttons->keycode = ptc->button_keycode;
> > +				buttons->keycodesize = sizeof(ptc->button_keycode[0]);
> > +				buttons->keycodemax = ATMEL_PTC_MAX_NODES;
> > +				ptc->buttons_input = buttons;
> > +				first_button = false;
> > +			}
> > +
> > +			input_set_capability(buttons, EV_KEY, keycode);
> > +		} else if (!strcmp(sensor->name, "slider") ||
> > +			   !strcmp(sensor->name, "wheel")) {
> > +			u32 scroller_id;
> > +			struct input_dev *scroller;
> > +
> > +			if (of_property_read_u32(sensor, "reg", &scroller_id)) {
> > +				dev_err(ptc->dev, "reg is missing (%s)\n",
> > +					sensor->full_name);
> > +				return -EINVAL;
> > +			}
> > +
> > +			if (scroller_id > ATMEL_PTC_MAX_SCROLLERS - 1) {
> > +				dev_err(ptc->dev, "wrong scroller id (%s)\n",
> > +					sensor->full_name);
> > +				return -EINVAL;
> > +			}
> > +
> > +			scroller = devm_input_allocate_device(ptc->dev);
> > +			if (!scroller)
> > +				return -ENOMEM;
> > +
> > +			scroller->dev.parent = ptc->dev;
> > +			ptc->scroller_input[scroller_id] = scroller;
> > +
> > +			if (!strcmp(sensor->name, "slider")) {
> > +				scroller->name = "atmel_ptc_slider";
> > +				input_set_capability(scroller, EV_ABS, ABS_X);
> > +				input_set_capability(scroller, EV_KEY, BTN_TOUCH);
> > +			} else {
> > +				scroller->name = "atmel_ptc_wheel";
> > +				input_set_capability(scroller, EV_ABS, ABS_WHEEL);
> > +				input_set_capability(scroller, EV_KEY, BTN_TOUCH);
> > +			}
> > +		} else {
> > +			dev_err(ptc->dev, "%s is not supported\n", sensor->name);
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void atmel_qtm_conf_callback(const struct firmware *conf, void *context)
> > +{
> > +	struct atmel_ptc *ptc = context;
> > +	struct atmel_qtm_cmd qtm_cmd;
> > +	char *dst;
> > +	struct atmel_qtm_node_group_config node_group_config;
> > +
> > +	if (!conf) {
> > +		dev_err(ptc->dev, "cannot load QTM configuration, it has to be set manually.\n");
> > +		return;
> > +	}
> > +
> > +	atmel_ppp_irq_enable(ptc, ATMEL_PPP_IRQ1);
> > +	atmel_ppp_irq_disable(ptc, ATMEL_PPP_IRQ2 | ATMEL_PPP_IRQ3);
> > +
> > +	qtm_cmd.id = ATMEL_QTM_CMD_STOP;
> > +	atmel_qtm_cmd_send(ptc, &qtm_cmd);
> > +
> > +	/* Load QTM configuration. */
> > +	dst = (char *)ptc->qtm_mb + ATMEL_QTM_MB_NODE_GROUP_CONFIG_OFFSET;
> > +	/* memcpy doesn't work for an unknown reason. */
> > +	_memcpy_toio(dst, conf->data, conf->size);
> > +	release_firmware(conf);
> > +
> > +	if (atmel_ptc_of_parse(ptc))
> > +		dev_err(ptc->dev, "ptc_of_parse failed\n");
> > +
> > +	memcpy(&node_group_config,
> > +	       ptc->qtm_mb + ATMEL_QTM_MB_NODE_GROUP_CONFIG_OFFSET,
> > +	       sizeof(node_group_config));
> > +
> > +	/* Start QTM. */
> > +	qtm_cmd.id = ATMEL_QTM_CMD_INIT;
> > +	qtm_cmd.data = node_group_config.count;
> > +	atmel_qtm_cmd_send(ptc, &qtm_cmd);
> > +	qtm_cmd.id = ATMEL_QTM_CMD_SET_ACQ_MODE_TIMER;
> > +	qtm_cmd.data = 20;
> > +	atmel_qtm_cmd_send(ptc, &qtm_cmd);
> > +	qtm_cmd.id = ATMEL_QTM_CMD_RUN;
> > +	qtm_cmd.data = node_group_config.count;
> > +	atmel_qtm_cmd_send(ptc, &qtm_cmd);
> > +}
> > +
> > +static void atmel_ppp_fw_callback(const struct firmware *fw, void *context)
> > +{
> > +	struct atmel_ptc *ptc = context;
> > +	int ret;
> > +	struct atmel_qtm_cmd cmd;
> > +
> > +	if (!fw || !fw->size) {
> > +		dev_err(ptc->dev, "cannot load firmware.\n");
> > +		release_firmware(fw);
> > +		device_release_driver(ptc->dev);
> > +		return;
> > +	}
> > +
> > +	/* Command sequence to start from a clean state. */
> > +	atmel_ppp_cmd_send(ptc, ATMEL_PPP_CMD_ABORT);
> > +	atmel_ppp_irq_pending_clr(ptc, ATMEL_PPP_IRQ_MASK);
> > +	atmel_ppp_cmd_send(ptc, ATMEL_PPP_CMD_RESET);
> > +
> > +	memcpy(ptc->firmware, fw->data, fw->size);
> > +	release_firmware(fw);
> > +
> > +	atmel_ppp_cmd_send(ptc, ATMEL_PPP_CMD_RUN);
> > +
> > +	cmd.id = ATMEL_QTM_CMD_FIRM_VERSION;
> > +	atmel_qtm_cmd_send(ptc, &cmd);
> > +	dev_info(ptc->dev, "firmware version: %u\n", cmd.data);
> 
> dev_dbg().
> 

Ok.

> > +
> > +	/* PPP is running, it's time to load the QTM configuration. */
> > +	ret = request_firmware_nowait(THIS_MODULE, 1, ATMEL_QTM_CONF_NAME, ptc->dev,
> > +				      GFP_KERNEL, ptc, atmel_qtm_conf_callback);
> > +	if (ret)
> > +		dev_err(ptc->dev, "QTM configuration loading failed.\n");
> > +}
> > +
> > +static int atmel_ptc_probe(struct platform_device *pdev)
> > +{
> > +	struct atmel_ptc *ptc;
> > +	struct resource	*res;
> > +	void *shared_memory;
> > +	int ret;
> > +
> > +	ptc = devm_kzalloc(&pdev->dev, sizeof(*ptc), GFP_KERNEL);
> > +	if (!ptc)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, ptc);
> > +	ptc->dev = &pdev->dev;
> > +	ptc->dev->driver_data = ptc;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!res)
> > +		return -ENODEV;
> > +
> > +	shared_memory = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(shared_memory))
> > +		return PTR_ERR(shared_memory);
> > +
> > +	ptc->firmware = shared_memory;
> > +	ptc->qtm_mb = shared_memory + ATMEL_QTM_MB_OFFSET;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > +	if (!res)
> > +		return -EINVAL;
> > +
> > +	ptc->ppp_regs = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(ptc->ppp_regs))
> > +		return PTR_ERR(ptc->ppp_regs);
> > +
> > +	ptc->irq = platform_get_irq(pdev, 0);
> > +	if (ptc->irq <= 0) {
> > +		if (!ptc->irq)
> > +			ptc->irq = -ENXIO;
> > +
> > +		return ptc->irq;
> > +	}
> > +
> > +	ptc->clk_per = devm_clk_get(&pdev->dev, "ptc_clk");
> > +	if (IS_ERR(ptc->clk_per))
> > +		return PTR_ERR(ptc->clk_per);
> > +
> > +	ptc->clk_int_osc = devm_clk_get(&pdev->dev, "ptc_int_osc");
> > +	if (IS_ERR(ptc->clk_int_osc))
> > +		return PTR_ERR(ptc->clk_int_osc);
> > +
> > +	ptc->clk_slow = devm_clk_get(&pdev->dev, "slow_clk");
> > +	if (IS_ERR(ptc->clk_slow))
> > +		return PTR_ERR(ptc->clk_slow);
> > +
> > +	ret = devm_request_irq(&pdev->dev, ptc->irq, atmel_ppp_irq_handler, 0,
> > +			       pdev->dev.driver->name, ptc);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = clk_prepare_enable(ptc->clk_int_osc);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = clk_prepare_enable(ptc->clk_per);
> > +	if (ret)
> > +		goto disable_clk_int_osc;
> > +
> > +	ret = clk_prepare_enable(ptc->clk_slow);
> > +	if (ret)
> > +		goto disable_clk_per;
> 
> You really want to enable clocks before you request IRQ. And since you
> mixing devm and non-devm APIs you need to be extra careful on teardown
> path to preserve order of releasing resources.
> devm_add_action_or_reset() is your friend here.
> 

I'll rework this part.

> Is your IRQ routine ready to be fired even though not all device
> structures are allocated/initialized?
> 

Probably not, I will defer it.

> > +
> > +	/* Needed to avoid unexpected behaviors. */
> > +	memset(ptc->firmware, 0, ATMEL_QTM_MB_OFFSET + sizeof(*ptc->qtm_mb));
> > +	ptc->imr = 0;
> > +	init_completion(&ptc->ppp_ack);
> > +
> > +	/*
> > +	 * Expose a file to give an access to the QTM mailbox to a user space
> > +	 * application in order to configure it or to send commands.
> > +	 */

Is it a good approach to offer a userspace access to the mailbox in this
way?

At the beginning, the mailbox should be accessed from userspace only for
debug purpose or to set the configuration manually.

Nowadays, we are developing tools to help the user to set the right
configuration. In the future, we may also provide tools to visualize in
'realtime' some values of the mailbox. I'm scared to miss some data and
to not use the right interface to reach these goals. What's your opinion?

> > +	ret = sysfs_create_bin_file(&pdev->dev.kobj, &atmel_ptc_qtm_mb_attr);
> > +	if (ret)
> > +		goto disable_clk_slow;
> 
> devm_device_add_group()?
>

Ok.

> > +
> > +	ret = request_firmware_nowait(THIS_MODULE, 1, ATMEL_PPP_FIRMWARE_NAME,
> > +				      ptc->dev, GFP_KERNEL, ptc,
> > +				      atmel_ppp_fw_callback);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "firmware loading failed\n");
> > +		ret = -EPROBE_DEFER;
> 
> Always defer? That is optimistic.
> 

Probably.

> > +		goto remove_qtm_mb;
> > +	}
> > +
> > +	return 0;
> > +
> > +remove_qtm_mb:
> > +	sysfs_remove_bin_file(&pdev->dev.kobj, &atmel_ptc_qtm_mb_attr);
> > +disable_clk_slow:
> > +	clk_disable_unprepare(ptc->clk_slow);
> > +disable_clk_per:
> > +	clk_disable_unprepare(ptc->clk_per);
> > +disable_clk_int_osc:
> > +	clk_disable_unprepare(ptc->clk_int_osc);
> > +
> > +	return ret;
> > +}
> > +
> > +static int atmel_ptc_remove(struct platform_device *pdev)
> > +{
> > +	struct atmel_ptc *ptc = platform_get_drvdata(pdev);
> > +	int i;
> > +
> 
> You need a completion in your fimrware loading callback and wait for it
> here, otherwise quick bind/unbind will crash the kernel.
> 
> 

I see, thanks.

> > +	if (ptc->buttons_registered)
> > +		input_unregister_device(ptc->buttons_input);
> > +
> > +	for (i = 0; i < ATMEL_PTC_MAX_SCROLLERS; i++) {
> > +		struct input_dev *scroller = ptc->scroller_input[i];
> > +
> > +		if (!scroller || !ptc->scroller_registered[i])
> > +			continue;
> > +		input_unregister_device(scroller);
> > +	}
> > +
> > +	sysfs_remove_bin_file(&pdev->dev.kobj, &atmel_ptc_qtm_mb_attr);
> > +	clk_disable_unprepare(ptc->clk_slow);
> > +	clk_disable_unprepare(ptc->clk_per);
> > +	clk_disable_unprepare(ptc->clk_int_osc);
> > +
> > +	return 0;
> > +}
> > +
> 
> #ifdef CONFIG_OF

I have depends on OF in Kconfig.

> > +static const struct of_device_id atmel_ptc_dt_match[] = {
> > +	{
> > +		.compatible = "atmel,sama5d2-ptc",
> > +	}, {
> > +		/* sentinel */
> > +	}
> > +};
> > +MODULE_DEVICE_TABLE(of, atmel_ptc_dt_match);
> #endif
> 
> > +
> > +static struct platform_driver atmel_ptc_driver = {
> > +	.probe = atmel_ptc_probe,
> > +	.remove = atmel_ptc_remove,
> > +	.driver = {
> > +		.name = "atmel_ptc",
> > +		.of_match_table = atmel_ptc_dt_match,
> 
> of_match_ptr() - you are allowing it to be compiled without OF.
> 

ditto but I can change it if you want.

> > +	},
> > +};
> > +module_platform_driver(atmel_ptc_driver)
> > +
> > +MODULE_AUTHOR("Ludovic Desroches <ludovic.desroches@microchip.com>");
> > +MODULE_DESCRIPTION("Atmel PTC subsystem");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_FIRMWARE(ATMEL_PPP_FIRMWARE_NAME);
> > +MODULE_FIRMWARE(ATMEL_QTM_CONF_NAME);
> > -- 
> > 2.12.2
> > 
> 
> Thanks.

Thanks for your review.

Regards

Ludovic