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=-11.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 0705BC433DF for ; Wed, 19 Aug 2020 01:57:42 +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 C2D38204EC for ; Wed, 19 Aug 2020 01:57:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="VZE+CuwY"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=mg.codeaurora.org header.i=@mg.codeaurora.org header.b="u9fG5EZE" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C2D38204EC Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=codeaurora.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=KAhiSbgPRDbCicGR57iuTOhHFyNST5+wfbE82Y7QMDM=; b=VZE+CuwYEi73Q5/USUjVfsq90 pQTX4t15w9LC1tzIbP4ghHQSbDEwSU6kot2nYcQ5jb5GS5qcNRD1jgwloai50XPfxbDi/uBsu+MVn yEPdcmpAJuXnVIjIe41U93Q4ynvUhhzSL+y7ttGxvI5bUx++BZrO57nAW+uY/zptN+yPyNqEnpvto 6hUJb8UrDfAoGZJBJ5+8qemUx/zNLFf3XFcIwx4E4VdBIr3wT6lTPa7NTX3Vel38Q6nFSbONClNdm e3qZxUBBoP+TkPYmAxQ5AXIGuQikToaL5Qx0riIBzRE7PlaKNSUMyX2XLkFuauzvnblwoC0j0InCU eGDwLY2ng==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8DKd-0002PQ-Bl; Wed, 19 Aug 2020 01:56:03 +0000 Received: from mail29.static.mailgun.info ([104.130.122.29]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8DKX-0002Os-Dp for linux-arm-kernel@lists.infradead.org; Wed, 19 Aug 2020 01:56:01 +0000 DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=mg.codeaurora.org; q=dns/txt; s=smtp; t=1597802159; h=In-Reply-To: Content-Type: MIME-Version: References: Message-ID: Subject: Cc: To: From: Date: Sender; bh=7l9vVQjtRBiLRCMVX0vZ4W4MR6LBKwxYEaQvizHJMOY=; b=u9fG5EZEKporyO180ekjDOg0WsBfgfKede6xWR7IxsP2k0gKJwtT8px5kcDqSpukEn8Tledf 2CClaCsxJhr6ArLpZTHuJ9LBerzHhE5223YKAjvSrfdb7IjX2wYPpPyYa/9NYEo3t3PkneG5 b4b2erMYfvorZsAt7IuAJNc7A3c= X-Mailgun-Sending-Ip: 104.130.122.29 X-Mailgun-Sid: WyJiYzAxZiIsICJsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmciLCAiYmU5ZTRhIl0= Received: from smtp.codeaurora.org (ec2-35-166-182-171.us-west-2.compute.amazonaws.com [35.166.182.171]) by smtp-out-n03.prod.us-east-1.postgun.com with SMTP id 5f3c86a3856720175145c732 (version=TLS1.2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256); Wed, 19 Aug 2020 01:55:47 GMT Received: by smtp.codeaurora.org (Postfix, from userid 1001) id 476DAC433A0; Wed, 19 Aug 2020 01:55:46 +0000 (UTC) Received: from codeaurora.org (unknown [180.166.53.21]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: tingwei) by smtp.codeaurora.org (Postfix) with ESMTPSA id B6B5AC433CA; Wed, 19 Aug 2020 01:55:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org B6B5AC433CA Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; spf=none smtp.mailfrom=tingweiz@codeaurora.org Date: Wed, 19 Aug 2020 09:55:33 +0800 From: Tingwei Zhang To: Mathieu Poirier Subject: Re: [PATCH v8 19/24] coresight: cti: don't disable ect device if it's not enabled Message-ID: <20200819015533.GA19500@codeaurora.org> References: <20200807111153.7784-1-tingwei@codeaurora.org> <20200807111153.7784-20-tingwei@codeaurora.org> <20200817163843.GD3614061@xps15> <20200818173909.GB3801581@xps15> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200818173909.GB3801581@xps15> User-Agent: Mutt/1.5.24 (2015-08-30) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200818_215600_178634_A3A3BE24 X-CRM114-Status: GOOD ( 50.68 ) 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: tsoni@codeaurora.org, Sai Prakash Ranjan , Kim Phillips , Suzuki K Poulose , Alexander Shishkin , Greg Kroah-Hartman , Coresight ML , Mao Jinlong , Mian Yousaf Kaukab , Russell King , Randy Dunlap , Tingwei Zhang , Leo Yan , linux-arm-kernel , Mike Leach 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 On Wed, Aug 19, 2020 at 01:39:09AM +0800, Mathieu Poirier wrote: > On Mon, Aug 17, 2020 at 08:04:06PM +0100, Mike Leach wrote: > > Hi Mathieu, > > > > On Mon, 17 Aug 2020 at 17:38, Mathieu Poirier > > wrote: > > > > > > On Fri, Aug 07, 2020 at 07:11:48PM +0800, Tingwei Zhang wrote: > > > > If associated ect device is not enabled at first place, disable > > > > routine should not be called. Add ect_enabled flag to check whether > > > > ect device is enabled. Fix the issue in below case. Ect device is > > > > not available when associated coresight device enabled and the > > > > association is established after coresight device is enabled. > > > > > > > > Signed-off-by: Mike Leach > > > > Signed-off-by: Tingwei Zhang > > > > --- > > > > drivers/hwtracing/coresight/coresight.c | 11 ++++++++--- > > > > include/linux/coresight.h | 1 + > > > > 2 files changed, 9 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/hwtracing/coresight/coresight.c > b/drivers/hwtracing/coresight/coresight.c > > > > index d066411aa794..27ad8317e3cf 100644 > > > > --- a/drivers/hwtracing/coresight/coresight.c > > > > +++ b/drivers/hwtracing/coresight/coresight.c > > > > @@ -245,13 +245,18 @@ coresight_control_assoc_ectdev(struct > coresight_device *csdev, bool enable) > > > > > > > > if (!ect_csdev) > > > > return 0; > > > > + if ((!ect_ops(ect_csdev)->enable) || > (!ect_ops(ect_csdev)->disable)) > > > > + return 0; > > > > > > > > if (enable) { > > > > - if (ect_ops(ect_csdev)->enable) > > > > - ect_ret = > ect_ops(ect_csdev)->enable(ect_csdev); > > > > + ect_ret = ect_ops(ect_csdev)->enable(ect_csdev); > > > > + if (!ect_ret) > > > > + csdev->ect_enabled = true; > > > > } else { > > > > - if (ect_ops(ect_csdev)->disable) > > > > + if (csdev->ect_enabled) { > > > > ect_ret = > ect_ops(ect_csdev)->disable(ect_csdev); > > > > + csdev->ect_enabled = false; > > > > + } > > > > } > > > > > > > > /* output warning if ECT enable is preventing trace operation > */ > > > > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > > > > index 3bb738f9a326..7d3c87e5b97c 100644 > > > > --- a/include/linux/coresight.h > > > > +++ b/include/linux/coresight.h > > > > @@ -208,6 +208,7 @@ struct coresight_device { > > > > /* sysfs links between components */ > > > > int nr_links; > > > > bool has_conns_grp; > > > > + bool ect_enabled; /* true only if associated ect device is > enabled */ > > > > > > We have cti_config::enable_req_count and cti_config::hw_enabled, both > used in > > > cti_enable_hw() and cti_disable_hw(). I would have thought they'd be > sufficient > > > to address the counting problems. If they are not I would much rather > see a > > > solution confined to the cti driver than in the core itself. > > > > > > > This is related to the fact that under sysfs it is possible under > > sysfs to enable an etm e.g. etm1 without the cti module present, then > > insert the CTI module, then enable another ETM e.g etm2. > > This is an issue that is caused by the possibility of module load and > > unload, and though inadvisable from a system usage point of view - > > Tingwei correctly points out that it could happen. > > > > At the point that the first ETM is enabled, the associated ect pointer > > would be NULL, and thus no attempt to enable the ect/CTI is made. The > > CTI module on load will set the ect pointers on all registered csdev > > devices, including ones that are already enabled - etm1. > > So when we come to disable etm1, it will try to disable a CTI that it > > did not enable - a fact that cannot be counted in the CTI driver as it > > was not there when the etm was enabled. So we have a flag in csdev to > > record if this csdev did in fact enable the associated device, so it > > is clear to disable it on shutdown. > > In the above example the csdev structures associated with ETM1 and ETM2 > will > have different csdev->ect_dev pointers, one for each CTI it is assocated > with. > And I would think the reference count for CTI1 would not have been > incremented > since it was never enabled. > > What am I missing? > The issue here is we could have two or more coresght devices associated to single CTI device, especially for sys cti case. For example, we have coresight devcie A and B. They are assocaited to CTI device C. CTI module is not loaded at first. Device A is enabled. CTI C is not enabled. Then CTI module is loaded. Device B is enabled. CTI C is enabled with enable_req_count = 1. Device A is disabled. It decrease enable_req_count to 0, so CTI C is disabled. It's kinda weird to me that CTI C is enabled with device B and disabled by device A. The point is we need some flag in coresight device structure to save the status of associated CTI enablement when CTI module is not loaded yet. Thanks, Tingwei > > > > Regards > > > > Mike > > > > > > > > > Thanks, > > > Mathieu > > > > > > > }; > > > > > > > > /* > > > > -- > > > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > Forum, > > > > a Linux Foundation Collaborative Project > > > > > > > > > > > > -- > > 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel