All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/5] add support for DMO embedded controller
@ 2016-10-27 10:47 Zahari Doychev
  2016-10-27 10:47 ` [RFC PATCH 1/5] dmec: add DMO mfd driver Zahari Doychev
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Zahari Doychev @ 2016-10-27 10:47 UTC (permalink / raw)
  To: linux-kernel, gregkh, lee.jones, wsa, linus.walleij, wim, linux
  Cc: linux-i2c, linux-gpio, gnurou, linux-watchdog, Zahari Doychev

This patch series adds support for the Data Modul Embedded Controller(dmec)
which is implemented within an on board FPGA found on Data Modul embedded
CPU modules.

The dmec is connected to the host through the LPC bus and its registers are
mapped into the host I/O space. The controller supports two addressing modes:
linear and indexed.

The driver adds support for the following functionality:

- i2c
- gpio
- watchdog
- running time meter (rtm)

Zahari Doychev (5):
  dmec: add DMO mfd driver
  i2c-dmec: add i2c bus support for dmec
  gpio-dmec: gpio support for dmec
  wdt-dmec: watchdog support for dmec
  rtm-dmec: running time meter support

 drivers/staging/Kconfig          |   2 +-
 drivers/staging/Makefile         |   1 +-
 drivers/staging/dmec/Kconfig     |  50 +++-
 drivers/staging/dmec/Makefile    |   5 +-
 drivers/staging/dmec/dmec-core.c | 500 ++++++++++++++++++++++++++++-
 drivers/staging/dmec/dmec.h      |  20 +-
 drivers/staging/dmec/gpio-dmec.c | 390 ++++++++++++++++++++++-
 drivers/staging/dmec/i2c-dmec.c  | 524 +++++++++++++++++++++++++++++-
 drivers/staging/dmec/rtm-dmec.c  | 203 +++++++++++-
 drivers/staging/dmec/wdt-dmec.c  | 569 ++++++++++++++++++++++++++++++++-
 10 files changed, 2264 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/dmec/Kconfig
 create mode 100644 drivers/staging/dmec/Makefile
 create mode 100644 drivers/staging/dmec/dmec-core.c
 create mode 100644 drivers/staging/dmec/dmec.h
 create mode 100644 drivers/staging/dmec/gpio-dmec.c
 create mode 100644 drivers/staging/dmec/i2c-dmec.c
 create mode 100644 drivers/staging/dmec/rtm-dmec.c
 create mode 100644 drivers/staging/dmec/wdt-dmec.c

base-commit: 9fe68cad6e74967b88d0c6aeca7d9cd6b6e91942
-- 
git-series 0.8.10

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

* [RFC PATCH 1/5] dmec: add DMO mfd driver
  2016-10-27 10:47 [RFC PATCH 0/5] add support for DMO embedded controller Zahari Doychev
@ 2016-10-27 10:47 ` Zahari Doychev
  2016-10-27 10:47 ` [RFC PATCH 2/5] i2c-dmec: add i2c bus support for dmec Zahari Doychev
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Zahari Doychev @ 2016-10-27 10:47 UTC (permalink / raw)
  To: linux-kernel, gregkh, lee.jones, wsa, linus.walleij, wim, linux
  Cc: linux-i2c, linux-gpio, gnurou, linux-watchdog, Zahari Doychev

This is a core mfd driver for the on board embedded controllers found on the
Data Modul embedded CPU modules. The embedded controller may provide the
following functions: i2c bus, gpio, watchdog, uart and rtm.

Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
---
 drivers/staging/Kconfig          |   2 +-
 drivers/staging/Makefile         |   1 +-
 drivers/staging/dmec/Kconfig     |   9 +-
 drivers/staging/dmec/Makefile    |   1 +-
 drivers/staging/dmec/dmec-core.c | 500 ++++++++++++++++++++++++++++++++-
 drivers/staging/dmec/dmec.h      |   6 +-
 6 files changed, 519 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/dmec/Kconfig
 create mode 100644 drivers/staging/dmec/Makefile
 create mode 100644 drivers/staging/dmec/dmec-core.c
 create mode 100644 drivers/staging/dmec/dmec.h

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 58a7b35..d95bee2 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -106,4 +106,6 @@ source "drivers/staging/greybus/Kconfig"
 
 source "drivers/staging/vc04_services/Kconfig"
 
+source "drivers/staging/dmec/Kconfig"
+
 endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 2fa9745..73b4833 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -42,3 +42,4 @@ obj-$(CONFIG_ISDN_I4L)		+= i4l/
 obj-$(CONFIG_KS7010)		+= ks7010/
 obj-$(CONFIG_GREYBUS)		+= greybus/
 obj-$(CONFIG_BCM2708_VCHIQ)	+= vc04_services/
+obj-$(CONFIG_MFD_DMEC)		+= dmec/
diff --git a/drivers/staging/dmec/Kconfig b/drivers/staging/dmec/Kconfig
new file mode 100644
index 0000000..3641907
--- /dev/null
+++ b/drivers/staging/dmec/Kconfig
@@ -0,0 +1,9 @@
+config MFD_DMEC
+	tristate "Data Modul Embedded Controller MFD"
+	select MFD_CORE
+	help
+	    Say Y here to enable support for a Data Module embedded controller
+	    found on Data Module CPU Modules
+
+	    To compile this driver as a module, say M here: the module will be
+            called dmec
diff --git a/drivers/staging/dmec/Makefile b/drivers/staging/dmec/Makefile
new file mode 100644
index 0000000..859163b
--- /dev/null
+++ b/drivers/staging/dmec/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_MFD_DMEC)		+= dmec-core.o
diff --git a/drivers/staging/dmec/dmec-core.c b/drivers/staging/dmec/dmec-core.c
new file mode 100644
index 0000000..40f5481
--- /dev/null
+++ b/drivers/staging/dmec/dmec-core.c
@@ -0,0 +1,500 @@
+/*
+ * dmec-core: Data Modul AG mfd embedded controller driver
+ *
+ * Zahari Doychev  <zahari.doychev@linux.com>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+#include <linux/platform_device.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/dmi.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/regmap.h>
+#include <linux/acpi.h>
+#include <linux/nls.h>
+#include "dmec.h"
+
+#define DMEC_LBAR			0x00 /* Index addressing 4 bytes */
+#define DMEC_ECVER0			0x04
+#define DMEC_ECVER1			0x05
+#define DMEC_ECFTR0			0x07
+#define DMEC_ECFTR1			0x08
+#define DMEC_FPGAVER0			0x0c
+#define DMEC_FPGAVER1			0x0d
+#define DMEC_FPGABLD0			0x0e
+#define DMEC_FPGABLD1			0x0f
+#define DMEC_IRQCFG0			0x10
+#define DMEC_IRQCFG1			0x11
+#define DMEC_RTM_START			0x60
+#define DMEC_RTM_END			0x6e
+
+#define DMEC_MAX_GPIO_CHIPS		2
+
+#define DMEC_VERSION_LEN		32
+
+#define DMEC_FEATURE_BIT_I2C		BIT(0)
+#define DMEC_FEATURE_BIT_WDT		BIT(4)
+#define DMEC_FEATURE_BIT_GPIO		(3 << 6)
+
+#define DMEC_REG_MAX		0x7f
+#define DMEC_MAX_DEVS		ARRAY_SIZE(dmec_devs)
+#define DMEC_MAX_IO_RES		2
+#define DMEC_STR_SZ		128
+
+static bool i_addr;
+module_param(i_addr, bool, 0644);
+MODULE_PARM_DESC(i_addr, "Enable register index addressing usage");
+
+static const char * const fw_types[] = {"release", "custom",
+					"debug", "reserved"};
+
+enum dmec_cells {
+	DMEC_I2C = 0,
+	DMEC_GPIOA,
+	DMEC_GPIOB,
+	DMEC_WDT,
+	DMEC_RTM
+};
+
+struct dmec_features {
+	unsigned int i2c_buses:2;
+	unsigned int uarts:2;
+	unsigned int wdt:1;
+	unsigned int rsvd1:1;
+	unsigned int gpio_chips:2;
+	unsigned int spi_buses:2;
+	unsigned int can_buses:2;
+	unsigned int rsvd2:1;
+	unsigned int nmi:1;
+	unsigned int sci:1;
+	unsigned int smi:1;
+};
+
+struct dmec_info {
+	unsigned int ec_ver:4;
+	unsigned int ec_rev:4;
+	unsigned int ec_num:2;
+	unsigned int ec_type:2;
+	unsigned int ec_dbg:4;
+	u16 fpga_ver;
+	u16 fpga_bld;
+	char version[DMEC_VERSION_LEN];
+};
+
+struct dmec_device_data {
+	void __iomem *io_base;
+	void __iomem *io_index;
+	void __iomem *io_data;
+	union {
+		u16 feature_mask;
+		struct dmec_features ftr;
+	} u;
+	struct device *dev;
+	struct dmec_info info;
+	struct regmap *regmap;
+	/* use index addressing for register access if set*/
+	bool i_addr;
+};
+
+struct dmec_platform_data {
+	int (*get_info)(struct dmec_device_data *);
+	int (*register_cells)(struct dmec_device_data *);
+};
+
+static struct dmec_i2c_platform_data dmec_i2c_data = {
+	.reg_shift	= 0,		/* two bytes between registers */
+	.reg_io_width	= 1,		/* register io read/write width */
+	.clock_khz	= 50000,	/* input clock of 50MHz */
+};
+
+static struct dmec_gpio_platform_data dmec_gpio_pdata[DMEC_MAX_GPIO_CHIPS] = {
+	{
+		.gpio_base = -1,
+		.chip_num = 0,
+	},
+	{
+		.gpio_base = -1,
+		.chip_num = 1,
+	},
+};
+
+/* The gpio block can use up to DMEC_GPIO_MAX_IRQS APIC irqs */
+static struct resource dmec_gpio_irq_resources[DMEC_MAX_GPIO_CHIPS];
+static struct resource dmec_wdt_irq_resource;
+static struct resource dmec_i2c_irq_resource;
+
+static struct mfd_cell dmec_devs[] = {
+	[DMEC_I2C] = {
+		.name = "dmec-i2c",
+		.platform_data = &dmec_i2c_data,
+		.pdata_size = sizeof(dmec_i2c_data),
+		.resources = &dmec_i2c_irq_resource,
+		.num_resources = 1,
+	},
+	[DMEC_GPIOA] = {
+		.name = "dmec-gpio",
+		.platform_data = &dmec_gpio_pdata[0],
+		.pdata_size = sizeof(struct dmec_gpio_platform_data),
+		.resources = &dmec_gpio_irq_resources[0],
+		.num_resources = 1,
+	},
+	[DMEC_GPIOB] = {
+		.name = "dmec-gpio",
+		.platform_data = &dmec_gpio_pdata[1],
+		.pdata_size = sizeof(struct dmec_gpio_platform_data),
+		.resources = &dmec_gpio_irq_resources[1],
+		.num_resources = 1,
+	},
+	[DMEC_WDT] = {
+		.name = "dmec-wdt",
+		.resources = &dmec_wdt_irq_resource,
+		.num_resources = 1,
+	},
+	[DMEC_RTM] = {
+		.name = "dmec-rtm",
+	},
+};
+
+static void dmec_get_gpio_irqs(struct dmec_device_data *ec)
+{
+	unsigned int irq, val;
+
+	regmap_read(ec->regmap, DMEC_IRQCFG1, &val);
+	irq = (val >> 4) & 0xf;
+	dmec_gpio_irq_resources[0].start = irq;
+	dmec_gpio_irq_resources[0].end = irq;
+	dmec_gpio_irq_resources[0].flags = IORESOURCE_IRQ;
+	irq = val & 0xf;
+	dmec_gpio_irq_resources[1].start = irq;
+	dmec_gpio_irq_resources[1].end = irq;
+	dmec_gpio_irq_resources[1].flags = IORESOURCE_IRQ;
+}
+
+static void dmec_get_wdt_irq(struct dmec_device_data *ec)
+{
+	unsigned int irq, val;
+
+	regmap_read(ec->regmap, DMEC_IRQCFG0, &val);
+	irq = val & 0xf;
+	dmec_wdt_irq_resource.start = irq;
+	dmec_wdt_irq_resource.end = irq;
+	dmec_wdt_irq_resource.flags = IORESOURCE_IRQ;
+}
+
+static void dmec_get_i2c_irq(struct dmec_device_data *ec)
+{
+	unsigned int irq, val;
+
+	regmap_read(ec->regmap, DMEC_IRQCFG0, &val);
+	irq = (val >> 4) & 0xf;
+	dmec_i2c_irq_resource.start = irq;
+	dmec_i2c_irq_resource.end = irq;
+	dmec_i2c_irq_resource.flags = IORESOURCE_IRQ;
+}
+
+static int dmec_rtm_detect(struct dmec_device_data *ec)
+{
+	unsigned int val, n;
+
+	for (n = DMEC_RTM_START; n <= DMEC_RTM_END; n++) {
+		regmap_read(ec->regmap, n, &val);
+		if (val != 0xff)
+			return 1;
+	}
+
+	return 0;
+}
+
+static int dmec_register_cells(struct dmec_device_data *ec)
+{
+	struct mfd_cell cells[DMEC_MAX_DEVS];
+	u8 n_dev = 0;
+
+	if (ec->u.feature_mask & DMEC_FEATURE_BIT_I2C) {
+		dmec_get_i2c_irq(ec);
+		dmec_devs[DMEC_I2C].id = n_dev;
+		cells[n_dev++] = dmec_devs[DMEC_I2C];
+	}
+
+	if (ec->u.feature_mask & DMEC_FEATURE_BIT_GPIO) {
+		dmec_get_gpio_irqs(ec);
+		dmec_devs[DMEC_GPIOA].id = n_dev;
+		cells[n_dev++] = dmec_devs[DMEC_GPIOA];
+		if (ec->u.ftr.gpio_chips > 1) {
+			dmec_devs[DMEC_GPIOB].id = n_dev;
+			cells[n_dev++] = dmec_devs[DMEC_GPIOB];
+		}
+	}
+
+	if (ec->u.feature_mask & DMEC_FEATURE_BIT_WDT) {
+		dmec_get_wdt_irq(ec);
+		dmec_devs[DMEC_WDT].id = n_dev;
+		cells[n_dev++] = dmec_devs[DMEC_WDT];
+	}
+
+	if (dmec_rtm_detect(ec)) {
+		dmec_devs[DMEC_RTM].id = n_dev;
+		cells[n_dev++] = dmec_devs[DMEC_RTM];
+	}
+
+	return devm_mfd_add_devices(ec->dev, 0,
+				    cells, n_dev, NULL, 0, NULL);
+}
+
+static int dmec_read16(struct dmec_device_data *ec, u8 reg)
+{
+	unsigned int lsb, msb;
+	int ret;
+
+	ret = regmap_read(ec->regmap, reg, &lsb);
+	ret = regmap_read(ec->regmap, reg + 0x1, &msb);
+
+	return (msb << 8) | lsb;
+}
+
+static int dmec_get_info(struct dmec_device_data *ec)
+{
+	unsigned int ver0, ver1;
+
+	regmap_read(ec->regmap, DMEC_ECVER0, &ver0);
+	regmap_read(ec->regmap, DMEC_ECVER1, &ver1);
+	if (ver0 == 0xff && ver1 == 0xff)
+		return -ENODEV;
+
+	ec->u.feature_mask = dmec_read16(ec, DMEC_ECFTR0);
+
+	ec->info.ec_ver = (ver0 >> 4) & 0xf;
+	ec->info.ec_rev = ver0 & 0xf;
+	ec->info.ec_num = ver1 & 0x3;
+	ec->info.ec_type = (ver1 >> 2) & 0x3;
+	ec->info.ec_dbg = (ver1 >> 4) & 0xf;
+
+	ec->info.fpga_ver = dmec_read16(ec, DMEC_FPGAVER0);
+	ec->info.fpga_bld = dmec_read16(ec, DMEC_FPGABLD0);
+
+	return 0;
+}
+
+static int dmec_regmap_reg_read(void *context,
+				unsigned int reg, unsigned int *val)
+{
+	struct dmec_device_data *ec = context;
+
+	if (ec->i_addr) {
+		iowrite8(reg, ec->io_index);
+		*val = ioread8(ec->io_data);
+	} else {
+		*val = ioread8(ec->io_base + reg);
+	}
+
+	return 0;
+}
+
+static int dmec_regmap_reg_write(void *context,
+				 unsigned int reg, unsigned int val)
+{
+	struct dmec_device_data *ec = context;
+
+	if (ec->i_addr) {
+		iowrite8(reg, ec->io_index);
+		iowrite8(val, ec->io_data);
+	} else {
+		iowrite8(val, ec->io_base + reg);
+	}
+
+	return 0;
+}
+
+struct regmap *dmec_get_regmap(struct device *dev)
+{
+	struct dmec_device_data *ec = dev_get_drvdata(dev);
+
+	return ec->regmap;
+}
+EXPORT_SYMBOL(dmec_get_regmap);
+
+static ssize_t dmec_version_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct dmec_device_data *ec = dev_get_drvdata(dev);
+
+	return scnprintf(buf, PAGE_SIZE, "%s\n", ec->info.version);
+}
+
+static ssize_t dmec_fw_type_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct dmec_device_data *ec = dev_get_drvdata(dev);
+
+	return scnprintf(buf, PAGE_SIZE,
+			"ver:        %u.%u\n"
+			"ec:         %u\n"
+			"type:       %s\n"
+			"debug:      %u\n"
+			"fpga ver:   %x\n"
+			"fpga build: %x\n",
+			ec->info.ec_ver,
+			ec->info.ec_rev,
+			ec->info.ec_num,
+			fw_types[ec->info.ec_type % (ARRAY_SIZE(fw_types) - 1)],
+			ec->info.ec_dbg,
+			ec->info.fpga_ver,
+			ec->info.fpga_bld);
+}
+
+static ssize_t dmec_features_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	struct dmec_device_data *ec = dev_get_drvdata(dev);
+	struct dmec_features *ftr = &ec->u.ftr;
+
+	return scnprintf(buf, PAGE_SIZE,
+			 "i2c buses:    %2u\n"
+			 "uarts:        %2u\n"
+			 "watchdog:     %2u\n"
+			 "gpio chips:   %2u\n"
+			 "spi buses:    %2u\n"
+			 "can buses:    %2u\n"
+			 "nmi:          %2u\n"
+			 "sci:          %2u\n"
+			 "smi:          %2u\n",
+			 ftr->i2c_buses, ftr->uarts, ftr->wdt,
+			 ftr->gpio_chips, ftr->spi_buses, ftr->can_buses,
+			 ftr->nmi, ftr->sci, ftr->smi);
+}
+
+static DEVICE_ATTR(version, 0444, dmec_version_show, NULL);
+static DEVICE_ATTR(fw_type, 0444, dmec_fw_type_show, NULL);
+static DEVICE_ATTR(features, 0444, dmec_features_show, NULL);
+
+static struct attribute *dmec_attributes[] = {
+	&dev_attr_version.attr,
+	&dev_attr_fw_type.attr,
+	&dev_attr_features.attr,
+	NULL
+};
+
+static const struct attribute_group ec_attr_group = {
+	.attrs = dmec_attributes,
+};
+
+static int dmec_detect_device(struct dmec_device_data *ec)
+{
+	int ret;
+
+	ret = dmec_get_info(ec);
+	if (ret)
+		return ret;
+
+	ret = scnprintf(ec->info.version, sizeof(ec->info.version),
+			"%u.%u",
+			ec->info.ec_ver, ec->info.ec_rev);
+	if (ret < 0)
+		return ret;
+
+	dev_info(ec->dev, "v%s (%s) features: %#x\n",
+		 ec->info.version,
+		 fw_types[ec->info.ec_type % (ARRAY_SIZE(fw_types) - 1)],
+		 ec->u.feature_mask);
+
+	ret = dmec_register_cells(ec);
+	if (ret)
+		return ret;
+
+	return sysfs_create_group(&ec->dev->kobj, &ec_attr_group);
+}
+
+static const struct regmap_config dmec_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.reg_stride = 1,
+	.max_register =	DMEC_REG_MAX,
+	.reg_write = dmec_regmap_reg_write,
+	.reg_read = dmec_regmap_reg_read,
+	.cache_type = REGCACHE_NONE,
+	.fast_io = true,
+};
+
+static int dmec_mfd_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct dmec_device_data *ec;
+	struct resource *io_idx, *io;
+	int ret;
+
+	ret = platform_device_add_data(pdev, NULL, 0);
+	if (ret)
+		return ret;
+
+	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
+	if (!ec)
+		return -ENOMEM;
+
+	io_idx = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	if (!io_idx)
+		return -EINVAL;
+
+	io = platform_get_resource(pdev, IORESOURCE_IO, 1);
+	if (!io) {
+		dev_info(dev, "falling back to index addressing\n");
+		ec->io_base = devm_ioport_map(dev, io_idx->start,
+					      io_idx->end - io_idx->start);
+		i_addr = true;
+	} else
+		ec->io_base = devm_ioport_map(dev, io->start,
+					      io->end - io->start);
+
+	if (IS_ERR(ec->io_base))
+		return PTR_ERR(ec->io_base);
+
+	/* In index mode registers @ 0x0 and 0x2 are used by the BIOS */
+	ec->i_addr = i_addr;
+	ec->io_index = ec->io_base + 1;
+	ec->io_data = ec->io_base + 3;
+	ec->dev = dev;
+
+	ec->regmap = devm_regmap_init(dev, NULL, ec, &dmec_regmap_config);
+	if (IS_ERR(ec->regmap))
+		return PTR_ERR(ec->regmap);
+	regcache_cache_bypass(ec->regmap, true);
+
+	platform_set_drvdata(pdev, ec);
+
+	return dmec_detect_device(ec);
+}
+
+static int dmec_mfd_remove(struct platform_device *pdev)
+{
+	struct dmec_device_data *ec = platform_get_drvdata(pdev);
+
+	sysfs_remove_group(&ec->dev->kobj, &ec_attr_group);
+
+	return 0;
+}
+
+static const struct acpi_device_id dmec_acpi_ids[] = {
+	{"DMEC0001", 0},
+	{"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, dmec_acpi_ids);
+
+static struct platform_driver dmec_driver = {
+	.probe = dmec_mfd_probe,
+	.remove = dmec_mfd_remove,
+	.driver = {
+		.name = "dmec",
+		.acpi_match_table = ACPI_PTR(dmec_acpi_ids)
+	},
+};
+
+module_platform_driver(dmec_driver);
+
+MODULE_DESCRIPTION("DMO embedded controller core driver");
+MODULE_AUTHOR("Zahari Doychev <zahari.doychev@linux.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:dmec-core");
diff --git a/drivers/staging/dmec/dmec.h b/drivers/staging/dmec/dmec.h
new file mode 100644
index 0000000..4d8712d
--- /dev/null
+++ b/drivers/staging/dmec/dmec.h
@@ -0,0 +1,6 @@
+#ifndef _LINUX_MFD_DMEC_H
+#define _LINUX_MFD_DMEC_H
+
+struct regmap *dmec_get_regmap(struct device *dev);
+
+#endif
-- 
git-series 0.8.10

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

* [RFC PATCH 2/5] i2c-dmec: add i2c bus support for dmec
  2016-10-27 10:47 [RFC PATCH 0/5] add support for DMO embedded controller Zahari Doychev
  2016-10-27 10:47 ` [RFC PATCH 1/5] dmec: add DMO mfd driver Zahari Doychev
