[bug report] accel/amdxdna: Enhance power management settings

[bug report] accel/amdxdna: Enhance power management settings

[Dan Carpenter]

Hello Lizhi Hou,

Commit f4d7b8a6bc8c ("accel/amdxdna: Enhance power management
settings") from Dec 13, 2024 (linux-next), leads to the following
Smatch static checker warning:

	drivers/accel/amdxdna/aie2_smu.c:68 npu1_set_dpm()
	error: uninitialized symbol 'freq'.

drivers/accel/amdxdna/aie2_smu.c
    57 int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level)
    58 {
    59         u32 freq;
    60         int ret;
    61 
    62         ret = aie2_smu_exec(ndev, AIE2_SMU_SET_MPNPUCLK_FREQ,
    63                             ndev->priv->dpm_clk_tbl[dpm_level].npuclk, &freq);
    64         if (ret) {
    65                 XDNA_ERR(ndev->xdna, "Set npu clock to %d failed, ret %d\n",
    66                          ndev->priv->dpm_clk_tbl[dpm_level].npuclk, ret);

return ret; ?

    67         }
--> 68         ndev->npuclk_freq = freq;
    69 
    70         ret = aie2_smu_exec(ndev, AIE2_SMU_SET_HCLK_FREQ,
    71                             ndev->priv->dpm_clk_tbl[dpm_level].hclk, &freq);
    72         if (ret) {
    73                 XDNA_ERR(ndev->xdna, "Set h clock to %d failed, ret %d\n",
    74                          ndev->priv->dpm_clk_tbl[dpm_level].hclk, ret);

Here too?

    75         }
    76         ndev->hclk_freq = freq;
    77         ndev->dpm_level = dpm_level;
    78 
    79         XDNA_DBG(ndev->xdna, "MP-NPU clock %d, H clock %d\n",
    80                  ndev->npuclk_freq, ndev->hclk_freq);
    81 
    82         return 0;
    83 }

regards,
dan carpenter

Re: [bug report] accel/amdxdna: Enhance power management settings

[Lizhi Hou]

On 1/8/25 00:40, Dan Carpenter wrote:
> Hello Lizhi Hou,
>
> Commit f4d7b8a6bc8c ("accel/amdxdna: Enhance power management
> settings") from Dec 13, 2024 (linux-next), leads to the following
> Smatch static checker warning:
>
> 	drivers/accel/amdxdna/aie2_smu.c:68 npu1_set_dpm()
> 	error: uninitialized symbol 'freq'.
>
> drivers/accel/amdxdna/aie2_smu.c

Thanks for pointing this out. I will fix it.

I actually did run smatch_scripts/kchecker and it is clean.

Maybe I did not use the correct command. Could you share the instruction 
to reproduce this?


Thanks,

Lizhi

>      57 int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level)
>      58 {
>      59         u32 freq;
>      60         int ret;
>      61
>      62         ret = aie2_smu_exec(ndev, AIE2_SMU_SET_MPNPUCLK_FREQ,
>      63                             ndev->priv->dpm_clk_tbl[dpm_level].npuclk, &freq);
>      64         if (ret) {
>      65                 XDNA_ERR(ndev->xdna, "Set npu clock to %d failed, ret %d\n",
>      66                          ndev->priv->dpm_clk_tbl[dpm_level].npuclk, ret);
>
> return ret; ?
>
>      67         }
> --> 68         ndev->npuclk_freq = freq;
>      69
>      70         ret = aie2_smu_exec(ndev, AIE2_SMU_SET_HCLK_FREQ,
>      71                             ndev->priv->dpm_clk_tbl[dpm_level].hclk, &freq);
>      72         if (ret) {
>      73                 XDNA_ERR(ndev->xdna, "Set h clock to %d failed, ret %d\n",
>      74                          ndev->priv->dpm_clk_tbl[dpm_level].hclk, ret);
>
> Here too?
>
>      75         }
>      76         ndev->hclk_freq = freq;
>      77         ndev->dpm_level = dpm_level;
>      78
>      79         XDNA_DBG(ndev->xdna, "MP-NPU clock %d, H clock %d\n",
>      80                  ndev->npuclk_freq, ndev->hclk_freq);
>      81
>      82         return 0;
>      83 }
>
> regards,
> dan carpenter

Re: [bug report] accel/amdxdna: Enhance power management settings

[Dan Carpenter]

On Wed, Jan 08, 2025 at 10:14:47AM -0800, Lizhi Hou wrote:
> 
> On 1/8/25 00:40, Dan Carpenter wrote:
> > Hello Lizhi Hou,
> > 
> > Commit f4d7b8a6bc8c ("accel/amdxdna: Enhance power management
> > settings") from Dec 13, 2024 (linux-next), leads to the following
> > Smatch static checker warning:
> > 
> > 	drivers/accel/amdxdna/aie2_smu.c:68 npu1_set_dpm()
> > 	error: uninitialized symbol 'freq'.
> > 
> > drivers/accel/amdxdna/aie2_smu.c
> 
> Thanks for pointing this out. I will fix it.
> 
> I actually did run smatch_scripts/kchecker and it is clean.
> 
> Maybe I did not use the correct command. Could you share the instruction to
> reproduce this?
> 

This requires the cross function database which is simple to build but
takes hours. ~/smatch/smatch_scripts/build_kernel_data.sh.  Each time you
build the cross function database it adds a level to the call tree.
Having a one level call tree is slightly annoying because it give Smatch
a false sense of confidence.  I rebuild my database every day.  Probably
seven times is where it stops adding data.

regards,
dan carpenter