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

[Stephen Boyd]

Quoting Brian Masney (2026-04-24 04:15:46)
> 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.
> 

Why not use a regmap instead? That would enforce locking on registers
and then you use the right regmap APIs to update the register under the
lock (like regmap_update_bits() or something).