@ 2016-10-27 10:47 ` Zahari Doychev
  2016-10-27 10:47 ` [RFC PATCH 3/5] gpio-dmec: gpio " Zahari Doychev
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Zahari Doychev @ 2016-10-27 10:47 UTC (permalink / raw)
  To: linux-kernel, gregkh, lee.jones, wsa, linus.walleij, wim, linux
  Cc: linux-i2c, linux-gpio, gnurou, linux-watchdog, Zahari Doychev

This is support for the i2c bus functionality of the Data Modul embedded
controllers.

Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
---
 drivers/staging/dmec/Kconfig    |  10 +-
 drivers/staging/dmec/Makefile   |   1 +-
 drivers/staging/dmec/dmec.h     |   9 +-
 drivers/staging/dmec/i2c-dmec.c | 524 +++++++++++++++++++++++++++++++++-
 4 files changed, 544 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/dmec/i2c-dmec.c

diff --git a/drivers/staging/dmec/Kconfig b/drivers/staging/dmec/Kconfig
index 3641907..0067b0b 100644
--- a/drivers/staging/dmec/Kconfig
+++ b/drivers/staging/dmec/Kconfig
@@ -7,3 +7,13 @@ config MFD_DMEC
 
 	    To compile this driver as a module, say M here: the module will be
             called dmec
+
+config I2C_DMEC
+	tristate "Data Modul I2C"
+	depends on MFD_DMEC && I2C
+	help
+	  Say Y here to enable support for a i2c bus on the Data Modul
+	  embedded controller.
+
+	  To compile this driver as a module, say M here: the module will be
+          called i2c-dmec
diff --git a/drivers/staging/dmec/Makefile b/drivers/staging/dmec/Makefile
index 859163b..c51a37e 100644
--- a/drivers/staging/dmec/Makefile
+++ b/drivers/staging/dmec/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_MFD_DMEC)		+= dmec-core.o
+obj-$(CONFIG_I2C_DMEC)		+= i2c-dmec.o
diff --git a/drivers/staging/dmec/dmec.h b/drivers/staging/dmec/dmec.h
index 4d8712d..178937d 100644
--- a/drivers/staging/dmec/dmec.h
+++ b/drivers/staging/dmec/dmec.h
@@ -1,6 +1,15 @@
 #ifndef _LINUX_MFD_DMEC_H
 #define _LINUX_MFD_DMEC_H
 
+struct dmec_i2c_platform_data {
+	u32 reg_shift; /* register offset shift value */
+	u32 reg_io_width; /* register io read/write width */
+	u32 clock_khz; /* input clock in kHz */
+	bool big_endian; /* registers are big endian */
+	u8 num_devices; /* number of devices in the devices list */
+	struct i2c_board_info const *devices; /* devices connected to the bus */
+};
+
 struct regmap *dmec_get_regmap(struct device *dev);
 
 #endif
