Re: [PATCH 1/9 v2] coresight: add CoreSight core layer framework
From: Rob Herring
Date: Fri Jun 27 2014 - 18:02:19 EST
On Fri, Jun 27, 2014 at 1:04 PM, <mathieu.poirier@xxxxxxxxxx> wrote:
> From: Pratik Patel <pratikp@xxxxxxxxxxxxxx>
>
> CoreSight components are compliant with the ARM CoreSight
> architecture specification and can be connected in various
> topologies to suite a particular SoCs tracing needs. These trace
> components can generally be classified as sources, links and
> sinks. Trace data produced by one or more sources flows through
> the intermediate links connecting the source to the currently
> selected sink.
>
> CoreSight framework provides an interface for the CoreSight trace
> drivers to register themselves with. It's intended to build up a
> topological view of the CoreSight components and configure the
> right series of components on user input via debugfs.
>
> For eg., when enabling a source, framework builds up a path
> consisting of all the components connecting the source to the
> currently selected sink and enables all of them.
>
> Framework also supports switching between available sinks and
> also provides status information to user space applications
> through the debugfs interface.
>
> Signed-off-by: Pratik Patel <pratikp@xxxxxxxxxxxxxx>
> Signed-off-by: Panchaxari Prasannamurthy <panchaxari.prasannamurthy@xxxxxxxxxx>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> ---
> .../devicetree/bindings/arm/coresight.txt | 141 +++++
We prefer bindings as separate patch and you have not cc'd all maintainers.
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> new file mode 100644
> index 0000000..9d4eb53
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -0,0 +1,141 @@
> +* CoreSight Components
> +
> +CoreSight components are compliant with the ARM CoreSight architecture
> +specification and can be connected in various topologies to suit a particular
> +SoCs tracing needs. These trace components can generally be classified as sinks,
> +links and sources. Trace data produced by one or more sources flows through the
> +intermediate links connecting the source to the currently selected sink. Each
> +CoreSight component device should use these properties to describe its hardware
> +characteristcs.
> +
> +Required properties:
> +
> +- compatible : name of the component used for driver matching. Possible values
> + include: "arm,coresight-etb", "arm,coresight-tpiu", "arm,coresight-tmc",
> + "arm,coresight-funnel", and "arm,coresight-etm". All of these have to
> + be supplemented with "arm,primecell" as drivers are using the AMBA bus
> + interface.
Is there no versioning needed with any of these?
> +- reg : physical base address and length of the register set(s) of the component
> +- clocks : the clock associated to this component
> +- clock-names: the name of the clock as referenced by the code. Since we are
> + using the AMBA framework, the name should be "apb_pclk".
> +- ports or port: The representation of the component's port layout using the
> + generic DT graph presentation found in "bindings/graph.txt"
> +
> +Optional properties for Sinks:
> +- coresight-default-sink: must be specified for one of the sink devices that is
> +intended to be made the default sink. Other sink devices must not have this
> +specified. Not specifying this property on any of the sinks is invalid.
> +
> +Optional properties for ETM/PTMs:
> +- arm,cp14 : access to ETM/PTM management registers is made via cp14.
> +- cpu: the cpu this ETM/PTM is affined to.
Why optional?
Clarify that this is a phandle.
> +
> +Optional property for TMC:
> +- arm,buffer-size : size of contiguous buffer space for TMC ETR (embedded trace router)
ETBs don't need a size?
Not really much more to say given you are using the graph binding!
> +
> +
> +Example:
> +
> +1. Sinks
> + etb: etb@20010000 {
> + compatible = "arm,coresight-etb", "arm,primecell";
> + reg = <0 0x20010000 0 0x1000>;
> +
> + coresight-default-sink;
> + clocks = <&oscclk6a>;
> + clock-names = "apb_pclk";
> + port {
> + etb_in_port: endpoint@0 {
> + slave-mode;
> + remote-endpoint = <&funnel_out_port0>;
> + };
> + };
> + };
> +
> + tpiu: tpiu@20030000 {
> + compatible = "arm,coresight-tpiu", "arm,primecell";
> + reg = <0 0x20030000 0 0x1000>;
> +
> + clocks = <&oscclk6a>;
> + clock-names = "apb_pclk";
> + port {
> + tpiu_in_port: endpoint@0 {
> + slave-mode;
> + remote-endpoint = <&funnel_out_port1>;
> + };
> + };
> + };
> +
> +2. Links
> + funnel {
> + compatible = "arm,coresight-funnel", "arm,primecell";
> + reg = <0 0x20040000 0 0x1000>;
> +
> + clocks = <&oscclk6a>;
> + clock-names = "apb_pclk";
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
You are getting this from the graph binding, but this node is a bit
pointless. #address-cells and #size-cells can be in the funnel node.
The extra layer is not necessary for that. The documentation says it
is optional, but perhaps the code or other uses require it.
Philipp, any comments on this?
[...]
> +/*
> + * Coresight management registers (0xF00-0xFCC)
> + * 0xFA0 - 0xFA4: Management registers in PFTv1.0
> + * Trace registers in PFTv1.1
> + */
> +#define CORESIGHT_ITCTRL (0xF00)
> +#define CORESIGHT_CLAIMSET (0xFA0)
> +#define CORESIGHT_CLAIMCLR (0xFA4)
> +#define CORESIGHT_LAR (0xFB0)
> +#define CORESIGHT_LSR (0xFB4)
> +#define CORESIGHT_AUTHSTATUS (0xFB8)
> +#define CORESIGHT_DEVID (0xFC8)
> +#define CORESIGHT_DEVTYPE (0xFCC)
> +
> +#define TIMEOUT_US (100)
Parentheses are not needed for constants and lower case hex is preferred.
> +
> +#define BM(lsb, msb) ((BIT(msb) - BIT(lsb)) + BIT(msb))
> +#define BMVAL(val, lsb, msb) ((val & BM(lsb, msb)) >> lsb)
> +#define BVAL(val, n) ((val & BIT(n)) >> n)
> +
> +#define cs_writel(addr, val, off) __raw_writel((val), addr + off)
> +#define cs_readl(addr, off) __raw_readl(addr + off)
Remove these. They don't do anything, but make someone reading the
code look up what exactly cs_writel/cs_readl do.
[...]
> +/* Returns the base address found in @node. Seriously
> + * tailored on @of_device_make_bus_id().
> + */
> +static u32 of_get_coresight_id(struct device_node *node)
> +{
> + u32 addr = 0;
> + const __be32 *reg, *addrp;
> +
> + reg = of_get_property(node, "reg", NULL);
> + if (reg) {
> + if (of_can_translate_address(node)) {
rebase to 3.16-rc, this is gone.
> + addr = of_translate_address(node, reg);
> + } else {
> + addrp = of_get_address(node, 0, NULL, NULL);
> + if (addrp)
> + addr = of_read_ulong(addrp, 1);
> + else
> + addr = 0;
> + }
> + }
> +
> + return addr;
> +}
> +
> +static struct device_node *of_get_coresight_endpoint(
> + const struct device_node *parent, struct device_node *prev)
> +{
> + struct device_node *node = of_graph_get_next_endpoint(parent, prev);
> + of_node_put(prev);
> + return node;
> +}
> +
> +static bool of_coresight_is_input_port(struct device_node *port)
> +{
> + if (of_find_property(port, "slave-mode", NULL))
return of_property_read_bool()
> + return true;
> + else
> + return false;
> +}
> +
> +static void of_coresight_get_ports(struct device_node *node,
> + int *nr_inports, int *nr_outports)
> +{
> + struct device_node *ep = NULL;
> + int in = 0, out = 0;
> +
> + do {
> + ep = of_get_coresight_endpoint(node, ep);
Does for_each_child_of_node not work here?
> + if (!ep)
> + break;
> + of_coresight_is_input_port(ep) ? in++ : out++;
> +
> + } while (ep);
> +
> + *nr_inports = in;
> + *nr_outports = out;
> +}
> +
> +static int of_coresight_alloc_memory(struct device *dev,
> + struct coresight_platform_data *pdata)
> +{
> + /* list of output port on this component */
> + pdata->outports = devm_kzalloc(dev, pdata->nr_outports *
> + sizeof(*pdata->outports),
> + GFP_KERNEL);
> + if (!pdata->outports)
> + return -ENOMEM;
> +
> +
> + /* children connected to this component via @outport */
> + pdata->child_ids = devm_kzalloc(dev, pdata->nr_outports *
> + sizeof(*pdata->child_ids),
> + GFP_KERNEL);
> + if (!pdata->child_ids)
> + return -ENOMEM;
> +
> + /* port number on the child this component is connected to */
> + pdata->child_ports = devm_kzalloc(dev, pdata->nr_outports *
> + sizeof(*pdata->child_ports),
> + GFP_KERNEL);
> + if (!pdata->child_ports)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +struct coresight_platform_data *of_get_coresight_platform_data(
> + struct device *dev, struct device_node *node)
> +{
> + u32 id;
> + int i = 0, ret = 0;
> + struct device_node *cpu;
> + struct coresight_platform_data *pdata;
> + struct of_endpoint endpoint, rendpoint;
> + struct device_node *ep = NULL;
> + struct device_node *rparent = NULL;
> + struct device_node *rport = NULL;
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return ERR_PTR(-ENOMEM);
> +
> + /* use the base address as id */
> + id = of_get_coresight_id(node);
> + if (id == 0)
> + return ERR_PTR(-EINVAL);
> +
> + pdata->id = id;
> +
> + /* use device name as debugfs handle */
> + pdata->name = dev_name(dev);
> +
> + /* get the number of input and output port for this component */
> + of_coresight_get_ports(node, &pdata->nr_inports, &pdata->nr_outports);
> +
> + if (pdata->nr_outports) {
> + ret = of_coresight_alloc_memory(dev, pdata);
> + if (ret)
> + return ERR_PTR(-ENOMEM);
> +
> + /* iterate through each port to discover topology */
> + do {
> + /* get a handle on a port */
> + ep = of_get_coresight_endpoint(node, ep);
> + if (!ep)
> + break;
for_each_child_of_node
> +
> + /* no need to deal with input ports, processing for as
> + * processing for output ports will deal with them.
> + */
> + if (of_coresight_is_input_port(ep))
> + continue;
> +
> + /* get a handle on the local endpoint */
> + ret = of_graph_parse_endpoint(ep, &endpoint);
> +
> + if (!ret) {
You can save some indentation with:
if (ret)
continue;
> + /* the local out port number */
> + *(u32 *)&pdata->outports[i] = endpoint.id;
Can't you avoid this casting?
> +
> + /* get a handle the remote port and parent
> + * attached to it.
> + */
> + rparent = of_graph_get_remote_port_parent(ep);
> + rport = of_graph_get_remote_port(ep);
> +
> + if (!rparent || !rport)
> + continue;
> +
> + if (of_graph_parse_endpoint(rport,
> + &rendpoint))
> + continue;
> +
> + *(u32 *)&pdata->child_ids[i] =
> + of_get_coresight_id(rparent);
> + *(u32 *)&pdata->child_ports[i] = rendpoint.id;
and these?
> +
> + i++;
> + }
> +
> + } while (ep);
> + }
> +
> + pdata->default_sink = of_property_read_bool(node,
> + "coresight-default-sink");
> +
> + /* affinity defaults to CPU0 */
> + pdata->cpu = 0;
> + cpu = of_parse_phandle(node, "cpu", 0);
> + if (cpu) {
> + const u32 *mpidr;
> + int len, index;
> +
> + mpidr = of_get_property(cpu, "reg", &len);
> + if (mpidr && len == 4) {
> + index = get_logical_index(be32_to_cpup(mpidr));
Don't we have some helper for translating a cpu phandle to logical index?
> + if (index != -EINVAL)
> + pdata->cpu = index;
> + }
> + }
> +
> + return pdata;
> +}
> +EXPORT_SYMBOL_GPL(of_get_coresight_platform_data);
> diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
> index fdd7e1b..cdddabe 100644
> --- a/include/linux/amba/bus.h
> +++ b/include/linux/amba/bus.h
> @@ -23,6 +23,7 @@
>
> #define AMBA_NR_IRQS 9
> #define AMBA_CID 0xb105f00d
> +#define CORESIGHT_CID 0xb105900d
>
> struct clk;
>
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> new file mode 100644
> index 0000000..a19420e
> --- /dev/null
> +++ b/include/linux/coresight.h
> @@ -0,0 +1,190 @@
> +/* Copyright (c) 2012, 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.
> + */
> +
> +#ifndef _LINUX_CORESIGHT_H
> +#define _LINUX_CORESIGHT_H
> +
> +#include <linux/device.h>
> +
> +/* Peripheral id registers (0xFD0-0xFEC) */
> +#define CORESIGHT_PERIPHIDR4 (0xFD0)
> +#define CORESIGHT_PERIPHIDR5 (0xFD4)
> +#define CORESIGHT_PERIPHIDR6 (0xFD8)
> +#define CORESIGHT_PERIPHIDR7 (0xFDC)
> +#define CORESIGHT_PERIPHIDR0 (0xFE0)
> +#define CORESIGHT_PERIPHIDR1 (0xFE4)
> +#define CORESIGHT_PERIPHIDR2 (0xFE8)
> +#define CORESIGHT_PERIPHIDR3 (0xFEC)
> +/* Component id registers (0xFF0-0xFFC) */
> +#define CORESIGHT_COMPIDR0 (0xFF0)
> +#define CORESIGHT_COMPIDR1 (0xFF4)
> +#define CORESIGHT_COMPIDR2 (0xFF8)
> +#define CORESIGHT_COMPIDR3 (0xFFC)
> +
> +#define ETM_ARCH_V3_3 (0x23)
> +#define ETM_ARCH_V3_5 (0x25)
> +#define PFT_ARCH_V1_1 (0x31)
> +
> +#define CORESIGHT_UNLOCK (0xC5ACCE55)
Parentheses are not necessary.
> +enum coresight_clk_rate {
> + CORESIGHT_CLK_RATE_OFF,
> + CORESIGHT_CLK_RATE_TRACE,
> + CORESIGHT_CLK_RATE_HSTRACE,
> +};
> +
> +enum coresight_dev_type {
> + CORESIGHT_DEV_TYPE_NONE,
> + CORESIGHT_DEV_TYPE_SINK,
> + CORESIGHT_DEV_TYPE_LINK,
> + CORESIGHT_DEV_TYPE_LINKSINK,
> + CORESIGHT_DEV_TYPE_SOURCE,
> +};
> +
> +enum coresight_dev_subtype_sink {
> + CORESIGHT_DEV_SUBTYPE_SINK_NONE,
> + CORESIGHT_DEV_SUBTYPE_SINK_PORT,
> + CORESIGHT_DEV_SUBTYPE_SINK_BUFFER,
> +};
> +
> +enum coresight_dev_subtype_link {
> + CORESIGHT_DEV_SUBTYPE_LINK_NONE,
> + CORESIGHT_DEV_SUBTYPE_LINK_MERG,
> + CORESIGHT_DEV_SUBTYPE_LINK_SPLIT,
> + CORESIGHT_DEV_SUBTYPE_LINK_FIFO,
> +};
> +
> +enum coresight_dev_subtype_source {
> + CORESIGHT_DEV_SUBTYPE_SOURCE_NONE,
> + CORESIGHT_DEV_SUBTYPE_SOURCE_PROC,
> + CORESIGHT_DEV_SUBTYPE_SOURCE_BUS,
> + CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE,
> +};
> +
> +struct coresight_ops_entry {
> + const char *name;
> + umode_t mode;
> + const struct file_operations *ops;
> +};
> +
> +struct coresight_dev_subtype {
> + enum coresight_dev_subtype_sink sink_subtype;
> + enum coresight_dev_subtype_link link_subtype;
> + enum coresight_dev_subtype_source source_subtype;
> +};
> +
> +struct coresight_platform_data {
> + int id;
> + int cpu;
> + const char *name;
> + int nr_inports;
> + const int *outports;
> + const int *child_ids;
> + const int *child_ports;
> + int nr_outports;
> + bool default_sink;
> + struct clk *clk;
> +};
> +
> +struct coresight_desc {
> + enum coresight_dev_type type;
> + struct coresight_dev_subtype subtype;
> + const struct coresight_ops *ops;
> + struct coresight_platform_data *pdata;
> + struct device *dev;
> + const struct coresight_ops_entry **debugfs_ops;
> + struct module *owner;
> +};
> +
> +struct coresight_connection {
> + int outport;
> + int child_id;
> + int child_port;
> + struct coresight_device *child_dev;
> + struct list_head link;
> +};
> +
> +struct coresight_refcnt {
> + int sink_refcnt;
> + int *link_refcnts;
> + int source_refcnt;
> +};
> +
> +struct coresight_device {
> + int id;
> + struct coresight_connection *conns;
> + int nr_conns;
> + enum coresight_dev_type type;
> + struct coresight_dev_subtype subtype;
> + const struct coresight_ops *ops;
> + struct dentry *de;
> + struct device dev;
> + struct coresight_refcnt refcnt;
> + struct list_head dev_link;
> + struct list_head path_link;
> + struct module *owner;
> + bool enable;
> +};
> +
> +#define to_coresight_device(d) container_of(d, struct coresight_device, dev)
> +
> +#define CORESIGHT_DEBUGFS_ENTRY(__name, __entry_name, \
> + __mode, __get, __set, __fmt) \
> +DEFINE_SIMPLE_ATTRIBUTE(__name ## _ops, __get, __set, __fmt) \
> +static const struct coresight_ops_entry __name ## _entry = { \
> + .name = __entry_name, \
> + .mode = __mode, \
> + .ops = &__name ## _ops \
> +}
> +
> +struct coresight_ops_sink {
> + int (*enable)(struct coresight_device *csdev);
> + void (*disable)(struct coresight_device *csdev);
> + void (*abort)(struct coresight_device *csdev);
> +};
> +
> +struct coresight_ops_link {
> + int (*enable)(struct coresight_device *csdev, int iport, int oport);
> + void (*disable)(struct coresight_device *csdev, int iport, int oport);
> +};
> +
> +struct coresight_ops_source {
> + int (*enable)(struct coresight_device *csdev);
> + void (*disable)(struct coresight_device *csdev);
> +};
> +
> +struct coresight_ops {
> + const struct coresight_ops_sink *sink_ops;
> + const struct coresight_ops_link *link_ops;
> + const struct coresight_ops_source *source_ops;
> +};
> +
> +#ifdef CONFIG_CORESIGHT
> +extern struct coresight_device *
> +coresight_register(struct coresight_desc *desc);
> +extern void coresight_unregister(struct coresight_device *csdev);
> +extern int coresight_enable(struct coresight_device *csdev);
> +extern void coresight_disable(struct coresight_device *csdev);
> +extern void coresight_abort(void);
> +extern struct clk *coresight_get_clk(void);
> +#else
> +static inline struct coresight_device *
> +coresight_register(struct coresight_desc *desc) { return NULL; }
> +static inline void coresight_unregister(struct coresight_device *csdev) {}
> +static inline int
> +coresight_enable(struct coresight_device *csdev) { return -ENOSYS; }
> +static inline void coresight_disable(struct coresight_device *csdev) {}
> +static inline void coresight_abort(void) {}
> +extern struct clk *coresight_get_clk(void) {};
> +#endif
> +
> +#endif
> diff --git a/include/linux/of_coresight.h b/include/linux/of_coresight.h
> new file mode 100644
> index 0000000..6a5e4d4
> --- /dev/null
> +++ b/include/linux/of_coresight.h
I would just put this into coresight.h.
> @@ -0,0 +1,27 @@
> +/* Copyright (c) 2012, 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.
> + */
> +
> +#ifndef __LINUX_OF_CORESIGHT_H
> +#define __LINUX_OF_CORESIGHT_H
> +
> +#ifdef CONFIG_OF
> +extern struct coresight_platform_data *of_get_coresight_platform_data(
> + struct device *dev, struct device_node *node);
> +#else
> +static inline struct coresight_platform_data *of_get_coresight_platform_data(
> + struct device *dev, struct device_node *node)
> +{
> + return NULL;
> +}
> +#endif
> +
> +#endif
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/