Re: [PATCH] accel/amdxdna: Return error when setting clock failed for npu1

[Mario Limonciello]

On 1/8/2025 17:41, Lizhi Hou wrote:
> Due to miss returning error when setting clock, the smatch static
> checker reports warning:
>    drivers/accel/amdxdna/aie2_smu.c:68 npu1_set_dpm()
>    error: uninitialized symbol 'freq'.
>
> Fixes: f4d7b8a6bc8c ("accel/amdxdna: Enhance power management settings")
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Closes: https://lore.kernel.org/dri-devel/202267d0-882e-4593-b58d-be9274592f9b@stanley.mountain/
> Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxx>
> ---
>   drivers/accel/amdxdna/aie2_smu.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_smu.c b/drivers/accel/amdxdna/aie2_smu.c
> index 73388443c676..e68aaf7cd9f2 100644
> --- a/drivers/accel/amdxdna/aie2_smu.c
> +++ b/drivers/accel/amdxdna/aie2_smu.c
> @@ -1,6 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0
>   /*
> - * Copyright (C) 2022-2024, Advanced Micro Devices, Inc.
> + * Copyright (C) 2022-2025, Advanced Micro Devices, Inc.

Can you please refrain from updating the copyright in unrelated patches. 
 This should be it's own patch.  And that being said, why even have 
years in this copyright?  It's just one more thing to update all the 
time.  Maybe just drop the year entirely?

After removing that from this patch, the remaining change is obviously 
correct so you can include this for v2.

Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx>

>    */
>   
>   #include <drm/drm_device.h>
> @@ -64,6 +64,7 @@ int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level)
>   	if (ret) {
>   		XDNA_ERR(ndev->xdna, "Set npu clock to %d failed, ret %d\n",
>   			 ndev->priv->dpm_clk_tbl[dpm_level].npuclk, ret);
> +		return ret;
>   	}
>   	ndev->npuclk_freq = freq;
>   
> @@ -72,6 +73,7 @@ int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level)
>   	if (ret) {
>   		XDNA_ERR(ndev->xdna, "Set h clock to %d failed, ret %d\n",
>   			 ndev->priv->dpm_clk_tbl[dpm_level].hclk, ret);
> +		return ret;
>   	}
>   	ndev->hclk_freq = freq;
>   	ndev->dpm_level = dpm_level;