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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] A `bool` Kconfig option `CRYPTO_DEV_SPACC_HASH` selects `tristate` crypto algorithms, which forces them to be built-in (`=y`) even when the main driver is compiled as a module (`=m`).
- [Medium] Hardware-specific tuning parameters (arbitration priority and watchdog counter) are hardcoded in Kconfig.
--

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

crypto: spacc - Add SPAcc Kconfig and Makefile

This commit introduces the Kconfig and Makefile configurations for the SPAcc
driver. It adds options for driver inclusion, hashing, debug traces, and
hardware-specific tunables.

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

[ ... ]

> +if CRYPTO_DEV_SPACC
> +
> +config CRYPTO_DEV_SPACC_HASH
> +	bool "Enable HASH functionality"
> +	default CRYPTO_DEV_SPACC
> +	select CRYPTO_HASH
> +	select CRYPTO_SHA1
> +	select CRYPTO_MD5
> +	select CRYPTO_SHA256
> +	select CRYPTO_SHA512
> +	select CRYPTO_HMAC
> +	select CRYPTO_CMAC
> +	select CRYPTO_XCBC
> +	select CRYPTO_AES
> +	select CRYPTO_SM4_GENERIC

[Severity: Medium]
Since CRYPTO_DEV_SPACC_HASH is a boolean option, if the main driver is
compiled as a module, will this select statement force all these tristate
algorithms to be built-in rather than modules?

It looks like this might cause unnecessary vmlinux bloat when
CRYPTO_DEV_SPACC is set to m.

> +
> +	help
> +	  Say y to enable Hash functionality of SPAcc.

[ ... ]

> +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]
Does hardcoding these hardware tuning parameters in Kconfig break the ability
to support a multi-platform kernel?

For systems where a single kernel image boots on multiple boards, different
SoC instances might require different tuning values.

Would it be better to provide these via Device Tree properties or module
parameters instead?

[ ... ]

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