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

From: Leo Yan

Date: Mon May 11 2026 - 10:48:00 EST


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].

[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?

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