Hi Dan,
> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Sent: 2019年10月2日 19:30
> To: Jun Li <jun.li@xxxxxxx>
> Cc: Pengutronix Kernel Team <kernel@xxxxxxxxxxxxxx>; dl-linux-imx
> <linux-imx@xxxxxxx>; linux-usb@xxxxxxxxxxxxxxx
> Subject: [bug report] 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
Thanks for the reporting.
>
> 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.
Yes, I will submit a patch to fix this.
Li Jun
>
> 443
> 444 ret = imx_usbmisc_init_post(data->usbmisc_data);
>
> regards,
> dan carpenter
