Re: [PATCH v1] coresight: tpda: fix the logic to setup the element size

[Jie Gan <jie.gan@oss.qualcomm.com>]

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

> 
> Suzuki
> 
>>
>> Fixes: e6d7f5252f73 ("coresight-tpda: Add support to configure CMB 
>> element")
>> Signed-off-by: Jie Gan <jie.gan@oss.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);
> 
>