Message-ID: <55B7B5DB.6050105@ge.com>
Date:	Tue, 28 Jul 2015 18:03:23 +0100
From:	Martyn Welch <martyn.welch@...com>
To:	Dmitry Kalinkin <dmitry.kalinkin@...il.com>,
	<linux-kernel@...r.kernel.org>, <devel@...verdev.osuosl.org>
CC:	Manohar Vanga <manohar.vanga@...il.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Hans J. Koch" <hjk@...sjkoch.de>,
	"Igor Alekseev" <igor.alekseev@...p.ru>
Subject: Re: [RFC] Generic VME UIO

Hi Dmitry,

On 22/07/15 19:09, Dmitry Kalinkin wrote:
> Linux kernel has supported VME bus since 2009. The support comes in a form
> of kernel driver API that is backed by a couple drivers for PCI-VME
> bridges.  There is also a vme_user driver that provides a generic userpsace
> interface to do data transfer and generate interrupts on the bus. Due to
> specifics of the VME bus, this interface doesn't fit into the UIO
> framework. The other useful feature is to be able to receive VME interrupts
> in the userspace can, on the other hand, benefit from reusing UIO.
>
> VME bus offers seven interrupt request lines IRQ1-IRQ7 corresponding to
> seven interrupt levels.  In the event of interrupt, the interrupt handler
> board is to prioritize interrupts in accordance to their levels.  The
> interrupt handler then takes ownership over the data bus to perform an
> interrupt acknowledge cycle where it supplies an interrupt level to be
> acknowledged. When multiple interrupters are producing interrupt at the
> same level, only one interrupt gets acknowledged based on interrupters
> position in IACKIN/IACKOUT daisy- chain.  The response of the interrupter
> to a relevant interrupt will contain a 8, 16 or 32 bit interrupt vector
> (also called STATUS/ID). After the interrupt acknowledge cycle, the
> interrupter is to remove it's interrupt request from the bus. Such
> standartized scheme is called "Release On AcKnowledge" (ROAK).
>
> Like PCI, VME has it's own corner case where interrupt request is removed
> on VME device register access. VME specification acknowledges this
> behaviour and calls it "Release On Register Access" (RORA) and requires
> RORA devices to still provide interrupt vector value in acknowledge cycles.
> I'm not aware how widespread the RORA devices are.
>

 From memory all the hardware I've seen is RORA, but from talking to 
you, it seems you've had the opposite experience, so I suspect it might 
be quite market dependent.

> The driver below provides a generic userspace interface to handle ROAK VME
> device interrupts.
>
> The user is to enable interrupt vectors through a sysfs interface. For
> example, enabling handler for interrupt vector 0x6b at the level 1 will
> look like:
>
>    echo 1 > /sys/bus/vme/devices/vme_uio.0-0/irq/1/6b/enabled
>
> A separate UIO device is created for each handler. Waiting for event can be
> done as:
>
>    dd if=/dev/uio0 bs=4 count=1
>
> In response for this RFC I would like to hear your comments or suggestions
> on the proposed sysfs interface, about the idea in general. Some tips on
> how to better handle kobject cleanup are also very welcome.
>

Looks like a good start to me, please provide some documentation under 
Documentation/ before submitting (and please clearly state that it will 
only work for ROAK hardware!).

