RE: [bug report] usb: chipidea: imx: enable vbus and id wakeup only for OTG events

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux