[PATCH v4 0/2] dt-bindings: mfd: Add max7360 mfd driver and DT documentation

[PATCH v4 0/2] dt-bindings: mfd: Add max7360 mfd driver and DT documentation

[Valentin Sitdikov]

This series add initial support of mfd core driver for max7360 chip

Andrei Dranitca (1):
  mfd: max7360: Add mfd core device driver

Valentin Sitdikov (1):
  dt-bindings: mfd: Add DT bindings documentation for the max7360 mfd
    driver

---
Changes since v1:

  - Use proper node names
  - Drop unused addresses in node names
  - Fix description of interrupt-names property

Changes since v2:

  - Fix rotary-encoder node name
  - Add description of interrupts property
  - Drop unused code
  - Comment max7360 structure
  - Add comments to the code
 
Changes since v3:

  - Fix formatting
  - Use devm_mfd_add_devices

Andrei Dranitca (1):
  mfd: max7360: Add mfd core device driver

Valentin Sitdikov (1):
  dt-bindings: mfd: Add DT bindings documentation for the max7360 mfd
    driver

 Documentation/devicetree/bindings/mfd/max7360.txt |  74 ++++++
 drivers/mfd/Kconfig                               |  16 ++
 drivers/mfd/Makefile                              |   1 +
 drivers/mfd/max7360.c                             | 291 ++++++++++++++++++++++
 include/linux/mfd/max7360.h                       |  90 +++++++
 5 files changed, 472 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/max7360.txt
 create mode 100644 drivers/mfd/max7360.c
 create mode 100644 include/linux/mfd/max7360.h

-- 
2.9.3

[PATCH 1/2] dt-bindings: mfd: Add DT bindings documentation for the max7360 mfd driver

[Valentin Sitdikov]

This patch adds documentation for the max7360 bindings.
The max7360 is Multi-functional Device containing gpio,
keypad, pwm and rotary encoder submodules.

Signed-off-by: Andrei Dranitca <Andrei_Dranitca@mentor.com>
Signed-off-by: Valentin Sitdikov <valentin_sitdikov@mentor.com>
---
 Documentation/devicetree/bindings/mfd/max7360.txt | 74 +++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/max7360.txt

diff --git a/Documentation/devicetree/bindings/mfd/max7360.txt b/Documentation/devicetree/bindings/mfd/max7360.txt
new file mode 100644
index 0000000..c2ceaf9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max7360.txt
@@ -0,0 +1,74 @@
+* Maxim MAX7360 Multi-function Device
+
+The Maxim MAX7360 is a Multi-function Device which includes
+64 key switches, 8 LED drivers/GPIOs, PWM intensity control,
+and rotary switch control.
+
+Required properties:
+- compatible:	Should be the following: "maxim,max7360".
+- reg:		Specifies the I2C slave address of the max7360 block.
+		  It can be 0x38, 0x3a, 0x3c or 0x3e IIUC.
+
+Optional properties:
+- interrupt-parent:	Specifies the phandle of the interrupt controller to
+			  which the interrupts from MAX7360 are routed to.
+- interrupts:		List of IRQ lines specifiers.
+- interrupt-names:	List of "inti" and "intk".
+- interrupt-controller:	Identifies the device as an interrupt controller.
+- #interrupt-cells :	Number of cells to encode an interrupt source,
+			  shall be 1.
+
+Examples:
+
+Without subnodes:
+	max7360@38 {
+		compatible = "maxim,max7360";
+		reg = <0x38>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <23 IRQ_TYPE_LEVEL_LOW>, <23 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-names = "inti", "intk";
+		interrupt-controller;
+		#interrupt-cells = <0x1>;
+	};
+
+With subnodes:
+	max7360@38 {
+		compatible = "maxim,max7360";
+		reg = <0x38>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <23 IRQ_TYPE_LEVEL_LOW>, <23 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-names = "inti", "intk";
+		interrupt-controller;
+		#interrupt-cells = <0x1>;
+
+		gpio {
+			compatible = "maxim,max7360-gpio";
+			gpio-controller;
+			#gpio-cells = <0x2>;
+			interrupt-controller;
+			#interrupt-cells = <0x2>;
+			interrupts = <0>;
+		};
+
+		keypad {
+			compatible = "maxim,max7360-keypad";
+			maxim,debounce_reg = /bits/ 8 <0xef>;
+			maxim,ports_reg = /bits/ 8 <0xae>;
+			linux,keymap = < MATRIX_KEY(0, 0, KEY_F5)
+					 MATRIX_KEY(1, 0, KEY_F4) >;
+			keypad,num-rows = <2>;
+			keypad,num-columns = <1>;
+			interrupts = <1>;
+		};
+
+		pwm {
+			compatible = "maxim,max7360-pwm";
+			#pwm-cells = <0x2>;
+		};
+
+		rotary-encoder {
+			compatible = "maxim,max7360-rotary";
+			interrupts = <2>;
+		};
+
+	};
-- 
2.9.3

[PATCH 2/2] mfd: max7360: Add mfd core device driver

[Valentin Sitdikov]

From: Andrei Dranitca <Andrei_Dranitca@mentor.com>

This patch adds core/irq driver to support MAX7360 i2c chip
which contains keypad, gpio, pwm, gpo and rotary encoder submodules.

Signed-off-by: Andrei Dranitca <Andrei_Dranitca@mentor.com>
Signed-off-by: Valentin Sitdikov <valentin_sitdikov@mentor.com>
---
 drivers/mfd/Kconfig         |  16 +++
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/max7360.c       | 291 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/max7360.h |  90 ++++++++++++++
 4 files changed, 398 insertions(+)
 create mode 100644 drivers/mfd/max7360.c
 create mode 100644 include/linux/mfd/max7360.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3eb5c93..894c2e9 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -721,6 +721,22 @@ config MFD_MAX8998
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MAX7360
+	tristate "Maxim Semiconductor MAX7360 support"
+	depends on I2C && OF
+	select MFD_CORE
+	select REGMAP_I2C
+	select IRQ_DOMAIN
+	help
+	  Say yes here to add support for Maxim Semiconductor MAX7360.
+	  This provides microprocessors with management of up to 64 key switches,
+	  with an additional eight LED drivers/GPIOs that feature constant-current,
+	  PWM intensity control, and rotary switch control options.
+
+	  This driver provides common support for accessing the device,
+	  additional drivers must be enabled in order to use the functionality
+	  of the device.
+
 config MFD_MT6397
 	tristate "MediaTek MT6397 PMIC Support"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c16bf1e..9e721c0 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -137,6 +137,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
 obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
 
 obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
+obj-$(CONFIG_MFD_MAX7360)	+= max7360.o
 obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
 obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
 obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
