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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 6E40AC43334 for ; Thu, 21 Jul 2022 15:39:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc: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=pObh7zjuvSd0QLdWh89qSERi4n9EIiqo1W5QYae58Fk=; b=XwPO8+MKLJms71 4kFVxf+wKtB1UEtmlZ0fUXv5/h8zfJBuORaJlsJYeSxRMVM5tayjcO6avYB3K602QlADy4qrmNSCE a5mUjo2DzHoliLK0tTiYFsrR0SNzGSfBOf0fVecZzQqedEdEDBAXGhdHrFwHvXG2r+sDFwkZFQ57p 1krPQg+vOdBMm3Fa9GvQ3Y9+x28Dkr2kySKn4vh7oI6mU9DCWMfGlIGbnaYavlu7Zb7WfqdQaV8jQ 1hlhVs2AujYUTkgpsemOhgfG29Cn2eY1zjf0g1omeQVWUgpee7U1jbAABuu4pSS1+NO62AIGXj69/ AQgebTCrfiGLW/ZKXFyQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oEYFO-009CHj-JD; Thu, 21 Jul 2022 15:37:55 +0000 Received: from mail-wm1-x336.google.com ([2a00:1450:4864:20::336]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oEYDX-009BCS-ET for linux-arm-kernel@lists.infradead.org; Thu, 21 Jul 2022 15:36:01 +0000 Received: by mail-wm1-x336.google.com with SMTP id c22so1263144wmr.2 for ; Thu, 21 Jul 2022 08:35:57 -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=gDBJvFECTm7Pq0C9lMhNGhK5Ebl1v9EZPxP9t1ZpvwA=; b=sGunr33dt6mX3jqDWKkLep+kA83yczxElAtS14nQZ2Vfu6DTWEcJUj3a2SwFQF7NlK ICpXXVaaB0UkZ5ddtVY4rub5bljSbOCSlp3enDvZhNEFUaWpgiWdo0xSZBhJS9SCmM8V SyK5uRcUtwsVQj1JXBbTzJBonqTvH/09+WyGPWwtKWiLsdkN/Au74V+ydLRv3FNkbTRF /lxF8Op8JhqpRZw27Ld71b7mke+o7/qhVC2gg1ls4Xl+IsLiunL2/YqJibtNhLW25SNt P4SYnjkcjpSGAn/GB8bZ8GbLT4KX4MQSzMJPfhx2V307hfiDErz2vC8j0hfVYSVD/1vu 2BpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=gDBJvFECTm7Pq0C9lMhNGhK5Ebl1v9EZPxP9t1ZpvwA=; b=HmdpEIKVZQ2ttC9LCmTq2qltuAqW9PfofeROHf3gNa4VhHfVAfq5Ddr6YwFVYv1h+V 1OJdvJuCx26wuATfps8RQeptHAIqx7v0JVqov1E8MwEkJkOqIqDmFhhtY3S5xs7b1ywK qt/mlRxjm5/wrJnT6eez5FigXOZeRhW8xNhcxlLiiG9h9GvZWSjcuY5L6KM1099ZsyIM 4cimsZtY3Gs6xurUmEQiX1KtislniAAQdFZ6tL8TTq8XSml2AwVJgraO2bSvWFm0F2bR PMiyj1C7NvftUZsvpZbLZO16NXH+on3zt441UpTf8y4JstWS1rLJ2b3gtuwgvJtvqRFt 6rPQ== X-Gm-Message-State: AJIora8NYq9OEab35MlshNzX4ZyzOmMlfww4ZFCrp6dN2fKtZDeKi3Nl 1QzCNlgGtMt1Gw4tau/TMFQ2YEBJaZjYM5Hq6lXs7Q== X-Google-Smtp-Source: AGRyM1tP93XG4GyFGMmlNGqL2//7Nm5AvTy0LETAggQ8g6455bgaHFfidkzefjUu26ZycBY8KBYeE0czOkrQgtO0wZE= X-Received: by 2002:a05:600c:4f8e:b0:3a3:30b7:4901 with SMTP id n14-20020a05600c4f8e00b003a330b74901mr3126400wmq.17.1658417756135; Thu, 21 Jul 2022 08:35:56 -0700 (PDT) MIME-Version: 1.0 References: <20220621055035.31766-1-bwicaksono@nvidia.com> <20220621055035.31766-2-bwicaksono@nvidia.com> <73dafe08-d1f1-90b6-995e-7d38e9e1dce7@arm.com> <20220712163638.GA2945984@p14s> <632f5c80-2be3-ace5-6b0d-ee0c9e5560ff@arm.com> <9d36f053-f929-204b-fb47-60c685b06717@arm.com> In-Reply-To: <9d36f053-f929-204b-fb47-60c685b06717@arm.com> From: Mathieu Poirier Date: Thu, 21 Jul 2022 09:35:44 -0600 Message-ID: Subject: Re: [RESEND PATCH v3 1/2] perf: coresight_pmu: Add support for ARM CoreSight PMU driver To: Suzuki K Poulose Cc: Besar Wicaksono , Robin Murphy , "catalin.marinas@arm.com" , "will@kernel.org" , "mark.rutland@arm.com" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-tegra@vger.kernel.org" , "sudeep.holla@arm.com" , "thanu.rangarajan@arm.com" , "Michael.Williams@arm.com" , Thierry Reding , Jonathan Hunter , Vikram Sethi , "mike.leach@linaro.org" , "leo.yan@linaro.org" X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220721_083559_518703_958BC07F X-CRM114-Status: GOOD ( 53.46 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 Thu, 21 Jul 2022 at 03:19, Suzuki K Poulose wrote: > > Hi > > On 14/07/2022 05:47, Besar Wicaksono wrote: > > > > > >> -----Original Message----- > >> From: Robin Murphy > >> Sent: Wednesday, July 13, 2022 3:13 AM > >> To: Mathieu Poirier ; Besar Wicaksono > >> > >> Cc: Suzuki K Poulose ; catalin.marinas@arm.com; > >> will@kernel.org; mark.rutland@arm.com; linux-arm- > >> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux- > >> tegra@vger.kernel.org; sudeep.holla@arm.com; > >> thanu.rangarajan@arm.com; Michael.Williams@arm.com; Thierry Reding > >> ; Jonathan Hunter ; Vikram > >> Sethi ; mike.leach@linaro.org; leo.yan@linaro.org > >> Subject: Re: [RESEND PATCH v3 1/2] perf: coresight_pmu: Add support for > >> ARM CoreSight PMU driver > >> > >> External email: Use caution opening links or attachments > >> > >> > >> On 2022-07-12 17:36, Mathieu Poirier wrote: > >> [...] > >>>>> If we have decied to call this arm_system_pmu, (which I am perfectly > >>>>> happy with), could we please stick to that name for functions that we > >>>>> export ? > >>>>> > >>>>> e.g, > >>>>> > >> s/coresight_pmu_sysfs_event_show/arm_system_pmu_event_show()/ > >>>>> > >>>> > >>>> Just want to confirm, is it just the public functions or do we need to > >> replace > >>>> all that has "coresight" naming ? Including the static functions, structs, > >> filename. > >>> > >>> I think all references to "coresight" should be changed to > >> "arm_system_pmu", > >>> including filenames. That way there is no doubt this IP block is not > >>> related, and does not interoperate, with the any of the "coresight" IP > >> blocks > >>> already supported[1] in the kernel. > >>> > >>> I have looked at the documentation[2] in the cover letter and I agree > >>> with an earlier comment from Sudeep that this IP has very little to do with > >> any > >>> of the other CoreSight IP blocks found in the CoreSight framework[1]. > >> Using the > >>> "coresight" naming convention in this driver would be _extremely_ > >> confusing, > >>> especially when it comes to exported functions. > >> > >> But conversely, how is it not confusing to make up completely different > >> names for things than what they're actually called? The CoreSight > >> Performance Monitoring Unit is a part of the Arm CoreSight architecture, > >> it says it right there on page 1. What if I instinctively associate the > >> name Mathieu with someone more familiar to me, so to avoid confusion I'd > >> prefer to call you Steve? Is that OK? > >> > > > > What is the naming convention for modules under drivers/perf ? > > In my observation, the names there correspond to the part monitored by > > the PMU. The confusion on using "coresight_pmu" naming could be that > > people may think the PMU monitors coresight system, i.e the trace system under hwtracing. > > However, the driver in this patch is for a new PMU standard that monitors uncore > > parts. Uncore was considered as terminology from Intel, so "system" was picked instead. > > Please see this thread for reference: > > https://lore.kernel.org/linux-arm-kernel/20220510111318.GD27557@willie-the-truck/ > > I think we all understand the state of affairs. > > - We have an architecutre specification for PMUs, Arm CoreSight PMU > Architecutre, which has absolutely no relationship with : > > either CoreSight Self-Hosted Tracing (handled by "coresight" > subsystem in the kernel under drivers/hwtracing/coresight/, with a user > visible pmu as "cs_etm") > > or the CoreSight Architecture (except for the name). This is of less > significance in general. But has a significant impact on the "name" > users might expect for the driver/Kconfig etc. > > - We want to be able to make it easier for the users/developers to > choose what they want without causing confusion. > > For an end-user: Having the PMU instance named after the "System IP" > (as implememented in the driver solves the problem and falling back to > arm_system_pmu is a good enough choice. So let us stick with that) > > Kconfig: May be we can choose > CONFIG_ARM_CORESIGHT_PMU_ARCH_PMU > or even > CONFIG_ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU > > with appropriate help text to ensure there is enough stress about what > this is and what this is not would be sufficient. > > Now the remaining contention is about the name of the "subsystem" and > also the dir/files. This may sound insignificant. But it is also > important to get this right. e.g., helps the reviewers unambiguously > identify the change or maintainers accepting pull requests (remember > these two PMUs (cs_etm and this one) go via different trees.). Not > everyone who deals with this in the community may be aware of how > these are different. > > We could choose arm_cspmu_ or simply cspmu. Given that only the > "normal" users care about the "association" with the "architecture" > and more advanced users (e.g, developers) can easily map "Kconfig" > to driver files, may be we could even stick to the "arm_syspmu" > (from "arm system pmu") ? > +1 on "arm_syspmu" > Suzuki > > > > > >> As it happens, Steve, I do actually agree with you that "coresight_" is > >> a bad prefix here, but only for the reason that it's too general. TBH I > >> think that's true of the existing Linux subsystem too, but that damage > >> is already done, and I'd concur that there's little value in trying to > >> unpick that now, despite the clear existence of products like CoreSight > >> DAP and CoreSight ELA which don't have all that much to do with program > >> trace either. > >> > >> However, hindsight and inertia are hardly good reasons to double down on > >> poor decisions, so if I was going to vote for anything here it would be > >> "cspmu_", which is about as > >> obviously-related-to-the-thing-it-actually-is as we can get while also > >> being pleasantly concise. > >> > >> [ And no, this isn't bikeshedding. Naming things right is *important* ] > >> > > > > I agree having the correct name is important, especially at this early stage. > > A direction of what the naming should describe would be very helpful here. > > > >> Cheers, > >> Robin. > >> > >>> > >>> Thanks, > >>> Steve > >>> > >>> [1]. drivers/hwtracing/coresight/ > >>> [2]. https://developer.arm.com/documentation/ihi0091/latest > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel