[PATCH 0/3] drivers/mcb: Bus support for MEN Chameleon Bus

[PATCH 0/3] drivers/mcb: Bus support for MEN Chameleon Bus

[Johannes Thumshirn]

This patch-set introduces the MEN Chameleon Bus (MCB). MCB is not a "real" bus
like PCI, but a header section in the beginning of the FPGAs memory. This
headers are used to describe the IP-Cores inside that FPGA.

The MCB driver parses this header section and creates devices for the single
IP-Cores. It also provides an interface for device drivers to access these
resources.

Currently the bus driver has some limitations, for example IRQ handling or
memory mapped memory support, which will be addressed once drivers that need
these features are submitted.

As a first driver using the new bus, men_z188_adc.c, an IIO ADC driver, is
included in this patch-set.

Johannes Thumshirn (3):
  drivers: Introduce MEN Chameleon Bus
  mcb: Add PCI carrier for MEN Chameleon Bus
  iio: adc: Add MEN 16z188 ADC driver

 MAINTAINERS                     |   6 +
 drivers/Kconfig                 |   2 +
 drivers/Makefile                |   1 +
 drivers/iio/adc/Kconfig         |  10 +
 drivers/iio/adc/Makefile        |   1 +
 drivers/iio/adc/men_z188_adc.c  | 172 ++++++++++++++++
 drivers/mcb/Kconfig             |  29 +++
 drivers/mcb/Makefile            |   7 +
 drivers/mcb/mcb-core.c          | 420 ++++++++++++++++++++++++++++++++++++++++
 drivers/mcb/mcb-internal.h      | 118 +++++++++++
 drivers/mcb/mcb-parse.c         | 159 +++++++++++++++
 drivers/mcb/mcb-pci.c           | 108 +++++++++++
 include/linux/mcb.h             | 113 +++++++++++
 include/linux/mod_devicetable.h |   5 +
 14 files changed, 1151 insertions(+)
 create mode 100644 drivers/iio/adc/men_z188_adc.c
 create mode 100644 drivers/mcb/Kconfig
 create mode 100644 drivers/mcb/Makefile
 create mode 100644 drivers/mcb/mcb-core.c
 create mode 100644 drivers/mcb/mcb-internal.h
 create mode 100644 drivers/mcb/mcb-parse.c
 create mode 100644 drivers/mcb/mcb-pci.c
 create mode 100644 include/linux/mcb.h

--
1.8.5.4

[PATCH 1/3] drivers: Introduce MEN Chameleon Bus

[Johannes Thumshirn]

The MCB (MEN Chameleon Bus) is a Bus specific to MEN Mikroelektronik
FPGA based devices. It is used to identify MCB based IP-Cores within
an FPGA and provide the necessary framework for instantiating drivers
for these devices.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 MAINTAINERS                     |   6 +
 drivers/Kconfig                 |   2 +
 drivers/Makefile                |   1 +
 drivers/mcb/Kconfig             |  16 ++
 drivers/mcb/Makefile            |   5 +
 drivers/mcb/mcb-core.c          | 420 ++++++++++++++++++++++++++++++++++++++++
 drivers/mcb/mcb-internal.h      | 118 +++++++++++
 drivers/mcb/mcb-parse.c         | 159 +++++++++++++++
 include/linux/mcb.h             | 113 +++++++++++
 include/linux/mod_devicetable.h |   5 +
 10 files changed, 845 insertions(+)
 create mode 100644 drivers/mcb/Kconfig
 create mode 100644 drivers/mcb/Makefile
 create mode 100644 drivers/mcb/mcb-core.c
 create mode 100644 drivers/mcb/mcb-internal.h
 create mode 100644 drivers/mcb/mcb-parse.c
 create mode 100644 include/linux/mcb.h

diff --git a/MAINTAINERS b/MAINTAINERS
index fb08dce..b5300f2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5653,6 +5653,12 @@ L:	linux-watchdog@vger.kernel.org
 S:	Supported
 F:	drivers/watchdog/mena21_wdt.c
 
+MEN CHAMELEON BUS (mcb)
+M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
+S:	Supported
+F:	drivers/mcb/
+F:	include/linux/mcb.h
+
 METAG ARCHITECTURE
 M:	James Hogan <james.hogan@imgtec.com>
 L:	linux-metag@vger.kernel.org
diff --git a/drivers/Kconfig b/drivers/Kconfig
index b3138fb..df2ac52 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -170,4 +170,6 @@ source "drivers/phy/Kconfig"
 
 source "drivers/powercap/Kconfig"
 
+source "drivers/mcb/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 8e3b8b0..c5bf50c 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -155,3 +155,4 @@ obj-$(CONFIG_IPACK_BUS)		+= ipack/
 obj-$(CONFIG_NTB)		+= ntb/
 obj-$(CONFIG_FMC)		+= fmc/
 obj-$(CONFIG_POWERCAP)		+= powercap/