new file mode 100644
index 0000000..d18ea96
--- /dev/null
+++ b/drivers/mfd/max7360.c
@@ -0,0 +1,291 @@
+/*
+ * Copyright (C) 2017 Mentor Graphics
+ *
+ * Author: Andrei Dranitca <Andrei_Dranitca@mentor.com>
+ * Author: Valentin Sitdikov <Valentin.Sitdikov@mentor.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max7360.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+
+static const struct mfd_cell max7360_devices[] = {
+	{
+		.name           = "max7360-gpio",
+		.of_compatible	= "maxim,max7360-gpio",
+	},
+	{
+		.name           = "max7360-keypad",
+		.of_compatible	= "maxim,max7360-keypad",
+	},
+	{
+		.name           = "max7360-pwm",
+		.of_compatible	= "maxim,max7360-pwm",
+	},
+	{
+		.name           = "max7360-rotary",
+		.of_compatible	= "maxim,max7360-rotary",
+	},
+};
+
+static irqreturn_t max7360_irq(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqi(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqk(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static int max7360_irq_map(struct irq_domain *d, unsigned int virq,
+			  irq_hw_number_t hwirq)
+{
+	struct max7360 *max7360 = d->host_data;
+
+	irq_set_chip_data(virq, max7360);
+	irq_set_chip_and_handler(virq, &dummy_irq_chip,
+				handle_edge_irq);
+	irq_set_nested_thread(virq, 1);
+	irq_set_noprobe(virq);
+
+	return 0;
+}
+
+static void max7360_irq_unmap(struct irq_domain *d, unsigned int virq)
+{
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+static const struct irq_domain_ops max7360_irq_ops = {
+	.map    = max7360_irq_map,
+	.unmap  = max7360_irq_unmap,
+	.xlate  = irq_domain_xlate_onecell,
+};
+
+static void max7360_free_irqs(struct max7360 *max7360)
+{
+	if (max7360->shared_irq)
+		free_irq(max7360->shared_irq, max7360);
+	else {
+		free_irq(max7360->irq_inti, max7360);
+		free_irq(max7360->irq_intk, max7360);
+	}
+}
+
+static int max7360_irq_init(struct max7360 *max7360, struct device_node *np)
+{
+	int ret;
+
+	max7360->irq_inti = of_irq_get_byname(np, "inti");
+	if (max7360->irq_inti < 0) {
+		dev_err(max7360->dev, "no inti provided");
+		return -ENODEV;
+	}
+
+	max7360->irq_intk = of_irq_get_byname(np, "intk");
+	if (max7360->irq_intk < 0) {
+		dev_err(max7360->dev, "no intk provided");
+		return -ENODEV;
+	}
+
+	if (max7360->irq_inti == max7360->irq_intk) {
+		/*
+		 * In case of pin inti and pin intk are the connected
+		 * to the same soc`s irq pin.
+		 */
+		max7360->shared_irq = max7360->irq_inti;
+		ret = request_threaded_irq(max7360->shared_irq, NULL,
+					  max7360_irq,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			dev_err(max7360->dev, "failed to request IRQ: %d\n",
+				ret);
+			return ret;
+		}
+	} else {
+		max7360->shared_irq = 0;
+		ret = request_threaded_irq(max7360->irq_inti, NULL,
+					  max7360_irqi,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			dev_err(max7360->dev, "failed to request inti IRQ: %d\n",
+			       ret);
+			return ret;
+		}
+
+		ret = request_threaded_irq(max7360->irq_intk, NULL,
+					  max7360_irqk,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			free_irq(max7360->irq_inti, max7360);
+			dev_err(max7360->dev, "failed to request intk IRQ: %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	max7360->domain = irq_domain_add_simple(np, MAX7360_NR_INTERNAL_IRQS,
+					       0, &max7360_irq_ops, max7360);
+
+	if (!max7360->domain) {
+		max7360_free_irqs(max7360);
+		dev_err(max7360->dev, "Failed to create irqdomain\n");
+		return -ENODEV;
+	}
+
+	ret = irq_create_mapping(max7360->domain, MAX7360_INT_GPIO);
+	if (!ret) {
+		max7360_free_irqs(max7360);
+		dev_err(max7360->dev, "Failed to map GPIO IRQ\n");
+		return -EINVAL;
+	}
+
+	ret = irq_create_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	if (!ret) {
+		max7360_free_irqs(max7360);
+		dev_err(max7360->dev, "Failed to map KEYPAD IRQ\n");
+		return -EINVAL;
+	}
+
+	ret = irq_create_mapping(max7360->domain, MAX7360_INT_ROTARY);
+	if (!ret) {
+		max7360_free_irqs(max7360);
+		dev_err(max7360->dev, "Failed to map ROTARY IRQ\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+static int max7360_device_init(struct max7360 *max7360)
+{
+	int ret;
+
+	ret = devm_mfd_add_devices(max7360->dev, PLATFORM_DEVID_NONE,
+			     max7360_devices,
+			     ARRAY_SIZE(max7360_devices), NULL,
+			     0, max7360->domain);
+	if (ret)
+		dev_err(max7360->dev, "failed to add child devices\n");
+
+	return ret;
+}
+
+static const struct regmap_range max7360_volatile_ranges[] = {
+	{
+		.range_min = MAX7360_REG_KEYFIFO,
+		.range_max = MAX7360_REG_KEYFIFO,
+	}, {
+		.range_min = MAX7360_REG_GPIOIN,
+		.range_max = MAX7360_REG_GPIOIN,
+	},
+};
+
+static const struct regmap_access_table max7360_volatile_table = {
+	.yes_ranges = max7360_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
+};
+
+static const struct regmap_config max7360_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0xff,
+	.volatile_table = &max7360_volatile_table,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+static int max7360_probe(struct i2c_client *i2c,
+			const struct i2c_device_id *id)
+{
+	struct device_node *np = i2c->dev.of_node;
+	struct max7360 *max7360;
+	int ret;
+
+	max7360 = devm_kzalloc(&i2c->dev, sizeof(struct max7360),
+			      GFP_KERNEL);
+	if (!max7360)
+		return -ENOMEM;
+
+	max7360->dev = &i2c->dev;
+	max7360->i2c = i2c;
+
+	i2c_set_clientdata(i2c, max7360);
+
+	max7360->regmap = devm_regmap_init_i2c(i2c, &max7360_regmap_config);
+
+	ret = max7360_irq_init(max7360, np);
+	if (ret)
+		return ret;
+
+	ret = max7360_device_init(max7360);
+	if (ret) {
+		dev_err(max7360->dev, "failed to add child devices\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id max7360_match[] = {
+	{ .compatible = "maxim,max7360" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, max7360_match);
+
+static struct i2c_driver max7360_driver = {
+	.driver = {
+		.name = "max7360",
+		.of_match_table = max7360_match,
+	},
+	.probe = max7360_probe,
+};
+builtin_i2c_driver(max7360_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MAX7360 MFD core driver");
diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
new file mode 100644
index 0000000..c59941c
--- /dev/null
+++ b/include/linux/mfd/max7360.h
@@ -0,0 +1,90 @@
+#ifndef __LINUX_MFD_MAX7360_H
+#define __LINUX_MFD_MAX7360_H
+#include <linux/regmap.h>
+
+#define MAX7360_MAX_KEY_ROWS	8
+#define MAX7360_MAX_KEY_COLS	8
+#define MAX7360_MAX_KEY_NUM	(MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
+#define MAX7360_ROW_SHIFT	3
+
+#define MAX7360_MAX_GPIO 8
+#define MAX7360_MAX_GPO 6
+#define MAX7360_COL_GPO_PINS 8
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_KEYFIFO	0x00
+#define MAX7360_REG_CONFIG	0x01
+#define MAX7360_REG_DEBOUNCE	0x02
+#define MAX7360_REG_INTERRUPT	0x03
+#define MAX7360_REG_PORTS	0x04
+#define MAX7360_REG_KEYREP	0x05
+#define MAX7360_REG_SLEEP	0x06
+
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_GPIOCFG	0x40
+#define MAX7360_REG_GPIOCTRL	0x41
+#define MAX7360_REG_GPIODEB	0x42
+#define MAX7360_REG_GPIOCURR	0x43
+#define MAX7360_REG_GPIOOUTM	0x44
+#define MAX7360_REG_PWMCOM	0x45
+#define MAX7360_REG_RTRCFG	0x46
+#define MAX7360_REG_GPIOIN	0x49
+#define MAX7360_REG_RTR_CNT	0x4A
+#define MAX7360_REG_PWMBASE	0x50
+#define MAX7360_REG_PWMCFG	0x58
+
+
+#define MAX7360_REG_PORTCFGBASE 0x58
+
+/*
+ * Configuration register bits
+ */
+#define MAX7360_CFG_SLEEP	BIT(7)
+#define MAX7360_CFG_INTERRUPT	BIT(5)
+#define MAX7360_CFG_KEY_RELEASE	BIT(3)
+#define MAX7360_CFG_WAKEUP	BIT(1)
+#define MAX7360_CFG_TIMEOUT	BIT(0)
+
+/*
+ * Autosleep register values (ms)
+ */
+#define MAX7360_AUTOSLEEP_8192	0x01
+#define MAX7360_AUTOSLEEP_4096	0x02
+#define MAX7360_AUTOSLEEP_2048	0x03
+#define MAX7360_AUTOSLEEP_1024	0x04
+#define MAX7360_AUTOSLEEP_512	0x05
+#define MAX7360_AUTOSLEEP_256	0x06
+
+#define MAX7360_INT_INTI	0
+#define MAX7360_INT_INTK	1
+
+#define MAX7360_INT_GPIO   0
+#define MAX7360_INT_KEYPAD 1
+#define MAX7360_INT_ROTARY 2
+
+#define MAX7360_NR_INTERNAL_IRQS	3
+
+/**
+ * struct max7360
+ * @dev:	Parent device pointer
+ * @i2c:	I2c client pointer
+ * @domain:	Irq domain pointer
+ * @regmap:	Used for I2C communication on accessing registers
+ * @shared_irq:	Irq number if inti and intk pins are connected together
+ * @irq_inti:	Irq number for inti pin
+ * @irq_intk:	Irq number for inik pin
+ */
+struct max7360 {
+	struct device		*dev;
+	struct i2c_client	*i2c;
+	struct irq_domain	*domain;
+	struct regmap		*regmap;
+
+	int			shared_irq;
+	int			irq_inti;
+	int			irq_intk;
+};
+#endif
-- 
2.9.3

Re: [PATCH 1/2] dt-bindings: mfd: Add DT bindings documentation for the max7360 mfd driver

[Vladimir Zapolskiy]

Hi Valentin,

On 06/13/2017 03:20 PM, Valentin Sitdikov wrote:
> This patch adds documentation for the max7360 bindings.
> The max7360 is Multi-functional Device containing gpio,
> keypad, pwm and rotary encoder submodules.
> 
> Signed-off-by: Andrei Dranitca <Andrei_Dranitca@mentor.com>
> Signed-off-by: Valentin Sitdikov <valentin_sitdikov@mentor.com>
> ---
>  Documentation/devicetree/bindings/mfd/max7360.txt | 74 +++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/max7360.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/max7360.txt b/Documentation/devicetree/bindings/mfd/max7360.txt
> new file mode 100644
> index 0000000..c2ceaf9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/max7360.txt
> @@ -0,0 +1,74 @@
> +* Maxim MAX7360 Multi-function Device
> +
> +The Maxim MAX7360 is a Multi-function Device which includes
> +64 key switches, 8 LED drivers/GPIOs, PWM intensity control,
> +and rotary switch control.
> +
> +Required properties:
> +- compatible:	Should be the following: "maxim,max7360".
> +- reg:		Specifies the I2C slave address of the max7360 block.
> +		  It can be 0x38, 0x3a, 0x3c or 0x3e IIUC.

"IIUC" note is highly undesirable ;)

> +
> +Optional properties:
> +- interrupt-parent:	Specifies the phandle of the interrupt controller to
> +			  which the interrupts from MAX7360 are routed to.
> +- interrupts:		List of IRQ lines specifiers.
> +- interrupt-names:	List of "inti" and "intk".
> +- interrupt-controller:	Identifies the device as an interrupt controller.
> +- #interrupt-cells :	Number of cells to encode an interrupt source,
> +			  shall be 1.
> +
> +Examples:
> +
> +Without subnodes:
> +	max7360@38 {
> +		compatible = "maxim,max7360";
> +		reg = <0x38>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <23 IRQ_TYPE_LEVEL_LOW>, <23 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-names = "inti", "intk";
> +		interrupt-controller;
> +		#interrupt-cells = <0x1>;
> +	};

Can you drop the example above? It does not bring any new information.

> +
> +With subnodes:
> +	max7360@38 {

Commonly for device node names generic/device class names are utilized,
however I can not suggest what name is better.

> +		compatible = "maxim,max7360";
> +		reg = <0x38>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <23 IRQ_TYPE_LEVEL_LOW>, <23 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-names = "inti", "intk";
> +		interrupt-controller;
> +		#interrupt-cells = <0x1>;

For #*-cells properties plese specify plain integers, #interrupt-cells = <1>;

> +
> +		gpio {
> +			compatible = "maxim,max7360-gpio";
> +			gpio-controller;
> +			#gpio-cells = <0x2>;

Please use a plain integer.

> +			interrupt-controller;
> +			#interrupt-cells = <0x2>;

Please use a plain integer.

> +			interrupts = <0>;
> +		};
> +
> +		keypad {
> +			compatible = "maxim,max7360-keypad";
> +			maxim,debounce_reg = /bits/ 8 <0xef>;
> +			maxim,ports_reg = /bits/ 8 <0xae>;

Here I'd rather suggest

1) use 32-bit values,
2) rename properties by replacing underscore by a hyphen.

> +			linux,keymap = < MATRIX_KEY(0, 0, KEY_F5)
> +					 MATRIX_KEY(1, 0, KEY_F4) >;
> +			keypad,num-rows = <2>;
> +			keypad,num-columns = <1>;
> +			interrupts = <1>;
> +		};
> +
> +		pwm {
> +			compatible = "maxim,max7360-pwm";
> +			#pwm-cells = <0x2>;

Please use a plain integer.

> +		};
> +
> +		rotary-encoder {
> +			compatible = "maxim,max7360-rotary";
> +			interrupts = <2>;
> +		};
> +

Please drop an empty line above.

> +	};
> 

--
With best wishes,
Vladimir

Re: [PATCH 2/2] mfd: max7360: Add mfd core device driver

[Vladimir Zapolskiy]

Hi Valentin,

On 06/13/2017 03:20 PM, Valentin Sitdikov wrote:
> From: Andrei Dranitca <Andrei_Dranitca@mentor.com>
> 
> This patch adds core/irq driver to support MAX7360 i2c chip
> which contains keypad, gpio, pwm, gpo and rotary encoder submodules.
> 
> Signed-off-by: Andrei Dranitca <Andrei_Dranitca@mentor.com>
> Signed-off-by: Valentin Sitdikov <valentin_sitdikov@mentor.com>
> ---
>  drivers/mfd/Kconfig         |  16 +++
>  drivers/mfd/Makefile        |   1 +
>  drivers/mfd/max7360.c       | 291 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/max7360.h |  90 ++++++++++++++
>  4 files changed, 398 insertions(+)
>  create mode 100644 drivers/mfd/max7360.c
>  create mode 100644 include/linux/mfd/max7360.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 3eb5c93..894c2e9 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -721,6 +721,22 @@ config MFD_MAX8998
>  	  additional drivers must be enabled in order to use the functionality
>  	  of the device.
>  
> +config MFD_MAX7360

Can you consider to move the new section upwards to preserve
alphanumerical order at least locally?

> +	tristate "Maxim Semiconductor MAX7360 support"
> +	depends on I2C && OF
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	select IRQ_DOMAIN
> +	help
> +	  Say yes here to add support for Maxim Semiconductor MAX7360.
> +	  This provides microprocessors with management of up to 64 key switches,
> +	  with an additional eight LED drivers/GPIOs that feature constant-current,
> +	  PWM intensity control, and rotary switch control options.
> +
> +	  This driver provides common support for accessing the device,
> +	  additional drivers must be enabled in order to use the functionality
> +	  of the device.
> +
>  config MFD_MT6397
>  	tristate "MediaTek MT6397 PMIC Support"
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index c16bf1e..9e721c0 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -137,6 +137,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
>  obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
>  
>  obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
> +obj-$(CONFIG_MFD_MAX7360)	+= max7360.o
>  obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
>  obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
>  obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
> diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
> new file mode 100644
> index 0000000..d18ea96
> --- /dev/null
> +++ b/drivers/mfd/max7360.c
> @@ -0,0 +1,291 @@
> +/*
> + * Copyright (C) 2017 Mentor Graphics
> + *
> + * Author: Andrei Dranitca <Andrei_Dranitca@mentor.com>
> + * Author: Valentin Sitdikov <Valentin.Sitdikov@mentor.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max7360.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/slab.h>

Add <linux/regmap.h> into the list.

> +
> +
> +static const struct mfd_cell max7360_devices[] = {
> +	{
> +		.name           = "max7360-gpio",
> +		.of_compatible	= "maxim,max7360-gpio",
> +	},
> +	{
> +		.name           = "max7360-keypad",
> +		.of_compatible	= "maxim,max7360-keypad",
> +	},
> +	{
> +		.name           = "max7360-pwm",
> +		.of_compatible	= "maxim,max7360-pwm",
> +	},
> +	{
> +		.name           = "max7360-rotary",
> +		.of_compatible	= "maxim,max7360-rotary",
> +	},
> +};

Since the driver depends on OF, you don't need to list the cells.

Consider to replace devm_mfd_add_devices() with devm_of_platform_populate().

> +
> +static irqreturn_t max7360_irq(int irq, void *data)
> +{
> +	struct max7360 *max7360 = data;
> +	int virq;
> +
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
> +	handle_nested_irq(virq);
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
> +	handle_nested_irq(virq);
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
> +	handle_nested_irq(virq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t max7360_irqi(int irq, void *data)
> +{
> +	struct max7360 *max7360 = data;
> +	int virq;
> +
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
> +	handle_nested_irq(virq);
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
> +	handle_nested_irq(virq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t max7360_irqk(int irq, void *data)
> +{
> +	struct max7360 *max7360 = data;
> +	int virq;
> +
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
> +	handle_nested_irq(virq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int max7360_irq_map(struct irq_domain *d, unsigned int virq,
> +			  irq_hw_number_t hwirq)
> +{
> +	struct max7360 *max7360 = d->host_data;
> +
> +	irq_set_chip_data(virq, max7360);
> +	irq_set_chip_and_handler(virq, &dummy_irq_chip,
> +				handle_edge_irq);
> +	irq_set_nested_thread(virq, 1);
> +	irq_set_noprobe(virq);
> +
> +	return 0;
> +}
> +
> +static void max7360_irq_unmap(struct irq_domain *d, unsigned int virq)
> +{
> +	irq_set_chip_and_handler(virq, NULL, NULL);
> +	irq_set_chip_data(virq, NULL);
> +}
> +
> +static const struct irq_domain_ops max7360_irq_ops = {
> +	.map    = max7360_irq_map,
> +	.unmap  = max7360_irq_unmap,
> +	.xlate  = irq_domain_xlate_onecell,
> +};
> +
> +static void max7360_free_irqs(struct max7360 *max7360)
> +{
> +	if (max7360->shared_irq)
> +		free_irq(max7360->shared_irq, max7360);
> +	else {
> +		free_irq(max7360->irq_inti, max7360);
> +		free_irq(max7360->irq_intk, max7360);
> +	}
> +}
> +
> +static int max7360_irq_init(struct max7360 *max7360, struct device_node *np)
> +{
> +	int ret;
> +
> +	max7360->irq_inti = of_irq_get_byname(np, "inti");
> +	if (max7360->irq_inti < 0) {
> +		dev_err(max7360->dev, "no inti provided");
> +		return -ENODEV;
> +	}
> +
> +	max7360->irq_intk = of_irq_get_byname(np, "intk");
> +	if (max7360->irq_intk < 0) {
> +		dev_err(max7360->dev, "no intk provided");
> +		return -ENODEV;
> +	}
> +
> +	if (max7360->irq_inti == max7360->irq_intk) {
> +		/*
> +		 * In case of pin inti and pin intk are the connected
> +		 * to the same soc`s irq pin.
> +		 */
> +		max7360->shared_irq = max7360->irq_inti;
> +		ret = request_threaded_irq(max7360->shared_irq, NULL,
> +					  max7360_irq,
> +					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					  "max7360", max7360);
> +		if (ret) {
> +			dev_err(max7360->dev, "failed to request IRQ: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	} else {
> +		max7360->shared_irq = 0;
> +		ret = request_threaded_irq(max7360->irq_inti, NULL,
> +					  max7360_irqi,
> +					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					  "max7360", max7360);
> +		if (ret) {
> +			dev_err(max7360->dev, "failed to request inti IRQ: %d\n",
> +			       ret);
> +			return ret;
> +		}
> +
> +		ret = request_threaded_irq(max7360->irq_intk, NULL,
> +					  max7360_irqk,
> +					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					  "max7360", max7360);
> +		if (ret) {
> +			free_irq(max7360->irq_inti, max7360);
> +			dev_err(max7360->dev, "failed to request intk IRQ: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
> +	max7360->domain = irq_domain_add_simple(np, MAX7360_NR_INTERNAL_IRQS,
> +					       0, &max7360_irq_ops, max7360);
> +
> +	if (!max7360->domain) {
> +		max7360_free_irqs(max7360);
> +		dev_err(max7360->dev, "Failed to create irqdomain\n");
> +		return -ENODEV;
> +	}
> +
> +	ret = irq_create_mapping(max7360->domain, MAX7360_INT_GPIO);
> +	if (!ret) {
> +		max7360_free_irqs(max7360);
> +		dev_err(max7360->dev, "Failed to map GPIO IRQ\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = irq_create_mapping(max7360->domain, MAX7360_INT_KEYPAD);
> +	if (!ret) {
> +		max7360_free_irqs(max7360);
> +		dev_err(max7360->dev, "Failed to map KEYPAD IRQ\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = irq_create_mapping(max7360->domain, MAX7360_INT_ROTARY);
> +	if (!ret) {
> +		max7360_free_irqs(max7360);
> +		dev_err(max7360->dev, "Failed to map ROTARY IRQ\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +static int max7360_device_init(struct max7360 *max7360)
> +{
> +	int ret;
> +
> +	ret = devm_mfd_add_devices(max7360->dev, PLATFORM_DEVID_NONE,
> +			     max7360_devices,
> +			     ARRAY_SIZE(max7360_devices), NULL,
> +			     0, max7360->domain);
> +	if (ret)
> +		dev_err(max7360->dev, "failed to add child devices\n");
> +
> +	return ret;
> +}
> +
> +static const struct regmap_range max7360_volatile_ranges[] = {
> +	{
> +		.range_min = MAX7360_REG_KEYFIFO,
> +		.range_max = MAX7360_REG_KEYFIFO,
> +	}, {
> +		.range_min = MAX7360_REG_GPIOIN,
> +		.range_max = MAX7360_REG_GPIOIN,
> +	},
> +};
> +
> +static const struct regmap_access_table max7360_volatile_table = {
> +	.yes_ranges = max7360_volatile_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
> +};
> +
> +static const struct regmap_config max7360_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = 0xff,
> +	.volatile_table = &max7360_volatile_table,
> +	.cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int max7360_probe(struct i2c_client *i2c,
> +			const struct i2c_device_id *id)
> +{
> +	struct device_node *np = i2c->dev.of_node;
> +	struct max7360 *max7360;
> +	int ret;
> +
> +	max7360 = devm_kzalloc(&i2c->dev, sizeof(struct max7360),
> +			      GFP_KERNEL);
> +	if (!max7360)
> +		return -ENOMEM;
> +
> +	max7360->dev = &i2c->dev;
> +	max7360->i2c = i2c;
> +
> +	i2c_set_clientdata(i2c, max7360);
> +
> +	max7360->regmap = devm_regmap_init_i2c(i2c, &max7360_regmap_config);

Missing error check.

> +
> +	ret = max7360_irq_init(max7360, np);
> +	if (ret)
> +		return ret;

On driver unload, device removal and error path below a plenty of requested
IRQ resources are leaked.

Also can you evaluate if regmap-irq interface is suitable to be used
within the driver?

> +
> +	ret = max7360_device_init(max7360);
> +	if (ret) {
> +		dev_err(max7360->dev, "failed to add child devices\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id max7360_match[] = {
> +	{ .compatible = "maxim,max7360" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, max7360_match);
> +
> +static struct i2c_driver max7360_driver = {
> +	.driver = {
> +		.name = "max7360",
> +		.of_match_table = max7360_match,
> +	},
> +	.probe = max7360_probe,
> +};
> +builtin_i2c_driver(max7360_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("MAX7360 MFD core driver");
> diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
> new file mode 100644
> index 0000000..c59941c
> --- /dev/null
> +++ b/include/linux/mfd/max7360.h
> @@ -0,0 +1,90 @@
> +#ifndef __LINUX_MFD_MAX7360_H
> +#define __LINUX_MFD_MAX7360_H
> +#include <linux/regmap.h>
> +
> +#define MAX7360_MAX_KEY_ROWS	8
> +#define MAX7360_MAX_KEY_COLS	8
> +#define MAX7360_MAX_KEY_NUM	(MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
> +#define MAX7360_ROW_SHIFT	3
> +
> +#define MAX7360_MAX_GPIO 8
> +#define MAX7360_MAX_GPO 6
> +#define MAX7360_COL_GPO_PINS 8
> +/*
> + * MAX7360 registers
> + */
> +#define MAX7360_REG_KEYFIFO	0x00
> +#define MAX7360_REG_CONFIG	0x01
> +#define MAX7360_REG_DEBOUNCE	0x02
> +#define MAX7360_REG_INTERRUPT	0x03
> +#define MAX7360_REG_PORTS	0x04
> +#define MAX7360_REG_KEYREP	0x05
> +#define MAX7360_REG_SLEEP	0x06
> +
> +/*
> + * MAX7360 registers
> + */
> +#define MAX7360_REG_GPIOCFG	0x40
> +#define MAX7360_REG_GPIOCTRL	0x41
> +#define MAX7360_REG_GPIODEB	0x42
> +#define MAX7360_REG_GPIOCURR	0x43
> +#define MAX7360_REG_GPIOOUTM	0x44
> +#define MAX7360_REG_PWMCOM	0x45
> +#define MAX7360_REG_RTRCFG	0x46
> +#define MAX7360_REG_GPIOIN	0x49
> +#define MAX7360_REG_RTR_CNT	0x4A
> +#define MAX7360_REG_PWMBASE	0x50
> +#define MAX7360_REG_PWMCFG	0x58
> +
> +
> +#define MAX7360_REG_PORTCFGBASE 0x58
> +
> +/*
> + * Configuration register bits
> + */
> +#define MAX7360_CFG_SLEEP	BIT(7)
> +#define MAX7360_CFG_INTERRUPT	BIT(5)
> +#define MAX7360_CFG_KEY_RELEASE	BIT(3)
> +#define MAX7360_CFG_WAKEUP	BIT(1)
> +#define MAX7360_CFG_TIMEOUT	BIT(0)
> +
> +/*
> + * Autosleep register values (ms)
> + */
> +#define MAX7360_AUTOSLEEP_8192	0x01
> +#define MAX7360_AUTOSLEEP_4096	0x02
> +#define MAX7360_AUTOSLEEP_2048	0x03
> +#define MAX7360_AUTOSLEEP_1024	0x04
> +#define MAX7360_AUTOSLEEP_512	0x05
> +#define MAX7360_AUTOSLEEP_256	0x06
> +
> +#define MAX7360_INT_INTI	0
> +#define MAX7360_INT_INTK	1
> +
> +#define MAX7360_INT_GPIO   0
> +#define MAX7360_INT_KEYPAD 1
> +#define MAX7360_INT_ROTARY 2
> +
> +#define MAX7360_NR_INTERNAL_IRQS	3
> +
> +/**
> + * struct max7360
> + * @dev:	Parent device pointer
> + * @i2c:	I2c client pointer
> + * @domain:	Irq domain pointer
> + * @regmap:	Used for I2C communication on accessing registers
> + * @shared_irq:	Irq number if inti and intk pins are connected together
> + * @irq_inti:	Irq number for inti pin
> + * @irq_intk:	Irq number for inik pin
> + */
> +struct max7360 {
> +	struct device		*dev;
> +	struct i2c_client	*i2c;
> +	struct irq_domain	*domain;
> +	struct regmap		*regmap;
> +
> +	int			shared_irq;
> +	int			irq_inti;
> +	int			irq_intk;
> +};

I'd recommend to hide this structure inside the MFD driver, it should
be possible.

> +#endif
> 

--
With best wishes,
Vladimir

Re: [PATCH 2/2] mfd: max7360: Add mfd core device driver

[Lee Jones]

On Fri, 02 Jun 2017, Valentin Sitdikov wrote:

> From: Andrei Dranitca <Andrei_Dranitca@mentor.com>
> 
> This patch adds core/IRQ driver to support MAX7360 I2C chip
> which contains keypad, GPIO, PWM, GPO and rotary encoder submodules.
> 
> Signed-off-by: Andrei Dranitca <Andrei_Dranitca@mentor.com>
> Signed-off-by: Valentin Sitdikov <valentin_sitdikov@mentor.com>
> ---
>  drivers/mfd/Kconfig         |  16 +++
>  drivers/mfd/Makefile        |   1 +
>  drivers/mfd/max7360.c       | 302 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/max7360.h |  90 +++++++++++++
>  4 files changed, 409 insertions(+)
>  create mode 100644 drivers/mfd/max7360.c
>  create mode 100644 include/linux/mfd/max7360.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 3eb5c93..894c2e9 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -721,6 +721,22 @@ config MFD_MAX8998
>  	  additional drivers must be enabled in order to use the functionality
>  	  of the device.
>  
> +config MFD_MAX7360
> +	tristate "Maxim Semiconductor MAX7360 support"
> +	depends on I2C && OF
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	select IRQ_DOMAIN
> +	help
> +	  Say yes here to add support for Maxim Semiconductor MAX7360.
> +	  This provides microprocessors with management of up to 64 key switches,
> +	  with an additional eight LED drivers/GPIOs that feature constant-current,
> +	  PWM intensity control, and rotary switch control options.
> +
> +	  This driver provides common support for accessing the device,
> +	  additional drivers must be enabled in order to use the functionality
> +	  of the device.
> +
>  config MFD_MT6397
>  	tristate "MediaTek MT6397 PMIC Support"
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index c16bf1e..9e721c0 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -137,6 +137,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
>  obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
>  
>  obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
> +obj-$(CONFIG_MFD_MAX7360)	+= max7360.o
>  obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
>  obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
>  obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
> diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
> new file mode 100644
> index 0000000..2bfd3e2
> --- /dev/null
> +++ b/drivers/mfd/max7360.c
> @@ -0,0 +1,302 @@
> +/*
> + * Copyright (C) 2017 Mentor Graphics
> + *
> + * Author: Andrei Dranitca <Andrei_Dranitca@mentor.com>
> + * Author: Valentin Sitdikov <Valentin.Sitdikov@mentor.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max7360.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/slab.h>
> +
> +
> +static const struct mfd_cell max7360_devices[] = {
> +	{
> +		.name           = "max7360-gpio",
> +		.of_compatible	= "maxim,max7360-gpio",
> +	},
> +	{
> +		.name           = "max7360-keypad",
> +		.of_compatible	= "maxim,max7360-keypad",
> +	},
> +	{
> +		.name           = "max7360-pwm",
> +		.of_compatible	= "maxim,max7360-pwm",
> +	},
> +	{
> +		.name           = "max7360-rotary",
> +		.of_compatible	= "maxim,max7360-rotary",
> +	},
> +};
> +
> +static irqreturn_t max7360_irq(int irq, void *data)
> +{
> +	struct max7360 *max7360 = data;
> +	int virq;
> +
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
> +	handle_nested_irq(virq);
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
> +	handle_nested_irq(virq);
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
> +	handle_nested_irq(virq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t max7360_irqi(int irq, void *data)
> +{
> +	struct max7360 *max7360 = data;
> +	int virq;
> +
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
> +	handle_nested_irq(virq);
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
> +	handle_nested_irq(virq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t max7360_irqk(int irq, void *data)
> +{
> +	struct max7360 *max7360 = data;
> +	int virq;
> +
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
> +	handle_nested_irq(virq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int max7360_irq_map(struct irq_domain *d, unsigned int virq,
> +			  irq_hw_number_t hwirq)
> +{
> +	struct max7360 *max7360 = d->host_data;
> +
> +	irq_set_chip_data(virq, max7360);
> +	irq_set_chip_and_handler(virq, &dummy_irq_chip,
> +				handle_edge_irq);
> +	irq_set_nested_thread(virq, 1);
> +	irq_set_noprobe(virq);
> +
> +	return 0;
> +}
> +
> +static void max7360_irq_unmap(struct irq_domain *d, unsigned int virq)
> +{
> +	irq_set_chip_and_handler(virq, NULL, NULL);
> +	irq_set_chip_data(virq, NULL);
> +}
> +
> +static const struct irq_domain_ops max7360_irq_ops = {
> +	.map    = max7360_irq_map,
> +	.unmap  = max7360_irq_unmap,
> +	.xlate  = irq_domain_xlate_onecell,
> +};
> +
> +static void max7360_free_irqs(struct max7360 *max7360)
> +{
> +	if (max7360->shared_irq)
> +		free_irq(max7360->shared_irq, max7360);
> +	else {
> +		free_irq(max7360->irq_inti, max7360);
> +		free_irq(max7360->irq_intk, max7360);
> +	}
> +}
> +
> +static int max7360_irq_init(struct max7360 *max7360, struct device_node *np)
> +{
> +	int ret;
> +
> +	max7360->irq_inti = of_irq_get_byname(np, "inti");
> +	if (max7360->irq_inti < 0) {
> +		dev_err(max7360->dev, "no inti provided");
> +		return -ENODEV;
> +	}
> +
> +	max7360->irq_intk = of_irq_get_byname(np, "intk");
> +	if (max7360->irq_intk < 0) {
> +		dev_err(max7360->dev, "no intk provided");
> +		return -ENODEV;
> +	}
> +
> +	if (max7360->irq_inti == max7360->irq_intk) {
> +		/*
> +		 * In case of pin inti and pin intk are the connected
> +		 * to the same soc`s irq pin.
> +		 */
> +		max7360->shared_irq = max7360->irq_inti;
> +		ret = request_threaded_irq(max7360->shared_irq, NULL,
> +					  max7360_irq,
> +					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					  "max7360", max7360);
> +		if (ret) {
> +			dev_err(max7360->dev, "failed to request IRQ: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	} else {
> +		max7360->shared_irq = 0;
> +		ret = request_threaded_irq(max7360->irq_inti, NULL,
> +					  max7360_irqi,
> +					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					  "max7360", max7360);
> +		if (ret) {
> +			dev_err(max7360->dev, "failed to request inti IRQ: %d\n",
> +			       ret);
> +			return ret;
> +		}
> +
> +		ret = request_threaded_irq(max7360->irq_intk, NULL,
> +					  max7360_irqk,
> +					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					  "max7360", max7360);
> +		if (ret) {
> +			free_irq(max7360->irq_inti, max7360);
> +			dev_err(max7360->dev, "failed to request intk IRQ: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
> +	max7360->domain = irq_domain_add_simple(np, MAX7360_NR_INTERNAL_IRQS,
> +					       0, &max7360_irq_ops, max7360);
> +
> +	if (!max7360->domain) {
> +		max7360_free_irqs(max7360);
> +		dev_err(max7360->dev, "Failed to create irqdomain\n");
> +		return -ENODEV;
> +	}
> +
> +	ret = irq_create_mapping(max7360->domain, MAX7360_INT_GPIO);
> +	if (!ret) {
> +		max7360_free_irqs(max7360);
> +		dev_err(max7360->dev, "Failed to map GPIO IRQ\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = irq_create_mapping(max7360->domain, MAX7360_INT_KEYPAD);
> +	if (!ret) {
> +		max7360_free_irqs(max7360);
> +		dev_err(max7360->dev, "Failed to map KEYPAD IRQ\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = irq_create_mapping(max7360->domain, MAX7360_INT_ROTARY);
> +	if (!ret) {
> +		max7360_free_irqs(max7360);
> +		dev_err(max7360->dev, "Failed to map ROTARY IRQ\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +static int max7360_device_init(struct max7360 *max7360)
> +{
> +	int ret;
> +
> +	ret = mfd_add_devices(max7360->dev, PLATFORM_DEVID_NONE,
> +			     max7360_devices,
> +			     ARRAY_SIZE(max7360_devices), NULL,
> +			     0, max7360->domain);
> +	if (ret)
> +		dev_err(max7360->dev, "failed to add child devices\n");
> +
> +	return ret;
> +}
> +
> +static const struct regmap_range max7360_volatile_ranges[] = {
> +	{
> +		.range_min = MAX7360_REG_KEYFIFO,
> +		.range_max = MAX7360_REG_KEYFIFO,
> +	}, {
> +		.range_min = MAX7360_REG_GPIOIN,
> +		.range_max = MAX7360_REG_GPIOIN,
> +	},
> +};
> +
> +static const struct regmap_access_table max7360_volatile_table = {
> +	.yes_ranges = max7360_volatile_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
> +};
> +
> +static const struct regmap_config max7360_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = 0xff,
> +	.volatile_table = &max7360_volatile_table,
> +	.cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int max7360_probe(struct i2c_client *i2c,
> +			const struct i2c_device_id *id)
> +{
> +	struct device_node *np = i2c->dev.of_node;
> +	struct max7360 *max7360;
> +	int ret;
> +
> +	max7360 = devm_kzalloc(&i2c->dev, sizeof(struct max7360),
> +			      GFP_KERNEL);
> +	if (!max7360)
> +		return -ENOMEM;
> +
> +	max7360->dev = &i2c->dev;
> +	max7360->i2c = i2c;
> +
> +	i2c_set_clientdata(i2c, max7360);
> +
> +	max7360->regmap = devm_regmap_init_i2c(i2c, &max7360_regmap_config);
> +
> +	ret = max7360_irq_init(max7360, np);
> +	if (ret)
> +		return ret;
> +
> +	ret = max7360_device_init(max7360);
> +	if (ret) {
> +		dev_err(max7360->dev, "failed to add child devices\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int max7360_remove(struct i2c_client *client)
> +{
> +	struct max7360 *max7360 = i2c_get_clientdata(client);
> +
> +	mfd_remove_devices(max7360->dev);

Any reason you can't use devm_*?

Then this whole function can go away.

> +	return 0;
> +}
> +
> +static const struct of_device_id max7360_match[] = {
> +	{ .compatible = "maxim,max7360" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, max7360_match);
> +
> +static struct i2c_driver max7360_driver = {
> +	.driver = {
> +		.name	= "max7360",
> +		.of_match_table = max7360_match,
> +	},
> +	.probe		= max7360_probe,
> +	.remove		= max7360_remove,

I'd rather you didn't try to line up the '='.

It doesn't improve the code/readability IMHO.

> +};
> +

Remove this line.

> +builtin_i2c_driver(max7360_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("MAX7360 MFD core driver");
> diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
> new file mode 100644
> index 0000000..c350d5e
> --- /dev/null
> +++ b/include/linux/mfd/max7360.h
> @@ -0,0 +1,90 @@
> +#ifndef __LINUX_MFD_MAX7360_H
> +#define __LINUX_MFD_MAX7360_H
> +#include <linux/regmap.h>
> +
> +#define MAX7360_MAX_KEY_ROWS	8
> +#define MAX7360_MAX_KEY_COLS	8
> +#define MAX7360_MAX_KEY_NUM	(MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
> +#define MAX7360_ROW_SHIFT	3
> +
> +#define MAX7360_MAX_GPIO 8
> +#define MAX7360_MAX_GPO 6
> +#define MAX7360_COL_GPO_PINS 8
> +/*
> + * MAX7360 registers
> + */
> +#define MAX7360_REG_KEYFIFO	0x00
> +#define MAX7360_REG_CONFIG	0x01
> +#define MAX7360_REG_DEBOUNCE	0x02
> +#define MAX7360_REG_INTERRUPT	0x03
> +#define MAX7360_REG_PORTS	0x04
> +#define MAX7360_REG_KEYREP	0x05
> +#define MAX7360_REG_SLEEP	0x06
> +
> +/*
> + * MAX7360 registers
> + */
> +#define MAX7360_REG_GPIOCFG	0x40
> +#define MAX7360_REG_GPIOCTRL	0x41
> +#define MAX7360_REG_GPIODEB	0x42
> +#define MAX7360_REG_GPIOCURR	0x43
> +#define MAX7360_REG_GPIOOUTM	0x44
> +#define MAX7360_REG_PWMCOM	0x45
> +#define MAX7360_REG_RTRCFG	0x46
> +#define MAX7360_REG_GPIOIN	0x49
> +#define MAX7360_REG_RTR_CNT	0x4A
> +#define MAX7360_REG_PWMBASE	0x50
> +#define MAX7360_REG_PWMCFG	0x58
> +
> +
> +#define MAX7360_REG_PORTCFGBASE 0x58
> +
> +/*
> + * Configuration register bits
> + */
> +#define MAX7360_CFG_SLEEP	BIT(7)
> +#define MAX7360_CFG_INTERRUPT	BIT(5)
> +#define MAX7360_CFG_KEY_RELEASE	BIT(3)
> +#define MAX7360_CFG_WAKEUP	BIT(1)
> +#define MAX7360_CFG_TIMEOUT	BIT(0)
> +
> +/*
> + * Autosleep register values (ms)
> + */
> +#define MAX7360_AUTOSLEEP_8192	0x01
> +#define MAX7360_AUTOSLEEP_4096	0x02
> +#define MAX7360_AUTOSLEEP_2048	0x03
> +#define MAX7360_AUTOSLEEP_1024	0x04
> +#define MAX7360_AUTOSLEEP_512	0x05
> +#define MAX7360_AUTOSLEEP_256	0x06
> +
> +#define MAX7360_INT_INTI	0
> +#define MAX7360_INT_INTK	1
> +
> +#define MAX7360_INT_GPIO   0
> +#define MAX7360_INT_KEYPAD 1
> +#define MAX7360_INT_ROTARY 2
> +
> +#define MAX7360_NR_INTERNAL_IRQS	3
> +
> +/**
> + * struct max7360
> + * @dev: Parent device pointer
> + * @i2c: I2c client pointer
> + * @domain: Irq domain pointer
> + * @regmap: Used for I2C communication on accessing registers
> + * shared_irq: Irq number if inti and intk pins are connected together
> + * irq_inti: Irq number for inti pin
> + * irq_intk: Irq number for inik pin
> + */

Don't you think:

> + * @dev:		Parent device pointer
> + * @i2c: 		I2c client pointer
> + * @domain: 		Irq domain pointer
> + * @regmap: 		Used for I2C communication on accessing registers
> + * shared_irq: 	Irq number if inti and intk pins are connected together
> + * irq_inti: 	Irq number for inti pin
> + * irq_intk: 	Irq number for inik pin

.. is better?

> +struct max7360 {
> +	struct device *dev;
> +	struct i2c_client *i2c;
> +	struct irq_domain *domain;
> +	struct regmap *regmap;
> +
> +	int shared_irq;
> +	int irq_inti;
> +	int irq_intk;

Same here:

> +	struct device		*dev;
> +	struct i2c_client	*i2c;
> +	struct irq_domain	*domain;
> +	struct regmap 		*regmap;
> +
> +	int			shared_irq;
> +	int 			irq_inti;
> +	int 			irq_intk;

> +};
> +#endif

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 2/2] mfd: max7360: Add mfd core device driver

[Valentin Sitdikov]

From: Andrei Dranitca <Andrei_Dranitca@mentor.com>

This patch adds core/IRQ driver to support MAX7360 I2C chip
which contains keypad, GPIO, PWM, GPO and rotary encoder submodules.

Signed-off-by: Andrei Dranitca <Andrei_Dranitca@mentor.com>
Signed-off-by: Valentin Sitdikov <valentin_sitdikov@mentor.com>
---
 drivers/mfd/Kconfig         |  16 +++
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/max7360.c       | 302 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/max7360.h |  90 +++++++++++++
 4 files changed, 409 insertions(+)
 create mode 100644 drivers/mfd/max7360.c
 create mode 100644 include/linux/mfd/max7360.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3eb5c93..894c2e9 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -721,6 +721,22 @@ config MFD_MAX8998
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MAX7360
+	tristate "Maxim Semiconductor MAX7360 support"
+	depends on I2C && OF
+	select MFD_CORE
+	select REGMAP_I2C
+	select IRQ_DOMAIN
+	help
+	  Say yes here to add support for Maxim Semiconductor MAX7360.
+	  This provides microprocessors with management of up to 64 key switches,
+	  with an additional eight LED drivers/GPIOs that feature constant-current,
+	  PWM intensity control, and rotary switch control options.
+
+	  This driver provides common support for accessing the device,
+	  additional drivers must be enabled in order to use the functionality
+	  of the device.
+
 config MFD_MT6397
 	tristate "MediaTek MT6397 PMIC Support"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c16bf1e..9e721c0 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -137,6 +137,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
 obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
 
 obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
+obj-$(CONFIG_MFD_MAX7360)	+= max7360.o
 obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
 obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
 obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
new file mode 100644
index 0000000..2bfd3e2
--- /dev/null
+++ b/drivers/mfd/max7360.c
@@ -0,0 +1,302 @@
+/*
+ * Copyright (C) 2017 Mentor Graphics
+ *
+ * Author: Andrei Dranitca <Andrei_Dranitca@mentor.com>
+ * Author: Valentin Sitdikov <Valentin.Sitdikov@mentor.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max7360.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+
+static const struct mfd_cell max7360_devices[] = {
+	{
+		.name           = "max7360-gpio",
+		.of_compatible	= "maxim,max7360-gpio",
+	},
+	{
+		.name           = "max7360-keypad",
+		.of_compatible	= "maxim,max7360-keypad",
+	},
+	{
+		.name           = "max7360-pwm",
+		.of_compatible	= "maxim,max7360-pwm",
+	},
+	{
+		.name           = "max7360-rotary",
+		.of_compatible	= "maxim,max7360-rotary",
+	},
+};
+
+static irqreturn_t max7360_irq(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqi(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqk(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static int max7360_irq_map(struct irq_domain *d, unsigned int virq,
+			  irq_hw_number_t hwirq)
+{
+	struct max7360 *max7360 = d->host_data;
+
+	irq_set_chip_data(virq, max7360);
+	irq_set_chip_and_handler(virq, &dummy_irq_chip,
+				handle_edge_irq);
+	irq_set_nested_thread(virq, 1);
+	irq_set_noprobe(virq);
+
+	return 0;
+}
+
+static void max7360_irq_unmap(struct irq_domain *d, unsigned int virq)
+{
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+static const struct irq_domain_ops max7360_irq_ops = {
+	.map    = max7360_irq_map,
+	.unmap  = max7360_irq_unmap,
+	.xlate  = irq_domain_xlate_onecell,
+};
+
+static void max7360_free_irqs(struct max7360 *max7360)
+{
+	if (max7360->shared_irq)
+		free_irq(max7360->shared_irq, max7360);
+	else {
+		free_irq(max7360->irq_inti, max7360);
+		free_irq(max7360->irq_intk, max7360);
+	}
+}
+
+static int max7360_irq_init(struct max7360 *max7360, struct device_node *np)
+{
+	int ret;
+
+	max7360->irq_inti = of_irq_get_byname(np, "inti");
+	if (max7360->irq_inti < 0) {
+		dev_err(max7360->dev, "no inti provided");
+		return -ENODEV;
+	}
+
+	max7360->irq_intk = of_irq_get_byname(np, "intk");
+	if (max7360->irq_intk < 0) {
+		dev_err(max7360->dev, "no intk provided");
+		return -ENODEV;
+	}
+
+	if (max7360->irq_inti == max7360->irq_intk) {
+		/*
+		 * In case of pin inti and pin intk are the connected
+		 * to the same soc`s irq pin.
+		 */
+		max7360->shared_irq = max7360->irq_inti;
+		ret = request_threaded_irq(max7360->shared_irq, NULL,
+					  max7360_irq,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			dev_err(max7360->dev, "failed to request IRQ: %d\n",
+				ret);
+			return ret;
+		}
+	} else {
+		max7360->shared_irq = 0;
+		ret = request_threaded_irq(max7360->irq_inti, NULL,
+					  max7360_irqi,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			dev_err(max7360->dev, "failed to request inti IRQ: %d\n",
+			       ret);
+			return ret;
+		}
+
+		ret = request_threaded_irq(max7360->irq_intk, NULL,
+					  max7360_irqk,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			free_irq(max7360->irq_inti, max7360);
+			dev_err(max7360->dev, "failed to request intk IRQ: %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	max7360->domain = irq_domain_add_simple(np, MAX7360_NR_INTERNAL_IRQS,
+					       0, &max7360_irq_ops, max7360);
+
+	if (!max7360->domain) {
+		max7360_free_irqs(max7360);
+		dev_err(max7360->dev, "Failed to create irqdomain\n");
+		return -ENODEV;
+	}
+
+	ret = irq_create_mapping(max7360->domain, MAX7360_INT_GPIO);
+	if (!ret) {
+		max7360_free_irqs(max7360);
+		dev_err(max7360->dev, "Failed to map GPIO IRQ\n");
+		return -EINVAL;
+	}
+
+	ret = irq_create_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	if (!ret) {
+		max7360_free_irqs(max7360);
+		dev_err(max7360->dev, "Failed to map KEYPAD IRQ\n");
+		return -EINVAL;
+	}
+
+	ret = irq_create_mapping(max7360->domain, MAX7360_INT_ROTARY);
+	if (!ret) {
+		max7360_free_irqs(max7360);
+		dev_err(max7360->dev, "Failed to map ROTARY IRQ\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+static int max7360_device_init(struct max7360 *max7360)
+{
+	int ret;
+
+	ret = mfd_add_devices(max7360->dev, PLATFORM_DEVID_NONE,
+			     max7360_devices,
+			     ARRAY_SIZE(max7360_devices), NULL,
+			     0, max7360->domain);
+	if (ret)
+		dev_err(max7360->dev, "failed to add child devices\n");
+
+	return ret;
+}
+
+static const struct regmap_range max7360_volatile_ranges[] = {
+	{
+		.range_min = MAX7360_REG_KEYFIFO,
+		.range_max = MAX7360_REG_KEYFIFO,
+	}, {
+		.range_min = MAX7360_REG_GPIOIN,
+		.range_max = MAX7360_REG_GPIOIN,
+	},
+};
+
+static const struct regmap_access_table max7360_volatile_table = {
+	.yes_ranges = max7360_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
+};
+
+static const struct regmap_config max7360_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0xff,
+	.volatile_table = &max7360_volatile_table,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+static int max7360_probe(struct i2c_client *i2c,
+			const struct i2c_device_id *id)
+{
+	struct device_node *np = i2c->dev.of_node;
+	struct max7360 *max7360;
+	int ret;
+
+	max7360 = devm_kzalloc(&i2c->dev, sizeof(struct max7360),
+			      GFP_KERNEL);
+	if (!max7360)
+		return -ENOMEM;
+
+	max7360->dev = &i2c->dev;
+	max7360->i2c = i2c;
+
+	i2c_set_clientdata(i2c, max7360);
+
+	max7360->regmap = devm_regmap_init_i2c(i2c, &max7360_regmap_config);
+
+	ret = max7360_irq_init(max7360, np);
+	if (ret)
+		return ret;
+
+	ret = max7360_device_init(max7360);
+	if (ret) {
+		dev_err(max7360->dev, "failed to add child devices\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int max7360_remove(struct i2c_client *client)
+{
+	struct max7360 *max7360 = i2c_get_clientdata(client);
+
+	mfd_remove_devices(max7360->dev);
+
+	return 0;
+}
+
+static const struct of_device_id max7360_match[] = {
+	{ .compatible = "maxim,max7360" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, max7360_match);
+
+static struct i2c_driver max7360_driver = {
+	.driver = {
+		.name	= "max7360",
+		.of_match_table = max7360_match,
+	},
+	.probe		= max7360_probe,
+	.remove		= max7360_remove,
+};
+
+builtin_i2c_driver(max7360_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MAX7360 MFD core driver");
diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
new file mode 100644
index 0000000..c350d5e
--- /dev/null
+++ b/include/linux/mfd/max7360.h
@@ -0,0 +1,90 @@
+#ifndef __LINUX_MFD_MAX7360_H
+#define __LINUX_MFD_MAX7360_H
+#include <linux/regmap.h>
+
+#define MAX7360_MAX_KEY_ROWS	8
+#define MAX7360_MAX_KEY_COLS	8
+#define MAX7360_MAX_KEY_NUM	(MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
+#define MAX7360_ROW_SHIFT	3
+
+#define MAX7360_MAX_GPIO 8
+#define MAX7360_MAX_GPO 6
+#define MAX7360_COL_GPO_PINS 8
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_KEYFIFO	0x00
+#define MAX7360_REG_CONFIG	0x01
+#define MAX7360_REG_DEBOUNCE	0x02
+#define MAX7360_REG_INTERRUPT	0x03
+#define MAX7360_REG_PORTS	0x04
+#define MAX7360_REG_KEYREP	0x05
+#define MAX7360_REG_SLEEP	0x06
+
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_GPIOCFG	0x40
+#define MAX7360_REG_GPIOCTRL	0x41
+#define MAX7360_REG_GPIODEB	0x42
+#define MAX7360_REG_GPIOCURR	0x43
+#define MAX7360_REG_GPIOOUTM	0x44
+#define MAX7360_REG_PWMCOM	0x45
+#define MAX7360_REG_RTRCFG	0x46
+#define MAX7360_REG_GPIOIN	0x49
+#define MAX7360_REG_RTR_CNT	0x4A
+#define MAX7360_REG_PWMBASE	0x50
+#define MAX7360_REG_PWMCFG	0x58
+
+
+#define MAX7360_REG_PORTCFGBASE 0x58
+
+/*
+ * Configuration register bits
+ */
+#define MAX7360_CFG_SLEEP	BIT(7)
+#define MAX7360_CFG_INTERRUPT	BIT(5)
+#define MAX7360_CFG_KEY_RELEASE	BIT(3)
+#define MAX7360_CFG_WAKEUP	BIT(1)
+#define MAX7360_CFG_TIMEOUT	BIT(0)
+
+/*
+ * Autosleep register values (ms)
+ */
+#define MAX7360_AUTOSLEEP_8192	0x01
+#define MAX7360_AUTOSLEEP_4096	0x02
+#define MAX7360_AUTOSLEEP_2048	0x03
+#define MAX7360_AUTOSLEEP_1024	0x04
+#define MAX7360_AUTOSLEEP_512	0x05
+#define MAX7360_AUTOSLEEP_256	0x06
+
+#define MAX7360_INT_INTI	0
+#define MAX7360_INT_INTK	1
+
+#define MAX7360_INT_GPIO   0
+#define MAX7360_INT_KEYPAD 1
+#define MAX7360_INT_ROTARY 2
+
+#define MAX7360_NR_INTERNAL_IRQS	3
+
+/**
+ * struct max7360
+ * @dev: Parent device pointer
+ * @i2c: I2c client pointer
+ * @domain: Irq domain pointer
+ * @regmap: Used for I2C communication on accessing registers
+ * shared_irq: Irq number if inti and intk pins are connected together
+ * irq_inti: Irq number for inti pin
+ * irq_intk: Irq number for inik pin
+ */
+struct max7360 {
+	struct device *dev;
+	struct i2c_client *i2c;
+	struct irq_domain *domain;
+	struct regmap *regmap;
+
+	int shared_irq;
+	int irq_inti;
+	int irq_intk;
+};
+#endif
-- 
2.9.3

Re: [PATCH 2/2] mfd: max7360: Add mfd core device driver

[Lee Jones]

On Thu, 18 May 2017, Valentin Sitdikov wrote:

> From: Andrei Dranitca <Andrei_Dranitca@mentor.com>
> 
> This patch adds core/irq driver to support MAX7360 i2c chip

IRQ and I2C

> which contains keypad, gpio, pwm, gpo and rotary encoder submodules.

GPIO, PWM and GPO

> Signed-off-by: Valentin Sitdikov <valentin_sitdikov@mentor.com>
> Signed-off-by: Andrei Dranitca <Andrei_Dranitca@mentor.com>

These are in the wrong order.

> ---
>  drivers/mfd/Kconfig         |  16 ++
>  drivers/mfd/Makefile        |   1 +
>  drivers/mfd/max7360.c       | 397 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/max7360.h | 130 +++++++++++++++
>  4 files changed, 544 insertions(+)
>  create mode 100644 drivers/mfd/max7360.c
>  create mode 100644 include/linux/mfd/max7360.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 3eb5c93..894c2e9 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -721,6 +721,22 @@ config MFD_MAX8998
>  	  additional drivers must be enabled in order to use the functionality
>  	  of the device.
>  
> +config MFD_MAX7360
> +	tristate "Maxim Semiconductor MAX7360 support"
> +	depends on I2C && OF
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	select IRQ_DOMAIN
> +	help
> +	  Say yes here to add support for Maxim Semiconductor MAX7360.
> +	  This provides microprocessors with management of up to 64 key switches,
> +	  with an additional eight LED drivers/GPIOs that feature constant-current,
> +	  PWM intensity control, and rotary switch control options.
> +
> +	  This driver provides common support for accessing the device,
> +	  additional drivers must be enabled in order to use the functionality
> +	  of the device.
> +
>  config MFD_MT6397
>  	tristate "MediaTek MT6397 PMIC Support"
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index c16bf1e..9e721c0 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -137,6 +137,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
>  obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
>  
>  obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
> +obj-$(CONFIG_MFD_MAX7360)	+= max7360.o
>  obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
>  obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
>  obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
> diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
> new file mode 100644
> index 0000000..566434e
> --- /dev/null
> +++ b/drivers/mfd/max7360.c
> @@ -0,0 +1,397 @@
> +/*
> + * Copyright (C) 2017 Mentor Graphics
> + *
> + * Author: Valentin Sitdikov <Valentin.Sitdikov@mentor.com>
> + * Author: Andrei Dranitca <Andrei_Dranitca@mentor.com>

Order?

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

Remove the line above.

Do you have to use the full licence header?  There is a short version,
any reason why you can't use it?

> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max7360.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/slab.h>
> +
> +
> +int max7360_request_pin(struct max7360 *max7360, u8 pin)
> +{
> +	struct i2c_client *client = max7360->i2c;
> +	int ret = 0;

No need to pre-initialise.  Just return 0 at the end.

> +	spin_lock(&max7360->lock);
> +	if (max7360->gpio_pins & BIT(pin)) {
> +		dev_err(&client->dev, "pin %d already requested, mask %x",
> +		       pin, max7360->gpio_pins);
> +		spin_unlock(&max7360->lock);
> +		return -EBUSY;
> +	}
> +	max7360->gpio_pins |= BIT(pin);
> +	dev_dbg(&client->dev, "pin %d requested successfully", pin);
> +	spin_unlock(&max7360->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(max7360_request_pin);
> +
> +void max7360_free_pin(struct max7360 *max7360, u8 pin)
> +{
> +	spin_lock(&max7360->lock);
> +	max7360->gpio_pins &= ~BIT(pin);
> +	spin_unlock(&max7360->lock);
> +}
> +EXPORT_SYMBOL_GPL(max7360_free_pin);

What do these pins do?  Are they GPIOs?

If so, why aren't you using the GPIO API?

> +static const struct mfd_cell max7360_devices[] = {
> +	{
> +		.name           = "max7360-gpio",
> +		.of_compatible	= "maxim,max7360-gpio",
> +	},
> +	{
> +		.name           = "max7360-keypad",
> +		.of_compatible	= "maxim,max7360-keypad",
> +	},
> +	{
> +		.name           = "max7360-pwm",
> +		.of_compatible	= "maxim,max7360-pwm",
> +	},
> +	{
> +		.name           = "max7360-rotary",
> +		.of_compatible	= "maxim,max7360-rotary",
> +	},
> +};
> +
> +static irqreturn_t max7360_irq(int irq, void *data)
> +{
> +	struct max7360 *max7360 = data;
> +	int virq;
> +
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
> +	handle_nested_irq(virq);
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
> +	handle_nested_irq(virq);
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
> +	handle_nested_irq(virq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t max7360_irqi(int irq, void *data)
> +{
> +	struct max7360 *max7360 = data;
> +	int virq;
> +
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
> +	handle_nested_irq(virq);
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
> +	handle_nested_irq(virq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t max7360_irqk(int irq, void *data)
> +{
> +	struct max7360 *max7360 = data;
> +	int virq;
> +
> +	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
> +	handle_nested_irq(virq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int max7360_irq_map(struct irq_domain *d, unsigned int virq,
> +			  irq_hw_number_t hwirq)
> +{
> +	struct max7360 *max7360 = d->host_data;
> +
> +	irq_set_chip_data(virq, max7360);
> +	irq_set_chip_and_handler(virq, &dummy_irq_chip,
> +				handle_edge_irq);
> +	irq_set_nested_thread(virq, 1);
> +	irq_set_noprobe(virq);
> +
> +	return 0;
> +}
> +
> +static void max7360_irq_unmap(struct irq_domain *d, unsigned int virq)
> +{
> +	irq_set_chip_and_handler(virq, NULL, NULL);
> +	irq_set_chip_data(virq, NULL);
> +}
> +
> +static const struct irq_domain_ops max7360_irq_ops = {
> +	.map    = max7360_irq_map,
> +	.unmap  = max7360_irq_unmap,
> +	.xlate  = irq_domain_xlate_onecell,
> +};
> +
> +static int max7360_irq_init(struct max7360 *max7360, struct device_node *np)
> +{
> +	int ret;
> +
> +	max7360->inti = of_irq_get_byname(np, "inti");
> +	max7360->intk = of_irq_get_byname(np, "intk");
> +
> +	if (max7360->inti < 0) {
> +		dev_err(max7360->dev, "no inti provided");
> +		return -ENODEV;
> +	}
> +
> +	if (max7360->intk < 0) {
> +		dev_err(max7360->dev, "no intk provided");
> +		return -ENODEV;
> +	}

It's more conformant to place the ifs directly after where the
variable under test is initialised.

> +	if (max7360->inti == max7360->intk) {

You need a comment here to explain exactly why this is required.

> +		max7360->shared_irq = max7360->inti;
> +		ret = request_threaded_irq(max7360->shared_irq, NULL,
> +					  max7360_irq,
> +					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					  "max7360", max7360);
> +		if (ret) {
> +			dev_err(max7360->dev, "failed to request IRQ: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	} else {
> +		max7360->shared_irq = 0;
> +		ret = request_threaded_irq(max7360->inti, NULL, max7360_irqi,
> +					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					  "max7360", max7360);
> +		if (ret) {
> +			dev_err(max7360->dev, "failed to request inti IRQ: %d\n",
> +			       ret);
> +			return ret;
> +		}
> +
> +		ret = request_threaded_irq(max7360->intk, NULL, max7360_irqk,
> +					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					  "max7360", max7360);
> +		if (ret) {
> +			free_irq(max7360->inti, max7360);
> +			dev_err(max7360->dev, "failed to request intk IRQ: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
> +	max7360->domain = irq_domain_add_simple(np, MAX7360_NR_INTERNAL_IRQS,
> +					       0, &max7360_irq_ops, max7360);
> +
> +	if (!max7360->domain) {
> +		if (max7360->shared_irq)
> +			free_irq(max7360->shared_irq, max7360);
> +		else {
> +			free_irq(max7360->inti, max7360);
> +			free_irq(max7360->intk, max7360);
> +		}
> +		dev_err(max7360->dev, "Failed to create irqdomain\n");
> +		return -ENODEV;
> +	}
> +
> +	irq_create_mapping(max7360->domain, MAX7360_INT_GPIO);
> +	irq_create_mapping(max7360->domain, MAX7360_INT_KEYPAD);
> +	irq_create_mapping(max7360->domain, MAX7360_INT_ROTARY);

Why aren't you checking the return values of these calls?

> +	return 0;
> +}
> +
> +void max7360_fall_deepsleep(struct max7360 *max7360)

Drop the "_fall"

> +{
> +	max7360_write_reg(max7360, MAX7360_REG_SLEEP, MAX7360_AUTOSLEEP_8192);
> +}
> +EXPORT_SYMBOL_GPL(max7360_fall_deepsleep);
> +
> +void max7360_take_catnap(struct max7360 *max7360)

The naming of the function can and should be improved.

> +{
> +	max7360_write_reg(max7360, MAX7360_REG_SLEEP, MAX7360_AUTOSLEEP_256);
> +}
> +EXPORT_SYMBOL_GPL(max7360_take_catnap);

What is calling these functions?

> +static int max7360_chip_init(struct max7360 *max7360)
> +{
> +	max7360->gpio_pins = MAX7360_MAX_GPIO;
> +	max7360->gpo_count = 0;
> +	max7360->col_count = MAX7360_COL_GPO_PINS;

This does not require its own function.

> +	return 0;
> +}
> +
> +static int max7360_device_init(struct max7360 *max7360)
> +{
> +	int ret = 0;

No need to pre-initialise.

> +	ret = mfd_add_devices(max7360->dev, -1, max7360_devices,

Use the #defines, not -1.

> +			     ARRAY_SIZE(max7360_devices), NULL,
> +			     0, max7360->domain);
> +	if (ret)
> +		dev_err(max7360->dev, "failed to add child devices\n");
> +
> +	return ret;
> +}
> +
> +int max7360_request_gpo_pin_count(struct max7360 *max7360, u8 count)
> +{
> +	if (count > MAX7360_MAX_GPO)
> +		return -EINVAL;

'\n'

> +	if (max7360->col_count + count > MAX7360_COL_GPO_PINS) {
> +		dev_err(max7360->dev,
> +		       "trying to request %d pins as gpo while %d pins already used as COL\n",
> +		       count, max7360->col_count);
> +		return -EINVAL;
> +	}
> +	max7360->gpo_count = count;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(max7360_request_gpo_pin_count);
> +
> +int max7360_request_col_count(struct max7360 *max7360, u8 count)
> +{
> +	if (max7360->gpo_count + count > MAX7360_COL_GPO_PINS) {
> +		dev_err(max7360->dev,
> +		       "trying to request %d pins as COL while %d pins already used as gpo\n",
> +		       count, max7360->gpo_count);
> +		return -EINVAL;
> +	}
> +	max7360->col_count = count;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(max7360_request_col_count);

What is the purpose of these two functions?

> +static const struct regmap_range max7360_volatile_ranges[] = {
> +	{
> +		.range_min = MAX7360_REG_KEYFIFO,
> +		.range_max = MAX7360_REG_KEYFIFO,
> +	}, {
> +		.range_min = 0x48,
> +		.range_max = 0x4a,

No magic numbers please.

> +	},
> +};
> +
> +static const struct regmap_access_table max7360_volatile_table = {
> +	.yes_ranges = max7360_volatile_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
> +};
> +
> +static const struct regmap_config max7360_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = 0xff,
> +	.volatile_table = &max7360_volatile_table,
> +	.cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int max7360_probe(struct i2c_client *i2c,
> +			const struct i2c_device_id *id)
> +{
> +	struct device_node *np = i2c->dev.of_node;
> +	struct max7360 *max7360;
> +

Remove this line.

> +	int ret;
> +
> +	max7360 = devm_kzalloc(&i2c->dev, sizeof(struct max7360),
> +			      GFP_KERNEL);
> +	if (!max7360)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&max7360->lock);
> +
> +	max7360->dev = &i2c->dev;
> +	max7360->i2c = i2c;
> +
> +	i2c_set_clientdata(i2c, max7360);
> +
> +	max7360->regmap = devm_regmap_init_i2c(i2c, &max7360_regmap_config);
> +	ret = max7360_chip_init(max7360);
> +	if (ret)
> +		return ret;
> +
> +	ret = max7360_irq_init(max7360, np);
> +	if (ret)
> +		return ret;
> +
> +	ret = max7360_device_init(max7360);
> +	if (ret) {
> +		dev_err(max7360->dev, "failed to add child devices\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int max7360_remove(struct i2c_client *client)
> +{
> +	struct max7360 *max7360 = i2c_get_clientdata(client);
> +
> +	mfd_remove_devices(max7360->dev);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int max7360_suspend(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static int max7360_resume(struct device *dev)
> +{
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(max7360_dev_pm_ops, max7360_suspend, max7360_resume);

Why are you pretending that you support runtime PM?

> +static const struct of_device_id max7360_match[] = {
> +	{ .compatible = "maxim,max7360" },
> +	{ }
> +};
> +

Remove this line.

> +MODULE_DEVICE_TABLE(of, max7360_match);
> +
> +static const struct i2c_device_id max7360_id[] = {
> +	{ "max7360", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, max7360_id);

What are you using this table for?

> +static struct i2c_driver max7360_driver = {
> +	.driver = {
> +		.name	= "max7360",
> +		.pm	= &max7360_dev_pm_ops,
> +		.of_match_table = max7360_match,
> +	},
> +	.probe		= max7360_probe,
> +	.remove		= max7360_remove,
> +	.id_table	= max7360_id,
> +};
> +
> +static int __init max7360_init(void)
> +{
> +	return i2c_add_driver(&max7360_driver);
> +}
> +subsys_initcall(max7360_init);
> +
> +static void __exit max7360_exit(void)
> +{
> +	i2c_del_driver(&max7360_driver);
> +}
> +module_exit(max7360_exit);

This looks like boiler plate.

Please see if there is a helper function for this.

> +MODULE_LICENSE("GPL v2");

This does not match the header comment.

> +MODULE_DESCRIPTION("MAX7360 MFD core driver");
> diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
> new file mode 100644
> index 0000000..d139ddd
> --- /dev/null
> +++ b/include/linux/mfd/max7360.h
> @@ -0,0 +1,130 @@
> +#ifndef __LINUX_MFD_MAX7360_H
> +#define __LINUX_MFD_MAX7360_H
> +#include <linux/regmap.h>
> +
> +#define MAX7360_MAX_KEY_ROWS	8
> +#define MAX7360_MAX_KEY_COLS	8
> +#define MAX7360_MAX_KEY_NUM	(MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
> +#define MAX7360_ROW_SHIFT	3
> +
> +#define MAX7360_MAX_GPIO 8
> +#define MAX7360_MAX_GPO 6
> +#define MAX7360_COL_GPO_PINS 8
> +/*
> + * MAX7360 registers
> + */
> +#define MAX7360_REG_KEYFIFO	0x00
> +#define MAX7360_REG_CONFIG	0x01
> +#define MAX7360_REG_DEBOUNCE	0x02
> +#define MAX7360_REG_INTERRUPT	0x03
> +#define MAX7360_REG_PORTS	0x04
> +#define MAX7360_REG_KEYREP	0x05
> +#define MAX7360_REG_SLEEP	0x06
> +
> +/*
> + * MAX7360 registers
> + */
> +#define MAX7360_REG_GPIOCFG	0x40
> +#define MAX7360_REG_GPIOCTRL	0x41
> +#define MAX7360_REG_GPIODEB	0x42
> +#define MAX7360_REG_GPIOCURR	0x43
> +#define MAX7360_REG_GPIOOUTM	0x44
> +#define MAX7360_REG_PWMCOM	0x45
> +#define MAX7360_REG_RTRCFG	0x46
> +#define MAX7360_REG_GPIOIN	0x49
> +#define MAX7360_REG_RTR_CNT	0x4A
> +#define MAX7360_REG_PWMBASE	0x50
> +#define MAX7360_REG_PWMCFG	0x58
> +
> +
> +#define MAX7360_REG_PORTCFGBASE 0x58
> +
> +/*
> + * Configuration register bits
> + */
> +#define MAX7360_CFG_SLEEP	(1 << 7)
> +#define MAX7360_CFG_INTERRUPT	(1 << 5)
> +#define MAX7360_CFG_KEY_RELEASE	(1 << 3)
> +#define MAX7360_CFG_WAKEUP	(1 << 1)
> +#define MAX7360_CFG_TIMEOUT	(1 << 0)

Use BIT()

> +/*
> + * Autosleep register values (ms)
> + */
> +#define MAX7360_AUTOSLEEP_8192	0x01
> +#define MAX7360_AUTOSLEEP_4096	0x02
> +#define MAX7360_AUTOSLEEP_2048	0x03
> +#define MAX7360_AUTOSLEEP_1024	0x04
> +#define MAX7360_AUTOSLEEP_512	0x05
> +#define MAX7360_AUTOSLEEP_256	0x06
> +
> +#define MAX7360_INT_INTI	0
> +#define MAX7360_INT_INTK	1
> +
> +#define MAX7360_INT_GPIO   0
> +#define MAX7360_INT_KEYPAD 1
> +#define MAX7360_INT_ROTARY 2
> +
> +#define MAX7360_NR_INTERNAL_IRQS	3
> +
> +struct max7360 {
> +	spinlock_t lock;		/* lock access to the structure */
> +	struct device *dev;
> +	struct i2c_client *i2c;
> +	struct irq_domain *domain;
> +	struct regmap *regmap;
> +
> +	int irq_base;
> +	int num_gpio;
> +	int shared_irq;
> +	int inti;
> +	int intk;

At no point do you explain what inti and inik is or the differences
between them.  I suggest you add a kerneldoc header to this struct,
AND consider renaming them to something a little more descriptive,
since 'l' and 'k', even when prefixed with 'int' are not good variable
names.

> +	u8 gpio_pins;
> +	u8 col_count;
> +	u8 gpo_count;
> +};
> +
> +static inline int max7360_read_reg(struct max7360 *max7360, int reg)
> +{
> +	unsigned int ival;
> +	int ret;
> +
> +	ret = regmap_read(max7360->regmap, reg, &ival);
> +	if (!ret)
> +		return ival;
> +	return 0;
> +}
> +
> +static inline int max7360_write_reg(struct max7360 *max7360, u8 reg, u8 val)
> +{
> +	return regmap_write(max7360->regmap, reg, val);
> +}
> +
> +static inline int max7360_set_bits(struct max7360 *max7360, u8 reg,
> +				  unsigned int bit_mask)
> +{
> +	return regmap_update_bits(max7360->regmap, reg, bit_mask, bit_mask);
> +}
> +
> +static inline int max7360_clr_bits(struct max7360 *max7360, u8 reg,
> +				  unsigned int bit_mask)
> +{
> +	return regmap_update_bits(max7360->regmap, reg, bit_mask, 0);
> +}
> +
> +static inline int max7360_update(struct max7360 *max7360, u8 reg, u8 val,
> +				unsigned int bit_mask)
> +{
> +	return regmap_update_bits(max7360->regmap, reg, bit_mask, val);
> +}
> +
> +int max7360_request_pin(struct max7360 *max7360, u8 pin);
> +void max7360_free_pin(struct max7360 *max7360, u8 pin);
> +
> +void max7360_take_catnap(struct max7360 *max7360);
> +void max7360_fall_deepsleep(struct max7360 *max7360);
> +
> +int max7360_request_gpo_pin_count(struct max7360 *max7360, u8 count);
> +int max7360_request_col_count(struct max7360 *max7360, u8 count);

What are all these functions for?

I think you should remove them all.

> +#endif

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 2/2] mfd: max7360: Add mfd core device driver

[Valentin Sitdikov]

From: Andrei Dranitca <Andrei_Dranitca@mentor.com>

This patch adds core/irq driver to support MAX7360 i2c chip
which contains keypad, gpio, pwm, gpo and rotary encoder submodules.

Signed-off-by: Valentin Sitdikov <valentin_sitdikov@mentor.com>
Signed-off-by: Andrei Dranitca <Andrei_Dranitca@mentor.com>
---
 drivers/mfd/Kconfig         |  16 ++
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/max7360.c       | 397 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/max7360.h | 130 +++++++++++++++
 4 files changed, 544 insertions(+)
 create mode 100644 drivers/mfd/max7360.c
 create mode 100644 include/linux/mfd/max7360.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3eb5c93..894c2e9 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -721,6 +721,22 @@ config MFD_MAX8998
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MAX7360
+	tristate "Maxim Semiconductor MAX7360 support"
+	depends on I2C && OF
+	select MFD_CORE
+	select REGMAP_I2C
+	select IRQ_DOMAIN
+	help
+	  Say yes here to add support for Maxim Semiconductor MAX7360.
+	  This provides microprocessors with management of up to 64 key switches,
+	  with an additional eight LED drivers/GPIOs that feature constant-current,
+	  PWM intensity control, and rotary switch control options.
+
+	  This driver provides common support for accessing the device,
+	  additional drivers must be enabled in order to use the functionality
+	  of the device.
+
 config MFD_MT6397
 	tristate "MediaTek MT6397 PMIC Support"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c16bf1e..9e721c0 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -137,6 +137,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
 obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
 
 obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
+obj-$(CONFIG_MFD_MAX7360)	+= max7360.o
 obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
 obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
 obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
new file mode 100644
index 0000000..566434e
--- /dev/null
+++ b/drivers/mfd/max7360.c
@@ -0,0 +1,397 @@
+/*
+ * Copyright (C) 2017 Mentor Graphics
+ *
+ * Author: Valentin Sitdikov <Valentin.Sitdikov@mentor.com>
+ * Author: Andrei Dranitca <Andrei_Dranitca@mentor.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/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max7360.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+
+int max7360_request_pin(struct max7360 *max7360, u8 pin)
+{
+	struct i2c_client *client = max7360->i2c;
+	int ret = 0;
+
+	spin_lock(&max7360->lock);
+	if (max7360->gpio_pins & BIT(pin)) {
+		dev_err(&client->dev, "pin %d already requested, mask %x",
+		       pin, max7360->gpio_pins);
+		spin_unlock(&max7360->lock);
+		return -EBUSY;
+	}
+	max7360->gpio_pins |= BIT(pin);
+	dev_dbg(&client->dev, "pin %d requested successfully", pin);
+	spin_unlock(&max7360->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(max7360_request_pin);
+
+void max7360_free_pin(struct max7360 *max7360, u8 pin)
+{
+	spin_lock(&max7360->lock);
+	max7360->gpio_pins &= ~BIT(pin);
+	spin_unlock(&max7360->lock);
+}
+EXPORT_SYMBOL_GPL(max7360_free_pin);
+
+static const struct mfd_cell max7360_devices[] = {
+	{
+		.name           = "max7360-gpio",
+		.of_compatible	= "maxim,max7360-gpio",
+	},
+	{
+		.name           = "max7360-keypad",
+		.of_compatible	= "maxim,max7360-keypad",
+	},
+	{
+		.name           = "max7360-pwm",
+		.of_compatible	= "maxim,max7360-pwm",
+	},
+	{
+		.name           = "max7360-rotary",
+		.of_compatible	= "maxim,max7360-rotary",
+	},
+};
+
+static irqreturn_t max7360_irq(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqi(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqk(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static int max7360_irq_map(struct irq_domain *d, unsigned int virq,
+			  irq_hw_number_t hwirq)
+{
+	struct max7360 *max7360 = d->host_data;
+
+	irq_set_chip_data(virq, max7360);
+	irq_set_chip_and_handler(virq, &dummy_irq_chip,
+				handle_edge_irq);
+	irq_set_nested_thread(virq, 1);
+	irq_set_noprobe(virq);
+
+	return 0;
+}
+
+static void max7360_irq_unmap(struct irq_domain *d, unsigned int virq)
+{
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+static const struct irq_domain_ops max7360_irq_ops = {
+	.map    = max7360_irq_map,
+	.unmap  = max7360_irq_unmap,
+	.xlate  = irq_domain_xlate_onecell,
+};
+
+static int max7360_irq_init(struct max7360 *max7360, struct device_node *np)
+{
+	int ret;
+
+	max7360->inti = of_irq_get_byname(np, "inti");
+	max7360->intk = of_irq_get_byname(np, "intk");
+
+	if (max7360->inti < 0) {
+		dev_err(max7360->dev, "no inti provided");
+		return -ENODEV;
+	}
+
+	if (max7360->intk < 0) {
+		dev_err(max7360->dev, "no intk provided");
+		return -ENODEV;
+	}
+
+	if (max7360->inti == max7360->intk) {
+		max7360->shared_irq = max7360->inti;
+		ret = request_threaded_irq(max7360->shared_irq, NULL,
+					  max7360_irq,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			dev_err(max7360->dev, "failed to request IRQ: %d\n",
+				ret);
+			return ret;
+		}
+	} else {
+		max7360->shared_irq = 0;
+		ret = request_threaded_irq(max7360->inti, NULL, max7360_irqi,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			dev_err(max7360->dev, "failed to request inti IRQ: %d\n",
+			       ret);
+			return ret;
+		}
+
+		ret = request_threaded_irq(max7360->intk, NULL, max7360_irqk,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			free_irq(max7360->inti, max7360);
+			dev_err(max7360->dev, "failed to request intk IRQ: %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	max7360->domain = irq_domain_add_simple(np, MAX7360_NR_INTERNAL_IRQS,
+					       0, &max7360_irq_ops, max7360);
+
+	if (!max7360->domain) {
+		if (max7360->shared_irq)
+			free_irq(max7360->shared_irq, max7360);
+		else {
+			free_irq(max7360->inti, max7360);
+			free_irq(max7360->intk, max7360);
+		}
+		dev_err(max7360->dev, "Failed to create irqdomain\n");
+		return -ENODEV;
+	}
+
+	irq_create_mapping(max7360->domain, MAX7360_INT_GPIO);
+	irq_create_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	irq_create_mapping(max7360->domain, MAX7360_INT_ROTARY);
+
+	return 0;
+}
+
+void max7360_fall_deepsleep(struct max7360 *max7360)
+{
+	max7360_write_reg(max7360, MAX7360_REG_SLEEP, MAX7360_AUTOSLEEP_8192);
+}
+EXPORT_SYMBOL_GPL(max7360_fall_deepsleep);
+
+void max7360_take_catnap(struct max7360 *max7360)
+{
+	max7360_write_reg(max7360, MAX7360_REG_SLEEP, MAX7360_AUTOSLEEP_256);
+}
+EXPORT_SYMBOL_GPL(max7360_take_catnap);
+
+static int max7360_chip_init(struct max7360 *max7360)
+{
+	max7360->gpio_pins = MAX7360_MAX_GPIO;
+	max7360->gpo_count = 0;
+	max7360->col_count = MAX7360_COL_GPO_PINS;
+	return 0;
+}
+
+static int max7360_device_init(struct max7360 *max7360)
+{
+	int ret = 0;
+
+	ret = mfd_add_devices(max7360->dev, -1, max7360_devices,
+			     ARRAY_SIZE(max7360_devices), NULL,
+			     0, max7360->domain);
+	if (ret)
+		dev_err(max7360->dev, "failed to add child devices\n");
+
+	return ret;
+}
+
+int max7360_request_gpo_pin_count(struct max7360 *max7360, u8 count)
+{
+	if (count > MAX7360_MAX_GPO)
+		return -EINVAL;
+	if (max7360->col_count + count > MAX7360_COL_GPO_PINS) {
+		dev_err(max7360->dev,
+		       "trying to request %d pins as gpo while %d pins already used as COL\n",
+		       count, max7360->col_count);
+		return -EINVAL;
+	}
+	max7360->gpo_count = count;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max7360_request_gpo_pin_count);
+
+int max7360_request_col_count(struct max7360 *max7360, u8 count)
+{
+	if (max7360->gpo_count + count > MAX7360_COL_GPO_PINS) {
+		dev_err(max7360->dev,
+		       "trying to request %d pins as COL while %d pins already used as gpo\n",
+		       count, max7360->gpo_count);
+		return -EINVAL;
+	}
+	max7360->col_count = count;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max7360_request_col_count);
+
+static const struct regmap_range max7360_volatile_ranges[] = {
+	{
+		.range_min = MAX7360_REG_KEYFIFO,
+		.range_max = MAX7360_REG_KEYFIFO,
+	}, {
+		.range_min = 0x48,
+		.range_max = 0x4a,
+	},
+};
+
+static const struct regmap_access_table max7360_volatile_table = {
+	.yes_ranges = max7360_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
+};
+
+static const struct regmap_config max7360_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0xff,
+	.volatile_table = &max7360_volatile_table,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+static int max7360_probe(struct i2c_client *i2c,
+			const struct i2c_device_id *id)
+{
+	struct device_node *np = i2c->dev.of_node;
+	struct max7360 *max7360;
+
+	int ret;
+
+	max7360 = devm_kzalloc(&i2c->dev, sizeof(struct max7360),
+			      GFP_KERNEL);
+	if (!max7360)
+		return -ENOMEM;
+
+	spin_lock_init(&max7360->lock);
+
+	max7360->dev = &i2c->dev;
+	max7360->i2c = i2c;
+
+	i2c_set_clientdata(i2c, max7360);
+
+	max7360->regmap = devm_regmap_init_i2c(i2c, &max7360_regmap_config);
+	ret = max7360_chip_init(max7360);
+	if (ret)
+		return ret;
+
+	ret = max7360_irq_init(max7360, np);
+	if (ret)
+		return ret;
+
+	ret = max7360_device_init(max7360);
+	if (ret) {
+		dev_err(max7360->dev, "failed to add child devices\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int max7360_remove(struct i2c_client *client)
+{
+	struct max7360 *max7360 = i2c_get_clientdata(client);
+
+	mfd_remove_devices(max7360->dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int max7360_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int max7360_resume(struct device *dev)
+{
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(max7360_dev_pm_ops, max7360_suspend, max7360_resume);
+
+static const struct of_device_id max7360_match[] = {
+	{ .compatible = "maxim,max7360" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, max7360_match);
+
+static const struct i2c_device_id max7360_id[] = {
+	{ "max7360", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, max7360_id);
+
+static struct i2c_driver max7360_driver = {
+	.driver = {
+		.name	= "max7360",
+		.pm	= &max7360_dev_pm_ops,
+		.of_match_table = max7360_match,
+	},
+	.probe		= max7360_probe,
+	.remove		= max7360_remove,
+	.id_table	= max7360_id,
+};
+
+static int __init max7360_init(void)
+{
+	return i2c_add_driver(&max7360_driver);
+}
+subsys_initcall(max7360_init);
+
+static void __exit max7360_exit(void)
+{
+	i2c_del_driver(&max7360_driver);
+}
+module_exit(max7360_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MAX7360 MFD core driver");
diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
new file mode 100644
index 0000000..d139ddd
--- /dev/null
+++ b/include/linux/mfd/max7360.h
@@ -0,0 +1,130 @@
+#ifndef __LINUX_MFD_MAX7360_H
+#define __LINUX_MFD_MAX7360_H
+#include <linux/regmap.h>
+
+#define MAX7360_MAX_KEY_ROWS	8
+#define MAX7360_MAX_KEY_COLS	8
+#define MAX7360_MAX_KEY_NUM	(MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
+#define MAX7360_ROW_SHIFT	3
+
+#define MAX7360_MAX_GPIO 8
+#define MAX7360_MAX_GPO 6
+#define MAX7360_COL_GPO_PINS 8
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_KEYFIFO	0x00
+#define MAX7360_REG_CONFIG	0x01
+#define MAX7360_REG_DEBOUNCE	0x02
+#define MAX7360_REG_INTERRUPT	0x03
+#define MAX7360_REG_PORTS	0x04
+#define MAX7360_REG_KEYREP	0x05
+#define MAX7360_REG_SLEEP	0x06
+
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_GPIOCFG	0x40
+#define MAX7360_REG_GPIOCTRL	0x41
+#define MAX7360_REG_GPIODEB	0x42
+#define MAX7360_REG_GPIOCURR	0x43
+#define MAX7360_REG_GPIOOUTM	0x44
+#define MAX7360_REG_PWMCOM	0x45
+#define MAX7360_REG_RTRCFG	0x46
+#define MAX7360_REG_GPIOIN	0x49
+#define MAX7360_REG_RTR_CNT	0x4A
+#define MAX7360_REG_PWMBASE	0x50
+#define MAX7360_REG_PWMCFG	0x58
+
+
+#define MAX7360_REG_PORTCFGBASE 0x58
+
+/*
+ * Configuration register bits
+ */
+#define MAX7360_CFG_SLEEP	(1 << 7)
+#define MAX7360_CFG_INTERRUPT	(1 << 5)
+#define MAX7360_CFG_KEY_RELEASE	(1 << 3)
+#define MAX7360_CFG_WAKEUP	(1 << 1)
+#define MAX7360_CFG_TIMEOUT	(1 << 0)
+
+/*
+ * Autosleep register values (ms)
+ */
+#define MAX7360_AUTOSLEEP_8192	0x01
+#define MAX7360_AUTOSLEEP_4096	0x02
+#define MAX7360_AUTOSLEEP_2048	0x03
+#define MAX7360_AUTOSLEEP_1024	0x04
+#define MAX7360_AUTOSLEEP_512	0x05
+#define MAX7360_AUTOSLEEP_256	0x06
+
+#define MAX7360_INT_INTI	0
+#define MAX7360_INT_INTK	1
+
+#define MAX7360_INT_GPIO   0
+#define MAX7360_INT_KEYPAD 1
+#define MAX7360_INT_ROTARY 2
+
+#define MAX7360_NR_INTERNAL_IRQS	3
+
+struct max7360 {
+	spinlock_t lock;		/* lock access to the structure */
+	struct device *dev;
+	struct i2c_client *i2c;
+	struct irq_domain *domain;
+	struct regmap *regmap;
+
+	int irq_base;
+	int num_gpio;
+	int shared_irq;
+	int inti;
+	int intk;
+	u8 gpio_pins;
+	u8 col_count;
+	u8 gpo_count;
+};
+
+static inline int max7360_read_reg(struct max7360 *max7360, int reg)
+{
+	unsigned int ival;
+	int ret;
+
+	ret = regmap_read(max7360->regmap, reg, &ival);
+	if (!ret)
+		return ival;
+	return 0;
+}
+
+static inline int max7360_write_reg(struct max7360 *max7360, u8 reg, u8 val)
+{
+	return regmap_write(max7360->regmap, reg, val);
+}
+
+static inline int max7360_set_bits(struct max7360 *max7360, u8 reg,
+				  unsigned int bit_mask)
+{
+	return regmap_update_bits(max7360->regmap, reg, bit_mask, bit_mask);
+}
+
+static inline int max7360_clr_bits(struct max7360 *max7360, u8 reg,
+				  unsigned int bit_mask)
+{
+	return regmap_update_bits(max7360->regmap, reg, bit_mask, 0);
+}
+
+static inline int max7360_update(struct max7360 *max7360, u8 reg, u8 val,
+				unsigned int bit_mask)
+{
+	return regmap_update_bits(max7360->regmap, reg, bit_mask, val);
+}
+
+int max7360_request_pin(struct max7360 *max7360, u8 pin);
+void max7360_free_pin(struct max7360 *max7360, u8 pin);
+
+void max7360_take_catnap(struct max7360 *max7360);
+void max7360_fall_deepsleep(struct max7360 *max7360);
+
+int max7360_request_gpo_pin_count(struct max7360 *max7360, u8 count);
+int max7360_request_col_count(struct max7360 *max7360, u8 count);
+
+#endif
-- 
2.9.3

[PATCH 2/2] mfd: max7360: Add mfd core device driver

[Valentin Sitdikov]

From: Andrei Dranitca <Andrei_Dranitca@mentor.com>

This patch adds core/irq driver to support MAX7360 i2c chip
which contains keypad, gpio, pwm, gpo and rotary encoder submodules.

Signed-off-by: Valentin Sitdikov <valentin_sitdikov@mentor.com>
Signed-off-by: Andrei Dranitca <Andrei_Dranitca@mentor.com>
---
 drivers/mfd/Kconfig         |  16 ++
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/max7360.c       | 393 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/max7360.h | 130 +++++++++++++++
 4 files changed, 540 insertions(+)
 create mode 100644 drivers/mfd/max7360.c
 create mode 100644 include/linux/mfd/max7360.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 55ecdfb..2227c65 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -688,6 +688,22 @@ config MFD_MAX8998
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MAX7360
+	tristate "Maxim Semiconductor MAX7360 support"
+	depends on I2C && OF
+	select MFD_CORE
+	select REGMAP_I2C
+	select IRQ_DOMAIN
+	help
+	  Say yes here to add support for Maxim Semiconductor MAX7360.
+	  This provides microprocessors with management of up to 64 key switches,
+	  with an additional eight LED drivers/GPIOs that feature constant-current,
+	  PWM intensity control, and rotary switch control options.
+
+	  This driver provides common support for accessing the device,
+	  additional drivers must be enabled in order to use the functionality
+	  of the device.
+
 config MFD_MT6397
 	tristate "MediaTek MT6397 PMIC Support"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 31ce076..f01f3a1 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -133,6 +133,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
 obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
 
 obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
+obj-$(CONFIG_MFD_MAX7360)	+= max7360.o
 obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
 obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
 obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
new file mode 100644
index 0000000..b4c2fdd
--- /dev/null
+++ b/drivers/mfd/max7360.c
@@ -0,0 +1,393 @@
+/*
+ * Copyright (C) 2017 Mentor Graphics
+ *
+ * Author: Valentin Sitdikov <Valentin.Sitdikov@mentor.com>
+ * Author: Andrei Dranitca <Andrei_Dranitca@mentor.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/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max7360.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+
+int max7360_request_pin(struct max7360 *max7360, u8 pin)
+{
+	struct i2c_client *client = max7360->i2c;
+	int ret = 0;
+
+	spin_lock(&max7360->lock);
+	if (max7360->gpio_pins & BIT(pin)) {
+		dev_err(&client->dev, "pin %d already requested, mask %x",
+		       pin, max7360->gpio_pins);
+		spin_unlock(&max7360->lock);
+		return -EBUSY;
+	}
+	max7360->gpio_pins |= BIT(pin);
+	dev_dbg(&client->dev, "pin %d requested successfully", pin);
+	spin_unlock(&max7360->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(max7360_request_pin);
+
+void max7360_free_pin(struct max7360 *max7360, u8 pin)
+{
+	spin_lock(&max7360->lock);
+	max7360->gpio_pins &= ~BIT(pin);
+	spin_unlock(&max7360->lock);
+}
+EXPORT_SYMBOL_GPL(max7360_free_pin);
+
+static const struct mfd_cell max7360_devices[] = {
+	{
+		.name           = "max7360-gpio",
+		.of_compatible	= "maxim,max7360-gpio",
+	},
+	{
+		.name           = "max7360-keypad",
+		.of_compatible	= "maxim,max7360-keypad",
+	},
+	{
+		.name           = "max7360-pwm",
+		.of_compatible	= "maxim,max7360-pwm",
+	},
+	{
+		.name           = "max7360-rotary",
+		.of_compatible	= "maxim,max7360-rotary",
+	},
+};
+
+static irqreturn_t max7360_irq(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqi(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+	handle_nested_irq(virq);
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqk(int irq, void *data)
+{
+	struct max7360 *max7360 = data;
+	int virq;
+
+	virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	handle_nested_irq(virq);
+
+	return IRQ_HANDLED;
+}
+
+static int max7360_irq_map(struct irq_domain *d, unsigned int virq,
+			  irq_hw_number_t hwirq)
+{
+	struct max7360 *max7360 = d->host_data;
+
+	irq_set_chip_data(virq, max7360);
+	irq_set_chip_and_handler(virq, &dummy_irq_chip,
+				handle_edge_irq);
+	irq_set_nested_thread(virq, 1);
+	irq_set_noprobe(virq);
+
+	return 0;
+}
+
+static void max7360_irq_unmap(struct irq_domain *d, unsigned int virq)
+{
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+static const struct irq_domain_ops max7360_irq_ops = {
+	.map    = max7360_irq_map,
+	.unmap  = max7360_irq_unmap,
+	.xlate  = irq_domain_xlate_onecell,
+};
+
+static int max7360_irq_init(struct max7360 *max7360, struct device_node *np)
+{
+	int ret;
+
+	max7360->inti = of_irq_get_byname(np, "inti");
+	max7360->intk = of_irq_get_byname(np, "intk");
+
+	if (max7360->inti < 0 || max7360->intk < 0) {
+		max7360->shared_irq = of_irq_get_byname(np, "int-shared");
+
+		if (max7360->shared_irq < 0) {
+			dev_err(max7360->dev, "failed to find IRQ in dts\n");
+			return -EINVAL;
+		}
+
+		ret = request_threaded_irq(max7360->shared_irq, NULL,
+					  max7360_irq,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			dev_err(max7360->dev, "failed to request IRQ: %d\n",
+				ret);
+			return ret;
+		}
+	} else {
+		max7360->shared_irq = 0;
+		ret = request_threaded_irq(max7360->inti, NULL, max7360_irqi,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			dev_err(max7360->dev, "failed to request inti IRQ: %d\n",
+			       ret);
+			return ret;
+		}
+
+		ret = request_threaded_irq(max7360->intk, NULL, max7360_irqk,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  "max7360", max7360);
+		if (ret) {
+			free_irq(max7360->inti, max7360);
+			dev_err(max7360->dev, "failed to request intk IRQ: %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	max7360->domain = irq_domain_add_simple(np, MAX7360_NR_INTERNAL_IRQS,
+					       0, &max7360_irq_ops, max7360);
+
+	if (!max7360->domain) {
+		if (max7360->shared_irq)
+			free_irq(max7360->shared_irq, max7360);
+		else {
+			free_irq(max7360->inti, max7360);
+			free_irq(max7360->intk, max7360);
+		}
+		dev_err(max7360->dev, "Failed to create irqdomain\n");
+		return -ENODEV;
+	}
+
+	irq_create_mapping(max7360->domain, MAX7360_INT_GPIO);
+	irq_create_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+	irq_create_mapping(max7360->domain, MAX7360_INT_ROTARY);
+
+	return 0;
+}
+
+void max7360_fall_deepsleep(struct max7360 *max7360)
+{
+	max7360_write_reg(max7360, MAX7360_REG_SLEEP, MAX7360_AUTOSLEEP_8192);
+}
+EXPORT_SYMBOL_GPL(max7360_fall_deepsleep);
+
+void max7360_take_catnap(struct max7360 *max7360)
+{
+	max7360_write_reg(max7360, MAX7360_REG_SLEEP, MAX7360_AUTOSLEEP_256);
+}
+EXPORT_SYMBOL_GPL(max7360_take_catnap);
+
+static int max7360_chip_init(struct max7360 *max7360)
+{
+	max7360->gpio_pins = MAX7360_MAX_GPIO;
+	max7360->gpo_count = 0;
+	max7360->col_count = MAX7360_COL_GPO_PINS;
+	return 0;
+}
+
+static int max7360_device_init(struct max7360 *max7360)
+{
+	int ret = 0;
+
+	ret = mfd_add_devices(max7360->dev, -1, max7360_devices,
+			     ARRAY_SIZE(max7360_devices), NULL,
+			     0, max7360->domain);
+	if (ret)
+		dev_err(max7360->dev, "failed to add child devices\n");
+
+	return ret;
+}
+
+int max7360_request_gpo_pin_count(struct max7360 *max7360, u8 count)
+{
+	if (count > MAX7360_MAX_GPO)
+		return -EINVAL;
+	if (max7360->col_count + count > MAX7360_COL_GPO_PINS) {
+		dev_err(max7360->dev,
+		       "trying to request %d pins as gpo while %d pins already used as COL\n",
+		       count, max7360->col_count);
+		return -EINVAL;
+	}
+	max7360->gpo_count = count;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max7360_request_gpo_pin_count);
+
+int max7360_request_col_count(struct max7360 *max7360, u8 count)
+{
+	if (max7360->gpo_count + count > MAX7360_COL_GPO_PINS) {
+		dev_err(max7360->dev,
+		       "trying to request %d pins as COL while %d pins already used as gpo\n",
+		       count, max7360->gpo_count);
+		return -EINVAL;
+	}
+	max7360->col_count = count;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max7360_request_col_count);
+
+static const struct regmap_range max7360_volatile_ranges[] = {
+	{
+		.range_min = MAX7360_REG_KEYFIFO,
+		.range_max = MAX7360_REG_KEYFIFO,
+	}, {
+		.range_min = 0x48,
+		.range_max = 0x4a,
+	},
+};
+
+static const struct regmap_access_table max7360_volatile_table = {
+	.yes_ranges = max7360_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
+};
+
+static const struct regmap_config max7360_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0xff,
+	.volatile_table = &max7360_volatile_table,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+static int max7360_probe(struct i2c_client *i2c,
+			const struct i2c_device_id *id)
+{
+	struct device_node *np = i2c->dev.of_node;
+	struct max7360 *max7360;
+
+	int ret;
+
+	max7360 = devm_kzalloc(&i2c->dev, sizeof(struct max7360),
+			      GFP_KERNEL);
+	if (!max7360)
+		return -ENOMEM;
+
+	spin_lock_init(&max7360->lock);
+
+	max7360->dev = &i2c->dev;
+	max7360->i2c = i2c;
+
+	i2c_set_clientdata(i2c, max7360);
+
+	max7360->regmap = devm_regmap_init_i2c(i2c, &max7360_regmap_config);
+	ret = max7360_chip_init(max7360);
+	if (ret)
+		return ret;
+
+	ret = max7360_irq_init(max7360, np);
+	if (ret)
+		return ret;
+
+	ret = max7360_device_init(max7360);
+	if (ret) {
+		dev_err(max7360->dev, "failed to add child devices\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int max7360_remove(struct i2c_client *client)
+{
+	struct max7360 *max7360 = i2c_get_clientdata(client);
+
+	mfd_remove_devices(max7360->dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int max7360_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int max7360_resume(struct device *dev)
+{
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(max7360_dev_pm_ops, max7360_suspend, max7360_resume);
+
+static const struct of_device_id max7360_match[] = {
+	{ .compatible = "maxim,max7360" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, max7360_match);
+
+static const struct i2c_device_id max7360_id[] = {
+	{ "max7360", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, max7360_id);
+
+static struct i2c_driver max7360_driver = {
+	.driver = {
+		.name	= "max7360",
+		.pm	= &max7360_dev_pm_ops,
+		.of_match_table = max7360_match,
+	},
+	.probe		= max7360_probe,
+	.remove		= max7360_remove,
+	.id_table	= max7360_id,
+};
+
+static int __init max7360_init(void)
+{
+	return i2c_add_driver(&max7360_driver);
+}
+subsys_initcall(max7360_init);
+
+static void __exit max7360_exit(void)
+{
+	i2c_del_driver(&max7360_driver);
+}
+module_exit(max7360_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MAX7360 MFD core driver");
diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
new file mode 100644
index 0000000..d139ddd
--- /dev/null
+++ b/include/linux/mfd/max7360.h
@@ -0,0 +1,130 @@
+#ifndef __LINUX_MFD_MAX7360_H
+#define __LINUX_MFD_MAX7360_H
+#include <linux/regmap.h>
+
+#define MAX7360_MAX_KEY_ROWS	8
+#define MAX7360_MAX_KEY_COLS	8
+#define MAX7360_MAX_KEY_NUM	(MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
+#define MAX7360_ROW_SHIFT	3
+
+#define MAX7360_MAX_GPIO 8
+#define MAX7360_MAX_GPO 6
+#define MAX7360_COL_GPO_PINS 8
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_KEYFIFO	0x00
+#define MAX7360_REG_CONFIG	0x01
+#define MAX7360_REG_DEBOUNCE	0x02
+#define MAX7360_REG_INTERRUPT	0x03
+#define MAX7360_REG_PORTS	0x04
+#define MAX7360_REG_KEYREP	0x05
+#define MAX7360_REG_SLEEP	0x06
+
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_GPIOCFG	0x40
+#define MAX7360_REG_GPIOCTRL	0x41
+#define MAX7360_REG_GPIODEB	0x42
+#define MAX7360_REG_GPIOCURR	0x43
+#define MAX7360_REG_GPIOOUTM	0x44
+#define MAX7360_REG_PWMCOM	0x45
+#define MAX7360_REG_RTRCFG	0x46
+#define MAX7360_REG_GPIOIN	0x49
+#define MAX7360_REG_RTR_CNT	0x4A
+#define MAX7360_REG_PWMBASE	0x50
+#define MAX7360_REG_PWMCFG	0x58
+
+
+#define MAX7360_REG_PORTCFGBASE 0x58
+
+/*
+ * Configuration register bits
+ */
+#define MAX7360_CFG_SLEEP	(1 << 7)
+#define MAX7360_CFG_INTERRUPT	(1 << 5)
+#define MAX7360_CFG_KEY_RELEASE	(1 << 3)
+#define MAX7360_CFG_WAKEUP	(1 << 1)
+#define MAX7360_CFG_TIMEOUT	(1 << 0)
+
+/*
+ * Autosleep register values (ms)
+ */
+#define MAX7360_AUTOSLEEP_8192	0x01
+#define MAX7360_AUTOSLEEP_4096	0x02
+#define MAX7360_AUTOSLEEP_2048	0x03
+#define MAX7360_AUTOSLEEP_1024	0x04
+#define MAX7360_AUTOSLEEP_512	0x05
+#define MAX7360_AUTOSLEEP_256	0x06
+
+#define MAX7360_INT_INTI	0
+#define MAX7360_INT_INTK	1
+
+#define MAX7360_INT_GPIO   0
+#define MAX7360_INT_KEYPAD 1
+#define MAX7360_INT_ROTARY 2
+
+#define MAX7360_NR_INTERNAL_IRQS	3
+
+struct max7360 {
+	spinlock_t lock;		/* lock access to the structure */
+	struct device *dev;
+	struct i2c_client *i2c;
+	struct irq_domain *domain;
+	struct regmap *regmap;
+
+	int irq_base;
+	int num_gpio;
+	int shared_irq;
+	int inti;
+	int intk;
+	u8 gpio_pins;
+	u8 col_count;
+	u8 gpo_count;
+};
+
+static inline int max7360_read_reg(struct max7360 *max7360, int reg)
+{
+	unsigned int ival;
+	int ret;
+
+	ret = regmap_read(max7360->regmap, reg, &ival);
+	if (!ret)
+		return ival;
+	return 0;
+}
+
+static inline int max7360_write_reg(struct max7360 *max7360, u8 reg, u8 val)
+{
+	return regmap_write(max7360->regmap, reg, val);
+}
+
+static inline int max7360_set_bits(struct max7360 *max7360, u8 reg,
+				  unsigned int bit_mask)
+{
+	return regmap_update_bits(max7360->regmap, reg, bit_mask, bit_mask);
+}
+
+static inline int max7360_clr_bits(struct max7360 *max7360, u8 reg,
+				  unsigned int bit_mask)
+{
+	return regmap_update_bits(max7360->regmap, reg, bit_mask, 0);
+}
+
+static inline int max7360_update(struct max7360 *max7360, u8 reg, u8 val,
+				unsigned int bit_mask)
+{
+	return regmap_update_bits(max7360->regmap, reg, bit_mask, val);
+}
+
+int max7360_request_pin(struct max7360 *max7360, u8 pin);
+void max7360_free_pin(struct max7360 *max7360, u8 pin);
+
+void max7360_take_catnap(struct max7360 *max7360);
+void max7360_fall_deepsleep(struct max7360 *max7360);
+
+int max7360_request_gpo_pin_count(struct max7360 *max7360, u8 count);
+int max7360_request_col_count(struct max7360 *max7360, u8 count);
+
+#endif
-- 
2.9.3