[RFC PATCH 07/11] coresight: add CoreSight ETM driver

[daniel.thompson@linaro.org (Daniel Thompson)]

On 30/05/14 14:43, mathieu.poirier at linaro.org wrote:
> diff --git a/drivers/coresight/coresight-etm-cp14.c b/drivers/coresight/coresight-etm-cp14.c
> new file mode 100644
> index 0000000..0088bbb
> --- /dev/null
> +++ b/drivers/coresight/coresight-etm-cp14.c
> @@ -0,0 +1,511 @@
> +/* 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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/bug.h>
> +#include <asm/hardware/cp14.h>
> +
> +static unsigned int etm_read_reg(uint32_t reg)
> +{
> +	switch (reg) {
> +	case 0x0:

Shouldn't this be:

case ETMCR/4:

Or an equivalent macro? Given the memory mappings are already spelt out
in another file it seems a shame to restate them again.

> +		return etm_read(ETMCR);

Maybe we could even condense the mapping with something like:

#define CASE_MAP_MM_READ_TO_CPIO(x) case (x)/4: return etm_read(x)

CASE_MAP_MM_READ_TO_CPIO(ETMCR);
CASE_MAP_MM_READ_TO_CPIO(ETMCCR);
CASE_MAP_MM_READ_TO_CPIO(ETMTRIGGER);
...

Note that the macro may not be perfect since it untested and I can't
remember how it will interact with the token pasting in etm_read(x).
Howevver but a macro with this interface can definitely be written.

> +	case 0x1:
> +		return etm_read(ETMCCR);
> +	case 0x2:
> +		return etm_read(ETMTRIGGER);
> +	case 0x4:
> +		return etm_read(ETMSR);
> +	case 0x5:
> +		return etm_read(ETMSCR);
> +	case 0x6:
> +		return etm_read(ETMTSSCR);
> +	case 0x8:
> +		return etm_read(ETMTEEVR);
> +	case 0x9:
> +		return etm_read(ETMTECR1);
> +	case 0xB:
> +		return etm_read(ETMFFLR);
> +	case 0x10:
> +		return etm_read(ETMACVR0);
> +	case 0x11:
> +		return etm_read(ETMACVR1);
> +	case 0x12:
> +		return etm_read(ETMACVR2);
> +	case 0x13:
> +		return etm_read(ETMACVR3);
> +	case 0x14:
> +		return etm_read(ETMACVR4);
> +	case 0x15:
> +		return etm_read(ETMACVR5);
> +	case 0x16:
> +		return etm_read(ETMACVR6);
> +	case 0x17:
> +		return etm_read(ETMACVR7);
> +	case 0x18:
> +		return etm_read(ETMACVR8);
> +	case 0x19:
> +		return etm_read(ETMACVR9);
> +	case 0x1A:
> +		return etm_read(ETMACVR10);
> +	case 0x1B:
> +		return etm_read(ETMACVR11);
> +	case 0x1C:
> +		return etm_read(ETMACVR12);
> +	case 0x1D:
> +		return etm_read(ETMACVR13);
> +	case 0x1E:
> +		return etm_read(ETMACVR14);
> +	case 0x1F:
> +		return etm_read(ETMACVR15);
> +	case 0x20:
> +		return etm_read(ETMACTR0);
> +	case 0x21:
> +		return etm_read(ETMACTR1);
> +	case 0x22:
> +		return etm_read(ETMACTR2);
> +	case 0x23:
> +		return etm_read(ETMACTR3);
> +	case 0x24:
> +		return etm_read(ETMACTR4);
> +	case 0x25:
> +		return etm_read(ETMACTR5);
> +	case 0x26:
> +		return etm_read(ETMACTR6);
> +	case 0x27:
> +		return etm_read(ETMACTR7);
> +	case 0x28:
> +		return etm_read(ETMACTR8);
> +	case 0x29:
> +		return etm_read(ETMACTR9);
> +	case 0x2A:
> +		return etm_read(ETMACTR10);
> +	case 0x2B:
> +		return etm_read(ETMACTR11);
> +	case 0x2C:
> +		return etm_read(ETMACTR12);
> +	case 0x2D:
> +		return etm_read(ETMACTR13);
> +	case 0x2E:
> +		return etm_read(ETMACTR14);
> +	case 0x2F:
> +		return etm_read(ETMACTR15);
> +	case 0x50:
> +		return etm_read(ETMCNTRLDVR0);
> +	case 0x51:
> +		return etm_read(ETMCNTRLDVR1);
> +	case 0x52:
> +		return etm_read(ETMCNTRLDVR2);
> +	case 0x53:
> +		return etm_read(ETMCNTRLDVR3);
> +	case 0x54:
> +		return etm_read(ETMCNTENR0);
> +	case 0x55:
> +		return etm_read(ETMCNTENR1);
> +	case 0x56:
> +		return etm_read(ETMCNTENR2);
> +	case 0x57:
> +		return etm_read(ETMCNTENR3);
> +	case 0x58:
> +		return etm_read(ETMCNTRLDEVR0);
> +	case 0x59:
> +		return etm_read(ETMCNTRLDEVR1);
> +	case 0x5A:
> +		return etm_read(ETMCNTRLDEVR2);
> +	case 0x5B:
> +		return etm_read(ETMCNTRLDEVR3);
> +	case 0x5C:
> +		return etm_read(ETMCNTVR0);
> +	case 0x5D:
> +		return etm_read(ETMCNTVR1);
> +	case 0x5E:
> +		return etm_read(ETMCNTVR2);
> +	case 0x5F:
> +		return etm_read(ETMCNTVR3);
> +	case 0x60:
> +		return etm_read(ETMSQ12EVR);
> +	case 0x61:
> +		return etm_read(ETMSQ21EVR);
> +	case 0x62:
> +		return etm_read(ETMSQ23EVR);
> +	case 0x63:
> +		return etm_read(ETMSQ31EVR);
> +	case 0x64:
> +		return etm_read(ETMSQ32EVR);
> +	case 0x65:
> +		return etm_read(ETMSQ13EVR);
> +	case 0x67:
> +		return etm_read(ETMSQR);
> +	case 0x68:
> +		return etm_read(ETMEXTOUTEVR0);
> +	case 0x69:
> +		return etm_read(ETMEXTOUTEVR1);
> +	case 0x6A:
> +		return etm_read(ETMEXTOUTEVR2);
> +	case 0x6B:
> +		return etm_read(ETMEXTOUTEVR3);
> +	case 0x6C:
> +		return etm_read(ETMCIDCVR0);
> +	case 0x6D:
> +		return etm_read(ETMCIDCVR1);
> +	case 0x6E:
> +		return etm_read(ETMCIDCVR2);
> +	case 0x6F:
> +		return etm_read(ETMCIDCMR);
> +	case 0x70:
> +		return etm_read(ETMIMPSPEC0);
> +	case 0x71:
> +		return etm_read(ETMIMPSPEC1);
> +	case 0x72:
> +		return etm_read(ETMIMPSPEC2);
> +	case 0x73:
> +		return etm_read(ETMIMPSPEC3);
> +	case 0x74:
> +		return etm_read(ETMIMPSPEC4);
> +	case 0x75:
> +		return etm_read(ETMIMPSPEC5);
> +	case 0x76:
> +		return etm_read(ETMIMPSPEC6);
> +	case 0x77:
> +		return etm_read(ETMIMPSPEC7);
> +	case 0x78:
> +		return etm_read(ETMSYNCFR);
> +	case 0x79:
> +		return etm_read(ETMIDR);
> +	case 0x7A:
> +		return etm_read(ETMCCER);
> +	case 0x7B:
> +		return etm_read(ETMEXTINSELR);
> +	case 0x7C:
> +		return etm_read(ETMTESSEICR);
> +	case 0x7D:
> +		return etm_read(ETMEIBCR);
> +	case 0x7E:
> +		return etm_read(ETMTSEVR);
> +	case 0x7F:
> +		return etm_read(ETMAUXCR);
> +	case 0x80:
> +		return etm_read(ETMTRACEIDR);
> +	case 0x90:
> +		return etm_read(ETMVMIDCVR);
> +	case 0xC1:
> +		return etm_read(ETMOSLSR);
> +	case 0xC2:
> +		return etm_read(ETMOSSRR);
> +	case 0xC4:
> +		return etm_read(ETMPDCR);
> +	case 0xC5:
> +		return etm_read(ETMPDSR);
> +	default:
> +		WARN(1, "invalid CP14 access to ETM reg: %lx",
> +							(unsigned long)reg);
> +		return 0;
> +	}
> +}
> +
> +static void etm_write_reg(uint32_t val, uint32_t reg)
> +{
> +	switch (reg) {
> +	case 0x0:
> +		etm_write(val, ETMCR);

Same comment as etm_read_reg but with a different macro.

> +		return;
> +	case 0x2:
> +		etm_write(val, ETMTRIGGER);
> +		return;
> +	case 0x4:
> +		etm_write(val, ETMSR);
> +		return;
> +	case 0x6:
> +		etm_write(val, ETMTSSCR);
> +		return;
> +	case 0x8:
> +		etm_write(val, ETMTEEVR);
> +		return;
> +	case 0x9:
> +		etm_write(val, ETMTECR1);
> +		return;
> +	case 0xB:
> +		etm_write(val, ETMFFLR);
> +		return;
> +	case 0x10:
> +		etm_write(val, ETMACVR0);
> +		return;
> +	case 0x11:
> +		etm_write(val, ETMACVR1);
> +		return;
> +	case 0x12:
> +		etm_write(val, ETMACVR2);
> +		return;
> +	case 0x13:
> +		etm_write(val, ETMACVR3);
> +		return;
> +	case 0x14:
> +		etm_write(val, ETMACVR4);
> +		return;
> +	case 0x15:
> +		etm_write(val, ETMACVR5);
> +		return;
> +	case 0x16:
> +		etm_write(val, ETMACVR6);
> +		return;
> +	case 0x17:
> +		etm_write(val, ETMACVR7);
> +		return;
> +	case 0x18:
> +		etm_write(val, ETMACVR8);
> +		return;
> +	case 0x19:
> +		etm_write(val, ETMACVR9);
> +		return;
> +	case 0x1A:
> +		etm_write(val, ETMACVR10);
> +		return;
> +	case 0x1B:
> +		etm_write(val, ETMACVR11);
> +		return;
> +	case 0x1C:
> +		etm_write(val, ETMACVR12);
> +		return;
> +	case 0x1D:
> +		etm_write(val, ETMACVR13);
> +		return;
> +	case 0x1E:
> +		etm_write(val, ETMACVR14);
> +		return;
> +	case 0x1F:
> +		etm_write(val, ETMACVR15);
> +		return;
> +	case 0x20:
> +		etm_write(val, ETMACTR0);
> +		return;
> +	case 0x21:
> +		etm_write(val, ETMACTR1);
> +		return;
> +	case 0x22:
> +		etm_write(val, ETMACTR2);
> +		return;
> +	case 0x23:
> +		etm_write(val, ETMACTR3);
> +		return;
> +	case 0x24:
> +		etm_write(val, ETMACTR4);
> +		return;
> +	case 0x25:
> +		etm_write(val, ETMACTR5);
> +		return;
> +	case 0x26:
> +		etm_write(val, ETMACTR6);
> +		return;
> +	case 0x27:
> +		etm_write(val, ETMACTR7);
> +		return;
> +	case 0x28:
> +		etm_write(val, ETMACTR8);
> +		return;
> +	case 0x29:
> +		etm_write(val, ETMACTR9);
> +		return;
> +	case 0x2A:
> +		etm_write(val, ETMACTR10);
> +		return;
> +	case 0x2B:
> +		etm_write(val, ETMACTR11);
> +		return;
> +	case 0x2C:
> +		etm_write(val, ETMACTR12);
> +		return;
> +	case 0x2D:
> +		etm_write(val, ETMACTR13);
> +		return;
> +	case 0x2E:
> +		etm_write(val, ETMACTR14);
> +		return;
> +	case 0x2F:
> +		etm_write(val, ETMACTR15);
> +		return;
> +	case 0x50:
> +		etm_write(val, ETMCNTRLDVR0);
> +		return;
> +	case 0x51:
> +		etm_write(val, ETMCNTRLDVR1);
> +		return;
> +	case 0x52:
> +		etm_write(val, ETMCNTRLDVR2);
> +		return;
> +	case 0x53:
> +		etm_write(val, ETMCNTRLDVR3);
> +		return;
> +	case 0x54:
> +		etm_write(val, ETMCNTENR0);
> +		return;
> +	case 0x55:
> +		etm_write(val, ETMCNTENR1);
> +		return;
> +	case 0x56:
> +		etm_write(val, ETMCNTENR2);
> +		return;
> +	case 0x57:
> +		etm_write(val, ETMCNTENR3);
> +		return;
> +	case 0x58:
> +		etm_write(val, ETMCNTRLDEVR0);
> +		return;
> +	case 0x59:
> +		etm_write(val, ETMCNTRLDEVR1);
> +		return;
> +	case 0x5A:
> +		etm_write(val, ETMCNTRLDEVR2);
> +		return;
> +	case 0x5B:
> +		etm_write(val, ETMCNTRLDEVR3);
> +		return;
> +	case 0x5C:
> +		etm_write(val, ETMCNTVR0);
> +		return;
> +	case 0x5D:
> +		etm_write(val, ETMCNTVR1);
> +		return;
> +	case 0x5E:
> +		etm_write(val, ETMCNTVR2);
> +		return;
> +	case 0x5F:
> +		etm_write(val, ETMCNTVR3);
> +		return;
> +	case 0x60:
> +		etm_write(val, ETMSQ12EVR);
> +		return;
> +	case 0x61:
> +		etm_write(val, ETMSQ21EVR);
> +		return;
> +	case 0x62:
> +		etm_write(val, ETMSQ23EVR);
> +		return;
> +	case 0x63:
> +		etm_write(val, ETMSQ31EVR);
> +		return;
> +	case 0x64:
> +		etm_write(val, ETMSQ32EVR);
> +		return;
> +	case 0x65:
> +		etm_write(val, ETMSQ13EVR);
> +		return;
> +	case 0x67:
> +		etm_write(val, ETMSQR);
> +		return;
> +	case 0x68:
> +		etm_write(val, ETMEXTOUTEVR0);
> +		return;
> +	case 0x69:
> +		etm_write(val, ETMEXTOUTEVR1);
> +		return;
> +	case 0x6A:
> +		etm_write(val, ETMEXTOUTEVR2);
> +		return;
> +	case 0x6B:
> +		etm_write(val, ETMEXTOUTEVR3);
> +		return;
> +	case 0x6C:
> +		etm_write(val, ETMCIDCVR0);
> +		return;
> +	case 0x6D:
> +		etm_write(val, ETMCIDCVR1);
> +		return;
> +	case 0x6E:
> +		etm_write(val, ETMCIDCVR2);
> +		return;
> +	case 0x6F:
> +		etm_write(val, ETMCIDCMR);
> +		return;
> +	case 0x70:
> +		etm_write(val, ETMIMPSPEC0);
> +		return;
> +	case 0x71:
> +		etm_write(val, ETMIMPSPEC1);
> +		return;
> +	case 0x72:
> +		etm_write(val, ETMIMPSPEC2);
> +		return;
> +	case 0x73:
> +		etm_write(val, ETMIMPSPEC3);
> +		return;
> +	case 0x74:
> +		etm_write(val, ETMIMPSPEC4);
> +		return;
> +	case 0x75:
> +		etm_write(val, ETMIMPSPEC5);
> +		return;
> +	case 0x76:
> +		etm_write(val, ETMIMPSPEC6);
> +		return;
> +	case 0x77:
> +		etm_write(val, ETMIMPSPEC7);
> +		return;
> +	case 0x78:
> +		etm_write(val, ETMSYNCFR);
> +		return;
> +	case 0x7B:
> +		etm_write(val, ETMEXTINSELR);
> +		return;
> +	case 0x7C:
> +		etm_write(val, ETMTESSEICR);
> +		return;
> +	case 0x7D:
> +		etm_write(val, ETMEIBCR);
> +		return;
> +	case 0x7E:
> +		etm_write(val, ETMTSEVR);
> +		return;
> +	case 0x7F:
> +		etm_write(val, ETMAUXCR);
> +		return;
> +	case 0x80:
> +		etm_write(val, ETMTRACEIDR);
> +		return;
> +	case 0x90:
> +		etm_write(val, ETMVMIDCVR);
> +		return;
> +	case 0xC0:
> +		etm_write(val, ETMOSLAR);
> +		return;
> +	case 0xC2:
> +		etm_write(val, ETMOSSRR);
> +		return;
> +	case 0xC4:
> +		etm_write(val, ETMPDCR);
> +		return;
> +	case 0xC5:
> +		etm_write(val, ETMPDSR);
> +		return;
> +	default:
> +		WARN(1, "invalid CP14 access to ETM reg: %lx",
> +							(unsigned long)reg);
> +		return;
> +	}
> +}
> +
> +static inline uint32_t offset_to_reg_num(uint32_t off)
> +{
> +	return off >> 2;
> +}
> +
> +unsigned int etm_readl_cp14(uint32_t off)
> +{
> +	uint32_t reg = offset_to_reg_num(off);
> +	return etm_read_reg(reg);
> +}
> +
> +void etm_writel_cp14(uint32_t val, uint32_t off)
> +{
> +	uint32_t reg = offset_to_reg_num(off);
> +	etm_write_reg(val, reg);
> +}

Revisiting previous comments... maybe we don't have to divide the MM
constants by four either? We could just not divide them by four here.


> diff --git a/drivers/coresight/coresight-etm.c b/drivers/coresight/coresight-etm.c
> new file mode 100644
> index 0000000..59589be
> --- /dev/null
> +++ b/drivers/coresight/coresight-etm.c
> @@ -0,0 +1,2360 @@
> +/* Copyright (c) 2011-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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/smp.h>
> +#include <linux/sysfs.h>
> +#include <linux/stat.h>
> +#include <linux/spinlock.h>
> +#include <linux/clk.h>
> +#include <linux/cpu.h>
> +#include <linux/of.h>
> +#include <linux/of_coresight.h>
> +#include <linux/coresight.h>
> +#include <asm/sections.h>
> +
> +#include "coresight-priv.h"
> +
> +#define etm_writel_mm(drvdata, val, off)  \
> +			__raw_writel((val), drvdata->base + off)
> +#define etm_readl_mm(drvdata, off)        \
> +			__raw_readl(drvdata->base + off)
> +
> +#define etm_writel(drvdata, val, off)					\
> +({									\
> +	if (drvdata->use_cp14)						\
> +		etm_writel_cp14(val, off);				\
> +	else								\
> +		etm_writel_mm(drvdata, val, off);			\
> +})
> +#define etm_readl(drvdata, off)						\
> +({									\
> +	uint32_t val;							\
> +	if (drvdata->use_cp14)						\
> +		val = etm_readl_cp14(off);				\
> +	else								\
> +		val = etm_readl_mm(drvdata, off);			\
> +	val;								\
> +})

Why macros rather than inlines?


> +
> +#define ETM_LOCK(drvdata)						\
> +do {									\
> +	/* Recommended by spec to ensure ETM writes are committed */	\
> +	/* prior to resuming execution */				\
> +	mb();								\
> +	isb();								\
> +	etm_writel_mm(drvdata, 0x0, CORESIGHT_LAR);			\
> +} while (0)
> +#define ETM_UNLOCK(drvdata)						\
> +do {									\
> +	etm_writel_mm(drvdata, CORESIGHT_UNLOCK, CORESIGHT_LAR);	\
> +	/* Ensure unlock and any pending writes are committed prior */	\
> +	/* to programming ETM registers */				\
> +	mb();								\
> +	isb();								\
> +} while (0)

