On 2023/6/6 16:27, Dan Carpenter wrote:
> Hello Walker Chen,
>
> The patch fd4762b6b5cf: "ASoC: starfive: Add JH7110 TDM driver" from
> May 26, 2023, leads to the following Smatch static checker warning:
>
> sound/soc/starfive/jh7110_tdm.c:584 jh7110_tdm_clk_reset_get()
> warn: passing zero to 'PTR_ERR'
>
> sound/soc/starfive/jh7110_tdm.c
> 564 static int jh7110_tdm_clk_reset_get(struct platform_device *pdev,
> 565 struct jh7110_tdm_dev *tdm)
> 566 {
> 567 int ret;
> 568
> 569 tdm->clks[0].id = "mclk_inner";
> 570 tdm->clks[1].id = "tdm_ahb";
> 571 tdm->clks[2].id = "tdm_apb";
> 572 tdm->clks[3].id = "tdm_internal";
> 573 tdm->clks[4].id = "tdm_ext";
> 574 tdm->clks[5].id = "tdm";
> 575
> 576 ret = devm_clk_bulk_get(&pdev->dev, ARRAY_SIZE(tdm->clks), tdm->clks);
> 577 if (ret) {
> 578 dev_err(&pdev->dev, "Failed to get tdm clocks\n");
> 579 return ret;
> 580 }
> 581
> 582 tdm->resets = devm_reset_control_array_get_exclusive(&pdev->dev);
>
> The devm_reset_control_array_get() function returns NULL if it's
> an optional request. But this is not optional.
>
> 583 if (IS_ERR_OR_NULL(tdm->resets)) {
>
> So that means this should be an if (IS_ERR()) check.
>
> --> 584 ret = PTR_ERR(tdm->resets);
> 585 dev_err(&pdev->dev, "Failed to get tdm resets");
>
> Or if optional was intended then NULL should not be treated as an error
> case, but as a special kind of success case (no error message). See
> my blog for a long form of this information:
>
> https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/
>
Hi Dan,
Thanks for your testing.
Yes, should use IS_ERR() to check. I am preparing to submit a patch to fix this issue.
Best regards,
Walker
