From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BCE26C3A5A1 for ; Wed, 28 Aug 2019 14:15:53 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8C18E2189D for ; Wed, 28 Aug 2019 14:15:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="YwqfbomM"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="IWMef2Q+" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8C18E2189D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=HX/gcmnIMqXZBcyPGlp4f6ifmO8wSF/fB7ed+GBHIOg=; b=YwqfbomMgTq0c7 TErRML4/NwgyGxMSSizfvx3GtI+XcO6rhUDC2yZekAoEP5yMZpT/qLhdGTaLwtl/8EkwSs0ozPtFF AhOUE8TbFI1eQLVzwb4NEXlwCraLEuONZ8590F9gV6QX4F/hBEpvPMQU86dDSobEiPZcDk5UL82rM Jt08JjEMl3FAMaXpNc/PYJV9XpBvX0tuuY4d7LiYWkNxxBva0kE4F4F1yiVGCgrkV9MM7pH8z5mlg 46Jsg90dS94lu6NdH1UnGFFui6jUBWgxAi00cWGgdh3bmcywMcT5XFeSi014dhUimVJpVF+PE38gM j5XVY63o07LkZiuBdl7A==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1i2yjg-00025i-Mu; Wed, 28 Aug 2019 14:15:44 +0000 Received: from mail-qt1-x841.google.com ([2607:f8b0:4864:20::841]) by bombadil.infradead.org with esmtps (Exim 4.92 #3 (Red Hat Linux)) id 1i2yjd-000257-AO for linux-arm-kernel@lists.infradead.org; Wed, 28 Aug 2019 14:15:43 +0000 Received: by mail-qt1-x841.google.com with SMTP id q64so3160513qtd.5 for ; Wed, 28 Aug 2019 07:15:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Ap/Cm+u1DjsmoSGJn+7UdP1qyu4V/MqMRJNVuJDCIWQ=; b=IWMef2Q+LL6ApZuR9F8vmQwVg2R3Wp04JbwDqM4IOYjH26mS/x8SY1/ZkurI6E9wUv ZVUPxJi5Jwx+7kW78YvzgxxfVL31HnTkwVHi15q7NARKfwjmRM0O3vx3g8j7qaTEqlJz L6QFiQrjHrpFXLrNzeP576ZJXQtVGI9cCkGyWOikajvDHjsUORBO/swTD0fOXcNJY3Xg wZu0qENV17GCrz/VwpicU0+U7iFBQEQ2pxZHGEU1m2ntn5SbMJ2fw7EqsPIWsZawaCTn 6rB5TnvnrrFLpHzpWmi4ne10FQ8IJrBt2TGpANx1ZYmluZRyHlX3xNVnTZ/W+GZH4Oaf rUCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Ap/Cm+u1DjsmoSGJn+7UdP1qyu4V/MqMRJNVuJDCIWQ=; b=QruSm533XlPngI12686j7iLzb5tBjSOd5MFUE6e9UvfjkERPkvKalikgbcy35Yyr9H k4asuh79cHMYcu/+LnzMcFpcWWEz/QFn17kJecbfd1efO1pDto9p6DIH/5LmOY6zRS5x ngdrz6zUN/SSvrDRnMNU9zROg/RkP8oen3kJ8ZBywvCiYMmLzketanczlGMYILcG+xPl 2BuBhFXpnYqHVIr147S4FGcg4TRapGp1f1gfamHMP6mdrGJ2RTMZiOx6RDtfBjmH4hxH ag3fdSEQnUOmCOx0VODYCXFaDjAMmyErBbF9AM5Ap/vdlC8mNCLIGJW/q6Fczq1psO4g soqQ== X-Gm-Message-State: APjAAAV+QZa9i1ri1xO4J0KB8PftwNh6CmAiql+LMVNWxlWX1AWx1FIZ WMmvHeqUxX6lsBHGcM2jG2u7DiEWzntrYwcJTs62Xh1a X-Google-Smtp-Source: APXvYqzsS4n5TWHSBz9UbE+s/yqc825FR6a1YbEDpO2HfsnbYtZhJWvn4rSlXWcKxfwLQzAlrYpmCv5PNjFwxtMve2U= X-Received: by 2002:a0c:e711:: with SMTP id d17mr2878148qvn.174.1567001739914; Wed, 28 Aug 2019 07:15:39 -0700 (PDT) MIME-Version: 1.0 References: <20190819205720.24457-1-mike.leach@linaro.org> <20190819205720.24457-8-mike.leach@linaro.org> <20190828051847.GG26133@leoy-ThinkPad-X240s> In-Reply-To: <20190828051847.GG26133@leoy-ThinkPad-X240s> From: Mike Leach Date: Wed, 28 Aug 2019 15:15:28 +0100 Message-ID: Subject: Re: [PATCH 7/8] coresight: etm4x: Add missing single-shot control API to sysfs To: Leo Yan X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190828_071541_420559_4A4F5A11 X-CRM114-Status: GOOD ( 25.88 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Coresight ML , linux-arm-kernel , Mathieu Poirier Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org HI Leo, On Wed, 28 Aug 2019 at 06:18, Leo Yan wrote: > > On Mon, Aug 19, 2019 at 09:57:19PM +0100, Mike Leach wrote: > > An API to control single-shot comparator operation was missing from sysfs. > > This adds the parameters to sysfs to allow programming of this feature. > > > > Signed-off-by: Mike Leach > > --- > > .../coresight/coresight-etm4x-sysfs.c | 122 ++++++++++++++++++ > > drivers/hwtracing/coresight/coresight-etm4x.c | 26 +++- > > drivers/hwtracing/coresight/coresight-etm4x.h | 3 + > > 3 files changed, 150 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > > index 483976074779..7c019dda1236 100644 > > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > > @@ -239,6 +239,7 @@ static ssize_t reset_store(struct device *dev, > > for (i = 0; i < drvdata->nr_resource; i++) > > config->res_ctrl[i] = 0x0; > > > > + config->ss_idx = 0x0; > > for (i = 0; i < drvdata->nr_ss_cmp; i++) { > > config->ss_ctrl[i] = 0x0; > > config->ss_pe_cmp[i] = 0x0; > > @@ -1713,6 +1714,123 @@ static ssize_t res_ctrl_store(struct device *dev, > > } > > static DEVICE_ATTR_RW(res_ctrl); > > > > +static ssize_t sshot_idx_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + unsigned long val; > > + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > > + struct etmv4_config *config = &drvdata->config; > > + > > + val = config->ss_idx; > > + return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > > +} > > + > > +static ssize_t sshot_idx_store(struct device *dev, > > + struct device_attribute *attr, > > + const char *buf, size_t size) > > +{ > > + unsigned long val; > > + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > > + struct etmv4_config *config = &drvdata->config; > > + > > + if (kstrtoul(buf, 16, &val)) > > + return -EINVAL; > > + if (val >= drvdata->nr_ss_cmp) > > + return -EINVAL; > > + > > + spin_lock(&drvdata->spinlock); > > + config->ss_idx = val; > > + spin_unlock(&drvdata->spinlock); > > + return size; > > +} > > +static DEVICE_ATTR_RW(sshot_idx); > > + > > +static ssize_t sshot_ctrl_show(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + unsigned long val; > > + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > > + struct etmv4_config *config = &drvdata->config; > > + > > + spin_lock(&drvdata->spinlock); > > + val = config->ss_ctrl[config->ss_idx]; > > + spin_unlock(&drvdata->spinlock); > > + return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > > +} > > + > > +static ssize_t sshot_ctrl_store(struct device *dev, > > + struct device_attribute *attr, > > + const char *buf, size_t size) > > +{ > > + u8 idx; > > + unsigned long val; > > + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > > + struct etmv4_config *config = &drvdata->config; > > + > > + if (kstrtoul(buf, 16, &val)) > > + return -EINVAL; > > + > > + spin_lock(&drvdata->spinlock); > > + idx = config->ss_idx; > > + config->ss_ctrl[idx] = val & GENMASK(24, 0); > > + /* must clear bit 31 in related status register on programming */ > > + config->ss_status[idx] &= ~BIT(31); > > Since function etm4_enable_hw() will clear ss_status's bit 31 when > program TRCSSCSRn, so is here redundant to clear bit 31? > > > + spin_unlock(&drvdata->spinlock); > > + return size; > > +} > > +static DEVICE_ATTR_RW(sshot_ctrl); > > + > > +static ssize_t sshot_status_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + unsigned long val; > > + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > > + struct etmv4_config *config = &drvdata->config; > > + > > + spin_lock(&drvdata->spinlock); > > + val = config->ss_status[config->ss_idx]; > > + spin_unlock(&drvdata->spinlock); > > + return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > > +} > > +static DEVICE_ATTR_RO(sshot_status); > > + > > +static ssize_t sshot_pe_ctrl_show(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + unsigned long val; > > + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > > + struct etmv4_config *config = &drvdata->config; > > + > > + spin_lock(&drvdata->spinlock); > > + val = config->ss_pe_cmp[config->ss_idx]; > > + spin_unlock(&drvdata->spinlock); > > + return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > > +} > > + > > +static ssize_t sshot_pe_ctrl_store(struct device *dev, > > + struct device_attribute *attr, > > + const char *buf, size_t size) > > +{ > > + u8 idx; > > + unsigned long val; > > + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > > + struct etmv4_config *config = &drvdata->config; > > + > > + if (kstrtoul(buf, 16, &val)) > > + return -EINVAL; > > + > > + spin_lock(&drvdata->spinlock); > > + idx = config->ss_idx; > > + config->ss_ctrl[idx] = val & GENMASK(7, 0); > > + /* must clear bit 31 in related status register on programming */ > > + config->ss_status[idx] &= ~BIT(31); > > Same question for if it's redundant to clear bit 31? > > Thanks, > Leo Yan > The sysfs representation is not the current state of these registers, but the values that will be programmed on enabling the hardware. We do not write anything to hardware immediately. So from a sysfs perspective, the bit clear is reflected in the status register here to recognise that it will be programmed as cleared. Should the same registers be set from a different path - e.g. perf, then the status will automatically be cleared each time the hw is enabled. Regards Mike > > + spin_unlock(&drvdata->spinlock); > > + return size; > > +} > > +static DEVICE_ATTR_RW(sshot_pe_ctrl); > > + > > static ssize_t ctxid_idx_show(struct device *dev, > > struct device_attribute *attr, > > char *buf) > > @@ -2169,6 +2287,10 @@ static struct attribute *coresight_etmv4_attrs[] = { > > &dev_attr_addr_exlevel_s_ns.attr, > > &dev_attr_addr_cmp_view.attr, > > &dev_attr_vinst_pe_cmp_start_stop.attr, > > + &dev_attr_sshot_idx.attr, > > + &dev_attr_sshot_ctrl.attr, > > + &dev_attr_sshot_pe_ctrl.attr, > > + &dev_attr_sshot_status.attr, > > &dev_attr_seq_idx.attr, > > &dev_attr_seq_state.attr, > > &dev_attr_seq_event.attr, > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c > > index d8b078d0cc7f..fb7083218410 100644 > > --- a/drivers/hwtracing/coresight/coresight-etm4x.c > > +++ b/drivers/hwtracing/coresight/coresight-etm4x.c > > @@ -149,6 +149,9 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > > drvdata->base + TRCRSCTLRn(i)); > > > > for (i = 0; i < drvdata->nr_ss_cmp; i++) { > > + /* always clear status bit on restart if using single-shot */ > > + if (config->ss_ctrl[i] || config->ss_pe_cmp[i]) > > + config->ss_status[i] &= ~BIT(31); > > writel_relaxed(config->ss_ctrl[i], > > drvdata->base + TRCSSCCRn(i)); > > writel_relaxed(config->ss_status[i], > > @@ -448,6 +451,9 @@ static void etm4_disable_hw(void *info) > > { > > u32 control; > > struct etmv4_drvdata *drvdata = info; > > + struct etmv4_config *config = &drvdata->config; > > + struct device *etm_dev = &drvdata->csdev->dev; > > + int i; > > > > CS_UNLOCK(drvdata->base); > > > > @@ -470,6 +476,18 @@ static void etm4_disable_hw(void *info) > > isb(); > > writel_relaxed(control, drvdata->base + TRCPRGCTLR); > > > > + /* wait for TRCSTATR.PMSTABLE to go to '1' */ > > + if (coresight_timeout(drvdata->base, TRCSTATR, > > + TRCSTATR_PMSTABLE_BIT, 1)) > > + dev_err(etm_dev, > > + "timeout while waiting for PM stable Trace Status\n"); > > + > > + /* read the status of the single shot comparators */ > > + for (i = 0; i < drvdata->nr_ss_cmp; i++) { > > + config->ss_status[i] = > > + readl_relaxed(drvdata->base + TRCSSCSRn(i)); > > + } > > + > > coresight_disclaim_device_unlocked(drvdata->base); > > > > CS_LOCK(drvdata->base); > > @@ -576,6 +594,7 @@ static void etm4_init_arch_data(void *info) > > u32 etmidr4; > > u32 etmidr5; > > struct etmv4_drvdata *drvdata = info; > > + int i; > > > > /* Make sure all registers are accessible */ > > etm4_os_unlock(drvdata); > > @@ -699,9 +718,14 @@ static void etm4_init_arch_data(void *info) > > drvdata->nr_resource = BMVAL(etmidr4, 16, 19) + 1; > > /* > > * NUMSSCC, bits[23:20] the number of single-shot > > - * comparator control for tracing > > + * comparator control for tracing. Read any status regs as these > > + * also contain RO capability data. > > */ > > drvdata->nr_ss_cmp = BMVAL(etmidr4, 20, 23); > > + for (i = 0; i < drvdata->nr_ss_cmp; i++) { > > + drvdata->config.ss_status[i] = > > + readl_relaxed(drvdata->base + TRCSSCSRn(i)); > > + } > > /* NUMCIDC, bits[27:24] number of Context ID comparators for tracing */ > > drvdata->numcidc = BMVAL(etmidr4, 24, 27); > > /* NUMVMIDC, bits[31:28] number of VMID comparators for tracing */ > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h > > index 60bc2fb5159b..be8b32ea1654 100644 > > --- a/drivers/hwtracing/coresight/coresight-etm4x.h > > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h > > @@ -175,6 +175,7 @@ > > ETM_MODE_EXCL_USER) > > > > #define TRCSTATR_IDLE_BIT 0 > > +#define TRCSTATR_PMSTABLE_BIT 1 > > #define ETM_DEFAULT_ADDR_COMP 0 > > > > /* PowerDown Control Register bits */ > > @@ -226,6 +227,7 @@ > > * @cntr_val: Sets or returns the value for a counter. > > * @res_idx: Resource index selector. > > * @res_ctrl: Controls the selection of the resources in the trace unit. > > + * @ss_idx: Single-shot index selector. > > * @ss_ctrl: Controls the corresponding single-shot comparator resource. > > * @ss_status: The status of the corresponding single-shot comparator. > > * @ss_pe_cmp: Selects the PE comparator inputs for Single-shot control. > > @@ -269,6 +271,7 @@ struct etmv4_config { > > u32 cntr_val[ETMv4_MAX_CNTR]; > > u8 res_idx; > > u32 res_ctrl[ETM_MAX_RES_SEL]; > > + u8 ss_idx; > > u32 ss_ctrl[ETM_MAX_SS_CMP]; > > u32 ss_status[ETM_MAX_SS_CMP]; > > u32 ss_pe_cmp[ETM_MAX_SS_CMP]; > > -- > > 2.17.1 > > > > _______________________________________________ > > CoreSight mailing list > > CoreSight@lists.linaro.org > > https://lists.linaro.org/mailman/listinfo/coresight -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel