Re: [RFC PATCH 07/11] coresight: add CoreSight ETM driver
From: Daniel Thompson
Date: Tue Jun 03 2014 - 06:27:08 EST
On 30/05/14 14:43, mathieu.poirier@xxxxxxxxxx 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");
>
--
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/