> The vme_uio driver is provided separately for the ease of review, it's code
> is intended for the merge into vme_user.
>
> Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@...il.com>
> Cc: Igor Alekseev <igor.alekseev@...p.ru>
>
> ---
>   drivers/staging/vme/devices/Kconfig   |  10 +++
>   drivers/staging/vme/devices/Makefile  |   1 +
>   drivers/staging/vme/devices/vme_uio.c | 158 ++++++++++++++++++++++++++++++++++
>   drivers/vme/vme_bridge.h              |   4 +-
>   include/linux/vme.h                   |   3 +
>   5 files changed, 175 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/staging/vme/devices/vme_uio.c
>
> diff --git a/drivers/staging/vme/devices/Kconfig b/drivers/staging/vme/devices/Kconfig
> index 1d2ff0c..0300226 100644
> --- a/drivers/staging/vme/devices/Kconfig
> +++ b/drivers/staging/vme/devices/Kconfig
> @@ -1,5 +1,15 @@
>   comment "VME Device Drivers"
>
> +config VME_UIO
> +	tristate "VME UIO user space access driver"
> +	depends on STAGING && VME_BUS && UIO
> +	help
> +	 Say Y here to include UIO interface to VME. This module currently
> +	 allows you to deliver VME interrupts to user space.
> +
> +	 To compile this driver as a module, choose M here. The module will
> +	 be called vme_uio. If unsure, say N.
> +
>   config VME_USER
>   	tristate "VME user space access driver"
>   	depends on STAGING
> diff --git a/drivers/staging/vme/devices/Makefile b/drivers/staging/vme/devices/Makefile
> index 172512c..c198004 100644
> --- a/drivers/staging/vme/devices/Makefile
> +++ b/drivers/staging/vme/devices/Makefile
> @@ -2,6 +2,7 @@
>   # Makefile for the VME device drivers.
>   #
>
> +obj-$(CONFIG_VME_UIO)		+= vme_uio.o
>   obj-$(CONFIG_VME_USER)		+= vme_user.o
>
>   vme_pio2-objs	:= vme_pio2_cntr.o vme_pio2_gpio.o vme_pio2_core.o
> diff --git a/drivers/staging/vme/devices/vme_uio.c b/drivers/staging/vme/devices/vme_uio.c
> new file mode 100644
> index 0000000..4c55a23
> --- /dev/null
> +++ b/drivers/staging/vme/devices/vme_uio.c
> @@ -0,0 +1,158 @@
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/uio_driver.h>
> +#include <linux/vme.h>
> +
> +static void vme_uio_int(int level, int status_id, void *priv)
> +{
> +	struct uio_info *info = priv;
> +
> +	uio_event_notify(info);
> +}
> +
> +struct int_sysfs_entry {
> +	struct kobj_attribute kobj_attr;
> +	struct vme_dev *vdev;
> +	struct uio_info uio;
> +	int level;
> +	int statid;
> +	int enabled;
> +};
> +
> +static ssize_t int_enabled_show(struct kobject *kobj,
> +				struct kobj_attribute *attr, char *buf)
> +{
> +	struct int_sysfs_entry *entry;
> +
> +	entry = container_of(attr, struct int_sysfs_entry, kobj_attr);
> +	return sprintf(buf, "%d\n", entry->enabled);
> +}
> +
> +static ssize_t int_enabled_store(struct kobject *kobj,
> +				 struct kobj_attribute *attr, const char *buf,
> +				 size_t count)
> +{
> +	int enabled;
> +	struct int_sysfs_entry *entry;
> +	int ret;
> +
> +	entry = container_of(attr, struct int_sysfs_entry, kobj_attr);
> +
> +	ret = kstrtoint(buf, 0, &enabled);
> +	if (ret)
> +		return ret;
> +	enabled = !!enabled;
> +
> +	if (enabled == entry->enabled)
> +		return count;
> +
> +	if (enabled) {
> +		ret = uio_register_device(&entry->vdev->dev, &entry->uio);
> +		if (ret) {
> +			enabled = 0;
> +			return ret;
> +		}
> +
> +		ret = vme_irq_request(entry->vdev, entry->level, entry->statid,
> +				      vme_uio_int, &entry->uio);
> +		if (ret) {

You need a uio_unregister_device() here, or create a exit path and jump 
to it with goto (which could also be used for disabling an interrupt I 
suppose).

> +			enabled = 0;
> +			return ret;
> +		}
> +	} else {
> +		vme_irq_free(entry->vdev, entry->level, entry->statid);
> +
> +		uio_unregister_device(&entry->uio);
> +	}
> +
> +	entry->enabled = enabled;
> +
> +	return count;
> +}
> +
> +static struct kobj_attribute int_enabled_attribute =
> +	__ATTR(enabled, 0644, int_enabled_show, int_enabled_store);
> +
> +static int vme_uio_match(struct vme_dev *vdev)
> +{
> +	return 1;
> +}
> +
> +static int vme_uio_probe(struct vme_dev *vdev)
> +{
> +	int ret, level, statid;
> +
> +	int bus_num = vme_bus_num(vdev);
> +
> +	struct kobject *kobj = kobject_create_and_add("irq", &vdev->dev.kobj);
> +
> +	for (level = 1; level <= 7; level++) {
> +		char *level_node_name = kasprintf(GFP_KERNEL, "%d", level);
> +		struct kobject *level_node = kobject_create_and_add(
> +			level_node_name, kobj);
> +		if (!level_node)
> +			return -ENOMEM;
> +
> +		for (statid = 0; statid < VME_NUM_STATUSID; statid++) {
> +			char *statid_node_name = kasprintf(GFP_KERNEL,
> +							   "%02x", statid);
> +			struct kobject *statid_node;
> +			struct int_sysfs_entry *entry;
> +
> +			statid_node = kobject_create_and_add(statid_node_name,
> +							     level_node);
> +			if (!statid_node)
> +				return -ENOMEM;
> +
> +			entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +			if (!entry)
> +				return -ENOMEM;
> +			entry->uio.name = kasprintf(GFP_KERNEL,
> +						    "vme_irq_%d_%d_%02x",
> +						    bus_num, level, statid);
> +			entry->uio.version = "1";
> +			entry->uio.irq = UIO_IRQ_CUSTOM;
> +			entry->level = level;
> +			entry->statid = statid;
> +			entry->vdev = vdev;
> +			entry->enabled = 0;
> +			entry->kobj_attr = int_enabled_attribute;
> +			ret = sysfs_create_file(statid_node,
> +						&entry->kobj_attr.attr);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int vme_uio_remove(struct vme_dev *vdev)
> +{
> +	/* XXX Cleanup here */
> +	return 0;
> +}
> +
> +static struct vme_driver vme_uio_driver = {
> +	.name = "vme_uio",
> +	.match = vme_uio_match,
> +	.probe = vme_uio_probe,
> +	.remove = vme_uio_remove,
> +};
> +
> +static int __init vme_uio_init(void)
> +{
> +	return vme_register_driver(&vme_uio_driver, 1);
> +}
> +
> +static void __exit vme_uio_exit(void)
> +{
> +	vme_unregister_driver(&vme_uio_driver);
> +}
> +
> +module_init(vme_uio_init);
> +module_exit(vme_uio_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Dmitry Kalinkin <dmitry.kalinkin@...il.com>");
> diff --git a/drivers/vme/vme_bridge.h b/drivers/vme/vme_bridge.h
> index 37d2fd7..a3ef63b 100644
> --- a/drivers/vme/vme_bridge.h
> +++ b/drivers/vme/vme_bridge.h
> @@ -1,6 +1,8 @@
>   #ifndef _VME_BRIDGE_H_
>   #define _VME_BRIDGE_H_
>
> +#include <linux/vme.h>
> +
>   #define VME_CRCSR_BUF_SIZE (508*1024)
>   /*
>    * Resource structures
> @@ -88,7 +90,7 @@ struct vme_callback {
>
>   struct vme_irq {
>   	int count;
> -	struct vme_callback callback[256];
> +	struct vme_callback callback[VME_NUM_STATUSID];
>   };
>
>   /* Allow 16 characters for name (including null character) */
> diff --git a/include/linux/vme.h b/include/linux/vme.h
> index c013135..71e4a6d 100644
> --- a/include/linux/vme.h
> +++ b/include/linux/vme.h
> @@ -81,6 +81,9 @@ struct vme_resource {
>
>   extern struct bus_type vme_bus_type;
>
> +/* Number of VME interrupt vectors */
> +#define VME_NUM_STATUSID	256
> +
>   /* VME_MAX_BRIDGES comes from the type of vme_bus_numbers */
>   #define VME_MAX_BRIDGES		(sizeof(unsigned int)*8)
>   #define VME_MAX_SLOTS		32
>

-- 
Martyn Welch (Lead Software Engineer)  | Registered in England and Wales
GE Intelligent Platforms               | (3828642) at 100 Barbirolli Square
T +44(0)1327322748                     | Manchester, M2 3AB
E martyn.welch@...com                  | VAT:GB 927559189
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists