Hi Neil, On Thu, Mar 26, 2020 at 2:45 PM Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote: [...] > -static int dwc3_meson_g12a_usb2_init(struct dwc3_meson_g12a *priv) > +static int dwc3_meson_g12a_usb2_init(struct dwc3_meson_g12a *priv, > + enum phy_mode mode) > { > int i, ret; > > - if (priv->otg_mode == USB_DR_MODE_PERIPHERAL) > - priv->otg_phy_mode = PHY_MODE_USB_DEVICE; > - else > - priv->otg_phy_mode = PHY_MODE_USB_HOST; > - > for (i = 0; i < priv->drvdata->num_phys; ++i) { > if (!priv->phys[i]) > continue; > @@ -284,9 +286,10 @@ static void dwc3_meson_g12a_usb3_init(struct dwc3_meson_g12a *priv) > FIELD_PREP(USB_R1_P30_PCS_TX_SWING_FULL_MASK, 127)); > } There is something strange with dwc3_meson_g12a_usb2_init. enum phy_mode mode is added here but it's not used inside this function I also think that we should not pass enum phy_mode to dwc3_meson_g12a_usb_otg_apply_mode I'm aware that the original function used enum phy_mode inside but this doesn't seem right: we're not configuring a PHY there instead we're setting up the OTG switch so I think we should use enum usb_role instead [...] not part of this patch but should be: there's a still a direct call to dwc3_meson_g12a_usb_init() in dwc3_meson_g12a_resume() I think that needs to be changed to priv->drvdata->usb_init(priv); as well