diff --git a/drivers/staging/dmec/i2c-dmec.c b/drivers/staging/dmec/i2c-dmec.c
new file mode 100644
index 0000000..456e61e
--- /dev/null
+++ b/drivers/staging/dmec/i2c-dmec.c
@@ -0,0 +1,524 @@
+/*
+ * i2c-dmec.c: I2C bus driver for OpenCores I2C controller
+ * (http://www.opencores.org/projects.cgi/web/i2c/overview).
+ *
+ * Peter Korsgaard <jacmet@sunsite.dk>
+ *
+ * Support for the GRLIB port of the controller by
+ * Andreas Larsson <andreas@gaisler.com>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/wait.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/log2.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/interrupt.h>
+#include "dmec.h"
+
+/* registers */
+#define DMECI2C_PRELOW		0
+#define DMECI2C_PREHIGH		1
+#define DMECI2C_CONTROL		2
+#define DMECI2C_DATA		3
+#define DMECI2C_CMD		4 /* write only */
+#define DMECI2C_STATUS		4 /* read only, same address as DMECI2C_CMD */
+#define DMECI2C_VER		5
+#define DMECI2C_MUX		6 /* i2c demultiplezer */
+
+#define DMECI2C_CTRL_EN		BIT(7)
+#define DMECI2C_CTRL_IEN	BIT(6)
+#define DMECI2C_CTRL_VSSCL	BIT(5)
+#define DMECI2C_CTRL_MABCLR	BIT(4)
+#define DMECI2C_CTRL_MMDIS	BIT(3)
+#define DMECI2C_CTRL_VSSDA	BIT(2)
+
+#define DMECI2C_CMD_START	0x91
+#define DMECI2C_CMD_STOP	0x41
+#define DMECI2C_CMD_READ	0x21
+#define DMECI2C_CMD_WRITE	0x11
+#define DMECI2C_CMD_READ_ACK	0x21
+#define DMECI2C_CMD_READ_NACK	0x29
+#define DMECI2C_CMD_IACK	0x01
+
+#define DMECI2C_STAT_IF		0x01
+#define DMECI2C_STAT_TIP	0x02
+#define DMECI2C_STAT_ARBLOST	0x20
+#define DMECI2C_STAT_BUSY	0x40
+#define DMECI2C_STAT_NACK	0x80
+
+#define STATE_DONE		0
+#define STATE_START		1
+#define STATE_WRITE		2
+#define STATE_READ		3
+#define STATE_ERROR		4
+
+#define TYPE_OCORES		0
+#define TYPE_GRLIB		1
+
+#define DMEC_I2C_OFFSET		0x20
+#define DMEC_I2C_MAX_BUS_NUM	3
+
+struct dmec_i2c {
+	struct device *dev;
+	void __iomem *base;
+	u32 reg_shift;
+	u32 reg_io_width;
+	wait_queue_head_t wait;
+	struct i2c_adapter adap;
+	struct i2c_mux_core *mux;
+	struct i2c_msg *msg;
+	int pos;
+	int nmsgs;
+	int state; /* see STATE_ */
+	struct clk *clk;
+	int ip_clock_khz;
+	int bus_clock_khz;
+
+	u8 irq;
+	struct regmap *regmap;
+};
+
+static int flags;
+module_param(flags, int, 0644);
+MODULE_PARM_DESC(flags, "additional flags for the i2c bus configuration");
+
+static inline void dmec_i2c_setreg(struct dmec_i2c *i2c, int reg, u8 value)
+{
+	struct regmap *regmap = i2c->regmap;
+
+	regmap_write(regmap, DMEC_I2C_OFFSET + reg, value);
+}
+
+static inline u8 dmec_i2c_getreg(struct dmec_i2c *i2c, int reg)
+{
+	struct regmap *regmap = i2c->regmap;
+	unsigned int val;
+
+	regmap_read(regmap, DMEC_I2C_OFFSET + reg, &val);
+
+	return val;
+}
+
+static int dmec_i2c_dmx_select(struct i2c_mux_core *mux, u32 chan)
+{
+	struct dmec_i2c *i2c = mux->priv;
+	u8 bus = chan & 0x3;
+
+	dmec_i2c_setreg(i2c, DMECI2C_MUX, bus);
+
+	return 0;
+}
+
+static void dmec_i2c_dmx_del(struct dmec_i2c *i2c)
+{
+	if (i2c->mux)
+		i2c_mux_del_adapters(i2c->mux);
+}
+
+static int dmec_i2c_dmx_add(struct dmec_i2c *i2c)
+{
+	u8 bus_mask;
+	int i, ret = 0;
+
+	bus_mask = dmec_i2c_getreg(i2c, DMECI2C_MUX);
+	bus_mask = (bus_mask & 0x70) >> 4;
+
+	i2c->mux = i2c_mux_alloc(&i2c->adap,
+				 i2c->dev,
+				 DMEC_I2C_MAX_BUS_NUM, 0, 0,
+				 dmec_i2c_dmx_select,
+				 NULL);
+	if (!i2c->mux)
+		return -ENOMEM;
+
+	i2c->mux->priv = i2c;
+
+	for (i = 0; i < DMEC_I2C_MAX_BUS_NUM; i++) {
+		if (!(bus_mask & (i + 1)))
+			/* bus is not present so skip */
+			continue;
+		ret = i2c_mux_add_adapter(i2c->mux, 0, i, 0);
+		if (ret) {
+			ret = -ENODEV;
+			dev_err(i2c->dev,
+				"i2c dmx failed to register adapter %d\n", i);
+			goto dmec_i2c_dmx_add_failed;
+		}
+	}
+
+	return 0;
+
+dmec_i2c_dmx_add_failed:
+	dmec_i2c_dmx_del(i2c);
+	return ret;
+}
+
+static void dmec_i2c_process(struct dmec_i2c *i2c)
+{
+	struct i2c_msg *msg = i2c->msg;
+	u8 stat = dmec_i2c_getreg(i2c, DMECI2C_STATUS);
+
+	if ((i2c->state == STATE_DONE) || (i2c->state == STATE_ERROR)) {
+		/* stop has been sent */
+		dmec_i2c_setreg(i2c, DMECI2C_CMD, DMECI2C_CMD_IACK);
+		wake_up(&i2c->wait);
+		return;
+	}
+
+	/* error? */
+	if (stat & DMECI2C_STAT_ARBLOST) {
+		i2c->state = STATE_ERROR;
+		dmec_i2c_setreg(i2c, DMECI2C_CMD, DMECI2C_CMD_STOP);
+		return;
+	}
+
+	if ((i2c->state == STATE_START) || (i2c->state == STATE_WRITE)) {
+		i2c->state =
+			(msg->flags & I2C_M_RD) ? STATE_READ : STATE_WRITE;
+
+		if (stat & DMECI2C_STAT_NACK) {
+			i2c->state = STATE_ERROR;
+			dmec_i2c_setreg(i2c, DMECI2C_CMD, DMECI2C_CMD_STOP);
+			return;
+		}
+	} else {
+		msg->buf[i2c->pos++] = dmec_i2c_getreg(i2c, DMECI2C_DATA);
+	}
+
+	/* end of msg? */
+	if (i2c->pos == msg->len) {
+		i2c->nmsgs--;
+		i2c->msg++;
+		i2c->pos = 0;
+		msg = i2c->msg;
+
+		if (i2c->nmsgs) {	/* end? */
+			/* send start? */
+			if (!(msg->flags & I2C_M_NOSTART)) {
+				u8 addr = (msg->addr << 1);
+
+				if (msg->flags & I2C_M_RD)
+					addr |= 1;
+
+				i2c->state = STATE_START;
+
+				dmec_i2c_setreg(i2c, DMECI2C_DATA, addr);
+				dmec_i2c_setreg(i2c, DMECI2C_CMD,
+						DMECI2C_CMD_START);
+				return;
+			}
+			i2c->state = (msg->flags & I2C_M_RD)
+					? STATE_READ : STATE_WRITE;
+		} else {
+			i2c->state = STATE_DONE;
+			dmec_i2c_setreg(i2c, DMECI2C_CMD, DMECI2C_CMD_STOP);
+			return;
+		}
+	}
+
+	if (i2c->state == STATE_READ) {
+		dmec_i2c_setreg(i2c, DMECI2C_CMD, i2c->pos == (msg->len - 1) ?
+			  DMECI2C_CMD_READ_NACK : DMECI2C_CMD_READ_ACK);
+	} else {
+		dmec_i2c_setreg(i2c, DMECI2C_DATA, msg->buf[i2c->pos++]);
+		dmec_i2c_setreg(i2c, DMECI2C_CMD, DMECI2C_CMD_WRITE);
+	}
+}
+
+static irqreturn_t dmec_i2c_isr(int irq, void *dev_id)
+{
+	struct dmec_i2c *i2c = dev_id;
+
+	dmec_i2c_process(i2c);
+
+	return IRQ_HANDLED;
+}
+
+static int dmec_i2c_xfer(struct i2c_adapter *adap,
+			 struct i2c_msg *msgs, int num)
+{
+	struct dmec_i2c *i2c = i2c_get_adapdata(adap);
+
+	i2c->msg = msgs;
+	i2c->pos = 0;
+	i2c->nmsgs = num;
+	i2c->state = STATE_START;
+
+	dmec_i2c_setreg(i2c, DMECI2C_DATA,
+			(i2c->msg->addr << 1) |
+			((i2c->msg->flags & I2C_M_RD) ? 1 : 0));
+
+	dmec_i2c_setreg(i2c, DMECI2C_CMD, DMECI2C_CMD_START);
+
+	if (wait_event_timeout(i2c->wait, (i2c->state == STATE_ERROR) ||
+			       (i2c->state == STATE_DONE), HZ))
+		return (i2c->state == STATE_DONE) ? num : -EIO;
+	else
+		return -ETIMEDOUT;
+}
+
+static int dmec_i2c_init(struct device *dev, struct dmec_i2c *i2c)
+{
+	int prescale;
+	int diff;
+	u8 stat;
+	u8 ctrl = dmec_i2c_getreg(i2c, DMECI2C_CONTROL);
+
+	/* make sure the device is disabled */
+	ctrl &= ~(DMECI2C_CTRL_EN | DMECI2C_CTRL_IEN |
+		  DMECI2C_CTRL_VSSCL | DMECI2C_CTRL_VSSDA |
+		  DMECI2C_CTRL_MABCLR | DMECI2C_CTRL_MMDIS);
+	dmec_i2c_setreg(i2c, DMECI2C_CONTROL, ctrl);
+
+	prescale = (i2c->ip_clock_khz / (8 * i2c->bus_clock_khz)) - 2;
+	prescale = clamp(prescale, 0, 0xffff);
+
+	diff = i2c->ip_clock_khz / (8 * (prescale + 2)) - i2c->bus_clock_khz;
+	if (abs(diff) > i2c->bus_clock_khz / 10) {
+		dev_err(dev,
+			"Unsupported clock: core: %d KHz, bus: %d KHz\n",
+			i2c->ip_clock_khz, i2c->bus_clock_khz);
+		return -EINVAL;
+	}
+
+	dmec_i2c_setreg(i2c, DMECI2C_PRELOW, prescale & 0xff);
+	dmec_i2c_setreg(i2c, DMECI2C_PREHIGH, prescale >> 8);
+
+	/* default to first bus */
+	dmec_i2c_setreg(i2c, DMECI2C_MUX, 0x0);
+
+	/* Init the device */
+	dmec_i2c_setreg(i2c, DMECI2C_CMD, DMECI2C_CMD_IACK);
+	ctrl |= DMECI2C_CTRL_EN;
+	if (!flags)
+		ctrl |= DMECI2C_CTRL_MABCLR | DMECI2C_CTRL_MMDIS;
+	else
+		ctrl |= (flags & 0x3c);
+	dmec_i2c_setreg(i2c, DMECI2C_CONTROL, ctrl);
+
+	stat = dmec_i2c_getreg(i2c, DMECI2C_STATUS);
+	if (stat & DMECI2C_STAT_BUSY) {
+		dev_warn(dev,
+			 "I2C bus is busy - generating stop signal\n");
+		dmec_i2c_setreg(i2c, DMECI2C_CMD, DMECI2C_CMD_STOP);
+	}
+
+	stat = dmec_i2c_getreg(i2c, DMECI2C_VER);
+	dev_info(dev, "v%u.%u\n", (stat >> 4) & 0xf, stat & 0xf);
+
+	return 0;
+}
+
+static int dmec_i2c_irq_enable(struct dmec_i2c *i2c)
+{
+	u8 ctrl;
+	unsigned int irq;
+	int ret;
+
+	irq = i2c->irq;
+
+	/* Initialize interrupt handlers if not already done */
+	init_waitqueue_head(&i2c->wait);
+
+	ret = devm_request_threaded_irq(i2c->dev, irq, NULL, dmec_i2c_isr,
+					IRQF_ONESHOT | IRQF_SHARED,
+					i2c->adap.name, i2c);
+	if (ret) {
+		dev_err(i2c->dev,
+			"Unable to claim IRQ\n");
+		return ret;
+	}
+
+	/* Now enable interrupts in the controller */
+	ctrl = dmec_i2c_getreg(i2c, DMECI2C_CONTROL);
+	ctrl |= DMECI2C_CTRL_IEN;
+	dmec_i2c_setreg(i2c, DMECI2C_CONTROL, ctrl);
+
+	return 0;
+}
+
+static int dmec_i2c_irq_disable(struct dmec_i2c *i2c)
+{
+	u8 ctrl;
+
+	ctrl = dmec_i2c_getreg(i2c, DMECI2C_CONTROL);
+	ctrl &= ~DMECI2C_CTRL_IEN;
+	dmec_i2c_setreg(i2c, DMECI2C_CONTROL, ctrl);
+
+	devm_free_irq(i2c->dev, i2c->irq, i2c);
+
+	return 0;
+}
+
+static u32 dmec_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm dmec_algorithm = {
+	.master_xfer = dmec_i2c_xfer,
+	.functionality = dmec_i2c_func,
+};
+
+static struct i2c_adapter dmec_adapter = {
+	.owner = THIS_MODULE,
+	.name = "i2c-dmec",
+	.class = I2C_CLASS_DEPRECATED,
+	.algo = &dmec_algorithm,
+};
+
+#define dmec_i2c_of_probe(pdev, i2c) -ENODEV
+
+static int dmec_i2c_probe(struct platform_device *pdev)
+{
+	struct dmec_i2c *i2c;
+	struct dmec_i2c_platform_data *pdata;
+	int ret;
+
+	i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
+	if (!i2c)
+		return -ENOMEM;
+
+	i2c->dev = &pdev->dev;
+	pdata = dev_get_platdata(&pdev->dev);
+	if (pdata) {
+		i2c->reg_shift = pdata->reg_shift;
+		i2c->reg_io_width = pdata->reg_io_width;
+		i2c->ip_clock_khz = pdata->clock_khz;
+		i2c->bus_clock_khz = 100;
+		i2c->regmap = dmec_get_regmap(pdev->dev.parent);
+	} else {
+		ret = dmec_i2c_of_probe(pdev, i2c);
+		if (ret)
+			return ret;
+	}
+
+	if (i2c->reg_io_width == 0)
+		i2c->reg_io_width = 1; /* Set to default value */
+
+	i2c->irq = platform_get_irq(pdev, 0);
+
+	ret = dmec_i2c_init(&pdev->dev, i2c);
+	if (ret)
+		return ret;
+
+	/* hook up driver to tree */
+	platform_set_drvdata(pdev, i2c);
+	i2c->adap = dmec_adapter;
+	i2c_set_adapdata(&i2c->adap, i2c);
+	i2c->adap.dev.parent = &pdev->dev;
+	i2c->adap.dev.of_node = pdev->dev.of_node;
+
+	if (dmec_i2c_irq_enable(i2c)) {
+		dev_err(&pdev->dev, "Cannot claim IRQ\n");
+		return ret;
+	}
+
+	/* add i2c adapter to i2c tree */
+	ret = i2c_add_adapter(&i2c->adap);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to add adapter\n");
+		return ret;
+	}
+
+	ret = dmec_i2c_dmx_add(i2c);
+
+	return ret;
+}
+
+static int dmec_i2c_remove(struct platform_device *pdev)
+{
+	struct dmec_i2c *i2c = platform_get_drvdata(pdev);
+
+	/* disable i2c logic */
+	dmec_i2c_setreg(i2c, DMECI2C_CONTROL,
+			dmec_i2c_getreg(i2c, DMECI2C_CONTROL)
+			& ~(DMECI2C_CTRL_EN | DMECI2C_CTRL_IEN));
+
+	/* remove adapter & data */
+	dmec_i2c_dmx_del(i2c);
+	i2c_del_adapter(&i2c->adap);
+
+	if (!IS_ERR(i2c->clk))
+		clk_disable_unprepare(i2c->clk);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int dmec_i2c_suspend(struct device *dev)
+{
+	struct dmec_i2c *i2c = dev_get_drvdata(dev);
+	u8 ctrl = dmec_i2c_getreg(i2c, DMECI2C_CONTROL);
+
+	dmec_i2c_irq_disable(i2c);
+
+	/* make sure the device is disabled */
+	dmec_i2c_setreg(i2c, DMECI2C_CONTROL,
+			ctrl & ~DMECI2C_CTRL_EN);
+
+	if (!IS_ERR(i2c->clk))
+		clk_disable_unprepare(i2c->clk);
+	return 0;
+}
+
+static int dmec_i2c_resume(struct device *dev)
+{
+	struct dmec_i2c *i2c = dev_get_drvdata(dev);
+	int ret;
+
+	if (!IS_ERR(i2c->clk)) {
+		unsigned long rate;
+
+		ret = clk_prepare_enable(i2c->clk);
+
+		if (ret) {
+			dev_err(dev,
+				"clk_prepare_enable failed: %d\n", ret);
+			return ret;
+		}
+		rate = clk_get_rate(i2c->clk) / 1000;
+		if (rate)
+			i2c->ip_clock_khz = rate;
+	}
+
+	ret = dmec_i2c_init(dev, i2c);
+	if (ret)
+		return ret;
+	return dmec_i2c_irq_enable(i2c);
+}
+
+static SIMPLE_DEV_PM_OPS(dmec_i2c_pm, dmec_i2c_suspend, dmec_i2c_resume);
+#define DMEC_I2C_PM	(&dmec_i2c_pm)
+#else
+#define DMEC_I2C_PM	NULL
+#endif
+
+static struct platform_driver dmec_i2c_driver = {
+	.probe   = dmec_i2c_probe,
+	.remove  = dmec_i2c_remove,
+	.driver  = {
+		.name = "dmec-i2c",
+		.pm = DMEC_I2C_PM,
+	},
+};
+
+module_platform_driver(dmec_i2c_driver);
+
+MODULE_AUTHOR("Peter Korsgaard <jacmet@sunsite.dk>");
+MODULE_AUTHOR("Zahari Doychev <zahari.doychev@linux.com>");
+MODULE_DESCRIPTION("DMO OpenCores based I2C bus driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:dmec-i2c");
-- 
git-series 0.8.10

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

* [RFC PATCH 3/5] gpio-dmec: gpio support for dmec
  2016-10-27 10:47 [RFC PATCH 0/5] add support for DMO embedded controller Zahari Doychev
  2016-10-27 10:47 ` [RFC PATCH 1/5] dmec: add DMO mfd driver Zahari Doychev
  2016-10-27 10:47 ` [RFC PATCH 2/5] i2c-dmec: add i2c bus support for dmec Zahari Doychev
@ 2016-10-27 10:47 ` Zahari Doychev
       [not found]   ` <fe8ac70204de48edd8a3da8a783b10810f3d4ca1.1477564996.git-series.zahari.doychev-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
  2016-10-27 10:47 ` [RFC PATCH 4/5] wdt-dmec: watchdog " Zahari Doychev
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Zahari Doychev @ 2016-10-27 10:47 UTC (permalink / raw)
  To: linux-kernel, gregkh, lee.jones, wsa, linus.walleij, wim, linux
  Cc: linux-i2c, linux-gpio, gnurou, linux-watchdog, Zahari Doychev

This is support for the gpio functionality found on the Data Modul embedded
controllers

Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
---
 drivers/staging/dmec/Kconfig     |  10 +-
 drivers/staging/dmec/Makefile    |   1 +-
 drivers/staging/dmec/dmec.h      |   5 +-
 drivers/staging/dmec/gpio-dmec.c | 390 ++++++++++++++++++++++++++++++++-
 4 files changed, 406 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/dmec/gpio-dmec.c

diff --git a/drivers/staging/dmec/Kconfig b/drivers/staging/dmec/Kconfig
index 0067b0b..9c4a8e5 100644
--- a/drivers/staging/dmec/Kconfig
+++ b/drivers/staging/dmec/Kconfig
@@ -17,3 +17,13 @@ config I2C_DMEC
 
 	  To compile this driver as a module, say M here: the module will be
           called i2c-dmec
+
+config GPIO_DMEC
+	tristate "Data Modul GPIO"
+	depends on MFD_DMEC && GPIOLIB
+	help
+	  Say Y here to enable support for a GPIOs on a Data Module embedded
+	  controller.
+
+	  To compile this driver as a module, say M here: the module will be
+          called gpio-dmec
diff --git a/drivers/staging/dmec/Makefile b/drivers/staging/dmec/Makefile
index c51a37e..b71b27b 100644
--- a/drivers/staging/dmec/Makefile
+++ b/drivers/staging/dmec/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_MFD_DMEC)		+= dmec-core.o
 obj-$(CONFIG_I2C_DMEC)		+= i2c-dmec.o
+obj-$(CONFIG_GPIO_DMEC) 	+= gpio-dmec.o
diff --git a/drivers/staging/dmec/dmec.h b/drivers/staging/dmec/dmec.h
index 178937d..cc42926 100644
--- a/drivers/staging/dmec/dmec.h
+++ b/drivers/staging/dmec/dmec.h
@@ -1,6 +1,11 @@
 #ifndef _LINUX_MFD_DMEC_H
 #define _LINUX_MFD_DMEC_H
 
+struct dmec_gpio_platform_data {
+	int gpio_base;
+	int chip_num;
+};
+
 struct dmec_i2c_platform_data {
 	u32 reg_shift; /* register offset shift value */
 	u32 reg_io_width; /* register io read/write width */
diff --git a/drivers/staging/dmec/gpio-dmec.c b/drivers/staging/dmec/gpio-dmec.c
new file mode 100644
index 0000000..4cefbbf
--- /dev/null
+++ b/drivers/staging/dmec/gpio-dmec.c
@@ -0,0 +1,390 @@
+/*
+ * GPIO driver for Data Modul AG Embedded Controller
+ *
+ * Copyright (C) 2016 Data Modul AG
+ *
+ * Authors: Zahari Doychev <zahari.doychev@linux.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/acpi.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/gpio.h>
+#include <linux/seq_file.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/spinlock.h>
+
+#include "dmec.h"
+
+#define DMEC_GPIO_BANKS			2
+#define DMEC_GPIO_MAX_NUM		8
+#define DMEC_GPIO_BASE(x)		(0x40 + 0x10 * ((x)->chip_num))
+#define DMEC_GPIO_SET_OFFSET(x)		(DMEC_GPIO_BASE(x) + 0x1)
+#define DMEC_GPIO_GET_OFFSET(x)		(DMEC_GPIO_BASE(x) + 0x1)
+#define DMEC_GPIO_CLR_OFFSET(x)		(DMEC_GPIO_BASE(x) + 0x2)
+#define DMEC_GPIO_VER_OFFSET(x)		(DMEC_GPIO_BASE(x) + 0x2)
+#define DMEC_GPIO_DIR_OFFSET(x)		(DMEC_GPIO_BASE(x) + 0x3)
+#define DMEC_GPIO_IRQTYPE_OFFSET(x)	(DMEC_GPIO_BASE(x) + 0x4)
+#define DMEC_GPIO_EVTSTA_OFFSET(x)	(DMEC_GPIO_BASE(x) + 0x6)
+#define DMEC_GPIO_IRQCFG_OFFSET(x)	(DMEC_GPIO_BASE(x) + 0x8)
+#define DMEC_GPIO_NOPS_OFFSET(x)	(DMEC_GPIO_BASE(x) + 0xa)
+#define DMEC_GPIO_IRQSTA_OFFSET(x)	(DMEC_GPIO_BASE(x) + 0xb)
+
+#ifdef CONFIG_PM
+struct dmec_reg_ctx {
+	u32 dat;
+	u32 dir;
+	u32 imask;
+	u32 icfg[2];
+	u32 emask[2];
+};
+#endif
+
+struct dmec_gpio_priv {
+	struct regmap *regmap;
+	struct gpio_chip gpio_chip;
+	struct irq_chip irq_chip;
+	unsigned int chip_num;
+	unsigned int irq;
+	u8 ver;
+#ifdef CONFIG_PM
+	struct dmec_reg_ctx regs;
+#endif
+};
+
+static int dmec_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
+						   gpio_chip);
+	struct regmap *regmap = priv->regmap;
+	unsigned int val;
+
+	/* read get register */
+	regmap_read(regmap, DMEC_GPIO_GET_OFFSET(priv), &val);
+
+	return !!(val & BIT(offset));
+}
+
+static void dmec_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+	struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
+						   gpio_chip);
+	struct regmap *regmap = priv->regmap;
+
+	if (value)
+		regmap_write(regmap, DMEC_GPIO_SET_OFFSET(priv), BIT(offset));
+	else
+		regmap_write(regmap, DMEC_GPIO_CLR_OFFSET(priv), BIT(offset));
+}
+
+static int dmec_gpio_direction_input(struct gpio_chip *gc, unsigned int offset)
+{
+	struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
+						   gpio_chip);
+	struct regmap *regmap = priv->regmap;
+
+	/* set pin as input */
+	regmap_update_bits(regmap, DMEC_GPIO_DIR_OFFSET(priv), BIT(offset), 0);
+
+	return 0;
+}
+
+static int dmec_gpio_direction_output(struct gpio_chip *gc, unsigned int offset,
+				      int value)
+{
+	struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
+						   gpio_chip);
+	struct regmap *regmap = priv->regmap;
+	unsigned int val = BIT(offset);
+
+	if (value)
+		regmap_write(regmap, DMEC_GPIO_SET_OFFSET(priv), val);
+	else
+		regmap_write(regmap, DMEC_GPIO_CLR_OFFSET(priv), val);
+
+	/* set pin as output */
+	regmap_update_bits(regmap, DMEC_GPIO_DIR_OFFSET(priv), val, val);
+
+	return 0;
+}
+
+static int dmec_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+	struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
+						   gpio_chip);
+	struct regmap *regmap = priv->regmap;
+	unsigned int val;
+
+	regmap_read(regmap, DMEC_GPIO_DIR_OFFSET(priv), &val);
+
+	return !(val & BIT(offset));
+}
+
+static int dmec_gpio_pincount(struct dmec_gpio_priv *priv)
+{
+	struct regmap *regmap = priv->regmap;
+	unsigned int val;
+
+	regmap_read(regmap, DMEC_GPIO_NOPS_OFFSET(priv), &val);
+
+	/* number of pins is val + 1 */
+	return val == 0xff ? 0 : (val & 7) + 1;
+}
+
+static int dmec_gpio_get_version(struct gpio_chip *gc)
+{
+	struct device *dev = gc->parent;
+	struct dmec_gpio_priv *p = container_of(gc, struct dmec_gpio_priv,
+						gpio_chip);
+	unsigned int v;
+
+	regmap_read(p->regmap, DMEC_GPIO_VER_OFFSET(p), &v);
+	p->ver = v;
+	dev_info(dev, "chip%u v%u.%u\n", p->chip_num, (v >> 4) & 0xf, v & 0xf);
+
+	return 0;
+}
+
+static void dmec_gpio_irq_enable(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
+						   gpio_chip);
+	struct regmap *regmap = priv->regmap;
+	int offset, mask;
+
+	offset = DMEC_GPIO_IRQCFG_OFFSET(priv) + (d->hwirq >> 2);
+	mask = BIT((d->hwirq & 3) << 1);
+
+	regmap_update_bits(regmap, offset, mask, mask);
+}
+
+static void dmec_gpio_irq_disable(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
+						   gpio_chip);
+	struct regmap *regmap = priv->regmap;
+	int offset, mask;
+
+	offset = DMEC_GPIO_IRQCFG_OFFSET(priv) + (d->hwirq >> 2);
+	mask = 3 << ((d->hwirq & 3) << 1);
+
+	regmap_update_bits(regmap, offset, mask, 0);
+}
+
+static int dmec_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
+						   gpio_chip);
+	struct regmap *regmap = priv->regmap;
+	unsigned int offset, mask, val;
+
+	offset = DMEC_GPIO_IRQTYPE_OFFSET(priv) + (d->hwirq >> 2);
+	mask = ((d->hwirq & 3) << 1);
+
+	regmap_read(regmap, offset, &val);
+
+	val &= ~(3 << mask);
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_LEVEL_LOW:
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		val |= (1 << mask);
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		val |= (2 << mask);
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		val |= (3 << mask);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_write(regmap, offset, val);
+
+	return 0;
+}
+
+static irqreturn_t dmec_gpio_irq_handler(int irq, void *dev_id)
+{
+	struct dmec_gpio_priv *p = dev_id;
+	struct irq_domain *d = p->gpio_chip.irqdomain;
+	unsigned int irqs_handled = 0;
+	unsigned int val = 0, stat = 0;
+
+	regmap_read(p->regmap, DMEC_GPIO_IRQSTA_OFFSET(p), &val);
+	stat = val;
+	while (stat) {
+		int line = __ffs(stat);
+		int child_irq = irq_find_mapping(d, line);
+
+		handle_nested_irq(child_irq);
+		stat &= ~(BIT(line));
+		irqs_handled++;
+	}
+	regmap_write(p->regmap, DMEC_GPIO_EVTSTA_OFFSET(p), val);
+
+	return irqs_handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static int dmec_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct dmec_gpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	struct dmec_gpio_priv *priv;
+	struct gpio_chip *gpio_chip;
+	struct irq_chip *irq_chip;
+	int ret = 0;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->regmap = dmec_get_regmap(pdev->dev.parent);
+	priv->chip_num = pdata->chip_num;
+
+	gpio_chip = &priv->gpio_chip;
+	gpio_chip->label = "gpio-dmec";
+	gpio_chip->owner = THIS_MODULE;
+	gpio_chip->parent = dev;
+	gpio_chip->label = dev_name(dev);
+	gpio_chip->can_sleep = true;
+
+	gpio_chip->base = pdata->gpio_base;
+
+	gpio_chip->direction_input = dmec_gpio_direction_input;
+	gpio_chip->direction_output = dmec_gpio_direction_output;
+	gpio_chip->get_direction = dmec_gpio_get_direction;
+	gpio_chip->get = dmec_gpio_get;
+	gpio_chip->set = dmec_gpio_set;
+	gpio_chip->ngpio = dmec_gpio_pincount(priv);
+	if (gpio_chip->ngpio == 0) {
+		dev_err(dev, "No GPIOs detected\n");
+		return -ENODEV;
+	}
+
+	dmec_gpio_get_version(gpio_chip);
+
+	irq_chip = &priv->irq_chip;
+	irq_chip->name = dev_name(dev);
+	irq_chip->irq_mask = dmec_gpio_irq_disable;
+	irq_chip->irq_unmask = dmec_gpio_irq_enable;
+	irq_chip->irq_set_type = dmec_gpio_irq_set_type;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, gpio_chip, priv);
+	if (ret) {
+		dev_err(dev, "Could not register GPIO chip\n");
+		return ret;
+	}
+
+	ret = gpiochip_irqchip_add(gpio_chip, irq_chip, 0,
+				   handle_simple_irq, IRQ_TYPE_NONE);
+	if (ret) {
+		dev_err(dev, "cannot add irqchip\n");
+		return ret;
+	}
+
+	priv->irq = platform_get_irq(pdev, 0);
+	ret = devm_request_threaded_irq(dev, priv->irq,
+					NULL, dmec_gpio_irq_handler,
+					IRQF_ONESHOT | IRQF_SHARED,
+					dev_name(dev), priv);
+	if (ret) {
+		dev_err(dev, "unable to get irq: %d\n", ret);
+		return ret;
+	}
+
+	gpiochip_set_chained_irqchip(gpio_chip, irq_chip, priv->irq, NULL);
+
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+}
+
+static int dmec_gpio_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int dmec_gpio_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct dmec_gpio_priv *p = platform_get_drvdata(pdev);
+	struct regmap *regmap = p->regmap;
+	struct dmec_reg_ctx *ctx = &p->regs;
+
+	regmap_read(regmap, DMEC_GPIO_BASE(p), &ctx->dat);
+	regmap_read(regmap, DMEC_GPIO_DIR_OFFSET(p), &ctx->dir);
+	regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), &ctx->imask);
+	regmap_read(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p), &ctx->emask[0]);
+	regmap_read(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p) + 1, &ctx->emask[1]);
+	regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), &ctx->icfg[0]);
+	regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p) + 1, &ctx->icfg[1]);
+
+	devm_free_irq(&pdev->dev, p->irq, p);
+
+	return 0;
+}
+
+static int dmec_gpio_resume(struct platform_device *pdev)
+{
+	struct dmec_gpio_priv *p = platform_get_drvdata(pdev);
+	struct regmap *regmap = p->regmap;
+	struct dmec_reg_ctx *ctx = &p->regs;
+	int ret;
+
+	regmap_write(regmap, DMEC_GPIO_BASE(p), ctx->dat);
+	regmap_write(regmap, DMEC_GPIO_DIR_OFFSET(p), ctx->dir);
+	regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), ctx->icfg[0]);
+	regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p) + 1, ctx->icfg[1]);
+	regmap_write(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p), ctx->emask[0]);
+	regmap_write(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p) + 1, ctx->emask[1]);
+	regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), ctx->imask);
+	regmap_write(regmap, DMEC_GPIO_EVTSTA_OFFSET(p), 0xff);
+
+	ret = devm_request_threaded_irq(&pdev->dev, p->irq,
+					NULL, dmec_gpio_irq_handler,
+					IRQF_ONESHOT | IRQF_SHARED,
+					dev_name(&pdev->dev), p);
+	if (ret)
+		dev_err(&pdev->dev, "unable to get irq: %d\n", ret);
+
+	return ret;
+}
+#else
+#define dmec_gpio_suspend NULL
+#define dmec_gpio_resume NULL
+#endif
+
+static struct platform_driver dmec_gpio_driver = {
+	.driver = {
+		.name = "dmec-gpio",
+		.owner = THIS_MODULE,
+	},
+	.probe = dmec_gpio_probe,
+	.remove	= dmec_gpio_remove,
+	.suspend = dmec_gpio_suspend,
+	.resume = dmec_gpio_resume,
+};
+
+module_platform_driver(dmec_gpio_driver);
+
+MODULE_DESCRIPTION("dmec gpio driver");
+MODULE_AUTHOR("Zahari Doychev <zahari.doychev@linux.com");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:dmec-gpio");
-- 
git-series 0.8.10

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

* [RFC PATCH 4/5] wdt-dmec: watchdog support for dmec
  2016-10-27 10:47 [RFC PATCH 0/5] add support for DMO embedded controller Zahari Doychev
                   ` (2 preceding siblings ...)
  2016-10-27 10:47 ` [RFC PATCH 3/5] gpio-dmec: gpio " Zahari Doychev
@ 2016-10-27 10:47 ` Zahari Doychev
  2016-10-27 10:47 ` [RFC PATCH 5/5] rtm-dmec: running time meter support Zahari Doychev
  2016-10-27 15:01 ` [RFC PATCH 0/5] add support for DMO embedded controller Greg KH
  5 siblings, 0 replies; 13+ messages in thread
From: Zahari Doychev @ 2016-10-27 10:47 UTC (permalink / raw)
  To: linux-kernel, gregkh, lee.jones, wsa, linus.walleij, wim, linux
  Cc: linux-i2c, linux-gpio, gnurou, linux-watchdog, Zahari Doychev

This is support for the watchdog found on the Data Modul embedded boards.

Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
---
 drivers/staging/dmec/Kconfig    |  11 +-
 drivers/staging/dmec/Makefile   |   1 +-
 drivers/staging/dmec/wdt-dmec.c | 569 +++++++++++++++++++++++++++++++++-
 3 files changed, 581 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/dmec/wdt-dmec.c

diff --git a/drivers/staging/dmec/Kconfig b/drivers/staging/dmec/Kconfig
index 9c4a8e5..eddf0bb 100644
--- a/drivers/staging/dmec/Kconfig
+++ b/drivers/staging/dmec/Kconfig
@@ -27,3 +27,14 @@ config GPIO_DMEC
 
 	  To compile this driver as a module, say M here: the module will be
           called gpio-dmec
+
+config WDT_DMEC
+	tristate "Data Modul Watchdog"
+	depends on MFD_DMEC
+	select WATCHDOG_CORE
+	help
+	  Say Y to enable support for a watchdog on a Data Modul embedded
+	  controllers.
+
+	  To compile this driver as a module, say M here: the module will be
+          called wdt-dmec
diff --git a/drivers/staging/dmec/Makefile b/drivers/staging/dmec/Makefile
index b71b27b..8b363cc 100644
--- a/drivers/staging/dmec/Makefile
+++ b/drivers/staging/dmec/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_MFD_DMEC)		+= dmec-core.o
 obj-$(CONFIG_I2C_DMEC)		+= i2c-dmec.o
 obj-$(CONFIG_GPIO_DMEC) 	+= gpio-dmec.o