Why macros rather than inlines?


> +
> +#define PORT_SIZE_MASK		(BM(21, 21) | BM(4, 6))
> +
> +/*
> + * Device registers:
> + * 0x000 - 0x2FC: Trace		registers
> + * 0x300 - 0x314: Management	registers
> + * 0x318 - 0xEFC: Trace		registers
> + *
> + * Coresight registers
> + * 0xF00 - 0xF9C: Management	registers
> + * 0xFA0 - 0xFA4: Management	registers in PFTv1.0
> + *		  Trace		registers in PFTv1.1
> + * 0xFA8 - 0xFFC: Management	registers
> + */
> +
> +/* Trace registers (0x000-0x2FC) */
> +#define ETMCR			(0x000)
> +#define ETMCCR			(0x004)
> +#define ETMTRIGGER		(0x008)
> +#define ETMSR			(0x010)
> +#define ETMSCR			(0x014)
> +#define ETMTSSCR		(0x018)
> +#define ETMTECR2		(0x01c)
> +#define ETMTEEVR		(0x020)
> +#define ETMTECR1		(0x024)
> +#define ETMFFLR			(0x02C)
> +#define ETMACVRn(n)		(0x040 + (n * 4))
> +#define ETMACTRn(n)		(0x080 + (n * 4))
> +#define ETMCNTRLDVRn(n)		(0x140 + (n * 4))
> +#define ETMCNTENRn(n)		(0x150 + (n * 4))
> +#define ETMCNTRLDEVRn(n)	(0x160 + (n * 4))
> +#define ETMCNTVRn(n)		(0x170 + (n * 4))
> +#define ETMSQ12EVR		(0x180)
> +#define ETMSQ21EVR		(0x184)
> +#define ETMSQ23EVR		(0x188)
> +#define ETMSQ31EVR		(0x18C)
> +#define ETMSQ32EVR		(0x190)
> +#define ETMSQ13EVR		(0x194)
> +#define ETMSQR			(0x19C)
> +#define ETMEXTOUTEVRn(n)	(0x1A0 + (n * 4))
> +#define ETMCIDCVRn(n)		(0x1B0 + (n * 4))
> +#define ETMCIDCMR		(0x1BC)
> +#define ETMIMPSPEC0		(0x1C0)
> +#define ETMIMPSPEC1		(0x1C4)
> +#define ETMIMPSPEC2		(0x1C8)
> +#define ETMIMPSPEC3		(0x1CC)
> +#define ETMIMPSPEC4		(0x1D0)
> +#define ETMIMPSPEC5		(0x1D4)
> +#define ETMIMPSPEC6		(0x1D8)
> +#define ETMIMPSPEC7		(0x1DC)
> +#define ETMSYNCFR		(0x1E0)
> +#define ETMIDR			(0x1E4)
> +#define ETMCCER			(0x1E8)
> +#define ETMEXTINSELR		(0x1EC)
> +#define ETMTESSEICR		(0x1F0)
> +#define ETMEIBCR		(0x1F4)
> +#define ETMTSEVR		(0x1F8)
> +#define ETMAUXCR		(0x1FC)
> +#define ETMTRACEIDR		(0x200)
> +#define ETMVMIDCVR		(0x240)
> +/* Management registers (0x300-0x314) */
> +#define ETMOSLAR		(0x300)
> +#define ETMOSLSR		(0x304)
> +#define ETMOSSRR		(0x308)
> +#define ETMPDCR			(0x310)
> +#define ETMPDSR			(0x314)

Move to a header file so the CP14 code can use them ;-)

> +
> +#define ETM_MAX_ADDR_CMP	(16)
> +#define ETM_MAX_CNTR		(4)
> +#define ETM_MAX_CTXID_CMP	(3)
> +
> +#define ETM_MODE_EXCLUDE	BIT(0)
> +#define ETM_MODE_CYCACC		BIT(1)
> +#define ETM_MODE_STALL		BIT(2)
> +#define ETM_MODE_TIMESTAMP	BIT(3)
> +#define ETM_MODE_CTXID		BIT(4)
> +#define ETM_MODE_ALL		(0x1F)
> +
> +#define ETM_EVENT_MASK		(0x1FFFF)
> +#define ETM_SYNC_MASK		(0xFFF)
> +#define ETM_ALL_MASK		(0xFFFFFFFF)
> +
> +#define ETM_SEQ_STATE_MAX_VAL	(0x2)
> +
> +enum etm_addr_type {
> +	ETM_ADDR_TYPE_NONE,
> +	ETM_ADDR_TYPE_SINGLE,
> +	ETM_ADDR_TYPE_RANGE,
> +	ETM_ADDR_TYPE_START,
> +	ETM_ADDR_TYPE_STOP,
> +};
> +
> +#ifdef CONFIG_CORESIGHT_SOURCE_ETM_DEFAULT_ENABLE
> +static int boot_enable = 1;
> +#else
> +static int boot_enable;
> +#endif
> +module_param_named(
> +	boot_enable, boot_enable, int, S_IRUGO
> +);
> +
> +struct etm_drvdata {
> +	void __iomem			*base;
> +	struct device			*dev;
> +	struct coresight_device		*csdev;
> +	struct clk			*clk;
> +	spinlock_t			spinlock;
> +	int				cpu;
> +	int				port_size;
> +	uint8_t				arch;
> +	bool				use_cp14;
> +	bool				enable;
> +	bool				sticky_enable;
> +	bool				boot_enable;
> +	bool				os_unlock;
> +	uint8_t				nr_addr_cmp;
> +	uint8_t				nr_cntr;
> +	uint8_t				nr_ext_inp;
> +	uint8_t				nr_ext_out;
> +	uint8_t				nr_ctxid_cmp;
> +	uint8_t				reset;
> +	uint32_t			mode;
> +	uint32_t			ctrl;
> +	uint32_t			trigger_event;
> +	uint32_t			startstop_ctrl;
> +	uint32_t			enable_event;
> +	uint32_t			enable_ctrl1;
> +	uint32_t			fifofull_level;
> +	uint8_t				addr_idx;
> +	uint32_t			addr_val[ETM_MAX_ADDR_CMP];
> +	uint32_t			addr_acctype[ETM_MAX_ADDR_CMP];
> +	uint32_t			addr_type[ETM_MAX_ADDR_CMP];
> +	uint8_t				cntr_idx;
> +	uint32_t			cntr_rld_val[ETM_MAX_CNTR];
> +	uint32_t			cntr_event[ETM_MAX_CNTR];
> +	uint32_t			cntr_rld_event[ETM_MAX_CNTR];
> +	uint32_t			cntr_val[ETM_MAX_CNTR];
> +	uint32_t			seq_12_event;
> +	uint32_t			seq_21_event;
> +	uint32_t			seq_23_event;
> +	uint32_t			seq_31_event;
> +	uint32_t			seq_32_event;
> +	uint32_t			seq_13_event;
> +	uint32_t			seq_curr_state;
> +	uint8_t				ctxid_idx;
> +	uint32_t			ctxid_val[ETM_MAX_CTXID_CMP];
> +	uint32_t			ctxid_mask;
> +	uint32_t			sync_freq;
> +	uint32_t			timestamp_event;
> +};
> +
> +static struct etm_drvdata *etmdrvdata[NR_CPUS];
> +
> +/*
> + * Memory mapped writes to clear os lock are not supported on some processors
> + * and OS lock must be unlocked before any memory mapped access on such
> + * processors, otherwise memory mapped reads/writes will be invalid.
> + */
> +static void etm_os_unlock(void *info)
> +{
> +	struct etm_drvdata *drvdata = (struct etm_drvdata *)info;
> +	etm_writel(drvdata, 0x0, ETMOSLAR);
> +	isb();
> +}
> +
> +static void etm_set_pwrdwn(struct etm_drvdata *drvdata)
> +{
> +	uint32_t etmcr;
> +
> +	/* Ensure pending cp14 accesses complete before setting pwrdwn */
> +	mb();
> +	isb();
> +	etmcr = etm_readl(drvdata, ETMCR);
> +	etmcr |= BIT(0);
> +	etm_writel(drvdata, etmcr, ETMCR);
> +}
> +
> +static void etm_clr_pwrdwn(struct etm_drvdata *drvdata)
> +{
> +	uint32_t etmcr;
> +
> +	etmcr = etm_readl(drvdata, ETMCR);
> +	etmcr &= ~BIT(0);
> +	etm_writel(drvdata, etmcr, ETMCR);
> +	/* Ensure pwrup completes before subsequent cp14 accesses */
> +	mb();
> +	isb();
> +}
> +
> +static void etm_set_pwrup(struct etm_drvdata *drvdata)
> +{
> +	uint32_t etmpdcr;
> +
> +	etmpdcr = etm_readl_mm(drvdata, ETMPDCR);
> +	etmpdcr |= BIT(3);
> +	etm_writel_mm(drvdata, etmpdcr, ETMPDCR);

Why are register accesses _mm here? They are not in pwrdown.


> +	/* Ensure pwrup completes before subsequent cp14 accesses */
> +	mb();
> +	isb();
> +}
> +
> +static void etm_clr_pwrup(struct etm_drvdata *drvdata)
> +{
> +	uint32_t etmpdcr;
> +
> +	/* Ensure pending cp14 accesses complete before clearing pwrup */
> +	mb();
> +	isb();
> +	etmpdcr = etm_readl_mm(drvdata, ETMPDCR);
> +	etmpdcr &= ~BIT(3);
> +	etm_writel_mm(drvdata, etmpdcr, ETMPDCR);
> +}

Same here. Why _mm?


> +static void etm_set_prog(struct etm_drvdata *drvdata)
> +{
> +	uint32_t etmcr;
> +	int count;
> +
> +	etmcr = etm_readl(drvdata, ETMCR);
> +	etmcr |= BIT(10);
> +	etm_writel(drvdata, etmcr, ETMCR);
> +	/*
> +	 * Recommended by spec for cp14 accesses to ensure etmcr write is
> +	 * complete before polling etmsr
> +	 */
> +	isb();
> +	for (count = TIMEOUT_US; BVAL(etm_readl(drvdata, ETMSR), 1) != 1
> +				&& count > 0; count--)
> +		udelay(1);
> +	WARN(count == 0, "timeout while setting prog bit, ETMSR: %#x\n",
> +	     etm_readl(drvdata, ETMSR));
> +}
> +
> +static void etm_clr_prog(struct etm_drvdata *drvdata)
> +{
> +	uint32_t etmcr;
> +	int count;
> +
> +	etmcr = etm_readl(drvdata, ETMCR);
> +	etmcr &= ~BIT(10);
> +	etm_writel(drvdata, etmcr, ETMCR);
> +	/*
> +	 * Recommended by spec for cp14 accesses to ensure etmcr write is
> +	 * complete before polling etmsr
> +	 */
> +	isb();
> +	for (count = TIMEOUT_US; BVAL(etm_readl(drvdata, ETMSR), 1) != 0
> +				&& count > 0; count--)
> +		udelay(1);
> +	WARN(count == 0, "timeout while clearing prog bit, ETMSR: %#x\n",
> +	     etm_readl(drvdata, ETMSR));
> +}
> +
> +static void __etm_enable(void *info)
> +{
> +	int i;
> +	uint32_t etmcr;
> +	struct etm_drvdata *drvdata = info;
> +
> +	ETM_UNLOCK(drvdata);
> +
> +	/* turn engine on */
> +	etm_clr_pwrdwn(drvdata);
> +	/* apply power to trace registers */
> +	etm_set_pwrup(drvdata);
> +	/* make sure all registers are accessible */
> +	etm_os_unlock(drvdata);
> +
> +	etm_set_prog(drvdata);
> +
> +	etmcr = etm_readl(drvdata, ETMCR);
> +	etmcr &= (BIT(10) | BIT(0));
> +	etmcr |= drvdata->port_size;
> +	etm_writel(drvdata, drvdata->ctrl | etmcr, ETMCR);
> +	etm_writel(drvdata, drvdata->trigger_event, ETMTRIGGER);
> +	etm_writel(drvdata, drvdata->startstop_ctrl, ETMTSSCR);
> +	etm_writel(drvdata, drvdata->enable_event, ETMTEEVR);
> +	etm_writel(drvdata, drvdata->enable_ctrl1, ETMTECR1);
> +	etm_writel(drvdata, drvdata->fifofull_level, ETMFFLR);
> +	for (i = 0; i < drvdata->nr_addr_cmp; i++) {
> +		etm_writel(drvdata, drvdata->addr_val[i], ETMACVRn(i));
> +		etm_writel(drvdata, drvdata->addr_acctype[i], ETMACTRn(i));
> +	}
> +	for (i = 0; i < drvdata->nr_cntr; i++) {
> +		etm_writel(drvdata, drvdata->cntr_rld_val[i], ETMCNTRLDVRn(i));
> +		etm_writel(drvdata, drvdata->cntr_event[i], ETMCNTENRn(i));
> +		etm_writel(drvdata, drvdata->cntr_rld_event[i],
> +			   ETMCNTRLDEVRn(i));
> +		etm_writel(drvdata, drvdata->cntr_val[i], ETMCNTVRn(i));
> +	}
> +	etm_writel(drvdata, drvdata->seq_12_event, ETMSQ12EVR);
> +	etm_writel(drvdata, drvdata->seq_21_event, ETMSQ21EVR);
> +	etm_writel(drvdata, drvdata->seq_23_event, ETMSQ23EVR);
> +	etm_writel(drvdata, drvdata->seq_31_event, ETMSQ31EVR);
> +	etm_writel(drvdata, drvdata->seq_32_event, ETMSQ32EVR);
> +	etm_writel(drvdata, drvdata->seq_13_event, ETMSQ13EVR);
> +	etm_writel(drvdata, drvdata->seq_curr_state, ETMSQR);
> +	for (i = 0; i < drvdata->nr_ext_out; i++)
> +		etm_writel(drvdata, 0x0000406F, ETMEXTOUTEVRn(i));
> +	for (i = 0; i < drvdata->nr_ctxid_cmp; i++)
> +		etm_writel(drvdata, drvdata->ctxid_val[i], ETMCIDCVRn(i));
> +	etm_writel(drvdata, drvdata->ctxid_mask, ETMCIDCMR);
> +	etm_writel(drvdata, drvdata->sync_freq, ETMSYNCFR);
> +	etm_writel(drvdata, 0x00000000, ETMEXTINSELR);
> +	etm_writel(drvdata, drvdata->timestamp_event, ETMTSEVR);
> +	etm_writel(drvdata, 0x00000000, ETMAUXCR);
> +	etm_writel(drvdata, drvdata->cpu + 1, ETMTRACEIDR);
> +	etm_writel(drvdata, 0x00000000, ETMVMIDCVR);
> +
> +	/* ensures trace output is enabled from this ETM */
> +	etm_writel(drvdata, drvdata->ctrl | BIT(11) | etmcr, ETMCR);
> +
> +	etm_clr_prog(drvdata);
> +	ETM_LOCK(drvdata);
> +
> +	dev_dbg(drvdata->dev, "cpu: %d enable smp call done\n", drvdata->cpu);
> +}
> +
> +static int etm_enable(struct coresight_device *csdev)
> +{
> +	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	int ret;
> +
> +	ret = clk_prepare_enable(drvdata->clk);
> +	if (ret)
> +		goto err_clk;
> +
> +	spin_lock(&drvdata->spinlock);
> +
> +	/*
> +	 * Executing __etm_enable on the cpu whose ETM is being enabled
> +	 * ensures that register writes occur when cpu is powered.
> +	 */
> +	ret = smp_call_function_single(drvdata->cpu, __etm_enable, drvdata, 1);
> +	if (ret)
> +		goto err;
> +	drvdata->enable = true;
> +	drvdata->sticky_enable = true;
> +
> +	spin_unlock(&drvdata->spinlock);
> +
> +	dev_info(drvdata->dev, "ETM tracing enabled\n");
> +	return 0;
> +err:
> +	spin_unlock(&drvdata->spinlock);
> +	clk_disable_unprepare(drvdata->clk);
> +err_clk:
> +	return ret;
> +}
> +
> +static void __etm_disable(void *info)
> +{
> +	struct etm_drvdata *drvdata = info;
> +
> +	ETM_UNLOCK(drvdata);
> +	etm_set_prog(drvdata);
> +
> +	/* Program trace enable to low by using always false event */
> +	etm_writel(drvdata, 0x6F | BIT(14), ETMTEEVR);
> +
> +	etm_set_pwrdwn(drvdata);
> +	ETM_LOCK(drvdata);
> +
> +	dev_dbg(drvdata->dev, "cpu: %d disable smp call done\n", drvdata->cpu);
> +}
> +
> +static void etm_disable(struct coresight_device *csdev)
> +{
> +	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	/*
> +	 * Taking hotplug lock here protects from clocks getting disabled
> +	 * with tracing being left on (crash scenario) if user disable occurs
> +	 * after cpu online mask indicates the cpu is offline but before the
> +	 * DYING hotplug callback is serviced by the ETM driver.
> +	 */
> +	get_online_cpus();
> +	spin_lock(&drvdata->spinlock);
> +
> +	/*
> +	 * Executing __etm_disable on the cpu whose ETM is being disabled
> +	 * ensures that register writes occur when cpu is powered.
> +	 */
> +	smp_call_function_single(drvdata->cpu, __etm_disable, drvdata, 1);
> +	drvdata->enable = false;
> +
> +	spin_unlock(&drvdata->spinlock);
> +	put_online_cpus();
> +
> +	clk_disable_unprepare(drvdata->clk);
> +
> +	dev_info(drvdata->dev, "ETM tracing disabled\n");
> +}
> +
> +static const struct coresight_ops_source etm_source_ops = {
> +	.enable		= etm_enable,
> +	.disable	= etm_disable,
> +};
> +
> +static const struct coresight_ops etm_cs_ops = {
> +	.source_ops	= &etm_source_ops,
> +};
> +
> +static ssize_t debugfs_show_nr_addr_cmp(struct file *file,
> +					char __user *user_buf,
> +					size_t count, loff_t *ppos)
> +{
> +	int ret;
> +	unsigned long val;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	val = drvdata->nr_addr_cmp;
> +	ret = scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static const struct file_operations debugfs_nr_addr_cmp_ops = {
> +	.open = simple_open,
> +	.read = debugfs_show_nr_addr_cmp,
> +};


DEFINE_SIMPLE_ATTRIBUTE() would acheive the above with smaller code size
and no bugs.


> +static const struct coresight_ops_entry debugfs_nr_addr_cmp_entry = {
> +	.name = "nr_addr_cmp",
> +	.mode =  S_IRUGO,
> +	.ops = &debugfs_nr_addr_cmp_ops,
> +};

This (and its friends futher down) look samey enough to merit a
DEFINE_CORESIGHT_ENTRY() macro.


> +static ssize_t debugfs_show_nr_cntr(struct file *file,
> +				    char __user *user_buf,
> +				    size_t count, loff_t *ppos)
> +{
> +	int ret;
> +	unsigned long val;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	val = drvdata->nr_cntr;
> +	ret = scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static const struct file_operations debugfs_nr_cntr_ops = {
> +	.open = simple_open,
> +	.read = debugfs_show_nr_cntr,
> +};
> +
> +static const struct coresight_ops_entry debugfs_nr_cntr_entry = {
> +	.name = "nr_cntr",
> +	.mode =  S_IRUGO,
> +	.ops = &debugfs_nr_cntr_ops,
> +};

DEFINE_SIMPLE_ATTRIBITE() and DEFINE_CORESIGHT_ENTRY()

> +
> +static ssize_t debugfs_show_nr_ctxid_cmp(struct file *file,
> +					 char __user *user_buf,
> +					 size_t count, loff_t *ppos)
> +{
> +	int ret;
> +	unsigned long val;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	val = drvdata->nr_ctxid_cmp;
> +	ret = scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static const struct file_operations debugfs_nr_ctxid_cmp_ops = {
> +	.open = simple_open,
> +	.read = debugfs_show_nr_ctxid_cmp,
> +};
> +
> +static const struct coresight_ops_entry debugfs_nr_ctxid_cmp_entry = {
> +	.name = "nr_ctxid_cmp",
> +	.mode =  S_IRUGO,
> +	.ops = &debugfs_nr_ctxid_cmp_ops,
> +};

DEFINE_SIMPLE_ATTRIBITE() and DEFINE_CORESIGHT_ENTRY()

> +static ssize_t debugfs_show_reset(struct file *file,
> +				  char __user *user_buf,
> +				  size_t count, loff_t *ppos)
> +{
> +	int ret;
> +	unsigned long val;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	val = drvdata->reset;
> +	ret = scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +
> +	kfree(buf);
> +	return ret;
> +}
> +
> +/* Reset to trace everything i.e. exclude nothing. */
> +static ssize_t debugfs_store_reset(struct file *file,
> +				   const char __user *user_buf,
> +				   size_t count, loff_t *ppos)
> +{
> +	int i;
> +	unsigned long val;
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	if (sscanf(user_buf, "%lx", &val) != 1)
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	if (val) {
> +		drvdata->mode = ETM_MODE_EXCLUDE;
> +		drvdata->ctrl = 0x0;
> +		drvdata->trigger_event = 0x406F;
> +		drvdata->startstop_ctrl = 0x0;
> +		drvdata->enable_event = 0x6F;
> +		drvdata->enable_ctrl1 = 0x1000000;
> +		drvdata->fifofull_level = 0x28;
> +		drvdata->addr_idx = 0x0;
> +		for (i = 0; i < drvdata->nr_addr_cmp; i++) {
> +			drvdata->addr_val[i] = 0x0;
> +			drvdata->addr_acctype[i] = 0x0;
> +			drvdata->addr_type[i] = ETM_ADDR_TYPE_NONE;
> +		}
> +		drvdata->cntr_idx = 0x0;
> +		for (i = 0; i < drvdata->nr_cntr; i++) {
> +			drvdata->cntr_rld_val[i] = 0x0;
> +			drvdata->cntr_event[i] = 0x406F;
> +			drvdata->cntr_rld_event[i] = 0x406F;
> +			drvdata->cntr_val[i] = 0x0;
> +		}
> +		drvdata->seq_12_event = 0x406F;
> +		drvdata->seq_21_event = 0x406F;
> +		drvdata->seq_23_event = 0x406F;
> +		drvdata->seq_31_event = 0x406F;
> +		drvdata->seq_32_event = 0x406F;
> +		drvdata->seq_13_event = 0x406F;
> +		drvdata->seq_curr_state = 0x0;
> +		drvdata->ctxid_idx = 0x0;
> +		for (i = 0; i < drvdata->nr_ctxid_cmp; i++)
> +			drvdata->ctxid_val[i] = 0x0;
> +		drvdata->ctxid_mask = 0x0;
> +		drvdata->sync_freq = 0x100;
> +		drvdata->timestamp_event = 0x406F;
> +	}
> +	spin_unlock(&drvdata->spinlock);
> +	return count;
> +}

This smells like it shared lots of code with __etm_enable(). Not sure
though with all those 0x406F.

Whatever the case, this code should probably be hoisted out of the
debugfs code and into a named function.

> +
> +static const struct file_operations debugfs_reset_ops = {
> +	.open = simple_open,
> +	.read = debugfs_show_reset,
> +	.write = debugfs_store_reset,
> +};
> +
> +static const struct coresight_ops_entry debugfs_reset_entry = {
> +	.name = "reset",
> +	.mode =  S_IRUGO | S_IWUSR,
> +	.ops = &debugfs_reset_ops,
> +};
> +
> +static ssize_t debugfs_show_mode(struct file *file,
> +				 char __user *user_buf,
> +				 size_t count, loff_t *ppos)
> +{
> +	int ret;
> +	unsigned long val;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	val = drvdata->mode;
> +	ret = scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static ssize_t debugfs_store_mode(struct file *file,
> +				  const char __user *user_buf,
> +				  size_t count, loff_t *ppos)
> +{
> +	unsigned long val;
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	if (sscanf(user_buf, "%lx", &val) != 1)
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	drvdata->mode = val & ETM_MODE_ALL;
> +
> +	if (drvdata->mode & ETM_MODE_EXCLUDE)
> +		drvdata->enable_ctrl1 |= BIT(24);
> +	else
> +		drvdata->enable_ctrl1 &= ~BIT(24);
> +
> +	if (drvdata->mode & ETM_MODE_CYCACC)
> +		drvdata->ctrl |= BIT(12);
> +	else
> +		drvdata->ctrl &= ~BIT(12);
> +
> +	if (drvdata->mode & ETM_MODE_STALL)
> +		drvdata->ctrl |= BIT(7);
> +	else
> +		drvdata->ctrl &= ~BIT(7);
> +
> +	if (drvdata->mode & ETM_MODE_TIMESTAMP)
> +		drvdata->ctrl |= BIT(28);
> +	else
> +		drvdata->ctrl &= ~BIT(28);
> +
> +	if (drvdata->mode & ETM_MODE_CTXID)
> +		drvdata->ctrl |= (BIT(14) | BIT(15));
> +	else
> +		drvdata->ctrl &= ~(BIT(14) | BIT(15));
> +	spin_unlock(&drvdata->spinlock);
> +
> +	return count;
> +}
> +
> +static const struct file_operations debugfs_mode_ops = {
> +	.open = simple_open,
> +	.read = debugfs_show_mode,
> +	.write = debugfs_store_mode,
> +};
> +
> +static const struct coresight_ops_entry debugfs_mode_entry = {
> +	.name = "mode",
> +	.mode =  S_IRUGO | S_IWUSR,
> +	.ops = &debugfs_mode_ops,
> +};

DEFINE_SIMPLE_ATTRIBITE() and DEFINE_CORESIGHT_ENTRY()

> +
> +static ssize_t debugfs_show_trigger_event(struct file *file,
> +					  char __user *user_buf,
> +					  size_t count, loff_t *ppos)
> +{
> +	int ret;
> +	unsigned long val;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	val = drvdata->trigger_event;
> +	ret = scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static ssize_t debugfs_store_trigger_event(struct file *file,
> +					   const char __user *user_buf,
> +					   size_t count, loff_t *ppos)
> +{
> +	unsigned long val;
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	if (sscanf(user_buf, "%lx", &val) != 1)
> +		return -EINVAL;
> +
> +	drvdata->trigger_event = val & ETM_EVENT_MASK;
> +	return count;
> +}
> +
> +static const struct file_operations debugfs_trigger_event_ops = {
> +	.open = simple_open,
> +	.read = debugfs_show_trigger_event,
> +	.write = debugfs_store_trigger_event,
> +};
> +
> +static const struct coresight_ops_entry debugfs_trigger_events_entry = {
> +	.name = "trigger_event",
> +	.mode =  S_IRUGO | S_IWUSR,
> +	.ops = &debugfs_trigger_event_ops,
> +};

DEFINE_SIMPLE_ATTRIBITE() and DEFINE_CORESIGHT_ENTRY()

> +
> +static ssize_t debugfs_show_enable_event(struct file *file,
> +					 char __user *user_buf,
> +					 size_t count, loff_t *ppos)
> +{
> +	int ret;
> +	unsigned long val;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	val = drvdata->enable_event;
> +	ret = scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static ssize_t debugfs_store_enable_event(struct file *file,
> +					  const char __user *user_buf,
> +					  size_t count, loff_t *ppos)
> +{
> +	unsigned long val;
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	if (sscanf(user_buf, "%lx", &val) != 1)
> +		return -EINVAL;
> +
> +	drvdata->enable_event = val & ETM_EVENT_MASK;
> +	return count;
> +}
> +
> +static const struct file_operations debugfs_enable_event_ops = {
> +	.open = simple_open,
> +	.read = debugfs_show_enable_event,
> +	.write = debugfs_store_enable_event,
> +};
> +
> +static const struct coresight_ops_entry debugfs_enable_events_entry = {
> +	.name = "enable_event",
> +	.mode =  S_IRUGO | S_IWUSR,
> +	.ops = &debugfs_enable_event_ops,
> +};
> +

DEFINE_SIMPLE_ATTRIBITE() and DEFINE_CORESIGHT_ENTRY()

> +static ssize_t debugfs_show_fifofull_level(struct file *file,
> +					   char __user *user_buf,
> +					   size_t count, loff_t *ppos)
> +{
> +	int ret;
> +	unsigned long val;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	val = drvdata->fifofull_level;
> +	ret = scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static ssize_t debugfs_store_fifofull_level(struct file *file,
> +					    const char __user *user_buf,
> +					    size_t count, loff_t *ppos)
> +{
> +	unsigned long val;
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	if (sscanf(user_buf, "%lx", &val) != 1)
> +		return -EINVAL;
> +
> +	drvdata->fifofull_level = val;
> +	return count;
> +}
> +
> +static const struct file_operations debugfs_fifofull_level_ops = {
> +	.open = simple_open,
> +	.read = debugfs_show_fifofull_level,
> +	.write = debugfs_store_fifofull_level,
> +};
> +
> +static const struct coresight_ops_entry debugfs_fifofull_level_entry = {
> +	.name = "fifofull_level",
> +	.mode =  S_IRUGO | S_IWUSR,
> +	.ops = &debugfs_fifofull_level_ops,
> +};
> +

DEFINE_SIMPLE_ATTRIBITE() and DEFINE_CORESIGHT_ENTRY()

> +static ssize_t debugfs_show_addr_idx(struct file *file,
> +				     char __user *user_buf,
> +				     size_t count, loff_t *ppos)
> +{
> +	int ret;
> +	unsigned long val;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	val = drvdata->addr_idx;
> +	ret = scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static ssize_t debugfs_store_addr_idx(struct file *file,
> +				      const char __user *user_buf,
> +				      size_t count, loff_t *ppos)
> +{
> +	unsigned long val;
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	if (sscanf(user_buf, "%lx", &val) != 1)
> +		return -EINVAL;
> +	if (val >= drvdata->nr_addr_cmp)
> +		return -EINVAL;
> +
> +	/*
> +	 * Use spinlock to ensure index doesn't change while it gets
> +	 * dereferenced multiple times within a spinlock block elsewhere.
> +	 */
> +	spin_lock(&drvdata->spinlock);
> +	drvdata->addr_idx = val;
> +	spin_unlock(&drvdata->spinlock);
> +	return count;
> +}
> +
> +static const struct file_operations debugfs_addr_idx_ops = {
> +	.open = simple_open,
> +	.read = debugfs_show_addr_idx,
> +	.write = debugfs_store_addr_idx,
> +};
> +
> +static const struct coresight_ops_entry debugfs_addr_idx_entry = {
> +	.name = "addr_idx",
> +	.mode =  S_IRUGO | S_IWUSR,
> +	.ops = &debugfs_addr_idx_ops,
> +};
> +

DEFINE_SIMPLE_ATTRIBITE() and DEFINE_CORESIGHT_ENTRY()

> +static ssize_t debugfs_show_addr_single(struct file *file,
> +					char __user *user_buf,
> +					size_t count, loff_t *ppos)
> +{
> +	int ret;
> +	uint8_t idx;
> +	unsigned long val;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	spin_lock(&drvdata->spinlock);
> +	idx = drvdata->addr_idx;
> +	if (!(drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
> +	      drvdata->addr_type[idx] == ETM_ADDR_TYPE_SINGLE)) {
> +		spin_unlock(&drvdata->spinlock);
> +		return -EPERM;

-EPERM?

Is this really a permissions check.

> +	}
> +
> +	val = drvdata->addr_val[idx];
> +	spin_unlock(&drvdata->spinlock);
> +	ret = scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static ssize_t debugfs_store_addr_single(struct file *file,
> +					 const char __user *user_buf,
> +					 size_t count, loff_t *ppos)
> +{
> +	uint8_t idx;
> +	unsigned long val;
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	if (sscanf(user_buf, "%lx", &val) != 1)
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	idx = drvdata->addr_idx;
> +	if (!(drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
> +	      drvdata->addr_type[idx] == ETM_ADDR_TYPE_SINGLE)) {
> +		spin_unlock(&drvdata->spinlock);
> +		return -EPERM;

-EPERM?

> +	}
> +
> +	drvdata->addr_val[idx] = val;
> +	drvdata->addr_type[idx] = ETM_ADDR_TYPE_SINGLE;
> +	spin_unlock(&drvdata->spinlock);
> +	return count;
> +}
> +
> +static const struct file_operations debugfs_addr_single_ops = {
> +	.open = simple_open,
> +	.read = debugfs_show_addr_single,
> +	.write = debugfs_store_addr_single,
> +};
> +
> +static const struct coresight_ops_entry debugfs_addr_single_entry = {
> +	.name = "addr_single",
> +	.mode =  S_IRUGO | S_IWUSR,
> +	.ops = &debugfs_addr_single_ops,
> +};

DEFINE_SIMPLE_ATTRIBITE() and DEFINE_CORESIGHT_ENTRY()

> +static ssize_t debugfs_show_addr_range(struct file *file,
> +				       char __user *user_buf,
> +				       size_t count, loff_t *ppos)
> +{
> +	int ret;
> +	uint8_t idx;
> +	unsigned long val1, val2;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	spin_lock(&drvdata->spinlock);
> +	idx = drvdata->addr_idx;
> +	if (idx % 2 != 0) {
> +		spin_unlock(&drvdata->spinlock);
> +		return -EPERM;

-EPERM?

> +	}
> +	if (!((drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE &&
> +	       drvdata->addr_type[idx + 1] == ETM_ADDR_TYPE_NONE) ||
> +	      (drvdata->addr_type[idx] == ETM_ADDR_TYPE_RANGE &&
> +	       drvdata->addr_type[idx + 1] == ETM_ADDR_TYPE_RANGE))) {
> +		spin_unlock(&drvdata->spinlock);
> +		return -EPERM;

-EPERM?

> +	}
> +
> +	val1 = drvdata->addr_val[idx];
> +	val2 = drvdata->addr_val[idx + 1];
> +	spin_unlock(&drvdata->spinlock);
> +	ret = scnprintf(buf, PAGE_SIZE, "%#lx %#lx\n", val1, val2);
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static ssize_t debugfs_store_addr_range(struct file *file,
> +					const char __user *user_buf,
> +					size_t count, loff_t *ppos)
> +{
> +	uint8_t idx;
> +	unsigned long val1, val2;
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	if (sscanf(user_buf, "%lx %lx", &val1, &val2) != 2)
> +		return -EINVAL;
> +	/* Lower address comparator cannot have a higher address value */
> +	if (val1 > val2)
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	idx = drvdata->addr_idx;
> +	if (idx % 2 != 0) {
> +		spin_unlock(&drvdata->spinlock);
> +		return -EPERM;

-EPERM?

> +	}
> +	if (!((drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE &&
> +	       drvdata->addr_type[idx + 1] == ETM_ADDR_TYPE_NONE) ||
> +	      (drvdata->addr_type[idx] == ETM_ADDR_TYPE_RANGE &&
> +	       drvdata->addr_type[idx + 1] == ETM_ADDR_TYPE_RANGE))) {
> +		spin_unlock(&drvdata->spinlock);
> +		return -EPERM;

-EPERM?

> +	}
> +
> +	drvdata->addr_val[idx] = val1;
> +	drvdata->addr_type[idx] = ETM_ADDR_TYPE_RANGE;
> +	drvdata->addr_val[idx + 1] = val2;
> +	drvdata->addr_type[idx + 1] = ETM_ADDR_TYPE_RANGE;
> +	drvdata->enable_ctrl1 |= (1 << (idx/2));
> +	spin_unlock(&drvdata->spinlock);
> +	return count;
> +}
> +
> +static const struct file_operations debugfs_addr_range_ops = {
> +	.open = simple_open,
> +	.read = debugfs_show_addr_range,
> +	.write = debugfs_store_addr_range,
> +};
> +
> +static const struct coresight_ops_entry debugfs_addr_range_entry = {
> +	.name = "addr_range",
> +	.mode =  S_IRUGO | S_IWUSR,
> +	.ops = &debugfs_addr_range_ops,
> +};

DEFINE_SIMPLE_ATTRIBITE() and DEFINE_CORESIGHT_ENTRY()

SNIP!!!

My comments of debugfs get a bit samey from here on down so I've deleted
a big chunk.


> +static ssize_t debugfs_status_read(struct file *file, char __user *user_buf,
> +				   size_t count, loff_t *ppos)
> +{
> +	ssize_t ret;
> +	uint32_t val;
> +	unsigned long flags;
> +	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	struct etm_drvdata *drvdata = file->private_data;
> +
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	ret = clk_prepare_enable(drvdata->clk);
> +	if (ret)
> +		goto out;
> +
> +	spin_lock_irqsave(&drvdata->spinlock, flags);
> +
> +	ETM_UNLOCK(drvdata);
> +	val = etm_readl(drvdata, ETMCCR);
> +	ret += sprintf(buf, "ETMCCR: 0x%08x\n", val);
> +	val = etm_readl(drvdata, ETMCCER);
> +	ret += sprintf(buf + ret, "ETMCCER: 0x%08x\n", val);
> +	val = etm_readl(drvdata, ETMSCR);
> +	ret += sprintf(buf + ret, "ETMSCR: 0x%08x\n", val);
> +	val = etm_readl(drvdata, ETMIDR);
> +	ret += sprintf(buf + ret, "ETMIDR: 0x%08x\n", val);
> +	val = etm_readl(drvdata, ETMCR);
> +	ret += sprintf(buf + ret, "ETMCR: 0x%08x\n", val);
> +	val = etm_readl(drvdata, ETMTEEVR);
> +	ret += sprintf(buf + ret, "Enable event: 0x%08x\n", val);
> +	val = etm_readl(drvdata, ETMTSSCR);
> +	ret += sprintf(buf + ret, "Enable start/stop: 0x%08x\n", val);
> +	ret += sprintf(buf + ret,
> +		       "Enable control: CR1 0x%08x CR2 0x%08x\n",
> +		       etm_readl(drvdata, ETMTECR1),
> +		       etm_readl(drvdata, ETMTECR2));
> +
> +	ETM_LOCK(drvdata);
> +
> +	spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +	clk_disable_unprepare(drvdata->clk);
> +
> +	ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +out:
> +	kfree(buf);
> +	return ret;
> +}

Really not sure whether this should be in the read method. If we don't
read the file in one go the spin_lock() we'll not get a cohesive set of
registers.


> +
> +static const struct file_operations debugfs_status_ops = {
> +	.open = simple_open,
> +	.read = debugfs_status_read,
> +};
> +
> +static const struct coresight_ops_entry debugfs_status_entry = {
> +	.name = "status",
> +	.mode =  S_IRUGO,
> +	.ops = &debugfs_status_ops,
> +};
> +
> +static const struct coresight_ops_entry *etm_attr_grps[] = {
> +	&debugfs_nr_addr_cmp_entry,
> +	&debugfs_nr_cntr_entry,
> +	&debugfs_nr_ctxid_cmp_entry,
> +	&debugfs_reset_entry,
> +	&debugfs_mode_entry,
> +	&debugfs_trigger_events_entry,
> +	&debugfs_enable_events_entry,
> +	&debugfs_fifofull_level_entry,
> +	&debugfs_addr_idx_entry,
> +	&debugfs_addr_single_entry,
> +	&debugfs_addr_range_entry,
> +	&debugfs_addr_start_entry,
> +	&debugfs_addr_stop_entry,
> +	&debugfs_addr_acctype_entry,
> +	&debugfs_cntr_idx_entry,
> +	&debugfs_cntr_rld_val_entry,
> +	&debugfs_cntr_event_entry,
> +	&debugfs_cntr_rld_event_entry,
> +	&debugfs_cntr_val_entry,
> +	&debugfs_12_event_entry,
> +	&debugfs_21_event_entry,
> +	&debugfs_23_event_entry,
> +	&debugfs_31_event_entry,
> +	&debugfs_32_event_entry,
> +	&debugfs_13_event_entry,
> +	&debugfs_seq_curr_state_entry,
> +	&debugfs_ctxid_idx_entry,
> +	&debugfs_ctxid_val_entry,
> +	&debugfs_ctxid_mask_entry,
> +	&debugfs_sync_freq_entry,
> +	&debugfs_timestamp_event_entry,
> +	&debugfs_status_entry,
> +	NULL,
> +};
> +
> +static int etm_cpu_callback(struct notifier_block *nfb, unsigned long action,
> +			    void *hcpu)
> +{
> +	unsigned int cpu = (unsigned long)hcpu;
> +
> +	if (!etmdrvdata[cpu])
> +		goto out;
> +
> +	switch (action & (~CPU_TASKS_FROZEN)) {
> +	case CPU_STARTING:
> +		spin_lock(&etmdrvdata[cpu]->spinlock);
> +		if (!etmdrvdata[cpu]->os_unlock) {
> +			etm_os_unlock(etmdrvdata[cpu]);
> +			etmdrvdata[cpu]->os_unlock = true;
> +		}
> +
> +		if (etmdrvdata[cpu]->enable)
> +			__etm_enable(etmdrvdata[cpu]);
> +		spin_unlock(&etmdrvdata[cpu]->spinlock);
> +		break;
> +
> +	case CPU_ONLINE:
> +		if (etmdrvdata[cpu]->boot_enable &&
> +		    !etmdrvdata[cpu]->sticky_enable)
> +			coresight_enable(etmdrvdata[cpu]->csdev);
> +		break;
> +
> +	case CPU_DYING:
> +		spin_lock(&etmdrvdata[cpu]->spinlock);
> +		if (etmdrvdata[cpu]->enable)
> +			__etm_disable(etmdrvdata[cpu]);
> +		spin_unlock(&etmdrvdata[cpu]->spinlock);
> +		break;
> +	}
> +out:
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block etm_cpu_notifier = {
> +	.notifier_call = etm_cpu_callback,
> +};
> +
> +static bool etm_arch_supported(uint8_t arch)
> +{
> +	switch (arch) {
> +	case ETM_ARCH_V3_3:
> +		break;
> +	case ETM_ARCH_V3_5:
> +		break;
> +	case PFT_ARCH_V1_1:
> +		break;
> +	default:
> +		return false;
> +	}
> +	return true;
> +}
> +
> +static void etm_init_arch_data(void *info)
> +{
> +	uint32_t etmidr;
> +	uint32_t etmccr;
> +	struct etm_drvdata *drvdata = info;
> +
> +	ETM_UNLOCK(drvdata);
> +
> +	/* first dummy read */
> +	(void)etm_readl(drvdata, ETMPDSR);
> +	/* Provide power to ETM: ETMPDCR[3] == 1 */
> +	etm_set_pwrup(drvdata);
> +	/*
> +	 * Clear power down bit since when this bit is set writes to
> +	 * certain registers might be ignored.
> +	 */
> +	etm_clr_pwrdwn(drvdata);
> +	/*
> +	 * Set prog bit. It will be set from reset but this is included to
> +	 * ensure it is set
> +	 */
> +	etm_set_prog(drvdata);
> +
> +	/* Find all capabilities */
> +	etmidr = etm_readl(drvdata, ETMIDR);
> +	drvdata->arch = BMVAL(etmidr, 4, 11);
> +	drvdata->port_size = etm_readl(drvdata, ETMCR) & PORT_SIZE_MASK;
> +
> +	etmccr = etm_readl(drvdata, ETMCCR);
> +	drvdata->nr_addr_cmp = BMVAL(etmccr, 0, 3) * 2;
> +	drvdata->nr_cntr = BMVAL(etmccr, 13, 15);
> +	drvdata->nr_ext_inp = BMVAL(etmccr, 17, 19);
> +	drvdata->nr_ext_out = BMVAL(etmccr, 20, 22);
> +	drvdata->nr_ctxid_cmp = BMVAL(etmccr, 24, 25);
> +
> +	etm_set_pwrdwn(drvdata);
> +	etm_clr_pwrup(drvdata);
> +	ETM_LOCK(drvdata);
> +}
> +
> +static void etm_init_default_data(struct etm_drvdata *drvdata)
> +{
> +	int i;
> +
> +	uint32_t flags = (1 << 0 | /* instruction execute*/
> +			  3 << 3 | /* ARM instruction */
> +			  0 << 5 | /* No data value comparison */
> +			  0 << 7 | /* No exact mach */
> +			  0 << 8 | /* Ignore context ID */
> +			  0 << 10); /* Security ignored */
> +
> +	drvdata->ctrl = (BIT(12) | /* cycle accurate */
> +			 BIT(28)); /* timestamp */
> +	drvdata->trigger_event = 0x406F;
> +	drvdata->enable_event = 0x6F;
> +	drvdata->enable_ctrl1 = 0x1;
> +	drvdata->fifofull_level	= 0x28;
> +	if (drvdata->nr_addr_cmp >= 2) {
> +		drvdata->addr_val[0] = (uint32_t) _stext;
> +		drvdata->addr_val[1] = (uint32_t) _etext;
> +		drvdata->addr_acctype[0] = flags;
> +		drvdata->addr_acctype[1] = flags;
> +		drvdata->addr_type[0] = ETM_ADDR_TYPE_RANGE;
> +		drvdata->addr_type[1] = ETM_ADDR_TYPE_RANGE;
> +	}
> +	for (i = 0; i < drvdata->nr_cntr; i++) {
> +		drvdata->cntr_event[i] = 0x406F;
> +		drvdata->cntr_rld_event[i] = 0x406F;
> +	}
> +	drvdata->seq_12_event = 0x406F;
> +	drvdata->seq_21_event = 0x406F;
> +	drvdata->seq_23_event = 0x406F;
> +	drvdata->seq_31_event = 0x406F;
> +	drvdata->seq_32_event = 0x406F;
> +	drvdata->seq_13_event = 0x406F;
> +	drvdata->sync_freq = 0x100;
> +	drvdata->timestamp_event = 0x406F;
> +}

Ah... here's all those 0x406F I thought looked odd in the implementation
of the reset attribute. This code should be commoned up as much as
possible with the reset code.

Also perhaps a #define to explain what 0x406F means?


> +
> +static int etm_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct device *dev = &pdev->dev;
> +	struct coresight_platform_data *pdata = NULL;
> +	struct etm_drvdata *drvdata;
> +	struct resource *res;
> +	static int count;

That "static" is very well concealed. I missed that until I started
studying the error paths.

> +	struct coresight_desc *desc;
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	if (pdev->dev.of_node) {
> +		pdata = of_get_coresight_platform_data(dev, pdev->dev.of_node);
> +		if (IS_ERR(pdata))
> +			return PTR_ERR(pdata);
> +		pdev->dev.platform_data = pdata;
> +		drvdata->use_cp14 = of_property_read_bool(pdev->dev.of_node,
> +							  "arm,cp14");
> +	}
> +
> +	drvdata->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, drvdata);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +
> +	drvdata->base = devm_ioremap(dev, res->start, resource_size(res));

Leak on error paths?

> +	if (!drvdata->base)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&drvdata->spinlock);
> +
> +	if (pdata && pdata->clk) {
> +		drvdata->clk = pdata->clk;
> +		ret = clk_prepare_enable(drvdata->clk);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	drvdata->cpu = pdata ? pdata->cpu : 0;
> +
> +	get_online_cpus();
> +	etmdrvdata[drvdata->cpu] = drvdata;
> +
> +	if (!smp_call_function_single(drvdata->cpu, etm_os_unlock, drvdata, 1))
> +		drvdata->os_unlock = true;
> +
> +	if (smp_call_function_single(drvdata->cpu,
> +				     etm_init_arch_data,  drvdata, 1))
> +		dev_err(dev, "ETM arch init failed\n");
> +
> +	if (!count++)

count is mishandled on the error paths?

> +		register_hotcpu_notifier(&etm_cpu_notifier);

Leak on (some of the) error paths?

> +
> +	put_online_cpus();
> +
> +	if (etm_arch_supported(drvdata->arch) == false) {
> +		clk_disable_unprepare(drvdata->clk);
> +		return -EINVAL;
> +	}
> +	etm_init_default_data(drvdata);
> +
> +	clk_disable_unprepare(drvdata->clk);
> +
> +	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);

Leak on error paths?

> +	if (!desc) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +	desc->type = CORESIGHT_DEV_TYPE_SOURCE;
> +	desc->subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_PROC;
> +	desc->ops = &etm_cs_ops;
> +	desc->pdata = pdev->dev.platform_data;
> +	desc->dev = &pdev->dev;
> +	desc->debugfs_ops = etm_attr_grps;
> +	desc->owner = THIS_MODULE;
> +	drvdata->csdev = coresight_register(desc);
> +	if (IS_ERR(drvdata->csdev)) {
> +		ret = PTR_ERR(drvdata->csdev);
> +		goto err;
> +	}
> +
> +	dev_info(dev, "ETM initialized\n");
> +
> +	if (boot_enable) {
> +		coresight_enable(drvdata->csdev);
> +		drvdata->boot_enable = true;
> +	}
> +
> +	return 0;
> +err:
> +	if (drvdata->cpu == 0)
> +		unregister_hotcpu_notifier(&etm_cpu_notifier);
> +	return ret;
> +}
> +
> +static int etm_remove(struct platform_device *pdev)
> +{
> +	struct etm_drvdata *drvdata = platform_get_drvdata(pdev);
> +
> +	coresight_unregister(drvdata->csdev);
> +	if (drvdata->cpu == 0)
> +		unregister_hotcpu_notifier(&etm_cpu_notifier);
> +	return 0;
> +}
> +
> +static struct of_device_id etm_match[] = {
> +	{.compatible = "arm,coresight-etm"},
> +	{}
> +};
> +
> +static struct platform_driver etm_driver = {
> +	.probe          = etm_probe,
> +	.remove         = etm_remove,
> +	.driver         = {
> +		.name   = "coresight-etm",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = etm_match,
> +	},
> +};
> +
> +int __init etm_init(void)
> +{
> +	return platform_driver_register(&etm_driver);
> +}
> +module_init(etm_init);
> +
> +void __exit etm_exit(void)
> +{
> +	platform_driver_unregister(&etm_driver);
> +}
> +module_exit(etm_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("CoreSight Program Flow Trace driver");
>