On 20-08-27 16:09:36, Felipe Balbi wrote: > Peter Chen <peter.chen@xxxxxxx> writes: > > > Since we have both USB2 and USB3 PHYs for cdns3 controller, it is > > better we have unity APIs to handle both USB2 and USB3's power, it > > could simplify code for error handling and further power management > > implementation. > > > > Reviewed-by: Pawel Laszczak <pawell@xxxxxxxxxxx> > > Reviewed-by: Jun Li <jun.li@xxxxxxx> > > Signed-off-by: Peter Chen <peter.chen@xxxxxxx> > > --- > > drivers/usb/cdns3/core.c | 43 ++++++++++++++++++++++++++-------------- > > 1 file changed, 28 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c > > index 5c1586ec7824..e56dbb6a898c 100644 > > --- a/drivers/usb/cdns3/core.c > > +++ b/drivers/usb/cdns3/core.c > > @@ -371,6 +371,27 @@ static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role) > > return ret; > > } > > > > +static int set_phy_power_on(struct cdns3 *cdns) > > care to keep the cdns3_ prefix? It's useful when you want to use > e.g. ftrace and function_graph trace I original added it, but Greg suggested it is not needed. https://www.spinics.net/lists/linux-usb/msg197076.html > > > +{ > > + int ret; > > + > > + ret = phy_power_on(cdns->usb2_phy); > > + if (ret) > > + return ret; > > + > > + ret = phy_power_on(cdns->usb3_phy); > > let's say we fail here > > > + if (ret) > > + phy_power_off(cdns->usb2_phy); > > usb2_phy will be powered off.. If usb3_phy's power_on has failed, it powers off usb2 phy's power which is powered on before, and return fail. > > > + return ret; > > +} > > + > > +static void set_phy_power_off(struct cdns3 *cdns) > > +{ > > + phy_power_off(cdns->usb3_phy); > > + phy_power_off(cdns->usb2_phy); > > Won't this cause a problem? > What's the problem you have seen? -- Thanks, Peter Chen
