2021-07-30 13:50:00

by Dan Carpenter

[permalink] [raw]
Subject: [kbuild] drivers/pinctrl/pinctrl-k210.c:970 k210_fpioa_probe() warn: 'pdata->clk' not released on lines: 962,968.

tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 764a5bc89b12b82c18ce7ca5d7c1b10dd748a440
commit: d4c34d09ab03e1e631fe195ddf35365a1273be9c pinctrl: Add RISC-V Canaan Kendryte K210 FPIOA driver
config: riscv-randconfig-m031-20210730 (attached as .config)
compiler: riscv64-linux-gcc (GCC) 10.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>

smatch warnings:
drivers/pinctrl/pinctrl-k210.c:970 k210_fpioa_probe() warn: 'pdata->clk' not released on lines: 962,968.

vim +970 drivers/pinctrl/pinctrl-k210.c

d4c34d09ab03e1 Damien Le Moal 2021-01-12 925 static int k210_fpioa_probe(struct platform_device *pdev)
d4c34d09ab03e1 Damien Le Moal 2021-01-12 926 {
d4c34d09ab03e1 Damien Le Moal 2021-01-12 927 struct device *dev = &pdev->dev;
d4c34d09ab03e1 Damien Le Moal 2021-01-12 928 struct device_node *np = dev->of_node;
d4c34d09ab03e1 Damien Le Moal 2021-01-12 929 struct k210_fpioa_data *pdata;
d4c34d09ab03e1 Damien Le Moal 2021-01-12 930 int ret;
d4c34d09ab03e1 Damien Le Moal 2021-01-12 931
d4c34d09ab03e1 Damien Le Moal 2021-01-12 932 dev_info(dev, "K210 FPIOA pin controller\n");
d4c34d09ab03e1 Damien Le Moal 2021-01-12 933
d4c34d09ab03e1 Damien Le Moal 2021-01-12 934 pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
d4c34d09ab03e1 Damien Le Moal 2021-01-12 935 if (!pdata)
d4c34d09ab03e1 Damien Le Moal 2021-01-12 936 return -ENOMEM;
d4c34d09ab03e1 Damien Le Moal 2021-01-12 937
d4c34d09ab03e1 Damien Le Moal 2021-01-12 938 pdata->dev = dev;
d4c34d09ab03e1 Damien Le Moal 2021-01-12 939 platform_set_drvdata(pdev, pdata);
d4c34d09ab03e1 Damien Le Moal 2021-01-12 940
d4c34d09ab03e1 Damien Le Moal 2021-01-12 941 pdata->fpioa = devm_platform_ioremap_resource(pdev, 0);
d4c34d09ab03e1 Damien Le Moal 2021-01-12 942 if (IS_ERR(pdata->fpioa))
d4c34d09ab03e1 Damien Le Moal 2021-01-12 943 return PTR_ERR(pdata->fpioa);
d4c34d09ab03e1 Damien Le Moal 2021-01-12 944
d4c34d09ab03e1 Damien Le Moal 2021-01-12 945 pdata->clk = devm_clk_get(dev, "ref");
d4c34d09ab03e1 Damien Le Moal 2021-01-12 946 if (IS_ERR(pdata->clk))
d4c34d09ab03e1 Damien Le Moal 2021-01-12 947 return PTR_ERR(pdata->clk);
d4c34d09ab03e1 Damien Le Moal 2021-01-12 948
d4c34d09ab03e1 Damien Le Moal 2021-01-12 949 ret = clk_prepare_enable(pdata->clk);
^^^^^^^^^^^^^^^^^^


d4c34d09ab03e1 Damien Le Moal 2021-01-12 950 if (ret)
d4c34d09ab03e1 Damien Le Moal 2021-01-12 951 return ret;
d4c34d09ab03e1 Damien Le Moal 2021-01-12 952
d4c34d09ab03e1 Damien Le Moal 2021-01-12 953 pdata->pclk = devm_clk_get_optional(dev, "pclk");
d4c34d09ab03e1 Damien Le Moal 2021-01-12 954 if (!IS_ERR(pdata->pclk))
d4c34d09ab03e1 Damien Le Moal 2021-01-12 955 clk_prepare_enable(pdata->pclk);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
No error handling

d4c34d09ab03e1 Damien Le Moal 2021-01-12 956
d4c34d09ab03e1 Damien Le Moal 2021-01-12 957 pdata->sysctl_map =
d4c34d09ab03e1 Damien Le Moal 2021-01-12 958 syscon_regmap_lookup_by_phandle_args(np,
d4c34d09ab03e1 Damien Le Moal 2021-01-12 959 "canaan,k210-sysctl-power",
d4c34d09ab03e1 Damien Le Moal 2021-01-12 960 1, &pdata->power_offset);
d4c34d09ab03e1 Damien Le Moal 2021-01-12 961 if (IS_ERR(pdata->sysctl_map))
d4c34d09ab03e1 Damien Le Moal 2021-01-12 962 return PTR_ERR(pdata->sysctl_map);

Do we need to clk_unprepare_disable() before returning?


d4c34d09ab03e1 Damien Le Moal 2021-01-12 963
d4c34d09ab03e1 Damien Le Moal 2021-01-12 964 k210_fpioa_init_ties(pdata);
d4c34d09ab03e1 Damien Le Moal 2021-01-12 965
d4c34d09ab03e1 Damien Le Moal 2021-01-12 966 pdata->pctl = pinctrl_register(&k210_pinctrl_desc, dev, (void *)pdata);
d4c34d09ab03e1 Damien Le Moal 2021-01-12 967 if (IS_ERR(pdata->pctl))
d4c34d09ab03e1 Damien Le Moal 2021-01-12 968 return PTR_ERR(pdata->pctl);

Here too.

d4c34d09ab03e1 Damien Le Moal 2021-01-12 969
d4c34d09ab03e1 Damien Le Moal 2021-01-12 @970 return 0;
d4c34d09ab03e1 Damien Le Moal 2021-01-12 971 }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

_______________________________________________
kbuild mailing list -- [email protected]
To unsubscribe send an email to [email protected]



2021-08-01 22:53:34

by Damien Le Moal

[permalink] [raw]
Subject: Re: [kbuild] drivers/pinctrl/pinctrl-k210.c:970 k210_fpioa_probe() warn: 'pdata->clk' not released on lines: 962,968.

On 2021/07/30 22:46, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 764a5bc89b12b82c18ce7ca5d7c1b10dd748a440
> commit: d4c34d09ab03e1e631fe195ddf35365a1273be9c pinctrl: Add RISC-V Canaan Kendryte K210 FPIOA driver
> config: riscv-randconfig-m031-20210730 (attached as .config)
> compiler: riscv64-linux-gcc (GCC) 10.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
>
> smatch warnings:
> drivers/pinctrl/pinctrl-k210.c:970 k210_fpioa_probe() warn: 'pdata->clk' not released on lines: 962,968.
>
> vim +970 drivers/pinctrl/pinctrl-k210.c
>
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 925 static int k210_fpioa_probe(struct platform_device *pdev)
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 926 {
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 927 struct device *dev = &pdev->dev;
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 928 struct device_node *np = dev->of_node;
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 929 struct k210_fpioa_data *pdata;
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 930 int ret;
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 931
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 932 dev_info(dev, "K210 FPIOA pin controller\n");
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 933
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 934 pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 935 if (!pdata)
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 936 return -ENOMEM;
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 937
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 938 pdata->dev = dev;
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 939 platform_set_drvdata(pdev, pdata);
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 940
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 941 pdata->fpioa = devm_platform_ioremap_resource(pdev, 0);
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 942 if (IS_ERR(pdata->fpioa))
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 943 return PTR_ERR(pdata->fpioa);
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 944
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 945 pdata->clk = devm_clk_get(dev, "ref");
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 946 if (IS_ERR(pdata->clk))
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 947 return PTR_ERR(pdata->clk);
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 948
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 949 ret = clk_prepare_enable(pdata->clk);
> ^^^^^^^^^^^^^^^^^^
>
>
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 950 if (ret)
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 951 return ret;
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 952
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 953 pdata->pclk = devm_clk_get_optional(dev, "pclk");
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 954 if (!IS_ERR(pdata->pclk))
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 955 clk_prepare_enable(pdata->pclk);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> No error handling
>
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 956
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 957 pdata->sysctl_map =
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 958 syscon_regmap_lookup_by_phandle_args(np,
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 959 "canaan,k210-sysctl-power",
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 960 1, &pdata->power_offset);
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 961 if (IS_ERR(pdata->sysctl_map))
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 962 return PTR_ERR(pdata->sysctl_map);
>
> Do we need to clk_unprepare_disable() before returning?
>
>
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 963
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 964 k210_fpioa_init_ties(pdata);
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 965
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 966 pdata->pctl = pinctrl_register(&k210_pinctrl_desc, dev, (void *)pdata);
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 967 if (IS_ERR(pdata->pctl))
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 968 return PTR_ERR(pdata->pctl);
>
> Here too.

I can add the clk_unprepare_disable() call to avoid the warning, but that is
rather pointless as the system will not boot at all if there is an error here.
Thoughts ?

>
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 969
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 @970 return 0;
> d4c34d09ab03e1 Damien Le Moal 2021-01-12 971 }
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
>
> _______________________________________________
> kbuild mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
>
>


