Hi Dan,
On 12/16/21 09:26, Dan Carpenter wrote:
Hello Julien Massot,
The patch 285892a74f13: "remoteproc: Add Renesas rcar driver" from
Dec 7, 2021, leads to the following Smatch static checker warning:
drivers/remoteproc/rcar_rproc.c:171 rcar_rproc_probe()
warn: pm_runtime_get_sync() also returns 1 on success
Indeed the patch was not correct.
drivers/remoteproc/rcar_rproc.c
147 static int rcar_rproc_probe(struct platform_device *pdev)
148 {
149 struct device *dev = &pdev->dev;
150 struct device_node *np = dev->of_node;
151 struct rcar_rproc *priv;
152 struct rproc *rproc;
153 int ret;
154
155 rproc = devm_rproc_alloc(dev, np->name, &rcar_rproc_ops,
156 NULL, sizeof(*priv));
157 if (!rproc)
158 return -ENOMEM;
159
160 priv = rproc->priv;
161
162 priv->rst = devm_reset_control_get_exclusive(dev, NULL);
163 if (IS_ERR(priv->rst)) {
164 ret = PTR_ERR(priv->rst);
165 dev_err_probe(dev, ret, "fail to acquire rproc reset\n");
166 return ret;;
167 }
168
169 pm_runtime_enable(dev);
170 ret = pm_runtime_get_sync(dev);
--> 171 if (ret) {
The pm_runtime_get_sync() returns both 0 and 1 on success. The comments
to that function suggest that this should be changed to instead use:
ret = pm_runtime_resume_and_get();
if (ret) {
I've got no idea what that function does but it has standard error codes
and cleanup, so I *love* it.
Indeed checking for non 0 error code is not enough.
After looking at the pm_runtime_resume_and_get error handling, I also realize
that this driver never call pm_runtime_put, so pm_runtime_get/put is unbalanced.
I plan to address both issues in a single patch.
172 dev_err(dev, "failed to power up\n");
173 return ret;
174 }
175
176 dev_set_drvdata(dev, rproc);
177
178 /* Manually start the rproc */
179 rproc->auto_boot = false;
180
181 ret = devm_rproc_add(dev, rproc);
182 if (ret) {
183 dev_err(dev, "rproc_add failed\n");
184 goto pm_disable;
185 }
186
187 return 0;
188
189 pm_disable:
190 pm_runtime_disable(dev);
missing
pm_runtime_put(dev);
(it also applies to the remove function)
191
192 return ret;
193 }
regards,
dan carpenter
Will try to address both issues today.
Regards,
--
Julien Massot [IoT.bzh]
