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=-9.8 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,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 3B188C433E1 for ; Tue, 18 Aug 2020 17:17:38 +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 E8A6820658 for ; Tue, 18 Aug 2020 17:17:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="tgJ+9w+P"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="cgjs3xoS" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E8A6820658 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=DljuEspbAz1E5AC+d9jFHaX8MeAGU8fyTADjty7r6Lk=; b=tgJ+9w+Pa33GeP+9XPCZIWfxC m9vbfC8EFBk021V5Yszsl8B7ywNfT1EzTAqOywDWks4NGEfGPn2kzWuaXcZJqxJHtRMzPl95af7gN EzKvwmfwEU4juo2/uZXwtN6/EsCSTdTP8KIsxQHuLv+EKdbeemjHKQnSgObjeOJPiuU85MaGLAoHO yjrQ+LdFgG19s20G4hf+iUXQnN9oTMwCkLEIdzwLM7nKLedXCtvCamMCPhL7REsmEqe53KWkIOr5e sxC24lia9iiOlSjFRgGT6nWchO3e5yyMZjZFWzeLElTq9/Yon/ncf91TQMnirhHSk3sA6UCsESqFS 5ehLyDXMQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k85DX-0006fk-05; Tue, 18 Aug 2020 17:16:11 +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 1k85DT-0006eY-Nv for linux-arm-kernel@lists.infradead.org; Tue, 18 Aug 2020 17:16:09 +0000 Received: by mail-pl1-x641.google.com with SMTP id g7so8383915plq.1 for ; Tue, 18 Aug 2020 10:16:06 -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=BJc/uDTJB5ct4fKW1/VRpqlj+W/EwHhWGgiSrxtSQbg=; b=cgjs3xoSOJrFNslWsg4Nkn7XzlMpeoUQ0D86SRW8kS3K2oYXdAucWHqRo+MIVG+hPU XK/DORzj408PGHcsGofZUz59gVYOvMTTsZkkl8/MqNRgCWbIUF/lcGwCkfUrEECuErt5 3McHB+DdmXi6vdgxs14X3PK/+Mb1iKJ/AZPivQiKglQ3yuZhpLmWEu6JrvYvPyb/A6mJ 6hNwDGHALQ/nkui3jsNct/IsRZnaSbxDb1Id1pzcBpyHFdm2JdMbZ0Qp+htdZEoHFHQG GZQs2wzNvJmPRZMXfrzTA8GU/rIVPknBeqB6crkhhXGpOSPAqCetDFKzkdWlkcZItiIJ 79YQ== 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=BJc/uDTJB5ct4fKW1/VRpqlj+W/EwHhWGgiSrxtSQbg=; b=XELiPGW9uufFhuMR2T0dGdODs5s4xWZ6csbpnGux0beiO0LFrkqHWsIPVeSIj8yVwg IVs+j7WU+Gl9/UNjakMApE/ltXgaxISLYjI0lMKxH+WY3IfCPkZYML4WovpBgyLT62v4 CMzc+FQPikNfetD1jgR9EH4hiDIVUH8VUQd0Axkg2KmdjiMUDNs2zLyT9UCCZNES43Uf VjQxZanQ9soT94tCJMG3p1YkDMLAMDa9SRIp3MIIa9VwSheaXA1IXsZUl+Q6WIltIk/e rwHl8g7AL7cXO04uSxeDLL28g4nZfL/eHH/YJ0v2j+XuLXARDaUpCj00++uVB4m31KQr qr1w== X-Gm-Message-State: AOAM533CuCXICbmkDYwJVRdNfgjvxFGGM7ZciyHULEuMZzgR5qN8vjuP +iLTfKd4wvPL7W45sFS4uSMcGg== X-Google-Smtp-Source: ABdhPJxuJ8t4JL9hsHK08j6bJwpedhjRUNrwfneG8o96I+Cy3aklcfKkqWQzkE21TQt+AQkGk7ngWg== X-Received: by 2002:a17:90b:358c:: with SMTP id mm12mr785156pjb.154.1597770964107; Tue, 18 Aug 2020 10:16:04 -0700 (PDT) Received: from xps15 (S0106002369de4dac.cg.shawcable.net. [68.147.8.254]) by smtp.gmail.com with ESMTPSA id i185sm22981056pgd.28.2020.08.18.10.16.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Aug 2020 10:16:03 -0700 (PDT) Date: Tue, 18 Aug 2020 11:16:01 -0600 From: Mathieu Poirier To: Mike Leach Subject: Re: [PATCH v8 16/24] coresight: cti: add function to register cti associate ops Message-ID: <20200818171601.GA3801581@xps15> References: <20200807111153.7784-1-tingwei@codeaurora.org> <20200807111153.7784-17-tingwei@codeaurora.org> <20200817160235.GA3614061@xps15> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200818_131608_147521_ECBF6B3C X-CRM114-Status: GOOD ( 37.71 ) 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 , Mao Jinlong , Suzuki K Poulose , Alexander Shishkin , Greg Kroah-Hartman , Coresight ML , Randy Dunlap , Mian Yousaf Kaukab , Russell King , Tingwei Zhang , Leo Yan , linux-arm-kernel 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 Good day, On Mon, Aug 17, 2020 at 07:49:16PM +0100, Mike Leach wrote: > Hi Mathieu, > > On Mon, 17 Aug 2020 at 17:02, Mathieu Poirier > wrote: > > > > On Fri, Aug 07, 2020 at 07:11:45PM +0800, Tingwei Zhang wrote: > > > Add static cti_assoc_ops to coresight core driver. Let cti > > > driver register the add_assoc and remove_assoc call back. > > > Avoid coresight core driver to depend on cti driver. > > > > > > Signed-off-by: Tingwei Zhang > > > Tested-by: Mike Leach > > > --- > > > drivers/hwtracing/coresight/coresight-cti.c | 32 ++++++++++++++++++-- > > > drivers/hwtracing/coresight/coresight-priv.h | 14 ++++----- > > > drivers/hwtracing/coresight/coresight.c | 21 +++++++++++-- > > > 3 files changed, 53 insertions(+), 14 deletions(-) > > > > > > diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c > > > index 3ccc703dc940..1f470c47ba70 100644 > > > --- a/drivers/hwtracing/coresight/coresight-cti.c > > > +++ b/drivers/hwtracing/coresight/coresight-cti.c > > > @@ -589,7 +589,6 @@ void cti_add_assoc_to_csdev(struct coresight_device *csdev) > > > cti_add_done: > > > mutex_unlock(&ect_mutex); > > > } > > > -EXPORT_SYMBOL_GPL(cti_add_assoc_to_csdev); > > > > > > /* > > > * Removing the associated devices is easier. > > > @@ -616,7 +615,15 @@ void cti_remove_assoc_from_csdev(struct coresight_device *csdev) > > > } > > > mutex_unlock(&ect_mutex); > > > } > > > -EXPORT_SYMBOL_GPL(cti_remove_assoc_from_csdev); > > > > Mike and Tingwei, > > > > It seems to me that functions cti_add_assoc_to_csdev() and > > cti_remote_assoc_from_csdev() don't have to be exported. They could be called > > in cti_probe() and cti_device_release() with the ect_mutex held, the same way > > cti_update_conn_xrefs() and cti_remove_conn_xrefs() are. > > > > The different functions handle two different initialisation & two > different teardown ordering scenarios > a) csdev - e.g. ETM is discovered & initialised before the associated > CTI. Therefore the CTI has to establish interconnections on creation - > calls cti_update_conn_xrefs() . > b) CTI is discovered before csdev - therefore the csdev calls > cti_add_assoc_to_csdev(). > c) csdev is torn down before CTI - therefore csdev calls > cti_remove_assoc_from_csdev() > d) CTI is torn down before csdev - therefore calls cti_remove_conn_xrefs() > > We can't call cti_add_assoc_to_csdev() in cti probe as the csdev does > not exist yet. Bear in mind that one CTI can actually be associated > with multiple different csdev devices - especially in the none CPU > bound infrastructure such as funnels and sinks. Yes, I see your point now. The problem needs to be viewed from other devices' (than CTI) perspective. And that part is done in coresight_register/unregister(). Reviewed-by: Mathieu Poirier > This has always been the case since the first CTI set - but is even > more important now the modules can be removed / added as required. > > Regards > > Mike > > > > If my assessment is correct this patch would be simpler. > > > > Thanks, > > Mathieu > > > > > > > + > > > +/* > > > + * Operations to add and remove associated CTI. > > > + * Register to coresight core driver as call back function. > > > + */ > > > +static struct cti_assoc_op cti_assoc_ops = { > > > + .add = cti_add_assoc_to_csdev, > > > + .remove = cti_remove_assoc_from_csdev > > > +}; > > > > > > /* > > > * Update the cross references where the associated device was found > > > @@ -972,4 +979,23 @@ static struct amba_driver cti_driver = { > > > .probe = cti_probe, > > > .id_table = cti_ids, > > > }; > > > -builtin_amba_driver(cti_driver); > > > + > > > +static int __init cti_init(void) > > > +{ > > > + int ret; > > > + > > > + ret = amba_driver_register(&cti_driver); > > > + if (ret) > > > + pr_info("Error registering cti driver\n"); > > > + coresight_set_cti_ops(&cti_assoc_ops); > > > + return ret; > > > +} > > > + > > > +static void __exit cti_exit(void) > > > +{ > > > + coresight_remove_cti_ops(); > > > + amba_driver_unregister(&cti_driver); > > > +} > > > + > > > +module_init(cti_init); > > > +module_exit(cti_exit); > > > diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h > > > index dcb8aeb6af62..6cde6cf42554 100644 > > > --- a/drivers/hwtracing/coresight/coresight-priv.h > > > +++ b/drivers/hwtracing/coresight/coresight-priv.h > > > @@ -173,15 +173,13 @@ static inline int etm_readl_cp14(u32 off, unsigned int *val) { return 0; } > > > static inline int etm_writel_cp14(u32 off, u32 val) { return 0; } > > > #endif > > > > > > -#ifdef CONFIG_CORESIGHT_CTI > > > -extern void cti_add_assoc_to_csdev(struct coresight_device *csdev); > > > -extern void cti_remove_assoc_from_csdev(struct coresight_device *csdev); > > > +struct cti_assoc_op { > > > + void (*add)(struct coresight_device *csdev); > > > + void (*remove)(struct coresight_device *csdev); > > > +}; > > > > > > -#else > > > -static inline void cti_add_assoc_to_csdev(struct coresight_device *csdev) {} > > > -static inline void > > > -cti_remove_assoc_from_csdev(struct coresight_device *csdev) {} > > > -#endif > > > +extern void coresight_set_cti_ops(const struct cti_assoc_op *cti_op); > > > +extern void coresight_remove_cti_ops(void); > > > > > > /* > > > * Macros and inline functions to handle CoreSight UCI data and driver > > > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c > > > index 0b0e31577b9b..31b9ec8d3b9c 100644 > > > --- a/drivers/hwtracing/coresight/coresight.c > > > +++ b/drivers/hwtracing/coresight/coresight.c > > > @@ -57,6 +57,20 @@ const u32 coresight_barrier_pkt[4] = { > > > 0x7fffffff, 0x7fffffff, 0x7fffffff, 0x7fffffff}; > > > EXPORT_SYMBOL_GPL(coresight_barrier_pkt); > > > > > > +static const struct cti_assoc_op *cti_assoc_ops; > > > + > > > +void coresight_set_cti_ops(const struct cti_assoc_op *cti_op) > > > +{ > > > + cti_assoc_ops = cti_op; > > > +} > > > +EXPORT_SYMBOL_GPL(coresight_set_cti_ops); > > > + > > > +void coresight_remove_cti_ops(void) > > > +{ > > > + cti_assoc_ops = NULL; > > > +} > > > +EXPORT_SYMBOL_GPL(coresight_remove_cti_ops); > > > + > > > static int coresight_id_match(struct device *dev, void *data) > > > { > > > int trace_id, i_trace_id; > > > @@ -1242,7 +1256,8 @@ static void coresight_device_release(struct device *dev) > > > { > > > struct coresight_device *csdev = to_coresight_device(dev); > > > > > > - cti_remove_assoc_from_csdev(csdev); > > > + if (cti_assoc_ops && cti_assoc_ops->remove) > > > + cti_assoc_ops->remove(csdev); > > > fwnode_handle_put(csdev->dev.fwnode); > > > kfree(csdev->refcnt); > > > kfree(csdev); > > > @@ -1553,8 +1568,8 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > > > ret = coresight_fixup_device_conns(csdev); > > > if (!ret) > > > ret = coresight_fixup_orphan_conns(csdev); > > > - if (!ret) > > > - cti_add_assoc_to_csdev(csdev); > > > + if (!ret && cti_assoc_ops && cti_assoc_ops->add) > > > + cti_assoc_ops->add(csdev); > > > > > > mutex_unlock(&coresight_mutex); > > > if (ret) { > > > -- > > > 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