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

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

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@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);
>>
>>
>