* [bug report] usb: chipidea: imx: enable vbus and id wakeup only for OTG events
@ 2019-10-02 11:29 Dan Carpenter
2019-10-08 2:55 ` Jun Li
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2019-10-02 11:29 UTC (permalink / raw)
To: jun.li; +Cc: Pengutronix Kernel Team, NXP Linux Team, linux-usb
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
^ permalink raw reply [flat|nested] 2+ messages in thread* RE: [bug report] usb: chipidea: imx: enable vbus and id wakeup only for OTG events
2019-10-02 11:29 [bug report] usb: chipidea: imx: enable vbus and id wakeup only for OTG events Dan Carpenter
@ 2019-10-08 2:55 ` Jun Li
0 siblings, 0 replies; 2+ messages in thread
From: Jun Li @ 2019-10-08 2:55 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Pengutronix Kernel Team, dl-linux-imx, linux-usb
Hi Dan,
> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: 2019年10月2日 19:30
> To: Jun Li <jun.li@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; dl-linux-imx
> <linux-imx@nxp.com>; linux-usb@vger.kernel.org
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-10-08 2:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 11:29 [bug report] usb: chipidea: imx: enable vbus and id wakeup only for OTG events Dan Carpenter
2019-10-08 2:55 ` Jun Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).