From: Jun Li <jun.li@nxp.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>,
dl-linux-imx <linux-imx@nxp.com>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Subject: RE: [bug report] usb: chipidea: imx: enable vbus and id wakeup only for OTG events
Date: Tue, 8 Oct 2019 02:55:48 +0000 [thread overview]
Message-ID: <VE1PR04MB65282B69FD17EC1583C52672899A0@VE1PR04MB6528.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20191002112954.GA1890@mwanda>
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
prev parent reply other threads:[~2019-10-08 2:55 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=VE1PR04MB65282B69FD17EC1583C52672899A0@VE1PR04MB6528.eurprd04.prod.outlook.com \
--to=jun.li@nxp.com \
--cc=dan.carpenter@oracle.com \
--cc=kernel@pengutronix.de \
--cc=linux-imx@nxp.com \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).