Re: [PATCH 10/19] coresight: etm4x: Define DEVARCH register fields

From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: mike.leach@linaro.org
Cc: coresight@lists.linaro.org, anshuman.khandual@arm.com,
	mathieu.poirier@linaro.org, linux-arm-kernel@lists.infradead.org,
	leo.yan@linaro.org
Subject: Re: [PATCH 10/19] coresight: etm4x: Define DEVARCH register fields
Date: Tue, 22 Sep 2020 11:20:36 +0100	[thread overview]
Message-ID: <8f3a92b5-8593-0dc5-591e-6548d422a1a4@arm.com> (raw)
In-Reply-To: <CAJ9a7VjMFbY1q_MnBsFNuVesxPiWQoHcGtZADneJuqBk02bbyw@mail.gmail.com>

Hi Mike

On 09/18/2020 04:34 PM, Mike Leach wrote:
> Hi Suzuki,
> 
> On Fri, 11 Sep 2020 at 09:41, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>>
>> Define the fields of the DEVARCH register for identifying
>> a component as an ETMv4.x unit. Going forward, we use the
>> DEVARCH register for the component identification, rather
>> than the TRCIDR3.
>>
> 
> TRCIDR1? - but either way, we are not using this for component ID. For

Sorry, it is TRCIDR1. Got it mixed with the TRCIDR3 for exlevel mask.

> the AMBA path component ID is made using CID + PID + optionally if in
> the table UCI - which includes DEVARCH.
> TRCIDR1 is simply used to get the architecture version so we can be
> sure the driver supports it, and can adjust behaviour for version
> dependent elements.

Correct. The idea is to use the TRCDEVARCH instead of the TRCIDR1 for
drvdata->arch. We use drvdata->arch to detect if the ETM is supported.
At the moment, we blindly support all ETMv4.x components.
With the system instruction support, we want to make sure we use a
CoreSight architected register and not an ETM architected one to
detect if the component is an ETM.

> 
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-etm4x.c |  4 ++--
>>   drivers/hwtracing/coresight/coresight-etm4x.h | 18 ++++++++++++++++++
>>   2 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
>> index 40f8113191e0..34b27c26591b 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
>> @@ -1598,8 +1598,8 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
>>   static struct amba_cs_uci_id uci_id_etm4[] = {
>>          {
>>                  /*  ETMv4 UCI data */
>> -               .devarch        = 0x47704a13,
>> -               .devarch_mask   = 0xfff0ffff,
>> +               .devarch        = ETM_DEVARCH_ETMv4x_ARCH,
>> +               .devarch_mask   = ETM_DEVARCH_ID_MASK,
>>                  .devtype        = 0x00000013,
> 
> Perhaps a good time to change this to a #define constant too.

Yes, I can change it.

> I assume that if the system access is going to use the coresight
> architected registers for ID - it should use the same set as the AMBA
> path - i.e. DEVARCH + DEVTYPE.

I doubt if DEVTYPE is necessary when we are doing a system instruction access
from the CPU. The DEVTYPE identifies the component as PE and a trace source.
Both of which are implicit with the system instruction access, but is mandatory
for an AMBA bus discovery as it is memory mapped. Moreover the TRCDEVTYPE is
only accessible from the memory mapped interface and the external debugger interface.
(as per ETM TRM).

Thanks for the review.

Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel