Re: [PATCH] spi: sprd-adi: Fix probe succeeding without registering the controller

On 7/26/26 3:22 PM, Babanpreet Singh wrote:
> With CONFIG_HWSPINLOCK=n the of_hwspin_lock_get_id() stub returns 0
> unconditionally. In sprd_adi_probe() the guard
> 
> 	if (ret > 0 || (IS_ENABLED(CONFIG_HWSPINLOCK) && ret == 0))
> 
> is false for that 0, so it takes the else branch, where the switch has no
> case for 0 and lands in
> 
> 	default:
> 		return dev_err_probe(&pdev->dev, ret, "failed to find hwlock id\n");
> 
> dev_err_probe() returns its err argument unchanged, so probe logs
> "failed to find hwlock id" and then returns 0, reporting success.
> sprd_adi_hw_init(), the restart handler and devm_spi_register_controller()
> are all skipped: the device binds but no SPI controller is ever
> registered. Since the stub is a constant-returning static inline, the
> compiler folds the whole remainder of probe away as dead code - an
> object built in that configuration contains no reference to
> devm_spi_register_controller() at all.
> 
> The hardware spinlock is optional for this controller and the -ENOENT arm
> already covers "no hardware spinlock supplied". Treat the stub's 0 the
> same way and continue without a lock; all four users of sadi->hwlock
> already test it for NULL.
> 
> This is not reachable on production kernels. Kconfig has
> 
> 	depends on HWSPINLOCK || (COMPILE_TEST && !HWSPINLOCK)
> 
> so the affected configuration exists only under COMPILE_TEST, where no
> real hardware is present. Object code for CONFIG_HWSPINLOCK=y builds is
> byte-identical before and after this change.
> 
> Found by smatch:
> drivers/spi/spi-sprd-adi.c:560 sprd_adi_probe() warn: passing zero to 'dev_err_probe'
> 
> Fixes: f9adf61e983f ("spi: sprd: adi: Change hwlock to be optional")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Babanpreet Singh <[email protected]>
> ---
>   drivers/spi/spi-sprd-adi.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
> index e7d83c16b46c..7c29115c5b8b 100644
> --- a/drivers/spi/spi-sprd-adi.c
> +++ b/drivers/spi/spi-sprd-adi.c
> @@ -553,6 +553,11 @@ static int sprd_adi_probe(struct platform_device *pdev)
>   			return -ENXIO;
>   	} else {
>   		switch (ret) {
> +		case 0:
> +			/*
> +			 * Only reachable with CONFIG_HWSPINLOCK=n, where the
> +			 * of_hwspin_lock_get_id() stub returns 0.
> +			 */

Please add a 'fallthrough' here. With that, you can add:

Reviewed-by: Baolin Wang <[email protected]>

>   		case -ENOENT:
>   			dev_info(&pdev->dev, "no hardware spinlock supplied\n");
>   			break;