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

[Xuyang Dong]

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

Hi Stephen,

The common gate API, the HSP private API, and the reset driver all access 
the same register space.
Therefore, they need to be protected by the same data->lock.

Best regards,
Xuyang Dong