+obj-$(CONFIG_WDT_DMEC)		+= wdt-dmec.o
diff --git a/drivers/staging/dmec/wdt-dmec.c b/drivers/staging/dmec/wdt-dmec.c
new file mode 100644
index 0000000..714ed11
--- /dev/null
+++ b/drivers/staging/dmec/wdt-dmec.c
@@ -0,0 +1,569 @@
+/*
+ * Watchdog driver for Data Modul AG Embedded Controller
+ *
+ * Copyright (C) 2016 Zahari Doychev, Data Modul AG
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/fs.h>
+#include <linux/ioport.h>
+#include <linux/init.h>
+#include <linux/uaccess.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+#include <linux/bitops.h>
+
+#include "dmec.h"
+
+#define DMEC_WDT_VER		0x30
+#define DMEC_WDT_SRV		0x30
+#define DMEC_WDT_CFG		0x31
+#define DMEC_WDT_S0CFG		0x32
+#define DMEC_WDT_S0MOD0		0x33
+#define DMEC_WDT_S0MOD1		0x34
+#define DMEC_WDT_S1CFG		(DMEC_WDT_S0CFG + 3)
+#define DMEC_WDT_S1MOD0		(DMEC_WDT_S0MOD0 + 3)
+#define DMEC_WDT_S1MOD1		(DMEC_WDT_S0MOD1 + 3)
+#define DMEC_WDT_S2CFG		(DMEC_WDT_S1CFG + 3)
+#define DMEC_WDT_S2MOD0		(DMEC_WDT_S1MOD0 + 3)
+#define DMEC_WDT_S2MOD1		(DMEC_WDT_S1MOD1 + 3)
+
+#define DMEC_WDT_EN		BIT(0)
+#define DMEC_WDT_LOCK		BIT(1)
+#define DMEC_WDT_WIN_MODE	BIT(2)
+#define DMEC_WDT_AL		BIT(3)
+
+#define DMEC_WDT_PRESCALER	BIT(4)
+#define DMEC_WDT_WDTEN		BIT(3)
+#define DMEC_WDT_WDSTS		BIT(5)
+
+#define DMEC_WDT_TIMEOUT_MIN	1 /* s */
+#define DMEC_WDT_TIMEOUT_MAX	(2 * 3600) /* s */
+
+#define DMEC_WDT_TIME_MAX	(65 * 1000)
+
+/* S0 used only during boot */
+#define DEFAULT_S0_TIMEOUT	0
+#define DEFAULT_S1_TIMEOUT	3
+#define DEFAULT_S2_TIMEOUT	5
+
+enum wdt_actions {
+	WDT_DISABLE = 0,
+	WDT_DELAY,
+	WDT_RESET,
+	WDT_SYSIRQ0,
+	WDT_SYSIRQ1,
+	WDT_SYSIRQ2,
+	WDT_IRQ,
+	WDT_RESERVED
+};
+
+enum wdt_stages {
+	S0,
+	S1,
+	S2
+};
+
+static unsigned int s1_timeout = DEFAULT_S1_TIMEOUT;
+module_param(s1_timeout, uint, 0644);
+MODULE_PARM_DESC(s1_timeout,
+		 "Watchdog stage 1 timeout in [s], default=3");
+
+static unsigned int s2_timeout = DEFAULT_S2_TIMEOUT;
+module_param(s2_timeout, uint, 0644);
+MODULE_PARM_DESC(s2_timeout,
+		 "Watchdog stage 2 timeout in [s], default=5");
+
+static enum wdt_actions action = WDT_DELAY;
+module_param(action, uint, 0644);
+MODULE_PARM_DESC(action,
+		 "Watchdog action for stage 1, default=1 (delay)");
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0644);
+MODULE_PARM_DESC(nowayout,
+		 "Watchdog cannot be stopped once started (default="
+				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+static bool win_mode;
+module_param(win_mode, bool, 0644);
+MODULE_PARM_DESC(win_mode, "Use watchdog window mode, default=0");
+
+static bool stop_on_reboot;
+module_param(stop_on_reboot, bool, 0644);
+MODULE_PARM_DESC(stop_on_reboot, "stop watchdog on system reboot, default=0");
+
+struct dmec_wdt_data {
+	struct watchdog_device	wdd;
+	struct regmap *regmap;
+	unsigned int s1_time;
+	unsigned int s2_time;
+	unsigned int status;
+	unsigned int boot_cfg;
+	bool boot_mode;
+};
+
+static int dmec_wdt_set_stage_action(struct dmec_wdt_data *wdat,
+				     unsigned int stage,
+				     enum wdt_actions action)
+{
+	unsigned int offset = DMEC_WDT_S0CFG + (3 * stage);
+	unsigned int val;
+
+	regmap_read(wdat->regmap, offset, &val);
+	val &= ~DMEC_WDT_WDSTS;
+	val |= action | DMEC_WDT_WDTEN;
+	regmap_write(wdat->regmap, offset, val);
+
+	return 0;
+}
+
+static int dmec_wdt_clear_action(struct dmec_wdt_data *wdat, int stage)
+{
+	unsigned int val;
+
+	regmap_read(wdat->regmap, DMEC_WDT_S0CFG + 3 * stage, &val);
+	val |= DMEC_WDT_WDSTS;
+	val &= ~0xf;
+	regmap_write(wdat->regmap, DMEC_WDT_S0CFG + 3 * stage, val);
+
+	return 0;
+}
+
+static unsigned int dmec_wdt_get_stage_timeout(struct dmec_wdt_data *wdat,
+					       unsigned int stage)
+{
+	unsigned int val, timeout = 0, cfg;
+
+	regmap_read(wdat->regmap, DMEC_WDT_S0CFG + 3 * stage, &cfg);
+	regmap_read(wdat->regmap, DMEC_WDT_S0MOD0 + 3 * stage, &val);
+	timeout = val;
+	regmap_read(wdat->regmap, DMEC_WDT_S0MOD1 + 3 * stage, &val);
+	timeout |= (val << 8);
+
+	if (cfg & DMEC_WDT_PRESCALER)
+		timeout <<= 7;
+
+	return timeout / 1000;
+}
+
+static int dmec_wdt_set_stage_timeout(struct dmec_wdt_data *wdat,
+				      unsigned int stage,
+				      unsigned int timeout)
+{
+	unsigned int val;
+
+	timeout *= 1000;
+	if (timeout > DMEC_WDT_TIME_MAX) {
+		/* enable prescaler */
+		regmap_read(wdat->regmap, DMEC_WDT_S0CFG + (3 * stage), &val);
+		val |= DMEC_WDT_PRESCALER;
+		regmap_write(wdat->regmap, DMEC_WDT_S0CFG + (3 * stage), val);
+		timeout >>= 7;
+	} else {
+		regmap_read(wdat->regmap, DMEC_WDT_S0CFG + (3 * stage), &val);
+		val &= ~DMEC_WDT_PRESCALER;
+		regmap_write(wdat->regmap, DMEC_WDT_S0CFG + (3 * stage), val);
+	}
+
+	val = timeout & 0xff;
+	regmap_write(wdat->regmap, DMEC_WDT_S0MOD0 + (3 * stage), val);
+	val = (timeout >> 8) & 0xff;
+	regmap_write(wdat->regmap, DMEC_WDT_S0MOD1 + (3 * stage), val);
+
+	return 0;
+}
+
+static int dmec_wdt_set_timeouts(struct dmec_wdt_data *wdat)
+{
+	dmec_wdt_clear_action(wdat, S0);
+	dmec_wdt_clear_action(wdat, S1);
+	dmec_wdt_clear_action(wdat, S2);
+
+	wdat->s1_time = s1_timeout;
+	wdat->s2_time = s2_timeout;
+	dmec_wdt_set_stage_timeout(wdat, S1, s1_timeout);
+	dmec_wdt_set_stage_timeout(wdat, S2, s2_timeout);
+	dmec_wdt_set_stage_action(wdat, S2, WDT_RESET);
+
+	return 0;
+}
+
+static int dmec_wdt_stop(struct watchdog_device *wdd)
+{
+	struct dmec_wdt_data *wdat = watchdog_get_drvdata(wdd);
+
+	regmap_update_bits(wdat->regmap, DMEC_WDT_CFG, DMEC_WDT_EN, 0);
+
+	return 0;
+}
+
+static int dmec_wdt_start(struct watchdog_device *wdd)
+{
+	struct dmec_wdt_data *wdat = watchdog_get_drvdata(wdd);
+
+	regmap_update_bits(wdat->regmap, DMEC_WDT_CFG,
+			   DMEC_WDT_EN, DMEC_WDT_EN);
+
+	wdat->boot_mode = false;
+
+	return 0;
+}
+
+static int dmec_wdt_ping(struct watchdog_device *wdd)
+{
+	struct dmec_wdt_data *wdat = watchdog_get_drvdata(wdd);
+	unsigned int val;
+
+	/* We should try avoiding resets in window mode until the watchdog
+	 * daemon takes control
+	 */
+	if (win_mode && wdat->boot_mode) {
+		regmap_read(wdat->regmap, DMEC_WDT_S1CFG, &val);
+		if (!(val & DMEC_WDT_WDSTS))
+			return 0;
+		val |= DMEC_WDT_WDSTS;
+		regmap_write(wdat->regmap, DMEC_WDT_S1CFG, val);
+	}
+
+	regmap_write(wdat->regmap, DMEC_WDT_SRV, 0xff);
+
+	return 0;
+}
+
+static int dmec_wdt_set_win_mode(struct watchdog_device *wdd)
+{
+	struct dmec_wdt_data *wdat = watchdog_get_drvdata(wdd);
+
+	regmap_update_bits(wdat->regmap, DMEC_WDT_CFG,
+			   DMEC_WDT_WIN_MODE, DMEC_WDT_WIN_MODE);
+
+	return 0;
+}
+
+static int dmec_wdt_set_std_mode(struct watchdog_device *wdd)
+{
+	struct dmec_wdt_data *wdat = watchdog_get_drvdata(wdd);
+
+	regmap_update_bits(wdat->regmap, DMEC_WDT_CFG, DMEC_WDT_WIN_MODE, 0);
+
+	return 0;
+}
+
+static int dmec_wdt_set_timeout(struct watchdog_device *wdd,
+				unsigned int timeout)
+{
+	struct dmec_wdt_data *wdat = watchdog_get_drvdata(wdd);
+
+	wdd->timeout = timeout;
+	wdat->boot_mode = false;
+	dmec_wdt_stop(wdd);
+	dmec_wdt_clear_action(wdat, S0);
+
+	if (win_mode) {
+		dmec_wdt_clear_action(wdat, S1);
+		dmec_wdt_set_stage_timeout(wdat, S1, wdat->s1_time);
+		dmec_wdt_set_stage_action(wdat, S1, WDT_DELAY);
+		dmec_wdt_set_win_mode(wdd);
+	} else {
+		dmec_wdt_set_std_mode(wdd);
+	}
+
+	wdat->s2_time = timeout;
+	dmec_wdt_clear_action(wdat, S2);
+	dmec_wdt_set_stage_timeout(wdat, S2, timeout);
+	dmec_wdt_set_stage_action(wdat, S2, WDT_RESET);
+	clear_bit(WDOG_HW_RUNNING, &wdd->status);
+	dmec_wdt_start(wdd);
+
+	return 0;
+}
+
+static long dmec_wdt_ioctl(struct watchdog_device *wdd, unsigned int cmd,
+			   unsigned long arg)
+{
+	struct dmec_wdt_data *wdat = watchdog_get_drvdata(wdd);
+	void __user *argp = (void __user *)arg;
+	int ret = -ENOIOCTLCMD;
+	int __user *p = argp;
+	int val;
+
+	switch (cmd) {
+	case WDIOC_SETPRETIMEOUT:
+		if (get_user(val, p) && val < 0)
+			return -EFAULT;
+		dmec_wdt_clear_action(wdat, S1);
+		if (val > 0) {
+			dmec_wdt_set_stage_timeout(wdat, S1, val);
+			dmec_wdt_set_stage_action(wdat, S1, action);
+		}
+		wdat->s1_time = val;
+		wdat->boot_mode = false;
+		ret = 0;
+		if (!win_mode)
+			ret = dmec_wdt_ping(wdd);
+		break;
+	case WDIOC_GETPRETIMEOUT:
+		ret = put_user(wdat->s1_time, (int __user *)arg);
+		break;
+	}
+
+	return ret;
+}
+
+static unsigned int dmec_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+	struct dmec_wdt_data *wdat = watchdog_get_drvdata(wdd);
+
+	return dmec_wdt_get_stage_timeout(wdat, 2);
+}
+
+static unsigned int dmec_wdt_status(struct watchdog_device *wdd)
+{
+	struct dmec_wdt_data *wdat = watchdog_get_drvdata(wdd);
+	unsigned int status;
+
+	regmap_read(wdat->regmap, DMEC_WDT_CFG, &status);
+
+	return status;
+}
+
+static struct watchdog_info dmec_wdt_info = {
+	.options		= WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
+				  WDIOF_MAGICCLOSE | WDIOF_PRETIMEOUT,
+	.identity		= "DMEC WDT",
+};
+
+static const struct watchdog_ops dmec_wdt_ops = {
+	.owner		= THIS_MODULE,
+	.start		= dmec_wdt_start,
+	.stop		= dmec_wdt_stop,
+	.ping		= dmec_wdt_ping,
+	.set_timeout	= dmec_wdt_set_timeout,
+	.status		= dmec_wdt_status,
+	.ioctl		= dmec_wdt_ioctl,
+	.get_timeleft	= dmec_wdt_get_timeleft,
+};
+
+static int dmec_wdt_get_hw_ping_time(struct dmec_wdt_data *wdat)
+{
+	struct watchdog_device *wdd = &wdat->wdd;
+	unsigned int val, t;
+
+	regmap_read(wdat->regmap, DMEC_WDT_S0CFG, &val);
+	t = dmec_wdt_get_stage_timeout(wdat, S0);
+	if (t && (val & 0x7) > 1)
+		dmec_wdt_clear_action(wdat, S0);
+
+	wdat->s1_time = dmec_wdt_get_stage_timeout(wdat, S1);
+	wdat->s2_time = dmec_wdt_get_stage_timeout(wdat, S2);
+
+	regmap_read(wdat->regmap, DMEC_WDT_S1CFG, &val);
+	if (wdat->s1_time && (val & 0x7) > 1 && !(val & DMEC_WDT_WDSTS)) {
+		wdd->timeout = wdat->s1_time;
+		return 1;
+	}
+
+	regmap_read(wdat->regmap, DMEC_WDT_S2CFG, &val);
+	if (wdat->s2_time && (val & 0x7) > 1 && !(val & DMEC_WDT_WDSTS)) {
+		wdd->timeout = wdat->s2_time;
+		return 2;
+	}
+
+	return -1;
+}
+
+static int dmec_wdt_config_win_mode(struct dmec_wdt_data *wdat)
+{
+	struct watchdog_device *wdd = &wdat->wdd;
+	unsigned int t;
+
+	/* The timeout should be ok until the watchdog daemon appears */
+	t = (min_t(unsigned int, wdat->s1_time, wdat->s2_time) / 2);
+	wdd->timeout = t;
+
+	return 0;
+}
+
+static int dmec_wdt_config_mode(struct dmec_wdt_data *wdat)
+{
+	struct watchdog_device *wdd = &wdat->wdd;
+
+	if (!(wdat->boot_cfg & DMEC_WDT_EN))
+		return -1;
+
+	if (dmec_wdt_get_hw_ping_time(wdat) < 0)
+		return -1;
+
+	wdd->max_hw_heartbeat_ms = DMEC_WDT_TIMEOUT_MAX * 1000;
+	set_bit(WDOG_HW_RUNNING, &wdd->status);
+
+	if (wdat->boot_cfg & DMEC_WDT_WIN_MODE)
+		dmec_wdt_config_win_mode(wdat);
+
+	wdat->boot_mode = true;
+
+	return 0;
+}
+
+static int dmec_wdt_setup(struct dmec_wdt_data *wdat)
+{
+	struct watchdog_device *wdd = &wdat->wdd;
+	int ret = 0;
+
+	regmap_read(wdat->regmap, DMEC_WDT_CFG, &wdat->boot_cfg);
+
+	ret = dmec_wdt_config_mode(wdat);
+
+	if (ret < 0)
+		dmec_wdt_set_timeouts(wdat);
+
+	if (wdat->boot_cfg & DMEC_WDT_LOCK && !nowayout) {
+		dev_info(wdd->parent, "watchdog lock is enabled.\n");
+		nowayout = true;
+		return 0;
+	}
+
+	return 0;
+}
+
+static int dmec_wdt_probe(struct platform_device *pdev)
+{
+	struct dmec_wdt_data *wdat;
+	struct device *dev = &pdev->dev;
+	struct watchdog_device *wdd;
+	int ret = 0;
+
+	wdat = devm_kzalloc(dev, sizeof(*wdat), GFP_KERNEL);
+	if (!wdat)
+		return -ENOMEM;
+
+	wdat->regmap = dmec_get_regmap(pdev->dev.parent);
+	wdd = &wdat->wdd;
+	wdd->parent = dev;
+
+	wdd->info = &dmec_wdt_info;
+	wdd->ops = &dmec_wdt_ops;
+	wdd->min_timeout = DMEC_WDT_TIMEOUT_MIN;
+	wdd->max_timeout = DMEC_WDT_TIMEOUT_MAX;
+
+	wdat->s1_time = s1_timeout;
+	wdat->s2_time = s2_timeout;
+
+	watchdog_set_drvdata(wdd, wdat);
+	platform_set_drvdata(pdev, wdat);
+
+	dmec_wdt_setup(wdat);
+
+	watchdog_set_nowayout(wdd, nowayout);
+	if (stop_on_reboot)
+		watchdog_stop_on_reboot(wdd);
+
+	ret = watchdog_register_device(wdd);
+	if (ret)
+		return ret;
+
+	regmap_read(wdat->regmap, DMEC_WDT_VER,
+		    &dmec_wdt_info.firmware_version);
+
+	dev_info(dev, "registered. v%u.%u sta: %#lx mode:%d\n",
+		 (dmec_wdt_info.firmware_version >> 4) & 0xf,
+		 dmec_wdt_info.firmware_version & 0xf,
+		 wdd->status, win_mode);
+
+	return 0;
+}
+
+static int dmec_wdt_remove(struct platform_device *pdev)
+{
+	struct dmec_wdt_data *wdat = platform_get_drvdata(pdev);
+
+	dmec_wdt_stop(&wdat->wdd);
+	watchdog_unregister_device(&wdat->wdd);
+
+	return 0;
+}
+
+static void dmec_wdt_shutdown(struct platform_device *pdev)
+{
+	struct dmec_wdt_data *wdat = platform_get_drvdata(pdev);
+
+	dmec_wdt_stop(&wdat->wdd);
+}
+
+#ifdef CONFIG_PM
+/* Disable watchdog if it is active during suspend */
+static int dmec_wdt_suspend(struct platform_device *pdev,
+			    pm_message_t message)
+{
+	struct dmec_wdt_data *wdat = platform_get_drvdata(pdev);
+	struct watchdog_device *wdd = &wdat->wdd;
+
+	regmap_read(wdat->regmap, DMEC_WDT_CFG, &wdat->status);
+
+	if (wdat->status & DMEC_WDT_EN)
+		return dmec_wdt_stop(wdd);
+
+	return 0;
+}
+
+/* Enable watchdog and configure it if necessary */
+static int dmec_wdt_resume(struct platform_device *pdev)
+{
+	struct dmec_wdt_data *wdat = platform_get_drvdata(pdev);
+	struct watchdog_device *wdd = &wdat->wdd;
+
+	if (!win_mode && wdat->status & DMEC_WDT_EN) {
+		dmec_wdt_stop(wdd);
+		dmec_wdt_clear_action(wdat, S0);
+		dmec_wdt_clear_action(wdat, S1);
+		dmec_wdt_clear_action(wdat, S2);
+
+		if (wdat->s1_time) {
+			dmec_wdt_set_stage_timeout(wdat, S1, wdat->s1_time);
+			dmec_wdt_set_stage_action(wdat, S1, WDT_DELAY);
+		}
+		dmec_wdt_set_stage_timeout(wdat, S2, wdat->s2_time);
+		dmec_wdt_set_stage_action(wdat, S2, WDT_RESET);
+		dmec_wdt_get_timeleft(wdd);
+		return dmec_wdt_start(wdd);
+	}
+
+	return dmec_wdt_stop(wdd);
+}
+#else
+#define dmec_wdt_suspend NULL
+#define dmec_wdt_resume	NULL
+#endif
+
+static struct platform_driver dmec_wdt_driver = {
+	.driver = {
+		.name = "dmec-wdt",
+		.owner = THIS_MODULE,
+	},
+	.probe = dmec_wdt_probe,
+	.remove = dmec_wdt_remove,
+	.shutdown = dmec_wdt_shutdown,
+	.suspend = dmec_wdt_suspend,
+	.resume = dmec_wdt_resume,
+};
+
+module_platform_driver(dmec_wdt_driver);
+
+MODULE_DESCRIPTION("dmec watchdog driver");
+MODULE_AUTHOR("Zahari Doychev <zahari.doychev@linux.com");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:dmec-wdt");
-- 
git-series 0.8.10

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

* [RFC PATCH 5/5] rtm-dmec: running time meter support
  2016-10-27 10:47 [RFC PATCH 0/5] add support for DMO embedded controller Zahari Doychev
                   ` (3 preceding siblings ...)
  2016-10-27 10:47 ` [RFC PATCH 4/5] wdt-dmec: watchdog " Zahari Doychev
@ 2016-10-27 10:47 ` Zahari Doychev
  2016-10-27 15:01 ` [RFC PATCH 0/5] add support for DMO embedded controller Greg KH
  5 siblings, 0 replies; 13+ messages in thread
From: Zahari Doychev @ 2016-10-27 10:47 UTC (permalink / raw)
  To: linux-kernel, gregkh, lee.jones, wsa, linus.walleij, wim, linux
  Cc: linux-i2c, linux-gpio, gnurou, linux-watchdog, Zahari Doychev

This is support for the running time meter(RTM) found on the Data Modul
embedded boards.

Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
---
 drivers/staging/dmec/Kconfig    |  10 ++-
 drivers/staging/dmec/Makefile   |   1 +-
 drivers/staging/dmec/rtm-dmec.c | 203 +++++++++++++++++++++++++++++++++-
 3 files changed, 214 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/dmec/rtm-dmec.c

diff --git a/drivers/staging/dmec/Kconfig b/drivers/staging/dmec/Kconfig
index eddf0bb..f6f4146 100644
--- a/drivers/staging/dmec/Kconfig
+++ b/drivers/staging/dmec/Kconfig
@@ -38,3 +38,13 @@ config WDT_DMEC
 
 	  To compile this driver as a module, say M here: the module will be
           called wdt-dmec
