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

Xuyang Dong
2026-04-24T10:45:10Z

> >
> > Add driver for the ESWIN EIC7700 high-speed peripherals system
> > clock controller and register an auxiliary device for system
> > reset controller which is named as "hsp-reset".
> >
> > Signed-off-by: Xuyang Dong <dongxuyang@eswincomputing.com>
>
> Reviewed-by: Brian Masney <bmasney@redhat.com>
>
> There's one minor bit I am not sure of.
>
> > +static void eic7700_hsp_regmap_lock(void *arg)
> > +__acquires(lock_ctx->lock)
> > +{
> > + struct eic7700_hsp_regmap_lock *const lock_ctx = arg;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(lock_ctx->lock, flags);
> > + lock_ctx->flags = flags;
> > +}
>
> I don't know if the __acquires() is accurate syntax. If it needs to
> be arg, lock_ctx->lock, or ((struct eic7700_hsp_regmap_lock *)arg)->lock.
>
> It looks like this code is triggered with clang, and I tried compiling
> this driver with:
>
> make LLVM=1 KCFLAGS="-ferror-limit=10000 -DWARN_CONTEXT_ANALYSIS -Wthread-safety" \
> drivers/clk/eswin/clk-eic7700-hsp.o
>
> I also tried with 'make C=2' and I don't see any locking messages from
> this driver.
>

Hi Brian,

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.

Best regards,
Xuyang Dong