Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220621055035.31766-1-bwicaksono@nvidia.com> <20220621055035.31766-2-bwicaksono@nvidia.com>
 <73dafe08-d1f1-90b6-995e-7d38e9e1dce7@arm.com> <SJ0PR12MB567600F730B47F3A1007775AA0829@SJ0PR12MB5676.namprd12.prod.outlook.com>
 <20220712163638.GA2945984@p14s> <632f5c80-2be3-ace5-6b0d-ee0c9e5560ff@arm.com>
In-Reply-To: <632f5c80-2be3-ace5-6b0d-ee0c9e5560ff@arm.com>
From:   Mathieu Poirier <mathieu.poirier@linaro.org>
Date:   Wed, 13 Jul 2022 10:26:34 -0600
Message-ID: <CANLsYkyCcRHad+oGXNRG-rieAu9b386XnGoJUTTgHAMFP28=xA@mail.gmail.com>
Subject: Re: [RESEND PATCH v3 1/2] perf: coresight_pmu: Add support for ARM
 CoreSight PMU driver
To:     Robin Murphy <robin.murphy@arm.com>
Cc:     Besar Wicaksono <bwicaksono@nvidia.com>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        "catalin.marinas@arm.com" <catalin.marinas@arm.com>,
        "will@kernel.org" <will@kernel.org>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
        "sudeep.holla@arm.com" <sudeep.holla@arm.com>,
        "thanu.rangarajan@arm.com" <thanu.rangarajan@arm.com>,
        "Michael.Williams@arm.com" <Michael.Williams@arm.com>,
        Thierry Reding <treding@nvidia.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Vikram Sethi <vsethi@nvidia.com>,
        "mike.leach@linaro.org" <mike.leach@linaro.org>,
        "leo.yan@linaro.org" <leo.yan@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 12 Jul 2022 at 14:13, Robin Murphy <robin.murphy@arm.com> wrote:
>
> 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?
>

Not sure how the above helps moving the conversation forward.

> 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.
>

Happy to see that we are in complete agreement.

> 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* ]
>
> Cheers,
> Robin.
>
> >
> > Thanks,
> > Steve
> >
> > [1]. drivers/hwtracing/coresight/
> > [2]. https://developer.arm.com/documentation/ihi0091/latest