Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1356206 > unrolled thread

[RFC PATCH 1/3] siox: new driver/bus framework for Eckelmann SIOX

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC PATCH 1/3] siox: new driver/bus framework for Eckelmann SIOX Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-03-11 23:00 +0100
    Re: [RFC PATCH 1/3] siox: new driver/bus framework for Eckelmann SIOX Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-03-11 23:30 +0100
      Re: [RFC PATCH 1/3] siox: new driver/bus framework for Eckelmann SIOX Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-03-12 14:50 +0100
        Re: [RFC PATCH 1/3] siox: new driver/bus framework for Eckelmann SIOX Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-03-13 04:50 +0100

#1356206 — [RFC PATCH 1/3] siox: new driver/bus framework for Eckelmann SIOX

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/Kconfig          |   2 +
 drivers/Makefile         |   1 +
 drivers/siox/Kconfig     |   2 +
 drivers/siox/Makefile    |   1 +
 drivers/siox/siox-core.c | 572 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/siox/siox.h      |  48 ++++
 include/linux/siox.h     |  53 +++++
 7 files changed, 679 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 6e973b8e3a3b..c2fbf214f8a3 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -184,4 +184,6 @@ source "drivers/android/Kconfig"
 
 source "drivers/nvdimm/Kconfig"
 
+source "drivers/siox/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index b64b49f6e01b..dd7b3cd1fd72 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -165,3 +165,4 @@ obj-$(CONFIG_RAS)		+= ras/
 obj-$(CONFIG_THUNDERBOLT)	+= thunderbolt/
 obj-$(CONFIG_CORESIGHT)		+= hwtracing/coresight/
 obj-$(CONFIG_ANDROID)		+= android/
