[PATCH v1 0/5] siox: add support for new bus type

[PATCH v1 0/5] siox: add support for new bus type

[Uwe Kleine-König]

Hello,

this series adds a new bus type for SIOX devices. I already sent a
similar series a few years ago; since then the subsystem got more mature
and is operated with this code base in the field.

For the device tree doc changes I'm unsure if I should split them in
separate patches, please advice.

Best regards
Uwe

Uwe Kleine-König (5):
  siox: new driver framework for eckelmann SIOX
  siox: add support for tracing
  siox: add gpio bus driver
  gpio: new driver to work with a 8x12 siox
  MAINTAINERS: Add entry for SIOX

 .../bindings/siox/eckelmann,siox-gpio.txt          |  19 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 MAINTAINERS                                        |   8 +
 drivers/Kconfig                                    |   2 +
 drivers/Makefile                                   |   1 +
 drivers/gpio/Kconfig                               |   8 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-siox.c                           | 293 +++++++
 drivers/siox/Kconfig                               |  18 +
 drivers/siox/Makefile                              |   2 +
 drivers/siox/siox-bus-gpio.c                       | 172 ++++
 drivers/siox/siox-core.c                           | 937 +++++++++++++++++++++
 drivers/siox/siox.h                                |  49 ++
 include/linux/siox.h                               |  77 ++
 include/trace/events/siox.h                        |  66 ++
 15 files changed, 1654 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/siox/eckelmann,siox-gpio.txt
 create mode 100644 drivers/gpio/gpio-siox.c
 create mode 100644 drivers/siox/Kconfig
 create mode 100644 drivers/siox/Makefile
 create mode 100644 drivers/siox/siox-bus-gpio.c
 create mode 100644 drivers/siox/siox-core.c
 create mode 100644 drivers/siox/siox.h
 create mode 100644 include/linux/siox.h
 create mode 100644 include/trace/events/siox.h

-- 
2.11.0

[PATCH v1 1/5] siox: new driver framework for eckelmann SIOX

[Uwe Kleine-König]

SIOX is a bus system invented at Eckelmann AG to control their building
management and refrigeration systems. Traditionally the bus was
implemented on custom microcontrollers, today Linux based machines are
in use, too.

