Re: [PATCH v16 4/4] crypto: spacc - Add SPAcc Kconfig and Makefile

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Hardware tuning and instance configuration parameters are hardcoded as compile-time Kconfig options.
- [Medium] Boolean Kconfig option `CRYPTO_DEV_SPACC_HASH` unconditionally selects tristate algorithms, causing kernel bloat.
- [Low] Unused dependency on `CRYPTO_SM3` selected by `CRYPTO_DEV_SPACC_HASH`.
--

--- Patch [4]: [PATCH v16 4/4] crypto: spacc - Add SPAcc Kconfig and Makefile ---
commit cd06e4262110289b96679278cd22ee6ebd1683fa
Author: Pavitrakumar Managutte <pavitrakumarm@vayavyalabs.com>

crypto: spacc - Add SPAcc Kconfig and Makefile

Add Makefile and Kconfig for SPAcc driver.

> diff --git a/drivers/crypto/dwc-spacc/Kconfig b/drivers/crypto/dwc-spacc/Kconfig
> new file mode 100644
> index 0000000000000..a9172606d77aa
> --- /dev/null
> +++ b/drivers/crypto/dwc-spacc/Kconfig

[ ... ]

> +if CRYPTO_DEV_SPACC
> +
> +config CRYPTO_DEV_SPACC_HASH
> +	bool "Enable HASH functionality"
> +	default y
> +	select CRYPTO_HASH
> +	select CRYPTO_SHA1
> +	select CRYPTO_MD5
> +	select CRYPTO_SHA256
> +	select CRYPTO_SHA512
> +	select CRYPTO_HMAC
> +	select CRYPTO_SM3

[Severity: Low]
Does this driver actually require CRYPTO_SM3? The spacc_ahash.c frontend does
not appear to include sm3 or hmac(sm3) in its supported hashes, nor does it
seem to use the algorithm for software fallbacks during key preparation.

> +	select CRYPTO_CMAC
> +	select CRYPTO_XCBC
> +	select CRYPTO_AES
> +	select CRYPTO_SM4_GENERIC

[Severity: Medium]
Since CRYPTO_DEV_SPACC_HASH is a bool, if CRYPTO_DEV_SPACC is built as a
module (=m), won't these selects evaluate to =y? This would force all these
core crypto algorithms to be statically compiled into the vmlinux image,
which might unnecessarily bloat the kernel size for users expecting them
to remain as modules.

[ ... ]

> +config CRYPTO_DEV_SPACC_PRIORITY
> +	int "VSPACC priority value"
> +	range 0 15
> +	default 1
> +	help
> +	  Default arbitration priority weight for this Virtual SPAcc instance.
> +	  Hardware resets this to 1. Higher values means higher priority.
> +
> +config CRYPTO_DEV_SPACC_INTERNAL_COUNTER
> +	int "SPAcc internal counter value"
> +	range 100000 1048575
> +	default 100000
> +	help
> +	  This value configures a hardware watchdog counter in the SPAcc engine.
> +	  The counter starts ticking when a completed cryptographic job is
> +	  sitting in the STATUS FIFO. If the job remains unprocessed for the
> +	  configured duration, an interrupt is triggered to ensure it is serviced.

[Severity: Medium]
Are these hardware tuning parameters appropriate for compile-time Kconfig
options?

In a multiplatform kernel, statically defining the virtual instance arbitration
priority and watchdog timeout forces all devices using this kernel binary to
share the exact same configuration.

Could these parameters be passed dynamically at runtime via Device Tree,
ACPI, or module parameters to support per-board flexibility?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707125311.2398031-1-pavitrakumarm@vayavyalabs.com?part=4