[PATCH v2 0/4] Add support for the Gateworks System Controller

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v2 0/4] Add support for the Gateworks System Controller
@ 2018-03-05 22:02 ` Tim Harvey
  0 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-05 22:02 UTC (permalink / raw)
  To: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-hwmon, linux-input

This series adds support for the Gateworks System Controller used on Gateworks
Laguna, Ventana, and Newport product families.

The GSC is an MSP430 I2C slave controller whose firmware embeds the following
features:
 - I/O expander (16 GPIO's emulating a PCA955x)
 - EEPROM (enumating AT24)
 - RTC (enumating DS1672)
 - HWMON
 - Interrupt controller with tamper detect, user pushbotton
 - Watchdog controller capable of full board power-cycle
 - Power Control capable of full board power-cycle

see http://trac.gateworks.com/wiki/gsc for more details

Tim Harvey (4):
  dt-bindings: mfd: Add Gateworks System Controller bindings
  mfd: add Gateworks System Controller core driver
  hwmon: add Gateworks System Controller support
  input: misc: Add Gateworks System Controller support

 Documentation/devicetree/bindings/mfd/gsc.txt | 159 +++++++++++++
 drivers/hwmon/Kconfig                         |   9 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/gsc-hwmon.c                     | 330 ++++++++++++++++++++++++++
 drivers/input/misc/Kconfig                    |   9 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/input/misc/gsc-input.c                | 180 ++++++++++++++
 drivers/mfd/Kconfig                           |  13 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/gsc.c                             | 330 ++++++++++++++++++++++++++
 include/linux/mfd/gsc.h                       |  82 +++++++
 include/linux/platform_data/gsc_hwmon.h       |  34 +++
 include/linux/platform_data/gsc_input.h       |  30 +++
 13 files changed, 1179 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/gsc.txt
 create mode 100644 drivers/hwmon/gsc-hwmon.c
 create mode 100644 drivers/input/misc/gsc-input.c
 create mode 100644 drivers/mfd/gsc.c
 create mode 100644 include/linux/mfd/gsc.h
 create mode 100644 include/linux/platform_data/gsc_hwmon.h
 create mode 100644 include/linux/platform_data/gsc_input.h

-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 0/4] Add support for the Gateworks System Controller
@ 2018-03-05 22:02 ` Tim Harvey
  0 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-05 22:02 UTC (permalink / raw)
  To: linux-arm-kernel

This series adds support for the Gateworks System Controller used on Gateworks
Laguna, Ventana, and Newport product families.

The GSC is an MSP430 I2C slave controller whose firmware embeds the following
features:
 - I/O expander (16 GPIO's emulating a PCA955x)
 - EEPROM (enumating AT24)
 - RTC (enumating DS1672)
 - HWMON
 - Interrupt controller with tamper detect, user pushbotton
 - Watchdog controller capable of full board power-cycle
 - Power Control capable of full board power-cycle

see http://trac.gateworks.com/wiki/gsc for more details

Tim Harvey (4):
  dt-bindings: mfd: Add Gateworks System Controller bindings
  mfd: add Gateworks System Controller core driver
  hwmon: add Gateworks System Controller support
  input: misc: Add Gateworks System Controller support

 Documentation/devicetree/bindings/mfd/gsc.txt | 159 +++++++++++++
 drivers/hwmon/Kconfig                         |   9 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/gsc-hwmon.c                     | 330 ++++++++++++++++++++++++++
 drivers/input/misc/Kconfig                    |   9 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/input/misc/gsc-input.c                | 180 ++++++++++++++
 drivers/mfd/Kconfig                           |  13 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/gsc.c                             | 330 ++++++++++++++++++++++++++
 include/linux/mfd/gsc.h                       |  82 +++++++
 include/linux/platform_data/gsc_hwmon.h       |  34 +++
 include/linux/platform_data/gsc_input.h       |  30 +++
 13 files changed, 1179 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/gsc.txt
 create mode 100644 drivers/hwmon/gsc-hwmon.c
 create mode 100644 drivers/input/misc/gsc-input.c
 create mode 100644 drivers/mfd/gsc.c
 create mode 100644 include/linux/mfd/gsc.h
 create mode 100644 include/linux/platform_data/gsc_hwmon.h
 create mode 100644 include/linux/platform_data/gsc_input.h

-- 
2.7.4

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings
  2018-03-05 22:02 ` Tim Harvey
@ 2018-03-05 22:02   ` Tim Harvey
  -1 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-05 22:02 UTC (permalink / raw)
  To: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-hwmon, linux-input

This patch adds documentation of device-tree bindings for the
Gateworks System Controller (GSC).

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 Documentation/devicetree/bindings/mfd/gsc.txt | 159 ++++++++++++++++++++++++++
 1 file changed, 159 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/gsc.txt

diff --git a/Documentation/devicetree/bindings/mfd/gsc.txt b/Documentation/devicetree/bindings/mfd/gsc.txt
new file mode 100644
index 0000000..fe5d114
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/gsc.txt
@@ -0,0 +1,159 @@
+Gateworks System Controller multi-function device
+
+The GSC is a Multifunction I2C slave device with the following submodules:
+- WDT
+- GPIO
+- Pushbutton controller
+- HWMON
+
+Required properties:
+- compatible : Must be "gw,gsc_v1", "gw,gsc_v2", "gw,gsc_v3"
+- reg: I2C address of the device
+- interrupts: interrupt triggered by GSC_IRQ# signal
+- interrupt-parent: Interrupt controller GSC is connected to
+- #interrupt-cells: should be <1>, index of the interrupt within the
+  controller, in accordance with the "one cell" variant of
+  <devicetree/bindings/interrupt-controller/interrupt.txt>
+
+Optional nodes:
+* watchdog:
+The GSC provides a Watchdog monitor which can power cycle the board's
+primary power supply on most board models when tripped.
+
+Required properties:
+- compatible: must be "gw,gsc-watchdog"
+
+* input:
+The GSC provides an input device capable of dispatching Linux Input events
+for user pushbutton events, tamper switch events, etc.
+
+Required properties:
+- compatible: must be "gw,gsc-input"
+
+* hwmon:
+The GSC provides a set of Analog to Digitcal Converter (ADC) pins used for
+temperature and/or voltage monitoring.
+
+Required properties:
+- compatible: must be "gw,gsc-hwmon"
+
+Example:
+
+	gsc: gsc@20 {
+		compatible = "gw,gsc_v2";
+		reg = <0x20>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <4 GPIO_ACTIVE_LOW>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+
+		gsc_input {
+			compatible = "gw,gsc-input";
+		};
+
+		gsc_watchdog {
+			compatible = "gw,gsc-watchdog";
+		};
+
+		gsc_hwmon {
+			compatible = "gw,gsc-hwmon";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			hwmon@0 { /* A0: Board Temperature */
+				type = <0>;
+				reg = <0x00>;
+				label = "temp";
+			};
+
+			hwmon@1 { /* A1: Input Voltage */
+				type = <1>;
+				reg = <0x02>;
+				label = "Vin";
+			};
+
+			hwmon@2 { /* A2: 5P0 */
+				type = <1>;
+				reg = <0x0b>;
+				label = "5P0";
+			};
+
+			hwmon@4 { /* A4: 0-5V input */
+				type = <1>;
+				reg = <0x14>;
+				label = "ANL0";
+			};
+
+			hwmon@5 { /* A5: 2P5 PCIe/GigE */
+				type = <1>;
+				reg = <0x23>;
+				label = "2P5";
+			};
+
+			hwmon@6 { /* A6: 1P8 Aud/Vid */
+				type = <1>;
+				reg = <0x1d>;
+				label = "1P8";
+			};
+
+			hwmon@7 { /* A7: GPS */
+				type = <1>;
+				reg = <0x26>;
+				label = "GPS";
+			};
+
+			hwmon@12 { /* A12: VDD_CORE */
+				type = <1>;
+				reg = <0x3>;
+				label = "VDD_CORE";
+			};
+
+			hwmon@13 { /* A13: VDD_SOC */
+				type = <1>;
+				reg = <0x11>;
+				label = "VDD_SOC";
+			};
+
+			hwmon@14 { /* A14: 1P0 PCIe SW */
+				type = <1>;
+				reg = <0x20>;
+				label = "1P0";
+			};
+
+			hwmon@15 { /* fan0 */
+				type = <2>;
+				reg = <0x2c>;
+				label = "fan_50p";
+			};
+
+			hwmon@16 { /* fan1 */
+				type = <2>;
+				reg = <0x2e>;
+				label = "fan_60p";
+			};
+
+			hwmon@17 { /* fan2 */
+				type = <2>;
+				reg = <0x30>;
+				label = "fan_70p";
+			};
+
+			hwmon@18 { /* fan3 */
+				type = <2>;
+				reg = <0x32>;
+				label = "fan_80p";
+			};
+
+			hwmon@19 { /* fan4 */
+				type = <2>;
+				reg = <0x34>;
+				label = "fan_90p";
+			};
+
+			hwmon@20 { /* fan5 */
+				type = <2>;
+				reg = <0x36>;
+				label = "fan_100p";
+			};
+		};
+	};
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings
@ 2018-03-05 22:02   ` Tim Harvey
  0 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-05 22:02 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds documentation of device-tree bindings for the
Gateworks System Controller (GSC).

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 Documentation/devicetree/bindings/mfd/gsc.txt | 159 ++++++++++++++++++++++++++
 1 file changed, 159 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/gsc.txt

diff --git a/Documentation/devicetree/bindings/mfd/gsc.txt b/Documentation/devicetree/bindings/mfd/gsc.txt
new file mode 100644
index 0000000..fe5d114
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/gsc.txt
@@ -0,0 +1,159 @@
+Gateworks System Controller multi-function device
+
+The GSC is a Multifunction I2C slave device with the following submodules:
+- WDT
+- GPIO
+- Pushbutton controller
+- HWMON
+
+Required properties:
+- compatible : Must be "gw,gsc_v1", "gw,gsc_v2", "gw,gsc_v3"
+- reg: I2C address of the device
+- interrupts: interrupt triggered by GSC_IRQ# signal
+- interrupt-parent: Interrupt controller GSC is connected to
+- #interrupt-cells: should be <1>, index of the interrupt within the
+  controller, in accordance with the "one cell" variant of
+  <devicetree/bindings/interrupt-controller/interrupt.txt>
+
+Optional nodes:
+* watchdog:
+The GSC provides a Watchdog monitor which can power cycle the board's
+primary power supply on most board models when tripped.
+
+Required properties:
+- compatible: must be "gw,gsc-watchdog"
+
+* input:
+The GSC provides an input device capable of dispatching Linux Input events
+for user pushbutton events, tamper switch events, etc.
+
+Required properties:
+- compatible: must be "gw,gsc-input"
+
+* hwmon:
+The GSC provides a set of Analog to Digitcal Converter (ADC) pins used for
+temperature and/or voltage monitoring.
+
+Required properties:
+- compatible: must be "gw,gsc-hwmon"
+
+Example:
+
+	gsc: gsc at 20 {
+		compatible = "gw,gsc_v2";
+		reg = <0x20>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <4 GPIO_ACTIVE_LOW>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+
+		gsc_input {
+			compatible = "gw,gsc-input";
+		};
+
+		gsc_watchdog {
+			compatible = "gw,gsc-watchdog";
+		};
+
+		gsc_hwmon {
+			compatible = "gw,gsc-hwmon";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			hwmon at 0 { /* A0: Board Temperature */
+				type = <0>;
+				reg = <0x00>;
+				label = "temp";
+			};
+
+			hwmon at 1 { /* A1: Input Voltage */
+				type = <1>;
+				reg = <0x02>;
+				label = "Vin";
+			};
+
+			hwmon at 2 { /* A2: 5P0 */
+				type = <1>;
+				reg = <0x0b>;
+				label = "5P0";
+			};
+
+			hwmon at 4 { /* A4: 0-5V input */
+				type = <1>;
+				reg = <0x14>;
+				label = "ANL0";
+			};
+
+			hwmon at 5 { /* A5: 2P5 PCIe/GigE */
+				type = <1>;
+				reg = <0x23>;
+				label = "2P5";
+			};
+
+			hwmon at 6 { /* A6: 1P8 Aud/Vid */
+				type = <1>;
+				reg = <0x1d>;
+				label = "1P8";
+			};
+
+			hwmon at 7 { /* A7: GPS */
+				type = <1>;
+				reg = <0x26>;
+				label = "GPS";
+			};
+
+			hwmon at 12 { /* A12: VDD_CORE */
+				type = <1>;
+				reg = <0x3>;
+				label = "VDD_CORE";
+			};
+
+			hwmon at 13 { /* A13: VDD_SOC */
+				type = <1>;
+				reg = <0x11>;
+				label = "VDD_SOC";
+			};
+
+			hwmon at 14 { /* A14: 1P0 PCIe SW */
+				type = <1>;
+				reg = <0x20>;
+				label = "1P0";
+			};
+
+			hwmon at 15 { /* fan0 */
+				type = <2>;
+				reg = <0x2c>;
+				label = "fan_50p";
+			};
+
+			hwmon at 16 { /* fan1 */
+				type = <2>;
+				reg = <0x2e>;
+				label = "fan_60p";
+			};
+
+			hwmon at 17 { /* fan2 */
+				type = <2>;
+				reg = <0x30>;
+				label = "fan_70p";
+			};
+
+			hwmon at 18 { /* fan3 */
+				type = <2>;
+				reg = <0x32>;
+				label = "fan_80p";
+			};
+
+			hwmon at 19 { /* fan4 */
+				type = <2>;
+				reg = <0x34>;
+				label = "fan_90p";
+			};
+
+			hwmon at 20 { /* fan5 */
+				type = <2>;
+				reg = <0x36>;
+				label = "fan_100p";
+			};
+		};
+	};
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/4] mfd: add Gateworks System Controller core driver
  2018-03-05 22:02 ` Tim Harvey
@ 2018-03-05 22:02   ` Tim Harvey
  -1 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-05 22:02 UTC (permalink / raw)
  To: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
	linux-input, Randy Dunlap

The Gateworks System Controller (GSC) is an I2C slave controller
implemented with an MSP430 micro-controller whose firmware embeds the
following features:
 - I/O expander (16 GPIO's) using PCA955x protocol
 - Real Time Clock using DS1672 protocol
 - User EEPROM using AT24 protocol
 - HWMON using custom protocol
 - Interrupt controller with tamper detect, user pushbotton
 - Watchdog controller capable of full board power-cycle
 - Power Control capable of full board power-cycle

see http://trac.gateworks.com/wiki/gsc for more details

Cc: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
v2:
- change license comment block style
- remove COMPILE_TEST (Randy)
- fixed whitespace issues
- replaced a printk with dev_err

 drivers/mfd/Kconfig     |  13 ++
 drivers/mfd/Makefile    |   1 +
 drivers/mfd/gsc.c       | 330 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/gsc.h |  82 ++++++++++++
 4 files changed, 426 insertions(+)
 create mode 100644 drivers/mfd/gsc.c
 create mode 100644 include/linux/mfd/gsc.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 1d20a80..5694013 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -341,6 +341,19 @@ config MFD_EXYNOS_LPASS
 	  Select this option to enable support for Samsung Exynos Low Power
 	  Audio Subsystem.
 
+config MFD_GSC
+	tristate "Gateworks System Controller"
+	depends on (I2C && OF)
+	select MFD_CORE
+	select REGMAP_I2C
+	select REGMAP_IRQ
+	help
+	  Enable support for the Gateworks System Controller found
+	  on Gateworks Single Board Computers.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called gsc.
+
 config MFD_MC13XXX
 	tristate
 	depends on (SPI_MASTER || I2C)
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index d9474ad..aede0bc 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_MFD_CROS_EC)	+= cros_ec_core.o
 obj-$(CONFIG_MFD_CROS_EC_I2C)	+= cros_ec_i2c.o
 obj-$(CONFIG_MFD_CROS_EC_SPI)	+= cros_ec_spi.o
 obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
+obj-$(CONFIG_MFD_GSC)		+= gsc.o
 
 rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
 obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
diff --git a/drivers/mfd/gsc.c b/drivers/mfd/gsc.c
new file mode 100644
index 0000000..4ac400a
--- /dev/null
+++ b/drivers/mfd/gsc.c
@@ -0,0 +1,330 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2018 Gateworks Corporation
+ *
+ * The Gateworks System Controller (GSC) is a family of a multi-function
+ * "Power Management and System Companion Device" chips originally designed for
+ * use in Gateworks Single Board Computers. The control interface is I2C,
+ * at 100kbps, with an interrupt.
+ *
+ */
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/gsc.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/*
+ * The GSC suffers from an errata where occasionally during
+ * ADC cycles the chip can NAK i2c transactions. To ensure we have reliable
+ * register access we place retries around register access.
+ */
+#define I2C_RETRIES	3
+
+static int gsc_regmap_regwrite(void *context, unsigned int reg,
+			       unsigned int val)
+{
+	struct i2c_client *client = context;
+	int retry, ret;
+
+	for (retry = 0; retry < I2C_RETRIES; retry++) {
+		ret = i2c_smbus_write_byte_data(client, reg, val);
+		/*
+		 * -EAGAIN returned when the i2c host controller is busy
+		 * -EIO returned when i2c device is busy
+		 */
+		if (ret != -EAGAIN && ret != -EIO)
+			break;
+	}
+	if (ret < 0) {
+		dev_err(&client->dev, ">> 0x%02x %d\n", reg, ret);
+		return ret;
+	}
+	dev_dbg(&client->dev, ">> 0x%02x=0x%02x (%d)\n", reg, val, retry);
+
+	return 0;
+}
+
+static int gsc_regmap_regread(void *context, unsigned int reg,
+			      unsigned int *val)
+{
+	struct i2c_client *client = context;
+	int retry, ret;
+
+	for (retry = 0; retry < I2C_RETRIES; retry++) {
+		ret = i2c_smbus_read_byte_data(client, reg);
+		/*
+		 * -EAGAIN returned when the i2c host controller is busy
+		 * -EIO returned when i2c device is busy
+		 */
+		if (ret != -EAGAIN && ret != -EIO)
+			break;
+	}
+	if (ret < 0) {
+		dev_err(&client->dev, "<< 0x%02x %d\n", reg, ret);
+		return ret;
+	}
+
+	*val = ret & 0xff;
+	dev_dbg(&client->dev, "<< 0x%02x=0x%02x (%d)\n", reg, *val, retry);
+
+	return 0;
+}
+
+static struct regmap_bus regmap_gsc = {
+	.reg_write = gsc_regmap_regwrite,
+	.reg_read = gsc_regmap_regread,
+};
+
+/*
+ * gsc_powerdown - API to use GSC to power down board for a specific time
+ *
+ * secs - number of seconds to remain powered off
+ */
+static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
+{
+	int ret;
+	unsigned char regs[4];
+
+	dev_info(&gsc->i2c->dev, "GSC powerdown for %ld seconds\n",
+		 secs);
+	regs[0] = secs & 0xff;
+	regs[1] = (secs >> 8) & 0xff;
+	regs[2] = (secs >> 16) & 0xff;
+	regs[3] = (secs >> 24) & 0xff;
+	ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
+
+	return ret;
+}
+
+/*
+ * sysfs hooks
+ */
+static ssize_t gsc_show(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct gsc_dev *gsc = dev_get_drvdata(dev);
+	const char *name = attr->attr.name;
+	int rz = 0;
+
+	if (strcasecmp(name, "fw_version") == 0)
+		rz = sprintf(buf, "%d\n", gsc->fwver);
+	else if (strcasecmp(name, "fw_crc") == 0)
+		rz = sprintf(buf, "0x%04x\n", gsc->fwcrc);
+
+	return rz;
+}
+
+static ssize_t gsc_store(struct device *dev, struct device_attribute *attr,
+			 const char *buf, size_t count)
+{
+	struct gsc_dev *gsc = dev_get_drvdata(dev);
+	const char *name = attr->attr.name;
+	int ret;
+
+	if (strcasecmp(name, "powerdown") == 0) {
+		long value;
+
+		ret = kstrtol(buf, 0, &value);
+		if (ret == 0)
+			gsc_powerdown(gsc, value);
+	} else
+		dev_err(dev, "invalid name '%s\n", name);
+
+	return count;
+}
+
+
+/*
+ * Create a group of attributes so that we can create and destroy them all
+ * at once.
+ */
+static struct device_attribute attr_fwver =
+	__ATTR(fw_version, 0440, gsc_show, NULL);
+static struct device_attribute attr_fwcrc =
+	__ATTR(fw_crc, 0440, gsc_show, NULL);
+static struct device_attribute attr_pwrdown =
+	__ATTR(powerdown, 0220, NULL, gsc_store);
+
+static struct attribute *gsc_attrs[] = {
+	&attr_fwver.attr,
+	&attr_fwcrc.attr,
+	&attr_pwrdown.attr,
+	NULL,
+};
+
+static struct attribute_group attr_group = {
+	.attrs = gsc_attrs,
+};
+
+static const struct i2c_device_id gsc_i2c_ids[] = {
+	{ "gsc_v1", gsc_v1 },
+	{ "gsc_v2", gsc_v2 },
+	{ "gsc_v3", gsc_v3 },
+	{ },
+};
+
+static const struct of_device_id gsc_of_match[] = {
+	{ .compatible = "gw,gsc_v1", .data = (void *)gsc_v1 },
+	{ .compatible = "gw,gsc_v2", .data = (void *)gsc_v2 },
+	{ .compatible = "gw,gsc_v3", .data = (void *)gsc_v3 },
+	{ }
+};
+
+static const struct regmap_config gsc_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_NONE,
+	.max_register = 0xf,
+};
+
+static const struct regmap_config gsc_regmap_hwmon_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_NONE,
+	.max_register = 0x37,
+};
+
+static const struct regmap_irq gsc_irqs[] = {
+	REGMAP_IRQ_REG(GSC_IRQ_PB, 0, BIT(GSC_IRQ_PB)),
+	REGMAP_IRQ_REG(GSC_IRQ_KEY_ERASED, 0, BIT(GSC_IRQ_KEY_ERASED)),
+	REGMAP_IRQ_REG(GSC_IRQ_EEPROM_WP, 0, BIT(GSC_IRQ_EEPROM_WP)),
+	REGMAP_IRQ_REG(GSC_IRQ_RESV, 0, BIT(GSC_IRQ_RESV)),
+	REGMAP_IRQ_REG(GSC_IRQ_GPIO, 0, BIT(GSC_IRQ_GPIO)),
+	REGMAP_IRQ_REG(GSC_IRQ_TAMPER, 0, BIT(GSC_IRQ_TAMPER)),
+	REGMAP_IRQ_REG(GSC_IRQ_WDT_TIMEOUT, 0, BIT(GSC_IRQ_WDT_TIMEOUT)),
+	REGMAP_IRQ_REG(GSC_IRQ_SWITCH_HOLD, 0, BIT(GSC_IRQ_SWITCH_HOLD)),
+};
+
+static const struct regmap_irq_chip gsc_irq_chip = {
+	.name = KBUILD_MODNAME,
+	.irqs = gsc_irqs,
+	.num_irqs = ARRAY_SIZE(gsc_irqs),
+	.num_regs = 1,
+	.status_base = GSC_IRQ_STATUS,
+	.mask_base = GSC_IRQ_ENABLE,
+	.mask_invert = true,
+	.ack_base = GSC_IRQ_STATUS,
+	.ack_invert = true,
+};
+
+static int gsc_of_probe(struct device_node *np, struct gsc_dev *gsc)
+{
+	const struct of_device_id *of_id;
+
+	if (!np)
+		return -ENODEV;
+
+	of_id = of_match_device(gsc_of_match, gsc->dev);
+	if (of_id)
+		gsc->type = (enum gsc_type)of_id->data;
+
+	return 0;
+}
+
+static int
+gsc_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct gsc_dev *gsc;
+	int ret;
+	unsigned int reg;
+
+	gsc = devm_kzalloc(dev, sizeof(*gsc), GFP_KERNEL);
+	if (!gsc)
+		return -ENOMEM;
+
+	gsc->dev = &client->dev;
+	gsc->i2c = client;
+	gsc->irq = client->irq;
+	i2c_set_clientdata(client, gsc);
+
+	gsc->regmap = devm_regmap_init(dev, &regmap_gsc, client,
+				       &gsc_regmap_config);
+	if (IS_ERR(gsc->regmap))
+		return PTR_ERR(gsc->regmap);
+
+	ret = gsc_of_probe(dev->of_node, gsc);
+	if (reg < 0)
+		return ret;
+
+	if (regmap_read(gsc->regmap, GSC_FW_VER, &reg))
+		return -EIO;
+	gsc->fwver = reg;
+	regmap_read(gsc->regmap, GSC_FW_CRC, &reg);
+	gsc->fwcrc = reg;
+	regmap_read(gsc->regmap, GSC_FW_CRC + 1, &reg);
+	gsc->fwcrc |= reg << 8;
+
+	gsc->i2c_hwmon = i2c_new_dummy(client->adapter, GSC_HWMON);
+	if (!gsc->i2c_hwmon) {
+		dev_err(dev, "Failed to allocate I2C device for HWMON\n");
+		return -ENODEV;
+	}
+	i2c_set_clientdata(gsc->i2c_hwmon, gsc);
+
+	gsc->regmap_hwmon = devm_regmap_init(dev, &regmap_gsc, gsc->i2c_hwmon,
+					     &gsc_regmap_hwmon_config);
+	if (IS_ERR(gsc->regmap_hwmon)) {
+		ret = PTR_ERR(gsc->regmap_hwmon);
+		dev_err(dev, "failed to allocate register map: %d\n", ret);
+		goto err_regmap;
+	}
+
+	ret = devm_regmap_add_irq_chip(dev, gsc->regmap, gsc->irq,
+				       IRQF_ONESHOT | IRQF_SHARED |
+				       IRQF_TRIGGER_FALLING, 0,
+				       &gsc_irq_chip, &gsc->irq_chip_data);
+	if (ret)
+		goto err_regmap;
+
+	dev_info(dev, "Gateworks System Controller v%d: fw v%02d 0x%04x\n",
+		 gsc->type, gsc->fwver, gsc->fwcrc);
+
+	/* sysfs hooks */
+	ret = sysfs_create_group(&dev->kobj, &attr_group);
+	if (ret)
+		dev_err(dev, "failed to create sysfs attrs\n");
+
+	ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+	if (ret)
+		goto err_sysfs;
+
+	return 0;
+
+err_sysfs:
+	sysfs_remove_group(&dev->kobj, &attr_group);
+err_regmap:
+	i2c_unregister_device(gsc->i2c_hwmon);
+
+	return ret;
+}
+
+static int gsc_remove(struct i2c_client *client)
+{
+	sysfs_remove_group(&client->dev.kobj, &attr_group);
+
+	return 0;
+}
+
+static struct i2c_driver gsc_driver = {
+	.driver = {
+		.name	= KBUILD_MODNAME,
+		.of_match_table = of_match_ptr(gsc_of_match),
+	},
+	.probe		= gsc_probe,
+	.remove		= gsc_remove,
+	.id_table	= gsc_i2c_ids,
+};
+
+module_i2c_driver(gsc_driver);
+
+MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
+MODULE_DESCRIPTION("I2C Core interface for GSC");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/gsc.h b/include/linux/mfd/gsc.h
new file mode 100644
index 0000000..e0d8c8f
--- /dev/null
+++ b/include/linux/mfd/gsc.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2018 Gateworks Corporation
+ */
+#ifndef __LINUX_MFD_GSC_H_
+#define __LINUX_MFD_GSC_H_
+
+/* Device Addresses */
+#define GSC_MISC	0x20
+#define GSC_UPDATE	0x21
+#define GSC_GPIO	0x23
+#define GSC_HWMON	0x29
+#define GSC_EEPROM0	0x50
+#define GSC_EEPROM1	0x51
+#define GSC_EEPROM2	0x52
+#define GSC_EEPROM3	0x53
+#define GSC_RTC		0x68
+
+/* Register offsets */
+#define GSC_CTRL_0	0x00
+#define GSC_CTRL_1	0x01
+#define GSC_TIME	0x02
+#define GSC_TIME_ADD	0x06
+#define GSC_IRQ_STATUS	0x0A
+#define GSC_IRQ_ENABLE	0x0B
+#define GSC_FW_CRC	0x0C
+#define GSC_FW_VER	0x0E
+#define GSC_WP		0x0F
+
+/* Bit definitions */
+#define GSC_CTRL_0_PB_HARD_RESET	0
+#define GSC_CTRL_0_PB_CLEAR_SECURE_KEY	1
+#define GSC_CTRL_0_PB_SOFT_POWER_DOWN	2
+#define GSC_CTRL_0_PB_BOOT_ALTERNATE	3
+#define GSC_CTRL_0_PERFORM_CRC		4
+#define GSC_CTRL_0_TAMPER_DETECT	5
+#define GSC_CTRL_0_SWITCH_HOLD		6
+
+#define GSC_CTRL_1_SLEEP_ENABLE		0
+#define GSC_CTRL_1_ACTIVATE_SLEEP	1
+#define GSC_CTRL_1_LATCH_SLEEP_ADD	2
+#define GSC_CTRL_1_SLEEP_NOWAKEPB	3
+#define GSC_CTRL_1_WDT_TIME		4
+#define GSC_CTRL_1_WDT_ENABLE		5
+#define GSC_CTRL_1_SWITCH_BOOT_ENABLE	6
+#define GSC_CTRL_1_SWITCH_BOOT_CLEAR	7
+
+#define GSC_IRQ_PB			0
+#define GSC_IRQ_KEY_ERASED		1
+#define GSC_IRQ_EEPROM_WP		2
+#define GSC_IRQ_RESV			3
+#define GSC_IRQ_GPIO			4
+#define GSC_IRQ_TAMPER			5
+#define GSC_IRQ_WDT_TIMEOUT		6
+#define GSC_IRQ_SWITCH_HOLD		7
+
+/* Max registers */
+#define GSC_HWMON_MAX_REG	56
+
+enum gsc_type {
+	gsc_v1 = 1,
+	gsc_v2 = 2,
+	gsc_v3 = 3,
+};
+
+struct gsc_dev {
+	struct device *dev;
+
+	struct i2c_client *i2c;		/* 0x20: interrupt controller, WDT */
+	struct i2c_client *i2c_hwmon;	/* 0x29: hwmon, fan controller */
+
+	struct regmap *regmap;
+	struct regmap *regmap_hwmon;
+	struct regmap_irq_chip_data *irq_chip_data;
+
+	int irq;
+	enum gsc_type type;
+	unsigned int fwver;
+	unsigned short fwcrc;
+};
+
+#endif /* __LINUX_MFD_GSC_H_ */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/4] mfd: add Gateworks System Controller core driver
@ 2018-03-05 22:02   ` Tim Harvey
  0 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-05 22:02 UTC (permalink / raw)
  To: linux-arm-kernel

The Gateworks System Controller (GSC) is an I2C slave controller
implemented with an MSP430 micro-controller whose firmware embeds the
following features:
 - I/O expander (16 GPIO's) using PCA955x protocol
 - Real Time Clock using DS1672 protocol
 - User EEPROM using AT24 protocol
 - HWMON using custom protocol
 - Interrupt controller with tamper detect, user pushbotton
 - Watchdog controller capable of full board power-cycle
 - Power Control capable of full board power-cycle

see http://trac.gateworks.com/wiki/gsc for more details

Cc: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
v2:
- change license comment block style
- remove COMPILE_TEST (Randy)
- fixed whitespace issues
- replaced a printk with dev_err

 drivers/mfd/Kconfig     |  13 ++
 drivers/mfd/Makefile    |   1 +
 drivers/mfd/gsc.c       | 330 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/gsc.h |  82 ++++++++++++
 4 files changed, 426 insertions(+)
 create mode 100644 drivers/mfd/gsc.c
 create mode 100644 include/linux/mfd/gsc.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 1d20a80..5694013 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -341,6 +341,19 @@ config MFD_EXYNOS_LPASS
 	  Select this option to enable support for Samsung Exynos Low Power
 	  Audio Subsystem.
 
+config MFD_GSC
+	tristate "Gateworks System Controller"
+	depends on (I2C && OF)
+	select MFD_CORE
+	select REGMAP_I2C
+	select REGMAP_IRQ
+	help
+	  Enable support for the Gateworks System Controller found
+	  on Gateworks Single Board Computers.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called gsc.
+
 config MFD_MC13XXX
 	tristate
 	depends on (SPI_MASTER || I2C)
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index d9474ad..aede0bc 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_MFD_CROS_EC)	+= cros_ec_core.o
 obj-$(CONFIG_MFD_CROS_EC_I2C)	+= cros_ec_i2c.o
 obj-$(CONFIG_MFD_CROS_EC_SPI)	+= cros_ec_spi.o
 obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
+obj-$(CONFIG_MFD_GSC)		+= gsc.o
 
 rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
 obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
diff --git a/drivers/mfd/gsc.c b/drivers/mfd/gsc.c
new file mode 100644
index 0000000..4ac400a
--- /dev/null
+++ b/drivers/mfd/gsc.c
@@ -0,0 +1,330 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2018 Gateworks Corporation
+ *
+ * The Gateworks System Controller (GSC) is a family of a multi-function
+ * "Power Management and System Companion Device" chips originally designed for
+ * use in Gateworks Single Board Computers. The control interface is I2C,
+ * at 100kbps, with an interrupt.
+ *
+ */
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/gsc.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/*
+ * The GSC suffers from an errata where occasionally during
+ * ADC cycles the chip can NAK i2c transactions. To ensure we have reliable
+ * register access we place retries around register access.
+ */
+#define I2C_RETRIES	3
+
+static int gsc_regmap_regwrite(void *context, unsigned int reg,
+			       unsigned int val)
+{
+	struct i2c_client *client = context;
+	int retry, ret;
+
+	for (retry = 0; retry < I2C_RETRIES; retry++) {
+		ret = i2c_smbus_write_byte_data(client, reg, val);
+		/*
+		 * -EAGAIN returned when the i2c host controller is busy
+		 * -EIO returned when i2c device is busy
+		 */
+		if (ret != -EAGAIN && ret != -EIO)
+			break;
+	}
+	if (ret < 0) {
+		dev_err(&client->dev, ">> 0x%02x %d\n", reg, ret);
+		return ret;
+	}
+	dev_dbg(&client->dev, ">> 0x%02x=0x%02x (%d)\n", reg, val, retry);
+
+	return 0;
+}
+
+static int gsc_regmap_regread(void *context, unsigned int reg,
+			      unsigned int *val)
+{
+	struct i2c_client *client = context;
+	int retry, ret;
+
+	for (retry = 0; retry < I2C_RETRIES; retry++) {
+		ret = i2c_smbus_read_byte_data(client, reg);
+		/*
+		 * -EAGAIN returned when the i2c host controller is busy
+		 * -EIO returned when i2c device is busy
+		 */
+		if (ret != -EAGAIN && ret != -EIO)
+			break;
+	}
+	if (ret < 0) {
+		dev_err(&client->dev, "<< 0x%02x %d\n", reg, ret);
+		return ret;
+	}
+
+	*val = ret & 0xff;
+	dev_dbg(&client->dev, "<< 0x%02x=0x%02x (%d)\n", reg, *val, retry);
+
+	return 0;
+}
+
+static struct regmap_bus regmap_gsc = {
+	.reg_write = gsc_regmap_regwrite,
+	.reg_read = gsc_regmap_regread,
+};
+
+/*
+ * gsc_powerdown - API to use GSC to power down board for a specific time
+ *
+ * secs - number of seconds to remain powered off
+ */
+static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
+{
+	int ret;
+	unsigned char regs[4];
+
+	dev_info(&gsc->i2c->dev, "GSC powerdown for %ld seconds\n",
+		 secs);
+	regs[0] = secs & 0xff;
+	regs[1] = (secs >> 8) & 0xff;
+	regs[2] = (secs >> 16) & 0xff;
+	regs[3] = (secs >> 24) & 0xff;
+	ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
+
+	return ret;
+}
+
+/*
+ * sysfs hooks
+ */
+static ssize_t gsc_show(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct gsc_dev *gsc = dev_get_drvdata(dev);
+	const char *name = attr->attr.name;
+	int rz = 0;
+
+	if (strcasecmp(name, "fw_version") == 0)
+		rz = sprintf(buf, "%d\n", gsc->fwver);
+	else if (strcasecmp(name, "fw_crc") == 0)
+		rz = sprintf(buf, "0x%04x\n", gsc->fwcrc);
+
+	return rz;
+}
+
+static ssize_t gsc_store(struct device *dev, struct device_attribute *attr,
+			 const char *buf, size_t count)
+{
+	struct gsc_dev *gsc = dev_get_drvdata(dev);
+	const char *name = attr->attr.name;
+	int ret;
+
+	if (strcasecmp(name, "powerdown") == 0) {
+		long value;
+
+		ret = kstrtol(buf, 0, &value);
+		if (ret == 0)
+			gsc_powerdown(gsc, value);
+	} else
+		dev_err(dev, "invalid name '%s\n", name);
+
+	return count;
+}
+
+
+/*
+ * Create a group of attributes so that we can create and destroy them all
+ * at once.
+ */
+static struct device_attribute attr_fwver =
+	__ATTR(fw_version, 0440, gsc_show, NULL);
+static struct device_attribute attr_fwcrc =
+	__ATTR(fw_crc, 0440, gsc_show, NULL);
+static struct device_attribute attr_pwrdown =
+	__ATTR(powerdown, 0220, NULL, gsc_store);
+
+static struct attribute *gsc_attrs[] = {
+	&attr_fwver.attr,
+	&attr_fwcrc.attr,
+	&attr_pwrdown.attr,
+	NULL,
+};
+
+static struct attribute_group attr_group = {
+	.attrs = gsc_attrs,
+};
+
+static const struct i2c_device_id gsc_i2c_ids[] = {
+	{ "gsc_v1", gsc_v1 },
+	{ "gsc_v2", gsc_v2 },
+	{ "gsc_v3", gsc_v3 },
+	{ },
+};
+
+static const struct of_device_id gsc_of_match[] = {
+	{ .compatible = "gw,gsc_v1", .data = (void *)gsc_v1 },
+	{ .compatible = "gw,gsc_v2", .data = (void *)gsc_v2 },
+	{ .compatible = "gw,gsc_v3", .data = (void *)gsc_v3 },
+	{ }
+};
+
+static const struct regmap_config gsc_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_NONE,
+	.max_register = 0xf,
+};
+
+static const struct regmap_config gsc_regmap_hwmon_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_NONE,
+	.max_register = 0x37,
+};
+
+static const struct regmap_irq gsc_irqs[] = {
+	REGMAP_IRQ_REG(GSC_IRQ_PB, 0, BIT(GSC_IRQ_PB)),
+	REGMAP_IRQ_REG(GSC_IRQ_KEY_ERASED, 0, BIT(GSC_IRQ_KEY_ERASED)),
+	REGMAP_IRQ_REG(GSC_IRQ_EEPROM_WP, 0, BIT(GSC_IRQ_EEPROM_WP)),
+	REGMAP_IRQ_REG(GSC_IRQ_RESV, 0, BIT(GSC_IRQ_RESV)),
+	REGMAP_IRQ_REG(GSC_IRQ_GPIO, 0, BIT(GSC_IRQ_GPIO)),
+	REGMAP_IRQ_REG(GSC_IRQ_TAMPER, 0, BIT(GSC_IRQ_TAMPER)),
+	REGMAP_IRQ_REG(GSC_IRQ_WDT_TIMEOUT, 0, BIT(GSC_IRQ_WDT_TIMEOUT)),
+	REGMAP_IRQ_REG(GSC_IRQ_SWITCH_HOLD, 0, BIT(GSC_IRQ_SWITCH_HOLD)),
+};
+
+static const struct regmap_irq_chip gsc_irq_chip = {
+	.name = KBUILD_MODNAME,
+	.irqs = gsc_irqs,
+	.num_irqs = ARRAY_SIZE(gsc_irqs),
+	.num_regs = 1,
+	.status_base = GSC_IRQ_STATUS,
+	.mask_base = GSC_IRQ_ENABLE,
+	.mask_invert = true,
+	.ack_base = GSC_IRQ_STATUS,
+	.ack_invert = true,
+};
+
+static int gsc_of_probe(struct device_node *np, struct gsc_dev *gsc)
+{
+	const struct of_device_id *of_id;
+
+	if (!np)
+		return -ENODEV;
+
+	of_id = of_match_device(gsc_of_match, gsc->dev);
+	if (of_id)
+		gsc->type = (enum gsc_type)of_id->data;
+
+	return 0;
+}
+
+static int
+gsc_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct gsc_dev *gsc;
+	int ret;
+	unsigned int reg;
+
+	gsc = devm_kzalloc(dev, sizeof(*gsc), GFP_KERNEL);
+	if (!gsc)
+		return -ENOMEM;
+
+	gsc->dev = &client->dev;
+	gsc->i2c = client;
+	gsc->irq = client->irq;
+	i2c_set_clientdata(client, gsc);
+
+	gsc->regmap = devm_regmap_init(dev, &regmap_gsc, client,
+				       &gsc_regmap_config);
+	if (IS_ERR(gsc->regmap))
+		return PTR_ERR(gsc->regmap);
+
+	ret = gsc_of_probe(dev->of_node, gsc);
+	if (reg < 0)
+		return ret;
+
+	if (regmap_read(gsc->regmap, GSC_FW_VER, &reg))
+		return -EIO;
+	gsc->fwver = reg;
+	regmap_read(gsc->regmap, GSC_FW_CRC, &reg);
+	gsc->fwcrc = reg;
+	regmap_read(gsc->regmap, GSC_FW_CRC + 1, &reg);
+	gsc->fwcrc |= reg << 8;
+
+	gsc->i2c_hwmon = i2c_new_dummy(client->adapter, GSC_HWMON);
+	if (!gsc->i2c_hwmon) {
+		dev_err(dev, "Failed to allocate I2C device for HWMON\n");
+		return -ENODEV;
+	}
+	i2c_set_clientdata(gsc->i2c_hwmon, gsc);
+
+	gsc->regmap_hwmon = devm_regmap_init(dev, &regmap_gsc, gsc->i2c_hwmon,
+					     &gsc_regmap_hwmon_config);
+	if (IS_ERR(gsc->regmap_hwmon)) {
+		ret = PTR_ERR(gsc->regmap_hwmon);
+		dev_err(dev, "failed to allocate register map: %d\n", ret);
+		goto err_regmap;
+	}
+
+	ret = devm_regmap_add_irq_chip(dev, gsc->regmap, gsc->irq,
+				       IRQF_ONESHOT | IRQF_SHARED |
+				       IRQF_TRIGGER_FALLING, 0,
+				       &gsc_irq_chip, &gsc->irq_chip_data);
+	if (ret)
+		goto err_regmap;
+
+	dev_info(dev, "Gateworks System Controller v%d: fw v%02d 0x%04x\n",
+		 gsc->type, gsc->fwver, gsc->fwcrc);
+
+	/* sysfs hooks */
+	ret = sysfs_create_group(&dev->kobj, &attr_group);
+	if (ret)
+		dev_err(dev, "failed to create sysfs attrs\n");
+
+	ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+	if (ret)
+		goto err_sysfs;
+
+	return 0;
+
+err_sysfs:
+	sysfs_remove_group(&dev->kobj, &attr_group);
+err_regmap:
+	i2c_unregister_device(gsc->i2c_hwmon);
+
+	return ret;
+}
+
+static int gsc_remove(struct i2c_client *client)
+{
+	sysfs_remove_group(&client->dev.kobj, &attr_group);
+
+	return 0;
+}
+
+static struct i2c_driver gsc_driver = {
+	.driver = {
+		.name	= KBUILD_MODNAME,
+		.of_match_table = of_match_ptr(gsc_of_match),
+	},
+	.probe		= gsc_probe,
+	.remove		= gsc_remove,
+	.id_table	= gsc_i2c_ids,
+};
+
+module_i2c_driver(gsc_driver);
+
+MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
+MODULE_DESCRIPTION("I2C Core interface for GSC");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/gsc.h b/include/linux/mfd/gsc.h
new file mode 100644
index 0000000..e0d8c8f
--- /dev/null
+++ b/include/linux/mfd/gsc.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2018 Gateworks Corporation
+ */
+#ifndef __LINUX_MFD_GSC_H_
+#define __LINUX_MFD_GSC_H_
+
+/* Device Addresses */
+#define GSC_MISC	0x20
+#define GSC_UPDATE	0x21
+#define GSC_GPIO	0x23
+#define GSC_HWMON	0x29
+#define GSC_EEPROM0	0x50
+#define GSC_EEPROM1	0x51
+#define GSC_EEPROM2	0x52
+#define GSC_EEPROM3	0x53
+#define GSC_RTC		0x68
+
+/* Register offsets */
+#define GSC_CTRL_0	0x00
+#define GSC_CTRL_1	0x01
+#define GSC_TIME	0x02
+#define GSC_TIME_ADD	0x06
+#define GSC_IRQ_STATUS	0x0A
+#define GSC_IRQ_ENABLE	0x0B
+#define GSC_FW_CRC	0x0C
+#define GSC_FW_VER	0x0E
+#define GSC_WP		0x0F
+
+/* Bit definitions */
+#define GSC_CTRL_0_PB_HARD_RESET	0
+#define GSC_CTRL_0_PB_CLEAR_SECURE_KEY	1
+#define GSC_CTRL_0_PB_SOFT_POWER_DOWN	2
+#define GSC_CTRL_0_PB_BOOT_ALTERNATE	3
+#define GSC_CTRL_0_PERFORM_CRC		4
+#define GSC_CTRL_0_TAMPER_DETECT	5
+#define GSC_CTRL_0_SWITCH_HOLD		6
+
+#define GSC_CTRL_1_SLEEP_ENABLE		0
+#define GSC_CTRL_1_ACTIVATE_SLEEP	1
+#define GSC_CTRL_1_LATCH_SLEEP_ADD	2
+#define GSC_CTRL_1_SLEEP_NOWAKEPB	3
+#define GSC_CTRL_1_WDT_TIME		4
+#define GSC_CTRL_1_WDT_ENABLE		5
+#define GSC_CTRL_1_SWITCH_BOOT_ENABLE	6
+#define GSC_CTRL_1_SWITCH_BOOT_CLEAR	7
+
+#define GSC_IRQ_PB			0
+#define GSC_IRQ_KEY_ERASED		1
+#define GSC_IRQ_EEPROM_WP		2
+#define GSC_IRQ_RESV			3
+#define GSC_IRQ_GPIO			4
+#define GSC_IRQ_TAMPER			5
+#define GSC_IRQ_WDT_TIMEOUT		6
+#define GSC_IRQ_SWITCH_HOLD		7
+
+/* Max registers */
+#define GSC_HWMON_MAX_REG	56
+
+enum gsc_type {
+	gsc_v1 = 1,
+	gsc_v2 = 2,
+	gsc_v3 = 3,
+};
+
+struct gsc_dev {
+	struct device *dev;
+
+	struct i2c_client *i2c;		/* 0x20: interrupt controller, WDT */
+	struct i2c_client *i2c_hwmon;	/* 0x29: hwmon, fan controller */
+
+	struct regmap *regmap;
+	struct regmap *regmap_hwmon;
+	struct regmap_irq_chip_data *irq_chip_data;
+
+	int irq;
+	enum gsc_type type;
+	unsigned int fwver;
+	unsigned short fwcrc;
+};
+
+#endif /* __LINUX_MFD_GSC_H_ */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 3/4] hwmon: add Gateworks System Controller support
  2018-03-05 22:02 ` Tim Harvey
@ 2018-03-05 22:02   ` Tim Harvey
  -1 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-05 22:02 UTC (permalink / raw)
  To: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
	linux-input, Guenter Roeck

