diff mbox

[V3,6/6] coresight-stm: adding driver for CoreSight STM component

Message ID 1454756672-12790-7-git-send-email-zhang.chunyan@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Chunyan Zhang Feb. 6, 2016, 11:04 a.m. UTC
From: Pratik Patel <pratikp@codeaurora.org>

This driver adds support for the STM CoreSight IP block, allowing any
system compoment (HW or SW) to log and aggregate messages via a
single entity.

The CoreSight STM exposes an application defined number of channels
called stimulus port.  Configuration is done using entries in sysfs
and channels made available to userspace via configfs.

Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
 .../ABI/testing/sysfs-bus-coresight-devices-stm    |  53 ++
 Documentation/trace/coresight.txt                  |  37 +-
 drivers/hwtracing/coresight/Kconfig                |  11 +
 drivers/hwtracing/coresight/Makefile               |   1 +
 drivers/hwtracing/coresight/coresight-stm.c        | 928 +++++++++++++++++++++
 include/linux/coresight-stm.h                      |   6 +
 include/uapi/linux/coresight-stm.h                 |  12 +
 7 files changed, 1046 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
 create mode 100644 drivers/hwtracing/coresight/coresight-stm.c
 create mode 100644 include/linux/coresight-stm.h
 create mode 100644 include/uapi/linux/coresight-stm.h

Comments

Mathieu Poirier Feb. 11, 2016, 4:59 p.m. UTC | #1
On 6 February 2016 at 04:04, Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
> From: Pratik Patel <pratikp@codeaurora.org>
>
> This driver adds support for the STM CoreSight IP block, allowing any
> system compoment (HW or SW) to log and aggregate messages via a
> single entity.
>
> The CoreSight STM exposes an application defined number of channels
> called stimulus port.  Configuration is done using entries in sysfs
> and channels made available to userspace via configfs.
>
> Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
> ---
>  .../ABI/testing/sysfs-bus-coresight-devices-stm    |  53 ++
>  Documentation/trace/coresight.txt                  |  37 +-
>  drivers/hwtracing/coresight/Kconfig                |  11 +
>  drivers/hwtracing/coresight/Makefile               |   1 +
>  drivers/hwtracing/coresight/coresight-stm.c        | 928 +++++++++++++++++++++
>  include/linux/coresight-stm.h                      |   6 +
>  include/uapi/linux/coresight-stm.h                 |  12 +
>  7 files changed, 1046 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>  create mode 100644 drivers/hwtracing/coresight/coresight-stm.c
>  create mode 100644 include/linux/coresight-stm.h
>  create mode 100644 include/uapi/linux/coresight-stm.h
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
> new file mode 100644
> index 0000000..a1d7022
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
> @@ -0,0 +1,53 @@
> +What:          /sys/bus/coresight/devices/<memory_map>.stm/enable_source
> +Date:          February 2016
> +KernelVersion: 4.5
> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
> +Description:   (RW) Enable/disable tracing on this specific trace macrocell.
> +               Enabling the trace macrocell implies it has been configured
> +               properly and a sink has been identidifed for it.  The path
> +               of coresight components linking the source to the sink is
> +               configured and managed automatically by the coresight framework.
> +
> +What:          /sys/bus/coresight/devices/<memory_map>.stm/hwevent_enable
> +Date:          February 2016
> +KernelVersion: 4.5
> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
> +Description:   (RW) Provides access to the HW event enable register, used in
> +               conjunction with HW event bank select register.
> +
> +What:          /sys/bus/coresight/devices/<memory_map>.stm/hwevent_select
> +Date:          February 2016
> +KernelVersion: 4.5
> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
> +Description:   (RW) Gives access to the HW event block select register
> +               (STMHEBSR) in order to configure up to 256 channels.  Used in
> +               conjunction with "hwevent_enable" register as described above.
> +
> +What:          /sys/bus/coresight/devices/<memory_map>.stm/port_enable
> +Date:          February 2016
> +KernelVersion: 4.5
> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
> +Description:   (RW) Provides access to the stimlus port enable register
> +               (STMSPER).  Used in conjunction with "port_select" described
> +               below.
> +
> +What:          /sys/bus/coresight/devices/<memory_map>.stm/port_select
> +Date:          February 2016
> +KernelVersion: 4.5
> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
> +Description:   (RW) Used to determine which bank of stimulus port bit in
> +               register STMSPER (see above) apply to.
> +
> +What:          /sys/bus/coresight/devices/<memory_map>.stm/status
> +Date:          February 2016
> +KernelVersion: 4.5
> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
> +Description:   (R) List various control and status registers.  The specific
> +               layout and content is driver specific.
> +
> +What:          /sys/bus/coresight/devices/<memory_map>.stm/traceid
> +Date:          February 2016
> +KernelVersion: 4.5
> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
> +Description:   (RW) Holds the trace ID that will appear in the trace stream
> +               coming from this trace entity.
> diff --git a/Documentation/trace/coresight.txt b/Documentation/trace/coresight.txt
> index 0a5c329..a33c88c 100644
> --- a/Documentation/trace/coresight.txt
> +++ b/Documentation/trace/coresight.txt
> @@ -190,8 +190,8 @@ expected to be accessed and controlled using those entries.
>  Last but not least, "struct module *owner" is expected to be set to reflect
>  the information carried in "THIS_MODULE".
>
> -How to use
> -----------
> +How to use the tracer modules
> +-----------------------------
>
>  Before trace collection can start, a coresight sink needs to be identify.
>  There is no limit on the amount of sinks (nor sources) that can be enabled at
> @@ -297,3 +297,36 @@ Info                                    Tracing enabled
>  Instruction     13570831        0x8026B584      E28DD00C        false   ADD      sp,sp,#0xc
>  Instruction     0       0x8026B588      E8BD8000        true    LDM      sp!,{pc}
>  Timestamp                                       Timestamp: 17107041535
> +
> +How to use the STM module
> +-------------------------
> +
> +Using the System Trace Macrocell module is the same as the tracers - the only
> +difference is that clients are driving the trace capture rather
> +than the program flow through the code.
> +
> +As with any other CoreSight component, specifics about the STM tracer can be
> +found in sysfs with more information on each entry being found in [1]:
> +
> +root@genericarmv8:~# ls /sys/bus/coresight/devices/20100000.stm
> +enable_source   hwevent_select  port_enable     subsystem       uevent
> +hwevent_enable  mgmt            port_select     traceid
> +root@genericarmv8:~#
> +
> +Like any other source a sink needs to be identified and the STM enabled before
> +being used:
> +
> +root@genericarmv8:~# echo 1 > /sys/bus/coresight/devices/20010000.etf/enable_sink
> +root@genericarmv8:~# echo 1 > /sys/bus/coresight/devices/20100000.stm/enable_source
> +
> +From there user space applications can request and use channels using the devfs
> +interface provided for that purpose by the generic STM API:
> +
> +root@genericarmv8:~# ls -l /dev/20100000.stm
> +crw-------    1 root     root       10,  61 Jan  3 18:11 /dev/20100000.stm
> +root@genericarmv8:~#
> +
> +Details on how to use the generic STM API can be found here [2].
> +
> +[1]. Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
> +[2]. Documentation/trace/stm.txt
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index c85935f..f4a8bfa 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -77,4 +77,15 @@ config CORESIGHT_QCOM_REPLICATOR
>           programmable ATB replicator sends the ATB trace stream from the
>           ETB/ETF to the TPIUi and ETR.
>
> +config CORESIGHT_STM
> +       bool "CoreSight System Trace Macrocell driver"
> +       depends on (ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64
> +       select CORESIGHT_LINKS_AND_SINKS
> +       select STM
> +       help
> +         This driver provides support for hardware assisted software
> +         instrumentation based tracing. This is primarily used for
> +         logging useful software events or data coming from various entities
> +         in the system, possibly running different OSs
> +
>  endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 99f8e5f..1f6eaec 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \
>  obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o
>  obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o
>  obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
> +obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> new file mode 100644
> index 0000000..01b3521
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -0,0 +1,928 @@
> +/* Copyright (c) 2015-2016, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * Initial implementation by Pratik Patel
> + * (C) 2014-2015 Pratik Patel <pratikp@codeaurora.org>
> + *
> + * Serious refactoring, code cleanup and upgrading to the Coresight upstream
> + * framework by Mathieu Poirier
> + * (C) 2015-2016 Mathieu Poirier <mathieu.poirier@linaro.org>
> + *
> + * Guaranteed timing and support for various packet type coming from the
> + * generic STM API by Chunyan Zhang
> + * (C) 2015-2016 Chunyan Zhang <zhang.chunyan@linaro.org>
> + */
> +#include <linux/amba/bus.h>
> +#include <linux/bitmap.h>
> +#include <linux/clk.h>
> +#include <linux/coresight.h>
> +#include <linux/coresight-stm.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/stm.h>
> +
> +#include "coresight-priv.h"
> +
> +#define STMDMASTARTR                   0xc04
> +#define STMDMASTOPR                    0xc08
> +#define STMDMASTATR                    0xc0c
> +#define STMDMACTLR                     0xc10
> +#define STMDMAIDR                      0xcfc
> +#define STMHEER                                0xd00
> +#define STMHETER                       0xd20
> +#define STMHEBSR                       0xd60
> +#define STMHEMCR                       0xd64
> +#define STMHEMASTR                     0xdf4
> +#define STMHEFEAT1R                    0xdf8
> +#define STMHEIDR                       0xdfc
> +#define STMSPER                                0xe00
> +#define STMSPTER                       0xe20
> +#define STMPRIVMASKR                   0xe40
> +#define STMSPSCR                       0xe60
> +#define STMSPMSCR                      0xe64
> +#define STMSPOVERRIDER                 0xe68
> +#define STMSPMOVERRIDER                        0xe6c
> +#define STMSPTRIGCSR                   0xe70
> +#define STMTCSR                                0xe80
> +#define STMTSSTIMR                     0xe84
> +#define STMTSFREQR                     0xe8c
> +#define STMSYNCR                       0xe90
> +#define STMAUXCR                       0xe94
> +#define STMSPFEAT1R                    0xea0
> +#define STMSPFEAT2R                    0xea4
> +#define STMSPFEAT3R                    0xea8
> +#define STMITTRIGGER                   0xee8
> +#define STMITATBDATA0                  0xeec
> +#define STMITATBCTR2                   0xef0
> +#define STMITATBID                     0xef4
> +#define STMITATBCTR0                   0xef8
> +
> +#define STM_32_CHANNEL                 32
> +#define BYTES_PER_CHANNEL              256
> +#define STM_TRACE_BUF_SIZE             4096
> +#define STM_SW_MASTER_END              127
> +
> +/* Register bit definition */
> +#define STMTCSR_BUSY_BIT               23
> +/* Reserve the first 10 channels for kernel usage */
> +#define STM_CHANNEL_OFFSET             0
> +
> +enum stm_pkt_type {
> +       STM_PKT_TYPE_DATA       = 0x98,
> +       STM_PKT_TYPE_FLAG       = 0xE8,
> +       STM_PKT_TYPE_TRIG       = 0xF8,
> +};
> +
> +#define stm_channel_addr(drvdata, ch)  (drvdata->chs.base +    \
> +                                       (ch * BYTES_PER_CHANNEL))
> +#define stm_channel_off(type, opts)    (type & ~opts)
> +
> +static int boot_nr_channel;
> +
> +module_param_named(
> +       boot_nr_channel, boot_nr_channel, int, S_IRUGO
> +);
> +
> +/**
> + * struct channel_space - central management entity for extended ports
> + * @base:              memory mapped base address where channels start.
> + * @guaraneed:         is the channel delivery guaranteed.
> + */
> +struct channel_space {
> +       void __iomem            *base;
> +       unsigned long           *guaranteed;
> +};
> +
> +/**
> + * struct stm_drvdata - specifics associated to an STM component
> + * @base:              memory mapped base address for this component.
> + * @dev:               the device entity associated to this component.
> + * @atclk:             optional clock for the core parts of the STM.
> + * @csdev:             component vitals needed by the framework.
> + * @spinlock:          only one at a time pls.
> + * @chs:               the channels accociated to this STM.
> + * @stm:               structure associated to the generic STM interface.
> + * @enable:            this STM is being used.
> + * @traceid:           value of the current ID for this component.
> + * @write_max:         Maximus bytes this STM can write at a time.
> + * @write_64bit:       whether this STM supports 64 bit access.
> + * @stmsper:           settings for register STMSPER.
> + * @stmspscr:          settings for register STMSPSCR.
> + * @numsp:             the total number of stimulus port support by this STM.
> + * @stmheer:           settings for register STMHEER.
> + * @stmheter:          settings for register STMHETER.
> + * @stmhebsr:          settings for register STMHEBSR.
> + */
> +struct stm_drvdata {
> +       void __iomem            *base;
> +       struct device           *dev;
> +       struct clk              *atclk;
> +       struct coresight_device *csdev;
> +       spinlock_t              spinlock;
> +       struct channel_space    chs;
> +       struct stm_data         stm;
> +       bool                    enable;
> +       u8                      traceid;
> +       u8                      write_max;
> +       u32                     write_64bit;
> +       u32                     stmsper;
> +       u32                     stmspscr;
> +       u32                     numsp;
> +       u32                     stmheer;
> +       u32                     stmheter;
> +       u32                     stmhebsr;
> +};
> +
> +static void stm_hwevent_enable_hw(struct stm_drvdata *drvdata)
> +{
> +       CS_UNLOCK(drvdata->base);
> +
> +       writel_relaxed(drvdata->stmhebsr, drvdata->base + STMHEBSR);
> +       writel_relaxed(drvdata->stmheter, drvdata->base + STMHETER);
> +       writel_relaxed(drvdata->stmheer, drvdata->base + STMHEER);
> +       writel_relaxed(0x01 |   /* Enable HW event tracing */
> +                      0x04,    /* Error detection on event tracing */
> +                      drvdata->base + STMHEMCR);
> +
> +       CS_LOCK(drvdata->base);
> +}
> +
> +static void stm_port_enable_hw(struct stm_drvdata *drvdata)
> +{
> +       CS_UNLOCK(drvdata->base);
> +       /* ATB trigger enable on direct writes to TRIG locations */
> +       writel_relaxed(0x10,
> +                      drvdata->base + STMSPTRIGCSR);
> +       writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR);
> +       writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);
> +
> +       CS_LOCK(drvdata->base);
> +}
> +
> +static void stm_enable_hw(struct stm_drvdata *drvdata)
> +{
> +       if (drvdata->stmheer)
> +               stm_hwevent_enable_hw(drvdata);
> +
> +       stm_port_enable_hw(drvdata);
> +
> +       CS_UNLOCK(drvdata->base);
> +
> +       /* 4096 byte between synchronisation packets */
> +       writel_relaxed(0xFFF, drvdata->base + STMSYNCR);
> +       writel_relaxed((drvdata->traceid << 16 | /* trace id */
> +                       0x02 |                   /* timestamp enable */
> +                       0x01),                   /* global STM enable */
> +                       drvdata->base + STMTCSR);
> +
> +       CS_LOCK(drvdata->base);
> +}
> +
> +static int stm_enable(struct coresight_device *csdev)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +       pm_runtime_get_sync(drvdata->dev);
> +
> +       spin_lock(&drvdata->spinlock);
> +       stm_enable_hw(drvdata);
> +       drvdata->enable = true;
> +       spin_unlock(&drvdata->spinlock);
> +
> +       dev_info(drvdata->dev, "STM tracing enabled\n");
> +       return 0;
> +}
> +
> +static void stm_hwevent_disable_hw(struct stm_drvdata *drvdata)
> +{
> +       CS_UNLOCK(drvdata->base);
> +
> +       writel_relaxed(0x0, drvdata->base + STMHEMCR);
> +       writel_relaxed(0x0, drvdata->base + STMHEER);
> +       writel_relaxed(0x0, drvdata->base + STMHETER);
> +
> +       CS_LOCK(drvdata->base);
> +}
> +
> +static void stm_port_disable_hw(struct stm_drvdata *drvdata)
> +{
> +       CS_UNLOCK(drvdata->base);
> +
> +       writel_relaxed(0x0, drvdata->base + STMSPER);
> +       writel_relaxed(0x0, drvdata->base + STMSPTRIGCSR);
> +
> +       CS_LOCK(drvdata->base);
> +}
> +
> +static void stm_disable_hw(struct stm_drvdata *drvdata)
> +{
> +       u32 val;
> +
> +       CS_UNLOCK(drvdata->base);
> +
> +       val = readl_relaxed(drvdata->base + STMTCSR);
> +       val &= ~0x1; /* clear global STM enable [0] */
> +       writel_relaxed(val, drvdata->base + STMTCSR);
> +
> +       CS_LOCK(drvdata->base);
> +
> +       stm_port_disable_hw(drvdata);
> +       if (drvdata->stmheer)
> +               stm_hwevent_disable_hw(drvdata);
> +}
> +
> +static void stm_disable(struct coresight_device *csdev)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +       spin_lock(&drvdata->spinlock);
> +       stm_disable_hw(drvdata);
> +       drvdata->enable = false;
> +       spin_unlock(&drvdata->spinlock);
> +
> +       /* Wait until the engine has completely stopped */
> +       coresight_timeout(drvdata, STMTCSR, STMTCSR_BUSY_BIT, 0);
> +
> +       pm_runtime_put(drvdata->dev);
> +
> +       dev_info(drvdata->dev, "STM tracing disabled\n");
> +}
> +
> +static int stm_trace_id(struct coresight_device *csdev)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +       return drvdata->traceid;
> +}
> +
> +static const struct coresight_ops_source stm_source_ops = {
> +       .trace_id       = stm_trace_id,
> +       .enable         = stm_enable,
> +       .disable        = stm_disable,
> +};
> +
> +static const struct coresight_ops stm_cs_ops = {
> +       .source_ops     = &stm_source_ops,
> +};
> +
> +static inline bool stm_addr_unaligned(const void *addr, u8 max)
> +{
> +       return ((unsigned long)addr & (max - 1));
> +}
> +
> +static int stm_send(void *addr, const void *data, u32 size, u8 max)
> +{
> +       u32 len = size;
> +       u8 paload[8];
> +
> +       if (stm_addr_unaligned(data, max)) {
> +               memcpy(paload, data, size);
> +               data = paload;
> +       }
> +
> +       /* now we are 64bit/32bit aligned */
> +#ifdef CONFIG_64BIT
> +       if (size == 8)
> +               writeq_relaxed(*(u64 *)data, addr);
> +#endif

We probably don't need an #ifdef here.  Checking the size of the
transfer against the fundamental data size will result in the same
outcome, i.e preventing 64 bit transfers on a 32 bit architecture.  An
error (understandable by the caller) should probably be returned in
this case.

> +       if (size >= 4) {
> +               writel_relaxed(*(u32 *)data, addr);
> +               data += 4;
> +               size -= 4;
> +       }
> +
> +       if (size >= 2) {
> +               writew_relaxed(*(u16 *)data, addr);
> +               data += 2;
> +               size -= 2;
> +       }
> +
> +       if (size == 1)
> +               writeb_relaxed(*(u8 *)data, addr);
> +
> +       return len;
> +}
> +
> +static int stm_generic_link(struct stm_data *stm_data,
> +                           unsigned int master,  unsigned int channel)
> +{
> +       struct stm_drvdata *drvdata = container_of(stm_data,
> +                                                  struct stm_drvdata, stm);
> +       if (!drvdata || !drvdata->csdev)
> +               return -EINVAL;
> +
> +       return coresight_enable(drvdata->csdev);
> +}
> +
> +static void stm_generic_unlink(struct stm_data *stm_data,
> +                              unsigned int master,  unsigned int channel)
> +{
> +       struct stm_drvdata *drvdata = container_of(stm_data,
> +                                                  struct stm_drvdata, stm);
> +       if (!drvdata || !drvdata->csdev)
> +               return;
> +
> +       stm_disable(drvdata->csdev);
> +}
> +
> +static long stm_generic_set_options(struct stm_data *stm_data,
> +                                   unsigned int master,
> +                                   unsigned int channel,
> +                                   unsigned int nr_chans,
> +                                   unsigned long options)
> +{
> +       struct stm_drvdata *drvdata = container_of(stm_data,
> +                                                  struct stm_drvdata, stm);
> +       if (!(drvdata && drvdata->enable))
> +               return -EINVAL;
> +
> +       if (channel >= drvdata->numsp)
> +               return -EINVAL;
> +
> +       switch (options) {
> +       case STM_OPTION_GUARANTEED:
> +               set_bit(channel, drvdata->chs.guaranteed);
> +               break;
> +
> +       case STM_OPTION_INVARIANT:
> +               clear_bit(channel, drvdata->chs.guaranteed);
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static long stm_generic_get_options(struct stm_data *stm_data,
> +                                   unsigned int master,
> +                                   unsigned int channel,
> +                                   unsigned int nr_chans,
> +                                   u64 *options)
> +{
> +       struct stm_drvdata *drvdata = container_of(stm_data,
> +                                                  struct stm_drvdata, stm);
> +       if (!(drvdata && drvdata->enable))
> +               return -EINVAL;
> +
> +       if (channel >= drvdata->numsp)
> +               return -EINVAL;
> +
> +       switch (*options) {
> +       case STM_OPTION_GUARANTEED:
> +               *options = test_bit(channel, drvdata->chs.guaranteed);
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static ssize_t stm_generic_packet(struct stm_data *stm_data,
> +                                 unsigned int master,
> +                                 unsigned int channel,
> +                                 unsigned int packet,
> +                                 unsigned int flags,
> +                                 unsigned int size,
> +                                 const unsigned char *payload)
> +{
> +       unsigned long ch_addr;
> +       struct stm_drvdata *drvdata = container_of(stm_data,
> +                                                  struct stm_drvdata, stm);
> +
> +       if (!(drvdata && drvdata->enable))
> +               return 0;
> +
> +       if (channel >= drvdata->numsp)
> +               return 0;
> +
> +       ch_addr = (unsigned long)stm_channel_addr(drvdata, channel);
> +
> +       flags = (flags == STP_PACKET_TIMESTAMPED) ? STM_FLAG_TIMESTAMPED : 0;
> +       flags |= test_bit(channel, drvdata->chs.guaranteed) ?
> +                          STM_FLAG_GUARANTEED : 0;
> +
> +       if (size > drvdata->write_max)
> +               size = drvdata->write_max;
> +
> +       switch (packet) {
> +       case STP_PACKET_FLAG:
> +               ch_addr |= stm_channel_off(STM_PKT_TYPE_FLAG, flags);
> +
> +               /* the generic STM API set a size of zero on flag packets. */
> +               size = 1;
> +               break;
> +
> +       case STP_PACKET_DATA:
> +               ch_addr |= stm_channel_off(STM_PKT_TYPE_DATA, flags);
> +               break;
> +
> +       default:
> +               return 0;
> +       }
> +
> +       return stm_send((void *)ch_addr, payload, size, drvdata->write_max);
> +}
> +
> +static ssize_t hwevent_enable_show(struct device *dev,
> +                                  struct device_attribute *attr, char *buf)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       unsigned long val = drvdata->stmheer;
> +
> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +}
> +
> +static ssize_t hwevent_enable_store(struct device *dev,
> +                                   struct device_attribute *attr,
> +                                   const char *buf, size_t size)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       unsigned long val;
> +       int ret = 0;
> +
> +       ret = kstrtoul(buf, 16, &val);
> +       if (ret)
> +               return -EINVAL;
> +
> +       drvdata->stmheer = val;
> +       /* HW event enable and trigger go hand in hand */
> +       drvdata->stmheter = val;
> +
> +       return size;
> +}
> +static DEVICE_ATTR_RW(hwevent_enable);
> +
> +static ssize_t hwevent_select_show(struct device *dev,
> +                                  struct device_attribute *attr, char *buf)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       unsigned long val = drvdata->stmhebsr;
> +
> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +}
> +
> +static ssize_t hwevent_select_store(struct device *dev,
> +                                   struct device_attribute *attr,
> +                                   const char *buf, size_t size)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       unsigned long val;
> +       int ret = 0;
> +
> +       ret = kstrtoul(buf, 16, &val);
> +       if (ret)
> +               return -EINVAL;
> +
> +       drvdata->stmhebsr = val;
> +
> +       return size;
> +}
> +static DEVICE_ATTR_RW(hwevent_select);
> +
> +static ssize_t port_select_show(struct device *dev,
> +                               struct device_attribute *attr, char *buf)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       unsigned long val;
> +
> +       if (!drvdata->enable) {
> +               val = drvdata->stmspscr;
> +       } else {
> +               spin_lock(&drvdata->spinlock);
> +               val = readl_relaxed(drvdata->base + STMSPSCR);
> +               spin_unlock(&drvdata->spinlock);
> +       }
> +
> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +}
> +
> +static ssize_t port_select_store(struct device *dev,
> +                                struct device_attribute *attr,
> +                                const char *buf, size_t size)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       unsigned long val, stmsper;
> +       int ret = 0;
> +
> +       ret = kstrtoul(buf, 16, &val);
> +       if (ret)
> +               return ret;
> +
> +       spin_lock(&drvdata->spinlock);
> +       drvdata->stmspscr = val;
> +
> +       if (drvdata->enable) {
> +               CS_UNLOCK(drvdata->base);
> +               /* Process as per ARM's TRM recommendation */
> +               stmsper = readl_relaxed(drvdata->base + STMSPER);
> +               writel_relaxed(0x0, drvdata->base + STMSPER);
> +               writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR);
> +               writel_relaxed(stmsper, drvdata->base + STMSPER);
> +               CS_LOCK(drvdata->base);
> +       }
> +       spin_unlock(&drvdata->spinlock);
> +
> +       return size;
> +}
> +static DEVICE_ATTR_RW(port_select);
> +
> +static ssize_t port_enable_show(struct device *dev,
> +                               struct device_attribute *attr, char *buf)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       unsigned long val;
> +
> +       if (!drvdata->enable) {
> +               val = drvdata->stmsper;
> +       } else {
> +               spin_lock(&drvdata->spinlock);
> +               val = readl_relaxed(drvdata->base + STMSPER);
> +               spin_unlock(&drvdata->spinlock);
> +       }
> +
> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +}
> +
> +static ssize_t port_enable_store(struct device *dev,
> +                                struct device_attribute *attr,
> +                                const char *buf, size_t size)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       unsigned long val;
> +       int ret = 0;
> +
> +       ret = kstrtoul(buf, 16, &val);
> +       if (ret)
> +               return ret;
> +
> +       spin_lock(&drvdata->spinlock);
> +       drvdata->stmsper = val;
> +
> +       if (drvdata->enable) {
> +               CS_UNLOCK(drvdata->base);
> +               writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);
> +               CS_LOCK(drvdata->base);
> +       }
> +       spin_unlock(&drvdata->spinlock);
> +
> +       return size;
> +}
> +static DEVICE_ATTR_RW(port_enable);
> +
> +static ssize_t traceid_show(struct device *dev,
> +                           struct device_attribute *attr, char *buf)
> +{
> +       unsigned long val;
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +       val = drvdata->traceid;
> +       return sprintf(buf, "%#lx\n", val);
> +}
> +
> +static ssize_t traceid_store(struct device *dev,
> +                            struct device_attribute *attr,
> +                            const char *buf, size_t size)
> +{
> +       int ret;
> +       unsigned long val;
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +       ret = kstrtoul(buf, 16, &val);
> +       if (ret)
> +               return ret;
> +
> +       /* traceid field is 7bit wide on STM32 */
> +       drvdata->traceid = val & 0x7f;
> +       return size;
> +}
> +static DEVICE_ATTR_RW(traceid);
> +
> +#define coresight_simple_func(type, name, offset)                      \
> +static ssize_t name##_show(struct device *_dev,                                \
> +                          struct device_attribute *attr, char *buf)    \
> +{                                                                      \
> +       type *drvdata = dev_get_drvdata(_dev->parent);                  \
> +       return scnprintf(buf, PAGE_SIZE, "0x%08x\n",                    \
> +                        readl_relaxed(drvdata->base + offset));        \
> +}                                                                      \
> +DEVICE_ATTR_RO(name)
> +
> +#define coresight_stm_simple_func(name, offset)        \
> +       coresight_simple_func(struct stm_drvdata, name, offset)
> +
> +coresight_stm_simple_func(tcsr, STMTCSR);
> +coresight_stm_simple_func(tsfreqr, STMTSFREQR);
> +coresight_stm_simple_func(syncr, STMSYNCR);
> +coresight_stm_simple_func(sper, STMSPER);
> +coresight_stm_simple_func(spter, STMSPTER);
> +coresight_stm_simple_func(privmaskr, STMPRIVMASKR);
> +coresight_stm_simple_func(spscr, STMSPSCR);
> +coresight_stm_simple_func(spmscr, STMSPMSCR);
> +coresight_stm_simple_func(spfeat1r, STMSPFEAT1R);
> +coresight_stm_simple_func(spfeat2r, STMSPFEAT2R);
> +coresight_stm_simple_func(spfeat3r, STMSPFEAT3R);
> +coresight_stm_simple_func(devid, CORESIGHT_DEVID);
> +
> +static struct attribute *coresight_stm_attrs[] = {
> +       &dev_attr_hwevent_enable.attr,
> +       &dev_attr_hwevent_select.attr,
> +       &dev_attr_port_enable.attr,
> +       &dev_attr_port_select.attr,
> +       &dev_attr_traceid.attr,
> +       NULL,
> +};
> +
> +static struct attribute *coresight_stm_mgmt_attrs[] = {
> +       &dev_attr_tcsr.attr,
> +       &dev_attr_tsfreqr.attr,
> +       &dev_attr_syncr.attr,
> +       &dev_attr_sper.attr,
> +       &dev_attr_spter.attr,
> +       &dev_attr_privmaskr.attr,
> +       &dev_attr_spscr.attr,
> +       &dev_attr_spmscr.attr,
> +       &dev_attr_spfeat1r.attr,
> +       &dev_attr_spfeat2r.attr,
> +       &dev_attr_spfeat3r.attr,
> +       &dev_attr_devid.attr,
> +       NULL,
> +};
> +
> +static const struct attribute_group coresight_stm_group = {
> +       .attrs = coresight_stm_attrs,
> +};
> +
> +static const struct attribute_group coresight_stm_mgmt_group = {
> +       .attrs = coresight_stm_mgmt_attrs,
> +       .name = "mgmt",
> +};
> +
> +static const struct attribute_group *coresight_stm_groups[] = {
> +       &coresight_stm_group,
> +       &coresight_stm_mgmt_group,
> +       NULL,
> +};
> +
> +static int stm_get_resource_byname(struct device_node *np,
> +                                  char *ch_base, struct resource *res)
> +{
> +       const char *name = NULL;
> +       int index = 0, found = 0;
> +
> +       while (!of_property_read_string_index(np, "reg-names", index, &name)) {
> +               if (strcmp(ch_base, name)) {
> +                       index++;
> +                       continue;
> +               }
> +
> +               /* We have a match and @index is where it's at */
> +               found = 1;
> +               break;
> +       }
> +
> +       if (!found)
> +               return -EINVAL;
> +
> +       return of_address_to_resource(np, index, res);
> +}
> +
> +static u32 stm_fundamental_data_size(struct stm_drvdata *drvdata)
> +{
> +       u32 stmspfeat2r;
> +
> +       stmspfeat2r = readl_relaxed(drvdata->base + STMSPFEAT2R);
> +       return BMVAL(stmspfeat2r, 12, 15);
> +}
> +
> +static u32 stm_num_stimulus_port(struct stm_drvdata *drvdata)
> +{
> +       u32 numsp;
> +
> +       numsp = readl_relaxed(drvdata->base + CORESIGHT_DEVID);
> +       /*
> +        * NUMPS in STMDEVID is 17 bit long and if equal to 0x0,
> +        * 32 stimulus ports are supported.
> +        */
> +       numsp &= 0x1ffff;
> +       if (!numsp)
> +               numsp = STM_32_CHANNEL;
> +       return numsp;
> +}
> +
> +static void stm_init_default_data(struct stm_drvdata *drvdata)
> +{
> +       /* Don't use port selection */
> +       drvdata->stmspscr = 0x0;
> +       /*
> +        * Enable all channel regardless of their number.  When port
> +        * selection isn't used (see above) STMSPER applies to all
> +        * 32 channel group available, hence setting all 32 bits to 1
> +        */
> +       drvdata->stmsper = ~0x0;
> +
> +       /*
> +        * Select arbitrary value to start with.  If there is a conflict
> +        * with other tracers the framework will deal with it.
> +        */
> +       drvdata->traceid = 0x20;
> +
> +       /* Set invariant transaction timing on all channels */
> +       bitmap_clear(drvdata->chs.guaranteed, 0, drvdata->numsp);
> +}
> +
> +static void stm_init_generic_data(struct stm_drvdata *drvdata)
> +{
> +       drvdata->stm.name = dev_name(drvdata->dev);
> +       drvdata->stm.mstatic = true;
> +       drvdata->stm.sw_nchannels = drvdata->numsp;
> +       drvdata->stm.packet = stm_generic_packet;
> +       drvdata->stm.link = stm_generic_link;
> +       drvdata->stm.unlink = stm_generic_unlink;
> +       drvdata->stm.set_options = stm_generic_set_options;
> +       drvdata->stm.get_options = stm_generic_get_options;
> +}
> +
> +static int stm_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> +       int ret;
> +       void __iomem *base;
> +       unsigned long *guaranteed;
> +       struct device *dev = &adev->dev;
> +       struct coresight_platform_data *pdata = NULL;
> +       struct stm_drvdata *drvdata;
> +       struct resource *res = &adev->res;
> +       struct resource ch_res;
> +       size_t res_size, bitmap_size;
> +       struct coresight_desc *desc;
> +       struct device_node *np = adev->dev.of_node;
> +
> +       if (np) {
> +               pdata = of_get_coresight_platform_data(dev, np);
> +               if (IS_ERR(pdata))
> +                       return PTR_ERR(pdata);
> +               adev->dev.platform_data = pdata;
> +       }
> +       drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +       if (!drvdata)
> +               return -ENOMEM;
> +
> +       drvdata->dev = &adev->dev;
> +       drvdata->atclk = devm_clk_get(&adev->dev, "atclk"); /* optional */
> +       if (!IS_ERR(drvdata->atclk)) {
> +               ret = clk_prepare_enable(drvdata->atclk);
> +               if (ret)
> +                       return ret;
> +       }
> +       dev_set_drvdata(dev, drvdata);
> +
> +       base = devm_ioremap_resource(dev, res);
> +       if (IS_ERR(base))
> +               return PTR_ERR(base);
> +       drvdata->base = base;
> +
> +       ret = stm_get_resource_byname(np, "stm-stimulus-base", &ch_res);
> +       if (ret)
> +               return ret;
> +
> +       base = devm_ioremap_resource(dev, &ch_res);
> +       if (IS_ERR(base))
> +               return PTR_ERR(base);
> +       drvdata->chs.base = base;
> +
> +       drvdata->write_max = sizeof(u32);

