[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1cef4224-1f0a-4c51-937d-66823a22dec3@oss.qualcomm.com>
Date: Thu, 4 Sep 2025 09:12:19 +0800
From: Jie Gan <jie.gan@....qualcomm.com>
To: Suzuki K Poulose <suzuki.poulose@....com>,
Mike Leach <mike.leach@...aro.org>,
James Clark <james.clark@...aro.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Tao Zhang <quic_taozha@...cinc.com>,
Mao Jinlong <quic_jinlmao@...cinc.com>
Cc: tingwei.zhang@....qualcomm.com, coresight@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v1] coresight: tpda: fix the logic to setup the element
size
On 9/3/2025 5:45 PM, Jie Gan wrote:
>
>
> On 9/3/2025 4:57 PM, Suzuki K Poulose wrote:
>> On 06/08/2025 09:09, Jie Gan wrote:
>>> Some TPDM devices support both CMB and DSB datasets, requiring
>>> the system to enable the port with both corresponding element sizes.
>>>
>>> Currently, the logic treats tpdm_read_element_size as successful if
>>> the CMB element size is retrieved correctly, regardless of whether
>>> the DSB element size is obtained. This behavior causes issues
>>> when parsing data from TPDM devices that depend on both element sizes.
>>>
>>> To address this, the function should explicitly fail if the DSB
>>> element size cannot be read correctly.
>>
>> But what is the device only has CMB ? Back when this was originally
>
> We have CMB TPDM, DSB TPDM and CMB&&DSB TPDM.
>
>> merged, we raised this question and the answer was, "Only one is
>> supported, not both." But this sounds like that is wrong.
>
> I think we may not answer the previous question clearly. But it
> definitely has issue here.
>
>> Could we defer the "Warning" to the caller. i.e., Let the caller
>> figure out the if the DSB size is found and predicate that on the
>> DSB support on the TPDM.
>
> Understood, below codes will be added in the caller to check the error:
> if ((tpdm_data->dsb && !drvdata->dsb_esize) ||
> (tpdm_data->cmb && !drvdata->cmb_esize))
> goto err;
>
> Thanks,
> Jie
>
Hi Suzuki,
I've reviewed the logic here. It's not feasible for the caller to
perform the check, since we first retrieve TPDM's drvdata, which adds
complexity to the code. I believe it's better to handle this within the
function itself.
We are expecting the element_size for cmb if the condition is true, as
well as dsb:
if (tpdm_data->dsb)
...
should obtain a valid element size for dsb.
...
if (tpdm_data->cmb)
...
should obtain a valid element size for cmb.
...
Thanks,
Jie
>>
>> Suzuki
>>
>>>
>>> Fixes: e6d7f5252f73 ("coresight-tpda: Add support to configure CMB
>>> element")
>>> Signed-off-by: Jie Gan <jie.gan@....qualcomm.com>
>>> ---
>>> drivers/hwtracing/coresight/coresight-tpda.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/
>>> hwtracing/coresight/coresight-tpda.c
>>> index 0633f04beb24..333b3cb23685 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tpda.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>>> @@ -71,6 +71,8 @@ static int tpdm_read_element_size(struct
>>> tpda_drvdata *drvdata,
>>> if (tpdm_data->dsb) {
>>> rc = fwnode_property_read_u32(dev_fwnode(csdev->dev.parent),
>>> "qcom,dsb-element-bits", &drvdata->dsb_esize);
>>> + if (rc)
>>> + goto out;
>>> }
>>> if (tpdm_data->cmb) {
>>> @@ -78,6 +80,7 @@ static int tpdm_read_element_size(struct
>>> tpda_drvdata *drvdata,
>>> "qcom,cmb-element-bits", &drvdata->cmb_esize);
>>> }
>>> +out:
>>> if (rc)
>>> dev_warn_once(&csdev->dev,
>>> "Failed to read TPDM Element size: %d\n", rc);
>>
>>
>
Powered by blists - more mailing lists