Re: [PATCH v3] coresight: fix missing error code when trace ID is invalid

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

On 5/11/2026 10:45 PM, Leo Yan wrote:
> On Mon, May 11, 2026 at 05:27:10PM +0800, Jie Gan wrote:
> 
> [...]
> 
>>>> @@ -755,10 +755,16 @@ void coresight_path_assign_trace_id(struct coresight_path *path,
>>>>    		 * Non 0 is either success or fail.
>>>>    		 */
>>>>    		if (trace_id != 0) {
>>>> -			path->trace_id = trace_id;
>>>> -			return;
>>>> +			if (IS_VALID_CS_TRACE_ID(trace_id)) {
>>>> +				path->trace_id = trace_id;
>>>> +				return 0;
>>>> +			}
>>>> +
>>>> +			return -EINVAL;
> 
> I'd advocate a bit early exit style, like:
> 
>    /* 0 means the device has no ID assignment, so keep searching */
>    if (trace_id == 0)
>        continue;
> 
>    if (!IS_VALID_CS_TRACE_ID(trace_id))
>        return -EINVAL;
> 
>    path->trace_id = trace_id;
>    return 0;
> 
> Early exit can reduce indentation depth, and it handles simple cases
> first and then the complex logic. In some cases (maye not this case),
> we may benefit a bit from compiler optimization [1].
> 

Thanks, that's a good suggestion and much simpler than my solution.

> [1] https://xania.org/202512/18-partial-inlining
> 
> [...]
> 
>> The return value has been ignored in perf mode. It will introduce noisy by
>> adding __must_check. So I think its better without __must_check?
> 
> Wouldn't it need to update perf mode as well?

I will also update the perf mode for consistent usage.

Thanks,
Jie

> 
> Regarding __must_check, I searched Documentation but didn't find
> guidance on when it should be used. I don't want to use this annotation
> randomly (some functions use it and some not), this will be hard for
> everyone to follow up.
> 
> IMO, it's fine not to use __must_check here. I would leave this to
> Suzuki and other maintainers if have different opinions.
> 
> Thanks,
> Leo