Messages in this thread | | | Date | Wed, 29 Mar 2023 15:10:36 +0100 | Subject | Re: [PATCH v2 8/9] coresight: Enable and disable helper devices adjacent to the path | From | James Clark <> |
| |
On 29/03/2023 14:23, Suzuki K Poulose wrote: > 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.
Ok thanks I will add that warning then
> > > Suzuki >
| |