+
+config RTM_DMEC
+	tristate "Data Modul RTM"
+	depends on MFD_DMEC
+	help
+	  Say Y to enable support for a running time meter(RTM) on a Data Modul
+	  embedded controllers.
+
+	  To compile this driver as a module, say M here: the module will be
+          called wdt-dmec
diff --git a/drivers/staging/dmec/Makefile b/drivers/staging/dmec/Makefile
index 8b363cc..b55d56f 100644
--- a/drivers/staging/dmec/Makefile
+++ b/drivers/staging/dmec/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_MFD_DMEC)		+= dmec-core.o
 obj-$(CONFIG_I2C_DMEC)		+= i2c-dmec.o
 obj-$(CONFIG_GPIO_DMEC) 	+= gpio-dmec.o
 obj-$(CONFIG_WDT_DMEC)		+= wdt-dmec.o
+obj-$(CONFIG_RTM_DMEC)		+= rtm-dmec.o
diff --git a/drivers/staging/dmec/rtm-dmec.c b/drivers/staging/dmec/rtm-dmec.c
new file mode 100644
index 0000000..cd32536
--- /dev/null
+++ b/drivers/staging/dmec/rtm-dmec.c
@@ -0,0 +1,203 @@
+/*
+ * Running time meter for Data Modul AG Embedded Controller
+ *
+ * Copyright (C) 2016 Data Modul AG
+ *
+ * Author: Sebastian Wezel <sebastian@torvus-musica.de>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mutex.h>
+
+#include "dmec.h"
+
+#define DMEC_RTM_RTM_OFFSET             0x60
+#define DMEC_RTM_RTMREV_OFFSET          0x63
+#define DMEC_RTM_OOSRTM_OFFSET          0x64
+#define DMEC_RTM_BTCNT_OFFSET           0x68
+#define DMEC_RTM_BBCTNT_OFFSET          0x6c
+
+static DEFINE_MUTEX(rtm_lock);
+static struct regmap *regmap;
+static bool enable_reset;
+
+static unsigned int dmec_rtm_get_three_byte(unsigned int reg,
+					    struct device *dev)
+{
+	unsigned int low = 0, mid = 0, high = 0, val = 0;
+
+	regmap_read(regmap, reg, &low);
+	regmap_read(regmap, reg + 1, &mid);
+	regmap_read(regmap, reg + 2, &high);
+
+	val = ((high << 16) | (mid << 8) | (low & 0x0000ff));
+	return val;
+}
+
+static ssize_t dmec_rtm_time_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	unsigned int val = 0;
+
+	val = dmec_rtm_get_three_byte(DMEC_RTM_RTM_OFFSET, dev);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", val);
+}
+
+static ssize_t dmec_rtm_time_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf,
+				   size_t count)
+{
+	unsigned int low = 0, mid = 0, high = 0, higher = 0;
+	long val = 0, ret = -EPERM;
+	int reg = DMEC_RTM_RTM_OFFSET;
+
+	mutex_lock(&rtm_lock);
+	if (enable_reset && !kstrtol(buf, 10, &val)) {
+		higher = (val >> 24) & 0xff;
+		high = (val >> 16) & 0xff;
+		mid = (val >> 8) & 0xff;
+		low = val & 0xff;
+
+		regmap_write(regmap, reg, low);
+		regmap_write(regmap, reg + 1, mid);
+		regmap_write(regmap, reg + 2, high);
+		regmap_write(regmap, reg + 3, higher);
+		enable_reset = false;
+		ret = count;
+	}
+	mutex_unlock(&rtm_lock);
+
+	return ret;
+}
+
+static ssize_t dmec_rtm_version_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	unsigned int val = 0;
+
+	regmap_read(regmap, DMEC_RTM_RTMREV_OFFSET, &val);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", val);
+}
+
+static ssize_t dmec_rtm_out_of_spec_show(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	unsigned int val = 0;
+
+	val = dmec_rtm_get_three_byte(DMEC_RTM_OOSRTM_OFFSET, dev);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", val);
+}
+
+static ssize_t dmec_rtm_boot_count_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	unsigned int val = 0;
+
+	val = dmec_rtm_get_three_byte(DMEC_RTM_BTCNT_OFFSET, dev);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", val);
+}
+
+static ssize_t dmec_rtm_bios_boot_count_show(struct device *dev,
+					     struct device_attribute *attr,
+					     char *buf)
+{
+	unsigned int val = 0;
+
+	val = dmec_rtm_get_three_byte(DMEC_RTM_BBCTNT_OFFSET, dev);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", val);
+}
+
+static ssize_t dmec_rtm_enable_reset_show(struct device *dev,
+					  struct device_attribute *attr,
+					  char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "%d\n", enable_reset);
+}
+
+static ssize_t dmec_rtm_enable_reset_store(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf, size_t count)
+{
+	long val = 0;
+
+	mutex_lock(&rtm_lock);
+	if (kstrtol(buf, 10, &val) != 0)
+		enable_reset = false;
+
+	/* The enable should only work if the value is 1 */
+	if (val == 1)
+		enable_reset = true;
+	mutex_unlock(&rtm_lock);
+	return count;
+}
+
+static DEVICE_ATTR(rtm_time, 0664, dmec_rtm_time_show, dmec_rtm_time_store);
+static DEVICE_ATTR(rtm_version, 0444, dmec_rtm_version_show, NULL);
+static DEVICE_ATTR(rtm_out_of_spec, 0444, dmec_rtm_out_of_spec_show, NULL);
+static DEVICE_ATTR(rtm_boot_count, 0444, dmec_rtm_boot_count_show, NULL);
+static DEVICE_ATTR(rtm_bios_boot_count, 0444, dmec_rtm_bios_boot_count_show,
+			NULL);
+static DEVICE_ATTR(rtm_enable_reset, 0664, dmec_rtm_enable_reset_show,
+		   dmec_rtm_enable_reset_store);
+
+static struct attribute *rtm_attribute[] = {
+	&dev_attr_rtm_time.attr,
+	&dev_attr_rtm_version.attr,
+	&dev_attr_rtm_out_of_spec.attr,
+	&dev_attr_rtm_boot_count.attr,
+	&dev_attr_rtm_bios_boot_count.attr,
+	&dev_attr_rtm_enable_reset.attr,
+	NULL
+};
+
+static const struct attribute_group rtm_attr_group = {
+	.attrs = rtm_attribute,
+};
+
+static int dmec_rtm_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	regmap = dmec_get_regmap(pdev->dev.parent);
+	enable_reset = 0;
+
+	ret = sysfs_create_group(&pdev->dev.kobj, &rtm_attr_group);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
+static int dmec_rtm_remove(struct platform_device *pdev)
+{
+	sysfs_remove_group(&pdev->dev.kobj, &rtm_attr_group);
+
+	return 0;
+}
+
+static struct platform_driver dmec_rtm_driver = {
+	.driver = {
+			.name = "dmec-rtm",
+			.owner = THIS_MODULE,
+	},
+	.probe = dmec_rtm_probe,
+	.remove = dmec_rtm_remove,
+};
+
+module_platform_driver(dmec_rtm_driver);
+
+MODULE_DESCRIPTION("dmec rtm driver");
+MODULE_AUTHOR("Sebastian Wezel <sebastian@torvus-musica.de>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:dmec-rtm");
-- 
git-series 0.8.10

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

* Re: [RFC PATCH 0/5] add support for DMO embedded controller
  2016-10-27 10:47 [RFC PATCH 0/5] add support for DMO embedded controller Zahari Doychev
                   ` (4 preceding siblings ...)
  2016-10-27 10:47 ` [RFC PATCH 5/5] rtm-dmec: running time meter support Zahari Doychev
@ 2016-10-27 15:01 ` Greg KH
  2016-10-27 15:54   ` Zahari Doychev
  5 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2016-10-27 15:01 UTC (permalink / raw)
  To: Zahari Doychev
  Cc: linux-kernel, lee.jones, wsa, linus.walleij, wim, linux,
	linux-i2c, linux-gpio, gnurou, linux-watchdog

On Thu, Oct 27, 2016 at 12:47:11PM +0200, Zahari Doychev wrote:
> This patch series adds support for the Data Modul Embedded Controller(dmec)
> which is implemented within an on board FPGA found on Data Modul embedded
> CPU modules.
> 
> The dmec is connected to the host through the LPC bus and its registers are
> mapped into the host I/O space. The controller supports two addressing modes:
> linear and indexed.
> 
> The driver adds support for the following functionality:
> 
> - i2c
> - gpio
> - watchdog
> - running time meter (rtm)

Why do you want these in staging?  I need a TODO file listing what needs
to be done to the code to get it out of staging, and for someone to
agree to work toward that goal.

Please resend the series with that information.

thanks,

greg k-h

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

* Re: [RFC PATCH 0/5] add support for DMO embedded controller
  2016-10-27 15:01 ` [RFC PATCH 0/5] add support for DMO embedded controller Greg KH
@ 2016-10-27 15:54   ` Zahari Doychev
  0 siblings, 0 replies; 13+ messages in thread
From: Zahari Doychev @ 2016-10-27 15:54 UTC (permalink / raw)
  To: Greg KH
  Cc: Zahari Doychev, linux-kernel, lee.jones, wsa, linus.walleij, wim,
	linux, linux-i2c, linux-gpio, gnurou, linux-watchdog

On Thu, Oct 27, 2016 at 05:01:51PM +0200, Greg KH wrote:
> On Thu, Oct 27, 2016 at 12:47:11PM +0200, Zahari Doychev wrote:
> > This patch series adds support for the Data Modul Embedded Controller(dmec)
> > which is implemented within an on board FPGA found on Data Modul embedded
> > CPU modules.
> > 
> > The dmec is connected to the host through the LPC bus and its registers are
> > mapped into the host I/O space. The controller supports two addressing modes:
> > linear and indexed.
> > 
> > The driver adds support for the following functionality:
> > 
> > - i2c
> > - gpio
> > - watchdog
> > - running time meter (rtm)
> 
> Why do you want these in staging?  I need a TODO file listing what needs
> to be done to the code to get it out of staging, and for someone to
> agree to work toward that goal.

Actually there is no reason to have them in staging. I was not sure what was
the right way to go. I just wanted to get some feedback. The drivers have been
through a lot of testing so next time I will resend the patches putting
each driver in its subsystem.

Regards Zahari

> 
> Please resend the series with that information.
> 
> thanks,
> 
> greg k-h

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

* Re: [RFC PATCH 3/5] gpio-dmec: gpio support for dmec
  2016-10-27 10:47 ` [RFC PATCH 3/5] gpio-dmec: gpio " Zahari Doychev
@ 2016-10-29  9:05       ` Linus Walleij
  0 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2016-10-29  9:05 UTC (permalink / raw)
  To: Zahari Doychev
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg KH, Lee Jones,
	Wolfram Sang, Wim Van Sebroeck, Guenter Roeck,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, Alexandre Courbot,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA

On Thu, Oct 27, 2016 at 12:47 PM, Zahari Doychev
<zahari.doychev-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote:

> This is support for the gpio functionality found on the Data Modul embedded
> controllers
>
> Signed-off-by: Zahari Doychev <zahari.doychev-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/staging/dmec/Kconfig     |  10 +-
>  drivers/staging/dmec/Makefile    |   1 +-
>  drivers/staging/dmec/dmec.h      |   5 +-
>  drivers/staging/dmec/gpio-dmec.c | 390 ++++++++++++++++++++++++++++++++-

I guess Greg has already asked, but why is this in staging?
The driver doesn't seem very complex or anything, it would not
be overly troublesome to get it into the proper kernel subsystems.

> +config GPIO_DMEC
> +       tristate "Data Modul GPIO"
> +       depends on MFD_DMEC && GPIOLIB

So the depends on GPIOLIB can go away if you just put it into
drivers/gpio with the rest.

> +struct dmec_gpio_platform_data {
> +       int gpio_base;

NAK, always use -1. No hardcoding the GPIO base other than on
legacy systems.

> +       int chip_num;

I suspect you may not need this either. struct platform_device
already contains a ->id field, just use that when instantiating
your driver if you need an instance number.

So I think you need zero platform data for this.

> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/errno.h>
> +#include <linux/acpi.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/gpio.h>

You should only need <linux/gpio/driver.h>

> +#ifdef CONFIG_PM
> +struct dmec_reg_ctx {
> +       u32 dat;
> +       u32 dir;
> +       u32 imask;
> +       u32 icfg[2];
> +       u32 emask[2];
> +};
> +#endif
> +
> +struct dmec_gpio_priv {
> +       struct regmap *regmap;
> +       struct gpio_chip gpio_chip;
> +       struct irq_chip irq_chip;
> +       unsigned int chip_num;
> +       unsigned int irq;
> +       u8 ver;
> +#ifdef CONFIG_PM
> +       struct dmec_reg_ctx regs;
> +#endif
> +};

The #ifdef for saving the state is a bit kludgy. Can't you just have it there
all the time? Or is this a footprint-sensitive system?

> +static int dmec_gpio_get(struct gpio_chip *gc, unsigned int offset)
> +{
> +       struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
> +                                                  gpio_chip);

Use the new pattern with

struct dmec_gpio_priv *priv = gpiochip_get_data(gc);

This needs you to use devm_gpiochip_add_data() below.

> +static int dmec_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +       struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
> +                                                  gpio_chip);
> +       struct regmap *regmap = priv->regmap;
> +       unsigned int offset, mask, val;
> +
> +       offset = DMEC_GPIO_IRQTYPE_OFFSET(priv) + (d->hwirq >> 2);
> +       mask = ((d->hwirq & 3) << 1);
> +
> +       regmap_read(regmap, offset, &val);
> +
> +       val &= ~(3 << mask);
> +       switch (type & IRQ_TYPE_SENSE_MASK) {
> +       case IRQ_TYPE_LEVEL_LOW:
> +               break;
> +       case IRQ_TYPE_EDGE_RISING:
> +               val |= (1 << mask);
> +               break;
> +       case IRQ_TYPE_EDGE_FALLING:
> +               val |= (2 << mask);
> +               break;
> +       case IRQ_TYPE_EDGE_BOTH:
> +               val |= (3 << mask);
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       regmap_write(regmap, offset, val);
> +
> +       return 0;
> +}

This chip uses handle_simple_irq() which is fine if the chip really has no
edge detector ACK register.

What some chips have is a special register to clearing (ACK:ing) the edge
IRQ which makes it possible for a new IRQ to be handled as soon as
that has happened, and those need to use handle_edge_irq() for edge IRQs
and handle_level_irq() for level IRQs, with the side effect that the edge
IRQ path will additionally call the .irq_ack() callback on the irqchip
when handle_edge_irq() is used. In this case we set handle_bad_irq()
as default handler and set up the right handler i .set_type().

Look at drivers/gpio/gpio-pl061.c for an example.

If you DON'T have a special edge ACK register, keep it like this.

> +static irqreturn_t dmec_gpio_irq_handler(int irq, void *dev_id)
> +{
> +       struct dmec_gpio_priv *p = dev_id;
> +       struct irq_domain *d = p->gpio_chip.irqdomain;
> +       unsigned int irqs_handled = 0;
> +       unsigned int val = 0, stat = 0;
> +
> +       regmap_read(p->regmap, DMEC_GPIO_IRQSTA_OFFSET(p), &val);
> +       stat = val;
> +       while (stat) {
> +               int line = __ffs(stat);
> +               int child_irq = irq_find_mapping(d, line);
> +
> +               handle_nested_irq(child_irq);
> +               stat &= ~(BIT(line));
> +               irqs_handled++;
> +       }

I think you should re-read the status register in the loop. An IRQ may
appear while you are reading.

> +static int dmec_gpio_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct dmec_gpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
> +       struct dmec_gpio_priv *priv;
> +       struct gpio_chip *gpio_chip;
> +       struct irq_chip *irq_chip;
> +       int ret = 0;
> +
> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       priv->regmap = dmec_get_regmap(pdev->dev.parent);

Do you really need a special accessor function to get the regmap like this?
If you just use syscon you get these kind of helpers for free. I don't know
if you can use syscon though, just a suggestion.

> +       priv->chip_num = pdata->chip_num;

As mentioned, why not just use pdev->id

> +       gpio_chip = &priv->gpio_chip;
> +       gpio_chip->label = "gpio-dmec";

You set the label

> +       gpio_chip->owner = THIS_MODULE;
> +       gpio_chip->parent = dev;
> +       gpio_chip->label = dev_name(dev);

You set the label again?

> +       gpio_chip->can_sleep = true;
> +
> +       gpio_chip->base = pdata->gpio_base;

NAK. Use -1

> +#ifdef CONFIG_PM
> +static int dmec_gpio_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +       struct dmec_gpio_priv *p = platform_get_drvdata(pdev);
> +       struct regmap *regmap = p->regmap;
> +       struct dmec_reg_ctx *ctx = &p->regs;
> +
> +       regmap_read(regmap, DMEC_GPIO_BASE(p), &ctx->dat);
> +       regmap_read(regmap, DMEC_GPIO_DIR_OFFSET(p), &ctx->dir);
> +       regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), &ctx->imask);
> +       regmap_read(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p), &ctx->emask[0]);
> +       regmap_read(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p) + 1, &ctx->emask[1]);
> +       regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), &ctx->icfg[0]);
> +       regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p) + 1, &ctx->icfg[1]);
> +
> +       devm_free_irq(&pdev->dev, p->irq, p);

That seems a bit violent.

> +static int dmec_gpio_resume(struct platform_device *pdev)
> +{
> +       struct dmec_gpio_priv *p = platform_get_drvdata(pdev);
> +       struct regmap *regmap = p->regmap;
> +       struct dmec_reg_ctx *ctx = &p->regs;
> +       int ret;
> +
> +       regmap_write(regmap, DMEC_GPIO_BASE(p), ctx->dat);
> +       regmap_write(regmap, DMEC_GPIO_DIR_OFFSET(p), ctx->dir);
> +       regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), ctx->icfg[0]);
> +       regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p) + 1, ctx->icfg[1]);
> +       regmap_write(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p), ctx->emask[0]);
> +       regmap_write(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p) + 1, ctx->emask[1]);
> +       regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), ctx->imask);
> +       regmap_write(regmap, DMEC_GPIO_EVTSTA_OFFSET(p), 0xff);
> +
> +       ret = devm_request_threaded_irq(&pdev->dev, p->irq,
> +                                       NULL, dmec_gpio_irq_handler,
> +                                       IRQF_ONESHOT | IRQF_SHARED,
> +                                       dev_name(&pdev->dev), p);
> +       if (ret)
> +               dev_err(&pdev->dev, "unable to get irq: %d\n", ret);

Re-requesting the IRQ for every suspend/resume cycle?

That's not right.

Look into the wakeup API. You need to tell the core whether this IRQ
should be masked during suspend or not, the default is to mask it I think.

This is too big hammer to solve that.

> +static struct platform_driver dmec_gpio_driver = {
> +       .driver = {
> +               .name = "dmec-gpio",
> +               .owner = THIS_MODULE,
> +       },
> +       .probe = dmec_gpio_probe,
> +       .remove = dmec_gpio_remove,
> +       .suspend = dmec_gpio_suspend,
> +       .resume = dmec_gpio_resume,
> +};

Don't use the legacy suspend/resume callbacks.

Use the DEV_PM_OPS directly in .pm in .driver above. It should
work the same.

(You probably need to fix this on the other patches too.)

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC PATCH 3/5] gpio-dmec: gpio support for dmec
@ 2016-10-29  9:05       ` Linus Walleij
  0 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2016-10-29  9:05 UTC (permalink / raw)
  To: Zahari Doychev
  Cc: linux-kernel, Greg KH, Lee Jones, Wolfram Sang, Wim Van Sebroeck,
	Guenter Roeck, linux-i2c, linux-gpio, Alexandre Courbot,
	linux-watchdog

On Thu, Oct 27, 2016 at 12:47 PM, Zahari Doychev
<zahari.doychev@linux.com> wrote:

> This is support for the gpio functionality found on the Data Modul embedded
> controllers
>
> Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
> ---
>  drivers/staging/dmec/Kconfig     |  10 +-
>  drivers/staging/dmec/Makefile    |   1 +-
>  drivers/staging/dmec/dmec.h      |   5 +-
>  drivers/staging/dmec/gpio-dmec.c | 390 ++++++++++++++++++++++++++++++++-

I guess Greg has already asked, but why is this in staging?
The driver doesn't seem very complex or anything, it would not
be overly troublesome to get it into the proper kernel subsystems.

> +config GPIO_DMEC
> +       tristate "Data Modul GPIO"
> +       depends on MFD_DMEC && GPIOLIB

So the depends on GPIOLIB can go away if you just put it into
drivers/gpio with the rest.

> +struct dmec_gpio_platform_data {
> +       int gpio_base;

NAK, always use -1. No hardcoding the GPIO base other than on
legacy systems.

> +       int chip_num;

I suspect you may not need this either. struct platform_device
already contains a ->id field, just use that when instantiating
your driver if you need an instance number.

So I think you need zero platform data for this.

> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/errno.h>
> +#include <linux/acpi.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/gpio.h>

You should only need <linux/gpio/driver.h>

> +#ifdef CONFIG_PM
> +struct dmec_reg_ctx {
> +       u32 dat;
> +       u32 dir;
> +       u32 imask;
> +       u32 icfg[2];
> +       u32 emask[2];
> +};
> +#endif
> +
> +struct dmec_gpio_priv {
> +       struct regmap *regmap;
> +       struct gpio_chip gpio_chip;
> +       struct irq_chip irq_chip;
> +       unsigned int chip_num;
> +       unsigned int irq;
> +       u8 ver;
> +#ifdef CONFIG_PM
> +       struct dmec_reg_ctx regs;
> +#endif
> +};

The #ifdef for saving the state is a bit kludgy. Can't you just have it there
all the time? Or is this a footprint-sensitive system?

> +static int dmec_gpio_get(struct gpio_chip *gc, unsigned int offset)
> +{
> +       struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
> +                                                  gpio_chip);

Use the new pattern with

struct dmec_gpio_priv *priv = gpiochip_get_data(gc);

This needs you to use devm_gpiochip_add_data() below.

> +static int dmec_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +       struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
> +                                                  gpio_chip);
> +       struct regmap *regmap = priv->regmap;
> +       unsigned int offset, mask, val;
> +
> +       offset = DMEC_GPIO_IRQTYPE_OFFSET(priv) + (d->hwirq >> 2);
> +       mask = ((d->hwirq & 3) << 1);
> +
> +       regmap_read(regmap, offset, &val);
> +
> +       val &= ~(3 << mask);
> +       switch (type & IRQ_TYPE_SENSE_MASK) {
> +       case IRQ_TYPE_LEVEL_LOW:
> +               break;
> +       case IRQ_TYPE_EDGE_RISING:
> +               val |= (1 << mask);
> +               break;
> +       case IRQ_TYPE_EDGE_FALLING:
> +               val |= (2 << mask);
> +               break;
> +       case IRQ_TYPE_EDGE_BOTH:
> +               val |= (3 << mask);
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       regmap_write(regmap, offset, val);
> +
> +       return 0;
> +}

This chip uses handle_simple_irq() which is fine if the chip really has no
edge detector ACK register.

What some chips have is a special register to clearing (ACK:ing) the edge
IRQ which makes it possible for a new IRQ to be handled as soon as
that has happened, and those need to use handle_edge_irq() for edge IRQs
and handle_level_irq() for level IRQs, with the side effect that the edge
IRQ path will additionally call the .irq_ack() callback on the irqchip
when handle_edge_irq() is used. In this case we set handle_bad_irq()
as default handler and set up the right handler i .set_type().

Look at drivers/gpio/gpio-pl061.c for an example.

If you DON'T have a special edge ACK register, keep it like this.

> +static irqreturn_t dmec_gpio_irq_handler(int irq, void *dev_id)
> +{
> +       struct dmec_gpio_priv *p = dev_id;
> +       struct irq_domain *d = p->gpio_chip.irqdomain;
> +       unsigned int irqs_handled = 0;
> +       unsigned int val = 0, stat = 0;
> +
> +       regmap_read(p->regmap, DMEC_GPIO_IRQSTA_OFFSET(p), &val);
> +       stat = val;
> +       while (stat) {
> +               int line = __ffs(stat);
> +               int child_irq = irq_find_mapping(d, line);
> +
> +               handle_nested_irq(child_irq);
> +               stat &= ~(BIT(line));
> +               irqs_handled++;
> +       }

I think you should re-read the status register in the loop. An IRQ may
appear while you are reading.

> +static int dmec_gpio_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct dmec_gpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
> +       struct dmec_gpio_priv *priv;
> +       struct gpio_chip *gpio_chip;
> +       struct irq_chip *irq_chip;
> +       int ret = 0;
> +
> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       priv->regmap = dmec_get_regmap(pdev->dev.parent);

Do you really need a special accessor function to get the regmap like this?
If you just use syscon you get these kind of helpers for free. I don't know
if you can use syscon though, just a suggestion.

> +       priv->chip_num = pdata->chip_num;

As mentioned, why not just use pdev->id

> +       gpio_chip = &priv->gpio_chip;
> +       gpio_chip->label = "gpio-dmec";

You set the label

> +       gpio_chip->owner = THIS_MODULE;
> +       gpio_chip->parent = dev;
> +       gpio_chip->label = dev_name(dev);

You set the label again?

> +       gpio_chip->can_sleep = true;
> +
> +       gpio_chip->base = pdata->gpio_base;

NAK. Use -1

> +#ifdef CONFIG_PM
> +static int dmec_gpio_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +       struct dmec_gpio_priv *p = platform_get_drvdata(pdev);
> +       struct regmap *regmap = p->regmap;
> +       struct dmec_reg_ctx *ctx = &p->regs;
> +
> +       regmap_read(regmap, DMEC_GPIO_BASE(p), &ctx->dat);
> +       regmap_read(regmap, DMEC_GPIO_DIR_OFFSET(p), &ctx->dir);
> +       regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), &ctx->imask);
> +       regmap_read(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p), &ctx->emask[0]);
> +       regmap_read(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p) + 1, &ctx->emask[1]);
> +       regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), &ctx->icfg[0]);
> +       regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p) + 1, &ctx->icfg[1]);
> +
> +       devm_free_irq(&pdev->dev, p->irq, p);

That seems a bit violent.

> +static int dmec_gpio_resume(struct platform_device *pdev)
> +{
> +       struct dmec_gpio_priv *p = platform_get_drvdata(pdev);
> +       struct regmap *regmap = p->regmap;
> +       struct dmec_reg_ctx *ctx = &p->regs;
> +       int ret;
> +
> +       regmap_write(regmap, DMEC_GPIO_BASE(p), ctx->dat);
> +       regmap_write(regmap, DMEC_GPIO_DIR_OFFSET(p), ctx->dir);
> +       regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), ctx->icfg[0]);
> +       regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p) + 1, ctx->icfg[1]);
> +       regmap_write(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p), ctx->emask[0]);
> +       regmap_write(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p) + 1, ctx->emask[1]);
> +       regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), ctx->imask);
> +       regmap_write(regmap, DMEC_GPIO_EVTSTA_OFFSET(p), 0xff);
> +
> +       ret = devm_request_threaded_irq(&pdev->dev, p->irq,
> +                                       NULL, dmec_gpio_irq_handler,
> +                                       IRQF_ONESHOT | IRQF_SHARED,
> +                                       dev_name(&pdev->dev), p);
> +       if (ret)
> +               dev_err(&pdev->dev, "unable to get irq: %d\n", ret);

Re-requesting the IRQ for every suspend/resume cycle?

That's not right.

Look into the wakeup API. You need to tell the core whether this IRQ
should be masked during suspend or not, the default is to mask it I think.

This is too big hammer to solve that.

> +static struct platform_driver dmec_gpio_driver = {
> +       .driver = {
> +               .name = "dmec-gpio",
> +               .owner = THIS_MODULE,
> +       },
> +       .probe = dmec_gpio_probe,
> +       .remove = dmec_gpio_remove,
> +       .suspend = dmec_gpio_suspend,
> +       .resume = dmec_gpio_resume,
> +};

Don't use the legacy suspend/resume callbacks.

Use the DEV_PM_OPS directly in .pm in .driver above. It should
work the same.

(You probably need to fix this on the other patches too.)

Yours,
Linus Walleij

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

* Re: [RFC PATCH 3/5] gpio-dmec: gpio support for dmec
  2016-10-29  9:05       ` Linus Walleij
  (?)
@ 2016-10-31  8:50       ` Zahari Doychev
       [not found]         ` <20161031085044.GC9050-S5msFFqROX52XvNHrDenQA@public.gmane.org>
  -1 siblings, 1 reply; 13+ messages in thread
From: Zahari Doychev @ 2016-10-31  8:50 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-kernel, Greg KH, Lee Jones, Wolfram Sang, Wim Van Sebroeck,
	Guenter Roeck, linux-i2c, linux-gpio, Alexandre Courbot,
	linux-watchdog

On Sat, Oct 29, 2016 at 11:05:29AM +0200, Linus Walleij wrote:
> On Thu, Oct 27, 2016 at 12:47 PM, Zahari Doychev
> <zahari.doychev@linux.com> wrote:
> 
> > This is support for the gpio functionality found on the Data Modul embedded
> > controllers
> >
> > Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
> > ---
> >  drivers/staging/dmec/Kconfig     |  10 +-
> >  drivers/staging/dmec/Makefile    |   1 +-
> >  drivers/staging/dmec/dmec.h      |   5 +-
> >  drivers/staging/dmec/gpio-dmec.c | 390 ++++++++++++++++++++++++++++++++-
> 

Thanks for the review.

> I guess Greg has already asked, but why is this in staging?
> The driver doesn't seem very complex or anything, it would not
> be overly troublesome to get it into the proper kernel subsystems.

I was unsure what was the right way to go with this. Next time I will move it
out of staging.

> 
> > +config GPIO_DMEC
> > +       tristate "Data Modul GPIO"
> > +       depends on MFD_DMEC && GPIOLIB
> 
> So the depends on GPIOLIB can go away if you just put it into
> drivers/gpio with the rest.
> 
> > +struct dmec_gpio_platform_data {
> > +       int gpio_base;
> 
> NAK, always use -1. No hardcoding the GPIO base other than on
> legacy systems.
> 
> > +       int chip_num;
> 
> I suspect you may not need this either. struct platform_device
> already contains a ->id field, just use that when instantiating
> your driver if you need an instance number.
> 
> So I think you need zero platform data for this.
> 
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/io.h>
> > +#include <linux/slab.h>
> > +#include <linux/errno.h>
> > +#include <linux/acpi.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/gpio.h>
> 
> You should only need <linux/gpio/driver.h>
> 
> > +#ifdef CONFIG_PM
> > +struct dmec_reg_ctx {
> > +       u32 dat;
> > +       u32 dir;
> > +       u32 imask;
> > +       u32 icfg[2];
> > +       u32 emask[2];
> > +};
> > +#endif
> > +
> > +struct dmec_gpio_priv {
> > +       struct regmap *regmap;
> > +       struct gpio_chip gpio_chip;
> > +       struct irq_chip irq_chip;
> > +       unsigned int chip_num;
> > +       unsigned int irq;
> > +       u8 ver;
> > +#ifdef CONFIG_PM
> > +       struct dmec_reg_ctx regs;
> > +#endif
> > +};
> 
> The #ifdef for saving the state is a bit kludgy. Can't you just have it there
> all the time? Or is this a footprint-sensitive system?

It will be no problem to have it always there.

> 
> > +static int dmec_gpio_get(struct gpio_chip *gc, unsigned int offset)
> > +{
> > +       struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
> > +                                                  gpio_chip);
> 
> Use the new pattern with
> 
> struct dmec_gpio_priv *priv = gpiochip_get_data(gc);
> 
> This needs you to use devm_gpiochip_add_data() below.
> 
> > +static int dmec_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> > +{
> > +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> > +       struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
> > +                                                  gpio_chip);
> > +       struct regmap *regmap = priv->regmap;
> > +       unsigned int offset, mask, val;
> > +
> > +       offset = DMEC_GPIO_IRQTYPE_OFFSET(priv) + (d->hwirq >> 2);
> > +       mask = ((d->hwirq & 3) << 1);
> > +
> > +       regmap_read(regmap, offset, &val);
> > +
> > +       val &= ~(3 << mask);
> > +       switch (type & IRQ_TYPE_SENSE_MASK) {
> > +       case IRQ_TYPE_LEVEL_LOW:
> > +               break;
> > +       case IRQ_TYPE_EDGE_RISING:
> > +               val |= (1 << mask);
> > +               break;
> > +       case IRQ_TYPE_EDGE_FALLING:
> > +               val |= (2 << mask);
> > +               break;
> > +       case IRQ_TYPE_EDGE_BOTH:
> > +               val |= (3 << mask);
> > +               break;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +
> > +       regmap_write(regmap, offset, val);
> > +
> > +       return 0;
> > +}
> 
> This chip uses handle_simple_irq() which is fine if the chip really has no
> edge detector ACK register.
> 
> What some chips have is a special register to clearing (ACK:ing) the edge
> IRQ which makes it possible for a new IRQ to be handled as soon as
> that has happened, and those need to use handle_edge_irq() for edge IRQs
> and handle_level_irq() for level IRQs, with the side effect that the edge
> IRQ path will additionally call the .irq_ack() callback on the irqchip
> when handle_edge_irq() is used. In this case we set handle_bad_irq()
> as default handler and set up the right handler i .set_type().
> 
> Look at drivers/gpio/gpio-pl061.c for an example.
> 
> If you DON'T have a special edge ACK register, keep it like this.

What is the difference between this special edge ACK register and the normal
interrupt ACK register? I think I do not have such dedicated register
but I will have to check this again.

> 
> > +static irqreturn_t dmec_gpio_irq_handler(int irq, void *dev_id)
> > +{
> > +       struct dmec_gpio_priv *p = dev_id;
> > +       struct irq_domain *d = p->gpio_chip.irqdomain;
> > +       unsigned int irqs_handled = 0;
> > +       unsigned int val = 0, stat = 0;
> > +
> > +       regmap_read(p->regmap, DMEC_GPIO_IRQSTA_OFFSET(p), &val);
> > +       stat = val;
> > +       while (stat) {
> > +               int line = __ffs(stat);
> > +               int child_irq = irq_find_mapping(d, line);
> > +
> > +               handle_nested_irq(child_irq);
> > +               stat &= ~(BIT(line));
> > +               irqs_handled++;
> > +       }
> 
> I think you should re-read the status register in the loop. An IRQ may
> appear while you are reading.

The irqs are signalled over the SERIRQ of the LPC bus. Wnen irq comes in, it
should be acknowledged then the embedded controller can trigger the next one.
Nevertheless I will look at it and fix it if necessary.

> 
> > +static int dmec_gpio_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       struct dmec_gpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
> > +       struct dmec_gpio_priv *priv;
> > +       struct gpio_chip *gpio_chip;
> > +       struct irq_chip *irq_chip;
> > +       int ret = 0;
> > +
> > +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +       if (!priv)
> > +               return -ENOMEM;
> > +
> > +       priv->regmap = dmec_get_regmap(pdev->dev.parent);
> 
> Do you really need a special accessor function to get the regmap like this?
> If you just use syscon you get these kind of helpers for free. I don't know
> if you can use syscon though, just a suggestion.

The I/O memory is mapped in the mfd driver. The addresing mode is also set
there which should be the same for all child devices. So in this way I have
dependcy between the mfd and the rest of the drivers. I am not sure that I
can use syscon as the driver is getting its resources from acpi.

> 
> > +       priv->chip_num = pdata->chip_num;
> 
> As mentioned, why not just use pdev->id
> 
> > +       gpio_chip = &priv->gpio_chip;
> > +       gpio_chip->label = "gpio-dmec";
> 
> You set the label
> 
> > +       gpio_chip->owner = THIS_MODULE;
> > +       gpio_chip->parent = dev;
> > +       gpio_chip->label = dev_name(dev);
> 
> You set the label again?
> 
> > +       gpio_chip->can_sleep = true;
> > +
> > +       gpio_chip->base = pdata->gpio_base;
> 
> NAK. Use -1
> 
> > +#ifdef CONFIG_PM
> > +static int dmec_gpio_suspend(struct platform_device *pdev, pm_message_t state)
> > +{
> > +       struct dmec_gpio_priv *p = platform_get_drvdata(pdev);
> > +       struct regmap *regmap = p->regmap;
> > +       struct dmec_reg_ctx *ctx = &p->regs;
> > +
> > +       regmap_read(regmap, DMEC_GPIO_BASE(p), &ctx->dat);
> > +       regmap_read(regmap, DMEC_GPIO_DIR_OFFSET(p), &ctx->dir);
> > +       regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), &ctx->imask);
> > +       regmap_read(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p), &ctx->emask[0]);
> > +       regmap_read(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p) + 1, &ctx->emask[1]);
> > +       regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), &ctx->icfg[0]);
> > +       regmap_read(regmap, DMEC_GPIO_IRQCFG_OFFSET(p) + 1, &ctx->icfg[1]);
> > +
> > +       devm_free_irq(&pdev->dev, p->irq, p);
> 
> That seems a bit violent.
> 
> > +static int dmec_gpio_resume(struct platform_device *pdev)
> > +{
> > +       struct dmec_gpio_priv *p = platform_get_drvdata(pdev);
> > +       struct regmap *regmap = p->regmap;
> > +       struct dmec_reg_ctx *ctx = &p->regs;
> > +       int ret;
> > +
> > +       regmap_write(regmap, DMEC_GPIO_BASE(p), ctx->dat);
> > +       regmap_write(regmap, DMEC_GPIO_DIR_OFFSET(p), ctx->dir);
> > +       regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), ctx->icfg[0]);
> > +       regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p) + 1, ctx->icfg[1]);
> > +       regmap_write(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p), ctx->emask[0]);
> > +       regmap_write(regmap, DMEC_GPIO_IRQTYPE_OFFSET(p) + 1, ctx->emask[1]);
> > +       regmap_write(regmap, DMEC_GPIO_IRQCFG_OFFSET(p), ctx->imask);
> > +       regmap_write(regmap, DMEC_GPIO_EVTSTA_OFFSET(p), 0xff);
> > +
> > +       ret = devm_request_threaded_irq(&pdev->dev, p->irq,
> > +                                       NULL, dmec_gpio_irq_handler,
> > +                                       IRQF_ONESHOT | IRQF_SHARED,
> > +                                       dev_name(&pdev->dev), p);
> > +       if (ret)
> > +               dev_err(&pdev->dev, "unable to get irq: %d\n", ret);
> 
> Re-requesting the IRQ for every suspend/resume cycle?
> 
> That's not right.
> 
> Look into the wakeup API. You need to tell the core whether this IRQ
> should be masked during suspend or not, the default is to mask it I think.
> 
> This is too big hammer to solve that.
> 
> > +static struct platform_driver dmec_gpio_driver = {
> > +       .driver = {
> > +               .name = "dmec-gpio",
> > +               .owner = THIS_MODULE,
> > +       },
> > +       .probe = dmec_gpio_probe,
> > +       .remove = dmec_gpio_remove,
> > +       .suspend = dmec_gpio_suspend,
> > +       .resume = dmec_gpio_resume,
> > +};
> 
> Don't use the legacy suspend/resume callbacks.
> 
> Use the DEV_PM_OPS directly in .pm in .driver above. It should
> work the same.
> 
> (You probably need to fix this on the other patches too.)
> 

Thanks I will change this.

Regards Zahari

> Yours,
> Linus Walleij

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

* Re: [RFC PATCH 3/5] gpio-dmec: gpio support for dmec
  2016-10-31  8:50       ` Zahari Doychev
@ 2016-10-31 12:26             ` Linus Walleij
  0 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2016-10-31 12:26 UTC (permalink / raw)
  To: Zahari Doychev
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg KH, Lee Jones,
	Wolfram Sang, Wim Van Sebroeck, Guenter Roeck,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, Alexandre Courbot,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA

On Mon, Oct 31, 2016 at 9:50 AM, Zahari Doychev
<zahari.doychev-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote:
> On Sat, Oct 29, 2016 at 11:05:29AM +0200, Linus Walleij wrote:

>> > +static int dmec_gpio_irq_set_type(struct irq_data *d, unsigned int type)
>> > +{
>> > +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
>> > +       struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
>> > +                                                  gpio_chip);
>> > +       struct regmap *regmap = priv->regmap;
>> > +       unsigned int offset, mask, val;
>> > +
>> > +       offset = DMEC_GPIO_IRQTYPE_OFFSET(priv) + (d->hwirq >> 2);
>> > +       mask = ((d->hwirq & 3) << 1);
>> > +
>> > +       regmap_read(regmap, offset, &val);
>> > +
>> > +       val &= ~(3 << mask);
>> > +       switch (type & IRQ_TYPE_SENSE_MASK) {
>> > +       case IRQ_TYPE_LEVEL_LOW:
>> > +               break;
>> > +       case IRQ_TYPE_EDGE_RISING:
>> > +               val |= (1 << mask);
>> > +               break;
>> > +       case IRQ_TYPE_EDGE_FALLING:
>> > +               val |= (2 << mask);
>> > +               break;
>> > +       case IRQ_TYPE_EDGE_BOTH:
>> > +               val |= (3 << mask);
>> > +               break;
>> > +       default:
>> > +               return -EINVAL;
>> > +       }
>> > +
>> > +       regmap_write(regmap, offset, val);
>> > +
>> > +       return 0;
>> > +}
>>
>> This chip uses handle_simple_irq() which is fine if the chip really has no
>> edge detector ACK register.
>>
>> What some chips have is a special register to clearing (ACK:ing) the edge
>> IRQ which makes it possible for a new IRQ to be handled as soon as
>> that has happened, and those need to use handle_edge_irq() for edge IRQs
>> and handle_level_irq() for level IRQs, with the side effect that the edge
>> IRQ path will additionally call the .irq_ack() callback on the irqchip
>> when handle_edge_irq() is used. In this case we set handle_bad_irq()
>> as default handler and set up the right handler i .set_type().
>>
>> Look at drivers/gpio/gpio-pl061.c for an example.
>>
>> If you DON'T have a special edge ACK register, keep it like this.
>
> What is the difference between this special edge ACK register and the normal
> interrupt ACK register?

With level interrupts there is seldom use of any ACK register.
They will by definition just hold the line low until the clients stop
asserting their IRQs.

With edge triggered interrupts, you can have a transient event so
that you need an ACK register to tell the hardware that you have
seen and acknowledged this IRQ, so it can go ahead and fire a
second IRQ on the same line while you are still processing the
first one.

> I think I do not have such dedicated register
> but I will have to check this again.

Read the documentation for the register(s) and see what the
use case is.

>> > +static int dmec_gpio_probe(struct platform_device *pdev)
>> > +{
>> > +       struct device *dev = &pdev->dev;
>> > +       struct dmec_gpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
>> > +       struct dmec_gpio_priv *priv;
>> > +       struct gpio_chip *gpio_chip;
>> > +       struct irq_chip *irq_chip;
>> > +       int ret = 0;
>> > +
>> > +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> > +       if (!priv)
>> > +               return -ENOMEM;
>> > +
>> > +       priv->regmap = dmec_get_regmap(pdev->dev.parent);
>>
>> Do you really need a special accessor function to get the regmap like this?
>> If you just use syscon you get these kind of helpers for free. I don't know
>> if you can use syscon though, just a suggestion.
>
> The I/O memory is mapped in the mfd driver. The addresing mode is also set
> there which should be the same for all child devices. So in this way I have
> dependcy between the mfd and the rest of the drivers. I am not sure that I
> can use syscon as the driver is getting its resources from acpi.

OK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC PATCH 3/5] gpio-dmec: gpio support for dmec
@ 2016-10-31 12:26             ` Linus Walleij
  0 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2016-10-31 12:26 UTC (permalink / raw)
  To: Zahari Doychev
  Cc: linux-kernel, Greg KH, Lee Jones, Wolfram Sang, Wim Van Sebroeck,
	Guenter Roeck, linux-i2c, linux-gpio, Alexandre Courbot,
	linux-watchdog

On Mon, Oct 31, 2016 at 9:50 AM, Zahari Doychev
<zahari.doychev@linux.com> wrote:
> On Sat, Oct 29, 2016 at 11:05:29AM +0200, Linus Walleij wrote:

>> > +static int dmec_gpio_irq_set_type(struct irq_data *d, unsigned int type)
>> > +{
>> > +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
>> > +       struct dmec_gpio_priv *priv = container_of(gc, struct dmec_gpio_priv,
>> > +                                                  gpio_chip);
>> > +       struct regmap *regmap = priv->regmap;
>> > +       unsigned int offset, mask, val;
>> > +
>> > +       offset = DMEC_GPIO_IRQTYPE_OFFSET(priv) + (d->hwirq >> 2);
>> > +       mask = ((d->hwirq & 3) << 1);
>> > +
>> > +       regmap_read(regmap, offset, &val);
>> > +
>> > +       val &= ~(3 << mask);
>> > +       switch (type & IRQ_TYPE_SENSE_MASK) {
>> > +       case IRQ_TYPE_LEVEL_LOW:
>> > +               break;
>> > +       case IRQ_TYPE_EDGE_RISING:
>> > +               val |= (1 << mask);
>> > +               break;
>> > +       case IRQ_TYPE_EDGE_FALLING:
>> > +               val |= (2 << mask);
>> > +               break;
>> > +       case IRQ_TYPE_EDGE_BOTH:
>> > +               val |= (3 << mask);
>> > +               break;
>> > +       default:
>> > +               return -EINVAL;
>> > +       }
>> > +
>> > +       regmap_write(regmap, offset, val);
>> > +
>> > +       return 0;
>> > +}
>>
>> This chip uses handle_simple_irq() which is fine if the chip really has no
>> edge detector ACK register.
>>
>> What some chips have is a special register to clearing (ACK:ing) the edge
>> IRQ which makes it possible for a new IRQ to be handled as soon as
>> that has happened, and those need to use handle_edge_irq() for edge IRQs
>> and handle_level_irq() for level IRQs, with the side effect that the edge
>> IRQ path will additionally call the .irq_ack() callback on the irqchip
>> when handle_edge_irq() is used. In this case we set handle_bad_irq()
>> as default handler and set up the right handler i .set_type().
>>
>> Look at drivers/gpio/gpio-pl061.c for an example.
>>
>> If you DON'T have a special edge ACK register, keep it like this.
>
> What is the difference between this special edge ACK register and the normal
> interrupt ACK register?

With level interrupts there is seldom use of any ACK register.
They will by definition just hold the line low until the clients stop
asserting their IRQs.

With edge triggered interrupts, you can have a transient event so
that you need an ACK register to tell the hardware that you have
seen and acknowledged this IRQ, so it can go ahead and fire a
second IRQ on the same line while you are still processing the
first one.

> I think I do not have such dedicated register
> but I will have to check this again.

Read the documentation for the register(s) and see what the
use case is.

>> > +static int dmec_gpio_probe(struct platform_device *pdev)
>> > +{
>> > +       struct device *dev = &pdev->dev;
>> > +       struct dmec_gpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
>> > +       struct dmec_gpio_priv *priv;
>> > +       struct gpio_chip *gpio_chip;
>> > +       struct irq_chip *irq_chip;
>> > +       int ret = 0;
>> > +
>> > +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> > +       if (!priv)
>> > +               return -ENOMEM;
>> > +
>> > +       priv->regmap = dmec_get_regmap(pdev->dev.parent);
>>
>> Do you really need a special accessor function to get the regmap like this?
>> If you just use syscon you get these kind of helpers for free. I don't know
>> if you can use syscon though, just a suggestion.
>
> The I/O memory is mapped in the mfd driver. The addresing mode is also set
> there which should be the same for all child devices. So in this way I have
> dependcy between the mfd and the rest of the drivers. I am not sure that I
> can use syscon as the driver is getting its resources from acpi.

OK.

Yours,
Linus Walleij

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

end of thread, other threads:[~2016-10-31 12:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-27 10:47 [RFC PATCH 0/5] add support for DMO embedded controller Zahari Doychev
2016-10-27 10:47 ` [RFC PATCH 1/5] dmec: add DMO mfd driver Zahari Doychev
2016-10-27 10:47 ` [RFC PATCH 2/5] i2c-dmec: add i2c bus support for dmec Zahari Doychev
2016-10-27 10:47 ` [RFC PATCH 3/5] gpio-dmec: gpio " Zahari Doychev
     [not found]   ` <fe8ac70204de48edd8a3da8a783b10810f3d4ca1.1477564996.git-series.zahari.doychev-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
2016-10-29  9:05     ` Linus Walleij
2016-10-29  9:05       ` Linus Walleij
2016-10-31  8:50       ` Zahari Doychev
     [not found]         ` <20161031085044.GC9050-S5msFFqROX52XvNHrDenQA@public.gmane.org>
2016-10-31 12:26           ` Linus Walleij
2016-10-31 12:26             ` Linus Walleij
2016-10-27 10:47 ` [RFC PATCH 4/5] wdt-dmec: watchdog " Zahari Doychev
2016-10-27 10:47 ` [RFC PATCH 5/5] rtm-dmec: running time meter support Zahari Doychev
2016-10-27 15:01 ` [RFC PATCH 0/5] add support for DMO embedded controller Greg KH
2016-10-27 15:54   ` Zahari Doychev

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.