usb: chipidea: imx: enable vbus and id wakeup only for OTG events
Hello Li Jun,
The patch 15b80f7c3a7f: "usb: chipidea: imx: enable vbus and id wakeup only for OTG events" from Sep 9, 2019, leads to the following static checker warning:
drivers/usb/chipidea/ci_hdrc_imx.c:438 ci_hdrc_imx_probe() warn: 'data->usbmisc_data' can also be NULL
drivers/usb/chipidea/ci_hdrc_imx.c
317 data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
318 if (!data)
319 return -ENOMEM;
320
321 data->plat_data = imx_platform_flag;
322 pdata.flags |= imx_platform_flag->flags;
323 platform_set_drvdata(pdev, data);
324 data->usbmisc_data = usbmisc_get_init_data(dev);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^This can return NULL or error pointer.
325 if (IS_ERR(data->usbmisc_data))
326 return PTR_ERR(data->usbmisc_data);
327
328 if ((of_usb_get_phy_mode(dev->of_node) == USBPHY_INTERFACE_MODE_HSIC)
329 && data->usbmisc_data) {
^^^^^^^^^^^^^^^^^^This code checks for NULL.
330 pdata.flags |= CI_HDRC_IMX_IS_HSIC;
331 data->usbmisc_data->hsic = 1;
332 data->pinctrl = devm_pinctrl_get(dev);
333 if (IS_ERR(data->pinctrl)) {
334 dev_err(dev, "pinctrl get failed, err=%ld\n",
335 PTR_ERR(data->pinctrl));
336 return PTR_ERR(data->pinctrl);
337 }
338
339 pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
340 if (IS_ERR(pinctrl_hsic_idle)) {
341 dev_err(dev,
342 "pinctrl_hsic_idle lookup failed, err=%ld\n",
343 PTR_ERR(pinctrl_hsic_idle));
344 return PTR_ERR(pinctrl_hsic_idle);
345 }
346
347 ret = pinctrl_select_state(data->pinctrl, pinctrl_hsic_idle);
348 if (ret) {
349 dev_err(dev, "hsic_idle select failed, err=%d\n", ret);
350 return ret;
351 }
352
353 data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl,
354 "active");
355 if (IS_ERR(data->pinctrl_hsic_active)) {
356 dev_err(dev,
357 "pinctrl_hsic_active lookup failed, err=%ld\n",
358 PTR_ERR(data->pinctrl_hsic_active));
359 return PTR_ERR(data->pinctrl_hsic_active);
360 }
361
362 data->hsic_pad_regulator = devm_regulator_get(dev, "hsic");
363 if (PTR_ERR(data->hsic_pad_regulator) == -EPROBE_DEFER) {
364 return -EPROBE_DEFER;
365 } else if (PTR_ERR(data->hsic_pad_regulator) == -ENODEV) {
366 /* no pad regualator is needed */
367 data->hsic_pad_regulator = NULL;
368 } else if (IS_ERR(data->hsic_pad_regulator)) {
369 dev_err(dev, "Get HSIC pad regulator error: %ld\n",
370 PTR_ERR(data->hsic_pad_regulator));
371 return PTR_ERR(data->hsic_pad_regulator);
372 }
373
374 if (data->hsic_pad_regulator) {
375 ret = regulator_enable(data->hsic_pad_regulator);
376 if (ret) {
377 dev_err(dev,
378 "Failed to enable HSIC pad regulator\n");
379 return ret;
380 }
381 }
382 }
383
384 if (pdata.flags & CI_HDRC_PMQOS)
385 pm_qos_add_request(&data->pm_qos_req,
386 PM_QOS_CPU_DMA_LATENCY, 0);
387
388 ret = imx_get_clks(dev);
389 if (ret)
390 goto disable_hsic_regulator;
391
392 ret = imx_prepare_enable_clks(dev);
393 if (ret)
394 goto disable_hsic_regulator;
395
396 data->phy = devm_usb_get_phy_by_phandle(dev, "fsl,usbphy", 0);
397 if (IS_ERR(data->phy)) {
398 ret = PTR_ERR(data->phy);
399 /* Return -EINVAL if no usbphy is available */
400 if (ret == -ENODEV)
401 data->phy = NULL;
402 else
403 goto err_clk;
404 }
405
406 pdata.usb_phy = data->phy;
407
408 if ((of_device_is_compatible(np, "fsl,imx53-usb") ||
409 of_device_is_compatible(np, "fsl,imx51-usb")) && pdata.usb_phy &&
410 of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
411 pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
412 data->override_phy_control = true;
413 usb_phy_init(pdata.usb_phy);
414 }
415
416 if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
417 data->supports_runtime_pm = true;
418
419 ret = imx_usbmisc_init(data->usbmisc_data);
420 if (ret) {
421 dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
422 goto err_clk;
423 }
424
425 data->ci_pdev = ci_hdrc_add_device(dev,
426 pdev->resource, pdev->num_resources,
427 &pdata);
428 if (IS_ERR(data->ci_pdev)) {
429 ret = PTR_ERR(data->ci_pdev);
430 if (ret != -EPROBE_DEFER)
431 dev_err(dev, "ci_hdrc_add_device failed, err=%d\n",
432 ret);
433 goto err_clk;
434 }
435
436 if (!IS_ERR(pdata.id_extcon.edev) ||
437 of_property_read_bool(np, "usb-role-switch"))
438 data->usbmisc_data->ext_id = 1;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 439
440 if (!IS_ERR(pdata.vbus_extcon.edev) ||
441 of_property_read_bool(np, "usb-role-switch"))
442 data->usbmisc_data->ext_vbus = 1;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^These should probably check for NULL? This driver seems to check pretty consistently.
443 444 ret = imx_usbmisc_init_post(data->usbmisc_data);
regards,
dan carpenter
Hi Dan,
Quoted 8 lines
-----Original Message-----
From: Dan Carpenter <[email protected]>
Sent: 2019年10月2日 19:30
To: Jun Li <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>; dl-linux-imx <[email protected]>; [email protected]
Subject: [bug report] usb: chipidea: imx: enable vbus and id wakeup only for OTG events
Quoted 7 lines
Hello Li Jun,
The patch
15b80f7c3a7f: "usb: chipidea: imx: enable vbus and id wakeup only for OTG events" from Sep 9, 2019, leads to the following static checker warning:drivers/usb/chipidea/ci_hdrc_imx.c:438 ci_hdrc_imx_probe() warn: 'data->usbmisc_data' can also be NULL
Thanks for the reporting.
Quoted 144 lines
drivers/usb/chipidea/ci_hdrc_imx.c317 data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); 318 if (!data) 319 return -ENOMEM; 320 321 data->plat_data = imx_platform_flag; 322 pdata.flags |= imx_platform_flag->flags; 323 platform_set_drvdata(pdev, data); 324 data->usbmisc_data = usbmisc_get_init_data(dev); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^This can return NULL or error pointer.
325 if (IS_ERR(data->usbmisc_data)) 326 return PTR_ERR(data->usbmisc_data); 327 328 if ((of_usb_get_phy_mode(dev->of_node) == USBPHY_INTERFACE_MODE_HSIC) 329 && data->usbmisc_data) { ^^^^^^^^^^^^^^^^^^ This code checks for NULL.330 pdata.flags |= CI_HDRC_IMX_IS_HSIC; 331 data->usbmisc_data->hsic = 1; 332 data->pinctrl = devm_pinctrl_get(dev); 333 if (IS_ERR(data->pinctrl)) { 334 dev_err(dev, "pinctrl get failed, err=%ld\n", 335 PTR_ERR(data->pinctrl)); 336 return PTR_ERR(data->pinctrl); 337 } 338 339 pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle"); 340 if (IS_ERR(pinctrl_hsic_idle)) { 341 dev_err(dev, 342 "pinctrl_hsic_idle lookup failed, err=%ld\n", 343 PTR_ERR(pinctrl_hsic_idle)); 344 return PTR_ERR(pinctrl_hsic_idle); 345 } 346 347 ret = pinctrl_select_state(data->pinctrl, pinctrl_hsic_idle); 348 if (ret) { 349 dev_err(dev, "hsic_idle select failed, err=%d\n", ret); 350 return ret; 351 } 352 353 data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl, 354 "active"); 355 if (IS_ERR(data->pinctrl_hsic_active)) { 356 dev_err(dev, 357 "pinctrl_hsic_active lookup failed, err=%ld\n", 358 PTR_ERR(data->pinctrl_hsic_active)); 359 return PTR_ERR(data->pinctrl_hsic_active); 360 } 361 362 data->hsic_pad_regulator = devm_regulator_get(dev, "hsic"); 363 if (PTR_ERR(data->hsic_pad_regulator) == -EPROBE_DEFER) { 364 return -EPROBE_DEFER; 365 } else if (PTR_ERR(data->hsic_pad_regulator) == -ENODEV) { 366 /* no pad regualator is needed */ 367 data->hsic_pad_regulator = NULL; 368 } else if (IS_ERR(data->hsic_pad_regulator)) { 369 dev_err(dev, "Get HSIC pad regulator error: %ld\n", 370 PTR_ERR(data->hsic_pad_regulator)); 371 return PTR_ERR(data->hsic_pad_regulator); 372 } 373 374 if (data->hsic_pad_regulator) { 375 ret = regulator_enable(data->hsic_pad_regulator); 376 if (ret) { 377 dev_err(dev, 378 "Failed to enable HSIC pad regulator\n"); 379 return ret; 380 } 381 } 382 } 383 384 if (pdata.flags & CI_HDRC_PMQOS) 385 pm_qos_add_request(&data->pm_qos_req, 386 PM_QOS_CPU_DMA_LATENCY, 0); 387 388 ret = imx_get_clks(dev); 389 if (ret) 390 goto disable_hsic_regulator; 391 392 ret = imx_prepare_enable_clks(dev); 393 if (ret) 394 goto disable_hsic_regulator; 395 396 data->phy = devm_usb_get_phy_by_phandle(dev, "fsl,usbphy", 0); 397 if (IS_ERR(data->phy)) { 398 ret = PTR_ERR(data->phy); 399 /* Return -EINVAL if no usbphy is available */ 400 if (ret == -ENODEV) 401 data->phy = NULL; 402 else 403 goto err_clk; 404 } 405 406 pdata.usb_phy = data->phy; 407 408 if ((of_device_is_compatible(np, "fsl,imx53-usb") || 409 of_device_is_compatible(np, "fsl,imx51-usb")) && pdata.usb_phy && 410 of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) { 411 pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL; 412 data->override_phy_control = true; 413 usb_phy_init(pdata.usb_phy); 414 } 415 416 if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM) 417 data->supports_runtime_pm = true; 418 419 ret = imx_usbmisc_init(data->usbmisc_data); 420 if (ret) { 421 dev_err(dev, "usbmisc init failed, ret=%d\n", ret); 422 goto err_clk; 423 } 424 425 data->ci_pdev = ci_hdrc_add_device(dev, 426 pdev->resource, pdev->num_resources, 427 &pdata); 428 if (IS_ERR(data->ci_pdev)) { 429 ret = PTR_ERR(data->ci_pdev); 430 if (ret != -EPROBE_DEFER) 431 dev_err(dev, "ci_hdrc_add_device failed, err=%d\n", 432 ret); 433 goto err_clk; 434 } 435 436 if (!IS_ERR(pdata.id_extcon.edev) || 437 of_property_read_bool(np, "usb-role-switch")) 438 data->usbmisc_data->ext_id = 1; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 439 440 if (!IS_ERR(pdata.vbus_extcon.edev) || 441 of_property_read_bool(np, "usb-role-switch")) 442 data->usbmisc_data->ext_vbus = 1; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ These should probably
Quoted 1 line
check for NULL? This driver seems to check pretty consistently.
Yes, I will submit a patch to fix this.
Li Jun
Quoted 6 lines
443 444 ret = imx_usbmisc_init_post(data->usbmisc_data);regards,
dan carpenter