Isn't the fundamental data size the maximum an architecture can stand?
 Why not simply use that rather than adding another variable?

> +       drvdata->write_64bit = stm_fundamental_data_size(drvdata);
> +
> +#ifndef CONFIG_64BIT
> +       if (drvdata->write_64bit)
> +               drvdata->write_max = sizeof(u64);
> +#endif

In most cases the fundamental data size will follow the architecture
size, i.e, 32 bit architecture with 32 bit fundamental data size, 64
bit architecture with 64 bit fundamental data size.  A fundamental
data size of 32 is also possible on a 64 bit architecture.

What needs to be prevented is a 32 bit architecture with a fundamental
data size of 64.  In those cases the fundamental data size should be
adjusted to be 32 bit.  To do that I suggest to modify
stm_fundamental_data_size() to probe the STM's fundamental data size
but to adjust it down if on a 32 bit system.  Knowing whether running
on a 32/64 bit system is easy when using the IS_ENABLED() macro.

> +
> +       if (boot_nr_channel) {
> +               drvdata->numsp = boot_nr_channel;
> +               res_size = min((resource_size_t)(boot_nr_channel *
> +                                 BYTES_PER_CHANNEL), resource_size(res));
> +       } else {
> +               drvdata->numsp = stm_num_stimulus_port(drvdata);
> +               res_size = min((resource_size_t)(drvdata->numsp *
> +                                BYTES_PER_CHANNEL), resource_size(res));
> +       }
> +       bitmap_size = BITS_TO_LONGS(drvdata->numsp) * sizeof(long);
> +
> +       guaranteed = devm_kzalloc(dev, bitmap_size, GFP_KERNEL);
> +       if (!guaranteed)
> +               return -ENOMEM;
> +       drvdata->chs.guaranteed = guaranteed;
> +
> +       spin_lock_init(&drvdata->spinlock);
> +
> +       stm_init_default_data(drvdata);
> +       stm_init_generic_data(drvdata);
> +
> +       if (stm_register_device(dev, &drvdata->stm, THIS_MODULE)) {
> +               dev_info(dev,
> +                        "stm_register_device failed, probing deffered\n");
> +               return -EPROBE_DEFER;
> +       }
> +
> +       desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
> +       if (!desc) {
> +               ret = -ENOMEM;
> +               goto stm_unregister;
> +       }
> +
> +       desc->type = CORESIGHT_DEV_TYPE_SOURCE;
> +       desc->subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE;
> +       desc->ops = &stm_cs_ops;
> +       desc->pdata = pdata;
> +       desc->dev = dev;
> +       desc->groups = coresight_stm_groups;
> +       drvdata->csdev = coresight_register(desc);
> +       if (IS_ERR(drvdata->csdev)) {
> +               ret = PTR_ERR(drvdata->csdev);
> +               goto stm_unregister;
> +       }
> +
> +       pm_runtime_put(&adev->dev);
> +
> +       dev_info(dev, "%s initialized\n", (char *)id->data);
> +       return 0;
> +
> +stm_unregister:
> +       stm_unregister_device(&drvdata->stm);
> +       return ret;
> +}
> +
> +static int stm_remove(struct amba_device *adev)
> +{
> +       struct stm_drvdata *drvdata = amba_get_drvdata(adev);
> +
> +       stm_unregister_device(&drvdata->stm);
> +       coresight_unregister(drvdata->csdev);
> +       return 0;
> +}

The xyz_remove() functions is not needed for coresight devices -
unbinding a device from its driver will no longer be possible in the
4.6 cycle.

> +
> +#ifdef CONFIG_PM
> +static int stm_runtime_suspend(struct device *dev)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev);
> +
> +       if (drvdata && !IS_ERR(drvdata->atclk))
> +               clk_disable_unprepare(drvdata->atclk);
> +
> +       return 0;
> +}
> +
> +static int stm_runtime_resume(struct device *dev)
> +{
> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev);
> +
> +       if (drvdata && !IS_ERR(drvdata->atclk))
> +               clk_prepare_enable(drvdata->atclk);
> +
> +       return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops stm_dev_pm_ops = {
> +       SET_RUNTIME_PM_OPS(stm_runtime_suspend, stm_runtime_resume, NULL)
> +};
> +
> +static struct amba_id stm_ids[] = {
> +       {
> +               .id     = 0x0003b962,
> +               .mask   = 0x0003ffff,
> +               .data   = "STM32",
> +       },
> +       { 0, 0},
> +};
> +
> +static struct amba_driver stm_driver = {
> +       .drv = {
> +               .name   = "coresight-stm",
> +               .owner  = THIS_MODULE,
> +               .pm     = &stm_dev_pm_ops,
> +       },
> +       .probe          = stm_probe,
> +       .remove         = stm_remove,
> +       .id_table       = stm_ids,
> +};
> +
> +module_amba_driver(stm_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("CoreSight System Trace Macrocell driver");
> diff --git a/include/linux/coresight-stm.h b/include/linux/coresight-stm.h
> new file mode 100644
> index 0000000..a978bb8
> --- /dev/null
> +++ b/include/linux/coresight-stm.h
> @@ -0,0 +1,6 @@
> +#ifndef __LINUX_CORESIGHT_STM_H_
> +#define __LINUX_CORESIGHT_STM_H_
> +
> +#include <uapi/linux/coresight-stm.h>
> +
> +#endif
> diff --git a/include/uapi/linux/coresight-stm.h b/include/uapi/linux/coresight-stm.h
> new file mode 100644
> index 0000000..fa35f0b
> --- /dev/null
> +++ b/include/uapi/linux/coresight-stm.h
> @@ -0,0 +1,12 @@
> +#ifndef __UAPI_CORESIGHT_STM_H_
> +#define __UAPI_CORESIGHT_STM_H_
> +
> +#define STM_FLAG_TIMESTAMPED   BIT(3)
> +#define STM_FLAG_GUARANTEED    BIT(7)
> +
> +enum {
> +       STM_OPTION_GUARANTEED = 0,
> +       STM_OPTION_INVARIANT,
> +};
> +
> +#endif
> --
> 1.9.1
>
Michael Williams (ATG) Feb. 12, 2016, 2:55 p.m. UTC | #2
Mathieu Poirier [mailto:mathieu.poirier@linaro.org] wrote:
> On 6 February 2016 at 04:04, Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
>> From: Pratik Patel <pratikp@codeaurora.org>
>>
>> This driver adds support for the STM CoreSight IP block, allowing any
>> system compoment (HW or SW) to log and aggregate messages via a
>> single entity.
>>
>> The CoreSight STM exposes an application defined number of channels
>> called stimulus port.  Configuration is done using entries in sysfs
>> and channels made available to userspace via configfs.
>>
>> Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>> ---
>>  .../ABI/testing/sysfs-bus-coresight-devices-stm    |  53 ++
>>  Documentation/trace/coresight.txt                  |  37 +-
>>  drivers/hwtracing/coresight/Kconfig                |  11 +
>>  drivers/hwtracing/coresight/Makefile               |   1 +
>>  drivers/hwtracing/coresight/coresight-stm.c        | 928 +++++++++++++++++++++
>>  include/linux/coresight-stm.h                      |   6 +
>>  include/uapi/linux/coresight-stm.h                 |  12 +
>>  7 files changed, 1046 insertions(+), 2 deletions(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>>  create mode 100644 drivers/hwtracing/coresight/coresight-stm.c
>>  create mode 100644 include/linux/coresight-stm.h
>>  create mode 100644 include/uapi/linux/coresight-stm.h
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>> new file mode 100644
>> index 0000000..a1d7022
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>> @@ -0,0 +1,53 @@
>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/enable_source
>> +Date:          February 2016
>> +KernelVersion: 4.5
>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>> +Description:   (RW) Enable/disable tracing on this specific trace macrocell.
>> +               Enabling the trace macrocell implies it has been configured
>> +               properly and a sink has been identidifed for it.  The path
>> +               of coresight components linking the source to the sink is
>> +               configured and managed automatically by the coresight framework.
>> +
>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/hwevent_enable
>> +Date:          February 2016
>> +KernelVersion: 4.5
>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>> +Description:   (RW) Provides access to the HW event enable register, used in
>> +               conjunction with HW event bank select register.
>> +
>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/hwevent_select
>> +Date:          February 2016
>> +KernelVersion: 4.5
>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>> +Description:   (RW) Gives access to the HW event block select register
>> +               (STMHEBSR) in order to configure up to 256 channels.  Used in
>> +               conjunction with "hwevent_enable" register as described above.
>> +
>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/port_enable
>> +Date:          February 2016
>> +KernelVersion: 4.5
>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>> +Description:   (RW) Provides access to the stimlus port enable register
>> +               (STMSPER).  Used in conjunction with "port_select" described
>> +               below.
>> +
>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/port_select
>> +Date:          February 2016
>> +KernelVersion: 4.5
>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>> +Description:   (RW) Used to determine which bank of stimulus port bit in
>> +               register STMSPER (see above) apply to.
>> +
>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/status
>> +Date:          February 2016
>> +KernelVersion: 4.5
>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>> +Description:   (R) List various control and status registers.  The specific
>> +               layout and content is driver specific.
>> +
>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/traceid
>> +Date:          February 2016
>> +KernelVersion: 4.5
>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>> +Description:   (RW) Holds the trace ID that will appear in the trace stream
>> +               coming from this trace entity.
>> diff --git a/Documentation/trace/coresight.txt b/Documentation/trace/coresight.txt
>> index 0a5c329..a33c88c 100644
>> --- a/Documentation/trace/coresight.txt
>> +++ b/Documentation/trace/coresight.txt
>> @@ -190,8 +190,8 @@ expected to be accessed and controlled using those entries.
>>  Last but not least, "struct module *owner" is expected to be set to reflect
>>  the information carried in "THIS_MODULE".
>>
>> -How to use
>> -----------
>> +How to use the tracer modules
>> +-----------------------------
>>
>>  Before trace collection can start, a coresight sink needs to be identify.
>>  There is no limit on the amount of sinks (nor sources) that can be enabled at
>> @@ -297,3 +297,36 @@ Info                                    Tracing enabled
>>  Instruction     13570831        0x8026B584      E28DD00C        false   ADD
> sp,sp,#0xc
>>  Instruction     0       0x8026B588      E8BD8000        true    LDM      sp!,{pc}
>>  Timestamp                                       Timestamp: 17107041535
>> +
>> +How to use the STM module
>> +-------------------------
>> +
>> +Using the System Trace Macrocell module is the same as the tracers - the only
>> +difference is that clients are driving the trace capture rather
>> +than the program flow through the code.
>> +
>> +As with any other CoreSight component, specifics about the STM tracer can be
>> +found in sysfs with more information on each entry being found in [1]:
>> +
>> +root@genericarmv8:~# ls /sys/bus/coresight/devices/20100000.stm
>> +enable_source   hwevent_select  port_enable     subsystem       uevent
>> +hwevent_enable  mgmt            port_select     traceid
>> +root@genericarmv8:~#
>> +
>> +Like any other source a sink needs to be identified and the STM enabled before
>> +being used:
>> +
>> +root@genericarmv8:~# echo 1 > /sys/bus/coresight/devices/20010000.etf/enable_sink
>> +root@genericarmv8:~# echo 1 > /sys/bus/coresight/devices/20100000.stm/enable_source
>> +
>> +From there user space applications can request and use channels using the devfs
>> +interface provided for that purpose by the generic STM API:
>> +
>> +root@genericarmv8:~# ls -l /dev/20100000.stm
>> +crw-------    1 root     root       10,  61 Jan  3 18:11 /dev/20100000.stm
>> +root@genericarmv8:~#
>> +
>> +Details on how to use the generic STM API can be found here [2].
>> +
>> +[1]. Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>> +[2]. Documentation/trace/stm.txt
>> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
>> index c85935f..f4a8bfa 100644
>> --- a/drivers/hwtracing/coresight/Kconfig
>> +++ b/drivers/hwtracing/coresight/Kconfig
>> @@ -77,4 +77,15 @@ config CORESIGHT_QCOM_REPLICATOR
>>           programmable ATB replicator sends the ATB trace stream from the
>>           ETB/ETF to the TPIUi and ETR.
>>
>> +config CORESIGHT_STM
>> +       bool "CoreSight System Trace Macrocell driver"
>> +       depends on (ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64
>> +       select CORESIGHT_LINKS_AND_SINKS
>> +       select STM
>> +       help
>> +         This driver provides support for hardware assisted software
>> +         instrumentation based tracing. This is primarily used for
>> +         logging useful software events or data coming from various entities
>> +         in the system, possibly running different OSs
>> +
>>  endif
>> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
>> index 99f8e5f..1f6eaec 100644
>> --- a/drivers/hwtracing/coresight/Makefile
>> +++ b/drivers/hwtracing/coresight/Makefile
>> @@ -11,3 +11,4 @@ obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \
>>  obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o
>>  obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o
>>  obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
>> +obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
>> diff --git a/drivers/hwtracing/coresight/coresight-stm.c
> b/drivers/hwtracing/coresight/coresight-stm.c
>> new file mode 100644
>> index 0000000..01b3521
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-stm.c
>> @@ -0,0 +1,928 @@
>> +/* Copyright (c) 2015-2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that 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.
>> + *
>> + * Initial implementation by Pratik Patel
>> + * (C) 2014-2015 Pratik Patel <pratikp@codeaurora.org>
>> + *
>> + * Serious refactoring, code cleanup and upgrading to the Coresight upstream
>> + * framework by Mathieu Poirier
>> + * (C) 2015-2016 Mathieu Poirier <mathieu.poirier@linaro.org>
>> + *
>> + * Guaranteed timing and support for various packet type coming from the
>> + * generic STM API by Chunyan Zhang
>> + * (C) 2015-2016 Chunyan Zhang <zhang.chunyan@linaro.org>
>> + */
>> +#include <linux/amba/bus.h>
>> +#include <linux/bitmap.h>
>> +#include <linux/clk.h>
>> +#include <linux/coresight.h>
>> +#include <linux/coresight-stm.h>
>> +#include <linux/err.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/stm.h>
>> +
>> +#include "coresight-priv.h"
>> +
>> +#define STMDMASTARTR                   0xc04
>> +#define STMDMASTOPR                    0xc08
>> +#define STMDMASTATR                    0xc0c
>> +#define STMDMACTLR                     0xc10
>> +#define STMDMAIDR                      0xcfc
>> +#define STMHEER                                0xd00
>> +#define STMHETER                       0xd20
>> +#define STMHEBSR                       0xd60
>> +#define STMHEMCR                       0xd64
>> +#define STMHEMASTR                     0xdf4
>> +#define STMHEFEAT1R                    0xdf8
>> +#define STMHEIDR                       0xdfc
>> +#define STMSPER                                0xe00
>> +#define STMSPTER                       0xe20
>> +#define STMPRIVMASKR                   0xe40
>> +#define STMSPSCR                       0xe60
>> +#define STMSPMSCR                      0xe64
>> +#define STMSPOVERRIDER                 0xe68
>> +#define STMSPMOVERRIDER                        0xe6c
>> +#define STMSPTRIGCSR                   0xe70
>> +#define STMTCSR                                0xe80
>> +#define STMTSSTIMR                     0xe84
>> +#define STMTSFREQR                     0xe8c
>> +#define STMSYNCR                       0xe90
>> +#define STMAUXCR                       0xe94
>> +#define STMSPFEAT1R                    0xea0
>> +#define STMSPFEAT2R                    0xea4
>> +#define STMSPFEAT3R                    0xea8
>> +#define STMITTRIGGER                   0xee8
>> +#define STMITATBDATA0                  0xeec
>> +#define STMITATBCTR2                   0xef0
>> +#define STMITATBID                     0xef4
>> +#define STMITATBCTR0                   0xef8
>> +
>> +#define STM_32_CHANNEL                 32
>> +#define BYTES_PER_CHANNEL              256
>> +#define STM_TRACE_BUF_SIZE             4096
>> +#define STM_SW_MASTER_END              127
>> +
>> +/* Register bit definition */
>> +#define STMTCSR_BUSY_BIT               23
>> +/* Reserve the first 10 channels for kernel usage */
>> +#define STM_CHANNEL_OFFSET             0
>> +
>> +enum stm_pkt_type {
>> +       STM_PKT_TYPE_DATA       = 0x98,
>> +       STM_PKT_TYPE_FLAG       = 0xE8,
>> +       STM_PKT_TYPE_TRIG       = 0xF8,
>> +};
>> +
>> +#define stm_channel_addr(drvdata, ch)  (drvdata->chs.base +    \
>> +                                       (ch * BYTES_PER_CHANNEL))
>> +#define stm_channel_off(type, opts)    (type & ~opts)
>> +
>> +static int boot_nr_channel;
>> +
>> +module_param_named(
>> +       boot_nr_channel, boot_nr_channel, int, S_IRUGO
>> +);
>> +
>> +/**
>> + * struct channel_space - central management entity for extended ports
>> + * @base:              memory mapped base address where channels start.
>> + * @guaraneed:         is the channel delivery guaranteed.
>> + */
>> +struct channel_space {
>> +       void __iomem            *base;
>> +       unsigned long           *guaranteed;
>> +};
>> +
>> +/**
>> + * struct stm_drvdata - specifics associated to an STM component
>> + * @base:              memory mapped base address for this component.
>> + * @dev:               the device entity associated to this component.
>> + * @atclk:             optional clock for the core parts of the STM.
>> + * @csdev:             component vitals needed by the framework.
>> + * @spinlock:          only one at a time pls.
>> + * @chs:               the channels accociated to this STM.
>> + * @stm:               structure associated to the generic STM interface.
>> + * @enable:            this STM is being used.
>> + * @traceid:           value of the current ID for this component.
>> + * @write_max:         Maximus bytes this STM can write at a time.
>> + * @write_64bit:       whether this STM supports 64 bit access.
>> + * @stmsper:           settings for register STMSPER.
>> + * @stmspscr:          settings for register STMSPSCR.
>> + * @numsp:             the total number of stimulus port support by this STM.
>> + * @stmheer:           settings for register STMHEER.
>> + * @stmheter:          settings for register STMHETER.
>> + * @stmhebsr:          settings for register STMHEBSR.
>> + */
>> +struct stm_drvdata {
>> +       void __iomem            *base;
>> +       struct device           *dev;
>> +       struct clk              *atclk;
>> +       struct coresight_device *csdev;
>> +       spinlock_t              spinlock;
>> +       struct channel_space    chs;
>> +       struct stm_data         stm;
>> +       bool                    enable;
>> +       u8                      traceid;
>> +       u8                      write_max;
>> +       u32                     write_64bit;
>> +       u32                     stmsper;
>> +       u32                     stmspscr;
>> +       u32                     numsp;
>> +       u32                     stmheer;
>> +       u32                     stmheter;
>> +       u32                     stmhebsr;
>> +};
>> +
>> +static void stm_hwevent_enable_hw(struct stm_drvdata *drvdata)
>> +{
>> +       CS_UNLOCK(drvdata->base);
>> +
>> +       writel_relaxed(drvdata->stmhebsr, drvdata->base + STMHEBSR);
>> +       writel_relaxed(drvdata->stmheter, drvdata->base + STMHETER);
>> +       writel_relaxed(drvdata->stmheer, drvdata->base + STMHEER);
>> +       writel_relaxed(0x01 |   /* Enable HW event tracing */
>> +                      0x04,    /* Error detection on event tracing */
>> +                      drvdata->base + STMHEMCR);
>> +
>> +       CS_LOCK(drvdata->base);
>> +}
>> +
>> +static void stm_port_enable_hw(struct stm_drvdata *drvdata)
>> +{
>> +       CS_UNLOCK(drvdata->base);
>> +       /* ATB trigger enable on direct writes to TRIG locations */
>> +       writel_relaxed(0x10,
>> +                      drvdata->base + STMSPTRIGCSR);
>> +       writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR);
>> +       writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);
>> +
>> +       CS_LOCK(drvdata->base);
>> +}
>> +
>> +static void stm_enable_hw(struct stm_drvdata *drvdata)
>> +{
>> +       if (drvdata->stmheer)
>> +               stm_hwevent_enable_hw(drvdata);
>> +
>> +       stm_port_enable_hw(drvdata);
>> +
>> +       CS_UNLOCK(drvdata->base);
>> +
>> +       /* 4096 byte between synchronisation packets */
>> +       writel_relaxed(0xFFF, drvdata->base + STMSYNCR);
>> +       writel_relaxed((drvdata->traceid << 16 | /* trace id */
>> +                       0x02 |                   /* timestamp enable */
>> +                       0x01),                   /* global STM enable */
>> +                       drvdata->base + STMTCSR);
>> +
>> +       CS_LOCK(drvdata->base);
>> +}
>> +
>> +static int stm_enable(struct coresight_device *csdev)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +       pm_runtime_get_sync(drvdata->dev);
>> +
>> +       spin_lock(&drvdata->spinlock);
>> +       stm_enable_hw(drvdata);
>> +       drvdata->enable = true;
>> +       spin_unlock(&drvdata->spinlock);
>> +
>> +       dev_info(drvdata->dev, "STM tracing enabled\n");
>> +       return 0;
>> +}
>> +
>> +static void stm_hwevent_disable_hw(struct stm_drvdata *drvdata)
>> +{
>> +       CS_UNLOCK(drvdata->base);
>> +
>> +       writel_relaxed(0x0, drvdata->base + STMHEMCR);
>> +       writel_relaxed(0x0, drvdata->base + STMHEER);
>> +       writel_relaxed(0x0, drvdata->base + STMHETER);
>> +
>> +       CS_LOCK(drvdata->base);
>> +}
>> +
>> +static void stm_port_disable_hw(struct stm_drvdata *drvdata)
>> +{
>> +       CS_UNLOCK(drvdata->base);
>> +
>> +       writel_relaxed(0x0, drvdata->base + STMSPER);
>> +       writel_relaxed(0x0, drvdata->base + STMSPTRIGCSR);
>> +
>> +       CS_LOCK(drvdata->base);
>> +}
>> +
>> +static void stm_disable_hw(struct stm_drvdata *drvdata)
>> +{
>> +       u32 val;
>> +
>> +       CS_UNLOCK(drvdata->base);
>> +
>> +       val = readl_relaxed(drvdata->base + STMTCSR);
>> +       val &= ~0x1; /* clear global STM enable [0] */
>> +       writel_relaxed(val, drvdata->base + STMTCSR);
>> +
>> +       CS_LOCK(drvdata->base);
>> +
>> +       stm_port_disable_hw(drvdata);
>> +       if (drvdata->stmheer)
>> +               stm_hwevent_disable_hw(drvdata);
>> +}
>> +
>> +static void stm_disable(struct coresight_device *csdev)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +       spin_lock(&drvdata->spinlock);
>> +       stm_disable_hw(drvdata);
>> +       drvdata->enable = false;
>> +       spin_unlock(&drvdata->spinlock);
>> +
>> +       /* Wait until the engine has completely stopped */
>> +       coresight_timeout(drvdata, STMTCSR, STMTCSR_BUSY_BIT, 0);
>> +
>> +       pm_runtime_put(drvdata->dev);
>> +
>> +       dev_info(drvdata->dev, "STM tracing disabled\n");
>> +}
>> +
>> +static int stm_trace_id(struct coresight_device *csdev)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +       return drvdata->traceid;
>> +}
>> +
>> +static const struct coresight_ops_source stm_source_ops = {
>> +       .trace_id       = stm_trace_id,
>> +       .enable         = stm_enable,
>> +       .disable        = stm_disable,
>> +};
>> +
>> +static const struct coresight_ops stm_cs_ops = {
>> +       .source_ops     = &stm_source_ops,
>> +};
>> +
>> +static inline bool stm_addr_unaligned(const void *addr, u8 max)
>> +{
>> +       return ((unsigned long)addr & (max - 1));
>> +}
>> +
>> +static int stm_send(void *addr, const void *data, u32 size, u8 max)
>> +{
>> +       u32 len = size;
>> +       u8 paload[8];
>> +
>> +       if (stm_addr_unaligned(data, max)) {
>> +               memcpy(paload, data, size);
>> +               data = paload;
>> +       }
>> +
>> +       /* now we are 64bit/32bit aligned */
>> +#ifdef CONFIG_64BIT
>> +       if (size == 8)
>> +               writeq_relaxed(*(u64 *)data, addr);
>> +#endif
>
> We probably don't need an #ifdef here.  Checking the size of the
> transfer against the fundamental data size will result in the same
> outcome, i.e preventing 64 bit transfers on a 32 bit architecture.  An
> error (understandable by the caller) should probably be returned in
> this case.

In theory this is correct, but if the code is unreachable for non-32-bit architectures, why include it at all?

>> +       if (size >= 4) {
>> +               writel_relaxed(*(u32 *)data, addr);
>> +               data += 4;
>> +               size -= 4;
>> +       }
>> +
>> +       if (size >= 2) {
>> +               writew_relaxed(*(u16 *)data, addr);
>> +               data += 2;
>> +               size -= 2;
>> +       }
>> +
>> +       if (size == 1)
>> +               writeb_relaxed(*(u8 *)data, addr);
>> +
>> +       return len;
>> +}