The Gateworks System Controller has a hwmon sub-component that exposes
up to 16 ADC's, some of which are temperature sensors, others which are
voltage inputs. The ADC configuration (register mapping and name) is
configured via device-tree and varies board to board.

Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
v2:
- change license comment style
- remove DEBUG
- simplify regmap_bulk_read err check
- remove break after returns in switch statement
- fix fan setpoint buffer address
- remove unnecessary parens
- consistently use struct device *dev pointer
- change license/comment block
- add validation for hwmon child node props
- move parsing of of to own function
- use strlcpy to ensure null termination
- fix static array sizes and removed unnecessary initializers
- dynamically allocate channels
- fix fan input label
- support platform data
- fixed whitespace issues

 drivers/hwmon/Kconfig                   |   9 +
 drivers/hwmon/Makefile                  |   1 +
 drivers/hwmon/gsc-hwmon.c               | 330 ++++++++++++++++++++++++++++++++
 include/linux/platform_data/gsc_hwmon.h |  34 ++++
 4 files changed, 374 insertions(+)
 create mode 100644 drivers/hwmon/gsc-hwmon.c
 create mode 100644 include/linux/platform_data/gsc_hwmon.h

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 7ad0176..e052798b 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -475,6 +475,15 @@ config SENSORS_F75375S
 	  This driver can also be built as a module.  If so, the module
 	  will be called f75375s.
 
+config SENSORS_GSC
+        tristate "Gateworks System Controller ADC"
+        depends on MFD_GSC
+        help
+          Support for the Gateworks System Controller A/D converters.
+
+	  To compile this driver as a module, choose M here:
+	  the module will be called gsc-hwmon.
+
 config SENSORS_MC13783_ADC
         tristate "Freescale MC13783/MC13892 ADC"
         depends on MFD_MC13XXX
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 0fe489f..835a536 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_SENSORS_G760A)	+= g760a.o
 obj-$(CONFIG_SENSORS_G762)	+= g762.o
 obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
 obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
+obj-$(CONFIG_SENSORS_GSC)	+= gsc-hwmon.o
 obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
 obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
 obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
diff --git a/drivers/hwmon/gsc-hwmon.c b/drivers/hwmon/gsc-hwmon.c
new file mode 100644
index 0000000..cf55457
--- /dev/null
+++ b/drivers/hwmon/gsc-hwmon.c
@@ -0,0 +1,330 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2018 Gateworks Corporation
+ *
+ * This driver registers Linux HWMON attributes for GSC ADC's
+ */
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/mfd/gsc.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include <linux/platform_data/gsc_hwmon.h>
+
+#define GSC_HWMON_MAX_TEMP_CH	16
+#define GSC_HWMON_MAX_IN_CH	16
+#define GSC_HWMON_MAX_FAN_CH	6
+
+struct gsc_hwmon_data {
+	struct gsc_dev *gsc;
+	struct device *dev;
+	struct gsc_hwmon_platform_data *pdata;
+	const struct gsc_hwmon_channel *temp_ch[GSC_HWMON_MAX_TEMP_CH];
+	const struct gsc_hwmon_channel *in_ch[GSC_HWMON_MAX_IN_CH];
+	const struct gsc_hwmon_channel *fan_ch[GSC_HWMON_MAX_FAN_CH];
+	u32 temp_config[GSC_HWMON_MAX_TEMP_CH + 1];
+	u32 in_config[GSC_HWMON_MAX_IN_CH + 1];
+	u32 fan_config[GSC_HWMON_MAX_FAN_CH + 1];
+	struct hwmon_channel_info temp_info;
+	struct hwmon_channel_info in_info;
+	struct hwmon_channel_info fan_info;
+	const struct hwmon_channel_info *info[4];
+	struct hwmon_chip_info chip;
+};
+
+static int
+gsc_hwmon_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+	       int channel, long *val)
+{
+	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
+	int sz, ret;
+	u8 reg;
+	u8 buf[3];
+
+	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
+		channel);
+	switch (type) {
+	case hwmon_in:
+		if (channel > GSC_HWMON_MAX_IN_CH)
+			return -EOPNOTSUPP;
+		reg = hwmon->in_ch[channel]->reg;
+		sz = 3;
+		break;
+	case hwmon_temp:
+		if (channel > GSC_HWMON_MAX_TEMP_CH)
+			return -EOPNOTSUPP;
+		reg = hwmon->temp_ch[channel]->reg;
+		sz = 2;
+		break;
+	case hwmon_fan:
+		if (channel > GSC_HWMON_MAX_FAN_CH)
+			return -EOPNOTSUPP;
+		reg = hwmon->fan_ch[channel]->reg;
+		sz = 2;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	ret = regmap_bulk_read(hwmon->gsc->regmap_hwmon, reg, &buf, sz);
+	if (ret)
+		return ret;
+
+	*val = 0;
+	while (sz-- > 0)
+		*val |= (buf[sz] << (8*sz));
+	if ((type == hwmon_temp) && *val > 0x8000)
+		*val -= 0xffff;
+
+	return 0;
+}
+
+static int
+gsc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
+		      u32 attr, int channel, const char **buf)
+{
+	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
+		channel);
+	switch (type) {
+	case hwmon_in:
+		if (channel > GSC_HWMON_MAX_IN_CH)
+			return -EOPNOTSUPP;
+		*buf = hwmon->in_ch[channel]->name;
+		break;
+	case hwmon_temp:
+		if (channel > GSC_HWMON_MAX_TEMP_CH)
+			return -EOPNOTSUPP;
+		*buf = hwmon->temp_ch[channel]->name;
+		break;
+	case hwmon_fan:
+		if (channel > GSC_HWMON_MAX_FAN_CH)
+			return -EOPNOTSUPP;
+		*buf = hwmon->fan_ch[channel]->name;
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	return 0;
+}
+
+static int
+gsc_hwmon_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+		int channel, long val)
+{
+	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
+	u8 buf[3];
+
+	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
+		channel);
+	switch (type) {
+	case hwmon_fan:
+		if (channel == GSC_HWMON_MAX_FAN_CH)
+			return -EOPNOTSUPP;
+		buf[0] = val & 0xff;
+		buf[1] = (val >> 8) & 0xff;
+		return regmap_bulk_write(hwmon->gsc->regmap_hwmon,
+					 hwmon->fan_ch[channel]->reg, buf, 2);
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static umode_t
+gsc_hwmon_is_visible(const void *_data, enum hwmon_sensor_types type, u32 attr,
+		     int ch)
+{
+	const struct gsc_hwmon_data *hwmon = _data;
+	struct device *dev = hwmon->gsc->dev;
+	umode_t mode = 0;
+
+	switch (type) {
+	case hwmon_fan:
+		mode = S_IRUGO;
+		if (attr == hwmon_fan_input)
+			mode |= S_IWUSR;
+		break;
+	case hwmon_temp:
+	case hwmon_in:
+		mode = S_IRUGO;
+		break;
+	default:
+		break;
+	}
+	dev_dbg(dev, "%s type=%d attr=%d ch=%d mode=0x%x\n", __func__, type,
+		attr, ch, mode);
+
+	return mode;
+}
+
+static const struct hwmon_ops gsc_hwmon_ops = {
+	.is_visible = gsc_hwmon_is_visible,
+	.read = gsc_hwmon_read,
+	.read_string = gsc_hwmon_read_string,
+	.write = gsc_hwmon_write,
+};
+
+static struct gsc_hwmon_platform_data *
+gsc_hwmon_get_devtree_pdata(struct device *dev)
+{
+	struct gsc_hwmon_platform_data *pdata;
+	struct gsc_hwmon_channel *ch;
+	struct fwnode_handle *child;
+	int nchannels;
+
+	nchannels = device_get_child_node_count(dev);
+	dev_dbg(dev, "channels=%d\n", nchannels);
+	if (nchannels == 0)
+		return ERR_PTR(-ENODEV);
+
+	pdata = devm_kzalloc(dev,
+			     sizeof(*pdata) + nchannels * sizeof(*ch),
+			     GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+	ch = (struct gsc_hwmon_channel *)(pdata + 1);
+	pdata->channels = ch;
+	pdata->nchannels = nchannels;
+
+	/* allocate structures for channels and count instances of each type */
+	device_for_each_child_node(dev, child) {
+		if (fwnode_property_read_string(child, "label", &ch->name)) {
+			dev_err(dev, "channel without label\n");
+			fwnode_handle_put(child);
+			return ERR_PTR(-EINVAL);
+		}
+		if (fwnode_property_read_u32(child, "reg", &ch->reg)) {
+			dev_err(dev, "channel without reg\n");
+			fwnode_handle_put(child);
+			return ERR_PTR(-EINVAL);
+		}
+		if (fwnode_property_read_u32(child, "type", &ch->type)) {
+			dev_err(dev, "channel without type\n");
+			fwnode_handle_put(child);
+			return ERR_PTR(-EINVAL);
+		}
+		dev_dbg(dev, "of: reg=0x%02x type=%d %s\n", ch->reg, ch->type,
+			ch->name);
+		ch++;
+	}
+
+	return pdata;
+}
+
+static int gsc_hwmon_probe(struct platform_device *pdev)
+{
+	struct gsc_dev *gsc = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct gsc_hwmon_platform_data *pdata = dev_get_platdata(dev);
+	struct gsc_hwmon_data *hwmon;
+	int i, i_in, i_temp, i_fan;
+
+	if (!pdata) {
+		pdata = gsc_hwmon_get_devtree_pdata(dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
+
+	hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
+	if (!hwmon)
+		return -ENOMEM;
+	hwmon->gsc = gsc;
+	hwmon->pdata = pdata;
+
+	for (i = 0, i_in = 0, i_temp = 0, i_fan = 0;
+	     i < hwmon->pdata->nchannels; i++) {
+		const struct gsc_hwmon_channel *ch = &pdata->channels[i];
+
+		if (ch->reg > GSC_HWMON_MAX_REG) {
+			dev_err(dev, "invalid reg: 0x%02x\n", ch->reg);
+			return -EINVAL;
+		}
+		switch (ch->type) {
+		case type_temperature:
+			if (i_temp == GSC_HWMON_MAX_TEMP_CH) {
+				dev_err(dev, "too many temp channels\n");
+				return -EINVAL;
+			}
+			hwmon->temp_ch[i_temp] = ch;
+			hwmon->temp_config[i_temp] = HWMON_T_INPUT |
+						     HWMON_T_LABEL;
+			i_temp++;
+			break;
+		case type_voltage:
+			if (i_in == GSC_HWMON_MAX_IN_CH) {
+				dev_err(dev, "too many voltage channels\n");
+				return -EINVAL;
+			}
+			hwmon->in_ch[i_in] = ch;
+			hwmon->in_config[i_in] =
+				HWMON_I_INPUT | HWMON_I_LABEL;
+			i_in++;
+			break;
+		case type_fan:
+			if (i_fan == GSC_HWMON_MAX_FAN_CH) {
+				dev_err(dev, "too many voltage channels\n");
+				return -EINVAL;
+			}
+			hwmon->fan_ch[i_fan] = ch;
+			hwmon->fan_config[i_fan] =
+				HWMON_F_INPUT | HWMON_F_LABEL;
+			i_fan++;
+			break;
+		default:
+			dev_err(dev, "invalid type: %d\n", ch->type);
+			return -EINVAL;
+		}
+		dev_dbg(dev, "pdata: reg=0x%02x type=%d %s\n", ch->reg,
+			ch->type, ch->name);
+	}
+
+	/* terminate channel config lists */
+	hwmon->temp_config[i_temp] = 0;
+	hwmon->in_config[i_in] = 0;
+	hwmon->fan_config[i_fan] = 0;
+
+	/* setup config structures */
+	hwmon->chip.ops = &gsc_hwmon_ops;
+	hwmon->chip.info = hwmon->info;
+	hwmon->info[0] = &hwmon->temp_info;
+	hwmon->info[1] = &hwmon->in_info;
+	hwmon->info[2] = &hwmon->fan_info;
+	hwmon->temp_info.type = hwmon_temp;
+	hwmon->temp_info.config = hwmon->temp_config;
+	hwmon->in_info.type = hwmon_in;
+	hwmon->in_info.config = hwmon->in_config;
+	hwmon->fan_info.type = hwmon_fan;
+	hwmon->fan_info.config = hwmon->fan_config;
+
+	hwmon->dev = devm_hwmon_device_register_with_info(dev,
+							  KBUILD_MODNAME, hwmon,
+							  &hwmon->chip, NULL);
+	return PTR_ERR_OR_ZERO(hwmon->dev);
+}
+
+static const struct of_device_id gsc_hwmon_of_match[] = {
+	{ .compatible = "gw,gsc-hwmon", },
+	{}
+};
+
+static struct platform_driver gsc_hwmon_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = gsc_hwmon_of_match,
+	},
+	.probe = gsc_hwmon_probe,
+};
+
+module_platform_driver(gsc_hwmon_driver);
+
+MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
+MODULE_DESCRIPTION("GSC hardware monitor driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/platform_data/gsc_hwmon.h b/include/linux/platform_data/gsc_hwmon.h
new file mode 100644
index 0000000..ba4f811
--- /dev/null
+++ b/include/linux/platform_data/gsc_hwmon.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _GSC_HWMON_H
+#define _GSC_HWMON_H
+
+enum gsc_hwmon_type {
+	type_temperature,
+	type_voltage,
+	type_fan,
+};
+
+/**
+ * struct gsc_hwmon_channel - configuration parameters
+ * @reg:  I2C register offset
+ * @type: channel type
+ * @name: channel name
+ */
+struct gsc_hwmon_channel {
+	unsigned int reg;
+	unsigned int type;
+	const char *name;
+};
+
+/**
+ * struct gsc_hwmon_platform_data - platform data for gsc_hwmon driver
+ * @channels:	pointer to array of gsc_hwmon_channel structures
+ *		describing channels
+ * @nchannels:	number of elements in @channels array
+ */
+struct gsc_hwmon_platform_data {
+	const struct gsc_hwmon_channel *channels;
+	int nchannels;
+};
+
+#endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 3/4] hwmon: add Gateworks System Controller support
@ 2018-03-05 22:02   ` Tim Harvey
  0 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-05 22:02 UTC (permalink / raw)
  To: linux-arm-kernel

The Gateworks System Controller has a hwmon sub-component that exposes
up to 16 ADC's, some of which are temperature sensors, others which are
voltage inputs. The ADC configuration (register mapping and name) is
configured via device-tree and varies board to board.

Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
v2:
- change license comment style
- remove DEBUG
- simplify regmap_bulk_read err check
- remove break after returns in switch statement
- fix fan setpoint buffer address
- remove unnecessary parens
- consistently use struct device *dev pointer
- change license/comment block
- add validation for hwmon child node props
- move parsing of of to own function
- use strlcpy to ensure null termination
- fix static array sizes and removed unnecessary initializers
- dynamically allocate channels
- fix fan input label
- support platform data
- fixed whitespace issues

 drivers/hwmon/Kconfig                   |   9 +
 drivers/hwmon/Makefile                  |   1 +
 drivers/hwmon/gsc-hwmon.c               | 330 ++++++++++++++++++++++++++++++++
 include/linux/platform_data/gsc_hwmon.h |  34 ++++
 4 files changed, 374 insertions(+)
 create mode 100644 drivers/hwmon/gsc-hwmon.c
 create mode 100644 include/linux/platform_data/gsc_hwmon.h

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 7ad0176..e052798b 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -475,6 +475,15 @@ config SENSORS_F75375S
 	  This driver can also be built as a module.  If so, the module
 	  will be called f75375s.
 
+config SENSORS_GSC
+        tristate "Gateworks System Controller ADC"
+        depends on MFD_GSC
+        help
+          Support for the Gateworks System Controller A/D converters.
+
+	  To compile this driver as a module, choose M here:
+	  the module will be called gsc-hwmon.
+
 config SENSORS_MC13783_ADC
         tristate "Freescale MC13783/MC13892 ADC"
         depends on MFD_MC13XXX
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 0fe489f..835a536 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_SENSORS_G760A)	+= g760a.o
 obj-$(CONFIG_SENSORS_G762)	+= g762.o
 obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
 obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
+obj-$(CONFIG_SENSORS_GSC)	+= gsc-hwmon.o
 obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
 obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
 obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
diff --git a/drivers/hwmon/gsc-hwmon.c b/drivers/hwmon/gsc-hwmon.c
new file mode 100644
index 0000000..cf55457
--- /dev/null
+++ b/drivers/hwmon/gsc-hwmon.c
@@ -0,0 +1,330 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2018 Gateworks Corporation
+ *
+ * This driver registers Linux HWMON attributes for GSC ADC's
+ */
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/mfd/gsc.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include <linux/platform_data/gsc_hwmon.h>
+
+#define GSC_HWMON_MAX_TEMP_CH	16
+#define GSC_HWMON_MAX_IN_CH	16
+#define GSC_HWMON_MAX_FAN_CH	6
+
+struct gsc_hwmon_data {
+	struct gsc_dev *gsc;
+	struct device *dev;
+	struct gsc_hwmon_platform_data *pdata;
+	const struct gsc_hwmon_channel *temp_ch[GSC_HWMON_MAX_TEMP_CH];
+	const struct gsc_hwmon_channel *in_ch[GSC_HWMON_MAX_IN_CH];
+	const struct gsc_hwmon_channel *fan_ch[GSC_HWMON_MAX_FAN_CH];
+	u32 temp_config[GSC_HWMON_MAX_TEMP_CH + 1];
+	u32 in_config[GSC_HWMON_MAX_IN_CH + 1];
+	u32 fan_config[GSC_HWMON_MAX_FAN_CH + 1];
+	struct hwmon_channel_info temp_info;
+	struct hwmon_channel_info in_info;
+	struct hwmon_channel_info fan_info;
+	const struct hwmon_channel_info *info[4];
+	struct hwmon_chip_info chip;
+};
+
+static int
+gsc_hwmon_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+	       int channel, long *val)
+{
+	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
+	int sz, ret;
+	u8 reg;
+	u8 buf[3];
+
+	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
+		channel);
+	switch (type) {
+	case hwmon_in:
+		if (channel > GSC_HWMON_MAX_IN_CH)
+			return -EOPNOTSUPP;
+		reg = hwmon->in_ch[channel]->reg;
+		sz = 3;
+		break;
+	case hwmon_temp:
+		if (channel > GSC_HWMON_MAX_TEMP_CH)
+			return -EOPNOTSUPP;
+		reg = hwmon->temp_ch[channel]->reg;
+		sz = 2;
+		break;
+	case hwmon_fan:
+		if (channel > GSC_HWMON_MAX_FAN_CH)
+			return -EOPNOTSUPP;
+		reg = hwmon->fan_ch[channel]->reg;
+		sz = 2;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	ret = regmap_bulk_read(hwmon->gsc->regmap_hwmon, reg, &buf, sz);
+	if (ret)
+		return ret;
+
+	*val = 0;
+	while (sz-- > 0)
+		*val |= (buf[sz] << (8*sz));
+	if ((type == hwmon_temp) && *val > 0x8000)
+		*val -= 0xffff;
+
+	return 0;
+}
+
+static int
+gsc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
+		      u32 attr, int channel, const char **buf)
+{
+	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
+		channel);
+	switch (type) {
+	case hwmon_in:
+		if (channel > GSC_HWMON_MAX_IN_CH)
+			return -EOPNOTSUPP;
+		*buf = hwmon->in_ch[channel]->name;
+		break;
+	case hwmon_temp:
+		if (channel > GSC_HWMON_MAX_TEMP_CH)
+			return -EOPNOTSUPP;
+		*buf = hwmon->temp_ch[channel]->name;
+		break;
+	case hwmon_fan:
+		if (channel > GSC_HWMON_MAX_FAN_CH)
+			return -EOPNOTSUPP;
+		*buf = hwmon->fan_ch[channel]->name;
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	return 0;
+}
+
+static int
+gsc_hwmon_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+		int channel, long val)
+{
+	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
+	u8 buf[3];
+
+	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
+		channel);
+	switch (type) {
+	case hwmon_fan:
+		if (channel == GSC_HWMON_MAX_FAN_CH)
+			return -EOPNOTSUPP;
+		buf[0] = val & 0xff;
+		buf[1] = (val >> 8) & 0xff;
+		return regmap_bulk_write(hwmon->gsc->regmap_hwmon,
+					 hwmon->fan_ch[channel]->reg, buf, 2);
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static umode_t
+gsc_hwmon_is_visible(const void *_data, enum hwmon_sensor_types type, u32 attr,
+		     int ch)
+{
+	const struct gsc_hwmon_data *hwmon = _data;
+	struct device *dev = hwmon->gsc->dev;
+	umode_t mode = 0;
+
+	switch (type) {
+	case hwmon_fan:
+		mode = S_IRUGO;
+		if (attr == hwmon_fan_input)
+			mode |= S_IWUSR;
+		break;
+	case hwmon_temp:
+	case hwmon_in:
+		mode = S_IRUGO;
+		break;
+	default:
+		break;
+	}
+	dev_dbg(dev, "%s type=%d attr=%d ch=%d mode=0x%x\n", __func__, type,
+		attr, ch, mode);
+
+	return mode;
+}
+
+static const struct hwmon_ops gsc_hwmon_ops = {
+	.is_visible = gsc_hwmon_is_visible,
+	.read = gsc_hwmon_read,
+	.read_string = gsc_hwmon_read_string,
+	.write = gsc_hwmon_write,
+};
+
+static struct gsc_hwmon_platform_data *
+gsc_hwmon_get_devtree_pdata(struct device *dev)
+{
+	struct gsc_hwmon_platform_data *pdata;
+	struct gsc_hwmon_channel *ch;
+	struct fwnode_handle *child;
+	int nchannels;
+
+	nchannels = device_get_child_node_count(dev);
+	dev_dbg(dev, "channels=%d\n", nchannels);
+	if (nchannels == 0)
+		return ERR_PTR(-ENODEV);
+
+	pdata = devm_kzalloc(dev,
+			     sizeof(*pdata) + nchannels * sizeof(*ch),
+			     GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+	ch = (struct gsc_hwmon_channel *)(pdata + 1);
+	pdata->channels = ch;
+	pdata->nchannels = nchannels;
+
+	/* allocate structures for channels and count instances of each type */
+	device_for_each_child_node(dev, child) {
+		if (fwnode_property_read_string(child, "label", &ch->name)) {
+			dev_err(dev, "channel without label\n");
+			fwnode_handle_put(child);
+			return ERR_PTR(-EINVAL);
+		}
+		if (fwnode_property_read_u32(child, "reg", &ch->reg)) {
+			dev_err(dev, "channel without reg\n");
+			fwnode_handle_put(child);
+			return ERR_PTR(-EINVAL);
+		}
+		if (fwnode_property_read_u32(child, "type", &ch->type)) {
+			dev_err(dev, "channel without type\n");
+			fwnode_handle_put(child);
+			return ERR_PTR(-EINVAL);
+		}
+		dev_dbg(dev, "of: reg=0x%02x type=%d %s\n", ch->reg, ch->type,
+			ch->name);
+		ch++;
+	}
+
+	return pdata;
+}
+
+static int gsc_hwmon_probe(struct platform_device *pdev)
+{
+	struct gsc_dev *gsc = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct gsc_hwmon_platform_data *pdata = dev_get_platdata(dev);
+	struct gsc_hwmon_data *hwmon;
+	int i, i_in, i_temp, i_fan;
+
+	if (!pdata) {
+		pdata = gsc_hwmon_get_devtree_pdata(dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
+
+	hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
+	if (!hwmon)
+		return -ENOMEM;
+	hwmon->gsc = gsc;
+	hwmon->pdata = pdata;
+
+	for (i = 0, i_in = 0, i_temp = 0, i_fan = 0;
+	     i < hwmon->pdata->nchannels; i++) {
+		const struct gsc_hwmon_channel *ch = &pdata->channels[i];
+
+		if (ch->reg > GSC_HWMON_MAX_REG) {
+			dev_err(dev, "invalid reg: 0x%02x\n", ch->reg);
+			return -EINVAL;
+		}
+		switch (ch->type) {
+		case type_temperature:
+			if (i_temp == GSC_HWMON_MAX_TEMP_CH) {
+				dev_err(dev, "too many temp channels\n");
+				return -EINVAL;
+			}
+			hwmon->temp_ch[i_temp] = ch;
+			hwmon->temp_config[i_temp] = HWMON_T_INPUT |
+						     HWMON_T_LABEL;
+			i_temp++;
+			break;
+		case type_voltage:
+			if (i_in == GSC_HWMON_MAX_IN_CH) {
+				dev_err(dev, "too many voltage channels\n");
+				return -EINVAL;
+			}
+			hwmon->in_ch[i_in] = ch;
+			hwmon->in_config[i_in] =
+				HWMON_I_INPUT | HWMON_I_LABEL;
+			i_in++;
+			break;
+		case type_fan:
+			if (i_fan == GSC_HWMON_MAX_FAN_CH) {
+				dev_err(dev, "too many voltage channels\n");
+				return -EINVAL;
+			}
+			hwmon->fan_ch[i_fan] = ch;
+			hwmon->fan_config[i_fan] =
+				HWMON_F_INPUT | HWMON_F_LABEL;
+			i_fan++;
+			break;
+		default:
+			dev_err(dev, "invalid type: %d\n", ch->type);
+			return -EINVAL;
+		}
+		dev_dbg(dev, "pdata: reg=0x%02x type=%d %s\n", ch->reg,
+			ch->type, ch->name);
+	}
+
+	/* terminate channel config lists */
+	hwmon->temp_config[i_temp] = 0;
+	hwmon->in_config[i_in] = 0;
+	hwmon->fan_config[i_fan] = 0;
+
+	/* setup config structures */
+	hwmon->chip.ops = &gsc_hwmon_ops;
+	hwmon->chip.info = hwmon->info;
+	hwmon->info[0] = &hwmon->temp_info;
+	hwmon->info[1] = &hwmon->in_info;
+	hwmon->info[2] = &hwmon->fan_info;
+	hwmon->temp_info.type = hwmon_temp;
+	hwmon->temp_info.config = hwmon->temp_config;
+	hwmon->in_info.type = hwmon_in;
+	hwmon->in_info.config = hwmon->in_config;
+	hwmon->fan_info.type = hwmon_fan;
+	hwmon->fan_info.config = hwmon->fan_config;
+
+	hwmon->dev = devm_hwmon_device_register_with_info(dev,
+							  KBUILD_MODNAME, hwmon,
+							  &hwmon->chip, NULL);
+	return PTR_ERR_OR_ZERO(hwmon->dev);
+}
+
+static const struct of_device_id gsc_hwmon_of_match[] = {
+	{ .compatible = "gw,gsc-hwmon", },
+	{}
+};
+
+static struct platform_driver gsc_hwmon_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = gsc_hwmon_of_match,
+	},
+	.probe = gsc_hwmon_probe,
+};
+
+module_platform_driver(gsc_hwmon_driver);
+
+MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
+MODULE_DESCRIPTION("GSC hardware monitor driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/platform_data/gsc_hwmon.h b/include/linux/platform_data/gsc_hwmon.h
new file mode 100644
index 0000000..ba4f811
--- /dev/null
+++ b/include/linux/platform_data/gsc_hwmon.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _GSC_HWMON_H
+#define _GSC_HWMON_H
+
+enum gsc_hwmon_type {
+	type_temperature,
+	type_voltage,
+	type_fan,
+};
+
+/**
+ * struct gsc_hwmon_channel - configuration parameters
+ * @reg:  I2C register offset
+ * @type: channel type
+ * @name: channel name
+ */
+struct gsc_hwmon_channel {
+	unsigned int reg;
+	unsigned int type;
+	const char *name;
+};
+
+/**
+ * struct gsc_hwmon_platform_data - platform data for gsc_hwmon driver
+ * @channels:	pointer to array of gsc_hwmon_channel structures
+ *		describing channels
+ * @nchannels:	number of elements in @channels array
+ */
+struct gsc_hwmon_platform_data {
+	const struct gsc_hwmon_channel *channels;
+	int nchannels;
+};
+
+#endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 4/4] input: misc: Add Gateworks System Controller support
  2018-03-05 22:02 ` Tim Harvey
@ 2018-03-05 22:02   ` Tim Harvey
  -1 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-05 22:02 UTC (permalink / raw)
  To: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-hwmon, linux-input

Add support for dispatching Linux Input events for the various interrupts
the Gateworks System Controller provides.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
v2:
- reword Kconfig
- revise license comment block
- remove unnecessary read of status register
- remove unnecessary setting of input->dev.parent
- use dt bindings for irq to keycode mapping
- add support for platform-data
- remove work-queue

 drivers/input/misc/Kconfig              |   9 ++
 drivers/input/misc/Makefile             |   1 +
 drivers/input/misc/gsc-input.c          | 180 ++++++++++++++++++++++++++++++++
 include/linux/platform_data/gsc_input.h |  30 ++++++
 4 files changed, 220 insertions(+)
 create mode 100644 drivers/input/misc/gsc-input.c
 create mode 100644 include/linux/platform_data/gsc_input.h

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 9f082a3..e05f4fe 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -117,6 +117,15 @@ config INPUT_E3X0_BUTTON
 	  To compile this driver as a module, choose M here: the
 	  module will be called e3x0_button.
 
+config INPUT_GSC
+	tristate "Gateworks System Controller input support"
+	depends on MFD_GSC
+	help
+	  Support GSC events as Linux input events.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called gsc_input.
+
 config INPUT_PCSPKR
 	tristate "PC Speaker support"
 	depends on PCSPKR_PLATFORM
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 4b6118d..969debe 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GP2A)		+= gp2ap002a00f.o
 obj-$(CONFIG_INPUT_GPIO_BEEPER)		+= gpio-beeper.o
 obj-$(CONFIG_INPUT_GPIO_TILT_POLLED)	+= gpio_tilt_polled.o
 obj-$(CONFIG_INPUT_GPIO_DECODER)	+= gpio_decoder.o
+obj-$(CONFIG_INPUT_GSC)			+= gsc-input.o
 obj-$(CONFIG_INPUT_HISI_POWERKEY)	+= hisi_powerkey.o
 obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
 obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
diff --git a/drivers/input/misc/gsc-input.c b/drivers/input/misc/gsc-input.c
new file mode 100644
index 0000000..27b5e93
--- /dev/null
+++ b/drivers/input/misc/gsc-input.c
@@ -0,0 +1,180 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2018 Gateworks Corporation
+ *
+ * This driver dispatches Linux input events for GSC interrupt events
+ */
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/gsc.h>
+#include <linux/platform_data/gsc_input.h>
+
+struct gsc_input_button_priv {
+	struct input_dev *input;
+	const struct gsc_input_button *button;
+};
+
+struct gsc_input_data {
+	struct gsc_dev *gsc;
+	struct gsc_input_platform_data *pdata;
+	struct input_dev *input;
+	struct gsc_input_button_priv *buttons;
+	int nbuttons;
+	unsigned int irqs[];
+#if 0
+	int irq;
+	struct work_struct irq_work;
+	struct mutex mutex;
+#endif
+};
+
+static irqreturn_t gsc_input_irq(int irq, void *data)
+{
+	const struct gsc_input_button_priv *button_priv = data;
+	struct input_dev *input = button_priv->input;
+
+	dev_dbg(&input->dev, "irq%d code=%d\n", irq, button_priv->button->code);
+	input_report_key(input, button_priv->button->code, 1);
+	input_sync(input);
+	input_report_key(input, button_priv->button->code, 0);
+	input_sync(input);
+
+	return IRQ_HANDLED;
+}
+
+static struct gsc_input_platform_data *
+gsc_input_get_devtree_pdata(struct device *dev)
+{
+	struct gsc_input_platform_data *pdata;
+	struct fwnode_handle *child;
+	struct gsc_input_button *button;
+	int nbuttons;
+
+	nbuttons = device_get_child_node_count(dev);
+	dev_dbg(dev, "buttons=%d\n", nbuttons);
+	if (nbuttons == 0)
+		return ERR_PTR(-ENODEV);
+
+	pdata = devm_kzalloc(dev,
+			     sizeof(*pdata) + nbuttons * sizeof(*button),
+			     GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	button = (struct gsc_input_button *)(pdata + 1);
+	pdata->buttons = button;
+	pdata->nbuttons = nbuttons;
+
+	device_property_read_string(dev, "label", &pdata->name);
+
+	device_for_each_child_node(dev, child) {
+		if (is_of_node(child))
+			button->irq = irq_of_parse_and_map(to_of_node(child),
+							   0);
+		if (fwnode_property_read_u32(child, "linux,code",
+					     &button->code)) {
+			dev_err(dev, "Button without keycode\n");
+			fwnode_handle_put(child);
+			return ERR_PTR(-EINVAL);
+		}
+		fwnode_property_read_string(child, "label", &button->desc);
+		button++;
+	}
+
+	return pdata;
+}
+
+static int gsc_input_probe(struct platform_device *pdev)
+{
+	struct gsc_dev *gsc = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct gsc_input_platform_data *pdata = dev_get_platdata(dev);
+	struct gsc_input_data *input;
+	int i, ret;
+
+	if (!pdata) {
+		pdata = gsc_input_get_devtree_pdata(dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
+
+	input = devm_kzalloc(dev, sizeof(*input), GFP_KERNEL);
+	if (!input)
+		return -ENOMEM;
+	input->gsc = gsc;
+	input->pdata = pdata;
+
+	input->buttons = devm_kzalloc(dev, pdata->nbuttons *
+				      sizeof(struct gsc_input_button_priv),
+				      GFP_KERNEL);
+	if (!input->buttons)
+		return -ENOMEM;
+
+	/* Register input device */
+	input->input = devm_input_allocate_device(dev);
+	if (!input->input) {
+		dev_err(dev, "Can't allocate input device\n");
+		return -ENOMEM;
+	}
+	input->input->name = pdata->name ? : pdev->name;
+
+	platform_set_drvdata(pdev, gsc);
+
+	for (i = 0; i < pdata->nbuttons; i++) {
+		const struct gsc_input_button *button = &pdata->buttons[i];
+		struct gsc_input_button_priv *button_priv = &input->buttons[i];
+
+		button_priv = &input->buttons[i];
+		button_priv->button = &pdata->buttons[i];
+		button_priv->input = input->input;
+		input_set_capability(input->input, EV_KEY, button->code);
+
+		ret = devm_request_threaded_irq(dev, button->irq, NULL,
+						gsc_input_irq, 0,
+						button->desc,
+						button_priv);
+		if (ret) {
+			dev_err(dev, "irq%d request failed: %d\n)", button->irq,
+				ret);
+			return ret;
+		}
+		dev_dbg(dev, "button: irq%d/%d code=%d %s\n",
+			button->irq - pdata->buttons[0].irq,
+			button->irq,
+			button->code, button->desc);
+	}
+
+	ret = input_register_device(input->input);
+	if (ret < 0) {
+		dev_err(dev, "Can't register input device: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id gsc_input_dt_ids[] = {
+	{ .compatible = "gw,gsc-input", },
+	{}
+};
+
+static struct platform_driver gsc_input_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = gsc_input_dt_ids,
+	},
+	.probe = gsc_input_probe,
+};
+
+module_platform_driver(gsc_input_driver);
+
+MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
+MODULE_DESCRIPTION("GSC input driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/platform_data/gsc_input.h b/include/linux/platform_data/gsc_input.h
new file mode 100644
index 0000000..1a34284
--- /dev/null
+++ b/include/linux/platform_data/gsc_input.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _GSC_INPUT_H
+#define _GSC_INPUT_H
+
+/**
+ * struct gsc_input_button - configuration parameters
+ * @code:	input event code (KEY_*, SW_*)
+ * @irq:	irq index
+ * @desc:	name of interrupt
+ */
+struct gsc_input_button {
+	unsigned int irq;
+	unsigned int code;
+	const char *desc;
+};
+
+/**
+ * struct gsc_input_platform_data - platform data for gsc_input driver
+ * @buttons:	pointer to array of gsc_input_button structures
+ *		describing buttons mapped to interrupt events
+ * @nbuttons:	number of elements in @buttons array
+ * @name:	input device name
+ */
+struct gsc_input_platform_data {
+	const struct gsc_input_button *buttons;
+	int nbuttons;
+	const char *name;
+};
+
+#endif
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 4/4] input: misc: Add Gateworks System Controller support
@ 2018-03-05 22:02   ` Tim Harvey
  0 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-05 22:02 UTC (permalink / raw)
  To: linux-arm-kernel

Add support for dispatching Linux Input events for the various interrupts
the Gateworks System Controller provides.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
v2:
- reword Kconfig
- revise license comment block
- remove unnecessary read of status register
- remove unnecessary setting of input->dev.parent
- use dt bindings for irq to keycode mapping
- add support for platform-data
- remove work-queue

 drivers/input/misc/Kconfig              |   9 ++
 drivers/input/misc/Makefile             |   1 +
 drivers/input/misc/gsc-input.c          | 180 ++++++++++++++++++++++++++++++++
 include/linux/platform_data/gsc_input.h |  30 ++++++
 4 files changed, 220 insertions(+)
 create mode 100644 drivers/input/misc/gsc-input.c
 create mode 100644 include/linux/platform_data/gsc_input.h

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 9f082a3..e05f4fe 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -117,6 +117,15 @@ config INPUT_E3X0_BUTTON
 	  To compile this driver as a module, choose M here: the
 	  module will be called e3x0_button.
 
+config INPUT_GSC
+	tristate "Gateworks System Controller input support"
+	depends on MFD_GSC
+	help
+	  Support GSC events as Linux input events.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called gsc_input.
+
 config INPUT_PCSPKR
 	tristate "PC Speaker support"
 	depends on PCSPKR_PLATFORM
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 4b6118d..969debe 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GP2A)		+= gp2ap002a00f.o
 obj-$(CONFIG_INPUT_GPIO_BEEPER)		+= gpio-beeper.o
 obj-$(CONFIG_INPUT_GPIO_TILT_POLLED)	+= gpio_tilt_polled.o
 obj-$(CONFIG_INPUT_GPIO_DECODER)	+= gpio_decoder.o
+obj-$(CONFIG_INPUT_GSC)			+= gsc-input.o
 obj-$(CONFIG_INPUT_HISI_POWERKEY)	+= hisi_powerkey.o
 obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
 obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
diff --git a/drivers/input/misc/gsc-input.c b/drivers/input/misc/gsc-input.c
new file mode 100644
index 0000000..27b5e93
--- /dev/null
+++ b/drivers/input/misc/gsc-input.c
@@ -0,0 +1,180 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2018 Gateworks Corporation
+ *
+ * This driver dispatches Linux input events for GSC interrupt events
+ */
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/gsc.h>
+#include <linux/platform_data/gsc_input.h>
+
+struct gsc_input_button_priv {
+	struct input_dev *input;
+	const struct gsc_input_button *button;
+};
+
+struct gsc_input_data {
+	struct gsc_dev *gsc;
+	struct gsc_input_platform_data *pdata;
+	struct input_dev *input;
+	struct gsc_input_button_priv *buttons;
+	int nbuttons;
+	unsigned int irqs[];
+#if 0
+	int irq;
+	struct work_struct irq_work;
+	struct mutex mutex;
+#endif
+};
+
+static irqreturn_t gsc_input_irq(int irq, void *data)
+{
+	const struct gsc_input_button_priv *button_priv = data;
+	struct input_dev *input = button_priv->input;
+
+	dev_dbg(&input->dev, "irq%d code=%d\n", irq, button_priv->button->code);
+	input_report_key(input, button_priv->button->code, 1);
+	input_sync(input);
+	input_report_key(input, button_priv->button->code, 0);
+	input_sync(input);
+
+	return IRQ_HANDLED;
+}
+
+static struct gsc_input_platform_data *
+gsc_input_get_devtree_pdata(struct device *dev)
+{
+	struct gsc_input_platform_data *pdata;
+	struct fwnode_handle *child;
+	struct gsc_input_button *button;
+	int nbuttons;
+
+	nbuttons = device_get_child_node_count(dev);
+	dev_dbg(dev, "buttons=%d\n", nbuttons);
+	if (nbuttons == 0)
+		return ERR_PTR(-ENODEV);
+
+	pdata = devm_kzalloc(dev,
+			     sizeof(*pdata) + nbuttons * sizeof(*button),
+			     GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	button = (struct gsc_input_button *)(pdata + 1);
+	pdata->buttons = button;
+	pdata->nbuttons = nbuttons;
+
+	device_property_read_string(dev, "label", &pdata->name);
+
+	device_for_each_child_node(dev, child) {
+		if (is_of_node(child))
+			button->irq = irq_of_parse_and_map(to_of_node(child),
+							   0);
+		if (fwnode_property_read_u32(child, "linux,code",
+					     &button->code)) {
+			dev_err(dev, "Button without keycode\n");
+			fwnode_handle_put(child);
+			return ERR_PTR(-EINVAL);
+		}
+		fwnode_property_read_string(child, "label", &button->desc);
+		button++;
+	}
+
+	return pdata;
+}
+
+static int gsc_input_probe(struct platform_device *pdev)
+{
+	struct gsc_dev *gsc = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct gsc_input_platform_data *pdata = dev_get_platdata(dev);
+	struct gsc_input_data *input;
+	int i, ret;
+
+	if (!pdata) {
+		pdata = gsc_input_get_devtree_pdata(dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
+
+	input = devm_kzalloc(dev, sizeof(*input), GFP_KERNEL);
+	if (!input)
+		return -ENOMEM;
+	input->gsc = gsc;
+	input->pdata = pdata;
+
+	input->buttons = devm_kzalloc(dev, pdata->nbuttons *
+				      sizeof(struct gsc_input_button_priv),
+				      GFP_KERNEL);
+	if (!input->buttons)
+		return -ENOMEM;
+
+	/* Register input device */
+	input->input = devm_input_allocate_device(dev);
+	if (!input->input) {
+		dev_err(dev, "Can't allocate input device\n");
+		return -ENOMEM;
+	}
+	input->input->name = pdata->name ? : pdev->name;
+
+	platform_set_drvdata(pdev, gsc);
+
+	for (i = 0; i < pdata->nbuttons; i++) {
+		const struct gsc_input_button *button = &pdata->buttons[i];
+		struct gsc_input_button_priv *button_priv = &input->buttons[i];
+
+		button_priv = &input->buttons[i];
+		button_priv->button = &pdata->buttons[i];
+		button_priv->input = input->input;
+		input_set_capability(input->input, EV_KEY, button->code);
+
+		ret = devm_request_threaded_irq(dev, button->irq, NULL,
+						gsc_input_irq, 0,
+						button->desc,
+						button_priv);
+		if (ret) {
+			dev_err(dev, "irq%d request failed: %d\n)", button->irq,
+				ret);
+			return ret;
+		}
+		dev_dbg(dev, "button: irq%d/%d code=%d %s\n",
+			button->irq - pdata->buttons[0].irq,
+			button->irq,
+			button->code, button->desc);
+	}
+
+	ret = input_register_device(input->input);
+	if (ret < 0) {
+		dev_err(dev, "Can't register input device: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id gsc_input_dt_ids[] = {
+	{ .compatible = "gw,gsc-input", },
+	{}
+};
+
+static struct platform_driver gsc_input_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = gsc_input_dt_ids,
+	},
+	.probe = gsc_input_probe,
+};
+
+module_platform_driver(gsc_input_driver);
+
+MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
+MODULE_DESCRIPTION("GSC input driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/platform_data/gsc_input.h b/include/linux/platform_data/gsc_input.h
new file mode 100644
index 0000000..1a34284
--- /dev/null
+++ b/include/linux/platform_data/gsc_input.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _GSC_INPUT_H
+#define _GSC_INPUT_H
+
+/**
+ * struct gsc_input_button - configuration parameters
+ * @code:	input event code (KEY_*, SW_*)
+ * @irq:	irq index
+ * @desc:	name of interrupt
+ */
+struct gsc_input_button {
+	unsigned int irq;
+	unsigned int code;
+	const char *desc;
+};
+
+/**
+ * struct gsc_input_platform_data - platform data for gsc_input driver
+ * @buttons:	pointer to array of gsc_input_button structures
+ *		describing buttons mapped to interrupt events
+ * @nbuttons:	number of elements in @buttons array
+ * @name:	input device name
+ */
+struct gsc_input_platform_data {
+	const struct gsc_input_button *buttons;
+	int nbuttons;
+	const char *name;
+};
+
+#endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 3/4] hwmon: add Gateworks System Controller support
  2018-03-05 22:02   ` Tim Harvey
@ 2018-03-05 23:12     ` Guenter Roeck
  -1 siblings, 0 replies; 24+ messages in thread
From: Guenter Roeck @ 2018-03-05 23:12 UTC (permalink / raw)
  To: Tim Harvey
  Cc: Lee Jones, Rob Herring, Mark Rutland, Mark Brown,
	Dmitry Torokhov, linux-kernel, devicetree, linux-arm-kernel,
	linux-hwmon, linux-input

On Mon, Mar 05, 2018 at 02:02:40PM -0800, Tim Harvey wrote:
> The Gateworks System Controller has a hwmon sub-component that exposes
> up to 16 ADC's, some of which are temperature sensors, others which are
> voltage inputs. The ADC configuration (register mapping and name) is
> configured via device-tree and varies board to board.
> 
> Cc: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> v2:
> - change license comment style
> - remove DEBUG
> - simplify regmap_bulk_read err check
> - remove break after returns in switch statement
> - fix fan setpoint buffer address
> - remove unnecessary parens
> - consistently use struct device *dev pointer
> - change license/comment block
> - add validation for hwmon child node props
> - move parsing of of to own function
> - use strlcpy to ensure null termination
> - fix static array sizes and removed unnecessary initializers
> - dynamically allocate channels
> - fix fan input label
> - support platform data
> - fixed whitespace issues
> 
>  drivers/hwmon/Kconfig                   |   9 +
>  drivers/hwmon/Makefile                  |   1 +
>  drivers/hwmon/gsc-hwmon.c               | 330 ++++++++++++++++++++++++++++++++
>  include/linux/platform_data/gsc_hwmon.h |  34 ++++
>  4 files changed, 374 insertions(+)
>  create mode 100644 drivers/hwmon/gsc-hwmon.c
>  create mode 100644 include/linux/platform_data/gsc_hwmon.h
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 7ad0176..e052798b 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -475,6 +475,15 @@ config SENSORS_F75375S
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called f75375s.
>  
> +config SENSORS_GSC
> +        tristate "Gateworks System Controller ADC"
> +        depends on MFD_GSC
> +        help
> +          Support for the Gateworks System Controller A/D converters.
> +
> +	  To compile this driver as a module, choose M here:
> +	  the module will be called gsc-hwmon.
> +
>  config SENSORS_MC13783_ADC
>          tristate "Freescale MC13783/MC13892 ADC"
>          depends on MFD_MC13XXX
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 0fe489f..835a536 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -69,6 +69,7 @@ obj-$(CONFIG_SENSORS_G760A)	+= g760a.o
>  obj-$(CONFIG_SENSORS_G762)	+= g762.o
>  obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
>  obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
> +obj-$(CONFIG_SENSORS_GSC)	+= gsc-hwmon.o
>  obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
>  obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
>  obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
> diff --git a/drivers/hwmon/gsc-hwmon.c b/drivers/hwmon/gsc-hwmon.c
> new file mode 100644
> index 0000000..cf55457
> --- /dev/null
> +++ b/drivers/hwmon/gsc-hwmon.c
> @@ -0,0 +1,330 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2018 Gateworks Corporation
> + *
> + * This driver registers Linux HWMON attributes for GSC ADC's
> + */
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/mfd/gsc.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#include <linux/platform_data/gsc_hwmon.h>
> +
> +#define GSC_HWMON_MAX_TEMP_CH	16
> +#define GSC_HWMON_MAX_IN_CH	16
> +#define GSC_HWMON_MAX_FAN_CH	6
> +
> +struct gsc_hwmon_data {
> +	struct gsc_dev *gsc;
> +	struct device *dev;
> +	struct gsc_hwmon_platform_data *pdata;
> +	const struct gsc_hwmon_channel *temp_ch[GSC_HWMON_MAX_TEMP_CH];
> +	const struct gsc_hwmon_channel *in_ch[GSC_HWMON_MAX_IN_CH];
> +	const struct gsc_hwmon_channel *fan_ch[GSC_HWMON_MAX_FAN_CH];
> +	u32 temp_config[GSC_HWMON_MAX_TEMP_CH + 1];
> +	u32 in_config[GSC_HWMON_MAX_IN_CH + 1];
> +	u32 fan_config[GSC_HWMON_MAX_FAN_CH + 1];
> +	struct hwmon_channel_info temp_info;
> +	struct hwmon_channel_info in_info;
> +	struct hwmon_channel_info fan_info;
> +	const struct hwmon_channel_info *info[4];
> +	struct hwmon_chip_info chip;
> +};
> +
> +static int
> +gsc_hwmon_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> +	       int channel, long *val)
> +{
> +	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
> +	int sz, ret;
> +	u8 reg;
> +	u8 buf[3];
> +
> +	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
> +		channel);
> +	switch (type) {
> +	case hwmon_in:
> +		if (channel > GSC_HWMON_MAX_IN_CH)
> +			return -EOPNOTSUPP;
> +		reg = hwmon->in_ch[channel]->reg;
> +		sz = 3;
> +		break;
> +	case hwmon_temp:
> +		if (channel > GSC_HWMON_MAX_TEMP_CH)
> +			return -EOPNOTSUPP;
> +		reg = hwmon->temp_ch[channel]->reg;
> +		sz = 2;
> +		break;
> +	case hwmon_fan:
> +		if (channel > GSC_HWMON_MAX_FAN_CH)
> +			return -EOPNOTSUPP;
> +		reg = hwmon->fan_ch[channel]->reg;
> +		sz = 2;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	ret = regmap_bulk_read(hwmon->gsc->regmap_hwmon, reg, &buf, sz);
> +	if (ret)
> +		return ret;
> +
> +	*val = 0;
> +	while (sz-- > 0)
> +		*val |= (buf[sz] << (8*sz));
> +	if ((type == hwmon_temp) && *val > 0x8000)
> +		*val -= 0xffff;
> +
> +	return 0;
> +}
> +
> +static int
> +gsc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
> +		      u32 attr, int channel, const char **buf)
> +{
> +	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
> +
> +	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
> +		channel);
> +	switch (type) {
> +	case hwmon_in:
> +		if (channel > GSC_HWMON_MAX_IN_CH)
> +			return -EOPNOTSUPP;

First of all, this is wrong. in_ch only has GSC_HWMON_MAX_IN_CH
elements, meaning hwmon->in_ch[GSC_HWMON_MAX_IN_CH] will access
data after the end of the array. Second, valitations should not
be done here but in the is_visible function.

> +		*buf = hwmon->in_ch[channel]->name;
> +		break;
> +	case hwmon_temp:
> +		if (channel > GSC_HWMON_MAX_TEMP_CH)
> +			return -EOPNOTSUPP;

Same here and everywhere else.

> +		*buf = hwmon->temp_ch[channel]->name;
> +		break;
> +	case hwmon_fan:
> +		if (channel > GSC_HWMON_MAX_FAN_CH)
> +			return -EOPNOTSUPP;
> +		*buf = hwmon->fan_ch[channel]->name;
> +		break;
> +	default:
> +		return -ENOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +gsc_hwmon_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> +		int channel, long val)
> +{
> +	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
> +	u8 buf[3];
> +
> +	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
> +		channel);
> +	switch (type) {
> +	case hwmon_fan:
> +		if (channel == GSC_HWMON_MAX_FAN_CH)
> +			return -EOPNOTSUPP;
> +		buf[0] = val & 0xff;
> +		buf[1] = (val >> 8) & 0xff;
> +		return regmap_bulk_write(hwmon->gsc->regmap_hwmon,
> +					 hwmon->fan_ch[channel]->reg, buf, 2);
> +	default:
> +		break;
> +	}
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static umode_t
> +gsc_hwmon_is_visible(const void *_data, enum hwmon_sensor_types type, u32 attr,
> +		     int ch)
> +{
> +	const struct gsc_hwmon_data *hwmon = _data;
> +	struct device *dev = hwmon->gsc->dev;
> +	umode_t mode = 0;
> +
> +	switch (type) {
> +	case hwmon_fan:
> +		mode = S_IRUGO;
> +		if (attr == hwmon_fan_input)
> +			mode |= S_IWUSR;
> +		break;
> +	case hwmon_temp:
> +	case hwmon_in:
> +		mode = S_IRUGO;
> +		break;
> +	default:
> +		break;
> +	}
> +	dev_dbg(dev, "%s type=%d attr=%d ch=%d mode=0x%x\n", __func__, type,
> +		attr, ch, mode);
> +
> +	return mode;
> +}
> +
> +static const struct hwmon_ops gsc_hwmon_ops = {
> +	.is_visible = gsc_hwmon_is_visible,
> +	.read = gsc_hwmon_read,
> +	.read_string = gsc_hwmon_read_string,
> +	.write = gsc_hwmon_write,
> +};
> +
> +static struct gsc_hwmon_platform_data *
> +gsc_hwmon_get_devtree_pdata(struct device *dev)
> +{
> +	struct gsc_hwmon_platform_data *pdata;
> +	struct gsc_hwmon_channel *ch;
> +	struct fwnode_handle *child;
> +	int nchannels;
> +
> +	nchannels = device_get_child_node_count(dev);
> +	dev_dbg(dev, "channels=%d\n", nchannels);
> +	if (nchannels == 0)
> +		return ERR_PTR(-ENODEV);
> +
> +	pdata = devm_kzalloc(dev,
> +			     sizeof(*pdata) + nchannels * sizeof(*ch),
> +			     GFP_KERNEL);
> +	if (!pdata)
> +		return ERR_PTR(-ENOMEM);
> +	ch = (struct gsc_hwmon_channel *)(pdata + 1);
> +	pdata->channels = ch;
> +	pdata->nchannels = nchannels;
> +
> +	/* allocate structures for channels and count instances of each type */
> +	device_for_each_child_node(dev, child) {
> +		if (fwnode_property_read_string(child, "label", &ch->name)) {
> +			dev_err(dev, "channel without label\n");
> +			fwnode_handle_put(child);
> +			return ERR_PTR(-EINVAL);
> +		}
> +		if (fwnode_property_read_u32(child, "reg", &ch->reg)) {
> +			dev_err(dev, "channel without reg\n");
> +			fwnode_handle_put(child);
> +			return ERR_PTR(-EINVAL);
> +		}
> +		if (fwnode_property_read_u32(child, "type", &ch->type)) {
> +			dev_err(dev, "channel without type\n");
> +			fwnode_handle_put(child);
> +			return ERR_PTR(-EINVAL);
> +		}
> +		dev_dbg(dev, "of: reg=0x%02x type=%d %s\n", ch->reg, ch->type,
> +			ch->name);
> +		ch++;
> +	}
> +
> +	return pdata;
> +}
> +
> +static int gsc_hwmon_probe(struct platform_device *pdev)
> +{
> +	struct gsc_dev *gsc = dev_get_drvdata(pdev->dev.parent);
> +	struct device *dev = &pdev->dev;
> +	struct gsc_hwmon_platform_data *pdata = dev_get_platdata(dev);
> +	struct gsc_hwmon_data *hwmon;
> +	int i, i_in, i_temp, i_fan;
> +
> +	if (!pdata) {
> +		pdata = gsc_hwmon_get_devtree_pdata(dev);
> +		if (IS_ERR(pdata))
> +			return PTR_ERR(pdata);
> +	}
> +
> +	hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
> +	if (!hwmon)
> +		return -ENOMEM;
> +	hwmon->gsc = gsc;
> +	hwmon->pdata = pdata;
> +
> +	for (i = 0, i_in = 0, i_temp = 0, i_fan = 0;
> +	     i < hwmon->pdata->nchannels; i++) {
> +		const struct gsc_hwmon_channel *ch = &pdata->channels[i];
> +
> +		if (ch->reg > GSC_HWMON_MAX_REG) {
> +			dev_err(dev, "invalid reg: 0x%02x\n", ch->reg);
> +			return -EINVAL;
> +		}
> +		switch (ch->type) {
> +		case type_temperature:
> +			if (i_temp == GSC_HWMON_MAX_TEMP_CH) {
> +				dev_err(dev, "too many temp channels\n");
> +				return -EINVAL;
> +			}
> +			hwmon->temp_ch[i_temp] = ch;
> +			hwmon->temp_config[i_temp] = HWMON_T_INPUT |
> +						     HWMON_T_LABEL;
> +			i_temp++;
> +			break;
> +		case type_voltage:
> +			if (i_in == GSC_HWMON_MAX_IN_CH) {
> +				dev_err(dev, "too many voltage channels\n");
> +				return -EINVAL;
> +			}
> +			hwmon->in_ch[i_in] = ch;
> +			hwmon->in_config[i_in] =
> +				HWMON_I_INPUT | HWMON_I_LABEL;
> +			i_in++;
> +			break;
> +		case type_fan:
> +			if (i_fan == GSC_HWMON_MAX_FAN_CH) {
> +				dev_err(dev, "too many voltage channels\n");
> +				return -EINVAL;
> +			}
> +			hwmon->fan_ch[i_fan] = ch;
> +			hwmon->fan_config[i_fan] =
> +				HWMON_F_INPUT | HWMON_F_LABEL;
> +			i_fan++;
> +			break;
> +		default:
> +			dev_err(dev, "invalid type: %d\n", ch->type);
> +			return -EINVAL;
> +		}
> +		dev_dbg(dev, "pdata: reg=0x%02x type=%d %s\n", ch->reg,
> +			ch->type, ch->name);
> +	}
> +
> +	/* terminate channel config lists */
> +	hwmon->temp_config[i_temp] = 0;
> +	hwmon->in_config[i_in] = 0;
> +	hwmon->fan_config[i_fan] = 0;
> +
> +	/* setup config structures */
> +	hwmon->chip.ops = &gsc_hwmon_ops;
> +	hwmon->chip.info = hwmon->info;
> +	hwmon->info[0] = &hwmon->temp_info;
> +	hwmon->info[1] = &hwmon->in_info;
> +	hwmon->info[2] = &hwmon->fan_info;
> +	hwmon->temp_info.type = hwmon_temp;
> +	hwmon->temp_info.config = hwmon->temp_config;
> +	hwmon->in_info.type = hwmon_in;
> +	hwmon->in_info.config = hwmon->in_config;
> +	hwmon->fan_info.type = hwmon_fan;
> +	hwmon->fan_info.config = hwmon->fan_config;
> +
> +	hwmon->dev = devm_hwmon_device_register_with_info(dev,
> +							  KBUILD_MODNAME, hwmon,
> +							  &hwmon->chip, NULL);
> +	return PTR_ERR_OR_ZERO(hwmon->dev);
> +}
> +
> +static const struct of_device_id gsc_hwmon_of_match[] = {
> +	{ .compatible = "gw,gsc-hwmon", },
> +	{}
> +};
> +
> +static struct platform_driver gsc_hwmon_driver = {
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.of_match_table = gsc_hwmon_of_match,
> +	},
> +	.probe = gsc_hwmon_probe,
> +};
> +
> +module_platform_driver(gsc_hwmon_driver);
> +
> +MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
> +MODULE_DESCRIPTION("GSC hardware monitor driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/platform_data/gsc_hwmon.h b/include/linux/platform_data/gsc_hwmon.h
> new file mode 100644
> index 0000000..ba4f811
> --- /dev/null
> +++ b/include/linux/platform_data/gsc_hwmon.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _GSC_HWMON_H
> +#define _GSC_HWMON_H
> +
> +enum gsc_hwmon_type {
> +	type_temperature,
> +	type_voltage,
> +	type_fan,
> +};
> +
> +/**
> + * struct gsc_hwmon_channel - configuration parameters
> + * @reg:  I2C register offset
> + * @type: channel type
> + * @name: channel name
> + */
> +struct gsc_hwmon_channel {
> +	unsigned int reg;
> +	unsigned int type;
> +	const char *name;
> +};
> +
> +/**
> + * struct gsc_hwmon_platform_data - platform data for gsc_hwmon driver
> + * @channels:	pointer to array of gsc_hwmon_channel structures
> + *		describing channels
> + * @nchannels:	number of elements in @channels array
> + */
> +struct gsc_hwmon_platform_data {
> +	const struct gsc_hwmon_channel *channels;
> +	int nchannels;
> +};
> +
> +#endif
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 3/4] hwmon: add Gateworks System Controller support
@ 2018-03-05 23:12     ` Guenter Roeck
  0 siblings, 0 replies; 24+ messages in thread
From: Guenter Roeck @ 2018-03-05 23:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Mar 05, 2018 at 02:02:40PM -0800, Tim Harvey wrote:
> The Gateworks System Controller has a hwmon sub-component that exposes
> up to 16 ADC's, some of which are temperature sensors, others which are
> voltage inputs. The ADC configuration (register mapping and name) is
> configured via device-tree and varies board to board.
> 
> Cc: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> v2:
> - change license comment style
> - remove DEBUG
> - simplify regmap_bulk_read err check
> - remove break after returns in switch statement
> - fix fan setpoint buffer address
> - remove unnecessary parens
> - consistently use struct device *dev pointer
> - change license/comment block
> - add validation for hwmon child node props
> - move parsing of of to own function
> - use strlcpy to ensure null termination
> - fix static array sizes and removed unnecessary initializers
> - dynamically allocate channels
> - fix fan input label
> - support platform data
> - fixed whitespace issues
> 
>  drivers/hwmon/Kconfig                   |   9 +
>  drivers/hwmon/Makefile                  |   1 +
>  drivers/hwmon/gsc-hwmon.c               | 330 ++++++++++++++++++++++++++++++++
>  include/linux/platform_data/gsc_hwmon.h |  34 ++++
>  4 files changed, 374 insertions(+)
>  create mode 100644 drivers/hwmon/gsc-hwmon.c
>  create mode 100644 include/linux/platform_data/gsc_hwmon.h
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 7ad0176..e052798b 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -475,6 +475,15 @@ config SENSORS_F75375S
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called f75375s.
>  
> +config SENSORS_GSC
> +        tristate "Gateworks System Controller ADC"
> +        depends on MFD_GSC
> +        help
> +          Support for the Gateworks System Controller A/D converters.
> +
> +	  To compile this driver as a module, choose M here:
> +	  the module will be called gsc-hwmon.
> +
>  config SENSORS_MC13783_ADC
>          tristate "Freescale MC13783/MC13892 ADC"
>          depends on MFD_MC13XXX
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 0fe489f..835a536 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -69,6 +69,7 @@ obj-$(CONFIG_SENSORS_G760A)	+= g760a.o
>  obj-$(CONFIG_SENSORS_G762)	+= g762.o
>  obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
>  obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
> +obj-$(CONFIG_SENSORS_GSC)	+= gsc-hwmon.o
>  obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
>  obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
>  obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
> diff --git a/drivers/hwmon/gsc-hwmon.c b/drivers/hwmon/gsc-hwmon.c
> new file mode 100644
> index 0000000..cf55457
> --- /dev/null
> +++ b/drivers/hwmon/gsc-hwmon.c
> @@ -0,0 +1,330 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2018 Gateworks Corporation
> + *
> + * This driver registers Linux HWMON attributes for GSC ADC's
> + */
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/mfd/gsc.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#include <linux/platform_data/gsc_hwmon.h>
> +
> +#define GSC_HWMON_MAX_TEMP_CH	16
> +#define GSC_HWMON_MAX_IN_CH	16
> +#define GSC_HWMON_MAX_FAN_CH	6
> +
> +struct gsc_hwmon_data {
> +	struct gsc_dev *gsc;
> +	struct device *dev;
> +	struct gsc_hwmon_platform_data *pdata;
> +	const struct gsc_hwmon_channel *temp_ch[GSC_HWMON_MAX_TEMP_CH];
> +	const struct gsc_hwmon_channel *in_ch[GSC_HWMON_MAX_IN_CH];
> +	const struct gsc_hwmon_channel *fan_ch[GSC_HWMON_MAX_FAN_CH];
> +	u32 temp_config[GSC_HWMON_MAX_TEMP_CH + 1];
> +	u32 in_config[GSC_HWMON_MAX_IN_CH + 1];
> +	u32 fan_config[GSC_HWMON_MAX_FAN_CH + 1];
> +	struct hwmon_channel_info temp_info;
> +	struct hwmon_channel_info in_info;
> +	struct hwmon_channel_info fan_info;
> +	const struct hwmon_channel_info *info[4];
> +	struct hwmon_chip_info chip;
> +};
> +
> +static int
> +gsc_hwmon_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> +	       int channel, long *val)
> +{
> +	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
> +	int sz, ret;
> +	u8 reg;
> +	u8 buf[3];
> +
> +	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
> +		channel);
> +	switch (type) {
> +	case hwmon_in:
> +		if (channel > GSC_HWMON_MAX_IN_CH)
> +			return -EOPNOTSUPP;
> +		reg = hwmon->in_ch[channel]->reg;
> +		sz = 3;
> +		break;
> +	case hwmon_temp:
> +		if (channel > GSC_HWMON_MAX_TEMP_CH)
> +			return -EOPNOTSUPP;
> +		reg = hwmon->temp_ch[channel]->reg;
> +		sz = 2;
> +		break;
> +	case hwmon_fan:
> +		if (channel > GSC_HWMON_MAX_FAN_CH)
> +			return -EOPNOTSUPP;
> +		reg = hwmon->fan_ch[channel]->reg;
> +		sz = 2;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	ret = regmap_bulk_read(hwmon->gsc->regmap_hwmon, reg, &buf, sz);
> +	if (ret)
> +		return ret;
> +
> +	*val = 0;
> +	while (sz-- > 0)
> +		*val |= (buf[sz] << (8*sz));
> +	if ((type == hwmon_temp) && *val > 0x8000)
> +		*val -= 0xffff;
> +
> +	return 0;
> +}
> +
> +static int
> +gsc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
> +		      u32 attr, int channel, const char **buf)
> +{
> +	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
> +
> +	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
> +		channel);
> +	switch (type) {
> +	case hwmon_in:
> +		if (channel > GSC_HWMON_MAX_IN_CH)
> +			return -EOPNOTSUPP;

First of all, this is wrong. in_ch only has GSC_HWMON_MAX_IN_CH
elements, meaning hwmon->in_ch[GSC_HWMON_MAX_IN_CH] will access
data after the end of the array. Second, valitations should not
be done here but in the is_visible function.

> +		*buf = hwmon->in_ch[channel]->name;
> +		break;
> +	case hwmon_temp:
> +		if (channel > GSC_HWMON_MAX_TEMP_CH)
> +			return -EOPNOTSUPP;

Same here and everywhere else.

> +		*buf = hwmon->temp_ch[channel]->name;
> +		break;
> +	case hwmon_fan:
> +		if (channel > GSC_HWMON_MAX_FAN_CH)
> +			return -EOPNOTSUPP;
> +		*buf = hwmon->fan_ch[channel]->name;
> +		break;
> +	default:
> +		return -ENOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +gsc_hwmon_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> +		int channel, long val)
> +{
> +	struct gsc_hwmon_data *hwmon = dev_get_drvdata(dev);
> +	u8 buf[3];
> +
> +	dev_dbg(dev, "%s type=%d attr=%d channel=%d\n", __func__, type, attr,
> +		channel);
> +	switch (type) {
> +	case hwmon_fan:
> +		if (channel == GSC_HWMON_MAX_FAN_CH)
> +			return -EOPNOTSUPP;
> +		buf[0] = val & 0xff;
> +		buf[1] = (val >> 8) & 0xff;
> +		return regmap_bulk_write(hwmon->gsc->regmap_hwmon,
> +					 hwmon->fan_ch[channel]->reg, buf, 2);
> +	default:
> +		break;
> +	}
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static umode_t
> +gsc_hwmon_is_visible(const void *_data, enum hwmon_sensor_types type, u32 attr,
> +		     int ch)
> +{
> +	const struct gsc_hwmon_data *hwmon = _data;
> +	struct device *dev = hwmon->gsc->dev;
> +	umode_t mode = 0;
> +
> +	switch (type) {
> +	case hwmon_fan:
> +		mode = S_IRUGO;
> +		if (attr == hwmon_fan_input)
> +			mode |= S_IWUSR;
> +		break;
> +	case hwmon_temp:
> +	case hwmon_in:
> +		mode = S_IRUGO;
> +		break;
> +	default:
> +		break;
> +	}
> +	dev_dbg(dev, "%s type=%d attr=%d ch=%d mode=0x%x\n", __func__, type,
> +		attr, ch, mode);
> +
> +	return mode;
> +}
> +
> +static const struct hwmon_ops gsc_hwmon_ops = {
> +	.is_visible = gsc_hwmon_is_visible,
> +	.read = gsc_hwmon_read,
> +	.read_string = gsc_hwmon_read_string,
> +	.write = gsc_hwmon_write,
> +};
> +
> +static struct gsc_hwmon_platform_data *
> +gsc_hwmon_get_devtree_pdata(struct device *dev)
> +{
> +	struct gsc_hwmon_platform_data *pdata;
> +	struct gsc_hwmon_channel *ch;
> +	struct fwnode_handle *child;
> +	int nchannels;
> +
> +	nchannels = device_get_child_node_count(dev);
> +	dev_dbg(dev, "channels=%d\n", nchannels);
> +	if (nchannels == 0)
> +		return ERR_PTR(-ENODEV);
> +
> +	pdata = devm_kzalloc(dev,
> +			     sizeof(*pdata) + nchannels * sizeof(*ch),
> +			     GFP_KERNEL);
> +	if (!pdata)
> +		return ERR_PTR(-ENOMEM);
> +	ch = (struct gsc_hwmon_channel *)(pdata + 1);
> +	pdata->channels = ch;
> +	pdata->nchannels = nchannels;
> +
> +	/* allocate structures for channels and count instances of each type */
> +	device_for_each_child_node(dev, child) {
> +		if (fwnode_property_read_string(child, "label", &ch->name)) {
> +			dev_err(dev, "channel without label\n");
> +			fwnode_handle_put(child);
> +			return ERR_PTR(-EINVAL);
> +		}
> +		if (fwnode_property_read_u32(child, "reg", &ch->reg)) {
> +			dev_err(dev, "channel without reg\n");
> +			fwnode_handle_put(child);
> +			return ERR_PTR(-EINVAL);
> +		}
> +		if (fwnode_property_read_u32(child, "type", &ch->type)) {
> +			dev_err(dev, "channel without type\n");
> +			fwnode_handle_put(child);
> +			return ERR_PTR(-EINVAL);
> +		}
> +		dev_dbg(dev, "of: reg=0x%02x type=%d %s\n", ch->reg, ch->type,
> +			ch->name);
> +		ch++;
> +	}
> +
> +	return pdata;
> +}
> +
> +static int gsc_hwmon_probe(struct platform_device *pdev)
> +{
> +	struct gsc_dev *gsc = dev_get_drvdata(pdev->dev.parent);
> +	struct device *dev = &pdev->dev;
> +	struct gsc_hwmon_platform_data *pdata = dev_get_platdata(dev);
> +	struct gsc_hwmon_data *hwmon;
> +	int i, i_in, i_temp, i_fan;
> +
> +	if (!pdata) {
> +		pdata = gsc_hwmon_get_devtree_pdata(dev);
> +		if (IS_ERR(pdata))
> +			return PTR_ERR(pdata);
> +	}
> +
> +	hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
> +	if (!hwmon)
> +		return -ENOMEM;
> +	hwmon->gsc = gsc;
> +	hwmon->pdata = pdata;
> +
> +	for (i = 0, i_in = 0, i_temp = 0, i_fan = 0;
> +	     i < hwmon->pdata->nchannels; i++) {
> +		const struct gsc_hwmon_channel *ch = &pdata->channels[i];
> +
> +		if (ch->reg > GSC_HWMON_MAX_REG) {
> +			dev_err(dev, "invalid reg: 0x%02x\n", ch->reg);
> +			return -EINVAL;
> +		}
> +		switch (ch->type) {
> +		case type_temperature:
> +			if (i_temp == GSC_HWMON_MAX_TEMP_CH) {
> +				dev_err(dev, "too many temp channels\n");
> +				return -EINVAL;
> +			}
> +			hwmon->temp_ch[i_temp] = ch;
> +			hwmon->temp_config[i_temp] = HWMON_T_INPUT |
> +						     HWMON_T_LABEL;
> +			i_temp++;
> +			break;
> +		case type_voltage:
> +			if (i_in == GSC_HWMON_MAX_IN_CH) {
> +				dev_err(dev, "too many voltage channels\n");
> +				return -EINVAL;
> +			}
> +			hwmon->in_ch[i_in] = ch;
> +			hwmon->in_config[i_in] =
> +				HWMON_I_INPUT | HWMON_I_LABEL;
> +			i_in++;
> +			break;
> +		case type_fan:
> +			if (i_fan == GSC_HWMON_MAX_FAN_CH) {
> +				dev_err(dev, "too many voltage channels\n");
> +				return -EINVAL;
> +			}
> +			hwmon->fan_ch[i_fan] = ch;
> +			hwmon->fan_config[i_fan] =
> +				HWMON_F_INPUT | HWMON_F_LABEL;
> +			i_fan++;
> +			break;
> +		default:
> +			dev_err(dev, "invalid type: %d\n", ch->type);
> +			return -EINVAL;
> +		}
> +		dev_dbg(dev, "pdata: reg=0x%02x type=%d %s\n", ch->reg,
> +			ch->type, ch->name);
> +	}
> +
> +	/* terminate channel config lists */
> +	hwmon->temp_config[i_temp] = 0;
> +	hwmon->in_config[i_in] = 0;
> +	hwmon->fan_config[i_fan] = 0;
> +
> +	/* setup config structures */
> +	hwmon->chip.ops = &gsc_hwmon_ops;
> +	hwmon->chip.info = hwmon->info;
> +	hwmon->info[0] = &hwmon->temp_info;
> +	hwmon->info[1] = &hwmon->in_info;
> +	hwmon->info[2] = &hwmon->fan_info;
> +	hwmon->temp_info.type = hwmon_temp;
> +	hwmon->temp_info.config = hwmon->temp_config;
> +	hwmon->in_info.type = hwmon_in;
> +	hwmon->in_info.config = hwmon->in_config;
> +	hwmon->fan_info.type = hwmon_fan;
> +	hwmon->fan_info.config = hwmon->fan_config;
> +
> +	hwmon->dev = devm_hwmon_device_register_with_info(dev,
> +							  KBUILD_MODNAME, hwmon,
> +							  &hwmon->chip, NULL);
> +	return PTR_ERR_OR_ZERO(hwmon->dev);
> +}
> +
> +static const struct of_device_id gsc_hwmon_of_match[] = {
> +	{ .compatible = "gw,gsc-hwmon", },
> +	{}
> +};
> +
> +static struct platform_driver gsc_hwmon_driver = {
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.of_match_table = gsc_hwmon_of_match,
> +	},
> +	.probe = gsc_hwmon_probe,
> +};
> +
> +module_platform_driver(gsc_hwmon_driver);
> +
> +MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
> +MODULE_DESCRIPTION("GSC hardware monitor driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/platform_data/gsc_hwmon.h b/include/linux/platform_data/gsc_hwmon.h
> new file mode 100644
> index 0000000..ba4f811
> --- /dev/null
> +++ b/include/linux/platform_data/gsc_hwmon.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _GSC_HWMON_H
> +#define _GSC_HWMON_H
> +
> +enum gsc_hwmon_type {
> +	type_temperature,
> +	type_voltage,
> +	type_fan,
> +};
> +
> +/**
> + * struct gsc_hwmon_channel - configuration parameters
> + * @reg:  I2C register offset
> + * @type: channel type
> + * @name: channel name
> + */
> +struct gsc_hwmon_channel {
> +	unsigned int reg;
> +	unsigned int type;
> +	const char *name;
> +};
> +
> +/**
> + * struct gsc_hwmon_platform_data - platform data for gsc_hwmon driver
> + * @channels:	pointer to array of gsc_hwmon_channel structures
> + *		describing channels
> + * @nchannels:	number of elements in @channels array
> + */
> +struct gsc_hwmon_platform_data {
> +	const struct gsc_hwmon_channel *channels;
> +	int nchannels;
> +};
> +
> +#endif
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings
  2018-03-05 22:02   ` Tim Harvey
@ 2018-03-09 23:14     ` Rob Herring
  -1 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2018-03-09 23:14 UTC (permalink / raw)
  To: Tim Harvey
  Cc: Lee Jones, Mark Rutland, Mark Brown, Dmitry Torokhov,
	linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
	linux-input

On Mon, Mar 05, 2018 at 02:02:38PM -0800, Tim Harvey wrote:
> This patch adds documentation of device-tree bindings for the
> Gateworks System Controller (GSC).
> 
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  Documentation/devicetree/bindings/mfd/gsc.txt | 159 ++++++++++++++++++++++++++
>  1 file changed, 159 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/gsc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/gsc.txt b/Documentation/devicetree/bindings/mfd/gsc.txt
> new file mode 100644
> index 0000000..fe5d114
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/gsc.txt
> @@ -0,0 +1,159 @@
> +Gateworks System Controller multi-function device
> +
> +The GSC is a Multifunction I2C slave device with the following submodules:
> +- WDT
> +- GPIO
> +- Pushbutton controller
> +- HWMON
> +
> +Required properties:
> +- compatible : Must be "gw,gsc_v1", "gw,gsc_v2", "gw,gsc_v3"

s/_/-/

> +- reg: I2C address of the device
> +- interrupts: interrupt triggered by GSC_IRQ# signal
> +- interrupt-parent: Interrupt controller GSC is connected to
> +- #interrupt-cells: should be <1>, index of the interrupt within the
> +  controller, in accordance with the "one cell" variant of
> +  <devicetree/bindings/interrupt-controller/interrupt.txt>
> +
> +Optional nodes:
> +* watchdog:
> +The GSC provides a Watchdog monitor which can power cycle the board's
> +primary power supply on most board models when tripped.
> +
> +Required properties:
> +- compatible: must be "gw,gsc-watchdog"
> +
> +* input:
> +The GSC provides an input device capable of dispatching Linux Input events

Linux details that are not relevant to the binding.

> +for user pushbutton events, tamper switch events, etc.
> +
> +Required properties:
> +- compatible: must be "gw,gsc-input"
> +
> +* hwmon:
> +The GSC provides a set of Analog to Digitcal Converter (ADC) pins used for
> +temperature and/or voltage monitoring.
> +
> +Required properties:
> +- compatible: must be "gw,gsc-hwmon"
> +
> +Example:
> +
> +	gsc: gsc@20 {
> +		compatible = "gw,gsc_v2";
> +		reg = <0x20>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <4 GPIO_ACTIVE_LOW>;
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +
> +		gsc_input {

input {

> +			compatible = "gw,gsc-input";
> +		};
> +
> +		gsc_watchdog {

watchdog {

> +			compatible = "gw,gsc-watchdog";
> +		};
> +
> +		gsc_hwmon {

hwmon {

> +			compatible = "gw,gsc-hwmon";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			hwmon@0 { /* A0: Board Temperature */
> +				type = <0>;

Not documented.

> +				reg = <0x00>;
> +				label = "temp";
> +			};
> +
> +			hwmon@1 { /* A1: Input Voltage */
> +				type = <1>;
> +				reg = <0x02>;
> +				label = "Vin";
> +			};
> +
> +			hwmon@2 { /* A2: 5P0 */
> +				type = <1>;
> +				reg = <0x0b>;
> +				label = "5P0";
> +			};
> +
> +			hwmon@4 { /* A4: 0-5V input */
> +				type = <1>;
> +				reg = <0x14>;
> +				label = "ANL0";
> +			};
> +
> +			hwmon@5 { /* A5: 2P5 PCIe/GigE */
> +				type = <1>;
> +				reg = <0x23>;
> +				label = "2P5";
> +			};
> +
> +			hwmon@6 { /* A6: 1P8 Aud/Vid */
> +				type = <1>;
> +				reg = <0x1d>;
> +				label = "1P8";
> +			};
> +
> +			hwmon@7 { /* A7: GPS */
> +				type = <1>;
> +				reg = <0x26>;
> +				label = "GPS";
> +			};
> +
> +			hwmon@12 { /* A12: VDD_CORE */
> +				type = <1>;
> +				reg = <0x3>;
> +				label = "VDD_CORE";
> +			};
> +
> +			hwmon@13 { /* A13: VDD_SOC */
> +				type = <1>;
> +				reg = <0x11>;
> +				label = "VDD_SOC";
> +			};
> +
> +			hwmon@14 { /* A14: 1P0 PCIe SW */
> +				type = <1>;
> +				reg = <0x20>;
> +				label = "1P0";
> +			};
> +
> +			hwmon@15 { /* fan0 */
> +				type = <2>;
> +				reg = <0x2c>;
> +				label = "fan_50p";
> +			};
> +
> +			hwmon@16 { /* fan1 */
> +				type = <2>;
> +				reg = <0x2e>;
> +				label = "fan_60p";
> +			};
> +
> +			hwmon@17 { /* fan2 */
> +				type = <2>;
> +				reg = <0x30>;
> +				label = "fan_70p";
> +			};
> +
> +			hwmon@18 { /* fan3 */
> +				type = <2>;
> +				reg = <0x32>;
> +				label = "fan_80p";
> +			};
> +
> +			hwmon@19 { /* fan4 */
> +				type = <2>;
> +				reg = <0x34>;
> +				label = "fan_90p";
> +			};
> +
> +			hwmon@20 { /* fan5 */
> +				type = <2>;
> +				reg = <0x36>;
> +				label = "fan_100p";
> +			};
> +		};
> +	};
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings
@ 2018-03-09 23:14     ` Rob Herring
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2018-03-09 23:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Mar 05, 2018 at 02:02:38PM -0800, Tim Harvey wrote:
> This patch adds documentation of device-tree bindings for the
> Gateworks System Controller (GSC).
> 
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  Documentation/devicetree/bindings/mfd/gsc.txt | 159 ++++++++++++++++++++++++++
>  1 file changed, 159 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/gsc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/gsc.txt b/Documentation/devicetree/bindings/mfd/gsc.txt
> new file mode 100644
> index 0000000..fe5d114
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/gsc.txt
> @@ -0,0 +1,159 @@
> +Gateworks System Controller multi-function device
> +
> +The GSC is a Multifunction I2C slave device with the following submodules:
> +- WDT
> +- GPIO
> +- Pushbutton controller
> +- HWMON
> +
> +Required properties:
> +- compatible : Must be "gw,gsc_v1", "gw,gsc_v2", "gw,gsc_v3"

s/_/-/

> +- reg: I2C address of the device
> +- interrupts: interrupt triggered by GSC_IRQ# signal
> +- interrupt-parent: Interrupt controller GSC is connected to
> +- #interrupt-cells: should be <1>, index of the interrupt within the
> +  controller, in accordance with the "one cell" variant of
> +  <devicetree/bindings/interrupt-controller/interrupt.txt>
> +
> +Optional nodes:
> +* watchdog:
> +The GSC provides a Watchdog monitor which can power cycle the board's
> +primary power supply on most board models when tripped.
> +
> +Required properties:
> +- compatible: must be "gw,gsc-watchdog"
> +
> +* input:
> +The GSC provides an input device capable of dispatching Linux Input events

Linux details that are not relevant to the binding.

> +for user pushbutton events, tamper switch events, etc.
> +
> +Required properties:
> +- compatible: must be "gw,gsc-input"
> +
> +* hwmon:
> +The GSC provides a set of Analog to Digitcal Converter (ADC) pins used for
> +temperature and/or voltage monitoring.
> +
> +Required properties:
> +- compatible: must be "gw,gsc-hwmon"
> +
> +Example:
> +
> +	gsc: gsc at 20 {
> +		compatible = "gw,gsc_v2";
> +		reg = <0x20>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <4 GPIO_ACTIVE_LOW>;
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +
> +		gsc_input {

input {

> +			compatible = "gw,gsc-input";
> +		};
> +
> +		gsc_watchdog {

watchdog {

> +			compatible = "gw,gsc-watchdog";
> +		};
> +
> +		gsc_hwmon {

hwmon {

> +			compatible = "gw,gsc-hwmon";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			hwmon at 0 { /* A0: Board Temperature */
> +				type = <0>;

Not documented.

> +				reg = <0x00>;
> +				label = "temp";
> +			};
> +
> +			hwmon at 1 { /* A1: Input Voltage */
> +				type = <1>;
> +				reg = <0x02>;
> +				label = "Vin";
> +			};
> +
> +			hwmon at 2 { /* A2: 5P0 */
> +				type = <1>;
> +				reg = <0x0b>;
> +				label = "5P0";
> +			};
> +
> +			hwmon at 4 { /* A4: 0-5V input */
> +				type = <1>;
> +				reg = <0x14>;
> +				label = "ANL0";
> +			};
> +
> +			hwmon at 5 { /* A5: 2P5 PCIe/GigE */
> +				type = <1>;
> +				reg = <0x23>;
> +				label = "2P5";
> +			};
> +
> +			hwmon at 6 { /* A6: 1P8 Aud/Vid */
> +				type = <1>;
> +				reg = <0x1d>;
> +				label = "1P8";
> +			};
> +
> +			hwmon at 7 { /* A7: GPS */
> +				type = <1>;
> +				reg = <0x26>;
> +				label = "GPS";
> +			};
> +
> +			hwmon at 12 { /* A12: VDD_CORE */
> +				type = <1>;
> +				reg = <0x3>;
> +				label = "VDD_CORE";
> +			};
> +
> +			hwmon at 13 { /* A13: VDD_SOC */
> +				type = <1>;
> +				reg = <0x11>;
> +				label = "VDD_SOC";
> +			};
> +
> +			hwmon at 14 { /* A14: 1P0 PCIe SW */
> +				type = <1>;
> +				reg = <0x20>;
> +				label = "1P0";
> +			};
> +
> +			hwmon at 15 { /* fan0 */
> +				type = <2>;
> +				reg = <0x2c>;
> +				label = "fan_50p";
> +			};
> +
> +			hwmon at 16 { /* fan1 */
> +				type = <2>;
> +				reg = <0x2e>;
> +				label = "fan_60p";
> +			};
> +
> +			hwmon at 17 { /* fan2 */
> +				type = <2>;
> +				reg = <0x30>;
> +				label = "fan_70p";
> +			};
> +
> +			hwmon at 18 { /* fan3 */
> +				type = <2>;
> +				reg = <0x32>;
> +				label = "fan_80p";
> +			};
> +
> +			hwmon at 19 { /* fan4 */
> +				type = <2>;
> +				reg = <0x34>;
> +				label = "fan_90p";
> +			};
> +
> +			hwmon at 20 { /* fan5 */
> +				type = <2>;
> +				reg = <0x36>;
> +				label = "fan_100p";
> +			};
> +		};
> +	};
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings
  2018-03-05 22:02   ` Tim Harvey
@ 2018-03-10 13:57     ` Fabio Estevam
  -1 siblings, 0 replies; 24+ messages in thread
From: Fabio Estevam @ 2018-03-10 13:57 UTC (permalink / raw)
  To: Tim Harvey
  Cc: Lee Jones, Rob Herring, Mark Rutland, Mark Brown,
	Dmitry Torokhov, linux-kernel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-hwmon, linux-input

Hi Tim,

On Mon, Mar 5, 2018 at 7:02 PM, Tim Harvey <tharvey@gateworks.com> wrote:

> +
> +                       hwmon@1 { /* A1: Input Voltage */
> +                               type = <1>;
> +                               reg = <0x02>;

Unit address (@1) does not match the 'reg' value.

If someone build this example with W=1 a warning will be shown.

Same problem happens in other places of this example.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings
@ 2018-03-10 13:57     ` Fabio Estevam
  0 siblings, 0 replies; 24+ messages in thread
From: Fabio Estevam @ 2018-03-10 13:57 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Tim,

On Mon, Mar 5, 2018 at 7:02 PM, Tim Harvey <tharvey@gateworks.com> wrote:

> +
> +                       hwmon at 1 { /* A1: Input Voltage */
> +                               type = <1>;
> +                               reg = <0x02>;

Unit address (@1) does not match the 'reg' value.

If someone build this example with W=1 a warning will be shown.

Same problem happens in other places of this example.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 4/4] input: misc: Add Gateworks System Controller support
  2018-03-05 22:02   ` Tim Harvey
@ 2018-03-10 18:45     ` Dmitry Torokhov
  -1 siblings, 0 replies; 24+ messages in thread
From: Dmitry Torokhov @ 2018-03-10 18:45 UTC (permalink / raw)
  To: Tim Harvey
  Cc: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, linux-kernel,
	devicetree, linux-arm-kernel, linux-hwmon, linux-input

On Mon, Mar 05, 2018 at 02:02:41PM -0800, Tim Harvey wrote:
> Add support for dispatching Linux Input events for the various interrupts
> the Gateworks System Controller provides.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> v2:
> - reword Kconfig
> - revise license comment block
> - remove unnecessary read of status register
> - remove unnecessary setting of input->dev.parent
> - use dt bindings for irq to keycode mapping
> - add support for platform-data
> - remove work-queue
> 
>  drivers/input/misc/Kconfig              |   9 ++
>  drivers/input/misc/Makefile             |   1 +
>  drivers/input/misc/gsc-input.c          | 180 ++++++++++++++++++++++++++++++++
>  include/linux/platform_data/gsc_input.h |  30 ++++++
>  4 files changed, 220 insertions(+)
>  create mode 100644 drivers/input/misc/gsc-input.c
>  create mode 100644 include/linux/platform_data/gsc_input.h
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 9f082a3..e05f4fe 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -117,6 +117,15 @@ config INPUT_E3X0_BUTTON
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called e3x0_button.
>  
> +config INPUT_GSC
> +	tristate "Gateworks System Controller input support"
> +	depends on MFD_GSC
> +	help
> +	  Support GSC events as Linux input events.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called gsc_input.
> +
>  config INPUT_PCSPKR
>  	tristate "PC Speaker support"
>  	depends on PCSPKR_PLATFORM
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 4b6118d..969debe 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GP2A)		+= gp2ap002a00f.o
>  obj-$(CONFIG_INPUT_GPIO_BEEPER)		+= gpio-beeper.o
>  obj-$(CONFIG_INPUT_GPIO_TILT_POLLED)	+= gpio_tilt_polled.o
>  obj-$(CONFIG_INPUT_GPIO_DECODER)	+= gpio_decoder.o
> +obj-$(CONFIG_INPUT_GSC)			+= gsc-input.o
>  obj-$(CONFIG_INPUT_HISI_POWERKEY)	+= hisi_powerkey.o
>  obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
>  obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
> diff --git a/drivers/input/misc/gsc-input.c b/drivers/input/misc/gsc-input.c
> new file mode 100644
> index 0000000..27b5e93
> --- /dev/null
> +++ b/drivers/input/misc/gsc-input.c
> @@ -0,0 +1,180 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2018 Gateworks Corporation
> + *
> + * This driver dispatches Linux input events for GSC interrupt events
> + */
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/mfd/gsc.h>
> +#include <linux/platform_data/gsc_input.h>
> +
> +struct gsc_input_button_priv {
> +	struct input_dev *input;
> +	const struct gsc_input_button *button;
> +};
> +
> +struct gsc_input_data {
> +	struct gsc_dev *gsc;
> +	struct gsc_input_platform_data *pdata;
> +	struct input_dev *input;
> +	struct gsc_input_button_priv *buttons;
> +	int nbuttons;
> +	unsigned int irqs[];
> +#if 0
> +	int irq;
> +	struct work_struct irq_work;
> +	struct mutex mutex;
> +#endif
> +};
> +
> +static irqreturn_t gsc_input_irq(int irq, void *data)
> +{
> +	const struct gsc_input_button_priv *button_priv = data;
> +	struct input_dev *input = button_priv->input;
> +
> +	dev_dbg(&input->dev, "irq%d code=%d\n", irq, button_priv->button->code);
> +	input_report_key(input, button_priv->button->code, 1);
> +	input_sync(input);
> +	input_report_key(input, button_priv->button->code, 0);
> +	input_sync(input);
> +
> +	return IRQ_HANDLED;

Hmm, so in the end this is just a bunch of buttons connected to
interrupt lines. We already have a driver for that, called gpio-keys. It
can work in pure interrupt mode (i.e. do not need real GPIO pin, just
interrupt, and it will generate key down and up events when interrupt
arrives, with possible delay for the up event).

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 4/4] input: misc: Add Gateworks System Controller support
@ 2018-03-10 18:45     ` Dmitry Torokhov
  0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Torokhov @ 2018-03-10 18:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Mar 05, 2018 at 02:02:41PM -0800, Tim Harvey wrote:
> Add support for dispatching Linux Input events for the various interrupts
> the Gateworks System Controller provides.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> v2:
> - reword Kconfig
> - revise license comment block
> - remove unnecessary read of status register
> - remove unnecessary setting of input->dev.parent
> - use dt bindings for irq to keycode mapping
> - add support for platform-data
> - remove work-queue
> 
>  drivers/input/misc/Kconfig              |   9 ++
>  drivers/input/misc/Makefile             |   1 +
>  drivers/input/misc/gsc-input.c          | 180 ++++++++++++++++++++++++++++++++
>  include/linux/platform_data/gsc_input.h |  30 ++++++
>  4 files changed, 220 insertions(+)
>  create mode 100644 drivers/input/misc/gsc-input.c
>  create mode 100644 include/linux/platform_data/gsc_input.h
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 9f082a3..e05f4fe 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -117,6 +117,15 @@ config INPUT_E3X0_BUTTON
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called e3x0_button.
>  
> +config INPUT_GSC
> +	tristate "Gateworks System Controller input support"
> +	depends on MFD_GSC
> +	help
> +	  Support GSC events as Linux input events.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called gsc_input.
> +
>  config INPUT_PCSPKR
>  	tristate "PC Speaker support"
>  	depends on PCSPKR_PLATFORM
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 4b6118d..969debe 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GP2A)		+= gp2ap002a00f.o
>  obj-$(CONFIG_INPUT_GPIO_BEEPER)		+= gpio-beeper.o
>  obj-$(CONFIG_INPUT_GPIO_TILT_POLLED)	+= gpio_tilt_polled.o
>  obj-$(CONFIG_INPUT_GPIO_DECODER)	+= gpio_decoder.o
> +obj-$(CONFIG_INPUT_GSC)			+= gsc-input.o
>  obj-$(CONFIG_INPUT_HISI_POWERKEY)	+= hisi_powerkey.o
>  obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
>  obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
> diff --git a/drivers/input/misc/gsc-input.c b/drivers/input/misc/gsc-input.c
> new file mode 100644
> index 0000000..27b5e93
> --- /dev/null
> +++ b/drivers/input/misc/gsc-input.c
> @@ -0,0 +1,180 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2018 Gateworks Corporation
> + *
> + * This driver dispatches Linux input events for GSC interrupt events
> + */
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/mfd/gsc.h>
> +#include <linux/platform_data/gsc_input.h>
> +
> +struct gsc_input_button_priv {
> +	struct input_dev *input;
> +	const struct gsc_input_button *button;
> +};
> +
> +struct gsc_input_data {
> +	struct gsc_dev *gsc;
> +	struct gsc_input_platform_data *pdata;
> +	struct input_dev *input;
> +	struct gsc_input_button_priv *buttons;
> +	int nbuttons;
> +	unsigned int irqs[];
> +#if 0
> +	int irq;
> +	struct work_struct irq_work;
> +	struct mutex mutex;
> +#endif
> +};
> +
> +static irqreturn_t gsc_input_irq(int irq, void *data)
> +{
> +	const struct gsc_input_button_priv *button_priv = data;
> +	struct input_dev *input = button_priv->input;
> +
> +	dev_dbg(&input->dev, "irq%d code=%d\n", irq, button_priv->button->code);
> +	input_report_key(input, button_priv->button->code, 1);
> +	input_sync(input);
> +	input_report_key(input, button_priv->button->code, 0);
> +	input_sync(input);
> +
> +	return IRQ_HANDLED;

Hmm, so in the end this is just a bunch of buttons connected to
interrupt lines. We already have a driver for that, called gpio-keys. It
can work in pure interrupt mode (i.e. do not need real GPIO pin, just
interrupt, and it will generate key down and up events when interrupt
arrives, with possible delay for the up event).

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/4] mfd: add Gateworks System Controller core driver
  2018-03-05 22:02   ` Tim Harvey
@ 2018-03-12 14:41     ` Lee Jones
  -1 siblings, 0 replies; 24+ messages in thread
From: Lee Jones @ 2018-03-12 14:41 UTC (permalink / raw)
  To: Tim Harvey
  Cc: Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov,
	linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
	linux-input, Randy Dunlap

On Mon, 05 Mar 2018, Tim Harvey wrote:

> The Gateworks System Controller (GSC) is an I2C slave controller
> implemented with an MSP430 micro-controller whose firmware embeds the
> following features:
>  - I/O expander (16 GPIO's) using PCA955x protocol
>  - Real Time Clock using DS1672 protocol
>  - User EEPROM using AT24 protocol
>  - HWMON using custom protocol
>  - Interrupt controller with tamper detect, user pushbotton
>  - Watchdog controller capable of full board power-cycle
>  - Power Control capable of full board power-cycle
> 
> see http://trac.gateworks.com/wiki/gsc for more details
> 
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> v2:
> - change license comment block style
> - remove COMPILE_TEST (Randy)
> - fixed whitespace issues
> - replaced a printk with dev_err
> 
>  drivers/mfd/Kconfig     |  13 ++
>  drivers/mfd/Makefile    |   1 +
>  drivers/mfd/gsc.c       | 330 ++++++++++++++++++++++++++++++++++++++++++++++++

I'd prefer gateworks-sc.c

>  include/linux/mfd/gsc.h |  82 ++++++++++++
>  4 files changed, 426 insertions(+)
>  create mode 100644 drivers/mfd/gsc.c
>  create mode 100644 include/linux/mfd/gsc.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 1d20a80..5694013 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -341,6 +341,19 @@ config MFD_EXYNOS_LPASS
>  	  Select this option to enable support for Samsung Exynos Low Power
>  	  Audio Subsystem.
>  
> +config MFD_GSC

Prefer MFD_GATEWORKS_SC

> +	tristate "Gateworks System Controller"
> +	depends on (I2C && OF)
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	select REGMAP_IRQ
> +	help
> +	  Enable support for the Gateworks System Controller found
> +	  on Gateworks Single Board Computers.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called gsc.
> +
>  config MFD_MC13XXX
>  	tristate
>  	depends on (SPI_MASTER || I2C)
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index d9474ad..aede0bc 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_MFD_CROS_EC)	+= cros_ec_core.o
>  obj-$(CONFIG_MFD_CROS_EC_I2C)	+= cros_ec_i2c.o
>  obj-$(CONFIG_MFD_CROS_EC_SPI)	+= cros_ec_spi.o
>  obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
> +obj-$(CONFIG_MFD_GSC)		+= gsc.o
>  
>  rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
>  obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
> diff --git a/drivers/mfd/gsc.c b/drivers/mfd/gsc.c
> new file mode 100644
> index 0000000..4ac400a
> --- /dev/null
> +++ b/drivers/mfd/gsc.c
> @@ -0,0 +1,330 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2018 Gateworks Corporation
> + *
> + * The Gateworks System Controller (GSC) is a family of a multi-function
> + * "Power Management and System Companion Device" chips originally designed for
> + * use in Gateworks Single Board Computers. The control interface is I2C,
> + * at 100kbps, with an interrupt.
> + *
> + */
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>

What's this for?

> +#include <linux/mfd/gsc.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +/*
> + * The GSC suffers from an errata where occasionally during
> + * ADC cycles the chip can NAK i2c transactions. To ensure we have reliable

s/i2c/I2C/

> + * register access we place retries around register access.
> + */
> +#define I2C_RETRIES	3
> +
> +static int gsc_regmap_regwrite(void *context, unsigned int reg,
> +			       unsigned int val)
> +{
> +	struct i2c_client *client = context;
> +	int retry, ret;
> +
> +	for (retry = 0; retry < I2C_RETRIES; retry++) {
> +		ret = i2c_smbus_write_byte_data(client, reg, val);
> +		/*
> +		 * -EAGAIN returned when the i2c host controller is busy
> +		 * -EIO returned when i2c device is busy
> +		 */
> +		if (ret != -EAGAIN && ret != -EIO)
> +			break;
> +	}
> +	if (ret < 0) {
> +		dev_err(&client->dev, ">> 0x%02x %d\n", reg, ret);

">>" doesn't tell the user much.  To the unaware you are spitting out
random characters/values.  Please provide more information in your
error messages.


> +		return ret;
> +	}
> +	dev_dbg(&client->dev, ">> 0x%02x=0x%02x (%d)\n", reg, val, retry);

As above.  Or better still, consider removing this line completely.

> +	return 0;
> +}
> +
> +static int gsc_regmap_regread(void *context, unsigned int reg,
> +			      unsigned int *val)
> +{
> +	struct i2c_client *client = context;
> +	int retry, ret;
> +
> +	for (retry = 0; retry < I2C_RETRIES; retry++) {
> +		ret = i2c_smbus_read_byte_data(client, reg);
> +		/*
> +		 * -EAGAIN returned when the i2c host controller is busy
> +		 * -EIO returned when i2c device is busy
> +		 */
> +		if (ret != -EAGAIN && ret != -EIO)
> +			break;
> +	}
> +	if (ret < 0) {
> +		dev_err(&client->dev, "<< 0x%02x %d\n", reg, ret);

As above.

> +		return ret;
> +	}
> +
> +	*val = ret & 0xff;
> +	dev_dbg(&client->dev, "<< 0x%02x=0x%02x (%d)\n", reg, *val, retry);

As above.

> +	return 0;
> +}
> +
> +static struct regmap_bus regmap_gsc = {
> +	.reg_write = gsc_regmap_regwrite,
> +	.reg_read = gsc_regmap_regread,
> +};
> +
> +/*
> + * gsc_powerdown - API to use GSC to power down board for a specific time
> + *
> + * secs - number of seconds to remain powered off
> + */
> +static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
> +{
> +	int ret;
> +	unsigned char regs[4];
> +
> +	dev_info(&gsc->i2c->dev, "GSC powerdown for %ld seconds\n",
> +		 secs);
> +	regs[0] = secs & 0xff;
> +	regs[1] = (secs >> 8) & 0xff;
> +	regs[2] = (secs >> 16) & 0xff;
> +	regs[3] = (secs >> 24) & 0xff;
> +	ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
> +
> +	return ret;
> +}
> +
> +/*
> + * sysfs hooks
> + */

This comment is superfluous since we know what _show & _store functions
do.

> +static ssize_t gsc_show(struct device *dev, struct device_attribute *attr,
> +			char *buf)
> +{
> +	struct gsc_dev *gsc = dev_get_drvdata(dev);
> +	const char *name = attr->attr.name;
> +	int rz = 0;
> +
> +	if (strcasecmp(name, "fw_version") == 0)
> +		rz = sprintf(buf, "%d\n", gsc->fwver);
> +	else if (strcasecmp(name, "fw_crc") == 0)
> +		rz = sprintf(buf, "0x%04x\n", gsc->fwcrc);
> +
> +	return rz;
> +}
> +
> +static ssize_t gsc_store(struct device *dev, struct device_attribute *attr,
> +			 const char *buf, size_t count)
> +{
> +	struct gsc_dev *gsc = dev_get_drvdata(dev);
> +	const char *name = attr->attr.name;
> +	int ret;
> +
> +	if (strcasecmp(name, "powerdown") == 0) {
> +		long value;
> +
> +		ret = kstrtol(buf, 0, &value);
> +		if (ret == 0)
> +			gsc_powerdown(gsc, value);
> +	} else
> +		dev_err(dev, "invalid name '%s\n", name);
> +
> +	return count;
> +}
> +
> +

Please remove 2nd '\n'.

> +/*
> + * Create a group of attributes so that we can create and destroy them all
> + * at once.
> + */

No need for this comment.

> +static struct device_attribute attr_fwver =
> +	__ATTR(fw_version, 0440, gsc_show, NULL);
> +static struct device_attribute attr_fwcrc =
> +	__ATTR(fw_crc, 0440, gsc_show, NULL);
> +static struct device_attribute attr_pwrdown =
> +	__ATTR(powerdown, 0220, NULL, gsc_store);
> +
> +static struct attribute *gsc_attrs[] = {
> +	&attr_fwver.attr,
> +	&attr_fwcrc.attr,
> +	&attr_pwrdown.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group attr_group = {
> +	.attrs = gsc_attrs,
> +};
> +
> +static const struct i2c_device_id gsc_i2c_ids[] = {
> +	{ "gsc_v1", gsc_v1 },
> +	{ "gsc_v2", gsc_v2 },
> +	{ "gsc_v3", gsc_v3 },
> +	{ },
> +};
> +
> +static const struct of_device_id gsc_of_match[] = {
> +	{ .compatible = "gw,gsc_v1", .data = (void *)gsc_v1 },
> +	{ .compatible = "gw,gsc_v2", .data = (void *)gsc_v2 },
> +	{ .compatible = "gw,gsc_v3", .data = (void *)gsc_v3 },
> +	{ }
> +};
> +
> +static const struct regmap_config gsc_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.cache_type = REGCACHE_NONE,
> +	.max_register = 0xf,
> +};
> +
> +static const struct regmap_config gsc_regmap_hwmon_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.cache_type = REGCACHE_NONE,
> +	.max_register = 0x37,
> +};
> +
> +static const struct regmap_irq gsc_irqs[] = {
> +	REGMAP_IRQ_REG(GSC_IRQ_PB, 0, BIT(GSC_IRQ_PB)),
> +	REGMAP_IRQ_REG(GSC_IRQ_KEY_ERASED, 0, BIT(GSC_IRQ_KEY_ERASED)),
> +	REGMAP_IRQ_REG(GSC_IRQ_EEPROM_WP, 0, BIT(GSC_IRQ_EEPROM_WP)),
> +	REGMAP_IRQ_REG(GSC_IRQ_RESV, 0, BIT(GSC_IRQ_RESV)),
> +	REGMAP_IRQ_REG(GSC_IRQ_GPIO, 0, BIT(GSC_IRQ_GPIO)),
> +	REGMAP_IRQ_REG(GSC_IRQ_TAMPER, 0, BIT(GSC_IRQ_TAMPER)),
> +	REGMAP_IRQ_REG(GSC_IRQ_WDT_TIMEOUT, 0, BIT(GSC_IRQ_WDT_TIMEOUT)),
> +	REGMAP_IRQ_REG(GSC_IRQ_SWITCH_HOLD, 0, BIT(GSC_IRQ_SWITCH_HOLD)),
> +};
> +
> +static const struct regmap_irq_chip gsc_irq_chip = {
> +	.name = KBUILD_MODNAME,

Please just type the name like "name".

> +	.irqs = gsc_irqs,
> +	.num_irqs = ARRAY_SIZE(gsc_irqs),
> +	.num_regs = 1,
> +	.status_base = GSC_IRQ_STATUS,
> +	.mask_base = GSC_IRQ_ENABLE,
> +	.mask_invert = true,
> +	.ack_base = GSC_IRQ_STATUS,
> +	.ack_invert = true,
> +};
> +
> +static int gsc_of_probe(struct device_node *np, struct gsc_dev *gsc)
> +{
> +	const struct of_device_id *of_id;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	of_id = of_match_device(gsc_of_match, gsc->dev);
> +	if (of_id)
> +		gsc->type = (enum gsc_type)of_id->data;

Isn't there any way that you can fetch version information from the
device?

What is this value/information actually used for?

> +	return 0;
> +}
> +
> +static int
> +gsc_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
> +	struct device *dev = &client->dev;
> +	struct gsc_dev *gsc;
> +	int ret;
> +	unsigned int reg;
> +
> +	gsc = devm_kzalloc(dev, sizeof(*gsc), GFP_KERNEL);
> +	if (!gsc)
> +		return -ENOMEM;
> +
> +	gsc->dev = &client->dev;
> +	gsc->i2c = client;
> +	gsc->irq = client->irq;
> +	i2c_set_clientdata(client, gsc);
> +
> +	gsc->regmap = devm_regmap_init(dev, &regmap_gsc, client,
> +				       &gsc_regmap_config);
> +	if (IS_ERR(gsc->regmap))
> +		return PTR_ERR(gsc->regmap);
> +
> +	ret = gsc_of_probe(dev->of_node, gsc);
> +	if (reg < 0)
> +		return ret;
> +
> +	if (regmap_read(gsc->regmap, GSC_FW_VER, &reg))
> +		return -EIO;
> +	gsc->fwver = reg;

'\n' here.

> +	regmap_read(gsc->regmap, GSC_FW_CRC, &reg);
> +	gsc->fwcrc = reg;
> +	regmap_read(gsc->regmap, GSC_FW_CRC + 1, &reg);
> +	gsc->fwcrc |= reg << 8;
> +
> +	gsc->i2c_hwmon = i2c_new_dummy(client->adapter, GSC_HWMON);
> +	if (!gsc->i2c_hwmon) {
> +		dev_err(dev, "Failed to allocate I2C device for HWMON\n");
> +		return -ENODEV;
> +	}
> +	i2c_set_clientdata(gsc->i2c_hwmon, gsc);
> +
> +	gsc->regmap_hwmon = devm_regmap_init(dev, &regmap_gsc, gsc->i2c_hwmon,
> +					     &gsc_regmap_hwmon_config);
> +	if (IS_ERR(gsc->regmap_hwmon)) {
> +		ret = PTR_ERR(gsc->regmap_hwmon);
> +		dev_err(dev, "failed to allocate register map: %d\n", ret);
> +		goto err_regmap;
> +	}
> +
> +	ret = devm_regmap_add_irq_chip(dev, gsc->regmap, gsc->irq,
> +				       IRQF_ONESHOT | IRQF_SHARED |
> +				       IRQF_TRIGGER_FALLING, 0,
> +				       &gsc_irq_chip, &gsc->irq_chip_data);
> +	if (ret)
> +		goto err_regmap;
> +
> +	dev_info(dev, "Gateworks System Controller v%d: fw v%02d 0x%04x\n",
> +		 gsc->type, gsc->fwver, gsc->fwcrc);
> +
> +	/* sysfs hooks */

Remove this comment please.

> +	ret = sysfs_create_group(&dev->kobj, &attr_group);
> +	if (ret)
> +		dev_err(dev, "failed to create sysfs attrs\n");
> +
> +	ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
> +	if (ret)
> +		goto err_sysfs;
> +
> +	return 0;
> +
> +err_sysfs:
> +	sysfs_remove_group(&dev->kobj, &attr_group);
> +err_regmap:
> +	i2c_unregister_device(gsc->i2c_hwmon);
> +
> +	return ret;
> +}
> +
> +static int gsc_remove(struct i2c_client *client)
> +{
> +	sysfs_remove_group(&client->dev.kobj, &attr_group);

i2c_unregister_device()?

> +	return 0;
> +}
> +
> +static struct i2c_driver gsc_driver = {
> +	.driver = {
> +		.name	= KBUILD_MODNAME,

Just type the name please: "name".

> +		.of_match_table = of_match_ptr(gsc_of_match),
> +	},
> +	.probe		= gsc_probe,
> +	.remove		= gsc_remove,
> +	.id_table	= gsc_i2c_ids,
> +};
> +
> +module_i2c_driver(gsc_driver);
> +
> +MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
> +MODULE_DESCRIPTION("I2C Core interface for GSC");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/gsc.h b/include/linux/mfd/gsc.h
> new file mode 100644
> index 0000000..e0d8c8f
> --- /dev/null
> +++ b/include/linux/mfd/gsc.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2018 Gateworks Corporation
> + */
> +#ifndef __LINUX_MFD_GSC_H_
> +#define __LINUX_MFD_GSC_H_
> +
> +/* Device Addresses */
> +#define GSC_MISC	0x20
> +#define GSC_UPDATE	0x21
> +#define GSC_GPIO	0x23
> +#define GSC_HWMON	0x29
> +#define GSC_EEPROM0	0x50
> +#define GSC_EEPROM1	0x51
> +#define GSC_EEPROM2	0x52
> +#define GSC_EEPROM3	0x53
> +#define GSC_RTC		0x68
> +
> +/* Register offsets */
> +#define GSC_CTRL_0	0x00
> +#define GSC_CTRL_1	0x01
> +#define GSC_TIME	0x02
> +#define GSC_TIME_ADD	0x06
> +#define GSC_IRQ_STATUS	0x0A
> +#define GSC_IRQ_ENABLE	0x0B
> +#define GSC_FW_CRC	0x0C
> +#define GSC_FW_VER	0x0E
> +#define GSC_WP		0x0F
> +
> +/* Bit definitions */
> +#define GSC_CTRL_0_PB_HARD_RESET	0
> +#define GSC_CTRL_0_PB_CLEAR_SECURE_KEY	1
> +#define GSC_CTRL_0_PB_SOFT_POWER_DOWN	2
> +#define GSC_CTRL_0_PB_BOOT_ALTERNATE	3
> +#define GSC_CTRL_0_PERFORM_CRC		4
> +#define GSC_CTRL_0_TAMPER_DETECT	5
> +#define GSC_CTRL_0_SWITCH_HOLD		6
> +
> +#define GSC_CTRL_1_SLEEP_ENABLE		0
> +#define GSC_CTRL_1_ACTIVATE_SLEEP	1
> +#define GSC_CTRL_1_LATCH_SLEEP_ADD	2
> +#define GSC_CTRL_1_SLEEP_NOWAKEPB	3
> +#define GSC_CTRL_1_WDT_TIME		4
> +#define GSC_CTRL_1_WDT_ENABLE		5
> +#define GSC_CTRL_1_SWITCH_BOOT_ENABLE	6
> +#define GSC_CTRL_1_SWITCH_BOOT_CLEAR	7
> +
> +#define GSC_IRQ_PB			0
> +#define GSC_IRQ_KEY_ERASED		1
> +#define GSC_IRQ_EEPROM_WP		2
> +#define GSC_IRQ_RESV			3
> +#define GSC_IRQ_GPIO			4
> +#define GSC_IRQ_TAMPER			5
> +#define GSC_IRQ_WDT_TIMEOUT		6
> +#define GSC_IRQ_SWITCH_HOLD		7
> +
> +/* Max registers */
> +#define GSC_HWMON_MAX_REG	56
> +
> +enum gsc_type {
> +	gsc_v1 = 1,
> +	gsc_v2 = 2,
> +	gsc_v3 = 3,
> +};
> +
> +struct gsc_dev {
> +	struct device *dev;
> +
> +	struct i2c_client *i2c;		/* 0x20: interrupt controller, WDT */
> +	struct i2c_client *i2c_hwmon;	/* 0x29: hwmon, fan controller */
> +
> +	struct regmap *regmap;
> +	struct regmap *regmap_hwmon;
> +	struct regmap_irq_chip_data *irq_chip_data;
> +
> +	int irq;
> +	enum gsc_type type;
> +	unsigned int fwver;
> +	unsigned short fwcrc;
> +};
> +
> +#endif /* __LINUX_MFD_GSC_H_ */

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 2/4] mfd: add Gateworks System Controller core driver
@ 2018-03-12 14:41     ` Lee Jones
  0 siblings, 0 replies; 24+ messages in thread
From: Lee Jones @ 2018-03-12 14:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 05 Mar 2018, Tim Harvey wrote:

> The Gateworks System Controller (GSC) is an I2C slave controller
> implemented with an MSP430 micro-controller whose firmware embeds the
> following features:
>  - I/O expander (16 GPIO's) using PCA955x protocol
>  - Real Time Clock using DS1672 protocol
>  - User EEPROM using AT24 protocol
>  - HWMON using custom protocol
>  - Interrupt controller with tamper detect, user pushbotton
>  - Watchdog controller capable of full board power-cycle
>  - Power Control capable of full board power-cycle
> 
> see http://trac.gateworks.com/wiki/gsc for more details
> 
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> v2:
> - change license comment block style
> - remove COMPILE_TEST (Randy)
> - fixed whitespace issues
> - replaced a printk with dev_err
> 
>  drivers/mfd/Kconfig     |  13 ++
>  drivers/mfd/Makefile    |   1 +
>  drivers/mfd/gsc.c       | 330 ++++++++++++++++++++++++++++++++++++++++++++++++

I'd prefer gateworks-sc.c

>  include/linux/mfd/gsc.h |  82 ++++++++++++
>  4 files changed, 426 insertions(+)
>  create mode 100644 drivers/mfd/gsc.c
>  create mode 100644 include/linux/mfd/gsc.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 1d20a80..5694013 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -341,6 +341,19 @@ config MFD_EXYNOS_LPASS
>  	  Select this option to enable support for Samsung Exynos Low Power
>  	  Audio Subsystem.
>  
> +config MFD_GSC

Prefer MFD_GATEWORKS_SC

> +	tristate "Gateworks System Controller"
> +	depends on (I2C && OF)
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	select REGMAP_IRQ
> +	help
> +	  Enable support for the Gateworks System Controller found
> +	  on Gateworks Single Board Computers.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called gsc.
> +
>  config MFD_MC13XXX
>  	tristate
>  	depends on (SPI_MASTER || I2C)
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index d9474ad..aede0bc 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_MFD_CROS_EC)	+= cros_ec_core.o
>  obj-$(CONFIG_MFD_CROS_EC_I2C)	+= cros_ec_i2c.o
>  obj-$(CONFIG_MFD_CROS_EC_SPI)	+= cros_ec_spi.o
>  obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
> +obj-$(CONFIG_MFD_GSC)		+= gsc.o
>  
>  rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
>  obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
> diff --git a/drivers/mfd/gsc.c b/drivers/mfd/gsc.c
> new file mode 100644
> index 0000000..4ac400a
> --- /dev/null
> +++ b/drivers/mfd/gsc.c
> @@ -0,0 +1,330 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2018 Gateworks Corporation
> + *
> + * The Gateworks System Controller (GSC) is a family of a multi-function
> + * "Power Management and System Companion Device" chips originally designed for
> + * use in Gateworks Single Board Computers. The control interface is I2C,
> + * at 100kbps, with an interrupt.
> + *
> + */
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>

What's this for?

> +#include <linux/mfd/gsc.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +/*
> + * The GSC suffers from an errata where occasionally during
> + * ADC cycles the chip can NAK i2c transactions. To ensure we have reliable

s/i2c/I2C/

> + * register access we place retries around register access.
> + */
> +#define I2C_RETRIES	3
> +
> +static int gsc_regmap_regwrite(void *context, unsigned int reg,
> +			       unsigned int val)
> +{
> +	struct i2c_client *client = context;
> +	int retry, ret;
> +
> +	for (retry = 0; retry < I2C_RETRIES; retry++) {
> +		ret = i2c_smbus_write_byte_data(client, reg, val);
> +		/*
> +		 * -EAGAIN returned when the i2c host controller is busy
> +		 * -EIO returned when i2c device is busy
> +		 */
> +		if (ret != -EAGAIN && ret != -EIO)
> +			break;
> +	}
> +	if (ret < 0) {
> +		dev_err(&client->dev, ">> 0x%02x %d\n", reg, ret);

">>" doesn't tell the user much.  To the unaware you are spitting out
random characters/values.  Please provide more information in your
error messages.


> +		return ret;
> +	}
> +	dev_dbg(&client->dev, ">> 0x%02x=0x%02x (%d)\n", reg, val, retry);

As above.  Or better still, consider removing this line completely.

> +	return 0;
> +}
> +
> +static int gsc_regmap_regread(void *context, unsigned int reg,
> +			      unsigned int *val)
> +{
> +	struct i2c_client *client = context;
> +	int retry, ret;
> +
> +	for (retry = 0; retry < I2C_RETRIES; retry++) {
> +		ret = i2c_smbus_read_byte_data(client, reg);
> +		/*
> +		 * -EAGAIN returned when the i2c host controller is busy
> +		 * -EIO returned when i2c device is busy
> +		 */
> +		if (ret != -EAGAIN && ret != -EIO)
> +			break;
> +	}
> +	if (ret < 0) {
> +		dev_err(&client->dev, "<< 0x%02x %d\n", reg, ret);

As above.

> +		return ret;
> +	}
> +
> +	*val = ret & 0xff;
> +	dev_dbg(&client->dev, "<< 0x%02x=0x%02x (%d)\n", reg, *val, retry);

As above.

> +	return 0;
> +}
> +
> +static struct regmap_bus regmap_gsc = {
> +	.reg_write = gsc_regmap_regwrite,
> +	.reg_read = gsc_regmap_regread,
> +};
> +
> +/*
> + * gsc_powerdown - API to use GSC to power down board for a specific time
> + *
> + * secs - number of seconds to remain powered off
> + */
> +static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
> +{
> +	int ret;
> +	unsigned char regs[4];
> +
> +	dev_info(&gsc->i2c->dev, "GSC powerdown for %ld seconds\n",
> +		 secs);
> +	regs[0] = secs & 0xff;
> +	regs[1] = (secs >> 8) & 0xff;
> +	regs[2] = (secs >> 16) & 0xff;
> +	regs[3] = (secs >> 24) & 0xff;
> +	ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
> +
> +	return ret;
> +}
> +
> +/*
> + * sysfs hooks
> + */

This comment is superfluous since we know what _show & _store functions
do.

> +static ssize_t gsc_show(struct device *dev, struct device_attribute *attr,
> +			char *buf)
> +{
> +	struct gsc_dev *gsc = dev_get_drvdata(dev);
> +	const char *name = attr->attr.name;
> +	int rz = 0;
> +
> +	if (strcasecmp(name, "fw_version") == 0)
> +		rz = sprintf(buf, "%d\n", gsc->fwver);
> +	else if (strcasecmp(name, "fw_crc") == 0)
> +		rz = sprintf(buf, "0x%04x\n", gsc->fwcrc);
> +
> +	return rz;
> +}
> +
> +static ssize_t gsc_store(struct device *dev, struct device_attribute *attr,
> +			 const char *buf, size_t count)
> +{
> +	struct gsc_dev *gsc = dev_get_drvdata(dev);
> +	const char *name = attr->attr.name;
> +	int ret;
> +
> +	if (strcasecmp(name, "powerdown") == 0) {
> +		long value;
> +
> +		ret = kstrtol(buf, 0, &value);
> +		if (ret == 0)
> +			gsc_powerdown(gsc, value);
> +	} else
> +		dev_err(dev, "invalid name '%s\n", name);
> +
> +	return count;
> +}
> +
> +

Please remove 2nd '\n'.

> +/*
> + * Create a group of attributes so that we can create and destroy them all
> + * at once.
> + */

No need for this comment.

> +static struct device_attribute attr_fwver =
> +	__ATTR(fw_version, 0440, gsc_show, NULL);
> +static struct device_attribute attr_fwcrc =
> +	__ATTR(fw_crc, 0440, gsc_show, NULL);
> +static struct device_attribute attr_pwrdown =
> +	__ATTR(powerdown, 0220, NULL, gsc_store);
> +
> +static struct attribute *gsc_attrs[] = {
> +	&attr_fwver.attr,
> +	&attr_fwcrc.attr,
> +	&attr_pwrdown.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group attr_group = {
> +	.attrs = gsc_attrs,
> +};
> +
> +static const struct i2c_device_id gsc_i2c_ids[] = {
> +	{ "gsc_v1", gsc_v1 },
> +	{ "gsc_v2", gsc_v2 },
> +	{ "gsc_v3", gsc_v3 },
> +	{ },
> +};
> +
> +static const struct of_device_id gsc_of_match[] = {
> +	{ .compatible = "gw,gsc_v1", .data = (void *)gsc_v1 },
> +	{ .compatible = "gw,gsc_v2", .data = (void *)gsc_v2 },
> +	{ .compatible = "gw,gsc_v3", .data = (void *)gsc_v3 },
> +	{ }
> +};
> +
> +static const struct regmap_config gsc_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.cache_type = REGCACHE_NONE,
> +	.max_register = 0xf,
> +};
> +
> +static const struct regmap_config gsc_regmap_hwmon_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.cache_type = REGCACHE_NONE,
> +	.max_register = 0x37,
> +};
> +
> +static const struct regmap_irq gsc_irqs[] = {
> +	REGMAP_IRQ_REG(GSC_IRQ_PB, 0, BIT(GSC_IRQ_PB)),
> +	REGMAP_IRQ_REG(GSC_IRQ_KEY_ERASED, 0, BIT(GSC_IRQ_KEY_ERASED)),
> +	REGMAP_IRQ_REG(GSC_IRQ_EEPROM_WP, 0, BIT(GSC_IRQ_EEPROM_WP)),
> +	REGMAP_IRQ_REG(GSC_IRQ_RESV, 0, BIT(GSC_IRQ_RESV)),
> +	REGMAP_IRQ_REG(GSC_IRQ_GPIO, 0, BIT(GSC_IRQ_GPIO)),
> +	REGMAP_IRQ_REG(GSC_IRQ_TAMPER, 0, BIT(GSC_IRQ_TAMPER)),
> +	REGMAP_IRQ_REG(GSC_IRQ_WDT_TIMEOUT, 0, BIT(GSC_IRQ_WDT_TIMEOUT)),
> +	REGMAP_IRQ_REG(GSC_IRQ_SWITCH_HOLD, 0, BIT(GSC_IRQ_SWITCH_HOLD)),
> +};
> +
> +static const struct regmap_irq_chip gsc_irq_chip = {
> +	.name = KBUILD_MODNAME,

Please just type the name like "name".

> +	.irqs = gsc_irqs,
> +	.num_irqs = ARRAY_SIZE(gsc_irqs),
> +	.num_regs = 1,
> +	.status_base = GSC_IRQ_STATUS,
> +	.mask_base = GSC_IRQ_ENABLE,
> +	.mask_invert = true,
> +	.ack_base = GSC_IRQ_STATUS,
> +	.ack_invert = true,
> +};
> +
> +static int gsc_of_probe(struct device_node *np, struct gsc_dev *gsc)
> +{
> +	const struct of_device_id *of_id;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	of_id = of_match_device(gsc_of_match, gsc->dev);
> +	if (of_id)
> +		gsc->type = (enum gsc_type)of_id->data;

Isn't there any way that you can fetch version information from the
device?

What is this value/information actually used for?

> +	return 0;
> +}
> +
> +static int
> +gsc_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
> +	struct device *dev = &client->dev;
> +	struct gsc_dev *gsc;
> +	int ret;
> +	unsigned int reg;
> +
> +	gsc = devm_kzalloc(dev, sizeof(*gsc), GFP_KERNEL);
> +	if (!gsc)
> +		return -ENOMEM;
> +
> +	gsc->dev = &client->dev;
> +	gsc->i2c = client;
> +	gsc->irq = client->irq;
> +	i2c_set_clientdata(client, gsc);
> +
> +	gsc->regmap = devm_regmap_init(dev, &regmap_gsc, client,
> +				       &gsc_regmap_config);
> +	if (IS_ERR(gsc->regmap))
> +		return PTR_ERR(gsc->regmap);
> +
> +	ret = gsc_of_probe(dev->of_node, gsc);
> +	if (reg < 0)
> +		return ret;
> +
> +	if (regmap_read(gsc->regmap, GSC_FW_VER, &reg))
> +		return -EIO;
> +	gsc->fwver = reg;

'\n' here.

> +	regmap_read(gsc->regmap, GSC_FW_CRC, &reg);
> +	gsc->fwcrc = reg;
> +	regmap_read(gsc->regmap, GSC_FW_CRC + 1, &reg);
> +	gsc->fwcrc |= reg << 8;
> +
> +	gsc->i2c_hwmon = i2c_new_dummy(client->adapter, GSC_HWMON);
> +	if (!gsc->i2c_hwmon) {
> +		dev_err(dev, "Failed to allocate I2C device for HWMON\n");
> +		return -ENODEV;
> +	}
> +	i2c_set_clientdata(gsc->i2c_hwmon, gsc);
> +
> +	gsc->regmap_hwmon = devm_regmap_init(dev, &regmap_gsc, gsc->i2c_hwmon,
> +					     &gsc_regmap_hwmon_config);
> +	if (IS_ERR(gsc->regmap_hwmon)) {
> +		ret = PTR_ERR(gsc->regmap_hwmon);
> +		dev_err(dev, "failed to allocate register map: %d\n", ret);
> +		goto err_regmap;
> +	}
> +
> +	ret = devm_regmap_add_irq_chip(dev, gsc->regmap, gsc->irq,
> +				       IRQF_ONESHOT | IRQF_SHARED |
> +				       IRQF_TRIGGER_FALLING, 0,
> +				       &gsc_irq_chip, &gsc->irq_chip_data);
> +	if (ret)
> +		goto err_regmap;
> +
> +	dev_info(dev, "Gateworks System Controller v%d: fw v%02d 0x%04x\n",
> +		 gsc->type, gsc->fwver, gsc->fwcrc);
> +
> +	/* sysfs hooks */

Remove this comment please.

> +	ret = sysfs_create_group(&dev->kobj, &attr_group);
> +	if (ret)
> +		dev_err(dev, "failed to create sysfs attrs\n");
> +
> +	ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
> +	if (ret)
> +		goto err_sysfs;
> +
> +	return 0;
> +
> +err_sysfs:
> +	sysfs_remove_group(&dev->kobj, &attr_group);
> +err_regmap:
> +	i2c_unregister_device(gsc->i2c_hwmon);
> +
> +	return ret;
> +}
> +
> +static int gsc_remove(struct i2c_client *client)
> +{
> +	sysfs_remove_group(&client->dev.kobj, &attr_group);

i2c_unregister_device()?

> +	return 0;
> +}
> +
> +static struct i2c_driver gsc_driver = {
> +	.driver = {
> +		.name	= KBUILD_MODNAME,

Just type the name please: "name".

> +		.of_match_table = of_match_ptr(gsc_of_match),
> +	},
> +	.probe		= gsc_probe,
> +	.remove		= gsc_remove,
> +	.id_table	= gsc_i2c_ids,
> +};
> +
> +module_i2c_driver(gsc_driver);
> +
> +MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
> +MODULE_DESCRIPTION("I2C Core interface for GSC");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/gsc.h b/include/linux/mfd/gsc.h
> new file mode 100644
> index 0000000..e0d8c8f
> --- /dev/null
> +++ b/include/linux/mfd/gsc.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2018 Gateworks Corporation
> + */
> +#ifndef __LINUX_MFD_GSC_H_
> +#define __LINUX_MFD_GSC_H_
> +
> +/* Device Addresses */
> +#define GSC_MISC	0x20
> +#define GSC_UPDATE	0x21
> +#define GSC_GPIO	0x23
> +#define GSC_HWMON	0x29
> +#define GSC_EEPROM0	0x50
> +#define GSC_EEPROM1	0x51
> +#define GSC_EEPROM2	0x52
> +#define GSC_EEPROM3	0x53
> +#define GSC_RTC		0x68
> +
> +/* Register offsets */
> +#define GSC_CTRL_0	0x00
> +#define GSC_CTRL_1	0x01
> +#define GSC_TIME	0x02
> +#define GSC_TIME_ADD	0x06
> +#define GSC_IRQ_STATUS	0x0A
> +#define GSC_IRQ_ENABLE	0x0B
> +#define GSC_FW_CRC	0x0C
> +#define GSC_FW_VER	0x0E
> +#define GSC_WP		0x0F
> +
> +/* Bit definitions */
> +#define GSC_CTRL_0_PB_HARD_RESET	0
> +#define GSC_CTRL_0_PB_CLEAR_SECURE_KEY	1
> +#define GSC_CTRL_0_PB_SOFT_POWER_DOWN	2
> +#define GSC_CTRL_0_PB_BOOT_ALTERNATE	3
> +#define GSC_CTRL_0_PERFORM_CRC		4
> +#define GSC_CTRL_0_TAMPER_DETECT	5
> +#define GSC_CTRL_0_SWITCH_HOLD		6
> +
> +#define GSC_CTRL_1_SLEEP_ENABLE		0
> +#define GSC_CTRL_1_ACTIVATE_SLEEP	1
> +#define GSC_CTRL_1_LATCH_SLEEP_ADD	2
> +#define GSC_CTRL_1_SLEEP_NOWAKEPB	3
> +#define GSC_CTRL_1_WDT_TIME		4
> +#define GSC_CTRL_1_WDT_ENABLE		5
> +#define GSC_CTRL_1_SWITCH_BOOT_ENABLE	6
> +#define GSC_CTRL_1_SWITCH_BOOT_CLEAR	7
> +
> +#define GSC_IRQ_PB			0
> +#define GSC_IRQ_KEY_ERASED		1
> +#define GSC_IRQ_EEPROM_WP		2
> +#define GSC_IRQ_RESV			3
> +#define GSC_IRQ_GPIO			4
> +#define GSC_IRQ_TAMPER			5
> +#define GSC_IRQ_WDT_TIMEOUT		6
> +#define GSC_IRQ_SWITCH_HOLD		7
> +
> +/* Max registers */
> +#define GSC_HWMON_MAX_REG	56
> +
> +enum gsc_type {
> +	gsc_v1 = 1,
> +	gsc_v2 = 2,
> +	gsc_v3 = 3,
> +};
> +
> +struct gsc_dev {
> +	struct device *dev;
> +
> +	struct i2c_client *i2c;		/* 0x20: interrupt controller, WDT */
> +	struct i2c_client *i2c_hwmon;	/* 0x29: hwmon, fan controller */
> +
> +	struct regmap *regmap;
> +	struct regmap *regmap_hwmon;
> +	struct regmap_irq_chip_data *irq_chip_data;
> +
> +	int irq;
> +	enum gsc_type type;
> +	unsigned int fwver;
> +	unsigned short fwcrc;
> +};
> +
> +#endif /* __LINUX_MFD_GSC_H_ */

-- 
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 4/4] input: misc: Add Gateworks System Controller support
  2018-03-10 18:45     ` Dmitry Torokhov
@ 2018-03-14 17:13       ` Tim Harvey
  -1 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-14 17:13 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, linux-kernel,
	devicetree, linux-arm-kernel, linux-hwmon, linux-input

On Sat, Mar 10, 2018 at 10:45 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Mar 05, 2018 at 02:02:41PM -0800, Tim Harvey wrote:
>> Add support for dispatching Linux Input events for the various interrupts
>> the Gateworks System Controller provides.
>>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>> ---
>> v2:
>> - reword Kconfig
>> - revise license comment block
>> - remove unnecessary read of status register
>> - remove unnecessary setting of input->dev.parent
>> - use dt bindings for irq to keycode mapping
>> - add support for platform-data
>> - remove work-queue
>>
>>  drivers/input/misc/Kconfig              |   9 ++
>>  drivers/input/misc/Makefile             |   1 +
>>  drivers/input/misc/gsc-input.c          | 180 ++++++++++++++++++++++++++++++++
>>  include/linux/platform_data/gsc_input.h |  30 ++++++
>>  4 files changed, 220 insertions(+)
>>  create mode 100644 drivers/input/misc/gsc-input.c
>>  create mode 100644 include/linux/platform_data/gsc_input.h
>>
>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>> index 9f082a3..e05f4fe 100644
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -117,6 +117,15 @@ config INPUT_E3X0_BUTTON
>>         To compile this driver as a module, choose M here: the
>>         module will be called e3x0_button.
>>
>> +config INPUT_GSC
>> +     tristate "Gateworks System Controller input support"
>> +     depends on MFD_GSC
>> +     help
>> +       Support GSC events as Linux input events.
>> +
>> +       To compile this driver as a module, choose M here: the
>> +       module will be called gsc_input.
>> +
>>  config INPUT_PCSPKR
>>       tristate "PC Speaker support"
>>       depends on PCSPKR_PLATFORM
>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>> index 4b6118d..969debe 100644
>> --- a/drivers/input/misc/Makefile
>> +++ b/drivers/input/misc/Makefile
>> @@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GP2A)            += gp2ap002a00f.o
>>  obj-$(CONFIG_INPUT_GPIO_BEEPER)              += gpio-beeper.o
>>  obj-$(CONFIG_INPUT_GPIO_TILT_POLLED) += gpio_tilt_polled.o
>>  obj-$(CONFIG_INPUT_GPIO_DECODER)     += gpio_decoder.o
>> +obj-$(CONFIG_INPUT_GSC)                      += gsc-input.o
>>  obj-$(CONFIG_INPUT_HISI_POWERKEY)    += hisi_powerkey.o
>>  obj-$(CONFIG_HP_SDC_RTC)             += hp_sdc_rtc.o
>>  obj-$(CONFIG_INPUT_IMS_PCU)          += ims-pcu.o
>> diff --git a/drivers/input/misc/gsc-input.c b/drivers/input/misc/gsc-input.c
>> new file mode 100644
>> index 0000000..27b5e93
>> --- /dev/null
>> +++ b/drivers/input/misc/gsc-input.c
>> @@ -0,0 +1,180 @@
>> +/* SPDX-License-Identifier: GPL-2.0
>> + *
>> + * Copyright (C) 2018 Gateworks Corporation
>> + *
>> + * This driver dispatches Linux input events for GSC interrupt events
>> + */
>> +#include <linux/input.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <linux/mfd/gsc.h>
>> +#include <linux/platform_data/gsc_input.h>
>> +
>> +struct gsc_input_button_priv {
>> +     struct input_dev *input;
>> +     const struct gsc_input_button *button;
>> +};
>> +
>> +struct gsc_input_data {
>> +     struct gsc_dev *gsc;
>> +     struct gsc_input_platform_data *pdata;
>> +     struct input_dev *input;
>> +     struct gsc_input_button_priv *buttons;
>> +     int nbuttons;
>> +     unsigned int irqs[];
>> +#if 0
>> +     int irq;
>> +     struct work_struct irq_work;
>> +     struct mutex mutex;
>> +#endif
>> +};
>> +
>> +static irqreturn_t gsc_input_irq(int irq, void *data)
>> +{
>> +     const struct gsc_input_button_priv *button_priv = data;
>> +     struct input_dev *input = button_priv->input;
>> +
>> +     dev_dbg(&input->dev, "irq%d code=%d\n", irq, button_priv->button->code);
>> +     input_report_key(input, button_priv->button->code, 1);
>> +     input_sync(input);
>> +     input_report_key(input, button_priv->button->code, 0);
>> +     input_sync(input);
>> +
>> +     return IRQ_HANDLED;
>
> Hmm, so in the end this is just a bunch of buttons connected to
> interrupt lines. We already have a driver for that, called gpio-keys. It
> can work in pure interrupt mode (i.e. do not need real GPIO pin, just
> interrupt, and it will generate key down and up events when interrupt
> arrives, with possible delay for the up event).
>

Dmitry,

Yes that's what it does and yes your correct the gpio-keys driver will
work perfect for that. Thanks for calling me out on that - I had not
realized the gpio-keys driver could work from 'interrupts'.

I'll remove the input driver in my next submission. At least I know
how to properly write an input driver now from your previous review :)

Thanks,

Tim

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 4/4] input: misc: Add Gateworks System Controller support
@ 2018-03-14 17:13       ` Tim Harvey
  0 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-14 17:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Mar 10, 2018 at 10:45 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Mar 05, 2018 at 02:02:41PM -0800, Tim Harvey wrote:
>> Add support for dispatching Linux Input events for the various interrupts
>> the Gateworks System Controller provides.
>>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>> ---
>> v2:
>> - reword Kconfig
>> - revise license comment block
>> - remove unnecessary read of status register
>> - remove unnecessary setting of input->dev.parent
>> - use dt bindings for irq to keycode mapping
>> - add support for platform-data
>> - remove work-queue
>>
>>  drivers/input/misc/Kconfig              |   9 ++
>>  drivers/input/misc/Makefile             |   1 +
>>  drivers/input/misc/gsc-input.c          | 180 ++++++++++++++++++++++++++++++++
>>  include/linux/platform_data/gsc_input.h |  30 ++++++
>>  4 files changed, 220 insertions(+)
>>  create mode 100644 drivers/input/misc/gsc-input.c
>>  create mode 100644 include/linux/platform_data/gsc_input.h
>>
>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>> index 9f082a3..e05f4fe 100644
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -117,6 +117,15 @@ config INPUT_E3X0_BUTTON
>>         To compile this driver as a module, choose M here: the
>>         module will be called e3x0_button.
>>
>> +config INPUT_GSC
>> +     tristate "Gateworks System Controller input support"
>> +     depends on MFD_GSC
>> +     help
>> +       Support GSC events as Linux input events.
>> +
>> +       To compile this driver as a module, choose M here: the
>> +       module will be called gsc_input.
>> +
>>  config INPUT_PCSPKR
>>       tristate "PC Speaker support"
>>       depends on PCSPKR_PLATFORM
>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>> index 4b6118d..969debe 100644
>> --- a/drivers/input/misc/Makefile
>> +++ b/drivers/input/misc/Makefile
>> @@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GP2A)            += gp2ap002a00f.o
>>  obj-$(CONFIG_INPUT_GPIO_BEEPER)              += gpio-beeper.o
>>  obj-$(CONFIG_INPUT_GPIO_TILT_POLLED) += gpio_tilt_polled.o
>>  obj-$(CONFIG_INPUT_GPIO_DECODER)     += gpio_decoder.o
>> +obj-$(CONFIG_INPUT_GSC)                      += gsc-input.o
>>  obj-$(CONFIG_INPUT_HISI_POWERKEY)    += hisi_powerkey.o
>>  obj-$(CONFIG_HP_SDC_RTC)             += hp_sdc_rtc.o
>>  obj-$(CONFIG_INPUT_IMS_PCU)          += ims-pcu.o
>> diff --git a/drivers/input/misc/gsc-input.c b/drivers/input/misc/gsc-input.c
>> new file mode 100644
>> index 0000000..27b5e93
>> --- /dev/null
>> +++ b/drivers/input/misc/gsc-input.c
>> @@ -0,0 +1,180 @@
>> +/* SPDX-License-Identifier: GPL-2.0
>> + *
>> + * Copyright (C) 2018 Gateworks Corporation
>> + *
>> + * This driver dispatches Linux input events for GSC interrupt events
>> + */
>> +#include <linux/input.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <linux/mfd/gsc.h>
>> +#include <linux/platform_data/gsc_input.h>
>> +
>> +struct gsc_input_button_priv {
>> +     struct input_dev *input;
>> +     const struct gsc_input_button *button;
>> +};
>> +
>> +struct gsc_input_data {
>> +     struct gsc_dev *gsc;
>> +     struct gsc_input_platform_data *pdata;
>> +     struct input_dev *input;
>> +     struct gsc_input_button_priv *buttons;
>> +     int nbuttons;
>> +     unsigned int irqs[];
>> +#if 0
>> +     int irq;
>> +     struct work_struct irq_work;
>> +     struct mutex mutex;
>> +#endif
>> +};
>> +
>> +static irqreturn_t gsc_input_irq(int irq, void *data)
>> +{
>> +     const struct gsc_input_button_priv *button_priv = data;
>> +     struct input_dev *input = button_priv->input;
>> +
>> +     dev_dbg(&input->dev, "irq%d code=%d\n", irq, button_priv->button->code);
>> +     input_report_key(input, button_priv->button->code, 1);
>> +     input_sync(input);
>> +     input_report_key(input, button_priv->button->code, 0);
>> +     input_sync(input);
>> +
>> +     return IRQ_HANDLED;
>
> Hmm, so in the end this is just a bunch of buttons connected to
> interrupt lines. We already have a driver for that, called gpio-keys. It
> can work in pure interrupt mode (i.e. do not need real GPIO pin, just
> interrupt, and it will generate key down and up events when interrupt
> arrives, with possible delay for the up event).
>

Dmitry,

Yes that's what it does and yes your correct the gpio-keys driver will
work perfect for that. Thanks for calling me out on that - I had not
realized the gpio-keys driver could work from 'interrupts'.

I'll remove the input driver in my next submission. At least I know
how to properly write an input driver now from your previous review :)

Thanks,

Tim

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings
  2018-03-09 23:14     ` Rob Herring
@ 2018-03-23 17:11       ` Tim Harvey
  -1 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-23 17:11 UTC (permalink / raw)
  To: Rob Herring
  Cc: Lee Jones, Mark Rutland, Mark Brown, Dmitry Torokhov,
	linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
	linux-input

On Fri, Mar 9, 2018 at 3:14 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Mar 05, 2018 at 02:02:38PM -0800, Tim Harvey wrote:
>> This patch adds documentation of device-tree bindings for the
>> Gateworks System Controller (GSC).
>>
>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/gsc.txt | 159 ++++++++++++++++++++++++++
>>  1 file changed, 159 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mfd/gsc.txt
>>
<snip>
>> +
>> +* hwmon:
>> +The GSC provides a set of Analog to Digitcal Converter (ADC) pins used for
>> +temperature and/or voltage monitoring.
>> +
>> +Required properties:
>> +- compatible: must be "gw,gsc-hwmon"
>> +
<snip>
>> +
>> +             gsc_hwmon {
>
> hwmon {
>
>> +                     compatible = "gw,gsc-hwmon";
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +
>> +                     hwmon@0 { /* A0: Board Temperature */
>> +                             type = <0>;
>
> Not documented.
>
>> +                             reg = <0x00>;
>> +                             label = "temp";
>> +                     };
>> +
>> +                     hwmon@1 { /* A1: Input Voltage */
>> +                             type = <1>;
>> +                             reg = <0x02>;
>> +                             label = "Vin";
>> +                     };
>> +
<snip>
>> +
>> +                     hwmon@15 { /* fan0 */
>> +                             type = <2>;
>> +                             reg = <0x2c>;
>> +                             label = "fan_50p";
>> +                     };
>> +
<snip>
>> +             };
>> +     };

Hi Rob,

Thanks for the review. I will roll in your changes.

I'm looking for suggestions on how to better define the child nodes of
the hwmon sub-device:

I have 4 types of hwmon child nodes supported by this device:
1. temperature in C/10 (is that called deci-celcuis?): this is a 2byte value
2. input voltage in mV: this is a 3 byte value pre-scaled by the GSC
firmware from a 10bit ADC where the
ref/bit-resolution/resistor-divider info is baked into the per-board
firmware
3. fan setpoint in C/10
4. input voltage in mV: raw 2-byte value of a 12bit ADC that needs to
be scaled based on a voltage-devider (pre-scaler), a ref voltage, and
resolution (or bit width)

The example I posted above shows use of the first three types but not
the more complicated fourth which requires the driver know more
details in order to present a cooked milivolt value. Note that the two
input types above would not both be present in any single board dts as
they represent a change in the way ADC's are reported in the overall
GSC version. A GSC typically has up to 16 different input ADC's and
the voltage rails monitored vary per board.

You mention that I don't document 'type' and your correct. I'm
thinking this is best done with a string using compatible within the
child node.

Something like:

Required properties:
...
- hwmon: This is the list of child nodes that specify the hwmon nodes.
...

hwmon required properties:
- hwmon-compatible: Must be "fan-setpoint", "temperature",
"input-raw", "input-cooked"
- reg: register offset of the node
- label: name of the input node

hwmon optional properties:
- gw,voltage-divider: an array of two integers containing the resistor
values R1 and R2 of the voltage divider in ohms
- gw,reference-voltage: the internal reference voltage of the ADC
input in milivolts
- gw,resolution: the resolution of the ADC input (ie 4096 for 12bit resolution)
...

Example:
...
hwmon {
  compatible = "gw,gsc-hwmon";
  #address-cells = <1>;
  #size-cells = <0>;

  hwmon@0 {
    compatible = "temperature";
    reg <0x00>;
    label = "temp";
  };

  hwmon@2 {
    compatible = "fan-setpoint";
    reg <0x02>;
    label = "fan_setpoint0";
  };

  /* this one represents the 3-byte ADC value that has been
'pre-scaled' by the GSC from a raw 10bit ADC (older GSC
version/firmware) */
  hwmon@4 {
    compatible = "input-cooked";
    reg <0x02>;
    label = "voltage1";
  };

  /* this one represents the 2-byte 12-bit ADC value that is 'raw'
from the GSC and needs to be scaled by driver (newer GSC
version/firmware) */
  hwmon@4 {
    compatible = "input-raw";
    reg <0x04>;
    label = "voltage2";
    gw,voltage-divider = <10000 10000>; /* R1=10k R2=10k div-by-2 pre-scaler */
    gw,referance-voltage = <2500>; /* 2.5V reference */
    gw,resolution = <4096>; /* 12bit resolution */
  };
};

If I add a byte-size (2|3) or bit-width (16|24) I could essentially
combine the two input types.

Suggestions?

Regards,

Tim

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings
@ 2018-03-23 17:11       ` Tim Harvey
  0 siblings, 0 replies; 24+ messages in thread
From: Tim Harvey @ 2018-03-23 17:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Mar 9, 2018 at 3:14 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Mar 05, 2018 at 02:02:38PM -0800, Tim Harvey wrote:
>> This patch adds documentation of device-tree bindings for the
>> Gateworks System Controller (GSC).
>>
>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/gsc.txt | 159 ++++++++++++++++++++++++++
>>  1 file changed, 159 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mfd/gsc.txt
>>
<snip>
>> +
>> +* hwmon:
>> +The GSC provides a set of Analog to Digitcal Converter (ADC) pins used for
>> +temperature and/or voltage monitoring.
>> +
>> +Required properties:
>> +- compatible: must be "gw,gsc-hwmon"
>> +
<snip>
>> +
>> +             gsc_hwmon {
>
> hwmon {
>
>> +                     compatible = "gw,gsc-hwmon";
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +
>> +                     hwmon at 0 { /* A0: Board Temperature */
>> +                             type = <0>;
>
> Not documented.
>
>> +                             reg = <0x00>;
>> +                             label = "temp";
>> +                     };
>> +
>> +                     hwmon at 1 { /* A1: Input Voltage */
>> +                             type = <1>;
>> +                             reg = <0x02>;
>> +                             label = "Vin";
>> +                     };
>> +
<snip>
>> +
>> +                     hwmon at 15 { /* fan0 */
>> +                             type = <2>;
>> +                             reg = <0x2c>;
>> +                             label = "fan_50p";
>> +                     };
>> +
<snip>
>> +             };
>> +     };

Hi Rob,

Thanks for the review. I will roll in your changes.

I'm looking for suggestions on how to better define the child nodes of
the hwmon sub-device:

I have 4 types of hwmon child nodes supported by this device:
1. temperature in C/10 (is that called deci-celcuis?): this is a 2byte value
2. input voltage in mV: this is a 3 byte value pre-scaled by the GSC
firmware from a 10bit ADC where the
ref/bit-resolution/resistor-divider info is baked into the per-board
firmware
3. fan setpoint in C/10
4. input voltage in mV: raw 2-byte value of a 12bit ADC that needs to
be scaled based on a voltage-devider (pre-scaler), a ref voltage, and
resolution (or bit width)

The example I posted above shows use of the first three types but not
the more complicated fourth which requires the driver know more
details in order to present a cooked milivolt value. Note that the two
input types above would not both be present in any single board dts as
they represent a change in the way ADC's are reported in the overall
GSC version. A GSC typically has up to 16 different input ADC's and
the voltage rails monitored vary per board.

You mention that I don't document 'type' and your correct. I'm
thinking this is best done with a string using compatible within the
child node.

Something like:

Required properties:
...
- hwmon: This is the list of child nodes that specify the hwmon nodes.
...

hwmon required properties:
- hwmon-compatible: Must be "fan-setpoint", "temperature",
"input-raw", "input-cooked"
- reg: register offset of the node
- label: name of the input node

hwmon optional properties:
- gw,voltage-divider: an array of two integers containing the resistor
values R1 and R2 of the voltage divider in ohms
- gw,reference-voltage: the internal reference voltage of the ADC
input in milivolts
- gw,resolution: the resolution of the ADC input (ie 4096 for 12bit resolution)
...

Example:
...
hwmon {
  compatible = "gw,gsc-hwmon";
  #address-cells = <1>;
  #size-cells = <0>;

  hwmon at 0 {
    compatible = "temperature";
    reg <0x00>;
    label = "temp";
  };

  hwmon at 2 {
    compatible = "fan-setpoint";
    reg <0x02>;
    label = "fan_setpoint0";
  };

  /* this one represents the 3-byte ADC value that has been
'pre-scaled' by the GSC from a raw 10bit ADC (older GSC
version/firmware) */
  hwmon at 4 {
    compatible = "input-cooked";
    reg <0x02>;
    label = "voltage1";
  };

  /* this one represents the 2-byte 12-bit ADC value that is 'raw'
from the GSC and needs to be scaled by driver (newer GSC
version/firmware) */
  hwmon at 4 {
    compatible = "input-raw";
    reg <0x04>;
    label = "voltage2";
    gw,voltage-divider = <10000 10000>; /* R1=10k R2=10k div-by-2 pre-scaler */
    gw,referance-voltage = <2500>; /* 2.5V reference */
    gw,resolution = <4096>; /* 12bit resolution */
  };
};

If I add a byte-size (2|3) or bit-width (16|24) I could essentially
combine the two input types.

Suggestions?

Regards,

Tim

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2018-03-23 17:11 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-05 22:02 [PATCH v2 0/4] Add support for the Gateworks System Controller Tim Harvey
2018-03-05 22:02 ` Tim Harvey
2018-03-05 22:02 ` [PATCH v2 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings Tim Harvey
2018-03-05 22:02   ` Tim Harvey
2018-03-09 23:14   ` Rob Herring
2018-03-09 23:14     ` Rob Herring
2018-03-23 17:11     ` Tim Harvey
2018-03-23 17:11       ` Tim Harvey
2018-03-10 13:57   ` Fabio Estevam
2018-03-10 13:57     ` Fabio Estevam
2018-03-05 22:02 ` [PATCH v2 2/4] mfd: add Gateworks System Controller core driver Tim Harvey
2018-03-05 22:02   ` Tim Harvey
2018-03-12 14:41   ` Lee Jones
2018-03-12 14:41     ` Lee Jones
2018-03-05 22:02 ` [PATCH v2 3/4] hwmon: add Gateworks System Controller support Tim Harvey
2018-03-05 22:02   ` Tim Harvey
2018-03-05 23:12   ` Guenter Roeck
2018-03-05 23:12     ` Guenter Roeck
2018-03-05 22:02 ` [PATCH v2 4/4] input: misc: Add " Tim Harvey
2018-03-05 22:02   ` Tim Harvey
2018-03-10 18:45   ` Dmitry Torokhov
2018-03-10 18:45     ` Dmitry Torokhov
2018-03-14 17:13     ` Tim Harvey
2018-03-14 17:13       ` Tim Harvey

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.