Re: Re: [PATCH v3 2/3] clk: eswin: Add eic7700 HSP clock driver

[Brian Masney]

On Fri, Apr 24, 2026 at 6:45 AM Xuyang Dong
<dongxuyang@xxxxxxxxxxxxxxxxxx> wrote:
> Thanks for the feedback. I did some research based on your comments.
>
> lock_ctx is a local variable declared inside the function body. It is not
> in scope at the attribute site. The attribute expands to
> __attribute__((acquire_capability(lock_ctx->lock))), and since lock_ctx
> doesn't exist at the declaration point, clang's analysis cannot resolve it
> and silently drops the annotation. That's why you see no warnings from
> make C=2 or -Wthread-safety.
>
> Why -Wthread-safety produces no output
> Two reasons:
>   1. The lock_ctx->lock expression is unresolvable at the attribute site,
>   so clang drops the annotation silently — no acquire/release tracking,
>   no warnings.
>   2. Even if the expression were resolvable, spinlock_t in this driver is
>   a plain pointer field (spinlock_t *lock) accessed through a void *
>   callback — the analysis can't track lock state through that indirection.
>
> The closest correct expression would be:
> __acquires(((struct eic7700_hsp_regmap_lock *)arg)->lock)
> But that also won't work: arg is void *, and clang's thread-safety
> analysis is type-based. It can't trace through a void pointer cast to
> determine which spinlock_t instance is being acquired. The analysis
> would still silently ignore it.
>
> For void * regmap callbacks, there is no clean way to make __acquires()
> work, because the lock is always hidden behind the opaque pointer.
> The annotations should be dropped.
>
> Based on the above analysis, I suggest removing the annotations entirely.
> However, I'd like to hear your thoughts on this approach.

I agree to remove the annotations. Before you post a new version,
let's let this series sit out on the list for a week or two, and see
if anyone else replies with the proper way to do this.

Brian