The API for stm_data::packet (include/linux/stm.h) is not wholly clearly defined, but it does state "sends an STP packet," which I interpret as meaning exactly one packet and no more. This function (which is called from this module's implementation of stm_data::packet) will send multiple STP packets if called with a value that is not exactly 8, 4, 2, or 1.

To send only one packet, either:

* These "if" statements are replaced with "else if" (as appropriate), and the function returns the size of data consumed; or
* The stm_generic_packet() function (below) forces "size" to a power-of-two (<= write_max) before calling stm_send().

>> +static int stm_generic_link(struct stm_data *stm_data,
>> +                           unsigned int master,  unsigned int channel)
>> +{
>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                  struct stm_drvdata, stm);
>> +       if (!drvdata || !drvdata->csdev)
>> +               return -EINVAL;
>> +
>> +       return coresight_enable(drvdata->csdev);
>> +}
>> +
>> +static void stm_generic_unlink(struct stm_data *stm_data,
>> +                              unsigned int master,  unsigned int channel)
>> +{
>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                  struct stm_drvdata, stm);
>> +       if (!drvdata || !drvdata->csdev)
>> +               return;
>> +
>> +       stm_disable(drvdata->csdev);
>> +}
>> +
>> +static long stm_generic_set_options(struct stm_data *stm_data,
>> +                                   unsigned int master,
>> +                                   unsigned int channel,
>> +                                   unsigned int nr_chans,
>> +                                   unsigned long options)
>> +{
>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                  struct stm_drvdata, stm);
>> +       if (!(drvdata && drvdata->enable))
>> +               return -EINVAL;
>> +
>> +       if (channel >= drvdata->numsp)
>> +               return -EINVAL;
>> +
>> +       switch (options) {
>> +       case STM_OPTION_GUARANTEED:
>> +               set_bit(channel, drvdata->chs.guaranteed);
>> +               break;
>> +
>> +       case STM_OPTION_INVARIANT:
>> +               clear_bit(channel, drvdata->chs.guaranteed);
>> +               break;
>> +
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static long stm_generic_get_options(struct stm_data *stm_data,
>> +                                   unsigned int master,
>> +                                   unsigned int channel,
>> +                                   unsigned int nr_chans,
>> +                                   u64 *options)
>> +{
>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                  struct stm_drvdata, stm);
>> +       if (!(drvdata && drvdata->enable))
>> +               return -EINVAL;
>> +
>> +       if (channel >= drvdata->numsp)
>> +               return -EINVAL;
>> +
>> +       switch (*options) {
>> +       case STM_OPTION_GUARANTEED:
>> +               *options = test_bit(channel, drvdata->chs.guaranteed);
>> +               break;
>> +
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static ssize_t stm_generic_packet(struct stm_data *stm_data,
>> +                                 unsigned int master,
>> +                                 unsigned int channel,
>> +                                 unsigned int packet,
>> +                                 unsigned int flags,
>> +                                 unsigned int size,
>> +                                 const unsigned char *payload)
>> +{
>> +       unsigned long ch_addr;
>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                  struct stm_drvdata, stm);
>> +
>> +       if (!(drvdata && drvdata->enable))
>> +               return 0;
>> +
>> +       if (channel >= drvdata->numsp)
>> +               return 0;
>> +
>> +       ch_addr = (unsigned long)stm_channel_addr(drvdata, channel);
>> +
>> +       flags = (flags == STP_PACKET_TIMESTAMPED) ? STM_FLAG_TIMESTAMPED : 0;
>> +       flags |= test_bit(channel, drvdata->chs.guaranteed) ?
>> +                          STM_FLAG_GUARANTEED : 0;
>> +
>> +       if (size > drvdata->write_max)
>> +               size = drvdata->write_max;
>> +
>> +       switch (packet) {
>> +       case STP_PACKET_FLAG:
>> +               ch_addr |= stm_channel_off(STM_PKT_TYPE_FLAG, flags);
>> +
>> +               /* the generic STM API set a size of zero on flag packets. */
>> +               size = 1;
>> +               break;
>> +
>> +       case STP_PACKET_DATA:
>> +               ch_addr |= stm_channel_off(STM_PKT_TYPE_DATA, flags);
>> +               break;
>> +
>> +       default:
>> +               return 0;
>> +       }
>> +
>> +       return stm_send((void *)ch_addr, payload, size, drvdata->write_max);
>> +}
>> +
>> +static ssize_t hwevent_enable_show(struct device *dev,
>> +                                  struct device_attribute *attr, char *buf)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val = drvdata->stmheer;
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>> +}
>> +
>> +static ssize_t hwevent_enable_store(struct device *dev,
>> +                                   struct device_attribute *attr,
>> +                                   const char *buf, size_t size)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val;
>> +       int ret = 0;
>> +
>> +       ret = kstrtoul(buf, 16, &val);
>> +       if (ret)
>> +               return -EINVAL;
>> +
>> +       drvdata->stmheer = val;
>> +       /* HW event enable and trigger go hand in hand */
>> +       drvdata->stmheter = val;
>> +
>> +       return size;
>> +}
>> +static DEVICE_ATTR_RW(hwevent_enable);
>> +
>> +static ssize_t hwevent_select_show(struct device *dev,
>> +                                  struct device_attribute *attr, char *buf)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val = drvdata->stmhebsr;
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>> +}
>> +
>> +static ssize_t hwevent_select_store(struct device *dev,
>> +                                   struct device_attribute *attr,
>> +                                   const char *buf, size_t size)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val;
>> +       int ret = 0;
>> +
>> +       ret = kstrtoul(buf, 16, &val);
>> +       if (ret)
>> +               return -EINVAL;
>> +
>> +       drvdata->stmhebsr = val;
>> +
>> +       return size;
>> +}
>> +static DEVICE_ATTR_RW(hwevent_select);
>> +
>> +static ssize_t port_select_show(struct device *dev,
>> +                               struct device_attribute *attr, char *buf)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val;
>> +
>> +       if (!drvdata->enable) {
>> +               val = drvdata->stmspscr;
>> +       } else {
>> +               spin_lock(&drvdata->spinlock);
>> +               val = readl_relaxed(drvdata->base + STMSPSCR);
>> +               spin_unlock(&drvdata->spinlock);
>> +       }
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>> +}
>> +
>> +static ssize_t port_select_store(struct device *dev,
>> +                                struct device_attribute *attr,
>> +                                const char *buf, size_t size)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val, stmsper;
>> +       int ret = 0;
>> +
>> +       ret = kstrtoul(buf, 16, &val);
>> +       if (ret)
>> +               return ret;
>> +
>> +       spin_lock(&drvdata->spinlock);
>> +       drvdata->stmspscr = val;
>> +
>> +       if (drvdata->enable) {
>> +               CS_UNLOCK(drvdata->base);
>> +               /* Process as per ARM's TRM recommendation */
>> +               stmsper = readl_relaxed(drvdata->base + STMSPER);
>> +               writel_relaxed(0x0, drvdata->base + STMSPER);
>> +               writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR);
>> +               writel_relaxed(stmsper, drvdata->base + STMSPER);
>> +               CS_LOCK(drvdata->base);
>> +       }
>> +       spin_unlock(&drvdata->spinlock);
>> +
>> +       return size;
>> +}
>> +static DEVICE_ATTR_RW(port_select);
>> +
>> +static ssize_t port_enable_show(struct device *dev,
>> +                               struct device_attribute *attr, char *buf)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val;
>> +
>> +       if (!drvdata->enable) {
>> +               val = drvdata->stmsper;
>> +       } else {
>> +               spin_lock(&drvdata->spinlock);
>> +               val = readl_relaxed(drvdata->base + STMSPER);
>> +               spin_unlock(&drvdata->spinlock);
>> +       }
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>> +}
>> +
>> +static ssize_t port_enable_store(struct device *dev,
>> +                                struct device_attribute *attr,
>> +                                const char *buf, size_t size)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val;
>> +       int ret = 0;
>> +
>> +       ret = kstrtoul(buf, 16, &val);
>> +       if (ret)
>> +               return ret;
>> +
>> +       spin_lock(&drvdata->spinlock);
>> +       drvdata->stmsper = val;
>> +
>> +       if (drvdata->enable) {
>> +               CS_UNLOCK(drvdata->base);
>> +               writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);
>> +               CS_LOCK(drvdata->base);
>> +       }
>> +       spin_unlock(&drvdata->spinlock);
>> +
>> +       return size;
>> +}
>> +static DEVICE_ATTR_RW(port_enable);
>> +
>> +static ssize_t traceid_show(struct device *dev,
>> +                           struct device_attribute *attr, char *buf)
>> +{
>> +       unsigned long val;
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +       val = drvdata->traceid;
>> +       return sprintf(buf, "%#lx\n", val);
>> +}
>> +
>> +static ssize_t traceid_store(struct device *dev,
>> +                            struct device_attribute *attr,
>> +                            const char *buf, size_t size)
>> +{
>> +       int ret;
>> +       unsigned long val;
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +       ret = kstrtoul(buf, 16, &val);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* traceid field is 7bit wide on STM32 */
>> +       drvdata->traceid = val & 0x7f;
>> +       return size;
>> +}
>> +static DEVICE_ATTR_RW(traceid);
>> +
>> +#define coresight_simple_func(type, name, offset)                      \
>> +static ssize_t name##_show(struct device *_dev,                                \
>> +                          struct device_attribute *attr, char *buf)    \
>> +{                                                                      \
>> +       type *drvdata = dev_get_drvdata(_dev->parent);                  \
>> +       return scnprintf(buf, PAGE_SIZE, "0x%08x\n",                    \
>> +                        readl_relaxed(drvdata->base + offset));        \
>> +}                                                                      \
>> +DEVICE_ATTR_RO(name)
>> +
>> +#define coresight_stm_simple_func(name, offset)        \
>> +       coresight_simple_func(struct stm_drvdata, name, offset)
>> +
>> +coresight_stm_simple_func(tcsr, STMTCSR);
>> +coresight_stm_simple_func(tsfreqr, STMTSFREQR);
>> +coresight_stm_simple_func(syncr, STMSYNCR);
>> +coresight_stm_simple_func(sper, STMSPER);
>> +coresight_stm_simple_func(spter, STMSPTER);
>> +coresight_stm_simple_func(privmaskr, STMPRIVMASKR);
>> +coresight_stm_simple_func(spscr, STMSPSCR);
>> +coresight_stm_simple_func(spmscr, STMSPMSCR);
>> +coresight_stm_simple_func(spfeat1r, STMSPFEAT1R);
>> +coresight_stm_simple_func(spfeat2r, STMSPFEAT2R);
>> +coresight_stm_simple_func(spfeat3r, STMSPFEAT3R);
>> +coresight_stm_simple_func(devid, CORESIGHT_DEVID);
>> +
>> +static struct attribute *coresight_stm_attrs[] = {
>> +       &dev_attr_hwevent_enable.attr,
>> +       &dev_attr_hwevent_select.attr,
>> +       &dev_attr_port_enable.attr,
>> +       &dev_attr_port_select.attr,
>> +       &dev_attr_traceid.attr,
>> +       NULL,
>> +};
>> +
>> +static struct attribute *coresight_stm_mgmt_attrs[] = {
>> +       &dev_attr_tcsr.attr,
>> +       &dev_attr_tsfreqr.attr,
>> +       &dev_attr_syncr.attr,
>> +       &dev_attr_sper.attr,
>> +       &dev_attr_spter.attr,
>> +       &dev_attr_privmaskr.attr,
>> +       &dev_attr_spscr.attr,
>> +       &dev_attr_spmscr.attr,
>> +       &dev_attr_spfeat1r.attr,
>> +       &dev_attr_spfeat2r.attr,
>> +       &dev_attr_spfeat3r.attr,
>> +       &dev_attr_devid.attr,
>> +       NULL,
>> +};
>> +
>> +static const struct attribute_group coresight_stm_group = {
>> +       .attrs = coresight_stm_attrs,
>> +};
>> +
>> +static const struct attribute_group coresight_stm_mgmt_group = {
>> +       .attrs = coresight_stm_mgmt_attrs,
>> +       .name = "mgmt",
>> +};
>> +
>> +static const struct attribute_group *coresight_stm_groups[] = {
>> +       &coresight_stm_group,
>> +       &coresight_stm_mgmt_group,
>> +       NULL,
>> +};
>> +
>> +static int stm_get_resource_byname(struct device_node *np,
>> +                                  char *ch_base, struct resource *res)
>> +{
>> +       const char *name = NULL;
>> +       int index = 0, found = 0;
>> +
>> +       while (!of_property_read_string_index(np, "reg-names", index, &name)) {
>> +               if (strcmp(ch_base, name)) {
>> +                       index++;
>> +                       continue;
>> +               }
>> +
>> +               /* We have a match and @index is where it's at */
>> +               found = 1;
>> +               break;
>> +       }
>> +
>> +       if (!found)
>> +               return -EINVAL;
>> +
>> +       return of_address_to_resource(np, index, res);
>> +}
>> +
>> +static u32 stm_fundamental_data_size(struct stm_drvdata *drvdata)
>> +{
>> +       u32 stmspfeat2r;
>> +
>> +       stmspfeat2r = readl_relaxed(drvdata->base + STMSPFEAT2R);
>> +       return BMVAL(stmspfeat2r, 12, 15);
>> +}
>> +
>> +static u32 stm_num_stimulus_port(struct stm_drvdata *drvdata)
>> +{
>> +       u32 numsp;
>> +
>> +       numsp = readl_relaxed(drvdata->base + CORESIGHT_DEVID);
>> +       /*
>> +        * NUMPS in STMDEVID is 17 bit long and if equal to 0x0,
>> +        * 32 stimulus ports are supported.
>> +        */
>> +       numsp &= 0x1ffff;
>> +       if (!numsp)
>> +               numsp = STM_32_CHANNEL;
>> +       return numsp;
>> +}
>> +
>> +static void stm_init_default_data(struct stm_drvdata *drvdata)
>> +{
>> +       /* Don't use port selection */
>> +       drvdata->stmspscr = 0x0;
>> +       /*
>> +        * Enable all channel regardless of their number.  When port
>> +        * selection isn't used (see above) STMSPER applies to all
>> +        * 32 channel group available, hence setting all 32 bits to 1
>> +        */
>> +       drvdata->stmsper = ~0x0;
>> +
>> +       /*
>> +        * Select arbitrary value to start with.  If there is a conflict
>> +        * with other tracers the framework will deal with it.
>> +        */
>> +       drvdata->traceid = 0x20;
>> +
>> +       /* Set invariant transaction timing on all channels */
>> +       bitmap_clear(drvdata->chs.guaranteed, 0, drvdata->numsp);
>> +}
>> +
>> +static void stm_init_generic_data(struct stm_drvdata *drvdata)
>> +{
>> +       drvdata->stm.name = dev_name(drvdata->dev);
>> +       drvdata->stm.mstatic = true;
>> +       drvdata->stm.sw_nchannels = drvdata->numsp;
>> +       drvdata->stm.packet = stm_generic_packet;
>> +       drvdata->stm.link = stm_generic_link;
>> +       drvdata->stm.unlink = stm_generic_unlink;
>> +       drvdata->stm.set_options = stm_generic_set_options;
>> +       drvdata->stm.get_options = stm_generic_get_options;
>> +}
>> +
>> +static int stm_probe(struct amba_device *adev, const struct amba_id *id)
>> +{
>> +       int ret;
>> +       void __iomem *base;
>> +       unsigned long *guaranteed;
>> +       struct device *dev = &adev->dev;
>> +       struct coresight_platform_data *pdata = NULL;
>> +       struct stm_drvdata *drvdata;
>> +       struct resource *res = &adev->res;
>> +       struct resource ch_res;
>> +       size_t res_size, bitmap_size;
>> +       struct coresight_desc *desc;
>> +       struct device_node *np = adev->dev.of_node;
>> +
>> +       if (np) {
>> +               pdata = of_get_coresight_platform_data(dev, np);
>> +               if (IS_ERR(pdata))
>> +                       return PTR_ERR(pdata);
>> +               adev->dev.platform_data = pdata;
>> +       }
>> +       drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> +       if (!drvdata)
>> +               return -ENOMEM;
>> +
>> +       drvdata->dev = &adev->dev;
>> +       drvdata->atclk = devm_clk_get(&adev->dev, "atclk"); /* optional */
>> +       if (!IS_ERR(drvdata->atclk)) {
>> +               ret = clk_prepare_enable(drvdata->atclk);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +       dev_set_drvdata(dev, drvdata);
>> +
>> +       base = devm_ioremap_resource(dev, res);
>> +       if (IS_ERR(base))
>> +               return PTR_ERR(base);
>> +       drvdata->base = base;
>> +
>> +       ret = stm_get_resource_byname(np, "stm-stimulus-base", &ch_res);
>> +       if (ret)
>> +               return ret;
>> +
>> +       base = devm_ioremap_resource(dev, &ch_res);
>> +       if (IS_ERR(base))
>> +               return PTR_ERR(base);
>> +       drvdata->chs.base = base;
>> +
>> +       drvdata->write_max = sizeof(u32);
>
> Isn't the fundamental data size the maximum an architecture can stand?
>  Why not simply use that rather than adding another variable?
>
>> +       drvdata->write_64bit = stm_fundamental_data_size(drvdata);
>> +
>> +#ifndef CONFIG_64BIT
>> +       if (drvdata->write_64bit)
>> +               drvdata->write_max = sizeof(u64);
>> +#endif

It is a bit odd that this code is using sizeof(u64) / sizeof(u32) here, when it is using 8 / 4 explicitly in the functions above.

>
> In most cases the fundamental data size will follow the architecture
> size, i.e, 32 bit architecture with 32 bit fundamental data size, 64
> bit architecture with 64 bit fundamental data size.  A fundamental
> data size of 32 is also possible on a 64 bit architecture.
>
> What needs to be prevented is a 32 bit architecture with a fundamental
> data size of 64.  In those cases the fundamental data size should be
> adjusted to be 32 bit.  To do that I suggest to modify
> stm_fundamental_data_size() to probe the STM's fundamental data size
> but to adjust it down if on a 32 bit system.  Knowing whether running
> on a 32/64 bit system is easy when using the IS_ENABLED() macro.

Agreed. For ARM, 64-bit writes to the STM are supported only if BOTH the STM supports 64-bit data and the processor is in 64-bit execution state. Otherwise, the largest size is 32 bits. You are likely to see both 64-bit STM from 32-bit execution state, and 32-bit STM on a SoC with a 64-bit ARM processor.

>> +
>> +       if (boot_nr_channel) {
>> +               drvdata->numsp = boot_nr_channel;
>> +               res_size = min((resource_size_t)(boot_nr_channel *
>> +                                 BYTES_PER_CHANNEL), resource_size(res));
>> +       } else {
>> +               drvdata->numsp = stm_num_stimulus_port(drvdata);
>> +               res_size = min((resource_size_t)(drvdata->numsp *
>> +                                BYTES_PER_CHANNEL), resource_size(res));
>> +       }
>> +       bitmap_size = BITS_TO_LONGS(drvdata->numsp) * sizeof(long);
>> +
>> +       guaranteed = devm_kzalloc(dev, bitmap_size, GFP_KERNEL);
>> +       if (!guaranteed)
>> +               return -ENOMEM;
>> +       drvdata->chs.guaranteed = guaranteed;
>> +
>> +       spin_lock_init(&drvdata->spinlock);
>> +
>> +       stm_init_default_data(drvdata);
>> +       stm_init_generic_data(drvdata);
>> +
>> +       if (stm_register_device(dev, &drvdata->stm, THIS_MODULE)) {
>> +               dev_info(dev,
>> +                        "stm_register_device failed, probing deffered\n");
>> +               return -EPROBE_DEFER;
>> +       }
>> +
>> +       desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
>> +       if (!desc) {
>> +               ret = -ENOMEM;
>> +               goto stm_unregister;
>> +       }
>> +
>> +       desc->type = CORESIGHT_DEV_TYPE_SOURCE;
>> +       desc->subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE;
>> +       desc->ops = &stm_cs_ops;
>> +       desc->pdata = pdata;
>> +       desc->dev = dev;
>> +       desc->groups = coresight_stm_groups;
>> +       drvdata->csdev = coresight_register(desc);
>> +       if (IS_ERR(drvdata->csdev)) {
>> +               ret = PTR_ERR(drvdata->csdev);
>> +               goto stm_unregister;
>> +       }
>> +
>> +       pm_runtime_put(&adev->dev);
>> +
>> +       dev_info(dev, "%s initialized\n", (char *)id->data);
>> +       return 0;
>> +
>> +stm_unregister:
>> +       stm_unregister_device(&drvdata->stm);
>> +       return ret;
>> +}
>> +
>> +static int stm_remove(struct amba_device *adev)
>> +{
>> +       struct stm_drvdata *drvdata = amba_get_drvdata(adev);
>> +
>> +       stm_unregister_device(&drvdata->stm);
>> +       coresight_unregister(drvdata->csdev);
>> +       return 0;
>> +}
>
> The xyz_remove() functions is not needed for coresight devices -
> unbinding a device from its driver will no longer be possible in the
> 4.6 cycle.
>
>> +
>> +#ifdef CONFIG_PM
>> +static int stm_runtime_suspend(struct device *dev)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev);
>> +
>> +       if (drvdata && !IS_ERR(drvdata->atclk))
>> +               clk_disable_unprepare(drvdata->atclk);
>> +
>> +       return 0;
>> +}
>> +
>> +static int stm_runtime_resume(struct device *dev)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev);
>> +
>> +       if (drvdata && !IS_ERR(drvdata->atclk))
>> +               clk_prepare_enable(drvdata->atclk);
>> +
>> +       return 0;
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops stm_dev_pm_ops = {
>> +       SET_RUNTIME_PM_OPS(stm_runtime_suspend, stm_runtime_resume, NULL)
>> +};
>> +
>> +static struct amba_id stm_ids[] = {
>> +       {
>> +               .id     = 0x0003b962,
>> +               .mask   = 0x0003ffff,
>> +               .data   = "STM32",
>> +       },
>> +       { 0, 0},
>> +};
>> +
>> +static struct amba_driver stm_driver = {
>> +       .drv = {
>> +               .name   = "coresight-stm",
>> +               .owner  = THIS_MODULE,
>> +               .pm     = &stm_dev_pm_ops,
>> +       },
>> +       .probe          = stm_probe,
>> +       .remove         = stm_remove,
>> +       .id_table       = stm_ids,
>> +};
>> +
>> +module_amba_driver(stm_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("CoreSight System Trace Macrocell driver");
>> diff --git a/include/linux/coresight-stm.h b/include/linux/coresight-stm.h
>> new file mode 100644
>> index 0000000..a978bb8
>> --- /dev/null
>> +++ b/include/linux/coresight-stm.h
>> @@ -0,0 +1,6 @@
>> +#ifndef __LINUX_CORESIGHT_STM_H_
>> +#define __LINUX_CORESIGHT_STM_H_
>> +
>> +#include <uapi/linux/coresight-stm.h>
>> +
>> +#endif
>> diff --git a/include/uapi/linux/coresight-stm.h b/include/uapi/linux/coresight-stm.h
>> new file mode 100644
>> index 0000000..fa35f0b
>> --- /dev/null
>> +++ b/include/uapi/linux/coresight-stm.h
>> @@ -0,0 +1,12 @@
>> +#ifndef __UAPI_CORESIGHT_STM_H_
>> +#define __UAPI_CORESIGHT_STM_H_
>> +
>> +#define STM_FLAG_TIMESTAMPED   BIT(3)
>> +#define STM_FLAG_GUARANTEED    BIT(7)
>> +
>> +enum {
>> +       STM_OPTION_GUARANTEED = 0,
>> +       STM_OPTION_INVARIANT,
>> +};
>> +
>> +#endif
>> --
>> 1.9.1
>>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Alexander Shishkin Feb. 12, 2016, 3:28 p.m. UTC | #3
Chunyan Zhang <zhang.chunyan@linaro.org> writes:

> +static long stm_generic_set_options(struct stm_data *stm_data,
> +				    unsigned int master,
> +				    unsigned int channel,
> +				    unsigned int nr_chans,
> +				    unsigned long options)
> +{
> +	struct stm_drvdata *drvdata = container_of(stm_data,
> +						   struct stm_drvdata, stm);
> +	if (!(drvdata && drvdata->enable))
> +		return -EINVAL;
> +
> +	if (channel >= drvdata->numsp)
> +		return -EINVAL;
> +
> +	switch (options) {
> +	case STM_OPTION_GUARANTEED:
> +		set_bit(channel, drvdata->chs.guaranteed);
> +		break;
> +
> +	case STM_OPTION_INVARIANT:
> +		clear_bit(channel, drvdata->chs.guaranteed);
> +		break;

This is a bad interface. Firstly, neither option is described
anywhere. Secondly, I'm pretty sure "invariant" does not mean "not
guaranteed" in english, although this function seems to imply this.

> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static long stm_generic_get_options(struct stm_data *stm_data,
> +				    unsigned int master,
> +				    unsigned int channel,
> +				    unsigned int nr_chans,
> +				    u64 *options)
> +{
> +	struct stm_drvdata *drvdata = container_of(stm_data,
> +						   struct stm_drvdata, stm);
> +	if (!(drvdata && drvdata->enable))
> +		return -EINVAL;
> +
> +	if (channel >= drvdata->numsp)
> +		return -EINVAL;
> +
> +	switch (*options) {
> +	case STM_OPTION_GUARANTEED:
> +		*options = test_bit(channel, drvdata->chs.guaranteed);
> +		break;

This just doesn't work. @options here is an on-stack variable in
stm_char_ioctl(), hitherto uninitialized. The get_options ioctl command
as you implemented it doesn't fetch @options from userspace either, it
just passes a pointer to it to this callback, expecting that the
callback will set it so that it can be copy_to_user()ed back to the
user.

Then, when we figure this out, there is again the question of what
should one make of STM_OPTION_{GUARANTEED,INVARIANT} and how do they fit
into *options.

The idea behind set_options ioctl is that the user specifies a bit mask
of options that he/she wants to set.

[snip]

> +#ifndef __UAPI_CORESIGHT_STM_H_
> +#define __UAPI_CORESIGHT_STM_H_
> +
> +#define STM_FLAG_TIMESTAMPED   BIT(3)
> +#define STM_FLAG_GUARANTEED    BIT(7)
> +
> +enum {
> +	STM_OPTION_GUARANTEED = 0,
> +	STM_OPTION_INVARIANT,
> +};

Each of these guys could also use an explanation.

Regards,
--
Alex
Mathieu Poirier Feb. 12, 2016, 6:24 p.m. UTC | #4
On 12 February 2016 at 07:55, Michael Williams <Michael.Williams@arm.com> wrote:
> Mathieu Poirier [mailto:mathieu.poirier@linaro.org] wrote:
>> On 6 February 2016 at 04:04, Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
>>> From: Pratik Patel <pratikp@codeaurora.org>
>>>
>>> This driver adds support for the STM CoreSight IP block, allowing any
>>> system compoment (HW or SW) to log and aggregate messages via a
>>> single entity.
>>>
>>> The CoreSight STM exposes an application defined number of channels
>>> called stimulus port.  Configuration is done using entries in sysfs
>>> and channels made available to userspace via configfs.
>>>
>>> Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>>> ---
>>>  .../ABI/testing/sysfs-bus-coresight-devices-stm    |  53 ++
>>>  Documentation/trace/coresight.txt                  |  37 +-
>>>  drivers/hwtracing/coresight/Kconfig                |  11 +
>>>  drivers/hwtracing/coresight/Makefile               |   1 +
>>>  drivers/hwtracing/coresight/coresight-stm.c        | 928 +++++++++++++++++++++
>>>  include/linux/coresight-stm.h                      |   6 +
>>>  include/uapi/linux/coresight-stm.h                 |  12 +
>>>  7 files changed, 1046 insertions(+), 2 deletions(-)
>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>>>  create mode 100644 drivers/hwtracing/coresight/coresight-stm.c
>>>  create mode 100644 include/linux/coresight-stm.h
>>>  create mode 100644 include/uapi/linux/coresight-stm.h
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>>> new file mode 100644
>>> index 0000000..a1d7022
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>>> @@ -0,0 +1,53 @@
>>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/enable_source
>>> +Date:          February 2016
>>> +KernelVersion: 4.5
>>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>>> +Description:   (RW) Enable/disable tracing on this specific trace macrocell.
>>> +               Enabling the trace macrocell implies it has been configured
>>> +               properly and a sink has been identidifed for it.  The path
>>> +               of coresight components linking the source to the sink is
>>> +               configured and managed automatically by the coresight framework.
>>> +
>>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/hwevent_enable
>>> +Date:          February 2016
>>> +KernelVersion: 4.5
>>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>>> +Description:   (RW) Provides access to the HW event enable register, used in
>>> +               conjunction with HW event bank select register.
>>> +
>>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/hwevent_select
>>> +Date:          February 2016
>>> +KernelVersion: 4.5
>>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>>> +Description:   (RW) Gives access to the HW event block select register
>>> +               (STMHEBSR) in order to configure up to 256 channels.  Used in
>>> +               conjunction with "hwevent_enable" register as described above.
>>> +
>>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/port_enable
>>> +Date:          February 2016
>>> +KernelVersion: 4.5
>>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>>> +Description:   (RW) Provides access to the stimlus port enable register
>>> +               (STMSPER).  Used in conjunction with "port_select" described
>>> +               below.
>>> +
>>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/port_select
>>> +Date:          February 2016
>>> +KernelVersion: 4.5
>>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>>> +Description:   (RW) Used to determine which bank of stimulus port bit in
>>> +               register STMSPER (see above) apply to.
>>> +
>>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/status
>>> +Date:          February 2016
>>> +KernelVersion: 4.5
>>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>>> +Description:   (R) List various control and status registers.  The specific
>>> +               layout and content is driver specific.
>>> +
>>> +What:          /sys/bus/coresight/devices/<memory_map>.stm/traceid
>>> +Date:          February 2016
>>> +KernelVersion: 4.5
>>> +Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>>> +Description:   (RW) Holds the trace ID that will appear in the trace stream
>>> +               coming from this trace entity.
>>> diff --git a/Documentation/trace/coresight.txt b/Documentation/trace/coresight.txt
>>> index 0a5c329..a33c88c 100644
>>> --- a/Documentation/trace/coresight.txt
>>> +++ b/Documentation/trace/coresight.txt
>>> @@ -190,8 +190,8 @@ expected to be accessed and controlled using those entries.
>>>  Last but not least, "struct module *owner" is expected to be set to reflect
>>>  the information carried in "THIS_MODULE".
>>>
>>> -How to use
>>> -----------
>>> +How to use the tracer modules
>>> +-----------------------------
>>>
>>>  Before trace collection can start, a coresight sink needs to be identify.
>>>  There is no limit on the amount of sinks (nor sources) that can be enabled at
>>> @@ -297,3 +297,36 @@ Info                                    Tracing enabled
>>>  Instruction     13570831        0x8026B584      E28DD00C        false   ADD
>> sp,sp,#0xc
>>>  Instruction     0       0x8026B588      E8BD8000        true    LDM      sp!,{pc}
>>>  Timestamp                                       Timestamp: 17107041535
>>> +
>>> +How to use the STM module
>>> +-------------------------
>>> +
>>> +Using the System Trace Macrocell module is the same as the tracers - the only
>>> +difference is that clients are driving the trace capture rather
>>> +than the program flow through the code.
>>> +
>>> +As with any other CoreSight component, specifics about the STM tracer can be
>>> +found in sysfs with more information on each entry being found in [1]:
>>> +
>>> +root@genericarmv8:~# ls /sys/bus/coresight/devices/20100000.stm
>>> +enable_source   hwevent_select  port_enable     subsystem       uevent
>>> +hwevent_enable  mgmt            port_select     traceid
>>> +root@genericarmv8:~#
>>> +
>>> +Like any other source a sink needs to be identified and the STM enabled before
>>> +being used:
>>> +
>>> +root@genericarmv8:~# echo 1 > /sys/bus/coresight/devices/20010000.etf/enable_sink
>>> +root@genericarmv8:~# echo 1 > /sys/bus/coresight/devices/20100000.stm/enable_source
>>> +
>>> +From there user space applications can request and use channels using the devfs
>>> +interface provided for that purpose by the generic STM API:
>>> +
>>> +root@genericarmv8:~# ls -l /dev/20100000.stm
>>> +crw-------    1 root     root       10,  61 Jan  3 18:11 /dev/20100000.stm
>>> +root@genericarmv8:~#
>>> +
>>> +Details on how to use the generic STM API can be found here [2].
>>> +
>>> +[1]. Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>>> +[2]. Documentation/trace/stm.txt
>>> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
>>> index c85935f..f4a8bfa 100644
>>> --- a/drivers/hwtracing/coresight/Kconfig
>>> +++ b/drivers/hwtracing/coresight/Kconfig
>>> @@ -77,4 +77,15 @@ config CORESIGHT_QCOM_REPLICATOR
>>>           programmable ATB replicator sends the ATB trace stream from the
>>>           ETB/ETF to the TPIUi and ETR.
>>>
>>> +config CORESIGHT_STM
>>> +       bool "CoreSight System Trace Macrocell driver"
>>> +       depends on (ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64
>>> +       select CORESIGHT_LINKS_AND_SINKS
>>> +       select STM
>>> +       help
>>> +         This driver provides support for hardware assisted software
>>> +         instrumentation based tracing. This is primarily used for
>>> +         logging useful software events or data coming from various entities
>>> +         in the system, possibly running different OSs
>>> +
>>>  endif
>>> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
>>> index 99f8e5f..1f6eaec 100644
>>> --- a/drivers/hwtracing/coresight/Makefile
>>> +++ b/drivers/hwtracing/coresight/Makefile
>>> @@ -11,3 +11,4 @@ obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \
>>>  obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o
>>>  obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o
>>>  obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
>>> +obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
>>> diff --git a/drivers/hwtracing/coresight/coresight-stm.c
>> b/drivers/hwtracing/coresight/coresight-stm.c
>>> new file mode 100644
>>> index 0000000..01b3521
>>> --- /dev/null
>>> +++ b/drivers/hwtracing/coresight/coresight-stm.c
>>> @@ -0,0 +1,928 @@
>>> +/* Copyright (c) 2015-2016, The Linux Foundation. All rights reserved.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 and
>>> + * only version 2 as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that 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.
>>> + *
>>> + * Initial implementation by Pratik Patel
>>> + * (C) 2014-2015 Pratik Patel <pratikp@codeaurora.org>
>>> + *
>>> + * Serious refactoring, code cleanup and upgrading to the Coresight upstream
>>> + * framework by Mathieu Poirier
>>> + * (C) 2015-2016 Mathieu Poirier <mathieu.poirier@linaro.org>
>>> + *
>>> + * Guaranteed timing and support for various packet type coming from the
>>> + * generic STM API by Chunyan Zhang
>>> + * (C) 2015-2016 Chunyan Zhang <zhang.chunyan@linaro.org>
>>> + */
>>> +#include <linux/amba/bus.h>
>>> +#include <linux/bitmap.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/coresight.h>
>>> +#include <linux/coresight-stm.h>
>>> +#include <linux/err.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/stm.h>
>>> +
>>> +#include "coresight-priv.h"
>>> +
>>> +#define STMDMASTARTR                   0xc04
>>> +#define STMDMASTOPR                    0xc08
>>> +#define STMDMASTATR                    0xc0c
>>> +#define STMDMACTLR                     0xc10
>>> +#define STMDMAIDR                      0xcfc
>>> +#define STMHEER                                0xd00
>>> +#define STMHETER                       0xd20
>>> +#define STMHEBSR                       0xd60
>>> +#define STMHEMCR                       0xd64
>>> +#define STMHEMASTR                     0xdf4
>>> +#define STMHEFEAT1R                    0xdf8
>>> +#define STMHEIDR                       0xdfc
>>> +#define STMSPER                                0xe00
>>> +#define STMSPTER                       0xe20
>>> +#define STMPRIVMASKR                   0xe40
>>> +#define STMSPSCR                       0xe60
>>> +#define STMSPMSCR                      0xe64
>>> +#define STMSPOVERRIDER                 0xe68
>>> +#define STMSPMOVERRIDER                        0xe6c
>>> +#define STMSPTRIGCSR                   0xe70
>>> +#define STMTCSR                                0xe80
>>> +#define STMTSSTIMR                     0xe84
>>> +#define STMTSFREQR                     0xe8c
>>> +#define STMSYNCR                       0xe90
>>> +#define STMAUXCR                       0xe94
>>> +#define STMSPFEAT1R                    0xea0
>>> +#define STMSPFEAT2R                    0xea4
>>> +#define STMSPFEAT3R                    0xea8
>>> +#define STMITTRIGGER                   0xee8
>>> +#define STMITATBDATA0                  0xeec
>>> +#define STMITATBCTR2                   0xef0
>>> +#define STMITATBID                     0xef4
>>> +#define STMITATBCTR0                   0xef8
>>> +
>>> +#define STM_32_CHANNEL                 32
>>> +#define BYTES_PER_CHANNEL              256
>>> +#define STM_TRACE_BUF_SIZE             4096
>>> +#define STM_SW_MASTER_END              127
>>> +
>>> +/* Register bit definition */
>>> +#define STMTCSR_BUSY_BIT               23
>>> +/* Reserve the first 10 channels for kernel usage */
>>> +#define STM_CHANNEL_OFFSET             0
>>> +
>>> +enum stm_pkt_type {
>>> +       STM_PKT_TYPE_DATA       = 0x98,
>>> +       STM_PKT_TYPE_FLAG       = 0xE8,
>>> +       STM_PKT_TYPE_TRIG       = 0xF8,
>>> +};
>>> +
>>> +#define stm_channel_addr(drvdata, ch)  (drvdata->chs.base +    \
>>> +                                       (ch * BYTES_PER_CHANNEL))
>>> +#define stm_channel_off(type, opts)    (type & ~opts)
>>> +
>>> +static int boot_nr_channel;
>>> +
>>> +module_param_named(
>>> +       boot_nr_channel, boot_nr_channel, int, S_IRUGO
>>> +);
>>> +
>>> +/**
>>> + * struct channel_space - central management entity for extended ports
>>> + * @base:              memory mapped base address where channels start.
>>> + * @guaraneed:         is the channel delivery guaranteed.
>>> + */
>>> +struct channel_space {
>>> +       void __iomem            *base;
>>> +       unsigned long           *guaranteed;
>>> +};
>>> +
>>> +/**
>>> + * struct stm_drvdata - specifics associated to an STM component
>>> + * @base:              memory mapped base address for this component.
>>> + * @dev:               the device entity associated to this component.
>>> + * @atclk:             optional clock for the core parts of the STM.
>>> + * @csdev:             component vitals needed by the framework.
>>> + * @spinlock:          only one at a time pls.
>>> + * @chs:               the channels accociated to this STM.
>>> + * @stm:               structure associated to the generic STM interface.
>>> + * @enable:            this STM is being used.
>>> + * @traceid:           value of the current ID for this component.
>>> + * @write_max:         Maximus bytes this STM can write at a time.
>>> + * @write_64bit:       whether this STM supports 64 bit access.
>>> + * @stmsper:           settings for register STMSPER.
>>> + * @stmspscr:          settings for register STMSPSCR.
>>> + * @numsp:             the total number of stimulus port support by this STM.
>>> + * @stmheer:           settings for register STMHEER.
>>> + * @stmheter:          settings for register STMHETER.
>>> + * @stmhebsr:          settings for register STMHEBSR.
>>> + */
>>> +struct stm_drvdata {
>>> +       void __iomem            *base;
>>> +       struct device           *dev;
>>> +       struct clk              *atclk;
>>> +       struct coresight_device *csdev;
>>> +       spinlock_t              spinlock;
>>> +       struct channel_space    chs;
>>> +       struct stm_data         stm;
>>> +       bool                    enable;
>>> +       u8                      traceid;
>>> +       u8                      write_max;
>>> +       u32                     write_64bit;
>>> +       u32                     stmsper;
>>> +       u32                     stmspscr;
>>> +       u32                     numsp;
>>> +       u32                     stmheer;
>>> +       u32                     stmheter;
>>> +       u32                     stmhebsr;
>>> +};
>>> +
>>> +static void stm_hwevent_enable_hw(struct stm_drvdata *drvdata)
>>> +{
>>> +       CS_UNLOCK(drvdata->base);
>>> +
>>> +       writel_relaxed(drvdata->stmhebsr, drvdata->base + STMHEBSR);
>>> +       writel_relaxed(drvdata->stmheter, drvdata->base + STMHETER);
>>> +       writel_relaxed(drvdata->stmheer, drvdata->base + STMHEER);
>>> +       writel_relaxed(0x01 |   /* Enable HW event tracing */
>>> +                      0x04,    /* Error detection on event tracing */
>>> +                      drvdata->base + STMHEMCR);
>>> +
>>> +       CS_LOCK(drvdata->base);
>>> +}
>>> +
>>> +static void stm_port_enable_hw(struct stm_drvdata *drvdata)
>>> +{
>>> +       CS_UNLOCK(drvdata->base);
>>> +       /* ATB trigger enable on direct writes to TRIG locations */
>>> +       writel_relaxed(0x10,
>>> +                      drvdata->base + STMSPTRIGCSR);
>>> +       writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR);
>>> +       writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);
>>> +
>>> +       CS_LOCK(drvdata->base);
>>> +}
>>> +
>>> +static void stm_enable_hw(struct stm_drvdata *drvdata)
>>> +{
>>> +       if (drvdata->stmheer)
>>> +               stm_hwevent_enable_hw(drvdata);
>>> +
>>> +       stm_port_enable_hw(drvdata);
>>> +
>>> +       CS_UNLOCK(drvdata->base);
>>> +
>>> +       /* 4096 byte between synchronisation packets */
>>> +       writel_relaxed(0xFFF, drvdata->base + STMSYNCR);
>>> +       writel_relaxed((drvdata->traceid << 16 | /* trace id */
>>> +                       0x02 |                   /* timestamp enable */
>>> +                       0x01),                   /* global STM enable */
>>> +                       drvdata->base + STMTCSR);
>>> +
>>> +       CS_LOCK(drvdata->base);
>>> +}
>>> +
>>> +static int stm_enable(struct coresight_device *csdev)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>> +
>>> +       pm_runtime_get_sync(drvdata->dev);
>>> +
>>> +       spin_lock(&drvdata->spinlock);
>>> +       stm_enable_hw(drvdata);
>>> +       drvdata->enable = true;
>>> +       spin_unlock(&drvdata->spinlock);
>>> +
>>> +       dev_info(drvdata->dev, "STM tracing enabled\n");
>>> +       return 0;
>>> +}
>>> +
>>> +static void stm_hwevent_disable_hw(struct stm_drvdata *drvdata)
>>> +{
>>> +       CS_UNLOCK(drvdata->base);
>>> +
>>> +       writel_relaxed(0x0, drvdata->base + STMHEMCR);
>>> +       writel_relaxed(0x0, drvdata->base + STMHEER);
>>> +       writel_relaxed(0x0, drvdata->base + STMHETER);
>>> +
>>> +       CS_LOCK(drvdata->base);
>>> +}
>>> +
>>> +static void stm_port_disable_hw(struct stm_drvdata *drvdata)
>>> +{
>>> +       CS_UNLOCK(drvdata->base);
>>> +
>>> +       writel_relaxed(0x0, drvdata->base + STMSPER);
>>> +       writel_relaxed(0x0, drvdata->base + STMSPTRIGCSR);
>>> +
>>> +       CS_LOCK(drvdata->base);
>>> +}
>>> +
>>> +static void stm_disable_hw(struct stm_drvdata *drvdata)
>>> +{
>>> +       u32 val;
>>> +
>>> +       CS_UNLOCK(drvdata->base);
>>> +
>>> +       val = readl_relaxed(drvdata->base + STMTCSR);
>>> +       val &= ~0x1; /* clear global STM enable [0] */
>>> +       writel_relaxed(val, drvdata->base + STMTCSR);
>>> +
>>> +       CS_LOCK(drvdata->base);
>>> +
>>> +       stm_port_disable_hw(drvdata);
>>> +       if (drvdata->stmheer)
>>> +               stm_hwevent_disable_hw(drvdata);
>>> +}
>>> +
>>> +static void stm_disable(struct coresight_device *csdev)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>> +
>>> +       spin_lock(&drvdata->spinlock);
>>> +       stm_disable_hw(drvdata);
>>> +       drvdata->enable = false;
>>> +       spin_unlock(&drvdata->spinlock);
>>> +
>>> +       /* Wait until the engine has completely stopped */
>>> +       coresight_timeout(drvdata, STMTCSR, STMTCSR_BUSY_BIT, 0);
>>> +
>>> +       pm_runtime_put(drvdata->dev);
>>> +
>>> +       dev_info(drvdata->dev, "STM tracing disabled\n");
>>> +}
>>> +
>>> +static int stm_trace_id(struct coresight_device *csdev)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>> +
>>> +       return drvdata->traceid;
>>> +}
>>> +
>>> +static const struct coresight_ops_source stm_source_ops = {
>>> +       .trace_id       = stm_trace_id,
>>> +       .enable         = stm_enable,
>>> +       .disable        = stm_disable,
>>> +};
>>> +
>>> +static const struct coresight_ops stm_cs_ops = {
>>> +       .source_ops     = &stm_source_ops,
>>> +};
>>> +
>>> +static inline bool stm_addr_unaligned(const void *addr, u8 max)
>>> +{
>>> +       return ((unsigned long)addr & (max - 1));
>>> +}
>>> +
>>> +static int stm_send(void *addr, const void *data, u32 size, u8 max)
>>> +{
>>> +       u32 len = size;
>>> +       u8 paload[8];
>>> +
>>> +       if (stm_addr_unaligned(data, max)) {
>>> +               memcpy(paload, data, size);
>>> +               data = paload;
>>> +       }
>>> +
>>> +       /* now we are 64bit/32bit aligned */
>>> +#ifdef CONFIG_64BIT
>>> +       if (size == 8)
>>> +               writeq_relaxed(*(u64 *)data, addr);
>>> +#endif
>>
>> We probably don't need an #ifdef here.  Checking the size of the
>> transfer against the fundamental data size will result in the same
>> outcome, i.e preventing 64 bit transfers on a 32 bit architecture.  An
>> error (understandable by the caller) should probably be returned in
>> this case.
>
> In theory this is correct, but if the code is unreachable for non-32-bit architectures, why include it at all?

Why is this code unreachable on 64-bit systems?  In this latest
revision "stm_send()" is used for both architecture.

>
>>> +       if (size >= 4) {
>>> +               writel_relaxed(*(u32 *)data, addr);
>>> +               data += 4;
>>> +               size -= 4;
>>> +       }
>>> +
>>> +       if (size >= 2) {
>>> +               writew_relaxed(*(u16 *)data, addr);
>>> +               data += 2;
>>> +               size -= 2;
>>> +       }
>>> +
>>> +       if (size == 1)
>>> +               writeb_relaxed(*(u8 *)data, addr);
>>> +
>>> +       return len;
>>> +}
>
> The API for stm_data::packet (include/linux/stm.h) is not wholly clearly defined, but it does state "sends an STP packet," which I interpret as meaning exactly one packet and no more. This function (which is called from this module's implementation of stm_data::packet) will send multiple STP packets if called with a value that is not exactly 8, 4, 2, or 1.

I agree.

>
> To send only one packet, either:
>
> * These "if" statements are replaced with "else if" (as appropriate), and the function returns the size of data consumed; or

Please use a 'case' statement.

> * The stm_generic_packet() function (below) forces "size" to a power-of-two (<= write_max) before calling stm_send().

That will become a nightmare to debug/maintain - function stm_write()
[1] is already doing the count management.

[1]. http://lxr.free-electrons.com/source/drivers/hwtracing/stm/core.c#L383

>
>>> +static int stm_generic_link(struct stm_data *stm_data,
>>> +                           unsigned int master,  unsigned int channel)
>>> +{
>>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>>> +                                                  struct stm_drvdata, stm);
>>> +       if (!drvdata || !drvdata->csdev)
>>> +               return -EINVAL;
>>> +
>>> +       return coresight_enable(drvdata->csdev);
>>> +}
>>> +
>>> +static void stm_generic_unlink(struct stm_data *stm_data,
>>> +                              unsigned int master,  unsigned int channel)
>>> +{
>>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>>> +                                                  struct stm_drvdata, stm);
>>> +       if (!drvdata || !drvdata->csdev)
>>> +               return;
>>> +
>>> +       stm_disable(drvdata->csdev);
>>> +}
>>> +
>>> +static long stm_generic_set_options(struct stm_data *stm_data,
>>> +                                   unsigned int master,
>>> +                                   unsigned int channel,
>>> +                                   unsigned int nr_chans,
>>> +                                   unsigned long options)
>>> +{
>>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>>> +                                                  struct stm_drvdata, stm);
>>> +       if (!(drvdata && drvdata->enable))
>>> +               return -EINVAL;
>>> +
>>> +       if (channel >= drvdata->numsp)
>>> +               return -EINVAL;
>>> +
>>> +       switch (options) {
>>> +       case STM_OPTION_GUARANTEED:
>>> +               set_bit(channel, drvdata->chs.guaranteed);
>>> +               break;
>>> +
>>> +       case STM_OPTION_INVARIANT:
>>> +               clear_bit(channel, drvdata->chs.guaranteed);
>>> +               break;
>>> +
>>> +       default:
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static long stm_generic_get_options(struct stm_data *stm_data,
>>> +                                   unsigned int master,
>>> +                                   unsigned int channel,
>>> +                                   unsigned int nr_chans,
>>> +                                   u64 *options)
>>> +{
>>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>>> +                                                  struct stm_drvdata, stm);
>>> +       if (!(drvdata && drvdata->enable))
>>> +               return -EINVAL;
>>> +
>>> +       if (channel >= drvdata->numsp)
>>> +               return -EINVAL;
>>> +
>>> +       switch (*options) {
>>> +       case STM_OPTION_GUARANTEED:
>>> +               *options = test_bit(channel, drvdata->chs.guaranteed);
>>> +               break;
>>> +
>>> +       default:
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static ssize_t stm_generic_packet(struct stm_data *stm_data,
>>> +                                 unsigned int master,
>>> +                                 unsigned int channel,
>>> +                                 unsigned int packet,
>>> +                                 unsigned int flags,
>>> +                                 unsigned int size,
>>> +                                 const unsigned char *payload)
>>> +{
>>> +       unsigned long ch_addr;
>>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>>> +                                                  struct stm_drvdata, stm);
>>> +
>>> +       if (!(drvdata && drvdata->enable))
>>> +               return 0;
>>> +
>>> +       if (channel >= drvdata->numsp)
>>> +               return 0;
>>> +
>>> +       ch_addr = (unsigned long)stm_channel_addr(drvdata, channel);
>>> +
>>> +       flags = (flags == STP_PACKET_TIMESTAMPED) ? STM_FLAG_TIMESTAMPED : 0;
>>> +       flags |= test_bit(channel, drvdata->chs.guaranteed) ?
>>> +                          STM_FLAG_GUARANTEED : 0;
>>> +
>>> +       if (size > drvdata->write_max)
>>> +               size = drvdata->write_max;
>>> +
>>> +       switch (packet) {
>>> +       case STP_PACKET_FLAG:
>>> +               ch_addr |= stm_channel_off(STM_PKT_TYPE_FLAG, flags);
>>> +
>>> +               /* the generic STM API set a size of zero on flag packets. */
>>> +               size = 1;
>>> +               break;
>>> +
>>> +       case STP_PACKET_DATA:
>>> +               ch_addr |= stm_channel_off(STM_PKT_TYPE_DATA, flags);
>>> +               break;
>>> +
>>> +       default:
>>> +               return 0;
>>> +       }
>>> +
>>> +       return stm_send((void *)ch_addr, payload, size, drvdata->write_max);
>>> +}
>>> +
>>> +static ssize_t hwevent_enable_show(struct device *dev,
>>> +                                  struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val = drvdata->stmheer;
>>> +
>>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>>> +}
>>> +
>>> +static ssize_t hwevent_enable_store(struct device *dev,
>>> +                                   struct device_attribute *attr,
>>> +                                   const char *buf, size_t size)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val;
>>> +       int ret = 0;
>>> +
>>> +       ret = kstrtoul(buf, 16, &val);
>>> +       if (ret)
>>> +               return -EINVAL;
>>> +
>>> +       drvdata->stmheer = val;
>>> +       /* HW event enable and trigger go hand in hand */
>>> +       drvdata->stmheter = val;
>>> +
>>> +       return size;
>>> +}
>>> +static DEVICE_ATTR_RW(hwevent_enable);
>>> +
>>> +static ssize_t hwevent_select_show(struct device *dev,
>>> +                                  struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val = drvdata->stmhebsr;
>>> +
>>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>>> +}
>>> +
>>> +static ssize_t hwevent_select_store(struct device *dev,
>>> +                                   struct device_attribute *attr,
>>> +                                   const char *buf, size_t size)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val;
>>> +       int ret = 0;
>>> +
>>> +       ret = kstrtoul(buf, 16, &val);
>>> +       if (ret)
>>> +               return -EINVAL;
>>> +
>>> +       drvdata->stmhebsr = val;
>>> +
>>> +       return size;
>>> +}
>>> +static DEVICE_ATTR_RW(hwevent_select);
>>> +
>>> +static ssize_t port_select_show(struct device *dev,
>>> +                               struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val;
>>> +
>>> +       if (!drvdata->enable) {
>>> +               val = drvdata->stmspscr;
>>> +       } else {
>>> +               spin_lock(&drvdata->spinlock);
>>> +               val = readl_relaxed(drvdata->base + STMSPSCR);
>>> +               spin_unlock(&drvdata->spinlock);
>>> +       }
>>> +
>>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>>> +}
>>> +
>>> +static ssize_t port_select_store(struct device *dev,
>>> +                                struct device_attribute *attr,
>>> +                                const char *buf, size_t size)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val, stmsper;
>>> +       int ret = 0;
>>> +
>>> +       ret = kstrtoul(buf, 16, &val);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       spin_lock(&drvdata->spinlock);
>>> +       drvdata->stmspscr = val;
>>> +
>>> +       if (drvdata->enable) {
>>> +               CS_UNLOCK(drvdata->base);
>>> +               /* Process as per ARM's TRM recommendation */
>>> +               stmsper = readl_relaxed(drvdata->base + STMSPER);
>>> +               writel_relaxed(0x0, drvdata->base + STMSPER);
>>> +               writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR);
>>> +               writel_relaxed(stmsper, drvdata->base + STMSPER);
>>> +               CS_LOCK(drvdata->base);
>>> +       }
>>> +       spin_unlock(&drvdata->spinlock);
>>> +
>>> +       return size;
>>> +}
>>> +static DEVICE_ATTR_RW(port_select);
>>> +
>>> +static ssize_t port_enable_show(struct device *dev,
>>> +                               struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val;
>>> +
>>> +       if (!drvdata->enable) {
>>> +               val = drvdata->stmsper;
>>> +       } else {
>>> +               spin_lock(&drvdata->spinlock);
>>> +               val = readl_relaxed(drvdata->base + STMSPER);
>>> +               spin_unlock(&drvdata->spinlock);
>>> +       }
>>> +
>>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>>> +}
>>> +
>>> +static ssize_t port_enable_store(struct device *dev,
>>> +                                struct device_attribute *attr,
>>> +                                const char *buf, size_t size)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val;
>>> +       int ret = 0;
>>> +
>>> +       ret = kstrtoul(buf, 16, &val);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       spin_lock(&drvdata->spinlock);
>>> +       drvdata->stmsper = val;
>>> +
>>> +       if (drvdata->enable) {
>>> +               CS_UNLOCK(drvdata->base);
>>> +               writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);
>>> +               CS_LOCK(drvdata->base);
>>> +       }
>>> +       spin_unlock(&drvdata->spinlock);
>>> +
>>> +       return size;
>>> +}
>>> +static DEVICE_ATTR_RW(port_enable);
>>> +
>>> +static ssize_t traceid_show(struct device *dev,
>>> +                           struct device_attribute *attr, char *buf)
>>> +{
>>> +       unsigned long val;
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +
>>> +       val = drvdata->traceid;
>>> +       return sprintf(buf, "%#lx\n", val);
>>> +}
>>> +
>>> +static ssize_t traceid_store(struct device *dev,
>>> +                            struct device_attribute *attr,
>>> +                            const char *buf, size_t size)
>>> +{
>>> +       int ret;
>>> +       unsigned long val;
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +
>>> +       ret = kstrtoul(buf, 16, &val);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       /* traceid field is 7bit wide on STM32 */
>>> +       drvdata->traceid = val & 0x7f;
>>> +       return size;
>>> +}
>>> +static DEVICE_ATTR_RW(traceid);
>>> +
>>> +#define coresight_simple_func(type, name, offset)                      \
>>> +static ssize_t name##_show(struct device *_dev,                                \
>>> +                          struct device_attribute *attr, char *buf)    \
>>> +{                                                                      \
>>> +       type *drvdata = dev_get_drvdata(_dev->parent);                  \
>>> +       return scnprintf(buf, PAGE_SIZE, "0x%08x\n",                    \
>>> +                        readl_relaxed(drvdata->base + offset));        \
>>> +}                                                                      \
>>> +DEVICE_ATTR_RO(name)
>>> +
>>> +#define coresight_stm_simple_func(name, offset)        \
>>> +       coresight_simple_func(struct stm_drvdata, name, offset)
>>> +
>>> +coresight_stm_simple_func(tcsr, STMTCSR);
>>> +coresight_stm_simple_func(tsfreqr, STMTSFREQR);
>>> +coresight_stm_simple_func(syncr, STMSYNCR);
>>> +coresight_stm_simple_func(sper, STMSPER);
>>> +coresight_stm_simple_func(spter, STMSPTER);
>>> +coresight_stm_simple_func(privmaskr, STMPRIVMASKR);
>>> +coresight_stm_simple_func(spscr, STMSPSCR);
>>> +coresight_stm_simple_func(spmscr, STMSPMSCR);
>>> +coresight_stm_simple_func(spfeat1r, STMSPFEAT1R);
>>> +coresight_stm_simple_func(spfeat2r, STMSPFEAT2R);
>>> +coresight_stm_simple_func(spfeat3r, STMSPFEAT3R);
>>> +coresight_stm_simple_func(devid, CORESIGHT_DEVID);
>>> +
>>> +static struct attribute *coresight_stm_attrs[] = {
>>> +       &dev_attr_hwevent_enable.attr,
>>> +       &dev_attr_hwevent_select.attr,
>>> +       &dev_attr_port_enable.attr,
>>> +       &dev_attr_port_select.attr,
>>> +       &dev_attr_traceid.attr,
>>> +       NULL,
>>> +};
>>> +
>>> +static struct attribute *coresight_stm_mgmt_attrs[] = {
>>> +       &dev_attr_tcsr.attr,
>>> +       &dev_attr_tsfreqr.attr,
>>> +       &dev_attr_syncr.attr,
>>> +       &dev_attr_sper.attr,
>>> +       &dev_attr_spter.attr,
>>> +       &dev_attr_privmaskr.attr,
>>> +       &dev_attr_spscr.attr,
>>> +       &dev_attr_spmscr.attr,
>>> +       &dev_attr_spfeat1r.attr,
>>> +       &dev_attr_spfeat2r.attr,
>>> +       &dev_attr_spfeat3r.attr,
>>> +       &dev_attr_devid.attr,
>>> +       NULL,
>>> +};
>>> +
>>> +static const struct attribute_group coresight_stm_group = {
>>> +       .attrs = coresight_stm_attrs,
>>> +};
>>> +
>>> +static const struct attribute_group coresight_stm_mgmt_group = {
>>> +       .attrs = coresight_stm_mgmt_attrs,
>>> +       .name = "mgmt",
>>> +};
>>> +
>>> +static const struct attribute_group *coresight_stm_groups[] = {
>>> +       &coresight_stm_group,
>>> +       &coresight_stm_mgmt_group,
>>> +       NULL,
>>> +};
>>> +
>>> +static int stm_get_resource_byname(struct device_node *np,
>>> +                                  char *ch_base, struct resource *res)
>>> +{
>>> +       const char *name = NULL;
>>> +       int index = 0, found = 0;
>>> +
>>> +       while (!of_property_read_string_index(np, "reg-names", index, &name)) {
>>> +               if (strcmp(ch_base, name)) {
>>> +                       index++;
>>> +                       continue;
>>> +               }
>>> +
>>> +               /* We have a match and @index is where it's at */
>>> +               found = 1;
>>> +               break;
>>> +       }
>>> +
>>> +       if (!found)
>>> +               return -EINVAL;
>>> +
>>> +       return of_address_to_resource(np, index, res);
>>> +}
>>> +
>>> +static u32 stm_fundamental_data_size(struct stm_drvdata *drvdata)
>>> +{
>>> +       u32 stmspfeat2r;
>>> +
>>> +       stmspfeat2r = readl_relaxed(drvdata->base + STMSPFEAT2R);
>>> +       return BMVAL(stmspfeat2r, 12, 15);
>>> +}
>>> +
>>> +static u32 stm_num_stimulus_port(struct stm_drvdata *drvdata)
>>> +{
>>> +       u32 numsp;
>>> +
>>> +       numsp = readl_relaxed(drvdata->base + CORESIGHT_DEVID);
>>> +       /*
>>> +        * NUMPS in STMDEVID is 17 bit long and if equal to 0x0,
>>> +        * 32 stimulus ports are supported.
>>> +        */
>>> +       numsp &= 0x1ffff;
>>> +       if (!numsp)
>>> +               numsp = STM_32_CHANNEL;
>>> +       return numsp;
>>> +}
>>> +
>>> +static void stm_init_default_data(struct stm_drvdata *drvdata)
>>> +{
>>> +       /* Don't use port selection */
>>> +       drvdata->stmspscr = 0x0;
>>> +       /*
>>> +        * Enable all channel regardless of their number.  When port
>>> +        * selection isn't used (see above) STMSPER applies to all
>>> +        * 32 channel group available, hence setting all 32 bits to 1
>>> +        */
>>> +       drvdata->stmsper = ~0x0;
>>> +
>>> +       /*
>>> +        * Select arbitrary value to start with.  If there is a conflict
>>> +        * with other tracers the framework will deal with it.
>>> +        */
>>> +       drvdata->traceid = 0x20;
>>> +
>>> +       /* Set invariant transaction timing on all channels */
>>> +       bitmap_clear(drvdata->chs.guaranteed, 0, drvdata->numsp);
>>> +}
>>> +
>>> +static void stm_init_generic_data(struct stm_drvdata *drvdata)
>>> +{
>>> +       drvdata->stm.name = dev_name(drvdata->dev);
>>> +       drvdata->stm.mstatic = true;
>>> +       drvdata->stm.sw_nchannels = drvdata->numsp;
>>> +       drvdata->stm.packet = stm_generic_packet;
>>> +       drvdata->stm.link = stm_generic_link;
>>> +       drvdata->stm.unlink = stm_generic_unlink;
>>> +       drvdata->stm.set_options = stm_generic_set_options;
>>> +       drvdata->stm.get_options = stm_generic_get_options;
>>> +}
>>> +
>>> +static int stm_probe(struct amba_device *adev, const struct amba_id *id)
>>> +{
>>> +       int ret;
>>> +       void __iomem *base;
>>> +       unsigned long *guaranteed;
>>> +       struct device *dev = &adev->dev;
>>> +       struct coresight_platform_data *pdata = NULL;
>>> +       struct stm_drvdata *drvdata;
>>> +       struct resource *res = &adev->res;
>>> +       struct resource ch_res;
>>> +       size_t res_size, bitmap_size;
>>> +       struct coresight_desc *desc;
>>> +       struct device_node *np = adev->dev.of_node;
>>> +
>>> +       if (np) {
>>> +               pdata = of_get_coresight_platform_data(dev, np);
>>> +               if (IS_ERR(pdata))
>>> +                       return PTR_ERR(pdata);
>>> +               adev->dev.platform_data = pdata;
>>> +       }
>>> +       drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>>> +       if (!drvdata)
>>> +               return -ENOMEM;
>>> +
>>> +       drvdata->dev = &adev->dev;
>>> +       drvdata->atclk = devm_clk_get(&adev->dev, "atclk"); /* optional */
>>> +       if (!IS_ERR(drvdata->atclk)) {
>>> +               ret = clk_prepare_enable(drvdata->atclk);
>>> +               if (ret)
>>> +                       return ret;
>>> +       }
>>> +       dev_set_drvdata(dev, drvdata);
>>> +
>>> +       base = devm_ioremap_resource(dev, res);
>>> +       if (IS_ERR(base))
>>> +               return PTR_ERR(base);
>>> +       drvdata->base = base;
>>> +
>>> +       ret = stm_get_resource_byname(np, "stm-stimulus-base", &ch_res);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       base = devm_ioremap_resource(dev, &ch_res);
>>> +       if (IS_ERR(base))
>>> +               return PTR_ERR(base);
>>> +       drvdata->chs.base = base;
>>> +
>>> +       drvdata->write_max = sizeof(u32);
>>
>> Isn't the fundamental data size the maximum an architecture can stand?
>>  Why not simply use that rather than adding another variable?
>>
>>> +       drvdata->write_64bit = stm_fundamental_data_size(drvdata);
>>> +
>>> +#ifndef CONFIG_64BIT
>>> +       if (drvdata->write_64bit)
>>> +               drvdata->write_max = sizeof(u64);
>>> +#endif
>
> It is a bit odd that this code is using sizeof(u64) / sizeof(u32) here, when it is using 8 / 4 explicitly in the functions above.
>
>>
>> In most cases the fundamental data size will follow the architecture
>> size, i.e, 32 bit architecture with 32 bit fundamental data size, 64
>> bit architecture with 64 bit fundamental data size.  A fundamental
>> data size of 32 is also possible on a 64 bit architecture.
>>
>> What needs to be prevented is a 32 bit architecture with a fundamental
>> data size of 64.  In those cases the fundamental data size should be
>> adjusted to be 32 bit.  To do that I suggest to modify
>> stm_fundamental_data_size() to probe the STM's fundamental data size
>> but to adjust it down if on a 32 bit system.  Knowing whether running
>> on a 32/64 bit system is easy when using the IS_ENABLED() macro.
>
> Agreed. For ARM, 64-bit writes to the STM are supported only if BOTH the STM supports 64-bit data and the processor is in 64-bit execution state. Otherwise, the largest size is 32 bits. You are likely to see both 64-bit STM from 32-bit execution state, and 32-bit STM on a SoC with a 64-bit ARM processor.
>
>>> +
>>> +       if (boot_nr_channel) {
>>> +               drvdata->numsp = boot_nr_channel;
>>> +               res_size = min((resource_size_t)(boot_nr_channel *
>>> +                                 BYTES_PER_CHANNEL), resource_size(res));
>>> +       } else {
>>> +               drvdata->numsp = stm_num_stimulus_port(drvdata);
>>> +               res_size = min((resource_size_t)(drvdata->numsp *
>>> +                                BYTES_PER_CHANNEL), resource_size(res));
>>> +       }
>>> +       bitmap_size = BITS_TO_LONGS(drvdata->numsp) * sizeof(long);
>>> +
>>> +       guaranteed = devm_kzalloc(dev, bitmap_size, GFP_KERNEL);
>>> +       if (!guaranteed)
>>> +               return -ENOMEM;
>>> +       drvdata->chs.guaranteed = guaranteed;
>>> +
>>> +       spin_lock_init(&drvdata->spinlock);
>>> +
>>> +       stm_init_default_data(drvdata);
>>> +       stm_init_generic_data(drvdata);
>>> +
>>> +       if (stm_register_device(dev, &drvdata->stm, THIS_MODULE)) {
>>> +               dev_info(dev,
>>> +                        "stm_register_device failed, probing deffered\n");
>>> +               return -EPROBE_DEFER;
>>> +       }
>>> +
>>> +       desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
>>> +       if (!desc) {
>>> +               ret = -ENOMEM;
>>> +               goto stm_unregister;
>>> +       }
>>> +
>>> +       desc->type = CORESIGHT_DEV_TYPE_SOURCE;
>>> +       desc->subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE;
>>> +       desc->ops = &stm_cs_ops;
>>> +       desc->pdata = pdata;
>>> +       desc->dev = dev;
>>> +       desc->groups = coresight_stm_groups;
>>> +       drvdata->csdev = coresight_register(desc);
>>> +       if (IS_ERR(drvdata->csdev)) {
>>> +               ret = PTR_ERR(drvdata->csdev);
>>> +               goto stm_unregister;
>>> +       }
>>> +
>>> +       pm_runtime_put(&adev->dev);
>>> +
>>> +       dev_info(dev, "%s initialized\n", (char *)id->data);
>>> +       return 0;
>>> +
>>> +stm_unregister:
>>> +       stm_unregister_device(&drvdata->stm);
>>> +       return ret;
>>> +}
>>> +
>>> +static int stm_remove(struct amba_device *adev)
>>> +{
>>> +       struct stm_drvdata *drvdata = amba_get_drvdata(adev);
>>> +
>>> +       stm_unregister_device(&drvdata->stm);
>>> +       coresight_unregister(drvdata->csdev);
>>> +       return 0;
>>> +}
>>
>> The xyz_remove() functions is not needed for coresight devices -
>> unbinding a device from its driver will no longer be possible in the
>> 4.6 cycle.
>>
>>> +
>>> +#ifdef CONFIG_PM
>>> +static int stm_runtime_suspend(struct device *dev)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev);
>>> +
>>> +       if (drvdata && !IS_ERR(drvdata->atclk))
>>> +               clk_disable_unprepare(drvdata->atclk);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int stm_runtime_resume(struct device *dev)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev);
>>> +
>>> +       if (drvdata && !IS_ERR(drvdata->atclk))
>>> +               clk_prepare_enable(drvdata->atclk);
>>> +
>>> +       return 0;
>>> +}
>>> +#endif
>>> +
>>> +static const struct dev_pm_ops stm_dev_pm_ops = {
>>> +       SET_RUNTIME_PM_OPS(stm_runtime_suspend, stm_runtime_resume, NULL)
>>> +};
>>> +
>>> +static struct amba_id stm_ids[] = {
>>> +       {
>>> +               .id     = 0x0003b962,
>>> +               .mask   = 0x0003ffff,
>>> +               .data   = "STM32",
>>> +       },
>>> +       { 0, 0},
>>> +};
>>> +
>>> +static struct amba_driver stm_driver = {
>>> +       .drv = {
>>> +               .name   = "coresight-stm",
>>> +               .owner  = THIS_MODULE,
>>> +               .pm     = &stm_dev_pm_ops,
>>> +       },
>>> +       .probe          = stm_probe,
>>> +       .remove         = stm_remove,
>>> +       .id_table       = stm_ids,
>>> +};
>>> +
>>> +module_amba_driver(stm_driver);
>>> +
>>> +MODULE_LICENSE("GPL v2");
>>> +MODULE_DESCRIPTION("CoreSight System Trace Macrocell driver");
>>> diff --git a/include/linux/coresight-stm.h b/include/linux/coresight-stm.h
>>> new file mode 100644
>>> index 0000000..a978bb8
>>> --- /dev/null
>>> +++ b/include/linux/coresight-stm.h
>>> @@ -0,0 +1,6 @@
>>> +#ifndef __LINUX_CORESIGHT_STM_H_
>>> +#define __LINUX_CORESIGHT_STM_H_
>>> +
>>> +#include <uapi/linux/coresight-stm.h>
>>> +
>>> +#endif
>>> diff --git a/include/uapi/linux/coresight-stm.h b/include/uapi/linux/coresight-stm.h
>>> new file mode 100644
>>> index 0000000..fa35f0b
>>> --- /dev/null
>>> +++ b/include/uapi/linux/coresight-stm.h
>>> @@ -0,0 +1,12 @@
>>> +#ifndef __UAPI_CORESIGHT_STM_H_
>>> +#define __UAPI_CORESIGHT_STM_H_
>>> +
>>> +#define STM_FLAG_TIMESTAMPED   BIT(3)
>>> +#define STM_FLAG_GUARANTEED    BIT(7)
>>> +
>>> +enum {
>>> +       STM_OPTION_GUARANTEED = 0,
>>> +       STM_OPTION_INVARIANT,
>>> +};
>>> +
>>> +#endif
>>> --
>>> 1.9.1
>>>
>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Mathieu Poirier Feb. 12, 2016, 7:02 p.m. UTC | #5
On 12 February 2016 at 08:28, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> Chunyan Zhang <zhang.chunyan@linaro.org> writes:
>
>> +static long stm_generic_set_options(struct stm_data *stm_data,
>> +                                 unsigned int master,
>> +                                 unsigned int channel,
>> +                                 unsigned int nr_chans,
>> +                                 unsigned long options)
>> +{
>> +     struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                struct stm_drvdata, stm);
>> +     if (!(drvdata && drvdata->enable))
>> +             return -EINVAL;
>> +
>> +     if (channel >= drvdata->numsp)
>> +             return -EINVAL;
>> +
>> +     switch (options) {
>> +     case STM_OPTION_GUARANTEED:
>> +             set_bit(channel, drvdata->chs.guaranteed);
>> +             break;
>> +
>> +     case STM_OPTION_INVARIANT:
>> +             clear_bit(channel, drvdata->chs.guaranteed);
>> +             break;
>
> This is a bad interface. Firstly, neither option is described
> anywhere. Secondly, I'm pretty sure "invariant" does not mean "not
> guaranteed" in english, although this function seems to imply this.

Regardless of the semantic associated to the word "invariant" in the
English language, the documentation characterises a channel configured
in best effort delivery mode as "invariant".  This is also the
opposite of the "guaranteed" mode where packets are guaranteed to be
delivered.  Adding a few comments here is probably a good idea.

>
>> +
>> +     default:
>> +             return -EINVAL;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static long stm_generic_get_options(struct stm_data *stm_data,
>> +                                 unsigned int master,
>> +                                 unsigned int channel,
>> +                                 unsigned int nr_chans,
>> +                                 u64 *options)
>> +{
>> +     struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                struct stm_drvdata, stm);
>> +     if (!(drvdata && drvdata->enable))
>> +             return -EINVAL;
>> +
>> +     if (channel >= drvdata->numsp)
>> +             return -EINVAL;
>> +
>> +     switch (*options) {
>> +     case STM_OPTION_GUARANTEED:
>> +             *options = test_bit(channel, drvdata->chs.guaranteed);
>> +             break;
>
> This just doesn't work. @options here is an on-stack variable in
> stm_char_ioctl(), hitherto uninitialized. The get_options ioctl command
> as you implemented it doesn't fetch @options from userspace either, it
> just passes a pointer to it to this callback, expecting that the
> callback will set it so that it can be copy_to_user()ed back to the
> user.

Yes, that was the intended behaviour - to allow user space to see in
what mode channels are configured (guaranteed/invariant).

>
> Then, when we figure this out, there is again the question of what
> should one make of STM_OPTION_{GUARANTEED,INVARIANT} and how do they fit
> into *options.

The interface asks if the channel is configured in "guaranteed" mode.
If not and because there isn't another mode available, it is
automatically in "invariant" mode.

But as I pointed out in my earlier email this may no longer needed.
One has to issue a system call anyway, might as well just go ahead
with the configuration request.

Thanks,
Mathieu

>
> The idea behind set_options ioctl is that the user specifies a bit mask
> of options that he/she wants to set.
>
> [snip]
>
>> +#ifndef __UAPI_CORESIGHT_STM_H_
>> +#define __UAPI_CORESIGHT_STM_H_
>> +
>> +#define STM_FLAG_TIMESTAMPED   BIT(3)
>> +#define STM_FLAG_GUARANTEED    BIT(7)
>> +
>> +enum {
>> +     STM_OPTION_GUARANTEED = 0,
>> +     STM_OPTION_INVARIANT,
>> +};
>
> Each of these guys could also use an explanation.
>
> Regards,
> --
> Alex
Chunyan Zhang Feb. 17, 2016, 3:18 a.m. UTC | #6
Hi Michael,

One question below need to be clarified.

On Fri, Feb 12, 2016 at 10:55 PM, Michael Williams
<Michael.Williams@arm.com> wrote:
> Mathieu Poirier [mailto:mathieu.poirier@linaro.org] wrote:
>> On 6 February 2016 at 04:04, Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
>>> From: Pratik Patel <pratikp@codeaurora.org>
>>>
>>> This driver adds support for the STM CoreSight IP block, allowing any
>>> system compoment (HW or SW) to log and aggregate messages via a
>>> single entity.
>>>
>>> The CoreSight STM exposes an application defined number of channels
>>> called stimulus port.  Configuration is done using entries in sysfs
>>> and channels made available to userspace via configfs.
>>>
>>> Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>>> ---
>>>  .../ABI/testing/sysfs-bus-coresight-devices-stm    |  53 ++
>>>  Documentation/trace/coresight.txt                  |  37 +-
>>>  drivers/hwtracing/coresight/Kconfig                |  11 +
>>>  drivers/hwtracing/coresight/Makefile               |   1 +
>>>  drivers/hwtracing/coresight/coresight-stm.c        | 928 +++++++++++++++++++++
>>>  include/linux/coresight-stm.h                      |   6 +
>>>  include/uapi/linux/coresight-stm.h                 |  12 +
>>>  7 files changed, 1046 insertions(+), 2 deletions(-)
>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>>>  create mode 100644 drivers/hwtracing/coresight/coresight-stm.c
>>>  create mode 100644 include/linux/coresight-stm.h
>>>  create mode 100644 include/uapi/linux/coresight-stm.h
>>>

[...]

>>> +static int stm_send(void *addr, const void *data, u32 size, u8 max)
>>> +{
>>> +       u32 len = size;
>>> +       u8 paload[8];
>>> +
>>> +       if (stm_addr_unaligned(data, max)) {
>>> +               memcpy(paload, data, size);
>>> +               data = paload;
>>> +       }
>>> +
>>> +       /* now we are 64bit/32bit aligned */
>>> +#ifdef CONFIG_64BIT
>>> +       if (size == 8)
>>> +               writeq_relaxed(*(u64 *)data, addr);
>>> +#endif
>>
>> We probably don't need an #ifdef here.  Checking the size of the
>> transfer against the fundamental data size will result in the same
>> outcome, i.e preventing 64 bit transfers on a 32 bit architecture.  An
>> error (understandable by the caller) should probably be returned in
>> this case.
>
> In theory this is correct, but if the code is unreachable for non-32-bit architectures, why include it at all?

                            ^^^^^^^
I guess you mean the code is unreachable for "non-64-bit" architectures?

>
>>> +       if (size >= 4) {
>>> +               writel_relaxed(*(u32 *)data, addr);
>>> +               data += 4;
>>> +               size -= 4;
>>> +       }
>>> +
>>> +       if (size >= 2) {
>>> +               writew_relaxed(*(u16 *)data, addr);
>>> +               data += 2;
>>> +               size -= 2;
>>> +       }
>>> +
>>> +       if (size == 1)
>>> +               writeb_relaxed(*(u8 *)data, addr);
>>> +
>>> +       return len;
>>> +}
>
> The API for stm_data::packet (include/linux/stm.h) is not wholly clearly defined, but it does state "sends an STP packet," which I interpret as meaning exactly one packet and no more. This function (which is called from this module's implementation of stm_data::packet) will send multiple STP packets if called with a value that is not exactly 8, 4, 2, or 1.

Yes, understand. Since the only first packet in one STM trace should
be marked timestamp, others should not be(their timestamps are same),
the function "stm_send" should indeed send one STP packet at a time.

I will address this in the next version.

Thanks for your time,
Chunyan

>
> To send only one packet, either:
>
> * These "if" statements are replaced with "else if" (as appropriate), and the function returns the size of data consumed; or
> * The stm_generic_packet() function (below) forces "size" to a power-of-two (<= write_max) before calling stm_send().
>
>>> +static int stm_generic_link(struct stm_data *stm_data,
>>> +                           unsigned int master,  unsigned int channel)
>>> +{
>>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>>> +                                                  struct stm_drvdata, stm);
>>> +       if (!drvdata || !drvdata->csdev)
>>> +               return -EINVAL;
>>> +
>>> +       return coresight_enable(drvdata->csdev);
>>> +}
>>> +
>>> +static void stm_generic_unlink(struct stm_data *stm_data,
>>> +                              unsigned int master,  unsigned int channel)
>>> +{
>>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>>> +                                                  struct stm_drvdata, stm);
>>> +       if (!drvdata || !drvdata->csdev)
>>> +               return;
>>> +
>>> +       stm_disable(drvdata->csdev);
>>> +}
>>> +
>>> +static long stm_generic_set_options(struct stm_data *stm_data,
>>> +                                   unsigned int master,
>>> +                                   unsigned int channel,
>>> +                                   unsigned int nr_chans,
>>> +                                   unsigned long options)
>>> +{
>>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>>> +                                                  struct stm_drvdata, stm);
>>> +       if (!(drvdata && drvdata->enable))
>>> +               return -EINVAL;
>>> +
>>> +       if (channel >= drvdata->numsp)
>>> +               return -EINVAL;
>>> +
>>> +       switch (options) {
>>> +       case STM_OPTION_GUARANTEED:
>>> +               set_bit(channel, drvdata->chs.guaranteed);
>>> +               break;
>>> +
>>> +       case STM_OPTION_INVARIANT:
>>> +               clear_bit(channel, drvdata->chs.guaranteed);
>>> +               break;
>>> +
>>> +       default:
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static long stm_generic_get_options(struct stm_data *stm_data,
>>> +                                   unsigned int master,
>>> +                                   unsigned int channel,
>>> +                                   unsigned int nr_chans,
>>> +                                   u64 *options)
>>> +{
>>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>>> +                                                  struct stm_drvdata, stm);
>>> +       if (!(drvdata && drvdata->enable))
>>> +               return -EINVAL;
>>> +
>>> +       if (channel >= drvdata->numsp)
>>> +               return -EINVAL;
>>> +
>>> +       switch (*options) {
>>> +       case STM_OPTION_GUARANTEED:
>>> +               *options = test_bit(channel, drvdata->chs.guaranteed);
>>> +               break;
>>> +
>>> +       default:
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static ssize_t stm_generic_packet(struct stm_data *stm_data,
>>> +                                 unsigned int master,
>>> +                                 unsigned int channel,
>>> +                                 unsigned int packet,
>>> +                                 unsigned int flags,
>>> +                                 unsigned int size,
>>> +                                 const unsigned char *payload)
>>> +{
>>> +       unsigned long ch_addr;
>>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>>> +                                                  struct stm_drvdata, stm);
>>> +
>>> +       if (!(drvdata && drvdata->enable))
>>> +               return 0;
>>> +
>>> +       if (channel >= drvdata->numsp)
>>> +               return 0;
>>> +
>>> +       ch_addr = (unsigned long)stm_channel_addr(drvdata, channel);
>>> +
>>> +       flags = (flags == STP_PACKET_TIMESTAMPED) ? STM_FLAG_TIMESTAMPED : 0;
>>> +       flags |= test_bit(channel, drvdata->chs.guaranteed) ?
>>> +                          STM_FLAG_GUARANTEED : 0;
>>> +
>>> +       if (size > drvdata->write_max)
>>> +               size = drvdata->write_max;
>>> +
>>> +       switch (packet) {
>>> +       case STP_PACKET_FLAG:
>>> +               ch_addr |= stm_channel_off(STM_PKT_TYPE_FLAG, flags);
>>> +
>>> +               /* the generic STM API set a size of zero on flag packets. */
>>> +               size = 1;
>>> +               break;
>>> +
>>> +       case STP_PACKET_DATA:
>>> +               ch_addr |= stm_channel_off(STM_PKT_TYPE_DATA, flags);
>>> +               break;
>>> +
>>> +       default:
>>> +               return 0;
>>> +       }
>>> +
>>> +       return stm_send((void *)ch_addr, payload, size, drvdata->write_max);
>>> +}
>>> +
>>> +static ssize_t hwevent_enable_show(struct device *dev,
>>> +                                  struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val = drvdata->stmheer;
>>> +
>>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>>> +}
>>> +
>>> +static ssize_t hwevent_enable_store(struct device *dev,
>>> +                                   struct device_attribute *attr,
>>> +                                   const char *buf, size_t size)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val;
>>> +       int ret = 0;
>>> +
>>> +       ret = kstrtoul(buf, 16, &val);
>>> +       if (ret)
>>> +               return -EINVAL;
>>> +
>>> +       drvdata->stmheer = val;
>>> +       /* HW event enable and trigger go hand in hand */
>>> +       drvdata->stmheter = val;
>>> +
>>> +       return size;
>>> +}
>>> +static DEVICE_ATTR_RW(hwevent_enable);
>>> +
>>> +static ssize_t hwevent_select_show(struct device *dev,
>>> +                                  struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val = drvdata->stmhebsr;
>>> +
>>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>>> +}
>>> +
>>> +static ssize_t hwevent_select_store(struct device *dev,
>>> +                                   struct device_attribute *attr,
>>> +                                   const char *buf, size_t size)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val;
>>> +       int ret = 0;
>>> +
>>> +       ret = kstrtoul(buf, 16, &val);
>>> +       if (ret)
>>> +               return -EINVAL;
>>> +
>>> +       drvdata->stmhebsr = val;
>>> +
>>> +       return size;
>>> +}
>>> +static DEVICE_ATTR_RW(hwevent_select);
>>> +
>>> +static ssize_t port_select_show(struct device *dev,
>>> +                               struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val;
>>> +
>>> +       if (!drvdata->enable) {
>>> +               val = drvdata->stmspscr;
>>> +       } else {
>>> +               spin_lock(&drvdata->spinlock);
>>> +               val = readl_relaxed(drvdata->base + STMSPSCR);
>>> +               spin_unlock(&drvdata->spinlock);
>>> +       }
>>> +
>>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>>> +}
>>> +
>>> +static ssize_t port_select_store(struct device *dev,
>>> +                                struct device_attribute *attr,
>>> +                                const char *buf, size_t size)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val, stmsper;
>>> +       int ret = 0;
>>> +
>>> +       ret = kstrtoul(buf, 16, &val);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       spin_lock(&drvdata->spinlock);
>>> +       drvdata->stmspscr = val;
>>> +
>>> +       if (drvdata->enable) {
>>> +               CS_UNLOCK(drvdata->base);
>>> +               /* Process as per ARM's TRM recommendation */
>>> +               stmsper = readl_relaxed(drvdata->base + STMSPER);
>>> +               writel_relaxed(0x0, drvdata->base + STMSPER);
>>> +               writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR);
>>> +               writel_relaxed(stmsper, drvdata->base + STMSPER);
>>> +               CS_LOCK(drvdata->base);
>>> +       }
>>> +       spin_unlock(&drvdata->spinlock);
>>> +
>>> +       return size;
>>> +}
>>> +static DEVICE_ATTR_RW(port_select);
>>> +
>>> +static ssize_t port_enable_show(struct device *dev,
>>> +                               struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val;
>>> +
>>> +       if (!drvdata->enable) {
>>> +               val = drvdata->stmsper;
>>> +       } else {
>>> +               spin_lock(&drvdata->spinlock);
>>> +               val = readl_relaxed(drvdata->base + STMSPER);
>>> +               spin_unlock(&drvdata->spinlock);
>>> +       }
>>> +
>>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>>> +}
>>> +
>>> +static ssize_t port_enable_store(struct device *dev,
>>> +                                struct device_attribute *attr,
>>> +                                const char *buf, size_t size)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +       unsigned long val;
>>> +       int ret = 0;
>>> +
>>> +       ret = kstrtoul(buf, 16, &val);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       spin_lock(&drvdata->spinlock);
>>> +       drvdata->stmsper = val;
>>> +
>>> +       if (drvdata->enable) {
>>> +               CS_UNLOCK(drvdata->base);
>>> +               writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);
>>> +               CS_LOCK(drvdata->base);
>>> +       }
>>> +       spin_unlock(&drvdata->spinlock);
>>> +
>>> +       return size;
>>> +}
>>> +static DEVICE_ATTR_RW(port_enable);
>>> +
>>> +static ssize_t traceid_show(struct device *dev,
>>> +                           struct device_attribute *attr, char *buf)
>>> +{
>>> +       unsigned long val;
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +
>>> +       val = drvdata->traceid;
>>> +       return sprintf(buf, "%#lx\n", val);
>>> +}
>>> +
>>> +static ssize_t traceid_store(struct device *dev,
>>> +                            struct device_attribute *attr,
>>> +                            const char *buf, size_t size)
>>> +{
>>> +       int ret;
>>> +       unsigned long val;
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +
>>> +       ret = kstrtoul(buf, 16, &val);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       /* traceid field is 7bit wide on STM32 */
>>> +       drvdata->traceid = val & 0x7f;
>>> +       return size;
>>> +}
>>> +static DEVICE_ATTR_RW(traceid);
>>> +
>>> +#define coresight_simple_func(type, name, offset)                      \
>>> +static ssize_t name##_show(struct device *_dev,                                \
>>> +                          struct device_attribute *attr, char *buf)    \
>>> +{                                                                      \
>>> +       type *drvdata = dev_get_drvdata(_dev->parent);                  \
>>> +       return scnprintf(buf, PAGE_SIZE, "0x%08x\n",                    \
>>> +                        readl_relaxed(drvdata->base + offset));        \
>>> +}                                                                      \
>>> +DEVICE_ATTR_RO(name)
>>> +
>>> +#define coresight_stm_simple_func(name, offset)        \
>>> +       coresight_simple_func(struct stm_drvdata, name, offset)
>>> +
>>> +coresight_stm_simple_func(tcsr, STMTCSR);
>>> +coresight_stm_simple_func(tsfreqr, STMTSFREQR);
>>> +coresight_stm_simple_func(syncr, STMSYNCR);
>>> +coresight_stm_simple_func(sper, STMSPER);
>>> +coresight_stm_simple_func(spter, STMSPTER);
>>> +coresight_stm_simple_func(privmaskr, STMPRIVMASKR);
>>> +coresight_stm_simple_func(spscr, STMSPSCR);
>>> +coresight_stm_simple_func(spmscr, STMSPMSCR);
>>> +coresight_stm_simple_func(spfeat1r, STMSPFEAT1R);
>>> +coresight_stm_simple_func(spfeat2r, STMSPFEAT2R);
>>> +coresight_stm_simple_func(spfeat3r, STMSPFEAT3R);
>>> +coresight_stm_simple_func(devid, CORESIGHT_DEVID);
>>> +
>>> +static struct attribute *coresight_stm_attrs[] = {
>>> +       &dev_attr_hwevent_enable.attr,
>>> +       &dev_attr_hwevent_select.attr,
>>> +       &dev_attr_port_enable.attr,
>>> +       &dev_attr_port_select.attr,
>>> +       &dev_attr_traceid.attr,
>>> +       NULL,
>>> +};
>>> +
>>> +static struct attribute *coresight_stm_mgmt_attrs[] = {
>>> +       &dev_attr_tcsr.attr,
>>> +       &dev_attr_tsfreqr.attr,
>>> +       &dev_attr_syncr.attr,
>>> +       &dev_attr_sper.attr,
>>> +       &dev_attr_spter.attr,
>>> +       &dev_attr_privmaskr.attr,
>>> +       &dev_attr_spscr.attr,
>>> +       &dev_attr_spmscr.attr,
>>> +       &dev_attr_spfeat1r.attr,
>>> +       &dev_attr_spfeat2r.attr,
>>> +       &dev_attr_spfeat3r.attr,
>>> +       &dev_attr_devid.attr,
>>> +       NULL,
>>> +};
>>> +
>>> +static const struct attribute_group coresight_stm_group = {
>>> +       .attrs = coresight_stm_attrs,
>>> +};
>>> +
>>> +static const struct attribute_group coresight_stm_mgmt_group = {
>>> +       .attrs = coresight_stm_mgmt_attrs,
>>> +       .name = "mgmt",
>>> +};
>>> +
>>> +static const struct attribute_group *coresight_stm_groups[] = {
>>> +       &coresight_stm_group,
>>> +       &coresight_stm_mgmt_group,
>>> +       NULL,
>>> +};
>>> +
>>> +static int stm_get_resource_byname(struct device_node *np,
>>> +                                  char *ch_base, struct resource *res)
>>> +{
>>> +       const char *name = NULL;
>>> +       int index = 0, found = 0;
>>> +
>>> +       while (!of_property_read_string_index(np, "reg-names", index, &name)) {
>>> +               if (strcmp(ch_base, name)) {
>>> +                       index++;
>>> +                       continue;
>>> +               }
>>> +
>>> +               /* We have a match and @index is where it's at */
>>> +               found = 1;
>>> +               break;
>>> +       }
>>> +
>>> +       if (!found)
>>> +               return -EINVAL;
>>> +
>>> +       return of_address_to_resource(np, index, res);
>>> +}
>>> +
>>> +static u32 stm_fundamental_data_size(struct stm_drvdata *drvdata)
>>> +{
>>> +       u32 stmspfeat2r;
>>> +
>>> +       stmspfeat2r = readl_relaxed(drvdata->base + STMSPFEAT2R);
>>> +       return BMVAL(stmspfeat2r, 12, 15);
>>> +}
>>> +
>>> +static u32 stm_num_stimulus_port(struct stm_drvdata *drvdata)
>>> +{
>>> +       u32 numsp;
>>> +
>>> +       numsp = readl_relaxed(drvdata->base + CORESIGHT_DEVID);
>>> +       /*
>>> +        * NUMPS in STMDEVID is 17 bit long and if equal to 0x0,
>>> +        * 32 stimulus ports are supported.
>>> +        */
>>> +       numsp &= 0x1ffff;
>>> +       if (!numsp)
>>> +               numsp = STM_32_CHANNEL;
>>> +       return numsp;
>>> +}
>>> +
>>> +static void stm_init_default_data(struct stm_drvdata *drvdata)
>>> +{
>>> +       /* Don't use port selection */
>>> +       drvdata->stmspscr = 0x0;
>>> +       /*
>>> +        * Enable all channel regardless of their number.  When port
>>> +        * selection isn't used (see above) STMSPER applies to all
>>> +        * 32 channel group available, hence setting all 32 bits to 1
>>> +        */
>>> +       drvdata->stmsper = ~0x0;
>>> +
>>> +       /*
>>> +        * Select arbitrary value to start with.  If there is a conflict
>>> +        * with other tracers the framework will deal with it.
>>> +        */
>>> +       drvdata->traceid = 0x20;
>>> +
>>> +       /* Set invariant transaction timing on all channels */
>>> +       bitmap_clear(drvdata->chs.guaranteed, 0, drvdata->numsp);
>>> +}
>>> +
>>> +static void stm_init_generic_data(struct stm_drvdata *drvdata)
>>> +{
>>> +       drvdata->stm.name = dev_name(drvdata->dev);
>>> +       drvdata->stm.mstatic = true;
>>> +       drvdata->stm.sw_nchannels = drvdata->numsp;
>>> +       drvdata->stm.packet = stm_generic_packet;
>>> +       drvdata->stm.link = stm_generic_link;
>>> +       drvdata->stm.unlink = stm_generic_unlink;
>>> +       drvdata->stm.set_options = stm_generic_set_options;
>>> +       drvdata->stm.get_options = stm_generic_get_options;
>>> +}
>>> +
>>> +static int stm_probe(struct amba_device *adev, const struct amba_id *id)
>>> +{
>>> +       int ret;
>>> +       void __iomem *base;
>>> +       unsigned long *guaranteed;
>>> +       struct device *dev = &adev->dev;
>>> +       struct coresight_platform_data *pdata = NULL;
>>> +       struct stm_drvdata *drvdata;
>>> +       struct resource *res = &adev->res;
>>> +       struct resource ch_res;
>>> +       size_t res_size, bitmap_size;
>>> +       struct coresight_desc *desc;
>>> +       struct device_node *np = adev->dev.of_node;
>>> +
>>> +       if (np) {
>>> +               pdata = of_get_coresight_platform_data(dev, np);
>>> +               if (IS_ERR(pdata))
>>> +                       return PTR_ERR(pdata);
>>> +               adev->dev.platform_data = pdata;
>>> +       }
>>> +       drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>>> +       if (!drvdata)
>>> +               return -ENOMEM;
>>> +
>>> +       drvdata->dev = &adev->dev;
>>> +       drvdata->atclk = devm_clk_get(&adev->dev, "atclk"); /* optional */
>>> +       if (!IS_ERR(drvdata->atclk)) {
>>> +               ret = clk_prepare_enable(drvdata->atclk);
>>> +               if (ret)
>>> +                       return ret;
>>> +       }
>>> +       dev_set_drvdata(dev, drvdata);
>>> +
>>> +       base = devm_ioremap_resource(dev, res);
>>> +       if (IS_ERR(base))
>>> +               return PTR_ERR(base);
>>> +       drvdata->base = base;
>>> +
>>> +       ret = stm_get_resource_byname(np, "stm-stimulus-base", &ch_res);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       base = devm_ioremap_resource(dev, &ch_res);
>>> +       if (IS_ERR(base))
>>> +               return PTR_ERR(base);
>>> +       drvdata->chs.base = base;
>>> +
>>> +       drvdata->write_max = sizeof(u32);
>>
>> Isn't the fundamental data size the maximum an architecture can stand?
>>  Why not simply use that rather than adding another variable?
>>
>>> +       drvdata->write_64bit = stm_fundamental_data_size(drvdata);
>>> +
>>> +#ifndef CONFIG_64BIT
>>> +       if (drvdata->write_64bit)
>>> +               drvdata->write_max = sizeof(u64);
>>> +#endif
>
> It is a bit odd that this code is using sizeof(u64) / sizeof(u32) here, when it is using 8 / 4 explicitly in the functions above.
>
>>
>> In most cases the fundamental data size will follow the architecture
>> size, i.e, 32 bit architecture with 32 bit fundamental data size, 64
>> bit architecture with 64 bit fundamental data size.  A fundamental
>> data size of 32 is also possible on a 64 bit architecture.
>>
>> What needs to be prevented is a 32 bit architecture with a fundamental
>> data size of 64.  In those cases the fundamental data size should be
>> adjusted to be 32 bit.  To do that I suggest to modify
>> stm_fundamental_data_size() to probe the STM's fundamental data size
>> but to adjust it down if on a 32 bit system.  Knowing whether running
>> on a 32/64 bit system is easy when using the IS_ENABLED() macro.
>
> Agreed. For ARM, 64-bit writes to the STM are supported only if BOTH the STM supports 64-bit data and the processor is in 64-bit execution state. Otherwise, the largest size is 32 bits. You are likely to see both 64-bit STM from 32-bit execution state, and 32-bit STM on a SoC with a 64-bit ARM processor.
>
>>> +
>>> +       if (boot_nr_channel) {
>>> +               drvdata->numsp = boot_nr_channel;
>>> +               res_size = min((resource_size_t)(boot_nr_channel *
>>> +                                 BYTES_PER_CHANNEL), resource_size(res));
>>> +       } else {
>>> +               drvdata->numsp = stm_num_stimulus_port(drvdata);
>>> +               res_size = min((resource_size_t)(drvdata->numsp *
>>> +                                BYTES_PER_CHANNEL), resource_size(res));
>>> +       }
>>> +       bitmap_size = BITS_TO_LONGS(drvdata->numsp) * sizeof(long);
>>> +
>>> +       guaranteed = devm_kzalloc(dev, bitmap_size, GFP_KERNEL);
>>> +       if (!guaranteed)
>>> +               return -ENOMEM;
>>> +       drvdata->chs.guaranteed = guaranteed;
>>> +
>>> +       spin_lock_init(&drvdata->spinlock);
>>> +
>>> +       stm_init_default_data(drvdata);
>>> +       stm_init_generic_data(drvdata);
>>> +
>>> +       if (stm_register_device(dev, &drvdata->stm, THIS_MODULE)) {
>>> +               dev_info(dev,
>>> +                        "stm_register_device failed, probing deffered\n");
>>> +               return -EPROBE_DEFER;
>>> +       }
>>> +
>>> +       desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
>>> +       if (!desc) {
>>> +               ret = -ENOMEM;
>>> +               goto stm_unregister;
>>> +       }
>>> +
>>> +       desc->type = CORESIGHT_DEV_TYPE_SOURCE;
>>> +       desc->subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE;
>>> +       desc->ops = &stm_cs_ops;
>>> +       desc->pdata = pdata;
>>> +       desc->dev = dev;
>>> +       desc->groups = coresight_stm_groups;
>>> +       drvdata->csdev = coresight_register(desc);
>>> +       if (IS_ERR(drvdata->csdev)) {
>>> +               ret = PTR_ERR(drvdata->csdev);
>>> +               goto stm_unregister;
>>> +       }
>>> +
>>> +       pm_runtime_put(&adev->dev);
>>> +
>>> +       dev_info(dev, "%s initialized\n", (char *)id->data);
>>> +       return 0;
>>> +
>>> +stm_unregister:
>>> +       stm_unregister_device(&drvdata->stm);
>>> +       return ret;
>>> +}
>>> +
>>> +static int stm_remove(struct amba_device *adev)
>>> +{
>>> +       struct stm_drvdata *drvdata = amba_get_drvdata(adev);
>>> +
>>> +       stm_unregister_device(&drvdata->stm);
>>> +       coresight_unregister(drvdata->csdev);
>>> +       return 0;
>>> +}
>>
>> The xyz_remove() functions is not needed for coresight devices -
>> unbinding a device from its driver will no longer be possible in the
>> 4.6 cycle.
>>
>>> +
>>> +#ifdef CONFIG_PM
>>> +static int stm_runtime_suspend(struct device *dev)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev);
>>> +
>>> +       if (drvdata && !IS_ERR(drvdata->atclk))
>>> +               clk_disable_unprepare(drvdata->atclk);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int stm_runtime_resume(struct device *dev)
>>> +{
>>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev);
>>> +
>>> +       if (drvdata && !IS_ERR(drvdata->atclk))
>>> +               clk_prepare_enable(drvdata->atclk);
>>> +
>>> +       return 0;
>>> +}
>>> +#endif
>>> +
>>> +static const struct dev_pm_ops stm_dev_pm_ops = {
>>> +       SET_RUNTIME_PM_OPS(stm_runtime_suspend, stm_runtime_resume, NULL)
>>> +};
>>> +
>>> +static struct amba_id stm_ids[] = {
>>> +       {
>>> +               .id     = 0x0003b962,
>>> +               .mask   = 0x0003ffff,
>>> +               .data   = "STM32",
>>> +       },
>>> +       { 0, 0},
>>> +};
>>> +
>>> +static struct amba_driver stm_driver = {
>>> +       .drv = {
>>> +               .name   = "coresight-stm",
>>> +               .owner  = THIS_MODULE,
>>> +               .pm     = &stm_dev_pm_ops,
>>> +       },
>>> +       .probe          = stm_probe,
>>> +       .remove         = stm_remove,
>>> +       .id_table       = stm_ids,
>>> +};
>>> +
>>> +module_amba_driver(stm_driver);
>>> +
>>> +MODULE_LICENSE("GPL v2");
>>> +MODULE_DESCRIPTION("CoreSight System Trace Macrocell driver");
>>> diff --git a/include/linux/coresight-stm.h b/include/linux/coresight-stm.h
>>> new file mode 100644
>>> index 0000000..a978bb8
>>> --- /dev/null
>>> +++ b/include/linux/coresight-stm.h
>>> @@ -0,0 +1,6 @@
>>> +#ifndef __LINUX_CORESIGHT_STM_H_
>>> +#define __LINUX_CORESIGHT_STM_H_
>>> +
>>> +#include <uapi/linux/coresight-stm.h>
>>> +
>>> +#endif
>>> diff --git a/include/uapi/linux/coresight-stm.h b/include/uapi/linux/coresight-stm.h
>>> new file mode 100644
>>> index 0000000..fa35f0b
>>> --- /dev/null
>>> +++ b/include/uapi/linux/coresight-stm.h
>>> @@ -0,0 +1,12 @@
>>> +#ifndef __UAPI_CORESIGHT_STM_H_
>>> +#define __UAPI_CORESIGHT_STM_H_
>>> +
>>> +#define STM_FLAG_TIMESTAMPED   BIT(3)
>>> +#define STM_FLAG_GUARANTEED    BIT(7)
>>> +
>>> +enum {
>>> +       STM_OPTION_GUARANTEED = 0,
>>> +       STM_OPTION_INVARIANT,
>>> +};
>>> +
>>> +#endif
>>> --
>>> 1.9.1
>>>
>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Chunyan Zhang Feb. 17, 2016, 3:49 a.m. UTC | #7
On Fri, Feb 12, 2016 at 12:59 AM, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
> On 6 February 2016 at 04:04, Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
>> From: Pratik Patel <pratikp@codeaurora.org>
>>
>> This driver adds support for the STM CoreSight IP block, allowing any
>> system compoment (HW or SW) to log and aggregate messages via a
>> single entity.
>>
>> The CoreSight STM exposes an application defined number of channels
>> called stimulus port.  Configuration is done using entries in sysfs
>> and channels made available to userspace via configfs.
>>
>> Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>> ---
>>  .../ABI/testing/sysfs-bus-coresight-devices-stm    |  53 ++
>>  Documentation/trace/coresight.txt                  |  37 +-
>>  drivers/hwtracing/coresight/Kconfig                |  11 +
>>  drivers/hwtracing/coresight/Makefile               |   1 +
>>  drivers/hwtracing/coresight/coresight-stm.c        | 928 +++++++++++++++++++++
>>  include/linux/coresight-stm.h                      |   6 +
>>  include/uapi/linux/coresight-stm.h                 |  12 +
>>  7 files changed, 1046 insertions(+), 2 deletions(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>>  create mode 100644 drivers/hwtracing/coresight/coresight-stm.c
>>  create mode 100644 include/linux/coresight-stm.h
>>  create mode 100644 include/uapi/linux/coresight-stm.h
>>

[...]

>> +static int stm_send(void *addr, const void *data, u32 size, u8 max)
>> +{
>> +       u32 len = size;
>> +       u8 paload[8];
>> +
>> +       if (stm_addr_unaligned(data, max)) {
>> +               memcpy(paload, data, size);
>> +               data = paload;
>> +       }
>> +
>> +       /* now we are 64bit/32bit aligned */
>> +#ifdef CONFIG_64BIT
>> +       if (size == 8)
>> +               writeq_relaxed(*(u64 *)data, addr);
>> +#endif
>
> We probably don't need an #ifdef here.  Checking the size of the
> transfer against the fundamental data size will result in the same
> outcome, i.e preventing 64 bit transfers on a 32 bit architecture.  An
> error (understandable by the caller) should probably be returned in
> this case.

I think we still need this #ifdef, since this checking is only for
64-bit architecture, on 32-bit architecture it's unreachable, since if
the transfer data size on 32-bit architecture is larger than the
32-bit fundamental data size, it would be adjusted down in the caller
of this function.

>
>> +       if (size >= 4) {
>> +               writel_relaxed(*(u32 *)data, addr);
>> +               data += 4;
>> +               size -= 4;
>> +       }
>> +
>> +       if (size >= 2) {
>> +               writew_relaxed(*(u16 *)data, addr);
>> +               data += 2;
>> +               size -= 2;
>> +       }
>> +
>> +       if (size == 1)
>> +               writeb_relaxed(*(u8 *)data, addr);
>> +
>> +       return len;
>> +}
>> +
>> +static int stm_generic_link(struct stm_data *stm_data,
>> +                           unsigned int master,  unsigned int channel)
>> +{
>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                  struct stm_drvdata, stm);
>> +       if (!drvdata || !drvdata->csdev)
>> +               return -EINVAL;
>> +
>> +       return coresight_enable(drvdata->csdev);
>> +}
>> +
>> +static void stm_generic_unlink(struct stm_data *stm_data,
>> +                              unsigned int master,  unsigned int channel)
>> +{
>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                  struct stm_drvdata, stm);
>> +       if (!drvdata || !drvdata->csdev)
>> +               return;
>> +
>> +       stm_disable(drvdata->csdev);
>> +}
>> +
>> +static long stm_generic_set_options(struct stm_data *stm_data,
>> +                                   unsigned int master,
>> +                                   unsigned int channel,
>> +                                   unsigned int nr_chans,
>> +                                   unsigned long options)
>> +{
>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                  struct stm_drvdata, stm);
>> +       if (!(drvdata && drvdata->enable))
>> +               return -EINVAL;
>> +
>> +       if (channel >= drvdata->numsp)
>> +               return -EINVAL;
>> +
>> +       switch (options) {
>> +       case STM_OPTION_GUARANTEED:
>> +               set_bit(channel, drvdata->chs.guaranteed);
>> +               break;
>> +
>> +       case STM_OPTION_INVARIANT:
>> +               clear_bit(channel, drvdata->chs.guaranteed);
>> +               break;
>> +
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static long stm_generic_get_options(struct stm_data *stm_data,
>> +                                   unsigned int master,
>> +                                   unsigned int channel,
>> +                                   unsigned int nr_chans,
>> +                                   u64 *options)
>> +{
>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                  struct stm_drvdata, stm);
>> +       if (!(drvdata && drvdata->enable))
>> +               return -EINVAL;
>> +
>> +       if (channel >= drvdata->numsp)
>> +               return -EINVAL;
>> +
>> +       switch (*options) {
>> +       case STM_OPTION_GUARANTEED:
>> +               *options = test_bit(channel, drvdata->chs.guaranteed);
>> +               break;
>> +
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static ssize_t stm_generic_packet(struct stm_data *stm_data,
>> +                                 unsigned int master,
>> +                                 unsigned int channel,
>> +                                 unsigned int packet,
>> +                                 unsigned int flags,
>> +                                 unsigned int size,
>> +                                 const unsigned char *payload)
>> +{
>> +       unsigned long ch_addr;
>> +       struct stm_drvdata *drvdata = container_of(stm_data,
>> +                                                  struct stm_drvdata, stm);
>> +
>> +       if (!(drvdata && drvdata->enable))
>> +               return 0;
>> +
>> +       if (channel >= drvdata->numsp)
>> +               return 0;
>> +
>> +       ch_addr = (unsigned long)stm_channel_addr(drvdata, channel);
>> +
>> +       flags = (flags == STP_PACKET_TIMESTAMPED) ? STM_FLAG_TIMESTAMPED : 0;
>> +       flags |= test_bit(channel, drvdata->chs.guaranteed) ?
>> +                          STM_FLAG_GUARANTEED : 0;
>> +
>> +       if (size > drvdata->write_max)
>> +               size = drvdata->write_max;
>> +
>> +       switch (packet) {
>> +       case STP_PACKET_FLAG:
>> +               ch_addr |= stm_channel_off(STM_PKT_TYPE_FLAG, flags);
>> +
>> +               /* the generic STM API set a size of zero on flag packets. */
>> +               size = 1;
>> +               break;
>> +
>> +       case STP_PACKET_DATA:
>> +               ch_addr |= stm_channel_off(STM_PKT_TYPE_DATA, flags);
>> +               break;
>> +
>> +       default:
>> +               return 0;
>> +       }
>> +
>> +       return stm_send((void *)ch_addr, payload, size, drvdata->write_max);
>> +}
>> +
>> +static ssize_t hwevent_enable_show(struct device *dev,
>> +                                  struct device_attribute *attr, char *buf)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val = drvdata->stmheer;
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>> +}
>> +
>> +static ssize_t hwevent_enable_store(struct device *dev,
>> +                                   struct device_attribute *attr,
>> +                                   const char *buf, size_t size)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val;
>> +       int ret = 0;
>> +
>> +       ret = kstrtoul(buf, 16, &val);
>> +       if (ret)
>> +               return -EINVAL;
>> +
>> +       drvdata->stmheer = val;
>> +       /* HW event enable and trigger go hand in hand */
>> +       drvdata->stmheter = val;
>> +
>> +       return size;
>> +}
>> +static DEVICE_ATTR_RW(hwevent_enable);
>> +
>> +static ssize_t hwevent_select_show(struct device *dev,
>> +                                  struct device_attribute *attr, char *buf)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val = drvdata->stmhebsr;
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>> +}
>> +
>> +static ssize_t hwevent_select_store(struct device *dev,
>> +                                   struct device_attribute *attr,
>> +                                   const char *buf, size_t size)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val;
>> +       int ret = 0;
>> +
>> +       ret = kstrtoul(buf, 16, &val);
>> +       if (ret)
>> +               return -EINVAL;
>> +
>> +       drvdata->stmhebsr = val;
>> +
>> +       return size;
>> +}
>> +static DEVICE_ATTR_RW(hwevent_select);
>> +
>> +static ssize_t port_select_show(struct device *dev,
>> +                               struct device_attribute *attr, char *buf)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val;
>> +
>> +       if (!drvdata->enable) {
>> +               val = drvdata->stmspscr;
>> +       } else {
>> +               spin_lock(&drvdata->spinlock);
>> +               val = readl_relaxed(drvdata->base + STMSPSCR);
>> +               spin_unlock(&drvdata->spinlock);
>> +       }
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>> +}
>> +
>> +static ssize_t port_select_store(struct device *dev,
>> +                                struct device_attribute *attr,
>> +                                const char *buf, size_t size)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val, stmsper;
>> +       int ret = 0;
>> +
>> +       ret = kstrtoul(buf, 16, &val);
>> +       if (ret)
>> +               return ret;
>> +
>> +       spin_lock(&drvdata->spinlock);
>> +       drvdata->stmspscr = val;
>> +
>> +       if (drvdata->enable) {
>> +               CS_UNLOCK(drvdata->base);
>> +               /* Process as per ARM's TRM recommendation */
>> +               stmsper = readl_relaxed(drvdata->base + STMSPER);
>> +               writel_relaxed(0x0, drvdata->base + STMSPER);
>> +               writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR);
>> +               writel_relaxed(stmsper, drvdata->base + STMSPER);
>> +               CS_LOCK(drvdata->base);
>> +       }
>> +       spin_unlock(&drvdata->spinlock);
>> +
>> +       return size;
>> +}
>> +static DEVICE_ATTR_RW(port_select);
>> +
>> +static ssize_t port_enable_show(struct device *dev,
>> +                               struct device_attribute *attr, char *buf)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val;
>> +
>> +       if (!drvdata->enable) {
>> +               val = drvdata->stmsper;
>> +       } else {
>> +               spin_lock(&drvdata->spinlock);
>> +               val = readl_relaxed(drvdata->base + STMSPER);
>> +               spin_unlock(&drvdata->spinlock);
>> +       }
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>> +}
>> +
>> +static ssize_t port_enable_store(struct device *dev,
>> +                                struct device_attribute *attr,
>> +                                const char *buf, size_t size)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       unsigned long val;
>> +       int ret = 0;
>> +
>> +       ret = kstrtoul(buf, 16, &val);
>> +       if (ret)
>> +               return ret;
>> +
>> +       spin_lock(&drvdata->spinlock);
>> +       drvdata->stmsper = val;
>> +
>> +       if (drvdata->enable) {
>> +               CS_UNLOCK(drvdata->base);
>> +               writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);
>> +               CS_LOCK(drvdata->base);
>> +       }
>> +       spin_unlock(&drvdata->spinlock);
>> +
>> +       return size;
>> +}
>> +static DEVICE_ATTR_RW(port_enable);
>> +
>> +static ssize_t traceid_show(struct device *dev,
>> +                           struct device_attribute *attr, char *buf)
>> +{
>> +       unsigned long val;
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +       val = drvdata->traceid;
>> +       return sprintf(buf, "%#lx\n", val);
>> +}
>> +
>> +static ssize_t traceid_store(struct device *dev,
>> +                            struct device_attribute *attr,
>> +                            const char *buf, size_t size)
>> +{
>> +       int ret;
>> +       unsigned long val;
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +       ret = kstrtoul(buf, 16, &val);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* traceid field is 7bit wide on STM32 */
>> +       drvdata->traceid = val & 0x7f;
>> +       return size;
>> +}
>> +static DEVICE_ATTR_RW(traceid);
>> +
>> +#define coresight_simple_func(type, name, offset)                      \
>> +static ssize_t name##_show(struct device *_dev,                                \
>> +                          struct device_attribute *attr, char *buf)    \
>> +{                                                                      \
>> +       type *drvdata = dev_get_drvdata(_dev->parent);                  \
>> +       return scnprintf(buf, PAGE_SIZE, "0x%08x\n",                    \
>> +                        readl_relaxed(drvdata->base + offset));        \
>> +}                                                                      \
>> +DEVICE_ATTR_RO(name)
>> +
>> +#define coresight_stm_simple_func(name, offset)        \
>> +       coresight_simple_func(struct stm_drvdata, name, offset)
>> +
>> +coresight_stm_simple_func(tcsr, STMTCSR);
>> +coresight_stm_simple_func(tsfreqr, STMTSFREQR);
>> +coresight_stm_simple_func(syncr, STMSYNCR);
>> +coresight_stm_simple_func(sper, STMSPER);
>> +coresight_stm_simple_func(spter, STMSPTER);
>> +coresight_stm_simple_func(privmaskr, STMPRIVMASKR);
>> +coresight_stm_simple_func(spscr, STMSPSCR);
>> +coresight_stm_simple_func(spmscr, STMSPMSCR);
>> +coresight_stm_simple_func(spfeat1r, STMSPFEAT1R);
>> +coresight_stm_simple_func(spfeat2r, STMSPFEAT2R);
>> +coresight_stm_simple_func(spfeat3r, STMSPFEAT3R);
>> +coresight_stm_simple_func(devid, CORESIGHT_DEVID);
>> +
>> +static struct attribute *coresight_stm_attrs[] = {
>> +       &dev_attr_hwevent_enable.attr,
>> +       &dev_attr_hwevent_select.attr,
>> +       &dev_attr_port_enable.attr,
>> +       &dev_attr_port_select.attr,
>> +       &dev_attr_traceid.attr,
>> +       NULL,
>> +};
>> +
>> +static struct attribute *coresight_stm_mgmt_attrs[] = {
>> +       &dev_attr_tcsr.attr,
>> +       &dev_attr_tsfreqr.attr,
>> +       &dev_attr_syncr.attr,
>> +       &dev_attr_sper.attr,
>> +       &dev_attr_spter.attr,
>> +       &dev_attr_privmaskr.attr,
>> +       &dev_attr_spscr.attr,
>> +       &dev_attr_spmscr.attr,
>> +       &dev_attr_spfeat1r.attr,
>> +       &dev_attr_spfeat2r.attr,
>> +       &dev_attr_spfeat3r.attr,
>> +       &dev_attr_devid.attr,
>> +       NULL,
>> +};
>> +
>> +static const struct attribute_group coresight_stm_group = {
>> +       .attrs = coresight_stm_attrs,
>> +};
>> +
>> +static const struct attribute_group coresight_stm_mgmt_group = {
>> +       .attrs = coresight_stm_mgmt_attrs,
>> +       .name = "mgmt",
>> +};
>> +
>> +static const struct attribute_group *coresight_stm_groups[] = {
>> +       &coresight_stm_group,
>> +       &coresight_stm_mgmt_group,
>> +       NULL,
>> +};
>> +
>> +static int stm_get_resource_byname(struct device_node *np,
>> +                                  char *ch_base, struct resource *res)
>> +{
>> +       const char *name = NULL;
>> +       int index = 0, found = 0;
>> +
>> +       while (!of_property_read_string_index(np, "reg-names", index, &name)) {
>> +               if (strcmp(ch_base, name)) {
>> +                       index++;
>> +                       continue;
>> +               }
>> +
>> +               /* We have a match and @index is where it's at */
>> +               found = 1;
>> +               break;
>> +       }
>> +
>> +       if (!found)
>> +               return -EINVAL;
>> +
>> +       return of_address_to_resource(np, index, res);
>> +}
>> +
>> +static u32 stm_fundamental_data_size(struct stm_drvdata *drvdata)
>> +{
>> +       u32 stmspfeat2r;
>> +
>> +       stmspfeat2r = readl_relaxed(drvdata->base + STMSPFEAT2R);
>> +       return BMVAL(stmspfeat2r, 12, 15);
>> +}
>> +
>> +static u32 stm_num_stimulus_port(struct stm_drvdata *drvdata)
>> +{
>> +       u32 numsp;
>> +
>> +       numsp = readl_relaxed(drvdata->base + CORESIGHT_DEVID);
>> +       /*
>> +        * NUMPS in STMDEVID is 17 bit long and if equal to 0x0,
>> +        * 32 stimulus ports are supported.
>> +        */
>> +       numsp &= 0x1ffff;
>> +       if (!numsp)
>> +               numsp = STM_32_CHANNEL;
>> +       return numsp;
>> +}
>> +
>> +static void stm_init_default_data(struct stm_drvdata *drvdata)
>> +{
>> +       /* Don't use port selection */
>> +       drvdata->stmspscr = 0x0;
>> +       /*
>> +        * Enable all channel regardless of their number.  When port
>> +        * selection isn't used (see above) STMSPER applies to all
>> +        * 32 channel group available, hence setting all 32 bits to 1
>> +        */
>> +       drvdata->stmsper = ~0x0;
>> +
>> +       /*
>> +        * Select arbitrary value to start with.  If there is a conflict
>> +        * with other tracers the framework will deal with it.
>> +        */
>> +       drvdata->traceid = 0x20;
>> +
>> +       /* Set invariant transaction timing on all channels */
>> +       bitmap_clear(drvdata->chs.guaranteed, 0, drvdata->numsp);
>> +}
>> +
>> +static void stm_init_generic_data(struct stm_drvdata *drvdata)
>> +{
>> +       drvdata->stm.name = dev_name(drvdata->dev);
>> +       drvdata->stm.mstatic = true;
>> +       drvdata->stm.sw_nchannels = drvdata->numsp;
>> +       drvdata->stm.packet = stm_generic_packet;
>> +       drvdata->stm.link = stm_generic_link;
>> +       drvdata->stm.unlink = stm_generic_unlink;
>> +       drvdata->stm.set_options = stm_generic_set_options;
>> +       drvdata->stm.get_options = stm_generic_get_options;
>> +}
>> +
>> +static int stm_probe(struct amba_device *adev, const struct amba_id *id)
>> +{
>> +       int ret;
>> +       void __iomem *base;
>> +       unsigned long *guaranteed;
>> +       struct device *dev = &adev->dev;
>> +       struct coresight_platform_data *pdata = NULL;
>> +       struct stm_drvdata *drvdata;
>> +       struct resource *res = &adev->res;
>> +       struct resource ch_res;
>> +       size_t res_size, bitmap_size;
>> +       struct coresight_desc *desc;
>> +       struct device_node *np = adev->dev.of_node;
>> +
>> +       if (np) {
>> +               pdata = of_get_coresight_platform_data(dev, np);
>> +               if (IS_ERR(pdata))
>> +                       return PTR_ERR(pdata);
>> +               adev->dev.platform_data = pdata;
>> +       }
>> +       drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> +       if (!drvdata)
>> +               return -ENOMEM;
>> +
>> +       drvdata->dev = &adev->dev;
>> +       drvdata->atclk = devm_clk_get(&adev->dev, "atclk"); /* optional */
>> +       if (!IS_ERR(drvdata->atclk)) {
>> +               ret = clk_prepare_enable(drvdata->atclk);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +       dev_set_drvdata(dev, drvdata);
>> +
>> +       base = devm_ioremap_resource(dev, res);
>> +       if (IS_ERR(base))
>> +               return PTR_ERR(base);
>> +       drvdata->base = base;
>> +
>> +       ret = stm_get_resource_byname(np, "stm-stimulus-base", &ch_res);
>> +       if (ret)
>> +               return ret;
>> +
>> +       base = devm_ioremap_resource(dev, &ch_res);
>> +       if (IS_ERR(base))
>> +               return PTR_ERR(base);
>> +       drvdata->chs.base = base;
>> +
>> +       drvdata->write_max = sizeof(u32);
>
> Isn't the fundamental data size the maximum an architecture can stand?
>  Why not simply use that rather than adding another variable?
>
>> +       drvdata->write_64bit = stm_fundamental_data_size(drvdata);
>> +
>> +#ifndef CONFIG_64BIT
>> +       if (drvdata->write_64bit)
>> +               drvdata->write_max = sizeof(u64);
>> +#endif
>
> In most cases the fundamental data size will follow the architecture
> size, i.e, 32 bit architecture with 32 bit fundamental data size, 64
> bit architecture with 64 bit fundamental data size.  A fundamental
> data size of 32 is also possible on a 64 bit architecture.
>
> What needs to be prevented is a 32 bit architecture with a fundamental
> data size of 64.  In those cases the fundamental data size should be
> adjusted to be 32 bit.  To do that I suggest to modify
> stm_fundamental_data_size() to probe the STM's fundamental data size

I will modify that according to your suggestion.

> but to adjust it down if on a 32 bit system.  Knowing whether running
> on a 32/64 bit system is easy when using the IS_ENABLED() macro.
>
>> +
>> +       if (boot_nr_channel) {
>> +               drvdata->numsp = boot_nr_channel;
>> +               res_size = min((resource_size_t)(boot_nr_channel *
>> +                                 BYTES_PER_CHANNEL), resource_size(res));
>> +       } else {
>> +               drvdata->numsp = stm_num_stimulus_port(drvdata);
>> +               res_size = min((resource_size_t)(drvdata->numsp *
>> +                                BYTES_PER_CHANNEL), resource_size(res));
>> +       }
>> +       bitmap_size = BITS_TO_LONGS(drvdata->numsp) * sizeof(long);
>> +
>> +       guaranteed = devm_kzalloc(dev, bitmap_size, GFP_KERNEL);
>> +       if (!guaranteed)
>> +               return -ENOMEM;
>> +       drvdata->chs.guaranteed = guaranteed;
>> +
>> +       spin_lock_init(&drvdata->spinlock);
>> +
>> +       stm_init_default_data(drvdata);
>> +       stm_init_generic_data(drvdata);
>> +
>> +       if (stm_register_device(dev, &drvdata->stm, THIS_MODULE)) {
>> +               dev_info(dev,
>> +                        "stm_register_device failed, probing deffered\n");
>> +               return -EPROBE_DEFER;
>> +       }
>> +
>> +       desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
>> +       if (!desc) {
>> +               ret = -ENOMEM;
>> +               goto stm_unregister;
>> +       }
>> +
>> +       desc->type = CORESIGHT_DEV_TYPE_SOURCE;
>> +       desc->subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE;
>> +       desc->ops = &stm_cs_ops;
>> +       desc->pdata = pdata;
>> +       desc->dev = dev;
>> +       desc->groups = coresight_stm_groups;
>> +       drvdata->csdev = coresight_register(desc);
>> +       if (IS_ERR(drvdata->csdev)) {
>> +               ret = PTR_ERR(drvdata->csdev);
>> +               goto stm_unregister;
>> +       }
>> +
>> +       pm_runtime_put(&adev->dev);
>> +
>> +       dev_info(dev, "%s initialized\n", (char *)id->data);
>> +       return 0;
>> +
>> +stm_unregister:
>> +       stm_unregister_device(&drvdata->stm);
>> +       return ret;
>> +}
>> +
>> +static int stm_remove(struct amba_device *adev)
>> +{
>> +       struct stm_drvdata *drvdata = amba_get_drvdata(adev);
>> +
>> +       stm_unregister_device(&drvdata->stm);
>> +       coresight_unregister(drvdata->csdev);
>> +       return 0;
>> +}
>
> The xyz_remove() functions is not needed for coresight devices -
> unbinding a device from its driver will no longer be possible in the
> 4.6 cycle.

OK, will remove this.

Thanks for your review,
Chunyan

>
>> +
>> +#ifdef CONFIG_PM
>> +static int stm_runtime_suspend(struct device *dev)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev);
>> +
>> +       if (drvdata && !IS_ERR(drvdata->atclk))
>> +               clk_disable_unprepare(drvdata->atclk);
>> +
>> +       return 0;
>> +}
>> +
>> +static int stm_runtime_resume(struct device *dev)
>> +{
>> +       struct stm_drvdata *drvdata = dev_get_drvdata(dev);
>> +
>> +       if (drvdata && !IS_ERR(drvdata->atclk))
>> +               clk_prepare_enable(drvdata->atclk);
>> +
>> +       return 0;
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops stm_dev_pm_ops = {
>> +       SET_RUNTIME_PM_OPS(stm_runtime_suspend, stm_runtime_resume, NULL)
>> +};
>> +
>> +static struct amba_id stm_ids[] = {
>> +       {
>> +               .id     = 0x0003b962,
>> +               .mask   = 0x0003ffff,
>> +               .data   = "STM32",
>> +       },
>> +       { 0, 0},
>> +};
>> +
>> +static struct amba_driver stm_driver = {
>> +       .drv = {
>> +               .name   = "coresight-stm",
>> +               .owner  = THIS_MODULE,
>> +               .pm     = &stm_dev_pm_ops,
>> +       },
>> +       .probe          = stm_probe,
>> +       .remove         = stm_remove,
>> +       .id_table       = stm_ids,
>> +};
>> +
>> +module_amba_driver(stm_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("CoreSight System Trace Macrocell driver");
>> diff --git a/include/linux/coresight-stm.h b/include/linux/coresight-stm.h
>> new file mode 100644
>> index 0000000..a978bb8
>> --- /dev/null
>> +++ b/include/linux/coresight-stm.h
>> @@ -0,0 +1,6 @@
>> +#ifndef __LINUX_CORESIGHT_STM_H_
>> +#define __LINUX_CORESIGHT_STM_H_
>> +
>> +#include <uapi/linux/coresight-stm.h>
>> +
>> +#endif
>> diff --git a/include/uapi/linux/coresight-stm.h b/include/uapi/linux/coresight-stm.h
>> new file mode 100644
>> index 0000000..fa35f0b
>> --- /dev/null
>> +++ b/include/uapi/linux/coresight-stm.h
>> @@ -0,0 +1,12 @@
>> +#ifndef __UAPI_CORESIGHT_STM_H_
>> +#define __UAPI_CORESIGHT_STM_H_
>> +
>> +#define STM_FLAG_TIMESTAMPED   BIT(3)
>> +#define STM_FLAG_GUARANTEED    BIT(7)
>> +
>> +enum {
>> +       STM_OPTION_GUARANTEED = 0,
>> +       STM_OPTION_INVARIANT,
>> +};
>> +
>> +#endif
>> --
>> 1.9.1
>>
Michael Williams (ATG) Feb. 17, 2016, 12:08 p.m. UTC | #8
Hi Chunyan,

Chunyan Zhang wrote on 2016-02-17:
> Hi Michael,
>
> One question below need to be clarified.
>
> On Fri, Feb 12, 2016 at 10:55 PM, Michael Williams
> <Michael.Williams@arm.com> wrote:
>> Mathieu Poirier [mailto:mathieu.poirier@linaro.org] wrote:
>>> On 6 February 2016 at 04:04, Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
>>>> From: Pratik Patel <pratikp@codeaurora.org>
>>>>
>>>> This driver adds support for the STM CoreSight IP block, allowing any
>>>> system compoment (HW or SW) to log and aggregate messages via a
>>>> single entity.
>>>>
>>>> The CoreSight STM exposes an application defined number of channels
>>>> called stimulus port.  Configuration is done using entries in sysfs
>>>> and channels made available to userspace via configfs.
>>>>
>>>> Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
>>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>>>> ---
>>>>  .../ABI/testing/sysfs-bus-coresight-devices-stm    |  53 ++
>>>>  Documentation/trace/coresight.txt                  |  37 +-
>>>>  drivers/hwtracing/coresight/Kconfig                |  11 +
>>>>  drivers/hwtracing/coresight/Makefile               |   1 +
>>>>  drivers/hwtracing/coresight/coresight-stm.c        | 928 +++++++++++++++++++++
>>>>  include/linux/coresight-stm.h                      |   6 +
>>>>  include/uapi/linux/coresight-stm.h                 |  12 +
>>>>  7 files changed, 1046 insertions(+), 2 deletions(-)
>>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
>>>>  create mode 100644 drivers/hwtracing/coresight/coresight-stm.c
>>>>  create mode 100644 include/linux/coresight-stm.h
>>>>  create mode 100644 include/uapi/linux/coresight-stm.h
>
> [...]
>
>>>> +static int stm_send(void *addr, const void *data, u32 size, u8 max)
>>>> +{
>>>> +       u32 len = size;
>>>> +       u8 paload[8];
>>>> +
>>>> +       if (stm_addr_unaligned(data, max)) {
>>>> +               memcpy(paload, data, size);
>>>> +               data = paload;
>>>> +       }
>>>> +
>>>> +       /* now we are 64bit/32bit aligned */
>>>> +#ifdef CONFIG_64BIT
>>>> +       if (size == 8)
>>>> +               writeq_relaxed(*(u64 *)data, addr);
>>>> +#endif
>>>
>>> We probably don't need an #ifdef here.  Checking the size of the
>>> transfer against the fundamental data size will result in the same
>>> outcome, i.e preventing 64 bit transfers on a 32 bit architecture.  An
>>> error (understandable by the caller) should probably be returned in
>>> this case.
>>
>> In theory this is correct, but if the code is unreachable for non-32-bit architectures,
> why include it at all?
>
>                             ^^^^^^^
> I guess you mean the code is unreachable for "non-64-bit" architectures?

Yes; apologies for my mistake!

[snip]

Mike

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
diff mbox

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
new file mode 100644
index 0000000..a1d7022
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
@@ -0,0 +1,53 @@ 
+What:		/sys/bus/coresight/devices/<memory_map>.stm/enable_source
+Date:		February 2016
+KernelVersion:	4.5
+Contact:	Mathieu Poirier <mathieu.poirier@linaro.org>
+Description:	(RW) Enable/disable tracing on this specific trace macrocell.
+		Enabling the trace macrocell implies it has been configured
+		properly and a sink has been identidifed for it.  The path
+		of coresight components linking the source to the sink is
+		configured and managed automatically by the coresight framework.
+
+What:		/sys/bus/coresight/devices/<memory_map>.stm/hwevent_enable
+Date:		February 2016
+KernelVersion:	4.5
+Contact:	Mathieu Poirier <mathieu.poirier@linaro.org>
+Description:	(RW) Provides access to the HW event enable register, used in
+		conjunction with HW event bank select register.
+
+What:		/sys/bus/coresight/devices/<memory_map>.stm/hwevent_select
+Date:		February 2016
+KernelVersion:	4.5
+Contact:	Mathieu Poirier <mathieu.poirier@linaro.org>
+Description:	(RW) Gives access to the HW event block select register
+		(STMHEBSR) in order to configure up to 256 channels.  Used in
+		conjunction with "hwevent_enable" register as described above.
+
+What:		/sys/bus/coresight/devices/<memory_map>.stm/port_enable
+Date:		February 2016
+KernelVersion:	4.5
+Contact:	Mathieu Poirier <mathieu.poirier@linaro.org>
+Description:	(RW) Provides access to the stimlus port enable register
+		(STMSPER).  Used in conjunction with "port_select" described
+		below.
+
+What:		/sys/bus/coresight/devices/<memory_map>.stm/port_select
+Date:		February 2016
+KernelVersion:	4.5
+Contact:	Mathieu Poirier <mathieu.poirier@linaro.org>
+Description:	(RW) Used to determine which bank of stimulus port bit in
+		register STMSPER (see above) apply to.
+
+What:		/sys/bus/coresight/devices/<memory_map>.stm/status
+Date:		February 2016
+KernelVersion:	4.5
+Contact:	Mathieu Poirier <mathieu.poirier@linaro.org>
+Description:	(R) List various control and status registers.  The specific
+		layout and content is driver specific.
+
+What:		/sys/bus/coresight/devices/<memory_map>.stm/traceid
+Date:		February 2016
+KernelVersion:	4.5
+Contact:	Mathieu Poirier <mathieu.poirier@linaro.org>
+Description:	(RW) Holds the trace ID that will appear in the trace stream
+		coming from this trace entity.
diff --git a/Documentation/trace/coresight.txt b/Documentation/trace/coresight.txt
index 0a5c329..a33c88c 100644
--- a/Documentation/trace/coresight.txt
+++ b/Documentation/trace/coresight.txt
@@ -190,8 +190,8 @@  expected to be accessed and controlled using those entries.
 Last but not least, "struct module *owner" is expected to be set to reflect
 the information carried in "THIS_MODULE".
 
-How to use
-----------
+How to use the tracer modules
+-----------------------------
 
 Before trace collection can start, a coresight sink needs to be identify.
 There is no limit on the amount of sinks (nor sources) that can be enabled at
@@ -297,3 +297,36 @@  Info                                    Tracing enabled
 Instruction     13570831        0x8026B584      E28DD00C        false   ADD      sp,sp,#0xc
 Instruction     0       0x8026B588      E8BD8000        true    LDM      sp!,{pc}
 Timestamp                                       Timestamp: 17107041535
+
+How to use the STM module
+-------------------------
+
+Using the System Trace Macrocell module is the same as the tracers - the only
+difference is that clients are driving the trace capture rather
+than the program flow through the code.
+
+As with any other CoreSight component, specifics about the STM tracer can be
+found in sysfs with more information on each entry being found in [1]:
+
+root@genericarmv8:~# ls /sys/bus/coresight/devices/20100000.stm
+enable_source   hwevent_select  port_enable     subsystem       uevent
+hwevent_enable  mgmt            port_select     traceid
+root@genericarmv8:~#
+
+Like any other source a sink needs to be identified and the STM enabled before
+being used:
+
+root@genericarmv8:~# echo 1 > /sys/bus/coresight/devices/20010000.etf/enable_sink
+root@genericarmv8:~# echo 1 > /sys/bus/coresight/devices/20100000.stm/enable_source
+
+From there user space applications can request and use channels using the devfs
+interface provided for that purpose by the generic STM API:
+
+root@genericarmv8:~# ls -l /dev/20100000.stm
+crw-------    1 root     root       10,  61 Jan  3 18:11 /dev/20100000.stm
+root@genericarmv8:~#
+
+Details on how to use the generic STM API can be found here [2].
+
+[1]. Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
+[2]. Documentation/trace/stm.txt
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index c85935f..f4a8bfa 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -77,4 +77,15 @@  config CORESIGHT_QCOM_REPLICATOR
 	  programmable ATB replicator sends the ATB trace stream from the
 	  ETB/ETF to the TPIUi and ETR.
 
+config CORESIGHT_STM
+	bool "CoreSight System Trace Macrocell driver"
+	depends on (ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64
+	select CORESIGHT_LINKS_AND_SINKS
+	select STM
+	help
+	  This driver provides support for hardware assisted software
+	  instrumentation based tracing. This is primarily used for
+	  logging useful software events or data coming from various entities
+	  in the system, possibly running different OSs
+
 endif
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index 99f8e5f..1f6eaec 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -11,3 +11,4 @@  obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \
 obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o
 obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o
 obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
+obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
new file mode 100644
index 0000000..01b3521
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -0,0 +1,928 @@ 
+/* Copyright (c) 2015-2016, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that 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.
+ *
+ * Initial implementation by Pratik Patel
+ * (C) 2014-2015 Pratik Patel <pratikp@codeaurora.org>
+ *
+ * Serious refactoring, code cleanup and upgrading to the Coresight upstream
+ * framework by Mathieu Poirier
+ * (C) 2015-2016 Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * Guaranteed timing and support for various packet type coming from the
+ * generic STM API by Chunyan Zhang
+ * (C) 2015-2016 Chunyan Zhang <zhang.chunyan@linaro.org>
+ */
+#include <linux/amba/bus.h>
+#include <linux/bitmap.h>
+#include <linux/clk.h>
+#include <linux/coresight.h>
+#include <linux/coresight-stm.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/pm_runtime.h>
+#include <linux/stm.h>
+
+#include "coresight-priv.h"
+
+#define STMDMASTARTR			0xc04
+#define STMDMASTOPR			0xc08
+#define STMDMASTATR			0xc0c
+#define STMDMACTLR			0xc10
+#define STMDMAIDR			0xcfc
+#define STMHEER				0xd00
+#define STMHETER			0xd20
+#define STMHEBSR			0xd60
+#define STMHEMCR			0xd64
+#define STMHEMASTR			0xdf4
+#define STMHEFEAT1R			0xdf8
+#define STMHEIDR			0xdfc
+#define STMSPER				0xe00
+#define STMSPTER			0xe20
+#define STMPRIVMASKR			0xe40
+#define STMSPSCR			0xe60
+#define STMSPMSCR			0xe64
+#define STMSPOVERRIDER			0xe68
+#define STMSPMOVERRIDER			0xe6c
+#define STMSPTRIGCSR			0xe70
+#define STMTCSR				0xe80
+#define STMTSSTIMR			0xe84
+#define STMTSFREQR			0xe8c
+#define STMSYNCR			0xe90
+#define STMAUXCR			0xe94
+#define STMSPFEAT1R			0xea0
+#define STMSPFEAT2R			0xea4
+#define STMSPFEAT3R			0xea8
+#define STMITTRIGGER			0xee8
+#define STMITATBDATA0			0xeec
+#define STMITATBCTR2			0xef0
+#define STMITATBID			0xef4
+#define STMITATBCTR0			0xef8
+
+#define STM_32_CHANNEL			32
+#define BYTES_PER_CHANNEL		256
+#define STM_TRACE_BUF_SIZE		4096
+#define STM_SW_MASTER_END		127
+
+/* Register bit definition */
+#define STMTCSR_BUSY_BIT		23
+/* Reserve the first 10 channels for kernel usage */
+#define STM_CHANNEL_OFFSET		0
+
+enum stm_pkt_type {
+	STM_PKT_TYPE_DATA	= 0x98,
+	STM_PKT_TYPE_FLAG	= 0xE8,
+	STM_PKT_TYPE_TRIG	= 0xF8,
+};
+
+#define stm_channel_addr(drvdata, ch)	(drvdata->chs.base +	\
+					(ch * BYTES_PER_CHANNEL))
+#define stm_channel_off(type, opts)	(type & ~opts)
+
+static int boot_nr_channel;
+
+module_param_named(
+	boot_nr_channel, boot_nr_channel, int, S_IRUGO
+);
+
+/**
+ * struct channel_space - central management entity for extended ports
+ * @base:		memory mapped base address where channels start.
+ * @guaraneed:		is the channel delivery guaranteed.
+ */
+struct channel_space {
+	void __iomem		*base;
+	unsigned long		*guaranteed;
+};
+
+/**
+ * struct stm_drvdata - specifics associated to an STM component
+ * @base:		memory mapped base address for this component.
+ * @dev:		the device entity associated to this component.
+ * @atclk:		optional clock for the core parts of the STM.
+ * @csdev:		component vitals needed by the framework.
+ * @spinlock:		only one at a time pls.
+ * @chs:		the channels accociated to this STM.
+ * @stm:		structure associated to the generic STM interface.
+ * @enable:		this STM is being used.
+ * @traceid:		value of the current ID for this component.
+ * @write_max:	        Maximus bytes this STM can write at a time.
+ * @write_64bit:	whether this STM supports 64 bit access.
+ * @stmsper:		settings for register STMSPER.
+ * @stmspscr:		settings for register STMSPSCR.
+ * @numsp:		the total number of stimulus port support by this STM.
+ * @stmheer:		settings for register STMHEER.
+ * @stmheter:		settings for register STMHETER.
+ * @stmhebsr:		settings for register STMHEBSR.
+ */
+struct stm_drvdata {
+	void __iomem		*base;
+	struct device		*dev;
+	struct clk		*atclk;
+	struct coresight_device	*csdev;
+	spinlock_t		spinlock;
+	struct channel_space	chs;
+	struct stm_data		stm;
+	bool			enable;
+	u8			traceid;
+	u8			write_max;
+	u32			write_64bit;
+	u32			stmsper;
+	u32			stmspscr;
+	u32			numsp;
+	u32			stmheer;
+	u32			stmheter;
+	u32			stmhebsr;
+};
+
+static void stm_hwevent_enable_hw(struct stm_drvdata *drvdata)
+{
+	CS_UNLOCK(drvdata->base);
+
+	writel_relaxed(drvdata->stmhebsr, drvdata->base + STMHEBSR);
+	writel_relaxed(drvdata->stmheter, drvdata->base + STMHETER);
+	writel_relaxed(drvdata->stmheer, drvdata->base + STMHEER);
+	writel_relaxed(0x01 |	/* Enable HW event tracing */
+		       0x04,	/* Error detection on event tracing */
+		       drvdata->base + STMHEMCR);
+
+	CS_LOCK(drvdata->base);
+}
+
+static void stm_port_enable_hw(struct stm_drvdata *drvdata)
+{
+	CS_UNLOCK(drvdata->base);
+	/* ATB trigger enable on direct writes to TRIG locations */
+	writel_relaxed(0x10,
+		       drvdata->base + STMSPTRIGCSR);
+	writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR);
+	writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);
+
+	CS_LOCK(drvdata->base);
+}
+
+static void stm_enable_hw(struct stm_drvdata *drvdata)
+{
+	if (drvdata->stmheer)
+		stm_hwevent_enable_hw(drvdata);
+
+	stm_port_enable_hw(drvdata);
+
+	CS_UNLOCK(drvdata->base);
+
+	/* 4096 byte between synchronisation packets */
+	writel_relaxed(0xFFF, drvdata->base + STMSYNCR);
+	writel_relaxed((drvdata->traceid << 16 | /* trace id */
+			0x02 |			 /* timestamp enable */
+			0x01),			 /* global STM enable */
+			drvdata->base + STMTCSR);
+
+	CS_LOCK(drvdata->base);
+}
+
+static int stm_enable(struct coresight_device *csdev)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	pm_runtime_get_sync(drvdata->dev);
+
+	spin_lock(&drvdata->spinlock);
+	stm_enable_hw(drvdata);
+	drvdata->enable = true;
+	spin_unlock(&drvdata->spinlock);
+
+	dev_info(drvdata->dev, "STM tracing enabled\n");
+	return 0;
+}
+
+static void stm_hwevent_disable_hw(struct stm_drvdata *drvdata)
+{
+	CS_UNLOCK(drvdata->base);
+
+	writel_relaxed(0x0, drvdata->base + STMHEMCR);
+	writel_relaxed(0x0, drvdata->base + STMHEER);
+	writel_relaxed(0x0, drvdata->base + STMHETER);
+
+	CS_LOCK(drvdata->base);
+}
+
+static void stm_port_disable_hw(struct stm_drvdata *drvdata)
+{
+	CS_UNLOCK(drvdata->base);
+
+	writel_relaxed(0x0, drvdata->base + STMSPER);
+	writel_relaxed(0x0, drvdata->base + STMSPTRIGCSR);
+
+	CS_LOCK(drvdata->base);
+}
+
+static void stm_disable_hw(struct stm_drvdata *drvdata)
+{
+	u32 val;
+
+	CS_UNLOCK(drvdata->base);
+
+	val = readl_relaxed(drvdata->base + STMTCSR);
+	val &= ~0x1; /* clear global STM enable [0] */
+	writel_relaxed(val, drvdata->base + STMTCSR);
+
+	CS_LOCK(drvdata->base);
+
+	stm_port_disable_hw(drvdata);
+	if (drvdata->stmheer)
+		stm_hwevent_disable_hw(drvdata);
+}
+
+static void stm_disable(struct coresight_device *csdev)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	spin_lock(&drvdata->spinlock);
+	stm_disable_hw(drvdata);
+	drvdata->enable = false;
+	spin_unlock(&drvdata->spinlock);
+
+	/* Wait until the engine has completely stopped */
+	coresight_timeout(drvdata, STMTCSR, STMTCSR_BUSY_BIT, 0);
+
+	pm_runtime_put(drvdata->dev);
+
+	dev_info(drvdata->dev, "STM tracing disabled\n");
+}
+
+static int stm_trace_id(struct coresight_device *csdev)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	return drvdata->traceid;
+}
+
+static const struct coresight_ops_source stm_source_ops = {
+	.trace_id	= stm_trace_id,
+	.enable		= stm_enable,
+	.disable	= stm_disable,
+};
+
+static const struct coresight_ops stm_cs_ops = {
+	.source_ops	= &stm_source_ops,
+};
+
+static inline bool stm_addr_unaligned(const void *addr, u8 max)
+{
+	return ((unsigned long)addr & (max - 1));
+}
+
+static int stm_send(void *addr, const void *data, u32 size, u8 max)
+{
+	u32 len = size;
+	u8 paload[8];
+
+	if (stm_addr_unaligned(data, max)) {
+		memcpy(paload, data, size);
+		data = paload;
+	}
+
+	/* now we are 64bit/32bit aligned */
+#ifdef CONFIG_64BIT
+	if (size == 8)
+		writeq_relaxed(*(u64 *)data, addr);
+#endif
+	if (size >= 4) {
+		writel_relaxed(*(u32 *)data, addr);
+		data += 4;
+		size -= 4;
+	}
+
+	if (size >= 2) {
+		writew_relaxed(*(u16 *)data, addr);
+		data += 2;
+		size -= 2;
+	}
+
+	if (size == 1)
+		writeb_relaxed(*(u8 *)data, addr);
+
+	return len;
+}
+
+static int stm_generic_link(struct stm_data *stm_data,
+			    unsigned int master,  unsigned int channel)
+{
+	struct stm_drvdata *drvdata = container_of(stm_data,
+						   struct stm_drvdata, stm);
+	if (!drvdata || !drvdata->csdev)
+		return -EINVAL;
+
+	return coresight_enable(drvdata->csdev);
+}
+
+static void stm_generic_unlink(struct stm_data *stm_data,
+			       unsigned int master,  unsigned int channel)
+{
+	struct stm_drvdata *drvdata = container_of(stm_data,
+						   struct stm_drvdata, stm);
+	if (!drvdata || !drvdata->csdev)
+		return;
+
+	stm_disable(drvdata->csdev);
+}
+
+static long stm_generic_set_options(struct stm_data *stm_data,
+				    unsigned int master,
+				    unsigned int channel,
+				    unsigned int nr_chans,
+				    unsigned long options)
+{
+	struct stm_drvdata *drvdata = container_of(stm_data,
+						   struct stm_drvdata, stm);
+	if (!(drvdata && drvdata->enable))
+		return -EINVAL;
+
+	if (channel >= drvdata->numsp)
+		return -EINVAL;
+
+	switch (options) {
+	case STM_OPTION_GUARANTEED:
+		set_bit(channel, drvdata->chs.guaranteed);
+		break;
+
+	case STM_OPTION_INVARIANT:
+		clear_bit(channel, drvdata->chs.guaranteed);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static long stm_generic_get_options(struct stm_data *stm_data,
+				    unsigned int master,
+				    unsigned int channel,
+				    unsigned int nr_chans,
+				    u64 *options)
+{
+	struct stm_drvdata *drvdata = container_of(stm_data,
+						   struct stm_drvdata, stm);
+	if (!(drvdata && drvdata->enable))
+		return -EINVAL;
+
+	if (channel >= drvdata->numsp)
+		return -EINVAL;
+
+	switch (*options) {
+	case STM_OPTION_GUARANTEED:
+		*options = test_bit(channel, drvdata->chs.guaranteed);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static ssize_t stm_generic_packet(struct stm_data *stm_data,
+				  unsigned int master,
+				  unsigned int channel,
+				  unsigned int packet,
+				  unsigned int flags,
+				  unsigned int size,
+				  const unsigned char *payload)
+{
+	unsigned long ch_addr;
+	struct stm_drvdata *drvdata = container_of(stm_data,
+						   struct stm_drvdata, stm);
+
+	if (!(drvdata && drvdata->enable))
+		return 0;
+
+	if (channel >= drvdata->numsp)
+		return 0;
+
+	ch_addr = (unsigned long)stm_channel_addr(drvdata, channel);
+
+	flags = (flags == STP_PACKET_TIMESTAMPED) ? STM_FLAG_TIMESTAMPED : 0;
+	flags |= test_bit(channel, drvdata->chs.guaranteed) ?
+			   STM_FLAG_GUARANTEED : 0;
+
+	if (size > drvdata->write_max)
+		size = drvdata->write_max;
+
+	switch (packet) {
+	case STP_PACKET_FLAG:
+		ch_addr |= stm_channel_off(STM_PKT_TYPE_FLAG, flags);
+
+		/* the generic STM API set a size of zero on flag packets. */
+		size = 1;
+		break;
+
+	case STP_PACKET_DATA:
+		ch_addr |= stm_channel_off(STM_PKT_TYPE_DATA, flags);
+		break;
+
+	default:
+		return 0;
+	}
+
+	return stm_send((void *)ch_addr, payload, size, drvdata->write_max);
+}
+
+static ssize_t hwevent_enable_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val = drvdata->stmheer;
+
+	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t hwevent_enable_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t size)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+	int ret = 0;
+
+	ret = kstrtoul(buf, 16, &val);
+	if (ret)
+		return -EINVAL;
+
+	drvdata->stmheer = val;
+	/* HW event enable and trigger go hand in hand */
+	drvdata->stmheter = val;
+
+	return size;
+}
+static DEVICE_ATTR_RW(hwevent_enable);
+
+static ssize_t hwevent_select_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val = drvdata->stmhebsr;
+
+	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t hwevent_select_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t size)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+	int ret = 0;
+
+	ret = kstrtoul(buf, 16, &val);
+	if (ret)
+		return -EINVAL;
+
+	drvdata->stmhebsr = val;
+
+	return size;
+}
+static DEVICE_ATTR_RW(hwevent_select);
+
+static ssize_t port_select_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+
+	if (!drvdata->enable) {
+		val = drvdata->stmspscr;
+	} else {
+		spin_lock(&drvdata->spinlock);
+		val = readl_relaxed(drvdata->base + STMSPSCR);
+		spin_unlock(&drvdata->spinlock);
+	}
+
+	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t port_select_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t size)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val, stmsper;
+	int ret = 0;
+
+	ret = kstrtoul(buf, 16, &val);
+	if (ret)
+		return ret;
+
+	spin_lock(&drvdata->spinlock);
+	drvdata->stmspscr = val;
+
+	if (drvdata->enable) {
+		CS_UNLOCK(drvdata->base);
+		/* Process as per ARM's TRM recommendation */
+		stmsper = readl_relaxed(drvdata->base + STMSPER);
+		writel_relaxed(0x0, drvdata->base + STMSPER);
+		writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR);
+		writel_relaxed(stmsper, drvdata->base + STMSPER);
+		CS_LOCK(drvdata->base);
+	}
+	spin_unlock(&drvdata->spinlock);
+
+	return size;
+}
+static DEVICE_ATTR_RW(port_select);
+
+static ssize_t port_enable_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+
+	if (!drvdata->enable) {
+		val = drvdata->stmsper;
+	} else {
+		spin_lock(&drvdata->spinlock);
+		val = readl_relaxed(drvdata->base + STMSPER);
+		spin_unlock(&drvdata->spinlock);
+	}
+
+	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t port_enable_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t size)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+	int ret = 0;
+
+	ret = kstrtoul(buf, 16, &val);
+	if (ret)
+		return ret;
+
+	spin_lock(&drvdata->spinlock);
+	drvdata->stmsper = val;
+
+	if (drvdata->enable) {
+		CS_UNLOCK(drvdata->base);
+		writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);
+		CS_LOCK(drvdata->base);
+	}
+	spin_unlock(&drvdata->spinlock);
+
+	return size;
+}
+static DEVICE_ATTR_RW(port_enable);
+
+static ssize_t traceid_show(struct device *dev,
+			    struct device_attribute *attr, char *buf)
+{
+	unsigned long val;
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	val = drvdata->traceid;
+	return sprintf(buf, "%#lx\n", val);
+}
+
+static ssize_t traceid_store(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t size)
+{
+	int ret;
+	unsigned long val;
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	ret = kstrtoul(buf, 16, &val);
+	if (ret)
+		return ret;
+
+	/* traceid field is 7bit wide on STM32 */
+	drvdata->traceid = val & 0x7f;
+	return size;
+}
+static DEVICE_ATTR_RW(traceid);
+
+#define coresight_simple_func(type, name, offset)			\
+static ssize_t name##_show(struct device *_dev,				\
+			   struct device_attribute *attr, char *buf)	\
+{									\
+	type *drvdata = dev_get_drvdata(_dev->parent);			\
+	return scnprintf(buf, PAGE_SIZE, "0x%08x\n",			\
+			 readl_relaxed(drvdata->base + offset));	\
+}									\
+DEVICE_ATTR_RO(name)
+
+#define coresight_stm_simple_func(name, offset)	\
+	coresight_simple_func(struct stm_drvdata, name, offset)
+
+coresight_stm_simple_func(tcsr, STMTCSR);
+coresight_stm_simple_func(tsfreqr, STMTSFREQR);
+coresight_stm_simple_func(syncr, STMSYNCR);
+coresight_stm_simple_func(sper, STMSPER);
+coresight_stm_simple_func(spter, STMSPTER);
+coresight_stm_simple_func(privmaskr, STMPRIVMASKR);
+coresight_stm_simple_func(spscr, STMSPSCR);
+coresight_stm_simple_func(spmscr, STMSPMSCR);
+coresight_stm_simple_func(spfeat1r, STMSPFEAT1R);
+coresight_stm_simple_func(spfeat2r, STMSPFEAT2R);
+coresight_stm_simple_func(spfeat3r, STMSPFEAT3R);
+coresight_stm_simple_func(devid, CORESIGHT_DEVID);
+
+static struct attribute *coresight_stm_attrs[] = {
+	&dev_attr_hwevent_enable.attr,
+	&dev_attr_hwevent_select.attr,
+	&dev_attr_port_enable.attr,
+	&dev_attr_port_select.attr,
+	&dev_attr_traceid.attr,
+	NULL,
+};
+
+static struct attribute *coresight_stm_mgmt_attrs[] = {
+	&dev_attr_tcsr.attr,
+	&dev_attr_tsfreqr.attr,
+	&dev_attr_syncr.attr,
+	&dev_attr_sper.attr,
+	&dev_attr_spter.attr,
+	&dev_attr_privmaskr.attr,
+	&dev_attr_spscr.attr,
+	&dev_attr_spmscr.attr,
+	&dev_attr_spfeat1r.attr,
+	&dev_attr_spfeat2r.attr,
+	&dev_attr_spfeat3r.attr,
+	&dev_attr_devid.attr,
+	NULL,
+};
+
+static const struct attribute_group coresight_stm_group = {
+	.attrs = coresight_stm_attrs,
+};
+
+static const struct attribute_group coresight_stm_mgmt_group = {
+	.attrs = coresight_stm_mgmt_attrs,
+	.name = "mgmt",
+};
+
+static const struct attribute_group *coresight_stm_groups[] = {
+	&coresight_stm_group,
+	&coresight_stm_mgmt_group,
+	NULL,
+};
+
+static int stm_get_resource_byname(struct device_node *np,
+				   char *ch_base, struct resource *res)
+{
+	const char *name = NULL;
+	int index = 0, found = 0;
+
+	while (!of_property_read_string_index(np, "reg-names", index, &name)) {
+		if (strcmp(ch_base, name)) {
+			index++;
+			continue;
+		}
+
+		/* We have a match and @index is where it's at */
+		found = 1;
+		break;
+	}
+
+	if (!found)
+		return -EINVAL;
+
+	return of_address_to_resource(np, index, res);
+}
+
+static u32 stm_fundamental_data_size(struct stm_drvdata *drvdata)
+{
+	u32 stmspfeat2r;
+
+	stmspfeat2r = readl_relaxed(drvdata->base + STMSPFEAT2R);
+	return BMVAL(stmspfeat2r, 12, 15);
+}
+
+static u32 stm_num_stimulus_port(struct stm_drvdata *drvdata)
+{
+	u32 numsp;
+
+	numsp = readl_relaxed(drvdata->base + CORESIGHT_DEVID);
+	/*
+	 * NUMPS in STMDEVID is 17 bit long and if equal to 0x0,
+	 * 32 stimulus ports are supported.
+	 */
+	numsp &= 0x1ffff;
+	if (!numsp)
+		numsp = STM_32_CHANNEL;
+	return numsp;
+}
+
+static void stm_init_default_data(struct stm_drvdata *drvdata)
+{
+	/* Don't use port selection */
+	drvdata->stmspscr = 0x0;
+	/*
+	 * Enable all channel regardless of their number.  When port
+	 * selection isn't used (see above) STMSPER applies to all
+	 * 32 channel group available, hence setting all 32 bits to 1
+	 */
+	drvdata->stmsper = ~0x0;
+
+	/*
+	 * Select arbitrary value to start with.  If there is a conflict
+	 * with other tracers the framework will deal with it.
+	 */
+	drvdata->traceid = 0x20;
+
+	/* Set invariant transaction timing on all channels */
+	bitmap_clear(drvdata->chs.guaranteed, 0, drvdata->numsp);
+}
+
+static void stm_init_generic_data(struct stm_drvdata *drvdata)
+{
+	drvdata->stm.name = dev_name(drvdata->dev);
+	drvdata->stm.mstatic = true;
+	drvdata->stm.sw_nchannels = drvdata->numsp;
+	drvdata->stm.packet = stm_generic_packet;
+	drvdata->stm.link = stm_generic_link;
+	drvdata->stm.unlink = stm_generic_unlink;
+	drvdata->stm.set_options = stm_generic_set_options;
+	drvdata->stm.get_options = stm_generic_get_options;
+}
+
+static int stm_probe(struct amba_device *adev, const struct amba_id *id)
+{
+	int ret;
+	void __iomem *base;
+	unsigned long *guaranteed;
+	struct device *dev = &adev->dev;
+	struct coresight_platform_data *pdata = NULL;
+	struct stm_drvdata *drvdata;
+	struct resource *res = &adev->res;
+	struct resource ch_res;
+	size_t res_size, bitmap_size;
+	struct coresight_desc *desc;
+	struct device_node *np = adev->dev.of_node;
+
+	if (np) {
+		pdata = of_get_coresight_platform_data(dev, np);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+		adev->dev.platform_data = pdata;
+	}
+	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	drvdata->dev = &adev->dev;
+	drvdata->atclk = devm_clk_get(&adev->dev, "atclk"); /* optional */
+	if (!IS_ERR(drvdata->atclk)) {
+		ret = clk_prepare_enable(drvdata->atclk);
+		if (ret)
+			return ret;
+	}
+	dev_set_drvdata(dev, drvdata);
+
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+	drvdata->base = base;
+
+	ret = stm_get_resource_byname(np, "stm-stimulus-base", &ch_res);
+	if (ret)
+		return ret;
+
+	base = devm_ioremap_resource(dev, &ch_res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+	drvdata->chs.base = base;
+
+	drvdata->write_max = sizeof(u32);
+	drvdata->write_64bit = stm_fundamental_data_size(drvdata);
+
+#ifndef CONFIG_64BIT
+	if (drvdata->write_64bit)
+		drvdata->write_max = sizeof(u64);
+#endif
+
+	if (boot_nr_channel) {
+		drvdata->numsp = boot_nr_channel;
+		res_size = min((resource_size_t)(boot_nr_channel *
+				  BYTES_PER_CHANNEL), resource_size(res));
+	} else {
+		drvdata->numsp = stm_num_stimulus_port(drvdata);
+		res_size = min((resource_size_t)(drvdata->numsp *
+				 BYTES_PER_CHANNEL), resource_size(res));
+	}
+	bitmap_size = BITS_TO_LONGS(drvdata->numsp) * sizeof(long);
+
+	guaranteed = devm_kzalloc(dev, bitmap_size, GFP_KERNEL);
+	if (!guaranteed)
+		return -ENOMEM;
+	drvdata->chs.guaranteed = guaranteed;
+
+	spin_lock_init(&drvdata->spinlock);
+
+	stm_init_default_data(drvdata);
+	stm_init_generic_data(drvdata);
+
+	if (stm_register_device(dev, &drvdata->stm, THIS_MODULE)) {
+		dev_info(dev,
+			 "stm_register_device failed, probing deffered\n");
+		return -EPROBE_DEFER;
+	}
+
+	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
+	if (!desc) {
+		ret = -ENOMEM;
+		goto stm_unregister;
+	}
+
+	desc->type = CORESIGHT_DEV_TYPE_SOURCE;
+	desc->subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE;
+	desc->ops = &stm_cs_ops;
+	desc->pdata = pdata;
+	desc->dev = dev;
+	desc->groups = coresight_stm_groups;
+	drvdata->csdev = coresight_register(desc);
+	if (IS_ERR(drvdata->csdev)) {
+		ret = PTR_ERR(drvdata->csdev);
+		goto stm_unregister;
+	}
+
+	pm_runtime_put(&adev->dev);
+
+	dev_info(dev, "%s initialized\n", (char *)id->data);
+	return 0;
+
+stm_unregister:
+	stm_unregister_device(&drvdata->stm);
+	return ret;
+}
+
+static int stm_remove(struct amba_device *adev)
+{
+	struct stm_drvdata *drvdata = amba_get_drvdata(adev);
+
+	stm_unregister_device(&drvdata->stm);
+	coresight_unregister(drvdata->csdev);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int stm_runtime_suspend(struct device *dev)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev);
+
+	if (drvdata && !IS_ERR(drvdata->atclk))
+		clk_disable_unprepare(drvdata->atclk);
+
+	return 0;
+}
+
+static int stm_runtime_resume(struct device *dev)
+{
+	struct stm_drvdata *drvdata = dev_get_drvdata(dev);
+
+	if (drvdata && !IS_ERR(drvdata->atclk))
+		clk_prepare_enable(drvdata->atclk);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops stm_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(stm_runtime_suspend, stm_runtime_resume, NULL)
+};
+
+static struct amba_id stm_ids[] = {
+	{
+		.id     = 0x0003b962,
+		.mask   = 0x0003ffff,
+		.data	= "STM32",
+	},
+	{ 0, 0},
+};
+
+static struct amba_driver stm_driver = {
+	.drv = {
+		.name   = "coresight-stm",
+		.owner	= THIS_MODULE,
+		.pm	= &stm_dev_pm_ops,
+	},
+	.probe          = stm_probe,
+	.remove         = stm_remove,
+	.id_table	= stm_ids,
+};
+
+module_amba_driver(stm_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("CoreSight System Trace Macrocell driver");
diff --git a/include/linux/coresight-stm.h b/include/linux/coresight-stm.h
new file mode 100644
index 0000000..a978bb8
--- /dev/null
+++ b/include/linux/coresight-stm.h
@@ -0,0 +1,6 @@ 
+#ifndef __LINUX_CORESIGHT_STM_H_
+#define __LINUX_CORESIGHT_STM_H_
+
+#include <uapi/linux/coresight-stm.h>
+
+#endif
diff --git a/include/uapi/linux/coresight-stm.h b/include/uapi/linux/coresight-stm.h
new file mode 100644
index 0000000..fa35f0b
--- /dev/null
+++ b/include/uapi/linux/coresight-stm.h
@@ -0,0 +1,12 @@ 
+#ifndef __UAPI_CORESIGHT_STM_H_
+#define __UAPI_CORESIGHT_STM_H_
+
+#define STM_FLAG_TIMESTAMPED   BIT(3)
+#define STM_FLAG_GUARANTEED    BIT(7)
+
+enum {
+	STM_OPTION_GUARANTEED = 0,
+	STM_OPTION_INVARIANT,
+};
+
+#endif