[bug report] drm/meson: fix unbind path if HDMI fails to bind
Neil Armstrong
neil.armstrong at linaro.org
Mon Jun 10 07:43:04 UTC 2024
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
More information about the dri-devel
mailing list