[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9f414290-0219-302f-ca8b-231217e68efb@arm.com>
Date: Thu, 25 May 2023 10:08:15 +0100
From: Suzuki K Poulose <suzuki.poulose@....com>
To: Tao Zhang <quic_taozha@...cinc.com>,
Mathieu Poirier <mathieu.poirier@...aro.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Konrad Dybcio <konradybcio@...il.com>,
Mike Leach <mike.leach@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Cc: Jinlong Mao <quic_jinlmao@...cinc.com>,
Leo Yan <leo.yan@...aro.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
Tingwei Zhang <quic_tingweiz@...cinc.com>,
Yuanfang Zhang <quic_yuanfang@...cinc.com>,
Trilok Soni <quic_tsoni@...cinc.com>,
Hao Zhang <quic_hazha@...cinc.com>,
linux-arm-msm@...r.kernel.org, andersson@...nel.org
Subject: Re: [PATCH v4 02/11] coresight-tpda: Add DSB dataset support
On 25/05/2023 08:16, Tao Zhang wrote:
>
> On 5/23/2023 6:07 PM, Suzuki K Poulose wrote:
>> On 27/04/2023 10:00, Tao Zhang wrote:
>>> Read the DSB element size from the device tree. Set the register
>>> bit that controls the DSB element size of the corresponding port.
>>>
>>> Signed-off-by: Tao Zhang <quic_taozha@...cinc.com>
>>> ---
>>> drivers/hwtracing/coresight/coresight-core.c | 1 +
>>> drivers/hwtracing/coresight/coresight-tpda.c | 92
>>> +++++++++++++++++++++++++---
>>> drivers/hwtracing/coresight/coresight-tpda.h | 4 ++
>>> drivers/hwtracing/coresight/coresight-tpdm.c | 2 +-
>>> include/linux/coresight.h | 1 +
>>> 5 files changed, 90 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>> b/drivers/hwtracing/coresight/coresight-core.c
>>> index 2af416b..f1eacbb 100644
>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>> @@ -1092,6 +1092,7 @@ static int coresight_validate_source(struct
>>> coresight_device *csdev,
>>> if (subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_PROC &&
>>> subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE &&
>>> + subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM &&
>>> subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS) {
>>> dev_err(&csdev->dev, "wrong device subtype in %s\n",
>>> function);
>>> return -EINVAL;
>>
>> Please see the comment at the bottom.
>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c
>>> b/drivers/hwtracing/coresight/coresight-tpda.c
>>> index 8d2b9d2..af9c72f 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tpda.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>>> @@ -21,6 +21,56 @@
>>> DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
>>> +/* Search and read element data size from the TPDM node in
>>> + * the devicetree. Each input port of TPDA is connected to
>>> + * a TPDM. Different TPDM supports different types of dataset,
>>> + * and some may support more than one type of dataset.
>>> + * Parameter "inport" is used to pass in the input port number
>>> + * of TPDA, and it is set to 0 in the recursize call.
>>> + * Parameter "parent" is used to pass in the original call.
>>> + */
>>> +static int tpda_set_element_size(struct tpda_drvdata *drvdata,
>>> + struct coresight_device *csdev, int inport, bool parent)
>>> +{
>>> + static int nr_inport;
>>> + int i;
>>> + static bool tpdm_found;
>>> + struct coresight_device *in_csdev;
>>> +
>>> + if (inport > (TPDA_MAX_INPORTS - 1))
>>> + return -EINVAL;
>>> +
>>> + if (parent) {
>>> + nr_inport = inport;
>>> + tpdm_found = false;
>>> + }
>>> +
>>> + for (i = 0; i < csdev->pdata->nr_inconns; i++) {
>>> + in_csdev = csdev->pdata->in_conns[i]->src_dev;
>>> + if (!in_csdev)
>>> + break;
>>> +
>>> + if (parent)
>>> + if (csdev->pdata->in_conns[i]->dest_port != inport)
>>> + continue;
>>> +
>>> + if (in_csdev->subtype.source_subtype
>>
>> We must match the in_csdev->type to be SOURCE && the subtype.
> Sure, I will update it in the next patch series.
>>
>>> + == CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM) {
>>> + of_property_read_u8(in_csdev->dev.parent->of_node,
>>> + "qcom,dsb-element-size",
>>> &drvdata->dsb_esize[nr_inport]);
>>> + if (!tpdm_found)
>>> + tpdm_found = true;
>>> + else
>>> + dev_warn(drvdata->dev,
>>> + "More than one TPDM is mapped to the TPDA input
>>> port %d.\n",
>>> + nr_inport);
>>
>> When we know, we have found a source device, we don't need to recurse
>> down and could simply 'continue' to the next one in the list and skip
>> the call below.
>
> Actually, one input port on TPDA only can connect to one TPDM. In the
> current design, it will
>
> find out all the TPDMs on one input port and warn the users all the
> TPDMs it found. If we
>
> replace 'recurse down' as 'continue' here, it may not find some TPDMs
> that might be connected
>
> incorrectly.
What do you mean ? When you enter the if () above, the in_csdev is a
source and it is TPDM. There must be no input connections TPDM, i.e.
in_csdev, so no need to go further up the connection chain looking at
the in_csdev. The loop will continue to analyse this device (where we
found one TPDM already) and detect any further duplicate TPDMs
connected.
Suzuki
Powered by blists - more mailing lists