Messages in this thread | | | From | Mike Leach <> | Date | Wed, 22 Mar 2023 09:29:36 +0000 | Subject | Re: [PATCH v2 8/9] coresight: Enable and disable helper devices adjacent to the path |
| |
On Fri, 17 Mar 2023 at 11:04, Suzuki K Poulose <suzuki.poulose@arm.com> 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. > > > > control |= BIT(CATU_CONTROL_ENABLE); > > > > if (etr_buf && etr_buf->mode == ETR_MODE_CATU) { > > @@ -441,13 +466,14 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, void *data) > > return 0; > > } > > > > -static int catu_enable(struct coresight_device *csdev, void *data) > > +static int catu_enable(struct coresight_device *csdev, enum cs_mode mode, > > + void *data) > > { > > int rc; > > struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev); > > > > CS_UNLOCK(catu_drvdata->base); > > - rc = catu_enable_hw(catu_drvdata, data); > > + rc = catu_enable_hw(catu_drvdata, mode, data); > > CS_LOCK(catu_drvdata->base); > > return rc; > > } > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > > index a8ba7493c09a..3e6ccd9e8d4e 100644 > > --- a/drivers/hwtracing/coresight/coresight-core.c > > +++ b/drivers/hwtracing/coresight/coresight-core.c > > @@ -441,6 +441,34 @@ static int coresight_enable_source(struct coresight_device *csdev, > > return 0; > > } > > > > +static int coresight_enable_helper(struct coresight_device *csdev, > > + enum cs_mode mode, void *sink_data) > > minor nit: s/sink_data/data/ ? Though it is always either sink_data > (perf mode) or NULL (sysfs mode), for the core code it is simply an > opaque data. > > Rest looks fine to me. > > Suzuki > > Reviewed by: Mike Leach <mike.leach@linaro.org>
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
| |