On 03/04/2025 18:53, Yabin Cui wrote:
On Wed, Apr 2, 2025 at 6:01 AM Leo Yan leo.yan@arm.com wrote:
Hi Yabin,
On Tue, Apr 01, 2025 at 06:21:59PM -0700, Yabin Cui wrote:
[...]
@@ -486,12 +491,17 @@ static int catu_disable_hw(struct catu_drvdata *drvdata)
static int catu_disable(struct coresight_device *csdev, void *__unused) {
int rc;
int rc = 0; struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev);guard(raw_spinlock_irqsave)(&catu_drvdata->spinlock);
CS_UNLOCK(catu_drvdata->base);rc = catu_disable_hw(catu_drvdata);CS_LOCK(catu_drvdata->base);
if (--csdev->refcnt == 0) {CS_UNLOCK(catu_drvdata->base);rc = catu_disable_hw(catu_drvdata);CS_LOCK(catu_drvdata->base);} else {rc = -EBUSY;This is not an error if the decremented reference counter is not zero. It should return 0. Otherwise, the change looks good to me.
In coresight_disable_helpers(), the return value of catu_disable() isn't checked. The -EBUSY return was used for consistency with other refcounted disable functions like tmc_disable_etf_sink() and tmc_disable_etr_sink(). I'm happy to change it back to 0 if you believe that would be the more accurate return value here.
Please stick to 0 here. This indicates there was no errors w.r.t the current session. This is similar to what we do for TMC ETR for e.g.
Suzuki
Thanks, Leo