Re: [PATCH RFC v2 15/15] tools/ctlv: Add ctlvctl

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] Documentation: ctlv: driver-api guide drops sections and cites
  missing tests
- [Low] tools/ctlv: uncleaned libctlv build artifacts in ctlv_clean
  target
--

commit 44c8b60bf07b7ca2b82f88cb4a596bb865f2b940
Author: Jiri Pirko <jiri@resnulli.us>

tools/ctlv: Add ctlvctl

This commit adds the ctlvctl command-line client for discovering and
querying CTLV devices, along with runtime manifest and RST documentation
generators in ctlvgen.

> diff --git a/Documentation/driver-api/ctlv.rst b/Documentation/driver-api/ctlv.rst
> index f6a4f9632e3a5..e8e6ecdf6ab66 100644
> --- a/Documentation/driver-api/ctlv.rst
> +++ b/Documentation/driver-api/ctlv.rst
[ ... ]
> @@ -684,20 +687,24 @@ The node is of the foo driver's own class, ``foo_class``, registered as
>  that ``ctlv_family_inherits()`` says is not ``ctlv-example`` or a descendant,
>  then calls ``ctlv_init()``.
>  
> -Reporting an error
> -------------------
> +Tests and examples
> +==================
> +
> +``Documentation/ctlv/specs/ctlv-test.yaml``
> +    Every schema kind, records, actions, queries, blobs, availability, hooks,
> +    and an extension point.
> +
> +``drivers/ctlv/tests/ctlv_test.c``
> +    The ``ctlv-test`` family implementation.
>  
> -A callback returns a negative errno, and that is what the ioctl returns. Where
> -CTLV itself refuses a message it also writes error information into the
> -caller's reply buffer, naming the reason and the offset of the offending
> -TLV. A family implementation does not build that; it adds its own errno
> -and its reply.
> +``drivers/ctlv/tests/ctlv_test_child.c``
> +    A separately built inheriting family and extension-point fill.
>  
> -Configuration
> -=============
> +``lib/ctlv/tests/*_kunit.c``
> +    Unit tests of the engine and of its nodes.
>  
> -``CONFIG_CTLV``
> -    CTLV. A family implementation selects it.
> +``tools/testing/selftests/ctlv/``
> +    Raw-message, generated-binding, allocation-failure, and mutation selftests.
>  
>  Kernel API
>  ==========

[Severity: Low]
Were the "Reporting an error" and "Configuration" sections added in
commit "Documentation: ctlv: Describe how to write a family" dropped here
by mistake?  Also, none of the five test paths listed under "Tests and
examples" exist in the tree at this commit.

[ ... ]

> diff --git a/tools/Makefile b/tools/Makefile
> index cb40961a740f7..d8d0003b16a5f 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
[ ... ]
> @@ -179,6 +183,9 @@ acpi_clean:
>  cpupower_clean:
>  	$(call descend,power/cpupower,clean)
>  
> +ctlv_clean:
> +	$(call descend,ctlv/ctlvctl,clean)
> +
[ ... ]
> diff --git a/tools/ctlv/ctlvctl/Makefile b/tools/ctlv/ctlvctl/Makefile
> new file mode 100644
> index 0000000000000..dcfca390d4024
> --- /dev/null
> +++ b/tools/ctlv/ctlvctl/Makefile
[ ... ]
> +$(LIB): FORCE
> +	$(if $(LIBCTLV_OUTPUT),$(Q)mkdir -p $(LIBCTLV_OUTPUT))
> +	$(Q)$(MAKE) -C $(LIBDIR) OUTPUT=$(LIBCTLV_OUTPUT)
> +
> +clean:
> +	rm -f $(OUTPUT)ctlvctl $(OBJS) $(MANIFESTS)
> +	$(if $(LIBCTLV_OUTPUT),rm -rf $(LIBCTLV_OUTPUT))

[Severity: Low]
Does the clean target leave libctlv build artifacts behind when building in
tree without O=?  When OUTPUT and LIBCTLV_OUTPUT are empty, $(LIB) builds
objects and libctlv.a inside tools/ctlv/libctlv/, but neither ctlv_clean in
tools/Makefile nor clean in tools/ctlv/ctlvctl/Makefile cleans $(LIBDIR).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113155515.2510194-1-jiri@resnulli.us?part=15