The topology on a SIOX bus looks as follows:

      ,------->--DCLK-->---------------+----------------------.
      ^                                v                      v
 ,--------.                ,----------------------.       ,------
 |        |                |   ,--------------.   |       |
 |        |--->--DOUT-->---|->-|shift register|->-|--->---|
 |        |                |   `--------------'   |       |
 | master |                |        device        |       |  device
 |        |                |   ,--------------.   |       |
 |        |---<--DIN---<---|-<-|shift register|-<-|---<---|
 |        |                |   `--------------'   |       |
 `--------'                `----------------------'       `------
      v                                ^                      ^
      `----------DLD-------------------+----------------------'

There are two control lines (DCLK and DLD) driven from the bus master to
all devices in parallel and two daisy chained data lines, one for input
and one for output. DCLK is the clock to shift both chains by a single
bit. On an edge of DLD the devices latch both their input and output
shift registers.

This patch adds a framework for this bus type.

Acked-by: Gavin Schenk <g.schenk@eckelmann.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/Kconfig          |   2 +
 drivers/Makefile         |   1 +
 drivers/siox/Kconfig     |   9 +
 drivers/siox/Makefile    |   1 +
 drivers/siox/siox-core.c | 924 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/siox/siox.h      |  49 +++
 include/linux/siox.h     |  77 ++++
 7 files changed, 1063 insertions(+)
 create mode 100644 drivers/siox/Kconfig
 create mode 100644 drivers/siox/Makefile
 create mode 100644 drivers/siox/siox-core.c
 create mode 100644 drivers/siox/siox.h
 create mode 100644 include/linux/siox.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 152744c5ef0f..5458b623c00c 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -211,4 +211,6 @@ source "drivers/mux/Kconfig"
 
 source "drivers/opp/Kconfig"
 
+source "drivers/siox/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 1d034b680431..c90f8acd0fd1 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -183,3 +183,4 @@ obj-$(CONFIG_FPGA)		+= fpga/
 obj-$(CONFIG_FSI)		+= fsi/
 obj-$(CONFIG_TEE)		+= tee/
 obj-$(CONFIG_MULTIPLEXER)	+= mux/
+obj-$(CONFIG_SIOX)		+= siox/
diff --git a/drivers/siox/Kconfig b/drivers/siox/Kconfig
new file mode 100644
index 000000000000..bd24d9b50dc6
--- /dev/null
+++ b/drivers/siox/Kconfig
@@ -0,0 +1,9 @@
+menuconfig SIOX
+	tristate "Eckelmann SIOX Support"
+	help
+	  SIOX stands for Serial Input Output eXtension and is a synchronous
+	  bus system invented by Eckelmann AG. It is used in their control and
+	  remote monitoring systems for commercial and industrial refrigeration
+	  to drive additional I/O units.
+
+	  Unless you know better, it is probably safe to say "no" here.
diff --git a/drivers/siox/Makefile b/drivers/siox/Makefile
new file mode 100644
index 000000000000..d55cb5e08868
--- /dev/null
+++ b/drivers/siox/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SIOX) += siox-core.o
diff --git a/drivers/siox/siox-core.c b/drivers/siox/siox-core.c
new file mode 100644
index 000000000000..8578972ba18b
--- /dev/null
+++ b/drivers/siox/siox-core.c
@@ -0,0 +1,924 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2015-2017 Pengutronix, Uwe Kleine-König <kernel@pengutronix.de>
+ */
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+
+#include "siox.h"
+
+/*
+ * The lowest bit in the SIOX status word signals if the in-device watchdog is
+ * ok. If the bit is set, the device is functional.
+ *
+ * On writing the watchdog timer is reset when this bit toggles.
+ */
+#define SIOX_STATUS_WDG			0x01
+
+/*
+ * Bits 1 to 3 of the status word read as the bitwise negation of what was
+ * clocked in before. The value clocked in is changed in each cycle and so
+ * allows to detect transmit/receive problems.
+ */
+#define SIOX_STATUS_COUNTER		0x0e
+
+/*
+ * Each Siox-Device has a 4 bit type number that is neither 0 nor 15. This is
+ * available in the upper nibble of the read status.
+ *
+ * On write these bits are DC.
+ */
+#define SIOX_STATUS_TYPE		0xf0
+
+static bool siox_is_registered;
+
+static void siox_master_lock(struct siox_master *smaster)
+{
+	mutex_lock(&smaster->lock);
+}
+
+static void siox_master_unlock(struct siox_master *smaster)
+{
+	mutex_unlock(&smaster->lock);
+}
+
+static inline u8 siox_status_clean(u8 status_read, u8 status_written)
+{
+	/*
+	 * bits 3:1 of status sample the respective bit in the status
+	 * byte written in the previous cycle but inverted. So if you wrote the
+	 * status word as 0xa before (counter = 0b101), it is expected to get
+	 * back the counter bits as 0b010.
+	 *
+	 * So given the last status written this function toggles the there
+	 * unset counter bits in the read value such that the counter bits in
+	 * the return value are all zero iff the bits were read as expected to
+	 * simplify error detection.
+	 */
+
+	return status_read ^ (~status_written & 0xe);
+}
+
+static bool siox_device_counter_error(struct siox_device *sdevice,
+				      u8 status_clean)
+{
+	return (status_clean & SIOX_STATUS_COUNTER) != 0;
+}
+
+static bool siox_device_type_error(struct siox_device *sdevice, u8 status_clean)
+{
+	u8 statustype = (status_clean & SIOX_STATUS_TYPE) >> 4;
+
+	/*
+	 * If the device knows which value the type bits should have, check
+	 * against this value otherwise just rule out the invalid values 0b0000
+	 * and 0b1111.
+	 */
+	if (sdevice->statustype) {
+		if (statustype != sdevice->statustype)
+			return true;
+	} else {
+		switch (statustype) {
+		case 0:
+		case 0xf:
+			return true;
+		}
+	}
+
+	return false;
+}
+
+static bool siox_device_wdg_error(struct siox_device *sdevice, u8 status_clean)
+{
+	return (status_clean & SIOX_STATUS_WDG) == 0;
+}
+
+/*
+ * If there is a type or counter error the device is called "unsynced".
+ */
+bool siox_device_synced(struct siox_device *sdevice)
+{
+	if (siox_device_type_error(sdevice, sdevice->status_read_clean))
+		return false;
+
+	return !siox_device_counter_error(sdevice, sdevice->status_read_clean);
+
+}
+EXPORT_SYMBOL_GPL(siox_device_synced);
+
+/*
+ * A device is called "connected" if it is synced and the watchdog is not
+ * asserted.
+ */
+bool siox_device_connected(struct siox_device *sdevice)
+{
+	if (!siox_device_synced(sdevice))
+		return false;
+
+	return !siox_device_wdg_error(sdevice, sdevice->status_read_clean);
+}
+EXPORT_SYMBOL_GPL(siox_device_connected);
+
+static void siox_poll(struct siox_master *smaster)
+{
+	struct siox_device *sdevice;
+	size_t i = smaster->setbuf_len;
+	int unsync_error = 0;
+
+	smaster->last_poll = jiffies;
+
+	/*
+	 * The counter bits change in each second cycle, the watchdog bit
+	 * toggles each time.
+	 * The counter bits hold values from [0, 6]. 7 would be possible
+	 * theoretically but the protocol designer considered that a bad idea
+	 * for reasons unknown today. (Maybe that's because then the status read
+	 * back has only zeros in the counter bits then which might be confused
+	 * with a stuck-at-0 error. But for the same reason (with s/0/1/) 0
+	 * could be skipped.)
+	 */
+	if (++smaster->status > 0x0d)
+		smaster->status = 0;
+
+	memset(smaster->buf, 0, smaster->setbuf_len);
+
+	/* prepare data pushed out to devices in buf[0..setbuf_len) */
+	list_for_each_entry(sdevice, &smaster->devices, node) {
+		struct siox_driver *sdriver =
+			to_siox_driver(sdevice->dev.driver);
+		sdevice->status_written = smaster->status;
+
+		i -= sdevice->inbytes;
+
+		/*
+		 * If the device or a previous one is unsynced, don't pet the
+		 * watchdog. This is done to ensure that the device is kept in
+		 * reset when something is wrong.
+		 */
+		if (!siox_device_synced(sdevice))
+			unsync_error = 1;
+
+		if (sdriver && !unsync_error)
+			sdriver->set_data(sdevice, sdevice->status_written,
+					  &smaster->buf[i + 1]);
+		else
+			/*
+			 * Don't trigger watchdog if there is no driver or a
+			 * sync problem
+			 */
+			sdevice->status_written &= ~SIOX_STATUS_WDG;
+
+		smaster->buf[i] = sdevice->status_written;
+	}
+
+	WARN_ON(i);
+
+	smaster->pushpull(smaster, smaster->setbuf_len, smaster->buf,
+			  smaster->getbuf_len,
+			  smaster->buf + smaster->setbuf_len);
+
+	unsync_error = 0;
+
+	/* interpret data pulled in from devices in buf[setbuf_len..] */
+	i = smaster->setbuf_len;
+	list_for_each_entry(sdevice, &smaster->devices, node) {
+		struct siox_driver *sdriver =
+			to_siox_driver(sdevice->dev.driver);
+		u8 status = smaster->buf[i + sdevice->outbytes - 1];
+		u8 status_clean;
+		u8 prev_status_clean = sdevice->status_read_clean;
+		bool synced = true;
+		bool connected = true;
+
+		if (!siox_device_synced(sdevice))
+			unsync_error = 1;
+
+		/*
+		 * If the watchdog bit wasn't toggled in this cycle, report the
+		 * watchdog as active to give a consistent view for drivers and
+		 * sysfs consumers.
+		 */
+		if (!sdriver || unsync_error)
+			status &= ~SIOX_STATUS_WDG;
+
+		status_clean =
+			siox_status_clean(status,
+					  sdevice->status_written_lastcycle);
+
+		/* Check counter bits */
+		if (siox_device_counter_error(sdevice, status_clean)) {
+			bool prev_counter_error;
+
+			synced = false;
+
+			/* only report a new error if the last cycle was ok */
+			prev_counter_error =
+				siox_device_counter_error(sdevice,
+							  prev_status_clean);
+			if (!prev_counter_error) {
+				sdevice->status_errors++;
+				sysfs_notify_dirent(sdevice->status_errors_kn);
+			}
+		}
+
+		/* Check type bits */
+		if (siox_device_type_error(sdevice, status_clean))
+			synced = false;
+
+		/* If the device is unsynced report the watchdog as active */
+		if (!synced) {
+			status &= ~SIOX_STATUS_WDG;
+			status_clean &= ~SIOX_STATUS_WDG;
+		}
+
+		if (siox_device_wdg_error(sdevice, status_clean))
+			connected = false;
+
+		/* The watchdog state changed just now */
+		if ((status_clean ^ prev_status_clean) & SIOX_STATUS_WDG) {
+			sysfs_notify_dirent(sdevice->watchdog_kn);
+
+			if (siox_device_wdg_error(sdevice, status_clean)) {
+				struct kernfs_node *wd_errs =
+					sdevice->watchdog_errors_kn;
+
+				sdevice->watchdog_errors++;
+				sysfs_notify_dirent(wd_errs);
+			}
+		}
+
+		if (connected != sdevice->connected)
+			sysfs_notify_dirent(sdevice->connected_kn);
+
+		sdevice->status_read_clean = status_clean;
+		sdevice->status_written_lastcycle = sdevice->status_written;
+		sdevice->connected = connected;
+
+		/* only give data read to driver if the device is connected */
+		if (sdriver && connected)
+			sdriver->get_data(sdevice, &smaster->buf[i]);
+
+		i += sdevice->outbytes;
+	}
+}
+
+static int siox_poll_thread(void *data)
+{
+	struct siox_master *smaster = data;
+	signed long timeout = 0;
+
+	get_device(&smaster->dev);
+
+	for (;;) {
+		if (kthread_should_stop()) {
+			put_device(&smaster->dev);
+			return 0;
+		}
+
+		siox_master_lock(smaster);
+
+		if (smaster->active) {
+			unsigned long next_poll =
+				smaster->last_poll + smaster->poll_interval;
+			if (time_is_before_eq_jiffies(next_poll))
+				siox_poll(smaster);
+
+			timeout = smaster->poll_interval -
+				(jiffies - smaster->last_poll);
+		} else {
+			timeout = MAX_SCHEDULE_TIMEOUT;
+		}
+
+		/*
+		 * Set the task to idle while holding the lock. This makes sure
+		 * that we don't sleep too long when the bus is reenabled before
+		 * schedule_timeout is reached.
+		 */
+		if (timeout > 0)
+			set_current_state(TASK_IDLE);
+
+		siox_master_unlock(smaster);
+
+		if (timeout > 0)
+			schedule_timeout(timeout);
+
+		/*
+		 * I'm not clear if/why it is important to set the state to
+		 * RUNNING again, but it fixes a "do not call blocking ops when
+		 * !TASK_RUNNING;"-warning.
+		 */
+		set_current_state(TASK_RUNNING);
+	}
+}
+
+static int __siox_start(struct siox_master *smaster)
+{
+	if (!(smaster->setbuf_len + smaster->getbuf_len))
+		return -ENODEV;
+
+	if (!smaster->buf)
+		return -ENOMEM;
+
+	if (smaster->active)
+		return 0;
+
+	smaster->active = 1;
+	wake_up_process(smaster->poll_thread);
+
+	return 1;
+}
+
+static int siox_start(struct siox_master *smaster)
+{
+	int ret;
+
+	siox_master_lock(smaster);
+	ret = __siox_start(smaster);
+	siox_master_unlock(smaster);
+
+	return ret;
+}
+
+static int __siox_stop(struct siox_master *smaster)
+{
+	if (smaster->active) {
+		struct siox_device *sdevice;
+
+		smaster->active = 0;
+
+		list_for_each_entry(sdevice, &smaster->devices, node) {
+			if (sdevice->connected)
+				sysfs_notify_dirent(sdevice->connected_kn);
+			sdevice->connected = false;
+		}
+
+		return 1;
+	}
+	return 0;
+}
+
+static int siox_stop(struct siox_master *smaster)
+{
+	int ret;
+
+	siox_master_lock(smaster);
+	ret = __siox_stop(smaster);
+	siox_master_unlock(smaster);
+
+	return ret;
+}
+
+static ssize_t type_show(struct device *dev,
+			 struct device_attribute *attr, char *buf)
+{
+	struct siox_device *sdev = to_siox_device(dev);
+
+	return sprintf(buf, "%s\n", sdev->type);
+}
+
+static DEVICE_ATTR_RO(type);
+
+static ssize_t inbytes_show(struct device *dev,
+			    struct device_attribute *attr, char *buf)
+{
+	struct siox_device *sdev = to_siox_device(dev);
+
+	return sprintf(buf, "%zu\n", sdev->inbytes);
+}
+
+static DEVICE_ATTR_RO(inbytes);
+
+static ssize_t outbytes_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct siox_device *sdev = to_siox_device(dev);
+
+	return sprintf(buf, "%zu\n", sdev->outbytes);
+}
+
+static DEVICE_ATTR_RO(outbytes);
+
+static ssize_t status_errors_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct siox_device *sdev = to_siox_device(dev);
+	unsigned int status_errors;
+
+	siox_master_lock(sdev->smaster);
+
+	status_errors = sdev->status_errors;
+
+	siox_master_unlock(sdev->smaster);
+
+	return sprintf(buf, "%u\n", status_errors);
+}
+
+static DEVICE_ATTR_RO(status_errors);
+
+static ssize_t connected_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct siox_device *sdev = to_siox_device(dev);
+	bool connected;
+
+	siox_master_lock(sdev->smaster);
+
+	connected = sdev->connected;
+
+	siox_master_unlock(sdev->smaster);
+
+	return sprintf(buf, "%u\n", connected);
+}
+
+static DEVICE_ATTR_RO(connected);
+
+static ssize_t watchdog_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct siox_device *sdev = to_siox_device(dev);
+	u8 status;
+
+	siox_master_lock(sdev->smaster);
+
+	status = sdev->status_read_clean;
+
+	siox_master_unlock(sdev->smaster);
+
+	return sprintf(buf, "%d\n", status & SIOX_STATUS_WDG);
+}
+
+static DEVICE_ATTR_RO(watchdog);
+
+static ssize_t watchdog_errors_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct siox_device *sdev = to_siox_device(dev);
+	unsigned int watchdog_errors;
+
+	siox_master_lock(sdev->smaster);
+
+	watchdog_errors = sdev->watchdog_errors;
+
+	siox_master_unlock(sdev->smaster);
+
+	return sprintf(buf, "%u\n", watchdog_errors);
+}
+
+static DEVICE_ATTR_RO(watchdog_errors);
+
+static struct attribute *siox_device_attrs[] = {
+	&dev_attr_type.attr,
+	&dev_attr_inbytes.attr,
+	&dev_attr_outbytes.attr,
+	&dev_attr_status_errors.attr,
+	&dev_attr_connected.attr,
+	&dev_attr_watchdog.attr,
+	&dev_attr_watchdog_errors.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(siox_device);
+
+static void siox_device_release(struct device *dev)
+{
+	struct siox_device *sdevice = to_siox_device(dev);
+
+	kfree(sdevice);
+}
+
+static struct device_type siox_device_type = {
+	.groups = siox_device_groups,
+	.release = siox_device_release,
+};
+
+static int siox_match(struct device *dev, struct device_driver *drv)
+{
+	if (dev->type != &siox_device_type)
+		return 0;
+
+	/* up to now there is only a single driver so keeping this simple */
+	return 1;
+}
+
+static struct bus_type siox_bus_type = {
+	.name = "siox",
+	.match = siox_match,
+};
+
+static int siox_driver_probe(struct device *dev)
+{
+	struct siox_driver *sdriver = to_siox_driver(dev->driver);
+	struct siox_device *sdevice = to_siox_device(dev);
+	int ret;
+
+	ret = sdriver->probe(sdevice);
+	return ret;
+}
+
+static int siox_driver_remove(struct device *dev)
+{
+	struct siox_driver *sdriver =
+		container_of(dev->driver, struct siox_driver, driver);
+	struct siox_device *sdevice = to_siox_device(dev);
+	int ret;
+
+	ret = sdriver->remove(sdevice);
+	return ret;
+}
+
+static void siox_driver_shutdown(struct device *dev)
+{
+	struct siox_driver *sdriver =
+		container_of(dev->driver, struct siox_driver, driver);
+	struct siox_device *sdevice = to_siox_device(dev);
+
+	sdriver->shutdown(sdevice);
+}
+
+static ssize_t active_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	struct siox_master *smaster = to_siox_master(dev);
+
+	return sprintf(buf, "%d\n", smaster->active);
+}
+
+static ssize_t active_store(struct device *dev,
+			    struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	struct siox_master *smaster = to_siox_master(dev);
+	int ret;
+	int active;
+
+	ret = kstrtoint(buf, 0, &active);
+	if (ret < 0)
+		return ret;
+
+	if (active)
+		ret = siox_start(smaster);
+	else
+		ret = siox_stop(smaster);
+
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(active);
+
+static struct siox_device *siox_device_add(struct siox_master *smaster,
+					   const char *type, size_t inbytes,
+					   size_t outbytes, u8 statustype);
+
+static ssize_t device_add_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct siox_master *smaster = to_siox_master(dev);
+	int ret;
+	char type[20] = "";
+	size_t inbytes = 0, outbytes = 0;
+	u8 statustype = 0;
+
+	ret = sscanf(buf, "%20s %zu %zu %hhu", type, &inbytes,
+		     &outbytes, &statustype);
+	if (ret != 3 && ret != 4)
+		return -EINVAL;
+
+	if (strcmp(type, "siox-12x8") || inbytes != 2 || outbytes != 4)
+		return -EINVAL;
+
+	siox_device_add(smaster, "siox-12x8", inbytes, outbytes, statustype);
+
+	return count;
+}
+
+static DEVICE_ATTR_WO(device_add);
+
+static void siox_device_remove(struct siox_master *smaster);
+
+static ssize_t device_remove_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	struct siox_master *smaster = to_siox_master(dev);
+
+	/* XXX? require to write <type> <inbytes> <outbytes> */
+	siox_device_remove(smaster);
+
+	return count;
+}
+
+static DEVICE_ATTR_WO(device_remove);
+
+static ssize_t poll_interval_ns_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct siox_master *smaster = to_siox_master(dev);
+
+	return sprintf(buf, "%lld\n", jiffies_to_nsecs(smaster->poll_interval));
+}
+
+static ssize_t poll_interval_ns_store(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct siox_master *smaster = to_siox_master(dev);
+	int ret;
+	u64 val;
+
+	ret = kstrtou64(buf, 0, &val);
+	if (ret < 0)
+		return ret;
+
+	siox_master_lock(smaster);
+
+	smaster->poll_interval = nsecs_to_jiffies(val);
+
+	siox_master_unlock(smaster);
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(poll_interval_ns);
+
+static struct attribute *siox_master_attrs[] = {
+	&dev_attr_active.attr,
+	&dev_attr_device_add.attr,
+	&dev_attr_device_remove.attr,
+	&dev_attr_poll_interval_ns.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(siox_master);
+
+static void siox_master_release(struct device *dev)
+{
+	struct siox_master *smaster = to_siox_master(dev);
+
+	kfree(smaster);
+}
+
+static struct device_type siox_master_type = {
+	.groups = siox_master_groups,
+	.release = siox_master_release,
+};
+
+struct siox_master *siox_master_alloc(struct device *dev,
+				      size_t size)
+{
+	struct siox_master *smaster;
+
+	if (!dev)
+		return NULL;
+
+	smaster = kzalloc(sizeof(*smaster) + size, GFP_KERNEL);
+	if (!smaster)
+		return NULL;
+
+	device_initialize(&smaster->dev);
+
+	smaster->busno = -1;
+	smaster->dev.bus = &siox_bus_type;
+	smaster->dev.type = &siox_master_type;
+	smaster->dev.parent = dev;
+	smaster->poll_interval = DIV_ROUND_UP(HZ, 40);
+
+	dev_set_drvdata(&smaster->dev, &smaster[1]);
+
+	return smaster;
+}
+EXPORT_SYMBOL_GPL(siox_master_alloc);
+
+int siox_master_register(struct siox_master *smaster)
+{
+	int ret;
+
+	if (!siox_is_registered)
+		return -EPROBE_DEFER;
+
+	if (!smaster->pushpull)
+		return -EINVAL;
+
+	dev_set_name(&smaster->dev, "siox-%d", smaster->busno);
+
+	smaster->last_poll = jiffies;
+	smaster->poll_thread = kthread_create(siox_poll_thread, smaster,
+					      "siox-%d", smaster->busno);
+	if (IS_ERR(smaster->poll_thread)) {
+		smaster->active = 0;
+		return PTR_ERR(smaster->poll_thread);
+	}
+
+	mutex_init(&smaster->lock);
+	INIT_LIST_HEAD(&smaster->devices);
+
+	ret = device_add(&smaster->dev);
+	if (ret)
+		kthread_stop(smaster->poll_thread);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(siox_master_register);
+
+void siox_master_unregister(struct siox_master *smaster)
+{
+	/* remove device */
+	device_del(&smaster->dev);
+
+	siox_master_lock(smaster);
+
+	__siox_stop(smaster);
+
+	while (smaster->num_devices) {
+		struct siox_device *sdevice;
+
+		sdevice = container_of(smaster->devices.prev,
+				       struct siox_device, node);
+		list_del(&sdevice->node);
+		smaster->num_devices--;
+
+		siox_master_unlock(smaster);
+
+		device_unregister(&sdevice->dev);
+
+		siox_master_lock(smaster);
+	}
+
+	siox_master_unlock(smaster);
+
+	put_device(&smaster->dev);
+}
+EXPORT_SYMBOL_GPL(siox_master_unregister);
+
+static struct siox_device *siox_device_add(struct siox_master *smaster,
+					   const char *type, size_t inbytes,
+					   size_t outbytes, u8 statustype)
+{
+	struct siox_device *sdevice;
+	int ret;
+	size_t buf_len;
+
+	sdevice = kzalloc(sizeof(*sdevice), GFP_KERNEL);
+	if (!sdevice)
+		return ERR_PTR(-ENOMEM);
+
+	sdevice->type = type;
+	sdevice->inbytes = inbytes;
+	sdevice->outbytes = outbytes;
+	sdevice->statustype = statustype;
+
+	sdevice->smaster = smaster;
+	sdevice->dev.parent = &smaster->dev;
+	sdevice->dev.bus = &siox_bus_type;
+	sdevice->dev.type = &siox_device_type;
+
+	siox_master_lock(smaster);
+
+	dev_set_name(&sdevice->dev, "siox-%d-%d",
+		     smaster->busno, smaster->num_devices);
+
+	buf_len = smaster->setbuf_len + inbytes +
+		smaster->getbuf_len + outbytes;
+	if (smaster->buf_len < buf_len) {
+		u8 *buf = krealloc(smaster->buf, buf_len, GFP_KERNEL);
+
+		if (!buf) {
+			dev_err(&smaster->dev,
+				"failed to realloc buffer to %zu\n", buf_len);
+			ret = -ENOMEM;
+			goto err_buf_alloc;
+		}
+
+		smaster->buf_len = buf_len;
+		smaster->buf = buf;
+	}
+
+	ret = device_register(&sdevice->dev);
+	if (ret) {
+		dev_err(&smaster->dev, "failed to register device: %d\n", ret);
+
+		goto err_device_register;
+	}
+
+	smaster->num_devices++;
+	list_add_tail(&sdevice->node, &smaster->devices);
+
+	smaster->setbuf_len += sdevice->inbytes;
+	smaster->getbuf_len += sdevice->outbytes;
+
+	sdevice->status_errors_kn = sysfs_get_dirent(sdevice->dev.kobj.sd,
+						     "status_errors");
+	sdevice->watchdog_kn = sysfs_get_dirent(sdevice->dev.kobj.sd,
+						"watchdog");
+	sdevice->watchdog_errors_kn = sysfs_get_dirent(sdevice->dev.kobj.sd,
+						       "watchdog_errors");
+	sdevice->connected_kn = sysfs_get_dirent(sdevice->dev.kobj.sd,
+						 "connected");
+
+	siox_master_unlock(smaster);
+
+	return sdevice;
+
+err_device_register:
+	/* don't care to make the buffer smaller again */
+
+err_buf_alloc:
+	siox_master_unlock(smaster);
+
+	kfree(sdevice);
+
+	return ERR_PTR(ret);
+}
+
+static void siox_device_remove(struct siox_master *smaster)
+{
+	struct siox_device *sdevice;
+
+	siox_master_lock(smaster);
+
+	if (!smaster->num_devices) {
+		siox_master_unlock(smaster);
+		return;
+	}
+
+	sdevice = container_of(smaster->devices.prev, struct siox_device, node);
+	list_del(&sdevice->node);
+	smaster->num_devices--;
+
+	smaster->setbuf_len -= sdevice->inbytes;
+	smaster->getbuf_len -= sdevice->outbytes;
+
+	if (!smaster->num_devices)
+		__siox_stop(smaster);
+
+	siox_master_unlock(smaster);
+
+	/*
+	 * This must be done without holding the master lock because we're
+	 * called from device_remove_store which also holds a sysfs mutex.
+	 * device_unregister tries to aquire the same lock.
+	 */
+	device_unregister(&sdevice->dev);
+}
+
+int __siox_driver_register(struct siox_driver *sdriver, struct module *owner)
+{
+	int ret;
+
+	if (unlikely(!siox_is_registered))
+		return -EPROBE_DEFER;
+
+	if (!sdriver->set_data && !sdriver->get_data) {
+		pr_err("Driver %s doesn't provide needed callbacks\n",
+		       sdriver->driver.name);
+		return -EINVAL;
+	}
+
+	sdriver->driver.owner = owner;
+	sdriver->driver.bus = &siox_bus_type;
+
+	if (sdriver->probe)
+		sdriver->driver.probe = siox_driver_probe;
+	if (sdriver->remove)
+		sdriver->driver.remove = siox_driver_remove;
+	if (sdriver->shutdown)
+		sdriver->driver.shutdown = siox_driver_shutdown;
+
+	ret = driver_register(&sdriver->driver);
+	if (ret)
+		pr_err("Failed to register siox driver %s (%d)\n",
+		       sdriver->driver.name, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__siox_driver_register);
+
+static int __init siox_init(void)
+{
+	int ret;
+
+	ret = bus_register(&siox_bus_type);
+	if (ret) {
+		pr_err("Registration of SIOX bus type failed: %d\n", ret);
+		return ret;
+	}
+
+	siox_is_registered = true;
+
+	return 0;
+}
+subsys_initcall(siox_init);
+
+static void __exit siox_exit(void)
+{
+	bus_unregister(&siox_bus_type);
+}
+module_exit(siox_exit);
+
+MODULE_AUTHOR("Uwe Kleine-Koenig <u.kleine-koenig@pengutronix.de>");
+MODULE_DESCRIPTION("Eckelmann SIOX driver core");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/siox/siox.h b/drivers/siox/siox.h
new file mode 100644
index 000000000000..c674bf6fb119
--- /dev/null
+++ b/drivers/siox/siox.h
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2015-2017 Pengutronix, Uwe Kleine-König <kernel@pengutronix.de>
+ */
+#include <linux/kernel.h>
+#include <linux/kthread.h>
+#include <linux/siox.h>
+
+#define to_siox_master(_dev)	container_of((_dev), struct siox_master, dev)
+struct siox_master {
+	/* these fields should be initialized by the driver */
+	int busno;
+	int (*pushpull)(struct siox_master *smaster,
+			size_t setbuf_len, const u8 setbuf[],
+			size_t getbuf_len, u8 getbuf[]);
+
+	/* might be initialized by the driver, if 0 it is set to HZ / 40 */
+	unsigned long poll_interval; /* in jiffies */
+
+	/* framework private stuff */
+	struct mutex lock;
+	bool active;
+	struct module *owner;
+	struct device dev;
+	unsigned int num_devices;
+	struct list_head devices;
+
+	size_t setbuf_len, getbuf_len;
+	size_t buf_len;
+	u8 *buf;
+	u8 status;
+
+	unsigned long last_poll;
+	struct task_struct *poll_thread;
+};
+
+static inline void *siox_master_get_devdata(struct siox_master *smaster)
+{
+	return dev_get_drvdata(&smaster->dev);
+}
+
+struct siox_master *siox_master_alloc(struct device *dev, size_t size);
+static inline void siox_master_put(struct siox_master *smaster)
+{
+	put_device(&smaster->dev);
+}
+
+int siox_master_register(struct siox_master *smaster);
+void siox_master_unregister(struct siox_master *smaster);
diff --git a/include/linux/siox.h b/include/linux/siox.h
new file mode 100644
index 000000000000..d79624e83134
--- /dev/null
+++ b/include/linux/siox.h
@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2015 Pengutronix, Uwe Kleine-König <kernel@pengutronix.de>
+ *
+ * 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/device.h>
+
+#define to_siox_device(_dev)	container_of((_dev), struct siox_device, dev)
+struct siox_device {
+	struct list_head node; /* node in smaster->devices */
+	struct siox_master *smaster;
+	struct device dev;
+
+	const char *type;
+	size_t inbytes;
+	size_t outbytes;
+	u8 statustype;
+
+	u8 status_read_clean;
+	u8 status_written;
+	u8 status_written_lastcycle;
+	bool connected;
+
+	/* statistics */
+	unsigned int watchdog_errors;
+	unsigned int status_errors;
+
+	struct kernfs_node *status_errors_kn;
+	struct kernfs_node *watchdog_kn;
+	struct kernfs_node *watchdog_errors_kn;
+	struct kernfs_node *connected_kn;
+};
+
+bool siox_device_synced(struct siox_device *sdevice);
+bool siox_device_connected(struct siox_device *sdevice);
+
+struct siox_driver {
+	int (*probe)(struct siox_device *sdevice);
+	int (*remove)(struct siox_device *sdevice);
+	void (*shutdown)(struct siox_device *sdevice);
+
+	/*
+	 * buf is big enough to hold sdev->inbytes - 1 bytes, the status byte
+	 * is in the scope of the framework.
+	 */
+	int (*set_data)(struct siox_device *sdevice, u8 status, u8 buf[]);
+	/*
+	 * buf is big enough to hold sdev->outbytes - 1 bytes, the status byte
+	 * is in the scope of the framework
+	 */
+	int (*get_data)(struct siox_device *sdevice, const u8 buf[]);
+
+	struct device_driver driver;
+};
+
+static inline struct siox_driver *to_siox_driver(struct device_driver *driver)
+{
+	if (driver)
+		return container_of(driver, struct siox_driver, driver);
+	else
+		return NULL;
+}
+
+int __siox_driver_register(struct siox_driver *sdriver, struct module *owner);
+
+static inline int siox_driver_register(struct siox_driver *sdriver)
+{
+	return __siox_driver_register(sdriver, THIS_MODULE);
+}
+
+static inline void siox_driver_unregister(struct siox_driver *sdriver)
+{
+	return driver_unregister(&sdriver->driver);
+}
-- 
2.11.0

[PATCH v1 2/5] siox: add support for tracing

[Uwe Kleine-König]

Implement tracing for SIOX. There are events for the data that is
written to the bus and for data being read from it.

Acked-by: Gavin Schenk <g.schenk@eckelmann.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/siox/siox-core.c    | 13 +++++++++
 include/trace/events/siox.h | 66 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)
 create mode 100644 include/trace/events/siox.h

diff --git a/drivers/siox/siox-core.c b/drivers/siox/siox-core.c
index 8578972ba18b..1a6383df9f32 100644
--- a/drivers/siox/siox-core.c
+++ b/drivers/siox/siox-core.c
@@ -33,6 +33,9 @@
  */
 #define SIOX_STATUS_TYPE		0xf0
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/siox.h>
+
 static bool siox_is_registered;
 
 static void siox_master_lock(struct siox_master *smaster)
@@ -126,6 +129,7 @@ static void siox_poll(struct siox_master *smaster)
 {
 	struct siox_device *sdevice;
 	size_t i = smaster->setbuf_len;
+	unsigned int devno = 0;
 	int unsync_error = 0;
 
 	smaster->last_poll = jiffies;
@@ -172,9 +176,14 @@ static void siox_poll(struct siox_master *smaster)
 			sdevice->status_written &= ~SIOX_STATUS_WDG;
 
 		smaster->buf[i] = sdevice->status_written;
+
+		trace_siox_set_data(smaster, sdevice, devno, i);
+
+		devno++;
 	}
 
 	WARN_ON(i);
+	WARN_ON(devno != smaster->num_devices);
 
 	smaster->pushpull(smaster, smaster->setbuf_len, smaster->buf,
 			  smaster->getbuf_len,
@@ -183,6 +192,7 @@ static void siox_poll(struct siox_master *smaster)
 	unsync_error = 0;
 
 	/* interpret data pulled in from devices in buf[setbuf_len..] */
+	devno = 0;
 	i = smaster->setbuf_len;
 	list_for_each_entry(sdevice, &smaster->devices, node) {
 		struct siox_driver *sdriver =
@@ -257,10 +267,13 @@ static void siox_poll(struct siox_master *smaster)
 		sdevice->status_written_lastcycle = sdevice->status_written;
 		sdevice->connected = connected;
 
+		trace_siox_get_data(smaster, sdevice, devno, status_clean, i);
+
 		/* only give data read to driver if the device is connected */
 		if (sdriver && connected)
 			sdriver->get_data(sdevice, &smaster->buf[i]);
 
+		devno++;
 		i += sdevice->outbytes;
 	}
 }
diff --git a/include/trace/events/siox.h b/include/trace/events/siox.h
new file mode 100644
index 000000000000..6ab0b2cc37fe
--- /dev/null
+++ b/include/trace/events/siox.h
@@ -0,0 +1,66 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM siox
+
+#if !defined(_TRACE_SIOX_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SIOX_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(siox_set_data,
+	    TP_PROTO(const struct siox_master *smaster,
+		     const struct siox_device *sdevice,
+		     unsigned int devno, size_t bufoffset),
+	    TP_ARGS(smaster, sdevice, devno, bufoffset),
+	    TP_STRUCT__entry(
+			     __field(int, busno)
+			     __field(unsigned int, devno)
+			     __field(size_t, inbytes)
+			     __dynamic_array(u8, buf, sdevice->inbytes)
+			    ),
+	    TP_fast_assign(
+			   __entry->busno = smaster->busno;
+			   __entry->devno = devno;
+			   __entry->inbytes = sdevice->inbytes;
+			   memcpy(__get_dynamic_array(buf),
+				  smaster->buf + bufoffset, sdevice->inbytes);
+			  ),
+	    TP_printk("siox-%d-%u [%*phD]",
+		      __entry->busno,
+		      __entry->devno,
+		      __entry->inbytes, __get_dynamic_array(buf)
+		     )
+);
+
+TRACE_EVENT(siox_get_data,
+	    TP_PROTO(const struct siox_master *smaster,
+		     const struct siox_device *sdevice,
+		     unsigned int devno, u8 status_clean,
+		     size_t bufoffset),
+	    TP_ARGS(smaster, sdevice, devno, status_clean, bufoffset),
+	    TP_STRUCT__entry(
+			     __field(int, busno)
+			     __field(unsigned int, devno)
+			     __field(u8, status_clean)
+			     __field(size_t, outbytes)
+			     __dynamic_array(u8, buf, sdevice->outbytes)
+			    ),
+	    TP_fast_assign(
+			   __entry->busno = smaster->busno;
+			   __entry->devno = devno;
+			   __entry->status_clean = status_clean;
+			   __entry->outbytes = sdevice->outbytes;
+			   memcpy(__get_dynamic_array(buf),
+				  smaster->buf + bufoffset, sdevice->outbytes);
+			  ),
+	    TP_printk("siox-%d-%u (%02hhx) [%*phD]",
+		      __entry->busno,
+		      __entry->devno,
+		      __entry->status_clean,
+		      __entry->outbytes, __get_dynamic_array(buf)
+		     )
+);
+
+#endif /* if !defined(_TRACE_SIOX_H) || defined(TRACE_HEADER_MULTI_READ) */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
2.11.0

[PATCH v1 3/5] siox: add gpio bus driver

[Uwe Kleine-König]

This bus driver uses GPIOs to control the four SIOX bus lines.

Acked-by: Gavin Schenk <g.schenk@eckelmann.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../bindings/siox/eckelmann,siox-gpio.txt          |  19 +++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/siox/Kconfig                               |   9 ++
 drivers/siox/Makefile                              |   1 +
 drivers/siox/siox-bus-gpio.c                       | 172 +++++++++++++++++++++
 5 files changed, 202 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/siox/eckelmann,siox-gpio.txt
 create mode 100644 drivers/siox/siox-bus-gpio.c

diff --git a/Documentation/devicetree/bindings/siox/eckelmann,siox-gpio.txt b/Documentation/devicetree/bindings/siox/eckelmann,siox-gpio.txt
new file mode 100644
index 000000000000..55259cf39c25
--- /dev/null
+++ b/Documentation/devicetree/bindings/siox/eckelmann,siox-gpio.txt
@@ -0,0 +1,19 @@
+Eckelmann SIOX GPIO bus
+
+Required properties:
+- compatible : "eckelmann,siox-gpio"
+- din-gpios, dout-gpios, dclk-gpios, dld-gpios: references gpios for the
+    corresponding bus signals.
+
+Examples:
+
+        siox {
+                compatible = "eckelmann,siox-gpio";
+                pinctrl-names = "default";
+                pinctrl-0 = <&pinctrl_siox>;
+
+                din-gpios = <&gpio6 11 0>;
+                dout-gpios = <&gpio6 8 0>;
+                dclk-gpios = <&gpio6 9 0>;
+                dld-gpios = <&gpio6 10 0>;
+        };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 0994bdd82cd3..889d1c0f5050 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -97,6 +97,7 @@ dptechnics	DPTechnics
 dragino	Dragino Technology Co., Limited
 ea	Embedded Artists AB
 ebv	EBV Elektronik
+eckelmann	Eckelmann AG
 edt	Emerging Display Technologies
 eeti	eGalax_eMPIA Technology Inc
 elan	Elan Microelectronic Corp.
diff --git a/drivers/siox/Kconfig b/drivers/siox/Kconfig
index bd24d9b50dc6..083d2e62189a 100644
--- a/drivers/siox/Kconfig
+++ b/drivers/siox/Kconfig
@@ -7,3 +7,12 @@ menuconfig SIOX
 	  to drive additional I/O units.
 
 	  Unless you know better, it is probably safe to say "no" here.
+
+if SIOX
+
+config SIOX_BUS_GPIO
+	tristate "SIOX GPIO bus driver"
+	help
+	  SIOX bus driver that controls the four bus lines using GPIOs.
+
+endif
diff --git a/drivers/siox/Makefile b/drivers/siox/Makefile
index d55cb5e08868..a956f65206d5 100644
--- a/drivers/siox/Makefile
+++ b/drivers/siox/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_SIOX) += siox-core.o
+obj-$(CONFIG_SIOX_BUS_GPIO) += siox-bus-gpio.o
diff --git a/drivers/siox/siox-bus-gpio.c b/drivers/siox/siox-bus-gpio.c
new file mode 100644
index 000000000000..ea7ef982968b
--- /dev/null
+++ b/drivers/siox/siox-bus-gpio.c
@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2015-2017 Pengutronix, Uwe Kleine-König <kernel@pengutronix.de>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <linux/delay.h>
+
+#include "siox.h"
+
+#define DRIVER_NAME "siox-gpio"
+
+struct siox_gpio_ddata {
+	struct gpio_desc *din;
+	struct gpio_desc *dout;
+	struct gpio_desc *dclk;
+	struct gpio_desc *dld;
+};
+
+static unsigned int siox_clkhigh_ns = 1000;
+static unsigned int siox_loadhigh_ns;
+static unsigned int siox_bytegap_ns;
+
+static int siox_gpio_pushpull(struct siox_master *smaster,
+			      size_t setbuf_len, const u8 setbuf[],
+			      size_t getbuf_len, u8 getbuf[])
+{
+	struct siox_gpio_ddata *ddata = siox_master_get_devdata(smaster);
+	size_t i;
+	size_t cycles = max(setbuf_len, getbuf_len);
+
+	/* reset data and clock */
+	gpiod_set_value_cansleep(ddata->dout, 0);
+	gpiod_set_value_cansleep(ddata->dclk, 0);
+
+	gpiod_set_value_cansleep(ddata->dld, 1);
+	ndelay(siox_loadhigh_ns);
+	gpiod_set_value_cansleep(ddata->dld, 0);
+
+	for (i = 0; i < cycles; ++i) {
+		u8 set = 0, get = 0;
+		size_t j;
+
+		if (i >= cycles - setbuf_len)
+			set = setbuf[i - (cycles - setbuf_len)];
+
+		for (j = 0; j < 8; ++j) {
+			get <<= 1;
+			if (gpiod_get_value_cansleep(ddata->din))
+				get |= 1;
+
+			/* DOUT is logically inverted */
+			gpiod_set_value_cansleep(ddata->dout, !(set & 0x80));
+			set <<= 1;
+
+			gpiod_set_value_cansleep(ddata->dclk, 1);
+			ndelay(siox_clkhigh_ns);
+			gpiod_set_value_cansleep(ddata->dclk, 0);
+		}
+
+		if (i < getbuf_len)
+			getbuf[i] = get;
+
+		ndelay(siox_bytegap_ns);
+	}
+
+	gpiod_set_value_cansleep(ddata->dld, 1);
+	ndelay(siox_loadhigh_ns);
+	gpiod_set_value_cansleep(ddata->dld, 0);
+
+	/*
+	 * Resetting dout isn't necessary protocol wise, but it makes the
+	 * signals more pretty because the dout level is deterministic between
+	 * cycles. Note that this only affects dout between the master and the
+	 * first siox device. dout for the later devices depend on the output of
+	 * the previous siox device.
+	 */
+	gpiod_set_value_cansleep(ddata->dout, 0);
+
+	return 0;
+}
+
+static int siox_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct siox_gpio_ddata *ddata;
+	int ret;
+	struct siox_master *smaster;
+
+	smaster = siox_master_alloc(&pdev->dev, sizeof(*ddata));
+	if (!smaster) {
+		dev_err(dev, "failed to allocate siox master\n");
+		return -ENOMEM;
+	}
+
+	platform_set_drvdata(pdev, smaster);
+	ddata = siox_master_get_devdata(smaster);
+
+	ddata->din = devm_gpiod_get(dev, "din", GPIOD_IN);
+	if (IS_ERR(ddata->din)) {
+		ret = PTR_ERR(ddata->din);
+		dev_err(dev, "Failed to get %s GPIO: %d\n", "din", ret);
+		goto err;
+	}
+
+	ddata->dout = devm_gpiod_get(dev, "dout", GPIOD_OUT_LOW);
+	if (IS_ERR(ddata->dout)) {
+		ret = PTR_ERR(ddata->dout);
+		dev_err(dev, "Failed to get %s GPIO: %d\n", "dout", ret);
+		goto err;
+	}
+
+	ddata->dclk = devm_gpiod_get(dev, "dclk", GPIOD_OUT_LOW);
+	if (IS_ERR(ddata->dclk)) {
+		ret = PTR_ERR(ddata->dclk);
+		dev_err(dev, "Failed to get %s GPIO: %d\n", "dclk", ret);
+		goto err;
+	}
+
+	ddata->dld = devm_gpiod_get(dev, "dld", GPIOD_OUT_LOW);
+	if (IS_ERR(ddata->dld)) {
+		ret = PTR_ERR(ddata->dld);
+		dev_err(dev, "Failed to get %s GPIO: %d\n", "dld", ret);
+		goto err;
+	}
+
+	smaster->pushpull = siox_gpio_pushpull;
+	/* XXX: determine automatically like spi does */
+	smaster->busno = 0;
+
+	ret = siox_master_register(smaster);
+	if (ret) {
+		dev_err(dev, "Failed to register siox master: %d\n", ret);
+err:
+		siox_master_put(smaster);
+	}
+
+	return ret;
+}
+
+static int siox_gpio_remove(struct platform_device *pdev)
+{
+	struct siox_master *master = platform_get_drvdata(pdev);
+
+	siox_master_unregister(master);
+
+	return 0;
+}
+
+static const struct of_device_id siox_gpio_dt_ids[] = {
+	{ .compatible = "eckelmann,siox-gpio", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, siox_gpio_dt_ids);
+
+static struct platform_driver siox_gpio_driver = {
+	.probe = siox_gpio_probe,
+	.remove = siox_gpio_remove,
+
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = siox_gpio_dt_ids,
+	},
+};
+module_platform_driver(siox_gpio_driver);
+
+MODULE_AUTHOR("Uwe Kleine-Koenig <u.kleine-koenig@pengutronix.de>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
2.11.0

[PATCH v1 4/5] gpio: new driver to work with a 8x12 siox

[Uwe Kleine-König]

This driver controls a SIOX device that provides 20 I/O lines. The first
twelve are fixed inputs, the remaining eight are outputs.

Acked-by: Gavin Schenk <g.schenk@eckelmann.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/gpio/Kconfig     |   8 ++
 drivers/gpio/Makefile    |   1 +
 drivers/gpio/gpio-siox.c | 293 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+)
 create mode 100644 drivers/gpio/gpio-siox.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6a8e851ad13..0a0a15aa95ac 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -397,6 +397,14 @@ config GPIO_REG
 	  A 32-bit single register GPIO fixed in/out implementation.  This
 	  can be used to represent any register as a set of GPIO signals.
 
+config GPIO_SIOX
+	tristate "SIOX GPIO support"
+	depends on SIOX
+	select GPIOLIB_IRQCHIP
+	help
+	  Say yes here to support SIOX I/O devices. These are units connected
+	  via a SIOX bus and have a number of fixed-direction I/O lines.
+
 config GPIO_SPEAR_SPICS
 	bool "ST SPEAr13xx SPI Chip Select as GPIO support"
 	depends on PLAT_SPEAR
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4bc24febb889..d25ab1244242 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -119,6 +119,7 @@ obj-$(CONFIG_GPIO_TEGRA186)	+= gpio-tegra186.o
 obj-$(CONFIG_GPIO_THUNDERX)	+= gpio-thunderx.o
 obj-$(CONFIG_GPIO_TIMBERDALE)	+= gpio-timberdale.o
 obj-$(CONFIG_GPIO_PALMAS)	+= gpio-palmas.o
+obj-$(CONFIG_GPIO_SIOX)		+= gpio-siox.o
 obj-$(CONFIG_GPIO_TPIC2810)	+= gpio-tpic2810.o
 obj-$(CONFIG_GPIO_TPS65086)	+= gpio-tps65086.o
 obj-$(CONFIG_GPIO_TPS65218)	+= gpio-tps65218.o
diff --git a/drivers/gpio/gpio-siox.c b/drivers/gpio/gpio-siox.c
new file mode 100644
index 000000000000..5b4d858fe954
--- /dev/null
+++ b/drivers/gpio/gpio-siox.c
@@ -0,0 +1,293 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2015-2017 Pengutronix, Uwe Kleine-König <kernel@pengutronix.de>
+ */
+
+#include <linux/module.h>
+#include <linux/siox.h>
+#include <linux/gpio/driver.h>
+#include <linux/of.h>
+
+struct gpio_siox_ddata {
+	struct gpio_chip gchip;
+	struct irq_chip ichip;
+	struct mutex lock;
+	u8 setdata[1];
+	u8 getdata[3];
+
+	spinlock_t irqlock;
+	u32 irq_enable;
+	u32 irq_status;
+	u32 irq_type[20];
+};
+
+/*
+ * Note that this callback only sets the value that is clocked out in the next
+ * cycle.
+ */
+static int gpio_siox_set_data(struct siox_device *sdevice, u8 status, u8 buf[])
+{
+	struct gpio_siox_ddata *ddata = dev_get_drvdata(&sdevice->dev);
+
+	mutex_lock(&ddata->lock);
+	buf[0] = ddata->setdata[0];
+	mutex_unlock(&ddata->lock);
+
+	return 0;
+}
+
+static int gpio_siox_get_data(struct siox_device *sdevice, const u8 buf[])
+{
+	struct gpio_siox_ddata *ddata = dev_get_drvdata(&sdevice->dev);
+	size_t offset;
+	u32 trigger;
+
+	mutex_lock(&ddata->lock);
+
+	spin_lock_irq(&ddata->irqlock);
+
+	for (offset = 0; offset < 12; ++offset) {
+		unsigned int bitpos = 11 - offset;
+		unsigned int gpiolevel = buf[bitpos / 8] & (1 << bitpos % 8);
+		unsigned int prev_level =
+			ddata->getdata[bitpos / 8] & (1 << (bitpos % 8));
+		u32 irq_type = ddata->irq_type[offset];
+
+		if (gpiolevel) {
+			if ((irq_type & IRQ_TYPE_LEVEL_HIGH) ||
+			    ((irq_type & IRQ_TYPE_EDGE_RISING) && !prev_level))
+				ddata->irq_status |= 1 << offset;
+		} else {
+			if ((irq_type & IRQ_TYPE_LEVEL_LOW) ||
+			    ((irq_type & IRQ_TYPE_EDGE_FALLING) && prev_level))
+				ddata->irq_status |= 1 << offset;
+		}
+	}
+
+	trigger = ddata->irq_status & ddata->irq_enable;
+
+	spin_unlock_irq(&ddata->irqlock);
+
+	ddata->getdata[0] = buf[0];
+	ddata->getdata[1] = buf[1];
+	ddata->getdata[2] = buf[2];
+
+	mutex_unlock(&ddata->lock);
+
+	for (offset = 0; offset < 12; ++offset) {
+		if (trigger & (1 << offset)) {
+			struct irq_domain *irqdomain = ddata->gchip.irq.domain;
+			unsigned int irq = irq_find_mapping(irqdomain, offset);
+
+			/*
+			 * Conceptually handle_nested_irq should call the flow
+			 * handler of the irq chip. But it doesn't, so we have
+			 * to clean the irq_status here.
+			 */
+			spin_lock_irq(&ddata->irqlock);
+			ddata->irq_status &= ~(1 << offset);
+			spin_unlock_irq(&ddata->irqlock);
+
+			handle_nested_irq(irq);
+		}
+	}
+
+	return 0;
+}
+
+static void gpio_siox_irq_ack(struct irq_data *d)
+{
+	struct irq_chip *ic = irq_data_get_irq_chip(d);
+	struct gpio_siox_ddata *ddata =
+		container_of(ic, struct gpio_siox_ddata, ichip);
+
+	spin_lock_irq(&ddata->irqlock);
+	ddata->irq_status &= ~(1 << d->hwirq);
+	spin_unlock_irq(&ddata->irqlock);
+}
+
+static void gpio_siox_irq_mask(struct irq_data *d)
+{
+	struct irq_chip *ic = irq_data_get_irq_chip(d);
+	struct gpio_siox_ddata *ddata =
+		container_of(ic, struct gpio_siox_ddata, ichip);
+
+	spin_lock_irq(&ddata->irqlock);
+	ddata->irq_enable &= ~(1 << d->hwirq);
+	spin_unlock_irq(&ddata->irqlock);
+}
+
+static void gpio_siox_irq_unmask(struct irq_data *d)
+{
+	struct irq_chip *ic = irq_data_get_irq_chip(d);
+	struct gpio_siox_ddata *ddata =
+		container_of(ic, struct gpio_siox_ddata, ichip);
+
+	spin_lock_irq(&ddata->irqlock);
+	ddata->irq_enable |= 1 << d->hwirq;
+	spin_unlock_irq(&ddata->irqlock);
+}
+
+static int gpio_siox_irq_set_type(struct irq_data *d, u32 type)
+{
+	struct irq_chip *ic = irq_data_get_irq_chip(d);
+	struct gpio_siox_ddata *ddata =
+		container_of(ic, struct gpio_siox_ddata, ichip);
+
+	spin_lock_irq(&ddata->irqlock);
+	ddata->irq_type[d->hwirq] = type;
+	spin_unlock_irq(&ddata->irqlock);
+
+	return 0;
+}
+
+static int gpio_siox_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct gpio_siox_ddata *ddata =
+		container_of(chip, struct gpio_siox_ddata, gchip);
+	int ret;
+
+	mutex_lock(&ddata->lock);
+
+	if (offset >= 12) {
+		unsigned int bitpos = 19 - offset;
+
+		ret = ddata->setdata[0] & (1 << bitpos);
+	} else {
+		unsigned int bitpos = 11 - offset;
+
+		ret = ddata->getdata[bitpos / 8] & (1 << (bitpos % 8));
+	}
+
+	mutex_unlock(&ddata->lock);
+
+	return ret;
+}
+
+static void gpio_siox_set(struct gpio_chip *chip,
+			  unsigned int offset, int value)
+{
+	struct gpio_siox_ddata *ddata =
+		container_of(chip, struct gpio_siox_ddata, gchip);
+	u8 mask = 1 << (19 - offset);
+
+	mutex_lock(&ddata->lock);
+
+	if (value)
+		ddata->setdata[0] |= mask;
+	else
+		ddata->setdata[0] &= ~mask;
+
+	mutex_unlock(&ddata->lock);
+}
+
+static int gpio_siox_direction_input(struct gpio_chip *chip,
+				     unsigned int offset)
+{
+	if (offset >= 12)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int gpio_siox_direction_output(struct gpio_chip *chip,
+				      unsigned int offset, int value)
+{
+	if (offset < 12)
+		return -EINVAL;
+
+	gpio_siox_set(chip, offset, value);
+	return 0;
+}
+
+static int gpio_siox_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	if (offset < 12)
+		return 1; /* input */
+	else
+		return 0; /* output */
+}
+
+static int gpio_siox_probe(struct siox_device *sdevice)
+{
+	struct gpio_siox_ddata *ddata;
+	int ret;
+
+	ddata = devm_kzalloc(&sdevice->dev, sizeof(*ddata), GFP_KERNEL);
+	if (!ddata)
+		return -ENOMEM;
+
+	dev_set_drvdata(&sdevice->dev, ddata);
+
+	mutex_init(&ddata->lock);
+	spin_lock_init(&ddata->irqlock);
+
+	ddata->gchip.base = -1;
+	ddata->gchip.can_sleep = 1;
+	ddata->gchip.parent = &sdevice->dev;
+	ddata->gchip.owner = THIS_MODULE;
+	ddata->gchip.get = gpio_siox_get;
+	ddata->gchip.set = gpio_siox_set;
+	ddata->gchip.direction_input = gpio_siox_direction_input;
+	ddata->gchip.direction_output = gpio_siox_direction_output;
+	ddata->gchip.get_direction = gpio_siox_get_direction;
+	ddata->gchip.ngpio = 20;
+
+	ddata->ichip.name = "siox-gpio";
+	ddata->ichip.irq_ack = gpio_siox_irq_ack;
+	ddata->ichip.irq_mask = gpio_siox_irq_mask;
+	ddata->ichip.irq_unmask = gpio_siox_irq_unmask;
+	ddata->ichip.irq_set_type = gpio_siox_irq_set_type;
+
+	ret = gpiochip_add(&ddata->gchip);
+	if (ret) {
+		dev_err(&sdevice->dev,
+			"Failed to register gpio chip (%d)\n", ret);
+		goto err_gpiochip;
+	}
+
+	ret = gpiochip_irqchip_add(&ddata->gchip, &ddata->ichip,
+				   0, handle_level_irq, IRQ_TYPE_EDGE_RISING);
+	if (ret) {
+		dev_err(&sdevice->dev,
+			"Failed to register irq chip (%d)\n", ret);
+err_gpiochip:
+		gpiochip_remove(&ddata->gchip);
+	}
+
+	return ret;
+}
+
+static int gpio_siox_remove(struct siox_device *sdevice)
+{
+	struct gpio_siox_ddata *ddata = dev_get_drvdata(&sdevice->dev);
+
+	gpiochip_remove(&ddata->gchip);
+	return 0;
+}
+
+static struct siox_driver gpio_siox_driver = {
+	.probe = gpio_siox_probe,
+	.remove = gpio_siox_remove,
+	.set_data = gpio_siox_set_data,
+	.get_data = gpio_siox_get_data,
+	.driver = {
+		.name = "gpio-siox",
+	},
+};
+
+static int __init gpio_siox_init(void)
+{
+	return siox_driver_register(&gpio_siox_driver);
+}
+module_init(gpio_siox_init);
+
+static void __exit gpio_siox_exit(void)
+{
+	siox_driver_unregister(&gpio_siox_driver);
+}
+module_exit(gpio_siox_exit);
+
+MODULE_AUTHOR("Uwe Kleine-Koenig <u.kleine-koenig@pengutronix.de>");
+MODULE_DESCRIPTION("SIOX gpio driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

[PATCH v1 5/5] MAINTAINERS: Add entry for SIOX

[Uwe Kleine-König]

Maintenance is split between Gavin who works for Eckelmann and so has
the functional authority, knows the background and history of this bus
system and me who designed most of the actual code with the old
microcontroller code as reference.

Acked-by: Gavin Schenk <g.schenk@eckelmann.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index aa71ab52fd76..303999fa964c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12449,6 +12449,14 @@ F:	lib/siphash.c
 F:	lib/test_siphash.c
 F:	include/linux/siphash.h
 
+SIOX
+M:	Gavin Schenk <g.schenk@eckelmann.de>
+M:	Uwe Kleine-König <kernel@pengutronix.de>
+S:	Supported
+F:	drivers/siox/*
+F:	drivers/gpio/gpio-siox.c
+F:	include/trace/events/siox.h
+
 SIS 190 ETHERNET DRIVER
 M:	Francois Romieu <romieu@fr.zoreil.com>
 L:	netdev@vger.kernel.org
-- 
2.11.0

Re: [PATCH v1 1/5] siox: new driver framework for eckelmann SIOX

[Greg Kroah-Hartman]

On Thu, Dec 07, 2017 at 10:30:04AM +0100, Uwe Kleine-König wrote:
> SIOX is a bus system invented at Eckelmann AG to control their building
> management and refrigeration systems. Traditionally the bus was
> implemented on custom microcontrollers, today Linux based machines are
> in use, too.
> 
> The topology on a SIOX bus looks as follows:
> 
>       ,------->--DCLK-->---------------+----------------------.
>       ^                                v                      v
>  ,--------.                ,----------------------.       ,------
>  |        |                |   ,--------------.   |       |
>  |        |--->--DOUT-->---|->-|shift register|->-|--->---|
>  |        |                |   `--------------'   |       |
>  | master |                |        device        |       |  device
>  |        |                |   ,--------------.   |       |
>  |        |---<--DIN---<---|-<-|shift register|-<-|---<---|
>  |        |                |   `--------------'   |       |
>  `--------'                `----------------------'       `------
>       v                                ^                      ^
>       `----------DLD-------------------+----------------------'
> 
> There are two control lines (DCLK and DLD) driven from the bus master to
> all devices in parallel and two daisy chained data lines, one for input
> and one for output. DCLK is the clock to shift both chains by a single
> bit. On an edge of DLD the devices latch both their input and output
> shift registers.
> 
> This patch adds a framework for this bus type.