+obj-$(CONFIG_MCB)		+= mcb/
diff --git a/drivers/mcb/Kconfig b/drivers/mcb/Kconfig
new file mode 100644
index 0000000..9e7d6f5
--- /dev/null
+++ b/drivers/mcb/Kconfig
@@ -0,0 +1,16 @@
+#
+# MEN Chameleon Bus (MCB) support
+#
+
+menuconfig MCB
+	   tristate "MCB support"
+	   default m
+	   help
+
+	   The MCB (MEN Chameleon Bus) is a Bus specific to MEN Mikroelektronik
+	   FPGA based devices. It is used to identify MCB based IP-Cores within
+	   an FPGA and provide the necessary framework for instantiating drivers
+	   for these devices.
+
+	   If build as a module, the module is called mcb.ko
+
diff --git a/drivers/mcb/Makefile b/drivers/mcb/Makefile
new file mode 100644
index 0000000..2d9a751
--- /dev/null
+++ b/drivers/mcb/Makefile
@@ -0,0 +1,5 @@
+
+obj-$(CONFIG_MCB) += mcb.o
+
+mcb-y += mcb-core.o
+mcb-y += mcb-parse.o
diff --git a/drivers/mcb/mcb-core.c b/drivers/mcb/mcb-core.c
new file mode 100644
index 0000000..1ff3939
--- /dev/null
+++ b/drivers/mcb/mcb-core.c
@@ -0,0 +1,420 @@
+/*
+ * MEN Chameleon Bus.
+ *
+ * Copyright (C) 2013 MEN Mikroelektronik GmbH (www.men.de)
+ * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/rwsem.h>
+#include <linux/idr.h>
+#include <linux/mcb.h>
+
+static DECLARE_RWSEM(mcb_bus_sem);
+static DEFINE_IDA(mcb_ida);
+
+static const struct mcb_device_id *mcb_match_id(const struct mcb_device_id *ids,
+						struct mcb_device *dev)
+{
+	if (ids) {
+		while (ids->device) {
+			if (ids->device == dev->id)
+				return ids;
+			ids++;
+		}
+	}
+
+	return NULL;
+}
+
+static int mcb_match(struct device *dev, struct device_driver *drv)
+{
+	struct mcb_driver *mdrv = to_mcb_driver(drv);
+	struct mcb_device *mdev = to_mcb_device(dev);
+	const struct mcb_device_id *found_id;
+
+	found_id = mcb_match_id(mdrv->id_table, mdev);
+	if (found_id)
+		return 1;
+
+	return 0;
+}
+
+static int mcb_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	struct mcb_device *mdev = to_mcb_device(dev);
+	int ret;
+
+	ret = add_uevent_var(env, "MODALIAS=mcb:16z%03d", mdev->id);
+	if (ret)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int mcb_probe(struct device *dev)
+{
+	struct mcb_driver *mdrv = to_mcb_driver(dev->driver);
+	struct mcb_device *mdev = to_mcb_device(dev);
+	const struct mcb_device_id *found_id;
+
+	found_id = mcb_match_id(mdrv->id_table, mdev);
+	if (!found_id)
+		return -ENODEV;
+
+	return mdrv->probe(mdev, found_id);
+}
+
+static int mcb_remove(struct device *dev)
+{
+	struct mcb_driver *mdrv = to_mcb_driver(dev->driver);
+	struct mcb_device *mdev = to_mcb_device(dev);
+
+	mdrv->remove(mdev);
+
+	put_device(&mdev->dev);
+
+	return 0;
+}
+
+static void mcb_shutdown(struct device *dev)
+{
+	struct mcb_device *mdev = to_mcb_device(dev);
+	struct mcb_driver *mdrv = mdev->driver;
+
+	if (mdrv && mdrv->shutdown)
+		mdrv->shutdown(mdev);
+}
+
+static struct bus_type mcb_bus_type = {
+	.name = "mcb",
+	.match = mcb_match,
+	.uevent = mcb_uevent,
+	.probe = mcb_probe,
+	.remove = mcb_remove,
+	.shutdown = mcb_shutdown,
+};
+
+/**
+ * __mcb_register_driver() - Register a @mcb_driver at the system
+ * @drv: The @mcb_driver
+ * @owner: The @mcb_driver's module
+ * @mod_name: The name of the @mcb_driver's module
+ *
+ * Register a @mcb_driver at the system. Perform some sanity checks, if
+ * the .probe and .remove methods are provided by the driver.
+ */
+int __mcb_register_driver(struct mcb_driver *drv, struct module *owner,
+			const char *mod_name)
+{
+	if (!drv->probe || !drv->remove)
+		return -EINVAL;
+
+	drv->driver.owner = owner;
+	drv->driver.bus = &mcb_bus_type;
+	drv->driver.mod_name = mod_name;
+
+	return driver_register(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(__mcb_register_driver);
+
+/**
+ * mcb_unregister_driver() - Unregister a @mcb_driver from the system
+ * @drv: The @mcb_driver
+ *
+ * Unregister a @mcb_driver from the system.
+ */
+void mcb_unregister_driver(struct mcb_driver *drv)
+{
+	driver_unregister(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(mcb_unregister_driver);
+
+static void mcb_release_dev(struct device *dev)
+{
+	struct mcb_device *mdev = to_mcb_device(dev);
+
+	down_write(&mcb_bus_sem);
+	list_del(&mdev->bus_list);
+	up_write(&mcb_bus_sem);
+
+	mcb_bus_put(mdev->bus);
+	kfree(mdev);
+}
+
+/**
+ * mcb_device_register() - Register a mcb_device
+ * @bus: The @mcb_bus of the device
+ * @dev: The @mcb_device
+ *
+ * Register a specific @mcb_device at a @mcb_bus and the system itself.
+ */
+int mcb_device_register(struct mcb_bus *bus, struct mcb_device *dev)
+{
+	int ret;
+	int device_id;
+
+	device_initialize(&dev->dev);
+	dev->dev.bus = &mcb_bus_type;
+	dev->dev.parent = bus->dev.parent;
+	dev->dev.release = mcb_release_dev;
+
+
+	device_id = dev->id;
+	dev_set_name(&dev->dev, "mcb%d-16z%03d-%d:%d:%d",
+		bus->bus_nr, device_id, dev->inst, dev->group, dev->var);
+
+	down_write(&mcb_bus_sem);
+	list_add_tail(&dev->bus_list, &bus->devices);
+	up_write(&mcb_bus_sem);
+
+	ret = device_add(&dev->dev);
+	if (ret < 0) {
+		pr_err("Failed registering device 16z%03d on bus mcb%d (%d)\n",
+			device_id, bus->bus_nr, ret);
+		goto out;
+	}
+
+	return 0;
+
+out:
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mcb_device_register);
+
+/**
+ * mcb_alloc_bus() - Allocate a new @mcb_bus
+ *
+ * Allocate a new @mcb_bus.
+ */
+struct mcb_bus *mcb_alloc_bus(void)
+{
+	struct mcb_bus *bus;
+	int bus_nr;
+
+	bus = kzalloc(sizeof(struct mcb_bus), GFP_KERNEL);
+	if (!bus)
+		return NULL;
+
+	bus_nr = ida_simple_get(&mcb_ida, 0, 0, GFP_KERNEL);
+	if (bus_nr < 0) {
+		kfree(bus);
+		return ERR_PTR(bus_nr);
+	}
+
+	INIT_LIST_HEAD(&bus->node);
+	INIT_LIST_HEAD(&bus->children);
+	INIT_LIST_HEAD(&bus->devices);
+	bus->bus_nr = bus_nr;
+
+	return bus;
+}
+EXPORT_SYMBOL_GPL(mcb_alloc_bus);
+
+static void mcb_devices_unregister(struct mcb_bus *bus)
+{
+	struct mcb_device *child, *tmp;
+
+	list_for_each_entry_safe(child, tmp, &bus->devices, bus_list) {
+		if (&child->dev)
+			device_unregister(&child->dev);
+	}
+}
+/**
+ * mcb_release_bus() - Free a @mcb_bus
+ * @bus: The @mcb_bus to release
+ *
+ * Release an allocated @mcb_bus from the system.
+ */
+void mcb_release_bus(struct mcb_bus *bus)
+{
+	mcb_devices_unregister(bus);
+
+	down_write(&mcb_bus_sem);
+	list_del(&bus->node);
+	up_write(&mcb_bus_sem);
+
+	ida_simple_remove(&mcb_ida, bus->bus_nr);
+
+	kfree(bus);
+}
+EXPORT_SYMBOL_GPL(mcb_release_bus);
+
+/**
+ * mcb_bus_put() - Increment refcnt
+ * @bus: The @mcb_bus
+ *
+ * Get a @mcb_bus' ref
+ */
+struct mcb_bus *mcb_bus_get(struct mcb_bus *bus)
+{
+	if (bus)
+		get_device(&bus->dev);
+
+	return bus;
+}
+EXPORT_SYMBOL_GPL(mcb_bus_get);
+
+/**
+ * mcb_bus_put() - Decrement refcnt
+ * @bus: The @mcb_bus
+ *
+ * Release a @mcb_bus' ref
+ */
+void mcb_bus_put(struct mcb_bus *bus)
+{
+	if (bus)
+		put_device(&bus->dev);
+}
+EXPORT_SYMBOL_GPL(mcb_bus_put);
+
+/**
+ * mcb_alloc_dev() - Allocate a device
+ * @bus: The @mcb_bus the device is part of
+ *
+ * Allocate a @mcb_device and add bus.
+ */
+struct mcb_device *mcb_alloc_dev(struct mcb_bus *bus)
+{
+	struct mcb_device *dev;
+
+	dev = kzalloc(sizeof(struct mcb_device), GFP_KERNEL);
+	if (!dev)
+		return NULL;
+
+	INIT_LIST_HEAD(&dev->bus_list);
+	dev->bus = bus;
+
+	return dev;
+}
+EXPORT_SYMBOL_GPL(mcb_alloc_dev);
+
+/**
+ * mcb_free_dev() - Free @mcb_device
+ * @dev: The device to free
+ *
+ * Free a @mcb_device
+ */
+void mcb_free_dev(struct mcb_device *dev)
+{
+	kfree(dev);
+}
+EXPORT_SYMBOL_GPL(mcb_free_dev);
+
+/**
+ * mcb_bus_add_devices() - Add devices in the bus' internal device list
+ * @bus: The @mcb_bus we add the devices
+ *
+ * Add devices in the bus' internal device list to the system.
+ */
+void mcb_bus_add_devices(const struct mcb_bus *bus)
+{
+	struct mcb_device *dev;
+	struct mcb_bus *child;
+	int retval;
+
+	list_for_each_entry(dev, &bus->devices, bus_list) {
+		if (dev->is_added)
+			continue;
+
+		retval = device_attach(&dev->dev);
+		if (retval < 0)
+			dev_err(&dev->dev, "Error adding device (%d)",
+				retval);
+		dev->is_added = true;
+	}
+
+	list_for_each_entry(dev, &bus->devices, bus_list) {
+		BUG_ON(!dev->is_added);
+		child = dev->subordinate;
+
+		if (child)
+			mcb_bus_add_devices(child);
+	}
+}
+EXPORT_SYMBOL_GPL(mcb_bus_add_devices);
+
+/**
+ * mcb_request_mem() - Request memory
+ * @dev: The @mcb_device the memory is for
+ * @name: The name for the memory reference.
+ *
+ * Request memory for a @mcb_device. If @name is NULL the driver name will
+ * be used.
+ */
+struct resource *mcb_request_mem(struct mcb_device *dev, const char *name)
+{
+	struct resource *mem;
+	u32 size;
+
+	if (!name)
+		name = dev->dev.driver->name;
+
+	size = resource_size(&dev->mem);
+
+	mem = request_mem_region(dev->mem.start, size, name);
+	if (!mem)
+		return ERR_PTR(-EBUSY);
+
+	return mem;
+}
+EXPORT_SYMBOL_GPL(mcb_request_mem);
+
+/**
+ * mcb_release_mem() - Release memory requested by device
+ * @dev: The @mcb_device that requested the memory
+ *
+ * Release memory that was prior requested via @mcb_request_mem().
+ */
+void mcb_release_mem(struct resource *mem)
+{
+	u32 size;
+
+	size = resource_size(mem);
+	release_mem_region(mem->start, size);
+}
+EXPORT_SYMBOL_GPL(mcb_release_mem);
+
+/**
+ * mcb_get_irq() - Get device's IRQ number
+ * @dev: The @mcb_device the IRQ is for
+ *
+ * Get the IRQ number of a given @mcb_device.
+ */
+int mcb_get_irq(struct mcb_device *dev)
+{
+	struct resource *irq = &dev->irq;
+
+	return irq ? irq->start : -ENXIO;
+}
+EXPORT_SYMBOL_GPL(mcb_get_irq);
+
+static int mcb_init(void)
+{
+	ida_init(&mcb_ida);
+	return bus_register(&mcb_bus_type);
+}
+
+static void mcb_exit(void)
+{
+	bus_unregister(&mcb_bus_type);
+	ida_destroy(&mcb_ida);
+}
+
+/* mcb must be initialized after PCI but before the chameleon drivers.
+ * That means we must use some initcall between subsys_initcall and
+ * device_initcall.
+ */
+fs_initcall(mcb_init);
+module_exit(mcb_exit);
+
+MODULE_DESCRIPTION("MEN Chameleon Bus Driver");
+MODULE_AUTHOR("Johannes Thumshirn <johannes.thumshirn@men.de>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mcb/mcb-internal.h b/drivers/mcb/mcb-internal.h
new file mode 100644
index 0000000..ca253e3
--- /dev/null
+++ b/drivers/mcb/mcb-internal.h
@@ -0,0 +1,118 @@
+#ifndef __MCB_INTERNAL
+#define __MCB_INTERNAL
+
+#include <linux/types.h>
+
+#define PCI_VENDOR_ID_MEN		0x1a88
+#define PCI_DEVICE_ID_MEN_CHAMELEON	0x4d45
+#define CHAMELEON_FILENAME_LEN		12
+#define CHAMELEONV2_MAGIC		0xabce
+
+enum chameleon_descriptor_type {
+	CHAMELEON_DTYPE_GENERAL = 0x0,
+	CHAMELEON_DTYPE_BRIDGE = 0x1,
+	CHAMELEON_DTYPE_CPU = 0x2,
+	CHAMELEON_DTYPE_BAR = 0x3,
+	CHAMELEON_DTYPE_END = 0xf,
+};
+
+enum chameleon_bus_type {
+	CHAMELEON_BUS_WISHBONE,
+	CHAMELEON_BUS_AVALON,
+	CHAMELEON_BUS_LPC,
+	CHAMELEON_BUS_ISA,
+};
+
+/**
+ * struct chameleon_fpga_header
+ *
+ * @revision:	Revison of Chameleon table in FPGA
+ * @model:	Chameleon table model ASCII char
+ * @minor:	Revision minor
+ * @bus_type:	Bus type (usually %CHAMELEON_BUS_WISHBONE)
+ * @magic:	Chameleon header magic number (0xabce for version 2)
+ * @reserved:	Reserved
+ * @filename:	Filename of FPGA bitstream
+ */
+struct chameleon_fpga_header {
+	u8 revision;
+	char model;
+	u8 minor;
+	u8 bus_type;
+	u16 magic;
+	u16 reserved;
+	/* This one has no '\0' at the end!!! */
+	char filename[CHAMELEON_FILENAME_LEN];
+} __packed;
+#define HEADER_MAGIC_OFFSET 0x4
+
+/**
+ * struct chameleon_gdd - Chameleon General Device Descriptor
+ *
+ * @irq:	the position in the FPGA's IRQ controller vector
+ * @rev:	the revision of the variant's implementation
+ * @var:	the variant of the IP core
+ * @dev:	the device  the IP core is
+ * @dtype:	device descriptor type
+ * @bar:	BAR offset that must be added to module offset
+ * @inst:	the instance number of the device, 0 is first instance
+ * @group:	the group the device belongs to (0 = no group)
+ * @reserved:	reserved
+ * @offset:	beginning of the address window of desired module
+ * @size:	size of the module's address window
+ */
+struct chameleon_gdd {
+	__le32 reg1;
+	__le32 reg2;
+	__le32 offset;
+	__le32 size;
+
+} __packed;
+
+/* GDD Register 1 fields */
+#define GDD_IRQ(x) ((x) & 0x1f)
+#define GDD_REV(x) (((x) >> 5) & 0x3f)
+#define GDD_VAR(x) (((x) >> 11) & 0x3f)
+#define GDD_DEV(x) (((x) >> 18) & 0x3ff)
+#define GDD_DTY(x) (((x) >> 28) & 0xf)
+
+/* GDD Register 2 fields */
+#define GDD_BAR(x) ((x) & 0x7)
+#define GDD_INS(x) (((x) >> 3) & 0x3f)
+#define GDD_GRP(x) (((x) >> 9) & 0x3f)
+
+/**
+ * struct chameleon_bdd - Chameleon Bridge Device Descriptor
+ *
+ * @irq:	the position in the FPGA's IRQ controller vector
+ * @rev:	the revision of the variant's implementation
+ * @var:	the variant of the IP core
+ * @dev:	the device  the IP core is
+ * @dtype:	device descriptor type
+ * @bar:	BAR offset that must be added to module offset
+ * @inst:	the instance number of the device, 0 is first instance
+ * @dbar:	destination bar from the bus _behind_ the bridge
+ * @chamoff:	offset within the BAR of the source bus
+ * @offset:
+ * @size:
+ */
+struct chameleon_bdd {
+	unsigned int irq:6;
+	unsigned int rev:6;
+	unsigned int var:6;
+	unsigned int dev:10;
+	unsigned int dtype:4;
+	unsigned int bar:3;
+	unsigned int inst:6;
+	unsigned int dbar:3;
+	unsigned int group:6;
+	unsigned int reserved:14;
+	u32 chamoff;
+	u32 offset;
+	u32 size;
+} __packed;
+
+int parse_chameleon_cells(struct mcb_bus *bus, phys_addr_t mapbase,
+			  void __iomem *base);
+
+#endif
diff --git a/drivers/mcb/mcb-parse.c b/drivers/mcb/mcb-parse.c
new file mode 100644
index 0000000..82896fb
--- /dev/null
+++ b/drivers/mcb/mcb-parse.c
@@ -0,0 +1,159 @@
+#include <linux/types.h>
+#include <linux/ioport.h>
+#include <linux/slab.h>
+#include <linux/export.h>
+#include <linux/io.h>
+#include <linux/mcb.h>
+
+#include "mcb-internal.h"
+
+struct mcb_parse_priv {
+	phys_addr_t mapbase;
+	void __iomem *base;
+};
+
+#define for_each_chameleon_cell(dtype, p)	\
+	for ((dtype) = get_next_dtype((p));	\
+	     (dtype) != CHAMELEON_DTYPE_END;	\
+	     (dtype) = get_next_dtype((p)))
+
+static inline uint32_t get_next_dtype(void __iomem *p)
+{
+	uint32_t dtype;
+
+	dtype = readl(p);
+	return dtype >> 28;
+}
+
+static int parse_chameleon_bdd(struct mcb_bus *bus,
+			phys_addr_t mapbase,
+			void __iomem *base)
+{
+	return 0;
+}
+
+static int parse_chameleon_gdd(struct mcb_bus *bus,
+			phys_addr_t mapbase,
+			void __iomem *base)
+{
+	struct chameleon_gdd __iomem *gdd =
+		(struct chameleon_gdd __iomem *) base;
+	struct mcb_device *mdev;
+	u32 offset;
+	u32 size;
+	int ret;
+	__le32 reg1;
+	__le32 reg2;
+
+	mdev = mcb_alloc_dev(bus);
+	if (!mdev)
+		return -ENOMEM;
+
+	reg1 = readl(&gdd->reg1);
+	reg2 = readl(&gdd->reg2);
+	offset = readl(&gdd->offset);
+	size = readl(&gdd->size);
+
+	mdev->id = GDD_DEV(reg1);
+	mdev->rev = GDD_REV(reg1);
+	mdev->var = GDD_VAR(reg1);
+	mdev->bar = GDD_BAR(reg1);
+	mdev->group = GDD_GRP(reg2);
+	mdev->inst = GDD_INS(reg2);
+
+	pr_debug("Found a 16z%03d\n", mdev->id);
+
+	mdev->irq.start = GDD_IRQ(reg1);
+	mdev->irq.end = GDD_IRQ(reg1);
+	mdev->irq.flags = IORESOURCE_IRQ;
+
+	mdev->mem.start = mapbase + offset;
+	mdev->mem.end = mdev->mem.start + size - 1;
+	mdev->mem.flags = IORESOURCE_MEM;
+
+	mdev->is_added = false;
+
+	ret = mcb_device_register(bus, mdev);
+	if (ret < 0)
+		goto err;
+
+	return 0;
+
+err:
+	mcb_free_dev(mdev);
+
+	return ret;
+}
+
+int parse_chameleon_cells(struct mcb_bus *bus, phys_addr_t mapbase,
+			void __iomem *base)
+{
+	char __iomem *p = base;
+	struct chameleon_fpga_header *header;
+	uint32_t dtype;
+	int num_cells = 0;
+	int ret = 0;
+	u32 hsize;
+
+	hsize = sizeof(struct chameleon_fpga_header);
+
+	header = kzalloc(hsize, GFP_KERNEL);
+	if (!header)
+		return -ENOMEM;
+
+	/* Extract header information */
+	memcpy_fromio(header, p, hsize);
+	/* We only support chameleon v2 at the moment */
+	header->magic = le16_to_cpu(header->magic);
+	if (header->magic != CHAMELEONV2_MAGIC) {
+		pr_err("Unsupported chameleon version 0x%x\n",
+				header->magic);
+		kfree(header);
+		return -ENODEV;
+	}
+	p += hsize;
+
+	pr_debug("header->revision = %d\n", header->revision);
+	pr_debug("header->model = 0x%x ('%c')\n", header->model,
+		header->model);
+	pr_debug("header->minor = %d\n", header->minor);
+	pr_debug("header->bus_type = 0x%x\n", header->bus_type);
+
+
+	pr_debug("header->magic = 0x%x\n", header->magic);
+	pr_debug("header->filename = \"%.*s\"\n", CHAMELEON_FILENAME_LEN,
+		header->filename);
+
+	for_each_chameleon_cell(dtype, p) {
+		switch (dtype) {
+		case CHAMELEON_DTYPE_GENERAL:
+			ret = parse_chameleon_gdd(bus, mapbase, p);
+			if (ret < 0)
+				goto out;
+			p += sizeof(struct chameleon_gdd);
+			break;
+		case CHAMELEON_DTYPE_BRIDGE:
+			parse_chameleon_bdd(bus, mapbase, p);
+			p += sizeof(struct chameleon_bdd);
+			break;
+		case CHAMELEON_DTYPE_END:
+			break;
+		default:
+			pr_err("Invalid chameleon descriptor type 0x%x\n",
+				dtype);
+			return -EINVAL;
+		}
+		num_cells++;
+	}
+
+	if (num_cells == 0)
+		num_cells = -EINVAL;
+
+	kfree(header);
+	return num_cells;
+
+out:
+	kfree(header);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(parse_chameleon_cells);
diff --git a/include/linux/mcb.h b/include/linux/mcb.h
new file mode 100644
index 0000000..ee86286
--- /dev/null
+++ b/include/linux/mcb.h
@@ -0,0 +1,113 @@
+/*
+ * MEN Chameleon Bus.
+ *
+ * Copyright (C) 2014 MEN Mikroelektronik GmbH (www.men.de)
+ * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ */
+#ifndef _LINUX_MCB_H
+#define _LINUX_MCB_H
+
+#include <linux/mod_devicetable.h>
+#include <linux/device.h>
+#include <linux/irqreturn.h>
+
+struct mcb_driver;
+
+/**
+ * struct mcb_bus - MEN Chameleon Bus
+ *
+ * @parent: Pointer to carrier device
+ * @cores: Number of cores available on this bus
+ * @bus_nr: mcb bus number
+ */
+struct mcb_bus {
+	struct list_head node;
+	struct list_head devices;
+	struct list_head children;
+	struct device dev;
+	int bus_nr;
+};
+#define to_mcb_bus(b) container_of((b), struct mcb_bus, dev)
+
+/**
+ * struct mcb_device - MEN Chameleon Bus device
+ *
+ * @bus_list: internal list handling for bus code
+ * @dev: device in kernel representation
+ * @bus: mcb bus the device is plugged to
+ * @subordinate: subordinate MCBus in case of bridge
+ * @is_added: flag to check if device is added to bus
+ * @driver: associated mcb_driver
+ * @id: mcb device id
+ * @inst: instance in Chameleon table
+ * @group: group in Chameleon table
+ * @var: variant in Chameleon table
+ * @bar: BAR in Chameleon table
+ * @rev: revision in Chameleon table
+ * @irq: IRQ resource
+ * @memory: memory resource
+ */
+struct mcb_device {
+	struct list_head bus_list;
+	struct device dev;
+	struct mcb_bus *bus;
+	struct mcb_bus *subordinate;
+	bool is_added;
+	struct mcb_driver *driver;
+	u16 id;
+	int inst;
+	int group;
+	int var;
+	int bar;
+	int rev;
+	struct resource irq;
+	struct resource mem;
+};
+#define to_mcb_device(x) container_of((x), struct mcb_device, dev)
+
+struct mcb_driver {
+	struct list_head node;
+	struct device_driver driver;
+	const struct mcb_device_id *id_table;
+	int (*probe)(struct mcb_device *mdev, const struct mcb_device_id *id);
+	void (*remove)(struct mcb_device *mdev);
+	void (*shutdown)(struct mcb_device *mdev);
+};
+#define to_mcb_driver(x) container_of((x), struct mcb_driver, driver)
+
+static inline void *mcb_get_drvdata(struct mcb_device *dev)
+{
+	return dev_get_drvdata(&dev->dev);
+}
+
+static inline void mcb_set_drvdata(struct mcb_device *dev, void *data)
+{
+	dev_set_drvdata(&dev->dev, data);
+}
+
+extern int __must_check __mcb_register_driver(struct mcb_driver *drv,
+					struct module *owner,
+					const char *mod_name);
+#define mcb_register_driver(driver)		\
+	__mcb_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
+extern void mcb_unregister_driver(struct mcb_driver *driver);
+#define module_mcb_driver(__mcb_driver)		\
+	module_driver(__mcb_driver, mcb_register_driver, mcb_unregister_driver);
+extern void mcb_bus_add_devices(const struct mcb_bus *bus);
+extern int mcb_device_register(struct mcb_bus *bus, struct mcb_device *dev);
+extern struct mcb_bus *mcb_alloc_bus(void);
+extern struct mcb_bus *mcb_bus_get(struct mcb_bus *bus);
+extern void mcb_bus_put(struct mcb_bus *bus);
+extern struct mcb_device *mcb_alloc_dev(struct mcb_bus *bus);
+extern void mcb_free_dev(struct mcb_device *dev);
+extern void mcb_release_bus(struct mcb_bus *bus);
+extern struct resource *mcb_request_mem(struct mcb_device *dev,
+					const char *name);
+extern void mcb_release_mem(struct resource *mem);
+extern int mcb_get_irq(struct mcb_device *dev);
+
+#endif /* _LINUX_MCB_H */
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 45e9214..262cb85 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -599,4 +599,9 @@ struct rio_device_id {
 	__u16 asm_did, asm_vid;
 };
 
+struct mcb_device_id {
+	__u16 device;
+	kernel_ulong_t driver_data;
+};
+
 #endif /* LINUX_MOD_DEVICETABLE_H */
-- 
1.8.5.4

Re: [PATCH 1/3] drivers: Introduce MEN Chameleon Bus

[Greg Kroah-Hartman]

On Tue, Feb 18, 2014 at 04:34:12PM +0100, Johannes Thumshirn wrote:
> The MCB (MEN Chameleon Bus) is a Bus specific to MEN Mikroelektronik
> FPGA based devices. It is used to identify MCB based IP-Cores within
> an FPGA and provide the necessary framework for instantiating drivers
> for these devices.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>

Looks good to me, nice job with the driver core integration.

Want me to queue this up through my tree?  Or do you want to have it go
through somewhere else?

thanks,

greg k-h

Re: [PATCH 1/3] drivers: Introduce MEN Chameleon Bus

[Johannes Thumshirn]

On Tue, Feb 18, 2014 at 01:48:19PM -0800, Greg Kroah-Hartman wrote:
> On Tue, Feb 18, 2014 at 04:34:12PM +0100, Johannes Thumshirn wrote:
> > The MCB (MEN Chameleon Bus) is a Bus specific to MEN Mikroelektronik
> > FPGA based devices. It is used to identify MCB based IP-Cores within
> > an FPGA and provide the necessary framework for instantiating drivers
> > for these devices.
> >
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
>
> Looks good to me, nice job with the driver core integration.
>

Thanks.

> Want me to queue this up through my tree?  Or do you want to have it go
> through somewhere else?
>

I don't really care. If you want to take it, I'd be pleased. On the other hand
PATCH 3/3 is a IIO driver, which depends on the other 2 patches, so I could be
better to take this via the IIO tree. I don't know which is better.

But apart from that, I first want/need to work in Josh Cartwright's comments,
but this could be delayed till monday, sorry.

Thanks,
Johannes

Re: [PATCH 1/3] drivers: Introduce MEN Chameleon Bus

[Josh Cartwright]

Hello Johannes,

On Tue, Feb 18, 2014 at 04:34:12PM +0100, Johannes Thumshirn wrote:
[..]
> +++ b/drivers/mcb/mcb-core.c
> @@ -0,0 +1,420 @@
> +/*
> + * MEN Chameleon Bus.
> + *
> + * Copyright (C) 2013 MEN Mikroelektronik GmbH (www.men.de)
> + * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; version 2 of the License.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/rwsem.h>
> +#include <linux/idr.h>
> +#include <linux/mcb.h>
> +
> +static DECLARE_RWSEM(mcb_bus_sem);

Why a rwsemaphore and not a simple mutex?  It doesn't appear like you
attempt to handle any multiple-reader usecases.

> +static DEFINE_IDA(mcb_ida);
> +
[..]
> +static void mcb_release_dev(struct device *dev)
> +{
> +	struct mcb_device *mdev = to_mcb_device(dev);
> +
> +	down_write(&mcb_bus_sem);
> +	list_del(&mdev->bus_list);
> +	up_write(&mcb_bus_sem);

Why even maintain your own list of devices?  Doesn't the bus_type
already do so for you? (And provides conveniences such as
bus_for_each_dev, etc).

> +
> +	mcb_bus_put(mdev->bus);
> +	kfree(mdev);
> +}
> +
> +/**
> + * mcb_device_register() - Register a mcb_device
> + * @bus: The @mcb_bus of the device
> + * @dev: The @mcb_device
> + *
> + * Register a specific @mcb_device at a @mcb_bus and the system itself.
> + */
> +int mcb_device_register(struct mcb_bus *bus, struct mcb_device *dev)
> +{
> +	int ret;
> +	int device_id;
> +
> +	device_initialize(&dev->dev);
> +	dev->dev.bus = &mcb_bus_type;
> +	dev->dev.parent = bus->dev.parent;
> +	dev->dev.release = mcb_release_dev;
> +
> +

Nit: extraneous line.

> +	device_id = dev->id;
> +	dev_set_name(&dev->dev, "mcb%d-16z%03d-%d:%d:%d",
> +		bus->bus_nr, device_id, dev->inst, dev->group, dev->var);
> +
> +	down_write(&mcb_bus_sem);
> +	list_add_tail(&dev->bus_list, &bus->devices);
> +	up_write(&mcb_bus_sem);

Again, unsure why you maintain your own list :(.

> +
> +	ret = device_add(&dev->dev);
> +	if (ret < 0) {
> +		pr_err("Failed registering device 16z%03d on bus mcb%d (%d)\n",
> +			device_id, bus->bus_nr, ret);
> +		goto out;
> +	}
> +
> +	return 0;
> +
> +out:
> +
> +	return ret;
> +}
[..]
> +/**
> + * mcb_get_irq() - Get device's IRQ number
> + * @dev: The @mcb_device the IRQ is for
> + *
> + * Get the IRQ number of a given @mcb_device.
> + */
> +int mcb_get_irq(struct mcb_device *dev)
> +{
> +	struct resource *irq = &dev->irq;
> +
> +	return irq ? irq->start : -ENXIO;

How could irq ever be NULL?

> +}
> +EXPORT_SYMBOL_GPL(mcb_get_irq);
> +
> +static int mcb_init(void)
> +{
> +	ida_init(&mcb_ida);

This isn't explicitly necessary (DEFINE_IDA statically initializes the
ida).

> +	return bus_register(&mcb_bus_type);
> +}
> +
> +static void mcb_exit(void)
> +{
> +	bus_unregister(&mcb_bus_type);
> +	ida_destroy(&mcb_ida);

This is also unnecessary.

> +}
> +
> +/* mcb must be initialized after PCI but before the chameleon drivers.
> + * That means we must use some initcall between subsys_initcall and
> + * device_initcall.
> + */
> +fs_initcall(mcb_init);
> +module_exit(mcb_exit);
> +
> +MODULE_DESCRIPTION("MEN Chameleon Bus Driver");
> +MODULE_AUTHOR("Johannes Thumshirn <johannes.thumshirn@men.de>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mcb/mcb-internal.h b/drivers/mcb/mcb-internal.h
> new file mode 100644
> index 0000000..ca253e3
> --- /dev/null
> +++ b/drivers/mcb/mcb-internal.h
> @@ -0,0 +1,118 @@
> +#ifndef __MCB_INTERNAL
> +#define __MCB_INTERNAL
> +
> +#include <linux/types.h>
> +
> +#define PCI_VENDOR_ID_MEN		0x1a88
> +#define PCI_DEVICE_ID_MEN_CHAMELEON	0x4d45

This seems misplaced.  Perhaps it should really be in the PCI carrier
patch?

> +#define CHAMELEON_FILENAME_LEN		12
> +#define CHAMELEONV2_MAGIC		0xabce
> +
[..]
> +int parse_chameleon_cells(struct mcb_bus *bus, phys_addr_t mapbase,
> +			  void __iomem *base);

Nit: it'd be nicer if you consistently used the chameleon_ prefix.  That
is, rename this chameleon_parse_cells(...).

[..]
> +int parse_chameleon_cells(struct mcb_bus *bus, phys_addr_t mapbase,
> +			void __iomem *base)
> +{
> +	char __iomem *p = base;
> +	struct chameleon_fpga_header *header;
> +	uint32_t dtype;
> +	int num_cells = 0;
> +	int ret = 0;
> +	u32 hsize;
> +
> +	hsize = sizeof(struct chameleon_fpga_header);
> +
> +	header = kzalloc(hsize, GFP_KERNEL);
> +	if (!header)
> +		return -ENOMEM;
> +
> +	/* Extract header information */
> +	memcpy_fromio(header, p, hsize);
> +	/* We only support chameleon v2 at the moment */
> +	header->magic = le16_to_cpu(header->magic);
> +	if (header->magic != CHAMELEONV2_MAGIC) {
> +		pr_err("Unsupported chameleon version 0x%x\n",
> +				header->magic);
> +		kfree(header);
> +		return -ENODEV;
> +	}
> +	p += hsize;
> +
> +	pr_debug("header->revision = %d\n", header->revision);
> +	pr_debug("header->model = 0x%x ('%c')\n", header->model,
> +		header->model);
> +	pr_debug("header->minor = %d\n", header->minor);
> +	pr_debug("header->bus_type = 0x%x\n", header->bus_type);
> +
> +
> +	pr_debug("header->magic = 0x%x\n", header->magic);
> +	pr_debug("header->filename = \"%.*s\"\n", CHAMELEON_FILENAME_LEN,
> +		header->filename);
> +
> +	for_each_chameleon_cell(dtype, p) {
> +		switch (dtype) {
> +		case CHAMELEON_DTYPE_GENERAL:
> +			ret = parse_chameleon_gdd(bus, mapbase, p);

Same comment about using chameleon_ prefix.

> +			if (ret < 0)
> +				goto out;
> +			p += sizeof(struct chameleon_gdd);
> +			break;
> +		case CHAMELEON_DTYPE_BRIDGE:
> +			parse_chameleon_bdd(bus, mapbase, p);

And this one, too :).

[..]
> +++ b/include/linux/mcb.h
> @@ -0,0 +1,113 @@
> +/*
> + * MEN Chameleon Bus.
> + *
> + * Copyright (C) 2014 MEN Mikroelektronik GmbH (www.men.de)
> + * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; version 2 of the License.
> + */
> +#ifndef _LINUX_MCB_H
> +#define _LINUX_MCB_H
> +
> +#include <linux/mod_devicetable.h>
> +#include <linux/device.h>
> +#include <linux/irqreturn.h>
> +
> +struct mcb_driver;
> +
> +/**
> + * struct mcb_bus - MEN Chameleon Bus
> + *
> + * @parent: Pointer to carrier device
> + * @cores: Number of cores available on this bus

Looks like these comments could be updated...

> + * @bus_nr: mcb bus number
> + */
> +struct mcb_bus {
> +	struct list_head node;
> +	struct list_head devices;
> +	struct list_head children;
> +	struct device dev;
> +	int bus_nr;
> +};
> +#define to_mcb_bus(b) container_of((b), struct mcb_bus, dev)
> +
> +/**
> + * struct mcb_device - MEN Chameleon Bus device
> + *
> + * @bus_list: internal list handling for bus code
> + * @dev: device in kernel representation
> + * @bus: mcb bus the device is plugged to
> + * @subordinate: subordinate MCBus in case of bridge
> + * @is_added: flag to check if device is added to bus
> + * @driver: associated mcb_driver
> + * @id: mcb device id
> + * @inst: instance in Chameleon table
> + * @group: group in Chameleon table
> + * @var: variant in Chameleon table
> + * @bar: BAR in Chameleon table
> + * @rev: revision in Chameleon table
> + * @irq: IRQ resource
> + * @memory: memory resource
> + */
> +struct mcb_device {
> +	struct list_head bus_list;
> +	struct device dev;
> +	struct mcb_bus *bus;
> +	struct mcb_bus *subordinate;
> +	bool is_added;
> +	struct mcb_driver *driver;
> +	u16 id;
> +	int inst;
> +	int group;
> +	int var;
> +	int bar;
> +	int rev;
> +	struct resource irq;
> +	struct resource mem;
> +};
> +#define to_mcb_device(x) container_of((x), struct mcb_device, dev)
> +
> +struct mcb_driver {

This could probably use a kerneldoc.

> +	struct list_head node;

How is this node used?

> +	struct device_driver driver;
> +	const struct mcb_device_id *id_table;
> +	int (*probe)(struct mcb_device *mdev, const struct mcb_device_id *id);
> +	void (*remove)(struct mcb_device *mdev);
> +	void (*shutdown)(struct mcb_device *mdev);
> +};

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 1/3] drivers: Introduce MEN Chameleon Bus

[Johannes Thumshirn]

Hello Josh,

On Tue, Feb 18, 2014 at 05:02:10PM -0600, Josh Cartwright wrote:
> Hello Johannes,
>
> On Tue, Feb 18, 2014 at 04:34:12PM +0100, Johannes Thumshirn wrote:
> [..]
> > +++ b/drivers/mcb/mcb-core.c
> > @@ -0,0 +1,420 @@
> > +/*
> > + * MEN Chameleon Bus.
> > + *
> > + * Copyright (C) 2013 MEN Mikroelektronik GmbH (www.men.de)
> > + * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the Free
> > + * Software Foundation; version 2 of the License.
> > + */
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +#include <linux/rwsem.h>
> > +#include <linux/idr.h>
> > +#include <linux/mcb.h>
> > +
> > +static DECLARE_RWSEM(mcb_bus_sem);
>
> Why a rwsemaphore and not a simple mutex?  It doesn't appear like you
> attempt to handle any multiple-reader usecases.

Yup, you're right. It's actually a copy'n'paste from PCI.

>
> > +static DEFINE_IDA(mcb_ida);
> > +
> [..]
> > +static void mcb_release_dev(struct device *dev)
> > +{
> > +	struct mcb_device *mdev = to_mcb_device(dev);
> > +
> > +	down_write(&mcb_bus_sem);
> > +	list_del(&mdev->bus_list);
> > +	up_write(&mcb_bus_sem);
>
> Why even maintain your own list of devices?  Doesn't the bus_type
> already do so for you? (And provides conveniences such as
> bus_for_each_dev, etc).

Again, I looked how PCI handles this stuff and just copied everything I thought it could
be important to me.

>
> > +
> > +	mcb_bus_put(mdev->bus);
> > +	kfree(mdev);
> > +}
> > +
> > +/**
> > + * mcb_device_register() - Register a mcb_device
> > + * @bus: The @mcb_bus of the device
> > + * @dev: The @mcb_device
> > + *
> > + * Register a specific @mcb_device at a @mcb_bus and the system itself.
> > + */
> > +int mcb_device_register(struct mcb_bus *bus, struct mcb_device *dev)
> > +{
> > +	int ret;
> > +	int device_id;
> > +
> > +	device_initialize(&dev->dev);
> > +	dev->dev.bus = &mcb_bus_type;
> > +	dev->dev.parent = bus->dev.parent;
> > +	dev->dev.release = mcb_release_dev;
> > +
> > +
>
> Nit: extraneous line.

Yup.

>
> > +	device_id = dev->id;
> > +	dev_set_name(&dev->dev, "mcb%d-16z%03d-%d:%d:%d",
> > +		bus->bus_nr, device_id, dev->inst, dev->group, dev->var);
> > +
> > +	down_write(&mcb_bus_sem);
> > +	list_add_tail(&dev->bus_list, &bus->devices);
> > +	up_write(&mcb_bus_sem);
>
> Again, unsure why you maintain your own list :(.

See comment above.

>
> > +
> > +	ret = device_add(&dev->dev);
> > +	if (ret < 0) {
> > +		pr_err("Failed registering device 16z%03d on bus mcb%d (%d)\n",
> > +			device_id, bus->bus_nr, ret);
> > +		goto out;
> > +	}
> > +
> > +	return 0;
> > +
> > +out:
> > +
> > +	return ret;
> > +}
> [..]
> > +/**
> > + * mcb_get_irq() - Get device's IRQ number
> > + * @dev: The @mcb_device the IRQ is for
> > + *
> > + * Get the IRQ number of a given @mcb_device.
> > + */
> > +int mcb_get_irq(struct mcb_device *dev)
> > +{
> > +	struct resource *irq = &dev->irq;
> > +
> > +	return irq ? irq->start : -ENXIO;
>
> How could irq ever be NULL?
>

You're right.

> > +}
> > +EXPORT_SYMBOL_GPL(mcb_get_irq);
> > +
> > +static int mcb_init(void)
> > +{
> > +	ida_init(&mcb_ida);
>
> This isn't explicitly necessary (DEFINE_IDA statically initializes the
> ida).
>

Blindly copied from ipack.c. Looks like it should be changed over there as well then.

> > +	return bus_register(&mcb_bus_type);
> > +}
> > +
> > +static void mcb_exit(void)
> > +{
> > +	bus_unregister(&mcb_bus_type);
> > +	ida_destroy(&mcb_ida);
>
> This is also unnecessary.

See comment above.

>
> > +}
> > +
> > +/* mcb must be initialized after PCI but before the chameleon drivers.
> > + * That means we must use some initcall between subsys_initcall and
> > + * device_initcall.
> > + */
> > +fs_initcall(mcb_init);
> > +module_exit(mcb_exit);
> > +
> > +MODULE_DESCRIPTION("MEN Chameleon Bus Driver");
> > +MODULE_AUTHOR("Johannes Thumshirn <johannes.thumshirn@men.de>");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/mcb/mcb-internal.h b/drivers/mcb/mcb-internal.h
> > new file mode 100644
> > index 0000000..ca253e3
> > --- /dev/null
> > +++ b/drivers/mcb/mcb-internal.h
> > @@ -0,0 +1,118 @@
> > +#ifndef __MCB_INTERNAL
> > +#define __MCB_INTERNAL
> > +
> > +#include <linux/types.h>
> > +
> > +#define PCI_VENDOR_ID_MEN		0x1a88
> > +#define PCI_DEVICE_ID_MEN_CHAMELEON	0x4d45
>
> This seems misplaced.  Perhaps it should really be in the PCI carrier
> patch?

Yup.

>
> > +#define CHAMELEON_FILENAME_LEN		12
> > +#define CHAMELEONV2_MAGIC		0xabce
> > +
> [..]
> > +int parse_chameleon_cells(struct mcb_bus *bus, phys_addr_t mapbase,
> > +			  void __iomem *base);
>
> Nit: it'd be nicer if you consistently used the chameleon_ prefix.  That
> is, rename this chameleon_parse_cells(...).
>
> [..]
> > +int parse_chameleon_cells(struct mcb_bus *bus, phys_addr_t mapbase,
> > +			void __iomem *base)
> > +{
> > +	char __iomem *p = base;
> > +	struct chameleon_fpga_header *header;
> > +	uint32_t dtype;
> > +	int num_cells = 0;
> > +	int ret = 0;
> > +	u32 hsize;
> > +
> > +	hsize = sizeof(struct chameleon_fpga_header);
> > +
> > +	header = kzalloc(hsize, GFP_KERNEL);
> > +	if (!header)
> > +		return -ENOMEM;
> > +
> > +	/* Extract header information */
> > +	memcpy_fromio(header, p, hsize);
> > +	/* We only support chameleon v2 at the moment */
> > +	header->magic = le16_to_cpu(header->magic);
> > +	if (header->magic != CHAMELEONV2_MAGIC) {
> > +		pr_err("Unsupported chameleon version 0x%x\n",
> > +				header->magic);
> > +		kfree(header);
> > +		return -ENODEV;
> > +	}
> > +	p += hsize;
> > +
> > +	pr_debug("header->revision = %d\n", header->revision);
> > +	pr_debug("header->model = 0x%x ('%c')\n", header->model,
> > +		header->model);
> > +	pr_debug("header->minor = %d\n", header->minor);
> > +	pr_debug("header->bus_type = 0x%x\n", header->bus_type);
> > +
> > +
> > +	pr_debug("header->magic = 0x%x\n", header->magic);
> > +	pr_debug("header->filename = \"%.*s\"\n", CHAMELEON_FILENAME_LEN,
> > +		header->filename);
> > +
> > +	for_each_chameleon_cell(dtype, p) {
> > +		switch (dtype) {
> > +		case CHAMELEON_DTYPE_GENERAL:
> > +			ret = parse_chameleon_gdd(bus, mapbase, p);
>
> Same comment about using chameleon_ prefix.
>
> > +			if (ret < 0)
> > +				goto out;
> > +			p += sizeof(struct chameleon_gdd);
> > +			break;
> > +		case CHAMELEON_DTYPE_BRIDGE:
> > +			parse_chameleon_bdd(bus, mapbase, p);
>
> And this one, too :).

Agreed for all three above.

>
> [..]
> > +++ b/include/linux/mcb.h
> > @@ -0,0 +1,113 @@
> > +/*
> > + * MEN Chameleon Bus.
> > + *
> > + * Copyright (C) 2014 MEN Mikroelektronik GmbH (www.men.de)
> > + * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the Free
> > + * Software Foundation; version 2 of the License.
> > + */
> > +#ifndef _LINUX_MCB_H
> > +#define _LINUX_MCB_H
> > +
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/device.h>
> > +#include <linux/irqreturn.h>
> > +
> > +struct mcb_driver;
> > +
> > +/**
> > + * struct mcb_bus - MEN Chameleon Bus
> > + *
> > + * @parent: Pointer to carrier device
> > + * @cores: Number of cores available on this bus
>
> Looks like these comments could be updated...

args, yes.

>
> > + * @bus_nr: mcb bus number
> > + */
> > +struct mcb_bus {
> > +	struct list_head node;
> > +	struct list_head devices;
> > +	struct list_head children;
> > +	struct device dev;
> > +	int bus_nr;
> > +};
> > +#define to_mcb_bus(b) container_of((b), struct mcb_bus, dev)
> > +
> > +/**
> > + * struct mcb_device - MEN Chameleon Bus device
> > + *
> > + * @bus_list: internal list handling for bus code
> > + * @dev: device in kernel representation
> > + * @bus: mcb bus the device is plugged to
> > + * @subordinate: subordinate MCBus in case of bridge
> > + * @is_added: flag to check if device is added to bus
> > + * @driver: associated mcb_driver
> > + * @id: mcb device id
> > + * @inst: instance in Chameleon table
> > + * @group: group in Chameleon table
> > + * @var: variant in Chameleon table
> > + * @bar: BAR in Chameleon table
> > + * @rev: revision in Chameleon table
> > + * @irq: IRQ resource
> > + * @memory: memory resource
> > + */
> > +struct mcb_device {
> > +	struct list_head bus_list;
> > +	struct device dev;
> > +	struct mcb_bus *bus;
> > +	struct mcb_bus *subordinate;
> > +	bool is_added;
> > +	struct mcb_driver *driver;
> > +	u16 id;
> > +	int inst;
> > +	int group;
> > +	int var;
> > +	int bar;
> > +	int rev;
> > +	struct resource irq;
> > +	struct resource mem;
> > +};
> > +#define to_mcb_device(x) container_of((x), struct mcb_device, dev)
> > +
> > +struct mcb_driver {
>
> This could probably use a kerneldoc.

Yes.

>
> > +	struct list_head node;
>
> How is this node used?

Again, probably copied from somewhere (probably PCI) and forgotten to validate
if I actually use it.

>
> > +	struct device_driver driver;
> > +	const struct mcb_device_id *id_table;
> > +	int (*probe)(struct mcb_device *mdev, const struct mcb_device_id *id);
> > +	void (*remove)(struct mcb_device *mdev);
> > +	void (*shutdown)(struct mcb_device *mdev);
> > +};
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation


This will be changed in a v2 of this patch. But I'm not sure if I can get to it today,
and then I have 2 days off, so maybe v2 will hit the list on monday, sorry.

Thanks,
Johannes

[PATCH v2 1/3] drivers: Introduce MEN Chameleon Bus

[Johannes Thumshirn]

The MCB (MEN Chameleon Bus) is a Bus specific to MEN Mikroelektronik
FPGA based devices. It is used to identify MCB based IP-Cores within
an FPGA and provide the necessary framework for instantiating drivers
for these devices.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 MAINTAINERS                     |   6 +
 drivers/Kconfig                 |   2 +
 drivers/Makefile                |   1 +
 drivers/mcb/Kconfig             |  15 ++
 drivers/mcb/Makefile            |   5 +
 drivers/mcb/mcb-core.c          | 414 ++++++++++++++++++++++++++++++++++++++++
 drivers/mcb/mcb-internal.h      | 116 +++++++++++
 drivers/mcb/mcb-parse.c         | 159 +++++++++++++++
 include/linux/mcb.h             | 119 ++++++++++++
 include/linux/mod_devicetable.h |   5 +
 10 files changed, 842 insertions(+)
 create mode 100644 drivers/mcb/Kconfig
 create mode 100644 drivers/mcb/Makefile
 create mode 100644 drivers/mcb/mcb-core.c
 create mode 100644 drivers/mcb/mcb-internal.h
 create mode 100644 drivers/mcb/mcb-parse.c
 create mode 100644 include/linux/mcb.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 5aa1c50..68a3cad 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5666,6 +5666,12 @@ L:	linux-watchdog@vger.kernel.org
 S:	Supported
 F:	drivers/watchdog/mena21_wdt.c

+MEN CHAMELEON BUS (mcb)
+M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
+S:	Supported
+F:	drivers/mcb/
+F:	include/linux/mcb.h
+
 METAG ARCHITECTURE
 M:	James Hogan <james.hogan@imgtec.com>
 L:	linux-metag@vger.kernel.org
diff --git a/drivers/Kconfig b/drivers/Kconfig
index b3138fb..df2ac52 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -170,4 +170,6 @@ source "drivers/phy/Kconfig"

 source "drivers/powercap/Kconfig"

+source "drivers/mcb/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 8e3b8b0..c5bf50c 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -155,3 +155,4 @@ obj-$(CONFIG_IPACK_BUS)		+= ipack/
 obj-$(CONFIG_NTB)		+= ntb/
 obj-$(CONFIG_FMC)		+= fmc/
 obj-$(CONFIG_POWERCAP)		+= powercap/
+obj-$(CONFIG_MCB)		+= mcb/
diff --git a/drivers/mcb/Kconfig b/drivers/mcb/Kconfig
new file mode 100644
index 0000000..44c82d6
--- /dev/null
+++ b/drivers/mcb/Kconfig
@@ -0,0 +1,15 @@
+#
+# MEN Chameleon Bus (MCB) support
+#
+
+menuconfig MCB
+	   tristate "MCB support"
+	   default m
+	   help
+
+	   The MCB (MEN Chameleon Bus) is a Bus specific to MEN Mikroelektronik
+	   FPGA based devices. It is used to identify MCB based IP-Cores within
+	   an FPGA and provide the necessary framework for instantiating drivers
+	   for these devices.
+
+	   If build as a module, the module is called mcb.ko
diff --git a/drivers/mcb/Makefile b/drivers/mcb/Makefile
new file mode 100644
index 0000000..2d9a751
--- /dev/null
+++ b/drivers/mcb/Makefile
@@ -0,0 +1,5 @@
+
+obj-$(CONFIG_MCB) += mcb.o
+
+mcb-y += mcb-core.o
+mcb-y += mcb-parse.o
diff --git a/drivers/mcb/mcb-core.c b/drivers/mcb/mcb-core.c
new file mode 100644
index 0000000..bbe1293
--- /dev/null
+++ b/drivers/mcb/mcb-core.c
@@ -0,0 +1,414 @@
+/*
+ * MEN Chameleon Bus.
+ *
+ * Copyright (C) 2013 MEN Mikroelektronik GmbH (www.men.de)
+ * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/idr.h>
+#include <linux/mcb.h>
+
+static DEFINE_IDA(mcb_ida);
+
+static const struct mcb_device_id *mcb_match_id(const struct mcb_device_id *ids,
+						struct mcb_device *dev)
+{
+	if (ids) {
+		while (ids->device) {
+			if (ids->device == dev->id)
+				return ids;
+			ids++;
+		}
+	}
+
+	return NULL;
+}
+
+static int mcb_match(struct device *dev, struct device_driver *drv)
+{
+	struct mcb_driver *mdrv = to_mcb_driver(drv);
+	struct mcb_device *mdev = to_mcb_device(dev);
+	const struct mcb_device_id *found_id;
+
+	found_id = mcb_match_id(mdrv->id_table, mdev);
+	if (found_id)
+		return 1;
+
+	return 0;
+}
+
+static int mcb_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	struct mcb_device *mdev = to_mcb_device(dev);
+	int ret;
+
+	ret = add_uevent_var(env, "MODALIAS=mcb:16z%03d", mdev->id);
+	if (ret)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int mcb_probe(struct device *dev)
+{
+	struct mcb_driver *mdrv = to_mcb_driver(dev->driver);
+	struct mcb_device *mdev = to_mcb_device(dev);
+	const struct mcb_device_id *found_id;
+
+	found_id = mcb_match_id(mdrv->id_table, mdev);
+	if (!found_id)
+		return -ENODEV;
+
+	return mdrv->probe(mdev, found_id);
+}
+
+static int mcb_remove(struct device *dev)
+{
+	struct mcb_driver *mdrv = to_mcb_driver(dev->driver);
+	struct mcb_device *mdev = to_mcb_device(dev);
+
+	mdrv->remove(mdev);
+
+	put_device(&mdev->dev);
+
+	return 0;
+}
+
+static void mcb_shutdown(struct device *dev)
+{
+	struct mcb_device *mdev = to_mcb_device(dev);
+	struct mcb_driver *mdrv = mdev->driver;
+
+	if (mdrv && mdrv->shutdown)
+		mdrv->shutdown(mdev);
+}
+
+static struct bus_type mcb_bus_type = {
+	.name = "mcb",
+	.match = mcb_match,
+	.uevent = mcb_uevent,
+	.probe = mcb_probe,
+	.remove = mcb_remove,
+	.shutdown = mcb_shutdown,
+};
+
+/**
+ * __mcb_register_driver() - Register a @mcb_driver at the system
+ * @drv: The @mcb_driver
+ * @owner: The @mcb_driver's module
+ * @mod_name: The name of the @mcb_driver's module
+ *
+ * Register a @mcb_driver at the system. Perform some sanity checks, if
+ * the .probe and .remove methods are provided by the driver.
+ */
+int __mcb_register_driver(struct mcb_driver *drv, struct module *owner,
+			const char *mod_name)
+{
+	if (!drv->probe || !drv->remove)
+		return -EINVAL;
+
+	drv->driver.owner = owner;
+	drv->driver.bus = &mcb_bus_type;
+	drv->driver.mod_name = mod_name;
+
+	return driver_register(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(__mcb_register_driver);
+
+/**
+ * mcb_unregister_driver() - Unregister a @mcb_driver from the system
+ * @drv: The @mcb_driver
+ *
+ * Unregister a @mcb_driver from the system.
+ */
+void mcb_unregister_driver(struct mcb_driver *drv)
+{
+	driver_unregister(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(mcb_unregister_driver);
+
+static void mcb_release_dev(struct device *dev)
+{
+	struct mcb_device *mdev = to_mcb_device(dev);
+
+	mcb_bus_put(mdev->bus);
+	kfree(mdev);
+}
+
+/**
+ * mcb_device_register() - Register a mcb_device
+ * @bus: The @mcb_bus of the device
+ * @dev: The @mcb_device
+ *
+ * Register a specific @mcb_device at a @mcb_bus and the system itself.
+ */
+int mcb_device_register(struct mcb_bus *bus, struct mcb_device *dev)
+{
+	int ret;
+	int device_id;
+
+	device_initialize(&dev->dev);
+	dev->dev.bus = &mcb_bus_type;
+	dev->dev.parent = bus->dev.parent;
+	dev->dev.release = mcb_release_dev;
+
+	device_id = dev->id;
+	dev_set_name(&dev->dev, "mcb%d-16z%03d-%d:%d:%d",
+		bus->bus_nr, device_id, dev->inst, dev->group, dev->var);
+
+	ret = device_add(&dev->dev);
+	if (ret < 0) {
+		pr_err("Failed registering device 16z%03d on bus mcb%d (%d)\n",
+			device_id, bus->bus_nr, ret);
+		goto out;
+	}
+
+	return 0;
+
+out:
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mcb_device_register);
+
+/**
+ * mcb_alloc_bus() - Allocate a new @mcb_bus
+ *
+ * Allocate a new @mcb_bus.
+ */
+struct mcb_bus *mcb_alloc_bus(void)
+{
+	struct mcb_bus *bus;
+	int bus_nr;
+
+	bus = kzalloc(sizeof(struct mcb_bus), GFP_KERNEL);
+	if (!bus)
+		return NULL;
+
+	bus_nr = ida_simple_get(&mcb_ida, 0, 0, GFP_KERNEL);
+	if (bus_nr < 0) {
+		kfree(bus);
+		return ERR_PTR(bus_nr);
+	}
+
+	INIT_LIST_HEAD(&bus->children);
+	bus->bus_nr = bus_nr;
+
+	return bus;
+}
+EXPORT_SYMBOL_GPL(mcb_alloc_bus);
+
+static int __mcb_devices_unregister(struct device *dev, void *data)
+{
+	device_unregister(dev);
+	return 0;
+}
+
+static void mcb_devices_unregister(struct mcb_bus *bus)
+{
+	bus_for_each_dev(&mcb_bus_type, NULL, NULL, __mcb_devices_unregister);
+}
+/**
+ * mcb_release_bus() - Free a @mcb_bus
+ * @bus: The @mcb_bus to release
+ *
+ * Release an allocated @mcb_bus from the system.
+ */
+void mcb_release_bus(struct mcb_bus *bus)
+{
+	mcb_devices_unregister(bus);
+
+	ida_simple_remove(&mcb_ida, bus->bus_nr);
+
+	kfree(bus);
+}
+EXPORT_SYMBOL_GPL(mcb_release_bus);
+
+/**
+ * mcb_bus_put() - Increment refcnt
+ * @bus: The @mcb_bus
+ *
+ * Get a @mcb_bus' ref
+ */
+struct mcb_bus *mcb_bus_get(struct mcb_bus *bus)
+{
+	if (bus)
+		get_device(&bus->dev);
+
+	return bus;
+}
+EXPORT_SYMBOL_GPL(mcb_bus_get);
+
+/**
+ * mcb_bus_put() - Decrement refcnt
+ * @bus: The @mcb_bus
+ *
+ * Release a @mcb_bus' ref
+ */
+void mcb_bus_put(struct mcb_bus *bus)
+{
+	if (bus)
+		put_device(&bus->dev);
+}
+EXPORT_SYMBOL_GPL(mcb_bus_put);
+
+/**
+ * mcb_alloc_dev() - Allocate a device
+ * @bus: The @mcb_bus the device is part of
+ *
+ * Allocate a @mcb_device and add bus.
+ */
+struct mcb_device *mcb_alloc_dev(struct mcb_bus *bus)
+{
+	struct mcb_device *dev;
+
+	dev = kzalloc(sizeof(struct mcb_device), GFP_KERNEL);
+	if (!dev)
+		return NULL;
+
+	INIT_LIST_HEAD(&dev->bus_list);
+	dev->bus = bus;
+
+	return dev;
+}
+EXPORT_SYMBOL_GPL(mcb_alloc_dev);
+
+/**
+ * mcb_free_dev() - Free @mcb_device
+ * @dev: The device to free
+ *
+ * Free a @mcb_device
+ */
+void mcb_free_dev(struct mcb_device *dev)
+{
+	kfree(dev);
+}
+EXPORT_SYMBOL_GPL(mcb_free_dev);
+
+static int __mcb_bus_add_devices(struct device *dev, void *data)
+{
+	struct mcb_device *mdev = to_mcb_device(dev);
+	int retval;
+
+	if (mdev->is_added)
+		return 0;
+
+	retval = device_attach(dev);
+	if (retval < 0)
+		dev_err(dev, "Error adding device (%d)\n", retval);
+
+	mdev->is_added = true;
+
+	return 0;
+}
+
+static int __mcb_bus_add_child(struct device *dev, void *data)
+{
+	struct mcb_device *mdev = to_mcb_device(dev);
+	struct mcb_bus *child;
+
+	BUG_ON(!mdev->is_added);
+	child = mdev->subordinate;
+
+	if (child)
+		mcb_bus_add_devices(child);
+
+	return 0;
+}
+
+/**
+ * mcb_bus_add_devices() - Add devices in the bus' internal device list
+ * @bus: The @mcb_bus we add the devices
+ *
+ * Add devices in the bus' internal device list to the system.
+ */
+void mcb_bus_add_devices(const struct mcb_bus *bus)
+{
+	bus_for_each_dev(&mcb_bus_type, NULL, NULL, __mcb_bus_add_devices);
+	bus_for_each_dev(&mcb_bus_type, NULL, NULL, __mcb_bus_add_child);
+
+}
+EXPORT_SYMBOL_GPL(mcb_bus_add_devices);
+
+/**
+ * mcb_request_mem() - Request memory
+ * @dev: The @mcb_device the memory is for
+ * @name: The name for the memory reference.
+ *
+ * Request memory for a @mcb_device. If @name is NULL the driver name will
+ * be used.
+ */
+struct resource *mcb_request_mem(struct mcb_device *dev, const char *name)
+{
+	struct resource *mem;
+	u32 size;
+
+	if (!name)
+		name = dev->dev.driver->name;
+
+	size = resource_size(&dev->mem);
+
+	mem = request_mem_region(dev->mem.start, size, name);
+	if (!mem)
+		return ERR_PTR(-EBUSY);
+
+	return mem;
+}
+EXPORT_SYMBOL_GPL(mcb_request_mem);
+
+/**
+ * mcb_release_mem() - Release memory requested by device
+ * @dev: The @mcb_device that requested the memory
+ *
+ * Release memory that was prior requested via @mcb_request_mem().
+ */
+void mcb_release_mem(struct resource *mem)
+{
+	u32 size;
+
+	size = resource_size(mem);
+	release_mem_region(mem->start, size);
+}
+EXPORT_SYMBOL_GPL(mcb_release_mem);
+
+/**
+ * mcb_get_irq() - Get device's IRQ number
+ * @dev: The @mcb_device the IRQ is for
+ *
+ * Get the IRQ number of a given @mcb_device.
+ */
+int mcb_get_irq(struct mcb_device *dev)
+{
+	struct resource *irq = &dev->irq;
+
+	return irq->start;
+}
+EXPORT_SYMBOL_GPL(mcb_get_irq);
+
+static int mcb_init(void)
+{
+	return bus_register(&mcb_bus_type);
+}
+
+static void mcb_exit(void)
+{
+	bus_unregister(&mcb_bus_type);
+}
+
+/* mcb must be initialized after PCI but before the chameleon drivers.
+ * That means we must use some initcall between subsys_initcall and
+ * device_initcall.
+ */
+fs_initcall(mcb_init);
+module_exit(mcb_exit);
+
+MODULE_DESCRIPTION("MEN Chameleon Bus Driver");
+MODULE_AUTHOR("Johannes Thumshirn <johannes.thumshirn@men.de>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mcb/mcb-internal.h b/drivers/mcb/mcb-internal.h
new file mode 100644
index 0000000..db22777
--- /dev/null
+++ b/drivers/mcb/mcb-internal.h
@@ -0,0 +1,116 @@
+#ifndef __MCB_INTERNAL
+#define __MCB_INTERNAL
+
+#include <linux/types.h>
+
+#define CHAMELEON_FILENAME_LEN		12
+#define CHAMELEONV2_MAGIC		0xabce
+
+enum chameleon_descriptor_type {
+	CHAMELEON_DTYPE_GENERAL = 0x0,
+	CHAMELEON_DTYPE_BRIDGE = 0x1,
+	CHAMELEON_DTYPE_CPU = 0x2,
+	CHAMELEON_DTYPE_BAR = 0x3,
+	CHAMELEON_DTYPE_END = 0xf,
+};
+
+enum chameleon_bus_type {
+	CHAMELEON_BUS_WISHBONE,
+	CHAMELEON_BUS_AVALON,
+	CHAMELEON_BUS_LPC,
+	CHAMELEON_BUS_ISA,
+};
+
+/**
+ * struct chameleon_fpga_header
+ *
+ * @revision:	Revison of Chameleon table in FPGA
+ * @model:	Chameleon table model ASCII char
+ * @minor:	Revision minor
+ * @bus_type:	Bus type (usually %CHAMELEON_BUS_WISHBONE)
+ * @magic:	Chameleon header magic number (0xabce for version 2)
+ * @reserved:	Reserved
+ * @filename:	Filename of FPGA bitstream
+ */
+struct chameleon_fpga_header {
+	u8 revision;
+	char model;
+	u8 minor;
+	u8 bus_type;
+	u16 magic;
+	u16 reserved;
+	/* This one has no '\0' at the end!!! */
+	char filename[CHAMELEON_FILENAME_LEN];
+} __packed;
+#define HEADER_MAGIC_OFFSET 0x4
+
+/**
+ * struct chameleon_gdd - Chameleon General Device Descriptor
+ *
+ * @irq:	the position in the FPGA's IRQ controller vector
+ * @rev:	the revision of the variant's implementation
+ * @var:	the variant of the IP core
+ * @dev:	the device  the IP core is
+ * @dtype:	device descriptor type
+ * @bar:	BAR offset that must be added to module offset
+ * @inst:	the instance number of the device, 0 is first instance
+ * @group:	the group the device belongs to (0 = no group)
+ * @reserved:	reserved
+ * @offset:	beginning of the address window of desired module
+ * @size:	size of the module's address window
+ */
+struct chameleon_gdd {
+	__le32 reg1;
+	__le32 reg2;
+	__le32 offset;
+	__le32 size;
+
+} __packed;
+
+/* GDD Register 1 fields */
+#define GDD_IRQ(x) ((x) & 0x1f)
+#define GDD_REV(x) (((x) >> 5) & 0x3f)
+#define GDD_VAR(x) (((x) >> 11) & 0x3f)
+#define GDD_DEV(x) (((x) >> 18) & 0x3ff)
+#define GDD_DTY(x) (((x) >> 28) & 0xf)
+
+/* GDD Register 2 fields */
+#define GDD_BAR(x) ((x) & 0x7)
+#define GDD_INS(x) (((x) >> 3) & 0x3f)
+#define GDD_GRP(x) (((x) >> 9) & 0x3f)
+
+/**
+ * struct chameleon_bdd - Chameleon Bridge Device Descriptor
+ *
+ * @irq:	the position in the FPGA's IRQ controller vector
+ * @rev:	the revision of the variant's implementation
+ * @var:	the variant of the IP core
+ * @dev:	the device  the IP core is
+ * @dtype:	device descriptor type
+ * @bar:	BAR offset that must be added to module offset
+ * @inst:	the instance number of the device, 0 is first instance
+ * @dbar:	destination bar from the bus _behind_ the bridge
+ * @chamoff:	offset within the BAR of the source bus
+ * @offset:
+ * @size:
+ */
+struct chameleon_bdd {
+	unsigned int irq:6;
+	unsigned int rev:6;
+	unsigned int var:6;
+	unsigned int dev:10;
+	unsigned int dtype:4;
+	unsigned int bar:3;
+	unsigned int inst:6;
+	unsigned int dbar:3;
+	unsigned int group:6;
+	unsigned int reserved:14;
+	u32 chamoff;
+	u32 offset;
+	u32 size;
+} __packed;
+
+int chameleon_parse_cells(struct mcb_bus *bus, phys_addr_t mapbase,
+			  void __iomem *base);
+
+#endif
diff --git a/drivers/mcb/mcb-parse.c b/drivers/mcb/mcb-parse.c
new file mode 100644
index 0000000..d1278b5
--- /dev/null
+++ b/drivers/mcb/mcb-parse.c
@@ -0,0 +1,159 @@
+#include <linux/types.h>
+#include <linux/ioport.h>
+#include <linux/slab.h>
+#include <linux/export.h>
+#include <linux/io.h>
+#include <linux/mcb.h>
+
+#include "mcb-internal.h"
+
+struct mcb_parse_priv {
+	phys_addr_t mapbase;
+	void __iomem *base;
+};
+
+#define for_each_chameleon_cell(dtype, p)	\
+	for ((dtype) = get_next_dtype((p));	\
+	     (dtype) != CHAMELEON_DTYPE_END;	\
+	     (dtype) = get_next_dtype((p)))
+
+static inline uint32_t get_next_dtype(void __iomem *p)
+{
+	uint32_t dtype;
+
+	dtype = readl(p);
+	return dtype >> 28;
+}
+
+static int chameleon_parse_bdd(struct mcb_bus *bus,
+			phys_addr_t mapbase,
+			void __iomem *base)
+{
+	return 0;
+}
+
+static int chameleon_parse_gdd(struct mcb_bus *bus,
+			phys_addr_t mapbase,
+			void __iomem *base)
+{
+	struct chameleon_gdd __iomem *gdd =
+		(struct chameleon_gdd __iomem *) base;
+	struct mcb_device *mdev;
+	u32 offset;
+	u32 size;
+	int ret;
+	__le32 reg1;
+	__le32 reg2;
+
+	mdev = mcb_alloc_dev(bus);
+	if (!mdev)
+		return -ENOMEM;
+
+	reg1 = readl(&gdd->reg1);
+	reg2 = readl(&gdd->reg2);
+	offset = readl(&gdd->offset);
+	size = readl(&gdd->size);
+
+	mdev->id = GDD_DEV(reg1);
+	mdev->rev = GDD_REV(reg1);
+	mdev->var = GDD_VAR(reg1);
+	mdev->bar = GDD_BAR(reg1);
+	mdev->group = GDD_GRP(reg2);
+	mdev->inst = GDD_INS(reg2);
+
+	pr_debug("Found a 16z%03d\n", mdev->id);
+
+	mdev->irq.start = GDD_IRQ(reg1);
+	mdev->irq.end = GDD_IRQ(reg1);
+	mdev->irq.flags = IORESOURCE_IRQ;
+
+	mdev->mem.start = mapbase + offset;
+	mdev->mem.end = mdev->mem.start + size - 1;
+	mdev->mem.flags = IORESOURCE_MEM;
+
+	mdev->is_added = false;
+
+	ret = mcb_device_register(bus, mdev);
+	if (ret < 0)
+		goto err;
+
+	return 0;
+
+err:
+	mcb_free_dev(mdev);
+
+	return ret;
+}
+
+int chameleon_parse_cells(struct mcb_bus *bus, phys_addr_t mapbase,
+			void __iomem *base)
+{
+	char __iomem *p = base;
+	struct chameleon_fpga_header *header;
+	uint32_t dtype;
+	int num_cells = 0;
+	int ret = 0;
+	u32 hsize;
+
+	hsize = sizeof(struct chameleon_fpga_header);
+
+	header = kzalloc(hsize, GFP_KERNEL);
+	if (!header)
+		return -ENOMEM;
+
+	/* Extract header information */
+	memcpy_fromio(header, p, hsize);
+	/* We only support chameleon v2 at the moment */
+	header->magic = le16_to_cpu(header->magic);
+	if (header->magic != CHAMELEONV2_MAGIC) {
+		pr_err("Unsupported chameleon version 0x%x\n",
+				header->magic);
+		kfree(header);
+		return -ENODEV;
+	}
+	p += hsize;
+
+	pr_debug("header->revision = %d\n", header->revision);
+	pr_debug("header->model = 0x%x ('%c')\n", header->model,
+		header->model);
+	pr_debug("header->minor = %d\n", header->minor);
+	pr_debug("header->bus_type = 0x%x\n", header->bus_type);
+
+
+	pr_debug("header->magic = 0x%x\n", header->magic);
+	pr_debug("header->filename = \"%.*s\"\n", CHAMELEON_FILENAME_LEN,
+		header->filename);
+
+	for_each_chameleon_cell(dtype, p) {
+		switch (dtype) {
+		case CHAMELEON_DTYPE_GENERAL:
+			ret = chameleon_parse_gdd(bus, mapbase, p);
+			if (ret < 0)
+				goto out;
+			p += sizeof(struct chameleon_gdd);
+			break;
+		case CHAMELEON_DTYPE_BRIDGE:
+			chameleon_parse_bdd(bus, mapbase, p);
+			p += sizeof(struct chameleon_bdd);
+			break;
+		case CHAMELEON_DTYPE_END:
+			break;
+		default:
+			pr_err("Invalid chameleon descriptor type 0x%x\n",
+				dtype);
+			return -EINVAL;
+		}
+		num_cells++;
+	}
+
+	if (num_cells == 0)
+		num_cells = -EINVAL;
+
+	kfree(header);
+	return num_cells;
+
+out:
+	kfree(header);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(chameleon_parse_cells);
diff --git a/include/linux/mcb.h b/include/linux/mcb.h
new file mode 100644
index 0000000..2db284d
--- /dev/null
+++ b/include/linux/mcb.h
@@ -0,0 +1,119 @@
+/*
+ * MEN Chameleon Bus.
+ *
+ * Copyright (C) 2014 MEN Mikroelektronik GmbH (www.men.de)
+ * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ */
+#ifndef _LINUX_MCB_H
+#define _LINUX_MCB_H
+
+#include <linux/mod_devicetable.h>
+#include <linux/device.h>
+#include <linux/irqreturn.h>
+
+struct mcb_driver;
+
+/**
+ * struct mcb_bus - MEN Chameleon Bus
+ *
+ * @dev: pointer to carrier device
+ * @children: the child busses
+ * @bus_nr: mcb bus number
+ */
+struct mcb_bus {
+	struct list_head children;
+	struct device dev;
+	int bus_nr;
+};
+#define to_mcb_bus(b) container_of((b), struct mcb_bus, dev)
+
+/**
+ * struct mcb_device - MEN Chameleon Bus device
+ *
+ * @bus_list: internal list handling for bus code
+ * @dev: device in kernel representation
+ * @bus: mcb bus the device is plugged to
+ * @subordinate: subordinate MCBus in case of bridge
+ * @is_added: flag to check if device is added to bus
+ * @driver: associated mcb_driver
+ * @id: mcb device id
+ * @inst: instance in Chameleon table
+ * @group: group in Chameleon table
+ * @var: variant in Chameleon table
+ * @bar: BAR in Chameleon table
+ * @rev: revision in Chameleon table
+ * @irq: IRQ resource
+ * @memory: memory resource
+ */
+struct mcb_device {
+	struct list_head bus_list;
+	struct device dev;
+	struct mcb_bus *bus;
+	struct mcb_bus *subordinate;
+	bool is_added;
+	struct mcb_driver *driver;
+	u16 id;
+	int inst;
+	int group;
+	int var;
+	int bar;
+	int rev;
+	struct resource irq;
+	struct resource mem;
+};
+#define to_mcb_device(x) container_of((x), struct mcb_device, dev)
+
+/**
+ * struct mcb_driver - MEN Chameleon Bus device driver
+ *
+ * @driver: device_driver
+ * @id_table: mcb id table
+ * @probe: probe callback
+ * @remove: remove callback
+ * @shutdown: shutdown callback
+ */
+struct mcb_driver {
+	struct device_driver driver;
+	const struct mcb_device_id *id_table;
+	int (*probe)(struct mcb_device *mdev, const struct mcb_device_id *id);
+	void (*remove)(struct mcb_device *mdev);
+	void (*shutdown)(struct mcb_device *mdev);
+};
+#define to_mcb_driver(x) container_of((x), struct mcb_driver, driver)
+
+static inline void *mcb_get_drvdata(struct mcb_device *dev)
+{
+	return dev_get_drvdata(&dev->dev);
+}
+
+static inline void mcb_set_drvdata(struct mcb_device *dev, void *data)
+{
+	dev_set_drvdata(&dev->dev, data);
+}
+
+extern int __must_check __mcb_register_driver(struct mcb_driver *drv,
+					struct module *owner,
+					const char *mod_name);
+#define mcb_register_driver(driver)		\
+	__mcb_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
+extern void mcb_unregister_driver(struct mcb_driver *driver);
+#define module_mcb_driver(__mcb_driver)		\
+	module_driver(__mcb_driver, mcb_register_driver, mcb_unregister_driver);
+extern void mcb_bus_add_devices(const struct mcb_bus *bus);
+extern int mcb_device_register(struct mcb_bus *bus, struct mcb_device *dev);
+extern struct mcb_bus *mcb_alloc_bus(void);
+extern struct mcb_bus *mcb_bus_get(struct mcb_bus *bus);
+extern void mcb_bus_put(struct mcb_bus *bus);
+extern struct mcb_device *mcb_alloc_dev(struct mcb_bus *bus);
+extern void mcb_free_dev(struct mcb_device *dev);
+extern void mcb_release_bus(struct mcb_bus *bus);
+extern struct resource *mcb_request_mem(struct mcb_device *dev,
+					const char *name);
+extern void mcb_release_mem(struct resource *mem);
+extern int mcb_get_irq(struct mcb_device *dev);
+
+#endif /* _LINUX_MCB_H */
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 45e9214..262cb85 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -599,4 +599,9 @@ struct rio_device_id {
 	__u16 asm_did, asm_vid;
 };

+struct mcb_device_id {
+	__u16 device;
+	kernel_ulong_t driver_data;
+};
+
 #endif /* LINUX_MOD_DEVICETABLE_H */
--
1.8.5.3

[PATCH 2/3] mcb: Add PCI carrier for MEN Chameleon Bus

[Johannes Thumshirn]

Add support for MCB over PCI devices. Both PCI attached on-board Chameleon FPGAs
as well as CompactPCI based MCB carrier cards are supported with this driver.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 drivers/mcb/Kconfig   |  13 ++++++
 drivers/mcb/Makefile  |   2 +
 drivers/mcb/mcb-pci.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+)
 create mode 100644 drivers/mcb/mcb-pci.c

diff --git a/drivers/mcb/Kconfig b/drivers/mcb/Kconfig
index 9e7d6f5..8b058bc 100644
--- a/drivers/mcb/Kconfig
+++ b/drivers/mcb/Kconfig
@@ -14,3 +14,16 @@ menuconfig MCB
 
 	   If build as a module, the module is called mcb.ko
 
+if MCB
+config MCB_PCI
+	   tristate "PCI based MCB carrier"
+	   default m if MCB
+	   help
+
+	   This is a MCB carrier on a PCI device. Both PCI attached on-board
+	   FPGAs as well as CompactPCI attached MCB FPGAs are supported with
+	   this driver.
+
+	   If build as a module, the module is called mcb-pci.ko
+
+endif # MCB
diff --git a/drivers/mcb/Makefile b/drivers/mcb/Makefile
index 2d9a751..1ae1413 100644
--- a/drivers/mcb/Makefile
+++ b/drivers/mcb/Makefile
@@ -3,3 +3,5 @@ obj-$(CONFIG_MCB) += mcb.o
 
 mcb-y += mcb-core.o
 mcb-y += mcb-parse.o
+
+obj-$(CONFIG_MCB_PCI) += mcb-pci.o
diff --git a/drivers/mcb/mcb-pci.c b/drivers/mcb/mcb-pci.c
new file mode 100644
index 0000000..0b1eafa
--- /dev/null
+++ b/drivers/mcb/mcb-pci.c
@@ -0,0 +1,108 @@
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/mcb.h>
+
+#include "mcb-internal.h"
+
+static struct mcb_bus *bus;
+struct priv {
+	void __iomem *base;
+};
+
+static int mcb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct priv *priv;
+	phys_addr_t mapbase;
+	int ret = 0;
+	int num_cells;
+	unsigned long flags;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(struct priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = pci_enable_device(pdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to enable PCI device\n");
+		return -ENODEV;
+	}
+
+	mapbase = pci_resource_start(pdev, 0);
+	if (!mapbase) {
+		dev_err(&pdev->dev, "No PCI resource\n");
+		goto err_start;
+	}
+
+	ret = pci_request_region(pdev, 0, KBUILD_MODNAME);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to request PCI BARs\n");
+		goto err_start;
+	}
+
+	priv->base = pci_iomap(pdev, 0, 0);
+	if (!priv->base) {
+		dev_err(&pdev->dev, "Cannot ioremap\n");
+		ret = -ENOMEM;
+		goto err_ioremap;
+	}
+
+	flags = pci_resource_flags(pdev, 0);
+	if (flags & IORESOURCE_IO) {
+		ret = -ENOTSUPP;
+		dev_err(&pdev->dev,
+			"IO mapped PCI devices are not supported\n");
+		goto err_ioremap;
+	}
+
+	pci_set_drvdata(pdev, priv);
+
+	bus = mcb_alloc_bus();
+
+	ret = parse_chameleon_cells(bus, mapbase, priv->base);
+	if (ret < 0)
+		goto err_drvdata;
+	num_cells = ret;
+
+	dev_dbg(&pdev->dev, "Found %d cells\n", num_cells);
+
+	mcb_bus_add_devices(bus);
+
+err_drvdata:
+	pci_iounmap(pdev, priv->base);
+err_ioremap:
+	pci_release_region(pdev, 0);
+err_start:
+	pci_disable_device(pdev);
+	return ret;
+}
+
+static void mcb_pci_remove(struct pci_dev *pdev)
+{
+	struct priv *priv = pci_get_drvdata(pdev);
+
+	mcb_release_bus(bus);
+
+	pci_iounmap(pdev, priv->base);
+	pci_release_region(pdev, 0);
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
+}
+
+static struct pci_device_id mcb_pci_tbl[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_MEN, PCI_DEVICE_ID_MEN_CHAMELEON) },
+	{ 0 },
+};
+MODULE_DEVICE_TABLE(pci, mcb_pci_tbl);
+
+static struct pci_driver mcb_pci_driver = {
+	.name = "mcb-pci",
+	.id_table = mcb_pci_tbl,
+	.probe = mcb_pci_probe,
+	.remove = mcb_pci_remove,
+};
+
+module_pci_driver(mcb_pci_driver);
+
+MODULE_AUTHOR("Johannes Thumshirn <johannes.thumshirn@men.de>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("MCB over PCI support");
-- 
1.8.5.4

Re: [PATCH 2/3] mcb: Add PCI carrier for MEN Chameleon Bus

[Josh Cartwright]

On Tue, Feb 18, 2014 at 04:34:13PM +0100, Johannes Thumshirn wrote:
> Add support for MCB over PCI devices. Both PCI attached on-board Chameleon FPGAs
> as well as CompactPCI based MCB carrier cards are supported with this driver.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> ---
>  drivers/mcb/Kconfig   |  13 ++++++
>  drivers/mcb/Makefile  |   2 +
>  drivers/mcb/mcb-pci.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 123 insertions(+)
>  create mode 100644 drivers/mcb/mcb-pci.c
> 
> diff --git a/drivers/mcb/Kconfig b/drivers/mcb/Kconfig
> index 9e7d6f5..8b058bc 100644
> --- a/drivers/mcb/Kconfig
> +++ b/drivers/mcb/Kconfig
> @@ -14,3 +14,16 @@ menuconfig MCB
>  
>  	   If build as a module, the module is called mcb.ko
>  
> +if MCB
> +config MCB_PCI
> +	   tristate "PCI based MCB carrier"
> +	   default m if MCB

'if MCB' is redundant (MCB has to be set for this option to even be
available).

[..]
> +++ b/drivers/mcb/mcb-pci.c
> @@ -0,0 +1,108 @@

Copyright/license blurb?

> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/mcb.h>
> +
> +#include "mcb-internal.h"
> +
> +static struct mcb_bus *bus;
> +struct priv {
> +	void __iomem *base;
> +};

This seems weird.  Why is the bus not part of your private data?  Seems
like you're unnecessarily restricting yourself to supporting only one of
these devices at a time...

> +
> +static int mcb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct priv *priv;
> +	phys_addr_t mapbase;
> +	int ret = 0;

No need to initialize.

> +	int num_cells;
> +	unsigned long flags;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(struct priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to enable PCI device\n");
> +		return -ENODEV;
> +	}
> +
> +	mapbase = pci_resource_start(pdev, 0);
> +	if (!mapbase) {
> +		dev_err(&pdev->dev, "No PCI resource\n");
> +		goto err_start;
> +	}
> +
> +	ret = pci_request_region(pdev, 0, KBUILD_MODNAME);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to request PCI BARs\n");
> +		goto err_start;
> +	}
> +
> +	priv->base = pci_iomap(pdev, 0, 0);

Hmm.  There should really be a devm_* variant for PCI resources.

[..]
> +
> +static void mcb_pci_remove(struct pci_dev *pdev)
> +{
> +	struct priv *priv = pci_get_drvdata(pdev);
> +
> +	mcb_release_bus(bus);
> +
> +	pci_iounmap(pdev, priv->base);
> +	pci_release_region(pdev, 0);
> +	pci_disable_device(pdev);
> +	pci_set_drvdata(pdev, NULL);
> +}
> +
> +static struct pci_device_id mcb_pci_tbl[] = {

const?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 2/3] mcb: Add PCI carrier for MEN Chameleon Bus

[Johannes Thumshirn]

On Tue, Feb 18, 2014 at 05:20:17PM -0600, Josh Cartwright wrote:
> On Tue, Feb 18, 2014 at 04:34:13PM +0100, Johannes Thumshirn wrote:
> > Add support for MCB over PCI devices. Both PCI attached on-board Chameleon FPGAs
> > as well as CompactPCI based MCB carrier cards are supported with this driver.
> >
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> > ---
> >  drivers/mcb/Kconfig   |  13 ++++++
> >  drivers/mcb/Makefile  |   2 +
> >  drivers/mcb/mcb-pci.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 123 insertions(+)
> >  create mode 100644 drivers/mcb/mcb-pci.c
> >
> > diff --git a/drivers/mcb/Kconfig b/drivers/mcb/Kconfig
> > index 9e7d6f5..8b058bc 100644
> > --- a/drivers/mcb/Kconfig
> > +++ b/drivers/mcb/Kconfig
> > @@ -14,3 +14,16 @@ menuconfig MCB
> >
> >  	   If build as a module, the module is called mcb.ko
> >
> > +if MCB
> > +config MCB_PCI
> > +	   tristate "PCI based MCB carrier"
> > +	   default m if MCB
>
> 'if MCB' is redundant (MCB has to be set for this option to even be
> available).

OK

>
> [..]
> > +++ b/drivers/mcb/mcb-pci.c
> > @@ -0,0 +1,108 @@
>
> Copyright/license blurb?

Args, forgotten, sorry.

>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/mcb.h>
> > +
> > +#include "mcb-internal.h"
> > +
> > +static struct mcb_bus *bus;
> > +struct priv {
> > +	void __iomem *base;
> > +};
>
> This seems weird.  Why is the bus not part of your private data?  Seems
> like you're unnecessarily restricting yourself to supporting only one of
> these devices at a time...

You're right. Funny enough I didn't notice it, while testing as I used 2 identical
carrier cards. Card 2 must then have overwritten Card 1 :-(.

>
> > +
> > +static int mcb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > +{
> > +	struct priv *priv;
> > +	phys_addr_t mapbase;
> > +	int ret = 0;
>
> No need to initialize.

OK

>
> > +	int num_cells;
> > +	unsigned long flags;
> > +
> > +	priv = devm_kzalloc(&pdev->dev, sizeof(struct priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	ret = pci_enable_device(pdev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to enable PCI device\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	mapbase = pci_resource_start(pdev, 0);
> > +	if (!mapbase) {
> > +		dev_err(&pdev->dev, "No PCI resource\n");
> > +		goto err_start;
> > +	}
> > +
> > +	ret = pci_request_region(pdev, 0, KBUILD_MODNAME);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to request PCI BARs\n");
> > +		goto err_start;
> > +	}
> > +
> > +	priv->base = pci_iomap(pdev, 0, 0);
>
> Hmm.  There should really be a devm_* variant for PCI resources.

I don't think so, as I have to free the resources afterwards again so the mcb
devices can access these resources. Otherwise I would need to make them muxed,
but I don't see the reason to keep the resources after the parser code has
finished.

>
> [..]
> > +
> > +static void mcb_pci_remove(struct pci_dev *pdev)
> > +{
> > +	struct priv *priv = pci_get_drvdata(pdev);
> > +
> > +	mcb_release_bus(bus);
> > +
> > +	pci_iounmap(pdev, priv->base);
> > +	pci_release_region(pdev, 0);
> > +	pci_disable_device(pdev);
> > +	pci_set_drvdata(pdev, NULL);
> > +}
> > +

It's actually a "double free" of the resources, isn't it. Needs to be fixed.

> > +static struct pci_device_id mcb_pci_tbl[] = {
>
> const?
>

Yup.

> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation

As with the other patch, probably on monday. Sorry for the inconvenience.

Johannes

[PATCH 2/3] mcb: Add PCI carrier for MEN Chameleon Bus

[Johannes Thumshirn]

Add support for MCB over PCI devices. Both PCI attached on-board Chameleon FPGAs
as well as CompactPCI based MCB carrier cards are supported with this driver.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 drivers/mcb/Kconfig        |  14 ++++++
 drivers/mcb/Makefile       |   2 +
 drivers/mcb/mcb-internal.h |   2 +
 drivers/mcb/mcb-pci.c      | 114 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 132 insertions(+)
 create mode 100644 drivers/mcb/mcb-pci.c

diff --git a/drivers/mcb/Kconfig b/drivers/mcb/Kconfig
index 44c82d6..b8f5d46 100644
--- a/drivers/mcb/Kconfig
+++ b/drivers/mcb/Kconfig
@@ -13,3 +13,17 @@ menuconfig MCB
 	   for these devices.
 
 	   If build as a module, the module is called mcb.ko
+
+if MCB
+config MCB_PCI
+	   tristate "PCI based MCB carrier"
+	   default m
+	   help
+
+	   This is a MCB carrier on a PCI device. Both PCI attached on-board
+	   FPGAs as well as CompactPCI attached MCB FPGAs are supported with
+	   this driver.
+
+	   If build as a module, the module is called mcb-pci.ko
+
+endif # MCB
diff --git a/drivers/mcb/Makefile b/drivers/mcb/Makefile
index 2d9a751..1ae1413 100644
--- a/drivers/mcb/Makefile
+++ b/drivers/mcb/Makefile
@@ -3,3 +3,5 @@ obj-$(CONFIG_MCB) += mcb.o
 
 mcb-y += mcb-core.o
 mcb-y += mcb-parse.o
+
+obj-$(CONFIG_MCB_PCI) += mcb-pci.o
diff --git a/drivers/mcb/mcb-internal.h b/drivers/mcb/mcb-internal.h
index db22777..f956ef2 100644
--- a/drivers/mcb/mcb-internal.h
+++ b/drivers/mcb/mcb-internal.h
@@ -3,6 +3,8 @@
 
 #include <linux/types.h>
 
+#define PCI_VENDOR_ID_MEN		0x1a88
+#define PCI_DEVICE_ID_MEN_CHAMELEON	0x4d45
 #define CHAMELEON_FILENAME_LEN		12
 #define CHAMELEONV2_MAGIC		0xabce
 
diff --git a/drivers/mcb/mcb-pci.c b/drivers/mcb/mcb-pci.c
new file mode 100644
index 0000000..99c742c
--- /dev/null
+++ b/drivers/mcb/mcb-pci.c
@@ -0,0 +1,114 @@
+/*
+ * MEN Chameleon Bus.
+ *
+ * Copyright (C) 2014 MEN Mikroelektronik GmbH (www.men.de)
+ * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/mcb.h>
+
+#include "mcb-internal.h"
+
+struct priv {
+	struct mcb_bus *bus;
+	void __iomem *base;
+};
+
+static int mcb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct priv *priv;
+	phys_addr_t mapbase;
+	int ret;
+	int num_cells;
+	unsigned long flags;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(struct priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = pci_enable_device(pdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to enable PCI device\n");
+		return -ENODEV;
+	}
+
+	mapbase = pci_resource_start(pdev, 0);
+	if (!mapbase) {
+		dev_err(&pdev->dev, "No PCI resource\n");
+		goto err_start;
+	}
+
+	ret = pci_request_region(pdev, 0, KBUILD_MODNAME);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to request PCI BARs\n");
+		goto err_start;
+	}
+
+	priv->base = pci_iomap(pdev, 0, 0);
+	if (!priv->base) {
+		dev_err(&pdev->dev, "Cannot ioremap\n");
+		ret = -ENOMEM;
+		goto err_ioremap;
+	}
+
+	flags = pci_resource_flags(pdev, 0);
+	if (flags & IORESOURCE_IO) {
+		ret = -ENOTSUPP;
+		dev_err(&pdev->dev,
+			"IO mapped PCI devices are not supported\n");
+		goto err_ioremap;
+	}
+
+	pci_set_drvdata(pdev, priv);
+
+	priv->bus = mcb_alloc_bus();
+
+	ret = chameleon_parse_cells(priv->bus, mapbase, priv->base);
+	if (ret < 0)
+		goto err_drvdata;
+	num_cells = ret;
+
+	dev_dbg(&pdev->dev, "Found %d cells\n", num_cells);
+
+	mcb_bus_add_devices(priv->bus);
+
+err_drvdata:
+	pci_iounmap(pdev, priv->base);
+err_ioremap:
+	pci_release_region(pdev, 0);
+err_start:
+	pci_disable_device(pdev);
+	return ret;
+}
+
+static void mcb_pci_remove(struct pci_dev *pdev)
+{
+	struct priv *priv = pci_get_drvdata(pdev);
+
+	mcb_release_bus(priv->bus);
+}
+
+static const struct pci_device_id mcb_pci_tbl[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_MEN, PCI_DEVICE_ID_MEN_CHAMELEON) },
+	{ 0 },
+};
+MODULE_DEVICE_TABLE(pci, mcb_pci_tbl);
+
+static struct pci_driver mcb_pci_driver = {
+	.name = "mcb-pci",
+	.id_table = mcb_pci_tbl,
+	.probe = mcb_pci_probe,
+	.remove = mcb_pci_remove,
+};
+
+module_pci_driver(mcb_pci_driver);
+
+MODULE_AUTHOR("Johannes Thumshirn <johannes.thumshirn@men.de>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("MCB over PCI support");
-- 
1.8.5.3

[PATCH 3/3] iio: adc: Add MEN 16z188 ADC driver

[Johannes Thumshirn]

Add support for MEN 16z188 ADC IP Core on MCB FPGAs.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 drivers/iio/adc/Kconfig        |  10 +++
 drivers/iio/adc/Makefile       |   1 +
 drivers/iio/adc/men_z188_adc.c | 172 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 183 insertions(+)
 create mode 100644 drivers/iio/adc/men_z188_adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 2209f28..5209437 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -204,4 +204,14 @@ config VIPERBOARD_ADC
 	  Say yes here to access the ADC part of the Nano River
 	  Technologies Viperboard.
 
+config MEN_Z188_ADC
+	tristate "MEN 16z188 ADC IP Core support"
+	depends on MCB
+	help
+	  Say yes here to access support the MEN 16z188 ADC IP-Core on a MCB
+	  carrier.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called men_z188_adc.
+
 endmenu
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ba9a10a..1584391 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
 obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
 obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
+obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
diff --git a/drivers/iio/adc/men_z188_adc.c b/drivers/iio/adc/men_z188_adc.c
new file mode 100644
index 0000000..8bc66cf
--- /dev/null
+++ b/drivers/iio/adc/men_z188_adc.c
@@ -0,0 +1,172 @@
+/*
+ * MEN 16z188 Analoge to Digial Converter
+ *
+ * Copyright (C) 2014 MEN Mikroelektronik GmbH (www.men.de)
+ * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mcb.h>
+#include <linux/iio/iio.h>
+
+#define Z188_ADC_MAX_CHAN	8
+#define Z188_ADC_GAIN		0x0700000
+#define Z188_MODE_VOLTAGE	BIT(27)
+#define Z188_CFG_AUTO		0x1
+#define Z188_CTRL_REG		0x40
+
+#define ADC_DATA(x) (((x) >> 2) & 0x7ffffc)
+#define ADC_OVR(x) ((x) & 0x1)
+
+struct z188_adc {
+	struct resource *mem;
+	void __iomem *base;
+};
+
+#define Z188_ADC_CHANNEL(idx) {					\
+		.type = IIO_VOLTAGE,				\
+		.indexed = 1,					\
+		.channel = (idx),				\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
+}
+
+static struct iio_chan_spec z188_adc_iio_channels[] = {
+	Z188_ADC_CHANNEL(0),
+	Z188_ADC_CHANNEL(1),
+	Z188_ADC_CHANNEL(2),
+	Z188_ADC_CHANNEL(3),
+	Z188_ADC_CHANNEL(4),
+	Z188_ADC_CHANNEL(5),
+	Z188_ADC_CHANNEL(6),
+	Z188_ADC_CHANNEL(7),
+};
+
+static int z188_iio_read_raw(struct iio_dev *iio_dev,
+			struct iio_chan_spec const *chan,
+			int *val,
+			int *val2,
+			long info)
+{
+	struct z188_adc *adc = iio_priv(iio_dev);
+	int ret = 0;
+	u16 tmp = 0;
+
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		tmp = readw(adc->base + chan->channel * 4);
+
+		if (ADC_OVR(tmp)) {
+			dev_info(&iio_dev->dev,
+				"Oversampling error on ADC channel %d\n",
+				chan->channel);
+			return -EIO;
+		}
+		*val = ADC_DATA(tmp);
+		ret = IIO_VAL_INT;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static struct iio_info z188_adc_info = {
+	.read_raw = &z188_iio_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static void men_z188_config_channels(void __iomem *addr)
+{
+	int i;
+	u32 cfg;
+	u32 ctl;
+
+	ctl = readl(addr + Z188_CTRL_REG);
+	ctl |= Z188_CFG_AUTO;
+	writel(ctl, addr + Z188_CTRL_REG);
+
+	for (i = 0; i < Z188_ADC_MAX_CHAN; i++) {
+		cfg = readl(addr + i);
+		cfg &= ~Z188_ADC_GAIN;
+		cfg |= Z188_MODE_VOLTAGE;
+		writel(cfg, addr + i);
+	}
+}
+
+static int men_z188_probe(struct mcb_device *dev,
+			const struct mcb_device_id *id)
+{
+	struct z188_adc *adc;
+	struct iio_dev *indio_dev;
+	struct resource *mem;
+
+	indio_dev = devm_iio_device_alloc(&dev->dev, sizeof(struct z188_adc));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	adc = iio_priv(indio_dev);
+	indio_dev->name = "z188-adc";
+	indio_dev->dev.parent = &dev->dev;
+	indio_dev->info = &z188_adc_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = z188_adc_iio_channels;
+	indio_dev->num_channels = ARRAY_SIZE(z188_adc_iio_channels);
+
+	mem = mcb_request_mem(dev, "z188-adc");
+	if (!mem)
+		return -ENOMEM;
+
+	adc->base = ioremap(mem->start, resource_size(mem));
+	if (adc->base == NULL)
+		goto err;
+
+	men_z188_config_channels(adc->base);
+
+	adc->mem = mem;
+	mcb_set_drvdata(dev, indio_dev);
+
+	return  devm_iio_device_register(&dev->dev, indio_dev);
+
+err:
+	mcb_release_mem(mem);
+	return -ENXIO;
+}
+
+static void men_z188_remove(struct mcb_device *dev)
+{
+	struct iio_dev *indio_dev  = mcb_get_drvdata(dev);
+	struct z188_adc *adc = iio_priv(indio_dev);
+
+	iounmap(adc->base);
+	mcb_release_mem(adc->mem);
+
+	mcb_set_drvdata(dev, NULL);
+}
+
+static struct mcb_device_id men_z188_ids[] = {
+	{ .device = 0xbc },
+};
+MODULE_DEVICE_TABLE(mcb, men_z188_ids);
+
+static struct mcb_driver men_z188_driver = {
+	.driver = {
+		.name = "z188-adc",
+		.owner = THIS_MODULE,
+	},
+	.probe = men_z188_probe,
+	.remove = men_z188_remove,
+	.id_table = men_z188_ids,
+};
+module_mcb_driver(men_z188_driver);
+
+MODULE_AUTHOR("Johannes Thumshirn <johannes.thumshirn@men.de>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("IIO ADC driver for MEN 16z188 ADC Core");
+MODULE_ALIAS("mcb:16z188");
-- 
1.8.5.4

Re: [PATCH 3/3] iio: adc: Add MEN 16z188 ADC driver

[Peter Meerwald]

some quick comments below...

> Add support for MEN 16z188 ADC IP Core on MCB FPGAs.
 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> ---
>  drivers/iio/adc/Kconfig        |  10 +++
>  drivers/iio/adc/Makefile       |   1 +
>  drivers/iio/adc/men_z188_adc.c | 172 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 183 insertions(+)
>  create mode 100644 drivers/iio/adc/men_z188_adc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 2209f28..5209437 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -204,4 +204,14 @@ config VIPERBOARD_ADC
>  	  Say yes here to access the ADC part of the Nano River
>  	  Technologies Viperboard.
>  
> +config MEN_Z188_ADC
> +	tristate "MEN 16z188 ADC IP Core support"
> +	depends on MCB
> +	help
> +	  Say yes here to access support the MEN 16z188 ADC IP-Core on a MCB
> +	  carrier.

support _for_ the 

> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called men_z188_adc.
> +
>  endmenu
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ba9a10a..1584391 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -22,3 +22,4 @@ obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>  obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
>  obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
>  obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
> +obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
> diff --git a/drivers/iio/adc/men_z188_adc.c b/drivers/iio/adc/men_z188_adc.c
> new file mode 100644
> index 0000000..8bc66cf
> --- /dev/null
> +++ b/drivers/iio/adc/men_z188_adc.c
> @@ -0,0 +1,172 @@
> +/*
> + * MEN 16z188 Analoge to Digial Converter

Analog

> + *
> + * Copyright (C) 2014 MEN Mikroelektronik GmbH (www.men.de)
> + * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; version 2 of the License.
> + */

newline

> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mcb.h>
> +#include <linux/iio/iio.h>
> +
> +#define Z188_ADC_MAX_CHAN	8
> +#define Z188_ADC_GAIN		0x0700000
> +#define Z188_MODE_VOLTAGE	BIT(27)
> +#define Z188_CFG_AUTO		0x1
> +#define Z188_CTRL_REG		0x40
> +
> +#define ADC_DATA(x) (((x) >> 2) & 0x7ffffc)
> +#define ADC_OVR(x) ((x) & 0x1)
> +
> +struct z188_adc {
> +	struct resource *mem;
> +	void __iomem *base;
> +};
> +
> +#define Z188_ADC_CHANNEL(idx) {					\
> +		.type = IIO_VOLTAGE,				\
> +		.indexed = 1,					\
> +		.channel = (idx),				\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
> +}
> +
> +static struct iio_chan_spec z188_adc_iio_channels[] = {
> +	Z188_ADC_CHANNEL(0),
> +	Z188_ADC_CHANNEL(1),
> +	Z188_ADC_CHANNEL(2),
> +	Z188_ADC_CHANNEL(3),
> +	Z188_ADC_CHANNEL(4),
> +	Z188_ADC_CHANNEL(5),
> +	Z188_ADC_CHANNEL(6),
> +	Z188_ADC_CHANNEL(7),
> +};
> +
> +static int z188_iio_read_raw(struct iio_dev *iio_dev,
> +			struct iio_chan_spec const *chan,
> +			int *val,
> +			int *val2,
> +			long info)
> +{
> +	struct z188_adc *adc = iio_priv(iio_dev);
> +	int ret = 0;
> +	u16 tmp = 0;
> +

no need to init ret, tmp
delete extra newline

> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		tmp = readw(adc->base + chan->channel * 4);
> +
> +		if (ADC_OVR(tmp)) {
> +			dev_info(&iio_dev->dev,
> +				"Oversampling error on ADC channel %d\n",
> +				chan->channel);
> +			return -EIO;
> +		}
> +		*val = ADC_DATA(tmp);
> +		ret = IIO_VAL_INT;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static struct iio_info z188_adc_info = {
> +	.read_raw = &z188_iio_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static void men_z188_config_channels(void __iomem *addr)
> +{
> +	int i;
> +	u32 cfg;
> +	u32 ctl;
> +
> +	ctl = readl(addr + Z188_CTRL_REG);
> +	ctl |= Z188_CFG_AUTO;
> +	writel(ctl, addr + Z188_CTRL_REG);
> +
> +	for (i = 0; i < Z188_ADC_MAX_CHAN; i++) {
> +		cfg = readl(addr + i);
> +		cfg &= ~Z188_ADC_GAIN;
> +		cfg |= Z188_MODE_VOLTAGE;
> +		writel(cfg, addr + i);
> +	}
> +}
> +
> +static int men_z188_probe(struct mcb_device *dev,
> +			const struct mcb_device_id *id)
> +{
> +	struct z188_adc *adc;
> +	struct iio_dev *indio_dev;
> +	struct resource *mem;
> +
> +	indio_dev = devm_iio_device_alloc(&dev->dev, sizeof(struct z188_adc));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	adc = iio_priv(indio_dev);
> +	indio_dev->name = "z188-adc";
> +	indio_dev->dev.parent = &dev->dev;
> +	indio_dev->info = &z188_adc_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = z188_adc_iio_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(z188_adc_iio_channels);
> +
> +	mem = mcb_request_mem(dev, "z188-adc");
> +	if (!mem)
> +		return -ENOMEM;
> +
> +	adc->base = ioremap(mem->start, resource_size(mem));
> +	if (adc->base == NULL)
> +		goto err;
> +
> +	men_z188_config_channels(adc->base);
> +
> +	adc->mem = mem;
> +	mcb_set_drvdata(dev, indio_dev);
> +
> +	return  devm_iio_device_register(&dev->dev, indio_dev);

extra space 

> +
> +err:
> +	mcb_release_mem(mem);
> +	return -ENXIO;
> +}
> +
> +static void men_z188_remove(struct mcb_device *dev)
> +{
> +	struct iio_dev *indio_dev  = mcb_get_drvdata(dev);
> +	struct z188_adc *adc = iio_priv(indio_dev);
> +
> +	iounmap(adc->base);
> +	mcb_release_mem(adc->mem);
> +
> +	mcb_set_drvdata(dev, NULL);
> +}
> +
> +static struct mcb_device_id men_z188_ids[] = {
> +	{ .device = 0xbc },
> +};
> +MODULE_DEVICE_TABLE(mcb, men_z188_ids);
> +
> +static struct mcb_driver men_z188_driver = {
> +	.driver = {
> +		.name = "z188-adc",
> +		.owner = THIS_MODULE,
> +	},
> +	.probe = men_z188_probe,
> +	.remove = men_z188_remove,
> +	.id_table = men_z188_ids,
> +};
> +module_mcb_driver(men_z188_driver);
> +
> +MODULE_AUTHOR("Johannes Thumshirn <johannes.thumshirn@men.de>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("IIO ADC driver for MEN 16z188 ADC Core");
> +MODULE_ALIAS("mcb:16z188");
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 3/3] iio: adc: Add MEN 16z188 ADC driver

[Lars-Peter Clausen]

On top of what Peter said. Just some nitpicks...
[...]
> +static struct iio_chan_spec z188_adc_iio_channels[] = {

const

> +	Z188_ADC_CHANNEL(0),
> +	Z188_ADC_CHANNEL(1),
> +	Z188_ADC_CHANNEL(2),
> +	Z188_ADC_CHANNEL(3),
> +	Z188_ADC_CHANNEL(4),
> +	Z188_ADC_CHANNEL(5),
> +	Z188_ADC_CHANNEL(6),
> +	Z188_ADC_CHANNEL(7),
> +};
> +
[...]
> +static struct iio_info z188_adc_info = {

const

> +	.read_raw = &z188_iio_read_raw,
> +	.driver_module = THIS_MODULE,
> +};

 > +
 > +static void men_z188_remove(struct mcb_device *dev)
 > +{
[...]
 > +	mcb_set_drvdata(dev, NULL);

This is already done by the device driver core.

 > +}
[...]
 > +static struct mcb_device_id men_z188_ids[] = {

const

 > +	{ .device = 0xbc },
 > +};
 > +MODULE_DEVICE_TABLE(mcb, men_z188_ids);

Re: [PATCH 3/3] iio: adc: Add MEN 16z188 ADC driver

[Jonathan Cameron]

On February 18, 2014 3:34:14 PM GMT+00:00, Johannes Thumshirn <johannes.thumshirn@men.de> wrote:
>Add support for MEN 16z188 ADC IP Core on MCB FPGAs.
>
>Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
Looks pretty good apart from the nitpicks. One more little thing...
>---
> drivers/iio/adc/Kconfig        |  10 +++
> drivers/iio/adc/Makefile       |   1 +
>drivers/iio/adc/men_z188_adc.c | 172
>+++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 183 insertions(+)
> create mode 100644 drivers/iio/adc/men_z188_adc.c
>
>diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>index 2209f28..5209437 100644
>--- a/drivers/iio/adc/Kconfig
>+++ b/drivers/iio/adc/Kconfig
>@@ -204,4 +204,14 @@ config VIPERBOARD_ADC
> 	  Say yes here to access the ADC part of the Nano River
> 	  Technologies Viperboard.
> 
Alphabetical order please.
>+config MEN_Z188_ADC
>+	tristate "MEN 16z188 ADC IP Core support"
>+	depends on MCB
>+	help
>+	  Say yes here to access support the MEN 16z188 ADC IP-Core on a MCB
>+	  carrier.
>+
>+	  This driver can also be built as a module. If so, the module will
>be
>+	  called men_z188_adc.
>+
> endmenu
>diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>index ba9a10a..1584391 100644
>--- a/drivers/iio/adc/Makefile
>+++ b/drivers/iio/adc/Makefile
>@@ -22,3 +22,4 @@ obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
> obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
> obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
And alphabetical order here as well.
>+obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
>diff --git a/drivers/iio/adc/men_z188_adc.c
>b/drivers/iio/adc/men_z188_adc.c
>new file mode 100644
>index 0000000..8bc66cf
>--- /dev/null
>+++ b/drivers/iio/adc/men_z188_adc.c
>@@ -0,0 +1,172 @@
>+/*
>+ * MEN 16z188 Analoge to Digial Converter
>+ *
>+ * Copyright (C) 2014 MEN Mikroelektronik GmbH (www.men.de)
>+ * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
>+ *
>+ * This program is free software; you can redistribute it and/or
>modify it
>+ * under the terms of the GNU General Public License as published by
>the Free
>+ * Software Foundation; version 2 of the License.
>+ */
>+#include <linux/kernel.h>
>+#include <linux/module.h>
>+#include <linux/mcb.h>
>+#include <linux/iio/iio.h>
>+
>+#define Z188_ADC_MAX_CHAN	8
>+#define Z188_ADC_GAIN		0x0700000
>+#define Z188_MODE_VOLTAGE	BIT(27)
>+#define Z188_CFG_AUTO		0x1
>+#define Z188_CTRL_REG		0x40
>+
>+#define ADC_DATA(x) (((x) >> 2) & 0x7ffffc)
>+#define ADC_OVR(x) ((x) & 0x1)
>+
>+struct z188_adc {
>+	struct resource *mem;
>+	void __iomem *base;
>+};
>+
>+#define Z188_ADC_CHANNEL(idx) {					\
>+		.type = IIO_VOLTAGE,				\
>+		.indexed = 1,					\
>+		.channel = (idx),				\
>+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
>+}
>+
>+static struct iio_chan_spec z188_adc_iio_channels[] = {
>+	Z188_ADC_CHANNEL(0),
>+	Z188_ADC_CHANNEL(1),
>+	Z188_ADC_CHANNEL(2),
>+	Z188_ADC_CHANNEL(3),
>+	Z188_ADC_CHANNEL(4),
>+	Z188_ADC_CHANNEL(5),
>+	Z188_ADC_CHANNEL(6),
>+	Z188_ADC_CHANNEL(7),
>+};
>+
>+static int z188_iio_read_raw(struct iio_dev *iio_dev,
>+			struct iio_chan_spec const *chan,
>+			int *val,
>+			int *val2,
>+			long info)
>+{
>+	struct z188_adc *adc = iio_priv(iio_dev);
>+	int ret = 0;
>+	u16 tmp = 0;
>+
>+
>+	switch (info) {
>+	case IIO_CHAN_INFO_RAW:
>+		tmp = readw(adc->base + chan->channel * 4);
>+
>+		if (ADC_OVR(tmp)) {
>+			dev_info(&iio_dev->dev,
>+				"Oversampling error on ADC channel %d\n",
>+				chan->channel);
>+			return -EIO;
>+		}
>+		*val = ADC_DATA(tmp);
>+		ret = IIO_VAL_INT;
>+		break;
>+	default:
>+		ret = -EINVAL;
>+		break;
>+	}
>+
>+	return ret;
>+}
>+
>+static struct iio_info z188_adc_info = {
>+	.read_raw = &z188_iio_read_raw,
>+	.driver_module = THIS_MODULE,
>+};
>+
>+static void men_z188_config_channels(void __iomem *addr)
>+{
>+	int i;
>+	u32 cfg;
>+	u32 ctl;
>+
>+	ctl = readl(addr + Z188_CTRL_REG);
>+	ctl |= Z188_CFG_AUTO;
>+	writel(ctl, addr + Z188_CTRL_REG);
>+
>+	for (i = 0; i < Z188_ADC_MAX_CHAN; i++) {
>+		cfg = readl(addr + i);
>+		cfg &= ~Z188_ADC_GAIN;
>+		cfg |= Z188_MODE_VOLTAGE;
>+		writel(cfg, addr + i);
>+	}
>+}
>+
>+static int men_z188_probe(struct mcb_device *dev,
>+			const struct mcb_device_id *id)
>+{
>+	struct z188_adc *adc;
>+	struct iio_dev *indio_dev;
>+	struct resource *mem;
>+
>+	indio_dev = devm_iio_device_alloc(&dev->dev, sizeof(struct
>z188_adc));
>+	if (!indio_dev)
>+		return -ENOMEM;
>+
>+	adc = iio_priv(indio_dev);
>+	indio_dev->name = "z188-adc";
>+	indio_dev->dev.parent = &dev->dev;
>+	indio_dev->info = &z188_adc_info;
>+	indio_dev->modes = INDIO_DIRECT_MODE;
>+	indio_dev->channels = z188_adc_iio_channels;
>+	indio_dev->num_channels = ARRAY_SIZE(z188_adc_iio_channels);
>+
>+	mem = mcb_request_mem(dev, "z188-adc");
>+	if (!mem)
>+		return -ENOMEM;
>+
>+	adc->base = ioremap(mem->start, resource_size(mem));
>+	if (adc->base == NULL)
>+		goto err;
>+
>+	men_z188_config_channels(adc->base);
>+
>+	adc->mem = mem;
>+	mcb_set_drvdata(dev, indio_dev);
>+
>+	return  devm_iio_device_register(&dev->dev, indio_dev);
>+
>+err:
>+	mcb_release_mem(mem);
>+	return -ENXIO;
>+}
>+
>+static void men_z188_remove(struct mcb_device *dev)
>+{
>+	struct iio_dev *indio_dev  = mcb_get_drvdata(dev);
>+	struct z188_adc *adc = iio_priv(indio_dev);
>+
>+	iounmap(adc->base);
>+	mcb_release_mem(adc->mem);
>+
>+	mcb_set_drvdata(dev, NULL);
>+}
>+
>+static struct mcb_device_id men_z188_ids[] = {
>+	{ .device = 0xbc },
>+};
>+MODULE_DEVICE_TABLE(mcb, men_z188_ids);
>+
>+static struct mcb_driver men_z188_driver = {
>+	.driver = {
>+		.name = "z188-adc",
>+		.owner = THIS_MODULE,
>+	},
>+	.probe = men_z188_probe,
>+	.remove = men_z188_remove,
>+	.id_table = men_z188_ids,
>+};
>+module_mcb_driver(men_z188_driver);
>+
>+MODULE_AUTHOR("Johannes Thumshirn <johannes.thumshirn@men.de>");
>+MODULE_LICENSE("GPL");
>+MODULE_DESCRIPTION("IIO ADC driver for MEN 16z188 ADC Core");
>+MODULE_ALIAS("mcb:16z188");

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Re: [PATCH 3/3] iio: adc: Add MEN 16z188 ADC driver

[Johannes Thumshirn]

On Tue, Feb 18, 2014 at 07:17:04PM +0000, Jonathan Cameron wrote:
>
>
> On February 18, 2014 3:34:14 PM GMT+00:00, Johannes Thumshirn <johannes.thumshirn@men.de> wrote:
> >Add support for MEN 16z188 ADC IP Core on MCB FPGAs.
> >
> >Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> Looks pretty good apart from the nitpicks. One more little thing...

Thanks, I'll rework this and the comments from Peter Meerwald and Lars-Peter
Clausen in v2 of the patch series as well.

I think it'll be done by monday.

Thanks,
Johannes

[PATCH v2 3/3] iio: adc: Add MEN 16z188 ADC driver

[Johannes Thumshirn]

Add support for MEN 16z188 ADC IP Core on MCB FPGAs.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 drivers/iio/adc/Kconfig        |  10 +++
 drivers/iio/adc/Makefile       |   1 +
 drivers/iio/adc/men_z188_adc.c | 170 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 181 insertions(+)
 create mode 100644 drivers/iio/adc/men_z188_adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 2209f28..5c63f091 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -155,6 +155,16 @@ config MCP3422
 	  This driver can also be built as a module. If so, the module will be
 	  called mcp3422.

+config MEN_Z188_ADC
+	tristate "MEN 16z188 ADC IP Core support"
+	depends on MCB
+	help
+	  Say yes here to enable support for the MEN 16z188 ADC IP-Core on a MCB
+	  carrier.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called men_z188_adc.
+
 config NAU7802
 	tristate "Nuvoton NAU7802 ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ba9a10a..85a4a04 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_MCP320X) += mcp320x.o
 obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
 obj-$(CONFIG_NAU7802) += nau7802.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
diff --git a/drivers/iio/adc/men_z188_adc.c b/drivers/iio/adc/men_z188_adc.c
new file mode 100644
index 0000000..da7f3d0
--- /dev/null
+++ b/drivers/iio/adc/men_z188_adc.c
@@ -0,0 +1,170 @@
+/*
+ * MEN 16z188 Analog to Digial Converter
+ *
+ * Copyright (C) 2014 MEN Mikroelektronik GmbH (www.men.de)
+ * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mcb.h>
+#include <linux/iio/iio.h>
+
+#define Z188_ADC_MAX_CHAN	8
+#define Z188_ADC_GAIN		0x0700000
+#define Z188_MODE_VOLTAGE	BIT(27)
+#define Z188_CFG_AUTO		0x1
+#define Z188_CTRL_REG		0x40
+
+#define ADC_DATA(x) (((x) >> 2) & 0x7ffffc)
+#define ADC_OVR(x) ((x) & 0x1)
+
+struct z188_adc {
+	struct resource *mem;
+	void __iomem *base;
+};
+
+#define Z188_ADC_CHANNEL(idx) {					\
+		.type = IIO_VOLTAGE,				\
+		.indexed = 1,					\
+		.channel = (idx),				\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
+}
+
+static const struct iio_chan_spec z188_adc_iio_channels[] = {
+	Z188_ADC_CHANNEL(0),
+	Z188_ADC_CHANNEL(1),
+	Z188_ADC_CHANNEL(2),
+	Z188_ADC_CHANNEL(3),
+	Z188_ADC_CHANNEL(4),
+	Z188_ADC_CHANNEL(5),
+	Z188_ADC_CHANNEL(6),
+	Z188_ADC_CHANNEL(7),
+};
+
+static int z188_iio_read_raw(struct iio_dev *iio_dev,
+			struct iio_chan_spec const *chan,
+			int *val,
+			int *val2,
+			long info)
+{
+	struct z188_adc *adc = iio_priv(iio_dev);
+	int ret;
+	u16 tmp;
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		tmp = readw(adc->base + chan->channel * 4);
+
+		if (ADC_OVR(tmp)) {
+			dev_info(&iio_dev->dev,
+				"Oversampling error on ADC channel %d\n",
+				chan->channel);
+			return -EIO;
+		}
+		*val = ADC_DATA(tmp);
+		ret = IIO_VAL_INT;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static struct iio_info z188_adc_info = {
+	.read_raw = &z188_iio_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static void men_z188_config_channels(void __iomem *addr)
+{
+	int i;
+	u32 cfg;
+	u32 ctl;
+
+	ctl = readl(addr + Z188_CTRL_REG);
+	ctl |= Z188_CFG_AUTO;
+	writel(ctl, addr + Z188_CTRL_REG);
+
+	for (i = 0; i < Z188_ADC_MAX_CHAN; i++) {
+		cfg = readl(addr + i);
+		cfg &= ~Z188_ADC_GAIN;
+		cfg |= Z188_MODE_VOLTAGE;
+		writel(cfg, addr + i);
+	}
+}
+
+static int men_z188_probe(struct mcb_device *dev,
+			const struct mcb_device_id *id)
+{
+	struct z188_adc *adc;
+	struct iio_dev *indio_dev;
+	struct resource *mem;
+
+	indio_dev = devm_iio_device_alloc(&dev->dev, sizeof(struct z188_adc));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	adc = iio_priv(indio_dev);
+	indio_dev->name = "z188-adc";
+	indio_dev->dev.parent = &dev->dev;
+	indio_dev->info = &z188_adc_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = z188_adc_iio_channels;
+	indio_dev->num_channels = ARRAY_SIZE(z188_adc_iio_channels);
+
+	mem = mcb_request_mem(dev, "z188-adc");
+	if (!mem)
+		return -ENOMEM;
+
+	adc->base = ioremap(mem->start, resource_size(mem));
+	if (adc->base == NULL)
+		goto err;
+
+	men_z188_config_channels(adc->base);
+
+	adc->mem = mem;
+	mcb_set_drvdata(dev, indio_dev);
+
+	return  devm_iio_device_register(&dev->dev, indio_dev);
+
+err:
+	mcb_release_mem(mem);
+	return -ENXIO;
+}
+
+static void men_z188_remove(struct mcb_device *dev)
+{
+	struct iio_dev *indio_dev  = mcb_get_drvdata(dev);
+	struct z188_adc *adc = iio_priv(indio_dev);
+
+	iounmap(adc->base);
+	mcb_release_mem(adc->mem);
+}
+
+static const struct mcb_device_id men_z188_ids[] = {
+	{ .device = 0xbc },
+};
+MODULE_DEVICE_TABLE(mcb, men_z188_ids);
+
+static struct mcb_driver men_z188_driver = {
+	.driver = {
+		.name = "z188-adc",
+		.owner = THIS_MODULE,
+	},
+	.probe = men_z188_probe,
+	.remove = men_z188_remove,
+	.id_table = men_z188_ids,
+};
+module_mcb_driver(men_z188_driver);
+
+MODULE_AUTHOR("Johannes Thumshirn <johannes.thumshirn@men.de>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("IIO ADC driver for MEN 16z188 ADC Core");
+MODULE_ALIAS("mcb:16z188");
--
1.8.5.3

Re: [PATCH v2 3/3] iio: adc: Add MEN 16z188 ADC driver

[Jonathan Cameron]

On 24/02/14 17:16, Johannes Thumshirn wrote:
> Add support for MEN 16z188 ADC IP Core on MCB FPGAs.
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
Looks good.  One possible issue with devm usage. It looks like
userspace interfaces will get removed after some other bits that
they will rely on. I missed this the first time around.
> ---
>   drivers/iio/adc/Kconfig        |  10 +++
>   drivers/iio/adc/Makefile       |   1 +
>   drivers/iio/adc/men_z188_adc.c | 170 +++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 181 insertions(+)
>   create mode 100644 drivers/iio/adc/men_z188_adc.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 2209f28..5c63f091 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -155,6 +155,16 @@ config MCP3422
>   	  This driver can also be built as a module. If so, the module will be
>   	  called mcp3422.
>
> +config MEN_Z188_ADC
> +	tristate "MEN 16z188 ADC IP Core support"
> +	depends on MCB
> +	help
> +	  Say yes here to enable support for the MEN 16z188 ADC IP-Core on a MCB
> +	  carrier.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called men_z188_adc.
> +
>   config NAU7802
>   	tristate "Nuvoton NAU7802 ADC driver"
>   	depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ba9a10a..85a4a04 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>   obj-$(CONFIG_MAX1363) += max1363.o
>   obj-$(CONFIG_MCP320X) += mcp320x.o
>   obj-$(CONFIG_MCP3422) += mcp3422.o
> +obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
>   obj-$(CONFIG_NAU7802) += nau7802.o
>   obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>   obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
> diff --git a/drivers/iio/adc/men_z188_adc.c b/drivers/iio/adc/men_z188_adc.c
> new file mode 100644
> index 0000000..da7f3d0
> --- /dev/null
> +++ b/drivers/iio/adc/men_z188_adc.c
> @@ -0,0 +1,170 @@
> +/*
> + * MEN 16z188 Analog to Digial Converter
> + *
> + * Copyright (C) 2014 MEN Mikroelektronik GmbH (www.men.de)
> + * Author: Johannes Thumshirn <johannes.thumshirn@men.de>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; version 2 of the License.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mcb.h>
> +#include <linux/iio/iio.h>
> +
> +#define Z188_ADC_MAX_CHAN	8
> +#define Z188_ADC_GAIN		0x0700000
> +#define Z188_MODE_VOLTAGE	BIT(27)
> +#define Z188_CFG_AUTO		0x1
> +#define Z188_CTRL_REG		0x40
> +
> +#define ADC_DATA(x) (((x) >> 2) & 0x7ffffc)
> +#define ADC_OVR(x) ((x) & 0x1)
> +
> +struct z188_adc {
> +	struct resource *mem;
> +	void __iomem *base;
> +};
> +
> +#define Z188_ADC_CHANNEL(idx) {					\
> +		.type = IIO_VOLTAGE,				\
> +		.indexed = 1,					\
> +		.channel = (idx),				\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
> +}
> +
> +static const struct iio_chan_spec z188_adc_iio_channels[] = {
> +	Z188_ADC_CHANNEL(0),
> +	Z188_ADC_CHANNEL(1),
> +	Z188_ADC_CHANNEL(2),
> +	Z188_ADC_CHANNEL(3),
> +	Z188_ADC_CHANNEL(4),
> +	Z188_ADC_CHANNEL(5),
> +	Z188_ADC_CHANNEL(6),
> +	Z188_ADC_CHANNEL(7),
> +};
> +
> +static int z188_iio_read_raw(struct iio_dev *iio_dev,
> +			struct iio_chan_spec const *chan,
> +			int *val,
> +			int *val2,
> +			long info)
> +{
> +	struct z188_adc *adc = iio_priv(iio_dev);
> +	int ret;
> +	u16 tmp;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		tmp = readw(adc->base + chan->channel * 4);
> +
> +		if (ADC_OVR(tmp)) {
> +			dev_info(&iio_dev->dev,
> +				"Oversampling error on ADC channel %d\n",
> +				chan->channel);
> +			return -EIO;
> +		}
> +		*val = ADC_DATA(tmp);
> +		ret = IIO_VAL_INT;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static struct iio_info z188_adc_info = {
> +	.read_raw = &z188_iio_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static void men_z188_config_channels(void __iomem *addr)
> +{
> +	int i;
> +	u32 cfg;
> +	u32 ctl;
> +
> +	ctl = readl(addr + Z188_CTRL_REG);
> +	ctl |= Z188_CFG_AUTO;
> +	writel(ctl, addr + Z188_CTRL_REG);
> +
> +	for (i = 0; i < Z188_ADC_MAX_CHAN; i++) {
> +		cfg = readl(addr + i);
> +		cfg &= ~Z188_ADC_GAIN;
> +		cfg |= Z188_MODE_VOLTAGE;
> +		writel(cfg, addr + i);
> +	}
> +}
> +
> +static int men_z188_probe(struct mcb_device *dev,
> +			const struct mcb_device_id *id)
> +{
> +	struct z188_adc *adc;
> +	struct iio_dev *indio_dev;
> +	struct resource *mem;
> +
> +	indio_dev = devm_iio_device_alloc(&dev->dev, sizeof(struct z188_adc));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	adc = iio_priv(indio_dev);
> +	indio_dev->name = "z188-adc";
> +	indio_dev->dev.parent = &dev->dev;
> +	indio_dev->info = &z188_adc_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = z188_adc_iio_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(z188_adc_iio_channels);
> +
> +	mem = mcb_request_mem(dev, "z188-adc");
> +	if (!mem)
> +		return -ENOMEM;
> +
> +	adc->base = ioremap(mem->start, resource_size(mem));
> +	if (adc->base == NULL)
> +		goto err;
> +
> +	men_z188_config_channels(adc->base);
> +
> +	adc->mem = mem;
> +	mcb_set_drvdata(dev, indio_dev);
> +
> +	return  devm_iio_device_register(&dev->dev, indio_dev);
Using devm_iio_device_register means that the userspace interface
will only be removed after your remove function below had run.
That leaves room for some nasty race conditions.

The rule of thumb is don't use that particular devm function unless
absolutely everything is using devm interfaces.  E.g. you don't have
any remove function at all.

Sorry, I missed this in the previous version.

> +
> +err:
> +	mcb_release_mem(mem);
> +	return -ENXIO;
> +}
> +
> +static void men_z188_remove(struct mcb_device *dev)
> +{
> +	struct iio_dev *indio_dev  = mcb_get_drvdata(dev);
> +	struct z188_adc *adc = iio_priv(indio_dev);
> +
> +	iounmap(adc->base);
Currently the userspace interface is still active after you've unmapped
this?  Definitely a bad idea.
> +	mcb_release_mem(adc->mem);
> +}
> +
> +static const struct mcb_device_id men_z188_ids[] = {
> +	{ .device = 0xbc },
> +};
> +MODULE_DEVICE_TABLE(mcb, men_z188_ids);
> +
> +static struct mcb_driver men_z188_driver = {
> +	.driver = {
> +		.name = "z188-adc",
> +		.owner = THIS_MODULE,
> +	},
> +	.probe = men_z188_probe,
> +	.remove = men_z188_remove,
> +	.id_table = men_z188_ids,
> +};
> +module_mcb_driver(men_z188_driver);
> +
> +MODULE_AUTHOR("Johannes Thumshirn <johannes.thumshirn@men.de>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("IIO ADC driver for MEN 16z188 ADC Core");
> +MODULE_ALIAS("mcb:16z188");
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>