--
Damien Le Moal
Western Digital Research

2021-08-02 12:59:24

by Dan Carpenter

[permalink] [raw]
Subject: Re: [kbuild] drivers/pinctrl/pinctrl-k210.c:970 k210_fpioa_probe() warn: 'pdata->clk' not released on lines: 962,968.

On Sun, Aug 01, 2021 at 10:50:51PM +0000, Damien Le Moal wrote:
>
> I can add the clk_unprepare_disable() call to avoid the warning, but that is
> rather pointless as the system will not boot at all if there is an error here.
> Thoughts ?

These static checker warnings just a one time email service. If you
don't want to free resources then that's fine. If people run Smatch
manually then they'll still see the warnings but the kbuild bot won't
email about it again.


regards,
dan carpenter

2021-08-02 14:26:47

by Sean Anderson

[permalink] [raw]
Subject: Re: [kbuild] drivers/pinctrl/pinctrl-k210.c:970 k210_fpioa_probe() warn: 'pdata->clk' not released on lines: 962,968.

On 8/1/21 6:50 PM, Damien Le Moal wrote:
> On 2021/07/30 22:46, Dan Carpenter wrote:
>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>> head: 764a5bc89b12b82c18ce7ca5d7c1b10dd748a440
>> commit: d4c34d09ab03e1e631fe195ddf35365a1273be9c pinctrl: Add RISC-V Canaan Kendryte K210 FPIOA driver
>> config: riscv-randconfig-m031-20210730 (attached as .config)
>> compiler: riscv64-linux-gcc (GCC) 10.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <[email protected]>
>> Reported-by: Dan Carpenter <[email protected]>
>>
>> smatch warnings:
>> drivers/pinctrl/pinctrl-k210.c:970 k210_fpioa_probe() warn: 'pdata->clk' not released on lines: 962,968.
>>
>> vim +970 drivers/pinctrl/pinctrl-k210.c
>>
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 925 static int k210_fpioa_probe(struct platform_device *pdev)
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 926 {
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 927 struct device *dev = &pdev->dev;
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 928 struct device_node *np = dev->of_node;
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 929 struct k210_fpioa_data *pdata;
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 930 int ret;
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 931
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 932 dev_info(dev, "K210 FPIOA pin controller\n");
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 933
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 934 pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 935 if (!pdata)
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 936 return -ENOMEM;
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 937
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 938 pdata->dev = dev;
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 939 platform_set_drvdata(pdev, pdata);
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 940
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 941 pdata->fpioa = devm_platform_ioremap_resource(pdev, 0);
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 942 if (IS_ERR(pdata->fpioa))
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 943 return PTR_ERR(pdata->fpioa);
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 944
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 945 pdata->clk = devm_clk_get(dev, "ref");
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 946 if (IS_ERR(pdata->clk))
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 947 return PTR_ERR(pdata->clk);
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 948
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 949 ret = clk_prepare_enable(pdata->clk);
>> ^^^^^^^^^^^^^^^^^^
>>
>>
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 950 if (ret)
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 951 return ret;
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 952
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 953 pdata->pclk = devm_clk_get_optional(dev, "pclk");
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 954 if (!IS_ERR(pdata->pclk))
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 955 clk_prepare_enable(pdata->pclk);
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> No error handling
>>
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 956
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 957 pdata->sysctl_map =
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 958 syscon_regmap_lookup_by_phandle_args(np,
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 959 "canaan,k210-sysctl-power",
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 960 1, &pdata->power_offset);
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 961 if (IS_ERR(pdata->sysctl_map))
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 962 return PTR_ERR(pdata->sysctl_map);
>>
>> Do we need to clk_unprepare_disable() before returning?
>>
>>
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 963
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 964 k210_fpioa_init_ties(pdata);
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 965
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 966 pdata->pctl = pinctrl_register(&k210_pinctrl_desc, dev, (void *)pdata);
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 967 if (IS_ERR(pdata->pctl))
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 968 return PTR_ERR(pdata->pctl);
>>
>> Here too.
>
> I can add the clk_unprepare_disable() call to avoid the warning, but that is
> rather pointless as the system will not boot at all if there is an error here.
> Thoughts ?

IMO, you should still handle the error so the user gets some warning
about not being able to enable the clock instead of crashing at some
later point.

--Sean

>
>>
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 969
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 @970 return 0;
>> d4c34d09ab03e1 Damien Le Moal 2021-01-12 971 }
>>
>> ---
>> 0-DAY CI Kernel Test Service, Intel Corporation
>> https://lists.01.org/hyperkitty/list/[email protected]
>>
>> _______________________________________________
>> kbuild mailing list -- [email protected]
>> To unsubscribe send an email to [email protected]
>>
>>
>
>