Re: [Buildroot] [PATCH] package: new: add k3conf tool package

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello,

Thanks a lot for your patch! Please find below a review of your patch.

First the commit title should be:

	package/k3conf: new package

On Mon, 15 Dec 2025 18:54:25 +0530
Sadik SM <s-sadik@ti.com> wrote:

> From: sadik <s-sadik@ti.com>

Please use your full first name / last name, with proper capitalization.

> add the k3conf tool in the package section

This sentence is not needed.

> 
> K3CONF is a Linux user-space standalone application
> designed to provide a quick'n easy way to dynamically
> diagnose Texas Instruments' K3 architecture based
> processors. K3CONF is intended to provide similar
> experience to that of OMAPCONF that runs on legacy TI platforms.
> 
> K3CONF currently supports Texas Instruments AM654, J721E, J7200, 
> AM64x,AM62x,J721S2,J784S4,J722S, AM62Ax, AM62Px, and AM62Lx devices.
> Along with the BeagleBoard variants of the above mentioned TI SOC's.
> 
> Signed-off-by: sadik <s-sadik@ti.com>

Please use your full first name / last name, with proper capitalization.

> diff --git a/DEVELOPERS b/DEVELOPERS
> index 1b27df9beb..396452f261 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -2962,6 +2962,9 @@ F:	package/agent-proxy/
>  N:	Ryan Wilkins <ryan@deadfrog.net>
>  F:	package/biosdevname/
>  
> +N:  Sadik SM <s-sadik@ti.com>
> +F:  package/k3conf

Please use the same indentation as in the rest of the document. Put a
final / in package/k3conf/, like for other entries.

Use your full first name / last name.


> diff --git a/package/k3conf/Config.in b/package/k3conf/Config.in
> new file mode 100644
> index 0000000000..f282ad672b
> --- /dev/null
> +++ b/package/k3conf/Config.in
> @@ -0,0 +1,4 @@
> +config BR2_PACKAGE_K3CONF
> +    bool "k3conf"
> +    help
> +      A Powerful Diagnostic Tool for Texas Instruments K3 based Processors.

Please find the indentation to match other Config.in files: one tab for
bool/help, one tab + two spaces for the help text.

Add the upstream URL of the project at the end of the help text.

You can run "make check-package" to check for coding style issues.

I think you can add a "depends on BR2_aarch64" dependency to this
package since anyway it doesn't make sense for other CPU architectures.


> diff --git a/package/k3conf/k3conf.hash b/package/k3conf/k3conf.hash
> new file mode 100644
> index 0000000000..47d9eca6a0
> --- /dev/null
> +++ b/package/k3conf/k3conf.hash
> @@ -0,0 +1,2 @@
> +# Locally calculated sha256sum
> +sha256  a3459f80e7af5672da6b41bc0814114690dd549aac4cf81ba4e5dc84d42ab9c6  k3conf-v0.4-git4.tar.gz

Please add a hash for the license file.

> diff --git a/package/k3conf/k3conf.mk b/package/k3conf/k3conf.mk
> new file mode 100644
> index 0000000000..b248fb63d5
> --- /dev/null
> +++ b/package/k3conf/k3conf.mk
> @@ -0,0 +1,16 @@
> +################################################################################
> +#
> +# k3conf
> +#
> +################################################################################
> +
> +K3CONF_VERSION = v0.4
> +K3CONF_SITE = https://git.ti.com/git/k3conf/k3conf.git
> +K3CONF_SITE_METHOD = git
> +K3CONF_LICENSE = BSD-3-Clause
> +K3CONF_LICENSE_FILES = LICENSE
> +
> +

Only one blank line.

> +K3CONF_CONF_OPTS = -DCMAKE_BUILD_TYPE=Release

This is not needed, it's already passed by the cmake-package
infrastructure.

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot