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=-7.0 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 44C9BC433E0 for ; Mon, 29 Jun 2020 17:51:06 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 F2F8E255A5 for ; Mon, 29 Jun 2020 17:51:05 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="xUhPgI5l"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="D9GnTVA7" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F2F8E255A5 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+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=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=aWr7NaLojvI9w18ZrCya6Qu3J3jPpdXRb32I2zT9TnY=; b=xUhPgI5lDIw5kFz1sKlF8DSO0 B9ssb56ZJkS+WVoU7Wyjbt9C1U8oXQ1hl7P3kadkWF7dg+ptFRAMrjUeL6sTyv3f1KOTIVORZVuE6 kjq7J1D5O9lhubNHY7IThh0VSY8SquaN5hFHCodhdovanKkOgZi8aECS6NZgpwmGcQlX/z1eLa21U TDvRBrIGVyEeW5KLI+nmvTjA5JM7dNOxgBVGYiEc8JnVipFOuAQJWKfMif1kAB1QxAnwvYs/5rJcs t/XFmux2/8vxwcJapQlXCu6JiAblecQGP+ZLVpeeok5lC7UuSbV6u1atXFyxNFlpwPcuzB9RgW9GY 6gO8Y1XoA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jpxt0-0005O4-GI; Mon, 29 Jun 2020 17:48:06 +0000 Received: from mail-pl1-x641.google.com ([2607:f8b0:4864:20::641]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jpxsx-0005Mj-7K for linux-arm-kernel@lists.infradead.org; Mon, 29 Jun 2020 17:48:04 +0000 Received: by mail-pl1-x641.google.com with SMTP id s14so7374136plq.6 for ; Mon, 29 Jun 2020 10:48:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=XVsa/7jgqD/tCtEhtbq5P7SP0GjekJLQVl/koafNOZw=; b=D9GnTVA73GJ4bgKmDXAJab9PiX8TsI0txYixua7NV7rIPGUdAKmunxsnZC1OVhpwlX HzlzFtt+ZRmuPAjocI/Qo1OO33EyKKKA4RULPNh59glsc4j/l4ZrFkLTLG7+CxX9QHTX uQ749vuWWC+WbfATzjfrG1bb546BmgLIn3nYo/ex9OBbNzl5MJGT2CmD7CxGAlnMgyVd zjRJThGeijBfVyHYtXXptLsWunsRhtBCVmyNUt4PK1j/FMwihAVIKjPlBwKBKiu3k3tt YSnPis85muWHXYU1rvVYigFYJarMyXld1J+TqWRDzRu7pIkfMNF/hGBNLhG25HwnNVHY uYrQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=XVsa/7jgqD/tCtEhtbq5P7SP0GjekJLQVl/koafNOZw=; b=jvVJoXAaQKll4LmU8e5WbMAyJFvk6lq5wsRmNohyYumtFs5ujGDkkOp8hRcmihzj+0 dKnvwnA33fKFqIbgvNbB/JYQVExQEfbwUlLsrkG92rxrBNYGo684NKfhsr0aj9GmVprA PeFe0QIqpCyeMGZYjYorv4j6oEimGjFapcx7v13ND6JY7a89tr0WmsvWvETF758zSawB iugPnVz+YbU4fii6OEvEMYmncjWbe+UPXkY41qIjsJ+8QAj8NcM9mIiPdyximQwyJM4Z Nq8h8/9tlyyEOQTq4KFW2CBPmI7whZsfEJl7ZfgZg5Ezb7SS0k7PrcMHDUXpyc3OtSo1 g91Q== X-Gm-Message-State: AOAM532zzJJczONZ6uhXjIlvZGMy+aqahD0t2ZX1FH2CCvofd2PRkglP xLPDfAtnJdfuRGIqUC9JWG27Qw== X-Google-Smtp-Source: ABdhPJyYyhPHA3IrT2UbgCAhV0JzBoclpoISPsvyegTI3pLSc4N5Ot7mP8nK3t8GfGAEALvfEBwZ+Q== X-Received: by 2002:a17:90a:ea18:: with SMTP id w24mr17579805pjy.42.1593452878964; Mon, 29 Jun 2020 10:47:58 -0700 (PDT) Received: from xps15 (S0106002369de4dac.cg.shawcable.net. [68.147.8.254]) by smtp.gmail.com with ESMTPSA id h17sm299503pfo.168.2020.06.29.10.47.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 29 Jun 2020 10:47:58 -0700 (PDT) Date: Mon, 29 Jun 2020 11:47:56 -0600 From: Mathieu Poirier To: Mike Leach Subject: Re: [PATCH v5 4/5] coresight: sysfs: Allow select default sink on source enable. Message-ID: <20200629174756.GA3724199@xps15> References: <20200616164006.15309-1-mike.leach@linaro.org> <20200616164006.15309-5-mike.leach@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200616164006.15309-5-mike.leach@linaro.org> 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: corbet@lwn.net, coresight@lists.linaro.org, suzuki.poulose@arm.com, linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Mike, I have applied patches 1 to 3 of this set. Please see below for comments on this patch. On Tue, Jun 16, 2020 at 05:40:05PM +0100, Mike Leach wrote: > When enabling a trace source using sysfs, allow the CoreSight system to > auto-select a default sink if none has been enabled by the user. > > Uses the sink select algorithm that uses the default select priorities > set when sinks are registered with the system. At present this will > prefer ETR over ETB / ETF. > > Adds a new attribute 'last_sink' to source CoreSight devices. This is set > when a source is enabled using sysfs, to the sink that the device will > trace into. This applies for both user enabled and default enabled sinks. > > Signed-off-by: Mike Leach > --- > drivers/hwtracing/coresight/coresight.c | 39 +++++++++++++++++++++++-- > include/linux/coresight.h | 3 ++ > 2 files changed, 40 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c > index e9c90f2de34a..db39e0b56994 100644 > --- a/drivers/hwtracing/coresight/coresight.c > +++ b/drivers/hwtracing/coresight/coresight.c > @@ -934,6 +934,16 @@ static void coresight_clear_default_sink(struct coresight_device *csdev) > } > } > > +static void coresight_set_last_sink_name(struct coresight_device *source, > + struct coresight_device *sink) > +{ > + /* remove current value and set new one if *sink not NULL */ > + kfree(source->last_sink); > + source->last_sink = NULL; > + if (sink) > + source->last_sink = kstrdup(dev_name(&sink->dev), GFP_KERNEL); > +} > + > /** coresight_validate_source - make sure a source has the right credentials > * @csdev: the device structure for a source. > * @function: the function this was called from. > @@ -994,8 +1004,15 @@ int coresight_enable(struct coresight_device *csdev) > */ > sink = coresight_get_enabled_sink(false); > if (!sink) { > - ret = -EINVAL; > - goto out; > + /* look for a default sink if nothing enabled */ > + sink = coresight_find_default_sink(csdev); > + if (!sink) { > + ret = -EINVAL; > + goto out; > + } > + /* mark the default as enabled */ > + sink->activated = true; > + dev_info(&sink->dev, "Enabled default sink."); I'm very ambivalent about extending the automatic sink selection to the sysfs interface, mainly because of the new sysfs entry it requires. I find it clunky that users don't have to specify the sink to use but have to explicitly disable it after the trace session. We could automatically disable the sink after a trace session but that would break with the current sysfs heuristic where sinks have to be explicitly enabled and disabled. Thanks, Mathieu > } > > path = coresight_build_path(csdev, sink); > @@ -1033,6 +1050,9 @@ int coresight_enable(struct coresight_device *csdev) > break; > } > > + /* record name of sink used for this session */ > + coresight_set_last_sink_name(csdev, sink); > + > out: > mutex_unlock(&coresight_mutex); > return ret; > @@ -1145,6 +1165,19 @@ static ssize_t enable_source_store(struct device *dev, > } > static DEVICE_ATTR_RW(enable_source); > > +static ssize_t last_sink_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct coresight_device *csdev = to_coresight_device(dev); > + ssize_t size = 0; > + > + if (csdev->last_sink) > + size = scnprintf(buf, PAGE_SIZE, "%s\n", csdev->last_sink); > + return size; > +} > +static DEVICE_ATTR_RO(last_sink); > + > + > static struct attribute *coresight_sink_attrs[] = { > &dev_attr_enable_sink.attr, > NULL, > @@ -1153,6 +1186,7 @@ ATTRIBUTE_GROUPS(coresight_sink); > > static struct attribute *coresight_source_attrs[] = { > &dev_attr_enable_source.attr, > + &dev_attr_last_sink.attr, > NULL, > }; > ATTRIBUTE_GROUPS(coresight_source); > @@ -1524,6 +1558,7 @@ void coresight_unregister(struct coresight_device *csdev) > /* Remove references of that device in the topology */ > coresight_remove_conns(csdev); > coresight_clear_default_sink(csdev); > + coresight_set_last_sink_name(csdev, NULL); > coresight_release_platform_data(csdev, csdev->pdata); > device_unregister(&csdev->dev); > } > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index 58fffdecdbfd..fc320dd2cedc 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -184,6 +184,8 @@ struct coresight_sysfs_link { > * from source to that sink. > * @ea: Device attribute for sink representation under PMU directory. > * @def_sink: cached reference to default sink found for this device. > + * @last_sink: Name of last sink used for this source to trace into. Set when > + * enabling a source using sysfs - only set on a source device. > * @ect_dev: Associated cross trigger device. Not part of the trace data > * path or connections. > * @nr_links: number of sysfs links created to other components from this > @@ -203,6 +205,7 @@ struct coresight_device { > bool activated; /* true only if a sink is part of a path */ > struct dev_ext_attribute *ea; > struct coresight_device *def_sink; > + const char *last_sink; > /* cross trigger handling */ > struct coresight_device *ect_dev; > /* sysfs links between components */ > -- > 2.17.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel