Re: [PATCH net-next 03/10] dpll: add domain object

[Ivan Vecera]

On 11/10/26 18:53, netdev-bot+sashiko@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
>
> Low priority / nit picks for your consideration:
>
> - [Low] The comment on `lock` in the new struct dpll_domain
>    (drivers/dpll/dpll_core.h) says it "Serializes the driver callbacks of…
>
> --
>
> dpll: add domain object
>
> This commit adds struct dpll_domain, which groups all dpll devices and
> pins created with the same (module, clock_id) pair. Domains are
> refcounted, stored in an xarray and freed along with their last object.
> It prepares for serializing driver callbacks per driver instance instead
> of under the global dpll_lock, and states no functional change.
>
> > diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
> > index da8a369556ed1..6705e81bc11de 100644
> > --- a/drivers/dpll/dpll_core.h
> > +++ b/drivers/dpll/dpll_core.h
>
> [ ... ]
>
> > +struct dpll_domain {
> > +	u32 id;
> > +	u64 clock_id;
> > +	struct module *module;
> > +	refcount_t refcount;
> > +	/* Serializes the driver callbacks of all DPLL devices and pins
> > +	 * that share one (module, clock_id) pair.
> > +	 */
> > +	struct mutex lock;
>
> [Severity: Low]
> This isn't a bug, but is this comment accurate at this point in the
> series?

[...]
> Could the comment say the lock will serialize the callbacks, or could it
> be added in the patch that starts taking the lock?
Right, nothing takes it before patch 4. In v2 the lock will be added by the
patch that starts taking it.

Thanks,
Ivan