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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 72AC3C77B75 for ; Wed, 17 May 2023 04:33:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229509AbjEQEc6 (ORCPT ); Wed, 17 May 2023 00:32:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47764 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229437AbjEQEc5 (ORCPT ); Wed, 17 May 2023 00:32:57 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 4094F2D4F; Tue, 16 May 2023 21:32:55 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 63B5D1FB; Tue, 16 May 2023 21:33:39 -0700 (PDT) Received: from [10.163.70.237] (unknown [10.163.70.237]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id BDF003F793; Tue, 16 May 2023 21:32:48 -0700 (PDT) Message-ID: <28c7a088-bfdb-5172-c7c4-0a572ecda78a@arm.com> Date: Wed, 17 May 2023 10:02:45 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH V2 3/5] coresight: etm4x: Drop pid argument from etm4_probe() Content-Language: en-US To: Suzuki K Poulose , linux-arm-kernel@lists.infradead.org, coresight@lists.linaro.org Cc: scclevenger@os.amperecomputing.com, Rob Herring , Frank Rowand , Russell King , Greg Kroah-Hartman , "Rafael J. Wysocki" , Len Brown , Sudeep Holla , Lorenzo Pieralisi , Mathieu Poirier , Mike Leach , Leo Yan , devicetree@vger.kernel.org, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org References: <20230327050537.30861-1-anshuman.khandual@arm.com> <20230327050537.30861-4-anshuman.khandual@arm.com> From: Anshuman Khandual In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On 3/31/23 16:36, Suzuki K Poulose wrote: > On 27/03/2023 06:05, Anshuman Khandual wrote: >> Coresight device pid can be retrieved from its iomem base address, which is >> stored in 'struct etm4x_drvdata'. This drops pid argument from etm4_probe() >> and 'struct etm4_init_arg'. Instead etm4_check_arch_features() derives the >> coresight device pid with a new helper coresight_get_pid(), right before it >> is consumed in etm4_hisi_match_pid(). >> >> Cc: Mathieu Poirier >> Cc: Suzuki K Poulose >> Cc: Mike Leach >> Cc: Leo Yan >> Cc: coresight@lists.linaro.org >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual >> --- >>   .../coresight/coresight-etm4x-core.c          | 21 +++++++------------ >>   include/linux/coresight.h                     | 12 +++++++++++ >>   2 files changed, 20 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> index 5d77571a8df9..3521838ab4fb 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> @@ -66,7 +66,6 @@ static u64 etm4_get_access_type(struct etmv4_config *config); >>   static enum cpuhp_state hp_online; >>     struct etm4_init_arg { >> -    unsigned int        pid; >>       struct device        *dev; >>       struct csdev_access    *csa; >>   }; >> @@ -370,8 +369,10 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) >>   } >>     static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, >> -                      unsigned int id) >> +                     struct csdev_access *csa) >>   { >> +    unsigned int id = coresight_get_pid(csa); >> + > > This throws up the following error on an ETE. > > ete: trying to read unsupported register @fe0 > > So, I guess this must be performed only for iomem based > devices. System instruction based device must be identified > by MIDR_EL1/REVIDR_EL1 if needed for specific erratum. > This is not required now. So, we could bail out early > if we are system instruction based device. Will bail out on non iomem devices via drvdata->base address switch. --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -373,9 +373,10 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, struct csdev_access *csa) { - unsigned int id = coresight_get_pid(csa); + if (!drvdata->base) + return; - if (etm4_hisi_match_pid(id)) + if (etm4_hisi_match_pid(coresight_get_pid(csa))) set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features); } #else > > >>       if (etm4_hisi_match_pid(id)) >>           set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features); >>   } >> @@ -385,7 +386,7 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) >>   } >>     static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, >> -                     unsigned int id) >> +                     struct csdev_access *csa) >>   { >>   } >>   #endif /* CONFIG_ETM4X_IMPDEF_FEATURE */ >> @@ -1165,7 +1166,7 @@ static void etm4_init_arch_data(void *info) >>       etm4_os_unlock_csa(drvdata, csa); >>       etm4_cs_unlock(drvdata, csa); >>   -    etm4_check_arch_features(drvdata, init_arg->pid); >> +    etm4_check_arch_features(drvdata, csa); >>         /* find all capabilities of the tracing unit */ >>       etmidr0 = etm4x_relaxed_read32(csa, TRCIDR0); >> @@ -2048,7 +2049,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) >>       return 0; >>   } >>   -static int etm4_probe(struct device *dev, u32 etm_pid) >> +static int etm4_probe(struct device *dev) >>   { >>       struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); >>       struct csdev_access access = { 0 }; >> @@ -2077,7 +2078,6 @@ static int etm4_probe(struct device *dev, u32 etm_pid) >>         init_arg.dev = dev; >>       init_arg.csa = &access; >> -    init_arg.pid = etm_pid; >>         /* >>        * Serialize against CPUHP callbacks to avoid race condition >> @@ -2124,7 +2124,7 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) >>         drvdata->base = base; >>       dev_set_drvdata(dev, drvdata); >> -    ret = etm4_probe(dev, id->id); >> +    ret = etm4_probe(dev); >>       if (!ret) >>           pm_runtime_put(&adev->dev); >>   @@ -2146,12 +2146,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) >>       pm_runtime_set_active(&pdev->dev); >>       pm_runtime_enable(&pdev->dev); >>   -    /* >> -     * System register based devices could match the >> -     * HW by reading appropriate registers on the HW >> -     * and thus we could skip the PID. >> -     */ >> -    ret = etm4_probe(&pdev->dev, 0); >> +    ret = etm4_probe(&pdev->dev); >>         pm_runtime_put(&pdev->dev); >>       return ret; >> diff --git a/include/linux/coresight.h b/include/linux/coresight.h >> index f19a47b9bb5a..f85b041ea475 100644 >> --- a/include/linux/coresight.h >> +++ b/include/linux/coresight.h >> @@ -370,6 +370,18 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa, >>       return csa->read(offset, true, false); >>   } >>   +#define CORESIGHT_PIDRn(i)    (0xFE0 + ((i) * 4)) >> + >> +static inline u32 coresight_get_pid(struct csdev_access *csa) >> +{ >> +    u32 i, pid = 0; >> + >> +    for (i = 0; i < 4; i++) >> +        pid |= csdev_access_relaxed_read32(csa, CORESIGHT_PIDRn(i)) << (i * 8); > > Given the above, we could make this iomem specific. We could change coresight_get_pid() to take iomem base address instead and fetch the pid. But is not the existing csdev_access based approach better and more generic ? #define CORESIGHT_PIDRn(i) (0xFE0 + ((i) * 4)) static inline u32 coresight_get_pid(void __iomem *base) { u32 i, pid = 0; for (i = 0; i < 4; i++) pid |= readl(base + CORESIGHT_PIDRn(i)) << (i * 8); return pid; }