Hi Hanjie, On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin@xxxxxxxxxxx> wrote: [...] > - devm_add_action_or_reset(dev, > - (void(*)(void *))clk_disable_unprepare, > - priv->clk); > + ret = clk_bulk_prepare_enable(priv->drvdata->num_clks, > + priv->drvdata->clks); I don't see clk_bulk_disable_unprepare in dwc3_meson_g12a_remove() please test that the clocks are all disabled (see /sys/kernel/debug/clk/clk_summary for example) when unloading all USB related kernel modules > + > + if (!priv->drvdata->otg_switch_supported) > + goto setup_pm_runtime; my brain doesn't like the goto in this place because this is not an error condition. I was about to write that usb_role_switch_unregister() is now skipped even though we're calling usb_role_switch_register(). I want to hear Neil's opinion on this because I got confused while reading the code again. my proposal is to move all of this OTG related code from dwc3_meson_g12a_probe() into a new function, for example called dwc3_meson_g12a_otg_init() then only call that function when OTG switching is supported Martin