+obj-$(CONFIG_SIOX)		+= siox/
diff --git a/drivers/siox/Kconfig b/drivers/siox/Kconfig
new file mode 100644
index 000000000000..471fbc009521
--- /dev/null
+++ b/drivers/siox/Kconfig
@@ -0,0 +1,2 @@
+menuconfig SIOX
+	tristate "Eckelmann SIOX Support"
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..71c6b18f86ad
--- /dev/null
+++ b/drivers/siox/siox-core.c
@@ -0,0 +1,572 @@
+/*
+ * 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/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+
+#include "siox.h"
+
+struct workqueue_struct *wqueue;
+static bool siox_is_registered;
+
+void siox_master_lock(struct siox_master *smaster)
+{
+	mutex_lock(&smaster->lock);
+}
+
+void siox_master_unlock(struct siox_master *smaster)
+{
+	mutex_unlock(&smaster->lock);
+}
+
+struct siox_device *siox_device_add(struct siox_master *smaster);
+void siox_device_remove(struct siox_master *smaster);
+
+static void __siox_poll(struct siox_master *smaster)
+{
+	struct siox_device *sdevice;
+	size_t i = 0;
+	u8 prevstatus = smaster->status;
+
+	if (++smaster->status > 0x0d)
+		smaster->status = 0;
+
+	memset(smaster->buf, 0, smaster->setbuf_len);
+
+	list_for_each_entry_reverse(sdevice, &smaster->devices, node) {
+		struct siox_driver *sdriver =
+			sdevice->dev.driver ? to_siox_driver(sdevice->dev.driver) : NULL;
+
+		if (sdriver)
+			sdriver->set_data(sdevice, smaster->status,
+					  &smaster->buf[i + 1]);
+
+		smaster->buf[i] = smaster->status;
+
+		i += sdevice->inbytes;
+	}
+
+	BUG_ON(i != smaster->setbuf_len);
+
+	smaster->pushpull(smaster, smaster->setbuf_len, smaster->buf,
+			  smaster->getbuf_len,
+			  smaster->buf + smaster->setbuf_len);
+
+	list_for_each_entry(sdevice, &smaster->devices, node) {
+		struct siox_driver *sdriver =
+			sdevice->dev.driver ? to_siox_driver(sdevice->dev.driver) : NULL;
+		u8 sdev_status = smaster->buf[i + sdevice->outbytes - 1];
+
+		/*
+		 * bits 4:2 of status sample the respective bit in the status
+		 * byte written in the previous cycle. Mask them out
+		 * accordingly such that a set bit there indicates an error.
+		 */
+		sdev_status ^= ~prevstatus & 0xe;
+		sdevice->status = sdev_status;
+
+		/*
+		 * XXX trigger events for watchdog, changed jumper and misread
+		 * counter. Should the bus stop to poll in these cases?
+		 */
+
+		if (sdriver)
+			sdriver->get_data(sdevice, &smaster->buf[i]);
+
+		i += sdevice->outbytes;
+	}
+
+	if (smaster->active)
+		queue_delayed_work(wqueue, &smaster->poll, smaster->poll_interval);
+}
+
+static void siox_poll(struct work_struct *work)
+{
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct siox_master *smaster = container_of(dwork, struct siox_master, poll);
+
+	siox_master_lock(smaster);
+	if (likely(smaster->active))
+		__siox_poll(smaster);
+	siox_master_unlock(smaster);
+}
+
+static int __siox_start(struct siox_master *smaster)
+{
+	dev_dbg(&smaster->dev, "%s\n", __func__);
+
+	if (!(smaster->setbuf_len + smaster->getbuf_len))
+		return -ENODEV;
+
+	if (!smaster->buf)
+		return -ENOMEM;
+
+	smaster->active = 1;
+
+	__siox_poll(smaster);
+
+	return 0;
+}
+
+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)
+{
+	dev_dbg(&smaster->dev, "%s\n", __func__);
+	smaster->active = 0;
+	cancel_delayed_work(&smaster->poll);
+	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_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;
+
+	siox_master_unlock(sdev->smaster);
+
+	return sprintf(buf, "%hhu\n", status);
+}
+
+static DEVICE_ATTR_RO(status);
+
+static struct attribute *siox_device_attrs[] = {
+	&dev_attr_type.attr,
+	&dev_attr_inbytes.attr,
+	&dev_attr_outbytes.attr,
+	&dev_attr_status.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 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;
+
+	siox_master_lock(smaster);
+
+	ret = kstrtou64(buf, 0, &val);
+	if (ret)
+		goto out_unlock;
+
+	smaster->poll_interval = nsecs_to_jiffies(val);
+
+out_unlock:
+	siox_master_unlock(smaster);
+
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(poll_interval_ns);
+
+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 == !smaster->active)
+		/* no change */
+		return count;
+
+	if (active)
+		ret = siox_start(smaster);
+	else
+		ret = siox_stop(smaster);
+
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(active);
+
+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;
+
+	ret = sscanf(buf, "%20s %zu %zu", type, &inbytes, &outbytes);
+	if (ret != 3 || strcmp(type, "siox-12x8") ||
+	    inbytes != 2 || outbytes != 4)
+		return -EINVAL;
+
+	siox_device_add(smaster);
+
+	return count;
+}
+
+static DEVICE_ATTR_WO(device_add);
+
+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 struct attribute *siox_master_attrs[] = {
+	&dev_attr_poll_interval_ns.attr,
+	&dev_attr_active.attr,
+	&dev_attr_device_add.attr,
+	&dev_attr_device_remove.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(siox_master);
+
+static void siox_master_release(struct device *dev)
+{
+	struct siox_master *smaster = to_siox_master(dev);
+
+	__siox_stop(smaster);
+}
+
+static struct device_type siox_master_type = {
+	.groups = siox_master_groups,
+	.release = siox_master_release,
+};
+
+int __siox_register_master(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->dev.bus = &siox_bus_type;
+	smaster->dev.type = &siox_master_type;
+
+	if (!smaster->poll_interval)
+		smaster->poll_interval = DIV_ROUND_UP(HZ, 40);
+
+	mutex_init(&smaster->lock);
+	INIT_LIST_HEAD(&smaster->devices);
+	INIT_DELAYED_WORK(&smaster->poll, siox_poll);
+
+	ret = device_register(&smaster->dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__siox_register_master);
+
+void siox_unregister_master(struct siox_master *smaster)
+{
+	device_unregister(&smaster->dev);
+}
+EXPORT_SYMBOL_GPL(siox_unregister_master);
+
+struct siox_device *siox_device_add(struct siox_master *smaster)
+{
+	struct siox_device *sdevice;
+	int ret;
+
+	sdevice = kzalloc(sizeof(*sdevice), GFP_KERNEL);
+	if (!sdevice)
+		return NULL;
+
+	sdevice->type = "siox-12x8";
+	sdevice->inbytes = 2;
+	sdevice->outbytes = 4;
+
+	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);
+
+	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;
+
+	if (smaster->buf_len < smaster->setbuf_len + smaster->getbuf_len) {
+		smaster->buf_len = smaster->setbuf_len + smaster->getbuf_len;
+		smaster->buf = krealloc(smaster->buf,
+					smaster->buf_len, GFP_KERNEL);
+		if (!smaster->buf) {
+			dev_err(&smaster->dev, "failed to realloc buffer to %zu\n",
+				smaster->buf_len);
+			if (smaster->active)
+				__siox_stop(smaster);
+		}
+	}
+
+	siox_master_unlock(smaster);
+
+	return sdevice;
+
+err_device_register:
+	siox_master_unlock(smaster);
+
+	kfree(sdevice);
+
+	return ERR_PTR(ret);
+}
+
+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);
+
+	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->probe)
+		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;
+	}
+
+	wqueue = create_singlethread_workqueue("siox");
+	if (!wqueue) {
+		pr_err("Creation of siox workqueue failed\n");
+		bus_unregister(&siox_bus_type);
+		return -ENOMEM;
+	}
+
+	siox_is_registered = true;
+
+	return 0;
+}
+subsys_initcall(siox_init);
+
+static void __exit siox_exit(void)
+{
+	flush_workqueue(wqueue);
+	destroy_workqueue(wqueue);
+	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..9f0a8bc1f499
--- /dev/null
+++ b/drivers/siox/siox.h
@@ -0,0 +1,48 @@
+/*
+ * 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/kernel.h>
+#include <linux/workqueue.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 num_devices;
+	struct list_head devices;
+
+	size_t setbuf_len, getbuf_len;
+	size_t buf_len;
+	u8 *buf;
+	u8 status;
+
+	struct delayed_work poll;
+};
+
+int __siox_register_master(struct siox_master *smaster);
+
+static inline int siox_register_master(struct siox_master *smaster)
+{
+	smaster->owner = THIS_MODULE;
+	return __siox_register_master(smaster);
+}
+
+void siox_unregister_master(struct siox_master *smaster);
diff --git a/include/linux/siox.h b/include/linux/siox.h
new file mode 100644
index 000000000000..4ca0ab264be8
--- /dev/null
+++ b/include/linux/siox.h
@@ -0,0 +1,53 @@
+/*
+ * 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;
+
+	char *type;
+	size_t inbytes;
+	size_t outbytes;
+
+	u8 status;
+};
+
+#define to_siox_driver(_drv)	container_of((_drv), struct siox_driver, driver)
+struct siox_driver {
+	int (*probe)(struct siox_device *);
+	int (*remove)(struct siox_device *);
+	void (*shutdown)(struct siox_device *);
+
+	/*
+	 * 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 *, 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 *, const u8 buf[]);
+
+	struct device_driver driver;
+};
+
+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.7.0

[toc] | [next] | [standalone]

#1356236

On Fri, Mar 11, 2016 at 10:52:12PM +0100, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/Kconfig          |   2 +
>  drivers/Makefile         |   1 +
>  drivers/siox/Kconfig     |   2 +
>  drivers/siox/Makefile    |   1 +
>  drivers/siox/siox-core.c | 572 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/siox/siox.h      |  48 ++++
>  include/linux/siox.h     |  53 +++++
>  7 files changed, 679 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

What's the relationship between your "master" and "regular" devices
here?  I think that's getting things confused, you are unregistering
your 'master' yet the children it controls don't go away because you
never unregister them.  You need to fix this heirachy up a bit, and
maybe just drop the 'master' logic?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1356423

Hello Greg,

On Fri, Mar 11, 2016 at 02:20:39PM -0800, Greg Kroah-Hartman wrote:
> On Fri, Mar 11, 2016 at 10:52:12PM +0100, Uwe Kleine-König wrote:
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/Kconfig          |   2 +
> >  drivers/Makefile         |   1 +
> >  drivers/siox/Kconfig     |   2 +
> >  drivers/siox/Makefile    |   1 +
> >  drivers/siox/siox-core.c | 572 +++++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/siox/siox.h      |  48 ++++
> >  include/linux/siox.h     |  53 +++++
> >  7 files changed, 679 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
> 
> What's the relationship between your "master" and "regular" devices
> here?  I think that's getting things confused, you are unregistering
> your 'master' yet the children it controls don't go away because you
> never unregister them.  You need to fix this heirachy up a bit, and
> maybe just drop the 'master' logic?

A master is the bus controller (i.e. it drives DOUT, DCLK and DLD). A
device is a consumer and provides DIN to the master.

The topology looks as follows:

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

So there are two chains of shift registers, one for letting the master
write to the devices, and the other to let the master read from the
devices. DCLK is the clock to shift both chains by a single bit. An edge
on DLD is used to make the devices load the inputs and sample the
outputs.

I think I need the following:

	void siox_unregister_master(struct siox_master *smaster)
	{
		for (each siox_device d of smaster)
			siox_device_remove(d);

		device_unregister(&smaster->dev);
	}

But the locking scheme that is necessary isn't obvious for me. For
example I must prevent that after the for loop is done a new device is
registered. Do I need to prevent this explicitly (e.g. by doing
something like:

	lock(smaster);
	smaster->is_going_away = true;
	for (each siox_device d of smaster)
		siox_device_remove(d);
	unlock(smaster);

	device_unregister(&smaster->dev);

and let registering new devices fail if smaster->is_going_away is true?)
Or is there something that the kobject stuff/driver core can help me
with?

Best regards
Uwe

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

[toc] | [prev] | [next] | [standalone]

#1356604

On Sat, Mar 12, 2016 at 02:43:21PM +0100, Uwe Kleine-König wrote:
> Hello Greg,
> 
> On Fri, Mar 11, 2016 at 02:20:39PM -0800, Greg Kroah-Hartman wrote:
> > On Fri, Mar 11, 2016 at 10:52:12PM +0100, Uwe Kleine-König wrote:
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > >  drivers/Kconfig          |   2 +
> > >  drivers/Makefile         |   1 +
> > >  drivers/siox/Kconfig     |   2 +
> > >  drivers/siox/Makefile    |   1 +
> > >  drivers/siox/siox-core.c | 572 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  drivers/siox/siox.h      |  48 ++++
> > >  include/linux/siox.h     |  53 +++++
> > >  7 files changed, 679 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
> > 
> > What's the relationship between your "master" and "regular" devices
> > here?  I think that's getting things confused, you are unregistering
> > your 'master' yet the children it controls don't go away because you
> > never unregister them.  You need to fix this heirachy up a bit, and
> > maybe just drop the 'master' logic?
> 
> A master is the bus controller (i.e. it drives DOUT, DCLK and DLD). A
> device is a consumer and provides DIN to the master.
> 
> The topology looks as follows:
> 
>       ,------->--DCLK-->---------------+----------------------.
>       ^                                v                      v
>  ,--------.                ,----------------------.       ,------
>  |        |                |   ,--------------.   |       |
>  |        |--->--DOUT-->---|->-|shift register|->-|--->---|
>  |        |                |   `--------------'   |       |
>  | master |                |        device        |       |  device
>  |        |                |   ,--------------.   |       |
>  |        |---<--DIN---<---|-<-|shift register|-<-|---<---|
>  |        |                |   `--------------'   |       |
>  `--------'                `----------------------'       `------
>       v                                ^                      ^
>       `----------DLD-------------------+----------------------'
> 
> So there are two chains of shift registers, one for letting the master
> write to the devices, and the other to let the master read from the
> devices. DCLK is the clock to shift both chains by a single bit. An edge
> on DLD is used to make the devices load the inputs and sample the
> outputs.
> 
> I think I need the following:
> 
> 	void siox_unregister_master(struct siox_master *smaster)
> 	{
> 		for (each siox_device d of smaster)
> 			siox_device_remove(d);
> 
> 		device_unregister(&smaster->dev);
> 	}

Yes, you have to do this, as that's how USB and other busses do it when
their "controllers" go away.

> But the locking scheme that is necessary isn't obvious for me. For
> example I must prevent that after the for loop is done a new device is
> registered. Do I need to prevent this explicitly (e.g. by doing
> something like:
> 
> 	lock(smaster);
> 	smaster->is_going_away = true;
> 	for (each siox_device d of smaster)
> 		siox_device_remove(d);
> 	unlock(smaster);
> 
> 	device_unregister(&smaster->dev);
> 
> and let registering new devices fail if smaster->is_going_away is true?)
> Or is there something that the kobject stuff/driver core can help me
> with?

Look at what happens when you remove a USB host controller with the
locking there, it shouldn't be that hard.

thanks,

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web