On Sat, Mar 11, 2023 at 02:32:04PM +0800, David Gow wrote: > > > > diff --git a/drivers/clk/clk-kunit.c b/drivers/clk/clk-kunit.c > > > > new file mode 100644 > > > > index 000000000000..78d85b3a7a4a > > > > --- /dev/null > > > > +++ b/drivers/clk/clk-kunit.c > > > > @@ -0,0 +1,204 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +/* > > > > + * KUnit helpers for clk tests > > > > + */ > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > + > > > > +#include > > > > + > > > > +#include "clk-kunit.h" > > > > + > > > > +static void kunit_clk_disable_unprepare(struct kunit_resource *res) > > > > > > We need to decide on the naming scheme of these, and in particular if > > > they should be kunit_clk or clk_kunit (or something else). > > > > > > I'd lean to clk_kunit, if only to match DRM's KUnit helpers being > > > drm_kunit_helper better, and so that these are more tightly bound to > > > the subsystem being tested. > > > (i.e., so I don't have to scroll through every subsystem's helpers > > > when autocompleting kunit_). > > > > Ok, got it. I was trying to match kunit_kzalloc() style. It makes it > > easy to slap the 'kunit_' prefix on existing auto-completed function > > names like kzalloc() or clk_prepare_enable(). > > Yeah: my rule of thumb at the moment is to keep the kunit_ prefix for > things which are generic across the whole kernel (and tend to be > implemented in lib/kunit), and to use suffixes or infixes (whichever > works best) for things which are subsystem-specific. A suffix is kind of weird though when any other managed call is using a prefix: devm is always using a prefix including for clocks, kunit for some calls too (like kzalloc). Having clk_get vs devm_clk_get vs clk_get_kunit would be very inconsistent and throws me off completely :) Maxime