Messages in this thread | | | Date | Wed, 29 Mar 2023 14:23:19 +0100 | Subject | Re: [PATCH v2 8/9] coresight: Enable and disable helper devices adjacent to the path | From | Suzuki K Poulose <> |
| |
On 29/03/2023 13:04, James Clark wrote: > > > On 17/03/2023 11:04, Suzuki K Poulose wrote: >> On 10/03/2023 16:06, James Clark wrote: >>> Currently CATU is the only helper device, and its enable and disable >>> calls are hard coded. To allow more helper devices to be added in a >>> generic way, remove these hard coded calls and just enable and disable >>> all helper devices. >>> >>> This has to apply to helpers adjacent to the path, because they will >>> never be in the path. CATU was already discovered in this way, so >>> there is no change there. >>> >>> One change that is needed is for CATU to call back into ETR to allocate >>> the buffer. Because the enable call was previously hard coded, it was >>> done at a point where the buffer was already allocated, but this is no >>> longer the case. >>> >>> Signed-off-by: James Clark <james.clark@arm.com> >>> --- >>> drivers/hwtracing/coresight/coresight-catu.c | 34 ++++++++-- >>> drivers/hwtracing/coresight/coresight-core.c | 68 ++++++++++++++++++- >>> .../hwtracing/coresight/coresight-tmc-etr.c | 28 -------- >>> include/linux/coresight.h | 3 +- >>> 4 files changed, 99 insertions(+), 34 deletions(-) >>> >>> diff --git a/drivers/hwtracing/coresight/coresight-catu.c >>> b/drivers/hwtracing/coresight/coresight-catu.c >>> index bc90a03f478f..24a08a2b96b1 100644 >>> --- a/drivers/hwtracing/coresight/coresight-catu.c >>> +++ b/drivers/hwtracing/coresight/coresight-catu.c >>> @@ -395,13 +395,32 @@ static inline int catu_wait_for_ready(struct >>> catu_drvdata *drvdata) >>> return coresight_timeout(csa, CATU_STATUS, CATU_STATUS_READY, 1); >>> } >>> -static int catu_enable_hw(struct catu_drvdata *drvdata, void *data) >>> +static struct coresight_device * >>> +catu_get_etr_device(struct coresight_device *csdev) >>> +{ >>> + int i; >>> + struct coresight_device *tmp; >>> + >>> + for (i = 0; i < csdev->pdata->nr_inconns; i++) { >>> + tmp = csdev->pdata->in_conns[i].remote_dev; >>> + if (tmp && tmp->type == CORESIGHT_DEV_TYPE_SINK && >>> + tmp->subtype.sink_subtype == >>> + CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM) >>> + return tmp; >>> + } >>> + >>> + return NULL; >>> +} >>> + >>> +static int catu_enable_hw(struct catu_drvdata *drvdata, enum cs_mode >>> cs_mode, >>> + void *data) >>> { >>> int rc; >>> u32 control, mode; >>> - struct etr_buf *etr_buf = data; >>> + struct etr_buf *etr_buf = NULL; >>> struct device *dev = &drvdata->csdev->dev; >>> struct coresight_device *csdev = drvdata->csdev; >>> + struct coresight_device *etrdev; >>> if (catu_wait_for_ready(drvdata)) >>> dev_warn(dev, "Timeout while waiting for READY\n"); >>> @@ -416,6 +435,12 @@ static int catu_enable_hw(struct catu_drvdata >>> *drvdata, void *data) >>> if (rc) >>> return rc; >>> + etrdev = catu_get_etr_device(csdev); >>> + if (etrdev) { >>> + etr_buf = tmc_etr_get_buffer(etrdev, cs_mode, data); >>> + if (IS_ERR(etr_buf)) >>> + return PTR_ERR(etr_buf); >>> + } >> >> WARN_ON(!etrdev) ? We are not supposed to reach in the first place and >> return. >> > > I saw there was the pass-through mode below which I thought didn't need > an ETR device. I think I followed the code through and there was a way > for it to get there without an ETR in the existing version, but now I'm > not sure.
> Or does it still need the ETR device but it just doesn't > access the buffer?
The first part is correct. Without an ETR, CATU wouldn't be a helper device, and wouldn't get here in "enable CATU" via the helper route. The CATU chooses the mode depending on the etr_buf mode.
Suzuki
| |