[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a820b7c9-0919-4c78-a5cc-bf8289dd0b59@quicinc.com>
Date: Fri, 6 Jun 2025 10:56:33 +0800
From: songchai <quic_songchai@...cinc.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
CC: Suzuki K Poulose <suzuki.poulose@....com>,
Mike Leach
<mike.leach@...aro.org>, James Clark <james.clark@....com>,
Alexander
Shishkin <alexander.shishkin@...ux.intel.com>,
Andy Gross
<agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Rob Herring
<robh+dt@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>, <linux-kernel@...r.kernel.org>,
<coresight@...ts.linaro.org>, <linux-arm-kernel@...ts.infradead.org>,
<linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH v5 4/7] coresight-tgu: Add TGU decode support
On 5/29/2025 7:32 PM, Jonathan Cameron wrote:
> On Thu, 29 May 2025 16:19:45 +0800
> Songwei Chai <quic_songchai@...cinc.com> wrote:
>
>> Decoding is when all the potential pieces for creating a trigger
>> are brought together for a given step. Example - there may be a
>> counter keeping track of some occurrences and a priority-group that
>> is being used to detect a pattern on the sense inputs. These 2
>> inputs to condition_decode must be programmed, for a given step,
>> to establish the condition for the trigger, or movement to another
>> steps.
>>
>> Signed-off-by: Songwei Chai <quic_songchai@...cinc.com>
>> diff --git a/drivers/hwtracing/coresight/coresight-tgu.c b/drivers/hwtracing/coresight/coresight-tgu.c
>> index 6dbfd4c604b1..8dbe8ab30174 100644
>> --- a/drivers/hwtracing/coresight/coresight-tgu.c
>> +++ b/drivers/hwtracing/coresight/coresight-tgu.c
>> @@ -21,13 +21,35 @@ static int calculate_array_location(struct tgu_drvdata *drvdata,
>> +
>> static int tgu_enable(struct coresight_device *csdev, enum cs_mode mode,
>> void *data)
>> {
>> + int ret = 0;
> From what can be seen here, looks like ret is always set, so no need to init.
Would this initialization improve the determinism of the code? :-)
>> struct tgu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>
>> spin_lock(&drvdata->spinlock);
>> @@ -150,11 +264,15 @@ static int tgu_enable(struct coresight_device *csdev, enum cs_mode mode,
>> spin_unlock(&drvdata->spinlock);
>> return -EBUSY;
>> }
>> - tgu_write_all_hw_regs(drvdata);
>> + ret = tgu_write_all_hw_regs(drvdata);
>> +
>> + if (ret == -EINVAL)
>> + goto exit;
>> drvdata->enable = true;
>>
>> +exit:
>> spin_unlock(&drvdata->spinlock);
>> - return 0;
>> + return ret;
>> }
>> diff --git a/drivers/hwtracing/coresight/coresight-tgu.h b/drivers/hwtracing/coresight/coresight-tgu.h
>> index f07ead505365..691da393ffa3 100644
>> --- a/drivers/hwtracing/coresight/coresight-tgu.h
>> +++ b/drivers/hwtracing/coresight/coresight-tgu.h
>> enum operation_index {
>> TGU_PRIORITY0,
>> TGU_PRIORITY1,
>> TGU_PRIORITY2,
>> - TGU_PRIORITY3
>> + TGU_PRIORITY3,
> And here is why the previous patch should definitely have had the ,
Thanks for your clarification.
>
>> + TGU_CONDITION_DECODE
>>
>> };
>>
Powered by blists - more mailing lists