On Fri, Jun 19, 2020 at 04:25:12PM +0200, Mike Looijmans wrote: > +void dwc3_set_vbus(struct dwc3 *dwc, bool enable) > +{ > + int ret; > + > + if (enable != dwc->vbus_reg_enabled) { > + if (enable) > + ret = regulator_enable(dwc->vbus_reg); > + else > + ret = regulator_disable(dwc->vbus_reg); dwc->vbus_reg is set to NULL when the regulator is not present. These regulator_* functions expect a non-NULL pointer so a NULL check is required before calling them. > + if (!ret) > + dwc->vbus_reg_enabled = enable; > + } > + > + if (dwc->usb2_phy) > + otg_set_vbus(dwc->usb2_phy->otg, enable); > +} > + > static void __dwc3_set_mode(struct work_struct *work) > { > struct dwc3 *dwc = work_to_dwc(work); > @@ -164,8 +182,7 @@ static void __dwc3_set_mode(struct work_struct *work) > if (ret) { > dev_err(dwc->dev, "failed to initialize host\n"); > } else { > - if (dwc->usb2_phy) > - otg_set_vbus(dwc->usb2_phy->otg, true); > + dwc3_set_vbus(dwc, true); > phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST); > phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST); > } > @@ -173,8 +190,7 @@ static void __dwc3_set_mode(struct work_struct *work) > case DWC3_GCTL_PRTCAP_DEVICE: > dwc3_event_buffers_setup(dwc); > > - if (dwc->usb2_phy) > - otg_set_vbus(dwc->usb2_phy->otg, false); > + dwc3_set_vbus(dwc, false); > phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE); > phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE); > > @@ -1183,8 +1199,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) > case USB_DR_MODE_PERIPHERAL: > dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE); > > - if (dwc->usb2_phy) > - otg_set_vbus(dwc->usb2_phy->otg, false); > + dwc3_set_vbus(dwc, false); > phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE); > phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE); > > @@ -1198,8 +1213,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) > case USB_DR_MODE_HOST: > dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST); > > - if (dwc->usb2_phy) > - otg_set_vbus(dwc->usb2_phy->otg, true); > + dwc3_set_vbus(dwc, true); > phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST); > phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST); > > @@ -1470,6 +1484,14 @@ static int dwc3_probe(struct platform_device *pdev) > > dwc3_get_properties(dwc); > > + dwc->vbus_reg = devm_regulator_get_optional(dev, "vbus"); > + if (IS_ERR(dwc->vbus_reg)) { > + if (PTR_ERR(dwc->vbus_reg) == -EPROBE_DEFER) > + return -EPROBE_DEFER; Some drivers seem to do it this way, but I think it would be more correct to return all errors that aren't ENODEV, like drivers/gpu/drm/exynos/exynos_hdmi.c does. That way you would allow the regulator to not be present, but you also wouldn't silently ignore other errors such as ENOMEM. > + > + dwc->vbus_reg = NULL; > + } > +