* [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices [not found] <1425728161-164217-1-git-send-email-alexander.shishkin@linux.intel.com> @ 2015-03-07 11:35 ` Alexander Shishkin 2015-03-07 22:26 ` Paul Bolle [not found] ` <1425728161-164217-2-git-send-email-alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 0 siblings, 2 replies; 11+ messages in thread From: Alexander Shishkin @ 2015-03-07 11:35 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, Pratik Patel, mathieu.poirier, peter.lachner, norbert.schulz, keven.boell, yann.fouassier, laurent.fert, Alexander Shishkin, linux-api A System Trace Module (STM) is a device exporting data in System Trace Protocol (STP) format as defined by MIPI STP standards. Examples of such devices are Intel Trace Hub and Coresight STM. This abstraction provides a unified interface for software trace sources to send their data over an STM device to a debug host. In order to do that, such a trace source needs to be assigned a pair of master/channel identifiers that all the data from this source will be tagged with. The STP decoder on the debug host side will use these master/channel tags to distinguish different trace streams from one another inside one STP stream. This abstraction provides a configfs-based policy management mechanism for dynamic allocation of these master/channel pairs based on trace source-supplied string identifier. It has the flexibility of being defined at runtime and at the same time (provided that the policy definition is aligned with the decoding end) consistency. For userspace trace sources, this abstraction provides write()-based and mmap()-based (if the underlying stm device allows this) output mechanism. For kernel-side trace sources, we provide "stm_source" device class that can be connected to an stm device at run time. Cc: linux-api@vger.kernel.org Cc: Pratik Patel <pratikp@codeaurora.org> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com> --- Documentation/ABI/testing/configfs-stp-policy | 44 ++ Documentation/ABI/testing/sysfs-class-stm | 14 + Documentation/ABI/testing/sysfs-class-stm_source | 11 + Documentation/trace/stm.txt | 77 +++ drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/stm/Kconfig | 8 + drivers/stm/Makefile | 3 + drivers/stm/core.c | 839 +++++++++++++++++++++++ drivers/stm/policy.c | 470 +++++++++++++ drivers/stm/stm.h | 77 +++ include/linux/stm.h | 87 +++ include/uapi/linux/stm.h | 47 ++ 13 files changed, 1680 insertions(+) create mode 100644 Documentation/ABI/testing/configfs-stp-policy create mode 100644 Documentation/ABI/testing/sysfs-class-stm create mode 100644 Documentation/ABI/testing/sysfs-class-stm_source create mode 100644 Documentation/trace/stm.txt create mode 100644 drivers/stm/Kconfig create mode 100644 drivers/stm/Makefile create mode 100644 drivers/stm/core.c create mode 100644 drivers/stm/policy.c create mode 100644 drivers/stm/stm.h create mode 100644 include/linux/stm.h create mode 100644 include/uapi/linux/stm.h diff --git a/Documentation/ABI/testing/configfs-stp-policy b/Documentation/ABI/testing/configfs-stp-policy new file mode 100644 index 0000000000..1c7ab3dbcd --- /dev/null +++ b/Documentation/ABI/testing/configfs-stp-policy @@ -0,0 +1,44 @@ +What: /config/stp-policy +Date: Jan 2015 +KernelVersion: 3.20 +Description: + This group contains policies mandating Master/Channel allocation + for software sources wishing to send trace data over an STM + device. + +What: /config/stp-policy/<policy> +Date: Jan 2015 +KernelVersion: 3.20 +Description: + Root of a policy. This name is an arbitrary string. + +What: /config/stp-policy/<policy>/device +Date: Jan 2015 +KernelVersion: 3.20 +Description: + STM device to which this policy applies. Write a valid stm class + device name here to assign this policy to that device. + +What: /config/stp-policy/<policy>/<node> +Date: Jan 2015 +KernelVersion: 3.20 +Description: + Policy node is a string identifier that software clients will + use to request a master/channel to be allocated and assigned to + them. + +What: /config/stp-policy/<policy>/<node>/masters +Date: Jan 2015 +KernelVersion: 3.20 +Description: + Range of masters from which to allocate for users of this node. + Write two numbers: the first master and the last master number. + +What: /config/stp-policy/<policy>/<node>/channels +Date: Jan 2015 +KernelVersion: 3.20 +Description: + Range of channels from which to allocate for users of this node. + Write two numbers: the first channel and the last channel + number. + diff --git a/Documentation/ABI/testing/sysfs-class-stm b/Documentation/ABI/testing/sysfs-class-stm new file mode 100644 index 0000000000..186f8e66e1 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-stm @@ -0,0 +1,14 @@ +What: /sys/class/stm/<stm>/masters +Date: Jan 2015 +KernelVersion: 3.20 +Contact: Alexander Shishkin <alexander.shishkin@linux.intel.com> +Description: + Shows first and last available to software master numbers on + this STM device. + +What: /sys/class/stm/<stm>/channels +Date: Jan 2015 +KernelVersion: 3.20 +Contact: Alexander Shishkin <alexander.shishkin@linux.intel.com> +Description: + Shows the number of channels per master on this STM device. diff --git a/Documentation/ABI/testing/sysfs-class-stm_source b/Documentation/ABI/testing/sysfs-class-stm_source new file mode 100644 index 0000000000..735b0d657f --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-stm_source @@ -0,0 +1,11 @@ +What: /sys/class/stm_source/<stm_source>/stm_source_link +Date: Jan 2015 +KernelVersion: 3.20 +Contact: Alexander Shishkin <alexander.shishkin@linux.intel.com> +Description: + stm_source device linkage to stm device, where its tracing data + is directed. Reads return an existing connection or "<none>" if + this stm_source is not connected to any stm device yet. + Write an existing (registered) stm device's name here to + connect that device. If a device is already connected to this + stm_source, it will first be disconnected. diff --git a/Documentation/trace/stm.txt b/Documentation/trace/stm.txt new file mode 100644 index 0000000000..0ba5c9115c --- /dev/null +++ b/Documentation/trace/stm.txt @@ -0,0 +1,77 @@ +System Trace Module +=================== + +System Trace Module (STM) is a device described in MIPI STP specs as +STP trace stream generator. STP (System Trace Protocol) is a trace +protocol multiplexing data from multiple trace sources, each one of +which is assigned a unique pair of master and channel. While some of +these masters and channels are statically allocated to certain +hardware trace sources, others are available to software. Software +trace sources are usually free to pick for themselves any +master/channel combination from this pool. + +On the receiving end of this STP stream (the decoder side), trace +sources can only be identified by master/channel combination, so in +order for the decoder to be able to make sense of the trace that +involves multiple trace sources, it needs to be able to map those +master/channel pairs to the trace sources that it understands. + +For instance, it is helpful to know that syslog messages come on +master 7 channel 15, while arbitrary user applications can use masters +48 to 63 and channels 0 to 127. + +To solve this mapping problem, stm class provides a policy management +mechanism via configfs, that allows defining rules that map string +identifiers to ranges of masters and channels. If these rules (policy) +are consistent with what decoder expects, it will be able to properly +process the trace data. + +This policy is a tree structure containing rules (policy_node) that +have a name (string identifier) and a range of masters and channels +associated with it, located in "stp-policy" subsystem directory in +configfs. From the examle above, a rule may look like this: + +$ ls /config/stp-policy/my-policy/user +channels masters +$ cat /config/stp-policy/my-policy/user/masters +48 63 +$ cat /config/stp-policy/my-policy/user/channels +0 127 + +which means that the master allocation pool for this rule consists of +masters 48 through 63 and channel allocation pool has channels 0 +through 127 in it. Now, any producer (trace source) identifying itself +with "user" identification string will be allocated a master and +channel from within these ranges. + +These rules can be nested, for example, one can define a rule "dummy" +under "user" directory from the example above and this new rule will +be used for trace sources with the id string of "user/dummy". + +Trace sources have to open the stm class device's node and write their +trace data into its file descriptor. In order to identify themselves +to the policy, they need to do a STP_POLICY_ID_SET ioctl on this file +descriptor providing their id string. Otherwise, they will be +automatically allocated a master/channel pair upon first write to this +file descriptor according to the "default" rule of the policy, if such +exists. + +Some STM devices may allow direct mapping of the channel mmio regions +to userspace for zero-copy writing. One mappable page (in terms of +mmu) will usually contain multiple channels' mmios, so the user will +need to allocate that many channels to themselves (via the +aforementioned ioctl() call) to be able to do this. That is, if your +stm device's channel mmio region is 64 bytes and hardware page size is +4096 bytes, after a successful STP_POLICY_ID_SET ioctl() call with +width==64, you should be able to mmap() one page on this file +descriptor and obtain direct access to an mmio region for 64 channels. + +For kernel-based trace sources, there is "stm_source" device +class. Devices of this class can be connected and disconnected to/from +stm devices at runtime via a sysfs attribute. + +Examples of STM devices are Intel Trace Hub [1] and Coresight STM +[2]. + +[1] https://software.intel.com/sites/default/files/managed/d3/3c/intel-th-developer-manual.pdf +[2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0444b/index.html diff --git a/drivers/Kconfig b/drivers/Kconfig index c0cc96bab9..7bc80670bb 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -182,4 +182,6 @@ source "drivers/thunderbolt/Kconfig" source "drivers/android/Kconfig" +source "drivers/stm/Kconfig" + endmenu diff --git a/drivers/Makefile b/drivers/Makefile index 527a6da8d5..2d511b411a 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -165,3 +165,4 @@ obj-$(CONFIG_RAS) += ras/ obj-$(CONFIG_THUNDERBOLT) += thunderbolt/ obj-$(CONFIG_CORESIGHT) += coresight/ obj-$(CONFIG_ANDROID) += android/ +obj-$(CONFIG_STM) += stm/ diff --git a/drivers/stm/Kconfig b/drivers/stm/Kconfig new file mode 100644 index 0000000000..90ed327461 --- /dev/null +++ b/drivers/stm/Kconfig @@ -0,0 +1,8 @@ +config STM + tristate "System Trace Module devices" + help + A System Trace Module (STM) is a device exporting data in System + Trace Protocol (STP) format as defined by MIPI STP standards. + Examples of such devices are Intel Trace Hub and Coresight STM. + + Say Y here to enable System Trace Module device support. diff --git a/drivers/stm/Makefile b/drivers/stm/Makefile new file mode 100644 index 0000000000..adec701649 --- /dev/null +++ b/drivers/stm/Makefile @@ -0,0 +1,3 @@ +obj-$(CONFIG_STM) += stm_core.o + +stm_core-y := core.o policy.o diff --git a/drivers/stm/core.c b/drivers/stm/core.c new file mode 100644 index 0000000000..ba0ce55b0a --- /dev/null +++ b/drivers/stm/core.c @@ -0,0 +1,839 @@ +/* + * System Trace Module (STM) infrastructure + * Copyright (c) 2014, Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * STM class implements generic infrastructure for System Trace Module devices + * as defined in MIPI STPv2 specification. + */ + +#include <linux/uaccess.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/device.h> +#include <linux/compat.h> +#include <linux/kdev_t.h> +#include <linux/srcu.h> +#include <linux/slab.h> +#include <linux/stm.h> +#include <linux/fs.h> +#include <linux/mm.h> +#include "stm.h" + +#include <uapi/linux/stm.h> + +static unsigned int stm_core_up; + +/* + * The SRCU here makes sure that STM device doesn't disappear from under a + * stm_source_write() caller, which may want to have as little overhead as + * possible. + */ +static struct srcu_struct stm_source_srcu; + +static ssize_t masters_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct stm_device *stm = dev_get_drvdata(dev); + int ret; + + ret = sprintf(buf, "%u %u\n", stm->data->sw_start, stm->data->sw_end); + + return ret; +} + +static DEVICE_ATTR_RO(masters); + +static ssize_t channels_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct stm_device *stm = dev_get_drvdata(dev); + int ret; + + ret = sprintf(buf, "%u\n", stm->data->sw_nchannels); + + return ret; +} + +static DEVICE_ATTR_RO(channels); + +static struct attribute *stm_attrs[] = { + &dev_attr_masters.attr, + &dev_attr_channels.attr, + NULL, +}; + +static const struct attribute_group stm_group = { + .attrs = stm_attrs, +}; + +static const struct attribute_group *stm_groups[] = { + &stm_group, + NULL, +}; + +static struct class stm_class = { + .name = "stm", + .dev_groups = stm_groups, +}; + +static int stm_dev_match(struct device *dev, const void *data) +{ + const char *name = data; + + return sysfs_streq(name, dev_name(dev)); +} + +/** + * stm_find_device() - find stm device by name + * @buf: character buffer containing the name + * @len: length of the name in @buf + * + * This is called from attributes' store methods, so it will + * also trim the trailing newline if necessary. + * + * Return: device pointer or null if lookup failed. + */ +struct device *stm_find_device(const char *buf, size_t len) +{ + if (!stm_core_up) + return NULL; + + return class_find_device(&stm_class, NULL, buf, stm_dev_match); +} + +#define __stm_master(_s, _m) \ + ((_s)->masters[(_m) - (_s)->data->sw_start]) + +static inline struct stp_master * +stm_master(struct stm_device *stm, unsigned int idx) +{ + if (idx < stm->data->sw_start || idx > stm->data->sw_end) + return NULL; + + return __stm_master(stm, idx); +} + +static int stp_master_alloc(struct stm_device *stm, unsigned int idx) +{ + struct stp_master *master; + size_t size; + + size = ALIGN(stm->data->sw_nchannels, 8) / 8; + size += sizeof(struct stp_master); + master = kzalloc(size, GFP_ATOMIC); + if (!master) + return -ENOMEM; + + master->nr_free = stm->data->sw_nchannels; + __stm_master(stm, idx) = master; + + return 0; +} + +static void stp_master_free(struct stm_device *stm, unsigned int idx) +{ + struct stp_master *master = stm_master(stm, idx); + + if (!master) + return; + + __stm_master(stm, idx) = NULL; + kfree(master); +} + +static void stm_output_claim(struct stm_device *stm, struct stm_output *output) +{ + struct stp_master *master = stm_master(stm, output->master); + + if (WARN_ON_ONCE(master->nr_free < output->nr_chans)) + return; + + bitmap_allocate_region(&master->chan_map[0], output->channel, + ilog2(output->nr_chans)); + + master->nr_free -= output->nr_chans; +} + +static void +stm_output_disclaim(struct stm_device *stm, struct stm_output *output) +{ + struct stp_master *master = stm_master(stm, output->master); + + bitmap_release_region(&master->chan_map[0], output->channel, + ilog2(output->nr_chans)); + + master->nr_free += output->nr_chans; +} + +/* + * This is like bitmap_find_free_region(), except it can ignore @start bits + * at the beginning. + */ +static int find_free_channels(unsigned long *bitmap, unsigned int start, + unsigned int end, unsigned int width) +{ + unsigned int pos; + int i; + + for (pos = start; pos < end + 1; pos = ALIGN(pos, width)) { + pos = find_next_zero_bit(bitmap, end + 1, pos); + if (pos + width > end + 1) + break; + + if (pos & (width - 1)) + continue; + + for (i = 1; i < width && !test_bit(pos + i, bitmap); i++) + ; + if (i == width) + return pos; + } + + return -1; +} + +static unsigned int +stm_find_master_chan(struct stm_device *stm, unsigned int width, + unsigned int *mstart, unsigned int mend, + unsigned int *cstart, unsigned int cend) +{ + struct stp_master *master; + unsigned int midx; + int pos, err; + + for (midx = *mstart; midx <= mend; midx++) { + if (!stm_master(stm, midx)) { + err = stp_master_alloc(stm, midx); + if (err) + return err; + } + + master = stm_master(stm, midx); + + if (!master->nr_free) + continue; + + pos = find_free_channels(master->chan_map, *cstart, cend, + width); + if (pos < 0) + continue; + + *mstart = midx; + *cstart = pos; + return 0; + } + + return -ENOSPC; +} + +static int stm_output_assign(struct stm_device *stm, unsigned int width, + struct stp_policy_node *policy_node, + struct stm_output *output) +{ + unsigned int midx, cidx, mend, cend; + int ret = -EBUSY; + + if (width > stm->data->sw_nchannels) + return -EINVAL; + + if (policy_node) { + stp_policy_node_get_ranges(policy_node, + &midx, &mend, &cidx, &cend); + } else { + midx = stm->data->sw_start; + cidx = 0; + mend = stm->data->sw_end; + cend = stm->data->sw_nchannels - 1; + } + + spin_lock(&stm->mc_lock); + if (output->nr_chans) + goto unlock; + + ret = stm_find_master_chan(stm, width, &midx, mend, &cidx, cend); + if (ret) + goto unlock; + + output->master = midx; + output->channel = cidx; + output->nr_chans = width; + stm_output_claim(stm, output); + dev_dbg(stm->dev, "assigned %u:%u (+%u)\n", midx, cidx, width); + + ret = 0; +unlock: + spin_unlock(&stm->mc_lock); + + return ret; +} + +static void stm_output_free(struct stm_device *stm, struct stm_output *output) +{ + spin_lock(&stm->mc_lock); + if (output->nr_chans) + stm_output_disclaim(stm, output); + spin_unlock(&stm->mc_lock); +} + +static int major_match(struct device *dev, const void *data) +{ + unsigned int major = *(unsigned int *)data; + + return MAJOR(dev->devt) == major; +} + +static int stm_char_open(struct inode *inode, struct file *file) +{ + struct stm_file *stmf; + struct device *dev; + unsigned int major = imajor(inode); + int err = -ENODEV; + + dev = class_find_device(&stm_class, NULL, &major, major_match); + if (!dev) + return -ENODEV; + + stmf = kzalloc(sizeof(*stmf), GFP_KERNEL); + if (!stmf) + return -ENOMEM; + + stmf->stm = dev_get_drvdata(dev); + + if (!try_module_get(stmf->stm->owner)) + goto err_free; + + file->private_data = stmf; + + return nonseekable_open(inode, file); + +err_free: + kfree(stmf); + + return err; +} + +static int stm_char_release(struct inode *inode, struct file *file) +{ + struct stm_file *stmf = file->private_data; + + stm_output_free(stmf->stm, &stmf->output); + module_put(stmf->stm->owner); + kfree(stmf); + + return 0; +} + +static int stm_file_assign(struct stm_file *stmf, char *id, unsigned int width) +{ + struct stm_device *stm = stmf->stm; + int ret; + + mutex_lock(&stm->policy_mutex); + if (stm->policy) + stmf->policy_node = stp_policy_node_lookup(stm->policy, id); + + ret = stm_output_assign(stm, width, stmf->policy_node, &stmf->output); + mutex_unlock(&stm->policy_mutex); + + return ret; +} + +static ssize_t stm_char_write(struct file *file, const char __user *buf, + size_t count, loff_t *ppos) +{ + struct stm_file *stmf = file->private_data; + struct stm_device *stm = stmf->stm; + char *kbuf; + int err; + + /* + * if no m/c have been assigned to this writer up to this + * point, use "default" policy entry + */ + if (!stmf->output.nr_chans) { + err = stm_file_assign(stmf, "default", 1); + /* + * EBUSY means that somebody else just assigned this + * output, which is just fine for write() + */ + if (err && err != -EBUSY) + return err; + } + + kbuf = kmalloc(count + 1, GFP_KERNEL); + if (!kbuf) + return -ENOMEM; + + err = copy_from_user(kbuf, buf, count); + if (err) { + kfree(kbuf); + return -EFAULT; + } + + stm->data->write(stm->data, stmf->output.master, + stmf->output.channel, kbuf, count); + + + kfree(kbuf); + + return count; +} + +static int stm_char_mmap(struct file *file, struct vm_area_struct *vma) +{ + struct stm_file *stmf = file->private_data; + struct stm_device *stm = stmf->stm; + unsigned long size, phys; + + if (!stm->data->mmio_addr) + return -EOPNOTSUPP; + + if (vma->vm_pgoff) + return -EINVAL; + + size = vma->vm_end - vma->vm_start; + + if (stmf->output.nr_chans * stm->data->sw_mmiosz != size) + return -EINVAL; + + phys = stm->data->mmio_addr(stm->data, stmf->output.master, + stmf->output.channel, + stmf->output.nr_chans); + + if (!phys) + return -EINVAL; + + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; + vm_iomap_memory(vma, phys, size); + + return 0; +} + +static int stm_char_policy_set_ioctl(struct stm_file *stmf, void __user *arg) +{ + struct stm_device *stm = stmf->stm; + struct stp_policy_id *id; + int ret = -EFAULT; + u32 size; + + if (stmf->output.nr_chans) + return -EBUSY; + + if (copy_from_user(&size, arg, sizeof(size))) + return -EFAULT; + + if (size >= PATH_MAX + sizeof(*id)) + return -EINVAL; + + id = kzalloc(size + 1, GFP_KERNEL); + if (!id) + return -ENOMEM; + + if (copy_from_user(id, arg, size)) + goto err_free; + + if (id->__reserved_0 || id->__reserved_1) + return -EINVAL; + + if (id->width < 1 || + id->width > PAGE_SIZE / stm->data->sw_mmiosz) { + ret = -EINVAL; + goto err_free; + } + + ret = stm_file_assign(stmf, id->id, id->width); + if (ret) + goto err_free; + + if (stm->data->link) + stm->data->link(stm->data, stmf->output.master, + stmf->output.channel); + + ret = 0; + +err_free: + kfree(id); + + return ret; +} + +static int stm_char_policy_get_ioctl(struct stm_file *stmf, void __user *arg) +{ + struct stp_policy_id id = { + .size = sizeof(id), + .master = stmf->output.master, + .channel = stmf->output.channel, + .width = stmf->output.nr_chans, + .__reserved_0 = 0, + .__reserved_1 = 0, + }; + + return copy_to_user(arg, &id, id.size) ? -EFAULT : 0; +} + +static long +stm_char_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +{ + struct stm_file *stmf = file->private_data; + int err; + + switch (cmd) { + case STP_POLICY_ID_SET: + err = stm_char_policy_set_ioctl(stmf, (void __user *)arg); + if (err) + return err; + + return stm_char_policy_get_ioctl(stmf, (void __user *)arg); + + case STP_POLICY_ID_GET: + return stm_char_policy_get_ioctl(stmf, (void __user *)arg); + + default: + return -ENOTTY; + } + + return 0; +} + +#ifdef CONFIG_COMPAT +static long +stm_char_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +{ + return stm_char_ioctl(file, cmd, (unsigned long)compat_ptr(arg)); +} +#else +#define stm_char_compat_ioctl NULL +#endif + +static const struct file_operations stm_fops = { + .open = stm_char_open, + .release = stm_char_release, + .write = stm_char_write, + .mmap = stm_char_mmap, + .unlocked_ioctl = stm_char_ioctl, + .compat_ioctl = stm_char_compat_ioctl, + .llseek = no_llseek, +}; + +int stm_register_device(struct device *parent, struct stm_data *stm_data, + struct module *owner) +{ + struct stm_device *stm; + struct device *dev; + unsigned int nmasters; + int err = -ENOMEM; + + if (!stm_core_up) + return -EPROBE_DEFER; + + if (!stm_data->write || !stm_data->sw_nchannels) + return -EINVAL; + + nmasters = stm_data->sw_end - stm_data->sw_start; + stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL); + if (!stm) + return -ENOMEM; + + stm->major = register_chrdev(0, stm_data->name, &stm_fops); + if (stm->major < 0) + goto err_free; + + dev = device_create(&stm_class, parent, MKDEV(stm->major, 0), NULL, + "%s", stm_data->name); + if (IS_ERR(dev)) { + err = PTR_ERR(dev); + goto err_device; + } + + spin_lock_init(&stm->link_lock); + INIT_LIST_HEAD(&stm->link_list); + + spin_lock_init(&stm->mc_lock); + mutex_init(&stm->policy_mutex); + stm->sw_nmasters = nmasters; + stm->owner = owner; + stm->data = stm_data; + stm->dev = dev; + stm_data->stm = stm; + + dev_set_drvdata(dev, stm); + + return 0; + +err_device: + device_unregister(dev); +err_free: + kfree(stm); + + return err; +} +EXPORT_SYMBOL_GPL(stm_register_device); + +static void stm_source_link_drop(struct stm_source_device *src); + +void stm_unregister_device(struct stm_data *stm_data) +{ + struct stm_device *stm = stm_data->stm; + struct stm_source_device *src, *iter; + int i; + + spin_lock(&stm->link_lock); + list_for_each_entry_safe(src, iter, &stm->link_list, link_entry) { + stm_source_link_drop(src); + } + spin_unlock(&stm->link_lock); + + synchronize_srcu(&stm_source_srcu); + + unregister_chrdev(stm->major, stm_data->name); + + if (stm->policy) + stp_policy_unbind(stm->policy); + + for (i = 0; i < stm->sw_nmasters; i++) + stp_master_free(stm, i); + + device_unregister(stm->dev); + kfree(stm); + stm_data->stm = NULL; +} +EXPORT_SYMBOL_GPL(stm_unregister_device); + +static int stm_source_link_add(struct stm_source_device *src, + struct stm_device *stm) +{ + int err; + + spin_lock(&stm->link_lock); + spin_lock(&src->link_lock); + + /* src->link is dereferenced under stm_source_srcu but not the list */ + rcu_assign_pointer(src->link, stm); + list_add_tail(&src->link_entry, &stm->link_list); + + spin_unlock(&src->link_lock); + spin_unlock(&stm->link_lock); + + if (stm->policy) { + char *id = kstrdup(src->data->name, GFP_KERNEL); + + if (id) { + src->policy_node = + stp_policy_node_lookup(stm->policy, id); + + kfree(id); + } + } + + err = stm_output_assign(stm, src->data->nr_chans, + src->policy_node, &src->output); + if (err) + return err; + + if (stm->data->link) + stm->data->link(stm->data, src->output.master, + src->output.channel); + if (src->data->link) + src->data->link(src->data); + + return 0; +} + +static void stm_source_link_drop(struct stm_source_device *src) +{ + int idx = srcu_read_lock(&stm_source_srcu); + + if (src->link && src->data->unlink) + src->data->unlink(src->data); + + srcu_read_unlock(&stm_source_srcu, idx); + + spin_lock(&src->link_lock); + if (src->link) { + stm_output_free(src->link, &src->output); + list_del_init(&src->link_entry); + rcu_assign_pointer(src->link, NULL); + } + spin_unlock(&src->link_lock); +} + +static ssize_t stm_source_link_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct stm_source_device *src = dev_get_drvdata(dev); + int idx, ret; + + idx = srcu_read_lock(&stm_source_srcu); + ret = sprintf(buf, "%s\n", + src->link ? dev_name(src->link->dev) : "<none>"); + srcu_read_unlock(&stm_source_srcu, idx); + + return ret; +} + +static ssize_t stm_source_link_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct stm_source_device *src = dev_get_drvdata(dev); + struct stm_device *link; + struct device *linkdev; + int err; + + stm_source_link_drop(src); + + linkdev = stm_find_device(buf, count); + if (!linkdev) + return -EINVAL; + + link = dev_get_drvdata(linkdev); + + err = stm_source_link_add(src, link); + + return err ? : count; +} + +static DEVICE_ATTR_RW(stm_source_link); + +static struct attribute *stm_source_attrs[] = { + &dev_attr_stm_source_link.attr, + NULL, +}; + +static const struct attribute_group stm_source_group = { + .attrs = stm_source_attrs, +}; + +static const struct attribute_group *stm_source_groups[] = { + &stm_source_group, + NULL, +}; + +static struct class stm_source_class = { + .name = "stm_source", + .dev_groups = stm_source_groups, +}; + +/** + * stm_source_register_device() - register an stm_source device + * @parent: parent device + * @data: device description structure + * + * This will create a device of stm_source class that can write + * data to an stm device once linked. + * + * Return: 0 on success, -errno otherwise. + */ +int stm_source_register_device(struct device *parent, + struct stm_source_data *data) +{ + struct stm_source_device *src; + struct device *dev; + + if (!stm_core_up) + return -EPROBE_DEFER; + + src = kzalloc(sizeof(*src), GFP_KERNEL); + if (!src) + return -ENOMEM; + + dev = device_create(&stm_source_class, parent, MKDEV(0, 0), NULL, "%s", + data->name); + if (IS_ERR(dev)) { + kfree(src); + return PTR_ERR(dev); + } + + spin_lock_init(&src->link_lock); + INIT_LIST_HEAD(&src->link_entry); + src->dev = dev; + src->data = data; + data->src = src; + dev_set_drvdata(dev, src); + + return 0; +} +EXPORT_SYMBOL_GPL(stm_source_register_device); + +/** + * stm_source_unregister_device() - unregister an stm_source device + * @data: device description that was used to register the device + * + * This will remove a previously created stm_source device from the system. + */ +void stm_source_unregister_device(struct stm_source_data *data) +{ + struct stm_source_device *src = data->src; + + stm_source_link_drop(src); + + device_destroy(&stm_source_class, src->dev->devt); + + kfree(src); +} +EXPORT_SYMBOL_GPL(stm_source_unregister_device); + +int stm_source_write(struct stm_source_data *data, unsigned int chan, + const char *buf, size_t count) +{ + struct stm_source_device *src = data->src; + struct stm_device *stm; + int idx; + + if (!src->output.nr_chans) + return -ENODEV; + + if (chan >= src->output.nr_chans) + return -EINVAL; + + idx = srcu_read_lock(&stm_source_srcu); + + stm = srcu_dereference(src->link, &stm_source_srcu); + if (stm) + count = stm->data->write(stm->data, src->output.master, + src->output.channel + chan, buf, + count); + else + count = -ENODEV; + + srcu_read_unlock(&stm_source_srcu, idx); + + return count; +} +EXPORT_SYMBOL_GPL(stm_source_write); + +static int __init stm_core_init(void) +{ + int err; + + err = class_register(&stm_class); + if (err) + return err; + + err = class_register(&stm_source_class); + if (err) { + class_unregister(&stm_class); + return err; + } + + init_srcu_struct(&stm_source_srcu); + + stm_core_up++; + + return 0; +} + +postcore_initcall(stm_core_init); diff --git a/drivers/stm/policy.c b/drivers/stm/policy.c new file mode 100644 index 0000000000..6ce125da3b --- /dev/null +++ b/drivers/stm/policy.c @@ -0,0 +1,470 @@ +/* + * System Trace Module (STM) master/channel allocation policy management + * Copyright (c) 2014, Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * A master/channel allocation policy allows mapping string identifiers to + * master and channel ranges, where allocation can be done. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/types.h> +#include <linux/module.h> +#include <linux/device.h> +#include <linux/configfs.h> +#include <linux/slab.h> +#include <linux/stm.h> +#include "stm.h" + +/* + * STP Master/Channel allocation policy configfs layout. + */ + +struct stp_policy { + struct config_group group; + struct stm_device *stm; +}; + +struct stp_policy_node { + struct config_group group; + struct stm_device *stm; + struct stp_policy *policy; + unsigned int first_master; + unsigned int last_master; + unsigned int first_channel; + unsigned int last_channel; +}; + +void stp_policy_node_get_ranges(struct stp_policy_node *policy_node, + unsigned int *mstart, unsigned int *mend, + unsigned int *cstart, unsigned int *cend) +{ + *mstart = policy_node->first_master; + *mend = policy_node->last_master; + *cstart = policy_node->first_channel; + *cend = policy_node->last_channel; +} + +static inline char *stp_policy_node_name(struct stp_policy_node *policy_node) +{ + return policy_node->group.cg_item.ci_name ? : "<none>"; +} + +static inline struct stp_policy *to_stp_policy(struct config_item *item) +{ + return item ? + container_of(to_config_group(item), struct stp_policy, group) : + NULL; +} + +static inline struct stp_policy_node * +to_stp_policy_node(struct config_item *item) +{ + return item ? + container_of(to_config_group(item), struct stp_policy_node, + group) : + NULL; +} + +static ssize_t stp_policy_node_masters_show(struct stp_policy_node *policy_node, + char *page) +{ + ssize_t count; + + count = sprintf(page, "%u %u\n", policy_node->first_master, + policy_node->last_master); + + return count; +} + +static ssize_t +stp_policy_node_masters_store(struct stp_policy_node *policy_node, + const char *page, size_t count) +{ + struct stm_device *stm = policy_node->stm; + unsigned int first, last; + char *p = (char *) page; + + if (sscanf(p, "%u %u", &first, &last) != 2) + return -EINVAL; + + /* must be within [sw_start..sw_end], which is an inclusive range */ + if (first > INT_MAX || last > INT_MAX || first > last || + first < stm->data->sw_start || + last > stm->data->sw_end) + return -ERANGE; + + policy_node->first_master = first; + policy_node->last_master = last; + + return count; +} + +static ssize_t +stp_policy_node_channels_show(struct stp_policy_node *policy_node, char *page) +{ + ssize_t count; + + count = sprintf(page, "%u %u\n", policy_node->first_channel, + policy_node->last_channel); + + return count; +} + +static ssize_t +stp_policy_node_channels_store(struct stp_policy_node *policy_node, + const char *page, size_t count) +{ + unsigned int first, last; + char *p = (char *) page; + + if (sscanf(p, "%u %u", &first, &last) != 2) + return -EINVAL; + + if (first > INT_MAX || last > INT_MAX || first > last || + last >= policy_node->stm->data->sw_nchannels) + return -ERANGE; + + policy_node->first_channel = first; + policy_node->last_channel = last; + + return count; +} + +static void stp_policy_node_release(struct config_item *item) +{ + kfree(to_stp_policy_node(item)); +} + +struct stp_policy_node_attribute { + struct configfs_attribute attr; + ssize_t (*show)(struct stp_policy_node *, char *); + ssize_t (*store)(struct stp_policy_node *, const char *, size_t); +}; + +static ssize_t stp_policy_node_attr_show(struct config_item *item, + struct configfs_attribute *attr, + char *page) +{ + struct stp_policy_node *policy_node = to_stp_policy_node(item); + struct stp_policy_node_attribute *pn_attr = + container_of(attr, struct stp_policy_node_attribute, attr); + ssize_t count = 0; + + if (pn_attr->show) + count = pn_attr->show(policy_node, page); + + return count; +} + +static ssize_t stp_policy_node_attr_store(struct config_item *item, + struct configfs_attribute *attr, + const char *page, size_t len) +{ + struct stp_policy_node *policy_node = to_stp_policy_node(item); + struct stp_policy_node_attribute *pn_attr = + container_of(attr, struct stp_policy_node_attribute, attr); + ssize_t count = -EINVAL; + + if (pn_attr->store) + count = pn_attr->store(policy_node, page, len); + + return count; +} + +static struct configfs_item_operations stp_policy_node_item_ops = { + .release = stp_policy_node_release, + .show_attribute = stp_policy_node_attr_show, + .store_attribute = stp_policy_node_attr_store, +}; + +static struct stp_policy_node_attribute stp_policy_node_attr_range = { + .attr = { + .ca_owner = THIS_MODULE, + .ca_name = "masters", + .ca_mode = S_IRUGO | S_IWUSR, + }, + .show = stp_policy_node_masters_show, + .store = stp_policy_node_masters_store, +}; + +static struct stp_policy_node_attribute stp_policy_node_attr_channels = { + .attr = { + .ca_owner = THIS_MODULE, + .ca_name = "channels", + .ca_mode = S_IRUGO | S_IWUSR, + }, + .show = stp_policy_node_channels_show, + .store = stp_policy_node_channels_store, +}; + +static struct configfs_attribute *stp_policy_node_attrs[] = { + &stp_policy_node_attr_range.attr, + &stp_policy_node_attr_channels.attr, + NULL, +}; + +static struct config_item_type stp_policy_type; +static struct config_item_type stp_policy_node_type; + +static struct config_group * +stp_policy_node_make(struct config_group *group, const char *name) +{ + struct stp_policy_node *policy_node, *parent_node; + struct stp_policy *policy; + + if (group->cg_item.ci_type == &stp_policy_type) { + policy = container_of(group, struct stp_policy, group); + } else { + parent_node = container_of(group, struct stp_policy_node, + group); + policy = parent_node->policy; + } + + if (!policy->stm) + return ERR_PTR(-ENODEV); + + policy_node = kzalloc(sizeof(struct stp_policy_node), GFP_KERNEL); + if (!policy_node) + return ERR_PTR(-ENOMEM); + + config_group_init_type_name(&policy_node->group, name, + &stp_policy_node_type); + + policy_node->policy = policy; + policy_node->stm = policy->stm; + + /* default values for the attributes */ + policy_node->first_master = policy->stm->data->sw_start; + policy_node->last_master = policy->stm->data->sw_end; + policy_node->first_channel = 0; + policy_node->last_channel = policy->stm->data->sw_nchannels - 1; + + return &policy_node->group; +} + +static void +stp_policy_node_drop(struct config_group *group, struct config_item *item) +{ + config_item_put(item); +} + +static struct configfs_group_operations stp_policy_node_group_ops = { + .make_group = stp_policy_node_make, + .drop_item = stp_policy_node_drop, +}; + +static struct config_item_type stp_policy_node_type = { + .ct_item_ops = &stp_policy_node_item_ops, + .ct_group_ops = &stp_policy_node_group_ops, + .ct_attrs = stp_policy_node_attrs, + .ct_owner = THIS_MODULE, +}; + +/* + * Root group: policies. + */ +static struct configfs_attribute stp_policy_attr_device = { + .ca_owner = THIS_MODULE, + .ca_name = "device", + .ca_mode = S_IRUGO | S_IWUSR, +}; + +static struct configfs_attribute *stp_policy_attrs[] = { + &stp_policy_attr_device, + NULL, +}; + +static ssize_t stp_policy_attr_show(struct config_item *item, + struct configfs_attribute *attr, + char *page) +{ + struct stp_policy *policy = to_stp_policy(item); + + return sprintf(page, "%s\n", + (policy && policy->stm) ? + policy->stm->data->name : + "<none>"); +} + +static ssize_t stp_policy_attr_store(struct config_item *item, + struct configfs_attribute *attr, + const char *page, size_t len) +{ + struct stp_policy *policy = to_stp_policy(item); + ssize_t count = -EINVAL; + struct device *dev; + + dev = stm_find_device(page, len); + if (dev) { + count = len; + if (policy->stm) + put_device(policy->stm->dev); + + policy->stm = dev_get_drvdata(dev); + + mutex_lock(&policy->stm->policy_mutex); + policy->stm->policy = policy; + mutex_unlock(&policy->stm->policy_mutex); + } + + return count; +} + +void stp_policy_unbind(struct stp_policy *policy) +{ + put_device(policy->stm->dev); + + mutex_lock(&policy->stm->policy_mutex); + policy->stm->policy = NULL; + mutex_unlock(&policy->stm->policy_mutex); + + policy->stm = NULL; +} + +static void stp_policy_release(struct config_item *item) +{ + struct stp_policy *policy = to_stp_policy(item); + + stp_policy_unbind(policy); + kfree(policy); +} + +static struct configfs_item_operations stp_policy_item_ops = { + .release = stp_policy_release, + .show_attribute = stp_policy_attr_show, + .store_attribute = stp_policy_attr_store, +}; + +static struct configfs_group_operations stp_policy_group_ops = { + .make_group = stp_policy_node_make, +}; + +static struct config_item_type stp_policy_type = { + .ct_item_ops = &stp_policy_item_ops, + .ct_group_ops = &stp_policy_group_ops, + .ct_attrs = stp_policy_attrs, + .ct_owner = THIS_MODULE, +}; + +static struct config_group * +stp_policies_make(struct config_group *group, const char *name) +{ + struct stp_policy *policy; + + policy = kzalloc(sizeof(*policy), GFP_KERNEL); + if (!policy) + return ERR_PTR(-ENOMEM); + + config_group_init_type_name(&policy->group, name, + &stp_policy_type); + policy->stm = NULL; + + return &policy->group; +} + +static struct configfs_group_operations stp_policies_group_ops = { + .make_group = stp_policies_make, +}; + +static struct config_item_type stp_policies_type = { + .ct_group_ops = &stp_policies_group_ops, + .ct_owner = THIS_MODULE, +}; + +static struct configfs_subsystem stp_policy_subsys = { + .su_group = { + .cg_item = { + .ci_namebuf = "stp-policy", + .ci_type = &stp_policies_type, + }, + }, +}; + +/* + * Lock the policy mutex from the outside + */ +static struct stp_policy_node * +__stp_policy_node_lookup(struct stp_policy *policy, char *s) +{ + struct stp_policy_node *policy_node, *ret; + struct list_head *head = &policy->group.cg_children; + struct config_item *item; + char *start, *end = s; + + if (list_empty(head)) + return NULL; + + /* return the first entry if everything else fails */ + item = list_entry(head->next, struct config_item, ci_entry); + ret = to_stp_policy_node(item); + +next: + for (;;) { + start = strsep(&end, "/"); + if (!start) + break; + + if (!*start) + continue; + + list_for_each_entry(item, head, ci_entry) { + policy_node = to_stp_policy_node(item); + + if (!strcmp(start, + policy_node->group.cg_item.ci_name)) { + ret = policy_node; + + if (!end) + goto out; + + head = &policy_node->group.cg_children; + goto next; + } + } + break; + } + +out: + return ret; +} + +struct stp_policy_node * +stp_policy_node_lookup(struct stp_policy *policy, char *s) +{ + struct stp_policy_node *policy_node; + + mutex_lock(&stp_policy_subsys.su_mutex); + policy_node = __stp_policy_node_lookup(policy, s); + mutex_unlock(&stp_policy_subsys.su_mutex); + + return policy_node; +} + +static int stp_configfs_init(void) +{ + int err; + + config_group_init(&stp_policy_subsys.su_group); + mutex_init(&stp_policy_subsys.su_mutex); + err = configfs_register_subsystem(&stp_policy_subsys); + + return err; +} + +static void stp_configfs_done(void) +{ + configfs_unregister_subsystem(&stp_policy_subsys); +} + +module_init(stp_configfs_init); +module_exit(stp_configfs_done); diff --git a/drivers/stm/stm.h b/drivers/stm/stm.h new file mode 100644 index 0000000000..7d7d9954e2 --- /dev/null +++ b/drivers/stm/stm.h @@ -0,0 +1,77 @@ +/* + * System Trace Module (STM) infrastructure + * Copyright (c) 2014, Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * STM class implements generic infrastructure for System Trace Module devices + * as defined in MIPI STPv2 specification. + */ + +#ifndef _CLASS_STM_H_ +#define _CLASS_STM_H_ + +struct stp_policy; +struct stp_policy_node; + +struct stp_policy_node * +stp_policy_node_lookup(struct stp_policy *policy, char *s); +void stp_policy_unbind(struct stp_policy *policy); + +void stp_policy_node_get_ranges(struct stp_policy_node *policy_node, + unsigned int *mstart, unsigned int *mend, + unsigned int *cstart, unsigned int *cend); + +struct stp_master { + unsigned int nr_free; + unsigned long chan_map[0]; +}; + +struct stm_device { + struct device *dev; + struct module *owner; + struct stp_policy *policy; + struct mutex policy_mutex; + int major; + unsigned int sw_nmasters; + struct stm_data *data; + spinlock_t link_lock; + struct list_head link_list; + /* master allocation */ + spinlock_t mc_lock; + struct stp_master *masters[0]; +}; + +struct stm_output { + unsigned int master; + unsigned int channel; + unsigned int nr_chans; +}; + +struct stm_file { + struct stm_device *stm; + struct stp_policy_node *policy_node; + struct stm_output output; +}; + +struct device *stm_find_device(const char *name, size_t len); + +struct stm_source_device { + struct device *dev; + struct stm_source_data *data; + spinlock_t link_lock; + struct stm_device *link; + struct list_head link_entry; + /* one output per stm_source device */ + struct stp_policy_node *policy_node; + struct stm_output output; +}; + +#endif /* _CLASS_STM_H_ */ diff --git a/include/linux/stm.h b/include/linux/stm.h new file mode 100644 index 0000000000..d00fa9a3fc --- /dev/null +++ b/include/linux/stm.h @@ -0,0 +1,87 @@ +/* + * System Trace Module (STM) infrastructure apis + * Copyright (C) 2014 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#ifndef _STM_H_ +#define _STM_H_ + +struct stp_policy; + +struct stm_device; + +/** + * struct stm_data - STM device description and callbacks + * @name: device name + * @stm: internal structure, only used by stm class code + * @sw_start: first STP master + * @sw_end: last STP master + * @sw_nchannels: number of STP channels per master + * @sw_mmiosz: size of one channel's IO space, for mmap, optional + * @write: write callback + * @mmio_addr: mmap callback, optional + * + * Fill out this structure before calling stm_register_device() to create + * an STM device and stm_unregister_device() to destroy it. It will also be + * passed back to write() and mmio_addr() callbacks. + */ +struct stm_data { + const char *name; + struct stm_device *stm; + unsigned int sw_start; + unsigned int sw_end; + unsigned int sw_nchannels; + unsigned int sw_mmiosz; + ssize_t (*write)(struct stm_data *, unsigned int, + unsigned int, const char *, size_t); + phys_addr_t (*mmio_addr)(struct stm_data *, unsigned int, + unsigned int, unsigned int); + void (*link)(struct stm_data *, unsigned int, + unsigned int); + void (*unlink)(struct stm_data *, unsigned int, + unsigned int); +}; + +int stm_register_device(struct device *parent, struct stm_data *stm_data, + struct module *owner); +void stm_unregister_device(struct stm_data *stm_data); + +struct stm_source_device; + +/** + * struct stm_source_data - STM source device description and callbacks + * @name: device name, will be used for policy lookup + * @src: internal structure, only used by stm class code + * @nr_chans: number of channels to allocate + * @link: called when STM device gets linked to this source + * @unlink: called when STH device is about to be unlinked + * + * Fill in this structure before calling stm_source_register_device() to + * register a source device. Also pass it to unregister and write calls. + */ +struct stm_source_data { + const char *name; + struct stm_source_device *src; + unsigned int percpu; + unsigned int nr_chans; + int (*link)(struct stm_source_data *data); + void (*unlink)(struct stm_source_data *data); +}; + +int stm_source_register_device(struct device *parent, + struct stm_source_data *data); +void stm_source_unregister_device(struct stm_source_data *data); + +int stm_source_write(struct stm_source_data *data, unsigned int chan, + const char *buf, size_t count); + +#endif /* _STM_H_ */ diff --git a/include/uapi/linux/stm.h b/include/uapi/linux/stm.h new file mode 100644 index 0000000000..042b58b53b --- /dev/null +++ b/include/uapi/linux/stm.h @@ -0,0 +1,47 @@ +/* + * System Trace Module (STM) userspace interfaces + * Copyright (c) 2014, Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * STM class implements generic infrastructure for System Trace Module devices + * as defined in MIPI STPv2 specification. + */ + +#ifndef _UAPI_LINUX_STM_H +#define _UAPI_LINUX_STM_H + +/** + * struct stp_policy_id - identification for the STP policy + * @size: size of the structure including real id[] length + * @master: assigned master + * @channel: first assigned channel + * @width: number of requested channels + * @id: identification string + * + * User must calculate the total size of the structure and put it into + * @size field, fill out the @id and desired @width. In return, kernel + * fills out @master, @channel and @width. + */ +struct stp_policy_id { + __u32 size; + __u16 master; + __u16 channel; + __u16 width; + /* padding */ + __u16 __reserved_0; + __u32 __reserved_1; + char id[0]; +}; + +#define STP_POLICY_ID_SET _IOWR('%', 0, struct stp_policy_id) +#define STP_POLICY_ID_GET _IOR('%', 1, struct stp_policy_id) + +#endif /* _UAPI_LINUX_STM_H */ -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices 2015-03-07 11:35 ` [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices Alexander Shishkin @ 2015-03-07 22:26 ` Paul Bolle 2015-03-17 10:13 ` Alexander Shishkin [not found] ` <1425728161-164217-2-git-send-email-alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 1 sibling, 1 reply; 11+ messages in thread From: Paul Bolle @ 2015-03-07 22:26 UTC (permalink / raw) To: Alexander Shishkin Cc: Greg Kroah-Hartman, linux-kernel, Pratik Patel, mathieu.poirier, peter.lachner, norbert.schulz, keven.boell, yann.fouassier, laurent.fert, linux-api On Sat, 2015-03-07 at 13:35 +0200, Alexander Shishkin wrote: > Documentation/ABI/testing/configfs-stp-policy | 44 ++ git am whined about this file when I tried to apply this patch: Applying: stm class: Introduce an abstraction for System Trace Module devices [...]/.git/rebase-apply/patch:77: new blank line at EOF. > Documentation/ABI/testing/sysfs-class-stm | 14 + > Documentation/ABI/testing/sysfs-class-stm_source | 11 + > Documentation/trace/stm.txt | 77 +++ > drivers/Kconfig | 2 + > drivers/Makefile | 1 + > drivers/stm/Kconfig | 8 + > drivers/stm/Makefile | 3 + > drivers/stm/core.c | 839 +++++++++++++++++++++++ > drivers/stm/policy.c | 470 +++++++++++++ > drivers/stm/stm.h | 77 +++ > include/linux/stm.h | 87 +++ > include/uapi/linux/stm.h | 47 ++ > --- /dev/null > +++ b/drivers/stm/Kconfig > @@ -0,0 +1,8 @@ > +config STM > + tristate "System Trace Module devices" > + help > + A System Trace Module (STM) is a device exporting data in System > + Trace Protocol (STP) format as defined by MIPI STP standards. > + Examples of such devices are Intel Trace Hub and Coresight STM. > + > + Say Y here to enable System Trace Module device support. > diff --git a/drivers/stm/Makefile b/drivers/stm/Makefile > new file mode 100644 > index 0000000000..adec701649 > --- /dev/null > +++ b/drivers/stm/Makefile > @@ -0,0 +1,3 @@ > +obj-$(CONFIG_STM) += stm_core.o > + > +stm_core-y := core.o policy.o I tried to compile this as a module: $ make -C ../.. M=$PWD CONFIG_STM=m stm_core.ko make: Entering directory `[...]' LD [M] [...]/drivers/stm/stm_core.o [...]/drivers/stm/policy.o: In function `stp_configfs_init': policy.c:(.text+0x5f0): multiple definition of `init_module' [...]/drivers/stm/core.o:core.c:(.init.text+0x0): first defined here make[1]: *** [[...]/drivers/stm/stm_core.o] Error 1 make: *** [stm_core.ko] Error 2 make: Leaving directory `[...]' I think that's because postcore_initcall(stm_core_init); in core.c becomes module_init(stm_core_init); if this driver is compiled as a module. And that will clash with module_init(stp_configfs_init); in policy.c. Am I missing something obvious or should STM not be a tristate symbol? Paul Bolle ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices 2015-03-07 22:26 ` Paul Bolle @ 2015-03-17 10:13 ` Alexander Shishkin 0 siblings, 0 replies; 11+ messages in thread From: Alexander Shishkin @ 2015-03-17 10:13 UTC (permalink / raw) To: Paul Bolle Cc: Greg Kroah-Hartman, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pratik Patel, mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A, peter.lachner-ral2JQCrhuEAvxtiuMwx3w, norbert.schulz-ral2JQCrhuEAvxtiuMwx3w, keven.boell-ral2JQCrhuEAvxtiuMwx3w, yann.fouassier-ral2JQCrhuEAvxtiuMwx3w, laurent.fert-ral2JQCrhuEAvxtiuMwx3w, linux-api-u79uwXL29TY76Z2rM5mHXA Paul Bolle <pebolle-IWqWACnzNjzz+pZb47iToQ@public.gmane.org> writes: > On Sat, 2015-03-07 at 13:35 +0200, Alexander Shishkin wrote: >> Documentation/ABI/testing/configfs-stp-policy | 44 ++ > > git am whined about this file when I tried to apply this patch: > Applying: stm class: Introduce an abstraction for System Trace Module devices > [...]/.git/rebase-apply/patch:77: new blank line at EOF. > >> Documentation/ABI/testing/sysfs-class-stm | 14 + >> Documentation/ABI/testing/sysfs-class-stm_source | 11 + >> Documentation/trace/stm.txt | 77 +++ >> drivers/Kconfig | 2 + >> drivers/Makefile | 1 + >> drivers/stm/Kconfig | 8 + >> drivers/stm/Makefile | 3 + >> drivers/stm/core.c | 839 +++++++++++++++++++++++ >> drivers/stm/policy.c | 470 +++++++++++++ >> drivers/stm/stm.h | 77 +++ >> include/linux/stm.h | 87 +++ >> include/uapi/linux/stm.h | 47 ++ > >> --- /dev/null >> +++ b/drivers/stm/Kconfig >> @@ -0,0 +1,8 @@ >> +config STM >> + tristate "System Trace Module devices" >> + help >> + A System Trace Module (STM) is a device exporting data in System >> + Trace Protocol (STP) format as defined by MIPI STP standards. >> + Examples of such devices are Intel Trace Hub and Coresight STM. >> + >> + Say Y here to enable System Trace Module device support. >> diff --git a/drivers/stm/Makefile b/drivers/stm/Makefile >> new file mode 100644 >> index 0000000000..adec701649 >> --- /dev/null >> +++ b/drivers/stm/Makefile >> @@ -0,0 +1,3 @@ >> +obj-$(CONFIG_STM) += stm_core.o >> + >> +stm_core-y := core.o policy.o > > I tried to compile this as a module: > $ make -C ../.. M=$PWD CONFIG_STM=m stm_core.ko > make: Entering directory `[...]' > LD [M] [...]/drivers/stm/stm_core.o > [...]/drivers/stm/policy.o: In function `stp_configfs_init': > policy.c:(.text+0x5f0): multiple definition of `init_module' > [...]/drivers/stm/core.o:core.c:(.init.text+0x0): first defined here > make[1]: *** [[...]/drivers/stm/stm_core.o] Error 1 > make: *** [stm_core.ko] Error 2 > make: Leaving directory `[...]' > > I think that's because > postcore_initcall(stm_core_init); > > in core.c becomes > module_init(stm_core_init); > > if this driver is compiled as a module. And that will clash with > module_init(stp_configfs_init); > > in policy.c. Am I missing something obvious or should STM not be a > tristate symbol? My mistake, I think I fancied the policy to be a separate module in the beginning. But I don't see why STM shouldn't be a tristate after the above is fixed. Thanks, -- Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <1425728161-164217-2-git-send-email-alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices [not found] ` <1425728161-164217-2-git-send-email-alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2015-03-09 21:29 ` Mathieu Poirier 2015-03-17 10:37 ` Alexander Shishkin 0 siblings, 1 reply; 11+ messages in thread From: Mathieu Poirier @ 2015-03-09 21:29 UTC (permalink / raw) To: Alexander Shishkin Cc: Greg Kroah-Hartman, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Pratik Patel, peter.lachner-ral2JQCrhuEAvxtiuMwx3w, norbert.schulz-ral2JQCrhuEAvxtiuMwx3w, keven.boell-ral2JQCrhuEAvxtiuMwx3w, yann.fouassier-ral2JQCrhuEAvxtiuMwx3w, laurent.fert-ral2JQCrhuEAvxtiuMwx3w, linux-api-u79uwXL29TY76Z2rM5mHXA On 7 March 2015 at 04:35, Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote: > A System Trace Module (STM) is a device exporting data in System Trace > Protocol (STP) format as defined by MIPI STP standards. Examples of such > devices are Intel Trace Hub and Coresight STM. > > This abstraction provides a unified interface for software trace sources > to send their data over an STM device to a debug host. In order to do > that, such a trace source needs to be assigned a pair of master/channel > identifiers that all the data from this source will be tagged with. The > STP decoder on the debug host side will use these master/channel tags to > distinguish different trace streams from one another inside one STP > stream. > > This abstraction provides a configfs-based policy management mechanism > for dynamic allocation of these master/channel pairs based on trace > source-supplied string identifier. It has the flexibility of being > defined at runtime and at the same time (provided that the policy > definition is aligned with the decoding end) consistency. > > For userspace trace sources, this abstraction provides write()-based and > mmap()-based (if the underlying stm device allows this) output mechanism. > > For kernel-side trace sources, we provide "stm_source" device class that > can be connected to an stm device at run time. > > Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Cc: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> > Cc: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > Signed-off-by: Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> > --- > Documentation/ABI/testing/configfs-stp-policy | 44 ++ > Documentation/ABI/testing/sysfs-class-stm | 14 + > Documentation/ABI/testing/sysfs-class-stm_source | 11 + > Documentation/trace/stm.txt | 77 +++ > drivers/Kconfig | 2 + > drivers/Makefile | 1 + > drivers/stm/Kconfig | 8 + > drivers/stm/Makefile | 3 + > drivers/stm/core.c | 839 +++++++++++++++++++++++ > drivers/stm/policy.c | 470 +++++++++++++ > drivers/stm/stm.h | 77 +++ > include/linux/stm.h | 87 +++ > include/uapi/linux/stm.h | 47 ++ > 13 files changed, 1680 insertions(+) > create mode 100644 Documentation/ABI/testing/configfs-stp-policy > create mode 100644 Documentation/ABI/testing/sysfs-class-stm > create mode 100644 Documentation/ABI/testing/sysfs-class-stm_source > create mode 100644 Documentation/trace/stm.txt > create mode 100644 drivers/stm/Kconfig > create mode 100644 drivers/stm/Makefile > create mode 100644 drivers/stm/core.c > create mode 100644 drivers/stm/policy.c > create mode 100644 drivers/stm/stm.h > create mode 100644 include/linux/stm.h > create mode 100644 include/uapi/linux/stm.h > [Snip...] > + > +static int stm_output_assign(struct stm_device *stm, unsigned int width, > + struct stp_policy_node *policy_node, > + struct stm_output *output) > +{ > + unsigned int midx, cidx, mend, cend; > + int ret = -EBUSY; > + > + if (width > stm->data->sw_nchannels) > + return -EINVAL; > + > + if (policy_node) { Where does this get set? All I found is code that is switching on it. > + stp_policy_node_get_ranges(policy_node, > + &midx, &mend, &cidx, &cend); > + } else { > + midx = stm->data->sw_start; > + cidx = 0; > + mend = stm->data->sw_end; > + cend = stm->data->sw_nchannels - 1; > + } > + > + spin_lock(&stm->mc_lock); > + if (output->nr_chans) > + goto unlock; > + > + ret = stm_find_master_chan(stm, width, &midx, mend, &cidx, cend); > + if (ret) > + goto unlock; > + > + output->master = midx; > + output->channel = cidx; > + output->nr_chans = width; > + stm_output_claim(stm, output); > + dev_dbg(stm->dev, "assigned %u:%u (+%u)\n", midx, cidx, width); > + > + ret = 0; > +unlock: > + spin_unlock(&stm->mc_lock); > + > + return ret; > +} > + [Snip...] > + > +/** > + * stm_source_register_device() - register an stm_source device > + * @parent: parent device > + * @data: device description structure > + * > + * This will create a device of stm_source class that can write > + * data to an stm device once linked. > + * > + * Return: 0 on success, -errno otherwise. > + */ > +int stm_source_register_device(struct device *parent, > + struct stm_source_data *data) > +{ > + struct stm_source_device *src; > + struct device *dev; > + > + if (!stm_core_up) > + return -EPROBE_DEFER; > + > + src = kzalloc(sizeof(*src), GFP_KERNEL); > + if (!src) > + return -ENOMEM; > + > + dev = device_create(&stm_source_class, parent, MKDEV(0, 0), NULL, "%s", > + data->name); > + if (IS_ERR(dev)) { > + kfree(src); > + return PTR_ERR(dev); > + } > + > + spin_lock_init(&src->link_lock); > + INIT_LIST_HEAD(&src->link_entry); > + src->dev = dev; > + src->data = data; > + data->src = src; > + dev_set_drvdata(dev, src); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(stm_source_register_device); > + [Snip...] It's really not clear (at least to me) how stm_source_device works. They make a bridge between the kernel and the STM devices but the "link" between them is not obvious. More documentation perhaps? [Snip...] > + > +/** > + * struct stm_data - STM device description and callbacks > + * @name: device name > + * @stm: internal structure, only used by stm class code > + * @sw_start: first STP master > + * @sw_end: last STP master > + * @sw_nchannels: number of STP channels per master > + * @sw_mmiosz: size of one channel's IO space, for mmap, optional > + * @write: write callback > + * @mmio_addr: mmap callback, optional > + * > + * Fill out this structure before calling stm_register_device() to create > + * an STM device and stm_unregister_device() to destroy it. It will also be > + * passed back to write() and mmio_addr() callbacks. > + */ > +struct stm_data { > + const char *name; > + struct stm_device *stm; > + unsigned int sw_start; > + unsigned int sw_end; The above kernel doc is the only place where "sw_start" and "sw_end" are described as first and last STP masters. Other than that it takes a really long time to figure out what they really are. I think a better naming convention can be chosen here. > + unsigned int sw_nchannels; > + unsigned int sw_mmiosz; > + ssize_t (*write)(struct stm_data *, unsigned int, > + unsigned int, const char *, size_t); > + phys_addr_t (*mmio_addr)(struct stm_data *, unsigned int, > + unsigned int, unsigned int); > + void (*link)(struct stm_data *, unsigned int, > + unsigned int); > + void (*unlink)(struct stm_data *, unsigned int, > + unsigned int); It is really not clear to me what the "link" and "unlink" functions do - documenting what they're for and explain when to use them and (not use them) would be appreciated. I think we should also add two things to this structure: 1) a private field and 2) a (*stm_drv_ioctl) stub. The private field would be filled by the registrant and left alone by the generic-stm core. When parsing the commands in "stm_char_ioctl()", data->stm_drv_ioctl(private, cmd, arg) could be called to let architecture specific drivers deal with it. That way applications can deal with a single configuration file descriptor. > +}; > + > +int stm_register_device(struct device *parent, struct stm_data *stm_data, > + struct module *owner); > +void stm_unregister_device(struct stm_data *stm_data); > + > +struct stm_source_device; > + > +/** > + * struct stm_source_data - STM source device description and callbacks > + * @name: device name, will be used for policy lookup > + * @src: internal structure, only used by stm class code > + * @nr_chans: number of channels to allocate > + * @link: called when STM device gets linked to this source > + * @unlink: called when STH device is about to be unlinked > + * > + * Fill in this structure before calling stm_source_register_device() to > + * register a source device. Also pass it to unregister and write calls. > + */ > +struct stm_source_data { > + const char *name; > + struct stm_source_device *src; > + unsigned int percpu; > + unsigned int nr_chans; > + int (*link)(struct stm_source_data *data); > + void (*unlink)(struct stm_source_data *data); > +}; > + I didn't get the linking/unlinking process - it is also not clear as to why we need struct stm_data and struct stm_source_data. More explanation on this would be good. > +int stm_source_register_device(struct device *parent, > + struct stm_source_data *data); > +void stm_source_unregister_device(struct stm_source_data *data); > + > +int stm_source_write(struct stm_source_data *data, unsigned int chan, > + const char *buf, size_t count); > + > +#endif /* _STM_H_ */ > diff --git a/include/uapi/linux/stm.h b/include/uapi/linux/stm.h > new file mode 100644 > index 0000000000..042b58b53b > --- /dev/null > +++ b/include/uapi/linux/stm.h > @@ -0,0 +1,47 @@ > +/* > + * System Trace Module (STM) userspace interfaces > + * Copyright (c) 2014, Intel Corporation. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + * STM class implements generic infrastructure for System Trace Module devices > + * as defined in MIPI STPv2 specification. > + */ > + > +#ifndef _UAPI_LINUX_STM_H > +#define _UAPI_LINUX_STM_H > + > +/** > + * struct stp_policy_id - identification for the STP policy > + * @size: size of the structure including real id[] length > + * @master: assigned master > + * @channel: first assigned channel > + * @width: number of requested channels > + * @id: identification string > + * > + * User must calculate the total size of the structure and put it into > + * @size field, fill out the @id and desired @width. In return, kernel > + * fills out @master, @channel and @width. > + */ > +struct stp_policy_id { > + __u32 size; > + __u16 master; > + __u16 channel; > + __u16 width; > + /* padding */ > + __u16 __reserved_0; > + __u32 __reserved_1; > + char id[0]; > +}; > + > +#define STP_POLICY_ID_SET _IOWR('%', 0, struct stp_policy_id) > +#define STP_POLICY_ID_GET _IOR('%', 1, struct stp_policy_id) > + > +#endif /* _UAPI_LINUX_STM_H */ > -- > 2.1.4 > Aside from the above points I think this is all very good work. The patchset should likely be broken up in two sets, one for the generic STM architecture wrapper and another for the Intel TH part. That way things can be worked on independently. On my side I will get a prototype going that folds the current coresight-stm driver in this new mindset - I'll let you know how things go. Thanks, Mathieu ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices 2015-03-09 21:29 ` Mathieu Poirier @ 2015-03-17 10:37 ` Alexander Shishkin 2015-03-18 15:07 ` Mathieu Poirier [not found] ` <87mw3bewq8.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org> 0 siblings, 2 replies; 11+ messages in thread From: Alexander Shishkin @ 2015-03-17 10:37 UTC (permalink / raw) To: Mathieu Poirier Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org, Pratik Patel, peter.lachner, norbert.schulz, keven.boell, yann.fouassier, laurent.fert, linux-api Mathieu Poirier <mathieu.poirier@linaro.org> writes: > On 7 March 2015 at 04:35, Alexander Shishkin > <alexander.shishkin@linux.intel.com> wrote: >> A System Trace Module (STM) is a device exporting data in System Trace >> Protocol (STP) format as defined by MIPI STP standards. Examples of such >> devices are Intel Trace Hub and Coresight STM. >> >> This abstraction provides a unified interface for software trace sources >> to send their data over an STM device to a debug host. In order to do >> that, such a trace source needs to be assigned a pair of master/channel >> identifiers that all the data from this source will be tagged with. The >> STP decoder on the debug host side will use these master/channel tags to >> distinguish different trace streams from one another inside one STP >> stream. >> >> This abstraction provides a configfs-based policy management mechanism >> for dynamic allocation of these master/channel pairs based on trace >> source-supplied string identifier. It has the flexibility of being >> defined at runtime and at the same time (provided that the policy >> definition is aligned with the decoding end) consistency. >> >> For userspace trace sources, this abstraction provides write()-based and >> mmap()-based (if the underlying stm device allows this) output mechanism. >> >> For kernel-side trace sources, we provide "stm_source" device class that >> can be connected to an stm device at run time. >> >> Cc: linux-api@vger.kernel.org >> Cc: Pratik Patel <pratikp@codeaurora.org> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com> >> --- >> Documentation/ABI/testing/configfs-stp-policy | 44 ++ >> Documentation/ABI/testing/sysfs-class-stm | 14 + >> Documentation/ABI/testing/sysfs-class-stm_source | 11 + >> Documentation/trace/stm.txt | 77 +++ >> drivers/Kconfig | 2 + >> drivers/Makefile | 1 + >> drivers/stm/Kconfig | 8 + >> drivers/stm/Makefile | 3 + >> drivers/stm/core.c | 839 +++++++++++++++++++++++ >> drivers/stm/policy.c | 470 +++++++++++++ >> drivers/stm/stm.h | 77 +++ >> include/linux/stm.h | 87 +++ >> include/uapi/linux/stm.h | 47 ++ >> 13 files changed, 1680 insertions(+) >> create mode 100644 Documentation/ABI/testing/configfs-stp-policy >> create mode 100644 Documentation/ABI/testing/sysfs-class-stm >> create mode 100644 Documentation/ABI/testing/sysfs-class-stm_source >> create mode 100644 Documentation/trace/stm.txt >> create mode 100644 drivers/stm/Kconfig >> create mode 100644 drivers/stm/Makefile >> create mode 100644 drivers/stm/core.c >> create mode 100644 drivers/stm/policy.c >> create mode 100644 drivers/stm/stm.h >> create mode 100644 include/linux/stm.h >> create mode 100644 include/uapi/linux/stm.h >> > > [Snip...] > >> + >> +static int stm_output_assign(struct stm_device *stm, unsigned int width, >> + struct stp_policy_node *policy_node, >> + struct stm_output *output) >> +{ >> + unsigned int midx, cidx, mend, cend; >> + int ret = -EBUSY; >> + >> + if (width > stm->data->sw_nchannels) >> + return -EINVAL; >> + >> + if (policy_node) { > > Where does this get set? All I found is code that is switching on it. It comes from stp_policy_node_lookup() in stm_file_assign() or stm_source_link_add(). >> + stp_policy_node_get_ranges(policy_node, >> + &midx, &mend, &cidx, &cend); >> + } else { >> + midx = stm->data->sw_start; >> + cidx = 0; >> + mend = stm->data->sw_end; >> + cend = stm->data->sw_nchannels - 1; >> + } >> + >> + spin_lock(&stm->mc_lock); >> + if (output->nr_chans) >> + goto unlock; >> + >> + ret = stm_find_master_chan(stm, width, &midx, mend, &cidx, cend); >> + if (ret) >> + goto unlock; >> + >> + output->master = midx; >> + output->channel = cidx; >> + output->nr_chans = width; >> + stm_output_claim(stm, output); >> + dev_dbg(stm->dev, "assigned %u:%u (+%u)\n", midx, cidx, width); >> + >> + ret = 0; >> +unlock: >> + spin_unlock(&stm->mc_lock); >> + >> + return ret; >> +} >> + > > [Snip...] > >> + >> +/** >> + * stm_source_register_device() - register an stm_source device >> + * @parent: parent device >> + * @data: device description structure >> + * >> + * This will create a device of stm_source class that can write >> + * data to an stm device once linked. >> + * >> + * Return: 0 on success, -errno otherwise. >> + */ >> +int stm_source_register_device(struct device *parent, >> + struct stm_source_data *data) >> +{ >> + struct stm_source_device *src; >> + struct device *dev; >> + >> + if (!stm_core_up) >> + return -EPROBE_DEFER; >> + >> + src = kzalloc(sizeof(*src), GFP_KERNEL); >> + if (!src) >> + return -ENOMEM; >> + >> + dev = device_create(&stm_source_class, parent, MKDEV(0, 0), NULL, "%s", >> + data->name); >> + if (IS_ERR(dev)) { >> + kfree(src); >> + return PTR_ERR(dev); >> + } >> + >> + spin_lock_init(&src->link_lock); >> + INIT_LIST_HEAD(&src->link_entry); >> + src->dev = dev; >> + src->data = data; >> + data->src = src; >> + dev_set_drvdata(dev, src); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(stm_source_register_device); >> + > > [Snip...] > > It's really not clear (at least to me) how stm_source_device works. > They make a bridge between the kernel and the STM devices but the > "link" between them is not obvious. More documentation perhaps? Sure, I need to elaborate on this a bit. The idea the stm_source device gets "linked" to an stm device by way of writing that stm device name to its stm_source_link attribute and if everything's fine, stm_sounrce's .link callback is called, after which it can start sending data out with stm_sounce_write(). >> +/** >> + * struct stm_data - STM device description and callbacks >> + * @name: device name >> + * @stm: internal structure, only used by stm class code >> + * @sw_start: first STP master >> + * @sw_end: last STP master >> + * @sw_nchannels: number of STP channels per master >> + * @sw_mmiosz: size of one channel's IO space, for mmap, optional >> + * @write: write callback >> + * @mmio_addr: mmap callback, optional >> + * >> + * Fill out this structure before calling stm_register_device() to create >> + * an STM device and stm_unregister_device() to destroy it. It will also be >> + * passed back to write() and mmio_addr() callbacks. >> + */ >> +struct stm_data { >> + const char *name; >> + struct stm_device *stm; >> + unsigned int sw_start; >> + unsigned int sw_end; > > The above kernel doc is the only place where "sw_start" and "sw_end" > are described as first and last STP masters. Other than that it takes > a really long time to figure out what they really are. I think a > better naming convention can be chosen here. The idea is that only a certain subset of masters is available for software (others being statically assigned to different hw blocks or otherwise unavailable to software). These two mark start and end of this subset. > >> + unsigned int sw_nchannels; >> + unsigned int sw_mmiosz; >> + ssize_t (*write)(struct stm_data *, unsigned int, >> + unsigned int, const char *, size_t); >> + phys_addr_t (*mmio_addr)(struct stm_data *, unsigned int, >> + unsigned int, unsigned int); >> + void (*link)(struct stm_data *, unsigned int, >> + unsigned int); >> + void (*unlink)(struct stm_data *, unsigned int, >> + unsigned int); > > It is really not clear to me what the "link" and "unlink" functions do > - documenting what they're for and explain when to use them and (not > use them) would be appreciated. Will do. > I think we should also add two things to this structure: 1) a private > field and 2) a (*stm_drv_ioctl) stub. > The private field would be filled by the registrant and left alone by > the generic-stm core. When parsing the commands in > "stm_char_ioctl()", data->stm_drv_ioctl(private, cmd, arg) could be > called to let architecture specific drivers deal with it. That way > applications can deal with a single configuration file descriptor. Indeed, good idea. > >> +}; >> + >> +int stm_register_device(struct device *parent, struct stm_data *stm_data, >> + struct module *owner); >> +void stm_unregister_device(struct stm_data *stm_data); >> + >> +struct stm_source_device; >> + >> +/** >> + * struct stm_source_data - STM source device description and callbacks >> + * @name: device name, will be used for policy lookup >> + * @src: internal structure, only used by stm class code >> + * @nr_chans: number of channels to allocate >> + * @link: called when STM device gets linked to this source >> + * @unlink: called when STH device is about to be unlinked >> + * >> + * Fill in this structure before calling stm_source_register_device() to >> + * register a source device. Also pass it to unregister and write calls. >> + */ >> +struct stm_source_data { >> + const char *name; >> + struct stm_source_device *src; >> + unsigned int percpu; >> + unsigned int nr_chans; >> + int (*link)(struct stm_source_data *data); >> + void (*unlink)(struct stm_source_data *data); >> +}; >> + > > I didn't get the linking/unlinking process - it is also not clear as > to why we need struct stm_data and struct stm_source_data. More > explanation on this would be good. My idea is that connecting stm to stm_source device is done via sysfs attribute write like so: $ echo dummy_stm > /sys/class/stm_source/stm_console/stm_source_link This will result in stm_source's .link getting called, at which point stm_console, for example, will do a register_console(). stm_data is there for stm devices to pass their parameters and callbacks to stm_register_device(); stm_source data is that for stm_source devices. I'll try to be more elaborate about these in my followup. >> +int stm_source_register_device(struct device *parent, >> + struct stm_source_data *data); >> +void stm_source_unregister_device(struct stm_source_data *data); >> + >> +int stm_source_write(struct stm_source_data *data, unsigned int chan, >> + const char *buf, size_t count); >> + >> +#endif /* _STM_H_ */ >> diff --git a/include/uapi/linux/stm.h b/include/uapi/linux/stm.h >> new file mode 100644 >> index 0000000000..042b58b53b >> --- /dev/null >> +++ b/include/uapi/linux/stm.h >> @@ -0,0 +1,47 @@ >> +/* >> + * System Trace Module (STM) userspace interfaces >> + * Copyright (c) 2014, Intel Corporation. >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms and conditions of the GNU General Public License, >> + * version 2, as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope it will be useful, but WITHOUT >> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >> + * more details. >> + * >> + * STM class implements generic infrastructure for System Trace Module devices >> + * as defined in MIPI STPv2 specification. >> + */ >> + >> +#ifndef _UAPI_LINUX_STM_H >> +#define _UAPI_LINUX_STM_H >> + >> +/** >> + * struct stp_policy_id - identification for the STP policy >> + * @size: size of the structure including real id[] length >> + * @master: assigned master >> + * @channel: first assigned channel >> + * @width: number of requested channels >> + * @id: identification string >> + * >> + * User must calculate the total size of the structure and put it into >> + * @size field, fill out the @id and desired @width. In return, kernel >> + * fills out @master, @channel and @width. >> + */ >> +struct stp_policy_id { >> + __u32 size; >> + __u16 master; >> + __u16 channel; >> + __u16 width; >> + /* padding */ >> + __u16 __reserved_0; >> + __u32 __reserved_1; >> + char id[0]; >> +}; >> + >> +#define STP_POLICY_ID_SET _IOWR('%', 0, struct stp_policy_id) >> +#define STP_POLICY_ID_GET _IOR('%', 1, struct stp_policy_id) >> + >> +#endif /* _UAPI_LINUX_STM_H */ >> -- >> 2.1.4 >> > > Aside from the above points I think this is all very good work. The > patchset should likely be broken up in two sets, one for the generic > STM architecture wrapper and another for the Intel TH part. That way > things can be worked on independently. The Intel TH part is very much dependent on the STM part; I could split out the stm-dependant patch from Intel TH, but that just seems to make it more complex for me and reviewers. I'd prefer to keep them together unless there are strong objections. > On my side I will get a prototype going that folds the current > coresight-stm driver in this new mindset - I'll let you know how > things go. Thanks a lot! Regards, -- Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices 2015-03-17 10:37 ` Alexander Shishkin @ 2015-03-18 15:07 ` Mathieu Poirier [not found] ` <CANLsYkx6+mtTf0gPhu37mfbL84oOj0CkW_yOMwFHh_XyqQ7pAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> [not found] ` <87mw3bewq8.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org> 1 sibling, 1 reply; 11+ messages in thread From: Mathieu Poirier @ 2015-03-18 15:07 UTC (permalink / raw) To: Alexander Shishkin Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org, Pratik Patel, peter.lachner, norbert.schulz, keven.boell, yann.fouassier, laurent.fert, linux-api On 17 March 2015 at 04:37, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Mathieu Poirier <mathieu.poirier@linaro.org> writes: > >> On 7 March 2015 at 04:35, Alexander Shishkin >> <alexander.shishkin@linux.intel.com> wrote: >>> A System Trace Module (STM) is a device exporting data in System Trace >>> Protocol (STP) format as defined by MIPI STP standards. Examples of such >>> devices are Intel Trace Hub and Coresight STM. >>> >>> This abstraction provides a unified interface for software trace sources >>> to send their data over an STM device to a debug host. In order to do >>> that, such a trace source needs to be assigned a pair of master/channel >>> identifiers that all the data from this source will be tagged with. The >>> STP decoder on the debug host side will use these master/channel tags to >>> distinguish different trace streams from one another inside one STP >>> stream. >>> >>> This abstraction provides a configfs-based policy management mechanism >>> for dynamic allocation of these master/channel pairs based on trace >>> source-supplied string identifier. It has the flexibility of being >>> defined at runtime and at the same time (provided that the policy >>> definition is aligned with the decoding end) consistency. >>> >>> For userspace trace sources, this abstraction provides write()-based and >>> mmap()-based (if the underlying stm device allows this) output mechanism. >>> >>> For kernel-side trace sources, we provide "stm_source" device class that >>> can be connected to an stm device at run time. >>> >>> Cc: linux-api@vger.kernel.org >>> Cc: Pratik Patel <pratikp@codeaurora.org> >>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >>> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com> >>> --- >>> Documentation/ABI/testing/configfs-stp-policy | 44 ++ >>> Documentation/ABI/testing/sysfs-class-stm | 14 + >>> Documentation/ABI/testing/sysfs-class-stm_source | 11 + >>> Documentation/trace/stm.txt | 77 +++ >>> drivers/Kconfig | 2 + >>> drivers/Makefile | 1 + >>> drivers/stm/Kconfig | 8 + >>> drivers/stm/Makefile | 3 + >>> drivers/stm/core.c | 839 +++++++++++++++++++++++ >>> drivers/stm/policy.c | 470 +++++++++++++ >>> drivers/stm/stm.h | 77 +++ >>> include/linux/stm.h | 87 +++ >>> include/uapi/linux/stm.h | 47 ++ >>> 13 files changed, 1680 insertions(+) >>> create mode 100644 Documentation/ABI/testing/configfs-stp-policy >>> create mode 100644 Documentation/ABI/testing/sysfs-class-stm >>> create mode 100644 Documentation/ABI/testing/sysfs-class-stm_source >>> create mode 100644 Documentation/trace/stm.txt >>> create mode 100644 drivers/stm/Kconfig >>> create mode 100644 drivers/stm/Makefile >>> create mode 100644 drivers/stm/core.c >>> create mode 100644 drivers/stm/policy.c >>> create mode 100644 drivers/stm/stm.h >>> create mode 100644 include/linux/stm.h >>> create mode 100644 include/uapi/linux/stm.h >>> >> >> [Snip...] >> >>> + >>> +static int stm_output_assign(struct stm_device *stm, unsigned int width, >>> + struct stp_policy_node *policy_node, >>> + struct stm_output *output) >>> +{ >>> + unsigned int midx, cidx, mend, cend; >>> + int ret = -EBUSY; >>> + >>> + if (width > stm->data->sw_nchannels) >>> + return -EINVAL; >>> + >>> + if (policy_node) { >> >> Where does this get set? All I found is code that is switching on it. > > It comes from stp_policy_node_lookup() in stm_file_assign() or > stm_source_link_add(). > >>> + stp_policy_node_get_ranges(policy_node, >>> + &midx, &mend, &cidx, &cend); >>> + } else { >>> + midx = stm->data->sw_start; >>> + cidx = 0; >>> + mend = stm->data->sw_end; >>> + cend = stm->data->sw_nchannels - 1; >>> + } >>> + >>> + spin_lock(&stm->mc_lock); >>> + if (output->nr_chans) >>> + goto unlock; >>> + >>> + ret = stm_find_master_chan(stm, width, &midx, mend, &cidx, cend); >>> + if (ret) >>> + goto unlock; >>> + >>> + output->master = midx; >>> + output->channel = cidx; >>> + output->nr_chans = width; >>> + stm_output_claim(stm, output); >>> + dev_dbg(stm->dev, "assigned %u:%u (+%u)\n", midx, cidx, width); >>> + >>> + ret = 0; >>> +unlock: >>> + spin_unlock(&stm->mc_lock); >>> + >>> + return ret; >>> +} >>> + >> >> [Snip...] >> >>> + >>> +/** >>> + * stm_source_register_device() - register an stm_source device >>> + * @parent: parent device >>> + * @data: device description structure >>> + * >>> + * This will create a device of stm_source class that can write >>> + * data to an stm device once linked. >>> + * >>> + * Return: 0 on success, -errno otherwise. >>> + */ >>> +int stm_source_register_device(struct device *parent, >>> + struct stm_source_data *data) >>> +{ >>> + struct stm_source_device *src; >>> + struct device *dev; >>> + >>> + if (!stm_core_up) >>> + return -EPROBE_DEFER; >>> + >>> + src = kzalloc(sizeof(*src), GFP_KERNEL); >>> + if (!src) >>> + return -ENOMEM; >>> + >>> + dev = device_create(&stm_source_class, parent, MKDEV(0, 0), NULL, "%s", >>> + data->name); >>> + if (IS_ERR(dev)) { >>> + kfree(src); >>> + return PTR_ERR(dev); >>> + } >>> + >>> + spin_lock_init(&src->link_lock); >>> + INIT_LIST_HEAD(&src->link_entry); >>> + src->dev = dev; >>> + src->data = data; >>> + data->src = src; >>> + dev_set_drvdata(dev, src); >>> + >>> + return 0; >>> +} >>> +EXPORT_SYMBOL_GPL(stm_source_register_device); >>> + >> >> [Snip...] >> >> It's really not clear (at least to me) how stm_source_device works. >> They make a bridge between the kernel and the STM devices but the >> "link" between them is not obvious. More documentation perhaps? > > Sure, I need to elaborate on this a bit. The idea the stm_source device > gets "linked" to an stm device by way of writing that stm device name to > its stm_source_link attribute and if everything's fine, stm_sounrce's > .link callback is called, after which it can start sending data out with > stm_sounce_write(). > >>> +/** >>> + * struct stm_data - STM device description and callbacks >>> + * @name: device name >>> + * @stm: internal structure, only used by stm class code >>> + * @sw_start: first STP master >>> + * @sw_end: last STP master >>> + * @sw_nchannels: number of STP channels per master >>> + * @sw_mmiosz: size of one channel's IO space, for mmap, optional >>> + * @write: write callback >>> + * @mmio_addr: mmap callback, optional >>> + * >>> + * Fill out this structure before calling stm_register_device() to create >>> + * an STM device and stm_unregister_device() to destroy it. It will also be >>> + * passed back to write() and mmio_addr() callbacks. >>> + */ >>> +struct stm_data { >>> + const char *name; >>> + struct stm_device *stm; >>> + unsigned int sw_start; >>> + unsigned int sw_end; >> >> The above kernel doc is the only place where "sw_start" and "sw_end" >> are described as first and last STP masters. Other than that it takes >> a really long time to figure out what they really are. I think a >> better naming convention can be chosen here. > > The idea is that only a certain subset of masters is available for > software (others being statically assigned to different hw blocks or > otherwise unavailable to software). These two mark start and end of this > subset. > >> >>> + unsigned int sw_nchannels; >>> + unsigned int sw_mmiosz; >>> + ssize_t (*write)(struct stm_data *, unsigned int, >>> + unsigned int, const char *, size_t); >>> + phys_addr_t (*mmio_addr)(struct stm_data *, unsigned int, >>> + unsigned int, unsigned int); >>> + void (*link)(struct stm_data *, unsigned int, >>> + unsigned int); >>> + void (*unlink)(struct stm_data *, unsigned int, >>> + unsigned int); >> >> It is really not clear to me what the "link" and "unlink" functions do >> - documenting what they're for and explain when to use them and (not >> use them) would be appreciated. > > Will do. > >> I think we should also add two things to this structure: 1) a private >> field and 2) a (*stm_drv_ioctl) stub. >> The private field would be filled by the registrant and left alone by >> the generic-stm core. When parsing the commands in >> "stm_char_ioctl()", data->stm_drv_ioctl(private, cmd, arg) could be >> called to let architecture specific drivers deal with it. That way >> applications can deal with a single configuration file descriptor. > > Indeed, good idea. > >> >>> +}; >>> + >>> +int stm_register_device(struct device *parent, struct stm_data *stm_data, >>> + struct module *owner); >>> +void stm_unregister_device(struct stm_data *stm_data); >>> + >>> +struct stm_source_device; >>> + >>> +/** >>> + * struct stm_source_data - STM source device description and callbacks >>> + * @name: device name, will be used for policy lookup >>> + * @src: internal structure, only used by stm class code >>> + * @nr_chans: number of channels to allocate >>> + * @link: called when STM device gets linked to this source >>> + * @unlink: called when STH device is about to be unlinked >>> + * >>> + * Fill in this structure before calling stm_source_register_device() to >>> + * register a source device. Also pass it to unregister and write calls. >>> + */ >>> +struct stm_source_data { >>> + const char *name; >>> + struct stm_source_device *src; >>> + unsigned int percpu; >>> + unsigned int nr_chans; >>> + int (*link)(struct stm_source_data *data); >>> + void (*unlink)(struct stm_source_data *data); >>> +}; >>> + >> >> I didn't get the linking/unlinking process - it is also not clear as >> to why we need struct stm_data and struct stm_source_data. More >> explanation on this would be good. > > My idea is that connecting stm to stm_source device is done via sysfs > attribute write like so: > > $ echo dummy_stm > /sys/class/stm_source/stm_console/stm_source_link > > This will result in stm_source's .link getting called, at which point > stm_console, for example, will do a register_console(). > > stm_data is there for stm devices to pass their parameters and callbacks > to stm_register_device(); stm_source data is that for stm_source > devices. I'll try to be more elaborate about these in my followup. > >>> +int stm_source_register_device(struct device *parent, >>> + struct stm_source_data *data); >>> +void stm_source_unregister_device(struct stm_source_data *data); >>> + >>> +int stm_source_write(struct stm_source_data *data, unsigned int chan, >>> + const char *buf, size_t count); >>> + >>> +#endif /* _STM_H_ */ >>> diff --git a/include/uapi/linux/stm.h b/include/uapi/linux/stm.h >>> new file mode 100644 >>> index 0000000000..042b58b53b >>> --- /dev/null >>> +++ b/include/uapi/linux/stm.h >>> @@ -0,0 +1,47 @@ >>> +/* >>> + * System Trace Module (STM) userspace interfaces >>> + * Copyright (c) 2014, Intel Corporation. >>> + * >>> + * This program is free software; you can redistribute it and/or modify it >>> + * under the terms and conditions of the GNU General Public License, >>> + * version 2, as published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope it will be useful, but WITHOUT >>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >>> + * more details. >>> + * >>> + * STM class implements generic infrastructure for System Trace Module devices >>> + * as defined in MIPI STPv2 specification. >>> + */ >>> + >>> +#ifndef _UAPI_LINUX_STM_H >>> +#define _UAPI_LINUX_STM_H >>> + >>> +/** >>> + * struct stp_policy_id - identification for the STP policy >>> + * @size: size of the structure including real id[] length >>> + * @master: assigned master >>> + * @channel: first assigned channel >>> + * @width: number of requested channels >>> + * @id: identification string >>> + * >>> + * User must calculate the total size of the structure and put it into >>> + * @size field, fill out the @id and desired @width. In return, kernel >>> + * fills out @master, @channel and @width. >>> + */ >>> +struct stp_policy_id { >>> + __u32 size; >>> + __u16 master; >>> + __u16 channel; >>> + __u16 width; >>> + /* padding */ >>> + __u16 __reserved_0; >>> + __u32 __reserved_1; >>> + char id[0]; >>> +}; >>> + >>> +#define STP_POLICY_ID_SET _IOWR('%', 0, struct stp_policy_id) >>> +#define STP_POLICY_ID_GET _IOR('%', 1, struct stp_policy_id) >>> + >>> +#endif /* _UAPI_LINUX_STM_H */ >>> -- >>> 2.1.4 >>> >> >> Aside from the above points I think this is all very good work. The >> patchset should likely be broken up in two sets, one for the generic >> STM architecture wrapper and another for the Intel TH part. That way >> things can be worked on independently. > > The Intel TH part is very much dependent on the STM part; I could split > out the stm-dependant patch from Intel TH, but that just seems to make > it more complex for me and reviewers. I'd prefer to keep them together > unless there are strong objections. > >> On my side I will get a prototype going that folds the current >> coresight-stm driver in this new mindset - I'll let you know how >> things go. > > Thanks a lot! > > Regards, I forgot to mention in my previous email... I think the hierarchy of our respective tracing module along with the generic-stm probably needs a review. Currently we have drivers/coresight, drivers/intel_th and drivers/stm. To me it doesn't scale - what happens when other architectures come out with their own hw tracing technologies? I suggest we move everything under drivers/hwtracing and as such have: drivers/hwtracing drivers/hwtracing/intel_ht drivers/hwtracing/coresight drivers/hwtracing/stm That way other architectures can add drivers for their own hw tracing technology without further polluting the drivers/ directory and concentrating everything in the same area. What's your view on that? > -- > Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <CANLsYkx6+mtTf0gPhu37mfbL84oOj0CkW_yOMwFHh_XyqQ7pAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices [not found] ` <CANLsYkx6+mtTf0gPhu37mfbL84oOj0CkW_yOMwFHh_XyqQ7pAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-03-19 14:23 ` Alexander Shishkin [not found] ` <877fuddq2c.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org> 0 siblings, 1 reply; 11+ messages in thread From: Alexander Shishkin @ 2015-03-19 14:23 UTC (permalink / raw) To: Mathieu Poirier Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org, Pratik Patel, peter.lachner-ral2JQCrhuEAvxtiuMwx3w, norbert.schulz-ral2JQCrhuEAvxtiuMwx3w, keven.boell-ral2JQCrhuEAvxtiuMwx3w, yann.fouassier-ral2JQCrhuEAvxtiuMwx3w, laurent.fert-ral2JQCrhuEAvxtiuMwx3w, linux-api-u79uwXL29TY76Z2rM5mHXA Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes: > I forgot to mention in my previous email... I think the hierarchy of > our respective tracing module along with the generic-stm probably > needs a review. > > Currently we have drivers/coresight, drivers/intel_th and drivers/stm. > > To me it doesn't scale - what happens when other architectures come > out with their own hw tracing technologies? > > I suggest we move everything under drivers/hwtracing and as such have: > > drivers/hwtracing > drivers/hwtracing/intel_ht > drivers/hwtracing/coresight > drivers/hwtracing/stm > > That way other architectures can add drivers for their own hw tracing > technology without further polluting the drivers/ directory and > concentrating everything in the same area. What's your view on that? I wanted to suggest something similar, actually, if you don't mind moving drivers/coresight, then let's do it. Regards, -- Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <877fuddq2c.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org>]
* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices [not found] ` <877fuddq2c.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org> @ 2015-03-19 14:39 ` Mathieu Poirier 0 siblings, 0 replies; 11+ messages in thread From: Mathieu Poirier @ 2015-03-19 14:39 UTC (permalink / raw) To: Alexander Shishkin Cc: Greg Kroah-Hartman, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Pratik Patel, peter.lachner-ral2JQCrhuEAvxtiuMwx3w, norbert.schulz-ral2JQCrhuEAvxtiuMwx3w, keven.boell-ral2JQCrhuEAvxtiuMwx3w, yann.fouassier-ral2JQCrhuEAvxtiuMwx3w, laurent.fert-ral2JQCrhuEAvxtiuMwx3w, linux-api-u79uwXL29TY76Z2rM5mHXA On 19 March 2015 at 08:23, Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote: > Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes: > >> I forgot to mention in my previous email... I think the hierarchy of >> our respective tracing module along with the generic-stm probably >> needs a review. >> >> Currently we have drivers/coresight, drivers/intel_th and drivers/stm. >> >> To me it doesn't scale - what happens when other architectures come >> out with their own hw tracing technologies? >> >> I suggest we move everything under drivers/hwtracing and as such have: >> >> drivers/hwtracing >> drivers/hwtracing/intel_ht >> drivers/hwtracing/coresight >> drivers/hwtracing/stm >> >> That way other architectures can add drivers for their own hw tracing >> technology without further polluting the drivers/ directory and >> concentrating everything in the same area. What's your view on that? > > I wanted to suggest something similar, actually, if you don't mind > moving drivers/coresight, then let's do it. That's a deal - my next patchset will reflect that new organisation. > > Regards, > -- > Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <87mw3bewq8.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org>]
* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices [not found] ` <87mw3bewq8.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org> @ 2015-03-19 22:21 ` Mathieu Poirier 2015-03-20 14:53 ` Alexander Shishkin 0 siblings, 1 reply; 11+ messages in thread From: Mathieu Poirier @ 2015-03-19 22:21 UTC (permalink / raw) To: Alexander Shishkin Cc: Greg Kroah-Hartman, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Pratik Patel, peter.lachner-ral2JQCrhuEAvxtiuMwx3w, norbert.schulz-ral2JQCrhuEAvxtiuMwx3w, keven.boell-ral2JQCrhuEAvxtiuMwx3w, yann.fouassier-ral2JQCrhuEAvxtiuMwx3w, laurent.fert-ral2JQCrhuEAvxtiuMwx3w, linux-api-u79uwXL29TY76Z2rM5mHXA On 17 March 2015 at 04:37, Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote: > Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes: > >> On 7 March 2015 at 04:35, Alexander Shishkin >> <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote: >>> A System Trace Module (STM) is a device exporting data in System Trace >>> Protocol (STP) format as defined by MIPI STP standards. Examples of such >>> devices are Intel Trace Hub and Coresight STM. >>> >>> This abstraction provides a unified interface for software trace sources >>> to send their data over an STM device to a debug host. In order to do >>> that, such a trace source needs to be assigned a pair of master/channel >>> identifiers that all the data from this source will be tagged with. The >>> STP decoder on the debug host side will use these master/channel tags to >>> distinguish different trace streams from one another inside one STP >>> stream. >>> >>> This abstraction provides a configfs-based policy management mechanism >>> for dynamic allocation of these master/channel pairs based on trace >>> source-supplied string identifier. It has the flexibility of being >>> defined at runtime and at the same time (provided that the policy >>> definition is aligned with the decoding end) consistency. >>> >>> For userspace trace sources, this abstraction provides write()-based and >>> mmap()-based (if the underlying stm device allows this) output mechanism. >>> >>> For kernel-side trace sources, we provide "stm_source" device class that >>> can be connected to an stm device at run time. >>> >>> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >>> Cc: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> >>> Cc: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> >>> Signed-off-by: Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>> --- >>> Documentation/ABI/testing/configfs-stp-policy | 44 ++ >>> Documentation/ABI/testing/sysfs-class-stm | 14 + >>> Documentation/ABI/testing/sysfs-class-stm_source | 11 + >>> Documentation/trace/stm.txt | 77 +++ >>> drivers/Kconfig | 2 + >>> drivers/Makefile | 1 + >>> drivers/stm/Kconfig | 8 + >>> drivers/stm/Makefile | 3 + >>> drivers/stm/core.c | 839 +++++++++++++++++++++++ >>> drivers/stm/policy.c | 470 +++++++++++++ >>> drivers/stm/stm.h | 77 +++ >>> include/linux/stm.h | 87 +++ >>> include/uapi/linux/stm.h | 47 ++ >>> 13 files changed, 1680 insertions(+) >>> create mode 100644 Documentation/ABI/testing/configfs-stp-policy >>> create mode 100644 Documentation/ABI/testing/sysfs-class-stm >>> create mode 100644 Documentation/ABI/testing/sysfs-class-stm_source >>> create mode 100644 Documentation/trace/stm.txt >>> create mode 100644 drivers/stm/Kconfig >>> create mode 100644 drivers/stm/Makefile >>> create mode 100644 drivers/stm/core.c >>> create mode 100644 drivers/stm/policy.c >>> create mode 100644 drivers/stm/stm.h >>> create mode 100644 include/linux/stm.h >>> create mode 100644 include/uapi/linux/stm.h >>> >> >> [Snip...] >> >>> + >>> +static int stm_output_assign(struct stm_device *stm, unsigned int width, >>> + struct stp_policy_node *policy_node, >>> + struct stm_output *output) >>> +{ >>> + unsigned int midx, cidx, mend, cend; >>> + int ret = -EBUSY; >>> + >>> + if (width > stm->data->sw_nchannels) >>> + return -EINVAL; >>> + >>> + if (policy_node) { >> >> Where does this get set? All I found is code that is switching on it. > > It comes from stp_policy_node_lookup() in stm_file_assign() or > stm_source_link_add(). > >>> + stp_policy_node_get_ranges(policy_node, >>> + &midx, &mend, &cidx, &cend); >>> + } else { >>> + midx = stm->data->sw_start; >>> + cidx = 0; >>> + mend = stm->data->sw_end; >>> + cend = stm->data->sw_nchannels - 1; >>> + } >>> + >>> + spin_lock(&stm->mc_lock); >>> + if (output->nr_chans) >>> + goto unlock; >>> + >>> + ret = stm_find_master_chan(stm, width, &midx, mend, &cidx, cend); >>> + if (ret) >>> + goto unlock; >>> + >>> + output->master = midx; >>> + output->channel = cidx; >>> + output->nr_chans = width; >>> + stm_output_claim(stm, output); >>> + dev_dbg(stm->dev, "assigned %u:%u (+%u)\n", midx, cidx, width); >>> + >>> + ret = 0; >>> +unlock: >>> + spin_unlock(&stm->mc_lock); >>> + >>> + return ret; >>> +} >>> + >> >> [Snip...] >> >>> + >>> +/** >>> + * stm_source_register_device() - register an stm_source device >>> + * @parent: parent device >>> + * @data: device description structure >>> + * >>> + * This will create a device of stm_source class that can write >>> + * data to an stm device once linked. >>> + * >>> + * Return: 0 on success, -errno otherwise. >>> + */ >>> +int stm_source_register_device(struct device *parent, >>> + struct stm_source_data *data) >>> +{ >>> + struct stm_source_device *src; >>> + struct device *dev; >>> + >>> + if (!stm_core_up) >>> + return -EPROBE_DEFER; >>> + >>> + src = kzalloc(sizeof(*src), GFP_KERNEL); >>> + if (!src) >>> + return -ENOMEM; >>> + >>> + dev = device_create(&stm_source_class, parent, MKDEV(0, 0), NULL, "%s", >>> + data->name); >>> + if (IS_ERR(dev)) { >>> + kfree(src); >>> + return PTR_ERR(dev); >>> + } >>> + >>> + spin_lock_init(&src->link_lock); >>> + INIT_LIST_HEAD(&src->link_entry); >>> + src->dev = dev; >>> + src->data = data; >>> + data->src = src; >>> + dev_set_drvdata(dev, src); >>> + >>> + return 0; >>> +} >>> +EXPORT_SYMBOL_GPL(stm_source_register_device); >>> + >> >> [Snip...] >> >> It's really not clear (at least to me) how stm_source_device works. >> They make a bridge between the kernel and the STM devices but the >> "link" between them is not obvious. More documentation perhaps? > > Sure, I need to elaborate on this a bit. The idea the stm_source device > gets "linked" to an stm device by way of writing that stm device name to > its stm_source_link attribute and if everything's fine, stm_sounrce's > .link callback is called, after which it can start sending data out with > stm_sounce_write(). > >>> +/** >>> + * struct stm_data - STM device description and callbacks >>> + * @name: device name >>> + * @stm: internal structure, only used by stm class code >>> + * @sw_start: first STP master >>> + * @sw_end: last STP master >>> + * @sw_nchannels: number of STP channels per master >>> + * @sw_mmiosz: size of one channel's IO space, for mmap, optional >>> + * @write: write callback >>> + * @mmio_addr: mmap callback, optional >>> + * >>> + * Fill out this structure before calling stm_register_device() to create >>> + * an STM device and stm_unregister_device() to destroy it. It will also be >>> + * passed back to write() and mmio_addr() callbacks. >>> + */ >>> +struct stm_data { >>> + const char *name; >>> + struct stm_device *stm; >>> + unsigned int sw_start; >>> + unsigned int sw_end; >> >> The above kernel doc is the only place where "sw_start" and "sw_end" >> are described as first and last STP masters. Other than that it takes >> a really long time to figure out what they really are. I think a >> better naming convention can be chosen here. > > The idea is that only a certain subset of masters is available for > software (others being statically assigned to different hw blocks or > otherwise unavailable to software). These two mark start and end of this > subset. > >> >>> + unsigned int sw_nchannels; >>> + unsigned int sw_mmiosz; >>> + ssize_t (*write)(struct stm_data *, unsigned int, >>> + unsigned int, const char *, size_t); >>> + phys_addr_t (*mmio_addr)(struct stm_data *, unsigned int, >>> + unsigned int, unsigned int); >>> + void (*link)(struct stm_data *, unsigned int, >>> + unsigned int); >>> + void (*unlink)(struct stm_data *, unsigned int, >>> + unsigned int); >> >> It is really not clear to me what the "link" and "unlink" functions do >> - documenting what they're for and explain when to use them and (not >> use them) would be appreciated. > > Will do. > >> I think we should also add two things to this structure: 1) a private >> field and 2) a (*stm_drv_ioctl) stub. >> The private field would be filled by the registrant and left alone by >> the generic-stm core. When parsing the commands in >> "stm_char_ioctl()", data->stm_drv_ioctl(private, cmd, arg) could be >> called to let architecture specific drivers deal with it. That way >> applications can deal with a single configuration file descriptor. > > Indeed, good idea. > >> >>> +}; >>> + >>> +int stm_register_device(struct device *parent, struct stm_data *stm_data, >>> + struct module *owner); >>> +void stm_unregister_device(struct stm_data *stm_data); >>> + >>> +struct stm_source_device; >>> + >>> +/** >>> + * struct stm_source_data - STM source device description and callbacks >>> + * @name: device name, will be used for policy lookup >>> + * @src: internal structure, only used by stm class code >>> + * @nr_chans: number of channels to allocate >>> + * @link: called when STM device gets linked to this source >>> + * @unlink: called when STH device is about to be unlinked >>> + * >>> + * Fill in this structure before calling stm_source_register_device() to >>> + * register a source device. Also pass it to unregister and write calls. >>> + */ >>> +struct stm_source_data { >>> + const char *name; >>> + struct stm_source_device *src; >>> + unsigned int percpu; >>> + unsigned int nr_chans; >>> + int (*link)(struct stm_source_data *data); >>> + void (*unlink)(struct stm_source_data *data); >>> +}; >>> + >> >> I didn't get the linking/unlinking process - it is also not clear as >> to why we need struct stm_data and struct stm_source_data. More >> explanation on this would be good. > > My idea is that connecting stm to stm_source device is done via sysfs > attribute write like so: > > $ echo dummy_stm > /sys/class/stm_source/stm_console/stm_source_link > > This will result in stm_source's .link getting called, at which point > stm_console, for example, will do a register_console(). > > stm_data is there for stm devices to pass their parameters and callbacks > to stm_register_device(); stm_source data is that for stm_source > devices. I'll try to be more elaborate about these in my followup. > >>> +int stm_source_register_device(struct device *parent, >>> + struct stm_source_data *data); >>> +void stm_source_unregister_device(struct stm_source_data *data); >>> + >>> +int stm_source_write(struct stm_source_data *data, unsigned int chan, >>> + const char *buf, size_t count); >>> + >>> +#endif /* _STM_H_ */ >>> diff --git a/include/uapi/linux/stm.h b/include/uapi/linux/stm.h >>> new file mode 100644 >>> index 0000000000..042b58b53b >>> --- /dev/null >>> +++ b/include/uapi/linux/stm.h >>> @@ -0,0 +1,47 @@ >>> +/* >>> + * System Trace Module (STM) userspace interfaces >>> + * Copyright (c) 2014, Intel Corporation. >>> + * >>> + * This program is free software; you can redistribute it and/or modify it >>> + * under the terms and conditions of the GNU General Public License, >>> + * version 2, as published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope it will be useful, but WITHOUT >>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >>> + * more details. >>> + * >>> + * STM class implements generic infrastructure for System Trace Module devices >>> + * as defined in MIPI STPv2 specification. >>> + */ >>> + >>> +#ifndef _UAPI_LINUX_STM_H >>> +#define _UAPI_LINUX_STM_H >>> + >>> +/** >>> + * struct stp_policy_id - identification for the STP policy >>> + * @size: size of the structure including real id[] length >>> + * @master: assigned master >>> + * @channel: first assigned channel >>> + * @width: number of requested channels >>> + * @id: identification string >>> + * >>> + * User must calculate the total size of the structure and put it into >>> + * @size field, fill out the @id and desired @width. In return, kernel >>> + * fills out @master, @channel and @width. >>> + */ >>> +struct stp_policy_id { >>> + __u32 size; >>> + __u16 master; >>> + __u16 channel; >>> + __u16 width; >>> + /* padding */ >>> + __u16 __reserved_0; >>> + __u32 __reserved_1; >>> + char id[0]; >>> +}; >>> + >>> +#define STP_POLICY_ID_SET _IOWR('%', 0, struct stp_policy_id) >>> +#define STP_POLICY_ID_GET _IOR('%', 1, struct stp_policy_id) >>> + >>> +#endif /* _UAPI_LINUX_STM_H */ >>> -- >>> 2.1.4 >>> >> >> Aside from the above points I think this is all very good work. The >> patchset should likely be broken up in two sets, one for the generic >> STM architecture wrapper and another for the Intel TH part. That way >> things can be worked on independently. > > The Intel TH part is very much dependent on the STM part; I could split > out the stm-dependant patch from Intel TH, but that just seems to make > it more complex for me and reviewers. I'd prefer to keep them together > unless there are strong objections. > >> On my side I will get a prototype going that folds the current >> coresight-stm driver in this new mindset - I'll let you know how >> things go. > > Thanks a lot! > > Regards, > -- > Alex As promised I worked on a prototype that connects the coresight-stm driver with the generic STM interface you have suggested. Things work quite well and aside from the enhancement related to the ioctl() and private member as discussed above, we should move ahead with this. I will send out a new version of the coresight-stm driver as soon as I see your patches with those changes. Thanks, Mathieu ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices 2015-03-19 22:21 ` Mathieu Poirier @ 2015-03-20 14:53 ` Alexander Shishkin [not found] ` <87k2ybd8jl.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org> 0 siblings, 1 reply; 11+ messages in thread From: Alexander Shishkin @ 2015-03-20 14:53 UTC (permalink / raw) To: Mathieu Poirier Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org, Pratik Patel, peter.lachner, norbert.schulz, keven.boell, yann.fouassier, laurent.fert, linux-api Mathieu Poirier <mathieu.poirier@linaro.org> writes: > As promised I worked on a prototype that connects the coresight-stm > driver with the generic STM interface you have suggested. Things work > quite well and aside from the enhancement related to the ioctl() and > private member as discussed above, we should move ahead with this. > > I will send out a new version of the coresight-stm driver as soon as I > see your patches with those changes. Actually, instread of a private member I'd simply pass struct stm_data pointer to the callback (like we do with other callbacks) and the private data would be in the structure that embeds this struct stm_data, so that you can get to it using container_of(): struct my_stm { struct stm_data data; void *my_stuff; ... }; ... long my_ioctl(struct stm_data *data, unsigned int cmd, unsigned long arg) { struct my_stm *mine = container_of(data, struct my_stm, data); ... Would this work for you? I'm otherwise ready to send the second version of my patchset. Regards, -- Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <87k2ybd8jl.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org>]
* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices [not found] ` <87k2ybd8jl.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org> @ 2015-03-20 15:19 ` Mathieu Poirier 0 siblings, 0 replies; 11+ messages in thread From: Mathieu Poirier @ 2015-03-20 15:19 UTC (permalink / raw) To: Alexander Shishkin Cc: Greg Kroah-Hartman, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Pratik Patel, peter.lachner-ral2JQCrhuEAvxtiuMwx3w, norbert.schulz-ral2JQCrhuEAvxtiuMwx3w, keven.boell-ral2JQCrhuEAvxtiuMwx3w, yann.fouassier-ral2JQCrhuEAvxtiuMwx3w, laurent.fert-ral2JQCrhuEAvxtiuMwx3w, linux-api-u79uwXL29TY76Z2rM5mHXA On 20 March 2015 at 08:53, Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote: > Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes: > >> As promised I worked on a prototype that connects the coresight-stm >> driver with the generic STM interface you have suggested. Things work >> quite well and aside from the enhancement related to the ioctl() and >> private member as discussed above, we should move ahead with this. >> >> I will send out a new version of the coresight-stm driver as soon as I >> see your patches with those changes. > > Actually, instread of a private member I'd simply pass struct stm_data > pointer to the callback (like we do with other callbacks) and the > private data would be in the structure that embeds this struct stm_data, > so that you can get to it using container_of(): > > struct my_stm { > struct stm_data data; > void *my_stuff; > ... > }; > > ... > > long my_ioctl(struct stm_data *data, unsigned int cmd, unsigned long > arg) > { > struct my_stm *mine = container_of(data, struct my_stm, data); > > ... > > Would this work for you? That should be just fine yes. > I'm otherwise ready to send the second version > of my patchset. Cool > > Regards, > -- > Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-03-20 15:19 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1425728161-164217-1-git-send-email-alexander.shishkin@linux.intel.com>
2015-03-07 11:35 ` [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices Alexander Shishkin
2015-03-07 22:26 ` Paul Bolle
2015-03-17 10:13 ` Alexander Shishkin
[not found] ` <1425728161-164217-2-git-send-email-alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2015-03-09 21:29 ` Mathieu Poirier
2015-03-17 10:37 ` Alexander Shishkin
2015-03-18 15:07 ` Mathieu Poirier
[not found] ` <CANLsYkx6+mtTf0gPhu37mfbL84oOj0CkW_yOMwFHh_XyqQ7pAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-03-19 14:23 ` Alexander Shishkin
[not found] ` <877fuddq2c.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org>
2015-03-19 14:39 ` Mathieu Poirier
[not found] ` <87mw3bewq8.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org>
2015-03-19 22:21 ` Mathieu Poirier
2015-03-20 14:53 ` Alexander Shishkin
[not found] ` <87k2ybd8jl.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org>
2015-03-20 15:19 ` Mathieu Poirier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).