Messages in this thread | | | Date | Thu, 25 May 2023 10:08:15 +0100 | Subject | Re: [PATCH v4 02/11] coresight-tpda: Add DSB dataset support | From | Suzuki K Poulose <> |
| |
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@quicinc.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
| |