Looks very nice, great job!

Only one minor comment, and you can just send a follow-up patch for it:

> +	/* prepare data pushed out to devices in buf[0..setbuf_len) */
> +	list_for_each_entry(sdevice, &smaster->devices, node) {
> +		struct siox_driver *sdriver =
> +			to_siox_driver(sdevice->dev.driver);
> +		sdevice->status_written = smaster->status;
> +
> +		i -= sdevice->inbytes;
> +
> +		/*
> +		 * If the device or a previous one is unsynced, don't pet the
> +		 * watchdog. This is done to ensure that the device is kept in
> +		 * reset when something is wrong.
> +		 */
> +		if (!siox_device_synced(sdevice))
> +			unsync_error = 1;
> +
> +		if (sdriver && !unsync_error)
> +			sdriver->set_data(sdevice, sdevice->status_written,
> +					  &smaster->buf[i + 1]);
> +		else
> +			/*
> +			 * Don't trigger watchdog if there is no driver or a
> +			 * sync problem
> +			 */
> +			sdevice->status_written &= ~SIOX_STATUS_WDG;
> +
> +		smaster->buf[i] = sdevice->status_written;
> +	}
> +
> +	WARN_ON(i);

What can someone do if this WARN_ON() triggers?  Seems like it is
left-over debugging code?  If not, make it a "real" error and handle it
properly.  Or provide some sort of context as to what is going on here.

