Re: [PATCH v19 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 parameters are hardcoded via Kconfig options rather than dynamic configuration mechanisms.
- [Low] Redundant 'default n' and inconsistent indentation in help text.
- [Low] Missing 'depends on HAS_IOMEM' for an MMIO-based driver.
--

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

crypto: spacc - Add SPAcc Kconfig and Makefile
    
This commit introduces the Makefile and Kconfig definitions for the new
SPAcc (Security Protocol Accelerator) crypto driver.

> diff --git a/drivers/crypto/dwc-spacc/Kconfig b/drivers/crypto/dwc-spacc/Kconfig
> new file mode 100644
> index 0000000000000..b528c8e47b3b5
> --- /dev/null
> +++ b/drivers/crypto/dwc-spacc/Kconfig
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +config CRYPTO_DEV_SPACC
> +	tristate "Support for dwc_spacc Security Protocol Accelerator"
> +	depends on HAS_DMA

[Severity: Low]
Does this driver need a dependency on HAS_IOMEM? The Kconfig option
CRYPTO_DEV_SPACC_DEBUG_TRACE_IO notes tracing of MMIO, indicating the driver
uses memory-mapped I/O. Without this dependency, randconfig builds might fail
on architectures that lack MMIO support.

> +	select CRYPTO_ENGINE
> +	select CRYPTO_HASH if CRYPTO_DEV_SPACC_HASH

[ ... ]

> +config CRYPTO_DEV_SPACC_PRIORITY
> +	int "VSPACC priority value"
> +	range 0 15
> +	default 1

[Severity: Medium]
Would it be better to configure hardware tuning parameters like arbitration
priority and internal counter timeouts dynamically (e.g., via module
parameters or device tree)? Defining them in Kconfig bakes the values into
the kernel image at compile time, which prevents configuring different
SPAcc instances with varying parameters on a multi-platform kernel.

> +	help
> +	  Default arbitration priority weight for this Virtual SPAcc instance.
> +	  Hardware resets this to 1. Higher values means higher priority.

[ ... ]

> +config CRYPTO_DEV_SPACC_CONFIG_DEBUG
> +	bool "Enable SPAcc debug logs"
> +	default n
> +	help
> +          Say y to enable additional debug prints and diagnostics in the

[Severity: Low]
This isn't a bug, but should the 10-space indentation on this line be
replaced with a standard tab and two spaces to match Kconfig styling
guidelines? Also, the 'default n' on the preceding line is redundant
since Kconfig options default to 'n' if no other default is specified.

> +	  SPAcc driver. Disable this for production builds.
> +
> +endif # CRYPTO_DEV_SPACC

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