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: <20230406022249.2501-1-seanwang1@Lenovo.com>
In-Reply-To: <20230406022249.2501-1-seanwang1@Lenovo.com>
From:   Mike Leach <mike.leach@linaro.org>
Date:   Thu, 6 Apr 2023 15:07:41 +0100
Message-ID: <CAJ9a7VhgytZMUtgq7cJFzCDkbqu1krSO4xrubHe0sL1LrWUBrQ@mail.gmail.com>
Subject: Re: [PATCH] coresight: etm4x: add AMBA id for Cortex-A510
To:     Sean Wang <seanwang1@lenovo.com>
Cc:     asherbai@lenovo.com, mathieu.poirier@linaro.org,
        suzuki.poulose@arm.com, leo.yan@linaro.org,
        alexander.shishkin@linux.intel.com, janetliu@lenovo.com,
        Coresight ML <coresight@lists.linaro.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi,

Please also post to all the relevant lists when submitting a patch, as
defined in ./scripts/get_maintainer.pl


On Thu, 6 Apr 2023 at 03:23, Sean Wang <seanwang1@lenovo.com> wrote:
>
> From: Sean Wang <seanwang1@lenovo.com>
>
> Add AMBA UCI id to support Cortex-A510
>
> Signed-off-by: Sean Wang <seanwang1@lenovo.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 1ea8f173cca0..702704cf4f1f 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -2186,6 +2186,15 @@ static struct amba_cs_uci_id uci_id_etm4[] = {
>         }
>  };
>
> +static struct amba_cs_uci_id uci_id_ete[] = {
> +       {
> +               /*  ETE UCI data */
> +               .devarch        = ETM_DEVARCH_ETE_ARCH,
> +               .devarch_mask   = ETM_DEVARCH_ID_MASK,
> +               .devtype        = 0x00000013,
> +       }
> +};
> +
>  static void clear_etmdrvdata(void *info)
>  {
>         int cpu = *(int *)info;
> @@ -2264,6 +2273,7 @@ static const struct amba_id etm4_ids[] = {
>         CS_AMBA_UCI_ID(0x000cc0af, uci_id_etm4),/* Marvell ThunderX2 */
>         CS_AMBA_UCI_ID(0x000b6d01, uci_id_etm4),/* HiSilicon-Hip08 */
>         CS_AMBA_UCI_ID(0x000b6d02, uci_id_etm4),/* HiSilicon-Hip09 */
> +       CS_AMBA_UCI_ID(0x000bbd46, uci_id_ete),/* Cortex-A510 */
>         {},
>  };
>
> --
> 2.34.1
>

I do not believe that this is needed. ETE is architecturally required
to implement the system instructions interface to the ETE trace
registers, and memory mapped access to the ETE from the PE is removed
by the specification

Your board .dts should declare ETE with the compatible
"arm,embedded-trace-extension", and the system instruction platform
driver will be initialised, which does not require the AMBA matching
IDs.

If your solution does have the optional APB interface to the ETE we
would expect this to be used by any external debug system and not by
self hosted trace in the linux kernel.

Regards


Mike

--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK