Re: [PATCH v11 1/4] crypto: qce - convert to auxiliary bus

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] crypto: qce: ignored .driver.name initializer in
  qce_crypto_driver
- [Low] crypto: qce - stale ICC comment in qce_crypto_probe()
--

commit 29e94f7689b20cef79914c127c10bb58d32472f1
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

crypto: qce - convert to auxiliary bus

This commit converts the Qualcomm crypto engine driver from a platform
driver to an auxiliary bus driver, removes interconnect bandwidth scaling,
updates runtime PM handling, and uses the parent device for DMA mappings.

> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index ddb4df3d227f8..346e9ea0fe0f0 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c

[ ... ]

> @@ -228,10 +243,6 @@ static int qce_crypto_probe(struct platform_device *pdev)
>  	if (IS_ERR(qce->bus))
>  		return PTR_ERR(qce->bus);
>  
> -	qce->mem_path = devm_of_icc_get(dev, "memory");
> -	if (IS_ERR(qce->mem_path))
> -		return PTR_ERR(qce->mem_path);
> -
>  	/*
>  	 * Enable runtime PM after clocks and ICC path are acquired so that
>  	 * the resume callback can enable clocks and apply the ICC bandwidth

[Severity: Low]
This isn't a bug, but this comment in qce_crypto_probe() still mentions
acquiring the ICC path and applying the ICC bandwidth vote, which this
commit removes from the driver.

[ ... ]

> @@ -336,25 +330,22 @@ static const struct dev_pm_ops qce_crypto_pm_ops = {
>  	SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
>  };
>  
> -static const struct of_device_id qce_crypto_of_match[] = {
> -	{ .compatible = "qcom,crypto-v5.1", },
> -	{ .compatible = "qcom,crypto-v5.4", },
> -	{ .compatible = "qcom,qce", },
> +static const struct auxiliary_device_id qce_crypto_ids[] = {
> +	{ .name = "qce.crypto", },
>  	{}
>  };
> -MODULE_DEVICE_TABLE(of, qce_crypto_of_match);
> +MODULE_DEVICE_TABLE(auxiliary, qce_crypto_ids);
>  
> -static struct platform_driver qce_crypto_driver = {
> +static struct auxiliary_driver qce_crypto_driver = {
>  	.probe = qce_crypto_probe,
> +	.id_table = qce_crypto_ids,
>  	.driver = {
> -		.name = KBUILD_MODNAME,
> -		.of_match_table = qce_crypto_of_match,
> +		.name = "qce-crypto",
>  		.pm = pm_ptr(&qce_crypto_pm_ops),
>  	},
>  };

[Severity: Low]
Should qce_crypto_driver set .name instead of .driver.name, or drop
.driver.name?  __auxiliary_driver_register() unconditionally overwrites
auxdrv->driver.name based on auxdrv->name and the module name:

drivers/base/auxiliary.c:__auxiliary_driver_register() {
	...
	if (auxdrv->name)
		auxdrv->driver.name = kasprintf(GFP_KERNEL, "%s.%s", modname,
						auxdrv->name);
	else
		auxdrv->driver.name = kasprintf(GFP_KERNEL, "%s", modname);
	...
}

Because qce_crypto_driver.name is NULL, the "qce-crypto" initializer is
ignored and overwritten with KBUILD_MODNAME ("qcrypto").

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-qce-fix-self-tests-v11-0-b86a65ee7107@oss.qualcomm.com?part=1