thanks,

greg k-h

Re: [PATCH v1 1/5] siox: new driver framework for eckelmann SIOX

[Greg Kroah-Hartman]

On Thu, Dec 07, 2017 at 10:30:04AM +0100, Uwe Kleine-König wrote:
> SIOX is a bus system invented at Eckelmann AG to control their building
> management and refrigeration systems. Traditionally the bus was
> implemented on custom microcontrollers, today Linux based machines are
> in use, too.
> 
> The topology on a SIOX bus looks as follows:
> 
>       ,------->--DCLK-->---------------+----------------------.
>       ^                                v                      v
>  ,--------.                ,----------------------.       ,------
>  |        |                |   ,--------------.   |       |
>  |        |--->--DOUT-->---|->-|shift register|->-|--->---|
>  |        |                |   `--------------'   |       |
>  | master |                |        device        |       |  device
>  |        |                |   ,--------------.   |       |
>  |        |---<--DIN---<---|-<-|shift register|-<-|---<---|
>  |        |                |   `--------------'   |       |
>  `--------'                `----------------------'       `------
>       v                                ^                      ^
>       `----------DLD-------------------+----------------------'
> 
> There are two control lines (DCLK and DLD) driven from the bus master to
> all devices in parallel and two daisy chained data lines, one for input
> and one for output. DCLK is the clock to shift both chains by a single
> bit. On an edge of DLD the devices latch both their input and output
> shift registers.
> 
> This patch adds a framework for this bus type.

Oops, you forgot a Documentation/ABI/ patch for all of the new sysfs
files you are creating here.

Also, why use kernfs direct nodes?  That feels "odd" to me.

thanks,

greg k-h

Re: [PATCH v1 4/5] gpio: new driver to work with a 8x12 siox

[Greg Kroah-Hartman]

On Thu, Dec 07, 2017 at 10:30:07AM +0100, Uwe Kleine-König wrote:
> This driver controls a SIOX device that provides 20 I/O lines. The first
> twelve are fixed inputs, the remaining eight are outputs.
> 
> Acked-by: Gavin Schenk <g.schenk@eckelmann.de>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/gpio/Kconfig     |   8 ++
>  drivers/gpio/Makefile    |   1 +
>  drivers/gpio/gpio-siox.c | 293 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 302 insertions(+)
>  create mode 100644 drivers/gpio/gpio-siox.c

I can't take this through my tree, but if the gpio maintainer acks it, I
can :)

thanks,

greg k-h

Re: [PATCH v1 4/5] gpio: new driver to work with a 8x12 siox

[Uwe Kleine-König]

Hello Greg,

On Mon, Dec 18, 2017 at 04:09:10PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Dec 07, 2017 at 10:30:07AM +0100, Uwe Kleine-König wrote:
> > This driver controls a SIOX device that provides 20 I/O lines. The first
> > twelve are fixed inputs, the remaining eight are outputs.
> > 
> > Acked-by: Gavin Schenk <g.schenk@eckelmann.de>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/gpio/Kconfig     |   8 ++
> >  drivers/gpio/Makefile    |   1 +
> >  drivers/gpio/gpio-siox.c | 293 +++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 302 insertions(+)
> >  create mode 100644 drivers/gpio/gpio-siox.c
> 
> I can't take this through my tree, but if the gpio maintainer acks it, I
> can :)

I didn't consider you to take this but thought this more to be an
example for how to use this siox stuff. I didn't send it to the gpio
list and maintainer on purpose to prevent Linus W. being happy and
taking the patch and you shooting down the siox core stuff and so
consume reviewer time pointlessly.

When you're happy with the drivers/siox/ stuff, I will send this
gpio driver (maybe separately) again to the right people.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v1 1/5] siox: new driver framework for eckelmann SIOX

[Uwe Kleine-König]

Hello Greg,

thanks for your feedback.

On Mon, Dec 18, 2017 at 04:08:33PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Dec 07, 2017 at 10:30:04AM +0100, Uwe Kleine-König wrote:
> > SIOX is a bus system invented at Eckelmann AG to control their building
> > management and refrigeration systems. Traditionally the bus was
> > implemented on custom microcontrollers, today Linux based machines are
> > in use, too.
> > 
> > The topology on a SIOX bus looks as follows:
> > 
> >       ,------->--DCLK-->---------------+----------------------.
> >       ^                                v                      v
> >  ,--------.                ,----------------------.       ,------
> >  |        |                |   ,--------------.   |       |
> >  |        |--->--DOUT-->---|->-|shift register|->-|--->---|
> >  |        |                |   `--------------'   |       |
> >  | master |                |        device        |       |  device
> >  |        |                |   ,--------------.   |       |
> >  |        |---<--DIN---<---|-<-|shift register|-<-|---<---|
> >  |        |                |   `--------------'   |       |
> >  `--------'                `----------------------'       `------
> >       v                                ^                      ^
> >       `----------DLD-------------------+----------------------'
> > 
> > There are two control lines (DCLK and DLD) driven from the bus master to
> > all devices in parallel and two daisy chained data lines, one for input
> > and one for output. DCLK is the clock to shift both chains by a single
> > bit. On an edge of DLD the devices latch both their input and output
> > shift registers.
> > 
> > This patch adds a framework for this bus type.
> 
> Oops, you forgot a Documentation/ABI/ patch for all of the new sysfs
> files you are creating here.

Will look into what is already there and come up with something similar.

> Also, why use kernfs direct nodes?  That feels "odd" to me.

What is the better alternative?

In another mail Greg Kroah-Hartman wrote:
> Looks very nice, great job!

\o/

> Only one minor comment, and you can just send a follow-up patch for
> it:
> 
> > +   WARN_ON(i);
> 
> What can someone do if this WARN_ON() triggers?

I would expect this to be reported to Gavin and me.

> Seems like it is left-over debugging code?  If not, make it a "real"
> error and handle it properly.  Or provide some sort of context as to
> what is going on here.

Well, during development it was a BUG_ON :-) I will think for v2 about
how this can be better handled or drop it.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |