On 23/05/2021 18:03, Martin Blumenstingl wrote: > Hi Christophe, > > On Fri, May 21, 2021 at 6:55 PM Christophe JAILLET > <christophe.jaillet@xxxxxxxxxx> wrote: >> >> If an error occurs after a successful 'regulator_enable()' call, >> 'regulator_disable()' must be called. >> >> Fix the error handling path of the probe accordingly. >> >> The remove function doesn't need to be fixed, because the >> 'regulator_disable()' call is already hidden in 'dwc3_meson_g12a_suspend()' >> which is called via 'pm_runtime_set_suspended()' in the remove function. >> >> Fixes: c99993376f72 ("usb: dwc3: Add Amlogic G12A DWC3 glue") >> Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> > for me this makes sense so: > Reviewed-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> Acked-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> > >> --- >> Please review carefully. >> >> I'm not that sure about: >> The remove function doesn't need to be fixed, because the >> 'regulator_disable()' call is already hidden in 'dwc3_meson_g12a_suspend()' >> which is called via 'pm_runtime_set_suspended()' in the remove function. >> >> This is more a guess than anything else! > I am not 100% sure about the suspend code-path, so I am hoping that > someone else (for example Neil) can also review your patch Not 100% sure aswell, but the probe function is wrong anyway and must be fixed by this patch, so LGTM. Neil > > > Best regards, > Martin >