Hi,
On 08/06/2024 16:20, Dan Carpenter wrote:
Hello Neil Armstrong,
Commit 6a044642988b ("drm/meson: fix unbind path if HDMI fails to
bind") from May 30, 2023 (linux-next), leads to the following Smatch
static checker warning:
drivers/gpu/drm/meson/meson_drv.c:377 meson_drv_bind_master()
error: we previously assumed 'priv' could be null (see line 205)
Thanks, I'll look into this.
Neil
drivers/gpu/drm/meson/meson_drv.c
180 static int meson_drv_bind_master(struct device *dev, bool has_components)
181 {
182 struct platform_device *pdev = to_platform_device(dev);
183 const struct meson_drm_match_data *match;
184 struct meson_drm *priv;
185 struct drm_device *drm;
186 struct resource *res;
187 void __iomem *regs;
188 int ret, i;
189
190 /* Checks if an output connector is available */
191 if (!meson_vpu_has_available_connectors(dev)) {
192 dev_err(dev, "No output connector available\n");
193 return -ENODEV;
194 }
195
196 match = of_device_get_match_data(dev);
197 if (!match)
198 return -ENODEV;
199
200 drm = drm_dev_alloc(&meson_driver, dev);
201 if (IS_ERR(drm))
202 return PTR_ERR(drm);
203
204 priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
205 if (!priv) {
206 ret = -ENOMEM;
207 goto free_drm;
priv is NULL so the calls to meson_encoder_*_remove() will Oops.
208 }
209 drm->dev_private = priv;
210 priv->drm = drm;
211 priv->dev = dev;
212 priv->compat = match->compat;
213 priv->afbcd.ops = match->afbcd_ops;
214
215 regs = devm_platform_ioremap_resource_byname(pdev, "vpu");
216 if (IS_ERR(regs)) {
217 ret = PTR_ERR(regs);
218 goto free_drm;
219 }
220
221 priv->io_base = regs;
222
223 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hhi");
224 if (!res) {
225 ret = -EINVAL;
226 goto free_drm;
227 }
228 /* Simply ioremap since it may be a shared register zone */
229 regs = devm_ioremap(dev, res->start, resource_size(res));
230 if (!regs) {
231 ret = -EADDRNOTAVAIL;
232 goto free_drm;
233 }
234
235 priv->hhi = devm_regmap_init_mmio(dev, regs,
236 &meson_regmap_config);
237 if (IS_ERR(priv->hhi)) {
238 dev_err(&pdev->dev, "Couldn't create the HHI regmap\n");
239 ret = PTR_ERR(priv->hhi);
240 goto free_drm;
241 }
242
243 priv->canvas = meson_canvas_get(dev);
244 if (IS_ERR(priv->canvas)) {
245 ret = PTR_ERR(priv->canvas);
246 goto free_drm;
247 }
248
249 ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_osd1);
250 if (ret)
251 goto free_drm;
252 ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_0);
253 if (ret) {
254 meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
255 goto free_drm;
256 }
257 ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_1);
258 if (ret) {
259 meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
260 meson_canvas_free(priv->canvas, priv->canvas_id_vd1_0);
261 goto free_drm;
262 }
263 ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_2);
264 if (ret) {
265 meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
266 meson_canvas_free(priv->canvas, priv->canvas_id_vd1_0);
267 meson_canvas_free(priv->canvas, priv->canvas_id_vd1_1);
What about this clean up?
268 goto free_drm;
269 }
270
271 priv->vsync_irq = platform_get_irq(pdev, 0);
272
273 ret = drm_vblank_init(drm, 1);
274 if (ret)
275 goto free_drm;
Do we not need to do it here as well?
276
277 /* Assign limits per soc revision/package */
278 for (i = 0 ; i < ARRAY_SIZE(meson_drm_soc_attrs) ; ++i) {
279 if (soc_device_match(meson_drm_soc_attrs[i].attrs)) {
280 priv->limits = &meson_drm_soc_attrs[i].limits;
281 break;
282 }
283 }
284
285 /*
286 * Remove early framebuffers (ie. simplefb). The framebuffer can be
287 * located anywhere in RAM
288 */
289 ret = drm_aperture_remove_framebuffers(&meson_driver);
290 if (ret)
291 goto free_drm;
292
293 ret = drmm_mode_config_init(drm);
294 if (ret)
295 goto free_drm;
296 drm->mode_config.max_width = 3840;
297 drm->mode_config.max_height = 2160;
298 drm->mode_config.funcs = &meson_mode_config_funcs;
299 drm->mode_config.helper_private = &meson_mode_config_helpers;
300
301 /* Hardware Initialization */
302
303 meson_vpu_init(priv);
304 meson_venc_init(priv);
305 meson_vpp_init(priv);
306 meson_viu_init(priv);
307 if (priv->afbcd.ops) {
308 ret = priv->afbcd.ops->init(priv);
309 if (ret)
310 goto free_drm;
311 }
312
313 /* Encoder Initialization */
314
315 ret = meson_encoder_cvbs_probe(priv);
316 if (ret)
317 goto exit_afbcd;
318
319 if (has_components) {
320 ret = component_bind_all(dev, drm);
321 if (ret) {
322 dev_err(drm->dev, "Couldn't bind all components\n");
323 /* Do not try to unbind */
324 has_components = false;
325 goto exit_afbcd;
326 }
327 }
328
329 ret = meson_encoder_hdmi_probe(priv);
330 if (ret)
331 goto exit_afbcd;
332
333 if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
334 ret = meson_encoder_dsi_probe(priv);
335 if (ret)
336 goto exit_afbcd;
337 }
338
339 ret = meson_plane_create(priv);
340 if (ret)
341 goto exit_afbcd;
342
343 ret = meson_overlay_create(priv);
344 if (ret)
345 goto exit_afbcd;
346
347 ret = meson_crtc_create(priv);
348 if (ret)
349 goto exit_afbcd;
350
351 ret = request_irq(priv->vsync_irq, meson_irq, 0, drm->driver->name, drm);
352 if (ret)
353 goto exit_afbcd;
354
355 drm_mode_config_reset(drm);
356
357 drm_kms_helper_poll_init(drm);
358
359 platform_set_drvdata(pdev, priv);
360
361 ret = drm_dev_register(drm, 0);
362 if (ret)
363 goto uninstall_irq;
364
365 drm_fbdev_dma_setup(drm, 32);
366
367 return 0;
368
369 uninstall_irq:
370 free_irq(priv->vsync_irq, drm);
371 exit_afbcd:
372 if (priv->afbcd.ops)
373 priv->afbcd.ops->exit(priv);
374 free_drm:
375 drm_dev_put(drm);
376
--> 377 meson_encoder_dsi_remove(priv);
378 meson_encoder_hdmi_remove(priv);
379 meson_encoder_cvbs_remove(priv);
380
381 if (has_components)
382 component_unbind_all(dev, drm);
383
384 return ret;
385 }
regards,
dan carpenter
